fix(flows): pair the latest function response with the nearest call - #7271
chelsealong wants to merge 2 commits into
Conversation
rearrange_events_for_latest_function_response walked backward looking for the call matching the latest response's id, but never stopped once it found one. When a function-call id is reused across turns (some model providers reuse ids), the search kept walking past the nearest match and settled on the oldest call sharing that id instead, dropping every event in between and merging the response onto the wrong call. Stop at the first (nearest) match, matching the fix already applied to the sibling function rearrange_events_for_async_function_responses_in_history for the same class of bug (deee6d2, closing google#6761). Fixes google#7269
|
Thanks for this — I ran the branch against a few more histories (both ids of a batch reused, one reused id beside a fresh one, an id reused three times, id-less calls) and it pairs with the nearest call every time, with distinct-id histories unchanged. One coverage gap: on def test_rearrange_latest_response_reused_id_in_parallel_batch_pairs_with_nearest_call():
call2 = Event(author="test_agent", content=types.Content(role="model", parts=[
types.Part(function_call=types.FunctionCall(id="call_1", name="update", args={})),
types.Part(function_call=types.FunctionCall(id="call_2", name="list", args={}))]))
paused = Event(author="user", content=types.Content(role="user", parts=[
types.Part(function_response=types.FunctionResponse(id="call_1", name="update", response={"placeholder": True})),
types.Part(function_response=types.FunctionResponse(id="call_2", name="list", response={"rows": 3}))]))
events = [_call_event("call_1", "lookup"), _resp_event("call_1", "lookup", "looked up"), call2, paused, _resp_event("call_1", "update", {"applied": True})]
result = rearrange_events_for_latest_function_response(events)
assert result[:3] == events[:3]
assert [(r.id, r.response) for r in result[-1].get_function_responses()] == [("call_1", {"applied": True}), ("call_2", {"rows": 3})]Two more that change with this diff and are worth pinning: |
…atch Adds the batch-call regression test from review: on unfixed code the outer-loop walk continues past the nearest matching call event to an older one that doesn't carry every id in the batch, and the subset check raises ValueError instead of truncating.
|
Thanks for the extra coverage — added |
Link to Issue or Description of Change
1. Link to an existing issue (if applicable):
Problem:
rearrange_events_for_latest_function_response(ingoogle/adk/flows/llm_flows/tools/_rearranger.py) walks the event historybackward looking for the function-call event that matches the latest
function-response's id. When it finds a match it records the index — but the
inner
for function_call in function_calls: ... breakonly breaks the loopover calls within one event; the outer loop over event indices keeps
walking all the way back to the start of history.
If a function-call id is reused across turns (some model providers, e.g.
Gemini forwarded through LiteLLM, do this), the outer loop overwrites the
match with an earlier, already-answered call carrying the same id. Every
event between that stale call and the latest response — including
intervening user turns — is then silently dropped, and the response gets
merged onto the wrong call. This is the same bug class already fixed for the
sibling function
rearrange_events_for_async_function_responses_in_historyin
deee6d2c47(closing #6761): pair a response with the nearestpreceding call sharing its id, not the oldest one.
Solution:
Track whether the inner loop found a match and break the outer loop too, so
the search stops at the first (nearest) preceding call event with a matching
id, mirroring the fix already applied to the sibling rearranger function.
Testing Plan
Unit Tests:
Added
test_rearrange_latest_response_reused_call_id_pairs_with_nearest_callin
tests/unittests/flows/llm_flows/tools/test_rearranger.py, reproducingthe issue's repro scenario: a
lookupcall/response in turn 1, then aupdatecall reusing the same id in turn 2 with a placeholder response (HITLpause) followed by the real response (HITL resume).
Verified the test fails without the fix (checked out the pre-fix source with
git checkout HEAD~1 -- src/google/adk/flows/llm_flows/tools/_rearranger.pybefore this fix was committed):
With the fix applied:
Full unit suite (
pytest tests/unittests -n auto):16043 passed, 82 skipped, 26 xfailed, 2 xpassedplus one pre-existing, unrelated failure(
test_concurrent_prepare_tables_no_race_condition, a SQLiteprepare_tablesconcurrency test) that reproduces identically on unmodifiedupstream/mainbefore this change, confirming it is unrelated.pre-commit run --files src/google/adk/flows/llm_flows/tools/_rearranger.py tests/unittests/flows/llm_flows/tools/test_rearranger.py: all hooks passed.Manual End-to-End (E2E) Tests:
Not applicable — this is a pure function fix covered by unit tests; the
minimal repro in the issue is what the added test encodes.
Checklist
Additional context
This PR was prepared with AI assistance (Claude Code), including the root
cause analysis, patch, test, and verification steps described above.