feat(delegate): flag-gated per-leg call budget via orchestrator max_iterations (Layer 1) - #325
Merged
Merged
Conversation
Brian Krabach (bkrabach)
pushed a commit
to ramparte/amplifier-foundation
that referenced
this pull request
Aug 27, 2026
Reframes this PR's timeout as Layer 3 of the "Layered Bounding for Delegated Sessions" design (spec: 298-replacement) -- the orchestrator- independent wall-clock backstop that sits behind a per-leg LLM-call budget (Layer 1, see microsoft#325 and companion PR microsoft/amplifier-module-loop-streaming#43), not the primary bound. Net functional change: `1800` -> `14400` for `settings.timeout`'s default. Everything else in this PR is kept verbatim: `_DelegateTimeoutExpired`, `_validate_timeout`, `_await_child_with_deadline`, the hard parent-release semantics, the honest `resumable: false` / `resume_status: pending_child_cleanup` reporting, and all 30 focused timeout tests (only the default-value assertions are retargeted). Why 14400s: ~12x the measured healthy sub-session upper bound (996-1168s), and ~2x below the worst observed runaway (17h34m) -- generous enough that a working Layer 1 budget should make this backstop fire zero times in practice. If it ever fires with Layer 1 active, that's a Layer 1 bug report, not evidence this default is wrong. Docs updated to frame this as the backstop: module docstring, README's "Delegate Timeout" section (retitled "Layered bounding: call budget (Layer 1) + wall-clock backstop (Layer 3)"), and the settings.timeout config comment. Files: - modules/tool-delegate/amplifier_module_tool_delegate/__init__.py: module docstring `settings.timeout` description; default 1800 -> 14400 - modules/tool-delegate/README.md: "Delegate Timeout" section rewritten as "Layered bounding"; config example comment - modules/tool-delegate/tests/test_delegate_timeout.py: default-value assertions retargeted to 14400 (T3.1) Testing: - modules/tool-delegate/tests/test_delegate_timeout.py: 30 passed - Full tool-delegate module suite: 90 passed (zero regressions) - Full foundation repo suite (tests/, this branch's own base): 1549 passed - python_check: no new issues vs this branch's own baseline (pre-existing I001 import-sort warning unchanged, confirmed via stash diff) 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
Brian Krabach (bkrabach)
added a commit
to microsoft/amplifier-module-loop-streaming
that referenced
this pull request
Aug 27, 2026
… (Layer 1) (#43) feat(orchestrator): flag-gated per-leg call budget via max_iterations (Layer 1) Ships dark: max_iterations call-budget behavior defaults to unlimited/off — zero behavior change until an operator opts in. Includes one real correctness fix: budget wrap-up call now sends tools=None so the final call must produce a summary. 185/185 tests green. Companion to microsoft/amplifier-foundation#325. Self-authored; admin-merged at explicit user/operator direction (self-approval not possible; see PR comment for full basis).
…terations (Layer 1) Layered Bounding for Delegated Sessions (spec: 298-replacement, replacing the wall-clock-only default in #298). Adds a per-session-leg LLM-call budget as the first line of defense in front of the delegate's existing settings.timeout wall-clock backstop, delivered with zero new kernel surface: tool-delegate writes max_iterations (and a new budget_warn_ratio) into the orchestrator_config dict it already passes to spawn_fn, and amplifier-app-cli's session_spawner already does a caller-wins .update() into the child's config -- zero app-cli changes needed. Enforcement itself lives in the orchestrator loop (see companion PR microsoft/amplifier-module-loop-streaming#43), which already counts LLM calls via max_iterations and already exits exhaustion via a normal return (graceful wrap-up), so the resulting transcript is complete and resumable -- unlike a cancellation-based timeout. Ships DARK: settings.max_llm_calls defaults to None, so no budget is injected into any child session and orchestrator_config is byte-for-byte what it was before this change. Nothing here changes behavior until an operator explicitly sets settings.max_llm_calls. Precedence chain (highest first): 1. Per-call tool input (`max_llm_calls`) -- implemented 2. Per-agent frontmatter (`agents[name]["budget"]["max_llm_calls"]`) -- NOT implemented, see below 3. This module's settings.max_llm_calls (default None) -- implemented 4. Inherited parent orchestrator_config's max_iterations -- implemented (the pre-existing inheritance path, left untouched when no budget applies) Per-agent frontmatter override (rank 2) does not ship: verified empirically (not just read from source) that a top-level `budget:` block in an agent .md's frontmatter is dropped by amplifier_foundation.bundle._dataclass._load_agent_file_metadata, which only forwards a fixed allowlist of top-level keys (tools, providers, hooks, session, provider_preferences, model_role, agents) -- budget is not among them. Reproduced in tests/test_delegate_call_budget.py::test_agent_frontmatter_budget_key_is_dropped. Ranks 1, 3, and 4 ship; rank 2 is a follow-up requiring a change to the frontmatter loader itself, documented in this module's README "Known gaps" section. Also adds: - Eager validation (_validate_call_budget / _check_call_budget_type): reject bool, non-int, and negative values at the point supplied (module construction for the settings default, execute() for the per-call override) -- never at spawn time. - Negotiated-feature warning (spec §4.4): if a budget was requested but the child's orchestrator reports no llm_call_budget telemetry (e.g. a third-party orchestrator with no max_iterations support), logs a warning and sets metadata.budget_enforced = false on the returned ToolResult, so the gap is loud rather than silent. - max_llm_calls entry in the tool's input schema (kept a pure literal for the static token-cost estimator). Files: - modules/tool-delegate/amplifier_module_tool_delegate/__init__.py: _check_call_budget_type / _validate_call_budget module functions; settings.max_llm_calls / budget_warn_ratio in __init__; per-call max_llm_calls parsing + validation in execute(); _resolve_call_budget method; orchestrator_config build (copy-not-mutate + budget injection) and negotiated-feature warning in _spawn_new_session; max_llm_calls schema entry - modules/tool-delegate/README.md: "Layer 1 call budget" section + "Known gaps" - modules/tool-delegate/tests/test_delegate_call_budget.py (new): T2.1, T2.2, T2.4, T2.5, T2.6, T2.7, T2.8, T2.9, T2.10, T2.11 + the frontmatter round-trip verification test (14 tests) Testing: - New tests: 14 passed - Full tool-delegate module suite: 80 passed (was ~66; zero regressions) - Full foundation repo suite (tests/): 1634 passed, 1 skipped -- matches pre-change baseline exactly - ruff/pyright: no new issues vs baseline Part of the 298-replacement design (Layer 1 of 3). Companion PR: microsoft/amplifier-module-loop-streaming#43 (orchestrator-side enforcement). #298 is being revised separately to reframe its wall-clock default as the Layer 3 backstop behind this budget. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
Brian Krabach (bkrabach)
force-pushed
the
feat/delegate-call-budget
branch
from
August 27, 2026 15:08
b96829b to
3050860
Compare
Collaborator
Author
|
Rebased Conflict resolution — both additive features kept in full, nothing dropped:
Testing (post-rebase, both features exercised together):
Ships dark: Self-authored / merging with admin at user direction, per this week's precedent for author-approved same-repo branches. |
Brian Krabach (bkrabach)
pushed a commit
to ramparte/amplifier-foundation
that referenced
this pull request
Aug 28, 2026
Reframes this PR's timeout as Layer 3 of the "Layered Bounding for Delegated Sessions" design (spec: 298-replacement) -- the orchestrator- independent wall-clock backstop that sits behind a per-leg LLM-call budget (Layer 1, see microsoft#325 and companion PR microsoft/amplifier-module-loop-streaming#43), not the primary bound. Net functional change: `1800` -> `14400` for `settings.timeout`'s default. Everything else in this PR is kept verbatim: `_DelegateTimeoutExpired`, `_validate_timeout`, `_await_child_with_deadline`, the hard parent-release semantics, the honest `resumable: false` / `resume_status: pending_child_cleanup` reporting, and all 30 focused timeout tests (only the default-value assertions are retargeted). Why 14400s: ~12x the measured healthy sub-session upper bound (996-1168s), and ~2x below the worst observed runaway (17h34m) -- generous enough that a working Layer 1 budget should make this backstop fire zero times in practice. If it ever fires with Layer 1 active, that's a Layer 1 bug report, not evidence this default is wrong. Docs updated to frame this as the backstop: module docstring, README's "Delegate Timeout" section (retitled "Layered bounding: call budget (Layer 1) + wall-clock backstop (Layer 3)"), and the settings.timeout config comment. Files: - modules/tool-delegate/amplifier_module_tool_delegate/__init__.py: module docstring `settings.timeout` description; default 1800 -> 14400 - modules/tool-delegate/README.md: "Delegate Timeout" section rewritten as "Layered bounding"; config example comment - modules/tool-delegate/tests/test_delegate_timeout.py: default-value assertions retargeted to 14400 (T3.1) Testing: - modules/tool-delegate/tests/test_delegate_timeout.py: 30 passed - Full tool-delegate module suite: 90 passed (zero regressions) - Full foundation repo suite (tests/, this branch's own base): 1549 passed - python_check: no new issues vs this branch's own baseline (pre-existing I001 import-sort warning unchanged, confirmed via stash diff) 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
Brian Krabach (bkrabach)
pushed a commit
that referenced
this pull request
Aug 28, 2026
…k backstop, 14400s) * fix(delegate): bound delegated sessions by default * fix(delegate): raise the wall-clock backstop default to 14400s (Layer 3) Reframes this PR's timeout as Layer 3 of the "Layered Bounding for Delegated Sessions" design (spec: 298-replacement) -- the orchestrator- independent wall-clock backstop that sits behind a per-leg LLM-call budget (Layer 1, see #325 and companion PR microsoft/amplifier-module-loop-streaming#43), not the primary bound. Net functional change: `1800` -> `14400` for `settings.timeout`'s default. Everything else in this PR is kept verbatim: `_DelegateTimeoutExpired`, `_validate_timeout`, `_await_child_with_deadline`, the hard parent-release semantics, the honest `resumable: false` / `resume_status: pending_child_cleanup` reporting, and all 30 focused timeout tests (only the default-value assertions are retargeted). Why 14400s: ~12x the measured healthy sub-session upper bound (996-1168s), and ~2x below the worst observed runaway (17h34m) -- generous enough that a working Layer 1 budget should make this backstop fire zero times in practice. If it ever fires with Layer 1 active, that's a Layer 1 bug report, not evidence this default is wrong. Docs updated to frame this as the backstop: module docstring, README's "Delegate Timeout" section (retitled "Layered bounding: call budget (Layer 1) + wall-clock backstop (Layer 3)"), and the settings.timeout config comment. Files: - modules/tool-delegate/amplifier_module_tool_delegate/__init__.py: module docstring `settings.timeout` description; default 1800 -> 14400 - modules/tool-delegate/README.md: "Delegate Timeout" section rewritten as "Layered bounding"; config example comment - modules/tool-delegate/tests/test_delegate_timeout.py: default-value assertions retargeted to 14400 (T3.1) Testing: - modules/tool-delegate/tests/test_delegate_timeout.py: 30 passed - Full tool-delegate module suite: 90 passed (zero regressions) - Full foundation repo suite (tests/, this branch's own base): 1549 passed - python_check: no new issues vs this branch's own baseline (pre-existing I001 import-sort warning unchanged, confirmed via stash diff) 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> --------- Co-authored-by: Sam Schillace <ramparte@users.noreply.github.com> Co-authored-by: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Layer 1 of the "Layered Bounding for Delegated Sessions" spec (298-replacement — replaces the wall-clock-only default in #298, which is being revised separately rather than merged as-is). Gives
tool-delegatethe ability to inject a per-session-leg LLM-call budget into a child session, using the delegate's existingorchestrator_configchannel — zero new kernel surface, zero app-cli changes.Enforcement lives in the child's own orchestrator loop (
max_iterations), not in this module — this module only resolves what value to inject and reports what came back. See companion PR microsoft/amplifier-module-loop-streaming#43 for the orchestrator-side work this depends on for the budget to actually do anything.Ships DARK.
settings.max_llm_callsdefaults toNone. With no config change,orchestrator_configis byte-for-byte what it was before this PR — nothing is injected, nothing changes.Precedence chain
max_llm_calls)agents[name]["budget"]["max_llm_calls"])settings.max_llm_calls(defaultNone)orchestrator_config.max_iterationsAn explicit
0at rank 1 means "no Layer 1 budget for this call" (the wall-clocksettings.timeoutbackstop still applies) and correctly overrides rank 3's default — this required care: a naive "0 → None" collapse done too early loses the ability to distinguish "explicitly opted out" from "not specified at all," which would have silently ignored a caller's opt-out. Covered byTestOptOutin the new test file.Per-agent frontmatter override did NOT make the cut — verified, not assumed
The spec flagged this as an open item requiring verification before shipping: does a top-level
budget:block in an agent.mdfile's frontmatter actually reachcoordinator.config["agents"][name]["budget"]?Verified empirically (wrote and ran a test against the real loader, not just read the source): it does not round-trip.
amplifier_foundation.bundle._dataclass._load_agent_file_metadataonly forwards a fixed allowlist of top-level frontmatter keys (tools,providers,hooks,session,provider_preferences,model_role,agents) —budgetis silently dropped. Seetest_agent_frontmatter_budget_key_is_droppedin the new test file (usesmodel_role, an allowlisted key, as a control to prove this isn't a wholesale frontmatter-loading failure).Per the owner's decision, rank 2 is left out of this PR rather than faked. Ranks 1/3/4 ship and are fully tested. Closing this gap requires a change to
amplifier_foundation.bundle._dataclass._load_agent_file_metadata(addbudgetto the allowlist) — filed as a follow-up rather than bundled here, since it's a foundation-loader change, not a tool-delegate change. See the module README's "Known gaps" section.Also added
_check_call_budget_type/_validate_call_budget): rejectsbool, non-int, and negative values at the point supplied — module construction for the settings default,execute()for the per-call override — never silently coerced, never deferred to spawn time.llm_call_budgettelemetry (e.g. a third-party orchestrator withoutmax_iterationssupport), logs a warning naming the agent and setsmetadata.budget_enforced = falseon the returnedToolResult— silence is exactly the failure mode this design exists to eliminate.max_llm_callsentry in the tool's input schema (kept as a pure literal for the static token-cost estimator, per this file's existing convention).Files changed
modules/tool-delegate/amplifier_module_tool_delegate/__init__.py:_check_call_budget_type/_validate_call_budget(module-level validation helpers)__init__:settings.max_llm_calls/budget_warn_ratioexecute(): per-callmax_llm_callsparsing + validation_resolve_call_budget(new method): 2-rank precedence (1 and 3; rank 4 falls through untouched)_spawn_new_session:orchestrator_configbuild now copies (never mutates) the parent's config before injecting the budget; negotiated-feature warning +budget_enforcedmetadata_static_input_schema():max_llm_callsparametermodules/tool-delegate/README.md: "Layer 1 call budget" section + "Known gaps"modules/tool-delegate/tests/test_delegate_call_budget.py(new): 14 tests covering the precedence chain, opt-out, validation, status/metadata passthrough, the negotiated-feature warning, resume behavior, and the frontmatter verificationTesting
uv run pytest modules/tool-delegate/tests/test_delegate_call_budget.py -q→ 14 passeduv run pytest modules/tool-delegate/tests/ -q→ 80 passed (zero regressions)uv run pytest tests/ -q→ 1634 passed, 1 skipped — identical to the pre-change baselinepython_check(ruff lint/format, pyright): no new issues vs. baseline (all remaining warnings/errors are pre-existing, same lines shifted, confirmed via before/after diff)Relationship to other PRs
Do not merge — up for review alongside the companion PR and the #298 revision.
🤖 Generated with Amplifier