Skip to content

[#205] Make the budget/fallback seam engine-agnostic — hours as the primitive - #209

Merged
realproject7 merged 2 commits into
mainfrom
task/205-budget-fallback-seam
Aug 4, 2026
Merged

[#205] Make the budget/fallback seam engine-agnostic — hours as the primitive#209
realproject7 merged 2 commits into
mainfrom
task/205-budget-fallback-seam

Conversation

@realproject7

Copy link
Copy Markdown
Owner

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

The insight this rests on

The fallback decision was already not denominated in dollarsisBelowThreshold() compares estimatedHoursRemaining against 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() and isBelowThreshold() are consumed synchronously (the startOnFallback wiring 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<…>), and CreditAccountant holds 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 — async read(), plus a kind tag for the native detail.
  • HeadroomReading{known: true, windows} or {known: false, reason}.
  • hoursFromRate(unitsRemaining, unitsPerHour) and ratePerHour(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, percentPerHour measured exactly the way dollarsPerHour always 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.

GaugeState gains two fields: nativeDetail (display only — "$3.40 of $20.00" vs "62% used, resets in 3h") and headroomKnown. GaugeState stays structurally assignable to GaugeWire, so src/protocol.ts is untouched.

Unknown fails loud, not open

An unreadable source does not report Infinity hours — that would silently disable the safety net, which is the failure mode scope 5 exists to prevent. It is represented structurally as unknown, estimatedHoursRemaining reads 0, and both decision paths (isBelowThreshold and evaluate) gate on headroomKnown.

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

HeadroomUnknownReason is 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.

  1. The 23 existing credit-ledger tests pass completely unmodified — not one line touched.
  2. A new test inlines the pre-[refactor] Make the budget/fallback seam engine-agnostic — hours-remaining as the primitive, USD as only one derivation #205 derivation as an oracle and compares it against the current gauge across a matrix of 4 pools × 6 spend values × 4 metered-time values (96 combinations), asserting remainingUsd, dollarsPerHour, estimatedHoursRemaining, and fractionUsed are exactly equal (toBe, not toBeCloseTo). 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-guard all clean.

Seeded-violation proofs — every new guard confirmed load-bearing by reintroducing the defect:

Seeded defect Caught by
headroomKnown gate removed from isBelowThreshold 3 failures — unknown source auto-switches
Unknown reports Infinity hours "does not report infinite headroom"
First window used instead of the minimum 3 failures incl. the end-to-end two-window case
ratePerHour default handling broken (Claude drift) the pre-#205 oracle matrix + the rate unit test
hoursFromRate divides by zero "returns 0 — never Infinity"

Coverage 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

  • No session.ts wiring. The seam ships unused by design: with no headroomSource configured the accountant runs the USD path exactly as before, and refreshHeadroom() 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 that GaugeState gain 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.
  • resetsAt is 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 adding resetsAt changes the detail string and not the hours.
  • DEFAULT_PERCENT_PER_HOUR = 20 is the quota analogue of defaultDollarsPerHour = 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.
  • No threshold change, no settings-copy change, no adapter.

realproject7 and others added 2 commits August 4, 2026 22:48
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 project7-interns left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@realproject7

Copy link
Copy Markdown
Owner Author

RE2 — APPROVE

PR #209 (Fixes #205) @ caeb3a1 — all 5 CI checks green. 4 files, +681/−9, pure TS in packages/engine, no new dependencies (no manifest or lockfile change). Every acceptance criterion verified independently; two non-blocking notes, one of which is a positive finding.

Checked (evidence)

1. Claude bit-for-bit — verified far beyond the PR's own proof. The USD path now routes through the shared ratePerHour/hoursFromRate, so the real risk is an edge case where the lifted helpers differ from the inline arithmetic they replaced. I differenced main's derivation against the new one across 1568 finite combinations (7 pools × 8 spends × 7 metered-hour values × 4 default rates), comparing both dollarsPerHour and estimatedHoursRemaining with Object.is:

finite domain: 1568 combinations, 0 divergences

Two structural proofs back it up: credit-ledger.test.ts is not among the changed files, so "existing tests pass unmodified" is true by construction rather than by assertion, and the new oracle test inlines an independent legacyGauge() and compares four fields with exact toBe over 96 combinations. The switch-point test pins the crossing at $0.80 remaining with values either side.

2. The one divergence I found is unreachable — and it fixes a latent instance of this ticket's own bug. With a non-finite poolUsd:

poolUsd old hours switches? new hours switches?
NaN NaN no 0 yes
Infinity Infinity no 0 yes

hoursFromRate's Number.isFinite guard is what changes this. It is unreachable from settings — settings.rs:218-219 clamps a non-finite or non-positive pool_usd to the default before it can reach the accountant — and the direction is strictly safer. Worth stating plainly: the old behaviour on an infinite pool was estimatedHoursRemaining = Infinity, which never crosses the threshold and therefore silently disables the safety net — precisely the failure mode #205 scope 5 exists to prevent. The refactor closes it as a side effect. That is a point in the change's favour, not against it.

3. Unknown fails loud in both directions — the substantive judgment, and it is right. isBelowThreshold() is now headroomKnown && hours < threshold, and estimatedHoursRemaining reads 0 rather than Infinity when unknown. Both halves matter: Infinity would disable the net, while treating an unreadable source as empty would demote a healthy Claude session to Local on a transient read failure. "We don't know how much is left" is not "it's low", and the code says so structurally. A configured-but-never-refreshed source starts unknown (private headroom initialises to {known:false}), so the window between construction and first refresh cannot switch either. A source that throws is caught and treated as unreadable rather than escaping — same discipline as the ledger write path.

4. Two-window minimum, verified including the trap. quotaHeadroom computes hours per window and keeps the minimum, with each window's rate derived from its own usedPercent — so a nearly-exhausted weekly window governs while the rolling one looks healthy. is unaffected by window order is the test I'd have asked for: it is what distinguishes a genuine minimum from "first window wins". Exhausted windows clamp to 0 hours, never negative; usedPercent > 100, negative, and non-finite values are all handled.

5. Native detail is display-only, and provably so. lets resetsAt change the detail string but NOT the hours is the sharp version of scope 7, and never consults nativeDetail when deciding to switch plus keeps the decision fields and the display field independent cover scope 6. The HeadroomUnknownReason closed literal set is a good security choice: a source structurally cannot smuggle an account id into an event or log line through reason.

6. Gates. engine 303 (275 + 28 new), archive 110, app 185, both typechecks exit 0, pnpm lint exit 0. All 5 CI green at caeb3a1.

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 gauge()/isBelowThreshold() would rewrite them and the #37 rollover-resilience contract with them. Beyond that constraint it is also the better design on the merits: the decision is instantaneous and must stay callable from startOnFallback at session start, so the only thing that genuinely needs to be async is the fetch. Last-known-snapshot behind a sync read is the standard shape for exactly this. I would not renegotiate #37.

(b) DEFAULT_PERCENT_PER_HOUR = 20 is not a rate table. Confirmed as the precise analogue of defaultDollarsPerHour ?? 0.4 (credit-ledger.ts:140, consumed at :240 through the same ratePerHour). A rate table would map engine or model → price; this is a single bootstrap constant so the net is armed before any metered time exists. Same role, same position in the arithmetic.

(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, gauge() takes the USD branch and refreshHeadroom() returns immediately, so nothing about today's behaviour can move. The tests exercise the unwired paths thoroughly, which is the right substitute for production exercise at this stage.

(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.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[refactor] Make the budget/fallback seam engine-agnostic — hours-remaining as the primitive, USD as only one derivation

2 participants