Skip to content

fix(lib): preserve custom tool calls in parse_chat_completion - #3505

Open
PranavMishra28 wants to merge 4 commits into
openai:mainfrom
PranavMishra28:fix/parse-preserve-custom-tool-calls
Open

fix(lib): preserve custom tool calls in parse_chat_completion#3505
PranavMishra28 wants to merge 4 commits into
openai:mainfrom
PranavMishra28:fix/parse-preserve-custom-tool-calls

Conversation

@PranavMishra28

@PranavMishra28 PranavMishra28 commented Jul 14, 2026

Copy link
Copy Markdown

What

parse_chat_completion() — the helper behind client.chat.completions.parse() — logs a warning and then drops every custom-type tool call:

elif tool_call.type == "custom":
    # warn user that custom tool calls are not callable here
    log.warning("Custom tool calls are not callable. Ignoring tool call: %s - %s", ...)
    # <-- never appended, so the call is discarded

So a supported GPT-5 custom tool call that the model emits disappears from the parsed result — message.tool_calls omits it, and for a custom-only turn it comes back None. The raw (unparsed) ChatCompletion carries the call correctly; only .parse() loses it.

The handling is also internally inconsistent: the trailing else branch already preserves any non-function tool call by appending it unchanged — only custom is special-cased to discard.

Fix

Two parts, because appending the call turned out not to be enough.

1. Stop dropping it. Append the custom tool call, mirroring the else branch. Custom calls legitimately don't get parsed_arguments (no schema to parse their free-form input against), so they're surfaced as-is. The inaccurate "Ignoring tool call" warning, which fired on every custom call in normal use, is removed.

2. Make the annotation describe what the field holds. ParsedChatCompletionMessage.tool_calls was narrowed to list[ParsedFunctionToolCall], and pydantic serializes by the declared type, so dumping a whole parsed completion emitted the call with its payload silently gone:

p.model_dump()["choices"][0]["message"]["tool_calls"]
# [{'id': 'call_custom', 'type': 'custom'}]      <- `custom` missing entirely

That doesn't even validate back into an equivalent model, so a caller persisting and replaying a parsed completion loses the call. tool_calls is now list[ParsedChatCompletionMessageToolCallUnion], an annotated discriminated union mirroring ChatCompletionMessageToolCallUnion on the base class with the function member swapped for its parsed subclass. Function calls still resolve to ParsedFunctionToolCall with parsed_arguments intact; the parsed message now lines up with the raw one instead of diverging from it.

Consequences of the wider type, handled here rather than pushed onto callers:

  • The four assert_never exhaustiveness sites in the streaming accumulator get an explicit custom branch. A custom call carries no parsed_arguments, and the argument-delta/done events are function-tool specific, so each branch is a documented no-op — but being explicit keeps exhaustiveness real, so a future third tool-call type still trips assert_never.
  • examples/parsing_tools.py narrows on type before reaching for .function, which the raw completion has always required.

Tests

tests/lib/chat/test_parse_custom_tool_calls.py — pure unit tests, no mock server or live API: a custom-only message, a mixed function+custom message, a dump/validate round trip, and the streaming-boundary test above. The first three assert on the whole-completion dump rather than dumping each tool call on its own, and all three fail without this change.

Scope: non-streaming parse() only

A streamed custom tool call cannot reach this parser at all. ChoiceDeltaToolCall.type is Optional[Literal["function"]], so the chunk fails validation before get_final_completion() gets here:

Input should be 'function' [type=literal_error, input_value='custom']

Extending that path means widening the generated chunk delta types plus the streaming accumulator, and it depends on the wire shape the API actually emits for streamed custom calls, which I can't determine from outside. So this PR is scoped to non-streaming .parse(), and the boundary is pinned by test_streaming_deltas_cannot_carry_a_custom_tool_call_yet rather than left in prose: when the generated types gain a custom member that test fails and points at the streaming half. The branch comment in _completions.py says the same.

Corrections to my earlier description of this PR

