Repository navigation
refactor(downgrader): count a schema met again mid-cycle as loose, instead of converting again - #319
Conversation
… no oneOf branch apart, and takes null
Where several $refs enter the same cycle, the first copy made is reused, so
a later one can hold a $ref that was cut to {} while another target was
being inlined. isDisjoint and the nullable pair merge read such a $ref in
the input, saw its target's type or tag, and kept a oneOf that the {} then
let a value match twice. The holders of cut $refs are now recorded, and
follow() and throughRef() read them as keeping nothing.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VVaw7NmVyQoYtVBNPuE4uV
…operties beside other properties, out of enum merging A oneOf rejects a value two of its branches share, so mergeEnums keeps a oneOf whose literals repeat one. It compared them serialized, which tells equal objects with their keys in another order apart, so it merged such a oneOf into an enum that took the shared value. A oneOf of object or array literals now stays as it is. And an additionalProperties in a literal branch counted as repeating the one around it although that one sits beside other properties, which it then let through, so it repeats only beside the same properties. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VVaw7NmVyQoYtVBNPuE4uV
…side annotations enumNames read the names at the end of the propertyNames $ref chain, past a $ref with an enum, maxLength or other keyword beside it that narrows the names further. The record then got a property for each name of the last enum, allowing keys the input rejects, and counted as converted exactly, so a not or oneOf over it stayed. The chain now stops at such a $ref, so the record loses its key names and counts as loose. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VVaw7NmVyQoYtVBNPuE4uV
…iction around an alias component or a cycle of objects #309 converts the input again when a not or oneOf read a $ref's target before that target lost a restriction around a cycle. It missed two cycles. A component that is only a $ref, an alias, has no output of its own, so the output recorded for it was undefined; it now records the output of the target its chain of kept $refs ends at. And a dereferenced document closes a cycle with the object itself rather than a $ref; convertObject now tells convertSchema when it meets a schema again, and one that was not loose then but is once converted makes the next pass read it as loose. First visits pay nothing. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VVaw7NmVyQoYtVBNPuE4uV
…ed-union $refs The rerun recorded nothing for an alias whose chain passes through a $ref the conversion inlines, such as one into $defs, or that is a $ref to a tagged union beside only annotations, so a not over it around a cycle was kept over a target that lost a restriction. sharedOutput now follows every $ref convertSchema treats as an alias, and reads an inlined hop's output from its inlined copy. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VVaw7NmVyQoYtVBNPuE4uV
…form A schema that loses a restriction around a cycle made the conversion run again, so a not or oneOf that read it too early could drop or widen. Only such a choice changes when more schemas read as loose, so a pass that kept no not, and no oneOf without a loose branch, needs no other. A document with a recursive schema that loses a restriction and no not or oneOf now converts in one pass, about half the time. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VVaw7NmVyQoYtVBNPuE4uV
…y property as loose OAS 3.0 applies a required readOnly property to responses only and a required writeOnly property to requests only, while 3.1 requires it both ways. The 3.0 output so loses that restriction, so a not or oneOf over such an object must not keep it, and the property tells no oneOf branch apart. The property is read through a chain of $refs too, as 3.0 reads each target in its place. Such a schema in a cycle, as a recursive DTO with a required readOnly id, makes a document that keeps a not or oneOf convert twice, which the README performance note now says. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VVaw7NmVyQoYtVBNPuE4uV
… or a set per name CodSpeed read the order schema bench 19% slower with the check. It now loops over `required` once per schema, reads each property's fields directly, and makes a visited set only past the first `$ref`. Instructions per conversion of that schema drop from 4% over main to 2% under callgrind with CodSpeed's V8 flags. A cycle of `$ref`s, which never reaches a schema, is tested too. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VVaw7NmVyQoYtVBNPuE4uV
Marking the schema itself loose lets a $ref to it, and every object around it, read as loose through the existing checks, so no walk over required names is needed. Only the tag check still reads through $refs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VVaw7NmVyQoYtVBNPuE4uV
…stead of converting again A $ref or object cycle that reaches a schema before its conversion is done cannot know yet what that schema loses, so it now counts as loose. This replaces the second pass (#309, #316) and its skip (#317) with two checks, at the cost of a recursive schema always counting as loose: a not over it is removed, and a oneOf with it becomes anyOf unless its branches are disjoint. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VVaw7NmVyQoYtVBNPuE4uV
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Merging this PR will degrade performance by 59.36%2 warnings
Warning Please fix the performance issues or acknowledge them on CodSpeed. Performance Changes
Tip Investigate this regression by commenting Comparing Footnotes
|
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes — the whole net diff of this PR against main, verifying the single-pass cycle rule replaces the old fixpoint without under-marking any schema as loose.
v3.1-to-v3.0.ts— new mid-cycle rule. AddedDONE(WeakSetof converted outputs);finishSchemamarks an output done at the end, andrevisitSchema(out)adds any output met again before that toLOOSE. The kept-$refpath now returnsisLoose(converted) || (converted === DROP && target !== undefined)— aDROPfrominlinewith an existingtargetis a cycle (ctx.inlining), so it is loosened; a danglingtarget === undefinedis not.v3.1-to-v3.0.ts— fixpoint removed.downgradeV30runs the conversion once;keptStrict,refOutputs,looseTargets,revisits,looseInputs,sharedOutput,aliasRef,markLooseTargetsand the second-pass loop are gone, andaliasRefis inlined intoconvertSchema.finallyclears only the remaining per-pass state.shared.ts—convertObject'srevisitcallback now takes only the output ((out) => void) rather than(value, out).- Tests / README — the
oneOfover a pure$refcycle (A → B → A) now expectsanyOf; the READMEnotrow and performance note are updated for the removal of the second pass.
I traced the soundness direction (over-marking drops a not / widens oneOf to anyOf; under-marking is what breaks the contract) and found no under-marking: any read of an in-progress output goes through convertObject's memoized-hit branch into revisitSchema, so the removed per-target propagation is subsumed in one pass; reads of a finished output have final looseness. isLoose is consumed only by loosen, convertTuple's exactness, and convertSchemaRef propagation, all of which stay on the safe side. No stale references to the removed identifiers remain. oxlint/oxfmt/tsc are clean and v3.1-to-v3.0.test.ts passes 130/130.
The one thing worth remembering for follow-ups is the deliberate widening called out in the PR body: every recursive schema now counts as loose, including ones that lose nothing, so a not over a recursive tree is dropped and a oneOf with a recursive branch becomes anyOf unless its branches are told apart. That is consistent with the converter's established "loses detail, never meaning" contract and is documented in the README.
deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏
|
On the CodSpeed result: all four flagged benchmarks carry CodSpeed's own "ran on different hardware" warning, and the numbers do not reproduce. Measured locally the way the CodSpeed plugin does (CodSpeed's V8 flags, 7 warm-up calls,
That is −1% to +2.5%, from the one Generated by Claude Code |
…s still running (#328) #319 marked every converted schema done in a WeakSet, so a schema met again before that counts as loose. Adding each output to a WeakSet made 3.1 to 3.0 about 45% slower than 0.5.0 on real oRPC documents. convertObject now keeps the outputs whose conversion is running on a per-call stack in Context, and calls `revisit` only for one still on it. The output is the same: an output is on the stack exactly while it is not done. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VVaw7PYvCYrJkB3NF1rgbx

Requested by dinwwwh · project thread
What
Simplifies the cycle handling that #309, #316 and #317 added, following the request for the simplest approach with minimal logic: when there is a choice, drop a restriction rather than add logic.
The problem those PRs solved: around a cycle, a
notoroneOfcan read a schema before its conversion is done, so it does not know yet whether that schema lost a restriction. They fixed it by recording what each read saw, then converting the whole input again (#309), following alias chains and object cycles to find those outputs (#316), and skipping the second pass when it could not change anything (#317).This PR replaces all of that with one rule: a schema met again before its conversion is done counts as loose.
$refkept as written whose target is still being converted (inlinereturnsDROPfor a target that exists) is loosened.finishSchemamarked it done (newDONEset, read inrevisitSchema) is loosened.Removed:
keptStrict,refOutputs,looseTargets,revisits,looseInputs,sharedOutput,aliasRef,markLooseTargetsand the second-pass loop. Therevisitcallback inconvertObjectnow gets only the output. Net: +27/−118 lines.README: the
notrow now says a recursive schema counts too, and the performance note drops the second pass.Tests
One expectation changed: a
oneOfover a$refinto a pure$refcycle (A → B → A) now becomesanyOf. The cycle tests from #309 and #316 pass unchanged.pnpm check,pnpm testand coverage (100%) pass.Tradeoffs
notover one is removed, and aoneOfwith one as a branch becomesanyOfunless its branches are told apart by type or by a required tag. A recursive schema through$defsalready counted as loose before this, since recursion there is cut to{}. Code generators modelanyOfas a different union type thanoneOf.WeakSetadd per schema).Generated by Claude Code