fix(downgrader): inline $refs into parts removed by 3.2 to 3.1 - #24
Conversation
A $ref loop through a removed part, such as a recursive schema under components.mediaTypes reached through a content map, no longer produces a circular object: the reference is dropped where the loop repeats. The schema walker now keeps one field table and finish per conversion, so shared schemas still hit the per-conversion cache from #23 and convert once. Per-conversion state is reduced to four fields with a single memoize helper for pointer lookups and dangling checks.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Important
The main contract holds for the paths under test, but the two-pass dangling detection can still emit a dangling $ref when a dangling Reference Object is dropped by the cycle cut from a parameter/header array: the drop shifts indices relative to the draft snapshot that danglesIn was computed against, so a ref to a later index is judged alive and kept. See the inline note.
Reviewed changes
- Schema
$refinlining —finishSchemainlines a dangling schema$ref's converted target, merging intoallOfwhen the schema has siblings and returning the target directly when otherwise empty;createSchemaFields.$refdrops the keyword when it dangles. - Reference Object inlining —
convertRefOrinlines any Reference Object whose target dangles, following reference chains; content-map entries now resolve any local pointer viaresolveRefChain. - Two-pass detection — the first pass records every
$refand produces adraft;danglesIncompares source-vs-draft pointer resolution (with an array-length index-shift sentinel); the second pass inlines only when something dangles. - Generalized parameter/header removal —
resolveRefChain+losesEntireContentreplace the precomputed component-ref index, so refs through any pointer are removed, not only aliases insidecomponents. - New
shared.tshelpers —parseLocalRef,getChild,resolveLocalRef,isConverting, with unit tests. - README — documents
$refinlining and the new known limits.
DeepSeek Flash (default — pick a model for stronger reviews) | 𝕏
A Reference Object whose chain of references loops back on itself can never resolve. The second pass dropped it while the first pass kept it, so refs checked against the first-pass draft could still dangle: a later parameter index shifted, or a ref named a removed map entry. Both passes now drop such references, so the draft matches the output.
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
The prior review flagged that the second pass could drop a cyclic dangling Reference Object from a parameter/header array while dangles had been computed against the first-pass draft that still contained it, so a ref to a later index was judged alive and left dangling. 665b33e closes that gap.
- Made the cycle cut happen in both passes —
convertRefOrnow drops a value whenresolveRefChain(value, context) === DROP, so the first-passdraft(wheredanglesis recorded but refs are otherwise kept) already reflects the same array shrinking the second pass produces. resolveRefChainnow returnsDROPon a loop instead ofundefined, andresolveMediaType/losesEntireContentwere updated to treat that as dropped, so cyclic alias chains are consistently removed everywhere rather than preserved.- Updated the alias-cycle test (a cycle can never resolve, so
{ parameters: {} }is now correct) and added a parameter-index regression test. - I independently verified the new regression test fails without the guard: with
665b33ereverted the output leavesP: { $ref: '#/paths/~1a/get/parameters/1' }dangling, exactly the prior repro. With the fix, 401 tests, lint, and typecheck pass.
DeepSeek Flash (default — pick a model for stronger reviews) | 𝕏
…removed by 3.1 to 3.0 (#25) 3.1 → 3.0 no longer leaves `$ref`s pointing into `webhooks` or `components.pathItems`, which it removes. References into them from any position are replaced by their converted target, and Link `operationRef`s into them become an `operationId` or are removed. The output validates as 3.0. Documents without such references convert exactly as before. ## Fixes - Reference Objects, Path Item `$ref`s and Schema `$ref`s into the removed parts are inlined in converted form, following reference chains. A chain that leaves them ends at that `$ref`. A Reference Object's `summary`/`description` applies to the inlined target. - A Link `operationRef` into them becomes the target's `operationId` when that operation is still in the output. Otherwise the link is removed, together with Link `$ref`s that lead to it. - `discriminator.mapping` entries pointing into them are removed. - Recursion is cut: a Schema becomes `{}`, a Path Item keeps its own fields, and other references are dropped. Acyclic input always gives acyclic output, and object cycles in dereferenced input are kept. - Each inlined target is converted once and shared, so reference fan-out no longer grows the output exponentially. - Path Item `$ref`s inline only when they point at a Path Item. - Security scheme aliases resolve through any local ref, so `mutualTLS` removal and scope emptying also apply to escaped names and aliases into the removed parts. - Missing targets and reference loops are left as written, as before. ## Behavior changes - Path Item `$ref`s to `components.pathItems` that reach themselves through callbacks are now cut to their own fields instead of dangling. - A target inlined at several references is shared within the result, as 3.2 → 3.1 already does. The README contract says so. ## Performance | Document (2,000 operations, no refs into removed parts) | main | this PR | | --- | --- | --- | | 3.1 → 3.0 | 5.7 ms | 6.8–7.2 ms | | 3.2 → 3.1 | 10.3 ms | 10.5 ms | Output is identical to main on both. The 3.1 → 3.0 cost is the per-`$ref` check, similar to what #24 added for 3.2 → 3.1. ## Testing - 438 tests pass, with 100% coverage of `packages/downgrader/src`. Lint and type-check pass. - An end-to-end 3.1 document with refs from every position validates as 3.1 before and 3.0 after, and has no pointer into the removed parts. - Every issue confirmed in an adversarial review has a regression test. A fuzz run of 22,000 generated documents found no cyclic output. ## Known limits - A pointer that passes through another `$ref` partway is not followed. - A Link naming a removed operation only by `operationId` is kept. - Where a recursive cycle is cut can depend on document key order. - 3.2 → 3.1 still has the shared-object cycle case from #24. The new `convertInlined` helper makes that a one-line follow-up. --------- Co-authored-by: Claude <noreply@anthropic.com>

3.2 → 3.1 no longer leaves
$refs pointing at parts it removes. A reference intocomponents.mediaTypes, aqueryoradditionalOperationsoperation, a removed parameter, or a moveditemSchemais now replaced by its converted target, so the output validates as 3.1. Documents without such references convert exactly as before.Fixes
$refs into removed parts are inlined in converted form, following reference chains.#/components/mediaTypes/Pet/schemabecomes the Pet schema; beside other schema keywords the target is appended toallOf.JSON.stringifycannot serialize.querystringentry no longer point at the wrong parameter.components.$refs resolve any local media type, including escaped names such asa~1b.Performance
Every distinct
$refis checked against the first-pass result, and schemas are walked rather than copied. A second pass runs only when something dangles.Testing
packages/downgrader/src; lint and type-check pass.mediaTypes,query,additionalOperationsand a shifted parameter index validates as 3.1 and, chained, as 3.0.Known limits
{}, since 3.1 has nowhere to keep it without inventing a component name. Where the cut lands can depend on key order.operationRefand discriminatormappingvalues pointing into removed parts pass through unchanged; the README lists them.$defsdangle). That is a separate change.