Python: Fix streamed parallel tool calls merging into the wrong call - #8337
Conversation
_process_update only ever merged an incoming function_call chunk into message.contents[-1], so once two tool calls are in flight at the same time their interleaved argument deltas got mashed together or split into duplicate, unmergeable fragments instead of each call ending up complete. Match the merge target by call_id across all in-progress calls, falling back to the trailing item for continuation deltas that never repeat their call_id. Fixes microsoft#8336
GitHub Actions
Active
GitHub Actions
Active
agent-framework-automation
Bot
added
the
python
GitHub Actions
Active
GitHub Actions
Active
There was a problem hiding this comment.
🟡 Changes recommended
Tagged chunks can still merge into an unrelated untagged trailing call when no matching call ID is found.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Fixes aggregation of interleaved streamed parallel function calls by matching their call IDs.
Changes:
- Adds call-ID-aware function-call merging.
- Adds regression and fallback tests.
File summaries
| File | Description |
|---|---|
python/packages/core/agent_framework/_types.py |
Adds targeted function-call chunk aggregation. |
python/packages/core/tests/core/test_types.py |
Tests interleaved calls and ID-less continuations. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Balanced (auto)
Note
Copilot is running an experiment and ran this review at Balanced.
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
Content.__add__ only rejects a function_call merge when both sides carry a call_id and they differ, so a chunk tagged with a brand-new call_id could still fall into the trailing-item fallback and get silently absorbed by an untagged trailing call, adopting its id and mashing two unrelated calls' arguments together. Reserve that fallback for chunks that carry no call_id at all; a tagged chunk with no matching in-progress call is always a new call and should be appended.
GitHub Actions
Active
|
/review |
There was a problem hiding this comment.
🟡 Changes recommended
Matching solely by call_id regresses occurrence-aware streaming when IDs are reused or assigned late.
Get a fresh assessment by requesting another Copilot review.
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Balanced (auto)
Note
Copilot is running an experiment and ran this review at Balanced.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
MAF Automated Review — Iteration 1
Result: Findings reported
Scope: full PR (2 commit(s)): 2f3ae616972b, 3467e10007ed
Model: gpt-5.6-sol-fast
Overview
The PR correctly correlates interleaved tagged function-call deltas and preserves the legacy trailing-call fallback only for untagged continuations, with focused regressions for both behaviors. The provider path confirms OpenAI Responses deltas carry stable call IDs, and conflicting occurrence IDs prevent many accidental merges. Two residual issues remain: the unbounded backward scan can reopen a completed reused ID when occurrence identity is absent, and it makes finalization quadratic for streams containing many distinct calls.
Reviewed the supplied pull-request change set across correctness, security/reliability, architecture, and failure behavior.
2 verified findings remained after source verification (2 medium) across 1 file. Details are attached to the affected lines below.
Affected areas: python/packages/core/agent_framework/_types.py
Content.__add__ only rejects a function_call merge on id mismatch when both sides carry an id. The Chat Completions client stamps a stable occurrence id on every chunk it emits, but an untagged chunk (id=None) that happens to share an already-identified call's call_id slipped past that guard and got merged anyway, silently corrupting a finished call's arguments if a provider ever reused the call_id. Skip candidates whose id is already set when the incoming chunk has none, and keep scanning instead of assuming a call_id match is proof enough.
GitHub Actions
Active
|
Thanks for the update. Before this is ready, could you please:
Once those are addressed, please re-request review. Thanks! |
|
Resolved all three threads. Quick recap: the first one was a real bug (fixed in 173add7 — an untagged chunk could get merged into an already-identified call if the provider ever reused a call_id), the other two I pushed back on with a repro and evidence in-thread rather than changing code, happy to revisit either if you disagree. Couldn't hit the re-request-review button through the API since you're already a pending reviewer rather than someone who submitted a formal review, so flagging here instead — ready for another look. |
Motivation & Context
Parallel tool calling is a normal thing for a model to do — "get the weather in
NYC and the time in Boston" is one turn, two tool calls. When those two calls
stream, their argument fragments don't have to arrive one call fully at a time;
OpenAIResponsesClientalready tags everyresponse.function_call_arguments.deltaevent with its call's real
call_idspecifically because it can't assume thatordering. The shared response-aggregation code in
_types.pydidn't honor that:it only ever merged an incoming
function_callchunk into whatever the lastcontent item happened to be, regardless of whether that item was actually the
call the chunk belonged to. The result is silent corruption — one call's
arguments end up empty, the other's end up as an invalid-JSON mash of both
calls' fragments, or both calls get split across several duplicate, never-
reassembled content items. None of this raises an exception, so it surfaces
downstream as a confusing
JSONDecodeErrorinparse_arguments()or, worse, atool invoked with the wrong arguments.
Description & Review Guide
_process_updateinagent_framework/_types.pynow merges a streamedfunction_callchunk intothe specific in-progress call it belongs to.
_merge_function_call_contentmatches by
call_idacross all of the message's existing content (not justthe trailing item) when the chunk carries one, and only falls back to
merging with the trailing
function_callitem for chunks that don't carry acall_idat all — which some providers only send on the first chunk of acall, so that fallback preserves today's single-call-at-a-time behavior.
more concurrent tool calls now aggregate correctly regardless of how their
argument deltas interleave. This is provider-agnostic: it fixes any client
that (like
OpenAIResponsesClientalready does) tags every delta with thecall's real
call_id, with no client-specific changes required.path for call_id-less continuation chunks is the right compromise — it's
needed to keep existing single-call streaming (e.g. the legacy OpenAI Chat
Completions API, which only sends
call_idon a tool call's first delta)working exactly as before, but by construction it can't disambiguate two
concurrent untagged calls. Fixing that fully means teaching that one
client to track its own
index→call_idmapping, which felt like aseparate, client-specific change rather than something the shared
aggregation layer should own — noted as follow-up context in the issue.
Related Issue
Fixes #8336
Contribution Checklist
breaking changelabel (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.