Skip to content

feat(github): skip threads on files outside the PR diff - #169

Draft
pavelzw wants to merge 2 commits into
mainfrom
claude/github-comment-review-87isjy
Draft

pavelzw wants to merge 2 commits into
mainfrom
claude/github-comment-review-87isjy

Conversation

@pavelzw

@pavelzw pavelzw commented Sep 12, 2026

Copy link
Copy Markdown
Collaborator

GitHub accepts review comments on unchanged files but never displays them, causing reviewers to lose those comments. This change prevents posting threads on files outside the PR's diff and provides clear feedback to users.

Changes

  • Protocol: Added ChangedFile type to represent files in the PR diff; updated ExportInput to require snap.changed
  • Server: Modified buildReview() to accept a changed set of file paths and skip threads anchored outside the diff with reason 'outside the diff'
  • Client: Added inDiff check in CommentCard to disable the GitHub export button for threads on unchanged files, with a tooltip explaining why
  • Tests:
    • Added test fixtures (CHANGED set, changedFile() helper, snapshotOf() helper) to support diff-aware testing
    • Added test case verifying threads on unchanged files are skipped with correct reason
    • Added test case verifying the export button is disabled for out-of-diff threads
    • Updated all buildReview() calls to pass the changed parameter
  • Documentation: Updated README to clarify that threads on files outside the diff are skipped

Implementation details

  • buildReview() now filters threads by checking changed.has(t.anchor.path) before including them
  • The client reads snapshot.changed from store state to determine if a thread's file is in the diff
  • Button remains visible but disabled with explanatory tooltip, maintaining UI consistency with the stale-thread pattern

https://claude.ai/code/session_01APKj1AFTW3wEnoeL4VnnhX

diffle lets a reviewer comment on any tree file, but a GitHub review
comment on an unchanged file is accepted by the API and then shown
nowhere: the Files tab only renders the pull request's diff, so the
comment silently vanishes. Reported on #160 for file threads and line
threads alike.

The exporter now leaves such threads out with the reason `outside the
diff`, which the client's skip toast and the all-skipped 400 already
surface, and the comment card disables its GitHub button on those
threads with a title saying why.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01APKj1AFTW3wEnoeL4VnnhX
@github-actions github-actions Bot added the enhancement New feature or request label Sep 12, 2026
Two more ways a thread reached GitHub and went nowhere, reported on #169:

- The panel's "All" button posted every open thread, so threads on
  unchanged files were only caught by the server's skip. It now counts
  them out up front: its title says how many will be skipped, it is
  disabled when nothing would show on GitHub, and each such row carries
  an "outside the diff" mark while export is on.
- A line thread on a changed file but beyond GitHub's three lines of
  context, placed after expanding the diff or with a wider --context,
  drew a 422 from the API. The exporter now checks each line thread
  against the file's patch with GitHub's own context, provided by the
  session, and skips it as "outside the diff hunks".

A failing gh call also used to surface only the first stderr line, which
for a 422 is just the status. The full stderr and response body now go
to the server's stderr, and the toast carries every stderr line.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01APKj1AFTW3wEnoeL4VnnhX
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants