[gjtorikian]

Closes #374

What

An inline allOf: [{ $ref: Base }, { … }] merges Base's properties and the inline member's into one model. The contested-title walk stopped at the $ref, though, so no root contested a title that Base.shape and the inline marker.shape give different shapes. As a 2xx response (the issue's allof.json), both unions minted one Circle and one Square, and marker.shape resolved to Base's { radius }/{ side } with a "minted twice" warning. As a default, error or secondary body, a header or a parameter, the inherited shape was silently pointed at the inline member's { diameter }/{ len }. Now the title is contested between the two members wherever such an allOf is merged, each union keeps its own shapes, and no warning is printed. A union the merge drops or flattens into the model's fields is no model, so it contests nothing.

How

Root cause: markContestedTitlesUnderRoot (src/parser/inline-models.ts) returned at every $ref, a $ref member of an allOf the parser merges into one model included, and read the merged allOf's members as written rather than as the model the merge builds.

• Outside the components root, a schema the parser reads as one merged allOf (readInlineAllOf: 'merged') is now walked as that model and nothing else (mergedAllOfProperties). That means each property mergeAllOfMembers settles on, as a claim group of its own:

  • what a $ref member's component contributes, its own allOf chain included;
  • a property two members declare, as the declaration the merge keeps or conjoins (meta beside Base.meta; Greptile review);
  • a property only variants declare, as the union of their declarations.

  A declaration the merge drops (an inline shape after Base's, which types the field) and a union whose variants it flattens are not read (review). Neither are a $ref under a variant or a union component's own variants, since the merge does not read them.

• The components root reads its schemas as written, as before: every component is one of them, and a component's own allOf is built by extractAllOfModel, not by the merge. markContestedTitles gives markContestedTitlesUnderRoot the document's components.schemas (an empty map when it has none) for every other root.

• Each such property is read once per root (mergedRead). Without that guard, a chain of components that each merge the next three times over is exponential: every link tripled the time (0.66 s per document at 11 links). With it, a 14-link chain takes under 1 ms.

• The doc comments of markContestedTitles and markContestedTitlesUnderRoot and the inline-union paragraph of docs/architecture/ir-types.md state the rule.

Tests

Added to test/parser/inline-models.test.ts:

• #267 describe, "contests a title an inline response's $ref and inline allOf members give different shapes (A title clash between an inline response's allOf members is not contested when one member is a $ref, and the inline member's variants are lost #374)": the issue's allof.json. It pins every model name and shape, no warning, and consistent refs.
• #267 describe, it.each "keeps both members' shapes of a title they contest under $label (A title clash between an inline response's allOf members is not contested when one member is a $ref, and the inline member's variants are lost #374)": a 2xx response, a default response, an error response, a field of a default response, a response header, a query parameter, a request body and a webhook payload. In each, Base.shape, the merged model's shape and marker.shape point at their own shapes, with no warning and consistent refs.
• #267 describe, it.each "contests a title under a property both members declare, under $label (A title clash between an inline response's allOf members is not contested when one member is a $ref, and the inline member's variants are lost #374, Greptile)": a 2xx response, a default response and a request body, each with Base.meta.shape beside the inline meta.marker.shape.
• #267 describe, it.each "keeps the bare titles for $label, which is no model (A title clash between an inline response's allOf members is not contested when one member is a $ref, and the inline member's variants are lost #374 review)": a shape the merge drops for Base's, and a union the merge flattens, each in a response and a request body. Circle and Square stay bare, as on the base.
• markContestedTitles describe, it.each over how the merge reaches the component: a $ref member, the component's own allOf chain, an inline member's nested allOf, a nullable $ref member, properties beside a lone $ref member, and a property both members declare. Both unions are marked.
• markContestedTitles describe, it.each over what the merged model does not hold: a plain-reference allOf, a $ref under a variant, a union component, a declaration the merge drops, and a union it flattens. Nothing is marked.
• markContestedTitles describe: a 30-link chain plus a self-merging and two mutually merging components. The walk ends, and the deepest union is still contested.

Before the src/ changes, all 19 positive tests failed for the issue's reason (warnings, wrong shapes, unmarked variants), and the negative guards passed. The 4 review guards fail on the PR's first two commits and pass now. No existing expectation changed.

Stack

Based on issue/373-contested-title-later-variant (#496). Merge that first; GitHub retargets this PR when the base branch is deleted.

Decision / Assumptions

• Marking is symmetric. This follows the issue ("contested between the two members") and the existing rule for a schema object two roots share: a mark from either root stands. So Base's own variants are qualified wherever they appear, and Base.shape becomes (CircleBaseShape | SquareBaseShape). I did not take the alternative of marking only the root's own variants so the component keeps Circle. It renames less, but it makes the contest asymmetric, which the doc and the issue rule out.
• The rule applies wherever an allOf is merged outside the components root, not only to 2xx response bodies (the one path that re-mints inherited properties under the response's name). Default, error and secondary bodies, headers and parameters had the same clash, with silent data loss. Request bodies and webhook payloads kept both shapes through the rename pass (CreateDrawingCircle). They now take the contested names (CircleCreateDrawingRequestMarkerShape), so the response and body paths agree. I did not take the alternative of following $ref members only in response roots: it leaves headers and parameters broken, and the two paths would name the same document differently.
• A merged allOf is read as its model, a pure inline one included (outside the components root). This is the review's suggestion, and every extractor builds such a model from mergeAllOfMembers. A declaration the merge drops, or a union it flattens, no longer contests a title there. The components root keeps its as-written walk, because component allOfs (discriminated variants included) are built by their own readers.
• Following is transitive: a merged allOf among the model's properties is read as its own model. That can contest a title deeper than a given extraction path re-mints. This only qualifies names; it never drops a shape.
• A 2xx response's inherited shape gets its own qualified copy (CircleGetDrawingResponseShape, identical to CircleBaseShape). The response path already re-mints inherited inline models under the response's name (GetDrawingResponseMeta beside BaseMeta).
• Breaking (fix(parser)!): names change only in documents where a merged component's union and another union of the same root give one title two shapes. The IR, warnings included, of all 46 fixture specs, the github-subset example and four WorkOS specs is byte-identical before and after. Those documents hold 32 merged allOfs outside their component schemas, 4 of them through a $ref.

[greptile-apps]

Confidence Score: 5/5

[Medium impact] The PR appears safe to merge; no new blocking issue was found.

Summary

The parser now compares conflicting titles across the properties an inline allOf actually keeps.

• Merged allOf models give differently shaped title variants separate names.

_{Reviews (3) · Last reviewed commit: "fix(parser)!: read a merged allOf as the..." · Reviewed by Greptile}

[MichaelDeBoey]

Thanks, this fixes #374!

The issue's allof.json now gives GetDrawingResponseMarker { shape: (CircleGetDrawingResponseMarkerShape | SquareGetDrawingResponseMarkerShape) } with diameter and len, Base.shape as (CircleBaseShape | SquareBaseShape) and the response's own copies CircleGetDrawingResponseShape/SquareGetDrawingResponseShape, with no warning. OAS 3.0 enum: ['circle'] tags give the same result. A 404 body, a default body, array items, a nested drawing property, a request body and a content query parameter now keep both unions' shapes too. Before, the error, default, items and parameter cases pointed the inherited shape at the { diameter } models. Two $ref members, two $ref members that each declare meta, an inline member's nested allOf: [{ $ref: Base }], and Base also returned as a plain $ref by another operation all work. When both unions have the same shapes, the names stay Circle/Square. shape: { $ref: Shape } on the component is left to the existing cross-root rename (GetDrawingCircle), as before.

1. A property both members declare is still skipped, as Greptile points out in #498 (comment). With Holder: { properties: { meta: { properties: { shape: oneOf [Circle { radius }, Square { side }] } } } } and a 200 allOf: [{ $ref: Holder }, { properties: { meta: { properties: { marker: { properties: { shape: oneOf [Circle { diameter }, Square { len }] } } } } } }], the head still prints the two "minted twice" warnings, and GetDrawingResponseMetaMarker.shape is (Circle | Square) with radius/side. A request body gives the same. The merge combines the two meta declarations into one with origin: null, so inheritedAllOfProperties drops it. I'd expect GetDrawingResponseMetaMarker.shape to point at { diameter }/{ len } models, as it does when both members are $refs. The ir-types.md sentence ("every property the merge inherits through the $ref … is compared") reads as covering this case.

   oagen/src/parser/inline-models.ts

   Lines 1069 to 1075 in cb8efa6

   	function inheritedAllOfProperties(
   	schema: SchemaObject,
   	componentSchemas: Record<string, SchemaObject>,
   	): SchemaObject[] {
   	if (!schema.allOf || readInlineAllOf(schema.allOf, schema, componentSchemas) !== 'merged') return [];
   	return mergeAllOfMembers(schema, componentSchemas).properties.flatMap((p) => (p.origin ? [p.schema] : []));
   	}

2. A union the merge drops still contests the title, so names change where only one shape is emitted. With Base as in the issue and a 200 allOf: [{ $ref: Base }, { properties: { shape: oneOf [Circle { diameter }, Square { len }] } }], Base's shape wins the merge, so the inline shape is never typed. The head still renames Base.shape to (CircleBaseShape | SquareBaseShape) and gives the response CircleGetDrawingResponseShape/SquareGetDrawingResponseShape, all with radius/side. The base gives Circle/Square for both with no warning, and a request body does the same. The inline member is walked through its allOf keyword, and the property it inherits is now walked beside it, so both are compared even though the model keeps only one. I'd expect a merged allOf's unions to be read from what mergeAllOfMembers keeps (its properties), which would also cover the combined meta in 1.

   oagen/src/parser/inline-models.ts

   Lines 1043 to 1051 in cb8efa6

   	// A `$ref` member of an `allOf` merged into one model is no reference:
   	// the model carries what the merge inherits through it beside its inline
   	// members' properties, so those properties are read with this root, each
   	// a group of its own like any property.
   	for (const property of inheritedAllOfProperties(folded, componentSchemas)) {
   	if (inheritedRead.has(property)) continue;
   	inheritedRead.add(property);
   	walk(property);
   	}