From 77adf85ccd512aad3ea54362bbaceae240e5c8a0 Mon Sep 17 00:00:00 2001 From: AlexRV12 <71396855+AlexRV12@users.noreply.github.com> Date: Mon, 10 Aug 2026 19:51:29 +0200 Subject: [PATCH] feat: version history for session artifacts (#10574) * fix: never replace an in-flight indexeddb open, only a settled one Co-Authored-By: Claude Opus 5 (1M context) * feat: keep a version history for session artifacts Co-Authored-By: Claude Opus 5 (1M context) * feat: let the assistant browse an artifact's earlier versions Co-Authored-By: Claude Opus 5 (1M context) * feat: pick an older artifact version from the preview panel Co-Authored-By: Claude Opus 5 (1M context) * test(ai_evals): cover the change note the assistant writes on each edit Co-Authored-By: Claude Opus 5 (1M context) * fix: bound every indexeddb open, not only one told it is blocked --------- Co-authored-by: Claude Opus 5 (1M context) --- .../core/global/evalArtifactHelpers.test.ts | 22 +++ .../frontend/core/global/evalArtifactStore.ts | 93 ++++++++++ .../frontend/core/global/globalEvalRunner.ts | 62 +------ ai_evals/cases/global.yaml | 32 ++++ ai_evals/core/types.ts | 6 + ai_evals/core/validators.test.ts | 46 +++++ ai_evals/core/validators.ts | 13 ++ frontend/src/lib/components/TimeAgo.svelte | 55 +++++- .../artifacts/ArtifactVersionPicker.svelte | 109 +++++++++++ .../chat/artifacts/ArtifactViewer.svelte | 104 ++++++++++- .../chat/artifacts/ArtifactsSegment.svelte | 10 +- .../chat/artifacts/artifactTools.test.ts | 77 +++++++- .../copilot/chat/artifacts/artifactTools.ts | 90 ++++++++- .../chat/artifacts/artifactsDB.test.ts | 88 ++++++++- .../copilot/chat/artifacts/artifactsDB.ts | 170 +++++++++++++++-- .../chat/artifacts/artifactsState.svelte.ts | 80 +++++++- .../chat/artifacts/artifactsState.test.ts | 64 +++++++ .../components/copilot/chat/global/core.ts | 9 +- .../components/sessions/PreviewTabHost.svelte | 4 +- frontend/src/lib/userScopedDb.test.ts | 108 +++++++++++ frontend/src/lib/userScopedDb.ts | 175 +++++++++++++++--- 21 files changed, 1286 insertions(+), 131 deletions(-) create mode 100644 ai_evals/adapters/frontend/core/global/evalArtifactHelpers.test.ts create mode 100644 ai_evals/adapters/frontend/core/global/evalArtifactStore.ts create mode 100644 frontend/src/lib/components/copilot/chat/artifacts/ArtifactVersionPicker.svelte diff --git a/ai_evals/adapters/frontend/core/global/evalArtifactHelpers.test.ts b/ai_evals/adapters/frontend/core/global/evalArtifactHelpers.test.ts new file mode 100644 index 0000000000..c74f341735 --- /dev/null +++ b/ai_evals/adapters/frontend/core/global/evalArtifactHelpers.test.ts @@ -0,0 +1,22 @@ +import { describe, expect, it } from "bun:test"; +import { createEvalArtifactHelpers } from "./evalArtifactStore"; + +// A hand-written stand-in for SessionArtifactsStore (bun has no IndexedDB), so nothing +// makes it follow that class. A method missing from it surfaces as a tool throwing +// part-way through an eval run, which reads as a model failure rather than a harness one. +describe("eval artifact store", () => { + it("exposes every method the artifact tools call", () => { + const { helpers } = createEvalArtifactHelpers(); + for (const method of [ + "create", + "get", + "update", + "remove", + "listForSession", + "listVersions", + "getVersion", + ]) { + expect(typeof (helpers.artifacts as any)[method]).toBe("function"); + } + }); +}); diff --git a/ai_evals/adapters/frontend/core/global/evalArtifactStore.ts b/ai_evals/adapters/frontend/core/global/evalArtifactStore.ts new file mode 100644 index 0000000000..ed67cce468 --- /dev/null +++ b/ai_evals/adapters/frontend/core/global/evalArtifactStore.ts @@ -0,0 +1,93 @@ +// SessionArtifactsStore can't run here (bun has no IndexedDB, nor the compiled $state runes), +// so mirror only the shape the artifact tools call, not its scoping or race handling. +export const EVAL_SESSION_ID = "eval-session"; +export function createEvalArtifactHelpers() { + const items = new Map>(); + // Snapshots per artifact id, oldest first — the version tools read history from here. + const history = new Map>>(); + let seq = 0; + const snapshotOf = ( + artifact: Record, + version: number, + note?: string, + ) => ({ + key: `${artifact.id}:${version}`, + artifactId: artifact.id, + version, + name: artifact.name, + content: artifact.content, + savedAt: artifact.updatedAt, + note, + }); + const store = { + create: async (sessionId: string, input: Record) => { + const now = seq++; + const artifact = { + id: `eval-artifact-${now}`, + sessionId, + chatId: input.chatId, + kind: input.kind ?? "md", + name: input.name, + content: input.content, + createdAt: now, + updatedAt: now, + version: 1, + }; + items.set(artifact.id, artifact); + history.set(artifact.id, [snapshotOf(artifact, 1)]); + return artifact; + }, + get: async (id: string) => items.get(id), + update: async ( + id: string, + input: Record, + opts?: { sessionId?: string }, + ) => { + const existing = items.get(id); + if (!existing) return undefined; + if ( + opts?.sessionId !== undefined && + existing.sessionId !== opts.sessionId + ) + return undefined; + // Only a content change earns a version, as in SessionArtifactsStore. + const contentChanged = + input.content !== undefined && input.content !== existing.content; + const version = (existing.version ?? 1) + (contentChanged ? 1 : 0); + const updated = { + ...existing, + name: input.name ?? existing.name, + content: input.content ?? existing.content, + updatedAt: seq++, + version, + }; + items.set(id, updated); + if (contentChanged) { + history.set(id, [ + ...(history.get(id) ?? []), + snapshotOf(updated, version, input.note), + ]); + } + return updated; + }, + remove: async (id: string) => { + items.delete(id); + history.delete(id); + }, + listForSession: async (sessionId: string) => + [...items.values()].filter((a) => a.sessionId === sessionId), + listVersions: async (id: string) => + [...(history.get(id) ?? [])].sort((a, b) => b.version - a.version), + getVersion: async (id: string, version: number) => + (history.get(id) ?? []).find((v) => v.version === version), + }; + return { + helpers: { + artifacts: store, + sessionId: EVAL_SESSION_ID, + getChatId: () => "eval-chat", + openArtifact: () => {}, + }, + snapshot: () => [...items.values()], + }; +} diff --git a/ai_evals/adapters/frontend/core/global/globalEvalRunner.ts b/ai_evals/adapters/frontend/core/global/globalEvalRunner.ts index 8fe28d022a..dcd771663a 100644 --- a/ai_evals/adapters/frontend/core/global/globalEvalRunner.ts +++ b/ai_evals/adapters/frontend/core/global/globalEvalRunner.ts @@ -14,6 +14,7 @@ import { } from "../../../../../frontend/src/lib/components/copilot/chat/global/userDraftAdapter"; import type { Tool as ProductionTool } from "../../../../../frontend/src/lib/components/copilot/chat/shared"; import { UserDraft } from "../../../../../frontend/src/lib/userDraft.svelte"; +import { createEvalArtifactHelpers } from "./evalArtifactStore"; import type { ModeRunContext } from "../../../../core/types"; import type { GlobalDraftState } from "../../../../core/validators"; import type { WindmillBackendSettings } from "../../../../core/windmillBackendSettings"; @@ -43,67 +44,6 @@ const LIVE_EDITOR_ITEM_KINDS = { app: "raw_app", } as const; -// SessionArtifactsStore can't run here (bun has no IndexedDB, nor the compiled $state runes), -// so mirror only its tool-facing shape; its own logic (scoping, race guard) is unit-tested. -const EVAL_SESSION_ID = "eval-session"; -function createEvalArtifactHelpers() { - const items = new Map>(); - let seq = 0; - const store = { - create: async (sessionId: string, input: Record) => { - const now = seq++; - const artifact = { - id: `eval-artifact-${now}`, - sessionId, - chatId: input.chatId, - kind: input.kind ?? "md", - name: input.name, - content: input.content, - createdAt: now, - updatedAt: now, - }; - items.set(artifact.id, artifact); - return artifact; - }, - get: async (id: string) => items.get(id), - update: async ( - id: string, - input: Record, - opts?: { sessionId?: string }, - ) => { - const existing = items.get(id); - if (!existing) return undefined; - if ( - opts?.sessionId !== undefined && - existing.sessionId !== opts.sessionId - ) - return undefined; - const updated = { - ...existing, - name: input.name ?? existing.name, - content: input.content ?? existing.content, - updatedAt: seq++, - }; - items.set(id, updated); - return updated; - }, - remove: async (id: string) => { - items.delete(id); - }, - listForSession: async (sessionId: string) => - [...items.values()].filter((a) => a.sessionId === sessionId), - }; - return { - helpers: { - artifacts: store, - sessionId: EVAL_SESSION_ID, - getChatId: () => "eval-chat", - openArtifact: () => {}, - }, - snapshot: () => [...items.values()], - }; -} - export interface GlobalLiveEditorDraftFixture { type: keyof typeof LIVE_EDITOR_ITEM_KINDS; storagePath?: string; diff --git a/ai_evals/cases/global.yaml b/ai_evals/cases/global.yaml index bc2fa5da00..b2716671b1 100644 --- a/ai_evals/cases/global.yaml +++ b/ai_evals/cases/global.yaml @@ -1140,6 +1140,38 @@ - closes the runs preview tab in the side panel - does not write, deploy, or delete anything +# --- Artifact version history --- +# Every content change to an artifact is snapshotted, and update_artifact requires a +# change_note that the user reads in the version picker. A blank note makes the history +# unreadable, so pin that the model fills it on every edit. + +- id: global-artifact-note-on-each-edit + prompt: |- + Write up a short rollout plan for me as a doc I can come back to, covering a staged + rollout in three phases. Then add a rollback section to it, and after that tighten + the wording of phase 2. + runtime: + maxTurns: 12 + sessionChat: true + validate: + draftCountExactly: 0 + toolExpect: + requiredToolsUsed: + - create_artifact + - update_artifact + forbiddenToolsUsed: + - deploy_workspace_item + - delete_workspace_item + toolCallArgs: + # The note is what the version picker shows; a blank one makes history unreadable. + - tool: update_artifact + field: change_note + nonEmpty: true + skipJudge: true + judgeChecklist: + - creates one artifact and revises it rather than creating a second artifact + - each revision carries a short description of what changed + # --- Documentation search (search_docs) --- # Pure product-knowledge questions: the assistant should consult the docs via # search_docs and answer conversationally, not draft or mutate anything. No diff --git a/ai_evals/core/types.ts b/ai_evals/core/types.ts index 83e9a218f3..f269c5c4eb 100644 --- a/ai_evals/core/types.ts +++ b/ai_evals/core/types.ts @@ -166,6 +166,12 @@ export interface ToolCallArgumentRule { * tool — e.g. SQL where a mutation is mixed with verification SELECTs. */ stringIncludesAnyOf?: string[]; + /** + * Universal over calls: every recorded call to `tool` must carry `field` as a + * non-blank string. Use for a required argument whose value is free text, where + * the point is that the model filled it in at all rather than what it said. + */ + nonEmpty?: boolean; } export interface ToolValidationSpec { diff --git a/ai_evals/core/validators.test.ts b/ai_evals/core/validators.test.ts index d603dae626..96107397b7 100644 --- a/ai_evals/core/validators.test.ts +++ b/ai_evals/core/validators.test.ts @@ -174,6 +174,52 @@ describe("validateToolExpectations", () => { expect(checks.every((check) => check.passed)).toBe(true); }); + it("fails nonEmpty when any call left the field blank", () => { + const checks = validateToolExpectations({ + run: { + success: true, + actual: {}, + assistantMessageCount: 1, + toolCallCount: 2, + toolsUsed: ["update_artifact"], + toolCallDetails: [ + { name: "update_artifact", arguments: { change_note: "Added a rollback section" } }, + // A whitespace-only note is as unreadable in the picker as a missing one. + { name: "update_artifact", arguments: { change_note: " " } }, + ], + skillsInvoked: [], + }, + toolExpect: { + toolCallArgs: [{ tool: "update_artifact", field: "change_note", nonEmpty: true }], + }, + }); + + const nonEmptyCheck = checks.find((c) => c.name.includes("is filled in on every call")); + expect(nonEmptyCheck?.passed).toBe(false); + expect(nonEmptyCheck?.details).toContain("blank on 1 of 2"); + }); + + it("passes nonEmpty when every call filled the field", () => { + const checks = validateToolExpectations({ + run: { + success: true, + actual: {}, + assistantMessageCount: 1, + toolCallCount: 1, + toolsUsed: ["update_artifact"], + toolCallDetails: [ + { name: "update_artifact", arguments: { change_note: "Tightened phase 2" } }, + ], + skillsInvoked: [], + }, + toolExpect: { + toolCallArgs: [{ tool: "update_artifact", field: "change_note", nonEmpty: true }], + }, + }); + + expect(checks.every((check) => check.passed)).toBe(true); + }); + it("accepts a stringIncludesAnyOf substring inside an array-valued field", () => { const checks = validateToolExpectations({ run: { diff --git a/ai_evals/core/validators.ts b/ai_evals/core/validators.ts index 32ff225fc1..fa39d8611a 100644 --- a/ai_evals/core/validators.ts +++ b/ai_evals/core/validators.ts @@ -233,6 +233,19 @@ export function validateToolExpectations(input: { ); } + if (rule.nonEmpty) { + const blankValues = values.filter( + (value) => typeof value !== "string" || value.trim().length === 0 + ); + checks.push( + check( + `${rule.tool}.${rule.field} is filled in on every call`, + blankValues.length === 0, + `blank on ${blankValues.length} of ${values.length} call(s); values: ${summarizeToolValues(values)}` + ) + ); + } + if (rule.stringIncludesAnyOf && rule.stringIncludesAnyOf.length > 0) { // Existential: at least one call must contain one of the substrings. // Other calls to the same tool may do anything — this suits SQL, where a diff --git a/frontend/src/lib/components/TimeAgo.svelte b/frontend/src/lib/components/TimeAgo.svelte index 52423372e8..cf3f019432 100644 --- a/frontend/src/lib/components/TimeAgo.svelte +++ b/frontend/src/lib/components/TimeAgo.svelte @@ -9,6 +9,8 @@ noDate?: boolean isRecent?: boolean noSeconds?: boolean + /** Single-unit form ("2m", "3h", "5d") for rows too narrow for "2 mins ago". */ + compact?: boolean } let { @@ -16,7 +18,8 @@ agoOnlyIfRecent = false, noDate = false, isRecent = $bindable(true), - noSeconds = false + noSeconds = false, + compact = false }: Props = $props() let computedTimeAgo: string | undefined = $state(undefined) @@ -24,7 +27,10 @@ let interval onMount(() => { - // Update every minute for noSeconds mode, every second for regular mode + // compact schedules itself below; it needs no fixed rate. + if (compact) return + + // Update every minute for noSeconds mode, every second otherwise. const intervalMs = noSeconds ? 60000 : 1000 interval = setInterval(() => { computeDate() @@ -40,6 +46,36 @@ } }) + // Waking on the boundary of the unit on screen, rather than at a fixed rate: `2h` only + // changes on the hour, and a row that reads `5d` must not hold a 1s timer to find that + // out. Re-armed when `date` changes, so an item edited to now leaves its day-long wait. + $effect(() => { + if (!compact) return + const at = date + let handle: ReturnType | undefined + const tick = () => { + computeDate() + handle = setTimeout(tick, compactDelayMs(at)) + } + handle = setTimeout(tick, compactDelayMs(at)) + return () => { + handle && clearTimeout(handle) + } + }) + + function compactDelayMs(dateString: string): number { + const secs = secondsAgo(new Date(dateString)) + const left = + secs < 60 + ? 1 + : secs < 3600 + ? 60 - (secs % 60) + : secs < 86_400 + ? 3600 - (secs % 3600) + : 86_400 - (secs % 86_400) + return left * 1000 + } + async function computeDate() { computedTimeAgo = await displayDaysAgo(date) } @@ -80,6 +116,21 @@ async function displayDaysAgo(dateString: string): Promise { const date = new Date(dateString) + if (compact) { + // The s/m/h/d ladder the hand-rolled `ago()` helpers around the codebase use + // (PipelineEventLog, PipelineActivityPanel, IndexerMemorySettings): seconds stay + // meaningful right up to the minute mark. + const secs = secondsAgo(date) + if (secs < 60) return `${secs}s` + const mins = minutesAgo(date) + if (mins < 60) return `${mins}m` + const hours = hoursAgo(date) + if (hours < 24) return `${hours}h` + // Days all the way up, never an absolute date: callers read as " ago", and + // the hand-rolled ago() helpers this mirrors have no date fallback either. + return `${daysAgo(date)}d` + } + // New noSeconds mode if (noSeconds) { const mins = minutesAgo(date) diff --git a/frontend/src/lib/components/copilot/chat/artifacts/ArtifactVersionPicker.svelte b/frontend/src/lib/components/copilot/chat/artifacts/ArtifactVersionPicker.svelte new file mode 100644 index 0000000000..e305d3a80f --- /dev/null +++ b/frontend/src/lib/components/copilot/chat/artifacts/ArtifactVersionPicker.svelte @@ -0,0 +1,109 @@ + + + + {#snippet trigger()} + + v{shown} + + + {/snippet} + + {#snippet content()} +
+ +
+ {#if versions.length > 0 && versions.length < latest} + {versions.length} most recent of {latest} versions + {:else} + Versions + {/if} +
+
+ {#each versions as v (v.version)} + + {/each} +
+
+ {/snippet} + diff --git a/frontend/src/lib/components/copilot/chat/artifacts/ArtifactViewer.svelte b/frontend/src/lib/components/copilot/chat/artifacts/ArtifactViewer.svelte index 2aadcca63d..1ce7fe1d35 100644 --- a/frontend/src/lib/components/copilot/chat/artifacts/ArtifactViewer.svelte +++ b/frontend/src/lib/components/copilot/chat/artifacts/ArtifactViewer.svelte @@ -9,14 +9,68 @@ import { copyToClipboard, download } from '$lib/utils' import CodeDisplay from '../script/CodeDisplay.svelte' import LinkRenderer from '../LinkRenderer.svelte' - import { artifactFilename, artifactMimeType, type PersistedArtifact } from './artifactsDB' + import { + artifactFilename, + artifactMimeType, + currentVersion, + type ArtifactVersion, + type PersistedArtifact + } from './artifactsDB' import { markdownProse } from '$lib/components/markdownProse' + import ArtifactVersionPicker from './ArtifactVersionPicker.svelte' + import type { SessionArtifactsStore } from './artifactsState.svelte' + import { History } from 'lucide-svelte' + import TimeAgo from '$lib/components/TimeAgo.svelte' interface Props { artifact: PersistedArtifact + store: SessionArtifactsStore } - let { artifact }: Props = $props() + let { artifact, store }: Props = $props() + + const latest = $derived(currentVersion(artifact)) + // Nothing to pick between until a second version exists. + const hasHistory = $derived(latest > 1) + + // undefined = following the current version. An explicit pick survives later edits, so + // the AI writing v8 does not yank the reader out of the v3 they chose to read. + let pinned = $state(undefined) + let pinnedContent = $state(undefined) + + // The store hands us a fresh object on every edit, so this effect reruns constantly. + // Compare the id against the last one seen: clearing on every rerun would drop the + // reader's pin the moment the assistant writes a new version. + let pinnedFor: string | undefined + $effect(() => { + if (artifact.id === pinnedFor) return + pinnedFor = artifact.id + pinned = undefined + }) + + $effect(() => { + const version = pinned + if (version === undefined) { + pinnedContent = undefined + return + } + const id = artifact.id + void store.getVersion(id, version).then((snapshot) => { + if (pinned !== version || artifact.id !== id) return + // Pruned out from under the pin (history is capped): fall back to current rather + // than showing an empty document. + if (!snapshot) { + pinned = undefined + return + } + pinnedContent = snapshot + }) + }) + + const shown = $derived(pinnedContent ?? artifact) + // Label from the snapshot that is rendered, not from the one just requested: the read is + // async, so `pinned` names a version the body has not swapped to yet. + const shownVersion = $derived(pinnedContent?.version) // Markdown is the only rendered kind in v1; anything else shows source only. const canPreview = $derived(artifact.kind === 'md') @@ -25,12 +79,16 @@ let copied = $state(false) async function copyRaw() { - if (!(await copyToClipboard(artifact.content))) return + if (!(await copyToClipboard(shown.content))) return copied = true setTimeout(() => (copied = false), 1500) } function downloadFile() { - download(artifactFilename(artifact), artifact.content, artifactMimeType(artifact.kind)) + download( + artifactFilename({ name: shown.name, kind: artifact.kind }), + shown.content, + artifactMimeType(artifact.kind) + ) } const plugins = [gfmPlugin(), { renderer: { pre: CodeDisplay, a: LinkRenderer } }] @@ -40,9 +98,17 @@
- - {artifact.name} + + {shown.name} + {#if hasHistory} + (pinned = v)} + /> + {/if}
@@ -78,11 +144,31 @@
+ {#if pinnedContent} + +
+ + + + Viewing v{shownVersion} of {latest} · saved + ago + +
+ +
+
+ {/if} +
{#if source} - {#key `${artifact.id}:${artifact.updatedAt}`} - + {#key `${artifact.id}:${pinnedContent ? `v${pinnedContent.version}` : artifact.updatedAt}`} + {/key} {:else}