diff --git a/README.md b/README.md index 1d21f49..09d25d0 100644 --- a/README.md +++ b/README.md @@ -229,6 +229,11 @@ that is negative, `NaN`, or `±Inf`, is a construction error. | `costPerDayUSD` | `float64` | Fixed calendar-day window, UTC. | | `costPerMonthUSD` | `float64` | Fixed calendar-month window, UTC. | +Every window enforces correctly on the in-process fallback too, not only Redis — +see [Limits and accounting](#limits-and-accounting)'s "Fallback" bullet for the +retention floor that keeps `tokensPerMonth`/`costPerMonthUSD` a true calendar-month +budget rather than a rolling one when Redis is absent or down. + A user's own `limits`, when set, are checked **in addition to** their group's limits, not instead of them — a request is refused the moment it breaches whichever of the two is tighter. A user with no `limits` of their @@ -523,16 +528,21 @@ http: `failOpen: true`): counters live in an in-process map — correct for one Traefik replica, only approximate across several, since each replica counts independently. Exact limits across multiple Traefik replicas - require the shared Redis. Every counter, including this fallback one, - works as the usage-history API's data source, but its retention is - capped at **48 hours regardless of window** (`memoryStoreMaxTTL`, - limits.go — ruling, 2026-08-21: the in-process fallback is continuity, - not history; applying the real day/month TTLs to it would grow its live - key count roughly 30x, multiplying the cost of its own periodic sweep by - the same factor). A fallback-only deployment's usage-history charts - therefore show at most the trailing 48 hours, never the full day/month - span Redis-backed deployments get — full retention needs the shared - Redis. + require the shared Redis. **Enforcement stays correct on the fallback at + every window**, including `tokensPerMonth`/`costPerMonthUSD`: a + 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. - **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 @@ -989,11 +999,15 @@ A rebuilt instance keeps some state and resets the rest: counters the old one wrote. The response cache's keys work the same way. - **Resets on a rebuild**: the in-process fallback counters (used whenever no Redis is configured, and also transparently during a Redis outage - when `failOpen` is true — the default; see `limiter.storeIncrBy`, + when `failOpen` is true — the default; see `limiter.storeIncrMulti`, `limits.go`), the limiter's and cache's own Redis-outage down-latches, and the model registry's discovery cache — a new instance always runs its own full warm-fill before serving, rather than inheriting the old - instance's listing. + instance's listing. This is separate from the fallback's own + TTL-bounded retention (a window's counter still expires and resets on + its own, per-window schedule, even between rebuilds — see [Limits and + accounting](#limits-and-accounting)'s "Fallback" bullet): a rebuild + loses every fallback counter at once, TTL or not. Two practical implications follow: diff --git a/limits.go b/limits.go index dd50c2f..8221a61 100644 --- a/limits.go +++ b/limits.go @@ -57,14 +57,29 @@ const ( // so a key stays readable for the whole window it belongs to; the bucket // embedded in the key (see windowKey) already makes a rolled-over window // use a different key, so these TTLs only bound how long a stale key -// lingers in memoryStore before an opportunistic sweep reclaims it. -// dayWindowTTL/monthWindowTTL were bumped from their original 25h/32d -// (v0.2 data-layer task): the admin usage-history API's day/month charts -// need a bucket to stay readable for the whole retention span a chart -// can request (historyMaxSpan, admin.go — 35 daily, 13 monthly buckets), -// not just long enough to survive its own single rollover. The TTL > -// window-length invariant this comment already documented still holds at -// the new values, and at hourWindowTTL for the new hour window. +// lingers before an opportunistic sweep (memoryStore) or Redis itself +// reclaims it. dayWindowTTL/monthWindowTTL were bumped from their +// original 25h/32d (v0.2 data-layer task): the admin usage-history API's +// day/month charts need a bucket to stay readable for the whole +// retention span a chart can request (historyMaxSpan, admin.go — 35 +// daily, 13 monthly buckets), not just long enough to survive its own +// single rollover. The TTL > window-length invariant this comment +// already documented still holds for these consts exactly as applied to +// Redis (35d > 1d, 400d > ~31d, 48h > 1h, 2m > 1m). +// +// That invariant does NOT automatically extend to what memoryStore +// actually applies, though: its own ceiling (memoryStoreMaxTTL, 48h, +// applied only to bound the in-process fallback's live-key count) is +// shorter than a day or month window's natural length on its own. A +// round-1 fix (perf review, 2026-08-21) clamped every fallback TTL to +// that ceiling unconditionally, which broke the invariant for month — +// silently turning TokensPerMonth/CostPerMonthUSD into a rolling ~48h +// budget under the fallback. Round 2 restores it: memoryStore's clampTTL +// never cuts a TTL below enforceTTLFor's window-specific floor, so the +// EFFECTIVE TTL memoryStore ever applies still exceeds its window's +// natural length, exactly like these consts do for Redis — see +// clampTTL/enforceTTLFor/memoryStoreMaxTTL's own doc comments for the +// mechanism. const ( minWindowTTL = 2 * time.Minute hourWindowTTL = 48 * time.Hour @@ -139,6 +154,27 @@ type counterIncr struct { key string delta int64 ttl time.Duration + // enforceTTL is the minimum TTL window's own enforcement correctness + // requires (enforceTTLFor) — the floor memoryStore's clampTTL never + // cuts below, even when memoryStoreMaxTTL's history-retention ceiling + // is lower (review round 2, 2026-08-21, fixing a bug in round 1's + // single-floor clamp: unconditionally capping every TTL at 48h turned + // TokensPerMonth/CostPerMonthUSD into a rolling ~48h budget under the + // fallback, since a month counter's own key would expire and reset + // roughly every two days instead of once a month). redisStore ignores + // this field entirely — ttl is applied in full there; the ceiling and + // this floor only ever matter for the in-process fallback. + enforceTTL time.Duration +} + +// newCounterIncr builds one counterIncr for (kind, id, metric, window) at +// t: key from windowKey, delta and the history-retention ttl exactly as +// the caller gives them, and enforceTTL derived from window itself +// (enforceTTLFor). checkAndCount and account build every entry through +// this one helper so a (window, ttl) pair can never reach incrMulti +// without its matching enforcement floor. +func newCounterIncr(kind, id, metric, window string, t time.Time, delta int64, ttl time.Duration) counterIncr { + return counterIncr{key: windowKey(kind, id, metric, window, t), delta: delta, ttl: ttl, enforceTTL: enforceTTLFor(window)} } // counterStore is the storage backend the limiter uses for atomic windowed @@ -147,7 +183,11 @@ type counterIncr struct { type counterStore interface { // incrBy adds n to key's counter, creating it with an expiry of ttl // from now if it does not exist or has expired, and returns the - // counter's new value. + // 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. 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. @@ -192,28 +232,71 @@ type memoryEntry struct { // every 30s. const sweepEvery = 30 * time.Second -// memoryStoreMaxTTL caps any TTL memoryStore actually applies, regardless -// of what a caller requests. Ruling (perf review, 2026-08-21): the -// in-process fallback is continuity, not history. Applying the real -// hourWindowTTL/dayWindowTTL/monthWindowTTL values (48h/35d/400d) to the -// fallback map was measured to grow its live key count roughly 30x over -// the original three-window (min/day/month) shape — and sweepEvery's own -// doc comment above already records that a full-map sweep which frees -// nothing is a "167x incrBy cliff" once live keys cross a threshold; -// growing the map ~30x bigger multiplies that same cliff's cost by -// roughly the same factor. Clamping every fallback TTL to 48h keeps -// per-scope live keys bounded (a handful of metrics across four windows, -// not 35-400 days' worth) independent of which window a caller asks for. -// This is a documented capability gap, not a bug: with no Redis -// configured, or during a Redis outage under failOpen, memoryStore-backed -// usage-history charts only ever show the trailing 48h, never longer — -// see README's "fallback works too, but only for 48h" note. +// memoryStoreMaxTTL caps how long memoryStore keeps a counter alive +// purely for HISTORY-chart purposes, when that is safe to shorten. +// Ruling (perf review round 1, 2026-08-21): the in-process fallback's +// long-tail retention is continuity for charting, not enforcement itself +// — applying the real hourWindowTTL/dayWindowTTL/monthWindowTTL values +// (48h/35d/400d) to the fallback map was measured to grow its live key +// count roughly 30x over the original three-window (min/day/month) +// shape, and sweepEvery's own doc comment above already records that a +// full-map sweep which frees nothing is a "167x incrBy cliff" once live +// keys cross a threshold — growing the map ~30x bigger multiplies that +// same cliff's cost by roughly the same factor. +// +// This ceiling must never win over a window's own enforcement floor, +// though (round 2, 2026-08-21 — corrects a round-1 bug): clampTTL applies +// this ceiling AND enforceTTLFor's floor together, and the floor always +// wins where the two would conflict. Concretely: min/hour/day windows sit +// entirely under 48h already, so they are unaffected; month's own floor +// (enforceTTLFor's 32*24h) sits above this ceiling, so a month counter on +// the fallback keeps its full ~32-day life (1-2 live keys per scope/ +// metric — negligible next to the day-key savings that motivated this +// ceiling in the first place) instead of expiring every 48h. Only the +// CHARTING tail — a fallback-only deployment's usage-history buckets +// beyond 48h — is what this ceiling actually shortens; see README's +// "fallback retention" note. const memoryStoreMaxTTL = 48 * time.Hour -// clampTTL caps ttl at memoryStoreMaxTTL — see that const's doc comment. -func clampTTL(ttl time.Duration) time.Duration { +// enforceTTLFor returns the minimum TTL window's own counter must stay +// alive for to enforce correctly, independent of how long history +// retention (hourWindowTTL/dayWindowTTL/monthWindowTTL) asks the same key +// to live for charting. These are windowLength + margin, and not +// coincidentally match dayWindowTTL/monthWindowTTL's ORIGINAL, +// pre-history-retention-bump values (25h/32d, v0.2 data-layer task) — +// that bump only ever existed to serve the usage-history API's charts; +// enforcement itself only ever needed a key to outlive its own window's +// single rollover. Called by newCounterIncr for every checkAndCount/ +// account entry; a window not among the four handled here is a +// programming error, mirroring windowKey/windowEnd/bucketFor's own panic +// convention. +func enforceTTLFor(window string) time.Duration { + switch window { + case windowMin: + return minWindowTTL + case windowHour: + return 2 * time.Hour + case windowDay: + return 25 * time.Hour + case windowMonth: + return 32 * 24 * time.Hour + default: + panic(fmt.Sprintf("llmgateway: enforceTTLFor: unknown window %q", window)) + } +} + +// clampTTL bounds ttl to at most memoryStoreMaxTTL, but never below +// enforceTTL — see both consts'/enforceTTLFor's own doc comments for why +// the floor must always win a conflict with the ceiling. enforceTTL=0 +// (incrBy's own single-key path, which has no window to derive a floor +// from) means "ceiling only, no floor" — identical to memoryStore's +// pre-round-2 behavior for that path. +func clampTTL(ttl, enforceTTL time.Duration) time.Duration { if ttl > memoryStoreMaxTTL { - return memoryStoreMaxTTL + ttl = memoryStoreMaxTTL + } + if ttl < enforceTTL { + ttl = enforceTTL } return ttl } @@ -241,10 +324,27 @@ func (m *memoryStore) now() time.Time { return m.nowFn() } -// incrBy implements counterStore. ttl is clamped to memoryStoreMaxTTL -// before it is ever applied — see that const's doc comment. +// incrBy implements counterStore. ttl is clamped by clampTTL with no +// enforcement floor (enforceTTL=0) — incrBy has no window to derive one +// 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. func (m *memoryStore) incrBy(key string, n int64, ttl time.Duration) (int64, error) { - ttl = clampTTL(ttl) + return m.applyIncr(key, n, clampTTL(ttl, 0)), nil +} + +// applyIncr increments key by n — creating it fresh, expiring at +// now+ttl, if absent or already expired — and returns its new value. ttl +// must already be clamped by the caller: incrBy clamps with no +// enforcement floor, incrMulti clamps per entry with that entry's own +// enforceTTL floor (see clampTTL). Sharing this one map-mutation-plus- +// opportunistic-sweep implementation between both callers means the +// sweep logic, and the "already expired counts as absent" rule, exist +// exactly once. +func (m *memoryStore) applyIncr(key string, n int64, ttl time.Duration) int64 { now := m.now() m.mu.Lock() defer m.mu.Unlock() @@ -260,7 +360,7 @@ func (m *memoryStore) incrBy(key string, n int64, ttl time.Duration) (int64, err m.data[key] = e } e.value += n - return e.value, nil + return e.value } // get implements counterStore. @@ -289,16 +389,19 @@ func (m *memoryStore) getMulti(keys []string) ([]int64, error) { return out, nil } -// incrMulti implements counterStore by looping over incrBy: memoryStore's -// in-process map access is already effectively free per key, so there is -// no round-trip cost to batch away — this exists purely so memoryStore -// satisfies counterStore's incrMulti contract for the limiter's nil-store -// fallback path (storeIncrMulti). Each entry's ttl is still clamped by -// incrBy itself. +// incrMulti implements counterStore by looping over applyIncr: +// memoryStore's in-process map access is already effectively free per +// key, so there is no round-trip cost to batch away — this exists purely +// so memoryStore satisfies counterStore's incrMulti contract for the +// limiter's nil-store fallback path (storeIncrMulti). Each entry's own +// ttl is clamped with ITS OWN enforceTTL floor (clampTTL), not incrBy's +// floor-less clamp — this is the window-aware path real enforcement +// (checkAndCount/account) relies on; see counterIncr.enforceTTL's own +// doc comment for the bug this fixes. func (m *memoryStore) incrMulti(entries []counterIncr) ([]int64, error) { out := make([]int64, len(entries)) for i, e := range entries { - out[i], _ = m.incrBy(e.key, e.delta, e.ttl) // memoryStore.incrBy never errors + out[i] = m.applyIncr(e.key, e.delta, clampTTL(e.ttl, e.enforceTTL)) } return out, nil } @@ -481,7 +584,10 @@ func (l *limiter) redisStatus() (configured bool, lastErr string, lastErrAt time // or when the store-down latch (storeLatched) is already open from a // recent failure. ok is false only in the fail-closed case — a caller // must refuse the request for that, rather than treating a zero value as -// a real counter reading. +// a real counter reading. storeIncrBy has no production caller since +// checkAndCount/account moved to the batched storeIncrMulti (perf review, +// 2026-08-21) — its only caller, incrCounter below, is itself only +// called from tests, for direct counter seeding. func (l *limiter) storeIncrBy(key string, n int64, ttl time.Duration) (v int64, ok bool) { if l.store == nil { v, _ = l.fallback.incrBy(key, n, ttl) // fallback never errors @@ -672,7 +778,11 @@ func retryAfterSeconds(t time.Time, window string) int { // and returns its new value, together with whether the operation // succeeded under the limiter's fail-open/fail-closed policy (see // storeIncrBy). ok is false only in the fail-closed case: a configured -// store errored and failOpen is false. +// store errored and failOpen is false. incrCounter has no production +// caller since checkAndCount/account moved to the batched incrMulti +// (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). func (l *limiter) incrCounter(kind, id, metric, window string, t time.Time, n int64, ttl time.Duration) (int64, bool) { return l.storeIncrBy(windowKey(kind, id, metric, window, t), n, ttl) } @@ -720,9 +830,9 @@ func (l *limiter) checkAndCount(scopes []limitScope) *limitViolation { entries := make([]counterIncr, 0, len(scopes)*checkAndCountKeysPerScope) for _, sc := range scopes { entries = append(entries, - counterIncr{key: windowKey(sc.kind, sc.id, metricReq, windowMin, now), delta: 1, ttl: minWindowTTL}, - counterIncr{key: windowKey(sc.kind, sc.id, metricReq, windowDay, now), delta: 1, ttl: dayWindowTTL}, - counterIncr{key: windowKey(sc.kind, sc.id, metricReq, windowHour, now), delta: 1, ttl: hourWindowTTL}, + newCounterIncr(sc.kind, sc.id, metricReq, windowMin, now, 1, minWindowTTL), + newCounterIncr(sc.kind, sc.id, metricReq, windowDay, now, 1, dayWindowTTL), + newCounterIncr(sc.kind, sc.id, metricReq, windowHour, now, 1, hourWindowTTL), ) } @@ -869,23 +979,23 @@ func (l *limiter) account(scopes []limitScope, u usage, costMicros int64) { for _, sc := range scopes { if u.prompt != 0 { entries = append(entries, - counterIncr{key: windowKey(sc.kind, sc.id, metricTokIn, windowHour, now), delta: u.prompt, ttl: hourWindowTTL}, - counterIncr{key: windowKey(sc.kind, sc.id, metricTokIn, windowDay, now), delta: u.prompt, ttl: dayWindowTTL}, - counterIncr{key: windowKey(sc.kind, sc.id, metricTokIn, windowMonth, now), delta: u.prompt, ttl: monthWindowTTL}, + newCounterIncr(sc.kind, sc.id, metricTokIn, windowHour, now, u.prompt, hourWindowTTL), + newCounterIncr(sc.kind, sc.id, metricTokIn, windowDay, now, u.prompt, dayWindowTTL), + newCounterIncr(sc.kind, sc.id, metricTokIn, windowMonth, now, u.prompt, monthWindowTTL), ) } if u.completion != 0 { entries = append(entries, - counterIncr{key: windowKey(sc.kind, sc.id, metricTokOut, windowHour, now), delta: u.completion, ttl: hourWindowTTL}, - counterIncr{key: windowKey(sc.kind, sc.id, metricTokOut, windowDay, now), delta: u.completion, ttl: dayWindowTTL}, - counterIncr{key: windowKey(sc.kind, sc.id, metricTokOut, windowMonth, now), delta: u.completion, ttl: monthWindowTTL}, + newCounterIncr(sc.kind, sc.id, metricTokOut, windowHour, now, u.completion, hourWindowTTL), + newCounterIncr(sc.kind, sc.id, metricTokOut, windowDay, now, u.completion, dayWindowTTL), + newCounterIncr(sc.kind, sc.id, metricTokOut, windowMonth, now, u.completion, monthWindowTTL), ) } if costMicros != 0 { entries = append(entries, - counterIncr{key: windowKey(sc.kind, sc.id, metricCost, windowHour, now), delta: costMicros, ttl: hourWindowTTL}, - counterIncr{key: windowKey(sc.kind, sc.id, metricCost, windowDay, now), delta: costMicros, ttl: dayWindowTTL}, - counterIncr{key: windowKey(sc.kind, sc.id, metricCost, windowMonth, now), delta: costMicros, ttl: monthWindowTTL}, + newCounterIncr(sc.kind, sc.id, metricCost, windowHour, now, costMicros, hourWindowTTL), + newCounterIncr(sc.kind, sc.id, metricCost, windowDay, now, costMicros, dayWindowTTL), + newCounterIncr(sc.kind, sc.id, metricCost, windowMonth, now, costMicros, monthWindowTTL), ) } } diff --git a/limits_test.go b/limits_test.go index 2b4737c..222d2d1 100644 --- a/limits_test.go +++ b/limits_test.go @@ -1256,3 +1256,121 @@ func TestTokenBudget_ExactBoundaryViolates(t *testing.T) { t.Error("storeDown must be false for a real limit breach") } } + +// --- review round 2, 2026-08-21: enforcement-aware fallback TTL clamp --- + +// TestClampTTL_ExactMathTable pins clampTTL's ceiling-vs-floor precedence +// for every window's real (ttl, enforceTTL) pair. Round 1's clamp applied +// memoryStoreMaxTTL unconditionally; round 2 corrects it so a window's +// own enforceTTL floor always wins a conflict with that ceiling — this is +// the exact math that keeps min/hour/day capped at 48h while month lands +// back at its own ~32-day floor. +func TestClampTTL_ExactMathTable(t *testing.T) { + cases := []struct { + name string + ttl time.Duration + enforceTTL time.Duration + want time.Duration + }{ + {"min: 2m sits under both ceiling and floor, unchanged", minWindowTTL, enforceTTLFor(windowMin), minWindowTTL}, + {"hour: 48h ttl equals the 48h ceiling; its own 2h floor never applies", hourWindowTTL, enforceTTLFor(windowHour), memoryStoreMaxTTL}, + {"day: 35d ttl clamped down to the 48h ceiling, still above the 25h floor", dayWindowTTL, enforceTTLFor(windowDay), memoryStoreMaxTTL}, + {"month: 400d ttl would clamp to 48h, but the 32d floor wins instead", monthWindowTTL, enforceTTLFor(windowMonth), enforceTTLFor(windowMonth)}, + {"enforceTTL=0 (incrBy's own single-key path): pure ceiling, no floor", 400 * 24 * time.Hour, 0, memoryStoreMaxTTL}, + {"ttl already under both ceiling and floor passes through unchanged", 90 * time.Minute, time.Hour, 90 * time.Minute}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + if got := clampTTL(c.ttl, c.enforceTTL); got != c.want { + t.Errorf("clampTTL(%v, %v) = %v, want %v", c.ttl, c.enforceTTL, got, c.want) + } + }) + } +} + +// TestEnforceTTLFor_MatchesTheOriginalPreHistoryBumpValues pins +// enforceTTLFor's four values directly — they are not arbitrary, they +// match dayWindowTTL/monthWindowTTL's values from before the v0.2 +// data-layer task's history-retention bump (25h/32d), which were +// themselves already windowLength+margin for correct single-window +// enforcement. +func TestEnforceTTLFor_MatchesTheOriginalPreHistoryBumpValues(t *testing.T) { + cases := []struct { + window string + want time.Duration + }{ + {windowMin, 2 * time.Minute}, + {windowHour, 2 * time.Hour}, + {windowDay, 25 * time.Hour}, + {windowMonth, 32 * 24 * time.Hour}, + } + for _, c := range cases { + if got := enforceTTLFor(c.window); got != c.want { + t.Errorf("enforceTTLFor(%q) = %v, want %v", c.window, got, c.want) + } + } +} + +// TestMemoryStore_IncrMulti_MonthCounterSurvivesPast48hAndStillEnforces +// is the review-round-2 regression case: a month counter written via +// incrMulti — the real production path, checkAndCount/account — must +// stay alive, not silently reset to 0, well past 48h, and a +// TokensPerMonth limit must still enforce correctly at that point. This +// is exactly what round 1's unconditional 48h ceiling broke: a month +// counter's key would have expired and reset roughly every two days, +// turning the budget into a rolling ~48h one instead of a true +// calendar-month one whenever Redis is absent or down. +func TestMemoryStore_IncrMulti_MonthCounterSurvivesPast48hAndStillEnforces(t *testing.T) { + l := newLimiter(nil, true) // nil store -> every operation uses the in-process fallback + now := time.Date(2026, 8, 1, 0, 0, 0, 0, time.UTC) + l.nowFn = func() time.Time { return now } + scopes := []limitScope{{kind: "group", id: "g", limits: &LimitsConfig{TokensPerMonth: 100}}} + + l.account(scopes, usage{prompt: 60, completion: 20}, 0) // 80 tokens on day 1 + + // Advance 5 days: well past memoryStoreMaxTTL's 48h ceiling, far short + // of monthWindowTTL's own natural rollover. + now = now.Add(5 * 24 * time.Hour) + l.nowFn = func() time.Time { return now } + + tokIn, ok := l.getCounter("group", "g", metricTokIn, windowMonth, now) + if !ok || tokIn != 60 { + t.Fatalf("tokin:month after 5 days = %d, ok=%v, want 60 (the key must not have expired at 48h)", tokIn, ok) + } + + // Push the total over budget and confirm enforcement still fires — + // proves the surviving counter is actually read by + // tokenBudgetViolation, not merely present. + l.account(scopes, usage{completion: 21}, 0) // 60 + 41 = 101 > 100 + if v := l.checkAndCount(scopes); v == nil { + t.Fatal("want a TokensPerMonth violation 5 days in; the round-1 clamp bug would have reset the counter well before this") + } +} + +// TestMemoryStore_IncrMulti_DayCounterStillClampsAt48h asserts the +// round-1 clamp's original goal — bounding the fallback's own live-key +// growth — still holds for day, whose own enforcement floor (25h) sits +// below the 48h ceiling: a day counter written via incrMulti expires and +// resets once its key crosses 48h, well before dayWindowTTL's own 35-day +// retention would otherwise have kept it alive. +func TestMemoryStore_IncrMulti_DayCounterStillClampsAt48h(t *testing.T) { + m := newMemoryStore() + now := time.Date(2026, 8, 1, 0, 0, 0, 0, time.UTC) + m.nowFn = func() time.Time { return now } + + entry := counterIncr{key: "k", delta: 10, ttl: dayWindowTTL, enforceTTL: enforceTTLFor(windowDay)} + if _, err := m.incrMulti([]counterIncr{entry}); err != nil { + t.Fatal(err) + } + + now = now.Add(49 * time.Hour) // just past the 48h ceiling + m.nowFn = func() time.Time { return now } + + got, err := m.get("k") + if err != nil { + t.Fatal(err) + } + if got != 0 { + t.Errorf("day counter at 49h past write = %d, want 0 (expired at the 48h ceiling; day's own 25h floor never raises it)", got) + } +}