Skip to content
Closed
Show file tree
Hide file tree
Changes from 2 commits
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
6 changes: 5 additions & 1 deletion src/cli/primitives/OnlineEvalConfigPrimitive.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { findConfigRoot } from '../../lib';
import type { OnlineEvalConfig } from '../../schema';
import type { OnlineEvalConfig, OnlineEvalFilter } from '../../schema';
import { OnlineEvalConfigSchema } from '../../schema';
import { getErrorMessage } from '../errors';
import type { RemovalPreview, RemovalResult, SchemaChange } from '../operations/remove/types';
Expand All @@ -16,6 +16,8 @@ export interface AddOnlineEvalConfigOptions {
samplingRate: number;
enableOnCreate?: boolean;
endpoint?: string;
sessionTimeoutMinutes?: number;
filters?: OnlineEvalFilter[];
}

export type RemovableOnlineEvalConfig = RemovableResource;
Expand Down Expand Up @@ -235,6 +237,8 @@ export class OnlineEvalConfigPrimitive extends BasePrimitive<AddOnlineEvalConfig
samplingRate: options.samplingRate,
...(options.enableOnCreate !== undefined && { enableOnCreate: options.enableOnCreate }),
...(options.endpoint && { endpoint: options.endpoint }),
...(options.sessionTimeoutMinutes !== undefined && { sessionTimeoutMinutes: options.sessionTimeoutMinutes }),
...(options.filters && options.filters.length > 0 && { filters: options.filters }),
};

project.onlineEvalConfigs.push(config);
Expand Down
41 changes: 41 additions & 0 deletions src/cli/primitives/__tests__/OnlineEvalConfigPrimitive.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,47 @@ describe('OnlineEvalConfigPrimitive', () => {
expect(config.enableOnCreate).toBeUndefined();
});

it('stores sessionTimeoutMinutes and filters when provided', async () => {
mockReadProjectSpec.mockResolvedValue(makeProject());
mockWriteProjectSpec.mockResolvedValue(undefined);

const result = await primitive.add({
name: 'WithTimeoutAndFilters',
agent: 'MyAgent',
evaluators: ['Builtin.GoalSuccessRate'],
samplingRate: 10,
sessionTimeoutMinutes: 30,
filters: [
{ key: 'model', operator: 'Equals', value: { stringValue: 'claude-3' } },
{ key: 'latencyMs', operator: 'LessThan', value: { doubleValue: 1000 } },
],
});

expect(result.success).toBe(true);
const config = mockWriteProjectSpec.mock.calls[0]![0].onlineEvalConfigs[0];
expect(config.sessionTimeoutMinutes).toBe(30);
expect(config.filters).toEqual([
{ key: 'model', operator: 'Equals', value: { stringValue: 'claude-3' } },
{ key: 'latencyMs', operator: 'LessThan', value: { doubleValue: 1000 } },
]);
});

it('omits sessionTimeoutMinutes and filters when not provided', async () => {
mockReadProjectSpec.mockResolvedValue(makeProject());
mockWriteProjectSpec.mockResolvedValue(undefined);

await primitive.add({
name: 'NoTimeoutOrFilters',
agent: 'MyAgent',
evaluators: ['Builtin.GoalSuccessRate'],
samplingRate: 10,
});

const config = mockWriteProjectSpec.mock.calls[0]![0].onlineEvalConfigs[0];
expect(config.sessionTimeoutMinutes).toBeUndefined();
expect(config.filters).toBeUndefined();
});

