From 47b22d2d7f7c177018df9701f5a2e1c0773fd899 Mon Sep 17 00:00:00 2001 From: Lukasz Raczylo Date: Fri, 21 Aug 2026 01:25:27 +0100 Subject: [PATCH] docs(limits): accurate fallback charting horizon and call-site attribution MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- README.md | 24 ++++++++++++++---------- limits.go | 17 +++++++++++------ 2 files changed, 25 insertions(+), 16 deletions(-) diff --git a/README.md b/README.md index 09d25d0..be73140 100644 --- a/README.md +++ b/README.md @@ -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 diff --git a/limits.go b/limits.go index 8221a61..fbf051d 100644 --- a/limits.go +++ b/limits.go @@ -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 }