Repository navigation
Open
Labels
app/corePackage: core (pkgs/ or apps/ in this repo)Package: core (pkgs/ or apps/ in this repo)kind/bugSomething isn't working (prefixed form of the classic bug label)Something isn't working (prefixed form of the classic bug label)priority/lowNice to have; do when convenientNice to have; do when convenientqueue/queuedHas a queued task in the Supabase agent queueHas a queued task in the Supabase agent queuesource/pr-comment-reviewFiled by the pr-comment-review jobFiled by the pr-comment-review jobstatus/needs-triageNot yet triaged; needs a first lookNot yet triaged; needs a first looktask/triageQueue packet kind: verify, reproduce, and route this itemQueue packet kind: verify, reproduce, and route this item
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).
Metadata
Metadata
Assignees
Labels
app/corePackage: core (pkgs/ or apps/ in this repo)Package: core (pkgs/ or apps/ in this repo)kind/bugSomething isn't working (prefixed form of the classic bug label)Something isn't working (prefixed form of the classic bug label)priority/lowNice to have; do when convenientNice to have; do when convenientqueue/queuedHas a queued task in the Supabase agent queueHas a queued task in the Supabase agent queuesource/pr-comment-reviewFiled by the pr-comment-review jobFiled by the pr-comment-review jobstatus/needs-triageNot yet triaged; needs a first lookNot yet triaged; needs a first looktask/triageQueue packet kind: verify, reproduce, and route this itemQueue packet kind: verify, reproduce, and route this item
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).