mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-21 00:02:30 +00:00
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) <noreply@anthropic.com> * fix: refuse a non-UUID conversation_id and mint chat test-run ids in one place Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(ai-evals): add a chat-flow follow-up case and mock flow preview runs Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test: expect the conversation id argument on the manager's flow test bridge Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(ai-evals): require the chat-flow follow-up runs to share one conversation id Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * refactor: name the test_run_flow argument memory_id after the run parameter Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -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: <n>`: 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
|
||||
|
||||
@@ -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<string, unknown>
|
||||
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<string, unknown>
|
||||
}): Record<string, unknown> {
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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: {
|
||||
|
||||
@@ -320,6 +320,23 @@ export function validateToolExpectations(input: {
|
||||
);
|
||||
}
|
||||
|
||||
if (rule.sharedByAtLeast !== undefined) {
|
||||
const counts = new Map<string, number>();
|
||||
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.
|
||||
|
||||
@@ -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" }
|
||||
}
|
||||
}
|
||||
}
|
||||
]
|
||||
}
|
||||
}
|
||||
]
|
||||
}
|
||||
}
|
||||
@@ -2440,8 +2440,8 @@ export class AIChatManager implements ChatViewHost {
|
||||
openArtifact: this.openArtifact
|
||||
}
|
||||
: {}),
|
||||
testActiveFlow: async (storagePath: string, args?: Record<string, any>) =>
|
||||
this.flowEditorFor(storagePath)?.testFlow(args),
|
||||
testActiveFlow: async (storagePath: string, args?: Record<string, any>, memoryId?: string) =>
|
||||
this.flowEditorFor(storagePath)?.testFlow(args, memoryId),
|
||||
getModifiedItems: () => (this.modifiedItems ? [...this.modifiedItems] : undefined),
|
||||
attachedFiles: this.attachedFiles,
|
||||
getUserInstructions: () => getUserCustomPrompts()[AIMode.GLOBAL] ?? '',
|
||||
|
||||
@@ -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()
|
||||
})
|
||||
|
||||
@@ -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<ScriptLintResult> => {
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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<string, any>) => Promise<string | undefined>
|
||||
/** 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<string, any>,
|
||||
memoryId?: string
|
||||
) => Promise<string | undefined>
|
||||
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<string, any>) => Promise<string | undefined>) | undefined {
|
||||
): ((args?: Record<string, any>, memoryId?: string) => Promise<string | undefined>) | 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
|
||||
}
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user