Skip to content

perf(mr-hook): gate the quota banner on ledger freshness - #68

Closed
dmmdea wants to merge 2 commits into
mainfrom
perf/quota-hint-staleness-gate
Closed

dmmdea wants to merge 2 commits into
mainfrom
perf/quota-hint-staleness-gate

Conversation

@dmmdea

@dmmdea dmmdea commented Sep 21, 2026

Copy link
Copy Markdown
Owner

The problem

The quota hint re-rendered byte-identical content on every prompt. Observed live 2026-09-21: the same one-liner on every turn of a long session — costing context each turn while changing no decision.

Repeating an unchanged number is a tick, not a delta.

The change

A 15-minute freshness gate keyed on the ledger's own mtime. If nothing has metered a window recently there is no new pressure to report. A dispatch touches the ledger, so the next prompt renders fresh numbers again.

Stateless by design. mr-hook writes no state in production and that invariant is preserved — the signal is the existing mtime, not a new cache file. Fail-open stays absolute: a stat error yields "".

The GLM hard-stop latch is exempt — and that is the interesting part

The first cut placed the stat before rows were built. The GLM latch is deliberately ledger-independent: it renders with no buckets and with no ledger file at all. So the stat failed, and the gate silently swallowed a 1313 account-protection warning.

TestHookMismatchKeepsQuotaHintE2E caught it — the existing E2E seeds only glm-alert.json, no ledger.

The gate now runs after row construction and never applies when the latch is set. That signal is account protection, not quota reporting.

Tests — TDD, red before green

test asserts
TestQuotaHintStaleLedgerSuppressed 30m-old ledger → "" (previously rendered)
TestQuotaHintFreshLedgerRenders control arm — 1m-old ledger still renders with its signal, so the gate cannot pass by suppressing everything
TestQuotaHintGLMLatchExemptFromStaleness regression guard — no ledger at all, latch must still render

The guard is mutation-proven: replacing if !glmLatched with if true turns it red (got: ""); restoring the exemption turns it green.

B12 ratchet

Refused at exactly +29 non-test LOC — precisely the size of this change. Raised consciously to 30578 in the second commit, as the canary's own message instructs, rather than by trimming the comments that document the bug. A future reader moving that stat back would reintroduce the swallowed warning; the comment and the guard are what prevent it. _note history preserved via the append-only PRIOR: convention.

Gates

gate result
go test -count=1 ./... (whole repo, uncached) green
go test -count=1 ./cmd/mr-hook/ ok, 11.3s
go vet ./cmd/mr-hook/ exit 0
gofmt clean on LF-normalized copies of both touched files
semgrep --config=auto exit 0, 0 findings, 131 rules
B12 canary green after the conscious raise

Note on gofmt -l: it lists 12 files in cmd/mr-hook/, including ones untouched here. That is the Windows CRLF working-tree artifact — main.go is w/crlf and formats clean once LF-normalized. Both files in this PR are w/lf and clean.

🤖 Generated with Claude Code

dmmdea and others added 2 commits September 21, 2026 10:25
The quota hint re-rendered byte-identical content on EVERY prompt. Observed
live on 2026-09-21: the same one-liner on every turn of a long session,
costing context on each turn while changing no decision. Repeating an
unchanged number is a tick, not a delta.

Adds a 15-minute freshness gate keyed on the ledger's own mtime: if nothing
has metered a window recently there is no new pressure to report. A dispatch
touches the ledger, so the next prompt renders fresh numbers again.

Stateless BY DESIGN. mr-hook writes no state in production and that invariant
is preserved -- the signal is the ledger file's existing mtime, not a new
cache. Fail-open stays absolute: a stat error yields "".

THE GLM HARD-STOP LATCH IS EXEMPT, and that exemption is the point of the
regression guard below.

