diff --git a/src/main/java/dev/thiagogonzaga/thrillhousebot/review/ai/FindingVerificationService.java b/src/main/java/dev/thiagogonzaga/thrillhousebot/review/ai/FindingVerificationService.java
index 0aaef6cb..fbf68f5d 100644
--- a/src/main/java/dev/thiagogonzaga/thrillhousebot/review/ai/FindingVerificationService.java
+++ b/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.
*
+ *
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).
+ *
*
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 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).
+ *
+ * Sharing {@link #decisionOf} was only half of what #710 needs. The two readers also have to
+ * agree on which 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.
+ *
+ *
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 byCandidateId(
+ List verdicts) {
+ var byId = new HashMap();
+ 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.
+ *
+ * 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 findings) throws IOExceptio
}
ReviewResponse apply(ReviewResponse response, VerificationResponse verification) {
- var byId = new HashMap();
- for (VerificationResponse.Verdict verdict : verification.verdicts()) {
- byId.putIfAbsent(verdict.id(), verdict);
- }
-
+ var byId = byCandidateId(verification.verdicts());
var kept = new ArrayList();
var rejected = 0;
var downgraded = 0;
diff --git a/src/test/java/dev/thiagogonzaga/thrillhousebot/review/ai/FindingVerificationServiceTest.java b/src/test/java/dev/thiagogonzaga/thrillhousebot/review/ai/FindingVerificationServiceTest.java
index 40c86d62..cd804eb9 100644
--- a/src/test/java/dev/thiagogonzaga/thrillhousebot/review/ai/FindingVerificationServiceTest.java
+++ b/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();
+
+ 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();
+
+ 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();
+
+ 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"));