feat(adapters,engine): mid-session token-budget guard with off/warn/enforce modes (#158) - #176
Conversation
…nforce modes (#158) Sample cumulative session usage on the 30s heartbeat inside both adapter wait loops; a weighted spend crossing limits.max_tokens_per_session trips once per session — warn mirrors to ATTENTION, enforce nudges a wrap-up, arms a wall-co-bound grace window, then terminates over_budget (rides the existing retry→defer arm untouched). Claude-Session: https://claude.ai/code/session_01K1McZgyLxJumxC2U8KBtoP
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughAdds a configurable mid-session weighted token-budget guard to generic and OpenCode adapters. Sessions can warn or enforce a cap, record budget metadata, terminate as ChangesMid-session token-budget guard
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Engine
participant Adapter
participant UsageSource
participant Lifecycle
Engine->>Adapter: start session with budget settings
Adapter->>UsageSource: sample cumulative weighted usage
Adapter->>Lifecycle: record budget trip
Adapter-->>Engine: return completed or over_budget result
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
tests/test_opencode_http.py (1)
737-757: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake the under-cap test observe nonzero usage.
The
completedscenario finishes before the next normal heartbeat, so the only sample sees[]; this test passes without exercising weighted usage below the cap. Usebig-usage-then-completewith a cap above 5M and a shortened heartbeat.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_opencode_http.py` around lines 737 - 757, Update test_e2e_budget_inert_under_cap to use the big-usage-then-complete scenario, set token_budget above 5M, and shorten token_budget_grace_s so a heartbeat samples nonzero usage before completion. Preserve the assertions that the run completes without ATTENTION, a breadcrumb, or budget_weighted.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/bmad_loop/adapters/opencode_http.py`:
- Line 647: Replace the Bandit-only suppression on the budget mode condition in
src/bmad_loop/adapters/opencode_http.py:647 with the targeted Ruff suppression #
noqa: S105. Add # noqa: S106 to the corresponding budget mode enum usages in
tests/test_opencode_http.py at lines 684, 725, 747, 773, and 844; make no other
changes.
In `@src/bmad_loop/policy.py`:
- Around line 609-614: Update the LimitsPolicy construction for
max_tokens_per_session and session_budget_grace_s to validate the raw
configuration values as non-boolean integers before conversion, matching the
existing scm.preserve_keep validation pattern. Reject booleans and floats rather
than coercing or truncating them, while preserving the configured/default
integer values for valid inputs.
---
Nitpick comments:
In `@tests/test_opencode_http.py`:
- Around line 737-757: Update test_e2e_budget_inert_under_cap to use the
big-usage-then-complete scenario, set token_budget above 5M, and shorten
token_budget_grace_s so a heartbeat samples nonzero usage before completion.
Preserve the assertions that the run completes without ATTENTION, a breadcrumb,
or budget_weighted.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0573df78-8474-4b8d-8e8e-164f699e0776
📒 Files selected for processing (15)
CHANGELOG.mdREADME.mddocs/FEATURES.mddocs/tui-guide.mdsrc/bmad_loop/adapters/base.pysrc/bmad_loop/adapters/generic.pysrc/bmad_loop/adapters/opencode_http.pysrc/bmad_loop/data/settings/core.tomlsrc/bmad_loop/engine.pysrc/bmad_loop/policy.pytests/test_engine.pytests/test_generic_tmux.pytests/test_opencode_http.pytests/test_policy.pytests/test_settings_schema.py
- policy: reject bool/float/string for max_tokens_per_session and session_budget_grace_s instead of int()-coercing (true -> 1 token would terminate every enforce-mode session; strings crashed with a raw ValueError) — same strict pattern as scm.preserve_keep - tests: rework test_e2e_budget_inert_under_cap onto big-usage-then-complete with a shrunk heartbeat so samples actually observe the 5M weighted spend under the cap (the completed scenario finished before ever reporting usage, leaving the comparison untested); assert no budget-tripped breadcrumb as the docstring claims - rejected the S105/S106 noqa suggestion: repo Ruff runs default rules (no lint.select), S rules never fire; Bandit is the security gate and the existing nosec B105 covers it Claude-Session: https://claude.ai/code/session_01K1McZgyLxJumxC2U8KBtoP
|
Review round addressed in 631ff8c: strict non-bool-integer validation for the two new budget knobs (+ rejection tests), and the under-cap nitpick — |
|
@CodeRabbit review |
✅ Action performedReview finished.
|
Closes #158.
What
A runaway-but-busy session (265 turns / 49.7M raw tokens in the reference incident) was bounded only by
session_timeout_min: the stall detector fires on silence (pane output deliberately re-arms it), andmax_tokens_per_storyis advisory and evaluated only post-done. Nothing observed how much a running session was spending.Both adapter wait loops (generic tmux + opencode-http) now sample cumulative session usage on the existing ~30s heartbeat cadence and compare the weighted total (
cache_read_weight-discounted, same metric as the story cap) against a new per-session cap. Crossing it trips once per session, perlimits.session_budget_mode:warn(default) — one ATTENTION line + onebudget-trippedlifecycle breadcrumb; the session is never interfered with. Session-end journal entries carrybudget_weighted/budget/budget_mode.enforce— warn actions + a wrap-up nudge, thensession_budget_grace_s(default 240s, monotonic and wall-clock co-bound per the session-end journaled 2h19 after session_timeout_min fires when the session is wedged inside a tool call #157 lesson) to finish; a session that completes inside the grace is a normalcompleted, otherwise it terminates with the newover_budgetstatus.off— no sampling; behavior identical to today.over_budgetdeliberately rides the existing!= "completed"routing (retry with a fresh context → plateau-defer when attempts are exhausted) — zeroescalation.pychanges — and joins the post-kill-reconcile rescue set so an artifact flushed at kill time is still honored.New policy keys
Defaulting to
warn: upgrades gain visibility (the #158 ask — see the 49.7M session as it happens) with no behavior change to session outcomes;enforceis the opt-in hard bound.Design constraints held
UnicodeDecodeError), the nudge send and ATTENTION append are wrapped, HTTP sampling failures are inert ticks.crashed(artifact honored), notover_budget.over-budget-firedlifecycle breadcrumbs mirror the session-end journaled 2h19 after session_timeout_min fires when the session is wedged inside a tool call #157 timeout forensics.claude; codex/gemini sample the same cumulative transcript files best-effort (early transcript-path delivery / mid-turn flush are unverified vendor behavior);usage_parser = "none"(antigravity) and Copilot's shutdown-only flush leave the guard inert; the wrap-up nudge is a courtesy — the termination is the guarantee.Testing
Summary by CodeRabbit
off,warn(attention + breadcrumbs, no termination), andenforce(optional wrap-up grace thenover-budgettermination).limitssettings:session_budget_mode,max_tokens_per_session, andsession_budget_grace_s.over-budgetresults.