[Bug]: Review submission fails on lines outside the PR diff and hides GitHub’s validation error #12549

Activity

1. thedanielgoodwin

   added

   bugSomething is broken or behaving incorrectly.Something is broken or behaving incorrectly.

   needs-triageIssue needs maintainer review and initial categorization.Issue needs maintainer review and initial categorization.

   on Sep 19, 2026
2. juliusmarminge commented on Sep 19, 2026

   

   juliusmarminge

   on Sep 19, 2026

   Member

   More actions

   Triage

   Confirmed on current main (same version as the report: 0.0.42). This is a real product bug, not a GitHub outage or a one-off CLI glitch. I did not find a duplicate.

   GitHub’s POST /repos/{owner}/{repo}/pulls/{number}/reviews will not accept an inline comment whose path + line + side is outside the pull request’s returned hunks. One bad anchor 422s the entire review (errors: ["Line could not be resolved"]). T3 both creates those anchors and then hides GitHub’s message.

   What the UI is doing

   Reviewing a file hydrates it through Pierre’s full-file loader (createPullRequestDiffFileContentsLoader in apps/web/src/lib/diffFileContents.ts). Once the file is no longer partial, resolveDiffReviewPosition treats the gaps before / between / after GitHub hunks as commentable context:

   export function resolveDiffReviewPosition(
     fileDiff: FileDiffMetadata,
     lineNumber: number,
     side: SelectionSide | undefined,
   ): PullRequestReviewPosition | null {
     const lineIndex = findDiffReviewLineIndex(fileDiff, lineNumber, side);
     // ...
       case "context":
         return line.oldLineNumber === null || line.newLineNumber === null
           ? null
           : {
               kind: "context",
               oldLine: line.oldLineNumber,
               newLine: line.newLineNumber,
               side: side === "deletions" ? "left" : "right",
             };
   }

   findDiffReviewLineIndex / buildDiffReviewLines turn that on with includeExpandedContext = !fileDiff.isPartial. The Code tab then accepts the gutter click with no hunk check (beginComment in PullRequestCodeTab.tsx).

   That is exactly the reported draft: first GitHub hunk starts at line 2, T3 still lets you park a comment on line 1 as { kind: "context", oldLine: 1, newLine: 1, side: "right" }. buildReviewSubmissionJson sends that as { line: 1, side: "RIGHT" }. GitHub cannot resolve it, so the whole review dies.

   The helper we already have for “is this line actually in a hunk?” is isLineInFileDiff in apps/web/src/components/pullRequest/pullRequestDiff.logic.ts. It is used to place existing host threads, not to gate new pending comments. After expansion, pending comments are pinned to the expanded line anyway, so the draft looks legal.

   Why the toast is useless

   Failure is stripped at three layers:

   1. VcsProcessExitError.fromProcessExit throws away gh stderr and substitutes "Process exited with a non-zero status." (packages/contracts/src/vcs.ts).
   2. GitHubCliCommandError.detail is the constant "GitHub CLI command failed." (apps/server/src/sourceControl/GitHubCli.ts).
   3. The review bar ignores PullRequestOperationError.detail and toasts a hard-coded title, with reportFailure: false:

       if (result._tag === "Failure") {
         // The draft is kept: whatever went wrong, retyping the review is not the answer.
         toastManager.add({ type: "error", title: "The review could not be submitted" });

   The draft-preserving part is already correct. The missing piece is telling the reviewer which comment GitHub refused.

   Server-side submitReview only checks host capabilities, verdict, empty body, and write access. It does not validate comment coordinates against the patch it just fetched.

   Suggested direction

   Smallest useful fix, in this order:

   1. Stop creating illegal anchors. When starting a line comment, refuse (or warn and block submit for) any position that fails isLineInFileDiff against the unhydrated hunks. Expanded context can stay readable; it should not become a GitHub review target. Same check before submitReview so a leftover draft cannot take the rest of the review down with it.
   2. Name the bad comment. On client validation failure, toast the file and line (path:line) so the reviewer can move or drop that one draft. Keep the other pending comments, as today.
   3. Surface GitHub’s 422. If something still slips through, parse errors from the gh api body (or at least stop replacing stderr with a generic detail) and show that string in the toast. PullRequestOperationError already has a detail field; the UI just does not use it here.

   Do not try to “fix” this by posting out-of-hunk lines another way on the same reviews call. GitHub’s public API will not take them. A later enhancement could offer “move this to the review summary / file comment”; that is not required to close the hole.

   Related but different: #6348 taught GitLab to send both old_line and new_line for context. GitHub’s bug here is “this line is not in the PR diff at all,” not a missing field.

   Not present in a newer release than 0.0.42. Workaround remains: move the out-of-hunk comment onto a line that GitHub’s hunks actually include (the reporter’s line 1 → line 24 case).
3. juliusmarminge

   added

   acceptedfeature request acceptedfeature request accepted

   via-triageFiled through npx t3 triageFiled through npx t3 triage

   and removed

   needs-triageIssue needs maintainer review and initial categorization.Issue needs maintainer review and initial categorization.

   on Sep 19, 2026
4. agomesp

   added a commit that references this issue on Sep 23, 2026

   fix: show GitHub's reason when it refuses a pull request review

   ...

   92f68c6
5. kevin-david commented on Oct 20, 2026

   

   kevin-david

   on Oct 20, 2026

   More actions

   I hit this today on my first use of the PR diff view, Version 0.1.0 (81493efb71e8) I had four pending inline comments. Their submission failed with the generic toast and the server trace contained "Unprocessable Entity; Line could not be resolved".

   The recovery UX was also slightly annoying because T3 still showed four pending comments, but I couldn’t find them in the diff, they seemed to just disappear. With Codex's help I recovered them the client’s in-memory draft store. Three could be submitted at their original locations and the fourth pointed to unchanged context between hunks.

   Alongside the #22382 error-message fix, a few other ideas:

   • A list of pending comments with a way to jump to, move, or remove any comment (including an invalid one) would make this much easier to recover from. Or really just be a nice way to see the "pending" conversation.
   • Another option would be to submit the pending comments one by one as a GitHub pending review to the server. That would be a bit chattier but could be less likely to lose comments like I (nearly) did today
6. t3dotgg commented on Oct 21, 2026

   

   t3dotgg

   on Oct 21, 2026

   Member

   More actions

   Note

   🤖 Claude Opus 5.5 responding on behalf of Theo

   Thanks for the report. One part of this is fixed on main in #12583. Please test on the nightly built from main on 2026-10-21, or on the next stable release. One part is still open: Surface GitHub's validation error and identify any rejected draft comment. If that part still matters to you, please open a new issue for that part only.
7. t3dotgg

   closed this as completedon Oct 21, 2026