Repository navigation
Python: fix(ag-ui): start each replayed declaration-only tool call once - #9770
eatmoreduck wants to merge 2 commits into
Conversation
Guard TOOL_CALL_START with the started-call set instead of the last-id cursor: with two declaration-only calls in one response, the cursor sits on the second when the function loop replays the pauses, so both calls re-emitted START and duplicated in the thread snapshot. Fixes microsoft#9638
agent-framework-automation
Bot
added
the
python
github-actions
Bot
changed the title
There was a problem hiding this comment.
🟡 Changes recommended
Run-wide ID deduplication suppresses distinct calls that reuse completed IDs and can corrupt their snapshot arguments.
1 open issue
What changed in this PR
This PR addresses #9638 in the Python AG-UI integration by preventing duplicate tool-call starts and snapshot entries when multiple declaration-only calls are replayed.
Changes:
- Replace the last-call-ID guard with a lookup of already registered call IDs.
- Explain the replay scenario in a code comment.
- Add a regression test checking that two replayed calls each start and appear in the snapshot once.
| File | Description |
|---|---|
python/packages/ag-ui/tests/ag_ui/test_run_common.py |
Add coverage for replaying two declaration-only calls. |
python/packages/ag-ui/agent_framework_ag_ui/_run_common.py |
Change and document the tool-call START deduplication guard. |
Review effort: Balanced
| # Why a started-set, not the last-id cursor: declaration-only calls replay after the | ||
| # function loop pauses; with two calls the cursor sits on the second, so the replay of | ||
| # the first (and then the second) re-emitted TOOL_CALL_START (#9638). | ||
| if content.name and tool_call_id not in flow.tool_calls_by_id: |
There was a problem hiding this comment.
Good catch — addressed in 803db77. The guard is now "is this call still open" (tool_call_id in flow.tool_calls_by_id and tool_call_id not in flow.tool_calls_ended) rather than run-wide id membership:
- a declaration-only replay of an open call stays deduplicated (the Python: [Bug]: AG-UI starts each tool call twice when an agent calls two declaration-only tools in one response #9638 fix holds);
- a completed id reused by a later distinct call STARTs again, replaces the snapshot entry instead of appending arguments into the old one, and
discards the ended marker so the second occurrence can emit TOOL_CALL_END again.
I first tried deduplicating by Content.id per the spec, but _emit_content can receive id-less function-call content on both the streamed and replayed paths, where the occurrence identity degrades back to the call id — the open/ended lifecycle is the signal that separates "replay of this occurrence" from "a new occurrence reusing the id" in both cases. The regression you suggested is in: stream → result → a distinct call reusing the same provider id asserts a fresh START, arguments not concatenated, and the call endable again. Full ag-ui suite: 1,695 passed.
|
@microsoft-github-policy-service agree [company=""] |
Guard TOOL_CALL_START on whether the call is still open instead of started-id membership: a completed id reused by a later distinct call STARTs again with a fresh snapshot entry and becomes endable again, while declaration-only replay of an open call stays deduplicated.
|
@microsoft-github-policy-service agree |
Motivation & Context
When a model calls two declaration-only (client-side) tools in one response, the AG-UI endpoint starts each tool call twice: the client receives two
TOOL_CALL_STARTevents per call id and the thread snapshot records each call twice, so the next run fails when the provider rejects the duplicate call ids. A single declaration-only call per response is not affected. Reported in #9638, which includes a minimal reproduction againstFlowState/_emit_content.Description & Review Guide
_emit_tool_callnow guardsTOOL_CALL_STARTwith the started-call set (tool_call_id not in flow.tool_calls_by_id) instead of the last-id cursor (tool_call_id != flow.tool_call_id), and adds a comment explaining why.TOOL_CALL_STARTand re-appended the snapshot entry. Guarding on the set makes each call start exactly once. A single call was unaffected before only because the cursor happened to equal its own id; the set makes that correctness explicit rather than incidental. Resume semantics are unchanged: a freshFlowStatestarts nothing, so replayed ids on a new run still START normally.tool_calls_by_idis only populated on the START path here (the approvalconfirm_changesand MCP paths are separate), so membership means "already STARTed by_emit_tool_callin this run".The new regression test mirrors the issue's reproduction: stream two fully-argumented function calls, then replay both as declaration-only pauses, and assert exactly one
TOOL_CALL_STARTper id and no duplicate snapshot entries (fails on main, passes with the fix).Related Issue
Fixes #9638