it('supports multiple evaluators including ARNs', async () => {
mockReadProjectSpec.mockResolvedValue(makeProject());
mockWriteProjectSpec.mockResolvedValue(undefined);
Expand Down
3 changes: 3 additions & 0 deletions src/cli/tui/hooks/useCreateOnlineEval.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { onlineEvalConfigPrimitive } from '../../primitives/registry';
import type { OnlineEvalFilter } from '../../../schema';
import { withAddTelemetry } from '../../telemetry/cli-command-run.js';
import { useCallback, useEffect, useState } from 'react';

Expand All @@ -9,6 +10,7 @@ interface CreateOnlineEvalConfig {
evaluators: string[];
samplingRate: number;
sessionTimeoutMinutes?: number;
filters?: OnlineEvalFilter[];
enableOnCreate: boolean;
}

Expand All @@ -34,6 +36,7 @@ export function useCreateOnlineEval() {
evaluators: config.evaluators,
samplingRate: config.samplingRate,
...(config.sessionTimeoutMinutes !== undefined && { sessionTimeoutMinutes: config.sessionTimeoutMinutes }),
...(config.filters && config.filters.length > 0 && { filters: config.filters }),
enableOnCreate: config.enableOnCreate,
})
);
Expand Down
141 changes: 140 additions & 1 deletion src/cli/tui/screens/online-eval/AddOnlineEvalScreen.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { OnlineEvalConfigNameSchema } from '../../../../schema';
import type { OnlineEvalFilter, OnlineEvalFilterOperator } from '../../../../schema';
import type { SelectableItem } from '../../components';
import {
ConfirmReview,
Expand All @@ -13,7 +14,12 @@
import { useListNavigation, useMultiSelectNavigation } from '../../hooks';
import { generateUniqueName } from '../../utils';
import type { AddOnlineEvalConfig, EvaluatorItem, RuntimeEndpointEntry } from './types';
import { DEFAULT_SAMPLING_RATE, ONLINE_EVAL_STEP_LABELS } from './types';
import {
DEFAULT_SAMPLING_RATE,
DEFAULT_SESSION_TIMEOUT_MINUTES,
ONLINE_EVAL_FILTER_OPERATORS,
ONLINE_EVAL_STEP_LABELS,
} from './types';
import { useAddOnlineEvalWizard } from './useAddOnlineEvalWizard';
import { Box, Text } from 'ink';
import React, { useCallback, useEffect, useMemo } from 'react';
Expand Down Expand Up @@ -99,6 +105,8 @@
const isEndpointStep = wizard.step === 'endpoint';
const isEvaluatorsStep = wizard.step === 'evaluators';
const isSamplingRateStep = wizard.step === 'samplingRate';
const isSessionTimeoutStep = wizard.step === 'sessionTimeout';
const isFiltersStep = wizard.step === 'filters';
const isEnableOnCreateStep = wizard.step === 'enableOnCreate';
const isConfirmStep = wizard.step === 'confirm';

Expand Down Expand Up @@ -230,6 +238,73 @@
</Box>
)}

{isSessionTimeoutStep && (
<Box flexDirection="column">
<Text dimColor>
Minutes of inactivity before an agent session is considered complete (1–1440). Leave blank to use the
default of {DEFAULT_SESSION_TIMEOUT_MINUTES}.
</Text>
<TextInput
key="sessionTimeout"
prompt="Session timeout (minutes, blank=default)"
initialValue=""
onSubmit={value => {
const trimmed = value.trim();
if (trimmed === '') {
wizard.setSessionTimeoutMinutes(undefined);
return;
}
const minutes = parseInt(trimmed, 10);
if (isNaN(minutes) || minutes < 1 || minutes > 1440) return;
wizard.setSessionTimeoutMinutes(minutes);
}}
onCancel={() => wizard.goBack()}
customValidation={value => {
const trimmed = value.trim();
if (trimmed === '') return true;
const minutes = parseInt(trimmed, 10);
if (isNaN(minutes)) return 'Must be an integer or blank';
if (minutes < 1 || minutes > 1440) return 'Must be between 1 and 1440';
return true;
}}
/>
</Box>
)}

