fix(server): withhold the claim that fails its evidence check, not the review (#1512)

A quote that does not verify now discredits only its own claim, via a dedicated EvidenceQuoteUnverifiedException caught per observation; every other refusal in the evidence gate — unstaged source, malformed citation, wrong attribution — still fails the whole delivery. Occurrence keys index the submitted position so a retry with a different admitted set cannot renumber them. Logs when every negative claim is withheld, since the remainder reads as an all-clear.

Author: FelixTJDietrich

.changeset/evidence-boundary-withholds-the-claim.md

@@ -0,0 +1,5 @@
+---
+"hephaestus": patch
+---
+
+A review whose evidence check fails on one observation now delivers the others. The check that refuses to show a developer a claim it cannot trace back to the code applied to the whole review at once: one practice that mis-quoted its source — by a stray character, in a file the reader never sees — withheld every other finding in that review, including correct, fully evidenced ones. The developer saw nothing at all. Only a quote that does not match its source is treated this way; a citation to evidence the review never gathered still stops the whole delivery, as before. What was withheld is logged with the reason, and a review in which no claim can be verified still fails rather than arriving empty.

server/src/main/java/de/tum/cit/aet/hephaestus/agent/handler/PracticeDetectionDeliveryService.java

@@ -4,6 +4,7 @@
 import de.tum.cit.aet.hephaestus.agent.conversation.ConversationSourceLiveness;
 import de.tum.cit.aet.hephaestus.agent.documentation.DocumentProjection;
 import de.tum.cit.aet.hephaestus.agent.handler.PracticeDetectionResultParser.ValidatedObservation;
+import de.tum.cit.aet.hephaestus.agent.handler.spi.EvidenceQuoteUnverifiedException;
 import de.tum.cit.aet.hephaestus.agent.handler.spi.JobDeliveryException;
 import de.tum.cit.aet.hephaestus.agent.job.AgentJob;
 import de.tum.cit.aet.hephaestus.agent.runtime.ProvenanceDigest;
@@ -33,9 +34,9 @@
 import de.tum.cit.aet.hephaestus.practices.observation.PracticeDetectionCompletedEvent;
 import java.nio.charset.StandardCharsets;
 import java.time.Instant;
+import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.HashSet;
-import java.util.IdentityHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -144,7 +145,15 @@ public DeliveryResult deliver(AgentJob job, List<ValidatedObservation> validObse
         }
         Target target = resolveTarget(job, metadata);
         Map<String, PracticeRevision> revisionsBySlug = admittedRevisions(job, workspaceId);
-        for (ValidatedObservation observation : validObservations) {
+        // A quote that does not verify discredits its own claim, and only EvidenceQuoteUnverifiedException
+        // means that. Every other refusal here — an unstaged source, a malformed citation, work attributed
+        // to the wrong person — impugns the run, so it stays fatal.
+        List<Integer> admittedIndexes = new ArrayList<>(validObservations.size());
+        List<ValidatedObservation> admittedObservations = new ArrayList<>(validObservations.size());
+        List<String> withheldObservations = new ArrayList<>();
+        boolean withheldNegative = false;
+        for (int submittedIndex = 0; submittedIndex < validObservations.size(); submittedIndex++) {
+            ValidatedObservation observation = validObservations.get(submittedIndex);
             PracticeRevision revision = revisionsBySlug.get(observation.practiceSlug());
             if (revision == null) {
                 throw new JobDeliveryException(
@@ -155,7 +164,42 @@ public DeliveryResult deliver(AgentJob job, List<ValidatedObservation> validObse
                 );
             }
             enforceAttribution(observation, revision, job);
-            enforceEvidenceBoundary(observation, revision, evidenceBoundary, job);
+            try {
+                enforceEvidenceBoundary(observation, revision, evidenceBoundary, job);
+                admittedIndexes.add(submittedIndex);
+                admittedObservations.add(observation);
+            } catch (EvidenceQuoteUnverifiedException ex) {
+                withheldNegative |= observation.assessment() == Assessment.BAD;
+                withheldObservations.add(observation.practiceSlug() + ": " + ex.getMessage());
+            }
+        }
+        if (!withheldObservations.isEmpty()) {
+            // Per claim, because a model that cannot quote its own evidence is a defect an otherwise
+            // successful delivery would hide.
+            log.warn(
+                "Withheld {} of {} observation(s) whose quoted evidence did not verify, delivering the rest: jobId={} withheld={}",
+                withheldObservations.size(),
+                validObservations.size(),
+                job.getId(),
+                withheldObservations
+            );
+            // Withholding the only fault leaves an all-clear standing over a defect the model did find,
+            // which is a different statement to the reader than an incomplete review.
+            if (withheldNegative && admittedObservations.stream().noneMatch(o -> o.assessment() == Assessment.BAD)) {
+                log.error(
+                    "Withheld every negative observation; the remaining claims read as an all-clear: jobId={}",
+                    job.getId()
+                );
+            }
+        }
+        // Only when there was something to admit: a review that found nothing still publishes its zero.
+        if (admittedObservations.isEmpty() && !validObservations.isEmpty()) {
+            throw new JobDeliveryException(
+                "No observation survived the evidence check, so there is nothing to deliver: jobId=" +
+                    job.getId() +
+                    ", withheld=" +
+                    withheldObservations
+            );
         }
 
         ObservationOrigin origin = originOf(metadata);
@@ -171,20 +215,22 @@ public DeliveryResult deliver(AgentJob job, List<ValidatedObservation> validObse
         boolean hasNegative = false;
         Instant observedAt = Instant.now();
 
-        // Keyed by observation identity because equal observations still represent distinct occurrences.
-        Map<ValidatedObservation, ObservationKeys> observationKeys = new IdentityHashMap<>();
+        // Carries the keys each observation was persisted under.
+        List<ValidatedObservation> deliveredObservations = new ArrayList<>(admittedObservations.size());
 
-        for (int i = 0; i < validObservations.size(); i++) {
-            ValidatedObservation observation = validObservations.get(i);
+        for (int i = 0; i < admittedObservations.size(); i++) {
+            ValidatedObservation observation = admittedObservations.get(i);
 
             PracticeRevision revision = revisionsBySlug.get(observation.practiceSlug());
             Practice practice = revision.getPractice();
 
-            // Includes the index so distinct observations for the same practice on one artifact don't collide.
+            // The position the observation was SUBMITTED at, not its position among those admitted: this key
+            // is a retry's dedup grain, so a claim withheld on one attempt and not the next must not renumber
+            // the claims after it into keys that miss what is already stored.
             String occurrenceKey =
                 observation.practiceSlug() +
                 ":" +
-                i +
+                admittedIndexes.get(i) +
                 ":" +
                 artifactKind.value() +
                 ":" +
@@ -211,7 +257,7 @@ public DeliveryResult deliver(AgentJob job, List<ValidatedObservation> validObse
                 aboutUserId,
                 firstLocationPath(observation.evidence())
             );
-            observationKeys.put(observation, new ObservationKeys(occurrenceKey, recurrenceKey));
+            deliveredObservations.add(observation.withKeys(new ObservationKeys(occurrenceKey, recurrenceKey)));
 
             Long practiceRevisionId = revision.getId();
 
@@ -274,7 +320,7 @@ public DeliveryResult deliver(AgentJob job, List<ValidatedObservation> validObse
             )
         );
 
-        return new DeliveryResult(inserted, discardedDuplicate, hasNegative, observationKeys);
+        return new DeliveryResult(inserted, discardedDuplicate, hasNegative, deliveredObservations);
     }
 
     /**
@@ -419,7 +465,7 @@ private void enforceEvidenceBoundary(
                 );
             String artifactContent = new String(content, StandardCharsets.UTF_8);
             if (!"scm.pull-request.diff".equals(kind.value()) && !artifactContent.contains(exactQuote)) {
-                throw new JobDeliveryException(
+                throw new EvidenceQuoteUnverifiedException(
                     "Evidence quote does not occur in the cited artifact: path=" +
                         artifactPath.asText() +
                         ", jobId=" +
@@ -445,7 +491,7 @@ private void enforceEvidenceBoundary(
                           exactQuote
                       ))
             ) {
-                throw new JobDeliveryException(
+                throw new EvidenceQuoteUnverifiedException(
                     "Evidence quote does not match the cited diff location: path=" +
                         path.asText() +
                         ", line=" +
@@ -996,14 +1042,11 @@ static String firstLocationPath(JsonNode evidence) {
         return path != null && path.isString() ? path.asString() : null;
     }
 
-    /**
-     * @param observationKeys the keys persisted for each observation, by identity, so the caller stamps the same
-     *     keys onto its deliverable observations instead of recomputing them
-     */
+    /** @param delivered what this call persisted, each carrying the keys it was stored under. */
     public record DeliveryResult(
         int inserted,
         int discardedDuplicate,
         boolean hasNegative,
-        Map<ValidatedObservation, ObservationKeys> observationKeys
+        List<ValidatedObservation> delivered
     ) {}
 }

server/src/main/java/de/tum/cit/aet/hephaestus/agent/handler/PullRequestReviewHandler.java

@@ -494,16 +494,9 @@ private void processObservations(AgentJob job, JsonNode output, boolean admissio
             throw new JobDeliveryException("Delivery failed unexpectedly: jobId=" + job.getId(), e);
         }
 
-        // Stamp each observation with the exact keys deliver() persisted, by identity, so downstream stages
-        // address the stored observation without recomputing a key that could drift.
-        Map<PracticeDetectionResultParser.ValidatedObservation, ObservationKeys> keysByObservation =
-            result.observationKeys();
-        for (int i = 0; i < scopedObservations.size(); i++) {
-            scopedObservations.set(
-                i,
-                scopedObservations.get(i).withKeys(keysByObservation.get(scopedObservations.get(i)))
-            );
-        }
+        // What deliver() persisted, carrying the keys it stored them under, so a later stage addresses the
+        // stored observation rather than recomputing a key that could drift.
+        scopedObservations = new ArrayList<>(result.delivered());
 
         if (admissionOnly) return;
 

server/src/main/java/de/tum/cit/aet/hephaestus/agent/handler/spi/EvidenceQuoteUnverifiedException.java

@@ -0,0 +1,17 @@
+package de.tum.cit.aet.hephaestus.agent.handler.spi;
+
+/**
+ * An observation quoted evidence that does not appear where it said the evidence was.
+ *
+ * <p>Distinct from every other refusal in the evidence gate because it is the one that says nothing
+ * about the run: the sources were staged, the citation was well formed and authorized, and the model
+ * simply did not reproduce what it read. That discredits the claim and no other, so it is caught per
+ * observation. Every other refusal — an unstaged source, a malformed citation, a missing search —
+ * means the run itself cannot be trusted, and stays fatal.
+ */
+public class EvidenceQuoteUnverifiedException extends JobDeliveryException {
+
+    public EvidenceQuoteUnverifiedException(String message) {
+        super(message);
+    }
+}

server/src/test/java/de/tum/cit/aet/hephaestus/agent/handler/PracticeDetectionDeliveryServiceIntegrationTest.java

@@ -299,7 +299,7 @@ void validObservationsPersistedToDb() {
         }
 
         @Test
-        @DisplayName("returned observationKeys align exactly with the persisted recurrence_key set")
+        @DisplayName("returned delivered observations align exactly with the persisted recurrence_key set")
         void returnedFingerprintsMatchPersistedRecurrenceKeys() {
             var observations = List.of(
                 observation("pr-description-quality", Presence.PRESENT),
@@ -308,7 +308,13 @@ void returnedFingerprintsMatchPersistedRecurrenceKeys() {
 
             var result = deliveryService.deliver(agentJob, observations);
 
-            assertThat(result.observationKeys().values().stream().map(ObservationKeys::recurrenceKey).toList())
+            assertThat(
+                result
+                    .delivered()
+                    .stream()
+                    .map(o -> o.recurrenceKey())
+                    .toList()
+            )
                 .as("one stable key returned per delivered observation")
                 .hasSize(2)
                 .allMatch(k -> k != null && k.matches("[0-9a-f]{64}"));
@@ -321,7 +327,11 @@ void returnedFingerprintsMatchPersistedRecurrenceKeys() {
             assertThat(persistedKeys)
                 .as("every returned fingerprint is persisted as a recurrence_key, and vice versa")
                 .containsExactlyInAnyOrderElementsOf(
-                    result.observationKeys().values().stream().map(ObservationKeys::recurrenceKey).toList()
+                    result
+                        .delivered()
+                        .stream()
+                        .map(o -> o.recurrenceKey())
+                        .toList()
                 );
         }
 

server/src/test/java/de/tum/cit/aet/hephaestus/agent/handler/PracticeDetectionDeliveryServiceTest.java

@@ -390,6 +390,64 @@ void rejectsAQuoteThatIsNotInTheCitedArtifact() {
             verifyNoInteractions(observationRepository);
         }
 
+        @Test
+        @DisplayName("only the claim whose quote does not verify is withheld; the other is delivered")
+        void withholdsOnlyTheObservationWhoseQuoteDoesNotVerify() {
+            Practice second = new Practice();
+            ReflectionTestUtils.setField(second, "id", 20L);
+            second.setSlug("pr-scope");
+            second.setBindings(PracticeTestEvidence.bindings(ArtifactKinds.PULL_REQUEST));
+            second.setAutomatedReviewPolicy(PracticeTestEvidence.forArtifact(ArtifactKinds.PULL_REQUEST));
+            admit(second, 21L);
+
+            ValidatedObservation sound = validObservation("pr-description-quality", Presence.PRESENT);
+            ValidatedObservation misquoted = validObservation("pr-scope", Presence.PRESENT);
+            ((ObjectNode) misquoted.evidence().withArray("citations").get(0)).put("quote", "+ insecure();,");
+
+            var result = service.deliver(testJob, List.of(sound, misquoted));
+
+            assertThat(result.delivered())
+                .as("the claim that verified is the one persisted, and it is the only one")
+                .extracting(ValidatedObservation::practiceSlug)
+                .containsExactly("pr-description-quality");
+            assertThat(result.inserted()).isEqualTo(1);
+        }
+
+        @Test
+        @DisplayName("a citation to an unstaged source still fails the whole delivery, even beside a sound claim")
+        void anEvidenceFailureThatIsNotAQuoteMismatchStillFailsEverything() {
+            Practice second = new Practice();
+            ReflectionTestUtils.setField(second, "id", 20L);
+            second.setSlug("pr-scope");
+            second.setBindings(PracticeTestEvidence.bindings(ArtifactKinds.PULL_REQUEST));
+            second.setAutomatedReviewPolicy(PracticeTestEvidence.forArtifact(ArtifactKinds.PULL_REQUEST));
+            admit(second, 21L);
+
+            ValidatedObservation sound = validObservation("pr-description-quality", Presence.PRESENT);
+            ValidatedObservation unstaged = validObservation("pr-scope", Presence.PRESENT);
+            ObjectNode citation = (ObjectNode) unstaged.evidence().withArray("citations").get(0);
+            citation.put("sourceKind", "scm.repository.tree");
+            citation.remove("side");
+
+            assertThatThrownBy(() -> service.deliver(testJob, List.of(sound, unstaged)))
+                .as("an unstaged source impugns the run, not just the claim that cited it")
+                .isInstanceOf(JobDeliveryException.class)
+                .hasMessageContaining("misattributed evidence source");
+            verifyNoInteractions(observationRepository);
+        }
+
+        @Test
+        @DisplayName("a batch in which no quote verifies is still a failed delivery")
+        void refusesTheDeliveryWhenNoObservationSurvivesAdmission() {
+            ValidatedObservation misquoted = validObservation("pr-description-quality", Presence.PRESENT);
+            ((ObjectNode) misquoted.evidence().withArray("citations").get(0)).put("quote", "fabricated quote");
+
+            assertThatThrownBy(() -> service.deliver(testJob, List.of(misquoted)))
+                .isInstanceOf(JobDeliveryException.class)
+                .hasMessageContaining("No observation survived the evidence check");
+            verifyNoInteractions(observationRepository);
+        }
+
         @Test
         void acceptsASecretScannerCitationWithoutPersistingTheSecret() {
             ValidatedObservation observation = validObservation("pr-description-quality", Presence.PRESENT);
@@ -833,7 +891,7 @@ void persistsValidObservation() {
             assertThat(fingerprintCaptor.getValue())
                 .as("persisted recurrence_key matches the returned findingFingerprint")
                 .matches("[0-9a-f]{64}")
-                .isEqualTo(result.observationKeys().values().iterator().next().recurrenceKey());
+                .isEqualTo(result.delivered().get(0).keys().recurrenceKey());
 
             verify(eventPublisher).publishEvent(eventCaptor.capture());
             PracticeDetectionCompletedEvent event = eventCaptor.getValue();

server/src/test/java/de/tum/cit/aet/hephaestus/agent/handler/PullRequestReviewHandlerTest.java

@@ -464,7 +464,9 @@ void delegatesToDeliveryService() {
                 }
                 """;
             AgentJob job = jobWithOutput(rawOutput);
-            when(deliveryService.deliver(eq(job), any())).thenReturn(new DeliveryResult(1, 0, false, Map.of()));
+            when(deliveryService.deliver(eq(job), any())).thenAnswer(inv ->
+                new DeliveryResult(1, 0, false, inv.getArgument(1))
+            );
 
             admit(job, rawOutput);
 
@@ -499,8 +501,8 @@ void hardcodedSecretUsesPracticeSeverityCap() {
             ArgumentCaptor<List<PracticeDetectionResultParser.ValidatedObservation>> captor = ArgumentCaptor.forClass(
                 List.class
             );
-            when(deliveryService.deliver(eq(job), captor.capture())).thenReturn(
-                new DeliveryResult(1, 0, false, Map.of())
+            when(deliveryService.deliver(eq(job), captor.capture())).thenAnswer(inv ->
+                new DeliveryResult(1, 0, false, inv.getArgument(1))
             );
 
             admit(job, rawOutput);
@@ -606,8 +608,8 @@ void injectsSecretFindingWhenModelAbstainsButDiffCommitsCredential() {
             ArgumentCaptor<List<PracticeDetectionResultParser.ValidatedObservation>> captor = ArgumentCaptor.forClass(
                 List.class
             );
-            when(deliveryService.deliver(eq(job), captor.capture())).thenReturn(
-                new DeliveryResult(1, 0, false, Map.of())
+            when(deliveryService.deliver(eq(job), captor.capture())).thenAnswer(inv ->
+                new DeliveryResult(1, 0, false, inv.getArgument(1))
             );
 
             admit(job, rawOutput);