Skip to content

fix(pr-review): parallel drafter delegation, generated-file chunk skipping, drafter file-read hard cap#189

Merged
derekmisler merged 1 commit into
docker:mainfrom
docker-agent:fix/pr-review-parallel-drafters
May 7, 2026
Merged

fix(pr-review): parallel drafter delegation, generated-file chunk skipping, drafter file-read hard cap#189
derekmisler merged 1 commit into
docker:mainfrom
docker-agent:fix/pr-review-parallel-drafters

Conversation

@docker-agent
Copy link
Copy Markdown
Contributor

Summary

Three coordinated improvements to the PR review pipeline to address the timeout root causes identified in run #25480617546:

RC1 — Generated protobuf files dominated a chunk and caused the drafter to burn 426 read_file calls.
RC2 — Chunks are processed sequentially; 3 chunks × ~10 min = timeout.


Option A — Parallel drafter delegation (best-effort)

File: review-pr/agents/pr-review.yaml (root agent, step 4c)

Instruct the root agent to issue all transfer_task delegations in a single tool-call batch instead of one at a time. When the docker-agent framework supports parallel tool execution this reduces total wall-clock time from O(N chunks × per-chunk time) to O(max per-chunk time). If the framework executes them sequentially, behaviour is unchanged — no regression risk.


Option B — Drafter file-reading hard limits

File: review-pr/agents/pr-review.yaml (drafter agent guardrails)

The drafter's previous Cap at 10 source file reads was clearly not being respected (426 calls in one run). Replaced with explicit, numbered, mandatory circuit breakers:

Trigger Action
15 source files read Hard stop → return review_complete: false
10 reads + zero high-severity bugs found Early exit → return review_complete: true
3 consecutive not-found reads Full stop immediately

Added explicit note: these limits override domain-guide instructions to be "systematic" or "thorough" — which was the loophole the model was exploiting.


Option C — Skip excluded / auto-generated chunks

Files: review-pr/action.yml, .github/workflows/review-pr.yml, review-pr/agents/pr-review.yaml, review-pr/README.md

New exclude-paths input

# .github/workflows/pr-review.yml
jobs:
  review:
    uses: docker/cagent-action/.github/workflows/review-pr.yml@VERSION
    with:
      exclude-paths: |
        backend/gen/
        frontend/src/gen/

Chunks where every file starts with an excluded prefix are skipped entirely — no drafter delegation, no token spend.

Chunk manifest in agent context

The pre-split chunk manifest (JSON: chunk number → file list) and the exclude-paths value are now appended to the review prompt so the root agent has full visibility before delegating.

Root agent chunk-skip logic (step 4b)

Before each delegation the root agent now checks:

  1. Exclude-paths skip — all files in chunk match a listed prefix → skip
  2. Generated-file skip — all files end with .pb.go / .pb.ts / _grpc.go / _grpc.ts / .gen.go / .gen.ts / _gen.go AND the chunk contains a Code generated or DO NOT EDIT marker → skip

This directly prevents the session_service.pb.go / sandbox_session.pb.go scenario from the original analysis.


Testing

  • pnpm build && pnpm lint (biome CI + tsc + actionlint) — ✅ all pass
  • No TypeScript source files changed; dist/ is unchanged
  • Shell logic in review-pr/action.yml only adds new if blocks with no side-effects when exclude-paths is empty

@docker-agent docker-agent requested a review from a team as a code owner May 7, 2026 14:16
@docker-agent docker-agent requested a review from derekmisler May 7, 2026 14:16
Three improvements to reduce review timeout risk:

## Option A — Parallel drafter delegation (best-effort)
Instruct the root agent to issue ALL transfer_task delegations in a single
tool-call batch rather than sequentially. If the docker-agent framework
supports parallel tool execution this cuts wall-clock time from
O(N × per-chunk-time) to O(max per-chunk-time).

## Option B — Drafter file-reading hard limits
The drafter made 426 read_file calls in a single run (root cause of the
timeout in run #25480617546). Replace the soft advisory cap with mandatory
circuit breakers:
- Hard cap: 15 source files (not counting the diff). Stop and return
  review_complete: false when reached.
- Early-exit: if 10+ files read and zero high-severity bugs found, stop and
  return review_complete: true.
- Explicit: these limits override domain-guide 'be systematic' instructions.

## Option C — Skip excluded / auto-generated chunks
- New exclude-paths input on review-pr/action.yml and the reusable workflow.
  Newline-separated path prefixes; chunks where every file matches are skipped.
- Chunk manifest (JSON) and exclude-paths are now appended to the agent prompt
  context so the root agent can evaluate them before delegating.
- Root agent step 4 updated: for each chunk, check (a) exclude-paths prefixes
  and (b) generated-file path patterns (.pb.go, _grpc.ts, .gen.go, etc.) +
  'Code generated'/'DO NOT EDIT' marker. Skip matching chunks entirely.

Fixes RC2 (sequential chunks) directly. Fixes RC1 (generated-file over-
reading) via both Options B and C. Option C also lays groundwork for the
Fix A exclude-paths filtering at the bash/diff level.
@docker-agent docker-agent force-pushed the fix/pr-review-parallel-drafters branch from 8b2f6f3 to bd5ec11 Compare May 7, 2026 15:07
@derekmisler derekmisler merged commit b6b8713 into docker:main May 7, 2026
8 checks passed
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.

2 participants