Repository navigation
Python: feat(core): add max_duration_seconds bound to tool invocation loop - #7772
Eduard van Valkenburg (eavanvalkenburg) merged 12 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Adds duration bounds and machine-readable termination reasons to Python function-invocation loops.
Changes:
- Adds and validates
max_duration_seconds. - Tracks stop reasons across streaming and non-streaming paths.
- Adds tests and changelog documentation.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
python/packages/core/agent_framework/_tools.py |
Implements duration tracking and stop reasons. |
python/packages/core/tests/core/test_function_invocation_logic.py |
Tests duration limits and termination signals. |
python/CHANGELOG.md |
Documents the new behavior. |
Suppressed comments (2)
python/packages/core/agent_framework/_tools.py:3528
- The streaming path has the same enforcement gap: approved calls are replayed before this check, while the call-dropping/fallback logic at lines 3449-3466 recognizes only
max_function_calls. Consequently, an expired approval or a provider-emitted call despitetool_choice="none"can still execute. Include duration expiry in a shared pre-execution and fallback predicate.
if (
max_duration_seconds is not None
and (perf_counter() - budget_state["start_time"]) >= max_duration_seconds
):
python/packages/core/agent_framework/_tools.py:3518
- The streaming branch also leaks the internal action name
"stop"as a public stop reason. This is outside the documented value set and differs from approval-time error exhaustion, which reportscompleted. Use the same documented semantic reason for consecutive-error exhaustion in both paths.
budget_state.setdefault("stop_reason", "stop")
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
0ef8220 to
05ce001
Compare
…eason to AgentResponse
- **agent-framework-core**: Refactored _apply_batch_limit_decision to compute perf_counter exactly once per decision point, eliminating the structural fragility where the threshold check and log message used separate clock samples. - **agent-framework-core**: Re-ordered limit checking in Phase 1 to execute before approval response resolution, successfully preventing execution during approved replays when limits are reached. A post-approval check ensures consecutive error limits (�ction == stop) remain handled. - **agent-framework-core**: Rewrote 6 tests in est_function_invocation_logic.py that used a fragile call_count mock. The tests now use a mutable clock array that advances directly during the tool execution semantic step, providing true robustness against internal engine refactors. Note: The fallback response trigger (_ensure_function_invocation_limit_fallback_response) remains scoped strictly to the function call limit, preserving pre-existing behavior. Expanding this to cover consecutive errors (�ction == stop) or duration timeouts is intentionally left out of scope for this fix.
… streaming limit decision, fix double-counted approval calls against max_function_calls
|
Karthik Thota (@karthik-0306) I replied on the comment thread, please have a look |
|
Overal, this look good Karthik Thota (@karthik-0306) I am a bit on the fence myself on whether we should or should not include waits between runs, the pro, as you call out, is that waiting long for a approval can be part of the run, thereby controlling the overall throughpuyt, however the con of that is that we designed approval to exit the agent run altogether, because it might be offloaded to some other system and not come back until days later, and so there is a meaningful difference between two agents that runs for 3 days, where 1 waits 48 hours for a approval, and the other never waits. And what if the approval wait times range from seconds to days, then you can never really set a meaningful overall timeout. But like I said, I am on the fence... curious about your ideas for this. CC westey (@westey-m) |
Appreciate you laying out both sides rather than deciding fast — worth getting right. Before this issue there was no wall-clock bound at all, so either direction is a net win for the framework — no stake here in defending the current behavior. My lean: ship the simpler version now — not less work, but we don't have real data yet on how approval wait times actually distribute (seconds vs. days). Building pause/resume against a guess feels premature; building it once we see real usage doesn't. If you'd rather go pause/resume from the start:
Whatever you and Westey land on, happy to build it. |
Motivation & Context
Function invocation loops in FunctionInvocationLayer (_tools.py) currently allow capping LLM roundtrips via max_iterations and total function calls via max_function_calls, but lack a wall-clock time limit. Unattended or complex agent runs can execute tools repeatedly and stall for long periods without a bounded total duration.
This PR addresses #7587 by introducing max_duration_seconds to FunctionInvocationConfiguration.
The issue's motivating scenario is a single, continuous, unattended loop execution. This PR implements that core case, and additionally extends the duration bound to persist across human-approval round-trips.
Description & Review Guide
What are the major changes?
What is the impact of these changes?
Related Issue
Fixes #7587
Contribution Checklist