fix(stream): emit a single finish_reason for tool-call chat completions#114
Open
zerone-jang wants to merge 1 commit into
Open
fix(stream): emit a single finish_reason for tool-call chat completions#114zerone-jang wants to merge 1 commit into
zerone-jang wants to merge 1 commit into
Conversation
sse_translate_chat emitted finish_reason "tool_calls" for each completed function_call item and then an additional finish_reason "stop" chunk from the response.completed handler. Per the OpenAI streaming format a choice carries exactly one finish_reason; clients that honor the last one classify a mixed text + tool_call response as a plain text turn and silently drop the accumulated tool calls. Mark the stop chunk as already sent when the tool_calls finish chunk goes out, so response.completed no longer appends the spurious "stop". Adds a streaming regression test that fails without the fix.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When a streamed
/v1/chat/completionsresponse contains a tool call,sse_translate_chatcurrently emits twofinish_reasonchunks for the same choice:finish_reason: "tool_calls"— emitted right after each completedfunction_call(andweb_search_call) item, andfinish_reason: "stop"— emitted unconditionally from theresponse.completedhandler, becausesent_stop_chunkis only ever set on the (unreachable, see note below)response.output_text.donebranch and insideresponse.completeditself.Per the OpenAI streaming format, a choice carries exactly one terminal
finish_reason. This PR marks the stop chunk as already sent when thetool_callsfinish chunk goes out, soresponse.completedno longer appends the spurious"stop".Why it matters (real-world impact)
Clients that honor the last
finish_reasonclassify the response as a plain-text turn. Some agent frameworks then silently discard the accumulated tool calls whenever the model prefixed them with visible commentary text (which GPT-5.x models do routinely — "Let me look that up." + tool call). The observable symptom is an assistant that keeps announcing work ("I'll fetch those articles now.") and never executes it, since its tool calls evaporate at the proxy boundary.We debugged exactly this in production with OpenClaw as the client: the model verifiably emitted
function_callitems upstream (visible with--verbose-obfuscation), but every response that mixed commentary text with tool calls was delivered to the agent as text-only. Responses consisting of a bare tool call survived — which made the failure look maddeningly nondeterministic.Repro
Any streaming request where the model produces preamble text before the call:
Observed tail of the stream before this fix:
After the fix the
"stop"chunk is gone; plain-text responses are unaffected (their stop chunk still comes fromresponse.completed).Changes
chatmock/utils.py: setsent_stop_chunk = Trueafter emitting thefinish_reason: "tool_calls"chunk, in both theweb_search_callpath and theresponse.output_item.donefunction_callpath (2 lines).tests/test_routes.py: streaming regression test asserting that a mixed text +function_callresponse yields the tool-call delta and exactly["tool_calls"]as finish reasons. Fails without the fix, passes with it.Note for reviewers
There is an
elif kind == "response.output_text.done":branch that emits an earlyfinish_reason: "stop"— it is currently unreachable because the precedingelif isinstance(kind, str) and kind.endswith(".done"): passcatch-all shadows it. Please don't revive that branch as part of cleaning this up: emittingstopat text-done would guarantee the tool-call-after-commentary case breaks (text completes before thefunction_callitem arrives).How to try locally
python -m unittest tests.test_routes tests.test_models # 23 tests, all pass python -m unittest tests.test_routes.RouteTests.test_chat_completions_stream_single_finish_reason_with_tool_callManual verification: run
chatmock serve, issue the curl above, and check the stream tail contains a singlefinish_reason.Docs (README.md / DOCKER.md): not required — no user-facing flags or payload shapes change; this restores spec-conformant streaming.