Skip to content

feat: comment on an issue whose fix reaches beyond the pull request diff - #151

Merged
bugale merged 1 commit into
mainfrom
bugale/comment-outside-diff
Aug 14, 2026
Merged

feat: comment on an issue whose fix reaches beyond the pull request diff#151
bugale merged 1 commit into
mainfrom
bugale/comment-outside-diff

Conversation

@bugale

@bugale bugale commented Aug 14, 2026

Copy link
Copy Markdown
Owner

What

An issue was commented on only when the pull request diff showed every line it spans, since GitHub rejects a comment anchored outside the diff. That made the action fail a step without any comment explaining why.

Now the comment is anchored on the run of lines the diff does show around an added line of the range — the widest anchor GitHub accepts — and a fix reaching beyond that anchor is rendered as a plain code block with a note naming the lines it covers, instead of a suggestion that would rewrite lines the pull request does not show.

Why

Found on a real clang-format run (winagent run 11519055):

  • the PR changed one line (877) of a 16-line IoCreateFileEx(...) call;
  • git-clang-format correctly passed --lines=877:877, but clang-format's smallest unit is the whole statement, so it re-laid-out lines 868–883;
  • the PR diff has a single hunk covering 874–880.

isNewIssue (some line added) kept the issue → the step failed. isCommentableIssue (every line in the diff) rejected it → no comment at all. This closes that gap: since isNewIssue guarantees at least one added line, an anchor always exists.

Implementation

  • isCommentableIssue is replaced by getCommentAnchor, which returns the normalized path, the anchored range and whether it is partial, or undefined when the diff adds none of the issue's lines. It grows the anchor from the first added line outwards while the neighbouring lines are in the diff, staying inside both the issue range and a single hunk.
  • buildCommentBody takes a partial flag and renders the fix as a plain fence plus the note instead of a suggestion.
  • addComments uses the anchor for path/line/start_line, so the comment lands on lines GitHub accepts.

Testing

  • Unit tests for getCommentAnchor: full range, truncated range, path relativization, and the cases that anchor nothing (no added line, unknown file, missing path/line, end line before start line).
  • A fixBeyondDiff suite mirroring the winagent case exactly — issue 868–883 against a @@ -874,7 +874,7 @@ hunk — asserting the 874–880 anchor, the plain-fence body with the note, and that a fully shown fix still renders as a suggestion.
  • Verified end-to-end against the real data: the actual clang-format diff (reproduced locally byte-identical to the run's SARIF) plus the real PR diff now yield an anchor of 874–880 and a comment carrying the full 15-line replacement, where the run produced nothing.

An issue was commented on only when the diff showed every line it spans, because GitHub rejects a
comment anchored outside the diff. A formatter that reformats a whole statement because one of its
lines changed reports a range reaching past the three context lines the diff carries around that
change, so such an issue failed the step without any comment explaining it.

The comment is now anchored on the run of lines that the diff does show around an added line of the
range, which is the widest anchor GitHub accepts. The fix of an issue reaching beyond that anchor is
rendered as a plain code block naming the lines it covers rather than as a suggestion, since
applying it in one click would rewrite lines the pull request does not show.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@bugale
bugale marked this pull request as ready for review August 14, 2026 20:59
@bugale
bugale merged commit 092a7b1 into main Aug 14, 2026
28 checks passed
@bugale
bugale deleted the bugale/comment-outside-diff branch August 14, 2026 20:59
@github-actions

Copy link
Copy Markdown

🎉 This PR is included in version 5.1.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant