Skip to content

Fix PR review sub-agent count and diff-line verification#540

Merged
strawgate merged 3 commits intomainfrom
fix/pr-review-prompt-tuning
Mar 2, 2026
Merged

Fix PR review sub-agent count and diff-line verification#540
strawgate merged 3 commits intomainfrom
fix/pr-review-prompt-tuning

Conversation

@strawgate
Copy link
Collaborator

@strawgate strawgate commented Mar 2, 2026

Summary

Fixes three issues observed while analyzing run 22582465274:

  • Sub-agent over-provisioning: subagent-*.md files were always written, so medium PR reviews could still spawn all 3 sub-agents. Sub-agent instruction files are now written only for the active tier (small=none, medium=2, large=3).
  • Missing diff-line verification: Inline comments could target non-commentable lines and fail with GitHub 422 (Line could not be resolved). The review flow now requires validating targets against numbered per-file diffs before posting.
  • No fallback for non-commentable lines: If a finding targets a line without a diff line number (including deleted lines), the reviewer now puts that finding in the review body instead of posting an inline comment.
  • Comment-side guidance aligned with diff numbering: Inline comments are now documented as RIGHT-side only because deleted lines are unnumbered and not commentable.

Changes

File Change
.github/workflows/gh-aw-fragments/safe-output-code-review.md Scope sub-agent file generation by PR size tier and require numbered diff-line verification before inline comments
.github/workflows/gh-aw-fragments/pr-context.md Generate numbered per-file diffs (diffs/(path).diff) and document which lines are commentable
.github/workflows/gh-aw-pr-review.md Add explicit verification step: only numbered diff lines are eligible for inline comments; otherwise use review body
.github/workflows/gh-aw-fragments/review-process.md Clarify numbered vs unnumbered diff lines during per-file review
.github/workflows/gh-aw-fragments/safe-output-review-comment.md Update review-comment contract: line must come from numbered diff lines and side is RIGHT
Makefile, .github/aw/actions-lock.json Re-pin GH_AW_VERSION/lock metadata to v0.51.5 so generated workflow locks stay consistent
lock workflow outputs Regenerate affected .lock.yml workflows so compiled outputs match fragment and prompt updates

Test plan

  • make compile succeeds
  • CI passes
  • Verify on next medium PR review that only 2 sub-agents spawn
  • Verify finding on a non-commentable line is routed to the review body

Generated by Update PR Body for issue #540

Fixes three issues observed in run 22582465274:

1. Only write subagent-*.md files for the tiers that use them.
   Previously all 3 were written regardless of PR size, causing
   the agent to spawn 3 sub-agents for medium PRs (should be 2).

2. Add explicit diff-line verification to Step 3 — agent must
   check the per-file diff before posting inline comments. Lines
   outside the diff will fail with GitHub API 422.

3. Add fallback for findings on non-diff lines — include them in
   the review body instead of losing the entire review.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@coderabbitai
Copy link

coderabbitai bot commented Mar 2, 2026

📝 Walkthrough

Walkthrough

This change adds a write_subagent(key, desc) helper to generate sub-agent instruction files at /tmp/pr-context/subagent-<key>.md, replacing inline per-subagent file construction. PR-size branching now spawns az and za for medium PRs and az, za, largest for large PRs; small PRs remain unchanged. Per-file diffs are transformed into a line-numbered format where only lines with a number prefix are commentable and deletions remain unnumbered. The workflow includes a pre-comment verification step that ensures a target line exists in the numbered diff before posting inline comments. Manifest/README references were updated to use subagent-*.md.

Possibly related PRs

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/pr-review-prompt-tuning

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot]

This comment was marked as resolved.

Per-file diffs now include line numbers on commentable lines:
  405	+ added code
  406	  context line
    	- deleted line (no number — LEFT side only)

This makes it trivially verifiable whether a line is commentable
before the agent attempts to post a review comment. All diff
references in review-instructions, parent-review, safe-output docs,
and the main prompt now point to the numbered format.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
coderabbitai[bot]

