-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Python: defer turn-scoped after_run providers to the agent loop boundary #7289
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
17dd6e5
213829c
97bcea5
6dc2e17
228c9a6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -84,6 +84,15 @@ | |
|
|
||
| logger = logging.getLogger("agent_framework") | ||
|
|
||
| # AgentLoopMiddleware stamps this key into the run options while a loop | ||
| # iteration is running, so providers scoped to the whole user turn | ||
| # (``after_run_once_per_turn``) skip their per-iteration ``after_run`` and only | ||
| # fire once at the loop boundary. It rides the run's options rather than a | ||
| # context variable: options reach only the runs the loop itself drives, so a | ||
| # nested ``agent.run()`` (fresh options, its own session) keeps its own turn, | ||
| # and nothing leaks into the caller's context while a stream is paused. | ||
| _LOOP_ITERATION_TOKEN_KEY = "_agent_loop_iteration" | ||
|
|
||
| if TYPE_CHECKING: | ||
| ResponseModelBoundT = TypeVar("ResponseModelBoundT", bound=BaseModel) | ||
| else: | ||
|
|
@@ -545,6 +554,7 @@ async def _run_after_providers( | |
| *, | ||
| session: AgentSession | None, | ||
| context: SessionContext, | ||
| only_per_turn: bool = False, | ||
| ) -> None: | ||
| """Run after_run on all context providers in reverse order. | ||
|
|
||
|
|
@@ -557,6 +567,10 @@ async def _run_after_providers( | |
| Keyword Args: | ||
| session: The conversation session. | ||
| context: The invocation context with response populated. | ||
| only_per_turn: When True, run only providers that opted into | ||
| once-per-turn semantics (``after_run_once_per_turn``); used by | ||
| AgentLoopMiddleware when a loop ends. When False, those | ||
| providers are skipped while a loop iteration is in progress. | ||
| """ | ||
| if _defer_run_persistence(partial(self._run_after_providers, session=session, context=context)): | ||
| return | ||
|
|
@@ -570,9 +584,17 @@ async def _run_after_providers( | |
| per_service_call_history_required = self.require_per_service_call_history_persistence and any( | ||
| isinstance(provider, HistoryProvider) for provider in self.context_providers | ||
| ) | ||
| # The loop stamps the runs it drives via their options; anything else | ||
| # (nested run, caller-side run while a stream is paused) is its own turn. | ||
| in_loop_iteration = context.options.get(_LOOP_ITERATION_TOKEN_KEY) is not None | ||
| for provider in reversed(self.context_providers): | ||
| if per_service_call_history_required and isinstance(provider, HistoryProvider): | ||
| continue | ||
| once_per_turn = getattr(provider, "after_run_once_per_turn", False) | ||
| if only_per_turn and not once_per_turn: | ||
| continue | ||
| if in_loop_iteration and once_per_turn: | ||
| continue | ||
|
Comment on lines
+593
to
+597
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in 213829c. The contextvar now carries the looping agent instance and _run_after_providers only suppresses when it matches self, so a nested agent.run() inside an iteration keeps firing its own turn-scoped providers. A new test drives an inner agent from a tool inside a loop and asserts its provider still fires once. |
||
| if provider_session is None: | ||
| raise RuntimeError("Provider session must be available when context providers are configured.") | ||
| await provider.after_run( | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could the suppression state distinguish a loop invocation rather than only the agent instance? If an iteration recursively runs the same
Agentwith a separate session, the nested loop's boundary still sees the outer marker and skips its turn-scoped provider; the outer boundary later flushes only the outer session. That leaves the nested session without compaction or any other opted-in persistence hook.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same fix covers this: the stamp now travels in run options, which are fresh per
agent.run(), so a nested loop on the same agent with its own session is governed by its own stamp, not the outer one. Its boundary flush fires normally and only the outer session's flush is driven by the outer stamp.test_nested_same_agent_run_with_separate_session_is_not_suppresseddrives exactly that shape and asserts both boundary flushes fire.