From 1462f17643302127b4bd76bacfde80cc03f9d606 Mon Sep 17 00:00:00 2001 From: Guilhem Date: Mon, 31 Aug 2026 19:30:26 +0200 Subject: [PATCH] feat: rework the evals dataset drawer and run navigation (#10884) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: create eval datasets from the run dialog, not the empty table Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01BamH3sRkmn5nP7wo9iKYPJ * feat: paginate the evals dialog and show live run progress Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01BamH3sRkmn5nP7wo9iKYPJ * feat: rework the evals dataset drawer and run navigation Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01BamH3sRkmn5nP7wo9iKYPJ * fix: take the write lock on the empty-state add-a-case action Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01BamH3sRkmn5nP7wo9iKYPJ * fix: paginate the cases editor and add keyboard page navigation Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01BamH3sRkmn5nP7wo9iKYPJ * fix: wire cases pagination and select runs from the keyboard Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01BamH3sRkmn5nP7wo9iKYPJ * fix: one highlight for pointer and keyboard, and guard keys on the topmost overlay Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01BamH3sRkmn5nP7wo9iKYPJ * fix: restore page navigation and answer arrows outside the pages Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01BamH3sRkmn5nP7wo9iKYPJ * fix: scope eval keyboard navigation to the active topmost surface Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01BamH3sRkmn5nP7wo9iKYPJ * fix: leave Enter to focused controls and declare topmost from drawers too Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01BamH3sRkmn5nP7wo9iKYPJ * fix: highlight with the hover surface and open the highlighted run Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01BamH3sRkmn5nP7wo9iKYPJ * fix: restore the run's dataset on the arrow-right fallback Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01BamH3sRkmn5nP7wo9iKYPJ * fix: keep the chosen comparison when reopening the same run Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01BamH3sRkmn5nP7wo9iKYPJ * docs: state the Modal Enter caveat on EditableTextarea Modal handles Enter at `window` in the capture phase and stops propagation, so inside a dialog the key confirms the dialog rather than committing the edit. The docstring already carried this caveat for Escape; it now covers both keys and names `enterConfirms={false}` as the opt-out. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01BamH3sRkmn5nP7wo9iKYPJ * fix: render the case result with the chat's prose stack GfmMarkdown falls back to the legacy `!prose-xs` when no `prose` is given, so the case result read differently from a chat answer. Pass `sm`, the stack AssistantMessage renders with, and drop the wrapper whose `text-xs text-secondary` competed with it. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01BamH3sRkmn5nP7wo9iKYPJ * refactor: drop the narrating comment on the case result render Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01BamH3sRkmn5nP7wo9iKYPJ * fix: restore focus to the arriving page on keyboard navigation The restore checked `document.activeElement === document.body` one frame after the page changed, but the inert-driven reset lands after that frame: it read the element the user was about to lose and returned. Ask whether focus was inside the pages before navigating, then focus the arriving page unconditionally, which removes the race rather than re-timing it. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01BamH3sRkmn5nP7wo9iKYPJ * fix: restore focus for any keyboard-driven page change Arming only from the arrow handler missed Enter, which opens the highlighted run from the page itself and never reaches this component. Record whether the last interaction was a key pressed with focus inside the pages — cleared on pointerdown — so any caller-driven keyboard navigation restores focus too. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01BamH3sRkmn5nP7wo9iKYPJ * fix: track focus position rather than the key that navigated Arming from a keydown listener cannot work for Escape: Modal handles it at `window` in the capture phase, registered before this component, and steps a level back from there — Svelte flushes this component's effects inside that handler, before our listener runs. Track whether focus sits in the pages as it moves, so the answer is already settled when the page changes. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01BamH3sRkmn5nP7wo9iKYPJ * fix: take focus back when a navigation leaves it nowhere The trail's back button sits outside the pages and is removed as the level it returns from closes, so activating it from the keyboard left focus on a dead element. Claim the arriving page when focus was in the pages, or when it has ended up on the body — never off a control that outlives the navigation. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01BamH3sRkmn5nP7wo9iKYPJ * fix: type the TextInput binding as the textarea it renders `TextInput` is generic over its underlying element and defaults to `'input'`, so binding the textarea instance to a bare `TextInput` failed svelte-check. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01BamH3sRkmn5nP7wo9iKYPJ --------- Co-authored-by: Claude Opus 5 (1M context) --- .../components/aiEvals/AgentEvalModal.svelte | 8 +- .../components/aiEvals/EvalCasesGrid.svelte | 230 ++++---- .../aiEvals/EvalDatasetDrawer.svelte | 56 +- .../components/aiEvals/EvalRunDialog.svelte | 45 +- .../components/aiEvals/EvalRunsList.svelte | 135 ++++- .../lib/components/aiEvals/EvalScorers.svelte | 21 +- .../lib/components/aiEvals/EvalsPane.svelte | 507 +++++++++++------- .../display/table/multilineCellEditor.css | 26 - .../display/table/multilineCellEditor.ts | 108 ---- .../components/common/EditableTextarea.svelte | 173 ++++++ .../components/common/drawer/Drawer.svelte | 6 + .../common/emptyState/EmptyState.svelte | 18 +- .../lib/components/common/modal/Modal.svelte | 43 +- .../common/modal/PagedContent.svelte | 202 +++++++ .../components/common/overlayHost.svelte.ts | 20 + .../components/text_input/TextInput.svelte | 10 +- 16 files changed, 1099 insertions(+), 509 deletions(-) delete mode 100644 frontend/src/lib/components/apps/components/display/table/multilineCellEditor.css delete mode 100644 frontend/src/lib/components/apps/components/display/table/multilineCellEditor.ts create mode 100644 frontend/src/lib/components/common/EditableTextarea.svelte create mode 100644 frontend/src/lib/components/common/modal/PagedContent.svelte diff --git a/frontend/src/lib/components/aiEvals/AgentEvalModal.svelte b/frontend/src/lib/components/aiEvals/AgentEvalModal.svelte index ff014f403a..61d88819e7 100644 --- a/frontend/src/lib/components/aiEvals/AgentEvalModal.svelte +++ b/frontend/src/lib/components/aiEvals/AgentEvalModal.svelte @@ -27,18 +27,14 @@ let trail = $derived( location ? [{ label: TITLE, onclick: location.back }, { label: location.label }] : undefined ) - let description = $derived( - location - ? undefined - : 'Each run answers a dataset of cases with this agent and scores the answers, so runs can be compared.' - ) - import { createGrid, type GridApi } from 'ag-grid-community' - import 'ag-grid-community/styles/ag-grid.css' - import 'ag-grid-community/styles/ag-theme-alpine.css' - import '$lib/components/apps/components/display/table/theme/windmill-theme.css' - import { transformColumnDefs } from '$lib/components/apps/components/display/table/utils' - import { multilineCellColDef } from '$lib/components/apps/components/display/table/multilineCellEditor' - import DarkModeObserver from '$lib/components/DarkModeObserver.svelte' + import DataTable from '$lib/components/table/DataTable.svelte' + import Head from '$lib/components/table/Head.svelte' + import Cell from '$lib/components/table/Cell.svelte' + import Row from '$lib/components/table/Row.svelte' + import EditableTextarea from '$lib/components/common/EditableTextarea.svelte' + import { Button, EmptyState } from '$lib/components/common' + import { ListPlus, Plus, Trash2 } from 'lucide-svelte' import { untrack } from 'svelte' import type { CaseDraft } from './evalUtils' let { cases = $bindable(), onRemove, - locked = false, - onEditingChange + onAdd, + focusCaseId = undefined, + locked = false }: { /** The drawer's working copy. Edits land here as they are made; the drawer writes them. */ cases: CaseDraft[] /** Asked before a row goes, since a stored case has runs that executed it. */ onRemove: (c: CaseDraft) => void + /** Adds a case. The empty table offers this itself, where the first row would be. */ + onAdd: () => void + /** A case to open for typing as soon as it appears — the one just added, so a new row is + * ready to be filled in rather than waiting to be clicked. */ + focusCaseId?: string /** The cases are being written: an edit made now is one the request already left behind. */ locked?: boolean - /** A cell opened or closed. Reported up so Save can be pressed for an edit the list does not - * hold yet: the press is what commits the cell, so it has to reach a live button. */ - onEditingChange?: (editing: boolean) => void } = $props() - type Row = { id: string; question: string; expected: string } + /** A dataset is capped at 1000 cases and every row here mounts two editors and a button, so the + * whole set at once is thousands of components for a table you read a screenful of. ag-grid + * virtualised its rows; `DataTable` paginates instead, which is the primitive already here. */ + const CASES_PER_PAGE = 25 + let currentPage = $state(1) + /** Bound to `DataTable`'s own per-page selector, so changing it there re-slices here rather + * than only relabelling the footer. */ + let perPage = $state(CASES_PER_PAGE) + let lastPage = $derived(Math.max(1, Math.ceil(cases.length / perPage))) + let pageCases = $derived(cases.slice((currentPage - 1) * perPage, currentPage * perPage)) + // A case removed from the last page leaves it empty; step back rather than show nothing. + $effect(() => { + if (currentPage > lastPage) currentPage = lastPage + }) + /** `expected` is whatever the case holds: a bare string, or a value shown as JSON. Read back + * the same way, so a case that held an object keeps holding one. */ function expectedToText(value: unknown): string { if (value == undefined) return '' return typeof value === 'string' ? value : JSON.stringify(value, null, 2) } - function toRow(c: CaseDraft): Row { - return { - id: c.id ?? '', - question: c.input?.user_message ?? '', - expected: expectedToText(c.expected) - } - } - - let api: GridApi | undefined = $state() - let eGui: HTMLDivElement | undefined = $state() - let darkMode = $state(false) - - const defaultColDef = { - flex: 1, - minWidth: 120, - ...multilineCellColDef - } - - $effect(() => eGui && untrack(() => mountGrid())) - function mountGrid() { - if (!eGui || api) return - createGrid(eGui, { - rowData: untrack(() => cases.map(toRow)), - columnDefs: columnDefs(), - defaultColDef: { ...defaultColDef, editable: untrack(() => !locked) }, - onCellValueChanged: (e) => { - const target = cases.find((c) => c.id === (e.data as Row).id) - if (!target) return - if (e.colDef.field === 'question') { - target.input = { ...target.input, user_message: e.newValue ?? '' } - } else if (e.colDef.field === 'expected') { - setExpected(target, e.newValue ?? '') - } - }, - onCellEditingStarted: () => onEditingChange?.(true), - onCellEditingStopped: () => onEditingChange?.(false), - suppressColumnMoveAnimation: true, - suppressDragLeaveHidesColumns: true, - onGridReady: (e) => (api = e.api) + /** The question editors, by case id, so a newly added row can be opened. */ + let questionEditors = $state>({}) + $effect(() => { + const id = focusCaseId + if (!id) return + untrack(() => { + // Onto the page the case is on before reaching for its editor: paginated, a case added to + // a full page is not rendered yet, and there would be nothing to open. + const at = cases.findIndex((c) => c.id === id) + if (at < 0) return + currentPage = Math.floor(at / perPage) + 1 + // After the row has been rendered and registered itself. + requestAnimationFrame(() => questionEditors[id]?.edit()) }) - } + }) - /** Text that parses as JSON is stored as JSON, so a structured answer can be written by hand. */ function setExpected(c: CaseDraft, text: string) { + // Cleared means the case has no expected answer, which is not the same as expecting the + // empty string: a scorer reads `undefined` as "nothing to measure here" and leaves the case + // out of its mean, where `''` scores it a hard zero. const trimmed = text.trim() if (!trimmed) { c.expected = undefined @@ -88,45 +82,93 @@ c.expected = text } } - - function columnDefs() { - return transformColumnDefs({ - columnDefs: [ - { field: 'question', headerName: 'Question', flex: 3 }, - { field: 'expected', headerName: 'Expected', flex: 2 } - ] as any, - onDelete: (values) => { - if (locked) return - const target = cases.find((c) => c.id === (values as Row).id) - if (target) onRemove(target) - } - }) - } - - // Keyed on which cases are in the list rather than on what is in them: pushing rows back on - // each keystroke would reset the cell being typed in. - let rowKey = $derived(cases.map((c) => c.id).join(',')) - $effect(() => { - rowKey - untrack(() => api?.updateGridOptions({ rowData: cases.map(toRow) })) - }) - - $effect(() => { - const editable = !locked - untrack(() => api?.updateGridOptions({ defaultColDef: { ...defaultColDef, editable } })) - }) - - /** Commit whatever cell is open into `cases`, and wait for it to land there: the drawer reads - * the list to save it, and a cell still being typed in is an edit that press is saving. */ - export async function flush() { - api?.stopEditing() - // The commit reaches `cases` through the grid's own event queue, a macrotask away: reading in - // the same turn, or after a microtask, reads the list as it was before the cell was typed in. - await new Promise((resolve) => setTimeout(resolve, 0)) - } - - -
+{#if cases.length === 0} + +{:else} + perPage} + bind:currentPage + bind:perPage + rowCount={cases.length} + hasMore={currentPage < lastPage} + showPrev={currentPage > 1} + on:next={() => (currentPage = Math.min(currentPage + 1, lastPage))} + on:previous={() => (currentPage = Math.max(currentPage - 1, 1))} + > + + + + + + + + Question + Expected + + + + + {#each pageCases as c (c.id)} + + + + (c.input = { ...(c.input ?? {}), user_message: v })} + /> + + + setExpected(c, v)} + /> + + + -
+
(casesEditing = v)} />
@@ -397,7 +400,7 @@ unifiedSize="md" variant="accent" loading={saving} - disabled={writing || !path || !!pathError || (nothingToSave && !casesEditing)} + disabled={writing || !path || !!pathError || nothingToSave} onclick={saveDataset} > Save @@ -408,7 +411,8 @@ variant="accent" startIcon={{ icon: Plus }} loading={creating} - disabled={creating || !path || !!pathError} + disabled={creating || !path || !!pathError || noCases} + title={noCasesTitle} onclick={createDataset} > Create dataset diff --git a/frontend/src/lib/components/aiEvals/EvalRunDialog.svelte b/frontend/src/lib/components/aiEvals/EvalRunDialog.svelte index d4d1714fd0..63fe366628 100644 --- a/frontend/src/lib/components/aiEvals/EvalRunDialog.svelte +++ b/frontend/src/lib/components/aiEvals/EvalRunDialog.svelte @@ -47,16 +47,6 @@ let hasDraft = $derived(editedConfig !== undefined) let dataset = $state(undefined) let hoveringDataset = $state(false) - /** Set while the dialog stands aside for the dataset drawer, holding the pane's dataset as it - * was on the way out: coming back onto a different one is the detour's answer, and adopted. */ - let steppedAside = $state<{ dataset: string | undefined } | undefined>(undefined) - - /** Hands the screen to the dataset drawer, to be given back when it closes. */ - function stepAside(go: () => void) { - steppedAside = { dataset: defaultDataset } - open = false - go() - } /** The agent's versions, for pinning one. Loaded when the dialog opens rather than held: a * version list goes stale the moment the agent is saved again. */ @@ -76,21 +66,26 @@ } } + /** Whether the dialog was already open on the previous run of the effect below, which reads it + * to tell an open from the pane's dataset moving underneath. */ + let wasOpen = false + $effect(() => { - if (!open) return + const isOpen = open + const pane = defaultDataset untrack(() => { - loadVersions() - const aside = steppedAside - steppedAside = undefined - if (aside) { - // Back from the drawer: a dataset created or edited there moves the pane onto it, and - // anything else leaves the field as it was left. - if (defaultDataset !== aside.dataset) dataset = defaultDataset + if (!isOpen) { + wasOpen = false return } - // Seeded on every open: the dataset last worked in, and the state of the agent there is - // most reason to measure. - dataset = defaultDataset + // Followed while the dialog stands, not only read at open: the dataset drawer opens over + // this dialog rather than in place of it, so creating, renaming or deleting a dataset + // there moves the pane's selection with the dialog still up. Nothing else moves it then. + dataset = pane + if (wasOpen) return + wasOpen = true + loadVersions() + // The state of the agent there is most reason to measure. choice = hasDraft ? 'draft' : 'deployed' }) }) @@ -184,7 +179,7 @@ unifiedSize="sm" variant="default" startIcon={{ icon: Plus }} - onclick={() => stepAside(onNewDataset)} + onclick={onNewDataset} > New dataset @@ -215,7 +210,7 @@ title="Edit this dataset" on:click={() => { close() - stepAside(() => onEditDataset(item.value ?? '')) + onEditDataset(item.value ?? '') }} /> {/snippet} @@ -228,7 +223,7 @@ btnClasses="w-full !h-auto !justify-start !rounded-none flex items-center gap-2 px-3 py-2 text-xs !font-normal text-secondary hover:bg-surface-hover" onClick={() => { close() - stepAside(onNewDataset) + onNewDataset() }} > @@ -247,7 +242,7 @@ startIcon={{ icon: Pencil }} iconOnly title="Edit this dataset" - on:click={() => stepAside(() => onEditDataset(dataset ?? ''))} + on:click={() => onEditDataset(dataset ?? '')} /> {/if} diff --git a/frontend/src/lib/components/aiEvals/EvalRunsList.svelte b/frontend/src/lib/components/aiEvals/EvalRunsList.svelte index 5d84ef88ce..b544781902 100644 --- a/frontend/src/lib/components/aiEvals/EvalRunsList.svelte +++ b/frontend/src/lib/components/aiEvals/EvalRunsList.svelte @@ -7,17 +7,21 @@ import Skeleton from '$lib/components/common/skeleton/Skeleton.svelte' import TimeAgo from '$lib/components/TimeAgo.svelte' import { Button } from '$lib/components/common' - import { Bot, Code2, Loader2, Plus } from 'lucide-svelte' + import { Bot, ChevronRight, Code2, Loader2, Plus } from 'lucide-svelte' + import { overlayHostActive, topmostSurface } from '$lib/components/common/overlayHost.svelte' import type { EvalDataset, EvalExperiment, ExperimentScore } from '$lib/gen' import { datasetSummary, experimentName, formatScore, subjectLabel } from './evalUtils' let { experiments, datasets, + caseProgress, loaded, + active = false, deployedHash = undefined, currentVersion = undefined, onOpen, + onHighlight, onEditDataset, onNew }: { @@ -25,17 +29,87 @@ experiments: EvalExperiment[] /** Whether the list has been read: an empty table is a statement about the agent. */ loaded: boolean + /** Whether this list is the page on screen. The keyboard is only answered while it is: the + * run page keeps its own rows, and both would otherwise move on one press. */ + active?: boolean /** The workspace's datasets, for naming the one a run is of by what it is for. */ datasets: EvalDataset[] + /** How many cases each still-running run has finished, keyed by run id. A run the flow has + * not been read for yet is at none of them rather than absent: the count is on the row from + * the moment it appears, so it never arrives late and shifts the column. */ + caseProgress: Record /** What the agent hashes to as deployed, and the version it is on: they resolve a run of * edits that were later saved, so a run is labelled here as the run picker labels it. */ deployedHash?: string currentVersion?: number onOpen: (experiment: EvalExperiment) => void + /** The highlighted run, reported up so the surface can act on it — arrowing into the run + * page opens the run under the highlight rather than whichever was opened last. */ + onHighlight?: (id: string | undefined) => void onEditDataset: (dataset: string) => void onNew: () => void } = $props() + /** The highlighted run, by id. One state for both the pointer and the keyboard, as a melt menu + * does it: hovering a row moves the highlight to it, so the arrows carry on from wherever the + * pointer left off instead of running a second, invisible cursor of their own. It says where + * the highlight is, not what is chosen — a run is not opened until Enter. + * + * By id and not by index: the list is newest-first and the poll prepends to it, so an index + * would quietly come to mean a different run and Enter would open the wrong one. */ + let cursorId = $state(undefined) + let cursor = $derived( + cursorId === undefined ? -1 : experiments.findIndex((e) => e.id === cursorId) + ) + let body: HTMLTableSectionElement | undefined = $state() + + // A window listener answers keys aimed anywhere, so it has to ask two questions the DOM cannot: + // is my host the visible one — session preview tabs stay mounted when hidden — and is my surface + // still the one on top, rather than under a drawer or a dialog opened since. + const hostActive = overlayHostActive() + const onTop = topmostSurface() + const listening = () => hostActive() && onTop() + + $effect(() => { + onHighlight?.(cursorId) + }) + + // A highlight on a run that has since gone, and the highlight itself when the list is not the + // page on screen. + $effect(() => { + if (!active || (cursorId !== undefined && cursor < 0)) cursorId = undefined + }) + + function move(by: number) { + if (experiments.length === 0) return + const from = cursor < 0 ? (by > 0 ? -1 : experiments.length) : cursor + const at = Math.max(0, Math.min(experiments.length - 1, from + by)) + cursorId = experiments[at]?.id + // `nearest`, so arrowing through a long list scrolls by a row rather than jumping the table. + requestAnimationFrame(() => + body?.querySelectorAll('tr')[at]?.scrollIntoView({ block: 'nearest' }) + ) + } + + function onKeydown(event: KeyboardEvent) { + if (!active || !listening() || event.metaKey || event.ctrlKey || event.altKey) return + const el = event.target as HTMLElement | null + if (el?.closest?.('input, textarea, select, [contenteditable="true"], [role="listbox"]')) return + if (event.key === 'ArrowDown') { + event.preventDefault() + move(1) + } else if (event.key === 'ArrowUp') { + event.preventDefault() + move(-1) + } else if (event.key === 'Enter' && experiments[cursor]) { + // Enter belongs to whatever is focused if that thing does something with it. A highlighted + // row is not a reason to swallow the press on `New evaluation` or a row's dataset button. + if (el?.closest?.('button, a[href], [role="button"], summary')) return + event.preventDefault() + onOpen(experiments[cursor]) + } + } + /** The one number a column reports: a pass rate where it has a line to pass, the mean where it * does not. */ function headline(score: ExperimentScore): string | undefined { @@ -44,13 +118,16 @@ } + + - + + @@ -58,12 +135,20 @@ Dataset Cases Scores - When + When + - - {#each experiments as experiment (experiment.id)} - onOpen(experiment)}> + + {#each experiments as experiment, i (experiment.id)} + + onOpen(experiment)} + on:hover={(e) => e.detail && (cursorId = experiment.id)} + >
@@ -99,7 +184,18 @@ - {experiment.case_count} + {#if experiment.running} + + + + {caseProgress[experiment.id] ?? 0}/{experiment.case_count} + + {:else} + {experiment.case_count} + {/if}
@@ -117,8 +213,6 @@ {value} {:else if score.failed > 0} failed - {:else if experiment.running} - {:else} — {/if} @@ -126,33 +220,34 @@ {/each} {#if (experiment.scores ?? []).length === 0} - {#if experiment.running} - - - scoring - - {:else} - not scored - {/if} + + not scored {/if}
- + + + + {/each} {#if experiments.length === 0 && !loaded} - + {:else if experiments.length === 0} - +
No runs yet diff --git a/frontend/src/lib/components/aiEvals/EvalScorers.svelte b/frontend/src/lib/components/aiEvals/EvalScorers.svelte index 68287b0fbc..d00e52d77e 100644 --- a/frontend/src/lib/components/aiEvals/EvalScorers.svelte +++ b/frontend/src/lib/components/aiEvals/EvalScorers.svelte @@ -196,26 +196,31 @@
-
+
{#if scorers.length === 0} -
+
A scorer reads one run and returns a number. Every run of this dataset is measured by all of them, which is what makes two runs comparable.
{:else} -
+ +
{#each scorers as scorer (scorer.id)} -
+
{#if scorer.kind === 'agent'} - + {:else} - + {/if}
- + {scorerLabel(scorer)} - {scorer.path} + {scorer.path}
{#if scorer.pass_if != undefined} diff --git a/frontend/src/lib/components/aiEvals/EvalsPane.svelte b/frontend/src/lib/components/aiEvals/EvalsPane.svelte index d9e5b16fe4..55792ba8e2 100644 --- a/frontend/src/lib/components/aiEvals/EvalsPane.svelte +++ b/frontend/src/lib/components/aiEvals/EvalsPane.svelte @@ -9,9 +9,11 @@ import Label from '$lib/components/Label.svelte' import Popover from '$lib/components/Popover.svelte' import { Splitpanes, Pane } from 'svelte-splitpanes' + import AnimatedPane from '$lib/components/splitPanes/AnimatedPane.svelte' import { type AgentDraft, AiEvalsService, + JobService, type EvalCase, type EvalDataset, type EvalExperiment, @@ -35,9 +37,11 @@ Code2, ExternalLink } from 'lucide-svelte' + import PagedContent from '$lib/components/common/modal/PagedContent.svelte' import EvalDatasetDrawer from './EvalDatasetDrawer.svelte' import EvalRunsList from './EvalRunsList.svelte' import EvalRunDialog from './EvalRunDialog.svelte' + import Skeleton from '$lib/components/common/skeleton/Skeleton.svelte' import GfmMarkdown from '$lib/components/GfmMarkdown.svelte' import { caseLabel, @@ -52,6 +56,10 @@ /** A dataset is capped at this many cases, so one page holds the whole set. */ const CASE_PAGE_SIZE = 1000 + /** The id the run's flow gives the loop over its cases (`CASES_NODE_ID` in `ai_evals/run.rs`). + * Looked up by id rather than by position: the flow has a step after the loop too. */ + const CASES_MODULE_ID = 'cases' + let { agentPath, opWorkspace = undefined, @@ -91,12 +99,56 @@ /** What the agent hashes to as deployed: a run of edits carrying it ran what was then saved. */ let deployedHash = $state(undefined) let running = $state(false) - let scorers = $derived(dataset?.scorers ?? []) + /** The run on screen belongs to a dataset still being read. Until it arrives the rows and the + * scorer columns would both be built from the *previous* dataset, so both are held back. */ + let datasetLoading = $state(false) + let scorers = $derived(datasetLoading ? [] : (dataset?.scorers ?? [])) let selectedCaseId = $state(undefined) let datasetDrawer: EvalDatasetDrawer | undefined = $state() let runDialogOpen = $state(false) - let resumeRunDialog = $state(false) + /** The run the list has highlighted, so arrowing into the run page opens that one. Without it + * the arrow could only fall back to whichever run was opened last, which on a dialog just + * opened is none at all. */ + let highlightedRunId = $state(undefined) + + /** How many cases each still-running run has finished, keyed by run id. Read from the flow + * executing the run: the list carries the case total, and counting the finished ones there + * would be a per-case query for every run listed. The flow already records it — one slot per + * case in `flow_jobs_success`, null until that case's iteration is over. */ + let caseProgress = $state>({}) + + async function readCaseProgress() { + const workspace = ws + const live = experiments.filter((e) => e.running) + if (!workspace || live.length === 0) { + if (Object.keys(caseProgress).length > 0) caseProgress = {} + return + } + const read = await Promise.all( + live.map(async (e) => { + try { + const update = await JobService.getJobUpdates({ + workspace, + id: e.run_job_id, + running: true, + noLogs: true + }) + const cases = update.flow_status?.modules?.find((m) => m.id === CASES_MODULE_ID) + if (!cases) return undefined + return [ + e.id, + (cases.flow_jobs_success ?? []).filter((s) => s != undefined).length + ] as const + } catch { + // Left out of the map, so the row reads `0/total` until a later poll answers. A + // flow that cannot be read is already the list's problem to report, not this one's. + return undefined + } + }) + ) + caseProgress = Object.fromEntries(read.filter((e) => e !== undefined)) + } let experiment = $derived(experiments.find((e) => e.id === experimentId)) @@ -120,6 +172,7 @@ runsLoadError = false try { experiments = await listSubjectExperiments() + await readCaseProgress() } catch (e) { runsLoadError = true sendUserToast(`Failed to load the runs: ${e}`, true) @@ -183,6 +236,8 @@ // Switching datasets leaves the previous request in flight; only the newest may write, or a // slow response for the dataset you just left replaces the one you are looking at. let loadGeneration = 0 + /** Which run the pane is opening; only the newest may clear `datasetLoading`. */ + let openGeneration = 0 async function loadDataset(path: string | undefined): Promise { const generation = ++loadGeneration @@ -345,6 +400,7 @@ await loadResults() } else { experiments = await listSubjectExperiments() + await readCaseProgress() } } finally { refreshing = false @@ -354,27 +410,46 @@ /** Opens a run, bringing its dataset with it and offering the run before it as the baseline. * Reading the cells is left to the effect on the selection, so every way in opens one alike. */ async function openRun(id: string) { - // Against the dataset that is loaded, not the one that is selected: skipping on the selection - // alone would leave a run open over a dataset whose cases and scorers were never read. const target = experiments.find((e) => e.id === id) - if (target && target.dataset !== dataset?.path) { - await useDataset(target.dataset) + // Only when the run itself changes: re-showing the one already open — arrowing back into it + // from the list — must keep whatever comparison the user picked. + if (id !== experimentId) { + const index = experiments.findIndex((e) => e.id === id) + // The run before it *of the same dataset*: the list spans datasets, and a run of another + // set of cases is not a baseline for this one. + baselineId = experiments.slice(index + 1).find((e) => e.dataset === target?.dataset)?.id } - const index = experiments.findIndex((e) => e.id === id) - // The run before it *of the same dataset*: the list spans datasets, and a run of another set - // of cases is not a baseline for this one. - baselineId = experiments.slice(index + 1).find((e) => e.dataset === target?.dataset)?.id experimentId = id - viewingRun = true selectedCaseId = undefined + // Opened first, read second: the dataset is a request, and waiting on it here is a click + // that does nothing at all until the network answers. The page carries the wait instead. + // + // Against what is *selected* as well as what is loaded. `selectedDataset` moves the moment a + // read starts, so a load still in flight for another dataset shows up here: without that + // test, opening a run of the dataset already committed would skip `useDataset` entirely and + // leave the in-flight one free to commit its cases under this run. + const needsDataset = + !!target && (target.dataset !== dataset?.path || target.dataset !== selectedDataset) + datasetLoading = needsDataset + viewingRun = true + if (needsDataset) { + // Numbered like `loadDataset`'s own read, and for the same reason: opening a second run + // while the first is still loading leaves two `finally`s racing, and the loser clearing + // the flag would uncover the table with neither dataset in hand. + const generation = ++openGeneration + try { + await useDataset(target!.dataset) + } finally { + if (generation === openGeneration) datasetLoading = false + } + } } async function runAll(runSubject: EvalSubject, path: string): Promise { if (!ws || !path) return false running = true - let id: string try { - id = await AiEvalsService.runExperiment({ + await AiEvalsService.runExperiment({ workspace: ws, requestBody: { dataset: path, subject: runSubject } }) @@ -386,9 +461,10 @@ // From here the run exists and is billing: what can still fail is reading it back, and // saying "failed to run" to that invites a second, duplicate run. try { + // Onto the list rather than into the run: a run that has just started has no answers and + // no scores, and the list already fills its row in as they land. Reading it is a click. if (path !== dataset?.path) await useDataset(path) await loadRuns() - await openRun(id) } catch (e) { sendUserToast( `The run started but could not be read back: ${e}. Reload the runs list to see it.`, @@ -407,8 +483,6 @@ /** The dataset is gone and every run of it with it: back to the list, on no dataset. */ async function datasetDeleted(path: string) { - // A run dialog waiting behind the drawer has nothing to come back to. - resumeRunDialog = false if (selectedDataset === path) { viewingRun = false selectedCaseId = undefined @@ -442,6 +516,13 @@ } let selectedRow = $derived(displayRows.find((row) => row.case_id === selectedCaseId)) + /** The case the side panel is showing. Held rather than read straight off the selection: the + * pane animates shut over a few hundred milliseconds, and the selection is gone on the first + * of them, which would empty the panel before it had finished closing. */ + let openRow = $state(undefined) + $effect(() => { + if (selectedRow) openRow = selectedRow + }) $effect(() => { if (!ws) return @@ -570,34 +651,52 @@
+ {#if loaded && loadError} +
+ Could not load evals + + The datasets or runs could not be read. Check your access to this agent and reload. + +
+ {:else} + + { + // Right opens the run under the highlight, falling back to whichever was open before; + // left is the way back, the same as the breadcrumb. + if (key === 'run') { + // Both branches go through `openRun`: it is what brings the run's own dataset back, + // and the fallback run may be of a dataset the list has since moved off. + const id = highlightedRunId ?? experimentId + if (id) openRun(id) + } else if (key === 'list') { + viewingRun = false + selectedCaseId = undefined + } + }} + pages={[ + { key: 'list', content: listPage }, + { key: 'run', content: runPage } + ]} + /> + {/if} +
+ +{#snippet listPage()} + +

+ Each run answers a dataset of cases with this agent and scores the answers, so runs can be + compared. +

- {#if viewingRun} - - {/if}
- {#if viewingRun && experiment?.run_job_id} - - Open the job - - - {/if} - {#if !viewingRun && loaded && datasets.length > 0} + {#if loaded && datasets.length > 0} -
- {:else if !viewingRun || !loaded} - openRun(e.id)} - onEditDataset={async (path) => { - if (await useDataset(path)) datasetDrawer?.openDrawer('edit') - }} - onNew={() => (runDialogOpen = true)} - /> - {:else} - - - - - {#each scorers as scorer (scorer.id)} - - {/each} - - - - Case - Answer - {#each scorers as scorer, index (scorer.id)} - {@const mean = means.find((m) => m.scorer_id === scorer.id)} - {@const headline = columnHeadline(scorer, mean)} - - -
- - {#if scorer.kind === 'agent'} - - {:else} - - {/if} - {scorerLabel(scorer)} - - - {#if headline} - - {headline.value} + + + + + {#each scorers as scorer (scorer.id)} + + {/each} + + + + Case + Answer + {#each scorers as scorer, index (scorer.id)} + {@const mean = means.find((m) => m.scorer_id === scorer.id)} + {@const headline = columnHeadline(scorer, mean)} + + +
+ + {#if scorer.kind === 'agent'} + + {:else} + + {/if} + {scorerLabel(scorer)} + + + {#if headline} + + {headline.value} + + {#if headline.delta && headline.direction !== 0} + 0 ? 'text-green-500' : headline.direction < 0 ? 'text-red-500' : 'text-tertiary'}`} + > + {headline.delta} - {#if headline.delta && headline.direction !== 0} - 0 ? 'text-green-500' : headline.direction < 0 ? 'text-red-500' : 'text-tertiary'}`} - > - {headline.delta} - - {/if} {/if} - -
- - {/each} + {/if} + +
+ + {/each} + + + + {#if datasetLoading} + + + + + - - + {:else} {#each displayRows as row (row.case_id)} {@const status = statusOf(row.status)} {/each} - - - {/if} + {/if} + +
- {#if selectedRow} - {@const openRow = selectedRow} - + + + {#if openRow}
-
- + +
+ {openRow.input?.user_message ?? caseLabel(openRow)} -
- {#if openRow.job_id} - - Open the case job - - - {/if} -
-
+ +
{#if openRow.expected != undefined && openRow.expected !== ''} - +
+
+ Expected +
+
+ + {typeof openRow.expected === 'string' + ? openRow.expected + : JSON.stringify(openRow.expected, null, 2)} + +
+
{/if} {#if scorers.length > 0 && openRow.scores.length > 0} -
-
+{/snippet} { - if (await useDataset(path)) { - resumeRunDialog = true - datasetDrawer?.openDrawer('edit') - } - }} - onNewDataset={() => { - resumeRunDialog = true - datasetDrawer?.openDrawer('new') + if (await useDataset(path)) datasetDrawer?.openDrawer('edit') }} + onNewDataset={() => datasetDrawer?.openDrawer('new')} /> { - if (!resumeRunDialog) return - resumeRunDialog = false - // On the dataset the drawer was just in: the dialog opens on the pane's own, which - // creating or editing one has already moved to it. - runDialogOpen = true - }} /> + + diff --git a/frontend/src/lib/components/apps/components/display/table/multilineCellEditor.css b/frontend/src/lib/components/apps/components/display/table/multilineCellEditor.css deleted file mode 100644 index 1ac9f8b31c..0000000000 --- a/frontend/src/lib/components/apps/components/display/table/multilineCellEditor.css +++ /dev/null @@ -1,26 +0,0 @@ -/* MultilineCellEditor: a popup positioned over the cell, so it has to paint the cell's own frame - rather than inherit it. */ -.ag-theme-alpine .wm-multiline-cell-editor, -.ag-theme-alpine-dark .wm-multiline-cell-editor { - background-color: var(--ag-background-color); -} -.ag-theme-alpine .wm-multiline-cell-editor textarea, -.ag-theme-alpine-dark .wm-multiline-cell-editor textarea { - display: block; - box-sizing: border-box; - /* Horizontal only: the vertical padding is set by the editor, which knows the height of the row - it is replacing. `line-height` here is what it computes against. */ - padding: 0 calc(var(--ag-cell-horizontal-padding) - 1px); - border: 1px solid var(--ag-input-focus-border-color); - border-radius: 3px; - outline: none; - resize: none; - /* Past this it scrolls rather than growing. */ - max-height: 40vh; - overflow-y: auto; - background-color: var(--ag-background-color); - color: var(--ag-foreground-color); - font: inherit; - line-height: 20px; - white-space: pre-wrap; -} diff --git a/frontend/src/lib/components/apps/components/display/table/multilineCellEditor.ts b/frontend/src/lib/components/apps/components/display/table/multilineCellEditor.ts deleted file mode 100644 index 00975955c9..0000000000 --- a/frontend/src/lib/components/apps/components/display/table/multilineCellEditor.ts +++ /dev/null @@ -1,108 +0,0 @@ -import type { ColDef, ICellEditorComp, ICellEditorParams } from 'ag-grid-community' -// Beside the editor rather than in the AgGrid theme: that file is the vendored theme, and a rule -// added to it is one the next copy of it drops. -import './multilineCellEditor.css' - -/** Kept in step with the `line-height` the stylesheet gives the textarea. */ -const LINE_HEIGHT = 20 - -/** - * A text cell editor that starts the height of the cell and grows as lines are added, for columns - * holding prose rather than a value. Enter commits, Shift+Enter adds a line, Escape cancels. - * - * Rendered as a popup positioned over the cell: an in-cell editor is clipped to the row height, so - * growing is only visible if the editor is allowed to paint outside it. - */ -export class MultilineCellEditor implements ICellEditorComp { - private eGui!: HTMLDivElement - private textarea!: HTMLTextAreaElement - private params!: ICellEditorParams - private wasEmpty = false - - init(params: ICellEditorParams) { - this.params = params - this.eGui = document.createElement('div') - this.eGui.className = 'wm-multiline-cell-editor' - - this.wasEmpty = params.value == undefined - - this.textarea = document.createElement('textarea') - this.textarea.rows = 1 - // A keystroke that opened the edit replaces the value, as it does in every other cell; F2 - // and double-click keep it to be edited. - this.textarea.value = params.eventKey?.length === 1 ? params.eventKey : (params.value ?? '') - this.textarea.style.width = `${params.column.getActualWidth() - 2}px` - // Padded so one line fills the cell it replaces and a second costs a line rather than a row. - // From the row rather than from `--ag-row-height`, which is the theme's figure and not - // necessarily this grid's. - const rowHeight = params.node.rowHeight ?? 28 - const padding = Math.max(0, (rowHeight - LINE_HEIGHT - 2) / 2) - this.textarea.style.paddingTop = `${padding}px` - this.textarea.style.paddingBottom = `${padding}px` - - this.textarea.addEventListener('input', () => this.resize()) - this.textarea.addEventListener('keydown', (e) => { - if (e.key === 'Escape') { - // Kept from whatever is around the grid: a grid in a drawer or a dialog is under a - // surface that closes on Escape, and leaving an edit is not asking to leave that. - e.preventDefault() - e.stopPropagation() - this.params.api.stopEditing(true) - return - } - if (e.key !== 'Enter' || e.isComposing) return - // Both branches keep the key from the grid, which ends the edit on Enter whether or not - // Shift is held: Shift+Enter falls through to the textarea's own newline, and plain Enter - // ends the edit here instead. - e.stopPropagation() - if (!e.shiftKey) { - e.preventDefault() - this.params.stopEditing() - } - }) - this.eGui.appendChild(this.textarea) - } - - private resize() { - this.textarea.style.height = 'auto' - this.textarea.style.height = `${this.textarea.scrollHeight}px` - } - - getGui() { - return this.eGui - } - - afterGuiAttached() { - this.resize() - this.textarea.focus() - // At the end rather than selected: a selection is a keystroke away from erasing the cell. - const end = this.textarea.value.length - this.textarea.setSelectionRange(end, end) - } - - getValue() { - // Nothing typed into a cell that held nothing is not an edit: returning '' here would write - // an empty string over a null, which the grid would see as a change and commit. - if (this.wasEmpty && this.textarea.value === '') return this.params.value - return this.textarea.value - } - - isPopup() { - return true - } - - getPopupPosition(): 'over' | 'under' { - return 'over' - } -} - -/** - * What a column of prose needs, ready to spread into a colDef. `suppressKeyboardEvent` as well as - * the editor: the grid ends an edit on Enter from a handler a popup editor's DOM does not sit - * under, so the editor cannot keep Shift+Enter for itself on its own. - */ -export const multilineCellColDef: Pick = { - cellEditor: MultilineCellEditor, - suppressKeyboardEvent: (p) => - p.editing && (p.event as KeyboardEvent).key === 'Enter' && (p.event as KeyboardEvent).shiftKey -} diff --git a/frontend/src/lib/components/common/EditableTextarea.svelte b/frontend/src/lib/components/common/EditableTextarea.svelte new file mode 100644 index 0000000000..26a6574027 --- /dev/null +++ b/frontend/src/lib/components/common/EditableTextarea.svelte @@ -0,0 +1,173 @@ + + + +{#if editing} + + +{:else} + + +{/if} diff --git a/frontend/src/lib/components/common/drawer/Drawer.svelte b/frontend/src/lib/components/common/drawer/Drawer.svelte index d375bf506e..6bb5358940 100644 --- a/frontend/src/lib/components/common/drawer/Drawer.svelte +++ b/frontend/src/lib/components/common/drawer/Drawer.svelte @@ -8,6 +8,7 @@ import { onMount, createEventDispatcher, setContext, untrack } from 'svelte' import { BROWSER } from 'esm-env' import Disposable from './Disposable.svelte' + import { setTopmostSurface } from '$lib/components/common/overlayHost.svelte' import ConditionalPortal from './ConditionalPortal.svelte' import { chatState } from '$lib/components/copilot/chat/sharedChatState.svelte' import { useReducedMotion } from '$lib/svelte5Utils.svelte' @@ -51,6 +52,11 @@ let disposable: Disposable | undefined = $state(undefined) + // A drawer stacks like a dialog does, so content inside it gets the same answer about whether + // its keys are meant for it. Without this, a drawer opened over a dialog would inherit the + // dialog's answer — false, because the drawer itself is now on top — and go deaf. + setTopmostSurface(() => disposable?.isTopmost() ?? true) + let reducedMotion = useReducedMotion() let duration = $derived(reducedMotion.val ? 0 : _duration) let durationMs = $derived(duration * 1000) diff --git a/frontend/src/lib/components/common/emptyState/EmptyState.svelte b/frontend/src/lib/components/common/emptyState/EmptyState.svelte index 8715065ad0..49c3cd2b77 100644 --- a/frontend/src/lib/components/common/emptyState/EmptyState.svelte +++ b/frontend/src/lib/components/common/emptyState/EmptyState.svelte @@ -10,6 +10,16 @@ label: string icon?: any onClick: () => void + /** + * `default` unless the surface has no other live call to action. Accent is for the case + * where this button is the only thing to press — a form whose submit is disabled until + * this is done, say — so it is not competing with one. + */ + variant?: 'default' | 'accent' + /** Same write lock the surface's other controls take. An empty state is still a live + * control: without this it stays clickable while a request that has already read the + * empty list is in flight, and whatever it adds is discarded when that request lands. */ + disabled?: boolean aiId?: string aiDescription?: string } @@ -32,11 +42,13 @@ {/if}
{#if action} - +
diff --git a/frontend/src/lib/components/common/modal/PagedContent.svelte b/frontend/src/lib/components/common/modal/PagedContent.svelte new file mode 100644 index 0000000000..430a53e66f --- /dev/null +++ b/frontend/src/lib/components/common/modal/PagedContent.svelte @@ -0,0 +1,202 @@ + + + + + + + +
+ {#each pages as page, i (page.key)} + {#if visited.includes(page.key) || warmed} + + +
+ {@render page.content()} +
+ {/if} + {/each} +
+ + diff --git a/frontend/src/lib/components/common/overlayHost.svelte.ts b/frontend/src/lib/components/common/overlayHost.svelte.ts index 0e76709cbf..7c4374115b 100644 --- a/frontend/src/lib/components/common/overlayHost.svelte.ts +++ b/frontend/src/lib/components/common/overlayHost.svelte.ts @@ -72,3 +72,23 @@ export function overlayHostActive(): () => boolean { const host = getOverlayHost() return () => host?.active() ?? true } + +const TOPMOST_SURFACE_KEY = 'topmostSurface' + +/** + * Declare whether the surface enclosing this subtree is the one on top. Set by whatever owns the + * stacking — a dialog, a drawer — so content inside it can tell a key meant for itself from one + * meant for something opened over it. + */ +export function setTopmostSurface(isTopmost: () => boolean) { + setContext(TOPMOST_SURFACE_KEY, isTopmost) +} + +/** + * Whether the enclosing surface is on top. True when nothing declared otherwise, so content that + * is not inside such a surface is not silently made deaf. + */ +export function topmostSurface(): () => boolean { + const isTopmost = getContext<(() => boolean) | undefined>(TOPMOST_SURFACE_KEY) + return () => isTopmost?.() ?? true +} diff --git a/frontend/src/lib/components/text_input/TextInput.svelte b/frontend/src/lib/components/text_input/TextInput.svelte index c57f57120a..03fcedbd8e 100644 --- a/frontend/src/lib/components/text_input/TextInput.svelte +++ b/frontend/src/lib/components/text_input/TextInput.svelte @@ -81,6 +81,11 @@ size?: ButtonType.UnifiedSize unifiedHeight?: boolean underlyingInputEl?: UnderlyingInputElT + /** + * Passed to the `autosize` action on the `textarea` variant. Chiefly `minHeight: 0`, for a + * field that hugs one line instead of reserving the action's 30px floor. + */ + autosizeParams?: import('$lib/autosize').AutosizeParams } export function focus() { @@ -108,7 +113,8 @@ error, size = 'md', unifiedHeight = true, - underlyingInputEl: _underlyingInputEl + underlyingInputEl: _underlyingInputEl, + autosizeParams }: Props = $props() let underlyingInputEl = $derived(_underlyingInputEl ?? ('input' as const)) @@ -152,7 +158,7 @@ onpointerdown={(e) => e.stopImmediatePropagation()} bind:this={inputEl} bind:value - use:autosize + use:autosize={autosizeParams} > {:else if underlyingInputEl === 'input'}