Skip to content

fix: compare the schema behind a changed $ref - #668

Open
josephaxisa wants to merge 1 commit into
pb33f:mainfrom
josephaxisa:fix/663-ref-change-compares-content
Open

josephaxisa wants to merge 1 commit into
pb33f:mainfrom
josephaxisa:fix/663-ref-change-compares-content

Conversation

@josephaxisa

@josephaxisa josephaxisa commented Oct 22, 2026 •

Copy link
Copy Markdown

Fixes #663.

When a $ref was replaced by an inline schema, an inline schema by a $ref, or a $ref pointed at another schema, CompareSchemas recorded the non-breaking $ref change and returned. The new schema was never compared, so the repro in #663 ($ref: Pet becoming an inline schema with required: [name]) reported 1 non-breaking change, while the same edit between two inline schemas is breaking.

The $ref change is now kept, and the two schemas are then compared like any other two schemas, as e935b3e did. Comparison stops at the $ref change (keeping it rather than returning nil) when:

  • either side doesn't resolve (a missing target, or an external one with SkipExternalRefResolution). Otherwise everything would be reported as removed.
  • either side is on a reference loop, or the document wasn't checked for circular references (SkipCircularReferenceCheck), since a loop can't be recognised there. Rolodex gets a small HasCheckedForCircularReferences() getter for this.
  • a 3.1 $ref with sibling keywords is involved. Its internal allOf form doesn't line up with the referenced schema field by field. That case needs its own fix.

Three helpers that recognise equivalent forms now look through a reference, so a change made through a $ref is reported like the same change made inline:

Existing tests that asserted a single change now also assert the change behind the $ref, e.g. TestCompareSchemas_PropertyRefChange_Fail (int → string, breaking). In TestCheckExplodedFileCheck, both references change and c-alt.yaml has a new description, so the test now expects 3 non-breaking changes. The three CheckRefChangeCircular tests keep the $ref change instead of expecting nil.

New tests: the #663 repro both ways and checked against the inline equivalent, a retargeted $ref, an identical inline copy (still no change), the simple allOf view, the nullable wrap, siblings and circular targets stopping at the $ref, unresolved targets, a skipped circular check, and the equivalent scalar union. The touched functions are at 100% coverage. The full suite and openapi-changes' tests pass.

Things worth knowing:

  • Following a changed $ref costs what following an identical external $ref already costs: a deep graph of shared schemas is compared once per path. I checked a 25-level diamond where every level is renamed. Memoizing per schema pair would make the comparison cheap, but the report would still list the change once per path, so I left that for a separate discussion. On real Stripe history the time and memory are unchanged. Loaded the way openapi-changes loads it, and with $ref changes are dropped when a document has any circular reference #662's fix, the 2024-01...2026-09 diff gains 117 breaking changes.
  • Changes found behind a $ref point at the target schema's lines, as they do when an external $ref is followed today.
  • oneOf/anyOf pairing scores ref-vs-ref candidates on their content now, instead of one $ref change each.

Related: #662. In documents with any circular reference, this only takes effect with that fix.

When a schema $ref was replaced by an inline schema, an inline schema by
a $ref, or a $ref pointed somewhere else, CompareSchemas recorded a
non-breaking $ref change and returned, so changes in the new schema were
never reported. A $ref to a schema with type: int becoming an inline
type: string was a single non-breaking change.

Keep the $ref change, then compare the two schemas as it would compare
two inline schemas. It still stops at the $ref change, now keeping that
change instead of returning nil, when either side doesn't resolve, is on
a reference loop, or comes from a document that was not checked for
circular references (a loop can't be recognised there), and when a 3.1
$ref with sibling keywords is involved, since its internal allOf form
doesn't line up with the referenced schema field by field. Rolodex gains
HasCheckedForCircularReferences for that check.

The helpers that recognise equivalent forms now look through a
reference, so a change made through a $ref is reported like the same
inline change: the object composition handling from pb33f#650 ($ref: Thing
becoming anyOf: [$ref: Thing, type: 'null']), the simple allOf view, and
a type array matching an anyOf of scalar types.

Fixes pb33f#663
@codecov

codecov Bot commented Oct 22, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (e0e2f84) to head (bc53102).

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #668   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          309       309           
  Lines        39178     39197   +19     
=========================================
+ Hits         39178     39197   +19     
Flag Coverage Δ
unittests 100.00% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Breaking changes are missed when a schema $ref is replaced or retargeted

1 participant