Skip to content

test: direct suite for the extracted account rate-limit helpers#580

Merged
ndycode merged 2 commits into
mainfrom
claude/audit-61-rate-limits-tests
Jun 11, 2026
Merged

test: direct suite for the extracted account rate-limit helpers#580
ndycode merged 2 commits into
mainfrom
claude/audit-61-rate-limits-tests

Conversation

@ndycode

@ndycode ndycode commented Jun 10, 2026

Copy link
Copy Markdown
Owner

Summary

What Changed

One new mock-free test file pinning the module's contracts directly:

  • parseRateLimitReason keyword taxonomy: quota (quota/usage_limit), tokens (token/tpm/rpm), concurrent (concurrent/parallel), case-insensitivity, and the deliberate unknown bucket for generic 429 codes like rate_limit_exceeded that carry none of the keywords.
  • getQuotaKey: bare family key vs family:model scoping (null/undefined model falls back to the family key).
  • clampNonNegativeInt: fallback on non-numbers/NaN/Infinity, clamp-to-zero on negatives, floor on fractionals.
  • clearExpiredRateLimits boundary semantics: now >= reset deletes (an entry expiring exactly now is cleared), future entries and dangling undefined values survive.
  • isRateLimitedForQuotaKey strict-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 to 0s, negative clamps to 0s, the 1m 0s boundary, and minutes-only formatting for long waits (no hours unit).

Time-dependent cases use vi.useFakeTimers() + vi.setSystemTime with real timers restored in afterEach (nowMs() is a direct Date.now() wrapper).

Validation

  • npm run typecheck (also runs in the pre-commit hook)
  • npx eslint test/account-rate-limits.test.ts --max-warnings=0
  • npm test -- test/account-rate-limits.test.ts — 35/35

Docs and Governance Checklist

  • Test-only; no user-visible behavior, commands, settings, or paths changed

Risk and Rollback

  • Risk level: minimal — one new test file, no source changes.
  • Rollback plan: delete the test file.

Additional Notes

  • Independent of every other open branch; based on 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 of lib/accounts/rate-limits.ts directly rather than through the accounts.ts facade. both gaps flagged in the previous review round (Number.NEGATIVE_INFINITY in clampNonNegativeInt and the null model path in isRateLimitedForFamily) 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-level afterEach(() => 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

Filename Overview
test/account-rate-limits.test.ts new direct test suite (35 cases) for lib/accounts/rate-limits.ts — all behavioral contracts verified, fake-timer usage correct, previous review gaps (-Infinity and null model) addressed

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"]
Loading

Fix All in Codex

Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
test/account-rate-limits.test.ts:56-68
**`clampNonNegativeInt``null` input not covered**

`value: unknown` means `null` is a valid caller input: `typeof null !== "number"` short-circuits to the fallback, the same as `undefined`. it's a separate falsy type though, and the existing row labelled `"undefined"` doesn't pin this branch. adding `["null", null, 7, 7]` would explicitly document that `null` is treated identically to `undefined` and guard against a future tightening of the type guard.

Reviews (2): Last reviewed commit: "test: pin -Infinity fallback and null-mo..." | Re-trigger Greptile

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
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

@coderabbitai

coderabbitai Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@ndycode, we couldn't start this review because you've reached your PR review rate limit.

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 @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: ae00272a-4826-4c64-8e96-7b65ea40219e

📥 Commits

Reviewing files that changed from the base of the PR and between b566656 and ea81528.

📒 Files selected for processing (1)
  • test/account-rate-limits.test.ts
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/audit-61-rate-limits-tests
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch claude/audit-61-rate-limits-tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Comment thread test/account-rate-limits.test.ts
Comment thread test/account-rate-limits.test.ts
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
@ndycode ndycode merged commit cb371b6 into main Jun 11, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants