Repository navigation
feat: render the quota banner only when it carries news (semantic ChangedAt) - #69
Merged
Merged
Conversation
…ue moves Consumers that report NEWS need to know when a bucket's visible state last changed. Neither existing signal works: - the ledger FILE's mtime moves on every write, and Save() is unconditional, so every route/status/run consult looks like a change (this defeated PR #68); - ObservedAt moves on every poll even when the numbers are identical. Bucket.ChangedAt (omitzero) is stamped in ONE place: UpdateChecked, the only code path that saves the ledger. It snapshots each bucket's displayed value before the mutation and stamps the buckets that are new or whose value differs: - used_pct at the precision it is rendered (math.Round; -1 = unknown), so 42.1 -> 42.3 is not a change -- both render as 42%; - resets_at; - never observed_at. Because it runs around fn, it also catches roll() expiry inside any mutator. The stamp follows the CALLER's clock: the eight locked mutators that receive a `now` record it (noteNow) and the newest is used; wall time (UTC) is only the fallback when no mutator supplied one. The first cut stamped wall-clock time and broke TestCodex5hKnobOffIsByteIdenticalToMain -- two identical runs with an injected clock serialized differently, and in local time besides. Backward compatible: older readers ignore the unknown field (no DisallowUnknownFields anywhere in the package). Tests (red first; each also mutation-proven): NotRestampedWhenValueRepeats the #68 failure, through the real write path StampedWhenPercentMoves / WhenResetMoves / ByExpiryRoll IgnoresSubPercentDrift FollowsCallerClockNotWallClock (and UTC) Mutants: "stamp on every write" (== the old mtime semantics) fails 2 tests; "never stamp" fails all 5; "ignore the caller clock" fails the clock test. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The banner re-rendered byte-identical content on every prompt: context paid on every turn for a line that changed no decision. Report deltas, not ticks. quotaHintWithFreshness returns the hint and whether it is fresh, from a single ledger read (no separate stat, which removes the read/stat race the review of PR #68 found). Fresh when any of: - the GLM hard-stop latch is set -- account protection, never gated; - a bucket's ChangedAt is within 15 minutes; - a ChangedAt is in the FUTURE (clock skew must never silence it for good); - a window reset within 15 minutes. Freshness is checked on every bucket the banner is BUILT from, before the render decides whether a lane gets a row: a reset drops its lane's row, and a row disappearing ("no longer EXHAUSTED") is a change in what the banner says. The banner also shows on a session's FIRST prompt, so quota state is still visible at the start of every session. isFirstPrompt streams the transcript from the start and stops at the first assistant record. Measured on real transcripts that record sits 200-270 KB in, behind the first turn's hook attachments, so a fixed-prefix check would misread long sessions as new. Reads are exactly one chunk with a carried tail, so a marker split across reads is still found; a 16 MiB ceiling bounds the scan. No path / unreadable / not-yet-written all mean "first" -- the safe direction, since showing the banner is the old behaviour. hookInput now parses transcript_path. Content is unchanged: quotaHint(now) keeps its signature and every existing content test is untouched. Tests: 12 unit tests plus TestQuotaBannerGateWiredE2E, which drives the BUILT binary -- same stale ledger, established session -> no banner, first prompt -> banner -- so it fails if the gate in main.go is removed. Mutation-proven, with a harness that confirms each mutant COMPILES (two first attempts left an unused variable, never built, and read as "not caught"): every bucket always news -> 4 tests red (incl. real write path + E2E) GLM latch not fresh -> TestGLMLatchAlwaysFresh red no carry between reads -> the boundary test red (after bounding each read to one chunk; before that the buffer swallowed the straddling marker and the test was vacuous) gate bypassed in main.go -> the binary E2E red Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…reshness Measured by the canary itself. Rationale recorded in the _note via the file's append-only PRIOR: convention. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…leet note
silent-failure-hunter + code-reviewer findings on this PR.
HIGH (fixed, both sides). ChangedAt had no future-skew clamp, unlike its sibling
ObservedAt, which Observe clamps after a documented incident. One skewed write (a
jumped clock, a restored or synced ledger, a replay injecting the wrong now)
would have read as "fresh" until real time caught up -- the banner pinned on.
stamp side: stampChanges clamps a future stamp to the wall clock;
read side: a future ChangedAt is fresh only within the 15m window; further
ahead it is a clock error and not trusted. The first-prompt rule
still shows the banner once per session, so skew can neither
silence it nor pin it on.
MEDIUM (fixed). isFirstPrompt matched the exact bytes "type":"assistant"; a
writer emitting spaced JSON would have turned every prompt into a "first" one.
Now a whitespace-tolerant regexp with a 64-byte carried tail.
MEDIUM (refuted in writing, pinned by a test). SetCapacity / SetCapacityEstimate
take no now, so the wall-clock fallback looked reachable. It is not through them:
capacity is not a DISPLAYED value (displayOf reads only used_pct and resets_at),
so a capacity-only write never stamps. Changing a public API across nine callers
to guard a path that cannot stamp would be churn; TestSetCapacityAloneDoesNotStamp
now fails if capacity ever becomes displayed without a caller clock.
IMPORTANT (documented). An older binary loads the ledger, does not know
changed_at, and rewrites the file without it (reproduced by the reviewer). It
degrades safely -- zero reads as not-fresh -- but persists until every writer
of that state dir is upgraded. The field comment now says so; it is why
scripts/deploy-fleet.sh rebuilds every mr-* binary together.
LOW (cheap guard). A reader returning (0, nil) forever would spin; a
zero-progress read now counts as EOF after three tries.
New tests, each mutation-proven with a harness that first verifies the mutant
COMPILES:
M5 drop the stamp clamp -> FutureStampClampedToWallClock red
M6 trust any future ChangedAt -> FarFutureChangedAtIsNotTrusted red
M7 exact-byte marker -> NotFirstPromptWithSpacedMarker red
M8 capacity counted as displayed -> SetCapacityAloneDoesNotStamp red
Gates: go test -count=1 ./... green; go vet clean; gofmt clean on LF copies;
semgrep 0 findings; B12 30730 -> 30761, measured by the canary.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replaces #68, closed after review proved its file-mtime gate was defeated by the unconditional
Save()on everyrouteconsult.Problem
The
mr-hookquota banner re-rendered byte-identical content on every prompt — context paid each turn for a line that changed no decision.Design
Ledger —
Bucket.ChangedAt, stamped in one place:UpdateChecked, the only code path that saves the ledger. Buckets whose displayed value moved (the roundedused_pct, orresets_at) are stamped;observed_atis deliberately excluded. Because it wrapsfn, it also catches expiryroll()inside any mutator. The stamp follows the caller's clock (noteNowin the eight locked mutators that receive anow), falling back to UTC wall time only when none supplied one.Hook —
quotaHintWithFreshness, a single ledger read, no separate stat. Fresh when:changed_atis within 15m, or in the future (clock skew can't silence it)Plus first prompt:
isFirstPromptstreams the transcript and stops at the first assistant record, which on real transcripts sits 200–270 KB in. Content is unchanged; every existing content test is untouched.Why the obvious fix wasn't used
"Skip unchanged writes in
Save()" fails too:bucketsis a Gomap(random serialization order) andobserved_atmoves on every poll.Evidence
go test -count=1 ./...go vet ./..._noteMutation-proven (the harness confirms each mutant compiles — two earlier mutants failed to build and read as "not caught"):
FollowsCallerClockNotWallClockGLMLatchAlwaysFreshmain.goTestQuotaBannerGateWiredE2Eon the built binaryFound along the way: the first cut stamped wall-clock time and broke
TestCodex5hKnobOffIsByteIdenticalToMain.Compatibility
Older readers ignore the new field (no
DisallowUnknownFieldsin the package). Existing ledgers have nochanged_atand read as stale until something changes — the first-prompt rule still shows them once per session.🤖 Generated with Claude Code