Python: fix: preserve tool call/result order in AG-UI message split - #8005
Conversation
When a single Agent Framework message interleaved assistant content (function_call / text) with function_result content, the AG-UI converter emitted every tool result first and the assistant call/text segment afterwards. A [function_call, function_result] message therefore round-tripped as [function_result, function_call], reversing the transcript. Providers require a tool result to follow its matching assistant call, so the reversed order produced an orphan result (rejected or dropped) while the call stayed pending. Walk msg.contents in order and flush any accumulated assistant segment before each function_result, so calls always precede their results. Parallel results and trailing summary text are preserved, and the source message id is still kept on the first emitted message with independent generated ids for the rest. Follow-up to microsoft#7980 (addresses moonbox3's post-merge review feedback).
GitHub Actions
Inactive
GitHub Actions
Inactive
GitHub Actions
Inactive
GitHub Actions
Inactive
agent-framework-automation
Bot
added
the
python
There was a problem hiding this comment.
🔵 Needs a closer look
Provider-facing call/result serialization is explicitly classified as high risk and warrants final human review.
Pull request overview
Fixes AG-UI serialization so tool calls remain before their corresponding results.
Changes:
- Splits mixed messages while preserving content order and unique IDs.
- Adds regression tests for call/result/text ordering.
File summaries
| File | Description |
|---|---|
_message_adapters.py |
Implements ordered mixed-message splitting. |
test_message_adapters.py |
Tests ordering and ID preservation. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 0
- Review effort level: Balanced
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
GitHub Actions
Inactive
GitHub Actions
Inactive
|
/review |
![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)): 80d2a5e20df7, 598d43324088
Model: gpt-5.6-sol-fast
Overview
The new helper correctly preserves call-before-result ordering for the newly tested mixed-message shapes, keeps parallel results distinct, and assigns independent IDs to split messages. However, flushing every accumulated segment before a result regresses the supported text-before-result shape: it can place an assistant message between a pending call and its terminal result, after which production history normalization drops the real result.
Reviewed the supplied pull-request change set across correctness, security/reliability, architecture, and failure behavior.
1 verified finding remained after source verification (1 high) across 1 file. Details are attached to the affected lines below.
Affected areas: python/packages/ag-ui/agent_framework_ag_ui/_message_adapters.py
The mixed-message split flushed every accumulated segment before each function_result, including text-only segments. A [text, function_result] message whose result answers a call in a prior assistant message therefore emitted an assistant-only text message between the outstanding call and its result. _sanitize_tool_history then treats the call as abandoned, clears it, and drops the real result, leaving the provider with an unanswered tool call. Defer text-only content and emit it after the results; only a segment that carries tool calls is flushed before a result (with any buffered text coalesced), so a call still precedes its matching result and no assistant-only message is ever inserted between an outstanding call and its result. Addresses the MAF automated-review finding on this PR.
GitHub Actions
Inactive
Defer a new assistant segment while earlier emitted calls still await their results, so an interleaved batch like [call A, call B, result A, call C, result B, result C] no longer places assistant(C) between call B and result B (which _sanitize_tool_history would drop as orphaned). Extract a shared _encode_agui_segment() used by both the split and single-message paths so the text/function_call serialization can't drift.
GitHub Actions
Inactive
|
/review |
There was a problem hiding this comment.
MAF Automated Review — Iteration 2
Result: Findings reported
Scope: 2 net-new commit(s): 60afad1cdb7e, cbfdced00e7b
Model: gpt-5.6-sol-fast
Overview
The change centralizes assistant-segment encoding and adds explicit buffering plus regression coverage for text-before-result and one interleaved parallel-call sequence. Those guards preserve the previously reported cases, but the state machine still loses real tool results in two untested orderings: when an earlier call crosses a message boundary and when a deferred call's result arrives before the older batch closes. Both failures produce provider-invalid histories, so they remain Medium-risk correctness issues.
Reviewed the supplied incremental 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/ag-ui/agent_framework_ag_ui/_message_adapters.py
Carry the unresolved-call set across the whole conversion instead of rebuilding it per mixed message, so a call opened by an earlier message stays open and a later mixed message cannot place a new assistant segment between that call and its result. Queue a result whose own call is still buffered and release it only after that call is emitted, so a result never precedes its call. Add an exhaustive regression test over every valid call/result interleaving, in both single-message and split-message shapes, asserting that no result is dropped by _sanitize_tool_history and none is emitted ahead of its call.
GitHub Actions
Active
GitHub Actions
Active
Motivation & Context
Follow-up to #7980, addressing post-merge review feedback from Evan Mattson (@moonbox3): #7980 (comment)
#7980 taught
agent_framework_messages_to_aguito split a message that carries multiplefunction_resultcontents into one AG-UItoolmessage each. But it emitted all tool results first and the assistantfunction_call/textsegment afterwards. So a framework message with contents[function_call, function_result]round-tripped as[function_result, function_call]— the transcript was reversed. Providers require a tool result to follow its matching assistant call, so the reversed order produces an orphan result (rejected or dropped by the provider) while the call stays pending.Description & Review Guide
What are the major changes?
_split_mixed_message_to_agui()helper walksmsg.contentsin order and flushes any accumulated assistant segment (text +tool_calls) before eachfunction_result, then emits the result as its owntoolmessage. Calls therefore always precede their results.agent_framework_messages_to_aguinow dispatches to this helper whenever a message carries anyfunction_result; messages without a result keep the simple single-message path.What is the impact of these changes?
[function_call, function_result]→[assistant(tool_calls), tool(result)](was reversed).[function_result, function_result](parallel) and[function_result, text](result + trailing text) round-trip unchanged.What do you want reviewers to focus on?
_split_mixed_message_to_aguiand the id-assignment (first-emitted keeps the source id).Related Issue
Follow-up to #7980 (no separate issue was filed; this fixes a call/result ordering regression identified by Evan Mattson (@moonbox3) during review of the merged PR — linked above).
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.