This comment was marked as resolved.

…entable

With numbered diffs, deleted lines have no number prefix and can't be
targeted for inline comments. Clarify that Side is always RIGHT and
deleted-code findings should go in the review body instead.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
.github/workflows/gh-aw-pr-review.lock.yml (1)

410-417: ⚠️ Potential issue | 🟠 Major

Don’t noop when there are new review-body findings.

Line 410 routes non-commentable findings to the review body, but Step 4 can still noop when inline comment count is zero and verdict matches prior review. That can suppress new deleted-line/non-diff findings entirely.

Suggested prompt adjustment
- **Skip if nothing new:** If you left zero inline comments during this review AND your verdict would be the same as the most recent review from this bot (compare against reviews in Step 1), call `noop` with a message like "No new findings — prior review still applies" and stop. Do not submit a redundant review.
+ **Skip if nothing new:** If you left zero inline comments, have zero review-body findings (including findings on non-commentable/deleted lines), AND your verdict would be the same as the most recent review from this bot (compare against reviews in Step 1), call `noop` with a message like "No new findings — prior review still applies" and stop. Do not submit a redundant review.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/gh-aw-pr-review.lock.yml around lines 410 - 417, Step 4's
noop logic currently only checks for zero inline comments and matching prior
verdict, which allows new review-body findings (e.g., deleted-line or non-diff
issues routed to the review body per the "Before posting each comment" step) to
be suppressed; update the noop condition in the "Step 4: Submit the Review"
logic so it only calls noop when inlineCommentCount == 0 AND
reviewBodyFindingsCount == 0 AND verdict matches prior review, otherwise proceed
to submit a review that includes the review body; locate the "Step 4: Submit the
Review" block and the noop call to implement this additional check (introduce a
reviewBodyFindings counter or boolean as needed).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.github/workflows/gh-aw-mention-in-pr-no-sandbox.lock.yml:
- Line 706: The current fallback leaves /tmp/pr-context/pr.diff empty on gh pr
diff failure so DIFF_LINES=$(wc -l < /tmp/pr-context/pr.diff ...) becomes 0 and
may misclassify PR size; update the failure branch (the gh pr diff ->
/tmp/pr-context/pr.diff block and the DIFF_LINES calculation) to detect an empty
pr.diff and instead compute DIFF_LINES from files.json by summing (.additions //
0) + (.deletions // 0) (use jq on /tmp/pr-context/files.json) or write a
non-zero sentinel into pr.diff when falling back; ensure the DIFF_LINES variable
logic (the DIFF_LINES=... line) uses this fallback so large PRs aren't treated
as zero-line small PRs.

---

Outside diff comments:
In @.github/workflows/gh-aw-pr-review.lock.yml:
- Around line 410-417: Step 4's noop logic currently only checks for zero inline
comments and matching prior verdict, which allows new review-body findings
(e.g., deleted-line or non-diff issues routed to the review body per the "Before
posting each comment" step) to be suppressed; update the noop condition in the
"Step 4: Submit the Review" logic so it only calls noop when inlineCommentCount
== 0 AND reviewBodyFindingsCount == 0 AND verdict matches prior review,
otherwise proceed to submit a review that includes the review body; locate the
"Step 4: Submit the Review" block and the noop call to implement this additional
check (introduce a reviewBodyFindings counter or boolean as needed).

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e78b6e6 and 5407de8.

📒 Files selected for processing (6)
  • .github/workflows/gh-aw-estc-docs-pr-review.lock.yml
  • .github/workflows/gh-aw-fragments/safe-output-review-comment.md
  • .github/workflows/gh-aw-mention-in-pr-by-id.lock.yml
  • .github/workflows/gh-aw-mention-in-pr-no-sandbox.lock.yml
  • .github/workflows/gh-aw-mention-in-pr.lock.yml
  • .github/workflows/gh-aw-pr-review.lock.yml
🚧 Files skipped from review as they are similar to previous changes (1)
  • .github/workflows/gh-aw-estc-docs-pr-review.lock.yml

PR_NUMBER: ${{ github.event.pull_request.number || inputs.target-pr-number || github.event.issue.number }}
name: Fetch PR context to disk
run: "set -euo pipefail\nmkdir -p /tmp/pr-context\n\n# PR metadata\ngh pr view \"$PR_NUMBER\" --json title,body,author,baseRefName,headRefName,headRefOid,url \\\n > /tmp/pr-context/pr.json\n\n# Full diff\nif ! gh pr diff \"$PR_NUMBER\" > /tmp/pr-context/pr.diff; then\n echo \"::warning::Failed to fetch full PR diff; per-file diffs from files.json are still available.\"\n : > /tmp/pr-context/pr.diff\nfi\n\n# Changed files list (--paginate may output concatenated arrays; jq -s 'add' merges them)\ngh api \"repos/$GITHUB_REPOSITORY/pulls/$PR_NUMBER/files\" --paginate \\\n | jq -s 'add // []' > /tmp/pr-context/files.json\n\n# Per-file diffs\njq -c '.[]' /tmp/pr-context/files.json | while IFS= read -r entry; do\n filename=$(echo \"$entry\" | jq -r '.filename')\n mkdir -p \"/tmp/pr-context/diffs/$(dirname \"$filename\")\"\n echo \"$entry\" | jq -r '.patch // empty' > \"/tmp/pr-context/diffs/${filename}.diff\"\ndone\n\n# File orderings for sub-agent review (3 strategies)\njq -r '[.[] | .filename] | sort | .[]' /tmp/pr-context/files.json \\\n > /tmp/pr-context/file_order_az.txt\njq -r '[.[] | .filename] | sort | reverse | .[]' /tmp/pr-context/files.json \\\n > /tmp/pr-context/file_order_za.txt\njq -r '[.[] | {filename, size: ((.additions // 0) + (.deletions // 0))}] | sort_by(-.size) | .[].filename' /tmp/pr-context/files.json \\\n > /tmp/pr-context/file_order_largest.txt\n\n# Compute PR size metrics for review fan-out decisions\nFILE_COUNT=$(jq 'length' /tmp/pr-context/files.json)\nDIFF_LINES=$(wc -l < /tmp/pr-context/pr.diff | tr -d ' ')\necho \"${FILE_COUNT} files, ${DIFF_LINES} diff lines\" > /tmp/pr-context/pr-size.txt\necho \"PR size: ${FILE_COUNT} files, ${DIFF_LINES} diff lines\"\n\n# Existing reviews\ngh api \"repos/$GITHUB_REPOSITORY/pulls/$PR_NUMBER/reviews\" --paginate \\\n | jq -s 'add // []' > /tmp/pr-context/reviews.json\n\n# Review threads with resolution status (GraphQL — REST lacks isResolved/isOutdated)\ngh api graphql --paginate -f query='\n query($owner: String!, $repo: String!, $number: Int!, $endCursor: String) {\n repository(owner: $owner, name: $repo) {\n pullRequest(number: $number) {\n reviewThreads(first: 100, after: $endCursor) {\n pageInfo { hasNextPage endCursor }\n nodes {\n id\n isResolved\n isOutdated\n isCollapsed\n path\n line\n startLine\n comments(first: 100) {\n nodes {\n id\n databaseId\n body\n author { login }\n createdAt\n }\n }\n }\n }\n }\n }\n }\n' -F owner=\"${GITHUB_REPOSITORY%/*}\" -F repo=\"${GITHUB_REPOSITORY#*/}\" -F \"number=$PR_NUMBER\" \\\n --jq '.data.repository.pullRequest.reviewThreads.nodes' \\\n | jq -s 'add // []' > /tmp/pr-context/review_comments.json\n\n# Filtered review thread views (pre-computed so agents don't need to parse review_comments.json)\njq '[.[] | select(.isResolved == false)]' /tmp/pr-context/review_comments.json \\\n > /tmp/pr-context/unresolved_threads.json\njq '[.[] | select(.isResolved == true)]' /tmp/pr-context/review_comments.json \\\n > /tmp/pr-context/resolved_threads.json\njq '[.[] | select(.isOutdated == true)]' /tmp/pr-context/review_comments.json \\\n > /tmp/pr-context/outdated_threads.json\n\n# Per-file review threads (mirrors diffs/ structure)\njq -c '.[]' /tmp/pr-context/review_comments.json | while IFS= read -r thread; do\n filepath=$(echo \"$thread\" | jq -r '.path // empty')\n [ -z \"$filepath\" ] && continue\n mkdir -p \"/tmp/pr-context/threads/$(dirname \"$filepath\")\"\n echo \"$thread\" >> \"/tmp/pr-context/threads/${filepath}.jsonl\"\ndone\n# Convert per-file JSONL to proper JSON arrays\nmkdir -p /tmp/pr-context/threads\nfind /tmp/pr-context/threads -name '*.jsonl' 2>/dev/null | while IFS= read -r jsonl; do\n jq -s '.' \"$jsonl\" > \"${jsonl%.jsonl}.json\"\n rm \"$jsonl\"\ndone\n\n# PR discussion comments\ngh api \"repos/$GITHUB_REPOSITORY/issues/$PR_NUMBER/comments\" --paginate \\\n | jq -s 'add // []' > /tmp/pr-context/comments.json\n\n# Linked issues\njq -r '.body // \"\"' /tmp/pr-context/pr.json 2>/dev/null \\\n | grep -oiE '(fixes|closes|resolves)\\s+#[0-9]+' \\\n | grep -oE '[0-9]+$' \\\n | sort -u \\\n | while read -r issue; do\n gh api \"repos/$GITHUB_REPOSITORY/issues/$issue\" > \"/tmp/pr-context/issue-${issue}.json\" || true\n done || true\n\n# Write manifest\ncat > /tmp/pr-context/README.md << 'MANIFEST'\n# PR Context\n\nPre-fetched PR data. All files are in `/tmp/pr-context/`.\n\n| File | Description |\n| --- | --- |\n| `pr.json` | PR metadata — title, body, author, base/head branches, head commit SHA (`headRefOid`), URL |\n| `pr.diff` | Full unified diff of all changes |\n| `files.json` | Changed files array — each entry has `filename`, `status`, `additions`, `deletions`, `patch` |\n| `diffs/<path>.diff` | Per-file diffs — one file per changed file, mirroring the repo path under `diffs/` |\n| `file_order_az.txt` | Changed files sorted alphabetically (A→Z), one filename per line |\n| `file_order_za.txt` | Changed files sorted reverse-alphabetically (Z→A), one filename per line |\n| `file_order_largest.txt` | Changed files sorted by diff size descending (largest first), one filename per line |\n| `reviews.json` | Prior review submissions — author, state (APPROVED/CHANGES_REQUESTED/COMMENTED), body |\n| `review_comments.json` | All review threads (GraphQL) — each thread has `id` (node ID for resolving), `isResolved`, `isOutdated`, `path`, `line`, and nested `comments` with `id`, `databaseId` (numeric REST ID for replies), body/author |\n| `unresolved_threads.json` | Unresolved review threads — subset of `review_comments.json` where `isResolved` is false |\n| `resolved_threads.json` | Resolved review threads — subset of `review_comments.json` where `isResolved` is true |\n| `outdated_threads.json` | Outdated review threads — subset of `review_comments.json` where `isOutdated` is true (code changed since comment) |\n| `threads/<path>.json` | Per-file review threads — one file per changed file with existing threads, mirroring the repo path under `threads/` |\n| `comments.json` | PR discussion comments (not inline) |\n| `issue-{N}.json` | Linked issue details (one file per linked issue, if any) |\n| `pr-size.txt` | PR size metrics: `{N} files, {M} diff lines` — used by the agent to decide review fan-out |\n| `agents.md` | Not used in PR context — repository conventions are at `/tmp/agents.md` (pre-fetched) |\n| `review-instructions.md` | Review instructions, criteria, and calibration examples for sub-agents |\n| `agent-review.md` | Main agent instructions — review approach resolved from PR size (written when `ready_to_code_review` is called) |\n| `parent-review.md` | Comment format and inline severity threshold for the parent agent (written when `ready_to_code_review` is called) |\n| `subagent-az.md` | Sub-agent instructions: review files A → Z (written when `ready_to_code_review` is called) |\n| `subagent-za.md` | Sub-agent instructions: review files Z → A (written when `ready_to_code_review` is called) |\n| `subagent-largest.md` | Sub-agent instructions: review files largest diff first (written when `ready_to_code_review` is called) |\nMANIFEST\n\necho \"PR context written to /tmp/pr-context/\"\nls -la /tmp/pr-context/"
run: "set -euo pipefail\nmkdir -p /tmp/pr-context\n\n# PR metadata\ngh pr view \"$PR_NUMBER\" --json title,body,author,baseRefName,headRefName,headRefOid,url \\\n > /tmp/pr-context/pr.json\n\n# Full diff\nif ! gh pr diff \"$PR_NUMBER\" > /tmp/pr-context/pr.diff; then\n echo \"::warning::Failed to fetch full PR diff; per-file diffs from files.json are still available.\"\n : > /tmp/pr-context/pr.diff\nfi\n\n# Changed files list (--paginate may output concatenated arrays; jq -s 'add' merges them)\ngh api \"repos/$GITHUB_REPOSITORY/pulls/$PR_NUMBER/files\" --paginate \\\n | jq -s 'add // []' > /tmp/pr-context/files.json\n\n# Per-file diffs with line numbers\n# Lines with a number prefix are commentable (RIGHT side: added or context).\n# Deleted lines get no number (LEFT side only).\njq -c '.[]' /tmp/pr-context/files.json | while IFS= read -r entry; do\n filename=$(echo \"$entry\" | jq -r '.filename')\n mkdir -p \"/tmp/pr-context/diffs/$(dirname \"$filename\")\"\n echo \"$entry\" | jq -r '.patch // empty' | awk '\n /^@@/ { s=$3; gsub(/\\+/,\"\",s); split(s,a,\",\"); line=a[1]+0; print; next }\n /^\\+/ { printf \"%d\\t%s\\n\", line++, $0; next }\n /^-/ { printf \" \\t%s\\n\", $0; next }\n /^ / { printf \"%d\\t%s\\n\", line++, $0; next }\n { print }\n ' > \"/tmp/pr-context/diffs/${filename}.diff\"\ndone\n\n# File orderings for sub-agent review (3 strategies)\njq -r '[.[] | .filename] | sort | .[]' /tmp/pr-context/files.json \\\n > /tmp/pr-context/file_order_az.txt\njq -r '[.[] | .filename] | sort | reverse | .[]' /tmp/pr-context/files.json \\\n > /tmp/pr-context/file_order_za.txt\njq -r '[.[] | {filename, size: ((.additions // 0) + (.deletions // 0))}] | sort_by(-.size) | .[].filename' /tmp/pr-context/files.json \\\n > /tmp/pr-context/file_order_largest.txt\n\n# Compute PR size metrics for review fan-out decisions\nFILE_COUNT=$(jq 'length' /tmp/pr-context/files.json)\nDIFF_LINES=$(wc -l < /tmp/pr-context/pr.diff | tr -d ' ')\necho \"${FILE_COUNT} files, ${DIFF_LINES} diff lines\" > /tmp/pr-context/pr-size.txt\necho \"PR size: ${FILE_COUNT} files, ${DIFF_LINES} diff lines\"\n\n# Existing reviews\ngh api \"repos/$GITHUB_REPOSITORY/pulls/$PR_NUMBER/reviews\" --paginate \\\n | jq -s 'add // []' > /tmp/pr-context/reviews.json\n\n# Review threads with resolution status (GraphQL — REST lacks isResolved/isOutdated)\ngh api graphql --paginate -f query='\n query($owner: String!, $repo: String!, $number: Int!, $endCursor: String) {\n repository(owner: $owner, name: $repo) {\n pullRequest(number: $number) {\n reviewThreads(first: 100, after: $endCursor) {\n pageInfo { hasNextPage endCursor }\n nodes {\n id\n isResolved\n isOutdated\n isCollapsed\n path\n line\n startLine\n comments(first: 100) {\n nodes {\n id\n databaseId\n body\n author { login }\n createdAt\n }\n }\n }\n }\n }\n }\n }\n' -F owner=\"${GITHUB_REPOSITORY%/*}\" -F repo=\"${GITHUB_REPOSITORY#*/}\" -F \"number=$PR_NUMBER\" \\\n --jq '.data.repository.pullRequest.reviewThreads.nodes' \\\n | jq -s 'add // []' > /tmp/pr-context/review_comments.json\n\n# Filtered review thread views (pre-computed so agents don't need to parse review_comments.json)\njq '[.[] | select(.isResolved == false)]' /tmp/pr-context/review_comments.json \\\n > /tmp/pr-context/unresolved_threads.json\njq '[.[] | select(.isResolved == true)]' /tmp/pr-context/review_comments.json \\\n > /tmp/pr-context/resolved_threads.json\njq '[.[] | select(.isOutdated == true)]' /tmp/pr-context/review_comments.json \\\n > /tmp/pr-context/outdated_threads.json\n\n# Per-file review threads (mirrors diffs/ structure)\njq -c '.[]' /tmp/pr-context/review_comments.json | while IFS= read -r thread; do\n filepath=$(echo \"$thread\" | jq -r '.path // empty')\n [ -z \"$filepath\" ] && continue\n mkdir -p \"/tmp/pr-context/threads/$(dirname \"$filepath\")\"\n echo \"$thread\" >> \"/tmp/pr-context/threads/${filepath}.jsonl\"\ndone\n# Convert per-file JSONL to proper JSON arrays\nmkdir -p /tmp/pr-context/threads\nfind /tmp/pr-context/threads -name '*.jsonl' 2>/dev/null | while IFS= read -r jsonl; do\n jq -s '.' \"$jsonl\" > \"${jsonl%.jsonl}.json\"\n rm \"$jsonl\"\ndone\n\n# PR discussion comments\ngh api \"repos/$GITHUB_REPOSITORY/issues/$PR_NUMBER/comments\" --paginate \\\n | jq -s 'add // []' > /tmp/pr-context/comments.json\n\n# Linked issues\njq -r '.body // \"\"' /tmp/pr-context/pr.json 2>/dev/null \\\n | grep -oiE '(fixes|closes|resolves)\\s+#[0-9]+' \\\n | grep -oE '[0-9]+$' \\\n | sort -u \\\n | while read -r issue; do\n gh api \"repos/$GITHUB_REPOSITORY/issues/$issue\" > \"/tmp/pr-context/issue-${issue}.json\" || true\n done || true\n\n# Write manifest\ncat > /tmp/pr-context/README.md << 'MANIFEST'\n# PR Context\n\nPre-fetched PR data. All files are in `/tmp/pr-context/`.\n\n| File | Description |\n| --- | --- |\n| `pr.json` | PR metadata — title, body, author, base/head branches, head commit SHA (`headRefOid`), URL |\n| `pr.diff` | Full unified diff of all changes |\n| `files.json` | Changed files array — each entry has `filename`, `status`, `additions`, `deletions`, `patch` |\n| `diffs/<path>.diff` | Per-file diffs with line numbers — numbered lines (e.g. `405\\t+ code`) are commentable; lines without numbers are deleted (LEFT side only) |\n| `file_order_az.txt` | Changed files sorted alphabetically (A→Z), one filename per line |\n| `file_order_za.txt` | Changed files sorted reverse-alphabetically (Z→A), one filename per line |\n| `file_order_largest.txt` | Changed files sorted by diff size descending (largest first), one filename per line |\n| `reviews.json` | Prior review submissions — author, state (APPROVED/CHANGES_REQUESTED/COMMENTED), body |\n| `review_comments.json` | All review threads (GraphQL) — each thread has `id` (node ID for resolving), `isResolved`, `isOutdated`, `path`, `line`, and nested `comments` with `id`, `databaseId` (numeric REST ID for replies), body/author |\n| `unresolved_threads.json` | Unresolved review threads — subset of `review_comments.json` where `isResolved` is false |\n| `resolved_threads.json` | Resolved review threads — subset of `review_comments.json` where `isResolved` is true |\n| `outdated_threads.json` | Outdated review threads — subset of `review_comments.json` where `isOutdated` is true (code changed since comment) |\n| `threads/<path>.json` | Per-file review threads — one file per changed file with existing threads, mirroring the repo path under `threads/` |\n| `comments.json` | PR discussion comments (not inline) |\n| `issue-{N}.json` | Linked issue details (one file per linked issue, if any) |\n| `pr-size.txt` | PR size metrics: `{N} files, {M} diff lines` — used by the agent to decide review fan-out |\n| `agents.md` | Not used in PR context — repository conventions are at `/tmp/agents.md` (pre-fetched) |\n| `review-instructions.md` | Review instructions, criteria, and calibration examples for sub-agents |\n| `agent-review.md` | Main agent instructions — review approach resolved from PR size (written when `ready_to_code_review` is called) |\n| `parent-review.md` | Comment format and inline severity threshold for the parent agent (written when `ready_to_code_review` is called) |\n| `subagent-*.md` | Sub-agent instructions (only written for medium/large PRs when `ready_to_code_review` is called — check `agent-review.md` for which exist) |\nMANIFEST\n\necho \"PR context written to /tmp/pr-context/\"\nls -la /tmp/pr-context/"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

PR size fallback can silently misclassify large PRs as small.

When gh pr diff fails, Line 706 writes an empty pr.diff, then DIFF_LINES is computed from that empty file. This forces diff_lines=0, which can select the small-PR path and skip intended sub-agent fan-out.

Proposed fix
- # Compute PR size metrics for review fan-out decisions
- FILE_COUNT=$(jq 'length' /tmp/pr-context/files.json)
- DIFF_LINES=$(wc -l < /tmp/pr-context/pr.diff | tr -d ' ')
+ # Compute PR size metrics for review fan-out decisions
+ FILE_COUNT=$(jq 'length' /tmp/pr-context/files.json)
+ DIFF_LINES=$(wc -l < /tmp/pr-context/pr.diff | tr -d ' ')
+ if [ "${DIFF_LINES}" = "0" ]; then
+   # Fallback: estimate from per-file patches when full diff fetch fails
+   DIFF_LINES=$(jq -r '[.[] | (.patch // "") | split("\n") | length] | add // 0' /tmp/pr-context/files.json)
+ fi
  echo "${FILE_COUNT} files, ${DIFF_LINES} diff lines" > /tmp/pr-context/pr-size.txt
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/gh-aw-mention-in-pr-no-sandbox.lock.yml at line 706, The
current fallback leaves /tmp/pr-context/pr.diff empty on gh pr diff failure so
DIFF_LINES=$(wc -l < /tmp/pr-context/pr.diff ...) becomes 0 and may misclassify
PR size; update the failure branch (the gh pr diff -> /tmp/pr-context/pr.diff
block and the DIFF_LINES calculation) to detect an empty pr.diff and instead
compute DIFF_LINES from files.json by summing (.additions // 0) + (.deletions //
0) (use jq on /tmp/pr-context/files.json) or write a non-zero sentinel into
pr.diff when falling back; ensure the DIFF_LINES variable logic (the
DIFF_LINES=... line) uses this fallback so large PRs aren't treated as zero-line
small PRs.

@strawgate strawgate merged commit 466621f into main Mar 2, 2026
20 checks passed
@strawgate strawgate deleted the fix/pr-review-prompt-tuning branch March 2, 2026 16:58
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