From dd8db72e8e74bc6633a6736725c4d2d1b420aaa3 Mon Sep 17 00:00:00 2001
From: Christoph Blecker <admin@toph.ca>
Date: Fri, 3 Jul 2026 20:49:44 -0700
Subject: [PATCH] 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
---
 pr-review-toolkit/.claude-plugin/plugin.json  |  2 +-
 pr-review-toolkit/skills/review-pr/SKILL.md   | 34 +++++++++++++++++++
 .../skills/review-pr/review-pr.js             |  2 ++
 3 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/pr-review-toolkit/.claude-plugin/plugin.json b/pr-review-toolkit/.claude-plugin/plugin.json
index d0a4ec6..989c60f 100644
--- a/pr-review-toolkit/.claude-plugin/plugin.json
+++ b/pr-review-toolkit/.claude-plugin/plugin.json
@@ -1,6 +1,6 @@
 {
   "name": "pr-review-toolkit",
-  "version": "1.8.1",
+  "version": "1.9.0",
   "description": "Comprehensive PR review board using shared workflow context",
   "author": {
     "name": "cblecker",
diff --git a/pr-review-toolkit/skills/review-pr/SKILL.md b/pr-review-toolkit/skills/review-pr/SKILL.md
index c8edbff..99c0eab 100644
--- a/pr-review-toolkit/skills/review-pr/SKILL.md
+++ b/pr-review-toolkit/skills/review-pr/SKILL.md
@@ -268,6 +268,40 @@ Choose the proposed review event from the selected findings:
 - `APPROVE` when the user selected "Leave an approving review" from the
   nothing-postable menu and no findings are being posted.
 
+## Translate Line Numbers for Posting
+
+Skip this section if MCP fallback was used (no local checkout) — findings
+already use PR HEAD line numbers.
+
+Findings use merge-result line numbers. GitHub requires PR HEAD (`HEAD^2`)
+line numbers. Translate before posting.
+
+### Quick check
+
+```bash
+git diff --name-only HEAD^2 HEAD
+```
+
+No output → lines are identical, skip translation.
+
+### Per-file translation
+
+For each finding on a file listed above:
+
+1. Run `git diff HEAD^2 HEAD -- <path>`.
+2. If the finding's line falls on a `+`-only line (no PR HEAD equivalent),
+   move the finding to the review body.
+3. If the line falls within a hunk on a context line, find the PR HEAD line
+   by counting context and `-` lines from the hunk's PR HEAD start (`a` in
+   `@@ -a,b +c,d @@`).
+4. If the line falls between hunks, accumulate the offset from preceding
+   hunks: `offset += (b - d)` per `@@ -a,b +c,d @@`.
+   `PR_HEAD_line = merge_line + offset`.
+5. Validate the translated line falls within a PR diff hunk (from
+   `pull_request_read get_files`). If not, move to review body.
+
+Files absent from the diff output have identical line numbers — use as-is.
+
 ## Preview And Confirm
 
 Before posting, show an exact preview.
diff --git a/pr-review-toolkit/skills/review-pr/review-pr.js b/pr-review-toolkit/skills/review-pr/review-pr.js
index 10353b8..55c27c6 100644
--- a/pr-review-toolkit/skills/review-pr/review-pr.js
+++ b/pr-review-toolkit/skills/review-pr/review-pr.js
@@ -1365,6 +1365,7 @@ function patchInstructions(prContext) {
   if (sources.fullDiffIncluded && prContext.fullDiff) {
     return '## Diff context\n\n'
       + 'The full merge diff is included below. Use it as your primary source for understanding what changed. '
+      + 'Line numbers in your findings will correspond to the merge-result checkout and will be translated to PR HEAD line numbers before posting. '
       + 'You may also use Read and Grep on the merged checkout to inspect unchanged context, trace cross-file effects, or verify assumptions.\n\n'
       + 'Do not run Bash commands. Do not call GitHub write tools.\n\n'
       + '<full-merge-diff>\n' + prContext.fullDiff + '\n</full-merge-diff>'
@@ -1373,6 +1374,7 @@ function patchInstructions(prContext) {
   if (sources.manifestSource === 'local-git') {
     return '## Diff context\n\n'
       + 'The working tree is checked out to the merge result for this PR. Full diff text was omitted because it exceeded the size cap. '
+      + 'Line numbers in your findings will correspond to the merge-result checkout and will be translated to PR HEAD line numbers before posting. '
       + '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'
       + 'Do not run Bash commands. Do not call GitHub write tools.'
   }