diff --git a/mateclaw-server/src/main/java/vip/mate/goal/model/GoalCriteriaCodec.java b/mateclaw-server/src/main/java/vip/mate/goal/model/GoalCriteriaCodec.java index 470cd6f8..ad1de57f 100644 --- a/mateclaw-server/src/main/java/vip/mate/goal/model/GoalCriteriaCodec.java +++ b/mateclaw-server/src/main/java/vip/mate/goal/model/GoalCriteriaCodec.java @@ -60,6 +60,7 @@ public final class GoalCriteriaCodec { * Merge a per-round verdict delta into the full checklist by id. Criteria * absent from the delta retain their state unless their pass lacks evidence. * The criterion text is always kept from the existing item (the verdict never carries text). + * Duplicate verdict ids are ambiguous and rejected instead of taking the last value. */ public static List merge(List existing, List verdicts) { @@ -70,7 +71,9 @@ public final class GoalCriteriaCodec { if (verdicts != null) { for (GoalChecklistVerdict.CriterionVerdict v : verdicts) { if (v != null && v.id() != null) { - byId.put(v.id(), v); + if (byId.putIfAbsent(v.id(), v) != null) { + throw new IllegalArgumentException("Duplicate criterion verdict id"); + } } } } diff --git a/mateclaw-server/src/main/java/vip/mate/goal/service/GoalEvaluationService.java b/mateclaw-server/src/main/java/vip/mate/goal/service/GoalEvaluationService.java index 224365bb..1252d477 100644 --- a/mateclaw-server/src/main/java/vip/mate/goal/service/GoalEvaluationService.java +++ b/mateclaw-server/src/main/java/vip/mate/goal/service/GoalEvaluationService.java @@ -81,10 +81,8 @@ public class GoalEvaluationService implements Evaluator { private final ProviderChatModelFactory chatModelFactory; private final ObjectMapper objectMapper; - private final BeanOutputConverter draftConverter = - new BeanOutputConverter<>(GoalCriteriaDraft.class); - private final BeanOutputConverter verdictConverter = - new BeanOutputConverter<>(GoalChecklistVerdict.class); + private final BeanOutputConverter draftConverter; + private final BeanOutputConverter verdictConverter; public GoalEvaluationService(GoalProperties properties, ModelConfigService modelConfigService, @@ -94,6 +92,13 @@ public class GoalEvaluationService implements Evaluator { this.modelConfigService = modelConfigService; this.chatModelFactory = chatModelFactory; this.objectMapper = objectMapper; + // Preserve converter tolerance for extra fields, but never silently + // choose the last value of an ambiguous model-produced JSON key. + ObjectMapper evaluatorJson = objectMapper.copy() + .disable(com.fasterxml.jackson.databind.DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES) + .enable(com.fasterxml.jackson.core.JsonParser.Feature.STRICT_DUPLICATE_DETECTION); + this.draftConverter = new BeanOutputConverter<>(GoalCriteriaDraft.class, evaluatorJson); + this.verdictConverter = new BeanOutputConverter<>(GoalChecklistVerdict.class, evaluatorJson); } /** @@ -233,7 +238,8 @@ public class GoalEvaluationService implements Evaluator { + "Revoke it when such contradictory evidence exists, citing that evidence. " + "An attempted action, a goal description or a claim of completion is not proof. " + "Newly passed criteria require concrete observable evidence. Return only changed " - + "criterion verdicts; omitted criteria retain their previous state. Keep evidence concise. " + + "criterion verdicts; omitted criteria retain their previous state. Return at most one " + + "verdict per criterion id. Keep evidence concise. " + "Output only the requested JSON."; private String buildUserPrompt(GoalEntity goal, diff --git a/mateclaw-server/src/test/java/vip/mate/goal/model/GoalCriteriaCodecTest.java b/mateclaw-server/src/test/java/vip/mate/goal/model/GoalCriteriaCodecTest.java index 44bd3ace..55405916 100644 --- a/mateclaw-server/src/test/java/vip/mate/goal/model/GoalCriteriaCodecTest.java +++ b/mateclaw-server/src/test/java/vip/mate/goal/model/GoalCriteriaCodecTest.java @@ -12,6 +12,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.assertThrows; /** * Pure unit tests for the checklist (de)serialization + merge helpers. @@ -51,6 +52,17 @@ class GoalCriteriaCodecTest { assertNull(GoalCriteriaCodec.serialize(null, mapper)); } + @Test + void duplicateVerdictIdsAreRejectedInsteadOfLastWriteWinning() { + var existing = List.of(new GoalCriterion("C1", "report", false, "")); + var failed = new GoalChecklistVerdict.CriterionVerdict("C1", false, "missing"); + var passed = new GoalChecklistVerdict.CriterionVerdict("C1", true, "claimed written"); + assertThrows(IllegalArgumentException.class, () -> GoalCriteriaCodec.merge(existing, List.of(failed, passed))); + assertThrows(IllegalArgumentException.class, () -> GoalCriteriaCodec.merge(existing, List.of(passed, failed))); + assertThrows(IllegalArgumentException.class, () -> GoalCriteriaCodec.merge(existing, List.of(passed, passed))); + assertFalse(existing.getFirst().passed()); + } + // ---------- merge ---------- @Test diff --git a/mateclaw-server/src/test/java/vip/mate/goal/service/GoalEvaluationServiceTest.java b/mateclaw-server/src/test/java/vip/mate/goal/service/GoalEvaluationServiceTest.java index e840f4bd..6c3bdd4c 100644 --- a/mateclaw-server/src/test/java/vip/mate/goal/service/GoalEvaluationServiceTest.java +++ b/mateclaw-server/src/test/java/vip/mate/goal/service/GoalEvaluationServiceTest.java @@ -29,6 +29,7 @@ import java.util.List; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; @@ -269,6 +270,35 @@ class GoalEvaluationServiceTest { assertEquals(1.0, r.score(), 1e-9); } + @Test + void contradictoryDuplicateVerdictsCannotProduceCompletion() { + stubChatResponse("{\"criterionVerdicts\":[" + + "{\"id\":\"C1\",\"passed\":false,\"evidence\":\"DNS missing\"}," + + "{\"id\":\"C1\",\"passed\":true,\"evidence\":\"DNS claimed ready\"}," + + "{\"id\":\"C2\",\"passed\":true,\"evidence\":\"TLS ready\"}],\"summary\":\"done\"}"); + GoalEvaluationResult result = svc.evaluate(goalWithCriteria(), List.of(), "finished"); + assertFalse(result.completed()); + assertEquals(GoalEvaluationResult.DECISION_FALLBACK, result.decision()); + assertEquals(1, result.llmCallsConsumed()); + assertTrue(result.criterionVerdicts().isEmpty()); + } + + @Test + void duplicateJsonFieldsAreRejectedInVerdictAndBootstrap() { + for (boolean bootstrap : List.of(false, true)) { + stubChatResponse(bootstrap + ? "{\"criteria\":[{\"text\":\"original requirement\",\"text\":\"replacement\"}]}" + : "{\"criterionVerdicts\":[{\"id\":\"C1\",\"passed\":false,\"passed\":true,\"evidence\":\"claim\"}," + + "{\"id\":\"C2\",\"passed\":true,\"evidence\":\"claim\"}]}"); + GoalEvaluationResult result = svc.evaluate(bootstrap ? goal() : goalWithCriteria(), List.of(), "finished"); + assertEquals(GoalEvaluationResult.DECISION_FALLBACK, result.decision()); + assertFalse(result.completed()); + assertEquals(1, result.llmCallsConsumed()); + assertTrue(result.criterionVerdicts().isEmpty()); + assertNull(result.bootstrapCriteria()); + } + } + // ==================== Parser tolerance ==================== @Test