Skip to content

perf(review): bump max_concurrent_reviewers 3 → 10 to unblock review_dimension fan-out#19

Merged
AbirAbbas merged 1 commit into
mainfrom
perf/raise-max-concurrent-reviewers
May 6, 2026
Merged

perf(review): bump max_concurrent_reviewers 3 → 10 to unblock review_dimension fan-out#19
AbirAbbas merged 1 commit into
mainfrom
perf/raise-max-concurrent-reviewers

Conversation

@AbirAbbas
Copy link
Copy Markdown
Contributor

Why

Production data on a recent review run showed 8 review_dimension phases each taking ~25 min of LLM work but throttled by max_concurrent_reviewers = 3. Wall-clock time on the review phase was ~3× per-dimension cost — ≥75 min, with several dimensions blocked behind the semaphore doing nothing. This was the dominant reason pr-af.review runs were hitting the 7200s agent_call_timeout in github-buddy and burning the parent reasoner.

Per-dimension LLM time is what it is until the meta-selectors get refactored. But we were leaving free wall-clock time on the table by serializing dimensions that can already run in parallel.

What

  1. Default raised 3 → 10. Typical reviews have 6–8 dimensions; at 10 they all run in parallel and the phase is bounded by the slowest single dimension.
  2. New env override PR_AF_MAX_CONCURRENT_REVIEWERS so a deployment can dial it back if its provider has tighter limits.
  3. stagger_delay_seconds = 2.0 is unchanged — the 2-second stagger remains first-line defense against burst rate-limit hits, the semaphore is just no longer the bottleneck.

The original comment cited "OpenRouter or other rate-limited providers" as the reason for the cap. At 10 concurrent on Kimi K2.5 we're well within OpenRouter's per-key limits; if a deployment runs against a slower-rate provider it can lower via env without a code change.

Expected impact

Worst case observed today: review phase 75–100 min wall, total run 110+ min, hitting the 7200s caller timeout.

After this change: review phase ≈ slowest single dimension (~25–40 min), total run ~50–80 min. Should stop hitting the timeout on normal-sized PRs.

This is the highest-leverage single-line speedup. Not the last one — meta_semantic / meta_mechanical / meta_systemic are each one monolithic 14–38 min LLM call and would benefit from a separate fast-proposal + parallel-validation refactor. That's a bigger change and lands later.

Test plan

  • test_budget_config_defaults updated to pin the new default and clear PR_AF_MAX_CONCURRENT_REVIEWERS so it asserts the in-code default rather than the runtime override
  • Full pr-af test suite green (one pre-existing test_cost_tracker failure on main, unrelated to this PR)
  • After merge: trigger a real review on a multi-file PR via github-buddy and confirm review_dimension phases run all-at-once with no semaphore queuing

🤖 Generated with Claude Code

…dimension fan-out

Production data on a recent PR showed 8 review_dimensions running ~25
min each but throttled to a concurrency=3 semaphore — wall-clock time
was ~3× per-dimension cost (≥75 min for the review phase alone) even
though we were paying nothing during the wait. This was the single
biggest reason `review` runs were hitting the 7200s caller timeout in
github-buddy.

Two changes:
1. Default raised 3 → 10. With 6–8 dimensions per typical PR, all of
   them can now run in parallel and the phase is bounded by the
   slowest single dimension instead of the semaphore.
2. Surfaced the value as `PR_AF_MAX_CONCURRENT_REVIEWERS` so a
   deployment can dial it back if its provider hits rate limits. The
   stagger_delay_seconds (2s) is unchanged and remains the first line
   of defense against burst-rate-limit hits.

The previous comment on this field cited "OpenRouter or other
rate-limited providers" — at 10 concurrent on Kimi K2.5 we're well
within OpenRouter's per-key limits, and any provider that can't sustain
that for a single review session would need configuration anyway.

Tests: test_budget_config_defaults updated to pin the new default and
to clear PR_AF_MAX_CONCURRENT_REVIEWERS so the assertion reflects the
in-code default rather than the runtime env override. Full suite
green except for one pre-existing test_cost_tracker failure on main.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@AbirAbbas AbirAbbas merged commit 50f14ea into main May 6, 2026
2 checks passed
@AbirAbbas AbirAbbas deleted the perf/raise-max-concurrent-reviewers branch May 6, 2026 11:43
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.

1 participant