{isFiltersStep && (
<Box flexDirection="column">
<Text dimColor>
Optional filters that scope which traces are evaluated. Format: {'<key>'} {'<op>'} {'<value>'}, separated
by ";". Operators: {ONLINE_EVAL_FILTER_OPERATORS.join(', ')}. Values are parsed as boolean (true/false),

Check failure on line 278 in src/cli/tui/screens/online-eval/AddOnlineEvalScreen.tsx

View workflow job for this annotation

GitHub Actions / lint

`"` can be escaped with `&quot;`, `&ldquo;`, `&#34;`, `&rdquo;`

Check failure on line 278 in src/cli/tui/screens/online-eval/AddOnlineEvalScreen.tsx

View workflow job for this annotation

GitHub Actions / lint

`"` can be escaped with `&quot;`, `&ldquo;`, `&#34;`, `&rdquo;`
number, or string. Leave blank for no filters.
</Text>
<TextInput
key="filters"
prompt='Filters (e.g. "model Equals claude-3; latencyMs LessThan 1000")'
initialValue=""
onSubmit={value => {
const trimmed = value.trim();
if (trimmed === '') {
wizard.setFilters(undefined);
return;
}
const parsed = parseFiltersInput(trimmed);
if (!parsed) return;
wizard.setFilters(parsed);
}}
onCancel={() => wizard.goBack()}
customValidation={value => {
const trimmed = value.trim();
if (trimmed === '') return true;
const parsed = parseFiltersInput(trimmed);
if (!parsed)
return 'Each filter must be "<key> <operator> <value>" with a valid operator (separate with ";")';
return true;
}}
/>
</Box>
)}

{isEnableOnCreateStep && (
<WizardSelect
title="Enable on deploy?"
Expand All @@ -247,6 +322,20 @@
...(effectiveConfig.endpoint ? [{ label: 'Endpoint', value: effectiveConfig.endpoint }] : []),
{ label: 'Evaluators', value: effectiveConfig.evaluators.join(', ') },
{ label: 'Sampling Rate', value: `${effectiveConfig.samplingRate}%` },
{
label: 'Session Timeout',
value:
effectiveConfig.sessionTimeoutMinutes !== undefined
? `${effectiveConfig.sessionTimeoutMinutes} min`
: `${DEFAULT_SESSION_TIMEOUT_MINUTES} min (default)`,
},
{
label: 'Filters',
value:
effectiveConfig.filters && effectiveConfig.filters.length > 0
? effectiveConfig.filters.map(formatFilter).join('; ')
: '(none)',
},
{ label: 'Enable on Deploy', value: effectiveConfig.enableOnCreate ? 'Yes' : 'No' },
]}
/>
Expand All @@ -255,3 +344,53 @@
</Screen>
);
}

// ──────────────────────────────────────────────────────────────────────────────
// Filter parsing helpers
// ──────────────────────────────────────────────────────────────────────────────

function formatFilter(f: OnlineEvalFilter): string {
const v =
f.value.stringValue !== undefined

Check failure on line 354 in src/cli/tui/screens/online-eval/AddOnlineEvalScreen.tsx

View workflow job for this annotation

GitHub Actions / lint

Prefer using nullish coalescing operator (`??`) instead of a ternary expression, as it is simpler to read
? f.value.stringValue
: f.value.doubleValue !== undefined
? String(f.value.doubleValue)
: f.value.booleanValue !== undefined
? String(f.value.booleanValue)
: '';
return `${f.key} ${f.operator} ${v}`;
}

