feat(ir)!: land the IR 0.4.0 stack - #464
Open
fuad-daoud wants to merge 15 commits into
Open
Conversation
* build: pin the gate's Go toolchain to the one go.mod names (#449) The Makefile's claim is that `make gate` is what CI runs, and that is what makes a red gate worth believing. It was not true on a machine with a newer Go than CI's: golangci-lint's bundled staticcheck builds its own IR of every package it loads, the standard library included, so a stdlib it does not know panics it before it reaches a line of this repo. On Go 1.27, `make gate` failed at `lint` with five panics in `internal/poll`, and `coverage` then failed two rows that pin an encoding/json escape 1.27 spells differently — neither having anything to do with the change under test, which is the situation that teaches people to ignore a red gate. The gate now pins GOTOOLCHAIN, read from go.mod's own go directive so the version has one definition, and exported so the scripts and golangci-lint see it too — the linter reads the stdlib through `go list`. CI reads the same line through setup-go's go-version-file, replacing the literal that was a second copy of it. A GOTOOLCHAIN already set in the environment still wins and is reported, exactly as a local golangci-lint of the wrong version is. Measured on a Go 1.27 machine with nothing set: `make gate` exits 2 before this change and 0 after. This does not move the toolchain. Bumping it needs a golangci-lint whose staticcheck knows the newer stdlib and a rewrite of the two rawDivergences rows, which are now commented where each will be reached; that is a deliberate change of its own, not a side effect of this one. Closes #431 Claude-Session: https://claude.ai/code/session_016EHKV7ZYQJJXCPyynTWq4P Co-authored-by: Claude Opus 5 <noreply@anthropic.com> * fix(compilers/openapi): detect without decoding the root mapping (#448) Format detection decoded a document's whole root mapping into a two-field struct to read the `openapi` / `swagger` key. yaml.v3 compares every pair of a mapping's keys before it reads any of them, so a mapping repeating one key n times raises n(n-1)/2 errors and then abandons the mapping — the probe came back empty as well as expensive. A 32 KB source repeating one key 6,553 times produced 21,467,628 errors and a 1.2 GB diagnostic in 16.7 s; a 128 KB one did not finish in 150 s. Both were reported as unreadable, though the parser the compiler goes on to use reads them and reports the repeats itself, once each and sited. Detection now parses the document and reads the two keys off the tree, which is linear and answers the same for a mapping whose keys repeat as for one whose keys do not. The 32 KB case takes 0.048 s and prints 6,553 sited warnings; the 128 KB case takes 0.147 s. Separately, diag.OneLine now bounds what a foreign error contributes to a diagnostic message. That is the general form of the same defect — a message a library can make arbitrarily large — and it covers the two overlay callers as well, where the library's own decode is still slow but its complaint no longer reaches the terminal whole. The cut lands on a rune boundary, so a message never carries half a rune to a reader. Two rules the walk now has and the decoder could not, since it refused any mapping that repeated a key at all: a key written twice takes its last spelling, matching the parser that later records the dialect on ir.SourceInfo, so one document cannot get two answers; and a key written directly beats one merged in through `<<`. Deliberately out of scope: the merge chain is bounded at maxMergeDepth, where the decoder followed one as far as yaml's own alias limits, and detection still reports an unreadable version key only where declaresProbeKey sees it declared at column 0 — widening that guard would claim documents of formats that nest a key of the same name. Closes #443 Claude-Session: https://claude.ai/code/session_016EHKV7ZYQJJXCPyynTWq4P Co-authored-by: Claude Opus 5 <noreply@anthropic.com> * fix(compilers/openapi): keep a conflicting redeclaration's losing type When allOf branches declare one field with incompatible types the merge keeps the first declaration and warns, naming both pointers. The losing declaration was then dropped: it reached the IR in no form at all, so a consumer reading the document rather than the diagnostic stream saw no trace of it — and a diff across two revisions in which only the losing branch's type moved reported no change. GitHub's published spec writes this shape 102 times. Every other degradation in this compiler keeps what it could not model. This one now does too: the discarded ir.TypeRef is written to the merged property's Unmodeled under ReasonDegradedLowering, keyed by the redeclaration's own pointer so sibling branches never overwrite one another, and stamped with the losing declaration's provenance. The constraint half of the same diagnostic is deliberately left alone. It also discards the redeclaration's keyword, but the recorded direction there is to intersect the bounds so the merged field satisfies both branches (#10), and preserving the loser instead would settle a decision that already has one. The code comment on keepLosingType says so. Fixes #424 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SYeBgDsskwnyitLgPGCPn1 * fix(compilers/openapi): discard a losing redeclaration whole, and record every drop Addresses the review on #436. Three findings shared one root: the discard was partial and the preservation gate was narrower than the drop. **The discard was partial.** After a type conflict fired, reconcileProperty ran to completion anyway, folding the loser's Default, Constraints and Examples onto the winner. So `{id: integer}` allOf `{id: string, maxLength: 10, default: abc}` compiled to an integer carrying a string default and a string's maxLength, beside an Unmodeled entry saying the string declaration was discarded. Nothing in pass/validate or irverify compares a Value's kind to its property's type, so an emitter renders that pair into code that does not compile. The fold predates this branch; recording the loser is what made the document self-contradictory. recordRedeclarationConflict now returns whether the type was discarded and the three shape-bound folds are gated on it. Deprecation and XML are not shape bound and are adopted either way. **The gate was narrower than the drop.** Preservation hung off typesConflict, which deliberately answers false for two composites of one kind, an unresolvable target, and the top type against anything — "conflict detection does not guess". In every one of those dst kept its type and src's vanished with neither a diagnostic nor an entry, which is #424's own failure: a consumer diffing two versions sees no change. Preservation is now owed wherever a type is dropped; the diagnostic stays on the conflict predicate, because "dropped" and "contradictory" are different claims. **Nullability was neither reconciled nor preserved.** typesConflict returns on `a.Target == b.Target` before Nullable is read, so `{x: [string, null]}` allOf `{x: string}` merged to nullable with no diagnostic and no entry — and swapping the branches gave the opposite answer. The order oracle cannot see this: it never permutes sequences. Targets that agree now intersect nullability, the same conjunction foldNullVerdicts states for a single schema. Also from the review: the type route's diagnostic says the loser is kept, as every other preserving degradation in this compiler does; MergeProperty takes the declaration's position once, off p.Provenance, rather than as a second parameter equal to it by construction; keepLosingType guards an empty pointer or target, which would collapse the key to the bare prefix and let a second loser overwrite the first; the diagnose-named function is renamed for the recording it does; a false universal about every degradation preserving what it could not model is deleted; ir-design gains the §4.8 entry and the §14 mapping key that this key was already being cited against; and two tests stop re-implementing their helpers, one of which was failing on the wrong assertion when a key was absent. Two findings are deferred with the gap stated in code and issue: - **#445** — the Unmodeled value is an IR TypeRef where §12 asks for the source construct. Fixing it moves MergeProperty's signature and the golden. - **#446** — typesConflict reads a format-narrowed primitive as conflicting with its bare primitive, so uri-vs-string reports a degradation that did not happen, 102 times in the GitHub spec. The predicate is upstream of this change. Refs #424 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01R65r3qXXNM9jNbQu5gGj9v --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
* feat(ir)!: give Payload a Required field for body optionality Request-body optionality survived only as an inverted sentinel: the OpenAPI compiler wrote Payload.Unmodeled["openapi:required"] = false when a body was not required and wrote nothing when it was, so recovering the fact meant knowing an OpenAPI-specific key and reading its absence as true. A consumer that reads typed fields alone saw every body as required — 563 times across GitHub's and Stripe's published specs. ir/unmodeled.go grades no_ir_home as "a gap expected to close, not a boundary", and this is that gap. ir.Payload now carries Required *bool. The pointer is the point: a format that expresses body optionality treats an unstated body as optional, so folding "the format is silent" onto the same value as "the document says no" would lose the distinction a non-OpenAPI compiler needs. Response and message payloads leave it nil, because only a request body can be omitted. The OpenAPI compiler always sets it, since OpenAPI's own default makes an undeclared `required` mean false rather than unstated, and it no longer writes the openapi:required entry or the info diagnostic that announced the degradation — the fact is modeled now, so neither describes anything. ir-design.md is normative on the field shapes, so §7.2's Payload and §14's OpenAPI lowering summary are updated with it. BREAKING CHANGE: a consumer reading Payload.Unmodeled["openapi:required"] must read Payload.Required instead; the Unmodeled entry and its openapi/degraded-construct info diagnostic are no longer emitted. The per-reason reachability test moves its no_ir_home witness to a parameter's allowEmptyValue, which still has no typed home. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SYeBgDsskwnyitLgPGCPn1 * feat(ir)!: give Parameter a Provenance and promote its x-sunset ir.Parameter was the last lowered node carrying no Provenance, and two things followed from that. A parameter's vendor extensions were stranded. ir-design §12 rule 4 says a node with no provenance is not promoted into, because a promotion that cannot be marked Inferred cannot be audited — so the parameter position was the one ir.Deprecation carrier PromoteDeprecation was not wired at, and a deprecated parameter's x-sunset sat unread beside an empty Deprecation. It is wired now, and extension-promotion.yaml gains the parameter row so the sweep fails at that carrier rather than being covered by a neighbour. Parameter origin was erased. mergeParameters merges a path item's parameters into every operation on the path, and nothing afterwards recorded that a given parameter was inherited rather than declared. The stamp uses the pointer internal/operation already threads per parameter for the interning fix (#36, #107): an operation's own entry points under that operation, a $ref'd one at the component it names, and a path-item one at the path item — one declaration named by every operation that inherits it, which is what tells the two apart. BREAKING CHANGE: ir.Parameter gains a Provenance field, serialized without omitempty like every other node's. Every golden carrying a parameter moves, and a consumer decoding the IR sees a new object on each one. Closes #423 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SYeBgDsskwnyitLgPGCPn1 * docs(ir-design): name rule 4's live instances §12's fourth promotion rule read as if it had none left: `Parameter` was the instance it named, and the sentence recording that `Parameter` has since gained a `Provenance` left the rule with nothing to point at. `Variant` (§4.4) and `EnumMember` (§4.5) each still carry a `Deprecation` with no provenance of their own, so the rule governs them today. Name them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SYeBgDsskwnyitLgPGCPn1 * docs(ir): say what Parameter.Provenance records for a $ref'd entry The field's GoDoc, the §7.2 sketch and the params test all said the pointer tells an inherited parameter from a declared one. That holds only for an entry written inline: a referenced entry lowers at the component it names from either mount, so a path-item $ref and an operation-level $ref land on one pointer and the mount site is not recorded. Say exactly that, so a consumer reading the pointer for inherited-vs-declared knows which entries answer. Payload.Required's lead clause was inverted against its own mapping and the §7.2 sketch; it now reads the way the sketch does. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011T5no6iADeMGgYjsYcV5in * feat(pass): report a Payload.Required set outside a request ir.Payload says only a request body can be omitted and a response or message payload leaves Required nil, and nothing enforced it: a document carrying the field on a response passed every oracle, which is the reading GitHub #421 was filed about — an emitter rendering "required" off the boolean prints it on a response. The Payload-bearing fields checkEncodingKeys named by hand now live in one walk, forEachPayload, so the carrier guard covers every check built on it and a carrier added to the IR reaches both. The new check reports ir/payload-required-outside-request at severity error, with a hand-built fixture per carrier since no compiler produces the shape. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011T5no6iADeMGgYjsYcV5in * test(compilers/openapi): hold the promotion carriers to the IR promotionCarriers is the sweep over every node the compiler promotes a deprecation into, and it was hand-written with nothing holding it to the IR: the IR declares eight structs carrying a Deprecation, the map named five of them, and a carrier added to the compiler without a PromoteDeprecation call reddened nothing — the shape of defect Parameter had before #423. A reflective walk over the IR, seeded from Document and every TypeDef kind the ir sources declare, now requires each such struct to be mapped to the rows that witness it or exempted with a reason held to the IR: Variant and EnumMember until they gain a Provenance, Message because the OpenAPI compiler builds none. A second test holds the mapping and the corpus rows to each other. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011T5no6iADeMGgYjsYcV5in * refactor(compilers/openapi): unexport schema.Preserve Its last caller outside the package went with the openapi:required write; every remaining one is in-package. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011T5no6iADeMGgYjsYcV5in * test(compilers/openapi): assert a shared body's required at both uses The fixture's required: false contributed to nothing the test observed. Both operations now assert the flag, as two values rather than one aliased pointer. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011T5no6iADeMGgYjsYcV5in * docs(ir): note the 0.4.0 shape changes in flight on IRVersion The stack squash-merges bottom-up and the bump lands with its top, so a main between the first of those merges and the last carries a 0.4.0 shape stamped 0.3.0. Say so where a reader of main will look. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011T5no6iADeMGgYjsYcV5in --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
* feat(ir)!: make ErrorCase a response: name, headers, media types
ir.ErrorCase and ir.Response are two lowerings of one Response Object, but
only one of them could say what the source declared. Response carries a
Name whose Hint is the status spelling, Headers, and a Payload holding
every media type; ErrorCase carried none of the three — no name at all, no
headers, and one bare TypeRef where the content map belongs.
Everything that fell outside those fields went to ErrorCase.Unmodeled with
an info diagnostic, so a consumer's behaviour changed with the status
class and nothing said so:
- Retry-After and the rate-limit family live on 429 and 503, precisely the
side with no typed home for a header.
- A 4xx declaring application/json and application/problem+json kept the
first schema and lost the media-type key entirely; a 4xx declaring one
media type lost the key it was written under.
- "5XX" and "default" had no faithful round-trip: StatusRange renders
{500,599} and {0,0} with no record of how the source spelled them.
ErrorCase now has Name Naming, Headers []Property and Payload *Payload in
place of Type, each spelled as Response spells it, and the error path
lowers through the same responseName, lowerHeaders and lowerPayload the
success path uses. preserveErrorHeaders, fillErrorType, preserveErrorContent
and errorContentMessage existed only to soften this gap and are gone with
it, along with the two info diagnostics they emitted.
pass.checkEncodingKeys grows a fourth Payload carrier, reached at both
positions an ErrorCase hangs from — an operation's Errors and a service's
CommonErrors — since a check walking only the first would resolve a
service-level error's encoding keys against nothing in silence.
BREAKING CHANGE: ErrorCase.Type is removed; an error case's models are its
Payload.Contents entries' types. The JSON gains "name", "payload" and
"headers" and loses "type". ir.IRVersion is deliberately not moved here:
per ir-design.md §2.1 a line of work bumps it once, where it lands on main,
and two earlier shape changes on this branch left it alone for the same
reason.
The normative rows in docs/ir-design.md that described the old behaviour
are updated, as is the error-taxonomy example in docs/emitter-design.md.
The per-status-errors conformance fixture gains a 429 declaring two media
types and two rate-limit headers, which is what makes the new fields
witnessed rather than merely present.
Closes #422
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SYeBgDsskwnyitLgPGCPn1
* fix(compilers/openapi): derive an error payload's hint from its declaration
Addresses the review on #438.
**A shared response across the status boundary produced order-dependent IR.**
A components/responses entry mounted at both a success and an error status
lowers through lowerResponse and lowerErrorCase, which passed payload hints
"response" and "error"; both intern the body at the same declaration pointer, so
the two mints raced and the loser's hint was discarded. Reversing the two keys
renamed the type. Both hints now come from ids.DeclarationHint on the
declaration pointer, which is one pointer whichever side reaches it first. The
behaviour predates this branch; what was new is the prose claiming the two sides
are symmetric, corrected here and in ir-design's matrix row.
Nothing in the corpus asked the question — component-reuse.yaml mounts Listed
only at 200s and Failure only at default — so the order oracle never fired on
it. shared-response-across-status.yaml is added to close that.
**Two responses-map keys can name one range.** "4XX" beside "4xx" compiled with
exit 0 to two error cases identical in name and conditions, and an ErrorCase has
no ID, so those two are the whole of what tells them apart. Reported now as
openapi/duplicate-status-key, a warning, with both kept: neither key is wrong on
its own and dropping one picks a winner on declaration order.
Naming.Source was the other candidate and is not available: a responses-map key
is not a name the document declared, which TestResponses_NamedByStatusKey pins
for both sides, and Source without a derived Canonical breaks the pairing
NamingFor holds — irverify reports ir/naming-not-derived. The four sites
claiming the key reaches the IR "as written" are corrected instead: it is
neutralized, "5XX" arrives as "5_xx", and only "default" round-trips.
**Nothing pinned that the walk reaches ErrorCase.Name or ErrorCase.Headers.**
Planting a skip for either left the whole suite green, where the Response twins
and ErrorCase.Payload each redden a test. Both are guarded now — an "error case"
row in TestVerify_NamelessServerAndResponseAreViolations and a ghost header on
the error case in TestValidate_OperationHeadersAndItemWalked — and each was
confirmed to redden under its planted skip.
Also: the ErrorCase GoDoc no longer claims the two nodes differ only in fault
classification, and records why StatusCodeProp stays success-only; checkEncodingKeys
addresses a service by ID as checkServerIndices does, rather than positionally,
so one node stops having three spellings from one package; and the corpus
justification in verify_corpus_test.go names the real reason, since both things
it claimed the corpus does not reach are reached by it.
Deferred with the gap stated in the doc and filed as **#447**: PlannedError has
no Headers field and no media-type election, so the worked example's plan line
contradicts the IR line above it. That is emitter-design work rather than a
correction to what this PR changed.
Refs #422
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01R65r3qXXNM9jNbQu5gGj9v
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
* feat(ir)!: give Deprecation a RemovalDate and promote x-sunset into it x-sunset echoes RFC 8594's Sunset header, which is a date by definition, but the default promotion mapping read it into Deprecation.RemovalVersion — a field whose name, doc comment and sibling all say version. A consumer deciding whether removing a deprecated operation is breaking compares a sunset against a release date, and could not tell which spelling it had been handed without re-parsing the string. Take issue #417's option 1: a distinct RemovalDate beside RemovalVersion, with x-sunset promoting to the date. A version and a date are two facts, not two spellings of one — a document may state both ("gone in 3.0.0", "gone on 2026-08-01"), and neither is derivable from the other without a release calendar the IR does not have. A single field carrying which spelling it holds (option 2) would have to drop whichever fact it read second, so it costs losslessness to buy nothing a second field does not already give: the field a value arrives in is what says which fact it is. Deliberately out of scope, and stated in ir-design.md and at the reading site: RemovalDate is the source's own text, neither parsed nor normalized. No source format defines the field, so none defines its format; and the key→field mapping is caller policy, so a key pointed at the date field is the caller's statement that it holds a date. Morphic records which fact was stated and leaves the calendar to the consumer. BREAKING CHANGE: Deprecation gains removalDate, and x-sunset now fills it instead of removalVersion. A consumer reading removalVersion for a sunset reads an empty field until it moves. No default key names RemovalVersion any more — a document stating a removal version names its own key, per promotion rule 1 — so the corpus stops witnessing that field and it joins unwitnessed.golden.txt. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SYeBgDsskwnyitLgPGCPn1 * feat(compilers/openapi): promote x-extensible-enum onto Enum.Closed ir.Enum has carried a Closed bool since the IR was written, and the OpenAPI compiler set it true at both construction sites unconditionally. So the one key the format has for saying an enum is open, x-extensible-enum, survived only as a generic vendor_extension entry, and every consumer reading typed fields saw a closed enum whatever the document said. Open versus closed decides whether a generator emits a fallback member and whether a differ calls an added value breaking, so this was a wiring gap, not a modelling one. Add TargetEnumOpen to the promotion vocabulary, map x-extensible-enum onto it by default, and apply it in attachDeclaredAnnotations beside the deprecation promotion — the point at which a declaration's extensions have reached the node's map, which is what makes "the extension survives its own promotion" structural here as it is there. Every promotion property holds unchanged: the entry stays put with its vendor_extension reason, the node records extension-promotion in Provenance.Inferred, and a disabled policy writes nothing. The target names the fact rather than the field, which the rest of the vocabulary does not. Openness is the only half of that bool a document ever declares — a schema's `enum` is closed by definition — so a target named for Closed could only ever be written false and would read as its own opposite at every mapping naming it. For the same reason the key's presence is the statement rather than its value: the established spelling writes the member list as the value, and a list of members says nothing about openness the key naming it has not already said. A boolean is the one shape that does state it alone, so an explicit `false` is read as written rather than inverted. Deliberately out of scope, and stated in ir-design.md and at the reading site: a document writing x-extensible-enum *instead* of `enum`, with the members in the extension, lowers to no ir.Enum at all and there is no node to open. Minting one would be reading a member list out of a vendor key rather than promoting a field; the entry survives verbatim for a consumer that wants to. The corpus can now witness the matrix's open-enums row, so its matrixRowsUncovered reason is deleted rather than left to go stale, and extension-promotion.yaml gains the three enums that pin the three answers the reading has: the convention opens one, an explicit false declines to, and an enum naming no such key is untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SYeBgDsskwnyitLgPGCPn1 * fix(compilers/openapi): read a bare x-extensible-enum as open JSON null decodes into a bool without error and leaves it false, so a bare `x-extensible-enum:` — the presence-only spelling the reading exists for — read as the explicit false that is the one way a document declines, with no diagnostic. Decode into *bool so absence of a value is told apart from false, and pin the null shape beside the list, the true and the prose the test already covers. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011T5no6iADeMGgYjsYcV5in --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
* feat(ir)!: give contentSchema a home on Encoding and lower it 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 * docs(ir): say constraints never cross a $ref to a use site 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 * fix(ir)!: carry minimum and exclusiveMinimum as separate bounds 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 * fix(compilers/openapi): name what sits under contentSchema by its owner The contentSchema hoist added a structural position the outside-$ref namer did not know: structuralRole is the replay of the compile.SubHint call sites, and it was not extended, so an outside $ref reaching the position first named everything under it from the segment (content_schema_item) while the declaration named it from the owner (nested_content_item). The registry then depended on declaration order, and the harness said so. One case in structuralRole closes it. The two-order test that its doc named as the guard could not see the gap for any position: every row aimed the reference at the very node it asserted, which the declaration renames in either order (#372). Rows now say where the reference goes, and the row that pins this aims it above the node. The corpus gains the same shape, reference declared first, so the order oracle asks the question too; with the case reverted, the conformance golden, the two-order row and the in-repo harness sweep all redden. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011T5no6iADeMGgYjsYcV5in * docs(ir-design): restore the §14 clauses a stale paste dropped The contentSchema commit rewrote the OpenAPI row from a copy taken before the ErrorCase fixes below it landed, and three unrelated facts went with it: the responses-map key reaching the IR neutralized rather than as written, the duplicate-status-key warning, and the payload hint derived from the declaration on both status sides. All three describe behaviour live on this branch. Word-diffed against the base, the row now differs by the two clauses this stack means to add and nothing else. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011T5no6iADeMGgYjsYcV5in * test(ir): hold every populated fixture to leaving no field zero populatedEncoding is the round-trip half of TestEncoding_JSONContract and did not set the new Schema field, so retagging it json:"-" left the package green. Set it, and add a reflective guard over every populated* struct fixture so the next added field cannot slip through the same way; a field a fixture leaves zero on purpose is listed against its reason and asserted in both directions. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011T5no6iADeMGgYjsYcV5in --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
* fix(compilers/openapi): scope the key scan, keep the read's error The whole-source read #420 added got two answers wrong, both by reusing machinery written for a cut prefix. The flow decoder reports "this is a flow mapping" for anything opening with `{`, and drops the error that ended its walk. For a cut prefix that is right: the cut always breaks the token stream, so the error describes the cut and not the document. For a whole document it hides the document's own break. A JSON source past the cap whose `openapi` key sits behind a syntax error came back with a nil error, so Detect saw no failure to report and declined it as an unrecognized format — the very answer #420 set out to replace, still standing for every JSON source, which is the style the motivating spec is written in. The decoder now returns the error that stopped it and treats stopping on the mapping's own closing delimiter or on the entry cap as no error at all; sniffPrefix drops it along with the cut that caused it, and sniffWhole keeps it. The key scan was widened to the whole source without being scoped to the top level. Its block arm reads column 0 and always was top-level, but its quoted arm matched `"openapi":` at any depth, anywhere in the buffer. Bounded to the first 64 KiB that cost a needless parse; over a whole source it makes a claim, and a wrong one — another format's document that nests such a key and does not parse was reported as an undecodable OpenAPI source. Saying nothing about bytes that are not this compiler's own is the rule detection is built on. The quoted spelling is how flow style writes every key, so flow structure is what scopes it: a depth-tracking scan reads the root mapping's own entries and nothing under them, and a source that opens no mapping at all declares nothing here. It is a lexer rather than a parser because the case it exists for is a document broken before the key that names it, where there is no tree to ask. A block document that quotes its top-level key is no longer seen and is declined in silence, which is the direction to be wrong in. A valid document past the cap that declares its version last still compiles, which is what #420 was about. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SYeBgDsskwnyitLgPGCPn1 * fix(compilers/openapi)!: scan for the version key instead of parsing Detection parsed whole documents to find one key. sniffWhole ran yaml.Unmarshal over everything past the 64 KiB cap, before the loader's size and node budgets and with no context to cancel it: 32 MB cost 805 ms and 333 MB, for a file the node budget then refused anyway. main declines that file in 11 ms. #441 scoped the key search but left the parse. The scan the guard already ran is extended to read the value beside the key, which answers the whole question without building a tree: 8 MB 209 ms / 82 MB -> 1.9 ms / 0 allocations 32 MB 805 ms / 333 MB -> 4.1 ms / 0 allocations Reading the value, rather than only finding the key, closes two more holes. Key order stopped deciding the format: the prefix answered on whichever key it reached and returned, so a document declaring both read as swagger@2.0 above the cap and openapi@3.0 below it — one document, two answers, which is the property TestDetect_KeyOrderDoesNotDecideTheFormat claims. And a key alone is no longer a declaration: prose beside the word (`openapi: is a format` in a Markdown file, at any size) is declined in silence instead of claimed and reported under this compiler's parse error. BREAKING: detection past the cap no longer emits undecodable-source. It cannot: it has read one key, not the document. A source it names and the loader cannot parse is now the compile's finding, so openapi.Compile turns load.ErrParse into that diagnostic rather than returning a Go error. engine.Run wraps a compiler's Go error in its own and the CLI maps that to exit 2 — the code it uses for being invoked wrong — so without this a broken spec would be reported as a misuse of morphic. Exit 1 and the diagnostic are preserved, and the message now carries the loader's own position instead of a throwaway parse's. A document past the cap that declares the key with no version beside it is declined rather than claimed: a scan cannot tell that from another format's file, and claiming the wrong one of those two is the costlier mistake. The prefix machinery goes with the parse it existed to avoid — sniffPrefix, sniffWhole, decodeFlowEntries, wholeLines and the entry cap. Tests: the six mutations the review planted all stayed green. Twelve against the new code, including all six where they still apply, are all caught. Adds the engine.Run case over a generated >64 KiB JSON with the version last — nothing else in the suite reaches that path, the corpus's largest spec being 7 KiB — and the cap-boundary cases, where a broken document is what tells the two readings apart. Refs #420 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01R65r3qXXNM9jNbQu5gGj9v * docs(compilers/openapi): describe the cap as the scan boundary it is The commit that replaced the past-the-cap parse with a scan left maxSniffBytes' comment naming sniffWhole, which the same commit deleted, and describing a document past the cap as "read whole" when it is now not read as a tree at all. Says what the constant does: it is where detection stops parsing and scans, and nothing is declined for being large. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016EHKV7ZYQJJXCPyynTWq4P * fix(compilers/openapi): read past the cap what the parse would read The scan and the parse read a document differently, and the cap decides which answers, so every shape they disagree on names one format below 64 KiB and another above it. Three were the scan's to fix: a UTF-8 byte-order mark defeated both arms, so a spec an editor had marked compiled at the cap and was unrecognized one byte past it — the #420 failure back for a new input class; the block scan crossed YAML document boundaries, claiming a key in a document the compile never parses; and it took `openapi:3.1.0`, a plain scalar, for a key the parser says declares nothing. The mark is trimmed, both arms are bounded to the first document with the opening marker left behind so a flow document after `---` reads as one, and a block entry needs the separated colon YAML needs. The shapes the scan does not read by design — a root merge key, whose `<<` means resolving an anchor and so the parse the cap exists to avoid, on an input the source chooses; a quoted key in block style; an unquoted one in flow style; an anchor or a tag before the version — are declared in a differential table that runs both readings over one set of bytes, each declared row against its reason and asserted in both directions, so a tolerance added deletes a reason and a shape lost adds a row. declaresProbeKey is reached only after a parse failed, which only runs at or below the cap, so its doc described a call site this branch deleted and the test feeding it input past the cap exercised bytes it cannot be handed; the one row that was not a duplicate moves to the scoping test and the rest go. The key-order test is named for the one shape it pins rather than for a size-independence the branch does not have. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011T5no6iADeMGgYjsYcV5in * fix(compilers/openapi): site an unreadable source at NoSource undecodable said the source table exists once a parse has failed. It does not: the parse that failed is the one that would have built the document, Compile returns none, and a Source of 0 against the nil document engine.Run hands on resolves to no path — naming nothing while claiming to. The loader's own message carries the position, so the diagnostic is sited at NoSource, as Detect's sibling for the same condition already is. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011T5no6iADeMGgYjsYcV5in --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
* feat(ir)!: bump IRVersion to 0.4.0 for this branch's shape changes Six commits on this branch change the JSON shape of a Document and none bumped the constant, each correctly deferring per ir-design 2.1: a line of work bumps it ONCE, where it lands on main. This is that bump. The GoDoc on IRVersion names exactly the failure a missing bump causes -- "a shape change that reaches main without a bump leaves a consumer pinned to the old version accepting a document it cannot read, which is the one thing this constant exists to prevent" -- and nothing in the gate can see it, because TestVerify_CurrentIRVersionIsClean, TestVerify_IncompatibleIRVersionIsAViolation and openapi_test.go all compare against the same constant and stay green whatever it says. Found by review, not by CI. The log paragraph records all six, each framed as what a 0.3.0 consumer gets wrong rather than as a feature: ErrorCase loses Type and gains Name/Payload/ Headers; Payload gains Required; Parameter gains Provenance; Deprecation gains RemovalDate and x-sunset routes there; Encoding gains Schema; and Constraints.ExclusiveMin/Max change from bool to a decimal string, which is the one that fails a consumer's decode rather than degrading it. TestCompatibleVersion's neighbour rows were spelled against 0.3.0, so the bump made "a later generation" 0.4.0 assert that the build rejects its own documents. Re-anchored, with a comment saying they move with the constant. 79 goldens regenerated; the only key that moved is irVersion. BREAKING CHANGE: IR documents now stamp 0.4.0 and CompatibleVersion refuses 0.3.0. Consumers must recompile rather than migrate stored documents. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SYeBgDsskwnyitLgPGCPn1 * test(ir): spell the padded-version rows relative to IRVersion The bump left irverify's whitespace row at a literal " 0.3.0 ", which at 0.4.0 is rejected for being a prior generation whether or not the padding matters: a whitespace-tolerant CompatibleVersion failed only the ir test. The row now moves with the constant, and the comment on document_test's three literal rows says what they are — spelled against the current value and re-anchored by hand at every bump — rather than claiming they move. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011T5no6iADeMGgYjsYcV5in --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
fuad-daoud
commented
Sep 12, 2026
fuad-daoud
left a comment
fuad-daoud
left a comment
Collaborator
Author
There was a problem hiding this comment.
Review of f302d6e (origin/main...origin/stack/1-detection, 7 commits). Every correctness finding in the inline comments was confirmed by executing a probe against the PR head, and against origin/main where a regression is claimed.
Inline, most severe first:
merge.go:118— loser's default/constraints fold onto the winner whentypesConflictdeclines to judgemerge.go:119— the #424 mechanism is fixed forTypeonly;Default/Examples/Deprecation/XMLstill drop silentlyoperations.go:913—Naming.Hinton a shared response is order-dependentdetect.go:402— past the cap the block scan reads less than the parse in undeclared shapesdetect.go:368— the scan claims a format the parse rejects (flow/quoted continuation at column 0)operations.go:789—duplicateStatusKeyDiagis sited on the$reftarget, and dedup drops the second operation's warningconstraints.go:196— false orphan diagnostic on an unparseable minimumdetect.go:233— prerelease versions are silently declined (regression frommain)merge.go:331— doc comment cites code deleted by #440 in this stack (alsodetect.go:43)detect.go:431— double lexing in the flow scan, duplicated indeclaresFlowKey
Lower-value items, verified but not commented inline:
merge.go:359—raw, _ := json.Marshal(src.Type)blank discard; CLAUDE.md bans_ = err(rawjson.gohas the same precedent).merge.go:391— a maintained "102 occurrences in the published GitHub spec" count with no revision named.assertEnumOpennessis missingt.Helper().PromoteEnumOpennessre-spellsPromoteDeprecation's loop but iterates unsorted.- The
boundSidemapping is spelled twice inconstraints.go. - Two test fixtures bypass the
errorPayload()/multipartPayload()helpers this PR adds. forEachPayloadis now driven twice perValidate, with eagerSprintf.firstDocumentandscanBlockProbewalk the lines twice.- The pre-existing whole-struct
Encodingoverride rule would drop the newEncoding.Schema(no consumer implements the override yet).
Environmental, not a PR finding: go test ./... fails locally in compilers/openapi/internal/annotation on the !!binary raw-node test, identically on main. It is the local Go 1.27 toolchain's encoding/json spelling U+FFFD raw where the pinned 1.26.3 writes \ufffd; the file is not in the diff.
🤖 Generated with Claude Code
The allOf merge kept a dropped type under Unmodeled as the discarded ir.TypeRef, and only when the type was dropped. Two things were wrong with that. The skip that stops a loser's default and constraints from folding onto a winner whose type cannot hold them was keyed on the conflict diagnostic rather than on the drop, so two Models — dropped without being called a conflict — still folded the loser's default onto the winner beside an entry saying its type was gone. And every other adopt-if-absent field (default, examples, deprecation, xml) let a present-but-different loser vanish with no diagnostic and no entry. The entry is now the redeclaration's own schema node, rendered only when something of it was lost, which covers every field at once and closes GitHub #445 with it: ir-design §12 defines an Unmodeled value as the source construct, and a TypeID is one irverify's reference walk cannot see dangle inside a byte slice. A detail the first declaration already holds differently is named in an info diagnostic, and the skip is keyed on the drop. MergeProperty takes the node as a function so a property declared once — nearly all of them — renders nothing. Closes #424, closes #445. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Eg4SQKpyxnVvRPCW2vfvAr
lowerErrorCase was a copy of lowerResponse with one difference: the naming-hint fallback, "error" against "response". The fallback is used wherever the declaration pointer names no component, so a response $ref'd across operations by its path pointer and mounted once as a success and once as an error interned one type whose hint was whichever mount lowered first — while the function's own comment claimed the opposite. The two now share lowerResponseParts, and the shared-response-across-status fixture gains the path-pointer pair so the order-invariance oracle sees the shape. The status-key diagnostics were sited at the resolved pointer, which for a $ref'd response is the component: a warning about the operation's own map named no operation, and the second operation to make the same mistake lost its warning to dedup. They are sited at the operation now. Doing so exposed that the duplicate-key warning depended on which spelling came second, so it is reported once per colliding range, at the map, naming every key in sorted order. Closes #433. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Eg4SQKpyxnVvRPCW2vfvAr
A 3.0 exclusiveMinimum: true beside a minimum whose literal would not read drew a second, false diagnostic asserting the minimum was absent and parked the modifier under Unmodeled as an orphan, because the parsed slot is nil in both cases. The orphan branch now reads the raw node: a bound the schema wrote has already earned its own error, and its modifier goes with it. The exclusive keyword's name is derived from boundProps rather than spelled a second time. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Eg4SQKpyxnVvRPCW2vfvAr
Four ways the scan disagreed with the parse it stands in for, each a document that named one format below 64 KiB and another above it: - A prerelease or otherwise unserved version was declined silently by both readings, handing a file whose first line is `openapi:` to the engine's generic "unrecognized format" where load had reported the version by name. isVersion now admits one word beginning with a digit and keeps out only prose. - A tab before a trailing comment, a space before the colon, and a bare scalar in flow style were read below the cap and declined above it, in shapes the declared-divergence table did not list. - A column-0 `openapi:` inside a flow collection or quoted scalar still open from a line above was read as a root key, claiming a document the parse reads as nesting the key. The block scan now carries that state across lines, block scalars included; a construct never closed leaves its lines root lines, so an unparseable document still names itself. - flowValue ran for every quoted string at every depth before the cheap depth and name tests, lexing each value twice, in a loop duplicated in declaresFlowKey. One walker serves both, and a benchmark covers it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Eg4SQKpyxnVvRPCW2vfvAr
Two ErrorCase fixtures spelled out the body errorPayload and multipartPayload exist to build, and assertEnumOpenness lacked t.Helper(), so a failure pointed into the helper rather than its caller. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Eg4SQKpyxnVvRPCW2vfvAr
fuad-daoud
commented
Sep 12, 2026
fuad-daoud
left a comment
fuad-daoud
left a comment
Collaborator
Author
There was a problem hiding this comment.
Re-verified 13db930 (the five commits since f302d6e) by re-running every original probe against the new head and confirming each named regression test reddens with the pre-fix source, or with a planted mutation where the pre-fix file no longer compiles. All ten notes from the first review are fixed and hold up. make gate passes on the head (exit 0, 100% coverage) once a stale golangci-lint cache is cleared; the !!binary annotation test fails only under a bare go test with a local Go 1.27 toolchain, and the gate's pinned toolchain passes it.
Three residuals, all in detect.go and all in the construct tracker that af5c166 added, are inline. The first is the one that matters: it claims a format the parse declines, which is the direction the tracker exists to prevent.
Still standing from the first review's lower-value list, unclaimed and unchanged: PromoteEnumOpenness iterating unsorted (the doc comment's order-independence argument holds, so this is shape only), forEachPayload driven twice per Validate, the firstDocument/scanBlockProbe double line walk, and the whole-struct Encoding override rule in docs/ir-design.md that would shadow the new Encoding.Schema.
🤖 Generated with Claude Code
The construct tracker tested the first byte after a colon, a sequence
dash, or the document start for a flow opener or a quote, so an anchor
or tag in front of one — `a: &x {`, `!!map &x {`, `- &a "…"`, an
anchored root flow mapping — opened nothing, and every column-0 line
the construct then continued was read as a root key: the claiming
direction, on shapes yaml.v3 continues. The flow lexer had the same
gap at a token start, where `{a: &x "}"}` quoted its own closer. Every
opener site now skips up to two node properties first.
The block-scalar half of the tracker was unpinned: every row carrying
one left its opener unclosed, so the end-of-document adoption returned
the key whether or not the indicator was seen. A row whose brace a
later root line closes now reddens without it.
A version written on the line after its key is declared rather than
read: it is a plain scalar that may run on for several lines, and
reading its first line alone would claim `3.1.0 more` by its first
word.
PromoteEnumOpenness visits the policy sorted, as PromoteDeprecation
does; the order never changed its answer, but one spelling of the loop
for both carriers is one fewer place for them to drift.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Eg4SQKpyxnVvRPCW2vfvAr
Collaborator
Author
|
Second round addressed in fa9f5ed (three inline threads replied). On the four items still standing from the first review's lower-value list:
|
fuad-daoud
commented
Sep 12, 2026
fuad-daoud
left a comment
fuad-daoud
left a comment
Collaborator
Author
There was a problem hiding this comment.
Re-verified fa9f5ed by re-running every probe from the previous round and checking each redden claim.
- Node property before an opener: fixed. All probes agree between parse and scan, and Detect past the cap matches below it. The previous head's
detect.gounder the new tests reddens ten named rows. Removing the skip at each call site individually reddens at least one row, so no site is unguarded. One correction to the reply: there are four calls toskipNodeProperties(walkFlowRoot,scanBlockProbe,openValue,lexFlow); "five" countsopenValue's two positions inopenAt, which I also mutated separately, and both redden. - Continuation-line version: declared, and the behaviour matches the declaration.
- Block-scalar tracker: pinned.
blockScalarAtalways-false reddens exactly the two rows named. PromoteEnumOpennessiterates sorted; #465 frames the override rule as the IR decision it is; the two declined walks have their reasons.
make gate passes on fa9f5ed with zero lint issues and 100% coverage on every package.
Two inline observations, neither blocking.
🤖 Generated with Claude Code
…bles The declared reason for a version on the line after its key cites the multi-line shape, but only the single-line row was declared; a scan that read the first continuation line alone would have claimed `3.1.0` on `openapi:\n 3.1.0\n more` with nothing reddening. The run-on shape is a declared row now, and reddens under that change. The tables' doc comment says what they speak for: documents the parse accepts. A document yaml.v3 refuses is reported below the cap and read by the scan above it, which is the split TestDetect_TheCapDecidesWhichReadingAnswers pins rather than a divergence either table could name. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Eg4SQKpyxnVvRPCW2vfvAr
fuad-daoud
commented
Sep 12, 2026
fuad-daoud
left a comment
fuad-daoud
left a comment
Collaborator
Author
There was a problem hiding this comment.
Verified df33de1. The run-on row passes, and a planted scan that takes the first indented continuation line as a root key's value reddens exactly the two continuation rows and nothing else, which is the claim. The scope sentence on the tables says what it needs to. make gate passes on df33de1 with zero lint issues and 100% coverage on every package.
One doc-precision nit inline; nothing else outstanding from my side.
🤖 Generated with Claude Code
The readings tables' doc comment named TestDetect_TheCapDecidesWhichReadingAnswers for the split it describes — undecodable below the cap, claimed by the scan above it — but that test pins a document broken before any version, which neither reading claims. The split is pinned by TestDetect_Formats' "key past the cap on an unparseable prefix"; the comment now cites that, with the other test for the half it does pin. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Eg4SQKpyxnVvRPCW2vfvAr
Wahbeh-Mohammad
approved these changes
Sep 12, 2026
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.
Summary
Lands the reviewed stack on
main.stack/1-detectionholds the seven PRs of the stack, each already squash-merged into it bottom-up with its final Conventional Commits subject, followed by the five commits that address the review off302d6e. It is a clean fast-forward ofmain:TypeRefpayload; closes conflicting-redeclaration discards the losing type with no Unmodeled record #424 and conflicting-redeclaration keeps an IR TypeRef where ir-design 12 asks for the source construct #445&anchor/!tagwords at every opener site; the block-scalar half is pinned; a version on the line after its key is a declared divergence;PromoteEnumOpennessiterates sortedMerge with "Rebase and merge", not squash. Each of the first seven commits is already one PR's squash; squashing again would collapse the subjects into one and lose the per-PR history the review happened against.
Breaking
IR 0.3.0 → 0.4.0. Every shape change is recorded in
IRVersion's doc comment and in the individual PRs; #442 carries the summary. The review fixes change no IR shape;openapi:conflicting-redeclaration<pointer>entries now hold the source node rather than aTypeRef, which is what ir-design §12 always said anUnmodeledvalue is.Deliberately left as is
From the review's lower-value list:
forEachPayloadis driven once per check, matching every other check inpass/validate; the block-scan line walks are left as they are; the whole-structEncoding/XMLHintsoverride rule is filed as #465, being an IR semantic decision rather than a fix.Test plan
f302d6eand is green now; the two-order diffs are ininternal/operationand the corpus (shared-response-across-status.yamlreddens the order-invariance oracle onf302d6e).make gategreen in a clean worktree at 1be37b2.🤖 Generated with Claude Code
https://claude.ai/code/session_01Eg4SQKpyxnVvRPCW2vfvAr