From 6692e9d0c038df397698ec9cdbe6fca8aba8d49a Mon Sep 17 00:00:00 2001 From: Guilhem Lemouel Date: Wed, 16 Sep 2026 19:04:28 +0200 Subject: [PATCH] fix: scope a structured answer's claim to its own turn and document the row fields as stored Co-Authored-By: Claude Fable 5.1 --- backend/windmill-api/openapi.yaml | 13 +++++----- chat-sdk/src/api.ts | 5 ++-- chat-sdk/src/chat.ts | 14 ++++++---- chat-sdk/test/chat.test.ts | 43 +++++++++++++++++++++++++++++++ 4 files changed, 62 insertions(+), 13 deletions(-) diff --git a/backend/windmill-api/openapi.yaml b/backend/windmill-api/openapi.yaml index 0b9593ead1..af1fe7a2b9 100644 --- a/backend/windmill-api/openapi.yaml +++ b/backend/windmill-api/openapi.yaml @@ -28371,16 +28371,17 @@ components: type: string nullable: true description: >- - The call, for a tool that runs inside the agent's job and so has none of its - own: an MCP tool, or a provider-native one such as web search. A Windmill tool - is a script or flow with its own job, and its call is read from there instead. + The arguments of an MCP tool call, which runs inside the agent's job and so has + none of its own. Null for a provider-native web search, whose query the provider + does not return, and for a Windmill tool, a script or flow whose own job holds + its call. tool_result: type: string nullable: true description: >- - What that same call returned, including the citations of a provider-native web - search, and what it failed with when it failed — the row's own text names the - tool rather than the reason. Null for a tool whose job holds the answer. + An MCP tool's result, a provider-native web search's citations, and for any + failed tool what it failed with — the row's own text names the tool rather than + the reason. Null for a successful Windmill tool, whose own job holds the result. reasoning: type: string nullable: true diff --git a/chat-sdk/src/api.ts b/chat-sdk/src/api.ts index c86f757247..2cbfe446e9 100644 --- a/chat-sdk/src/api.ts +++ b/chat-sdk/src/api.ts @@ -40,10 +40,11 @@ export interface FlowConversationMessage { created_seq: number step_name?: string | null success?: boolean - /** The call a tool row carries itself, for a tool whose job cannot be asked for it. */ + /** An MCP tool call's arguments; null for a web search and for a Windmill tool, whose own job holds its call. */ tool_arguments?: string | null + /** An MCP tool's result, a web search's citations, and what any failed tool failed with. */ tool_result?: string | null - /** The thinking behind an answer, which is streamed and stored nowhere else. */ + /** The thinking of the iteration that produced this row; the agent job keeps the turn's thinking as one string. */ reasoning?: string | null } diff --git a/chat-sdk/src/chat.ts b/chat-sdk/src/chat.ts index bc62417e7d..ff05d7fd89 100644 --- a/chat-sdk/src/chat.ts +++ b/chat-sdk/src/chat.ts @@ -622,12 +622,16 @@ class ChatImpl implements Chat { (m.content === row.content || (row.tool !== undefined && m.tool?.name === row.tool.name)) ) // A structured answer streams as thinking alone, its text arriving as a tool call - // the stream never turns into a message, so its row claims the pending message - // that holds that thinking and nothing else. + // the stream never turns into a message, so its row claims the message that holds + // that thinking and nothing else. Only past the newest user message: a turn stopped + // before its rows landed leaves such a message behind, and it is not this answer's. if (i < 0 && row.role === 'assistant' && row.reasoning !== undefined) { - i = messages.findIndex( - (m) => m.seq === undefined && m.role === 'assistant' && m.content === '' && m.reasoning !== undefined - ) + let turnStart = messages.length - 1 + while (turnStart >= 0 && messages[turnStart].role !== 'user') turnStart-- + const j = messages + .slice(turnStart + 1) + .findIndex((m) => m.seq === undefined && m.role === 'assistant' && m.content === '' && m.reasoning !== undefined) + if (j >= 0) i = turnStart + 1 + j } if (i >= 0) { const m = messages[i] diff --git a/chat-sdk/test/chat.test.ts b/chat-sdk/test/chat.test.ts index 0abe471b5f..fbf7f595b7 100644 --- a/chat-sdk/test/chat.test.ts +++ b/chat-sdk/test/chat.test.ts @@ -666,6 +666,49 @@ describe('createChat with server history', () => { ]) }) + test('a structured answer row leaves a stopped earlier turn its thinking', async () => { + let jobs = 0 + const { fetch } = fetchMock( + (c) => (c.method === 'POST' && c.url.pathname.includes('/jobs/run/f/') ? text(`job-${++jobs}`) : undefined), + (c) => + c.url.pathname.endsWith('/getupdate_sse/job-1') + ? sse([{ type: 'update', new_result_stream: ndjson({ type: 'reasoning_token_delta', content: 'first thoughts' }), stream_offset: 1 }]) + : undefined, + (c) => + c.url.pathname.endsWith('/getupdate_sse/job-2') + ? sse([ + { + type: 'update', + new_result_stream: ndjson({ type: 'reasoning_token_delta', content: 'hmm' }), + stream_offset: 1, + completed: true, + only_result: { output: { n: 1 }, messages: [] } + } + ]) + : undefined, + (c) => (c.url.pathname.includes('/queue/cancel/') ? text('ok') : undefined), + (c) => + c.url.pathname.endsWith('/messages') + ? json([messageRow(41, 'user', 'first'), messageRow(42, 'user', 'again'), messageRow(43, 'assistant', '{"n":1}', { job_id: 'step-2', reasoning: 'hmm' })]) + : undefined, + (c) => (c.url.pathname === '/api/w/ws/flow_conversations/list' ? json([]) : undefined) + ) + const chat = createChat(options({}, fetch)) + const first = chat.sendMessage('first') + await new Promise((r) => setTimeout(r, 50)) + const stopped = chat.stop() + await first + const second = chat.sendMessage('again') + await stopped + await second + expect(chat.getState().messages.map((m) => [m.role, m.content, m.reasoning])).toEqual([ + ['user', 'first', undefined], + ['assistant', '', 'first thoughts'], + ['user', 'again', undefined], + ['assistant', '{"n":1}', 'hmm'] + ]) + }) + test('the stream asks for a server poll interval only when one is set', async () => { const answer: Route = (c) => c.url.pathname === streamPath ? sse([{ type: 'update', completed: true, only_result: 'ok' }]) : undefined