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 <noreply@anthropic.com>
This commit is contained in:
parent
058918f4d7
commit
ed128ac014
|
|
@ -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);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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());
|
||||
|
|
|
|||
|
|
@ -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<String, LLMProvider> llmProviders;
|
||||
private final LLMCredentialResolver credentialResolver;
|
||||
private final UserSecretService userSecretService;
|
||||
|
||||
public BiasImpactJudge(Map<String, LLMProvider> llmProviders, LLMCredentialResolver credentialResolver) {
|
||||
public BiasImpactJudge(Map<String, LLMProvider> 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<BiasJudgeTarget> 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.
|
||||
*
|
||||
* <p>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.
|
||||
* <p>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());
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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) {
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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<String> 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<LLMBlockType> 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<LLMBlockType> 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<LLMBlockType> 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());
|
||||
|
|
|
|||
Loading…
Reference in New Issue