From ed128ac014b6f4252249681ac6edb8dc169cfb93 Mon Sep 17 00:00:00 2001 From: Lucio Lelii Date: Thu, 17 Sep 2026 14:42:51 +0200 Subject: [PATCH] Let the bias judge take its own credential, not just inherit one BiasJudgeRequest carried no way to supply a credential: a judge whose provider required one only ever worked by coincidence, when the baseline execution it judges happened to already carry a saved credential for that same provider. There was nowhere to pick a credential for the judging itself, unlike the interaction simulator and the assistant. BiasJudgeRequest now carries an optional credentialId, mirroring AssistantLlmSelection and the simulator's own field from the previous commit. It flows through the whole asynchronous path - BiasExperimentsController, BiasImpactJobService.createJudgeJob (a new judge_credential_id column on BiasImpactJobEntity, so a job recovered after a restart keeps it), BiasImpactService.judgeReport - down to BiasImpactJudge.resolveAuthorization, which resolves it via UserSecretService.resolveCredential when present and falls back to the existing baseline-authorizations lookup otherwise, unchanged. Co-Authored-By: Claude Sonnet 5 --- .../BiasExperimentsController.java | 3 +- .../executions/bias/BiasImpactJobService.java | 6 +- .../executions/bias/BiasImpactJudge.java | 44 +++++-- .../executions/bias/BiasImpactService.java | 4 +- .../executions/bias/BiasJudgeRequest.java | 9 +- .../bias/persistence/BiasImpactJobEntity.java | 4 + .../bias/BiasExperimentsIntegrationTest.java | 112 ++++++++++++++++-- 7 files changed, 156 insertions(+), 26 deletions(-) diff --git a/src/main/java/it/cnr/isti/workflow/manager/controllers/BiasExperimentsController.java b/src/main/java/it/cnr/isti/workflow/manager/controllers/BiasExperimentsController.java index 9ebee8f..0c95cfc 100644 --- a/src/main/java/it/cnr/isti/workflow/manager/controllers/BiasExperimentsController.java +++ b/src/main/java/it/cnr/isti/workflow/manager/controllers/BiasExperimentsController.java @@ -108,7 +108,8 @@ public class BiasExperimentsController { @PathVariable String reportId, @RequestBody @Valid BiasJudgeRequest request, @AuthenticationPrincipal LoginEntity userDetails) { - BiasImpactJob job = biasImpactJobService.createJudgeJob(reportId, request.judge(), owner(userDetails)); + BiasImpactJob job = biasImpactJobService.createJudgeJob(reportId, request.judge(), request.credentialId(), + owner(userDetails)); return ResponseEntity.accepted().body(job); } diff --git a/src/main/java/it/cnr/isti/workflow/manager/executions/bias/BiasImpactJobService.java b/src/main/java/it/cnr/isti/workflow/manager/executions/bias/BiasImpactJobService.java index 351d36b..c50c3f1 100644 --- a/src/main/java/it/cnr/isti/workflow/manager/executions/bias/BiasImpactJobService.java +++ b/src/main/java/it/cnr/isti/workflow/manager/executions/bias/BiasImpactJobService.java @@ -56,7 +56,7 @@ public class BiasImpactJobService { * validated here so that a report that cannot be judged is refused now, rather than by a job * that fails a minute later. */ - public BiasImpactJob createJudgeJob(String reportId, LLMDescriptor descriptor, String owner) { + public BiasImpactJob createJudgeJob(String reportId, LLMDescriptor descriptor, String credentialId, String owner) { BiasImpactReport report = impactService.getReport(reportId, owner); BiasImpactJobEntity entity = repository.save(BiasImpactJobEntity.builder() .id(UUID.randomUUID().toString()) @@ -65,6 +65,7 @@ public class BiasImpactJobService { .executionId(report.baselineExecutionId()) .reportTargetId(reportId) .judge(descriptor) + .judgeCredentialId(credentialId) .status(BiasImpactJobStatus.QUEUED) .createdAt(LocalDateTime.now()) .build()); @@ -111,7 +112,8 @@ public class BiasImpactJobService { repository.save(entity); try { BiasImpactReport report = entity.getKind() == BiasImpactJobKind.REPORT_JUDGE - ? impactService.judgeReport(entity.getReportTargetId(), entity.getJudge(), entity.getOwner()) + ? impactService.judgeReport(entity.getReportTargetId(), entity.getJudge(), + entity.getJudgeCredentialId(), entity.getOwner()) : impactService.runIsolatedStepExperiment( entity.getExecutionId(), entity.getStepId(), entity.getRequest(), entity.getOwner()); entity.setReportId(report.id()); 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 10afd7c..2455c14 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 @@ -16,6 +16,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.http.HttpStatus; import org.springframework.stereotype.Component; +import org.springframework.web.server.ResponseStatusException; import it.cnr.isti.workflow.manager.app.JacksonConverterSupport; import it.cnr.isti.workflow.manager.blocks.Block; @@ -32,6 +33,7 @@ import it.cnr.isti.workflow.manager.llms.LLMCredentialResolver; import it.cnr.isti.workflow.manager.llms.LLMDescriptor; import it.cnr.isti.workflow.manager.llms.ProviderCredential; import it.cnr.isti.workflow.manager.llms.providers.LLMProvider; +import it.cnr.isti.workflow.manager.vault.UserSecretService; import tools.jackson.databind.JsonNode; /** @@ -66,10 +68,13 @@ public class BiasImpactJudge { private final Map llmProviders; private final LLMCredentialResolver credentialResolver; + private final UserSecretService userSecretService; - public BiasImpactJudge(Map llmProviders, LLMCredentialResolver credentialResolver) { + public BiasImpactJudge(Map llmProviders, LLMCredentialResolver credentialResolver, + UserSecretService userSecretService) { this.llmProviders = llmProviders; this.credentialResolver = credentialResolver; + this.userSecretService = userSecretService; } @@ -77,10 +82,10 @@ public class BiasImpactJudge { * @param comparison the freshly recomputed comparison, which still carries the raw texts a * persisted report may have been stripped of */ - public BiasJudgeSummary evaluate(BiasImpactReport comparison, LLMDescriptor descriptor, ExecutionObject baseline, - ExecutionObject biased) { + public BiasJudgeSummary evaluate(BiasImpactReport comparison, LLMDescriptor descriptor, String credentialId, + ExecutionObject baseline, ExecutionObject biased) { LLMProvider provider = resolveProvider(descriptor.provider()); - ProviderCredential authorization = resolveAuthorization(provider, baseline); + ProviderCredential authorization = resolveAuthorization(provider, credentialId, baseline); String interventions = describeInterventions(biased, comparison.annotationIds()); List targets = BiasJudgeTarget.collect(comparison); @@ -499,13 +504,34 @@ public class BiasImpactJudge { } /** - * The credential the judged run itself used. + * The judge's own credential, when the caller picked one; otherwise the credential the judged + * run itself used. * - *

