Repository navigation
fix(compilers/openapi): judge redeclared allOf field types - #1000
Conversation
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
A differing target that is not a narrowing is a drop, and dst keeps its own null admission; only the same-referent and narrowing paths intersect null. The doc claimed the intersection for every pair. Comment-only. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Cover the full null-admission truth table of a narrowing redeclaration, in both orders, and pin the drop arm of foldTypeTargets keeping the first declaration's null admission; each was previously unheld by any test. Reword the two redeclaration messages so they say the first declaration's value was kept, which stays true after a broad-first narrowing swaps the type; regenerate the one golden carrying the text. Correct four prose claims that overstated the narrowing rule (ir-design §4.8 and §14, detailDiffersDiag, recordRedeclarationConflict), and document why Narrows is exported. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
foldDocs folded only the description, so two declarations of one property that gave a title or external links differently lost the second silently: a narrowing pair, whose type is no drop, and a pair naming equal targets alike. It now folds summary, description and external links the same way: an empty side adopts the other, equal sides are one construct, and differing sides keep the first, report an info degraded-construct diagnostic and keep the redeclaration verbatim under Unmodeled. ir-design §4.8 now lists title and external link among the reported details, and says "a bare primitive one declaration narrows" where it had dropped the qualifier. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
The redeclaration diagnostic named a schema's title "summary", a keyword no Schema Object has, so it now says "title". A test now holds foldDocs to reporting each disagreeing docs field rather than stopping at the first. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Main's golden reader refuses irVersion, so the branch's golden drops it. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
OmarAlJarrah
left a comment
There was a problem hiding this comment.
Reviewed at 53b706f5, trial-merged with main at e939673f in a scratch worktree. Every probe below ran on that merge.
What holds. The core rule is right. When two bare primitives are such that one strictly contains the other, the narrower kind is the intersection, and it should win in either order. The mutations I re-planted were caught: skipping the null conjunction on the narrowing arm, and removing float → number. On the golden spec (cloudflare/api-schemas 59bd82aa), the IR and the diagnostics are byte-identical between main and this branch merged with main, with no redeclaration entries at all. So this PR doesn't move the golden spec, and anything deferred from it ranks off the golden spec. On the merged tree, the pinned golangci-lint v2.14.0 reports 0 issues, scripts/vet-rules.sh is clean, merge and schema are at 100% coverage, and go test ./... is green once the conflict is resolved as in the separate comment.
Blocking
- Values the narrower kind doesn't admit now land on it with no diagnostic, which is a regression against main for valid specs (
foldTypeTargetsthread). - The narrowing mechanism stays open, and silent, at enum, const, top-type and container-element sites. None of these is tracked, yet
Closes #598closes the issue whose title is that mechanism (bareNarrowingthread). - The conflict, and two failures that only show after merging main:
TestLoweringSpec_CasesAreTheCorpusand the census golden. The full resolution is in a separate comment.
Should fix: a single narrowing verdict instead of two (recordRedeclarationConflict); the relation's rationale, shape and overlap reporting (primParent); where the relation lives and what pins it (Narrows); the subsumption test's name; two fixture cases.
PR body, once the above settles
- "a
$refto a nameduuidcomponent" still warns only when the component lowers before the schema composing it. Declared after it, the pair is silent (#898). - "silencing them would make that order dependence silent" doesn't hold for the enum, const, top-type, container or same-kind
Scalarpairs, which are already silent on main. - The gate paragraph names
TestDetectCycles_MergeChainPastBoundStaysFastAndWarnsand #979. After #999, that test isTestDetectCycles_MergeChainPastBoundWarnsand no longer reads a clock, and #979 is closed. - The repository's squash default is
COMMIT_MESSAGES, which would paste the branch's eightCo-Authored-Bytrailers into main, whose recent history has none. Set the squash body by hand.
OmarAlJarrah
left a comment
There was a problem hiding this comment.
Second, broader pass, again at 53b706f5 merged with main at e939673f. The new evidence is in replies on four existing threads (defaults, non-bare sites, the relation's shape, where it lives) and in two new threads below.
What I ran beyond the first pass
-
Real-world differential. A local sweep of 1,840 public OpenAPI 3 documents (generator test suites), main against this branch. No document changes exit status, panics or times out. Five change:
- one loses 183 narrowing warnings and their kept entries, five of them with a type change;
- two baseline specs contain exactly #598's
numberToIntandstringToDate, which now narrow; - in one, a property declared
integer, thennumber, thenstringnow narrows the first pair, reports the differing example as info, and still warns onstring; - one negative test changes only message text.
No changed property carries a default or example, so the
foldTypeTargetsregression never fires in this corpus. -
Pair sweeps.
- All 406 pairs of 32 property spellings, in both orders, through the real lowering. The numbers are in the
bareNarrowingandprimParentreplies. - All 152 combinations of a pair the merge settles without a conflict warning and the two sides' null admission, in both 3.0 (
nullable) and 3.1 (type-array) spellings. Null admission comes out as the conjunction in every one, whatever the order.
- All 406 pairs of 32 property spellings, in both orders, through the real lowering. The numbers are in the
-
Fuzzing.
FuzzLowerSchemawith narrowing seeds ran 664k executions and found nothing. -
Other callers.
MergePropertyhas one production caller (fillModelProperties), andtypesConflictandresolvePrimKindare reached only from the merge. So the changed predicate doesn't leak anywhere else. -
Hygiene. The PR's new comments cite nothing stale, both branch merges are true two-parent merges, and CI is green at the head (that run is before main is merged in).
Adjacent, worth filing rather than fixing here
- An
allOfover primitive branches lowers to an emptyModel, at a schema's root and inside a property alike (p: {allOf: [{type: number}, {type: integer}]}). Both branches are kept verbatim, with a warning.allof-inline-residuedocuments this, and no issue tracks lowering related primitives to the narrower kind. That is exactly what this relation computes, which is one more argument for making it a reusable meet rather than a merge-private helper. - A subtype that redeclares a
$refbase's property is never judged:integerover a base'sstringis as silent as the narrowinguuid. That is #812's override rule; see the reply on theNarrowsthread. - The
ConflictingRedecldoc indiag.gosays an incompatible target type is unsatisfiable. None of the 61 numeric pairs that still warn is.
Interactions with open PRs
- #998 flattens nested inline
allOfinto the samefillModelProperties→MergePropertypath, so narrowing will reach nested branches. Whichever lands second should add a nested narrowing case. - #1006 also edits
testdata/unwitnessed.golden.txt. Whichever lands second re-runsgo test ./cmd/morphic-census -update.
Add PrimKind.DomainParents and PrimKind.StrictlyWithin: a strict partial order over primitive kinds by value containment. The edges are TypeSpec's declared scalar hierarchy plus the exact containments between widths that hold of every value; any is the top by rule. Containments that hold only by float rounding are deliberately not edges. ir-design 4.1 states the relation, its TypeSpec basis, the float decision, and that string-spelled kinds are not within string in this domain. Tests hold the edge table in both directions, the strict-order laws over every declared kind, the roots to the declared set, and the longest chain to the walk's cap. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
StrictlyWithin is read over finite values: NaN and the infinities are held only by the IEEE-754 kinds and by no JSON instance. Every finite binary float is an exact finite decimal, so float lies within decimal and decimal is the only kind directly under number. DomainParents changes in two places: integer drops decimal and keeps float, and float moves from number to decimal. The longest chain is seven edges, within maxDomainDepth. The chain test now fails at the first edge past the cap, and ir-design 4.1 states the finite-value reading. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Move formatTable out of schema into a spelling package of its own, with an exported Lookup, and relate two primitive kinds there: Relate classifies a pair as same, either one narrower, overlapping or disjoint. The value-domain half is ir.PrimKind.StrictlyWithin; the spelling half is that a string format's kind refines string, derived from the table's string rows. Numeric rows are not derived, so each must already narrow its bare row by domain. merge drops its private chain (primParent, maxNarrowDepth and the exported Narrows) and reads Relate. Containments the chain omitted now narrow, such as uint8 within int16. The datetime-to-string edge goes, since no spelling lowers to datetime. The top type stays unrelated in bareNarrowing, so no null result changes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Four comments and one test string said things the code does not: - the hopOnly reason named rows that narrow float; it now says no (type, format) pairing selects the arbitrary-precision binary float. - the spelling rule's comment claimed schema and merge may not reach each other; only merge may not import schema. - typesConflict's doc cited the removed Narrows and called it incomplete; it now cites bareNarrowing through spelling.Relate and says overlaps and Scalar/Enum-reached related kinds still conflict until #782. - Relate's doc gave one ground for disjoint kinds; it now gives both different families and two different string formats. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
The previous comment edit left one line past the paragraph's wrap. This reflows it with no wording change. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
A property redeclared with primitive kinds that share values without either holding the other (int32 and uint32, float32 and integer, int64 and float64) is no unsatisfiable type. It is now an info degraded-construct that keeps the first declaration's kind and the redeclaration verbatim under Unmodeled. Disjoint kinds, and a scalar against a structure, keep the conflicting-redeclaration warning. typesConflict now classifies a drop as conflict, overlap or silent, and is consulted only when the fold dropped src's type, so a narrowing never reaches it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A pair with an Enum side drew the overlap info, because spelling.Relate sees only the resolved kind. Kind alone cannot say whether an enum's members meet the other side: enum [a] against uuid, or enum [3000000000] against int32, have no value in common, and the info claimed they did. typesConflict now returns the kind comparison when either side is an Enum (resolved through the Base chain, so a Scalar over an enum counts): differing kinds are a conflicting-redeclaration warning, equal kinds are silent. These pairs keep the warning: enum of string against uuid, enum of integer against int32, and any enum against a differently-kinded type. Pairs without an Enum side are unchanged. The overlap info now says only what was checked: the kinds share values but the merge does not narrow them to one. The allof-redeclared-property rule and the ConflictingRedecl doc say the same. Tests cover the enum pairs at the merge and schema levels in both orders, isEnum's chain walk, the diagnostic's pointer and its being the only one at the property, and a narrowing pair that reaches the classifier. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The three statements of the redeclaration verdict (the verdictConflict and verdictOverlap docs, the ConflictingRedecl doc and the allof-redeclared-property rule) were false for an enum pair, which keeps the kind comparison. The typesConflict doc also had a 114-column line, and one test comment and message described only order 1. No behaviour changes. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
The verdictSilent doc and the conflictIf doc stated the verdict without the enum case, and the previous commit left them. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
A narrowing put one declaration's default and examples on the other declaration's type, and nothing asked whether that type admits them: a fractional default landed on an integer, an out-of-range one on int32, and a non-UUID example on a uuid, with no diagnostic in either order. admits judges a value against a primitive kind exactly: integer widths by range through a bounded decimal reader, so an exponent no machine type holds is refused rather than built; floats by finiteness; decimal128 by digit budget and exponent range; and each string format by its grammar. A kind with no judge admits nothing, and a null is judged against the merged field's nullability. When the broader side holds a value the winner refuses, that value is not adopted; one info names the keyword at the broad declaration and the declaration is kept verbatim under Unmodeled at its own key. Broad first, the declaration is dst's, so a per-model PropIndex now records each wire name's first Source beside its position. Both orders give the same field, diagnostics and Unmodeled. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A refused default or example was kept at the first declaration, but the merged field adopts a default or examples from any later declaration it lacked, so the info named a declaration that never wrote the value and the declaration that did was kept nowhere. PropIndex now records, per wire name, the declaration each of the default and the examples came from, and moves it when a reconcile leaves the field holding a value it did not hold before. Broad first, each refused keyword is reported and kept at its own origin, one info per declaration; the orders agree. The url, time and datetime_offset judges follow their RFC 3986 and RFC 3339 grammars: a byte scan before url.Parse, and the fraction, offset and separator shapes before time.Parse, so a comma fraction, a +24:00 offset, a space or a non-ASCII byte is refused and a lower-case t or z is adopted. Also: float32 and float64 edge rows in the admits table, NewPropIndex takes no argument, ErrNoSource is unexported, DecimalReading.Negative is dropped, and two over-wide comment lines are re-wrapped. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
A property's origins held one declaration per keyword, so when a later declaration wrote a value equal to the one the merged field already held, only the first writer was reported and kept when the broad side was the accumulated field. Judged narrow-first, each broad declaration stood on its own and both were kept, so the two orders differed. origins now lists every declaration that wrote the value the field holds: a value newly held replaces the list with its writer, an equal value appends the later writer, anything else leaves it. judgeAdopted reports and keeps each listed declaration. PropIndex holds a property's position and origins in one entry, so the guard for a position without origins, reachable only from a test, is gone with its test. The url judge now refuses a bracket outside the authority and a second fragment delimiter, which RFC 3986 does not allow; uriBytesValid hands the tail after the scheme to uriTailValid. Tests pin A, B and C (two equal writers and a narrowing integer) in all six orders for default and examples, and the new url rows. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
A fold conjoins null admission, so a non-nullable declaration takes null
away from a nullable one that wrote `default: null` or a null example.
judgeAdopted judged only the broader side of a narrowing, so a null on
the narrower side landed on a non-nullable field with no diagnostic.
It now judges null on both declarations wherever the conjunction
applies: the broad side keeps its kind judgment, the narrow side and
both sides of a same-target fold are judged by null alone. A refused
value is reported once and kept verbatim at its writer's pointer, in
every declaration order.
This changes the same-target case, which main left silent: a field
declared `{type: [integer, "null"], default: null}` and `{type:
integer}` now drops the default with an info and keeps the nullable
declaration under Unmodeled, where main kept a null default on a
non-nullable integer.
The refusal info no longer names the narrowed kind, which depended on
which fold refused first and differed by declaration order.
The url judge now places brackets only around the whole host: `]` in a
reg-name host (http://a]b/, http://x]:80/, http://u@x]/) is refused,
as RFC 3986 requires. An IPvFuture host is refused too, since
url.Parse refuses it; a refused value is kept verbatim.
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
A same-target fold judged a null against the merged type's nullable bit alone, so two declarations over the top type, or two $refs to an alias of it, lost default: null or examples: [null] to Unmodeled with an info saying the type does not admit them. The top type admits every instance, and main kept the value. judgeAdopted now counts the top type as admitting null, through isAnyType, which follows the Base chain. The url judge admitted a second "@" in the authority (http://a@b@x/), which RFC 3986 refuses, so such a default landed silently on a url field where main warned. uriTailValid now allows one "@" there. Tests pin a refused null example, the top type keeping a null in both orders, and the $ref case only the Base walk reaches; permutations is iterative, and two docs no longer say only the narrower type refuses. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
OmarAlJarrah
left a comment
There was a problem hiding this comment.
A third pass, at 2673a03b. I checked every thread from the last round against this head, probing each table in both branch orders. All nine are addressed, and I've resolved them. The rework goes further than they asked: one judgment over TypeRef, a value judge, the domain half in ir, formatTable beside the spelling relation, and the deferral.
The branch can't merge as it stands. Main has moved eight PRs past 17ad2ea1, and 13 files conflict (36 hunks). Two of those PRs changed the design this one builds on, so the merge isn't bookkeeping:
- #1034 closed #898 another way. Main's
Mergerreads no registry: a redeclaration of another type keeps the first declaration and hands the report toLateras amerge.Conflict, whichschema.redeclarationsettles once the registry is whole. This PR's fold reads nodes to decide which type wins, so it has to bring registry reads back, and absorbLateror replace it, so that there's one deferral, not two. compile.Typeshas no publicSettleorViolationsany more.Finishbuilds every promised node, then takes the settlements once, and aDeferafter that is refused. TheSettlethis PR adds between the component walk and the operations has no counterpart, and moving it across as is would be wrong (inline onopenapi.go).- Main keys multipart parts in a settlement (
partKeys), because the body model can be an unbuilt promise while the operations lower. The flags this PR reads off the merged property have to move there (inline oncontent.go).
I haven't resolved the merge, since how the two deferrals combine is a design choice. Everything below ran on this head against its own base 17ad2ea1, and against main @26e07617 where noted.
What holds at this head:
- CI is green.
morphic-harnessovertestdata/differs from the base only by the two new fixtures, which pass. - Pair sweep. 39 property spellings, adding constrained and named components, enums, consts, lists, maps, defaults and nullables to last round's set. That makes 741 pairs, each in both branch orders, with the components declared before and after the composing schemas.
- Silently order-dependent pairs: 147 on main, 4 here. The count is the same whichever side the components are declared on; on the base it went from 147 to 293.
- Of the four, three are a model against a map or a model, which the rule documents as unrelated drops. The fourth is new (inline on
typejudge.go). - No pair is reported in one order and not the other.
- Last round's tables all come out as asked:
- inadmissible values are refused and kept, in both orders;
- the enum, const, top-type, list, map and
hexpairs are order-invariant; uint8narrowsint16;- overlaps are infos.
- Mutations. I planted fourteen beyond the test plan's list, and thirteen go red.
- In the value judge: the integer and float limits, decimal128's lowest exponent, the offset hour, the duration designators and a second
@in an authority. - In the judgment: the container null check,
hoistedAt's exact-pointer case, a literal against a kind with no judge, the top type's null, number identity,admitsEmptyand the map key. - The survivor groups refusals by pointer alone. It looks equivalent, since a writer
dstalready holds never shares a pointer withsrc.
- In the value judge: the integer and float limits, decimal128's lowest exponent, the offset hour, the duration designators and a second
What I'd change:
- Merge main, with the design points above. This is blocking.
- A value is judged against the winner's kind, not its constraints. So a constrained node that wins adopts a default it refuses, with no diagnostic. That's a regression against main (inline on
typejudge.go). - Two equal containers, one written inline and one a component, are judged the same and the first is kept. The named type wins in only one order, silently (inline on
typejudge.go). - Nit: a stale "Transitional" comment in
schema_test.go(inline). - PR body: #1034 has closed #898.
Closes #898, the Summary's #898 sentence and the reviewer note about #898 describe main as it was. The Cost section's baseline predates eight PRs.
Main redesigned the code this branch builds on: #1034 fixed #898 by making the merge read no registry and deferring only the conflict report (Later), #1038 replaced compile.Types.Settle with Finish, which builds every promised node and then settles once, and #1065 added nullability markers to TypeRef. The resolution keeps one deferral path and main's single settle: - merge: the judgment reads the registry again, as narrowing must, and a fold whose judgment would read a node the walk has not built is deferred whole (foldOrDefer) and settled in Finish, after every promise is built. That replaces Later, Conflict, Nodes and the redeclaration settlement, which deferred only the report. foldTypeTargets intersects nullability through main's intersectNull, so the Inferred markers follow the bit. - compile: main's Finish is kept as it is. The branch's repeated Settle, the untaken-settlement violation and their tests are dropped, and so is the extra Settle after the component walk in openapi.run. - operation: the multipart flags read the merged property in partKeys.Settle, beside the key, since the body model may be a promise no lowering has built when the operation lowers. Every part goes to the settlement, which keeps those its encoding entry or its settled flags leave carrying something. - schema: fillModelProperties takes the PropIndex and main's requireMembers; settledSpec lowers through main's lowerSpec, which finishes the registry. - A null property entry is the top type on main, which loses to any other type with nothing to keep, so the test that kept it verbatim now pairs it with a degraded node, and a twin case pins it against a string. - The new conformance case is listed under intersection in matrix.md. Not carried: the settlements still run in the order deferred, so a fold deferred while Finish builds a promised form body runs after that body's part keys. The next commit orders them.
A form body that a $ref into another document names is built only in Finish, after its operation queued the body's part keys. When one of its parts is redeclared over a node built later still, the fold waits too, and Finish took the settlements in the order deferred: the part keys first, so the flags read the first declaration's type. A string part narrowed to binary came out with no Filename and was dropped from the encoding map. compile.Types gains DeferReader for a settlement that reads nodes and rewrites none. Finish takes those after every settlement Defer queued, so a reader sees each node as the settlements that rewrite it leave it, whichever was deferred first. partKeys is deferred that way.
deferredFold.settle skipped a model the registry no longer held under its ID, for the rebuild a $ref used to make of a model its declaration built again. Main no longer rebuilds: a reference promises its target and the owner builds it once. Compiling 2,234 specs, testdata and a real-world corpus, with the check turned into a panic never reached it; only its own unit test did. So a replaced model is now a compiler bug, reported as an openapi/internal-invariant error at the first redeclaration deferred, rather than a silent skip that would hide one.
A narrowing adopts the broader declaration's default and examples only where the winning type admits them, but the value judge read only the winner's kind. A constrained node that won adopted a value its own constraints refuse, silently and in both orders: `default: hello` beside `$ref: Short` (`maxLength: 3`) gave Short with that default, and a four-item default beside a list with `maxItems: 2` kept all four. Main never paired them. A Scalar, list or map now holds a value to its Constraints as well: each numeric bound and multipleOf, exactly (annotation.IsMultipleOf, past a bounded scale it claims nothing), the length in code points, item and property counts, and uniqueItems by JSON equality. A keyword with no judge, pattern, precision or scale, refuses every value it applies to, so a constrained node admits nothing it cannot be shown to fit. Constraints on the property rather than the node stay unjudged; the rule's Status leaves them to #10.
Two distinct lists or maps alike in every member and detail were judged the same type, so the merge kept whichever was declared first. An inline list against an equal component came out the anonymous list in one branch order and the component in the other, with no diagnostic in either: an emitter rendered a named type or an anonymous one depending on branch order. The enum, const and Scalar arm already settles two equal nodes by where they were declared (sameNodes): two inline copies are one type, a node declared elsewhere wins over an inline copy of itself in either order, and two declared apart overlap. Two alike containers now take that rule too, so the component wins in both orders, and two alike components are reported as an overlap rather than folded silently.
TestAllOf_CompatibleConstraintRedeclarationsStaySilent asserted only that
no conflict was reported, and a comment called its unknown-format row
transitional until the bare-against-Scalar narrowing landed. It has landed:
`{type: string, format: weird}` against `{type: string}` gives the Scalar in
either order with no diagnostic at all. Every row now asserts that no
diagnostic is drawn, as the test's name says, and the stale comment goes.
micro-compiler-design §6.2 said every decision reading another lowering's node is a settlement. The allOf merge now judges a redeclaration mid-walk where every node it reads is built and defers the whole fold where one is not, and a settlement reading what another rewrites is deferred as a reader, which Finish takes after the rest.
The exhaustive linter holds a switch over ir.ValueKind to every member, default or not. admitsConstraints and canonicalValue each left some to their default; both now name every kind, and the default refuses one ir has not declared, which a test row reaches.
The copy rule calls a hoisted node an inline copy where it was written at or under the field's declaration pointer, and compared pointers only. Since #1009 another document's nodes are hoisted at pointers of their own, so a node another document writes inline at the very pointer the root writes its copy at was taken for that copy. A field redeclared as a $ref to such an enum and as the same enum inline settled on whichever branch came first, with no diagnostic in either order. The declaration the rule reads (origins.typeAt, typeJudge.written) is now a Provenance, and hoistedAt requires the node's source to be the declaration's. Two tests compile across two files: the root's case in both branch orders, and models of the other document whose inline copies lose to its component, one after a narrowing has moved the declaration read.
Finish settled readers last: compile.Types.DeferReader queued a settlement that reads nodes and rewrites none after every one Defer queued, so a form body's part keys read a field after the allOf fold that rewrites it. That orders only the settlements marked as readers. One that both reads a node and rewrites another, as the discriminator settlement reads a model and may swap it for its map, still meets the field folded or not by the order the two were deferred in. The fold is the odd one out, not the reader: it finishes a node its walk left open. compile.Types.Complete now queues such a settlement, and Finish takes every completion before every settlement Defer queued, each kind in the order queued. Whatever a deferred settlement reads, or rewrites, it meets finished. DeferReader is gone, the merge's deferred folds are completions, and the part keys go back to Defer. TestTypes_SettleTakesEveryCompletionFirst replaces the reader test, Complete is refused after settling and when nil, as Defer is, and TestExternalContent_APartFlagReadsTheFoldedType keeps the end-to-end case under its new name. Running the deferred settlements before the completions reddens both, and so does queuing the folds with Defer.
A field redeclared under allOf was folded pair by pair in declaration order: the second declaration into the first, the third into what that left. Keeping the first of two overlapping types is a reported tie on its own, but in a chain it decided what the later folds saw. int32, uint32 and uint16 kept int32 over uint32, reporting the overlap and keeping uint32 verbatim, then narrowed to uint16 anyway, so the report was false; uint16, int32, uint32 drew nothing. A default the field refuses, written first, beat a different one it takes, written later: number with default 1.5, number with default 2 and integer gave no default in one order and 2 in another. The fold now reads every declaration of the field at once, once every type has lowered: - The field takes the type of the declaration whose type is the same as or narrower than the most declarations' types, the first on a tie. A type within every other is the intersection and wins in every order. - Each declaration that type holds folds in: null only where every one of them admits it, and their constraint keywords, the first to set one keeping it. - A default or examples the field does not take is refused before one is kept, so a value the field takes is never lost to a refused one written before it; of the rest the first written wins. - Docs, deprecation and XML hints come from every declaration, the first to give one keeping it; required and secret where any says so; visibility where every one admits it. - Every other declaration is dropped, kept verbatim and reported: a conflict, an overlap, or, where the judgment cannot compare the two (two models, a model and a map), an info saying so. That last drop was silent, so which type the field kept followed branch order unreported. Each diagnostic names the declaration whose say the field kept, and a constraint conflict the one that wrote the kept value. They used to name the first declaration whatever wrote it. Every redeclared field folds this way, so the walk reads no registry: the mid-walk judgment, misses() and its trial run, the per-property deferral and the origins and typeAt bookkeeping are gone. So is the check that reported a fold of a model no longer registered (dcbedf7). A model's folds are completions, which Finish takes after building every promised node and before any settlement that could swap one, so the model is the registered node by construction. On the 741-pair sweep, the pairs whose result depends on branch order with no diagnostic go from 3 to 0, the three model and map drops now being reported. Over 3,654 three-declaration triples, each in all six orders, 1,800 come out the same in every order (263 before, 4 on main), and none depends on order without a diagnostic (15 before, 685 on main). The Cloudflare schema compiles to the same registry and the same diagnostics, in the same order.
The code's doc listed a fold the merge deferred finding another node registered for its model. The set fold removed that check: a model's folds are completions, taken before any settlement could swap the model, so the case cannot arise and nothing reports it.
The set fold judges every pair of a field's declarations, so its cost is the square of their number, and so is the relation matrix it keeps. Only the source node budget bounds that number: a document of a hundred thousand inline allOf branches each redeclaring one field would have asked for five billion judgments. The pairwise chain before it was linear. A fold now judges a field's first 64 declarations. Each later one is kept verbatim beside the field, unjudged, under one openapi/budget-exceeded error at the first of them, which is how the compiler's other fixed bounds report. 64 is far past any real use: across 1,840 real-world OpenAPI 3 specs only seven fields fold, with three declarations at most, and the Cloudflare schema folds none. The hundred-thousand-branch document now compiles in about the time main takes (7.1 s against 6.8 s). TestFold_DeclarationsPastTheBoundAreKeptUnjudged pins the bound, the report and the keeping; lifting the bound, or keeping nothing past it, reddens it.
Main brought the allOf set fold (#1000, #1084), Model.Requires (#1066), the false-schema reader (#1065) and the threaded bindings (#1016). The branch's flattening now runs on each of them: - lowerAllOf and buildComposedVariant pass the flattened composition to fillAllOf, compositionRequired and applyFalseBranches, and fold into main's PropIndex. A required name from a flattened entry goes through requireMembers, so a name no property declares lands in Model.Requires; the branch's UnattachableRequired path is gone. - applyFalseBranches reads each flattened entry with annotation.IsFalseSchema and keys it by the entry's own pointer. - decoratedBranchHint selects its base from the flattened list, so a $ref wrapped in two allOf levels names the variant after the ref. - The multipart part walk carries the bindings in partVisits and builds main's schemas-based bodyPart; main's depth-bounded recursion gives way to the branch's ref-hop bound and cut warning. - allof-nested-inline joins the intersection witnesses in matrix.md and the conformance table; docs/compilers/openapi.md takes both sides' test lists.

Summary
When inline
allOfbranches redeclared a property with different types, the merge always kept the first declaration's type. Two different primitive kinds drew anopenapi/conflicting-redeclarationwarning even where one narrows the other, so a pair that loses nothing (stringagainststring+uri) was reported as a conflict and parked underUnmodeled. Most pairs that were not two primitives were dropped with no diagnostic: an enum against its own kind, aconstor{}against any kind, two lists or two maps. Either way the field's type followed branch order.One judgment compares two declarations' types and returns one verdict, whichever is written first: the same, one narrower, an overlap (the two share values but neither narrows the other), disjoint (no value but a shared null satisfies both), or unrelated (a pair the judgment has no arm for, such as two models).
ir(PrimKind.StrictlyWithin,docs/ir-design.md§4.1). Their OpenAPI spelling is in the newcompilers/openapi/internal/spellingpackage, which now holdsformatTable. The relation is stated over JSON instances and takes exact containments only, so a containment that holds only by rounding (int32withinfloat32) is an overlap.Scalar,Enumorconstagainst a bare primitive, and the top type. An enum is judged by its members, as aconstis by its value. An enum all of whose members the other kind refuses is disjoint.consts,Scalars, lists or maps alike in everything but where they were declared are one type where both were written inline at the field's own declarations, in their own document. A node declared elsewhere (a component, or a schema a$refreaches in this document or another) wins over an inline copy of itself in either order. Two such nodes declared alike are two types the source declared apart, an overlap.The fold reads a field's declarations as one set, once every type has lowered:
defaultorexamplesthe field does not take is refused, reported at each declaration that wrote it, and that declaration is kept verbatim. The field takes a value by the winning type's kind and by the constraints its node carries: each bound,multipleOf, lengths in code points, item and property counts, anduniqueItems. Apattern, which nothing here evaluates, admits no string. A null is refused wherever the field does not admit null; the top type admits it. Of the values the field takes, the first written is kept, and a declaration giving another is reported and kept. Values are refused before one is kept, so a value the field takes is never lost to a refused one written before it.openapi/budget-exceedederror.Folding the set rather than a chain of pairs is what makes the result order-free. Folded pair by pair,
int32,uint32,uint16reported an overlap and keptuint32verbatim although the field becameuint16, whileuint16,int32,uint32reported nothing.When the fold runs. The judgment reads the nodes both declarations name, and the walk may not have built them yet: a component declared after the composing schema, or a node a
$refinto another document promises. So nothing is folded mid-walk. A model's folds are onecompile.Settlement, queued with a newcompile.Types.Complete.Finishtakes completions after building every promised node and before every settlementDeferqueued, so whatever a settlement reads or rewrites, it meets the merged fields. #1034 settled #898 by deferring the conflict report alone (Merger.Later). Here the field's type itself needs the registry, so the whole fold waits and the report goes with it: one deferral path.Multipart flags. A part's
MultiandFilenameare read off the type its field folded to, in thepartKeyssettlement beside the part's key, so they agree with the field in either branch order. Where that type does not decide them (a union, enum, literal, model or map), or the body has no model, they come from every declaration of the part's name.The rules are
allof-redeclared-propertyandmultipart-encodingindocs/compilers/openapi.md; the primitive domains are §4.1 ofdocs/ir-design.md.Closes #598
Closes #782
Closes #446
Notes for the reviewer
00533c60mergesmain. The fold that judged a pair mid-walk when it could and deferred it otherwise (with a trial run to tell which), theDeferReaderordering (64200fa4) and the replaced-model check (dcbedf73) are all gone. Every redeclared field folds as a completion, the walk reads no registry, and a model's folds run before any settlement could swap the model. Each commit body says what it replaces.main@aee2ca6d, the pairwise fold @c65bcecb and this head:morphic-harnesspasses all of its oracles, order-invariance included, on the three generated specs.compose_test.gofixture. fix(compilers/openapi): keep format: password at every position #592 addedTestAllOf_RedeclarationOrsSecretand narrowed nothing. The intent is met byTestAllOf_ReconcileAccumulatesRicherDetailForAnUnknownFormat.Out of scope
hexagainstbyte, an enum whose members are a subset of another's, two different literals) are reported as an overlap and the first declared is kept, as is a list or map whose broader side carries a description, constraint, example or extension the narrower lacks. Tracked in openapi: an allOf narrowing between two enums or two lists is resolved by branch order #1042.int32againstfloat32) are overlaps, as §4.1 decides.minimum: 300adopted onto auint8lands silently, as the same schema written in one piece does onmain. Both belong to openapi: intersect constraints and defaults when reconciling allOf branches #10.allOfwhose branches are all primitives lowers to an emptyModel. Tracked in openapi: an allOf of primitive branches lowers to an empty model #1043.Test plan
The issues' examples. Each row of typesConflict reports a format-narrowed primitive as conflicting with its bare primitive #446's and openapi: an allOf that narrows a property's type is reported as a conflict and resolved by branch order #598's tables, and openapi: an allOf redeclaration of a property with an unknown format is resolved by branch order #782's example, compiled in both orders: the narrower type, with no diagnostic and no
Unmodeledentry.The judgment.
TestTypeJudge_RelateArmsandTestTypeJudge_AdmitsPerNodecover each arm;TestAdmits_JudgesEachKindExactly,TestAdmitsConstraints_JudgesEachKeywordandannotation.TestIsMultipleOf_JudgesExactlythe value judge;TestNarrows_*,TestRelate_ClassifiesEveryVerdict,TestPrimKind_*andTestFormatTable_*the relation and the domains it rests on.The set fold.
TestFold_TheSetDecidesWhateverTheOrderfolds each set in every order and requires one outcome: a type within every other, the type that loses the fewest, and a value the field takes from any declaration.TestFold_ATieGoesToTheFirstAndIsReported,TestFold_ADroppedFirstDeclarationIsKept,TestFold_AConstraintConflictNamesTheDeclarationKeptFrom,TestFold_ADroppedDeclarationStillSaysWhatIsNotShape,TestFold_UnmodeledUnionsLaterWinning,TestFold_ANarrowingCarriesTheNullMarkersandTestFold_DeclarationsPastTheBoundAreKeptUnjudged.TestFold_*, formerlyTestReconcileProperty_*) and the schema cases (TestAllOf_*), in both orders: narrowing, overlap, disjoint and enum pairs, copies, inadmissible values by kind and by constraint, null and the top type, lists, the docs fold, and drops the merge does not compare.Order and other documents.
TestPropIndex_ARedeclarationFoldsOnceTheRegistryIsWhole,TestAllOf_ARedeclarationIsJudgedOnceTheComponentsHaveLowered,testdata/openapi/allof_redeclared_ref_declared_later.yamlin the harness sweep, andTestExternalContent_ANodeAnotherDocumentHoistsIsNoCopyOfAnInlineOneandTestExternalContent_ACopyInAnotherDocumentIsJudgedThereacross two files.Settlements.
TestTypes_SettleTakesEveryCompletionFirst, and the completion rows ofTestTypes_DeferRefusesWhatSettleWouldNeverTake.Multipart flags.
TestPartEncoding_FlagsComeFromTheMergedTypein both branch orders,TestContent_MultipartRedeclaredPartFlagsComeFromTheSettledTypein both component orders, the otherTestPartEncoding_Flags*cases for the fallback, andTestExternalContent_APartFlagReadsTheFoldedTypefor a form body another document declares.Conformance.
allof-conflicting-typeheads with a pair that cannot both hold (uriagainstinteger); the judged pairs are in the newallof-narrowing-redeclaration, witnessingintersectioninmatrix.md. Their goldens changed in diagnostic wording only.morphic-harnessovertestdata/differs frommainonly by those two fixtures, which pass.Mutations. Each was planted at this head, run against
merge,schema,operationandcompilers/openapi(andcompilefor the settlement order), and reverted. Each turned the named tests red:TestTypes_SettleTakesEveryCompletionFirst,TestExternalContent_APartFlagReadsTheFoldedType; the folds queued withDefer→ the latter.TestFold_ATieGoesToTheFirstAndIsReported; the first declaration always winning, or the field's target read off it →TestFold_TheSetDecidesWhateverTheOrder; the cone leaving out declarations of the same type →TestAllOf_IdenticalNodesAreOneType,TestFold_AConstraintDisagreementIsReported; a dropped declaration not kept →TestFold_ADroppedTypeIsKeptEvenWhenItDoesNotConflict.TestFold_ANarrowingIntersectsNullability; a top type's bit counted under another type →TestFold_ANarrowingCarriesTheNullMarkers,TestAllOf_TheTopTypeAndANullableKindMeetAsNullableInEitherOrder.TestFold_TheSetDecidesWhateverTheOrder; a declaration's own value judged against its own type →TestAllOf_AnUnknownFormatWinsInEitherOrder; a dropped declaration's values adopted →TestFold_AConflictingTypeTakesNothingWithIt; a refused writer not kept →TestAllOf_AnInadmissibleBroadValueIsKeptInEitherOrder.TestFold_AConflictingTypeTakesNothingWithIt; the kept value credited to a later agreeing declaration →TestFold_AConstraintConflictNamesTheDeclarationKeptFrom; a disagreement not kept →TestKeepLosingDeclaration_AgreeingDeclarationsKeepNothing.TestFold_ADroppedDeclarationStillSaysWhatIsNotShape; Unmodeled not unioned →TestFold_UnmodeledUnionsLaterWinning; the disjoint-visibility warning dropped →TestFold_DisjointVisibilityIsARestrictionNotAConflict; a differing detail not kept →TestFold_EveryDocsDisagreementIsReported.TestPropIndex_ARedeclarationFoldsOnceTheRegistryIsWhole; the first declaration read as recorded rather than as the walk left it, losingrequired→TestMergeProperty_AppendsThenFolds; the bound lifted, or nothing kept past it →TestFold_DeclarationsPastTheBoundAreKeptUnjudged.TestAllOf_ACopyANarrowingKeptIsStillACopy,TestExternalContent_ACopyInAnotherDocumentIsJudgedThere.One plant stays green: merging constraints into the first declaration's own
Constraintsrather than a copy. It is equivalent today, since each declaration's constraints are built for it alone; the copy keeps the fold from writing to a node should that change.Earlier plants, at
c65bcecb, cover code this round leaves as it was: the copy rule's container arm and document check (TestAllOf_IdenticalNodesAreOneType,TestTypeJudge_RelateArms, the twoTestExternalContent_*Copy*tests), the multipart flags (TestPartEncoding_ADecidedTypeNeverFallsBack,TestPartEncoding_FlagsOfAnUndecidedMergedTypeComeFromEveryDeclaration), and each arm and keyword of the constraint judge, 17 plants (TestAdmitsConstraints_JudgesEachKeyword,TestIsMultipleOf_JudgesExactly,TestTypeJudge_AdmitsPerNode,TestAllOf_AWinnersConstraintsRefuseADefaultInEitherOrder).Locally at this head:
go test ./...;golangci-lintat the version theMakefilepins;make veton a fresh build cache; every statement covered incompile,merge,schema,operation,diagandcompilers/openapi. The full gate is left to CI.Cost
Instructions retired,
GOGC=off GOMAXPROCS=1, minimum of four runs,main@aee2ca6d against this head:cloudflare/api-schemasopenapi.json@536d02b4): 132.22G against 132.20G. It redeclares no field, and the IR and all 1,847 diagnostics are identical, in the same order.allOfof two inline branches redeclaringpasnumber/integerandqasstring/uuid): 16.63G against 15.82G, −4.9%; the narrowings no longer draw warnings.$refto a component declared after the composing schema): 21.33G against 20.52G, −3.8%.allOfs whose branches agree): 37.79G against 37.86G, +0.18%, one completion per redeclared model.main, 7.1 s here, the fold judging the first 64.Merging
The repository's squash default pastes every branch commit's body. Set the squash body by hand from this description's Summary.