The first cut placed the stat before the rows were built. The GLM latch is
deliberately ledger-INDEPENDENT -- it renders with no buckets and with no
ledger file at all -- so the stat failed and the gate silently swallowed a
1313 account-protection warning. TestHookMismatchKeepsQuotaHintE2E caught it.
The gate now runs after row construction and never applies when the latch is
set: that signal is account protection, not quota reporting.

Tests (TDD, red before green):
  TestQuotaHintStaleLedgerSuppressed        30m-old ledger -> "" (was: rendered)
  TestQuotaHintFreshLedgerRenders           control arm: 1m-old ledger still
                                            renders WITH its signal, so the gate
                                            cannot pass by suppressing everything
  TestQuotaHintGLMLatchExemptFromStaleness  regression guard, no ledger at all ->
                                            latch must still render

The guard is mutation-proven: replacing `if !glmLatched` with `if true`
turns it RED ("got: \"\""), and restoring the exemption turns it green.

Gates: full cmd/mr-hook suite ok (11.2s) · go vet exit 0 · gofmt clean on
LF-normalized copies of both touched files · semgrep exit 0, 0 findings,
131 rules.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The B12 complexity ratchet refused the previous commit at exactly +29 non-test
Go LOC, which is precisely the size of the change. Raised consciously here, as
the canary's own message instructs, rather than by trimming the comments that
document the bug.

The bulk of the +29 is the GLM exemption and its rationale, which is the part
worth keeping in the source: the first cut of the gate stat'd the ledger BEFORE
rows were built, and because the GLM hard-stop latch is deliberately
ledger-independent -- it renders with no buckets and with no ledger file at all
-- the gate silently swallowed a 1313 account-protection warning. A future
reader moving that stat back would reintroduce it; the comment and the
mutation-proven guard are what stop that.

_note history preserved via the file's append-only PRIOR: convention.

Verified: canary green, whole repo green uncached (go test -count=1 ./...).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@dmmdea

dmmdea commented Sep 21, 2026

Copy link
Copy Markdown
Owner Author

Closing unmerged — the gate does not achieve its goal

A silent-failure-hunter review found the design is defeated in production, and follow-up checks show the obvious fix fails too. Closing rather than shipping a gate proven not to work. Branch kept for reference.

Why the mtime gate fails

ledger.Save() writes unconditionally, and it is called on every mr-orchestrate route (route.go:438-448), run and status — including route, the very command this banner tells the caller to run before each task. The mtime is therefore refreshed constantly regardless of content, now.Sub(mtime) rarely exceeds 15 minutes, and the gate almost never fires in exactly the case it was built for.

My tests set mtime directly with os.Chtimes and never exercised the real write path, so they could not catch this. That is the test gap to learn from.

Why "skip unchanged writes in Save()" also fails

Verified before trying it:

  1. Ledger.buckets is a map[string]*Bucket (ledger.go:212). snapshotLocked() builds its slice by map iteration, so serialization order is random — identical content produces different bytes.
  2. Every Bucket carries observed_at (ledger.go:92), which each poll refreshes even when used_pct is unchanged.

A byte comparison would still write almost every time.

Other findings, recorded

  • TOCTOU (medium): the os.Stat is a second syscall after OpenChecked's read, so a concurrent tmp+rename can transiently drop the banner.
  • Future mtime (low): a negative age never exceeds the threshold, so clock skew makes the gate never fire — undocumented, untested.
  • The GLM exemption was confirmed complete: no other account-protection latch is rendered by quotaHint.

The correct design — deferred to its own review

Freshness has to be semantic, not filesystem: a per-bucket changed_at, set only when a displayed value changes (used_pct, resets_at, admission state), deliberately excluding observed_at. The banner renders when any rendered lane changed, or its window reset (derivable from resets_at), within the window. The GLM latch stays exempt. Stateless for mr-hook, deterministic, backward compatible as an omitempty field.

That touches ~17 write sites across ~10 mutators in the quota substrate that routing and the spend guards depend on. That is a design change to money-guarding code and deserves its own reviewed PR — not a side effect of a token-trimming pass.

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.

1 participant