refactor!: replace vendored partial JSON parser with native parsing - #2466
refactor!: replace vendored partial JSON parser with native parsing#2466HAYDEN-OAI wants to merge 2 commits into
Conversation
BREAKING CHANGE: The undocumented openai/_vendor/partial-json-parser/parser import path is removed; supported streaming APIs continue to produce partial structured values.
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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c2dc3ab96e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Requesting changes for the existing P1 discussion at src/internal/partial-json.ts:157. Inputs such as ], }, ,, or {}] enter consumeNumber without an owning container, make no index progress, and cause the outer parsing loop to spin forever. A malformed streamed structured-output or tool-argument chunk can therefore synchronously freeze the Node event loop. Reject unmatched root delimiters, enforce forward progress, and add bounded public-stream regressions. Please also address the existing P2 escaped-key corruption/double-parse discussion at line 225. Both findings already have inline threads, so no duplicate comments are needed.
Stack
This pull request is stacked on #2465. Its base is
codex/drop-zod-v3-vendor-20260825so the review contains only the partial-JSON change. Retarget it tomainafter #2465 lands.Summary
partial-jsonimplementation with a first-party JSON-prefix completion helper that delegates object creation and validation to nativeJSON.parse.src/_vendor/partial-json-parserfiles; after this PR the SDK has no runtimesrc/_vendortree. Build-timescripts/_vendortooling is unchanged.tests/internal, retain streaming-boundary and privacy coverage, remove obsolete lint exceptions, and assert that packed CJS/ESM artifacts contain the first-party helper but no vendored runtime code.Compatibility
The undocumented
openai/_vendor/partial-json-parser/parserimport path disappears. Some downstream projects import that internal path directly, so this PR is deliberately stacked on the next-major-release proposal in #2465. Supported streaming APIs and event payloads remain unchanged.Why not use the upstream package?
The currently published
partial-jsonpackage would introduce the SDK's first required runtime dependency and does not preserve the SDK's existing object-property compatibility guarantees. Other candidates either change incomplete-number semantics, do not support CommonJS, or simply copy the same vendored OpenAI source into a third-party package. NativeJSON.parseavoids those tradeoffs.Validation
pnpm install --frozen-lockfile --store-dir /tmp/openai-node-zod-v4-pnpm-storepnpm lintpnpm exec tscpnpm build./node_modules/typescript-4-9/bin/tsc --project dist/src/tsconfig.json --noEmit --incremental false./node_modules/typescript/bin/tsc --project dist/src/tsconfig.json --noEmit --incremental falsepnpm exec publint distOPENAI_TEST_SUITE=unit ./scripts/test --exclude tests/ecosystem-cloudflare-credential-lifecycle-security.test.ts— 143 suites / 5,559 tests passed._vendortree.The excluded Cloudflare fixture contains two existing inode tests that also fail unchanged on upstream
mainin this container because/proc/<parent-pid>/fd/<fd>is unavailable. Both tests remain enabled for normal GitHub CI.