flatten: hoisted cycle names come from the merged components - #1245
Merged
reuvenharrison merged 5 commits intoSep 13, 2026
Merged
flatten: hoisted cycle names come from the merged components#1245reuvenharrison merged 5 commits into
reuvenharrison merged 5 commits into
Conversation
Copy link
Copy Markdown
Collaborator
Sorry, something went wrong.
A position counter gave the same logical cycle different names in different revisions: an unrelated cycle earlier in the walk shifted the numbering, the components diff pairs by name, and a diff of two flattened revisions reported a phantom modification of a schema that did not change. The name is now built from the component names the cycle merges (AllOfMerged_NodeA_NodeB), which no unrelated edit can move, so flatten produces the same output standalone and inside diff. The numeric form remains as the fallback for hintless targets and collisions. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QDvUFsg5nu1NBWQffErGDw
Copy link
Copy Markdown
Codecov Report❌ Patch coverage is 78.78788% with 7 lines in your changes missing coverage. Please review.
@@ Coverage Diff @@
## main #1245 +/- ##
==========================================
- Coverage 92.27% 92.25% -0.03%
==========================================
Files 345 345
Lines 14336 14357 +21
==========================================
+ Hits 13229 13245 +16
- Misses 1107 1112 +5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness.
|
Sorry, something went wrong.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QDvUFsg5nu1NBWQffErGDw
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QDvUFsg5nu1NBWQffErGDw
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QDvUFsg5nu1NBWQffErGDw
One letter separated it from componentNamer, which allocates names in the components namespace; this function only derives the hint the namer consumes, and its new name says so at every call site. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QDvUFsg5nu1NBWQffErGDw
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.
Implements the principle from review: flatten must produce the same output standalone and inside diff.
The defect the counter had
AllOfMergedN was assigned in document walk order, so an unrelated recursive cycle added earlier in a revision shifted every later number. Base and revision are flattened independently, the components-table diff pairs by name, and oasdiff diff --flatten-allof reported a phantom modification of a logically unchanged cycle (measured: base's AllOfMerged1 = A∧B paired against revision's AllOfMerged1 = C∧D). breaking/changelog were unaffected (checkers compare usage sites by value), but any consumer of the structured diff — or of flattened output committed anywhere — saw names move for no reason.
The fix
The synthetic name is built from the component names the cycle merges, in input order: AllOfMerged_NodeA_NodeB. Nothing outside the allOf itself can move it, so the same logical cycle keeps its name across revisions and the phantom churn disappears (verified: the shifted-revision diff now reports only the genuine additions). The numeric AllOfMergedN remains as the fallback for hintless targets, and name collisions get a numeric suffix, both still in deterministic walk order.
Mechanically: flattenSchemas records a naming hint (the input set's component names) for the result it populates; MergeSpec carries hints across the write-back identity repair; nameAnchoredCycles prefers the hint.
Tests
Full suite and lint pass. The docs in #1244 will be updated to describe the name scheme.