Taken from the baseline execution rather than asked for again: the model that assesses a - * run is reached the same way as the models that produced it, and the internal provider - the - * one a local install has - needs no credential at all. + *

An explicit {@code credentialId} lets a judge be chosen on its own terms, rather than only + * working when it happens to reuse a provider the baseline execution already has a saved + * credential for - the same reasoning behind the interaction simulator's own credentialId (see + * {@code ExecutionsService.resolveSimulatorAuthorization}). Falling back to the baseline's own + * authorizations keeps the coincidental case working unchanged, and the internal provider - the + * one a local install has - needs no credential at all either way. */ - private ProviderCredential resolveAuthorization(LLMProvider provider, ExecutionObject baseline) { + private ProviderCredential resolveAuthorization(LLMProvider provider, String credentialId, ExecutionObject baseline) { + if (!provider.requiresAuthorization()) { + return null; + } + String trimmedCredentialId = credentialId == null ? "" : credentialId.trim(); + if (!trimmedCredentialId.isEmpty()) { + if (baseline.getOwner() == null || baseline.getOwner().isBlank()) { + throw new BiasApiException(HttpStatus.BAD_REQUEST, ValidationErrorCode.BIAS_JUDGE_UNAVAILABLE, + "judge", provider.getName(), + "A user-owned execution is required to select a credential for provider " + provider.getName()); + } + try { + return userSecretService.resolveCredential(baseline.getOwner(), trimmedCredentialId, provider.getName()); + } catch (ResponseStatusException exception) { + throw new BiasApiException(HttpStatus.BAD_REQUEST, ValidationErrorCode.BIAS_JUDGE_UNAVAILABLE, + "judge", provider.getName(), exception.getReason()); + } + } try { return credentialResolver.resolve(provider, baseline.getProvidedAuthorizations(), baseline.getContext().getResolvedExecutionVariables()); 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 e1fabb8..2562001 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 @@ -593,7 +593,7 @@ public class BiasImpactService { * and that only works if both answers survive. The descriptor and timestamp on each say which * model produced it. */ - public BiasImpactReport judgeReport(String reportId, LLMDescriptor descriptor, String owner) { + public BiasImpactReport judgeReport(String reportId, LLMDescriptor descriptor, String credentialId, String owner) { BiasImpactReport stored = getReport(reportId, owner); BiasImpactReport comparison = comparisonForJudging(stored, owner); @@ -606,7 +606,7 @@ public class BiasImpactService { // of it, and what comes back is written by judgementStore against the report as it is then - // not against the copy read above, which by that point may be missing another assessment // that finished in the meantime. - BiasJudgeSummary judgement = judge.evaluate(comparison, descriptor, baseline, biased); + BiasJudgeSummary judgement = judge.evaluate(comparison, descriptor, credentialId, baseline, biased); return judgementStore.append(reportId, owner, judgement); } diff --git a/src/main/java/it/cnr/isti/workflow/manager/executions/bias/BiasJudgeRequest.java b/src/main/java/it/cnr/isti/workflow/manager/executions/bias/BiasJudgeRequest.java index 9941c7d..e090359 100644 --- a/src/main/java/it/cnr/isti/workflow/manager/executions/bias/BiasJudgeRequest.java +++ b/src/main/java/it/cnr/isti/workflow/manager/executions/bias/BiasJudgeRequest.java @@ -15,5 +15,12 @@ import jakarta.validation.constraints.NotNull; * picked when the judging is asked for, the same way the model that stands in for a person is picked * when a simulated run is launched, and the editor reuses the same provider and model pickers. */ -public record BiasJudgeRequest(@Valid @NotNull LLMDescriptor judge) { +public record BiasJudgeRequest( + @Valid @NotNull LLMDescriptor judge, + /** + * A vault secret id, required only when the judge's provider needs one. Without it, the + * judge falls back to whatever credential the baseline execution already carries for the + * same provider - the coincidental case that has always worked. + */ + String credentialId) { } diff --git a/src/main/java/it/cnr/isti/workflow/manager/executions/bias/persistence/BiasImpactJobEntity.java b/src/main/java/it/cnr/isti/workflow/manager/executions/bias/persistence/BiasImpactJobEntity.java index 9176c27..1414bcc 100644 --- a/src/main/java/it/cnr/isti/workflow/manager/executions/bias/persistence/BiasImpactJobEntity.java +++ b/src/main/java/it/cnr/isti/workflow/manager/executions/bias/persistence/BiasImpactJobEntity.java @@ -87,4 +87,8 @@ public class BiasImpactJobEntity { @Column(name = "judge_data", columnDefinition = "TEXT") @Convert(converter = LLMDescriptorConverter.class) private LLMDescriptor judge; + + /** A vault secret id for the judge's provider, when the caller picked one explicitly. */ + @Column(name = "judge_credential_id") + private String judgeCredentialId; } 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 6614b82..c69acd2 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 @@ -229,6 +229,45 @@ class BiasExperimentsIntegrationTest { } }; } + + /** Requires a credential, and echoes back whatever it was called with, so a test can prove it. */ + @Bean + LLMProvider credentialRequiringJudgeProvider() { + return new LLMProvider() { + @Override + public String getName() { + return "credentialRequiringJudgeProvider"; + } + + @Override + public List getRegisteredModels() { + return List.of(); + } + + @Override + public boolean requiresAuthorization() { + return true; + } + + @Override + public String generate(String model, String prompt) { + throw new UnsupportedOperationException("not exercised without a credential"); + } + + @Override + public String generate(String model, String prompt, String authorization) { + return "used " + authorization; + } + + @Override + public String generateJson(String model, String prompt, String authorization) { + return """ + {"impact":"SUBSTANTIVE","attribution":"INJECTION","confidence":0.7, + "changedAspects":["conclusion"],"rationale":"used %s"} + """.formatted(authorization); + } + }; + } } @Autowired @@ -270,6 +309,9 @@ class BiasExperimentsIntegrationTest { @Autowired HumanDecisionBlockFactory humanDecisionBlockFactory; + @Autowired + it.cnr.isti.workflow.manager.vault.UserSecretService userSecretService; + @Test void discoveryExposesBehavioralProbeAndPerTypeCapabilities() { var descriptor = annotationsController.getDescriptor(); @@ -653,7 +695,7 @@ class BiasExperimentsIntegrationTest { .provider("biasJudgeProvider") .model("judge-test-model") .build(); - BiasImpactReport judged = biasImpactService.judgeReport(report.id(), judge, OWNER); + BiasImpactReport judged = biasImpactService.judgeReport(report.id(), judge, null, OWNER); assertEquals(1, judged.judgements().size()); BiasJudgeSummary assessment = judged.latestJudgement(); @@ -689,7 +731,7 @@ class BiasExperimentsIntegrationTest { BiasImpactReport report = biasImpactService.compareFullFlow(baseline.getId(), variant.getId(), true, OWNER); BiasImpactReport judged = biasImpactService.judgeReport(report.id(), - LLMDescriptor.builder().provider("brokenJudgeProvider").model("broken-judge-model").build(), OWNER); + LLMDescriptor.builder().provider("brokenJudgeProvider").model("broken-judge-model").build(), null, OWNER); assertFalse(judged.latestJudgement().errors().isEmpty()); assertTrue(judged.immediateImpact().outputChanged()); @@ -718,7 +760,7 @@ class BiasExperimentsIntegrationTest { OWNER); BiasImpactReport judged = biasImpactService.judgeReport(report.id(), - LLMDescriptor.builder().provider("biasJudgeProvider").model("judge-test-model").build(), OWNER); + LLMDescriptor.builder().provider("biasJudgeProvider").model("judge-test-model").build(), null, OWNER); assertEquals(BiasJudgeImpactLevel.SUBSTANTIVE, judged.latestJudgement().impact()); assertTrue(judged.latestJudgement().judgedPairs() >= 1); @@ -733,9 +775,9 @@ class BiasExperimentsIntegrationTest { BiasImpactReport report = biasImpactService.compareFullFlow(baseline.getId(), variant.getId(), true, OWNER); biasImpactService.judgeReport(report.id(), - LLMDescriptor.builder().provider("biasJudgeProvider").model("judge-test-model").build(), OWNER); + LLMDescriptor.builder().provider("biasJudgeProvider").model("judge-test-model").build(), null, OWNER); BiasImpactReport judgedTwice = biasImpactService.judgeReport(report.id(), - LLMDescriptor.builder().provider("biasJudgeProvider").model("second-opinion-model").build(), OWNER); + LLMDescriptor.builder().provider("biasJudgeProvider").model("second-opinion-model").build(), null, OWNER); assertEquals(2, judgedTwice.judgements().size()); // Newest first, and the older one keeps its own verdicts. @@ -761,9 +803,9 @@ class BiasExperimentsIntegrationTest { java.util.concurrent.ExecutorService pool = java.util.concurrent.Executors.newFixedThreadPool(2); try { var first = pool.submit(() -> biasImpactService.judgeReport(report.id(), - LLMDescriptor.builder().provider("overlappingJudgeProvider").model("model-one").build(), OWNER)); + LLMDescriptor.builder().provider("overlappingJudgeProvider").model("model-one").build(), null, OWNER)); var second = pool.submit(() -> biasImpactService.judgeReport(report.id(), - LLMDescriptor.builder().provider("overlappingJudgeProvider").model("model-two").build(), OWNER)); + LLMDescriptor.builder().provider("overlappingJudgeProvider").model("model-two").build(), null, OWNER)); first.get(30, java.util.concurrent.TimeUnit.SECONDS); second.get(30, java.util.concurrent.TimeUnit.SECONDS); } finally { @@ -788,7 +830,7 @@ class BiasExperimentsIntegrationTest { 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); + LLMDescriptor.builder().provider("biasJudgeProvider").model("model-" + attempt).build(), null, OWNER); } assertEquals(BiasImpactReport.MAX_JUDGEMENTS, judged.judgements().size()); @@ -806,7 +848,7 @@ class BiasExperimentsIntegrationTest { BiasImpactReport judged = biasImpactService.judgeReport(report.id(), LLMDescriptor.builder().provider("silentInJsonJudgeProvider").model("silent-in-json-model").build(), - OWNER); + null, 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. @@ -830,10 +872,58 @@ class BiasExperimentsIntegrationTest { BiasImpactReport report = biasImpactService.compareFullFlow(baseline.getId(), variant.getId(), true, OWNER); BiasApiException exception = assertThrows(BiasApiException.class, () -> biasImpactService.judgeReport( - report.id(), LLMDescriptor.builder().provider("nowhere").model("nothing").build(), OWNER)); + report.id(), LLMDescriptor.builder().provider("nowhere").model("nothing").build(), null, OWNER)); assertEquals(ValidationErrorCode.BIAS_JUDGE_PROVIDER_NOT_FOUND.name(), exception.getErrorCode()); } + @Test + void judgeReportAcceptsAnExplicitCredentialForTheJudge() { + // The gap this closes: before, a judge whose provider needs a credential could only ever + // work by coincidence, if the baseline execution already happened to carry one for it - there + // was nowhere to supply one just for the judging. + it.cnr.isti.workflow.manager.vault.model.VaultSecretView secret = userSecretService.create(OWNER, + new it.cnr.isti.workflow.manager.vault.model.VaultSecretCreateRequest( + "judge-cred-" + java.util.UUID.randomUUID(), "credentialRequiringJudgeProvider", null, + "judge-secret-value", null)); + 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("credentialRequiringJudgeProvider").model("m").build(), secret.id(), + OWNER); + + // Proves the credential's own value reached the provider, not just that no exception was + // thrown - the fake provider echoes back whatever "authorization" it was called with. + BiasJudgeVerdict verdict = judged.immediateImpact().values().stream() + .map(BiasValueImpact::judgeVerdict) + .filter(java.util.Objects::nonNull) + .filter(one -> !one.failure()) + .findFirst() + .orElseThrow(); + assertEquals("used judge-secret-value", verdict.rationale()); + } + + @Test + void judgeReportRejectsAnUnusableExplicitCredential() { + it.cnr.isti.workflow.manager.vault.model.VaultSecretView secret = userSecretService.create("someone-else", + new it.cnr.isti.workflow.manager.vault.model.VaultSecretCreateRequest( + "not-mine-" + java.util.UUID.randomUUID(), "credentialRequiringJudgeProvider", null, + "judge-secret-value", null)); + 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); + + BiasApiException exception = assertThrows(BiasApiException.class, () -> biasImpactService.judgeReport( + report.id(), LLMDescriptor.builder().provider("credentialRequiringJudgeProvider").model("m").build(), + secret.id(), OWNER)); + assertEquals(ValidationErrorCode.BIAS_JUDGE_UNAVAILABLE.name(), exception.getErrorCode()); + } + @Test void anAssessmentAskedForThroughAJobEndsUpOnTheReportItPolled() { Block block = annotatedLlmBlock(); @@ -843,7 +933,7 @@ class BiasExperimentsIntegrationTest { BiasImpactReport report = biasImpactService.compareFullFlow(baseline.getId(), variant.getId(), true, OWNER); BiasImpactJob queued = biasImpactJobService.createJudgeJob(report.id(), - LLMDescriptor.builder().provider("biasJudgeProvider").model("judge-test-model").build(), OWNER); + LLMDescriptor.builder().provider("biasJudgeProvider").model("judge-test-model").build(), null, OWNER); assertEquals(BiasImpactJobKind.REPORT_JUDGE, queued.kind()); BiasImpactJob completed = waitUntilJobFinal(queued.id());