Skip to content

Commit dd8db72

Browse files
committed
fix(pr-review-toolkit): translate merge-result line numbers to PR HEAD before posting
Review agents analyze the merge-result checkout for quality, but GitHub's add_comment_to_pending_review API expects line numbers from the PR branch HEAD (HEAD^2). When the base branch has diverged, lines shift between the two, causing API rejections for valid findings. Add a translation step in SKILL.md before posting that diffs HEAD^2 vs HEAD to compute the offset, and moves unmappable findings to the review body. Update workflow prompt strings to note that agent line numbers correspond to the merge result. Assisted-by: Claude:claude-opus-4-6
1 parent ff1f0ee commit dd8db72

3 files changed

Lines changed: 37 additions & 1 deletion

File tree

pr-review-toolkit/.claude-plugin/plugin.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "pr-review-toolkit",
3-
"version": "1.8.1",
3+
"version": "1.9.0",
44
"description": "Comprehensive PR review board using shared workflow context",
55
"author": {
66
"name": "cblecker",

pr-review-toolkit/skills/review-pr/SKILL.md

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,40 @@ Choose the proposed review event from the selected findings:
268268
- `APPROVE` when the user selected "Leave an approving review" from the
269269
nothing-postable menu and no findings are being posted.
270270

271+
## Translate Line Numbers for Posting
272+
273+
Skip this section if MCP fallback was used (no local checkout) — findings
274+
already use PR HEAD line numbers.
275+
276+
Findings use merge-result line numbers. GitHub requires PR HEAD (`HEAD^2`)
277+
line numbers. Translate before posting.
278+
279+
### Quick check
280+
281+
```bash
282+
git diff --name-only HEAD^2 HEAD
283+
```
284+
285+
No output → lines are identical, skip translation.
286+
287+
### Per-file translation
288+
289+
For each finding on a file listed above:
290+
291+
1. Run `git diff HEAD^2 HEAD -- <path>`.
292+
2. If the finding's line falls on a `+`-only line (no PR HEAD equivalent),
293+
move the finding to the review body.
294+
3. If the line falls within a hunk on a context line, find the PR HEAD line
295+
by counting context and `-` lines from the hunk's PR HEAD start (`a` in
296+
`@@ -a,b +c,d @@`).
297+
4. If the line falls between hunks, accumulate the offset from preceding
298+
hunks: `offset += (b - d)` per `@@ -a,b +c,d @@`.
299+
`PR_HEAD_line = merge_line + offset`.
300+
5. Validate the translated line falls within a PR diff hunk (from
301+
`pull_request_read get_files`). If not, move to review body.
302+
303+
Files absent from the diff output have identical line numbers — use as-is.
304+
271305
## Preview And Confirm
272306

273307
Before posting, show an exact preview.

pr-review-toolkit/skills/review-pr/review-pr.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1365,6 +1365,7 @@ function patchInstructions(prContext) {
13651365
if (sources.fullDiffIncluded && prContext.fullDiff) {
13661366
return '## Diff context\n\n'
13671367
+ 'The full merge diff is included below. Use it as your primary source for understanding what changed. '
1368+
+ 'Line numbers in your findings will correspond to the merge-result checkout and will be translated to PR HEAD line numbers before posting. '
13681369
+ 'You may also use Read and Grep on the merged checkout to inspect unchanged context, trace cross-file effects, or verify assumptions.\n\n'
13691370
+ 'Do not run Bash commands. Do not call GitHub write tools.\n\n'
13701371
+ '<full-merge-diff>\n' + prContext.fullDiff + '\n</full-merge-diff>'
@@ -1373,6 +1374,7 @@ function patchInstructions(prContext) {
13731374
if (sources.manifestSource === 'local-git') {
13741375
return '## Diff context\n\n'
13751376
+ 'The working tree is checked out to the merge result for this PR. Full diff text was omitted because it exceeded the size cap. '
1377+
+ 'Line numbers in your findings will correspond to the merge-result checkout and will be translated to PR HEAD line numbers before posting. '
13761378
+ 'Use the file manifest for whole-PR awareness, then use Read and Grep on the merged checkout to inspect changed files and trace cross-file effects.\n\n'
13771379
+ 'Do not run Bash commands. Do not call GitHub write tools.'
13781380
}

0 commit comments

Comments
 (0)