fix(codex): retain the K12 short-window quota end to end - #2141
Conversation
parseUsageQuota filled shortPercent and setAccountQuotaFromParsed dropped it, so the 5-hour burst window never reached the cache, the accounts DTO, the dashboard, or routing. A saturated short window was invisible to account selection. Carries @Ingwannu's #2056: shortPercent joins hasKnownQuotaValue, a new snapshotHasShort keeps a short-only snapshot from reading as empty, partial weekly/monthly snapshots no longer clobber a known short window, and updateAccountQuota carries the tuple. Also fixes the blocker raised in review on both #2056 and #2062: the scorer took Math.max over every finite window, so a snapshot carrying only shortPercent: 0 scored a flat 0 and made an account whose long windows were never observed look like the emptiest in the pool - pickLowestUsageAmong would then send every request to it. The burst window now refines a known long-window position instead of standing in for one, and returns CODEX_UNKNOWN_USAGE_SCORE until a governing window is actually observed. The ported test asserted the old behavior directly (computeCodexUsageScore({ shortPercent: 0 }) === 0); it is replaced by a case that pins the corrected contract in both directions. Closes #2047
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
✅ Deterministic PR hygiene checks passed. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
Included review availability: Your plan provides up to 10 included reviews per hour; 2 remain after this review. 📝 WalkthroughWalkthroughQuota handling now retains sub-day burst-window fields during detection, cache merges, credits-only updates, and incremental updates. Routing incorporates short-window usage only when weekly or monthly usage is known. Tests cover persistence, DTO projection, parsing, and plan-specific scoring. ChangesShort-window quota handling
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to The quota propagation and routing behavior is supported by passing tests and type checks, and no actionable merge-blocking risk remains. Sequence Diagram(s)sequenceDiagram
participant WHAM
participant parseUsageQuota
participant QuotaCache
participant AccountDTO
participant computeCodexUsageScore
WHAM->>parseUsageQuota: quota windows
parseUsageQuota->>QuotaCache: weekly, monthly, and short-window fields
QuotaCache->>AccountDTO: retained quota fields
QuotaCache->>computeCodexUsageScore: known long-window and short-window usage
computeCodexUsageScore-->>QuotaCache: usage score
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
리뷰 · 우선순위 62 / 80#2047 맞음. #2056을 가져오면서 리뷰가 막았던 함정을 고침. #2056이랑 #2062를 이걸로 닫으면 됨. #2063은 이미 #2055에 먹힘. #2046 점수는 62임. 풀 픽이 가짜 0으로 쏠리면 실사용임. 2.27 필수까지는 아님. #2137보다 뒤. 해결방안: 머지. 이 댓글은 grok-bot이 작성했습니다 |
1 participant
Summary
parseUsageQuotafilledshortPercentandsetAccountQuotaFromParseddropped it, so the K12 5-hour burst window never reached the cache, the accounts DTO, the dashboard, or routing (#2047). A saturated short window was invisible to account selection.Credit: @Ingwannu's #2056 is the implementation carried here —
shortPercentjoinshasKnownQuotaValue, a newsnapshotHasShortstops a short-only snapshot reading as empty, partial weekly/monthly snapshots no longer clobber a known short window, andupdateAccountQuotacarries the tuple. @yzxcj797's #2062 targeted the same issue with a narrower cache write.The blocker both PRs shared, now fixed
Review flagged this on #2056 and #2062:
computeCodexUsageScoretookMath.maxover every finite window, so a snapshot carrying onlyshortPercent: 0scored a flat 0. An account whose weekly/monthly usage was never observed then looked like the emptiest in the pool, andpickLowestUsageAmongwould route everything to it — the opposite of what the quota data is for.The burst window now refines a known long-window position rather than standing in for one: with no governing window observed, the score stays
CODEX_UNKNOWN_USAGE_SCORE.Worth flagging explicitly: the ported test asserted the old behavior directly (
computeCodexUsageScore({ shortPercent: 0 })→0). I did not delete it quietly — it is replaced by a case that pins the corrected contract in both directions, including that a hot burst still wins once a long window is known.Verification
src/codex/quota.tsandsrc/codex/routing.tsfails 6 tests, including the new short-only guard.bun test --isolate tests— 13,538 pass, 0 fail, 10 skip (855 files).bun test --isolate tests/codex-routing.test.ts tests/codex-auth-api.test.ts— 319 pass, 0 fail.bun run typecheck— clean.bun run privacy:scan— passed.Supersedes
Closes #2056 (@Ingwannu) and #2062 (@yzxcj797) once merged, both with attribution. #2063 (@yzxcj797) was already superseded by the merged #2055 and is closed.
Checklist
Closes #2047
Summary by CodeRabbit