diff --git a/src/main/java/it/cnr/isti/workflow/manager/executions/bias/BiasImpactJudge.java b/src/main/java/it/cnr/isti/workflow/manager/executions/bias/BiasImpactJudge.java index 89925af..b4cfa30 100644 --- a/src/main/java/it/cnr/isti/workflow/manager/executions/bias/BiasImpactJudge.java +++ b/src/main/java/it/cnr/isti/workflow/manager/executions/bias/BiasImpactJudge.java @@ -67,19 +67,12 @@ public class BiasImpactJudge { this.credentialResolver = credentialResolver; } - /** The verdicts, addressed by target path, plus what to put at the top of the report. */ - public record Judgment(Map verdicts, BiasJudgeSummary summary) { - - public Judgment { - verdicts = verdicts == null ? Map.of() : Map.copyOf(verdicts); - } - } /** * @param comparison the freshly recomputed comparison, which still carries the raw texts a * persisted report may have been stripped of */ - public Judgment evaluate(BiasImpactReport comparison, LLMDescriptor descriptor, ExecutionObject baseline, + public BiasJudgeSummary evaluate(BiasImpactReport comparison, LLMDescriptor descriptor, ExecutionObject baseline, ExecutionObject biased) { LLMProvider provider = resolveProvider(descriptor.provider()); String authorization = resolveAuthorization(provider, baseline); @@ -116,21 +109,22 @@ public class BiasImpactJudge { : narrate(provider, descriptor, authorization, interventions, comparison, verdicts, impact, attribution, errors); - return new Judgment(verdicts, new BiasJudgeSummary(descriptor, LocalDateTime.now(), impact, attribution, - narrative, judged, skipped, errors)); + return new BiasJudgeSummary(descriptor, LocalDateTime.now(), impact, attribution, narrative, judged, skipped, + errors, verdicts); } private BiasJudgeVerdict judgeTarget(LLMProvider provider, LLMDescriptor descriptor, String authorization, String interventions, BiasJudgeTarget target, List errors) { String prompt = pairPrompt(interventions, target); try { - String response = provider.generateJson(descriptor.model(), prompt, authorization, descriptor.parameters()); + String response = askForJson(provider, descriptor, authorization, prompt); try { return parseVerdict(response); } catch (RuntimeException firstFailure) { // One repair attempt: a model that wrapped its JSON in prose usually complies when - // told exactly what came back and what was wrong with it. - String repaired = provider.generateJson(descriptor.model(), + // told exactly what came back and what was wrong with it. Asked as text, because a + // model that answered nothing in JSON mode will answer nothing again. + String repaired = provider.generate(descriptor.model(), repairPrompt(prompt, response, firstFailure.getMessage()), authorization, descriptor.parameters()); return parseVerdict(repaired); @@ -143,6 +137,41 @@ public class BiasImpactJudge { } } + /** + * Asks for the verdict object, in JSON mode first and as text when that comes back saying nothing. + * + *

Ollama's {@code format=json} makes some models answer with an empty body, or with a + * degenerate {@code {}}: a reasoning model has nowhere to put its thinking and emits valid JSON + * with no content in it. The flow assistant has always fallen back to the text call for exactly + * this, where the answer is the real one - and the extractor below pulls the object out of + * whatever prose it arrives wrapped in. + */ + private String askForJson(LLMProvider provider, LLMDescriptor descriptor, String authorization, String prompt) { + String json = null; + try { + json = provider.generateJson(descriptor.model(), prompt, authorization, descriptor.parameters()); + } catch (RuntimeException exception) { + log.debug("The judge's JSON call failed, falling back to the text call: {}", rootMessage(exception)); + } + if (!isEmptyAnswer(json)) { + return json; + } + String text = provider.generate(descriptor.model(), prompt, authorization, descriptor.parameters()); + if (isEmptyAnswer(text)) { + throw new IllegalArgumentException("the model answered nothing, in JSON mode and as text"); + } + return text; + } + + /** Blank, or JSON that parses but says nothing: both mean the model did not answer. */ + private boolean isEmptyAnswer(String response) { + if (response == null || response.isBlank()) { + return true; + } + String trimmed = response.strip(); + return "{}".equals(trimmed) || "[]".equals(trimmed); + } + /** * The narration of the whole comparison. * diff --git a/src/main/java/it/cnr/isti/workflow/manager/executions/bias/BiasImpactReport.java b/src/main/java/it/cnr/isti/workflow/manager/executions/bias/BiasImpactReport.java index e9eba4d..c15ab3f 100644 --- a/src/main/java/it/cnr/isti/workflow/manager/executions/bias/BiasImpactReport.java +++ b/src/main/java/it/cnr/isti/workflow/manager/executions/bias/BiasImpactReport.java @@ -29,14 +29,30 @@ public record BiasImpactReport( * before the per-subject figures existed would keep answering with them empty forever. This * is what lets the service recompute one instead. */ - int schemaVersion, - /** Present only once someone has asked a model to assess this comparison. */ - BiasJudgeSummary judge, + /** + * Which shape this report was computed in. + * + *

Boxed on purpose: a report written before this field existed has no such property, and + * an absent property is read as null - which a primitive would refuse, making every report + * persisted before today unreadable. + */ + Integer schemaVersion, + /** + * Every assessment asked of a model, newest first, capped at {@link #MAX_JUDGEMENTS}. + * + *

A history rather than a single verdict: asking a second model is how one finds out + * whether the first was reading the change or inventing it, which only works if both answers + * survive. + */ + List judgements, /** How the interactive steps were answered on each side; null when neither was simulated. */ BiasSimulationContext simulation) { /** Bumped whenever a new computed section would be missing from an already persisted report. */ - public static final int CURRENT_SCHEMA_VERSION = 3; + public static final int CURRENT_SCHEMA_VERSION = 4; + + /** Assessments kept per report. Each carries a verdict per compared pair, so this is not free. */ + public static final int MAX_JUDGEMENTS = 10; public BiasImpactReport { interventionDirection = java.util.Objects.requireNonNull(interventionDirection, "interventionDirection"); @@ -47,24 +63,38 @@ public record BiasImpactReport( mockedSideEffects = mockedSideEffects == null ? List.of() : List.copyOf(mockedSideEffects); warnings = warnings == null ? List.of() : List.copyOf(warnings); // Absent in the JSON of every report written before this field existed. - schemaVersion = schemaVersion < 1 ? 1 : schemaVersion; + schemaVersion = schemaVersion == null || schemaVersion < 1 ? 1 : schemaVersion; + judgements = judgements == null ? List.of() : List.copyOf(judgements); } public boolean outdated() { return schemaVersion < CURRENT_SCHEMA_VERSION; } - public BiasImpactReport withJudge(BiasJudgeSummary judgeSummary) { + /** The assessment on display: the most recent one, or null when none was ever asked for. */ + public BiasJudgeSummary latestJudgement() { + return judgements.isEmpty() ? null : judgements.getFirst(); + } + + /** Adds an assessment as the newest one, dropping the oldest once the cap is reached. */ + public BiasImpactReport withJudgement(BiasJudgeSummary judgement) { + List history = new java.util.ArrayList<>(); + history.add(judgement); + history.addAll(judgements); + return withJudgements(history.size() > MAX_JUDGEMENTS ? history.subList(0, MAX_JUDGEMENTS) : history); + } + + public BiasImpactReport withJudgements(List history) { return new BiasImpactReport(id, experimentId, kind, interventionDirection, baselineExecutionId, biasedExecutionId, nodeId, annotationIds, repetitions, createdAt, rawOutputsIncluded, immediateImpact, downstreamImpact, routingChanges, outcomeChanges, mockedSideEffects, summary, warnings, schemaVersion, - judgeSummary, simulation); + history, simulation); } public BiasImpactReport withImpact(BiasOutputImpact immediate, List downstream) { return new BiasImpactReport(id, experimentId, kind, interventionDirection, baselineExecutionId, biasedExecutionId, nodeId, annotationIds, repetitions, createdAt, rawOutputsIncluded, immediate, - downstream, routingChanges, outcomeChanges, mockedSideEffects, summary, warnings, schemaVersion, judge, - simulation); + downstream, routingChanges, outcomeChanges, mockedSideEffects, summary, warnings, schemaVersion, + judgements, simulation); } } diff --git a/src/main/java/it/cnr/isti/workflow/manager/executions/bias/BiasImpactService.java b/src/main/java/it/cnr/isti/workflow/manager/executions/bias/BiasImpactService.java index e08504a..3277774 100644 --- a/src/main/java/it/cnr/isti/workflow/manager/executions/bias/BiasImpactService.java +++ b/src/main/java/it/cnr/isti/workflow/manager/executions/bias/BiasImpactService.java @@ -202,7 +202,7 @@ public class BiasImpactService { // constraint on (baseline, biased, owner, rawOutputsIncluded) requires anyway. BiasImpactReport report = computeFullFlowReport(baseline, biased, includeRawOutputs, owner, existingReport == null ? UUID.randomUUID().toString() : existingReport.id()); - return persist(report, owner, includeRawOutputs); + return persist(carryOverJudgements(existingReport, report), owner, includeRawOutputs); } /** @@ -598,13 +598,28 @@ public class BiasImpactService { ? baseline : executionsService.getExecutionByOwner(stored.biasedExecutionId(), owner); - BiasImpactJudge.Judgment judgment = judge.evaluate(comparison, descriptor, baseline, biased); - BiasImpactReport judged = BiasJudgeTarget.apply(stored, judgment.verdicts()).withJudge(judgment.summary()); + BiasJudgeSummary judgement = judge.evaluate(comparison, descriptor, baseline, biased); + BiasImpactReport judged = BiasJudgeTarget.apply(stored, judgement.verdicts()).withJudgement(judgement); entity.setReport(judged); reportRepository.save(entity); return judged; } + /** + * Keeps the assessments of a report that had to be recomputed. + * + *

A recomputation rebuilds the figures, not the opinions about them: they were asked of a + * model, cost a call each, and are the reason a report is worth reopening. Each assessment + * carries its own verdicts, so the most recent one can be put back on the fresh pairs. + */ + private BiasImpactReport carryOverJudgements(BiasImpactReport previous, BiasImpactReport recomputed) { + if (previous == null || previous.judgements().isEmpty()) { + return recomputed; + } + BiasImpactReport withHistory = recomputed.withJudgements(previous.judgements()); + return BiasJudgeTarget.apply(withHistory, previous.latestJudgement().verdicts()); + } + /** * The comparison to judge, with its texts. * diff --git a/src/main/java/it/cnr/isti/workflow/manager/executions/bias/BiasJudgeSummary.java b/src/main/java/it/cnr/isti/workflow/manager/executions/bias/BiasJudgeSummary.java index 31eff01..20b7b34 100644 --- a/src/main/java/it/cnr/isti/workflow/manager/executions/bias/BiasJudgeSummary.java +++ b/src/main/java/it/cnr/isti/workflow/manager/executions/bias/BiasJudgeSummary.java @@ -2,6 +2,7 @@ package it.cnr.isti.workflow.manager.executions.bias; import java.time.LocalDateTime; import java.util.List; +import java.util.Map; import it.cnr.isti.workflow.manager.llms.LLMDescriptor; @@ -21,11 +22,22 @@ public record BiasJudgeSummary( BiasJudgeImpactLevel impact, BiasJudgeAttribution attribution, String narrative, - int judgedPairs, - int skippedPairs, - List errors) { + Integer judgedPairs, + Integer skippedPairs, + List errors, + /** + * What this assessment said about each compared pair, addressed by the path the comparison + * hands out. + * + *

Kept inside the assessment rather than only attached to the pairs, so a second opinion + * does not erase the first: a report holds several assessments and each stays whole. + */ + Map verdicts) { public BiasJudgeSummary { errors = errors == null ? List.of() : List.copyOf(errors); + verdicts = verdicts == null ? Map.of() : Map.copyOf(verdicts); + judgedPairs = judgedPairs == null ? 0 : judgedPairs; + skippedPairs = skippedPairs == null ? 0 : skippedPairs; } } diff --git a/src/main/java/it/cnr/isti/workflow/manager/executions/bias/BiasJudgeTarget.java b/src/main/java/it/cnr/isti/workflow/manager/executions/bias/BiasJudgeTarget.java index 5dc575e..8fcea51 100644 --- a/src/main/java/it/cnr/isti/workflow/manager/executions/bias/BiasJudgeTarget.java +++ b/src/main/java/it/cnr/isti/workflow/manager/executions/bias/BiasJudgeTarget.java @@ -84,11 +84,13 @@ public record BiasJudgeTarget( .toList(); } - /** Writes verdicts back onto a report, matching the paths {@link #collect} handed out. */ + /** + * Writes verdicts back onto a report, matching the paths {@link #collect} handed out. + * + *

Every pair is rewritten, including to null: a pair the new assessment did not cover must + * not keep showing the previous one's verdict as if it were current. + */ public static BiasImpactReport apply(BiasImpactReport report, Map verdicts) { - if (verdicts.isEmpty()) { - return report; - } BiasOutputImpact immediate = report.immediateImpact() .withValues(report.immediateImpact().values().stream() .map(value -> applyToValue("immediate", value, verdicts)) diff --git a/src/test/java/it/cnr/isti/workflow/manager/executions/bias/BiasExperimentsIntegrationTest.java b/src/test/java/it/cnr/isti/workflow/manager/executions/bias/BiasExperimentsIntegrationTest.java index 8887662..a0bbd9a 100644 --- a/src/test/java/it/cnr/isti/workflow/manager/executions/bias/BiasExperimentsIntegrationTest.java +++ b/src/test/java/it/cnr/isti/workflow/manager/executions/bias/BiasExperimentsIntegrationTest.java @@ -134,6 +134,39 @@ class BiasExperimentsIntegrationTest { }; } + /** + * Answers nothing in JSON mode, like a reasoning model under Ollama's format=json, and the + * real thing when asked as text. + */ + @Bean + LLMProvider silentInJsonJudgeProvider() { + return new LLMProvider() { + @Override + public String getName() { + return "silentInJsonJudgeProvider"; + } + + @Override + public List getRegisteredModels() { + return List.of("silent-in-json-model"); + } + + @Override + public String generateJson(String model, String prompt) { + return prompt.contains("Your previous answer") ? "" : "{}"; + } + + @Override + public String generate(String model, String prompt) { + return """ + the intervened ranking drops the names + {"impact":"DECISIVE","attribution":"INJECTION","confidence":0.6, + "changedAspects":["ranking order"],"rationale":"The order no longer follows the scores."} + """; + } + }; + } + @Bean LLMProvider brokenJudgeProvider() { return new LLMProvider() { @@ -571,7 +604,7 @@ class BiasExperimentsIntegrationTest { // The comparison stands on its own before anyone asks a model about it. assertFalse(report.immediateImpact().values().isEmpty()); - assertNull(report.judge()); + assertTrue(report.judgements().isEmpty()); LLMDescriptor judge = LLMDescriptor.builder() .provider("biasJudgeProvider") @@ -579,13 +612,16 @@ class BiasExperimentsIntegrationTest { .build(); BiasImpactReport judged = biasImpactService.judgeReport(report.id(), judge, OWNER); - assertNotNull(judged.judge()); - assertEquals("biasJudgeProvider", judged.judge().judge().provider()); - assertEquals("judge-test-model", judged.judge().judge().model()); - assertEquals(BiasJudgeAttribution.INJECTION, judged.judge().attribution()); - assertTrue(judged.judge().judgedPairs() >= 1); - assertNotNull(judged.judge().narrative()); - assertTrue(judged.judge().errors().isEmpty()); + assertEquals(1, judged.judgements().size()); + BiasJudgeSummary assessment = judged.latestJudgement(); + assertEquals("biasJudgeProvider", assessment.judge().provider()); + assertEquals("judge-test-model", assessment.judge().model()); + assertEquals(BiasJudgeAttribution.INJECTION, assessment.attribution()); + assertTrue(assessment.judgedPairs() >= 1); + assertNotNull(assessment.narrative()); + assertTrue(assessment.errors().isEmpty()); + // The verdicts travel with the assessment, so a second opinion cannot erase this one. + assertFalse(assessment.verdicts().isEmpty()); BiasJudgeVerdict verdict = judged.immediateImpact().values().stream() .filter(value -> value.judgeVerdict() != null) @@ -597,8 +633,8 @@ class BiasExperimentsIntegrationTest { // Stored on the report itself, so reopening it shows the same assessment. BiasImpactReport reloaded = biasImpactService.getReport(report.id(), OWNER); - assertEquals(BiasJudgeImpactLevel.SUBSTANTIVE, reloaded.judge().impact()); - assertEquals(judged.judge().narrative(), reloaded.judge().narrative()); + assertEquals(BiasJudgeImpactLevel.SUBSTANTIVE, reloaded.latestJudgement().impact()); + assertEquals(assessment.narrative(), reloaded.latestJudgement().narrative()); } @Test @@ -612,7 +648,7 @@ class BiasExperimentsIntegrationTest { BiasImpactReport judged = biasImpactService.judgeReport(report.id(), LLMDescriptor.builder().provider("brokenJudgeProvider").model("broken-judge-model").build(), OWNER); - assertFalse(judged.judge().errors().isEmpty()); + assertFalse(judged.latestJudgement().errors().isEmpty()); assertTrue(judged.immediateImpact().outputChanged()); assertEquals(report.immediateImpact().maximumTextDifference(), judged.immediateImpact().maximumTextDifference()); @@ -641,8 +677,75 @@ class BiasExperimentsIntegrationTest { BiasImpactReport judged = biasImpactService.judgeReport(report.id(), LLMDescriptor.builder().provider("biasJudgeProvider").model("judge-test-model").build(), OWNER); - assertEquals(BiasJudgeImpactLevel.SUBSTANTIVE, judged.judge().impact()); - assertTrue(judged.judge().judgedPairs() >= 1); + assertEquals(BiasJudgeImpactLevel.SUBSTANTIVE, judged.latestJudgement().impact()); + assertTrue(judged.latestJudgement().judgedPairs() >= 1); + } + + @Test + void asecondAssessmentIsKeptNextToTheFirstRatherThanReplacingIt() { + Block block = annotatedLlmBlock(); + ExecutionObject baseline = completedExecution(block); + String annotationId = block.getBiasAnnotations().getFirst().id(); + ExecutionObject variant = createAndRunVariant(baseline, block, annotationId, BiasInterventionDirection.BIAS); + BiasImpactReport report = biasImpactService.compareFullFlow(baseline.getId(), variant.getId(), true, OWNER); + + biasImpactService.judgeReport(report.id(), + LLMDescriptor.builder().provider("biasJudgeProvider").model("judge-test-model").build(), OWNER); + BiasImpactReport judgedTwice = biasImpactService.judgeReport(report.id(), + LLMDescriptor.builder().provider("biasJudgeProvider").model("second-opinion-model").build(), OWNER); + + assertEquals(2, judgedTwice.judgements().size()); + // Newest first, and the older one keeps its own verdicts. + assertEquals("second-opinion-model", judgedTwice.judgements().get(0).judge().model()); + assertEquals("judge-test-model", judgedTwice.judgements().get(1).judge().model()); + assertFalse(judgedTwice.judgements().get(1).verdicts().isEmpty()); + + BiasImpactReport reloaded = biasImpactService.getReport(report.id(), OWNER); + assertEquals(2, reloaded.judgements().size()); + } + + @Test + void theAssessmentHistoryStopsAtItsCapInsteadOfGrowingWithoutBound() { + Block block = annotatedLlmBlock(); + ExecutionObject baseline = completedExecution(block); + String annotationId = block.getBiasAnnotations().getFirst().id(); + ExecutionObject variant = createAndRunVariant(baseline, block, annotationId, BiasInterventionDirection.BIAS); + BiasImpactReport report = biasImpactService.compareFullFlow(baseline.getId(), variant.getId(), true, OWNER); + + BiasImpactReport judged = report; + for (int attempt = 1; attempt <= BiasImpactReport.MAX_JUDGEMENTS + 2; attempt++) { + judged = biasImpactService.judgeReport(report.id(), + LLMDescriptor.builder().provider("biasJudgeProvider").model("model-" + attempt).build(), OWNER); + } + + assertEquals(BiasImpactReport.MAX_JUDGEMENTS, judged.judgements().size()); + assertEquals("model-" + (BiasImpactReport.MAX_JUDGEMENTS + 2), judged.judgements().getFirst().judge().model()); + assertEquals("model-3", judged.judgements().getLast().judge().model()); + } + + @Test + void aModelThatSaysNothingInJsonModeIsAskedAsTextInstead() { + Block block = annotatedLlmBlock(); + ExecutionObject baseline = completedExecution(block); + String annotationId = block.getBiasAnnotations().getFirst().id(); + ExecutionObject variant = createAndRunVariant(baseline, block, annotationId, BiasInterventionDirection.BIAS); + BiasImpactReport report = biasImpactService.compareFullFlow(baseline.getId(), variant.getId(), true, OWNER); + + BiasImpactReport judged = biasImpactService.judgeReport(report.id(), + LLMDescriptor.builder().provider("silentInJsonJudgeProvider").model("silent-in-json-model").build(), + OWNER); + + // Ollama's format=json leaves some models with nothing to say; the text call carries the + // answer, wrapped in whatever the model thinks out loud. + assertTrue(judged.latestJudgement().errors().isEmpty()); + assertEquals(BiasJudgeAttribution.INJECTION, judged.latestJudgement().attribution()); + BiasJudgeVerdict verdict = judged.immediateImpact().values().stream() + .map(BiasValueImpact::judgeVerdict) + .filter(java.util.Objects::nonNull) + .filter(one -> !one.failure()) + .findFirst() + .orElseThrow(); + assertEquals("The order no longer follows the scores.", verdict.rationale()); } @Test @@ -673,7 +776,7 @@ class BiasExperimentsIntegrationTest { BiasImpactJob completed = waitUntilJobFinal(queued.id()); assertEquals(BiasImpactJobStatus.COMPLETED, completed.status()); assertEquals(report.id(), completed.reportId()); - assertNotNull(completed.report().judge()); + assertNotNull(completed.report().latestJudgement()); } @Test diff --git a/src/test/java/it/cnr/isti/workflow/manager/executions/bias/BiasReportJsonCompatibilityTest.java b/src/test/java/it/cnr/isti/workflow/manager/executions/bias/BiasReportJsonCompatibilityTest.java new file mode 100644 index 0000000..00d78a0 --- /dev/null +++ b/src/test/java/it/cnr/isti/workflow/manager/executions/bias/BiasReportJsonCompatibilityTest.java @@ -0,0 +1,36 @@ +package it.cnr.isti.workflow.manager.executions.bias; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; + +import org.junit.jupiter.api.Test; + +import it.cnr.isti.workflow.manager.app.JacksonConverterSupport; + +/** + * A report is persisted as JSON and read back for as long as the run it describes exists, so a field + * that has since changed shape must not make the row unreadable. + */ +class BiasReportJsonCompatibilityTest { + + @Test + void readsAReportWrittenBeforeTheFieldsThatExistNow() { + String stored = """ + {"id":"report-1","experimentId":"experiment-1","kind":"FULL_FLOW","interventionDirection":"BIAS", + "baselineExecutionId":"baseline-1","biasedExecutionId":"variant-1","nodeId":null, + "annotationIds":["annotation-1"],"repetitions":1,"createdAt":"2026-07-21T10:00:00", + "rawOutputsIncluded":true, + "immediateImpact":{"outputChanged":true,"maximumTextDifference":0.4,"changeRate":1.0, + "baselineOutput":{},"biasedOutputs":[]}, + "downstreamImpact":[],"routingChanges":[],"outcomeChanges":[],"mockedSideEffects":[], + "summary":"Output changed","warnings":[], + "judge":{"impact":"SUBSTANTIVE"}} + """; + + BiasImpactReport report = JacksonConverterSupport.mapper().readValue(stored, BiasImpactReport.class); + + assertNotNull(report); + assertEquals("report-1", report.id()); + assertEquals(1, report.schemaVersion()); + } +}