feat(review): re-check a maintainer's decline against the code

.env.example

@@ -136,6 +136,13 @@ AI_TIMEOUT=300s
 # hedging guard still runs, and verifier errors always fail open.
 #REVIEW_VERIFIER_ENABLED=true
 
+# Re-check a maintainer's decline against the reviewed code before recording a
+# prior finding "justified" (default true). The finding stays open for one more
+# round only when the reviewed diff plainly contradicts the stated reason; style,
+# intent and accepted-risk rebuttals are respected, and a second reply on the
+# thread always ends the re-check. false = a reply closes the finding outright.
+#REVIEW_DECLINE_RECHECK_ENABLED=true
+
 # When findings block the merge (REQUEST_CHANGES). Default balanced matches v0.x:
 # only CRITICAL/HIGH risk with HIGH confidence. Use strict for security-heavy
 # repos (any CRITICAL/HIGH blocks, even after the verifier demotes confidence).

README.md

@@ -243,6 +243,7 @@ will change per provider:
 | `WEBHOOK_BASE_BRANCHES` | Comma-separated globs; only auto-review PRs whose base branch matches one (e.g. `main,release/*`). Globs are gitignore-style: `*` does **not** cross `/`, so use `**` to span slashes (`**` alone matches every branch) | _(empty — all branches)_ |
 | `WEBHOOK_IGNORED_BASE_BRANCHES` | Comma-separated globs; skip auto-review of PRs whose base branch matches one (wins over allowlist; same `*`/`**` rule — match nested branches with `**`, e.g. `dependabot/**`) | _(empty)_ |
 | `REVIEW_VERIFIER_ENABLED` | Second, skeptical AI pass that re-checks each finding against the diff before posting, dropping or downgrading what it can't confirm (see [AI call budget](#ai-call-budget)); fails open — a verifier error keeps the original findings | `true` |