Two things I claimed that were wrong, both found by executing rather than reading:

  • I described the serialization loss as out of scope and needing the generated type widened. The widening is a four-line type change in a file CONTRIBUTING.md says modifications are persisted through generation, and it is the actual fix — so it's in scope and it's here. I also understated the symptom: the payload isn't just dropped on the whole-completion path, the emitted object is structurally invalid.
  • I said pyright --strict was clean on the changed files. It was not: the first commit left an error at _completions.py:133, appending a custom call to a list annotated list[ParsedFunctionToolCall]. That's fixed here, and both checkers are now clean.
  • I described this as fixing the streaming get_final_completion() path as well. It does not, for the reason in the scope section above.

Verification

ruff check and ruff format --check clean on every file touched. pyright clean on all six. mypy reports one fewer error than main (the :133 one above); no new errors — checked by diffing the full error list against the base rather than eyeballing it. tests/lib/chat/ 36 passed. The remaining tests/lib/ failures are Bedrock/Azure and are identical to the base (31 in both, missing optional deps locally).

Rebased onto main and retargeted there (this PR originally targeted next; main is where external fixes land — 36 of the last 40 merges).

Developed with Claude Code; reviewed and tested by Pranav before marking ready for review.

@PranavMishra28
PranavMishra28 requested a review from a team as a code owner July 14, 2026 19:15
Comment thread src/openai/lib/_parsing/_completions.py
@PranavMishra28

Copy link
Copy Markdown
Author

@seratch would appreciate your eyes on this when you get a chance. small one: parse_chat_completion() drops GPT-5 custom tool calls on a custom-only turn (the raw completion keeps them, only .parse() loses them). the fix just appends the custom call instead of discarding it, mirroring how the else branch already preserves non-function calls. tests included, targeted at next. happy to adjust if you'd rather handle it a different way.

@stainless-app
stainless-app Bot force-pushed the next branch 3 times, most recently from 317260c to e67afa8 Compare July 22, 2026 17:46
`parse_chat_completion` (behind `client.chat.completions.parse()` and the
streaming `get_final_completion()`) logged a warning and then *dropped* every
`custom`-type tool call, so a supported GPT-5 tool call the model made vanished
from the parsed result (`tool_calls` could even come back `None`).

