Skip to content

Python: fire loop turn-scoped after_run only on clean streaming exit - #9818

Draft
RongJie G (CorgiBoyG) wants to merge 1 commit into
microsoft:mainfrom
CorgiBoyG:fix/loop-turn-scoped-after-run-clean-exit
Draft

RongJie G (CorgiBoyG) wants to merge 1 commit into
microsoft:mainfrom
CorgiBoyG:fix/loop-turn-scoped-after-run-clean-exit

Conversation

@CorgiBoyG

Copy link
Copy Markdown
Contributor

Motivation & Context

A streaming AgentLoopMiddleware run fires turn-scoped after_run providers (after_run_once_per_turn = True, e.g. CompactionProvider) from a finally block, so an iteration that raises — or an early stream close — still runs them with the stale final response from a prior iteration. The non-streaming path keeps that call outside its try/finally, and a normal non-loop run only fires result hooks on successful finalization. Streaming therefore diverged from both, silently advancing once-per-turn finalization (e.g. the compaction/summarization turn boundary) off a superseded response on a failed turn.

Description & Review Guide

  • What are the major changes? In the streaming loop generator, fire the turn-scoped after-providers only on a clean loop exit. The three normal-stop returns become break, a completed_cleanly flag is set after the loop body, and the finally call is gated on that flag. An exception, or an early aclose()/GeneratorExit, leaves the flag unset and skips the call. Only the loop-iteration token cleanup remains unconditional in finally.
  • What is the impact of these changes? Normal completion is unchanged (should_continue stop, max_iterations, approval escape hatch, MiddlewareTermination all still fire turn-scoped after_run exactly once). Exception and early-close paths now skip it, matching the non-streaming path and the non-loop run contract.
  • What do you want reviewers to focus on? That every clean stop still reaches completed_cleanly = True before the generator is finalized, and that no exception/early-close path sets it.

Contribution Checklist

  • I have read the CONTRIBUTING.md.
  • The code builds and tests pass locally.
  • I have added tests to cover my changes (deterministic red-on-main / green-on-fix: streaming exception, streaming early-close, non-streaming exception control; verified by reverting only the production file).
  • All unit tests pass locally (tests/core/: 7056 passed, 19 skipped, 0 failed).

AI Assistance

This is an AI-assisted contribution. The defect was found by source audit; AI assisted with drafting the regression tests, the fix, and this description. The root-cause analysis, the empirical contract verification, and the red-on-main / green-on-fix validation were reviewed and confirmed by me.

Related Issue

Fixes #9817

Draft pending maintainer confirmation of the scoped fix direction.

The streaming AgentLoopMiddleware path fired turn-scoped after_run providers
(after_run_once_per_turn, e.g. CompactionProvider) from a finally block, so an
iteration that raised - or an early stream close - still ran them with the
stale final response from a prior iteration. That diverged from the
non-streaming path and the normal non-loop run contract, both of which skip
after_run when the run raises.

Gate the streaming finally call on a completed_cleanly flag set only after a
clean loop exit (should_continue stop, max_iterations, approval escape hatch,
or MiddlewareTermination), converting the normal-stop returns to break. Normal
completion behavior is unchanged; exception and early-close paths no longer
fire turn-scoped after_run.

Adds deterministic regressions for the streaming exception, streaming
early-close, and the non-streaming exception control.

Fixes microsoft#9817

Signed-off-by: CorgiBoyG <111257566+CorgiBoyG@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No active review issues were identified for the change restricting streaming turn-scoped finalization to clean loop exits.

No open issues 🎉

What changed in this PR

Addresses #9817 by aligning streaming loop turn-scoped after_run behavior with non-streaming runs, preventing finalization with a stale response after an exception or early stream closure.

Changes:

  • Gates turn-scoped providers on clean loop completion while preserving unconditional iteration-token cleanup.
  • Converts normal-stop returns to breaks so successful exits still trigger finalization.
  • Adds regression tests for streaming exceptions, early closure, and the non-streaming exception control.
File Description
python/​packages/​core/​tests/​core/​test_harness_loop.py Adds regression coverage for failed and early-closed loop runs.
python/​packages/​core/​agent_framework/​_harness/​_loop.py Restricts streaming turn-scoped finalization to clean exits.

Review effort: Balanced

This branch was previously deployed

1 inactive deployment
github-app-auth — 40fb9acc Deployed Oct 29, 2026 by CorgiBoyG via team_check #7951
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

python Usage: [Issues, PRs], Target: Python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Python: [Bug]: AgentLoopMiddleware fires turn-scoped after_run on a failed/early-closed streaming iteration using a stale prior response

2 participants