+| `REVIEW_DECLINE_RECHECK_ENABLED` | Re-check a maintainer's decline against the reviewed code before a prior finding is recorded "justified" (see [Re-checking declines](#re-checking-declines)); the finding stays open for one more round only when the reviewed diff plainly contradicts the stated reason. `false` makes a maintainer reply close the finding unconditionally | `true` |
 | `REVIEW_BLOCKING_STRICTNESS` | When findings escalate to `REQUEST_CHANGES`: `balanced` (CRITICAL/HIGH + HIGH confidence), `strict` (any CRITICAL/HIGH), or `lenient` (CRITICAL + HIGH confidence only). See [Blocking strictness](#blocking-strictness) | `balanced` |
 | `REVIEW_CONVERSATIONAL_REPLIES_ENABLED` | Answer `@thrillhousebot` mentions in PR threads (including finding replies) with an AI reply | `true` |
 | `REVIEW_ADD_DOCS_ENABLED` | Allow the on-demand `/add-docs` command to generate docstrings as committable suggestions | `true` |
@@ -281,6 +282,34 @@ cost of more false positives; a deterministic hedging guard still runs, and a
 verifier failure never blocks the review (it fails open, keeping the original
 findings).
 
+### Re-checking declines
+
+When a maintainer replies to a finding to decline it, the follow-up analysis
+records that finding as **justified** and the bot moves on. A dismissal is a
+claim, though, not ground truth — a correct finding can be closed by an
+incorrect rebuttal, and the rebuttal often names the very mechanism that makes
+the bug real ("it only runs after the webhook is acked, so there's no race" —
+on an executor that starts a thread per event).
+
+`REVIEW_DECLINE_RECHECK_ENABLED=true` (the default) therefore traces a decline's
+stated reason against the code the review actually saw. When the reviewed diff
+**plainly contradicts** that reason, the finding is kept **open for one more
+round** with a note quoting both the claim and the contradicting line, instead
+of being recorded justified. It is deliberately conservative:
+
+- Trusting the maintainer is the default. A rebuttal about house style, intent,
+  accepted risk, or priority — anything not refutable from the code — is
+  respected, as is any premise whose supporting code is not in the diff.
+- **One push-back, then defer.** The re-check only fires while the thread carries
+  a single maintainer reply; replying again always ends it, so the bot can never
+  keep re-opening the same finding round after round.
+- The re-opened finding is never re-posted as a new comment — it stays tracked in
+  *Previous Findings Status*, so nobody is asked to answer the same comment twice.
+- The override holds approval (`APPROVE` → `COMMENT`) exactly like any other
+  unresolved previous finding; it never invents a new blocking finding.
+
+Set it to `false` to make a maintainer's reply final, unconditionally.
+
 ### Blocking strictness
 
 By default (`REVIEW_BLOCKING_STRICTNESS=balanced`), only **CRITICAL** or **HIGH**

src/main/java/dev/thiagogonzaga/thrillhousebot/config/ThrillhouseConfig.java

@@ -204,6 +204,17 @@ interface ReviewConfig {
     @WithName("verifier-enabled")
     boolean verifierEnabled();
 
+    /**
+     * Whether a maintainer's decline is re-checked against the reviewed code before a prior finding
+     * is recorded "justified". When the reviewed code plainly contradicts the rebuttal's premise
+     * the finding stays open for one more round; every other decline is respected, and a second
+     * reply on the thread always ends the re-check. Turn it off to make a maintainer's reply final,
+     * unconditionally.
+     */
+    @WithDefault("true")
+    @WithName("decline-recheck-enabled")
+    boolean declineRecheckEnabled();
+
     /**
      * How severely a finding must score before the review escalates to {@code REQUEST_CHANGES}. One
      * of {@code balanced} (default — CRITICAL/HIGH + HIGH confidence), {@code strict} (any

src/main/java/dev/thiagogonzaga/thrillhousebot/review/FollowUpAnalyzer.java

@@ -18,6 +18,7 @@
 import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import dev.thiagogonzaga.thrillhousebot.config.BotIdentity;
+import dev.thiagogonzaga.thrillhousebot.config.ThrillhouseConfig;
 import dev.thiagogonzaga.thrillhousebot.github.GitHubReviewClient;
 import dev.thiagogonzaga.thrillhousebot.review.ai.FindingVerificationService;
 import dev.thiagogonzaga.thrillhousebot.review.ai.ReviewResponse;
@@ -33,6 +34,7 @@
 import java.util.Map;
 import java.util.Set;
 import java.util.function.Predicate;
+import java.util.function.Supplier;
 import java.util.stream.Stream;
 
 /** Analyzes follow-up reviews by comparing new findings against prior reviews. */
@@ -71,9 +73,23 @@ public class FollowUpAnalyzer {
 
   private final ObjectMapper mapper;
 
+  /** Whether {@link #recheckDeclines} may override a maintainer decline; see the config key. */
+  private final boolean declineRecheckEnabled;
+
   @Inject
-  public FollowUpAnalyzer(ObjectMapper mapper) {
+  public FollowUpAnalyzer(ObjectMapper mapper, ThrillhouseConfig config) {
+    this(mapper, config.review().declineRecheckEnabled());
+  }
+
+  /** Visible for tests; the decline re-check is on, matching the shipped default. */
+  FollowUpAnalyzer(ObjectMapper mapper) {
+    this(mapper, true);
+  }
+
+  /** Visible for tests: pins the decline re-check flag. */
+  FollowUpAnalyzer(ObjectMapper mapper, boolean declineRecheckEnabled) {
     this.mapper = mapper;
+    this.declineRecheckEnabled = declineRecheckEnabled;
   }
 
   /**
@@ -705,6 +721,119 @@ private static boolean hasVanished(
     return !lineResolver.isFindingPresent(currentPath, finding.suggestionOld());
   }
 
+  /**
+   * Re-checks a maintainer's decline against the code before it is recorded {@code justified}. The
+   * model reports a decline as an outcome; this step treats it as a <em>claim</em>. When the code
+   * the review actually saw plainly contradicts the rebuttal's premise ({@link
+   * RebuttalContradiction}), the status is rewritten back to {@code unresolved} with a one-line
+   * note quoting both the claim and the contradicting line — so a correct finding is not closed by
+   * an incorrect rebuttal, and only declines that survive the re-check are safe to remember.
+   *
+   * <p>Trusting the maintainer stays the default; every leg below must hold before an override
+   * fires, and any one of them missing leaves the {@code justified} status untouched:
+   *
+   * <ul>
+   *   <li>the re-check is enabled ({@code thrillhousebot.review.decline-recheck-enabled});
+   *   <li>the finding's own thread is identifiable and carries <em>exactly one</em> maintainer
+   *       reply. One reply is the decline, and the re-check pushes back once; a second human reply
+   *       is the maintainer answering that push-back, and it always wins — that is the guaranteed
+   *       escape hatch, and it is why the override cannot recur round after round;
+   *   <li>{@link RebuttalContradiction} finds a contradiction, which it only reports for a premise
+   *       refutable from code text. A rebuttal about style, intent, accepted risk, or priority
+   *       matches nothing and keeps the decline.
+   * </ul>
+   *
+   * <p>Overridden findings re-enter the ordinary {@code unresolved} path — they hold APPROVE
+   * exactly like a model-reported unresolved status and are never re-posted as new findings, so the
+   * maintainer is not asked to answer the same comment twice.
+   *
+   * @param reviewedCode supplies the diff text the review call saw; resolved lazily because most
+   *     rounds have no declined finding at all
+   */
+  public List<ReviewResponse.PreviousFindingStatus> recheckDeclines(
+      List<ReviewResponse.Finding> previous,
+      List<ReviewResponse.PreviousFindingStatus> statuses,
+      List<GitHubReviewClient.PullRequestComment> inlineComments,
+      BotIdentity botIdentity,
+      Supplier<String> reviewedCode) {
+    if (statuses == null || statuses.isEmpty()) {
+      return statuses == null ? List.of() : statuses;
+    }
+    if (!declineRecheckEnabled || !hasDecline(statuses)) {
+      return statuses;
+    }
+    // An empty prior round and an empty comment list need no fast path of their own: the
+    // id-range check and the thread lookup below already yield "no contradiction" for both.
+    String code = reviewedCode == null ? null : reviewedCode.get();
+    if (previous == null || inlineComments == null || code == null || code.isBlank()) {
+      return statuses;
+    }
+    var rewritten = new ArrayList<ReviewResponse.PreviousFindingStatus>(statuses.size());
+    for (var status : statuses) {
+      var contradiction = declineContradiction(status, previous, inlineComments, botIdentity, code);
+      if (contradiction == null) {
+        rewritten.add(status);
+        continue;
+      }
+      Log.infof(
+          "Re-opening previous finding #%d: the maintainer's decline claims '%s' but the reviewed"
+              + " code shows '%s'",
+          status.id(), contradiction.claim(), contradiction.evidence());
+      rewritten.add(
+          new ReviewResponse.PreviousFindingStatus(
+              status.id(), STATUS_UNRESOLVED, contradiction.note()));
+    }
+    return rewritten;
+  }
+
+  /** Whether any status is a maintainer decline — the only kind this re-check looks at. */
+  private static boolean hasDecline(List<ReviewResponse.PreviousFindingStatus> statuses) {
+    return statuses.stream().anyMatch(s -> STATUS_JUSTIFIED.equalsIgnoreCase(s.status()));
+  }
+
+  /**
+   * The contradiction that disqualifies a {@code justified} status, or {@code null} when the
+   * decline stands. Returning {@code null} is the conservative outcome and is what every unmatched,
+   * absent, or ambiguous input produces.
+   */
+  private static RebuttalContradiction.Contradiction declineContradiction(
+      ReviewResponse.PreviousFindingStatus status,
+      List<ReviewResponse.Finding> previous,
+      List<GitHubReviewClient.PullRequestComment> inlineComments,
+      BotIdentity botIdentity,
+      String reviewedCode) {
+    if (!STATUS_JUSTIFIED.equalsIgnoreCase(status.status())) {
+      return null;
+    }
+    var id = status.id();
+    if (id < 1 || id > previous.size()) {
+      return null;
+    }
+    var finding = previous.get(id - 1);
+    Long rootId = rootCommentId(finding, id, inlineComments, botIdentity);
+    if (rootId == null) {
+      return null;
+    }
+    var humanReplies = humanReplies(rootId, inlineComments, botIdentity);
+    if (humanReplies.size() != 1) {
+      return null;
+    }
+    return RebuttalContradiction.find(finding, humanReplies.get(0), reviewedCode).orElse(null);
+  }
+
+  /** Bodies of the maintainer replies on a thread, oldest first; bot replies are not rebuttals. */
+  private static List<String> humanReplies(
+      Long rootId,
+      List<GitHubReviewClient.PullRequestComment> inlineComments,
+      BotIdentity botIdentity) {
+    return inlineComments.stream()
+        .filter(c -> rootId.equals(c.inReplyToId()))
+        .filter(c -> c.user() != null && !botIdentity.matches(c.user().login()))
+        .map(GitHubReviewClient.PullRequestComment::body)
+        .filter(body -> body != null && !body.isBlank())
+        .toList();
+  }
+
   /**
    * Deterministic approve backstop. The bot's own prior findings the model silently dropped — still
    * present in the current diff, carrying no maintainer reply, and not closed by any round —