Skip to content

fix(pr-review): judge earlier findings per head, rebase-safe follow-up delta - #1017

Merged
Makisuo merged 1 commit into
mainfrom
fix/pr-review-follow-up-delta
Sep 23, 2026
Merged

Makisuo merged 1 commit into
mainfrom
fix/pr-review-follow-up-delta

Conversation

@Makisuo

@Makisuo Makisuo commented Sep 23, 2026 •

Copy link
Copy Markdown
Collaborator

Follow-up to #1001, #1014 and #1016, rebased onto main. The kickoff's changed-files line keeps the exact shape followUpScope (#1014) parses, so the coverage gate reads a rebase-safe delta the same way.

Earlier findings

The mechanical repeat filter dropped any new finding with the same path and category within three lines of an open or dismissed one, which suppressed a new bug sitting next to an old one and missed an old issue whose code moved.

  • withoutRepeats now drops only near-exact restatements: same path, category, a line within 3, and titles sharing at least 60% of their words.
  • The follow-up kickoff and PR_REVIEW_SYSTEM_PROMPT ask the reviewer to judge each open finding at the new head: follow moved code, modified lines are not a fix, leave it open when unsure, never re-file an open finding, but do file a different defect nearby.

Follow-up delta across a rebase

fetchChangedPaths(previousSha, headSha) was a three-dot compare, so after a force-push rebase it listed every file the base branch changed.

  • VcsProviderClient.fetchChangesSince returns { paths, rewritten }.
  • GitHub: when the compare status is ahead/identical the forward diff is used as before. Otherwise each head is compared against the PR base and only files whose own change differs are kept (vcs/range-diff.ts, hunk-header line numbers ignored; binary/oversized files, added and dropped files count as changed).
  • No base sha, or a comparison at GitHub's 300-file cap, yields paths: undefined and the kickoff asks for a full review (previously a >300-file compare was truncated silently).

Tests

range-diff.test.ts, new findings.test.ts cases (new bug beside an old one kept, moved code left to the model, kickoff wording), and GithubProvider.pulls.test.ts cases for the ancestor path, a rebase that excludes base-branch files, and both fallbacks. bun run test for pr-review and the GitHub vendor: backend 112 and apps/ai chat 185 passing.


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

Summary by CodeRabbit

  • Bug Fixes
    • Follow-up reviews now account for rebases and force-pushes, focusing on files whose changes differ and requesting a full review when the comparison is unavailable.
    • Open findings are checked against the current code, including when code has moved. Restated findings stay open rather than being filed again, while distinct nearby defects can still be reported.
    • Follow-up review summaries show the review scope and indicate when branch history was rewritten.

@coderabbitai

coderabbitai Bot commented Sep 23, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: d16f55cd-3086-4669-9c8d-9783a6c50341

📥 Commits

Reviewing files that changed from the base of the PR and between fe61113 and 3e1bb74.

📒 Files selected for processing (13)
  • apps/ai/src/chat/prompts.ts
  • apps/ai/src/chat/review-coverage.test.ts
  • packages/backend/src/services/integrations/vcs/VcsProviderClient.ts
  • packages/backend/src/services/integrations/vcs/range-diff.test.ts
  • packages/backend/src/services/integrations/vcs/range-diff.ts
  • packages/backend/src/services/integrations/vcs/vendor/github/GithubAppClient.ts
  • packages/backend/src/services/integrations/vcs/vendor/github/GithubProvider.ts
  • packages/backend/src/services/integrations/vcs/vendor/github/__tests__/GithubProvider.pulls.test.ts
  • packages/backend/src/services/pr-review/PrReviewConversationService.test.ts
  • packages/backend/src/services/pr-review/PrReviewService.test.ts
  • packages/backend/src/services/pr-review/PrReviewService.ts
  • packages/backend/src/services/pr-review/findings.test.ts
  • packages/backend/src/services/pr-review/findings.ts
 ___________________________________________________
< Stack Overflow called, they want their code back. >
 ---------------------------------------------------
  \
   \   (\__/)
       (•ㅅ•)
       /   づ
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Base automatically changed from feat/pr-review-agent to main September 23, 2026 22:33
…p delta

The repeat filter dropped any new finding with the same path and category
within three lines of an open or dismissed one. That swallowed a new bug next
to an old one, and missed an old issue whose code moved. It now drops only
near-exact restatements (same path, category, nearby line and a title sharing
most of its words). The kickoff and the system prompt ask the reviewer to
judge each open finding at the new head, following moved code, never treating
modified lines as a fix, and leaving it open when unsure.

fetchChangedPaths was a three-dot compare between the last reviewed head and
the new one, so after a force-push rebase onto a newer base it listed every
file the base branch changed. fetchChangesSince keeps that compare when the
earlier head is still an ancestor. When history was rewritten it diffs each
head against the pull request's base and keeps only the files whose own change
differs, ignoring hunk-header line shifts. Without a base, or when a
comparison hits the 300-file cap, the delta is unknown and the kickoff asks
for a full review.
@Makisuo
Makisuo force-pushed the fix/pr-review-follow-up-delta branch from abf6f81 to 3e1bb74 Compare September 23, 2026 22:36
@Makisuo
Makisuo merged commit 5e68f34 into main Sep 23, 2026
11 of 12 checks passed
@Makisuo
Makisuo deleted the fix/pr-review-follow-up-delta branch September 23, 2026 22:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant