Repository navigation
[None][fix] Chat streaming: send each tool call index once per chunk - #21468
JunyiXu-nv wants to merge 1 commit into
Conversation
Several tool parsers report a call's name and its arguments as separate items of one streaming step: the DeepSeek V3.2, V4 and V4.1 DSML parsers and Poolside V1 for every call, Qwen3 for bare-JSON calls, and Qwen3-Coder when a call's name and arguments arrive in one step. The chat completion stream turned each item into its own delta.tool_calls entry, so one chunk carried the same index twice, which OpenAI never does. The openai-python stream helper (client.chat.completions.stream) before 3.14 takes a chunk's first tool_calls list as it is and ends up with two calls: one named with empty arguments, one nameless with the arguments. Merge the items of one step that share an index before building the entries: the entry keeps the first item's position and name (so its id is generated once) and the arguments joined in order. Parsers that never repeat an index within a step stream exactly as before, and the Anthropic Messages stream built on these chunks emits the same events. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Merge same-index parser results tensorrt_llm/serve/postprocess_handlers.py |
The streaming chat postprocessor merges parser items with the same tool-call index before creating deltas. It preserves item order, the first available name, and concatenated argument fragments. |
Test streamed tool-call output tests/unittest/llmapi/apps/openai_server_stub.py, tests/unittest/llmapi/apps/test_openai_chat_stream_tool_calls.py, tests/integration/test_lists/test-db/l0_cpu.yml |
The test server stub accepts and passes a tool_parser option. CPU tests check streamed calls, chunk ordering, SDK assembly, and parser output. The integration test list includes the new test file. |
Estimated code review effort: 3 (Moderate) | ~20 minutes
Change: Bug fix
Possibly related PRs
- NVIDIA/TensorRT-LLM#20626: Introduced the test server stub’s
tokenizerargument; this change extends the same helper withtool_parser. - NVIDIA/TensorRT-LLM#21149: Changes chat stream chunk construction, which is also affected by this change.
Suggested labels: api-compatible
Suggested reviewers: qijune
Merge Risk: 
9cefe
-
No actionable issue remains from this review; the change is mergeable after normal checks.
Pre-merge checks: 
-
Check name Status Explanation Resolution Docstring Coverage Docstring coverage is 78.57% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta… Write docstrings for the functions missing them to satisfy the coverage threshold. -
Check name Status Explanation Title check The title clearly and concisely describes the main change: merging streaming tool-call entries so each tool-call index appears once per chunk. Description check The description includes the required Description, Test Coverage, and PR Checklist sections. It clearly explains the issue, cause, solution, behavior, and test results. The checklist does not address … Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Description check
-
Explanation
The description includes the required Description, Test Coverage, and PR Checklist sections. It clearly explains the issue, cause, solution, behavior, and test results. The checklist does not address every optional item individually, but the description is otherwise complete.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 78.57% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 14 functions across 3 files. (1 skipped: 1 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
Finishing Touches
-
Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
|
/bot run --disable-fail-fast |
|
PR_Github #82115 [ run ] triggered by Bot. Commit: |
|
PR_Github #82115 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #82184 [ run ] triggered by Bot. Commit: |
|
PR_Github #82184 [ run ] completed with state |
Description
Symptom. On
/v1/chat/completionswithstream: true, a DeepSeek V3.2 / V4 / V4.1 model served with its DSML tool parser sends each tool call as two entries of the same chunk'sdelta.tool_calls, both with the sameindex: the first carries the id,typeand name with emptyarguments, the second repeatstype: "function"and carries the wholeargumentswithout id or name. OpenAI never repeats an index within a chunk. The openai-python stream helper (client.chat.completions.stream(...),ChatCompletionStreamState) before 3.14 takes the firsttool_callslist it receives as is, so it ends with two calls: one named with empty arguments and one nameless with the arguments. Every tool call of these models is broken for such a client; with several calls in one chunk the snapshot is scrambled further (two calls come back as four). Clients that accumulate the entries by index themselves are not affected.Cause. Several tool parsers report a call's name and its arguments as separate
ToolCallItems of one streaming step, andchat_stream_post_processorturned every item into its ownDeltaToolCall:"") and then the full arguments, for every call.DeepSeek V3 / V3.1, Kimi K2 / K3, GLM-4 / GLM-4.7, Gemma 4, MiniMax-M3 and Qwen3
<tool_call>calls never repeat an index within a step. MiniMax-M2 does and is merged too, but its streaming parser has a separate defect (a completed invoke is also reported again under the next index) that this PR does not address.Change.
chat_stream_post_processormerges the items of one step that share a tool index before it builds the entries (_merge_calls_by_index): the merged entry takes the position of the index's first item, the first name among its items (so the call id is generated once), and the arguments joined in order. This holds for every parser, so the chunk carries at most one entry per index.Behaviour. Only the number of entries in a chunk's
delta.tool_callschanges (at most one per index); a client that accumulates by index assembles the same calls as before. Chunk boundaries, content, reasoning, ids,finish_reasonand usage are unchanged, and parsers that never repeat an index in a step stream exactly as before, chunk for chunk. The Anthropic Messages stream, which is reframed from these chunks, emits the same events (checked frame by frame for one call, two calls, text followed by a call, and a call block split into small increments). The tool parsers themselves, the non-streaming chat response and the Responses API are not changed.Test Coverage
New
tests/unittest/llmapi/apps/test_openai_chat_stream_tool_calls.py(CPU only, added tol0_cpu.yml):test_dsml_block_in_one_step_streams_one_entry_per_call[one_call, two_calls]: POSTs a streaming request to anOpenAIServerbuilt withmake_stub_openai_serverandtool_parser="deepseek_v41", whose stub engine streams text and then a whole DSML block in one step. No chunk repeats an index; each call is one entry with id,type, name and its full arguments; the content andfinish_reason="tool_calls"are intact.test_openai_sdk_stream_helper_assembles_the_calls[one_call, two_calls]: feeds the same chunks to openai-python'sChatCompletionStreamStateand checks that it ends with exactly the calls made, names and arguments included (skipped ifopenaicannot be imported).test_parsers_reporting_name_and_arguments_in_one_step[deepseek_v32, deepseek_v4, poolside_v1, qwen3_bare_json, qwen3_coder]:chat_stream_post_processorwith each of these parsers and a whole call in one step sends the call as one entry.test_name_and_arguments_in_separate_steps_stream_as_parsed[qwen3, deepseek_v31]: for parsers that name a call in one step and stream its arguments in later ones, every chunk carries exactly the parser's items, compared step by step with the parser run on its own.make_stub_openai_servertakes an optionaltool_parser, passed to theOpenAIServerconstructor.Results (11 new tests): on
main, 7 fail (a chunk repeats an index) and 4 pass; with an openai-python release before 3.14 installed, 9 fail, since the SDK test then assembles 2 calls instead of 1 and 4 instead of 2. All 11 pass with this change, with either SDK. The two "separate steps" tests pass on both by design, and the SDK test passes onmainwith openai-python 3.14 or later, which merges repeated indices itself. Removing the merge call makes the same 7 tests fail again. Overtests/unittest/llmapi/appsandtests/unittest/executor/test_spec_decode_stats_payload.py, the only outcome changes frommainare those 7 tests going from failed to passed.PR Checklist
GitHub Bot Help
To see a list of available CI bot commands, please comment
/bot help.🤖 Generated with Claude Code
Dev Engineer Review
chat_stream_post_processornow mergesToolCallItems with the same index within each streaming step before buildingdelta.tool_calls. The merge keeps the first position and name, and joins argument fragments in order. This limits each chunk to one entry per index. The change does not modify parsers, non-streaming responses, or the Responses API. No review findings or test-run results were supplied.make_stub_openai_serveradds an optionaltool_parserargument and passes it toOpenAIServer. The argument defaults toNone, preserving existing calls.QA Engineer Review
The new CPU-only test module covers unique tool-call indexes within a chunk, assistant text, finish reasons, SDK stream assembly, parser output, and calls split across steps. The test is listed in
tests/integration/test_lists/test-db/l0_cpu.yml. The supplied change summary reports 11 tests; it does not provide test function names or execution results. Coverage verdict: sufficient for the described streaming behavior.Per-File QA Perspective
tensorrt_llm/serve/postprocess_handlers.py: Verify same-index fragments merge in order and preserve the first position and name. Also verify chunks with distinct indexes and parsers that do not repeat indexes retain expected output.tests/integration/test_lists/test-db/l0_cpu.yml: Adds the new CPU test module to the CI test list.tests/unittest/llmapi/apps/openai_server_stub.py: Verify the optionaltool_parseris passed to the server and that calls without it retain the default behavior.tests/unittest/llmapi/apps/test_openai_chat_stream_tool_calls.py: Tests streamed tool-call merging, SDK assembly, and separate-step behavior. The module is listed intest-db/l0_cpu.yml.