Python: stream tool results for approval-resolution execution - #7243
Python: stream tool results for approval-resolution execution#7243he-yufeng wants to merge 3 commits into
Conversation
The approval-resolution branch of _process_function_requests executed the approved calls and spliced their results into the local history, but returned update_role=None / function_call_results=None, and the first call site in the streaming loop has no yield gate. A tool approved and executed in-run therefore ran silently: nothing streamed, so AG-UI emits no TOOL_CALL_RESULT and the result is absent from MESSAGES_SNAPSHOT, asymmetric with statically executed approvals. Populate update_role/function_call_results from the approval branch when calls were actually executed, and mirror the existing yield gate at the first call site. Adds a regression test that approves a guarded tool over a streaming run and asserts the function_result shows up.
There was a problem hiding this comment.
Pull request overview
This PR fixes a streaming gap in the Python function-invocation layer where tools executed during approval resolution (rather than during the model’s normal tool-call turn) would run but never emit streamed function_result updates, preventing downstream streaming consumers (e.g., AG-UI) from observing TOOL_CALL_RESULT events.
Changes:
- Return
update_role="tool"andfunction_call_resultsfrom_process_function_requestswhen approval-resolution execution actually runs tools. - Add a yield gate after the first
_process_function_requestscall in the streaming loop so approval-resolution tool results stream like normal tool results. - Add a regression test covering approval-resolution tool-result streaming.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| python/packages/core/agent_framework/_tools.py | Streams tool results produced during approval-resolution by returning update_role/results and yielding a ChatResponseUpdate in the first streaming loop call site. |
| python/packages/core/tests/core/test_harness_tool_approval.py | Adds a regression test asserting approval-resolution execution produces a streamed function_result update. |
| "update_role": "tool" if executed_count else None, | ||
| "function_call_results": approved_function_results or None, |
There was a problem hiding this comment.
Good catch, fixed in a406324. The branch now checks for user-input-request items the same way _handle_function_call_results does, and those go out as assistant updates instead of being filtered out by the function_result count. Covered by a new regression test that drives a tool raising UserInputRequiredException through approval resolution and asserts the update lands with the assistant role.
| tool_results = [ | ||
| content | ||
| for update in second_updates | ||
| if update.role == "tool" | ||
| for content in update.contents | ||
| if content.type == "function_result" | ||
| ] | ||
| assert tool_results, "approval-resolution execution must stream its function_result" | ||
| assert calls == 1 |
There was a problem hiding this comment.
Tightened in a406324. The test now asserts exactly one streamed function_result with call_id == "call_guarded" and result == "guarded result", so a wrong or duplicated result would fail it.
|
@he-yufeng please have a look at Copilot's comments. |
…pdates The approval-resolution branch labeled the stream by result count only, so a user-input request raised mid-execution was dropped from the stream. Mirror _handle_function_call_results: those items go out as assistant updates. Also tighten the streaming regression test to assert call_id, result, and count.
GitHub Actions
Failure
|
Thank you @he-yufeng for identifying the approval-resolution streaming gap and for contributing the implementation and focused tests. We have consolidated this fix with rejected/non-streaming parity, immutable message handling, replay correlation, and the shared function-loop specification in draft PR #7345. Closing this PR as superseded so the remaining review can happen in one place. |
Fixes #7241.
Problem
When a tool is executed while resolving an approval (rather than in the model's normal tool-call turn), its
function_resultnever reaches streaming consumers. The approval-resolution branch of_process_function_requestsruns the approved calls and splices results into the local history for the next model call, but returnsupdate_role=None / function_call_results=None— and the first_process_function_requestscall site in the streaming loop has no yield gate. The approved tool fires its side effect invisibly: noTOOL_CALL_RESULTfrom the AG-UI emitter, and the result is absent fromMESSAGES_SNAPSHOT, asymmetric with statically executed approvals (which AG-UI emits explicitly via_make_approval_tool_result_events).Fix
update_role="tool"and the executedapproved_function_resultswhenever calls actually ran (rejected-only approvals still stream nothing).role="tool"update just like normal tool results.Normal (non-approval) tool streaming is untouched.
Verification
test_tool_approval_resolution_streams_tool_results: approves a guarded tool over a streaming run and asserts thefunction_resultappears in the updates. Fails without the fix (assert []), passes with it.pytest packages/core/tests/core/test_harness_tool_approval.py packages/core/tests/core/test_tools.py packages/core/tests/core/test_function_invocation_logic.py— 211 passed, 2 skipped.test_run.py -k approval) — 6 passed.