fix(review): collapse duplicate verdict ids the same way in both readers

src/main/java/dev/thiagogonzaga/thrillhousebot/review/ai/FindingVerificationService.java

@@ -32,6 +32,7 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Locale;
+import java.util.Map;
 import java.util.Set;
 import java.util.function.Consumer;
 import java.util.regex.Matcher;
@@ -728,6 +729,9 @@ private ReviewResponse salvageTruncatedVerdicts(
    * an id outside the 1-based candidate range; counting only the ids in range keeps the log honest
    * about what stayed unverified.
    *
+   * <p>Duplicate ids are collapsed through {@link #byCandidateId} before any label is read, so the
+   * verdict counted for an id is exactly the one {@link #apply} acted on (#735).
+   *
    * <p>An id alone is not coverage. A verdict whose decision label is absent, blank or not one of
    * {@link #ACTED_ON_DECISIONS} falls into {@link #apply}'s fail-open default, where the candidate
    * posts exactly as the reviewer raised it — the same state a candidate with no verdict at all
@@ -753,24 +757,55 @@ private ReviewResponse salvageTruncatedVerdicts(
    */
   private static long candidatesCovered(
       List<VerificationResponse.Verdict> salvaged, int candidates) {
-    return salvaged.stream()
-        .filter(verdict -> ACTED_ON_DECISIONS.contains(decisionOf(verdict)))
-        .mapToInt(VerificationResponse.Verdict::id)
-        .filter(id -> id >= 1 && id <= candidates)
-        .distinct()
+    return byCandidateId(salvaged).entrySet().stream()
+        .filter(entry -> entry.getKey() >= 1 && entry.getKey() <= candidates)
+        .filter(entry -> ACTED_ON_DECISIONS.contains(decisionOf(entry.getValue())))
         .count();
   }
 
+  /**
+   * The one verdict per candidate id, first element wins — the single collapse both {@link #apply}
+   * and {@link #candidatesCovered} resolve duplicates through, so neither can read a verdict the
+   * other ignored (#735).
+   *
+   * <p>Sharing {@link #decisionOf} was only half of what #710 needs. The two readers also have to
+   * agree on <em>which</em> verdict an id's decision is read from, and they did not: {@code apply}
+   * collapsed duplicates first and read the label from the survivor, while the count filtered by
+   * label first and de-duplicated the ids that were left. An id carrying an undecidable element
+   * followed by a decidable one therefore landed in {@code apply}'s fail-open default — the finding
+   * posts exactly as the reviewer raised it — while the count called that candidate screened, which
+   * is the published-set-reads-as-fully-screened harm #623 exists to prevent, reached by a narrower
+   * route. The drift is one-directional (the reverse order already agreed), so it only ever
+   * over-counted, i.e. exclusively toward the dangerous side.
+   *
+   * <p>Duplicates are model output, not a hypothetical: the collapse predates this fix, first-wins
+   * is the behaviour the audit already applies, and the count now inherits it rather than inventing
+   * a second resolution.
+   */
+  private static Map<Integer, VerificationResponse.Verdict> byCandidateId(
+      List<VerificationResponse.Verdict> verdicts) {
+    var byId = new HashMap<Integer, VerificationResponse.Verdict>();
+    for (VerificationResponse.Verdict verdict : verdicts) {
+      byId.putIfAbsent(verdict.id(), verdict);
+    }
+    return byId;
+  }
+
   /**
    * The verdict's decision, normalized the one way both {@link #apply} and {@link
    * #candidatesCovered} read it; the empty string for a candidate with no verdict and for a verdict
    * carrying no label. One reader so the two cannot disagree about what counts as a decision — the
    * drift #710 was filed on.
+   *
+   * <p>Stripped, so the strictness genuinely matches {@link #strictRisk}/{@link #strictConfidence}
+   * as documented: {@code "rejected "} was unreadable as a decision while {@code "high "} was a
+   * readable rating, an asymmetry nothing intended (#735). Both readers take the same value, so
+   * this never drove a drift — it only cost an over-cautious keep and an under-count.
    */
   private static String decisionOf(VerificationResponse.Verdict verdict) {
     return verdict == null || verdict.verdict() == null
         ? ""
-        : verdict.verdict().toLowerCase(Locale.ROOT);
+        : verdict.verdict().strip().toLowerCase(Locale.ROOT);
   }
 
   /**
@@ -1181,11 +1216,7 @@ String renderCandidates(List<ReviewResponse.Finding> findings) throws IOExceptio
   }
 
   ReviewResponse apply(ReviewResponse response, VerificationResponse verification) {
-    var byId = new HashMap<Integer, VerificationResponse.Verdict>();
-    for (VerificationResponse.Verdict verdict : verification.verdicts()) {
-      byId.putIfAbsent(verdict.id(), verdict);
-    }
-
+    var byId = byCandidateId(verification.verdicts());
     var kept = new ArrayList<ReviewResponse.Finding>();
     var rejected = 0;
     var downgraded = 0;

src/test/java/dev/thiagogonzaga/thrillhousebot/review/ai/FindingVerificationServiceTest.java

@@ -776,6 +776,84 @@ void doesNotCountAVerdictWhoseDecisionTheAuditCannotRead() {
     assertEquals(VerificationCoverage.Outcome.PARTIAL, reported.get(0).outcome());
   }
 
+  @Test
+  void doesNotCountADuplicateIdWhoseFirstVerdictTheAuditCannotRead() {
+    // #735: a complete, well-formed body carrying two verdicts for the same id, the first of them
+    // undecidable. apply() collapses duplicates first-wins and lands in its fail-open default, so
+    // the finding posts exactly as the reviewer raised it; the count has to read that same first
+    // verdict rather than the decidable duplicate behind it, or the published set claims a second
+    // stage ruled on a finding it never ruled on — #710's harm on an uncut body.
+    ReviewResponse original =
+        response(finding("critical", "high", "Undecided"), finding("high", "high", "Ruled on"));
+    when(verifier.verify(anyString(), anyString(), anyString(), anyString(), anyString()))
+        .thenReturn(
+            aiOk(
+                """
+            {"verdicts": [
+              {"id": 1, "reason": "still weighing this one"},
+              {"id": 1, "verdict": "rejected", "reason": "framework idiom"},
+              {"id": 2, "verdict": "confirmed", "reason": "real"}]}"""));
+    var reported = new ArrayList<VerificationCoverage>();
+
+    var result = service.verify(SESSION, original, "diff", "stack", "", reported::add);
+
+    assertEquals(2, result.findings().size());
+    assertEquals("Undecided", result.findings().get(0).title());
+    assertEquals(List.of(new VerificationCoverage(2, 1)), reported);
+    assertEquals(VerificationCoverage.Outcome.PARTIAL, reported.get(0).outcome());
+    assertTrue(reported.get(0).disclosed());
+  }
+
+  @Test
+  void countsADuplicateIdWhoseFirstVerdictTheAuditActedOn() {
+    // The control for the case above: the same duplicate in the other order. apply() rejects on the
+    // first element, so the count must include the id — the two readers agreed here already, and
+    // collapsing the duplicate in the count must not turn that agreement into an under-count.
+    ReviewResponse original =
+        response(finding("critical", "high", "Ruled on"), finding("high", "high", "Also"));
+    when(verifier.verify(anyString(), anyString(), anyString(), anyString(), anyString()))
+        .thenReturn(
+            aiOk(
+                """
+            {"verdicts": [
+              {"id": 1, "verdict": "rejected", "reason": "framework idiom"},
+              {"id": 1, "reason": "still weighing this one"},
+              {"id": 2, "verdict": "confirmed", "reason": "real"}]}"""));
+    var reported = new ArrayList<VerificationCoverage>();
+
+    var result = service.verify(SESSION, original, "diff", "stack", "", reported::add);
+
+    assertEquals(1, result.findings().size());
+    assertEquals("Also", result.findings().get(0).title());
+    assertEquals(List.of(new VerificationCoverage(2, 2)), reported);
+    assertFalse(reported.get(0).disclosed());
+  }
+
+  @Test
+  void readsADecisionLabelPaddedWithWhitespace() {
+    // #735: decisionOf did not strip while strictRisk/strictConfidence do, so "rejected " was
+    // unreadable as a decision even though "high " is a readable rating — an asymmetry against a
+    // javadoc claiming the strictness matches. Both readers take the same value, so the finding is
+    // acted on and counted together.
+    ReviewResponse original =
+        response(finding("critical", "high", "Padded"), finding("high", "high", "Kept"));
+    when(verifier.verify(anyString(), anyString(), anyString(), anyString(), anyString()))
+        .thenReturn(
+            aiOk(
+                """
+            {"verdicts": [
+              {"id": 1, "verdict": "rejected\\n", "reason": "framework idiom"},
+              {"id": 2, "verdict": " CONFIRMED ", "reason": "real"}]}"""));
+    var reported = new ArrayList<VerificationCoverage>();
+
+    var result = service.verify(SESSION, original, "diff", "stack", "", reported::add);
+
+    assertEquals(1, result.findings().size());
+    assertEquals("Kept", result.findings().get(0).title());
+    assertEquals(List.of(new VerificationCoverage(2, 2)), reported);
+    assertFalse(reported.get(0).disclosed());
+  }
+
   @Test
   void reportsZeroCoverageWhenTheCutLeavesNoCompleteVerdict() {
     ReviewResponse original = response(finding("critical", "high", "Bug"));