feat(ir)!: carry both exclusive bounds; give contentSchema a home - #440
Open
fuad-daoud wants to merge 3 commits into
Open
feat(ir)!: carry both exclusive bounds; give contentSchema a home#440fuad-daoud wants to merge 3 commits into
fuad-daoud wants to merge 3 commits into
Conversation
The 2020-12 content vocabulary was two-thirds modelled: contentEncoding reached Encoding.Name and contentMediaType Encoding.MediaType, while contentSchema had no field at any IR position and was always kept verbatim under Unmodeled. A consumer saw an opaque string where the source declared a full shape, and had to special-case one of three keywords (GitHub #426). ir.Encoding gains Schema *TypeRef. contentSchema's value is a schema, so it lowers like every other sub-schema position: hoisted at its own source pointer and referenced by ID, never carried beside the encoding as a raw payload a consumer would have to re-parse. The pointer it hoists at is the one the source wrote it at, which only that declaration can name, so the minted node needs no namespace of its own. The three keywords now share one home, so a position keeps them all or lowers them all: contentSchema joins contentKeywords, and the schema package decides its fate by asking the node that was built rather than the keyword that was written. That is why annotation.noIRHomeAt goes — whether a content keyword reached ir.Encoding is a question only the lowering can answer, and it was answering "never" from outside. Adding the scalar hoisters to the schema walk's recursion is what lets a contentSchema nest; the walk's depth counter already bounds it, and internal/archtest pins the widened cycle. BREAKING CHANGE: contentSchema no longer appears as an openapi:contentSchema Unmodeled entry at a position that lowers to a Scalar; it is Encoding.Schema there. A position with no Encoding field still keeps it verbatim, now alongside its two neighbours. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SYeBgDsskwnyitLgPGCPn1
At a $ref use site the compiler merges the referent's documentation, deprecation and default onto the referencing Property/Parameter, and leaves its constraints where they were declared. The split was deliberate and tested but written down nowhere a consumer reads, so an empty Constraints at a use site could be read as "this value is unbounded" when it means "this position declared no bound". The split is kept, because the two halves are not the same kind of fact. An annotation is a single value one position may restate for another, so use-site precedence is the only sensible rule and applying it once in the compiler keeps every carrier alike. A bound is not: maxLength 64 on the referent and maxLength 100 beside the $ref are both in force and the narrower wins, so merging under use-site precedence would publish 100 as the whole truth and lose the bound the document enforces. What changes is that the rule is now stated where it is read: a new ir-design §12.2, the Constraints, Property.Constraints, Parameter.Constraints and TypeRef field docs, and the two lowering sites that implement it. An absent Constraints at a use site means that position declared no bound; the effective bound is its conjunction with every node reached from its TypeRef. param-ref-inheritance now declares a bound beside the $ref at both carriers, so the split is witnessed rather than merely absent: the use site's 100 lands on the carrier, the referent's 64 stays on the referent, and the case reddens if either is copied onto the other. Fixes #428 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SYeBgDsskwnyitLgPGCPn1
In JSON Schema 2020-12 minimum and exclusiveMinimum are independent keywords that both apply; a schema may legally declare both, and the same holds on the upper side. ir.Constraints held one bound plus one exclusivity flag per side, so a co-declared pair had to be reconciled: the tighter keyword took the slot and the other was kept verbatim under Unmodeled as degraded_lowering. The loser was preserved, so nothing was lost outright. But a consumer comparing constraints across two revisions of a spec reads the fields, not the Unmodeled map: a revision that moved only the dropped keyword read as no change, and one that swapped which keyword was tighter read as a change of a different kind than the one that happened (#425). ir.Constraints now holds four bounds — Min, ExclusiveMin, Max, ExclusiveMax, each a *BigVal, each the keyword of the same name. A co-declared pair reaches two fields, keeps nothing beside them, and reports nothing: there is no degradation left to announce. The reconciliation, its exact-decimal tighter-of-two comparison, and its diagnostics are gone; merge adopts and compares each of the four the way it already did multipleOf. BREAKING CHANGE: ExclusiveMin and ExclusiveMax change from bool to *BigVal and their JSON keys gain omitempty, so `"exclusiveMin": false` no longer appears and an exclusive bound serializes as its literal rather than as a flag on `min`. The OpenAPI 3.0 spelling — a boolean modifying the minimum beside it — now lowers to the bound it means: `{minimum: 5, exclusiveMinimum: true}` becomes ExclusiveMin "5" with Min unset, which is what the 3.1 spelling of the same restriction produces, so a 3.0 document and its 3.1 translation no longer differ in the IR. A 3.0 modifier written with no bound to modify (invalid under draft-4, and unchecked by the loader) is kept verbatim under Unmodeled and reported as a warning rather than setting a flag over an absent bound. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SYeBgDsskwnyitLgPGCPn1
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.
Closes #426, #425 and #428. Breaking — one JSON type change, the only one in the stack that fails a decode rather than degrading it.
Constraints.ExclusiveMin/ExclusiveMax:bool→ decimal string (#425)In 2020-12,
minimumandexclusiveMinimumare independent keywords that both apply.Constraintsheld one bound per side, so the two were reconciled — tighter kept, other preserved asdegraded_lowering. A consumer comparing constraints across versions saw only the survivor: a change to the dropped keyword read as no change.Both bounds are now carried per side.
Worth a close read: this also changes the 3.0 boolean dialect's semantics (
{minimum: 5, exclusiveMinimum: true}→ExclusiveMin "5",Minunset). Bothtestdata/fixtures usingexclusiveMinimumare 3.1, so no golden, noirverify, no order-invariance and no round-trip oracle reaches that arm. Unit tests do bind it — removing*incl = nilreddens two — but it is tested rather than oracled, and a reviewer flagged that as the thinnest coverage in the stack.Encoding.Schema(#426)contentSchemahad no home at any position whilecontentEncodingandcontentMediaTypereachedEncoding.ir-design.mdalready named it "a gap expected to close". It lowers to aTypeRefinto the registry, like any other schema — not a raw blob.$refconstraint inheritance (#428) — answered with a doc, not a fixThe issue asked why a
$ref'd schema's constraints do not reach the use site while docs, deprecation and defaults do. The conclusion was that the split is deliberate, and it is now stated in the field docs so a consumer knows to read the target.That is the answer that required no work, and it deserves a second opinion. If you would rather constraints inherit, this is the commit to send back.
BREAKING CHANGE:
exclusiveMin/exclusiveMaxchange JSON type from boolean to a decimal string. A consumer decoding them as booleans fails rather than degrades.Stack 6 of 8. Base
stack/5-promotion— review and merge bottom-up. Every commit here passedmake gatewhen it landed, and the full gate was re-run on the top of the stack. Run it asGOTOOLCHAIN=go1.26.3 make gate; this machine's Go 1.27 fails it for reasons unrelated to any change (#431).🤖 Generated with Claude Code
https://claude.ai/code/session_01SYeBgDsskwnyitLgPGCPn1