[#205] Make the budget/fallback seam engine-agnostic — hours as the primitive - #209
Conversation
The fallback decision was already denominated in estimated meeting-hours, not dollars: isBelowThreshold() compares estimatedHoursRemaining to a threshold, and USD is only how the Claude tier derives that number. This keeps hours as the cross-engine primitive and makes the derivation pluggable, so a non-Claude engine can drive the safety net without inventing dollar figures. No token->USD rate card is added anywhere. The source seam is async-first (a future source is a network read), but the accountant keeps a last-known snapshot so its read path stays synchronous — startOnFallback and the #37 rollover-resilience contract both depend on that. Unknown headroom fails loud rather than open: an unreadable source yields an explicit unknown, never Infinity, and never auto-switches. Not knowing how much is left is not the same as knowing it is low, so a transient read failure must not degrade a working session to the local tier. Claude behaviour is unchanged: with no source configured the USD derivation runs as before, proven against an inlined pre-#205 oracle across a pool/spend/metered matrix, with the 23 existing ledger tests passing unmodified. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: APPROVE
Epic Alignment: PASS
PR #209 extracts the engine-agnostic hours-remaining seam required by #205/#204, preserves Claude/USD behavior, and keeps unknown headroom explicit and non-switching.
Checked (evidence)
- Structural gate: PR body includes EPIC Alignment, Self-Verification, and Deviations sections; no UI surface is changed.
- Verified final delta: headroom.ts provides async source reads, shared hours/rate arithmetic, quota two-window minimum, closed unknown reasons, and display-only native detail.
- Verified credit-ledger.ts keeps the synchronous gauge/isBelowThreshold contract, gates both decision paths on headroomKnown, and leaves the USD path mathematically unchanged; the 96-case oracle and existing tests cover this.
- Verified unknown/unreadable/never-refreshed sources do not report Infinity or auto-switch; resetsAt changes detail only.
- Riskiest part: async boundary at HeadroomSource.read() with a cached snapshot; this preserves startOnFallback and rollover-resilience sync consumers while making future adapters pluggable.
- Kill-list: scanned the full diff — clean.
- CI: gh pr checks 209 -> all five checks pass.
Findings
None.
Decision
The seam satisfies #205's contracts without wiring a speculative adapter or changing Claude behavior. Unknown headroom fails safe, quota windows bind by minimum hours, and all live CI checks pass.
RE2 — APPROVEPR #209 (Fixes #205) @ Checked (evidence)1. Claude bit-for-bit — verified far beyond the PR's own proof. The USD path now routes through the shared Two structural proofs back it up: 2. The one divergence I found is unreachable — and it fixes a latent instance of this ticket's own bug. With a non-finite
3. Unknown fails loud in both directions — the substantive judgment, and it is right. 4. Two-window minimum, verified including the trap. 5. Native detail is display-only, and provably so. 6. Gates. engine 303 (275 + 28 new), archive 110, app 185, both typechecks exit 0, Your four design questions, answered(a) Async at the source, not the accountant — right, and the ticket's own scope forces it. Scope 2 requires the existing tests to pass unmodified; an async (b) (c) Shipping the seam unwired is the right call. A caller today would be either dead code or a speculative adapter, and the adapter is explicitly #204. It is also what makes the bit-for-bit claim clean: with no source configured, (d) Unknown-in-both-directions: already covered in (3) — this is the judgment I would most defend if challenged. Notes (non-blocking)
(The shared bot token cannot file a formal GitHub approval, so this comment plus my chat message is the RE2 verdict of record.) |
Fixes #205
Makes the budget/fallback machinery engine-agnostic so a non-Claude engine can drive the "fall back to Local before this stops being free" safety net without inventing dollar figures. Claude-tier behaviour is unchanged.
EPIC Alignment
21e481b, this branches from it). Enables: [research→gate] Codex CLI as a third translation engine — technically feasible via app-server, BLOCKED on ToS + USD-budget model #204.The insight this rests on
The fallback decision was already not denominated in dollars —
isBelowThreshold()comparesestimatedHoursRemainingagainst a threshold, and USD is only how the Claude tier derives that number. Every paid engine answers the same user-facing question: "how much longer can I run before this stops being free?" So this PR keeps hours as the cross-engine primitive and makes only the derivation pluggable. No token→USD rate card is added anywhere.Design decision: where the async boundary goes
Scope 1 asks for async-first signatures;
gauge()andisBelowThreshold()are consumed synchronously (thestartOnFallbackwiring at session start, and the #37 rollover-read-path tests that exist precisely so a disk error can't fail session start). Making the accountant's read path async would break both, and scope 2 requires the existing tests to pass unmodified.So the async boundary is the
HeadroomSource(read(): Promise<…>), andCreditAccountantholds a last-known snapshot behind a sync read path. A future network-backed source fits without a repo-wide refactor, the sync contract is untouched, and the "unknown never switches" gate sits on the sync side where the decision is actually made.What the seam looks like
HeadroomSource— asyncread(), plus akindtag for the native detail.HeadroomReading—{known: true, windows}or{known: false, reason}.hoursFromRate(unitsRemaining, unitsPerHour)andratePerHour(consumed, meteredHours, fallback)— the shared arithmetic both derivations reduce to. Units cancel, which is the point: dollars and percent both produce hours.quotaHeadroom(reading, meteredHours, nowMs)— percent derivation,percentPerHourmeasured exactly the waydollarsPerHouralways has been, taking the minimum across windows.CreditAccountant.refreshHeadroom()— async refresh; a source that throws is treated as unreadable rather than allowed to escape, for the same reason a ledger write failure is surfaced rather than thrown: accounting can be lost, captions must not.GaugeStategains two fields:nativeDetail(display only —"$3.40 of $20.00"vs"62% used, resets in 3h") andheadroomKnown.GaugeStatestays structurally assignable toGaugeWire, sosrc/protocol.tsis untouched.Unknown fails loud, not open
An unreadable source does not report
Infinityhours — that would silently disable the safety net, which is the failure mode scope 5 exists to prevent. It is represented structurally as unknown,estimatedHoursRemainingreads 0, and both decision paths (isBelowThresholdandevaluate) gate onheadroomKnown.The gate matters in both directions, which is why it isn't simply "treat unknown as empty": not knowing how much is left is not the same as knowing it is low. Treating a transient read failure as "low" would degrade a perfectly healthy Claude session to the local tier. A configured-but-never-refreshed source also starts unknown, so the window between session start and the first refresh is not an unguarded free-for-all.
Security
HeadroomUnknownReasonis a closed set of literals ("unreadable" | "no-windows" | "invalid"), not free text. That is structural, not a convention: a source cannot smuggle an account id, token, or plan identifier into a gauge event or a log line through this type. Asserted in a test.Self-Verification
Claude behaviour is unchanged, proven two ways.
remainingUsd,dollarsPerHour,estimatedHoursRemaining, andfractionUsedare exactly equal (toBe, nottoBeCloseTo). Plus an explicit same-switch-point test bracketing the $0.80-remaining crossing at 19.19 / 19.21 spend.Tests: engine 303 (275 + 28 new), archive 110, app 185.
pnpm lint,pnpm typecheck(both configs),no-stub-gate,color-guardall clean.Seeded-violation proofs — every new guard confirmed load-bearing by reintroducing the defect:
headroomKnowngate removed fromisBelowThresholdInfinityhoursratePerHourdefault handling broken (Claude drift)hoursFromRatedivides by zeroCoverage of each acceptance criterion: existing tests unmodified ✓; quota source drives
isBelowThreshold()with a fake source including the two-window minimum ✓ (no external CLI is required anywhere in this suite); unreadable source yields unknown, never infinite, and does not auto-switch ✓; native detail never consulted by the decision path ✓ (asserted by rewriting the label to"CRITICAL — 0% LEFT"and confirming the verdict and hours are identical); no new dependencies ✓.Kill-list: clean — no new dependency, no TODO/FIXME/stub marker, no caption content logged or persisted, no credential/token/plan-id/account-id reachable through any type in this seam.
Deviations
session.tswiring. The seam ships unused by design: with noheadroomSourceconfigured the accountant runs the USD path exactly as before, andrefreshHeadroom()is a no-op. Wiring a caller now would mean either dead code or a speculative adapter, and the adapter is [research→gate] Codex CLI as a third translation engine — technically feasible via app-server, BLOCKED on ToS + USD-budget model #204's job. This is what keeps the "bit-for-bit" claim honest rather than merely tested.GaugeWire/ webview untouched. Scope 6 asks thatGaugeStategain the field; rendering it would be new user-facing copy, which is explicitly out of scope. The field is available on the wire structurally without changing any rendering.resetsAtis display-only, enforced by test. A soon-resetting window arguably means "wait" rather than "fall back", but scope 7 defers that product decision — so a test asserts that addingresetsAtchanges the detail string and not the hours.DEFAULT_PERCENT_PER_HOUR = 20is the quota analogue ofdefaultDollarsPerHour = 0.4: a deliberately conservative guess so the safety net is armed from the first minute rather than silent until metered time accrues. It is not a rate card — it is a bootstrap constant with the same role the USD path already had, and any real source's measured rate replaces it as soon as there is metered time.