fix(review-pr): drafter instruction cleanup — resolve read/delegation issues#7
Merged
Merged
Conversation
docker-agent
left a comment
docker-agent
left a comment
Contributor
There was a problem hiding this comment.
Assessment: 🟢 APPROVE
The four targeted fixes are correct in intent and well-scoped. Two minor wording inconsistencies introduced by the lines are noted below — neither affects runtime safety or correctness.
jedp-docker
force-pushed
the
instruction-cleanup
branch
from
55e9d98 to
57deb79
Compare
docker-agent
left a comment
docker-agent
left a comment
Contributor
There was a problem hiding this comment.
Assessment: 🟡 NEEDS ATTENTION
One medium-severity finding in the new drafter verification rule.
… issues
- Correct "Parallel delegation" to "Batch delegation": transfer_task is
sequential; the orchestrator batches all calls in one tool-call response
and awaits each result in turn
- Fix read-per-finding vs per-file conflict: "mandatory verification for
high-severity" previously implied one read per finding; now explicit
that it means one read per file regardless of finding count
- Add empty/non-JSON fallback in step 5; surface the drafter's summary
field in the incomplete-review comment so operators can distinguish a
crash/parse-error ("Drafter did not complete") from context exhaustion
- Retain drafter schema description in delegation instructions — it is
for the orchestrator's benefit (describes what format to expect back);
keep the existing title/body warning which addresses the actual retry-
loop trigger (wrong field names, not field name descriptions in general)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
jedp-docker
force-pushed
the
instruction-cleanup
branch
from
57deb79 to
e63cbaa
Compare
derekmisler
approved these changes
Jun 23, 2026
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
3 participants
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
Part of a broader effort to fix
review-pragent timeouts.Three targeted fixes to
review-pr/agents/pr-review.yaml:transfer_taskis sequential — the orchestrator batches all calls in one tool-call response and awaits each result in turn. The prior instructions claimed parallel execution, which is wrong.read_fileis mandatory for every high-severity finding; "File Reading Guardrails" rule 5 said each source file at most once. With multiple high-severity findings in the same file, these contradicted each other and could drive repeated read attempts. Now explicit: one read per file, not one read per finding.{"findings": [], "summary": "Drafter did not complete", "review_complete": false}rather than crashing. The drafter'ssummaryis surfaced in the posted comment so operators can distinguish a crash/parse-error from genuine context exhaustion.Test plan
review-pr/agents/evals/) — no regressions expected🤖 Generated with Claude Code