fix(zod): keep every declared tuple position - #2512
Open
cmun2 wants to merge 1 commit into
Open
Conversation
`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
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>
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes being requested
parseTupleDefin the vendored Zod v3 converter buildsitemswith amapthat drops any element whose parser returnsundefined, whileminItemsandmaxItemsare taken fromdef.items.length. The two disagree whenever an element produces no schema.The array-form
itemskeyword 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, whereadditionalItemsthen 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
parseObjectDefhas always setpropertyPath. The repro above needs no property at all and reproduces onmainas it stands. #2463 no longer touchesparseDef.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.mainat222f3d7and come from optional dependencies that are not installed in this environment (@aws-sdk/*,@smithy/*,ws)../scripts/lintand./scripts/format: clean for both files.