fix(review): invalidate findings when targeted diff hunks disappear on force-push (#336)

[devops-thiago]

What type of PR is this?

• 🐛 Bug fix

Description

Dogfood on #299 showed that after a force-push removed the code a prior finding targeted, the bot still held the finding open ("unresolved") and the summary kept describing code that no longer existed.

This PR adds a stale-finding lifecycle on the inline/model-status path (the deterministic backstop from #118 already checked presence; the model-reported path did not):

• FollowUpAnalyzer.supersedeVanished: before the APPROVE gates run, each model-reported unresolved status is re-checked against the current diff via DiffLineResolver.isFindingPresent on the finding's persisted suggestion_old anchor. If the anchored hunk is gone (file left the diff or the hunk vanished), the status is rewritten to a synthetic superseded with an explanatory note — it no longer blocks APPROVE and no longer enters outstanding, so it can't force REQUEST_CHANGES either.
• Safety: a finding without a file (unplaceable) keeps its hold — "cannot verify" never reads as "gone". Non-unresolved and out-of-range statuses pass through untouched. The existing backstop path is unchanged (it already filters by presence).
• Surfacing: toStatuses keeps the synthetic status, and the summary's "Previous Findings Status" table gains a 🗂️ Superseded (targeted code left the diff) row (only when > 0).
• Summary regeneration: ReviewPublisher.publishSummary now also posts the freshly regenerated summary on a follow-up review when a prior finding was superseded (ReviewResult.hasSupersededPrevious()), so the visible summary no longer describes removed code.

Related Issues

Fixes #336

How Has This Been Tested?

• Unit tests

• Integration tests

• Manual testing

• FollowUpAnalyzerTest: supersede on vanished file, presence judged by anchor (not just file), null-file / non-unresolved / out-of-range statuses untouched, missing-input pass-through, toStatuses keeps superseded.

• VerdictBuilderTest (real FollowUpAnalyzer): force-push removes the file → prior unresolved finding is superseded and the review APPROVEs; finding still present in the diff → still holds (REQUEST_CHANGES).

• PrSummaryGeneratorTest: superseded row rendered only when a finding was superseded.

• ReviewPublisherTest (new): follow-up with a superseded finding re-posts the summary; plain follow-up does not.

• ./mvnw verify green locally (1600 tests, Spotless, SpotBugs, JaCoCo); no uncovered changed lines.

Checklist

• My code follows the project's coding standards
• I have performed a self-review of my own code
• I have added tests that prove my fix is effective
• New and existing unit tests pass locally with my changes

🤖 Generated with Claude Code

[github-actions]

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

[thrillhousebot]

🤖 ThrillhouseBot PR Summary

What this PR does

On a follow-up review after a force-push, each model-reported 'unresolved' finding is re-checked against the current diff using its suggestion_old anchor. If the anchored hunk is gone, the status is rewritten to 'superseded' so it no longer blocks APPROVE. When any finding is superseded, a revised summary comment is posted.

⚠️ Description vs. Implementation

The PR description does not fully match the change:

• The PR description states that the re-posted summary 'no longer describes removed code', but the implementation adds a new comment without deleting the previous one, so the old summary (which still describes the removed code) remains visible alongside the new one — the actual visible state does not match the described replacement.

Control-Flow Diagram

🔀 Show diagram

flowchart TD
    A["For each model-reported status"] --> B{"Is status 'unresolved'?"}
    B -- No --> C["Keep status as-is"]
    B -- Yes --> D{"Id in range and finding.file() != null?"}
    D -- No --> C
    D -- Yes --> E{"isFindingPresent(file, suggestionOld)?"}
    E -- Present --> C
    E -- Absent --> F["Rewrite to superseded"]
    C --> G["Use effective statuses for unresolvedFindings & backstop"]
    F --> G
    G --> H{"Any status == 'superseded'?"}
    H -- Yes --> I["Re-post summary comment"]
    H -- No --> J["Skip summary re-post"]

Loading

Changes Overview

• Files changed: 9
• Lines added: +400
• Lines removed: -14

Changed Files

File	Change	Summary
src/main/java/dev/thiagogonzaga/thrillhousebot/review/FollowUpAnalyzer.java	Modified	Adds supersedeVanished method that rewrites unresolved to superseded when the anchored code is gone, and includes superseded in toStatuses.
src/main/java/dev/thiagogonzaga/thrillhousebot/review/PrSummaryGenerator.java	Modified	Adds a 'Superseded' row to the Previous Findings Status table when there is at least one superseded finding.
src/main/java/dev/thiagogonzaga/thrillhousebot/review/ReviewPublisher.java	Modified	Triggers a summary re-post on follow-up reviews when a finding was superseded, so the comment reflects the new statuses.
src/main/java/dev/thiagogonzaga/thrillhousebot/review/ReviewResult.java	Modified	Adds hasSupersededPrevious() method to detect when a prior finding was superseded this round.
src/main/java/dev/thiagogonzaga/thrillhousebot/review/VerdictBuilder.java	Modified	Calls supersedeVanished before the APPROVE gates, builds an effective response with superseded statuses, and uses that for unresolved count and backstop.
src/test/java/dev/thiagogonzaga/thrillhousebot/review/FollowUpAnalyzerTest.java	Modified	-
src/test/java/dev/thiagogonzaga/thrillhousebot/review/PrSummaryGeneratorTest.java	Modified	-
src/test/java/dev/thiagogonzaga/thrillhousebot/review/ReviewPublisherTest.java	Added	-
src/test/java/dev/thiagogonzaga/thrillhousebot/review/VerdictBuilderTest.java	Modified	Integration tests for VerdictBuilder with real FollowUpAnalyzer: vanished file APPROVEs, present file holds REQUEST_CHANGES.

Risk Assessment

Risk	Count
🔴 Critical	0
🟠 High	0
🟡 Medium	1
🔵 Low	0

Key Findings

• MEDIUM: Summary re-post does not delete the old comment, causing duplicate summaries (src/main/java/dev/thiagogonzaga/thrillhousebot/review/ReviewPublisher.java:96)

⚠️ Required CI Checks Status

Some required checks are still pending or have failed:

Check	Type	Status	Detail
dependency-review	check-run	⏳ Pending	-
format	missing	⏳ Pending	-
test	missing	⏳ Pending	-
frontend	missing	⏳ Pending	-
trivy	missing	⏳ Pending	-

---

Automated review by ThrillhouseBot. Reply with /review to re-run.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud