[Capability] Harden structuredContent extraction - #417
Open
chr-hertel wants to merge 3 commits into
Open
Conversation
chr-hertel
requested review from
CodeWithKyrian,
Nyholm and
soyuka
as code owners
August 14, 2026 21:17
Contributor
|
Did you want me to do a merge /rebase to resolve conflicts? |
Member
Author
|
@cancan101 no, all good - just a follow up. your commits will be dropped after merging #400 in a sec |
Three follow-ups from review of the resource_link work: * PromptResultFormatter: add a regression test proving the optional fields of a typed resource_link block (title, description, mimeType, size, annotations, _meta) survive formatting. The delegation to ResourceLink::fromArray() already landed while rebasing onto the PromptResultFormatter refactor, but the existing test only supplied uri/name and so could not catch a reintroduced field drop. * ResourceLink::fromArray(): validate optional fields consistently, the way the equivalent ResourceDefinition::fromArray() already does. description, mimeType and size now raise InvalidArgumentException instead of surfacing a TypeError (or silently coercing, for size's (int) cast); annotations goes through Annotations::tryFromArray(), which carries the is_array() guard that icons already had; icons uses Icon::listFromArray() so a non-array entry is reported in context. * ToolReference::extractStructuredContent(): never emit a list as structuredContent. The Content guard added earlier fixed only one instance of the real invariant - structuredContent must be a JSON object, and a PHP list can never be one. Tool::fromArray() already enforces the matching rule by rejecting an outputSchema whose type is not "object", so this aligns the runtime path with it. The test asserting the opposite for an array-typed outputSchema contradicted the phpstan type and the fromArray() check introduced alongside it in the same commit, so it is replaced by tests for the two list shapes.
chr-hertel
force-pushed
the
feat/resource-link-content-review
branch
from
August 14, 2026 21:25
64b3a3d to
fa158fc
Compare
`extractStructuredContent()` guards raw array results against being emitted as a JSON array, but the object branch handed back whatever `json_decode()` produced. A `JsonSerializable` returning a list or a scalar slipped straight through, producing exactly the `structuredContent` the array guard exists to prevent — and a return value that contradicts the method's own `array<string, mixed>|null` signature. Check the decoded value before returning it, and cover the object branch, which had no tests at all.
`outputSchema` and `structuredContent` were undocumented: the tool return value docs covered only the `content` side, and the schema generation section is about tool parameters. Add a "Structured Output" subsection covering how to declare the schema, which return values populate `structuredContent`, and why a list has to be wrapped in a key to get structured output at all.
chr-hertel
changed the title
structuredContent extraction, tighten ResourceLink validation
chr-hertel
changed the title
structuredContent extraction, tighten ResourceLink validationstructuredContent extraction
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #400.
structuredContentmust be a JSON object.extractStructuredContent()returned PHP lists as-is, and the object branch returned whateverJsonSerializableproduced — both contradict the method'sarray<string, mixed>|nullsignature and get the tool call rejected by strict clients.Tool::fromArray()already requires anoutputSchemaof typeobject; this aligns the runtime path with it.ResourceLink::fromArray()validated optional fields inconsistently:description/mimeTypereached the constructor as aTypeError,sizewas silently(int)-cast,annotationslacked theis_array()guardiconshad. Now mirrorsResourceDefinition::fromArray().resource_linkblock keeps its optional fields throughPromptResultFormatter.outputSchema/structuredContent, previously undocumented.Drops
testExtractStructuredContentReturnsArrayDirectlyForArrayOutputSchema: it contradicted c0938a1, which pinnedToolOutputSchematotype: 'object'and added the matchingfromArray()check in the same commit.BC: tools returning a list no longer advertise
structuredContent; the value stays incontent.🤖 Generated with Claude Code