feat: add shared AI rate limiting for batch analysis - #265
Merged
Conversation
Handle 429/Retry-After/RPM across the batch AI analysis pipeline: - Add AIRateLimiter (src/services/aiRequestLimiter.ts): shared cooldown + jittered exponential backoff, optional per-minute sliding window and concurrency semaphore, honors provider Retry-After (capped). - aiService: throw AIRequestError(status, retryAfterMs) on non-OK responses (OpenAI/Claude/Gemini), plus isRateLimitedError/getRetryAfterMsFromError helpers used by the limiter. - Server: relay upstream rate-limit headers (retry-after, retry-after-ms, x-ratelimit-*, anthropic-ratelimit-*) on /proxy/ai, /proxy/github*, /proxy/webdav; expose Retry-After via CORS so the renderer can read it. - backendAdapter: attach retryAfterMs to translated 429 proxy errors. - AIAnalysisOptimizer: acquire/release a limiter slot per request, notify on 429 with cooldown-aware retry delay; keep response-time sampling free of limiter queuing so adaptive concurrency is not distorted. - Wire optional AIConfig.requestsPerMinute into optimizer at both call sites (default unlimited). Tests: limiter unit tests, optimizer 429 integration tests, server relay test; all frontend/server suites green.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR forwards approved upstream rate-limit headers through proxy responses. It adds structured retry metadata, a shared ChangesProxy rate-limit header forwarding
Shared AI request rate limiting
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant AIAnalysisOptimizer
participant AIRateLimiter
participant AIService
AIAnalysisOptimizer->>AIRateLimiter: acquire()
AIRateLimiter-->>AIAnalysisOptimizer: release callback
AIAnalysisOptimizer->>AIService: execute AI request
AIService-->>AIAnalysisOptimizer: success or AIRequestError
AIAnalysisOptimizer->>AIRateLimiter: notifySuccess() or notifyRateLimit(retryAfterMs)
AIAnalysisOptimizer->>AIRateLimiter: acquire() before retry
🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@server/src/index.ts`:
- Line 30: Update the CORS configuration’s exposedHeaders list in the server
setup to include X-RateLimit-Remaining, X-RateLimit-Reset, and
x-ratelimit-remaining alongside the existing headers, preserving the proxy
response headers for browser clients.
In `@src/services/aiAnalysisOptimizer.ts`:
- Around line 253-259: Update src/services/aiAnalysisOptimizer.ts lines 253-259
in the worker analysis flow to use one optimizer-level batch abort signal for
both AIRateLimiter.acquire and aiService.analyzeRepository, rather than the
per-worker controller signal. Update lines 291-295 in the retry-delay logic to
make the wait abortable with that same batch signal, ensuring abort immediately
stops limiter waits, AI requests, and retry delays.
In `@src/services/aiRequestLimiter.ts`:
- Around line 70-78: Make the admission logic in acquire atomic: serialize
concurrent acquisitions or combine the concurrency and RPM checks into one loop
that revalidates both limits immediately before incrementing active and
recording requestTimestamps. Preserve abort handling and ensure every successful
reservation updates both counters within the same critical section. Add
concurrent Promise.all tests covering maxConcurrency and requestsPerMinute
limits.
- Around line 108-111: The retry wait calculation in the limiter must never
shorten an accepted provider Retry-After value: apply jitter only to local
exponential backoff, then preserve Retry-After as the minimum before applying
the existing cap and cooldown update. Update src/services/aiRequestLimiter.ts
lines 108-111 accordingly; update src/services/aiRequestLimiter.test.ts lines
65-72 to assert the wait is at least the accepted Retry-After duration.
In `@src/services/backendAdapter.ts`:
- Around line 232-248: Update the Retry-After parsing in the 429 handling of the
backend adapter: parse numeric seconds with Number, and when parsing fails,
compute the delay from Date.parse(retryAfter) minus Date.now(), converting it to
milliseconds and retaining only finite positive values. Preserve retry-after-ms
precedence, and add coverage for both numeric and HTTP-date formats.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a402b6e8-e769-4b13-b026-9f0baa684ff1
📒 Files selected for processing (13)
server/src/index.tsserver/src/routes/proxy.tsserver/tests/routes/githubProxyRoute.test.tssrc/components/DiscoveryView.tsxsrc/components/RepositoryList.tsxsrc/services/aiAnalysisOptimizer.test.tssrc/services/aiAnalysisOptimizer.tssrc/services/aiRequestLimiter.test.tssrc/services/aiRequestLimiter.tssrc/services/aiService.test.tssrc/services/aiService.tssrc/services/backendAdapter.tssrc/types/index.ts
- CORS: expose X-RateLimit-Remaining/X-RateLimit-Reset/x-ratelimit-remaining so browser clients can read the GitHub rate-limit headers the proxy relays. - Optimizer: use one batch-level AbortController shared by every worker so abort() immediately stops all limiter waits, in-flight AI requests and retry delays (previously only the most recent per-attempt controller was aborted); retry delay is now abortable via the same batch signal. - Limiter: make admission atomic by re-validating cooldown/RPM/concurrency synchronously right before reserving, so concurrent acquires can never exceed maxConcurrency or requestsPerMinute; add Promise.all concurrency tests. - Limiter: apply jitter only to local exponential backoff and keep the provider Retry-After as a floor, so an accepted Retry-After is never shortened. - BackendAdapter: parse HTTP-date Retry-After (fall back to Date.parse) in addition to numeric seconds; add tests for retry-after-ms, numeric and HTTP-date forms. - Docstrings added per CodeRabbit coverage warning.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/services/aiAnalysisOptimizer.ts`:
- Around line 82-86: Update the waitWhilePaused() and both concurrency-monitor
wait paths to use an abort-aware delay tied to batchAbortController.signal, so
abort() interrupts every pending wait. Propagate batchAbortController.signal
through the fetchReadme() backend and GitHub request APIs and their callers,
ensuring all pipelined requests observe cancellation and Promise.all() can
settle after abort.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 64baa851-0716-4c13-88f9-e84d16e3dc80
📒 Files selected for processing (7)
server/src/index.tssrc/services/aiAnalysisOptimizer.test.tssrc/services/aiAnalysisOptimizer.tssrc/services/aiRequestLimiter.test.tssrc/services/aiRequestLimiter.tssrc/services/backendAdapter.test.tssrc/services/backendAdapter.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- src/services/backendAdapter.ts
- src/services/aiAnalysisOptimizer.test.ts
- server/src/index.ts
- src/services/aiRequestLimiter.ts
Follow-up to CodeRabbit review: abort() now also interrupts: - waitWhilePaused() polling (abortableDelay instead of plain delay) - both concurrency monitors (abortableDelay(1000)) - the inter-batch delay in prefetchReadmes - in-flight README fetches in the pipelined flow by propagating the batchAbortController.signal into fetchReadme() -> backend.getRepositoryReadme / githubApi.getRepositoryReadme, so Promise.all() settles promptly after cancellation instead of staying pending on long-running README requests. Adds a regression test proving abort() settles analyzeRepositoriesPipelined even while a never-completing README request is still in flight.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/services/aiAnalysisOptimizer.test.ts`:
- Around line 149-155: Update the getRepositoryReadme mock and related
cancellation test to expose a request-start signal, await that signal instead of
relying on the fixed 100 ms delay, and handle an already-aborted AbortSignal
immediately. Track whether the signal’s abort callback fired, then assert that
the README request was actually aborted after the batch cancellation path
completes.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a6ffef9b-fe62-4938-9bf7-652a91ea40d5
📒 Files selected for processing (2)
src/services/aiAnalysisOptimizer.test.tssrc/services/aiAnalysisOptimizer.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/services/aiAnalysisOptimizer.ts
Address CodeRabbit review: the pipelined-abort test previously relied on a fixed 100ms delay and only asserted the README request was invoked. Await a request-start promise instead, handle an already-aborted AbortSignal, and assert the abort callback fired so the test proves the in-flight README request was actually cancelled (not silently abandoned) after abort().
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a shared rate limiter for the batch AI analysis pipeline so large runs (e.g. thousands of starred repos) respect provider 429 / per-minute limits instead of hammering the API.
Server
retry-after,retry-after-ms,x-ratelimit-*,anthropic-ratelimit-*,ratelimit-*,x-should-retry) back to the renderer on/proxy/ai,/proxy/github*,/proxy/webdavRetry-After/retry-after-msvia CORS so the rendererfetch()can read them (back-end proxy mode)Renderer / AI service
AIRateLimiter(src/services/aiRequestLimiter.ts): global cooldown + jittered exponential backoff, circuit breaker on consecutive 429s, optional per-minute sliding window and concurrency semaphore (both off by default)aiServicenow throwsAIRequestError(status, retryAfterMs)on non-OK responses (OpenAI Responses, Claude, Gemini) and exposesisRateLimitedError/getRetryAfterMsFromErrorhelpersbackendAdapterattachesretryAfterMsto translated 429 proxy errorsAIAnalysisOptimizeracquires/releases a limiter slot per request, notifies the limiter on 429 with cooldown-aware retry delay; response-time sampling now excludes limiter queuing so adaptive concurrency isn't distortedAIConfig.requestsPerMinutewired into the optimizer at both call sites (RepositoryList,DiscoveryView); default unlimited keeps existing configs unchangedTesting
AIRateLimiterunit tests (RPM window, cooldown/circuit breaker, Retry-After caps, abort)tsc, frontend 219 tests, server 68 tests, ESLint clean on touched filesNotes
server/src/mcp/provider.ts:321,src/services/electronProxy.ts:5) are unrelated and remainSummary by CodeRabbit
New Features
Bug Fixes
Tests