Compare the bias mode against the value the API actually sends
The backend enum is NORMAL | EXPERIMENT. isBiasVariantContext compared against 'BIAS_VARIANT', a value no endpoint has ever emitted, so it was always false and took a whole feature path down with it: - "Compare with baseline" is wrapped in @if (isBiasVariant()), so it never rendered - the only route to a FULL_FLOW bias report; - the run list never labelled a rerun as a bias variant, which is the very thing it was changed to do; - biasInterventionMix always returned null, so BIAS / MITIGATION / MIXED never showed. I introduced this while fixing a real bug - presence of biasExecutionContext was marking every run a variant - by correcting the condition to the wrong literal. The fixtures used the same invented value, so the tests passed and the change looked verified. They are corrected here too: with the old literal restored, eight assertions now fail. The two names are kept apart deliberately and both are commented: 'BIAS_VARIANT' remains the list's own TaskExecutionKind vocabulary, while the API mode is 'EXPERIMENT'. Treating them as interchangeable is what caused this. The dev fake was also seeding 'BIAS_VARIANT', so development agreed with the bug and disagreed with the service. 497 frontend tests green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
8659b798d5
commit
c35bbc0ccf
|
|
@ -29,7 +29,7 @@ function normalBias(): any {
|
|||
}
|
||||
|
||||
function variantBias(overrides: Record<string, unknown>): any {
|
||||
return { ...normalBias(), mode: 'BIAS_VARIANT', experimentId: 'x', ...overrides };
|
||||
return { ...normalBias(), mode: 'EXPERIMENT', experimentId: 'x', ...overrides };
|
||||
}
|
||||
|
||||
describe('TasksExecutor', () => {
|
||||
|
|
|
|||
|
|
@ -124,8 +124,11 @@ describe('isBiasVariantContext', () => {
|
|||
expect(isBiasVariantContext(undefined)).toBe(false);
|
||||
});
|
||||
|
||||
it('is true only for BIAS_VARIANT', () => {
|
||||
expect(isBiasVariantContext(context({ mode: 'BIAS_VARIANT' }))).toBe(true);
|
||||
it('is true for the mode the API actually sends, EXPERIMENT', () => {
|
||||
// The backend enum is NORMAL | EXPERIMENT. This used to assert BIAS_VARIANT - a value no
|
||||
// endpoint emits - so the predicate was always false and the fixtures protected the bug.
|
||||
expect(isBiasVariantContext(context({ mode: 'EXPERIMENT' }))).toBe(true);
|
||||
expect(isBiasVariantContext(context({ mode: 'BIAS_VARIANT' }))).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
|
|
@ -136,21 +139,21 @@ describe('biasInterventionMix', () => {
|
|||
|
||||
it('reads bias from active bias annotations', () => {
|
||||
expect(biasInterventionMix(context({
|
||||
mode: 'BIAS_VARIANT',
|
||||
mode: 'EXPERIMENT',
|
||||
activeBiasAnnotationIdsByNode: { n1: ['a1'] }
|
||||
}))).toBe('BIAS');
|
||||
});
|
||||
|
||||
it('reads mitigation from active mitigation annotations', () => {
|
||||
expect(biasInterventionMix(context({
|
||||
mode: 'BIAS_VARIANT',
|
||||
mode: 'EXPERIMENT',
|
||||
activeMitigationAnnotationIdsByNode: { n1: ['a1'] }
|
||||
}))).toBe('MITIGATION');
|
||||
});
|
||||
|
||||
it('reports both directions as mixed', () => {
|
||||
expect(biasInterventionMix(context({
|
||||
mode: 'BIAS_VARIANT',
|
||||
mode: 'EXPERIMENT',
|
||||
activeBiasAnnotationIdsByNode: { n1: ['a1'] },
|
||||
activeMitigationAnnotationIdsByNode: { n2: ['a2'] }
|
||||
}))).toBe('MIXED');
|
||||
|
|
@ -158,31 +161,31 @@ describe('biasInterventionMix', () => {
|
|||
|
||||
it('also counts a direction activated through a container subflow', () => {
|
||||
expect(biasInterventionMix(context({
|
||||
mode: 'BIAS_VARIANT',
|
||||
mode: 'EXPERIMENT',
|
||||
biasSubflowActivatedContainerIds: ['c1']
|
||||
}))).toBe('BIAS');
|
||||
expect(biasInterventionMix(context({
|
||||
mode: 'BIAS_VARIANT',
|
||||
mode: 'EXPERIMENT',
|
||||
mitigationSubflowActivatedContainerIds: ['c1']
|
||||
}))).toBe('MITIGATION');
|
||||
});
|
||||
|
||||
it('ignores a node whose annotation list is empty', () => {
|
||||
expect(biasInterventionMix(context({
|
||||
mode: 'BIAS_VARIANT',
|
||||
mode: 'EXPERIMENT',
|
||||
activeBiasAnnotationIdsByNode: { n1: [] }
|
||||
}))).toBeNull();
|
||||
});
|
||||
|
||||
it('returns null for a variant with nothing recorded, rather than guessing a direction', () => {
|
||||
expect(biasInterventionMix(context({ mode: 'BIAS_VARIANT' }))).toBeNull();
|
||||
expect(biasInterventionMix(context({ mode: 'EXPERIMENT' }))).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe('activeAnnotationIdsFor', () => {
|
||||
it('combines both directions for a node', () => {
|
||||
const ctx = context({
|
||||
mode: 'BIAS_VARIANT',
|
||||
mode: 'EXPERIMENT',
|
||||
activeBiasAnnotationIdsByNode: { n1: ['bias-1'] },
|
||||
activeMitigationAnnotationIdsByNode: { n1: ['mit-1'] }
|
||||
});
|
||||
|
|
|
|||
|
|
@ -36,7 +36,14 @@ export type BiasRerunRequest = {
|
|||
confirmExternalSideEffects: boolean;
|
||||
};
|
||||
|
||||
export type BiasExecutionMode = 'NORMAL' | 'BIAS_VARIANT' | string;
|
||||
/**
|
||||
* The values the API actually sends: the backend enum is NORMAL | EXPERIMENT.
|
||||
*
|
||||
* The product vocabulary is "variant", and this type used to say `BIAS_VARIANT` - a value no
|
||||
* endpoint has ever emitted, which silently made every run look ordinary. The names differ on
|
||||
* purpose; do not "correct" this one back to match the UI wording.
|
||||
*/
|
||||
export type BiasExecutionMode = 'NORMAL' | 'EXPERIMENT' | string;
|
||||
|
||||
/**
|
||||
* The shape the API actually sends. Bias and mitigation are tracked in *separate* collections, per
|
||||
|
|
@ -66,7 +73,7 @@ function hasAnnotations(byNode: Record<string, string[]> | undefined): boolean {
|
|||
|
||||
/** 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';
|
||||
return context?.mode === 'EXPERIMENT';
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -685,7 +685,9 @@ export class TaskExecutionsCallServiceFake extends TaskExecutionsCallServiceBase
|
|||
// mitigation one, and a fake that flattened them would hide that in development.
|
||||
biasExecutionContext: {
|
||||
experimentId: crypto.randomUUID(),
|
||||
mode: 'BIAS_VARIANT',
|
||||
// EXPERIMENT is the value the real API sends; a fake that invents its own would let the
|
||||
// app pass in development and fail against the service.
|
||||
mode: 'EXPERIMENT',
|
||||
activeBiasAnnotationIdsByNode: Object.fromEntries(request.activations
|
||||
.filter((activation) => activation.direction === 'BIAS')
|
||||
.map((activation) => [activation.nodeId, activation.annotationIds])),
|
||||
|
|
|
|||
|
|
@ -15,7 +15,13 @@ import { OrderViewState } from '@utilities/list-state-holder';
|
|||
|
||||
export type TaskExecutionFilter = 'all' | TaskExecutionStatusGroup;
|
||||
|
||||
/** What kind of run a row is, so a bias variant is never mistaken for an ordinary rerun. */
|
||||
/**
|
||||
* What kind of run a row is, so a bias variant is never mistaken for an ordinary rerun.
|
||||
*
|
||||
* This is the list's own vocabulary, not an API value: the mode the backend sends for a variant is
|
||||
* `EXPERIMENT` (see BiasExecutionMode). The two names looked interchangeable, and comparing an API
|
||||
* mode against this one is exactly how the variant check came to be always false.
|
||||
*/
|
||||
export type TaskExecutionKind = 'RUN' | 'RERUN' | 'BIAS_VARIANT';
|
||||
|
||||
export type TaskExecutionListItem = {
|
||||
|
|
|
|||
Loading…
Reference in New Issue