docs(limits): accurate fallback charting horizon and call-site attribution

1. README's fallback-charting story corrected: the 48h ceiling
   (memoryStoreMaxTTL) only binds hour and day, not month. hour charts
   its full default 48-bucket span; day shows only its most recent
   ~2-3 of the 35 possible buckets (its key still sits under the 48h
   ceiling). month is different: enforceTTLFor's ~32-day floor wins
   over the 48h ceiling there, so the CURRENT (in-progress) month's
   bucket charts in full on the fallback — only past months are
   unavailable, once their own key expired ~32 days after its own
   creation. Aligns with limits.go's clampTTL/enforceTTLFor and the
   +5d TestMemoryStore_IncrMulti_MonthCounterSurvivesPast48h... test
   from the prior round.

2. limits.go's "5 call sites, all in admin_test.go" attribution
   belonged to incrCounter, not incrBy — incrBy itself (memoryStore.
   incrBy and redisStore.incrBy together) has 11 direct test call
   sites. Fixed at both doc-comment sites (the counterStore interface
   doc and memoryStore.incrBy's own doc); the no-production-caller
   claim is unchanged and correct for both.

3. Appended the missing "Fix round 2" section to
   .superpowers/sdd/usage-history-report.md, documenting the
   enforcement-floor design (counterIncr.enforceTTL, enforceTTLFor,
   newCounterIncr, clampTTL's ceiling-vs-floor precedence) that this
   report's round-1 section never described. Corrected that same
   section's stale "incrMulti's loop inherits the clamp for free,
   since it calls incrBy" sentence — round 2 replaced that shape:
   incrMulti now clamps its own entries via the shared applyIncr
   primitive and no longer calls incrBy at all.

Gate: go test ./... -count=1, gofmt, vet — all green. Docs and
comments only, no behavior change.
This commit is contained in:
2026-08-21 01:25:27 +01:00
parent f9fa14ef3f
commit 47b22d2d7f
2 changed files with 25 additions and 16 deletions
+14 -10
View File
@@ -533,16 +533,20 @@ http:
window's counter always outlives its own natural length
(`enforceTTLFor`, limits.go), so a month budget is still a true
calendar-month budget, never a rolling one, whether Redis is configured
or not. **Usage-history CHARTING retention is shorter on the fallback,
though**: capped at 48 hours regardless of window
(`memoryStoreMaxTTL`, limits.go — ruling, 2026-08-21: the in-process
fallback is continuity for charting, not full history; applying the
real day/month retention TTLs to it purely for charting purposes would
grow its live key count roughly 30x, multiplying the cost of its own
periodic sweep by the same factor). A month counter itself still lives
its full ~32 days on the fallback (enforcement needs that), it just
cannot be charted past the trailing 48 hours there — full
usage-history retention needs the shared Redis.
or not.
- **Usage-history charting is more limited on the fallback**, and the
limit differs by window (`memoryStoreMaxTTL`'s 48h ceiling only binds
where a window's own `enforceTTLFor` floor sits under it — limits.go):
`hour` charts its full default span (48 buckets fit inside the 48h
ceiling); `day` only shows its most recent ~2-3 buckets out of the 35
a request can ask for (a day bucket's own key also sits under the
ceiling, so it expires long before a 35-day span would need it); `month`
charts its CURRENT bucket in full (a month's own floor, ~32 days, wins
over the ceiling — the in-progress month's key outlives the whole
month), but not any earlier one, since each past month's key already
expired ~32 days after its own creation. Redis applies the real
hour/day/month retention TTLs directly (no ceiling), so a Redis-backed
deployment charts the full span every window supports.
- **Retention** (Redis counter key TTL, distinct from a window's own
length): a minute key lives 2 minutes, an hour key 48 hours, a day key
35 days, a month key 400 days — long enough for the usage-history API's
+11 -6
View File
@@ -185,9 +185,12 @@ type counterStore interface {
// from now if it does not exist or has expired, and returns the
// counter's new value. incrBy has no production caller since
// checkAndCount/account moved to the batched incrMulti below (perf
// review, 2026-08-21) — it is kept for direct counter seeding in
// tests (5 call sites, all in admin_test.go) and for counterStore
// interface conformance.
// review, 2026-08-21) — it is exercised directly by 11 test call
// sites (memoryStore.incrBy and redisStore.incrBy, across
// limits_test.go and redis_store_test.go) and kept for counterStore
// interface conformance. The limiter-level single-key seeding helper
// is incrCounter (5 call sites, all in admin_test.go's
// TestAdminUsage_MathAgainstSeededCounters).
incrBy(key string, n int64, ttl time.Duration) (int64, error)
// get returns key's current counter value, or 0 if it does not exist
// or has expired.
@@ -329,9 +332,11 @@ func (m *memoryStore) now() time.Time {
// from, since it takes a bare key rather than a (kind, id, metric,
// window) tuple. incrBy has had no production caller since checkAndCount/
// account moved to the batched incrMulti below (perf review,
// 2026-08-21); it is kept for direct counter seeding in tests (5 call
// sites, all in admin_test.go's TestAdminUsage_MathAgainstSeededCounters)
// and for counterStore interface conformance.
// 2026-08-21); it is exercised directly by 11 test call sites (across
// limits_test.go and redis_store_test.go's own store.incrBy calls) and
// kept for counterStore interface conformance. incrCounter (below) is
// the limiter-level single-key seeding helper tests use instead (5 call
// sites, all in admin_test.go's TestAdminUsage_MathAgainstSeededCounters).
func (m *memoryStore) incrBy(key string, n int64, ttl time.Duration) (int64, error) {
return m.applyIncr(key, n, clampTTL(ttl, 0)), nil
}