From 0fe5a0109c324edf3ec29d0e4ee84ab0d5de89fd Mon Sep 17 00:00:00 2001 From: Lucio Lelii Date: Thu, 3 Sep 2026 11:40:33 +0200 Subject: [PATCH] Read the bias direction from the fields the API actually sends The direction badge always said "Bias". It derived the direction from biasExecutionContext.activeBiasProbes, and that field is not on the bias context at all - probes belong to a step view - so the lookup was always undefined and every variant fell through to the bias branch. My own change one commit ago. A persisted snapshot settles what the payload really is: bias and mitigation are tracked in four separate collections - annotations per node and activated subflows per container, one pair for each direction. The direction is which of them are non-empty, so a variant can now correctly read as Bias, Mitigation, or Bias + mitigation, including when an intervention was switched on for a whole container subflow rather than per annotation. A variant with nothing recorded says "unspecified" instead of guessing. The model was wrong in a second way, with a second victim: activeAnnotationIdsByNode is not sent either, so the bias highlighting on graph nodes read an absent field and never lit anything up. It now combines both directions. The model is corrected to the real shape and the two absent fields are gone from it, so nothing can quietly read undefined again. The derivation lives in pure, tested helpers rather than inline, and the dev fake now splits activations by direction like the real payload - flattening them hid this very difference in development. Co-Authored-By: Claude Opus 5 (1M context) --- .../tasks-executor/tasks-executor.spec.ts | 65 +++++++++-- .../layouts/tasks-executor/tasks-executor.ts | 11 +- src/app/models/bias-impact.spec.ts | 101 ++++++++++++++++-- src/app/models/bias-impact.ts | 66 ++++++++++-- .../task-executions-call.fake.ts | 17 ++- .../task-execution-viewer.html | 4 +- .../task-execution-viewer.ts | 22 ++-- 7 files changed, 240 insertions(+), 46 deletions(-) diff --git a/src/app/layouts/tasks-executor/tasks-executor.spec.ts b/src/app/layouts/tasks-executor/tasks-executor.spec.ts index c79a67c..e8bbe3e 100644 --- a/src/app/layouts/tasks-executor/tasks-executor.spec.ts +++ b/src/app/layouts/tasks-executor/tasks-executor.spec.ts @@ -14,6 +14,24 @@ import { vi } from 'vitest'; import { findInteractiveSubflowTargets, TasksExecutor } from './tasks-executor'; +/** The shape the API actually sends: bias and mitigation live in separate collections. */ +function normalBias(): any { + return { + experimentId: null, + mode: 'NORMAL', + activeBiasAnnotationIdsByNode: {}, + activeMitigationAnnotationIdsByNode: {}, + biasSubflowActivatedContainerIds: [], + mitigationSubflowActivatedContainerIds: [], + externalSideEffectPolicy: 'BLOCK', + externalSideEffectsConfirmed: false + }; +} + +function variantBias(overrides: Record): any { + return { ...normalBias(), mode: 'BIAS_VARIANT', experimentId: 'x', ...overrides }; +} + describe('TasksExecutor', () => { let component: TasksExecutor; let fixture: ComponentFixture; @@ -247,16 +265,13 @@ describe('TasksExecutor', () => { // marked everything a variant. Only the mode distinguishes them. const plain = { id: 'e1', name: 'Plain', creationTime: 1, runNumber: 1, - biasExecutionContext: { mode: 'NORMAL', experimentId: '', activeAnnotationIdsByNode: {} }, + biasExecutionContext: normalBias(), context: { inputs: {}, result: {}, errors: {}, warnings: {}, status: 'SUCCESS', waitingSteps: [], steps: {} } }; const rerun = { ...plain, id: 'e2', name: 'Rerun', runNumber: 2, rerunOfExecutionId: 'e1' }; const variant = { ...plain, id: 'e3', name: 'Variant', runNumber: 3, rerunOfExecutionId: 'e1', - biasExecutionContext: { - mode: 'BIAS_VARIANT', experimentId: 'x', activeAnnotationIdsByNode: {}, - activeBiasProbes: [{ annotationId: 'a1', direction: 'BIAS', activationMode: 'PROMPT_DIRECTIVE' }] - } + biasExecutionContext: variantBias({ activeBiasAnnotationIdsByNode: { n1: ['a1'] } }) }; taskExecutions.set([]); @@ -278,10 +293,7 @@ describe('TasksExecutor', () => { it('reports a mitigation-only variant as such', () => { const variant = { id: 'e1', name: 'Mitigated', creationTime: 1, runNumber: 1, - biasExecutionContext: { - mode: 'BIAS_VARIANT', experimentId: 'x', activeAnnotationIdsByNode: {}, - activeBiasProbes: [{ annotationId: 'a1', direction: 'MITIGATION', activationMode: 'PROMPT_DIRECTIVE' }] - }, + biasExecutionContext: variantBias({ activeMitigationAnnotationIdsByNode: { n1: ['a1'] } }), context: { inputs: {}, result: {}, errors: {}, warnings: {}, status: 'SUCCESS', waitingSteps: [], steps: {} } }; @@ -292,4 +304,39 @@ describe('TasksExecutor', () => { expect(rows[0].biasDirection).toBe('MITIGATION'); }); + + it('reports a variant that ran with both directions as mixed', () => { + const both = { + id: 'e1', name: 'Both', creationTime: 1, runNumber: 1, rerunOfExecutionId: 'e0', + biasExecutionContext: variantBias({ + activeBiasAnnotationIdsByNode: { n1: ['a1'] }, + activeMitigationAnnotationIdsByNode: { n2: ['a2'] } + }), + context: { inputs: {}, result: {}, errors: {}, warnings: {}, status: 'SUCCESS', waitingSteps: [], steps: {} } + }; + + const rows = (component as any).toGroupListItem({ + id: 'g1', sourceFlowId: 'f1', name: 'G', firstExecutionId: 'e1', latestExecutionId: 'e1', + creationTime: 1, lastExecutionTime: 1, executionCount: 1, executions: [both] + } as any).executions; + + expect(rows[0].kind).toBe('BIAS_VARIANT'); + expect(rows[0].biasDirection).toBe('MIXED'); + }); + + it('detects a direction activated only through a container subflow', () => { + // An intervention can be turned on for a whole subflow instead of per annotation. + const viaSubflow = { + id: 'e1', name: 'Subflow mitigation', creationTime: 1, runNumber: 1, + biasExecutionContext: variantBias({ mitigationSubflowActivatedContainerIds: ['c1'] }), + context: { inputs: {}, result: {}, errors: {}, warnings: {}, status: 'SUCCESS', waitingSteps: [], steps: {} } + }; + + const rows = (component as any).toGroupListItem({ + id: 'g1', sourceFlowId: 'f1', name: 'G', firstExecutionId: 'e1', latestExecutionId: 'e1', + creationTime: 1, lastExecutionTime: 1, executionCount: 1, executions: [viaSubflow] + } as any).executions; + + expect(rows[0].biasDirection).toBe('MITIGATION'); + }); }); diff --git a/src/app/layouts/tasks-executor/tasks-executor.ts b/src/app/layouts/tasks-executor/tasks-executor.ts index 6cbafdc..7343f9d 100644 --- a/src/app/layouts/tasks-executor/tasks-executor.ts +++ b/src/app/layouts/tasks-executor/tasks-executor.ts @@ -23,6 +23,7 @@ import { } from '@shared/tasks-executions-list/tasks-executions-list'; import { TaskExecutionViewerComponent } from '@shared/task-execution-viewer/task-execution-viewer'; import { MatTooltipModule } from '@angular/material/tooltip'; +import { biasInterventionMix, isBiasVariantContext } from '@models/bias-impact'; import { ExecutionTreeComponent, ExecutionTreeSelection, executionTreeHasContent } from '@shared/execution-tree/execution-tree'; import { formatDuration } from '@shared/task-execution-viewer/execution-viewer.utils'; import { BlocksService } from '@services/blocks/blocks'; @@ -331,8 +332,8 @@ export class TasksExecutor { private toExecutionListItem(execution: TaskExecution, fallbackRunNumber: number, runNumbers: Map = new Map()): TaskExecutionListItem { const bias = execution.biasExecutionContext; - const isBiasVariant = bias?.mode === 'BIAS_VARIANT'; - const directions = new Set((bias?.activeBiasProbes ?? []).map((probe) => probe.direction)); + const isBiasVariant = isBiasVariantContext(bias); + const mix = biasInterventionMix(bias); const rerunOf = execution.rerunOfExecutionId ?? null; return { @@ -348,11 +349,7 @@ export class TasksExecutor { // A bias variant is reported as such even when it is also a rerun: that it carries probes is // the thing that changes how its result should be read. kind: isBiasVariant ? 'BIAS_VARIANT' : (rerunOf ? 'RERUN' : 'RUN'), - biasDirection: !isBiasVariant - ? null - : directions.size > 1 - ? 'MIXED' - : directions.has('MITIGATION') ? 'MITIGATION' : 'BIAS', + biasDirection: mix, duration: this.formatExecutionDuration(execution.context.startTime ?? null, execution.context.endTime ?? null), simulated: execution.interactionSimulationEnabled === true }; diff --git a/src/app/models/bias-impact.spec.ts b/src/app/models/bias-impact.spec.ts index d29500e..c70fdbc 100644 --- a/src/app/models/bias-impact.spec.ts +++ b/src/app/models/bias-impact.spec.ts @@ -1,9 +1,4 @@ -import { - BIAS_EXPERIMENT_ERROR_CODES, - BIAS_PROBE_ERROR_CODES, - BiasImpactJob, - BiasImpactReport -} from './bias-impact'; +import { BIAS_EXPERIMENT_ERROR_CODES, BIAS_PROBE_ERROR_CODES, BiasImpactJob, BiasImpactReport, activeAnnotationIdsFor, biasInterventionMix, isBiasVariantContext } from './bias-impact'; import { BiasBehavioralProbe, isProbeExecutable } from './flow'; import { TaskExecution } from './task-execution'; @@ -103,3 +98,97 @@ describe('bias impact models', () => { expect(BIAS_EXPERIMENT_ERROR_CODES).toContain('BIAS_ACTIVATION_ANNOTATIONS_REQUIRED'); }); }); + + +/** The shape the API actually sends, taken from a real persisted snapshot. */ +function context(overrides: Record = {}): any { + return { + experimentId: null, + mode: 'NORMAL', + activeBiasAnnotationIdsByNode: {}, + activeMitigationAnnotationIdsByNode: {}, + biasSubflowActivatedContainerIds: [], + mitigationSubflowActivatedContainerIds: [], + externalSideEffectPolicy: 'BLOCK', + externalSideEffectsConfirmed: false, + ...overrides + }; +} + +describe('isBiasVariantContext', () => { + it('is false for the NORMAL context every execution carries', () => { + // The object is always present; only the mode distinguishes a variant. Testing for presence + // marked every single run a bias variant. + expect(isBiasVariantContext(context())).toBe(false); + expect(isBiasVariantContext(null)).toBe(false); + expect(isBiasVariantContext(undefined)).toBe(false); + }); + + it('is true only for BIAS_VARIANT', () => { + expect(isBiasVariantContext(context({ mode: 'BIAS_VARIANT' }))).toBe(true); + }); +}); + +describe('biasInterventionMix', () => { + it('is null for a run that is not a variant', () => { + expect(biasInterventionMix(context())).toBeNull(); + }); + + it('reads bias from active bias annotations', () => { + expect(biasInterventionMix(context({ + mode: 'BIAS_VARIANT', + activeBiasAnnotationIdsByNode: { n1: ['a1'] } + }))).toBe('BIAS'); + }); + + it('reads mitigation from active mitigation annotations', () => { + expect(biasInterventionMix(context({ + mode: 'BIAS_VARIANT', + activeMitigationAnnotationIdsByNode: { n1: ['a1'] } + }))).toBe('MITIGATION'); + }); + + it('reports both directions as mixed', () => { + expect(biasInterventionMix(context({ + mode: 'BIAS_VARIANT', + activeBiasAnnotationIdsByNode: { n1: ['a1'] }, + activeMitigationAnnotationIdsByNode: { n2: ['a2'] } + }))).toBe('MIXED'); + }); + + it('also counts a direction activated through a container subflow', () => { + expect(biasInterventionMix(context({ + mode: 'BIAS_VARIANT', + biasSubflowActivatedContainerIds: ['c1'] + }))).toBe('BIAS'); + expect(biasInterventionMix(context({ + mode: 'BIAS_VARIANT', + mitigationSubflowActivatedContainerIds: ['c1'] + }))).toBe('MITIGATION'); + }); + + it('ignores a node whose annotation list is empty', () => { + expect(biasInterventionMix(context({ + mode: 'BIAS_VARIANT', + activeBiasAnnotationIdsByNode: { n1: [] } + }))).toBeNull(); + }); + + it('returns null for a variant with nothing recorded, rather than guessing a direction', () => { + expect(biasInterventionMix(context({ mode: 'BIAS_VARIANT' }))).toBeNull(); + }); +}); + +describe('activeAnnotationIdsFor', () => { + it('combines both directions for a node', () => { + const ctx = context({ + mode: 'BIAS_VARIANT', + activeBiasAnnotationIdsByNode: { n1: ['bias-1'] }, + activeMitigationAnnotationIdsByNode: { n1: ['mit-1'] } + }); + + expect(activeAnnotationIdsFor(ctx, 'n1')).toEqual(['bias-1', 'mit-1']); + expect(activeAnnotationIdsFor(ctx, 'other')).toEqual([]); + expect(activeAnnotationIdsFor(null, 'n1')).toEqual([]); + }); +}); diff --git a/src/app/models/bias-impact.ts b/src/app/models/bias-impact.ts index 1d72cb8..a83a6bb 100644 --- a/src/app/models/bias-impact.ts +++ b/src/app/models/bias-impact.ts @@ -38,20 +38,70 @@ export type BiasRerunRequest = { export type BiasExecutionMode = 'NORMAL' | 'BIAS_VARIANT' | string; +/** + * The shape the API actually sends. Bias and mitigation are tracked in *separate* collections, per + * node for annotations and per container for activated subflows, so the direction of a variant is + * read from which of them are non-empty. + * + * There is deliberately no `activeAnnotationIdsByNode` and no `activeBiasProbes` here: neither is + * ever sent on this object - probes belong to a step view - and modelling them made code silently + * read undefined. + */ export type BiasExecutionContext = { - experimentId: string; + experimentId: string | null; mode: BiasExecutionMode; - activeAnnotationIdsByNode: Record; + activeBiasAnnotationIdsByNode: Record; + activeMitigationAnnotationIdsByNode: Record; + biasSubflowActivatedContainerIds: string[]; + mitigationSubflowActivatedContainerIds: string[]; externalSideEffectPolicy: ExternalSideEffectPolicy; externalSideEffectsConfirmed: boolean; - activeBiasProbes?: Array<{ - annotationId: string; - direction: 'BIAS' | 'MITIGATION'; - activationMode: BiasActivationMode; - instruction?: string; - }>; }; +export type BiasInterventionMix = 'BIAS' | 'MITIGATION' | 'MIXED'; + +function hasAnnotations(byNode: Record | undefined): boolean { + return Object.values(byNode ?? {}).some((ids) => (ids ?? []).length > 0); +} + +/** True only for a real variant: the object is present on every execution, defaulting to NORMAL. */ +export function isBiasVariantContext(context: BiasExecutionContext | null | undefined): boolean { + return context?.mode === 'BIAS_VARIANT'; +} + +/** + * Which interventions a variant actually ran with. Null when it is not a variant, or when it is one + * but nothing is recorded as active - which is worth showing as "unspecified" rather than guessing + * one of the two. + */ +export function biasInterventionMix( + context: BiasExecutionContext | null | undefined +): BiasInterventionMix | null { + if (!isBiasVariantContext(context)) return null; + + const bias = hasAnnotations(context!.activeBiasAnnotationIdsByNode) + || (context!.biasSubflowActivatedContainerIds ?? []).length > 0; + const mitigation = hasAnnotations(context!.activeMitigationAnnotationIdsByNode) + || (context!.mitigationSubflowActivatedContainerIds ?? []).length > 0; + + if (bias && mitigation) return 'MIXED'; + if (mitigation) return 'MITIGATION'; + if (bias) return 'BIAS'; + return null; +} + +/** Every annotation active on a node, whichever direction it points in. */ +export function activeAnnotationIdsFor( + context: BiasExecutionContext | null | undefined, + nodeId: string +): string[] { + if (!context) return []; + return [ + ...(context.activeBiasAnnotationIdsByNode?.[nodeId] ?? []), + ...(context.activeMitigationAnnotationIdsByNode?.[nodeId] ?? []) + ]; +} + export type BiasImpactJobStatus = 'QUEUED' | 'RUNNING' | 'COMPLETED' | 'FAILED'; export type BiasImpactJob = { diff --git a/src/app/services/task-executions/task-executions-call.fake.ts b/src/app/services/task-executions/task-executions-call.fake.ts index 047f169..bd2dc31 100644 --- a/src/app/services/task-executions/task-executions-call.fake.ts +++ b/src/app/services/task-executions/task-executions-call.fake.ts @@ -681,12 +681,23 @@ export class TaskExecutionsCallServiceFake extends TaskExecutionsCallServiceBase map((execution) => { const biasedExecution: TaskExecution = { ...execution, + // Split by direction, like the real payload: it is what tells a bias variant from a + // mitigation one, and a fake that flattened them would hide that in development. biasExecutionContext: { experimentId: crypto.randomUUID(), mode: 'BIAS_VARIANT', - activeAnnotationIdsByNode: Object.fromEntries( - request.activations.map((activation) => [activation.nodeId, activation.annotationIds]) - ), + activeBiasAnnotationIdsByNode: Object.fromEntries(request.activations + .filter((activation) => activation.direction === 'BIAS') + .map((activation) => [activation.nodeId, activation.annotationIds])), + activeMitigationAnnotationIdsByNode: Object.fromEntries(request.activations + .filter((activation) => activation.direction === 'MITIGATION') + .map((activation) => [activation.nodeId, activation.annotationIds])), + biasSubflowActivatedContainerIds: request.activations + .filter((activation) => activation.direction === 'BIAS' && activation.includeSubflow) + .map((activation) => activation.nodeId), + mitigationSubflowActivatedContainerIds: request.activations + .filter((activation) => activation.direction === 'MITIGATION' && activation.includeSubflow) + .map((activation) => activation.nodeId), externalSideEffectPolicy: request.externalSideEffectPolicy, externalSideEffectsConfirmed: request.confirmExternalSideEffects } diff --git a/src/app/shared/task-execution-viewer/task-execution-viewer.html b/src/app/shared/task-execution-viewer/task-execution-viewer.html index 0ed9cdf..8115260 100644 --- a/src/app/shared/task-execution-viewer/task-execution-viewer.html +++ b/src/app/shared/task-execution-viewer/task-execution-viewer.html @@ -71,9 +71,7 @@
Experiment: {{ biasContext.experimentId || 'not available' }} Baseline: {{ execution()!.rerunOfExecutionId || 'not available' }} - @for (probe of biasContext.activeBiasProbes ?? []; track probe.annotationId + probe.direction) { - {{ probe.direction === 'BIAS' ? 'Bias applied' : 'Mitigation applied' }}: {{ probe.annotationId }} - } + Interventions: {{ biasVariantLabel() }}
} diff --git a/src/app/shared/task-execution-viewer/task-execution-viewer.ts b/src/app/shared/task-execution-viewer/task-execution-viewer.ts index 412e97e..8e2089b 100644 --- a/src/app/shared/task-execution-viewer/task-execution-viewer.ts +++ b/src/app/shared/task-execution-viewer/task-execution-viewer.ts @@ -26,6 +26,7 @@ import { TaskExecutionAuthorizationRequirement, TaskExecutionStep } from '@models/task-execution'; +import { activeAnnotationIdsFor, biasInterventionMix, isBiasVariantContext } from '@models/bias-impact'; import { ExecutionVaultCredential, LlmProviderCapability } from '@models/llm-provider'; import { VaultSecret } from '@models/assistant'; import { @@ -397,7 +398,7 @@ export class TaskExecutionViewerComponent implements OnDestroy { const contextErrors = this.execution()?.context.errors ?? {}; const contextWarnings = this.execution()?.context.warnings ?? {}; const waitingSteps = this.execution()?.context.waitingSteps ?? []; - const activeAnnotationIdsByNode = this.execution()?.biasExecutionContext?.activeAnnotationIdsByNode ?? {}; + const biasContext = this.execution()?.biasExecutionContext ?? null; const globalInputsValue = this.execution()?.context.globalInputs ?? {}; const executionVariablesValue = this.execution()?.context.executionVariables ?? {}; const projectContextValue = this.execution()?.context.projectContext ?? {}; @@ -457,7 +458,7 @@ export class TaskExecutionViewerComponent implements OnDestroy { __executionWarnings: getExecutionWarnings(step.id, contextWarnings), __stepResultData: step.result ?? null, __executionPartialResult: this.execution()?.context.partialResult ?? null, - __biasActiveAnnotationIds: activeAnnotationIdsByNode[step.id] ?? [], + __biasActiveAnnotationIds: activeAnnotationIdsFor(biasContext, step.id), __globalInputs: globalInputsValue, __executionVariables: executionVariablesValue, __projectContext: projectContextValue, @@ -508,7 +509,7 @@ export class TaskExecutionViewerComponent implements OnDestroy { __executionWarnings: [], __stepResultData: null, __executionPartialResult: this.execution()?.context.partialResult ?? null, - __biasActiveAnnotationIds: activeAnnotationIdsByNode[sourceNode.id] ?? [], + __biasActiveAnnotationIds: activeAnnotationIdsFor(biasContext, sourceNode.id), __globalInputs: globalInputsValue, __executionVariables: executionVariablesValue, __projectContext: projectContextValue, @@ -550,11 +551,13 @@ export class TaskExecutionViewerComponent implements OnDestroy { readonly headerDetailsOpen = signal(false); readonly biasVariantLabel = computed(() => { - const directions = new Set((this.execution()?.biasExecutionContext?.activeBiasProbes ?? []) - .map((probe) => probe.direction)); - if (directions.size > 1) return 'Bias + mitigation'; - if (directions.has('MITIGATION')) return 'Mitigation variant'; - return 'Bias variant'; + switch (biasInterventionMix(this.execution()?.biasExecutionContext)) { + case 'MIXED': return 'Bias + mitigation'; + case 'MITIGATION': return 'Mitigation variant'; + case 'BIAS': return 'Bias variant'; + // A variant with nothing recorded as active: say so rather than pick one of the two. + default: return 'Bias variant (unspecified)'; + } }); toggleHeaderDetails() { @@ -659,8 +662,7 @@ export class TaskExecutionViewerComponent implements OnDestroy { * the object says nothing - only the mode does. Testing for presence marked every run a bias * variant, and also offered the bias comparison on any plain rerun. */ - readonly isBiasVariant = computed(() => - this.execution()?.biasExecutionContext?.mode === 'BIAS_VARIANT'); + readonly isBiasVariant = computed(() => isBiasVariantContext(this.execution()?.biasExecutionContext)); readonly canCreateBiasedRerun = computed(() => !this.isSubflowExecution() && getExecutionStatusGroup(this.execution()?.context.status) === 'FINAL'