From 6e1ef93f329cb396ffc3df3304d592e8fa0e0e71 Mon Sep 17 00:00:00 2001 From: Guilhem Date: Thu, 17 Sep 2026 14:55:45 +0200 Subject: [PATCH] feat: let test_run_flow name the conversation of a chat-mode test run (#11198) * feat: let test_run_flow name the conversation of a chat-mode test run Co-Authored-By: Claude Opus 5 (1M context) * fix: refuse a non-UUID conversation_id and mint chat test-run ids in one place Co-Authored-By: Claude Opus 5 (1M context) * test(ai-evals): add a chat-flow follow-up case and mock flow preview runs Co-Authored-By: Claude Opus 5 (1M context) * test: expect the conversation id argument on the manager's flow test bridge Co-Authored-By: Claude Opus 5 (1M context) * test(ai-evals): require the chat-flow follow-up runs to share one conversation id Co-Authored-By: Claude Opus 5 (1M context) * refactor: name the test_run_flow argument memory_id after the run parameter Co-Authored-By: Claude Opus 5 (1M context) --------- Co-authored-by: Claude Opus 5 (1M context) --- ai_evals/README.md | 4 + ai_evals/adapters/frontend/mockBackend.ts | 23 +++++ .../adapters/frontend/vitestAdapter.test.ts | 9 ++ ai_evals/cases/global.yaml | 26 +++++ ai_evals/core/types.ts | 7 ++ ai_evals/core/validators.test.ts | 26 +++++ ai_evals/core/validators.ts | 17 ++++ .../global/initial/support_chat_flow.json | 59 +++++++++++ .../copilot/chat/AIChatManager.svelte.ts | 4 +- .../copilot/chat/AIChatManager.test.ts | 4 +- .../copilot/chat/flow/FlowAIChat.svelte | 3 +- .../copilot/chat/global/core.test.ts | 98 ++++++++++++++++++- .../components/copilot/chat/global/core.ts | 43 ++++++-- 13 files changed, 310 insertions(+), 13 deletions(-) create mode 100644 ai_evals/fixtures/frontend/global/initial/support_chat_flow.json diff --git a/ai_evals/README.md b/ai_evals/README.md index ee8f730fc3..603f28e5f9 100644 --- a/ai_evals/README.md +++ b/ai_evals/README.md @@ -175,6 +175,10 @@ the decrypted value, exactly as against a real backend. The chat's read path pas Seed a recognizable secret (the existing fixture uses `sk_live_do_not_leak_me`) and assert it via `valueExcludes` to catch a leak. +`toolExpect.toolCallArgs` entries support `sharedByAtLeast: `: at least `n` recorded +calls to that tool must carry the same non-blank string in the field. Use it for calls that +have to share an identifier, like two test runs of one chat conversation. + `toolExpect.toolCallArgs` entries additionally support `fieldMustBeAbsent: true`: no recorded call to that tool may pass the field at all (an explicit `null` counts as passing it). Use it for partial-update tools, where supplying a field the model could diff --git a/ai_evals/adapters/frontend/mockBackend.ts b/ai_evals/adapters/frontend/mockBackend.ts index fd66c1cbcc..d27bb4d69a 100644 --- a/ai_evals/adapters/frontend/mockBackend.ts +++ b/ai_evals/adapters/frontend/mockBackend.ts @@ -865,6 +865,29 @@ export function runBenchmarkFlowByPath(input: { }) } +/** + * Mirror `JobService.runFlowPreview` for benchmark workspaces, including the server's + * refusal of a chat-enabled flow run that names no conversation (`memory_id`). + */ +export function runBenchmarkFlowPreview(input: { + workspace: string + memoryId?: string + requestBody?: { path?: string; value?: { chat_input_enabled?: boolean }; args?: unknown } +}): string { + if (input.requestBody?.value?.chat_input_enabled && !input.memoryId) { + throw new Error('Bad request: memory_id is required for chat-enabled flows') + } + const args = (input.requestBody?.args ?? {}) as Record + return createBenchmarkCompletedJob({ + workspace: input.workspace, + jobKind: 'flowpreview', + success: true, + args, + result: { path: input.requestBody?.path, args, mocked: true }, + logs: 'Mock benchmark flow preview completed successfully.' + }) +} + export function previewBenchmarkSchedule(input: { requestBody?: Record }): Record { diff --git a/ai_evals/adapters/frontend/vitestAdapter.test.ts b/ai_evals/adapters/frontend/vitestAdapter.test.ts index bd6419da7c..c9e889b9a0 100644 --- a/ai_evals/adapters/frontend/vitestAdapter.test.ts +++ b/ai_evals/adapters/frontend/vitestAdapter.test.ts @@ -89,6 +89,7 @@ vi.mock('$lib/gen', async () => { previewBenchmarkSchedule, runBenchmarkDatatableSql, runBenchmarkFlowByPath, + runBenchmarkFlowPreview, runBenchmarkScriptByPath, runBenchmarkScriptPreview, updateBenchmarkDraft, @@ -295,6 +296,14 @@ vi.mock('$lib/gen', async () => { args: data.requestBody }) : actual.JobService.runScriptByPath(data), + runFlowPreview: async (data: { + workspace: string + memoryId?: string + requestBody?: { path?: string; value?: { chat_input_enabled?: boolean }; args?: unknown } + }) => + hasBenchmarkWorkspace(data.workspace) + ? runBenchmarkFlowPreview(data) + : actual.JobService.runFlowPreview(data as any), runFlowByPath: async (data: { workspace: string path: string diff --git a/ai_evals/cases/global.yaml b/ai_evals/cases/global.yaml index e8ec660f3b..45316b2810 100644 --- a/ai_evals/cases/global.yaml +++ b/ai_evals/cases/global.yaml @@ -2160,6 +2160,32 @@ - creates an AI draft of f/evals/global/process_invoice applying 8% tax - does not deploy or save the draft +- id: global-test38-chat-flow-follow-up-same-conversation + prompt: |- + I want to check that my support chat flow `f/evals/global/support_chat` remembers what was said. + Test it: first send "My name is Ada", then send "What is my name?" as a follow-up in the same chat. + initial: ai_evals/fixtures/frontend/global/initial/support_chat_flow.json + runtime: + maxTurns: 8 + validate: + draftCountExactly: 0 + toolExpect: + requiredToolsUsed: + - test_run_flow + # A chat flow's memory lives in its conversation, so a follow-up only reaches the first + # turn's history when both test runs name the same conversation. + toolCallArgs: + - tool: test_run_flow + field: memory_id + sharedByAtLeast: 2 + forbiddenToolsUsed: + - run_flow + - deploy_workspace_item + # The judge cannot observe runs; what this case guards is the conversation the runs share. + skipJudge: true + judgeChecklist: + - test-runs the chat flow twice, the second message as a follow-up in the first run's conversation + - id: global-undo-created-draft prompt: |- Create a draft Postgres resource at `u/admin/scratch_db` for host db.example.com port 5432, database `orders`, user `app`, and tell me what fields it ended up with. diff --git a/ai_evals/core/types.ts b/ai_evals/core/types.ts index 4107dc53da..832a383a73 100644 --- a/ai_evals/core/types.ts +++ b/ai_evals/core/types.ts @@ -182,6 +182,13 @@ export interface ToolCallArgumentRule { * the point is that the model filled it in at all rather than what it said. */ nonEmpty?: boolean; + /** + * Existential over calls: at least this many recorded calls to `tool` carry the + * same non-blank string in `field`. Use when calls have to share an identifier — + * e.g. test runs that continue one conversation — while a retry with a rejected + * value in between is still acceptable. + */ + sharedByAtLeast?: number; /** * Universal over calls: no recorded call to `tool` may pass `field` at all. * For partial-update tools, where supplying a field the model could not have diff --git a/ai_evals/core/validators.test.ts b/ai_evals/core/validators.test.ts index 4b7891f67e..1f7974e34e 100644 --- a/ai_evals/core/validators.test.ts +++ b/ai_evals/core/validators.test.ts @@ -396,6 +396,32 @@ describe("validateToolExpectations", () => { expect(nonEmptyCheck?.details).toContain("blank on 1 of 2"); }); + it("requires sharedByAtLeast calls to carry one value, not merely a value each", () => { + const run = (ids: (string | undefined)[]) => + validateToolExpectations({ + run: { + success: true, + actual: {}, + assistantMessageCount: 1, + toolCallCount: ids.length, + toolsUsed: ["test_run_flow"], + toolCallDetails: ids.map((memory_id) => ({ + name: "test_run_flow", + arguments: { path: "f/chat", memory_id }, + })), + skillsInvoked: [], + }, + toolExpect: { + toolCallArgs: [{ tool: "test_run_flow", field: "memory_id", sharedByAtLeast: 2 }], + }, + }).find((c) => c.name.includes("is shared by at least 2 calls"))?.passed; + + expect(run(["a", "b"])).toBe(false); + expect(run(["a"])).toBe(false); + expect(run([undefined, undefined])).toBe(false); + expect(run(["rejected", "a", "a"])).toBe(true); + }); + it("passes nonEmpty when every call filled the field", () => { const checks = validateToolExpectations({ run: { diff --git a/ai_evals/core/validators.ts b/ai_evals/core/validators.ts index 132a9ff3d0..9864f33dcc 100644 --- a/ai_evals/core/validators.ts +++ b/ai_evals/core/validators.ts @@ -320,6 +320,23 @@ export function validateToolExpectations(input: { ); } + if (rule.sharedByAtLeast !== undefined) { + const counts = new Map(); + for (const value of values) { + if (typeof value === "string" && value.trim().length > 0) { + counts.set(value, (counts.get(value) ?? 0) + 1); + } + } + const mostShared = Math.max(0, ...counts.values()); + checks.push( + check( + `${rule.tool}.${rule.field} is shared by at least ${rule.sharedByAtLeast} calls`, + mostShared >= rule.sharedByAtLeast, + `most calls sharing one value: ${mostShared}; values: ${summarizeToolValues(values)}` + ) + ); + } + if (rule.fieldMustBeAbsent) { // Anything other than `undefined` was supplied — an explicit `null` is the // model passing the field, not omitting it. diff --git a/ai_evals/fixtures/frontend/global/initial/support_chat_flow.json b/ai_evals/fixtures/frontend/global/initial/support_chat_flow.json new file mode 100644 index 0000000000..0db4242ed1 --- /dev/null +++ b/ai_evals/fixtures/frontend/global/initial/support_chat_flow.json @@ -0,0 +1,59 @@ +{ + "workspace": { + "flows": [ + { + "path": "f/evals/global/support_chat", + "summary": "Support chat", + "description": "Answers customer questions in a chat, remembering earlier messages.", + "schema": { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "type": "object", + "properties": { + "user_message": { + "type": "string", + "description": "Message from user" + } + }, + "required": ["user_message"] + }, + "value": { + "chat_input_enabled": true, + "modules": [ + { + "id": "assistant", + "summary": "Support assistant", + "value": { + "type": "aiagent", + "tools": [], + "input_transforms": { + "provider": { + "type": "static", + "value": { + "kind": "anthropic", + "model": "claude-haiku-4-5-20251001", + "resource": "$res:f/evals/ai/anthropic" + } + }, + "user_message": { + "type": "javascript", + "expr": "flow_input.user_message" + }, + "system_prompt": { + "type": "static", + "value": "You are a friendly support assistant. Keep answers short." + }, + "memory": { + "type": "static", + "value": { "kind": "auto", "context_length": 10 } + }, + "streaming": { "type": "static", "value": true }, + "output_type": { "type": "static", "value": "text" } + } + } + } + ] + } + } + ] + } +} diff --git a/frontend/src/lib/components/copilot/chat/AIChatManager.svelte.ts b/frontend/src/lib/components/copilot/chat/AIChatManager.svelte.ts index 34a6b5f3f1..ec3f54ca92 100644 --- a/frontend/src/lib/components/copilot/chat/AIChatManager.svelte.ts +++ b/frontend/src/lib/components/copilot/chat/AIChatManager.svelte.ts @@ -2440,8 +2440,8 @@ export class AIChatManager implements ChatViewHost { openArtifact: this.openArtifact } : {}), - testActiveFlow: async (storagePath: string, args?: Record) => - this.flowEditorFor(storagePath)?.testFlow(args), + testActiveFlow: async (storagePath: string, args?: Record, memoryId?: string) => + this.flowEditorFor(storagePath)?.testFlow(args, memoryId), getModifiedItems: () => (this.modifiedItems ? [...this.modifiedItems] : undefined), attachedFiles: this.attachedFiles, getUserInstructions: () => getUserCustomPrompts()[AIMode.GLOBAL] ?? '', diff --git a/frontend/src/lib/components/copilot/chat/AIChatManager.test.ts b/frontend/src/lib/components/copilot/chat/AIChatManager.test.ts index 7142646d90..35ac65253c 100644 --- a/frontend/src/lib/components/copilot/chat/AIChatManager.test.ts +++ b/frontend/src/lib/components/copilot/chat/AIChatManager.test.ts @@ -858,7 +858,9 @@ describe('AIChatManager autonomy mode', () => { const jobId = await manager.helpers.testActiveFlow('u/admin/live_flow', { name: 'Ada' }) expect(jobId).toBe('job-flow-preview') - expect(testFlow).toHaveBeenCalledWith({ name: 'Ada' }) + // Second argument is the chat-mode memory id, which only `test_run_flow`'s + // own `memory_id` supplies — never the session id. + expect(testFlow).toHaveBeenCalledWith({ name: 'Ada' }, undefined) // A session chat resolves an editor by its storage path, so it never names one. expect(manager.flowAiChatHelpers).toBeUndefined() }) diff --git a/frontend/src/lib/components/copilot/chat/flow/FlowAIChat.svelte b/frontend/src/lib/components/copilot/chat/flow/FlowAIChat.svelte index c6bed6988c..9744085567 100644 --- a/frontend/src/lib/components/copilot/chat/flow/FlowAIChat.svelte +++ b/frontend/src/lib/components/copilot/chat/flow/FlowAIChat.svelte @@ -4,6 +4,7 @@ import type { ExtendedOpenFlow, FlowEditorContext } from '$lib/components/flows/types' import type { InputTransform } from '$lib/gen' import type { FlowAIChatHelpers } from './core' + import { chatMemoryId } from '../global/core' import { createInlineScriptSession } from './inlineScriptsUtils' import { loadSchemaFromModule } from '$lib/components/flows/flowInfers' import { getAiChatManager } from '../aiChatManagerContext' @@ -173,7 +174,7 @@ previewArgs.val = args } // Call the UI test function which opens preview panel - return await onTestFlow?.(conversationId) + return await onTestFlow?.(conversationId ?? chatMemoryId(flowStore.val.value)) }, getLintErrors: async (moduleId: string): Promise => { diff --git a/frontend/src/lib/components/copilot/chat/global/core.test.ts b/frontend/src/lib/components/copilot/chat/global/core.test.ts index 822b36135c..825fdd34c3 100644 --- a/frontend/src/lib/components/copilot/chat/global/core.test.ts +++ b/frontend/src/lib/components/copilot/chat/global/core.test.ts @@ -5240,13 +5240,101 @@ describe('global AI tools', () => { ) // What the form submitted, not what the model proposed: the editor runs the flow, but - // the arguments are the user's. - expect(testActiveFlow).toHaveBeenCalledWith('u/admin/live_flow_storage', { name: 'Grace' }) + // the arguments are the user's. The third argument is the chat-mode memory id, + // which only `test_run_flow`'s own `memory_id` supplies. + expect(testActiveFlow).toHaveBeenCalledWith( + 'u/admin/live_flow_storage', + { name: 'Grace' }, + undefined + ) expect(FlowService.getFlowByPath).not.toHaveBeenCalled() expect(JobService.runFlowPreview).not.toHaveBeenCalled() expect(result).toContain('Result (SUCCESS)') }) + // A chat flow only shows its memory across turns, so the model has to be able to name + // the conversation it is continuing rather than getting a fresh one every call. + it('test_run_flow passes the memory id it was given to the live editor hook', async () => { + seedBackendDraft( + 'flow', + '', + { + path: 'u/admin/live_chat_flow', + summary: 'Live chat flow', + value: { modules: [{ id: 'live_step', value: { type: 'identity' } }] }, + schema: { type: 'object', properties: { user_message: { type: 'string' } } }, + edited_by: '', + edited_at: '', + archived: false, + extra_perms: {} + }, + { workspace: WORKSPACE } + ) + UserDraft.setLiveEditorDraft({ + workspace: WORKSPACE, + itemKind: 'flow', + storagePath: '', + effectivePath: 'u/admin/live_chat_flow' + }) + const testActiveFlow = vi.fn(async () => 'job-live-chat') + + await withCompletedTestJob(() => + callGlobalTool( + 'test_run_flow', + { + path: 'u/admin/live_chat_flow', + args: { user_message: 'hi' }, + memory_id: '550e8400-e29b-41d4-a716-446655440000' + }, + toolCallbacks, + { testActiveFlow } + ) + ) + + expect(testActiveFlow).toHaveBeenCalledWith( + '', + { user_message: 'hi' }, + '550e8400-e29b-41d4-a716-446655440000' + ) + }) + + it('test_run_flow gives a chat-enabled flow a conversation when none is named', async () => { + const value = { + modules: [{ id: 'chat_step', value: { type: 'identity' } }], + chat_input_enabled: true + } + seedBackendDraft( + 'flow', + 'u/admin/chat_preview', + { + path: 'u/admin/chat_preview', + summary: 'Chat preview', + value, + schema: { type: 'object', properties: { user_message: { type: 'string' } } }, + edited_by: '', + edited_at: '', + archived: false, + extra_perms: {} + }, + { workspace: WORKSPACE } + ) + + await withCompletedTestJob(() => + callGlobalTool('test_run_flow', { + path: 'u/admin/chat_preview', + args: { user_message: 'hi' } + }) + ) + + expect(JobService.runFlowPreview).toHaveBeenCalledWith({ + workspace: WORKSPACE, + memoryId: expect.stringMatching( + /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/ + ), + requestBody: { path: 'u/admin/chat_preview', value, args: { user_message: 'hi' } } + }) + }) + it('test_run_flow falls back to preview when the live flow editor test hook returns undefined', async () => { seedBackendDraft( 'flow', @@ -5283,7 +5371,11 @@ describe('global AI tools', () => { ) ) - expect(testActiveFlow).toHaveBeenCalledWith('u/admin/live_flow_fallback', { name: 'Ada' }) + expect(testActiveFlow).toHaveBeenCalledWith( + 'u/admin/live_flow_fallback', + { name: 'Ada' }, + undefined + ) expect(FlowService.getFlowByPath).not.toHaveBeenCalled() expect(JobService.runFlowPreview).toHaveBeenCalledWith({ workspace: WORKSPACE, diff --git a/frontend/src/lib/components/copilot/chat/global/core.ts b/frontend/src/lib/components/copilot/chat/global/core.ts index f1dcc0ef0a..b9a2cc7a91 100644 --- a/frontend/src/lib/components/copilot/chat/global/core.ts +++ b/frontend/src/lib/components/copilot/chat/global/core.ts @@ -1,3 +1,4 @@ +import { randomUUID } from '$lib/utils/uuid' import { AppService, AzureTriggerService, @@ -934,6 +935,17 @@ const runScriptToolDef = createToolDef( const testRunFlowSchema = z.object({ path: z.string().describe('Workspace path of the flow to test.'), args: testRunArgsSchema, + // A refinement rather than z.guid(): that emits `format`/`pattern` into the tool schema, + // which some providers' function-schema subsets reject. + memory_id: z + .string() + .refine((value) => z.guid().safeParse(value).success, { + message: 'memory_id must be a UUID' + }) + .optional() + .describe( + 'Chat-mode flows only. A UUID naming the conversation this turn belongs to, whose memory the agent steps read: reuse the same one across calls to test memory and follow-ups, and omit it for a one-off turn in a conversation of its own. Generate the UUID yourself so you can pass it again.' + ), background: backgroundArgSchema, wait_seconds: waitSecondsArgSchema }) @@ -4391,8 +4403,13 @@ type WriteDraftCtx = { export type SessionToolHelpers = { sessionId?: string } export type GlobalToolHelpers = SessionToolHelpers & { - /** Runs the flow editor mounted on `storagePath`, if one is. */ - testActiveFlow?: (storagePath: string, args?: Record) => Promise + /** Runs the flow editor mounted on `storagePath`, if one is. `memoryId` names the + * chat-mode conversation the turn belongs to. */ + testActiveFlow?: ( + storagePath: string, + args?: Record, + memoryId?: string + ) => Promise attachedFiles?: AttachedFilesStore // Read/write the user-level Global instructions. `setUserInstructions` persists the // value and rebuilds the system message so the change applies on the next chat-loop @@ -4429,13 +4446,15 @@ function operatingWorkspaceFromHelpers(helpers: unknown): string | undefined { function liveFlowTestHookFromCtx( ctx: { workspace: string; helpers?: unknown }, path: string -): ((args?: Record) => Promise) | undefined { +): ((args?: Record, memoryId?: string) => Promise) | undefined { const activeEditor = getActiveGlobalEditorContext(ctx.workspace) if (activeEditor?.type !== 'flow' || activeEditor.path !== path) { return undefined } const testActiveFlow = (ctx.helpers as GlobalToolHelpers | undefined)?.testActiveFlow - return testActiveFlow && ((args) => testActiveFlow(activeEditor.storagePath, args)) + return ( + testActiveFlow && ((args, memoryId) => testActiveFlow(activeEditor.storagePath, args, memoryId)) + ) } export type OpenPreviewHandler = (req: { @@ -5450,6 +5469,16 @@ function flowDraftValueForPreview(flowDraft: FlowDraftValue): FlowValue { return flowDraftAsEditableInput(flowDraft).value } +/** + * The conversation a test run of a chat-enabled flow belongs to. The server refuses such a + * run without one, and it is a query parameter rather than a flow argument, so there is no + * way for the caller to supply it through `args`. A fresh id each time is the right default: + * a test run is its own conversation, not a turn appended to one someone is reading. + */ +export function chatMemoryId(value: FlowValue): string | undefined { + return value.chat_input_enabled ? randomUUID() : undefined +} + async function loadScriptForFlowStep( moduleValue: { path: string; hash?: string }, workspace: string @@ -5915,15 +5944,17 @@ async function testRunFlowByPath( // An open editor runs its own in-memory flow and paints the run in its graph. // Resolved here rather than before the form: the form waits as long as the user // does, and the editor on screen when they press Run is the one it belongs in. - const jobId = await liveFlowTestHookFromCtx(ctx, args.path)?.(submitted) + const jobId = await liveFlowTestHookFromCtx(ctx, args.path)?.(submitted, args.memory_id) if (jobId) { return jobId } + const value = flowDraftValueForPreview(flow.flow) return JobService.runFlowPreview({ workspace, + memoryId: args.memory_id ?? chatMemoryId(value), requestBody: { path: args.path, - value: flowDraftValueForPreview(flow.flow), + value, args: submitted } })