mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-21 16:02:36 +00:00
fix: claim a textless assistant row only within the newest turn
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5.1
parent
6692e9d0c0
commit
924ba92c68
@@ -1,7 +1,8 @@
|
||||
-- A chat is rebuilt from its rows without reading jobs. A tool row for a script or flow
|
||||
-- names the tool's own job, which holds its call; an MCP tool and a provider-native tool
|
||||
-- run inside the agent's job, whose result lists every call of the turn with nothing tying
|
||||
-- one to a row. For those the row carries the call itself.
|
||||
-- names the tool's own job, which holds its call. An MCP tool runs inside the agent's job,
|
||||
-- whose result lists every call of the turn with nothing tying one to a row, so its row
|
||||
-- carries the call itself; a provider-native web search carries only its citations, the
|
||||
-- provider never returning the query.
|
||||
ALTER TABLE flow_conversation_message ADD COLUMN tool_arguments TEXT;
|
||||
ALTER TABLE flow_conversation_message ADD COLUMN tool_result TEXT;
|
||||
|
||||
|
||||
@@ -36,9 +36,12 @@ pub struct FlowConversationMessage {
|
||||
pub created_seq: i64,
|
||||
pub step_name: Option<String>,
|
||||
pub success: bool,
|
||||
/// The call behind a tool row whose tool has no job of its own — an MCP tool, or a
|
||||
/// provider-native one. Read back from the job otherwise, and null here.
|
||||
/// An MCP tool call's arguments, which run inside the agent's job and so have none of
|
||||
/// their own. Null for a web search, whose query the provider does not return, and for
|
||||
/// a Windmill tool, whose own job holds its call.
|
||||
pub tool_arguments: Option<String>,
|
||||
/// 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<String>,
|
||||
/// The thinking of the iteration that produced this row. The agent job keeps the
|
||||
/// turn's thinking as one string.
|
||||
|
||||
+23
-17
@@ -612,26 +612,32 @@ 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)
|
||||
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 thinking alone, its text arriving as a tool call
|
||||
// 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) {
|
||||
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
|
||||
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)
|
||||
}
|
||||
}
|
||||
if (i >= 0) {
|
||||
const m = messages[i]
|
||||
|
||||
@@ -666,6 +666,62 @@ describe('createChat with server history', () => {
|
||||
])
|
||||
})
|
||||
|
||||
test('reasoning rows of a turn with a tool round stay with that turn after a stopped one', 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: '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' }
|
||||
),
|
||||
stream_offset: 4,
|
||||
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(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' })
|
||||
])
|
||||
: 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(
|
||||
|
||||
Reference in New Issue
Block a user