Say what --budget counts, and why a loop stalled - #221
Conversation
Issue #217 item 14. --budget's help claimed "(input + output)" while the hook sums the cache-read and cache-creation fields too, so on Claude Code a 3M budget burned in minutes of turns. And a budgeted loop that hit its bound went to .stalled — the same state a blown stall deadline uses — with the real reason written only to its memory log, so status and the card showed a bare STALLED. - Help text (CLI create/update, app create form, GoalSpec docs, and the budget sentence in the session's own opening prompt) states that every metered token counts, cache reads included, and that a Claude Code budget is a per-turn cost. - LoopNode carries a persisted stallReason; GraphStore records it at both stall sites. graphcode status prints it after the reason word and a stalled card's live line shows it in place of the goal it never finished.
scgopi
left a comment
There was a problem hiding this comment.
Verdict: approve. (Posted as a comment — this account authored the PR, so GitHub rejects a formal approval.) Reviewed against issue #217 item 14 and verified on the branch at 4d5925c (independent run, not the PR's own claims).
Correctness against the reported bug
| Claim in the PR | Verified |
|---|---|
--budget help said "(input + output)" while the hook counts four fields |
✅ PresenceHooks.usageScript sums input_tokens + cache_creation_input_tokens + cache_read_input_tokens + output_tokens (PresenceHooks.swift:172-175); the new help text on all four surfaces (CLI create/update, app create form, GoalSpec docs, session prompt) now states the counting accurately |
| Budget exhaustion was indistinguishable from a stall-timeout | ✅ Both .stalled sites record a why — enforceTokenBudget (GraphStore.swift:2120-2121) and markStalled (:2173-2174) are the only two sites that write .stalled |
graphcode status renders the why |
✅ ← Stalled: budget exhausted: 200 of 100 tokens spent; the append sits inside the AttentionRollup.reason branch, which fires only for failed/stalled/awaiting-input |
| The app card shows the why | ✅ liveLine is correctly guarded on displayState == .stalled (displayState passes .stalled through unchanged), falling back to the handed line when no why exists |
| Persistence is backward compatible | ✅ No hand-written encode(to:) — the synthesized encoder includes stallReason when non-nil; the hand-written decoder decodeIfPresent ?? nil, so graphs saved before the field load fine |
| Session prompt tells the loop what's counted | ✅ "counted over every token the API meters (cache reads included)" lands in sessionPrompt |
Tests and lint (run on the branch, worktree at 4d5925c)
- Full suite: 1296 tests in 140 suites passed, exit 0 — matches the PR's number. The three new tests (
statusRendersTheBudgetWhyInsteadOfABareStalled,aStalledLoopSaysWhyInsteadOfRestatingItsGoal,aStalledLoopWithoutAKnownWhyKeepsItsHandedLine) all ran and passed, and the stall-bound path assertion inGoalBasedLoopTestsholds. swiftlint: 0 errors.swift format: only the pre-existingDaemonBootstrap.swiftviolation, untouched by this PR (not in the diff). CI Linux build green.
One non-blocking finding
GraphcodeCommand.render (GraphcodeCommand.swift:686-691) appends stallReason under any attention reason, not just .stalled — unlike the card, which guards on displayState == .stalled. Reachable mislabel: reenterCycle (GraphStore.swift:1612-1616) resets cycle members to .idle without clearing stallReason, so a budget-stalled loop on a guarded cycle that re-enters and later fails renders ← Failed: budget exhausted: … with a stale why. Narrow (guarded cycles only) and cosmetic; a reason == .stalled guard in render — or clearing stallReason in reenterCycle — would close it. Fine to land as-is and address in a follow-up.
Review follow-up to #221: the reason now describes the stall that set it and only the stall sites leave one behind — every other state write goes through setNodeState, which clears it on the way out, so a future stall path that forgets to write a fresh reason cannot inherit a stale one. Also pins the create/update help lines' relationship with a test.
Closes #217 (item 14).
What was wrong
Two halves, both confirmed against the source at 0.1.56–0.1.57-beta1:
--budgethid what it counts. The help text said "(input + output)" (CLInode create, the app's create form, andGoalSpec's docs), butPresenceHooks.usageScriptsums all four transcript fields —input_tokens,cache_creation_input_tokens,cache_read_input_tokens,output_tokens. On Claude Code every turn re-meters the whole context as cache reads, so a 3M budget is a handful of turns, not hours. A bound sized from intuition burned in minutes with nothing in the help text explaining why.Budget exhaustion showed a bare Stalled with no why.
enforceTokenBudgetsetstate = .stalled— the same statemarkStalleduses for a blown stall deadline — and wrote the real reason ("budget exhausted: N of M tokens spent") only to the loop's memory log.graphcode statusand the app's card both showed a bare STALLED, leaving a budget that just needs raising indistinguishable from a loop that ground to a halt.The fix
node create/node updatehelp, the app create form's budget field,GoalSpec.tokenBudget's docs, and the budget sentence typed into the session's own opening prompt (a loop pacing itself must pace against what the orchestrator actually counts).LoopNode.stallReason: String?— persisted (hand-writtenCodabledecode falls back tonilfor graphs saved before the field existed).GraphStorerecords the why at both stall sites:"budget exhausted: N of M tokens spent"and"stall bound exceeded without resolving".setNodeState, which clearsstallReasonon any transition out of.stalled— a future stall path that forgets to write a fresh reason cannot inherit a stale one.graphcode statusrenders it:← Stalled: budget exhausted: 3000000 of 3000000 tokens spent.Deliberate scope (follow-ups)
AppSidebarMonitor) andActivityFeedstill show the bare STALLED word/icon. The card andgraphcode statusare where the decision gets made, so they carry the why; extending the rollup items (AttentionItem) with a detail line is a cheap follow-up if wanted.Tests
TokenBudgetTests: exhaustion records the reason on the node;statusrenders the why instead of a bare Stalled; the session prompt states cache reads count; the create/update help lines stay in sync (two flag lines; update points at create; the counting statement stays present).GoalBasedLoopTests: the stall-bound path records its own why.LoopCardPresentationTests: a stalled card with a known why shows it; without one, keeps the handed line.Full suite: 1297 tests passing.
swiftlintclean (0 errors);swift formatclean apart from a pre-existingDaemonBootstrap.swiftviolation untouched by this PR.