Repository navigation
fix(zod): omit impossible optional branch in strict schemas - #2751
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Co-authored-by: cmun2 <dkdkdkalalal@naver.com>
07dd324 to
3a14cd4
Compare
Castiron custom code✅ No new custom-code files detected. 45 mixed files remain; 0 existing customizations changed. Compared 45 existing customizations unchanged
5 more in the full report. A changed generated baseline means this report cannot reliably identify which handwritten lines changed. Inspect the custom-code diffDownload the exact patch produced by this run (requires repository access): gh run download 35117398579 --repo openai/openai-node \
--name castiron-custom-code-35117398579-1 --dir /tmp/castiron-custom-code-35117398579-1
git apply --stat /tmp/castiron-custom-code-35117398579-1/custom-code.patch
cat /tmp/castiron-custom-code-35117398579-1/custom-code.patchOr reproduce it from an SDK checkout containing the vendored reporter: git fetch --no-tags origin 425502d2bf2da1bdd37efda3b0fa5add96fc8a51 3a14cd427d6f16545c8d2eb48d3a7098a92f0b3b
python3 scripts/castiron/custom_code_report.py report \
--base 425502d2bf2da1bdd37efda3b0fa5add96fc8a51 \
--head 3a14cd427d6f16545c8d2eb48d3a7098a92f0b3b --fetch --require-head-hash --public \
--out /tmp/castiron-custom-code-3a14cd427d6f
cat /tmp/castiron-custom-code-3a14cd427d6f/custom-code.patchThis is the current full custom patch for mixed files, not an attribution of only the handwritten lines changed by this PR. |
markstuart-oai
left a comment
markstuart-oai
left a comment
There was a problem hiding this comment.
Reviewed 3a14cd427d6f16545c8d2eb48d3a7098a92f0b3b. This is a good simplification at the owning boundary: omit the impossible optional branch during generation and register the child at its final anyOf/0 path. Keeping the wrapper preserves annotations and optional-root diagnostics, while the explicit override guard retains existing callback output and path behavior.
No actionable findings after independent reference and compatibility reviews. Shared, nested, recursive, and supplied-definition cases preserve reference meaning; non-strict conversion, custom overrides, Zod v4, Realtime, and response parsing retain their existing contracts.
Validation: 1,127 schema/helper tests passed, including all 17 new regressions; lint, typecheck, and build passed. Built CJS and ESM helpers passed conversion, reference, parsing, and root-diagnostic probes on Node 22.0.0 and 24.20.0. CI for this commit passed, including the Node 22/24/26 matrix. All validation was local or CI; no live API calls were made.
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Reviewed exact head 3a14cd427d6f16545c8d2eb48d3a7098a92f0b3b; no blocking findings. The change removes only the impossible strict-mode optional branch and records the child at its final anyOf/0 path, while preserving the wrapper, non-strict generation, and caller override behavior. Shared, nested, recursive, and supplied-definition references remain valid. I ran the 17 focused regressions and the full Zod helper suite locally (811 tests); all passed. Current CI, CodeQL, Castiron, and OkTest (236/236) are green.
Automated Release PR --- ## [7.17.0](v7.16.0...v7.17.0) (2026-09-16) ### Features * **api:** add compaction progress events ([#2749](#2749)) ([425502d](425502d)) ### Bug Fixes * **ci:** retry transient Deno download failures ([#2752](#2752)) ([e39037b](e39037b)) * preserve chat runner abort reasons ([#2607](#2607)) ([eda4d0c](eda4d0c)) * **realtime:** preserve native WebSocket error causes ([#2715](#2715)) ([58fa72a](58fa72a)) * **responses:** reject function-based API keys in WebSocket constructors ([#2586](#2586)) ([eea7618](eea7618)) * **runners:** reject unfinished turns in streaming runTools ([#2716](#2716)) ([937ed67](937ed67)) * **streaming:** emit terminal SSE events missing a trailing blank line ([#2726](#2726)) ([94d6418](94d6418)) * **zod:** omit impossible optional branch in strict schemas ([#2751](#2751)) ([2b4c2c7](2b4c2c7)) ### Chores * update lint tooling and tailor anti-slop for the SDK ([#2753](#2753)) ([21a03c2](21a03c2)) ### Documentation * **examples:** preserve split UTF-8 in raw stream consumer ([#2714](#2714)) ([f44152a](f44152a)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: openai-sdks[bot] <284451331+openai-sdks[bot]@users.noreply.github.com>
Changes being requested
Shared Zod v3
.nullable().optional()fields can generate extracted definitions containinganyOf: [{ not: {} }, innerSchema]. Thenotbranch accepts no JSON values and is unsupported by strict Structured Outputs.For strict conversion without an override callback, generate
anyOf: [innerSchema]and parse the child atanyOf/0. Keeping the wrapper preserves existing optional-root diagnostics and metadata behavior, while generating references against the final layout avoids post-processing or reference repair. Non-strict and override-enabled conversion retain their existing behavior.The production change is six added lines and seven removed lines in the optional parser. Focused tests cover all four strict helpers, both nullable/optional orders, supplied definitions and descriptions, escaped definition names, shared nested and recursive schemas, and non-strict/override controls.
Additional context & links
Fixes #2462. Supersedes #2463 with a smaller fix at schema generation. The acceptance scope is removing the generated impossible optional branch while preserving valid references and existing diagnostics; general JSON Schema normalization is outside this change.
Validation:
tests/node-version-review.test.ts. The same four failures reproduce with the parser change removed../scripts/lint,pnpm exec tsc --noEmit, andpnpm buildpassed using Node 24.19.0.