Skip to content

fix(downgrader): loosen a oneOf or not that reaches a lost restriction through a kept $ref in 3.1 to 3.0 - #62

Open
dinwwwh wants to merge 1 commit into
mainfrom
claude/upbeat-wozniak-j5qxof
Open

dinwwwh wants to merge 1 commit into
mainfrom
claude/upbeat-wozniak-j5qxof

Conversation

@dinwwwh

@dinwwwh dinwwwh commented Oct 20, 2026

Copy link
Copy Markdown
Member

Problem

In 3.1 → 3.0, a schema that loses a restriction (such as a tuple, which becomes items: { anyOf: [...] }) is marked, so a surrounding oneOf becomes anyOf and a surrounding not is removed. That only worked inline. A $ref kept as written, such as one to components.schemas, came out as a fresh { $ref } that was never marked, so the oneOf and not stayed.

That makes the 3.0 schema stricter than the original, not looser. oRPC triggers it with @orpc/openapi 2.0.0-beta.44 and ZodToJsonSchemaConverter:

z.xor([
  z.tuple([z.string(), z.number()]).meta({ id: 'StrNum' }),
  z.tuple([z.number(), z.string()]).meta({ id: 'NumStr' }),
])

oRPC emits oneOf: [{ $ref: StrNum }, { $ref: NumStr }]. In 3.0, both components become "two items, each a string or a number", so every valid value matches both branches:

Value 3.1 3.0 on main (oneOf) 3.0 with this PR (anyOf)
["a", 1] ✅ ❌ matches both ✅
[1, "a"] ✅ ❌ matches both ✅
["a", "b"] ❌ ❌ ✅ (looser, allowed)

Named z.maps fail the same way. A not over a kept $ref has the same issue: not: { $ref: StrNum } rejected ["b", "c"], which the original accepts.

Fix (packages/downgrader/src/v3.1-to-v3.0.ts)

  • Kept $refs. hasLooseTarget resolves a kept $ref and converts its target. If the target lost a restriction, the $ref is marked too. The seen cache makes that the same object components.schemas reuses.
  • Bare $ref targets are cached per object. Bare $refs skip the seen cache, so without this every $ref to an alias would walk the whole chain again. With 1000 hops that took ~1 s instead of 13 ms. With the cache, a chain is followed once. The cache also ends bare-ref cycles (A={$ref:B}, B={$ref:A}, S={$ref:S}), which otherwise overflow the stack. So no separate visiting set is needed.
  • Top-level schemas skip the lookup. Nothing reads whether a top-level schema lost a restriction: a Parameter's or Media Type's schema, a components.schemas entry, or the root of downgradeSchemaV31ToV30. So a bare kept $ref there is left as written, without converting its target. This keeps ordinary documents close to their old cost, and their conversion order unchanged.
  • Every $ref in the output is still its own fresh object, as before.

Side effect: a discriminated union over named branches that lost a restriction now becomes anyOf too, as the inline form already did. Its const tags keep the branches exclusive, so anyOf means the same thing there.

Results

  • Real oRPC output (@orpc/openapi 2.0.0-beta.44, through both downgrade steps): the z.xor over named tuples, the z.xor over named maps, and the discriminated union all go from oneOf to anyOf. Components convert exactly as before, and the oRPC snapshot is unchanged.
  • Alias chain of n bare $refs, each referenced inside a oneOf: 1000 hops take ~20 ms, 2000 take ~30 ms, so the work stays linear. A version without the cache takes ~1 s at 1000 hops.
  • Benchmarks, measured locally against main (noise is about ±5%):
    • generated api, 100 resources (3.1 → 3.0): about +3%.
    • The chained 3.2 → 3.0 version: about +6%, since itemSchema puts $refs inside schemas, where the lookup has to run.
    • The rest are within noise.
    • Without the top-level skip, the generated API was +11–14%.

Tests

In packages/downgrader/tests/v3.1-to-v3.0.test.ts, under "a schema that lost a restriction › through a $ref kept in the output":

  • Named tuples and named maps under oneOf (as oRPC emits them), a not over a $ref with siblings, and an exactly converted tuple that stays oneOf.
  • A discriminated union over named branches, which matches the inline form.
  • An alias chain into a loose target, plus the bare-ref cycles A ↔ B and S → S.

All three fail on main. The cycle test overflows the stack if the per-object cache is replaced by a plain convertSchema call. pnpm test passes (200 tests), and so does pnpm run check (sherif, oxlint, oxfmt, tsc).

Known limits

The README limitation is narrowed to what is left. Around a cycle, a $ref or object pointing back into a schema still being converted does not yet count what that schema lost, so a not or oneOf over it can still reject values the original accepts. Which $ref misses it depends on which schema of the cycle is converted first. This does not arise in the oRPC cases above.


Generated by Claude Code

…n through a kept $ref in 3.1 to 3.0

A `$ref` kept as written, such as one to `components.schemas`, was never
marked as having lost a restriction, so a surrounding `oneOf` stayed
`oneOf` and a `not` stayed. oRPC emits this for `z.xor` over named tuples
or maps: both branches become arrays whose items match either type, so
every valid value matched both, and 3.0 rejected all of them.

A kept `$ref` now counts as loose when its converted target is. The
target converts once, and `components.schemas` reuses it. A bare `$ref`
target is cached per object as well, so a chain of them is followed once
and a cycle of them ends. Parameters, Media Types and `components.schemas`
skip the lookup for a bare `$ref`, since nothing reads whether their
schema lost a restriction.

A discriminated union over named branches that lost a restriction now
becomes `anyOf` too, as the inline form already did. Its tags keep the
branches exclusive, so this only loosens it.

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

codecov Bot commented Oct 20, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@codspeed

codspeed Bot commented Oct 20, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 20 untouched benchmarks


Comparing claude/upbeat-wozniak-j5qxof (2899e07) with main (a204b43)

Open in CodSpeed

@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

  • Kept $refs now propagate looseness — hasLooseTarget resolves a kept $ref and converts its target, marking the { $ref } output loose when the target lost a restriction, so an enclosing oneOf becomes anyOf and a not is dropped exactly as the inline path already did.
  • Bare-ref targets cached per object — a new KEPT_REF_FIELDS conversion memoizes bare $ref targets, keeping alias chains linear and terminating bare-ref cycles (A ↔ B, S → S).
  • Top-level schemas skip the lookup — convertTopSchema returns a bare kept $ref as written for components.schemas, Parameter/MediaType schema, and the downgradeSchemaV31ToV30 root; the emitted object is unchanged from the old convertSchema path, only the side-effect target conversion is skipped.
  • Tests and README — new cases cover named tuples/maps under oneOf, not over a $ref with siblings, an exactly-converted tuple that stays oneOf, a discriminated union over named branches, an alias chain, and bare-ref cycles; the README limitation is narrowed to the cycle case.

Pullfrog  | View workflow run | Using deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

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