The handling was also inconsistent: the trailing `else` branch already preserves
any non-function tool call by appending it unchanged; only `custom` was
special-cased to discard. Append the custom call the same way (it has no schema
to parse `parsed_arguments` against, so it's surfaced as-is) instead of dropping
it, and remove the now-inaccurate "Ignoring tool call" warning that fired on
every custom call in normal use.

Adds tests/lib/chat/test_parse_custom_tool_calls.py covering a custom-only and a
mixed function+custom message; both fail before this change.

Note: `ParsedChatCompletionMessage.tool_calls` is a generated type narrowed to
`list[ParsedFunctionToolCall]`, so a custom call round-trips through attribute
access and its own `model_dump()` but its `custom` payload is still dropped when
the whole completion is serialized — same limitation the existing `else` branch
already has for non-function calls. Fully fixing serialization needs the
generated type widened, which is out of scope for a lib-only change.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@PranavMishra28
PranavMishra28 force-pushed the fix/parse-preserve-custom-tool-calls branch from c11f3c9 to d0c07c8 Compare August 3, 2026 05:48
@PranavMishra28
PranavMishra28 changed the base branch from next to main August 3, 2026 05:48
@PranavMishra28

Copy link
Copy Markdown
Author

@apcha-oai @jbeckwith-oai small correction on my end: this originally targeted next, which was wrong. 36 of the last 40 merges go to main, and next currently points at the same commit (cbdc98b). I have retargeted it and rebased onto main, so it is now a single commit, +78/-7 across one source file and one new test file.

The bug: parse_chat_completion logs a warning and then drops every custom-type tool call, so a custom tool call the model made disappears from .parse() output (tool_calls comes back None for a custom-only turn). The else branch immediately below already preserves non-function tool calls unchanged, so custom was the only type being discarded. This appends it the same way.

One thing worth deciding before you spend review time: ParsedChatCompletionMessage.tool_calls is a generated type narrowed to list[ParsedFunctionToolCall], so with this change a custom call survives attribute access and its own model_dump(), but its custom payload is still dropped when the whole completion is serialized. Fixing that properly means widening the generated type, which I cannot do from here.

So: do you want this lib-only fix as-is, or should it be handled upstream in the Stainless spec instead? Happy to close this if it is the latter.

@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.

Blocking: this appends a known custom variant to a list and public field declared as list[ParsedFunctionToolCall]. Pyright reports an argument-type error at this append, and the mismatch causes Pydantic to serialize a custom call as only {id, type}, silently dropping custom.name and custom.input from ParsedChatCompletion.model_dump()/model_dump_json(). The new tests avoid that public path by casting to Any and dumping each element independently. Please widen the parsed tool-call type to a discriminated union of ParsedFunctionToolCall and ChatCompletionMessageCustomToolCall (including the local accumulator and the generated/source-of-truth definition so regeneration preserves it), then add regression assertions against serialization of the whole parsed completion. Also, the stated streaming behavior is not currently covered or functional: ChatCompletionChunk.ChoiceDeltaToolCall only accepts type='function', and validating a custom delta fails before get_final_completion() reaches this parser. Either extend the generated streaming delta union/accumulator and test sync+async final-completion paths, or scope this PR and its claims explicitly to non-streaming parse(). Direct parsing otherwise preserves the in-memory custom object as intended, and CodeQL/diff checks are clean.

Appending the custom tool call was not enough. `ParsedChatCompletionMessage`
narrowed `tool_calls` to `list[ParsedFunctionToolCall]`, and pydantic serializes
by the declared type, so dumping a whole parsed completion emitted
`{"id": ..., "type": "custom"}` with the `custom` payload silently gone. The
result did not even validate back into an equivalent model, so a caller
persisting and replaying a parsed completion lost the call.

`tool_calls` is now `list[ParsedChatCompletionMessageToolCallUnion]`, an
annotated discriminated union mirroring `ChatCompletionMessageToolCallUnion` on
the base class with the function member swapped for its parsed subclass. That
makes the annotation describe what the field actually holds, which is what
pydantic needs, and it lines the parsed message up with the raw one instead of
diverging from it.

Consequences of the wider type, all handled here rather than left to callers:

- The four `assert_never` exhaustiveness sites in the streaming accumulator now
  have an explicit `custom` branch. A custom call carries no `parsed_arguments`
  and the argument-delta/done events are function-tool specific, so each branch
  is a documented no-op, but being explicit keeps exhaustiveness real: a third
  tool-call type still trips `assert_never`.
- examples/parsing_tools.py narrows on `type` before reaching for `.function`,
  which the raw completion has always required.

This also removes the pyright/mypy error the previous commit introduced at
_completions.py:133, where a custom call was appended to a list annotated
`list[ParsedFunctionToolCall]`.

Tests now assert on the whole-completion dump and on a validate round trip
rather than dumping each tool call on its own, which was the hole that let the
serialization loss through. All three fail without this change.
@PranavMishra28

Copy link
Copy Markdown
Author

pushed a second commit, and it corrects two things i said earlier on this PR.

the first fix wasn't complete. appending the custom call makes it show up on attribute access, but ParsedChatCompletionMessage.tool_calls was narrowed to list[ParsedFunctionToolCall], and pydantic serializes by the declared type, so dumping a whole parsed completion gave:

[{'id': 'call_custom', 'type': 'custom'}]

custom gone entirely. that isn't just lossy, it doesn't validate back into an equivalent model, so anyone persisting and replaying a parsed completion loses the call. i had written this off in the description as out of scope; it isn't. tool_calls is now list[ParsedChatCompletionMessageToolCallUnion], an annotated discriminated union mirroring ChatCompletionMessageToolCallUnion on the base class with the function member swapped for its parsed subclass, so the annotation matches what the field actually holds and the parsed message stops diverging from the raw one. function calls still come back as ParsedFunctionToolCall with parsed_arguments intact.

i also said pyright was clean on the changed files. it wasn't: the first commit left an error at _completions.py:133 from appending a custom call to a list[ParsedFunctionToolCall]. widening the local accumulator to the same union clears it, so this commit removes an error rather than adding one.

the wider type has two consequences and i handled both here instead of leaving them for callers. the four assert_never sites in the streaming accumulator now have an explicit custom branch — no-ops with comments, since a custom call has no parsed_arguments and the argument-delta/done events are function-tool specific, but explicit so exhaustiveness stays real and a third tool type still trips assert_never. and examples/parsing_tools.py narrows on type before touching .function, which the raw completion has always required.

the tests were the reason i missed this. they dumped each tool call individually, which sidesteps the exact bug. they now assert on the whole-completion dump plus a validate round trip, and all three fail on the previous commit.

verification: ruff check and format clean, pyright clean on all six files, mypy one error below main with no new ones (diffed the full list against the base rather than eyeballing), tests/lib/chat/ 35 passed. remaining tests/lib/ failures are Bedrock/Azure, 31 on both my branch and the base, missing optional deps locally.

@apcha-oai @jbeckwith-oai still happy to split the type change out if you'd rather keep this to lib/, though i think it belongs with the fix — appending the call without widening the annotation only half-preserves it.

The streaming half of the earlier claim was wrong. `ChoiceDeltaToolCall.type` is
`Optional[Literal["function"]]`, so a streamed custom tool call fails chunk
validation before `get_final_completion()` reaches `parse_chat_completion` at
all:

    Input should be 'function' [type=literal_error, input_value='custom']

Extending that path means widening the generated chunk delta types plus the
streaming accumulator, which is a separate change and depends on the wire shape
the API actually emits for streamed custom calls.

Records the boundary in code rather than only in prose:
test_streaming_deltas_cannot_carry_a_custom_tool_call_yet asserts the validation
error, so when the generated types gain a custom member the test fails and points
at the streaming half instead of it being found in the field. The branch comment
in _completions.py now says non-streaming only and references that test.
@PranavMishra28

Copy link
Copy Markdown
Author

@jbeckwith-oai this is a good review and you caught the thing I got wrong. all four of the parse-side asks are in 27e8ae9a, which I pushed before I saw your review, so the overlap is convergent rather than me responding to it — the remaining item, the streaming claim, is in c80482a2.

on the union: tool_calls is now list[ParsedChatCompletionMessageToolCallUnion], an annotated discriminated union on type mirroring ChatCompletionMessageToolCallUnion on the base class with the function member swapped for its parsed subclass. local accumulator in _completions.py widened to the same alias, which is what clears the pyright error at :133. mypy now reports one error fewer than main rather than one more.

on source of truth: parsed_chat_completion.py is it. Parsed* are SDK-only types, not spec types — api.md has zero Parsed entries and there is no OpenAPI shape behind them, so there is nowhere else to put it. CONTRIBUTING says modifications to generated code are persisted through regeneration, so the definition lives with the other hand-maintained overrides in that file, next to the existing # type: ignore[assignment] and the reportIncompatibleVariableOverride suppression that are already there for the same reason. if the Stainless config needs a matching entry to survive a regen, that is on your side and I can't see it from here — worth confirming.

on serialization coverage: the tests now dump the whole parsed completion, plus a ChatCompletion.model_validate(parsed.model_dump()) round trip. you were right that dumping each element under a cast to Any was the tests dodging the public path; that is exactly why I missed this in the first place.

on streaming, you are right and my description was wrong. verified it rather than taking it on faith:

Input should be 'function' [type=literal_error, input_value='custom']

the chunk never validates, so get_final_completion() cannot reach the parser at all. I took the scoping option, not the extension. extending it means widening the generated delta types and the accumulator, and the part I genuinely cannot settle from outside is the wire shape the API emits for a streamed custom call — whether it arrives as type: "custom" deltas with incremental custom.input, or some other shape. guessing at that in generated types seemed worse than scoping. so the PR and its claims are now non-streaming .parse() only, and rather than leave that in prose I pinned it: test_streaming_deltas_cannot_carry_a_custom_tool_call_yet asserts the validation error, so whenever those types gain a custom member the test fails and points straight at the streaming half. the branch comment in _completions.py says the same thing.

happy to do the streaming half as a follow-up if you can tell me the delta shape to target.

state: ruff check and format clean, pyright clean on all six files, mypy 37 vs 38 on base with no new errors (diffed the full list, not eyeballed), tests/lib/chat/ 36 passed. the four new tests fail without the corresponding changes.

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