feat(review): CPU-aware default --jobs + --rate-limit-per-minute#89
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.
clawsweeper
Bot
added
the
rating: 🌊 off-meta tidepool
|
Codex review: needs real behavior proof before merge. Workflow note: Future ClawSweeper reviews update this same comment in place. How this review workflow works
Summary Reproducibility: unclear. The review failed before ClawSweeper could establish a reproduction path. PR rating What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. Real behavior proof Risk before merge
Maintainer options:
Next step before merge Review detailsBest possible solution: Retry the Codex review after fixing the execution failure. Do we have a high-confidence way to reproduce the issue? Unclear. The review failed before ClawSweeper could establish a reproduction path. Is this the best way to solve the issue? Unclear. Retry the review first so ClawSweeper can evaluate the actual issue and fix direction. Label changes:
Label justifications:
What I checked:
Likely related people:
Codex review notes: model gpt-5.5, reasoning high; reviewed against b93473fe446a. |
|
ClawSweeper PR egg 🥚 Incubating: this PR egg is tucked into the review nest. Hatch commandComment Hatchability rules:
What is this egg doing here?
|
# Conflicts: # CHANGELOG.md # src/app.ts # src/cli.ts
2 participants
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.