Python: add ag-ui tool result display channel - #5762
Conversation
Python Test Coverage Report •
Python Unit Test Overview
|
||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Pull request overview
This PR extends the Python AG-UI integration to support a separate, UI-only tool result display payload (via a reserved Content.additional_properties key), allowing tools to send concise text back to the LLM while streaming richer structured content to the frontend in TOOL_CALL_RESULT.
Changes:
- Add
tool_resultsupport tostate_update(...), backed byTOOL_RESULT_DISPLAY_KEY="__ag_ui_tool_result_display__", with symmetric fallback behavior between LLM text and UI display content. - Update the AG-UI emitter to route the display payload only to
ToolCallResultEvent.contentwhile keepingflow.tool_results[].contentas the LLM-bound text. - Add/extend unit + golden tests and document the helper usage in the package README.
Reviewed changes
Copilot reviewed 5 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| python/packages/ag-ui/agent_framework_ag_ui/_state.py | Introduces TOOL_RESULT_DISPLAY_KEY and extends state_update to carry a UI-only display payload (with fallback into text when text is empty). |
| python/packages/ag-ui/agent_framework_ag_ui/_run_common.py | Extracts the display marker and routes UI vs LLM tool result content to the appropriate channels. |
| python/packages/ag-ui/tests/ag_ui/test_run_common.py | Adds unit tests for marker construction and channel routing behavior. |
| python/packages/ag-ui/tests/ag_ui/golden/test_scenario_deterministic_state.py | Adds golden scenarios validating UI-only display payload behavior in the streamed event sequence. |
| python/packages/ag-ui/README.md | Documents state_update(..., tool_result=...) usage for split UI/LLM payloads. |
| .gitignore | Ignores **/issues/ directories. |
There was a problem hiding this comment.
Automated Code Review
Reviewers: 4 | Confidence: 91%
✓ Correctness
The PR cleanly adds a tool_result display channel to state_update by piggybacking on the existing additional_properties pattern. The refactoring of _extract_tool_result_state into the generic _extract_tool_result_marker_values helper preserves the original merge semantics. The dual-channel routing in _emit_tool_result_common correctly sends display content to the UI event while keeping the LM-bound flow.tool_results unchanged. Sentinel objects (_UNSET, _MISSING) are properly module-scoped and compared with 'is'. The backward-compatible relaxation of 'state' from required to optional is correctly guarded. No correctness issues found.
✓ Security Reliability
The PR adds a tool result display channel via a reserved
additional_propertieskey (__ag_ui_tool_result_display__), mirroring the existing__ag_ui_tool_result_state__pattern. The implementation is clean from a security and reliability perspective: sentinel objects are properly scoped and compared withis, marker keys are extracted and stripped so they don't leak to UI event content or LLM-bound flow, the_extract_tool_result_marker_valuesrefactoring preserves existing state-extraction behavior, serialization avoids double-encoding for string values, and both standard and MCP code paths are updated consistently. Thestateparameter change from required to optional is backward compatible. No injection, deserialization, resource leak, or unhandled failure concerns found.
✓ Test Coverage
Test coverage for the standard _emit_tool_result path is thorough, with unit tests at the state_update construction level, the _emit_tool_result routing level, and golden integration level. However, the MCP tool result path (_emit_mcp_tool_result) was updated in production code to extract and forward the display_result marker, but no corresponding test was added — the existing TestEmitMcpToolResultWithState class only covers the state marker. This is the only actionable coverage gap; the rest of the new behavior is well-tested including edge cases like fallback, pre-serialized strings, and coexistence with state snapshots.
✗ Design Approach
The split UI/LLM tool-result design looks sound in the main streaming paths, but it is not applied to the approval-resolution path. That means approved tools that return
state_update(..., tool_result=...)will still send the LM-facing text to the UI, so the new feature is incomplete for a real execution path.
Flagged Issues
- Approved tool executions bypass the new display-channel extraction. The display_result flow is wired through _emit_tool_result() / _emit_mcp_tool_result() in _run_common.py, but approved results are emitted via _make_approval_tool_result_events() in _agent_run.py:383-398, which always builds the UI event from resolved.result. Since _try_execute_function_calls() wraps tool output via Content.from_function_result() (in _tools.py:1580-1589), state_update(..., tool_result=...) markers remain on items but the approval path ignores them, silently losing the UI-only payload override.
Automated review by moonbox3's agents
Key decisions: - Add TOOL_RESULT_DISPLAY_KEY and make state_update accept optional state plus a tool_result display payload. - Keep text as the LLM-bound tool result while using the display marker only for ToolCallResultEvent.content. - Reuse one outer/inner Content additional_properties extraction helper for state and display markers, preserving fallback behavior when display is absent. Files changed: - python/packages/ag-ui/agent_framework_ag_ui/_state.py - python/packages/ag-ui/agent_framework_ag_ui/_run_common.py - python/packages/ag-ui/tests/ag_ui/test_run_common.py - python/packages/ag-ui/tests/ag_ui/golden/test_scenario_deterministic_state.py - python/issues/done/01-tool-result-display-channel.md Blockers/notes: - Slice 1 is complete and moved to issues/done. - Slice 2 remains for docstring and README documentation.
Key decisions: - Document state_update as the single helper for LLM text, UI-only tool_result display content, and durable shared state. - Keep the display guidance explicit that text remains LLM-bound while tool_result feeds ToolCallResultEvent.content. - List both reserved additional_properties markers in the docstring return contract. Files changed: - python/packages/ag-ui/agent_framework_ag_ui/_state.py - python/packages/ag-ui/README.md - python/issues/done/02-docs-tool-result-display.md Blockers/notes: - Slice 2 is complete and moved to issues/done. - Verification passed: uv run poe syntax -P ag-ui --check; uv run poe test -P ag-ui; uv run poe markdown-code-lint; uv run ruff check packages/ag-ui/agent_framework_ag_ui/_state.py. - Commit hooks were skipped after poe-check repeatedly rewrote uv.lock ordering; the same checks were run manually and passed.
Motivation and Context
Closes #5760. Resolves discussion #5391.
Today the ag-ui emitter sends one string to both
ToolCallResultEvent.content(UI) andflow.tool_results[].content(LLM). Tool authors with rich structured output have to choose: thin string the UI can't render, or large JSON the LLM doesn't need but pays tokens for every turn.Description
Extends
state_updatewith an optionaltool_resultparameter that overrides only the UI-bound payload. The LLM still receivestext. Backed by a private reserved key (__ag_ui_tool_result_display__) inContent.additional_properties, mirroring the existing__ag_ui_tool_result_state__pattern.Channel fallback is symmetric — each channel defaults to the other when unset, so existing tools are byte-identical.
stateis now optional. Non-stringtool_resultis JSON-serialised via the existingmake_json_safepath. Includes unit tests and three new golden scenarios intest_scenario_deterministic_state.py.Contribution Checklist