Repository navigation
fix(core): live recordQuota pruning loses quota events for a Location that registers later with a wider demotion window (PR #1605 CodeRabbit follow-up unaddressed) #1699
Description
Finding (from PR review)
CodeRabbit (coderabbitai[bot]) raised this finding as a follow-up in its review of merged PR #1605 and it was never fixed or responded to:
- PR: fix(core): judge demotion by the judging Location's settings (#1484) #1605 — "fix(core): judge demotion by the judging Location's settings (fix(core): demoted() ignores the judging Location's settings demotion — two #1423 model-route tests fail on main #1484)" (MERGED 2026-11-04T08:00:35Z)
- Review: fix(core): judge demotion by the judging Location's settings (#1484) #1605 (review) (coderabbitai[bot], 2026-11-04T07:32:44Z, COMMENTED; follow-up finding anchored at
packages/core/src/model-route.ts:1133-1135) - Essence: "Keep quota events for a Location whose wider settings window registers later.
recordQuotaprunes events with only the readers registered at that moment. A Location that registers later with a widerdemotion.windowMscannot recover events already pruned. Its demotion may lag until new quota errors accumulate. A fix needs a retention design change, not a localized edit." (Reviewer severity: Minor · Functional Correctness · Poor tradeoff.)
No-response evidence
- The PR's only commit (9748389, 2026-11-04T07:25:18Z) is the head CodeRabbit reviewed; the PR merged at 2026-11-04T08:00:35Z on that same head — no fix commit followed the review.
- The PR conversation contains only the Codex review summary (completed,
has_any_findings:false, on a different aspect) and CodeRabbit's own summary comment. No comment replies to, acknowledges, or defers this follow-up finding, and there is no linked tracking issue for it. - Dedupe (searched 2026-11-05, open + closed, by
recordQuota, PR number 1605, and demotion-retention keywords): issue fix(core): demoted() ignores the judging Location's settings demotion — two #1423 model-route tests fail on main #1484 (closed, the PR's own issue) covers judging a Location by its own settings and unioning both config layers for registered readers; issue fix(core): model_route_settings reader is a module global — route settings leak across Locations (#1142) #1423 (closed) covers the per-Location settings readers. Neither covers live-recorded events pruned before a wider-window Location registers.
Still live on current main (verified 2026-11-05 by two independent gh api raw reads)
packages/core/src/model-route.ts:
recordQuota(lines 1183–1192):retentionMs = retainedDemotionWindow()and the stored events are filtered tonow - at <= retentionMsat record time, then capped with.slice(-50).retainedDemotionWindow(lines 1124–1139) unions only the readers registered at that moment (demotionReaders+settingsReaders); a Location whose graph activates later is not — and cannot be — in that union.- Contrast with the restore path in the same file:
seedDemotion(lines 2396–2405) deliberately replays ledger rows withModelRouteLog.MAX_DEMOTION_WINDOW_MS(7,776,000,000 ms ≈ 90 days,model-route-log.ts:501) precisely because "a Location with a longer demotion window can activate after the restore, and pruning the replay against the then-current retention … would discard failures its policy still counts, unrecoverably". The live recording path has no equivalent protection, so in a long-running process the same loss happens for live events: they survive in the durable ledger but are gone from the in-memoryquotaEventscache until a restart re-seeds them, anddemoted()for the later Location under-counts in the meantime.
Acceptance criteria
- Live quota-event retention preserves events that a later-registering Location could still count under the widest supported demotion window (e.g. record with the
MAX_DEMOTION_WINDOW_MShorizon, asseedDemotionalready does), or another documented retention policy with the same guarantee;demoted()keeps applying each judging Location's own window over what is retained. - A regression test covers the scenario: quota events recorded while only a narrow-window Location is registered still count toward demotion for a wider-window Location that registers afterwards, without a restart.
- Or: the finding is resolved on this issue by a documented decision with reasoning recorded (e.g. an argued case that later registration cannot occur in-process, or that restart reseeding bounds the impact acceptably).
Activity
dezren39 commentedon Nov 5, 2026
🤖 Agent note · filed
recordQuotapruning loses quota events for a Location that registers later with a wider demotion window (packages/core/src/model-route.ts).dezren39 commentedon Nov 6, 2026
🤖 Agent note · picked_up
dezren39 commentedon Nov 6, 2026
🤖 Agent note · picked up
dezren39 commentedon Nov 6, 2026
Triage verdict — finding confirmed on current main
Fresh GitHub API and raw reads on 2026-11-06 confirm issue #1699 is still open and source PR #1605 is merged at . at prunes by , which only sees currently registered readers (); already passes the full horizon (, 7,776,000,000 ms). No competing open PR or active fix task was found. This confirms the later-activation retention gap.
Routed a scoped fix packet as task 10865 (): preserve the supported retention horizon, keep per-Location judgment windows, and add a no-restart regression test. The primary clone's existing uncommitted changes were left untouched.
🤖 Agent note · handed off
dezren39 commentedon Nov 6, 2026
Triage verdict — finding confirmed on current main
Fresh GitHub API and raw main reads on 2026-11-06 confirm issue #1699 is open and source PR #1605 is merged at 9748389. In packages/core/src/model-route.ts, recordQuota (lines 1183-1192) prunes by retainedDemotionWindow(), whose readers are those registered at record time (lines 1124-1137); seedDemotion (lines 2392-2405) already passes MAX_DEMOTION_WINDOW_MS (7,776,000,000 ms). No competing open PR or active fix task was found. The later-activation retention gap is confirmed.
Routed the scoped follow-up as task 10865 with dedupe key repo-health:v1:fix:developing-today/opencode:issue:1699. It preserves the supported retention horizon, keeps per-Location judgment windows, and adds a no-restart regression test. The primary clone's pre-existing uncommitted changes remain untouched.
🤖 Agent note · handed off