fix: preserve function-call IDs across partial and final SSE streaming events#4619
fix: preserve function-call IDs across partial and final SSE streaming events#4619stakeswky wants to merge 1 commit intogoogle:mainfrom
Conversation
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
|
Hello @stakeswky, thank you for your contribution! Before we can merge this PR, we need you to sign the Contributor License Agreement (CLA). You can find more information and sign the CLA at https://cla.developers.google.com/. Response from ADK Triaging Agent |
a063bd1 to
d47e4c6
Compare
|
Hi @stakeswky , Thank you for your contribution! We appreciate you taking the time to submit this pull request. Please fix formatting errors by running autoformat.sh |
|
Thanks! I ran and pushed the formatting fix in commit , and re-ran the targeted unit tests ().\n\nI still need to complete CLA on my side before merge can proceed. |
|
Correction to my previous comment (shell escaped text got mangled): I ran ./autoformat.sh via uv, pushed formatting updates in commit fe469b4, and re-ran the targeted unit test file (20 passed). CLA is still pending on my side before merge can proceed. |
fe469b4 to
86288b4
Compare
|
Updated this branch to fix CLA/email issues:\n- removed merge commit authored with non-matching email\n- replaced autoformat commit authored as user@example.com\n- all commits now use stakeswky@gmail.com\n\nPlease re-run CLA checks. |
…g events
When SSE streaming is active, _finalize_model_response_event is called
once per LlmResponse chunk (partial=True for streaming chunks, partial=False
for the final). Each call creates a fresh Event via model_validate, which
means the function calls in the final event have empty IDs. When
populate_client_function_call_id runs on the final event it generates a
brand-new adk-{uuid}, different from the one assigned to the partial event.
This breaks Human-in-the-Loop (HITL) workflows using LongRunningFunctionTool:
1. Partial event yields function call with ID-A → consumer captures ID-A
2. Final event yields same function call with ID-B → ADK persists ID-B
3. Consumer submits FunctionResponse with ID-A → ADK can't find it → error
Fix: after populate_client_function_call_id assigns IDs to a partial event,
write the content (including IDs) back to model_response_event. On the next
call (final event), extract those IDs before the merge overwrites content,
then restore them by position before populate_client_function_call_id runs.
This ensures partial and final events for the same function call share the
same adk-* ID.
Fixes google#4609
Signed-off-by: stakeswky <stakeswky@gmail.com>
86288b4 to
15225be
Compare
Summary
Fixes #4609
When SSE streaming is active (the default since ADK 1.22+),
_finalize_model_response_eventis called once perLlmResponsechunk — once withpartial=Truefor each streaming chunk, and once withpartial=Falsefor the final response. Each call creates a freshEventviamodel_validate, which means the function calls in the final event have empty IDs. Whenpopulate_client_function_call_idruns on the final event it generates a brand-newadk-{uuid}, different from the one assigned to the partial event.Root Cause
This breaks HITL workflows:
FunctionResponsewith ID-A → ADK can't find it → hard errorFix
After
populate_client_function_call_idassigns IDs to a partial event, write the content (including IDs) back tomodel_response_event. On the next call (final event), extract those IDs before the merge overwrites content, then restore them by position beforepopulate_client_function_call_idruns. This ensures partial and final events for the same function call share the sameadk-*ID.Testing
Added two unit tests:
test_finalize_model_response_event_consistent_fc_id_across_partial_and_final: verifies partial and final events share the same IDtest_finalize_model_response_event_preserves_llm_assigned_id: verifies LLM-assigned IDs are not overwrittenAll 350 existing
tests/unittests/flows/llm_flows/tests pass.