diff --git a/backend/windmill-api-flow-conversations/src/lib.rs b/backend/windmill-api-flow-conversations/src/lib.rs index 9b33aa8432..0230620cb4 100644 --- a/backend/windmill-api-flow-conversations/src/lib.rs +++ b/backend/windmill-api-flow-conversations/src/lib.rs @@ -43,8 +43,8 @@ pub struct FlowConversationMessage { /// An MCP tool's result, a web search's citations, and what any failed tool failed /// with. Null for a successful Windmill tool, whose own job holds the result. pub tool_result: Option, - /// The thinking of the iteration that produced this row. The agent job keeps the - /// turn's thinking as one string. + /// On an answer, the thinking that produced it; on a tool row, the thinking that led to + /// the call. The agent job keeps the turn's thinking as one string. pub reasoning: Option, } diff --git a/backend/windmill-api/openapi.yaml b/backend/windmill-api/openapi.yaml index af1fe7a2b9..678e82be6e 100644 --- a/backend/windmill-api/openapi.yaml +++ b/backend/windmill-api/openapi.yaml @@ -28386,8 +28386,9 @@ components: type: string nullable: true description: >- - The thinking behind this row, for the iteration that produced it. The agent - job's result keeps the turn's thinking as a single string. + On an answer, the thinking that produced it; on a tool row, the thinking that + led to the call. Each round's thinking is on one row. The agent job's result + keeps the turn's thinking as a single string. EndpointTool: type: object diff --git a/backend/windmill-worker/src/ai/tools.rs b/backend/windmill-worker/src/ai/tools.rs index 27403a782b..047d68c1f7 100644 --- a/backend/windmill-worker/src/ai/tools.rs +++ b/backend/windmill-worker/src/ai/tools.rs @@ -74,6 +74,9 @@ pub struct ToolExecutionContext<'a> { pub stream_event_processor: Option<&'a StreamEventProcessor>, pub flow_context: &'a mut FlowContext, pub omit_output_from_conversation: bool, + /// The thinking that led to this round's calls, stored on the first tool row written. + /// None when the round wrote text, whose row carries it. + pub reasoning: Option, pub previous_result: &'a Option>, pub id_context: &'a Option, @@ -879,6 +882,12 @@ async fn add_tool_message_to_chat( if ctx.omit_output_from_conversation { return; } + let extras = match ctx.reasoning.take() { + Some(reasoning) => { + Some(MessageExtras { reasoning: Some(reasoning), ..extras.unwrap_or_default() }) + } + None => extras, + }; let chat_enabled = ctx .flow_context diff --git a/backend/windmill-worker/src/ai_executor.rs b/backend/windmill-worker/src/ai_executor.rs index 35b204f364..04383fe7bd 100644 --- a/backend/windmill-worker/src/ai_executor.rs +++ b/backend/windmill-worker/src/ai_executor.rs @@ -1619,51 +1619,6 @@ pub async fn run_agent( } } - // An iteration that answered with tool calls has no message row to carry its - // thinking, and the next iteration's row holds only its own. Stored on a row - // of its own so a reader sees what led to the call. A call of the - // structured-output tool is the answer itself, and its row below carries - // the thinking: a bare row before it would read as the turn's last word. - let calls_structured_output = - structured_output_tool_name.as_ref().map_or(false, |name| { - tool_calls.iter().any(|tc| tc.function.name == *name) - }); - if persist_output_to_conversation - && !calls_structured_output - && response_content.as_deref().unwrap_or("").is_empty() - { - if let (Some(memory_id), Some(reasoning)) = - (memory_id, response_reasoning.clone()) - { - let agent_job_id = job.id; - let db_clone = db.clone(); - let step_name = step_name.clone(); - tokio::spawn(async move { - if let Err(e) = add_message_to_conversation( - &db_clone, - &memory_id, - Some(agent_job_id), - "", - MessageType::Assistant, - &step_name, - true, - Some(&MessageExtras { - reasoning: Some(reasoning), - ..Default::default() - }), - ) - .await - { - tracing::warn!( - "Failed to add reasoning message to conversation {}: {}", - memory_id, - e - ); - } - }); - } - } - if tool_calls.is_empty() { break; } else if i == max_iterations - 1 { @@ -1698,6 +1653,18 @@ pub async fn run_agent( ..Default::default() }); + // A round's thinking is stored on one row, the first the round writes, which is + // where the stream shows it: its text row when it wrote text, else the row of + // its first call — the answer row below when that call is the structured-output + // tool. Two rows carrying it would show it twice after a reload. + let call_reasoning = response_reasoning + .clone() + .filter(|_| response_content.as_deref().unwrap_or("").is_empty()); + let structured_output_first = structured_output_tool_name + .as_ref() + .zip(tool_calls.first()) + .map_or(false, |(name, tc)| tc.function.name == *name); + // Handle tool calls using extracted tools module let tool_execution_ctx = ToolExecutionContext { db, @@ -1716,6 +1683,11 @@ pub async fn run_agent( stream_event_processor: stream_event_processor.as_ref(), flow_context: &mut flow_context, omit_output_from_conversation, + reasoning: if structured_output_first { + None + } else { + call_reasoning.clone() + }, previous_result: &previous_result, id_context: &id_context, tool_abort_handles: tool_abort_handles.clone(), @@ -1736,8 +1708,7 @@ pub async fn run_agent( messages.extend(tool_messages); // A structured answer is the arguments of the structured-output tool call, - // on which the loop ends without a text iteration, so its row is written - // here with the thinking of the iteration that produced it. + // on which the loop ends without a text iteration, so its row is written here. if tool_used_structured_output && persist_output_to_conversation { if let (Some(memory_id), Some(OpenAIContent::Text(answer))) = (memory_id, tool_content.as_ref()) @@ -1746,10 +1717,13 @@ pub async fn run_agent( let db_clone = db.clone(); let message_content = answer.clone(); let step_name = step_name.clone(); - let extras = response_reasoning.clone().map(|reasoning| MessageExtras { - reasoning: Some(reasoning), - ..Default::default() - }); + let extras = call_reasoning + .clone() + .filter(|_| structured_output_first) + .map(|reasoning| MessageExtras { + reasoning: Some(reasoning), + ..Default::default() + }); tokio::spawn(async move { if let Err(e) = add_message_to_conversation( &db_clone, diff --git a/chat-sdk/src/ai-sdk.ts b/chat-sdk/src/ai-sdk.ts index 527ece1e5c..d610073995 100644 --- a/chat-sdk/src/ai-sdk.ts +++ b/chat-sdk/src/ai-sdk.ts @@ -322,6 +322,7 @@ export function toUIMessages(messages: ChatMessage[]): UIMessage[] { target = { id: m.id, role: 'assistant', parts: [] } out.push(target) } + if (m.reasoning) target.parts.push({ type: 'reasoning', text: m.reasoning, state: 'done' }) if (m.role === 'tool') { const toolCallId = m.tool?.callId ?? m.id const toolName = m.tool?.name ?? 'tool' @@ -333,7 +334,6 @@ export function toUIMessages(messages: ChatMessage[]): UIMessage[] { ) continue } - if (m.reasoning) target.parts.push({ type: 'reasoning', text: m.reasoning, state: 'done' }) if (m.content) target.parts.push({ type: 'text', text: m.content, state: 'done' }) } return out diff --git a/chat-sdk/src/api.ts b/chat-sdk/src/api.ts index 2cbfe446e9..3e09f6ff41 100644 --- a/chat-sdk/src/api.ts +++ b/chat-sdk/src/api.ts @@ -44,7 +44,7 @@ export interface FlowConversationMessage { 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 of the iteration that produced this row; the agent job keeps the turn's thinking as one string. */ + /** On an answer, the thinking that produced it; on a tool row, the thinking that led to the call. */ reasoning?: string | null } diff --git a/chat-sdk/src/assistant-ui.ts b/chat-sdk/src/assistant-ui.ts index ef22f28531..2ec4966149 100644 --- a/chat-sdk/src/assistant-ui.ts +++ b/chat-sdk/src/assistant-ui.ts @@ -91,6 +91,7 @@ export function toThreadMessage(turn: WindmillTurn): ThreadMessageLike { } const content: ThreadContentPart[] = [] for (const m of turn.messages) { + if (m.reasoning) content.push({ type: 'reasoning', text: m.reasoning }) if (m.role === 'tool') { const args = parseJsonOr(m.tool?.arguments) content.push({ @@ -104,7 +105,6 @@ export function toThreadMessage(turn: WindmillTurn): ThreadMessageLike { }) continue } - if (m.reasoning) content.push({ type: 'reasoning', text: m.reasoning }) if (m.content) content.push({ type: 'text', text: m.content }) } const last = turn.messages[turn.messages.length - 1] diff --git a/chat-sdk/src/chat.ts b/chat-sdk/src/chat.ts index 91a4f69b37..d7dddd6320 100644 --- a/chat-sdk/src/chat.ts +++ b/chat-sdk/src/chat.ts @@ -344,16 +344,21 @@ class ChatImpl implements Chat { tool: { ...existing.tool!, ...toolPatch } } } else { + // Thinking that produced no text led to this call, and is stored on its row. + const a = turn.assistantId ? messages.findIndex((m) => m.id === turn.assistantId) : -1 + const reasoning = a >= 0 && messages[a].content === '' ? messages.splice(a, 1)[0].reasoning : undefined messages.push({ id: `pending-${randomId()}`, role: 'tool', content: content ?? '', + reasoning, success: success ?? true, createdAt: now(), pending: true, tool: { callId, name, status: 'running', ...toolPatch } }) } + turn.assistantId = undefined } const appendAssistant = (text: string, reasoning: string) => { const i = turn.assistantId @@ -388,17 +393,14 @@ class ChatImpl implements Chat { case 'reasoning_token_delta': appendAssistant('', event.content) break + // A call completes the round's text: text after it is a new message. case 'tool_call': - // The round's text is complete; text after the tool result is a new message. - turn.assistantId = undefined upsertTool(event.call_id, event.function_name, { status: 'running' }) break case 'tool_call_arguments': - turn.assistantId = undefined upsertTool(event.call_id, event.function_name, { arguments: event.arguments }) break case 'tool_execution': - turn.assistantId = undefined upsertTool(event.call_id, event.function_name, { status: 'running' }) break case 'tool_result': @@ -507,9 +509,6 @@ class ChatImpl implements Chat { * job is one of the turn's, which leaves out an earlier turn whose job outlived * `stop()` (a token without `jobs:write` cannot cancel it); a tool row without one * (an MCP call runs inside the agent step) belongs to whatever turn is under way. - * An assistant row with no text carries only the thinking before a tool call and - * is not an answer: a structured answer the agent never wrote a row for would - * otherwise be dropped instead of read from the flow result. */ #answered(turn: Turn): boolean { const messages = this.#state.messages @@ -522,7 +521,7 @@ class ChatImpl implements Chat { if (m.seq === undefined || m.role === 'user' || !ownJob(m)) continue if (latest === undefined || m.seq > latest.seq!) latest = m } - return latest?.role === 'assistant' && latest.content !== '' + return latest?.role === 'assistant' } /** @@ -612,32 +611,19 @@ class ChatImpl implements Chat { if (rows.length === 0) return const messages = [...this.#state.messages] const known = new Set(messages.map((m) => m.serverId ?? m.id)) - // An assistant message with no text is the thinking before a tool call, or before a - // structured answer whose text arrives as a tool call the stream never turns into a - // message. Text cannot tell such messages apart, so one is only ever claimed within - // the newest turn: a turn stopped before its rows landed leaves one behind, and no - // later row is its. - let turnStart = messages.length - 1 - while (turnStart >= 0 && messages[turnStart].role !== 'user') turnStart-- - const inTurn = (test: (m: ChatMessage) => boolean): number => { - const j = messages.slice(turnStart + 1).findIndex(test) - return j < 0 ? -1 : turnStart + 1 + j - } for (const row of rows.map(fromRow)) { if (known.has(row.id)) continue known.add(row.id) - const sameShape = (m: ChatMessage) => - m.seq === undefined && - m.role === row.role && - (m.content === row.content || (row.tool !== undefined && m.tool?.name === row.tool.name)) - let i: number - if (row.role === 'assistant' && row.content === '') { - i = inTurn(sameShape) - } else { - i = messages.findIndex(sameShape) - if (i < 0 && row.role === 'assistant' && row.reasoning !== undefined) { - i = inTurn((m) => m.seq === undefined && m.role === 'assistant' && m.content === '' && m.reasoning !== undefined) - } + let i = messages.findIndex( + (m) => + m.seq === undefined && + m.role === row.role && + (m.content === row.content || (row.tool !== undefined && m.tool?.name === row.tool.name)) + ) + // A structured answer streams as the call of the structured-output tool, whose + // arguments are the answer's text: its row replaces that call. + if (i < 0 && row.role === 'assistant') { + i = messages.findIndex((m) => m.seq === undefined && m.role === 'tool' && m.tool?.arguments === row.content) } if (i >= 0) { const m = messages[i] @@ -645,7 +631,16 @@ class ChatImpl implements Chat { ...row, id: m.id, reasoning: m.reasoning ?? row.reasoning, - tool: m.tool ? { ...m.tool, status: row.tool?.status ?? m.tool.status } : row.tool + // The stream's call wins where it has a value; a stream cut short leaves gaps the row fills. + tool: + row.role === 'tool' && m.tool + ? { + ...m.tool, + arguments: m.tool.arguments ?? row.tool?.arguments, + result: m.tool.result ?? row.tool?.result, + status: row.tool?.status ?? m.tool.status + } + : row.tool } } else { messages.push(row) diff --git a/chat-sdk/test/ai-sdk.test.ts b/chat-sdk/test/ai-sdk.test.ts index 27251e519e..cbfb29dcb3 100644 --- a/chat-sdk/test/ai-sdk.test.ts +++ b/chat-sdk/test/ai-sdk.test.ts @@ -146,16 +146,17 @@ describe('createWindmillChatTransport', () => { c.method === 'GET' && c.url.pathname.endsWith('/messages') ? json([ messageRow(1, 'user', 'hi'), - messageRow(2, 'tool', 'Used lookup tool', { job_id: 'agent-job', tool_arguments: '{"q":1}', tool_result: '42' }), + messageRow(2, 'tool', 'Used lookup tool', { job_id: 'agent-job', reasoning: 'why', tool_arguments: '{"q":1}', tool_result: '42' }), messageRow(3, 'assistant', 'The answer is 42', { reasoning: 'hmm' }) ]) : undefined ) const transport = createWindmillChatTransport({ baseUrl: 'http://wm.test', workspace: 'ws', flowPath: FLOW, fetch }) const ui = await transport.loadMessages('c') - expect(ui.map((m) => m.parts.map((p) => p.type))).toEqual([['text'], ['dynamic-tool', 'reasoning', 'text']]) - expect(ui[1].parts[0]).toMatchObject({ toolName: 'lookup', state: 'output-available', input: { q: 1 }, output: 42 }) - expect(ui[1].parts[1]).toMatchObject({ type: 'reasoning', text: 'hmm' }) + expect(ui.map((m) => m.parts.map((p) => p.type))).toEqual([['text'], ['reasoning', 'dynamic-tool', 'reasoning', 'text']]) + expect(ui[1].parts[0]).toMatchObject({ type: 'reasoning', text: 'why' }) + expect(ui[1].parts[1]).toMatchObject({ toolName: 'lookup', state: 'output-available', input: { q: 1 }, output: 42 }) + expect(ui[1].parts[2]).toMatchObject({ type: 'reasoning', text: 'hmm' }) }) test('refuses attachments with a clear error', async () => { diff --git a/chat-sdk/test/chat.test.ts b/chat-sdk/test/chat.test.ts index 5ef99ebbe6..d7bd1a2795 100644 --- a/chat-sdk/test/chat.test.ts +++ b/chat-sdk/test/chat.test.ts @@ -615,29 +615,7 @@ describe('createChat with server history', () => { expect(chat.getState().messages.map((m) => m.content)).toEqual(['hi', 'Let me check', 'Used search tool', 'Final answer']) }) - test('a reasoning-only row is not the answer of a turn that streamed no text', async () => { - const { fetch } = fetchMock( - run, - (c) => - c.url.pathname === streamPath - ? sse([{ type: 'update', completed: true, only_result: { output: '{"n":1}', messages: [] } }]) - : undefined, - (c) => - c.url.pathname.endsWith('/messages') - ? json([messageRow(71, 'user', 'hi'), messageRow(72, 'assistant', '', { job_id: 'step-1', reasoning: 'hmm' })]) - : undefined, - (c) => (c.url.pathname === '/api/w/ws/flow_conversations/list' ? json([]) : undefined) - ) - const chat = createChat(options({}, fetch)) - await chat.sendMessage('hi') - expect(chat.getState().messages.map((m) => [m.role, m.content, m.reasoning])).toEqual([ - ['user', 'hi', undefined], - ['assistant', '', 'hmm'], - ['assistant', '{"n":1}', undefined] - ]) - }) - - test('a structured answer row claims the thinking that streamed before it', async () => { + test('thinking that led to a tool call rides on the call, live and once its row lands', async () => { const { fetch } = fetchMock( run, (c) => @@ -645,45 +623,53 @@ describe('createChat with server history', () => { ? sse([ { type: 'update', - new_result_stream: ndjson({ type: 'reasoning_token_delta', content: 'hmm' }), - stream_offset: 1, + // No `tool_call_arguments`: a stream cut short leaves the call without them. + new_result_stream: ndjson( + { type: 'reasoning_token_delta', content: 'r1' }, + { type: 'tool_call', call_id: 'c1', function_name: 'lookup' }, + { type: 'tool_result', call_id: 'c1', function_name: 'lookup', result: '1', success: true }, + { type: 'token_delta', content: 'Final' } + ), + stream_offset: 4, completed: true, - only_result: { output: { n: 1 }, messages: [] } + only_result: { output: 'Final', messages: [] } } ]) : undefined, (c) => c.url.pathname.endsWith('/messages') - ? json([messageRow(75, 'user', 'hi'), messageRow(76, 'assistant', '{"n":1}', { job_id: 'step-1', reasoning: 'hmm' })]) + ? json([ + messageRow(71, 'user', 'hi'), + messageRow(72, 'tool', 'Used lookup tool', { job_id: 'step-1', reasoning: 'r1', tool_arguments: '{"q":1}', tool_result: '1' }), + messageRow(73, 'assistant', 'Final', { job_id: 'step-1' }) + ]) : undefined, (c) => (c.url.pathname === '/api/w/ws/flow_conversations/list' ? json([]) : undefined) ) const chat = createChat(options({}, fetch)) await chat.sendMessage('hi') - expect(chat.getState().messages.map((m) => [m.role, m.content, m.reasoning])).toEqual([ - ['user', 'hi', undefined], - ['assistant', '{"n":1}', 'hmm'] + const messages = chat.getState().messages + expect(messages.map((m) => [m.role, m.content, m.reasoning, m.seq])).toEqual([ + ['user', 'hi', undefined, 71], + ['tool', 'Used lookup tool', 'r1', 72], + ['assistant', 'Final', undefined, 73] ]) + expect(messages[1].tool).toMatchObject({ callId: 'c1', arguments: '{"q":1}', result: '1', status: 'success' }) }) - test('reasoning rows of a turn with a tool round stay with that turn after a stopped one', async () => { - let jobs = 0 + test('a structured answer row replaces the call it streamed as', async () => { const { fetch } = fetchMock( - (c) => (c.method === 'POST' && c.url.pathname.includes('/jobs/run/f/') ? text(`job-${++jobs}`) : undefined), + run, (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') + c.url.pathname === streamPath ? sse([ { type: 'update', new_result_stream: ndjson( - { type: 'reasoning_token_delta', content: 'r1' }, - { type: 'tool_call', call_id: 'c1', function_name: 'lookup' }, - { type: 'tool_result', call_id: 'c1', function_name: 'lookup', result: '1', success: true }, - { type: 'reasoning_token_delta', content: 'r2' } + { type: 'reasoning_token_delta', content: 'hmm' }, + { type: 'tool_call', call_id: 'c9', function_name: 'structured_output' }, + { type: 'tool_call_arguments', call_id: 'c9', function_name: 'structured_output', arguments: '{"n": 1}' }, + { type: 'tool_execution', call_id: 'c9', function_name: 'structured_output' } ), stream_offset: 4, completed: true, @@ -691,77 +677,17 @@ describe('createChat with server history', () => { } ]) : undefined, - (c) => (c.url.pathname.includes('/queue/cancel/') ? text('ok') : undefined), (c) => c.url.pathname.endsWith('/messages') - ? json([ - messageRow(51, 'user', 'first'), - messageRow(52, 'user', 'again'), - messageRow(53, 'assistant', '', { job_id: 'step-2', reasoning: 'r1' }), - messageRow(54, 'tool', 'Used lookup tool', { job_id: 'tool-2' }), - messageRow(55, 'assistant', '{"n":1}', { job_id: 'step-2', reasoning: 'r2' }) - ]) + ? json([messageRow(75, 'user', 'hi'), messageRow(76, 'assistant', '{"n": 1}', { job_id: 'step-1', 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, m.seq])).toEqual([ - ['user', 'first', undefined, 51], - ['assistant', '', 'first thoughts', undefined], - ['user', 'again', undefined, 52], - ['assistant', '', 'r1', 53], - ['tool', 'Used lookup tool', undefined, 54], - ['assistant', '{"n":1}', 'r2', 55] - ]) - }) - - 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'] + await chat.sendMessage('hi') + expect(chat.getState().messages.map((m) => [m.role, m.content, m.reasoning, m.tool])).toEqual([ + ['user', 'hi', undefined, undefined], + ['assistant', '{"n": 1}', 'hmm', undefined] ]) }) diff --git a/frontend/src/lib/components/flows/conversations/flowChatViewHost.svelte.ts b/frontend/src/lib/components/flows/conversations/flowChatViewHost.svelte.ts index 78118546ac..dc58e49378 100644 --- a/frontend/src/lib/components/flows/conversations/flowChatViewHost.svelte.ts +++ b/frontend/src/lib/components/flows/conversations/flowChatViewHost.svelte.ts @@ -61,21 +61,23 @@ function lastTurnFailed(messages: readonly ChatMessage[]): boolean { export function toDisplayMessages(messages: readonly ChatMessage[]): DisplayMessage[] { let userIndex = 0 - return messages.map((message, i): DisplayMessage => { + return messages.flatMap((message, i): DisplayMessage[] => { switch (message.role) { case 'user': - return { - role: 'user', - index: userIndex++, - content: message.content, - // Drives the shared Retry button. - error: turnFailed(messages, i) || undefined - } + return [ + { + role: 'user', + index: userIndex++, + content: message.content, + // Drives the shared Retry button. + error: turnFailed(messages, i) || undefined + } + ] case 'tool': { const parameters = parseToolPayload(message.tool?.arguments) const result = parseToolPayload(message.tool?.result) const failed = message.success === false - return { + const call: DisplayMessage = { role: 'tool', tool_call_id: message.id, // The card's header is the row's text, which the server only words once the @@ -91,19 +93,36 @@ export function toDisplayMessages(messages: readonly ChatMessage[]): DisplayMess error: failed ? message.content : undefined, isLoading: message.pending && message.tool?.status === 'running' } + // The tool card has no thinking section: the thinking that led to the call reads + // as a card of its own, just before it. + return message.reasoning + ? [ + { + role: 'assistant', + content: '', + reasoning: message.reasoning, + stepName: message.stepName, + jobId: message.jobId, + createdAt: message.createdAt + }, + call + ] + : [call] } default: - return { - role: 'assistant', - content: message.content, - // Only the message a turn is still writing: a finalized reasoning-only - // message must not look in progress. - streaming: message.pending || undefined, - reasoning: message.reasoning, - stepName: message.stepName, - jobId: message.jobId, - createdAt: message.createdAt - } + return [ + { + role: 'assistant', + content: message.content, + // Only the message a turn is still writing: a finalized reasoning-only + // message must not look in progress. + streaming: message.pending || undefined, + reasoning: message.reasoning, + stepName: message.stepName, + jobId: message.jobId, + createdAt: message.createdAt + } + ] } }) } @@ -274,9 +293,10 @@ export class FlowChatViewHost implements ChatViewHost { // Per-message actions storedImages = () => undefined - /** Send the user message at this transcript position again. */ + /** Send the user message at this transcript position again. The position is in + * `displayMessages`, which holds more entries than the chat's messages. */ retryRequest = (messageIndex: number) => { - const message = this.#state.messages[messageIndex] + const message = this.displayMessages[messageIndex] if (!message || message.role !== 'user' || this.loading) return void this.sendRequest({ instructions: message.content }) } diff --git a/frontend/src/lib/components/flows/conversations/flowChatViewHost.test.ts b/frontend/src/lib/components/flows/conversations/flowChatViewHost.test.ts index a7e953ecf5..0b62a4bf5b 100644 --- a/frontend/src/lib/components/flows/conversations/flowChatViewHost.test.ts +++ b/frontend/src/lib/components/flows/conversations/flowChatViewHost.test.ts @@ -100,6 +100,38 @@ describe('toDisplayMessages', () => { }) }) + it('shows the thinking that led to a call as its own card, and retries by transcript position', async () => { + const rows = [ + message({ role: 'user', content: 'first' }), + message({ + role: 'tool', + content: 'Used search tool', + reasoning: 'why', + tool: { name: 'search', status: 'success' } + }), + message({ role: 'assistant', content: 'done' }), + message({ role: 'user', content: 'second' }) + ] + const display = toDisplayMessages(rows) + expect(display.map((m) => [m.role, m.content])).toEqual([ + ['user', 'first'], + ['assistant', ''], + ['tool', 'Used search tool'], + ['assistant', 'done'], + ['user', 'second'] + ]) + expect(display[1]).toMatchObject({ reasoning: 'why' }) + expect(display[1]).not.toHaveProperty('streaming') + + const { chat } = fakeChat(idleState({ messages: rows })) + const host = new FlowChatViewHost(chat) + host.retryRequest(4) + await vi.waitFor(() => + expect(chat.sendMessage).toHaveBeenCalledWith('second', expect.anything()) + ) + host.dispose() + }) + it('does not flag a turn whose tool failed but whose agent still answered', () => { const display = toDisplayMessages([ message({ role: 'user', content: 'try' }),