fix(request): remove double-exponential backoff in getRateLimitBackoffWithReason

[ndycode]

Addresses deep audit finding REQ-HIGH-01.

getRateLimitBackoff() already applies exponential + jitter. getRateLimitBackoffWithReason() was then calling calculateBackoffMs() which applied 2^(attempt-1) a second time, causing delays to hit the 60s cap after ~2 consecutive 429s.

Fix: calculateBackoffMs now applies only the reason multiplier to the already-exponential delayMs.

Regression test added asserting attempt=3 delay stays under cap and ratio vs attempt=1 is ~4x (single exponential) not >16x (double exponential).

See .sisyphus/notepads/deep-audit/reports/request.json for the original finding.

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

this PR fixes the double-exponential backoff bug (REQ-HIGH-01): calculateBackoffMs was re-applying 2^(attempt-1) on top of the already-exponential delayMs produced by getRateLimitBackoff, causing delays to saturate maxBackoffMs after ~2 consecutive 429s. the fix removes the exponential factor from calculateBackoffMs so it applies only the reason multiplier, and the regression test correctly pins the attempt=3/attempt=1 ratio at ~4x (single exponential). the change is self-contained and well-commented.

• _attempt is silently ignored in the public calculateBackoffMs signature — consumers reading the compiled .d.ts see attempt: number and will expect it to influence the result, but it is a no-op. the doc comment explains the intent but the parameter should carry a stronger @deprecated signal or be dropped from a future minor version.

Confidence Score: 5/5

safe to merge — core fix is correct, regression test is deterministic, no production callers of calculateBackoffMs outside getRateLimitBackoffWithReason

all findings are P2: the _attempt no-op in the public API and the zero-jitter-only regression test are style/coverage concerns, not present defects. the double-exponential is correctly removed, and the only internal caller (getRateLimitBackoffWithReason) is fully updated and covered.

lib/request/rate-limit-backoff.ts — calculateBackoffMs JSDoc should explicitly mark _attempt as ignored for external consumers

Important Files Changed

Filename	Overview
lib/request/rate-limit-backoff.ts	core fix: removes 2^(attempt-1) from calculateBackoffMs; _attempt is now a no-op in a publicly exported function, which is a silent API contract change for direct callers
test/rate-limit-backoff.test.ts	regression test added with deterministic jitter-mocked assertions; existing tests updated to match new semantics; no property-based coverage for non-zero jitter

Sequence Diagram

sequenceDiagram
    participant Caller
    participant GRLBWR as getRateLimitBackoffWithReason
    participant GRLB as getRateLimitBackoff
    participant State as rateLimitStateByAccountQuota
    participant CBM as calculateBackoffMs

    Caller->>GRLBWR: (accountIndex, quotaKey, serverRetryAfterMs, reason)
    GRLBWR->>GRLB: (accountIndex, quotaKey, serverRetryAfterMs)
    GRLB->>State: get previous state
    GRLB->>GRLB: baseDelay * 2^(attempt-1) + jitter
    GRLB->>State: store {attempt, jitteredDelayMs}
    GRLB-->>GRLBWR: {attempt, delayMs (exponential+jitter), isDuplicate}

    alt attempt === 1 && !isDuplicate
        GRLBWR->>CBM: (normalizedBaseDelay, _attempt, reason)
        Note over CBM: BEFORE: normalizedBaseDelay * 2^(attempt-1) * multiplier<br/>AFTER: normalizedBaseDelay * multiplier only
    else attempt > 1 OR isDuplicate
        GRLBWR->>CBM: (result.delayMs, _attempt, reason)
        Note over CBM: BEFORE: alreadyExponentialDelay * 2^(attempt-1) * multiplier double-exp<br/>AFTER: alreadyExponentialDelay * multiplier only
    end

    CBM-->>GRLBWR: min(baseDelayMs * multiplier, maxBackoffMs)
    GRLBWR-->>Caller: {attempt, delayMs (single-exponential + reason), isDuplicate, reason}

Loading

Prompt To Fix All With AI

This is a comment left during a code review.
Path: lib/request/rate-limit-backoff.ts
Line: 246-256

Comment:
**`_attempt` silently ignored in exported public API**

`calculateBackoffMs` is re-exported via `lib/index.ts` as part of this package's public surface. the compiled `.d.ts` will still show `_attempt: number` (TypeScript preserves the leading-underscore name in declarations), and any direct caller who passes `attempt > 1` expecting `baseDelayMs * 2^(attempt-1) * multiplier` now silently gets just `baseDelayMs * multiplier`. the JSDoc comment explains the reasoning well, but the parameter should carry a `@deprecated`/`@unused` annotation or a `@param _attempt` note so consumers don't have to read the body to understand the no-op:

```typescript
/**
 * ...existing note...
 *
 * @param _attempt - **ignored**; retained for call-site compatibility only.
 *   Do not rely on this parameter influencing the return value.
 */
export function calculateBackoffMs(
    baseDelayMs: number,
    _attempt: number,
    reason: RateLimitReason = "unknown",
): number {
```

no windows/token-safety risk here, but worth noting for consumers who import this function directly.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: test/rate-limit-backoff.test.ts
Line: 355-405

Comment:
**regression test exclusively uses zero-jitter mock — no property-based coverage**

all three attempt assertions use the `beforeEach` mock `Math.random = 0.5` which makes `Math.random() * 2 - 1 = 0`, zeroing jitter entirely. the ratio guard (`ratio < 8`) is good, but it only fires in a degenerate case. under real jitter the ratio could still approach 16x if positive jitter compounds across attempts without the fix — and the current test wouldn't catch a regression introduced per-attempt-jitter.

the project already has `test/property/` for rotation invariants. a fast-check property-based test seeding `Math.random` with arbitrary values and asserting `ratio < 8` for any jitter scenario would make this the canonical proof. even a single extra `it` that tests with `Math.random = 1` (max positive jitter, +20%) and `Math.random = 0` (max negative jitter) would add meaningful coverage without requiring fast-check.

not blocking, but flagging given the 80% coverage threshold and the existing property-test convention in this repo.

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (1): Last reviewed commit: "fix(request): remove double-exponential ..." | Re-trigger Greptile}

[chatgpt-codex-connector]

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]

Warning

Rate limit exceeded

@ndycode has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 58 minutes and 11 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 58 minutes and 11 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: ede94347-70e8-452a-a71f-7017640b8e57

📥 Commits

Reviewing files that changed from the base of the PR and between 3f1c1fe and 98ee7a2.

📒 Files selected for processing (2)

• lib/request/rate-limit-backoff.ts
• test/rate-limit-backoff.test.ts

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/rate-limit-backoff-double-exponential

✨ Simplify code

• Create PR with simplified code
• Commit simplified code in branch fix/rate-limit-backoff-double-exponential

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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