Skip to content

feat(downgrader): turn a propertyNames enum into one property per name in 3.1 to 3.0 - #100

Merged
dinwwwh merged 9 commits into
mainfrom
claude/project-thread-d4r2o8
Oct 25, 2026
Merged

dinwwwh merged 9 commits into
mainfrom
claude/project-thread-d4r2o8

Conversation

@dinwwwh

@dinwwwh dinwwwh commented Oct 24, 2026 •

Copy link
Copy Markdown
Member

Requested by dinwwwh · project thread

Zod z.record(z.enum(['a', 'b']), V) and z.partialRecord, and Valibot v.record(v.picklist([...]), V), come out of oRPC as:

{ "type": "object", "propertyNames": { "type": "string", "enum": ["a", "b"] }, "additionalProperties": V }

(Zod adds required.) Related shapes from the same libraries:

  • A literal key (z.literal('a'), v.literal('a')) writes propertyNames: { "type": "string", "const": "a" }.
  • .describe(), v.title/v.description, or a reusable enum's .meta({ title, description }) put a title or description beside the enum.
  • A record keyed by a reusable enum (Zod .meta({ id }), Pydantic dict[Enum, V]) writes propertyNames: { "$ref": "#/components/schemas/Lang" }, and Zod adds a description beside the $ref when the use is described (z.record(Lang.describe('key'), V)).

3.0 has no propertyNames, so it was removed, generators typed the record as Record<string, V>, and an enclosing oneOf became anyOf because the branch got looser.

In 3.1 to 3.0, such a propertyNames now becomes one property per name, each sharing the converted additionalProperties schema, plus additionalProperties: false. That is exact, so an enclosing oneOf stays. A $ref into or through that additionalProperties (zod-to-json-schema root refs, swagger-parser bundle()) is replaced by its converted target, as $defs refs are, since the schema it pointed at is now false.

It applies only when propertyNames holds just an enum or a const (plus optionally type: "string", title, description), or a $ref to such a schema with at most a title or description beside it, and no properties, patternProperties, unevaluatedProperties or additionalProperties: false sits beside propertyNames. Anything else keeps the current behavior (removed). A $ref with other keywords beside it (such as maxLength) is not followed, since those narrow the names further, and an enum beside a const is left alone too.

Two README limitations come with it:

  • A union of literal keys (z.union([z.literal('a'), z.literal('b')])) writes an anyOf of const branches; that is not handled.
  • Each name repeats the value schema once serialized, so large or nested enum-keyed records make large 3.0 documents (three levels of 20 names repeat the innermost schema 8,000 times). Registering the value schema with .meta({ id }) keeps each repeat to a $ref.

Changes:

  • enumNames helper and a matching line in losesRestriction in src/v3.1-to-v3.0.ts, plus six lines in finishSchema. ctx passes through loosen and losesRestriction so enumNames can follow a $ref with the existing resolve. isInlined also matches a $ref through such a record's additionalProperties (isIntoNamedValue).
  • Tests covering the Zod shape, the bare shape, const keys, title/description, the $ref shape with and without a description beside it, the cases where it is still removed, the enclosing oneOf, and a $ref into the value schema.
  • A README row in the 3.1 to 3.0 table and the two limitations above.

Checked: pnpm test (265 pass) and pnpm run check clean. A doc generated by oRPC 2.0.0-beta.45 with Zod for z.literal, z.literal([...]), a described z.enum, a .meta({ id, title, description }) enum key, and a described use of it now gives typed properties in 3.0 and passes @seriousme/openapi-schema-validator; the literal union keeps the old output. On 3,000 random docs with enum-keyed records and $refs into, through, and around them (1,008 refs through additionalProperties), the output always serializes (no cycles) and no $ref that resolved in the input dangles; before the isInlined change, 147 of them dangled. Earlier checks: Ajv agrees with the 3.1 input for the enum-key variants, and 700 fuzzed oRPC Zod/Valibot docs stay valid 3.0 with no lost instances.

🤖 Generated with Claude Code

https://claude.ai/code/session_01VvmRkSSBRTofwpppUksACY

…e in 3.1 to 3.0

Zod and Valibot write a record with enum keys as a propertyNames enum
beside additionalProperties. 3.0 has no propertyNames, so generators typed
it as a string-keyed map and an enclosing oneOf became anyOf. Each name now
becomes a property sharing the additionalProperties schema, with
additionalProperties: false.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VvmRkSSBRTofwpppUksACY
@dinwwwh dinwwwh self-assigned this Oct 24, 2026
@codecov

codecov Bot commented Oct 24, 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 24, 2026 •

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 21 untouched benchmarks


Comparing claude/project-thread-d4r2o8 (a765049) with main (7475a3c)

Open in CodSpeed

…moved

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VvmRkSSBRTofwpppUksACY

@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 critical issues — minor suggestions inline.

