diff --git a/frontend/src/lib/components/copilot/chat/ToolExecutionDisplay.svelte b/frontend/src/lib/components/copilot/chat/ToolExecutionDisplay.svelte index e9eac7e8f4..eb93d8376e 100644 --- a/frontend/src/lib/components/copilot/chat/ToolExecutionDisplay.svelte +++ b/frontend/src/lib/components/copilot/chat/ToolExecutionDisplay.svelte @@ -114,9 +114,13 @@ // otherwise the error set on its status would be invisible. const detailsAvailable = $derived(message.showDetails === true || message.error !== undefined) + // Queued and running calls stay closed: their label already says what they are doing, and + // opening them would only show empty logs and result. A call opens while its arguments + // stream, while it waits on the user, and once it has an error or details to keep in view. let isExpanded = $derived( Boolean( - (detailsAvailable && (!isSuccessful || !autoCollapseDetails)) || + (detailsAvailable && + (message.error !== undefined || (isSuccessful && !autoCollapseDetails))) || (message.isStreamingArguments && hasParameters) || (message.isLoading && message.needsConfirmation) ) diff --git a/frontend/src/lib/components/copilot/chat/datatableTools.ts b/frontend/src/lib/components/copilot/chat/datatableTools.ts index 0656bf43a9..7a743ab783 100644 --- a/frontend/src/lib/components/copilot/chat/datatableTools.ts +++ b/frontend/src/lib/components/copilot/chat/datatableTools.ts @@ -344,6 +344,7 @@ export function getDatatableTools(): SessionTool<{}>[] { def: getExecDatatableSqlToolDef(), requiresConfirmation: true, confirmationMessage: 'Execute SQL on datatable', + queuedLabel: (args) => `Execute SQL on "${args?.datatable_name ?? 'datatable'}"`, showDetails: true, fn: async ({ args, workspace, toolId, toolCallbacks }) => { const parsedArgs = getExecDatatableSqlSchema().parse(args) diff --git a/frontend/src/lib/components/copilot/chat/toolGroups.test.ts b/frontend/src/lib/components/copilot/chat/toolGroups.test.ts index f75332c87d..a4fc4ce0a8 100644 --- a/frontend/src/lib/components/copilot/chat/toolGroups.test.ts +++ b/frontend/src/lib/components/copilot/chat/toolGroups.test.ts @@ -91,6 +91,40 @@ describe('groupToolRuns', () => { ).toEqual([0, 1, 2, 3]) }) + it('folds the calls queued behind another call into one waiting row', () => { + const sql = (extra = {}) => + tool('exec_datatable_sql', { datatable_name: 'main', sql: 'select 1' }, extra) + const queued = { isQueued: true, content: 'Execute SQL on "main"' } + const batch = [ + sql({ needsConfirmation: true, isLoading: true }), + sql(queued), + tool('search_workspace', {}, { ...queued, content: 'Search workspace' }) + ] + expect(shape(batch)).toEqual([0, { waiting: [1, 2] }]) + expect(header(batch)).toBe('2 more calls waiting: Execute SQL on "main", Search workspace') + // The call next in line shows as itself, before anything runs and between two calls. + expect(shape([assistant(''), sql(queued), sql(queued)])).toEqual([0, 1, { waiting: [2] }]) + expect(shape([sql(), sql(queued), sql(queued)])).toEqual([0, 1, { waiting: [2] }]) + // Queued edits of one flow keep their edit group. + const flow = { path: 'f/a/flow' } + expect( + shape([ + tool('search_workspace', {}, { isLoading: true }), + tool('patch_flow_json', flow, queued), + tool('patch_flow_json', flow, queued) + ]) + ).toEqual([0, { edit: [1, 2] }]) + // A waiting row stops before queued calls that form their own group. + expect( + shape([ + sql({ needsConfirmation: true, isLoading: true }), + sql(queued), + tool('patch_flow_json', flow, queued), + tool('patch_flow_json', flow, queued) + ]) + ).toEqual([0, { waiting: [1] }, { edit: [2, 3] }]) + }) + it('keeps an edit whose arguments are still streaming in the group it follows', () => { const flow = { path: 'f/a/flow' } expect( @@ -115,7 +149,10 @@ describe('groupToolRuns', () => { it('lets a call with no arguments yet join a group but not start one', () => { const queued = (toolName: string) => tool(toolName, {}, { parameters: undefined, isQueued: true }) - expect(shape([queued('delete_app_file'), queued('delete_app_runnable')])).toEqual([0, 1]) + expect(shape([queued('delete_app_file'), queued('delete_app_runnable')])).toEqual([ + 0, + { waiting: [1] } + ]) expect(shape([tool('patch_app_file', { path: 'f/a/x' }), queued('delete_app_file')])).toEqual([ { edit: [0, 1] } ]) diff --git a/frontend/src/lib/components/copilot/chat/toolGroups.ts b/frontend/src/lib/components/copilot/chat/toolGroups.ts index fd4cbc28ee..94936d4293 100644 --- a/frontend/src/lib/components/copilot/chat/toolGroups.ts +++ b/frontend/src/lib/components/copilot/chat/toolGroups.ts @@ -88,8 +88,9 @@ const READ_TOOLS = new Set([ export type ToolGroup = { kind: 'group' - /** 'edit': edits of one flow or app. 'explore': consecutive lookups of anything. */ - groupKind: 'edit' | 'explore' + /** 'edit': edits of one flow or app. 'explore': consecutive lookups of anything. + * 'waiting': calls queued behind the one running or awaiting the user. */ + groupKind: 'edit' | 'explore' | 'waiting' /** First call's id, so the group keeps its identity (and expand state) as it grows. */ key: string /** Edit groups: the item's path, or '' for the flow open in the editor. */ @@ -229,18 +230,58 @@ function editGroupAt(messages: DisplayMessage[], start: number): ToolGroup | und return toolGroup(messages, start, end, 'edit', item.path) } +function isWaitingCall(message: DisplayMessage): boolean { + return message.role === 'tool' && message.isQueued === true && message.error === undefined +} + +function inFlight(message: DisplayMessage): boolean { + return ( + message.role === 'tool' && + Boolean(message.isLoading || message.isQueued || message.isStreamingArguments) + ) +} + +// Calls of one turn run one at a time, so the ones queued behind a call fold into a single +// row: each would otherwise sit as its own row until its turn. Only behind a call still in +// flight: between two calls the previous one has settled, and the call next in line must +// still show as itself rather than dip into the row and back out. +function waitingGroupAt( + messages: DisplayMessage[], + start: number, + previous: ChatItem | undefined +): ToolGroup | undefined { + if (!isWaitingCall(messages[start]) || !previous) return undefined + const ahead = + previous.kind === 'group' ? previous.entries.map((e) => e.message) : [previous.message] + if (!ahead.some(inFlight)) return undefined + const end = runEnd(messages, start, (m, j) => isWaitingCall(m) && !kindGroupAt(messages, j)) + return { + kind: 'group', + groupKind: 'waiting', + key: (messages[start] as ToolDisplayMessage).tool_call_id, + target: '', + entries: messages.slice(start, end + 1).map((message, k) => ({ message, index: start + k })) + } +} + +// An edit group wins over an explore group: its reads belong to the edits they prepare, so an +// explore run also stops before a read that starts one. Both win over a waiting row, which +// stops before either: its header says less than `Edit f/a/flow ยท 2 changes`. +function kindGroupAt(messages: DisplayMessage[], start: number): ToolGroup | undefined { + const explores = (m: DisplayMessage, j: number) => isReadCall(m) && !editGroupAt(messages, j) + return ( + editGroupAt(messages, start) ?? + (isReadCall(messages[start]) + ? toolGroup(messages, start, runEnd(messages, start, explores), 'explore', '') + : undefined) + ) +} + export function groupToolRuns(messages: DisplayMessage[]): ChatItem[] { const items: ChatItem[] = [] let i = 0 while (i < messages.length) { - // An edit group wins over an explore group: its reads belong to the edits they prepare, - // so an explore run also stops before a read that starts one. - const explores = (m: DisplayMessage, j: number) => isReadCall(m) && !editGroupAt(messages, j) - const group = - editGroupAt(messages, i) ?? - (isReadCall(messages[i]) - ? toolGroup(messages, i, runEnd(messages, i, explores), 'explore', '') - : undefined) + const group = kindGroupAt(messages, i) ?? waitingGroupAt(messages, i, items.at(-1)) if (group) { items.push(group) i = group.entries.at(-1)!.index + 1 @@ -307,6 +348,14 @@ export function groupHeader( const calls = group.entries .map((e) => e.message) .filter((m): m is ToolDisplayMessage => m.role === 'tool') + if (group.groupKind === 'waiting') { + // A queued row's content is its imperative label, e.g. `Execute SQL on "main"`. + const names = [...new Set(calls.map((m) => m.content))].join(', ') + return { + prefix: `${plural(calls.length, 'more call')} waiting:`, + label: names + } + } if (group.groupKind === 'edit') { const edits = calls.filter( (m) => Object.hasOwn(EDIT_TOOLS, m.toolName ?? '') && !callFailed(m)