Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ const SliderInputControl = ({
)}
<Input
id={inputId}
data-testid={inputId}
className="box-content w-[var(--input-width)] max-w-[5ch] border px-2 py-0 text-right [&:not(:focus)]:border-transparent [&:not(:focus)]:px-0.5"
Comment thread
AndreiCautisanu marked this conversation as resolved.
style={
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,10 @@ const RuleFilteringSection: React.FC<RuleFilteringSectionProps> = ({
className="-mb-4 w-full border-t border-border"
>
<AccordionItem value="filtering-sampling" className="border-none">
<AccordionTrigger className="px-3 py-2 hover:no-underline">
<AccordionTrigger
className="px-3 py-2 hover:no-underline"
data-testid="add-edit-rule-dialog-filtering-sampling-trigger"
>
<div className="flex items-center gap-1">
<Label className="text-sm font-medium">Filtering & Sampling</Label>
<ExplainerIcon
Expand Down
3 changes: 2 additions & 1 deletion tests_end_to_end/coverage/taxonomy.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -628,6 +628,7 @@ areas:
- online-evaluation/online-evaluation-smoke.spec.ts
- online-evaluation/online-evaluation-delete-rule.spec.ts
- online-evaluation/online-evaluation-enable-disable-rule.spec.ts
- online-evaluation/online-evaluation-sampling-rate.spec.ts
capabilities:
create-llm-judge-rule: { covered: true, tier: t1-smoke }
llm-judge-scores: { covered: true, tier: t1-smoke, note: "bimodal safe/unsafe" }
Expand All @@ -637,7 +638,7 @@ areas:
list-rules: { covered: false }
rule-scope-thread-span: { covered: false, note: "span/thread scope flags" }
rule-filters: { covered: false }
sampling-rate: { covered: false }
sampling-rate: { covered: true, tier: t2-cuj, note: "50% rule vs 100% control over one 30-trace batch; binomial band 15-85%" }
clone-rule: { covered: false }
edit-rule: { covered: false }
enable-disable-rule: { covered: true, tier: t2-cuj, note: "edit-dialog switch; control rule proves scoring stopped, then resumed" }
Expand Down
24 changes: 24 additions & 0 deletions tests_end_to_end/e2e/core/backend/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,12 @@ export interface AutomationRuleRef {
name: string;
projectIds: string[];
enabled: boolean;
/**
* Fraction in [0, 1] — the backend's own units. The dialog shows a
* percentage (50), the API stores a fraction (0.5); assertions must use the
* fraction.
*/
samplingRate: number;
}

export interface AnnotationQueueReviewerRef {
Expand Down Expand Up @@ -222,6 +228,23 @@ export function makeBackendClient(apiKey: string | null = null) {
}
};

/**
* The generated REST type marks `samplingRate` optional. Defaulting a missing
* value to 1 would present as "100% of traces", which is indistinguishable
* from a correctly-configured full-rate rule — so a sampling assertion built
* on that default could pass while the field was never returned at all.
* Fail loudly instead.
*/
const requireSamplingRate = (rate: number | undefined, ruleName: string): number => {
if (typeof rate !== 'number' || Number.isNaN(rate)) {
throw new Error(
`listAutomationRulesForProject: rule '${ruleName}' returned no samplingRate — ` +
`cannot assert on sampling behaviour.`,
);
}
return rate;
};

// Hoisted so pollTraceForFeedbackScore (a free function) can call it without
// depending on the not-yet-constructed return object.
const localGetTrace = async (traceId: string): Promise<TraceDetail | null> => {
Expand Down Expand Up @@ -544,6 +567,7 @@ export function makeBackendClient(apiKey: string | null = null) {
name: r.name,
projectIds: (r.projects ?? []).map((p) => String(p.projectId)),
enabled: r.enabled ?? true,
samplingRate: requireSamplingRate(r.samplingRate, r.name),
}));
},

Expand Down
83 changes: 83 additions & 0 deletions tests_end_to_end/e2e/pom/online-evaluation.page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,11 @@ export interface CreateRuleDialogPythonEqualsFields {
name: string;
/** The literal string the trace's output must equal to score 1.0. */
referenceValue: string;
/**
* Sampling rate as the PERCENTAGE shown in the dialog (0-100), not the
* fraction the API stores. Omit to leave the control at its 100% default.
*/
samplingRatePercent?: number;
}

/**
Expand Down Expand Up @@ -132,6 +137,77 @@ export class OnlineEvaluationPage {
return this.ruleRow(name).getByRole('cell', { name: status, exact: true });
}

/**
* The Sampling rate cell for a rule row, rendered by OnlineEvaluationPage's
* `sampling_rate` column as a formatted percentage ("50%", "100%") — note the
* list shows a PERCENTAGE while the API stores a fraction.
*/
ruleSamplingRateCell(name: string, displayValue: string): Locator {
return this.ruleRow(name).getByRole('cell', { name: displayValue, exact: true });
}

/**
* The "Filtering & Sampling" accordion inside the add/edit dialog. It renders
* COLLAPSED by default, and its content is unmounted while collapsed, so the
* sampling-rate control does not exist until this is expanded.
*/
get filteringSamplingTrigger(): Locator {
return this.dialog.getByTestId('add-edit-rule-dialog-filtering-sampling-trigger');
}

/**
* The sampling-rate number input (the percentage box next to the slider).
* SliderInputControl derives this testid from its `id` prop.
*/
get samplingRateInput(): Locator {
return this.dialog.getByTestId('sampling_rate-input');
}

/**
* Expand the Filtering & Sampling accordion, if it is not already open.
* Idempotent: switching the rule TYPE re-renders the dialog body but leaves
* the accordion open, so callers can invoke this without tracking state.
*/
async expandFilteringAndSampling(): Promise<void> {
return test.step('expand the Filtering & Sampling accordion', async () => {
const trigger = this.filteringSamplingTrigger;
await trigger.waitFor({ state: 'visible' });
if ((await trigger.getAttribute('aria-expanded')) !== 'true') {
await trigger.click();
}
await expect(trigger).toHaveAttribute('aria-expanded', 'true');
await this.samplingRateInput.waitFor({ state: 'visible' });
});
}

/**
* Set the sampling rate to a PERCENTAGE (0-100), as the dialog displays it.
*
* The blur is load-bearing, not defensive tidying. SliderInputControl commits
* the typed value to the form in `onBlur` (`validateAndHandleChange`), NOT in
* `onChange` — so filling the box and submitting straight away posts the
* PREVIOUS value. Verified against the live dialog: typing 25 and submitting
* without blurring persists `sampling_rate: 1.0`, silently discarding the
* input. That failure is invisible to a sampling assertion (a rule left at
* 100% scores everything, which is exactly what "sampling ignored" looks
* like), so the commit has to be forced here.
*
* Do not "verify" the value by reading the slider's `aria-valuenow`: the
* slider mirrors the component's local state, so it reports the typed number
* even when the form value is still stale. The only trustworthy check is the
* persisted rate on the created rule — assert that in the test.
*/
async setSamplingRatePercent(percent: number): Promise<void> {
return test.step(`set sampling rate to ${percent}%`, async () => {
await this.expandFilteringAndSampling();
const input = this.samplingRateInput;
await input.fill(String(percent));
// Commit via blur — see the note above; without this the value is dropped.
await input.blur();
await expect(input).toHaveValue(String(percent));
});
}

/** The "Enable rule" switch inside the add/edit dialog. */
get enableRuleSwitch(): Locator {
return this.dialog.getByRole('switch', { name: 'Enable rule' });
Expand Down Expand Up @@ -254,6 +330,13 @@ export class OnlineEvaluationPage {
// input to settle to the new shape, then override its path.
await this.setVariableMapping('output', 'output.output');

// Set the rate last: the sampling control lives in a collapsed accordion
// below the code editor, and switching TYPE / re-parsing the snippet
// re-renders the body above it.
if (fields.samplingRatePercent !== undefined) {
await this.setSamplingRatePercent(fields.samplingRatePercent);
}

await d.getByTestId('add-edit-rule-dialog-submit').click();
await d.waitFor({ state: 'hidden' });
}
Expand Down
Loading
Loading