Reviewed changes

  • propertyNames enum → one property per name — enumNames detects the Zod/Valibot record shape and finishSchema rewrites it into properties + additionalProperties: false; losesRestriction treats it as exact so an enclosing oneOf stays.
  • Test — covers the Zod shape, the bare shape, the removal cases, and the enclosing oneOf.
  • README — new 3.1→3.0 table row for the enum case.

I checked semantic equivalence with an Ajv 2020-12 differential (49 applicable schema combos × 56 instances): input and converted output agreed everywhere except __proto__ enum names, which is an Ajv quirk (reproducible with a hand-written properties: { "__proto__": {} } schema) rather than a converter defect. pnpm test passes (263) and uncheck is clean on the touched files.

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

@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 critical issues — one minor documentation suggestion inline.

Reviewed changes

Since the prior review (dbb858f), cea5a3e adds assertions to the propertyNames enum test pinning the cases where the conversion does not apply: additionalProperties: false, patternProperties, and a propertyNames carrying an extra keyword (minLength, type: "number"), plus an enum beside description: undefined. No production code changed.

  • propertyNames enum → one property per name — enumNames detects the Zod/Valibot record shape and finishSchema rewrites it into properties + additionalProperties: false; losesRestriction treats it as exact so an enclosing oneOf stays.
  • Tests — the Zod shape, the bare shape, the exact-conversion cases, the removal cases (now including additionalProperties: false and patternProperties), and the enclosing oneOf.
  • README — new 3.1→3.0 table row.

I re-ran the mutation check on the additionalProperties === false guard: with the new test in place, removing that guard now fails the suite (previously all 263 passed), so the guard is genuinely covered. pnpm test is green and uncheck is clean on the touched files.

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

@pullfrog

pullfrog Bot commented Oct 24, 2026

Copy link
Copy Markdown

Follow-up to my review (the inline anchor did not attach): the new README row lists properties, patternProperties, and unevaluatedProperties as the conditions that suppress the propertyNames enum conversion, but not additionalProperties: false, which also suppresses it (the guard in enumNames). Consider adding it so the documented condition matches the code — e.g. { propertyNames: { enum: ['a'] }, additionalProperties: false } is left as { additionalProperties: false } rather than expanded into properties. Everything else in the PR checks out: the conversion is exact (Ajv differential agrees), the enclosing oneOf correctly stays, and the new tests now fail if the additionalProperties: false / patternProperties guards are removed.

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

dinwwwh commented Oct 24, 2026

Copy link
Copy Markdown
Member Author

Re the README row: I'm leaving it as is. With additionalProperties: false beside it, only {} matches whether or not propertyNames is there, so dropping it changes nothing. The guard is there to keep the output short, and listing it in the row would only make the row longer.


Generated by Claude Code

Zod's .meta({ id }) enum keys and Pydantic dict[Enum, V] write a record's
keys as a $ref to a reusable enum. It is followed when nothing sits beside
it, so the record still gets one property per name.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VvmRkSSBRTofwpppUksACY

@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 critical issues — one minor documentation note inline.

Reviewed changes

Since the prior review (cea5a3e), a8c9208 extends the propertyNames enum conversion to follow a $ref.

  • $ref-keyed propertyNames — enumNames now resolves a $ref that stands alone in propertyNames to its target and reads the enum from there, and ctx threads through losesRestriction/loosen so the looseness check sees the same resolved names the rewrite does.
  • Tests — a new case covers a $ref to a reusable enum in components/$defs, the enclosing oneOf staying a oneOf, and the removals for a non-enum target and a $ref carrying a sibling keyword.
  • README — the 3.1→3.0 table row now names the $ref form.

pnpm test (264) and pnpm run check (sherif/oxlint/oxfmt/tsc) are green on this commit.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

Comment thread packages/downgrader/README.md Outdated
…propertyNames enum

Zod z.literal and Valibot v.literal write a record's single key as a const,
and .describe(), v.title, or a reusable enum's meta add a title or
description beside the enum. Both still give one property per name in 3.0.
A union of literal keys, an anyOf of consts, is listed as a limitation.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VvmRkSSBRTofwpppUksACY

@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

Since the prior review (a8c9208), 5847cca widens the propertyNames conversion to also accept a const key and title/description annotations.

  • const key — enumNames now treats a lone const as a single name ([names.const]) when no enum sits beside it, so a Zod z.literal / Valibot v.literal record key becomes one property. A enum beside a const stays on the removal path, since only the names in both would match; a non-string const reduces to properties: {} + additionalProperties: false, which is exact because JSON property names are always strings.
  • title/description — both are accepted as no-op annotation keys on propertyNames (and on a followed $ref target), then dropped in the output.
  • README — the 3.1→3.0 row now names const and the dropped annotations, and a new limitation records that a union of literal keys (anyOf of consts) still loses its names.

