test: direct suite for the extracted account rate-limit helpers#580
Conversation
lib/accounts/rate-limits.ts was reachable only through the accounts.ts facade; these 35 cases pin the module's contracts directly: - parseRateLimitReason keyword taxonomy incl. the generic-429 unknown bucket and case-insensitivity - getQuotaKey family vs model-scoped keys - clampNonNegativeInt fallback/floor/clamp rules - clearExpiredRateLimits boundary semantics (now >= reset deletes) - isRateLimitedForQuotaKey strict-future check - isRateLimitedForFamily model-key precedence, family-wide application, and the expired-entry pruning side effect - formatWaitTime rounding and minutes-only formatting https://claude.ai/code/session_01XNtnkLbBiXZxfQQYLMpucB
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
Warning Review limit reached
More reviews will be available in 27 minutes and 40 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
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 |
Review follow-ups: -Infinity short-circuits the non-finite guard before the negative clamp (fallback, not 0), and a null model skips the model-scoped key the same way undefined does. https://claude.ai/code/session_01XNtnkLbBiXZxfQQYLMpucB
Summary
test/account-rate-limits.test.ts(35 cases) forlib/accounts/rate-limits.ts, which was previously reachable only through theaccounts.tsfacade — the same extracted-module gap the test: cover login-oauth account selection and cancellation predicates #559–test: property-check write-queue serialization and clamp invariants #575 wave closed elsewhere.What Changed
One new mock-free test file pinning the module's contracts directly:
parseRateLimitReasonkeyword taxonomy: quota (quota/usage_limit), tokens (token/tpm/rpm), concurrent (concurrent/parallel), case-insensitivity, and the deliberateunknownbucket for generic 429 codes likerate_limit_exceededthat carry none of the keywords.getQuotaKey: bare family key vsfamily:modelscoping (null/undefined model falls back to the family key).clampNonNegativeInt: fallback on non-numbers/NaN/Infinity, clamp-to-zero on negatives, floor on fractionals.clearExpiredRateLimitsboundary semantics:now >= resetdeletes (an entry expiring exactly now is cleared), future entries and danglingundefinedvalues survive.isRateLimitedForQuotaKeystrict-future check (a reset time of exactly now is not limited).isRateLimitedForFamily: a model-scoped limit blocks only that model, a family-wide limit blocks every model in the family, and expired entries are pruned from the entity as a side effect before answering.formatWaitTime: sub-second floors to0s, negative clamps to0s, the1m 0sboundary, and minutes-only formatting for long waits (no hours unit).Time-dependent cases use
vi.useFakeTimers()+vi.setSystemTimewith real timers restored inafterEach(nowMs()is a directDate.now()wrapper).Validation
npm run typecheck(also runs in the pre-commit hook)npx eslint test/account-rate-limits.test.ts --max-warnings=0npm test -- test/account-rate-limits.test.ts— 35/35Docs and Governance Checklist
Risk and Rollback
Additional Notes
main.https://claude.ai/code/session_01XNtnkLbBiXZxfQQYLMpucB
Generated by Claude Code
note: greptile review for oc-chatgpt-multi-auth. cite files like
lib/foo.ts:123. confirm regression tests + windows concurrency/token redaction coverage.Greptile Summary
adds
test/account-rate-limits.test.ts, a 35-case mock-free suite that pins the full public contract oflib/accounts/rate-limits.tsdirectly rather than through theaccounts.tsfacade. both gaps flagged in the previous review round (Number.NEGATIVE_INFINITYinclampNonNegativeIntand thenullmodel path inisRateLimitedForFamily) are addressed in this revision.parseRateLimitReason: covers all four buckets (quota, tokens, concurrent, unknown) plus case-insensitivity and the deliberate unknown bucket for generic 429 codes.clearExpiredRateLimits/isRateLimitedForQuotaKey/isRateLimitedForFamily: fake-timer isolation via a top-levelafterEach(() => vi.useRealTimers())is correct; boundary semantics (reset-exactly-now is not limited, pruning as a side effect) are all asserted.formatWaitTime: sub-second floor, negative clamp, minute boundary, and large-minute (no hours) formatting are all verified.Confidence Score: 5/5
test-only change with no source modifications — safe to merge.
single new test file, no production code touched. fake-timer setup and teardown are correct, all behavioral contracts for the module are pinned, and the two gaps from the previous review round are resolved. no token handling, no filesystem operations, no concurrency surface introduced.
no files require special attention.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A["isRateLimitedForFamily(entity, family, model?)"] --> B["clearExpiredRateLimits(entity)\nnow >= reset → delete key"] B --> C{model truthy?} C -- yes --> D["getQuotaKey(family, model)\n→ family:model"] D --> E["isRateLimitedForQuotaKey\nnow < reset?"] E -- true --> F["return true"] E -- false --> G["getQuotaKey(family)\n→ family"] C -- no/null --> G G --> H["isRateLimitedForQuotaKey\nnow < reset?"] H -- true --> I["return true"] H -- false --> J["return false"]Prompt To Fix All With AI
Reviews (2): Last reviewed commit: "test: pin -Infinity fallback and null-mo..." | Re-trigger Greptile