fix(litellm): keep every tool call in a finish-bearing stream chunk - #7005
Open
elbourne12345 wants to merge 1 commit into
Open
fix(litellm): keep every tool call in a finish-bearing stream chunk#7005elbourne12345 wants to merge 1 commit into
elbourne12345 wants to merge 1 commit into
Conversation
_model_response_to_chunk yields one FunctionChunk per tool call in a delta, each paired with the same choice-level finish_reason, and the tool-call finalizer ran inside that per-chunk loop. A delta carrying N complete tool calls plus finish_reason "tool_calls" or "length" therefore rebuilt the aggregated response N times, each rebuild replacing the previous one after the buffers were reset, so only the last call survived -- silently, with a well-formed response. Any text or reasoning in the same delta was folded into the first, overwritten response and lost with it. Decide the finalization once per streamed part instead: record what the part asks for while its chunks accumulate, then finalize after the inner loop. The trigger conditions are unchanged, so the stop-only guard for LiteLLM 1.81+ partial chunks still applies. Tool calls take precedence over text for a part that asks for both, which loses nothing because _finalize_tool_call_response already carries the buffered text and reasoning. The shape does not arise through litellm.acompletion, whose CustomStreamWrapper moves finish_reason onto a trailing empty chunk, but it does arise with a custom LiteLlm.llm_client, and ADK's own tests drive this path with raw chunks.
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.
fix(litellm): keep every tool call in a finish-bearing stream chunk
finish_reasonis"length"(max output tokens reached) #4482, LiteLlmgenerate_content_asyncdoesn't handle multiple tool calls in the same model response in SSE mode #484, LiteLLM parallel tool calls does not have correct args in StreamingMode.SSE #1038Problem:
_model_response_to_chunkyields oneFunctionChunkper tool call in a delta, each pairedwith the same choice-level
finish_reason, and the tool-call finalizer ran inside thatper-chunk loop:
So a delta carrying N complete tool calls plus
finish_reason="tool_calls"(or"length")rebuilt the aggregated response N times, each rebuild replacing the previous one after
_reset_stream_buffers()had cleared the accumulator. Only the last call survived, silently,in a well-formed response — the end-of-stream fallback cannot rescue the earlier ones because
the buffers are empty and the aggregate is already set. Any
contentorreasoning_contentin the same delta was folded into the first, overwritten response and lost with it.
Scope, stated plainly: this shape does not arrive through
litellm.acompletion. ItsCustomStreamWrapperpopsfinish_reasonoff every non-empty chunk(
litellm_core_utils/streaming_handler.py) and re-emits it on a trailing empty-delta chunk,for fake-streamed and natively-streamed providers alike — I verified that end to end for
openai, azure, bedrock, vertex_ai, gemini, ollama_chat, anthropic, groq, together_ai,
hosted_vllm and custom providers. The shape does arrive via a custom
LiteLlm.llm_client(a public field), and ADK's own unit tests drive this code with raw
ModelResponseStreamchunks, i.e. through exactly the vulnerable path. So this is a latent robustness fix plus the
missing coverage, not a fix for a live provider regression. It is worth doing because the
behaviour contradicts
BaseLlm.generate_content_async's documented contract that the finalpartial=Falsechunk equals thestream=Falseoutput, and because it fails silently.The placement predates multi-call support: the check has been inside the inner loop since the
initial commit
982782014(2025-04-08), when the aggregator tracked a singlefunction_id,and was inherited unchanged by
05f48347(#759, index-keyed dict),e8019b1b(#4225, thestop-only guard),
4c6096baa(#4482, the"length"arm),36fd2c8eandeaed0aa8. No commitin that history considers more than one tool call per chunk.
Solution:
Decide the finalization once per streamed part. The per-chunk loop now records what the part
asks for; the finalization happens after the loop:
The trigger conditions are copied unchanged, so the
chunk is Noneguard that protectsagainst LiteLLM 1.81+ setting
finish_reason="stop"on partial chunks still applies exactlyas before. Tool calls take precedence over text for a part that asks for both, which loses
nothing:
_finalize_tool_call_responsealready carries the buffered text and reasoning intothe tool-call response.
I deliberately did not implement this as "merge instead of replace". The
"length"armroutes through
_parse_tool_call_argumentsand can produce an errorLlmResponse, so mergingwould need error/normal reconciliation for no benefit over deciding once.
Testing Plan
Unit Tests:
Added
_same_chunk_multiple_function_calls_stream(...)plus three cases intests/unittests/models/test_litellm.py:test_generate_content_async_same_chunk_multiple_function_calls[tool_calls]test_generate_content_async_same_chunk_multiple_function_calls[length]test_generate_content_async_same_chunk_calls_keep_text_and_reasoningNo existing fixture has more than one tool call per delta, or a tool-call delta sharing a chunk
with a finish reason, which is why
MULTIPLE_FUNCTION_CALLS_STREAM(calls in separate chunks,finish reason on its own empty chunk) passes both before and after.
All three fail on
mainand pass with the change. Reverting onlysrc/google/adk/models/lite_llm.pyand rerunning the new tests:With the change applied:
pre-commit run --files src/google/adk/models/lite_llm.py tests/unittests/models/test_litellm.pypasses (ruff, isort, pyink, addlicense, codespell, end-of-file, trailing-whitespace); the two
repo-local hooks were run directly because they shell out to
/bin/bashand a relative scriptpath that don't resolve on Windows —
scripts/compliance_checks.pyandscripts/check_new_py_files.pyboth exit 0, and the change adds no new Python files.Manual End-to-End (E2E) Tests:
Self-contained, no network and no credentials. Feeds
LiteLlmreallitellm.types.utils.ModelResponseStreamchunks through a customllm_clientacross sixstream shapes:
Before (google-adk 2.8.0 and
main@ d637d1b):After this change:
The three control shapes are unchanged, which is the point: the fix only affects a delta that
carries tool calls and a finish reason together.
Checklist
Additional context
Related but distinct, so this is not a duplicate: #4482 (closed 2026-03-10) reported a tool
call being dropped entirely when
finish_reason == "length", because that value was missingfrom the yield condition; its fix added the
"length"arm to the sameifthis PR restructures.That fixed "nothing is yielded"; this fixes "only the last of N is yielded". #484 / #1038 (fixed
by PR #759, which created this aggregation loop and the index-keyed dict) cover the
separate-chunk shape, where the finish reason arrives on its own empty chunk — the shape the
existing tests exercise and which was already correct.
Not addressed here, to keep this to one concern: text arriving after a mid-stream text
finalization is single-slot-overwritten in the same way, but
test_streaming_text_buffer_is_reset_between_aggregated_responses(from36fd2c8e) pins thecurrent last-segment-wins behaviour there, and moving the tool-call check does not affect it.
Happy to file that separately if it is worth changing.