Skip to content

refactor!: replace vendored partial JSON parser with native parsing - #2466

Open
HAYDEN-OAI wants to merge 2 commits into
codex/drop-zod-v3-vendor-20260825from
codex/replace-vendored-partial-json-20260825
Open

refactor!: replace vendored partial JSON parser with native parsing#2466
HAYDEN-OAI wants to merge 2 commits into
codex/drop-zod-v3-vendor-20260825from
codex/replace-vendored-partial-json-20260825

Conversation

@HAYDEN-OAI

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

Stack

This pull request is stacked on #2465. Its base is codex/drop-zod-v3-vendor-20260825 so the review contains only the partial-JSON change. Retarget it to main after #2465 lands.

Summary

  • Replace the copied partial-json implementation with a first-party JSON-prefix completion helper that delegates object creation and validation to native JSON.parse.
  • Delete the last runtime src/_vendor/partial-json-parser files; after this PR the SDK has no runtime src/_vendor tree. Build-time scripts/_vendor tooling is unchanged.
  • Keep incremental structured-response content and function-tool argument snapshots, incomplete strings and Unicode escapes, partial JSON literals, nested containers, unfinished-number handling, existing work/depth/size limits, and redacted parse failures.
  • Preserve native JSON object-property semantics, including safe handling of prototype-like property names.
  • Add no required or optional dependencies; package metadata and the lockfile are unchanged relative to the parent.
  • Move parser regressions into 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/parser import 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-json package 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. Native JSON.parse avoids those tradeoffs.

Validation

  • pnpm install --frozen-lockfile --store-dir /tmp/openai-node-zod-v4-pnpm-store
  • pnpm lint
  • pnpm exec tsc
  • pnpm 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 false
  • pnpm exec publint dist
  • OPENAI_TEST_SUITE=unit ./scripts/test --exclude tests/ecosystem-cloudflare-credential-lifecycle-security.test.ts143 suites / 5,559 tests passed.
  • Generated API coverage — 82 suites / 556 tests passed.
  • Exhaustive differential fuzzing: 109,939 valid-prefix cases, 1,000 deterministic randomized JSON documents, and 28 focused edge fixtures; zero differences from the previous parser.
  • Parser, streaming-boundary, prototype-safety, and parse-error privacy regressions — 380 focused tests passed, including the new linear-work test for 512 trailing-comma containers.
  • Structured-output/streaming benchmark — all 13 cases passed. A 5.8 KB representative payload was approximately 8.5x faster for complete JSON, 1.7x faster for incomplete JSON, and 2.1x faster across progressive 128-byte chunks than the previous vendored implementation.
  • Packed-package checks verify CommonJS/ESM behavior, optional-peer isolation, zero required runtime dependencies, the first-party helper's presence, and complete absence of the runtime _vendor tree.

The excluded Cloudflare fixture contains two existing inode tests that also fail unchanged on upstream main in this container because /proc/<parent-pid>/fd/<fd> is unavailable. Both tests remain enabled for normal GitHub CI.

BREAKING CHANGE: The undocumented openai/_vendor/partial-json-parser/parser import path is removed; supported streaming APIs continue to produce partial structured values.
@HAYDEN-OAI
HAYDEN-OAI requested a review from a team as a code owner August 25, 2026 18:51
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Aug 25, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-08-25T19:11:13.722698Z 3e011d4 New commits
🔒 Security Review Completed 2026-08-25T19:10:53.377294Z 3e011d4 New commits
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 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".

Comment thread src/internal/partial-json.ts
Comment thread src/internal/partial-json.ts Outdated

@jbeckwith-oai jbeckwith-oai left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

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