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());