Skip to content

Fix inlining of references to external files - #27

Merged
dinwwwh merged 2 commits into
mainfrom
claude/relaxed-keller-9tlncc
Sep 29, 2026
Merged

dinwwwh merged 2 commits into
mainfrom
claude/relaxed-keller-9tlncc

Conversation

@dinwwwh

@dinwwwh dinwwwh commented Sep 28, 2026

Copy link
Copy Markdown
Member

Summary

This PR fixes a bug in the reference inlining logic where references to external files were not being properly inlined when they were themselves targets of internal references.

Key Changes

  • Fixed isInlinable logic in shared.ts: Changed the condition to check if a reference resolves to an inlinable target (record or boolean) AND has a valid alias end, rather than checking the alias end first. This ensures that references to external files (which have no alias chain) are correctly identified as inlinable when they are direct targets.

  • Added test coverage for external file references:

    • Added test in v3.1-to-v3.0.test.ts to verify path items referencing external files are inlined correctly
    • Added test in v3.1-to-v3.0.test.ts to verify schema $defs entries referencing external files are inlined correctly
    • Added test in shared.test.ts to verify references whose targets are themselves external references are properly inlined
    • Updated test in v3.2-to-v3.1.test.ts to include external file reference scenarios

Implementation Details

The core issue was in the isInlinable function which was using aliasEnd(ref) to determine if a reference could be inlined. However, aliasEnd returns undefined for references that don't have an alias chain (like direct external file references), causing them to be incorrectly marked as non-inlinable. The fix reorders the logic to first resolve the reference and check if it's a valid target, then verify it has a proper alias chain, allowing external file references to be inlined when appropriate.

https://claude.ai/code/session_011uRMv7RDjXFSBo71sjcefJ

…file

`isInlinable` judged a ref by the last link of its alias chain instead of
by the ref's own target. When that chain ended in an external file (or a
missing target), a ref into a removed or shifted part was kept as written:

- 3.1 -> 3.0: `/pets: {$ref: '#/components/pathItems/Pets'}` with
  `Pets: {$ref: './paths/pets.yaml'}` kept pointing at the removed
  `components.pathItems`.
- `$defs` entries that alias external files were left dangling the same way.
- In a list that lost an entry, the kept ref silently resolved to the
  entry that shifted into its slot (e.g. a `secret` header instead of
  `Limit`).

Base the check on the ref's own target. Keep refs whose alias chain loops
as written, as before: they have no target to inline, and following them
would never end in `mergeRef`.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011uRMv7RDjXFSBo71sjcefJ
Keep one case per converter path instead of several that exercise the same
engine branch: drop the duplicate `$defs` assertion and the second path item
entry, fold the shifted external parameter into the existing shifted-list
test, and merge the removed-loop case into the existing alias-loop test.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011uRMv7RDjXFSBo71sjcefJ
@codecov

codecov Bot commented Sep 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes

  • isInlinable in downgrader/src/shared.ts: now checks the ref's immediate target (resolveRef(ref) is a record or boolean) plus that aliasEnd(ref) terminates, instead of resolving the chain's terminal target. Refs whose target is itself a $ref to an external file are now inlinable, so they no longer survive as dangling references inside removed parts.
  • Regression tests: shared.test.ts gains a removed-alias-to-external case; v3.1-to-v3.0.test.ts gains components.pathItems and $defs entries pointing at external files; the v3.2-to-v3.1.test.ts parameter-shift test now inserts an external entry so Shifted rebases.

I confirmed the four added/modified tests fail against the previous isInlinable condition and that the full suite (487 tests) passes at 21dc45e. Cycles remain non-inlinable because aliasEnd returns undefined for them, and direct external refs still stay as written. No README change needed — line 139 already documents "following the reference chain until it leaves the removed part".

Pullfrog  | View workflow run | Using DeepSeek Flash (default — pick a model for stronger reviews) | 𝕏

@dinwwwh
dinwwwh merged commit ca49649 into main Sep 29, 2026
7 checks passed
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