pnpm test (264) and uncheck (oxlint/oxfmt/tsc) are green on this commit; the const path is a single-value subset of the enum path already differential-tested against Ajv, and the looseness check agrees with the rewrite throughout.

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

…iption

Zod writes z.record(Lang.describe('key'), V) for a reusable enum as a $ref
with a description beside it, which does not narrow the names.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VvmRkSSBRTofwpppUksACY

@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

Since the prior review (5847cca), 06b568d follows a $ref in propertyNames even when a title or description sits beside it.

  • $ref beside annotations — enumNames now resolves the $ref when the only other propertyNames keys are title/description (both non-constraining annotations, dropped from the 3.0 output), so a described reusable-enum key converts instead of being removed. Other siblings still block following.
  • Test — a new case pins the described-use shape ({ $ref, title, description }), along with the enclosing oneOf variants.

pnpm test (264) and uncheck (oxlint/oxfmt/tsc) are green on this commit.

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

…pertyNames cases

description is now allowed beside the names, so it no longer shows that a
key holding undefined counts as missing.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VvmRkSSBRTofwpppUksACY
…ema per name in 3.0

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VvmRkSSBRTofwpppUksACY
…ema with its target

That additionalProperties becomes false in 3.0, so a $ref into or through
it, as zod-to-json-schema and swagger-parser bundle() write, dangled or
pointed at false. It is now inlined like a $defs ref.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VvmRkSSBRTofwpppUksACY

@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 critical issues — one narrow edge case inline.

Reviewed changes

Since the prior review (06b568d), 0ed9fa0 makes isInlined replace a $ref that points into the value schema of a record keyed by an enum, so it no longer dangles once that record's additionalProperties becomes false.

  • $ref through an enum-keyed record's additionalProperties — isInlined now also inlines when the new isIntoNamedValue finds an /additionalProperties segment whose holder has a convertible propertyNames, replacing the ref with its converted target instead of keeping it.
  • Test — pins the direct replacement plus the control case where a plain string-keyed record keeps its additionalProperties and its $ref.

Verified the recursive value-schema case (a record whose additionalProperties refers back into itself) collapses to {} rather than cycling, and the downgrader test file passes (93).

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

Comment thread packages/downgrader/src/v3.1-to-v3.0.ts
dinwwwh added a commit that referenced this pull request Oct 25, 2026
… true in 3.0 (#101)

<!-- claude-projects-attribution: {"github_login":"dinwwwh"} -->
_Requested by **dinwwwh** · [project
thread](https://claude.ai/code/project/chan_01UcKMz91tMarPV1kWLVgLYE?thread=cmsg_01UcKMz91tMarPV1kWLVgLYEGx42KWDe4jcasnYt6imLXt)_

3.0 has no `patternProperties`, so the 3.1 to 3.0 converter removes it,
along with any `additionalProperties` beside it (a schema or `false`
there would reject the properties `patternProperties` allowed). What's
left is often a bare `{ type: "object" }`, and openapi-typescript types
that as `Record<string, never>`. Code generators read a missing
`additionalProperties` as "no other properties allowed".

This shows up in real output:

- Zod `z.looseRecord` (`patternProperties`, no `additionalProperties`)
- Effect 4 through `@orpc/effect`: `Schema.Record(Schema.Number, V)`,
template-literal key records and `StructWithRest`
(`additionalProperties: true`), and records with `onExcessProperty:
"error"` (`additionalProperties: false`)
- `patternProperties` beside an `additionalProperties` schema, or beside
`properties`
- SendGrid's `contacts/search/emails` response

Now `finishSchema` sets `additionalProperties: true` whenever
`patternProperties` is present. The meaning doesn't change: a missing
`additionalProperties` already allowed every property, and the schema is
still marked as having lost a restriction.

- `src/v3.1-to-v3.0.ts`: the `additionalProperties = true` step in
`finishSchema`. The field converter still drops the input
`additionalProperties`, as on main.
- `README.md`: the `patternProperties` row says `additionalProperties`
becomes `true`.
- `tests/v3.1-to-v3.0.test.ts`: the existing test checks
`additionalProperties` set to undefined, `true`, `false` and a schema,
plus a case with `properties`. It fails without the fix.

Checks: 262 tests pass and `pnpm run check` is clean.

Note: #100's test expects `{ propertyNames: { enum }, patternProperties
}` to give `{}`. With this change it gives `{ additionalProperties: true
}`, so whichever PR merges second needs that expectation updated.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

https://claude.ai/code/session_01FZoWKNpDyGBHkdaur4kKtA

---------

Co-authored-by: Claude <noreply@anthropic.com>
#101 gives a schema with patternProperties additionalProperties: true, so
the propertyNames enum case beside patternProperties now expects it.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011jE3cBByuEorBPFj1djfGs
@dinwwwh
dinwwwh merged commit b450552 into main Oct 25, 2026
11 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