feat(review): CPU-aware default --jobs + --rate-limit-per-minute#89
Open
coletebou wants to merge 1 commit into
Open
feat(review): CPU-aware default --jobs + --rate-limit-per-minute#89coletebou wants to merge 1 commit into
coletebou wants to merge 1 commit into
Conversation
Replaces the hardcoded `--jobs 10` default with `floor(cpuCores / 2)` clamped to `[1, 10]`, so a 4-core box stops fanning out 10 parallel Bedrock streams it cannot sustain. Explicit `--jobs <n>` is honored unchanged (still capped at 32). Adds `--rate-limit-per-minute <n>` (and `CLAWPATCH_RPM` env var) which caps provider invocation starts within any rolling 60s window across all jobs. Default unset preserves prior behavior. Implementation is a serialized rolling-timestamp queue shared by all workers; the (N+1)th acquire sleeps until the oldest slot expires. Tests cover the CPU-aware default, explicit override, RPM no-op when unset, and the rolling-window delay path under a fake clock.
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
Two ergonomics improvements for
clawpatch review:CPU-aware default
--jobs: today defaults to a hard10, which is hot for laptops and saturates Bedrock RPM quotas. New default:min(max(floor(cpus/2), 1), 10). Explicit--jobs <n>is honored unchanged (32-cap preserved).New
--rate-limit-per-minute <n>(envCLAWPATCH_RPM): rolling-60s-window cap on provider invocations across all jobs. Off by default — fully backwards-compatible. The real bottleneck on most hosted LLMs is RPM, not local CPU.Files
src/rpm-limiter.ts(new) —createRpmLimiter(serialized rolling-timestamp queue with injectable clock),rpmFromFlag(flag > env),defaultJobs(cores)src/app.ts—reviewJobs(flags, coreCount?)exported, CPU-aware default; worker loop callsawait limiter.acquire()before eachreviewFeaturesrc/cli.ts— new flag registered incommandFlags.reviewandvalueFlagNamessrc/rpm-limiter.test.ts(new),src/review-jobs.test.ts(new) — 14 cases including a fake-clock delay pathValidation
pnpm format:check— cleanpnpm typecheck— cleanpnpm lint— cleanpnpm build— cleanpnpm test— 555 passed, 1 skippedNotes
Default behavior changes (lower
--jobson small machines), so this is a minor-bump candidate. Operators on 16+ core CI runners are unaffected.