fix(parser): recognize raw json_schema response format in auto-parsing checks - #2015
fix(parser): recognize raw json_schema response format in auto-parsing checks#2015hsusul wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0c57565d03
ℹ️ 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".
|
|
||
| export function hasAutoParseableInput(params: ResponseCreateParamsWithTools): boolean { | ||
| if (isAutoParsableResponseFormat(params.text?.format)) { | ||
| if (isAutoParsableResponseFormat(params.text?.format) || params.text?.format?.type === 'json_schema') { |
There was a problem hiding this comment.
Return a usable type for raw schema results
When a raw schema reaches this new branch through responses.stream(...).finalResponse(), ExtractParsedContentFromParams still resolves its parsed type to null because it recognizes only branded AutoParseableTextFormat values; the analogous Chat Completions extractor has the same problem. The runtime now returns the JSON.parse result while output_parsed, content parsed, and message.parsed are declared to be null, preventing TypeScript consumers from using the newly supported result without a cast. Update both extractors to represent raw JSON-schema results with a usable type such as unknown or a JSON value type.
Useful? React with 👍 / 👎.
| if (isAutoParsableResponseFormat(params.response_format)) { | ||
| if ( | ||
| isAutoParsableResponseFormat(params.response_format) || | ||
| params.response_format?.type === 'json_schema' |
There was a problem hiding this comment.
Parse raw schemas in stream completion events
When a raw json_schema format is used with chat.completions.stream(), this branch makes finalChatCompletion() return parsed JSON, but the stream's content.delta snapshots and content.done event still report parsed: null: #getAutoParseableResponseFormat() only accepts branded formats, and both event paths are gated through it. Consumers listening to the documented parsing events therefore receive a result inconsistent with the final completion; the raw-schema case also needs to be recognized in those streaming paths, using generic JSON parsing where no $parseRaw callback exists.
Useful? React with 👍 / 👎.
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Raw-schema recognition is currently added only to the coarse hasAutoParseableInput() gate, while the type extractors and streaming parser still use the old brand-only definition. That split leaves two blocking inconsistencies:
-
Both
ExtractParsedContentFromParamsaliases still resolve an unbranded{ type: "json_schema", ... }format tonull. The methods now return parsed JSON at runtime whilemessage.parsed,content[].parsed, andoutput_parsedare declared asnull. Please represent raw-schema output with a usable type (for exampleunknownor a JSON value type) in both Chat Completions and Responses, with compile-time coverage. -
ChatCompletionStreamstill gates partial/final event parsing through#getAutoParseableResponseFormat(), which accepts only branded helper formats. I reproduced a raw-schema stream where bothcontent.delta.parsedvalues andcontent.done.parsedwerenull, butfinalChatCompletion().choices[0].message.parsedwas{ city: "SF" }. Please unify the parsing decision so raw schemas are handled consistently in stream events too, and add regression coverage.
This is also a maintainability issue: adding the same special-case recognition at only two entry gates creates competing definitions of “auto-parseable.” A canonical predicate/parser path should own the branded-callback versus generic-JSON behavior.
Validation at 0c57565d03bd75c029b3f658b42e6e9adde60588: the three focused parser/stream suites passed (28 tests, 26 snapshots); changed-file Prettier and ESLint passed; TypeScript passed; package build passed; and git diff --check passed. The streaming inconsistency above was reproduced against the built package.
…arser
Raw `json_schema` recognition was only added to `hasAutoParseableInput()`, so
the type extractors and `ChatCompletionStream` still used the brand-only
definition. A raw-schema stream emitted `content.delta.parsed` and
`content.done.parsed` as `null` while `finalChatCompletion()` returned parsed
JSON, and `message.parsed` / `content[].parsed` / `output_parsed` were declared
`null` despite being populated at runtime.
Introduce `isParseableResponseFormat()` and `parseResponseFormatContent()` in
`src/lib/parser.ts` as the single owner of the branded-callback versus
generic-JSON decision, and route every gate through them:
- `hasAutoParseableInput()` (Chat Completions and Responses)
- `parseResponseFormat()` / `parseTextFormat()`
- `ChatCompletionStream` partial (`content.delta`) and final (`content.done`)
event parsing, replacing `#getAutoParseableResponseFormat()`
Both `ExtractParsedContentFromParams` aliases now resolve an unbranded
`{ type: 'json_schema', ... }` format to `unknown` instead of `null`, matching
the runtime behavior, with compile-time coverage over the extractors and the
`parse()` / `stream()` public surfaces.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1b161ba320
ℹ️ 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".
| export type ExtractParsedContentFromParams<Params extends AnyChatCompletionCreateParams> = | ||
| Params['response_format'] extends AutoParseableResponseFormat<infer P> ? P : null; | ||
| Params['response_format'] extends AutoParseableResponseFormat<infer P> ? P | ||
| : Params['response_format'] extends ResponseFormatJSONSchema ? unknown |
There was a problem hiding this comment.
Preserve raw-schema parsed types for typed params
When callers pass a params variable typed as the public ChatCompletionStreamParams/ResponseStreamParams rather than an inline literal, Params['response_format'] is the whole optional response-format union, so this conditional does not extend ResponseFormatJSONSchema and ParsedT still resolves to null. The fresh case is typed stream params: the new hasAutoParseableInput() gate now parses raw json_schema streams via JSON.parse, so finalChatCompletion().choices[].message.parsed (and the analogous Responses output_parsed) are declared null while containing parsed JSON; detect the json_schema member of the union here and mirror the fix in ResponsesParser.
Useful? React with 👍 / 👎.
Problem
When providing a raw
json_schemaresponse format object ({ type: 'json_schema', json_schema: {...} }) inresponse_format(Chat Completions) ortext.format(Responses API) without wrapping it in an SDK helper likezodResponseFormat,hasAutoParseableInputreturnedfalse.Root Cause
hasAutoParseableInputin bothsrc/lib/parser.tsandsrc/lib/ResponsesParser.tscheckedisAutoParsableResponseFormat(format), which only returnstruewhen the `` property is present on the format object. Consequently,maybeParseChatCompletionand `maybeParseResponse` bypassed auto-parsing, leaving the fallback `JSON.parse(content)` logic unreachable and returning `parsed: null` / `output_parsed: null`.Solution
Updated
hasAutoParseableInputinsrc/lib/parser.tsandsrc/lib/ResponsesParser.tsto recognize objects withtype === 'json_schema'.Validation
tests/lib/parser.test.tsandtests/lib/ResponsesParser.test.tsverifyingmaybeParseChatCompletionandmaybeParseResponsewith rawjson_schemainputs.pnpm build,pnpm test,pnpm lint,pnpm format, andgit diff --check.Generated Code Impact
None. Changes are strictly confined to manually maintained code in
src/lib/andtests/lib/.