FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

flatten: name the cycles the in-flight guard anchors by reuvenharrison · Pull Request #1240 · oasdiff/oasdiff · GitHub

flatten: name the cycles the in-flight guard anchors - #1240

Merged
reuvenharrison merged 2 commits into
mainfrom
flatten-hoist-anonymous-cycles
Sep 13, 2026
Merged

flatten: name the cycles the in-flight guard anchors#1240
reuvenharrison merged 2 commits into
mainfrom
flatten-hoist-anonymous-cycles

Conversation

Copy link
Copy Markdown
Collaborator

Closes the limitation documented in #1233: an allOf over two distinct recursive components still crashed oasdiff flatten with a stack overflow.

Why this shape resisted #1233

allOf: [$ref NodeA, $ref NodeB], each recursive, merges a child set with two distinct schemas — no lone schema to reuse — so flattenSchemas must run, and its in-flight guard anchors the recursive edge at the result being built. The anchor's value is semantically right: the merged node's recursion is the merged node itself (merge(A,B) recursively equals merge(A,B)). What's wrong is only the representation: the merged node is anonymous, the edge carries no $ref, and a ref-less cycle has no serialized form.

The fix: give the anchor a name

  • The merge records every edge the in-flight guard anchors (state.anchored).
  • MergeSpec then names each target: a target that is already a named component gets that component's $ref; an anonymous target is hoisted into components.schemas under a generated AllOfMergedN name, and the edge becomes a $ref to it.
  • Names are assigned in the document's deterministic walk order — not in the merge's map-iteration order — so identical inputs produce identical output (the Non-deterministic diff --format json output for schemas with multiple reference cycles #1230 lesson applied preemptively).

The fixture now flattens to:

components:
  schemas:
    AllOfMerged1:            # hoisted merged node
      type: object
      properties:
        child:
          $ref: '#/components/schemas/AllOfMerged1'

with the inline occurrence referencing it, and NodeA/NodeB untouched.

Scope note: the naming pass runs in MergeSpec (it needs the components map); the schema-level Merge API is unchanged and can still produce in-memory ref-less cycles for library callers, which the diff's in-flight guard (#1231) handles.

Regression test asserts the hoisted name, the shared identity between the inline node and the component, a successful marshal of the whole document, and byte-identical output across two loads. Full suite and lint pass; breaking --flatten-allof on the shape stays green as before.

An allOf over two distinct recursive components merges into a node
whose recursion the in-flight guard anchors at the result being built:
the value is right (a cycle in the input is a cycle in the merged
output) but the edge carries no $ref, and a ref-less cycle has no
serialized form, so 'oasdiff flatten' overflowed the stack marshaling
it.

The merge now records every edge the guard anchors, and MergeSpec
names their targets: a target that is a named component gets that
name, and an anonymous target is hoisted into components.schemas under
a generated AllOfMergedN name. Names are assigned in the document's
walk order, so identical inputs produce identical output.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QDvUFsg5nu1NBWQffErGDw

codecov-commenter commented Sep 12, 2026
edited
Loading

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.47619% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.26%. Comparing base (b026aab) to head (713c330).

Files with missing lines Patch % Lines
flatten/allof/merge_allof_spec.go 91.66% 3 Missing ⚠️
flatten/allof/merge_allof.go 83.33% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1240      +/-   ##
==========================================
- Coverage   92.26%   92.26%   -0.01%     
==========================================
  Files         345      345              
  Lines       14277    14312      +35     
==========================================
+ Hits        13173    13205      +32     
- Misses       1104     1107       +3     
Flag Coverage Δ
unittests 92.26% <90.47%> (-0.01%) ⬇️

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.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants


Back | FazBrowse Home | New Git URL