Skip to content

Python: feat(core): add max_duration_seconds bound and stop_reason signal to tool loop (#7587) #7772

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.

Sign up for GitHub

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

Open
Karthik Thota (karthik-0306) wants to merge 9 commits into
base: main
Choose a base branch
from
Open

Python: feat(core): add max_duration_seconds bound and stop_reason signal to tool loop (#7587) #7772

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
05ce001
feat(core): add max_duration_seconds bound and stop_reason signal to …
karthik-0306 Aug 19, 2026
7483e81
fix(core): unify batch limit decision precedence and propagate stop_r…
karthik-0306 Aug 21, 2026
10bb055
test: add missing tests for duration bounds, batch limits, and sessio…
karthik-0306 Aug 21, 2026
475d45b
Fix _apply_batch_limit_decision limits bug and Phase 1 test execution
karthik-0306 Aug 22, 2026
feb0479
fix(core): preserve budget start_time across approval resumes, dedupe…
karthik-0306 Aug 22, 2026
4ca7664
Merge main into fix-issue-7587, resolve CHNAGELOG conflict
karthik-0306 Aug 26, 2026
1d0e149
chore: remove CHANGELOG entry, updated centrally on release per review
karthik-0306 Aug 26, 2026
909beab
chore: fix ruff formatting in test_function_invocation_logic.py (exac…
karthik-0306 Aug 31, 2026
5fecf7b
Merge remote-tracking branch 'upstream/main' into fix-issue-7587
karthik-0306 Aug 31, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 25 additions & 1 deletion python/packages/core/agent_framework/_agents.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1284,10 +1284,34 @@ def _suppress_response_id(update: AgentResponseUpdate) -> AgentResponseUpdate:
return update

def _finalizer(updates: Sequence[AgentResponseUpdate]) -> AgentResponse[Any]:
return self._finalize_response_updates(
agent_response = self._finalize_response_updates(
updates,
response_format=context["chat_options"].get("response_format"),
)
# Propagate _agent_framework_stop_reason from the inner ChatResponse to the outer
# AgentResponse. _finalize_with_stop_reason sets it on the inner ChatResponse
# (a ResponseStream finalizer), but AgentResponse.from_updates only sees the
# AgentResponseUpdate sequence — none of which carries this key. The non-streaming
# path copies additional_properties directly via _build_agent_response_from_chat_response;
# for streaming we recover it here from the inner ChatResponse stored in the closure.
if inner_chat_responses:
inner = inner_chat_responses[0]
stop_reason = inner.additional_properties.get("_agent_framework_stop_reason")

@eavanvalkenburg Eduard van Valkenburg (eavanvalkenburg) Aug 26, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm not sure about this approach, is there a reason we can't use the finish_reason field, I wouldn't mind adding a custom reason into the existing FinishReasonLiteral (we would have to sync with Roger Barreto (@rogerbarreto) on a name for that and make it consistent between MEAI and here)

@karthik-0306 Karthik Thota (karthik-0306) Aug 26, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The reason we went with a separate additional_properties key rather than repurposing finish_reason is that they seemed like two different things: finish_reason today reflects why one specific model call ended (from the provider), while this is about why the framework cut the whole multi-turn loop short — a decision we make, not the model. Folding it into finish_reason on the final response would mean losing whatever the model's own real finish_reason was on that last call.

That said, this is your call to make — happy to move this to FinishReasonLiteral if that's what you and Roger land on for consistency with MEAI. Let me know what name(s) you'd like and I'll wire it up accordingly.

@eavanvalkenburg Eduard van Valkenburg (eavanvalkenburg) Aug 31, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

we had some internal discussion here, and we don't know what a user would do with this field. Updating finish_reason itself is also not ideal since that would be a breaking change in behavior. But we also do not like adding a lot of stuff into additional properties. So the question then becomes what would be the action the user needs to do that he needs this field? and are there other ways they could achieve the same?

@karthik-0306 Karthik Thota (karthik-0306) Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Two separate problems are getting conflated here, worth pulling apart first: the model confabulating a reason when it gets cut off with no explanation (from the issue) is already handled by the fallback-notice text we inject at that boundary — that talks to the model, not the caller, and doesn't touch additional_properties at all. Not what you're asking about.

For the field itself: the concrete action is a caller deciding whether to retry. If a run got cut short by max_duration_seconds or max_function_calls, retrying with a bigger budget is reasonable. If it got cut short by max_consecutive_errors, the tool's broken and retrying won't help — so which reason fired is what tells the caller whether retrying is even worth it, not just that something happened. That's the case for keeping more than a plain yes/no.

On other ways to get there: for max_iterations/max_function_calls, we don't see one — neither count is exposed anywhere else in the response today. For max_duration_seconds we did wonder if a caller could just measure elapsed time themselves, but that breaks down across approval round-trips, where the clock needs to keep running correctly across a pause of unknown length before the caller's code even runs again. We hit a real bug in that exact bookkeeping this week, so we're not confident that's something a caller could reliably redo outside the framework.

We don't have an actual caller's retry code to point to though, just the issue's own reasoning — if that's not concrete enough to justify it, that's a fair place to land.

We'll pull max_duration_seconds out as the core of this PR — no disagreement there, and it's what the issue asked for first. We'll open a separate issue for the stop-reason signal and keep working it there, so this doesn't hold up the part everyone already agrees on.

if (
stop_reason is not None
and "_agent_framework_stop_reason" not in agent_response.additional_properties
):
agent_response.additional_properties["_agent_framework_stop_reason"] = stop_reason
return agent_response

# Mutable container for the inner ChatResponse; populated by the result hook below before
# _finalizer is called (inner finalizer runs before outer finalizer per ResponseStream.map contract).
inner_chat_responses: list[ChatResponse[Any]] = []

def capture_inner_chat_response(inner: ChatResponse[Any]) -> None:
inner_chat_responses.append(inner)

stream_response = stream_response.with_result_hook(capture_inner_chat_response)

stream = stream_response.map(
transform=partial(
Expand Down
3 changes: 2 additions & 1 deletion python/packages/core/agent_framework/_harness/_tool_approval.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,8 @@ async def process(self, context: AgentContext, call_next: Callable[[], Awaitable
raise RuntimeError("ToolApprovalMiddleware requires an AgentSession.")

state = _get_state(context.session, source_id=self.source_id)
context.client_kwargs.setdefault(_FUNCTION_INVOCATION_BUDGET_STATE_KEY, {})
session_budget = context.session.state.setdefault(_FUNCTION_INVOCATION_BUDGET_STATE_KEY, {})
context.client_kwargs.setdefault(_FUNCTION_INVOCATION_BUDGET_STATE_KEY, session_budget)
context.messages = self._prepare_inbound_messages(context.messages, state, context.session)
await self._drain_auto_approvable_queue(state)
if next_queued := self._pop_next_queued_request(state):
Expand Down
Loading
Loading