diff --git a/backend/.sqlx/query-1ecabdaa9c64eb0ac5bcb62b872158a9e9b158626fdd3afa6dc8b4303fe0945c.json b/backend/.sqlx/query-1ecabdaa9c64eb0ac5bcb62b872158a9e9b158626fdd3afa6dc8b4303fe0945c.json new file mode 100644 index 0000000000..4d68381213 --- /dev/null +++ b/backend/.sqlx/query-1ecabdaa9c64eb0ac5bcb62b872158a9e9b158626fdd3afa6dc8b4303fe0945c.json @@ -0,0 +1,29 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT id, status::text AS \"status!\" FROM v2_job_completed\n WHERE id = ANY($1) AND workspace_id = $2", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "id", + "type_info": "Uuid" + }, + { + "ordinal": 1, + "name": "status!", + "type_info": "Text" + } + ], + "parameters": { + "Left": [ + "UuidArray", + "Text" + ] + }, + "nullable": [ + false, + null + ] + }, + "hash": "1ecabdaa9c64eb0ac5bcb62b872158a9e9b158626fdd3afa6dc8b4303fe0945c" +} diff --git a/backend/windmill-api/openapi.yaml b/backend/windmill-api/openapi.yaml index 8ddbfbefb3..6a4a1d7bfa 100644 --- a/backend/windmill-api/openapi.yaml +++ b/backend/windmill-api/openapi.yaml @@ -26246,7 +26246,7 @@ components: tool_inputs: type: object expected: - description: Reference output. Recorded at capture time; nothing scores it yet. + description: Reference output a scorer compares a rerun against. tags: type: array items: @@ -26283,7 +26283,7 @@ components: tool_inputs: type: object expected: - description: What the captured run answered. + description: What the captured run answered, kept as the reference to score against. source: $ref: "#/components/schemas/EvalCaseSource" agent_path: diff --git a/backend/windmill-api/src/ai_evals.rs b/backend/windmill-api/src/ai_evals.rs index 3c0c1bb5c9..6434615457 100644 --- a/backend/windmill-api/src/ai_evals.rs +++ b/backend/windmill-api/src/ai_evals.rs @@ -614,7 +614,23 @@ mod with_storage { // would let a failure between the two deletes leave the cases behind *and* unblock // recreating the path — the new dataset would open holding the old one's cases. This way // a failure leaves an empty dataset, which is visible and retryable. - for key in [cases_key(&path), meta_key(&path)] { + // Experiments first, for the same reason cases precede metadata: they hold copies of the + // cases, so a delete that stopped halfway must not leave them readable under a recreated + // dataset of the same path. + use futures::TryStreamExt; + let experiment_keys: Vec = client + .list(Some(&ObjectPath::from( + format!("{}/{}", EXPERIMENTS_PREFIX, path).as_str(), + ))) + .map_ok(|meta| meta.location.to_string()) + .try_collect() + .await + .map_err(object_store_error_to_error)?; + + for key in experiment_keys + .into_iter() + .chain([cases_key(&path), meta_key(&path)]) + { match client.delete(&ObjectPath::from(key.as_str())).await { Ok(()) => {} Err(ObjectStoreError::NotFound { .. }) => {} @@ -1397,11 +1413,26 @@ mod with_storage { .ok_or_else(|| Error::NotFound(format!("Experiment {} not found", payload.id)))?; let experiment: EvalExperiment = serde_json::from_slice(&bytes)?; + // One query for every case job's own status: reading the agent step's success reported a + // case as successful even when a scorer step had failed. + let job_ids: Vec = experiment.cases.iter().map(|c| c.job_id).collect(); + let statuses = sqlx::query!( + "SELECT id, status::text AS \"status!\" FROM v2_job_completed + WHERE id = ANY($1) AND workspace_id = $2", + &job_ids, + &w_id + ) + .fetch_all(&db) + .await? + .into_iter() + .map(|r| (r.id, r.status)) + .collect::>(); + let mut rows = Vec::with_capacity(experiment.cases.len()); for case in &experiment.cases { // Each case is its own flow, so its steps are read back by node id rather than by // walking a nested loop's status. - let (output, success) = windmill_queue::get_result_and_success_by_id_from_flow( + let output = windmill_queue::get_result_and_success_by_id_from_flow( &db, &w_id, &case.job_id, @@ -1409,8 +1440,8 @@ mod with_storage { None, ) .await - .map(|(r, s)| (agent_answer(&r), Some(s))) - .unwrap_or((None, None)); + .ok() + .and_then(|(r, _)| agent_answer(&r)); let mut scores = Vec::with_capacity(experiment.scorers.len()); for index in 0..experiment.scorers.len() { @@ -1433,11 +1464,10 @@ mod with_storage { input: case.input.clone(), expected: case.expected.clone(), job_id: case.job_id, - status: match success { - Some(true) => "success".to_string(), - Some(false) => "failure".to_string(), - None => "running".to_string(), - }, + status: statuses + .get(&case.job_id) + .cloned() + .unwrap_or_else(|| "running".to_string()), output, scores, }); diff --git a/docs/ai-agent-evals.md b/docs/ai-agent-evals.md index 73bf413103..a8e50342ad 100644 --- a/docs/ai-agent-evals.md +++ b/docs/ai-agent-evals.md @@ -78,8 +78,8 @@ draft for review rather than writing anything: everything before it replayed as the agent's memory and whatever the agent answered after it kept as `expected`. Splitting there rather than at the end is what makes a finished conversation — which ends on the assistant — yield a runnable case. Tool messages are left out: - their content is keyed to call ids this replay will not reissue. `expected` has no consumer - yet; it is recorded now because it is only available at capture time. + their content is keyed to call ids this replay will not reissue. `expected` is what a scorer + compares a rerun against, and capture time is the only moment it exists. ## Experiments diff --git a/frontend/src/lib/components/aiEvals/AgentEvalDrawer.svelte b/frontend/src/lib/components/aiEvals/AgentEvalDrawer.svelte index 5e1c32ff69..23471203be 100644 --- a/frontend/src/lib/components/aiEvals/AgentEvalDrawer.svelte +++ b/frontend/src/lib/components/aiEvals/AgentEvalDrawer.svelte @@ -82,6 +82,7 @@ let paneTab = $state<'cases' | 'results'>('cases') let scorers = $state<{ kind: 'script' | 'flow' | 'agent'; path: string; name?: string }[]>([]) let experimentToken = $state(0) + let startedExperimentId = $state(undefined) let startingExperiment = $state(false) async function loadDatasets() { @@ -281,7 +282,7 @@ } startingExperiment = true try { - await AiEvalsService.runExperiment({ + startedExperimentId = await AiEvalsService.runExperiment({ workspace: ws, requestBody: { dataset: selectedDataset, @@ -602,6 +603,7 @@ dataset={selectedDataset} workspace={ws} refreshToken={experimentToken} + selectExperimentId={startedExperimentId} /> {/if} diff --git a/frontend/src/lib/components/aiEvals/ExperimentResults.svelte b/frontend/src/lib/components/aiEvals/ExperimentResults.svelte index d419342365..d01a79e64e 100644 --- a/frontend/src/lib/components/aiEvals/ExperimentResults.svelte +++ b/frontend/src/lib/components/aiEvals/ExperimentResults.svelte @@ -14,12 +14,15 @@ let { dataset, workspace = undefined, - refreshToken = 0 + refreshToken = 0, + selectExperimentId = undefined }: { dataset: string | undefined workspace?: string /** Bumped by the parent when it starts an experiment, to pick the new one up. */ refreshToken?: number + /** The experiment the parent just started, which becomes the selected one. */ + selectExperimentId?: string } = $props() let ws = $derived(workspace ?? $workspaceStore) @@ -33,6 +36,7 @@ // against the run before the change says whether the change helped. let baselineId = $state(undefined) let baselineRows = $state([]) + let baselineLabels = $state([]) let onlyRegressions = $state(false) let listGeneration = 0 @@ -46,8 +50,11 @@ const found = await AiEvalsService.listExperiments({ workspace: ws, path }).catch(() => []) if (generation !== listGeneration) return experiments = found - // Newest first from the API, so an unset or vanished selection lands on the latest run. - if (!selectedId || !found.some((e) => e.id === selectedId)) { + // A just-started experiment wins; otherwise, newest first from the API means an unset or + // vanished selection lands on the latest run. + if (selectExperimentId && found.some((e) => e.id === selectExperimentId)) { + selectedId = selectExperimentId + } else if (!selectedId || !found.some((e) => e.id === selectedId)) { selectedId = found[0]?.id } } @@ -88,13 +95,19 @@ const id = baselineId const generation = ++baselineGeneration baselineRows = [] + baselineLabels = [] if (!ws || !path || !id) return AiEvalsService.experimentResults({ workspace: ws, requestBody: { dataset: path, id } }) .then((res) => { - if (generation === baselineGeneration) baselineRows = res.rows ?? [] + if (generation !== baselineGeneration) return + baselineRows = res.rows ?? [] + baselineLabels = res.scorer_labels ?? [] }) .catch(() => { - if (generation === baselineGeneration) baselineRows = [] + if (generation === baselineGeneration) { + baselineRows = [] + baselineLabels = [] + } }) }) @@ -102,9 +115,17 @@ Object.fromEntries(baselineRows.map((r) => [r.case_id, r])) ) + // The two experiments' scorer lists can differ, so a delta is only meaningful between the same + // scorer — matched by label, not by its position in the array. + function baselineIndex(index: number): number { + return baselineLabels.indexOf(scorerLabels[index]) + } + function delta(row: ExperimentRow, index: number): number | undefined { + const other = baselineIndex(index) + if (other < 0) return undefined const now = row.scores?.[index] - const before = baselineByCase[row.case_id]?.scores?.[index] + const before = baselineByCase[row.case_id]?.scores?.[other] if (typeof now !== 'number' || typeof before !== 'number') return undefined return now - before } @@ -135,8 +156,10 @@ let regressionCount = $derived(baselineId ? rows.filter(isRegression).length : 0) let meanDeltas = $derived( scorerLabels.map((_, index) => { + const other = baselineIndex(index) + if (other < 0) return undefined const before = baselineRows - .map((r) => r.scores?.[index]) + .map((r) => r.scores?.[other]) .filter((v): v is number => typeof v === 'number') if (before.length === 0 || means[index] == undefined) return undefined return means[index]! - before.reduce((a, b) => a + b, 0) / before.length