Skip to content

fix(zod): keep every declared tuple position - #2512

Open
cmun2 wants to merge 1 commit into
openai:mainfrom
cmun2:fix/zod-tuple-positions
Open

fix(zod): keep every declared tuple position#2512
cmun2 wants to merge 1 commit into
openai:mainfrom
cmun2:fix/zod-tuple-positions

Conversation

@cmun2

@cmun2 cmun2 commented Aug 28, 2026

Copy link
Copy Markdown
  • I understand that this repository is auto-generated and my pull request may not be merged

Changes being requested

parseTupleDef in the vendored Zod v3 converter builds items with a map that drops any element whose parser returns undefined, while minItems and maxItems are taken from def.items.length. The two disagree whenever an element produces no schema.

zodToJsonSchema(z.tuple([z.void(), z.string()]));
// before
{ "type": "array", "items": [{ "type": "string" }], "minItems": 2, "maxItems": 2 }

// after
{ "type": "array", "items": [{}, { "type": "string" }], "minItems": 2, "maxItems": 2 }

The array-form items keyword is positional, so before this change the schema requires exactly two entries and describes the string element at index 0 — the position the void element occupies. A value the Zod schema accepts is rejected by the schema generated from it, and vice versa. The same shift happens with a rest element, where additionalItems then applies one position too early.

Keeping an unconstrained {} for such an element preserves the alignment and constrains nothing, which is what the element was already saying. Zod v4 describes both positions here, so this also brings the v3 output back in line with v4.

Tuples whose elements all produce schemas are unaffected; a test pins that.

Additional context & links

Split out of #2463. It surfaced there as a regression an earlier revision of that branch introduced, and while fixing it I found the same drop happens without any of that change — a tuple under an object property already hit it, because parseObjectDef has always set propertyPath. The repro above needs no property at all and reproduces on main as it stands. #2463 no longer touches parseDef.ts, so the path that first exposed this is gone from it entirely and the two are independent.

Scoped to one function in src/_vendor/zod-to-json-schema/parsers/tuple.ts.

Testing

  • pnpm vitest run tests/helpers/zod-tuple-positions.test.ts — 4 passed.
  • With the source change reverted and the tests kept, 3 of the 4 fail. The fourth is a control asserting that ordinary tuples do not change, so it passes either way.
  • Full suite: 5353 passed. The 26 failing files are identical to those on main at 222f3d7 and come from optional dependencies that are not installed in this environment (@aws-sdk/*, @smithy/*, ws).
  • ./scripts/lint and ./scripts/format: clean for both files.

`parseTupleDef` builds `items` with a `map` that drops any element whose
parser returns `undefined`, while `minItems` and `maxItems` are taken from
`def.items.length`. `z.tuple([z.void(), z.string()])` therefore converts to
an array that requires exactly two entries but describes only one position,
and the string element is described at index 0 where the void element sits.

Keeping an unconstrained `{}` for such an element preserves the alignment and
constrains nothing, which is what the element already said.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@cmun2
cmun2 requested a review from a team as a code owner August 28, 2026 06:15
cmun2 added a commit to cmun2/openai-node that referenced this pull request Aug 28, 2026
Three things came out now that the decision is read off the emitted
document rather than inferred from `refs.seen`.

`Seen.propertyPath` and `Seen.referencePath` are written and never read.
They existed to carry the reference context forward; the emitted `$ref`
carries it instead. `Refs.ts` and `parseDef.ts` go back to their original
contents, so the converter's shared dispatch is untouched by this PR.

A recursive definition whose branch points back at itself was being read
as a call site outside a property, which kept the wrapper the inline
occurrence does not have. A definition describing itself does not decide
which encoding it owes; it still counts as a pointer for the stranding
check, where removing a wrapper strands a self-reference exactly as it
would an external one.

The tuple positioning fix is a separate, pre-existing bug -- it
reproduces with no part of this change applied -- and is now openai#2512.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.

1 participant