fix(schema): keep annotations across encodings - #7203
Open
spencerbeggs wants to merge 5 commits into
Open
Conversation
- Carry documentation annotations from the type side of an encoding chain onto the representation, so a schema that encodes to a different shape keeps its title, description and examples - Let annotations closer to the encoded side win, since a link that rewrites the data may already have folded the type side description into its own Signed-off-by: C. Spencer Beggs <spencer@beggs.codes>
🦋 Changeset detectedLatest commit: a464e39 The changes in this PR will be included in the next version bump. This PR includes changesets to release 30 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
spencerbeggs
changed the title
- Assert the carry at the representation layer: documentation keys travel, expected and the other behavioral keys stay behind, and the encoded side wins a collision - Cover the paths the JSON Schema tests missed: a referenced encoded definition, a two-link encoding chain, and default and examples values that are not valid JSON Signed-off-by: C. Spencer Beggs <spencer@beggs.codes>
- Type the encoding-chain test against the real signatures: numeric examples for NumberFromString, and passthroughSubtype for the Date to unknown link - Update the HttpApi OpenAPI snapshot, where the UserEncoded component schema now carries the description its Schema.Class declares Signed-off-by: C. Spencer Beggs <spencer@beggs.codes>
Contributor
Bundle Size AnalysisGenerated from PR build output; treat the content below as untrusted.
|
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
1 participant
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.
Fixes #7192.
What changed
Schema.toJsonSchemaDocumentnow keepstitle,description,examplesand the other JSON Schema annotations on schemas whose encoded form differs from their type.Schema.Numberwas the reported case, and it now behaves likeSchema.StringandSchema.NullOrdo:Root cause
The annotation was not lost in the JSON Schema compiler — it never reached it.
internal/schema/toRepresentation.tsbuilds every representation fromSchemaAST.getLastEncoding(input), which walks to the final link of the encoding chain, and then reads annotations off that node alone.Schema.Number.toCodecJson()callsreplaceEncoding(this, [numberToJson(this.checks)]), so the annotatedNumbernode is the type side and the encoded side is a freshUnion([finite, nonFiniteLiterals])carrying no annotations. Everything on the type side was dropped before the JSON Schema pass ran.That also explains the observations in the issue.
StringandNullOrhave no encoding, soinput === encodedand nothing is lost.FiniteandIntshort-circuittoCodecJson()back tothis, so they keep the annotation — theallOfnesting there is the separate, pre-existing behavior of.annotate()targeting the last check, which this PR does not change. And the 4-branchanyOfintoJsonSchemaDocument.tsis unrelated: the 2-branch shape the caller sees comes from the encodedUnion, not from that code path.Scope
Schema.Numberwas not the only casualty — the defect applies to every schema with an encoding link.Schema.BigInt,Schema.Date,Schema.URL,Schema.Option,Schema.ReadonlyMap,Schema.Unknown,Schema.Void,Schema.Undefined,Schema.ObjectKeywordandbigintliterals all dropped their annotations the same way, and six existing tests asserted that behavior. They are updated here, and the fix covers all of them rather than special-casingNumber.The fix
toRepresentationnow carries the type-side annotations forward across the encoding chain. Two deliberate constraints:jsonSchemaAnnotationKeystravel.representation,expected,identifier, theto*hooks and anything else stay bound to the node that declared them. Carryingexpectedin particular would be wrong — it describes the decoded value, soSchema.Option(...)would start emittingdescription: "Option"for its encoded union undergenerateDescriptions: true.unstable/ai/internal/structured-output.tscomposes the type-side description into its own ("Tuple encoded as an object with numeric string keys …; description"). My first attempt let the type side win and broke fourAnthropicStructuredOutput/OpenAiStructuredOutputtests, which is the evidence that a link rewriting the shape of the data gets to describe the result. This also matches the existing precedent inresolveReferenceIdentifier, which prefers the encoded identifier and falls back to the input's.Tests
The fix is covered at both layers, and every assertion was checked by mutation rather than by inspection. Reverting the fix fails 17 tests; each of the following single-line mutations was also confirmed to fail at least one test:
prefers an encoded-side documentation annotation over a type-side one, plus 4 structured-output testsjsonSchemaAnnotationKeyscollects documentation annotations from every link of an encoding chaincarries documentation annotations onto a referenced encoded representationtest/schema/representation/toRepresentation.test.tsgets the unit-level assertions (carry, key filtering, precedence, the reference path, a two-linkUnknown > Declaration > Stringchain).test/schema/toJsonSchemaDocument.test.tsgets the user-visible ones, including thatSchema.Date.annotate({ default: new Date(0) })still drops the non-JSON value rather than emitting it.Known remaining gap
.annotate()lands on the last check when a schema has one, and checks do not travel to the encoded side of a transformation. So this case is still annotation-free and is not fixed here:I left it out deliberately. Reading through
InternalAnnotations.resolveinstead ofast.annotationswould pick it up, but it would also double-emit forSchema.Number.check(...).annotate(...), where the checks do travel vianumberToJson(this.checks)and the annotation already surfaces inside the finite branch'sallOf. Untangling that is the separate question of what a check annotation means on an encoded shape, which the existingallOfnesting onSchema.Finite.annotate(...)already raises. Happy to take it on in this PR or a follow-up, whichever you prefer.Verification
pnpm test run(full workspace) — 9463 passed. The one remaining failure,Prompt.date > renders two-digit years, teen ordinals, and noon meridiem correctly, is locale-dependent, fails identically on a cleanmain, and passes in CI.pnpm check— clean.deno check .— clean.pnpm test-types --target '>=5.9'— 2072 tests, 5102 assertions, clean.pnpm lintanddprint checkreport nothing on the changed files.One snapshot moved, and it is worth a look:
HttpApi's OpenAPI fixture now emits"description": "Some description for User"on theUserEncodedcomponent schema.Useris aSchema.Classdeclaring that description, and the encoded component was dropping it — while the OpenAPI response objects already carried it through a different path. The two agree now.Note for reviewers
The precedence rule is the one judgement call in this PR. It is now pinned directly by
prefers an encoded-side documentation annotation over a type-side oneas well as by the structured-output tests. If you would rather the type side win, that is a one-line flip inwithCarriedAnnotations— but it would need the structured-output description composition reworked to match.