/**
* Parse a filter input string such as:
* "model Equals claude-3; latencyMs LessThan 1000; success Equals true"
* Returns undefined if any segment is malformed.
*/
function parseFiltersInput(input: string): OnlineEvalFilter[] | undefined {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

parseFiltersInput is the whole contract for filter input — regex-based segment parsing, quoted vs bare value handling, boolean/numeric coercion, operator validation — and it's untested. Given the failure modes are silent (customValidation just re-runs the parser), please add unit tests covering at minimum:

  • single vs multi-segment input
  • quoted string preserves "true", "false", "12345" as stringValue (the fix for the round-1 concern — worth locking in)
  • bare true / falsebooleanValue
  • bare -?\d+(\.\d+)?doubleValue, including negatives and decimals
  • bare non-numeric/non-boolean → stringValue
  • multi-word unquoted values (current regex allows them via (.+?)\s*$, so it's testable behavior worth pinning)
  • invalid operator → undefined
  • missing value or operator → undefined
  • stray ; and empty segments don't break parsing

Extracting this into a sibling helper module (e.g. ./filter-input.ts) would make it trivial to test and keep the screen component lean.

const segments = input
.split(';')
.map(s => s.trim())
.filter(s => s.length > 0);
if (segments.length === 0) return undefined;

const filters: OnlineEvalFilter[] = [];
for (const segment of segments) {
const parts = segment.split(/\s+/);
if (parts.length < 3) return undefined;
const key = parts[0]!;
const operator = parts[1] as OnlineEvalFilterOperator;
if (!ONLINE_EVAL_FILTER_OPERATORS.includes(operator)) return undefined;
const rawValue = parts.slice(2).join(' ');

let value: OnlineEvalFilter['value'];
if (rawValue === 'true' || rawValue === 'false') {
value = { booleanValue: rawValue === 'true' };
} else if (/^-?\d+(\.\d+)?$/.test(rawValue)) {
value = { doubleValue: parseFloat(rawValue) };
} else {
value = { stringValue: rawValue };
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The value auto-typing here is lossy: "true"/"false" always become booleans and anything matching -?\d+(\.\d+)? always becomes a double. Users cannot produce:

  • stringValue: "true" or stringValue: "false"
  • stringValue: "123", stringValue: "v1.2", or any numeric-looking identifier

Since filter keys are service-defined attributes, this will come up in real configs (e.g. a trace attribute whose value is the literal string "true", or an ID like "12345").

A couple of options:

  1. Explicit type prefix: require s:, d:, or b: prefixes, e.g. model Equals s:claude-3, latencyMs LessThan d:1000, success Equals b:true. Falls back to string if no prefix.
  2. Quoted strings force string type: e.g. key Equals "true"stringValue: "true", bare true/false → boolean, bare numeric → double, otherwise string.
  3. Per-filter wizard step with a value-type select (string / number / boolean) followed by a typed value prompt. More keystrokes but unambiguous, and avoids the ;/whitespace-in-values limitations the current parser has.

Whichever you pick, please also document the behavior in the <Text dimColor> help above so users aren't surprised.

filters.push({ key, operator, value });
}
return filters;
}
29 changes: 25 additions & 4 deletions src/cli/tui/screens/online-eval/types.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
// ─────────────────────────────────────────────────────────────────────────────
// ─────────────────────────────────────────────────────────────────────────────
// Online Eval Config Flow Types
// ─────────────────────────────────────────────────────────────────────────────
// ──────────────────────────────────────────────────────────────────────────────

import type { OnlineEvalFilter, OnlineEvalFilterOperator } from '../../../../schema';

export type AddOnlineEvalStep =
| 'name'
| 'agent'
| 'endpoint'
| 'evaluators'
| 'samplingRate'
| 'sessionTimeout'
| 'filters'
| 'enableOnCreate'
| 'confirm';

Expand All @@ -17,6 +21,8 @@ export interface AddOnlineEvalConfig {
endpoint?: string;
evaluators: string[];
samplingRate: number;
sessionTimeoutMinutes?: number;
filters?: OnlineEvalFilter[];
enableOnCreate: boolean;
description?: string;
}
Expand All @@ -33,13 +39,27 @@ export const ONLINE_EVAL_STEP_LABELS: Record<AddOnlineEvalStep, string> = {
endpoint: 'Endpoint',
evaluators: 'Evaluators',
samplingRate: 'Rate',
sessionTimeout: 'Timeout',
filters: 'Filters',
enableOnCreate: 'Enable',
confirm: 'Confirm',
};

// ─────────────────────────────────────────────────────────────────────────────
/** Filter operators offered in the wizard. */
export const ONLINE_EVAL_FILTER_OPERATORS: OnlineEvalFilterOperator[] = [
'Equals',
'NotEquals',
'GreaterThan',
'LessThan',
'GreaterThanOrEqual',
'LessThanOrEqual',
'Contains',
'NotContains',
];

// ──────────────────────────────────────────────────────────────────────────────
// Evaluator Items (fetched from API)
// ─────────────────────────────────────────────────────────────────────────────
// ─────────────────────────────────────────────────────────────────────────────

export interface EvaluatorItem {
/** ARN used as the stored identifier in the config */
Expand All @@ -53,3 +73,4 @@ export interface EvaluatorItem {
}

export const DEFAULT_SAMPLING_RATE = 10;
export const DEFAULT_SESSION_TIMEOUT_MINUTES = 5;
Loading
Loading