perf(ai): parallel async branch and pr review worker pool with semaphore concurrency (#120) - #226
Merged
Conversation
…ore concurrency (#120)
There was a problem hiding this comment.
🟡 Changes recommended
Critical and moderate findings remain unresolved, including unsafe process guidance, dropped worker failures, unsanitized provider errors, and concurrency/timeout enforcement gaps.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds bounded asynchronous worker pools, rate limiting, and error isolation to AI review workflows.
Changes:
- Parallelizes review, validation, pre-analysis, and persona stages.
- Adds configurable concurrency and rate-limit defaults.
- Expands tests and updates documentation, task tracking, and changelog entries.
File summaries
| File | Summary |
|---|---|
tests/test_review_runner.py |
Tests runner worker behavior and isolated failures. |
tests/test_review_pipeline.py |
Tests pipeline worker-pool integration. |
tests/test_ai_review_pool.py |
Tests pool defaults, clamping, and exception handling. |
src/devops_cli/config/defaults.py |
Defines review concurrency and rate-limit defaults. |
src/devops_cli/config/__init__.py |
Exports the new defaults. |
src/devops_cli/ai/review/runner.py |
Uses worker pools across review stages. |
src/devops_cli/ai/review/pool.py |
Adds the configured worker-pool factory. |
src/devops_cli/ai/review/pipeline.py |
Integrates pools into analysis, review, and verification. |
docs/SELF_IMPROVEMENT.md |
Documents review safeguards. |
docs/ROADMAP.md |
Adds review-system roadmap items. |
docs/agent/tasks/task-120-parallel-async-review-pool.md |
Tracks task objectives and deliverables. |
CHANGELOG.md |
Records performance improvements. |
AGENTS.md |
Adds engineering guidance. |
Review details
Suppressed comments (3)
src/devops_cli/ai/review/pool.py:100
- Each
create()call gets an independent bucket, while production paths create nested pools (for example, a persona pool and per-persona segment pools). Every bucket starts with a fresh burst, so aggregate LLM requests can exceed the advertised 10 requests/sec and still trigger provider throttling or memory pressure. Share one limiter at the provider/request boundary or pass it through child pools.
limiter = TokenBucketRateLimiter(rate=effective_rate, capacity=effective_capacity)
src/devops_cli/ai/review/runner.py:1108
- This outer persona pool calls
_run_reviewfrom each worker, and_run_reviewcreates another pool for its segments. With four personas and four segments, this can run up to 16 LLM tasks and four independent token buckets, so the advertised global concurrency/rate bound is not enforced and can still trigger the VRAM/429 failures this change is intended to prevent. Share one semaphore/rate limiter across both dimensions, or parallelize only one dimension.
pool = ReviewWorkerPool.create(concurrency=workers)
persona_results = pool.run_sync_all(_execute_persona, personas, return_exceptions=True)
src/devops_cli/ai/review/runner.py:758
- All production call sites construct the pool with only
concurrency, leavingtimeout_per_task=None; the timeout branch therefore never runs during real branch/PR reviews. A blocked synchronous worker can still hold the review indefinitely, despite the PR description promising task timeouts. Wire the configured review timeout (or another explicit bound) into each integration.
pool = ReviewWorkerPool.create(concurrency=workers)
- Files reviewed: 13/13 changed files
- Comments generated: 11
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
dan-petty
added a commit
that referenced
this pull request
Sep 18, 2026
* feat(release): v0.2.19 * fix(release): draft PR description generator and milestone deliverable resolution (#218) (#219) * feat(release): fix draft PR description generator and milestone deliverable resolution * docs(agent): track task 218 for release draft pr description generator fix * fix(release): prevent duplicate text by formatting milestone issues as clean autolinks * fix(release): isolate milestone cache, dynamic draft checklist, and mock run_gh in tests * fix(ai): support main branch diffing and base resolution in review branch (#220) (#221) * fix(ai): support main branch diffing and base resolution in review branch (#220) * fix(review): propagate target_branch to target_ref and update task 220 status to In Review (#221) * fix(github): treat copilot review as completed when zero unresolved threads remain (#216) (#222) * fix(github): treat copilot review as completed when zero unresolved threads remain (#216) * docs(agent): update task 216 with PR #222 and status In Review * test(github): test monitor_pr across get_pr_monitoring_status boundary with mock threads (#222) * feat(ai): adaptive embedding batch sizing circuit breaker and timeout fallback (#117) (#223) * feat(ai): adaptive embedding batch sizing circuit breaker and timeout fallback (#117) * docs(agent): link PR #223 and transition task 117 to In Review * fix(ai): resolve review feedback on fallback cache poisoning and query key isolation (#117) * perf(ai): high-performance AST context packer with binary search truncation (#118) (#224) * perf(ai): high-performance AST context packer with binary search truncation (#118) * docs(agent): link PR #224 and transition task 118 to In Review * fix(ai): resolve review feedback on full-body binary search, empty AST isolation, and tokenizer prewarming (#118) * fix(ai): accurately evaluate truncation boolean and expand test coverage (#118) * fix(ai): polyglot tree-sitter file size boundary guard and resource containment (#119) (#225) * fix(ai): polyglot tree-sitter file size boundary guard and resource containment (#119) * fix(ai): harden repomap symlink resolution and syntax tuple * test(ci): isolate ci workspace root to tmp_path to prevent unlinking active coverage * perf(ai): parallel async branch and pr review worker pool with semaphore concurrency (#120) (#226) * perf(ai): parallel async branch and pr review worker pool with semaphore concurrency (#120) * docs(agent): link PR #226 to task-120 tracking * fix(ai): address Copilot review feedback on error isolation and sanitization * fix(security): sanitize CodeQL clear-text logging by recording exception type names * feat(ai): llm structured output retry and json schema repair engine (#123) (#230) * feat(ai): llm structured output retry and json schema repair engine (#123) * fix(ci): remove redundant pr merge readiness gate and remediate review findings (#123) (#230) * chore(ci): deduplicate pre-commit code checks and scope hooks to pre-commit stage * chore(ci): real-time quality gate streaming, re-entrant rate limit locking, and repos clone destination (#231) (#232) * chore(ci): real-time quality gate streaming, re-entrant rate limit locking, and project quota safety * feat(repos): parse organization destination from clone url instead of hardcoding standalone * docs(agent): add task-231 tracking for realtime CI, rate limiter locking, and repos clone * docs(agent): link PR #232 to task-231 tracking * fix(gh): isolate disk quota lock yield and add fallback pacing for unknown quotas * docs(agent): document task-231 CI failure remediation and merge readiness * fix(github): resolve paginated url query replacement loop and optimize dry-run reconciliation (#233) (#234) * docs(agent): record PR #232 merge and close task 231 * fix(github): resolve paginated url replacement loop, optimize dry-run project reconciliation, and add pr edit milestone (#233) * docs(agent): update task 233 status to in-review and link PR #234 * fix(github): resolve PR review comments for #234 with milestone resolution, off-board dry-run test, and tuple except syntax * fix: config * fix(github): eliminate bare gh invocations, harden rate limiter quota tracking, and optimize project reconciliation (#235) (#236) * fix(github): eliminate bare gh invocations, harden rate limiter quota tracking, and optimize project reconciliation (#235) * fix(github): replace hardcoded gh CLI strings with CONST_GH_CLI constant (#235) * fix(github): resolve review feedback on mutation budgeting, rate limiter caching, and milestone editing (#235) - Pass shared MutationBudget across issue sync, candidate provisioning, and custom field edits to enforce a strict aggregate cap of 25 mutations - Reject all HTTP mutation methods (POST, PUT, PATCH, DELETE) and field arguments in _is_cacheable_api_call - Retain --milestone option on devops pr edit with REST fallback - Raise GitHubRateLimitError instead of bare TypeError on non-dictionary data in QuotaState.from_dict - Enforce max_backoff in calculate_backoff_delay and quota_max_age in QuotaState.is_valid - Account effective_cost during limiter.acquire() and eliminate redundant second decrement in _post_process_run - Update Task 235 tracking to PR #236 and Status: In Review * fix(github): eliminate delay-bypassing caps and mock external calls in tests (#235) - Remove DEFAULT_GH_MAX_PACING_DELAY_SECONDS and DEFAULT_GH_MAX_BACKOFF_SECONDS to ensure mandatory pauses are never bypassed or truncated - Ensure _acquire_locked pauses for full calculated request delay and calculate_backoff_delay pauses for full duration until reset - Properly mock _resolve_project_owner_arg in project tests to eliminate unmocked rate limit network calls * fix(github): eliminate bare gh invocations, error masking, and inaccurate quota defaults (#237) (#238) * fix(github): eliminate bare gh invocations, error masking, and inaccurate quota defaults (#237) * docs(agent): link PR #238 in task-237 tracking * fix(github): resolve review feedback on error classification, syntax, and sync resiliency (#237) * feat(k8s): minikube gpu detection and dynamic service nodeport reachability fallback (#126) (#239) * feat(k8s): minikube gpu detection and dynamic service nodeport reachability fallback (#126) * docs(agent): update task-126 status to in review with pr #239 * fix(k8s): resolve PR #239 review feedback on valkey normalization, scheme fallback, and sanitization * feat(devcontainer): build from debian:sid with python3.14, uv 0.12.16, and CI caching optimizations * fix(devcontainer): normalize VERSION_CODENAME to trixie for devcontainer feature compatibility * feat(ai): proactive model prewarming and vram eviction governance (#127) (#240) * feat(ai): streaming reasoning think token parser and bounded stream sanitizer (#128) (#241) * feat(ai): streaming reasoning think token parser and bounded stream sanitizer (#128) * docs(task): update task-128 PR link to #241 * fix(ai): resolve review feedback on streaming reasoning and error bounds * feat(ai): high-throughput LLM gateway and distributed model router (#142) (#242) * feat(ai): high-throughput LLM gateway and distributed model router (#142) * docs(agent): update task-142 tracking with PR #242 * fix(ai): remediate review comments on llm gateway, vllm manifests, and router (#142) * fix: address PR #217 comments, remediate uv diagnostics, and add uv-check/lockfile quality gates (#243) * fix: address PR #217 comments, remediate uv diagnostics, upgrade dependencies, and add uv-check/lockfile quality gates * fix(devcontainer): support sys_bin path parameter and add edge-case coverage for shadowed binary reconciliation * fix: remove reconcile_shadowed_user_binaries and debian_sid test, honor table payload options, integrate outdated check into CI
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
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
Resolves #120.
Implements an asynchronous review worker pool with bounded semaphore concurrency, token-bucket rate limiting, and per-task error isolation.
Key Changes:
ReviewWorkerPool.create()with bounded concurrency (1 <= C <= 8), token-bucket rate limiting (DEFAULT_REVIEW_RATE_LIMIT = 10.0), and task timeouts.ReviewPipelineOrchestratorandrunner.pyto use managed asynchronous worker pools.ThreadPoolExecutorand sequential single-worker fallbacks with concurrent execution, accelerating multi-file and multi-persona review cycles while preventing VRAM spikes, Ollama out-of-memory errors, and rate-limit throttling.return_exceptions=Trueacross parallel review stages, guaranteeing that an isolated LLM inference error or malformed payload does not fail remaining segment reviews.tests/test_ai_review_pool.py,tests/test_review_runner.py, andtests/test_review_pipeline.py.