mirror of
https://github.com/simstudioai/sim.git
synced 2026-09-24 15:45:35 +08:00
improvement(copilot): stop persisting tool-call result outputs in transcripts (#4829)
Opening a Mothership task could take many seconds because a single persisted assistant message in copilot_messages.content can reach hundreds of MB, almost entirely inside contentBlocks[].toolCall.result.output (e.g. a get_workflow_logs or run_workflow result). The DB query is ~2ms; the cost is detoasting that payload, shipping it to the browser, and parsing it. These outputs are dead weight on the Sim side: they are never rendered (the thread shows only tool name/title/status) and never replayed to the model (the upstream copilot service owns conversation memory). So drop result.output before it is persisted, keeping result.success/error plus the tool metadata. - add stripToolResultOutput() in persisted-message.ts - apply it in messages-store toRow (covers every write path) and in loadCopilotChatMessages (existing rows render fast on read) Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
919fa5274b
commit
3a2ebd12b9
@@ -71,6 +71,34 @@ describe('lifecycle copilot chat reads (cutover to copilot_messages)', () => {
|
||||
expect(dbChainMockFns.orderBy).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('strips tool-result output on read, keeping success/error', async () => {
|
||||
const toolMsg = {
|
||||
id: 'm-tool',
|
||||
role: 'assistant',
|
||||
content: '',
|
||||
timestamp: '2026-01-01T00:00:02.000Z',
|
||||
contentBlocks: [
|
||||
{
|
||||
type: 'tool',
|
||||
phase: 'call',
|
||||
toolCall: {
|
||||
id: 'tc-1',
|
||||
name: 'get_workflow_logs',
|
||||
state: 'success',
|
||||
result: { success: true, output: { huge: 'x'.repeat(5000) } },
|
||||
},
|
||||
},
|
||||
],
|
||||
}
|
||||
dbChainMockFns.limit.mockResolvedValueOnce([chatRow])
|
||||
dbChainMockFns.orderBy.mockResolvedValueOnce([{ content: toolMsg }])
|
||||
|
||||
const result = await getAccessibleCopilotChatWithMessages(CHAT_ID, USER_ID)
|
||||
|
||||
expect(result?.messages?.[0].contentBlocks?.[0].toolCall?.result).toEqual({ success: true })
|
||||
expect(JSON.stringify(result?.messages)).not.toContain('huge')
|
||||
})
|
||||
|
||||
it('returns an empty transcript for a chat with no messages', async () => {
|
||||
dbChainMockFns.limit.mockResolvedValueOnce([chatRow])
|
||||
dbChainMockFns.orderBy.mockResolvedValueOnce([])
|
||||
|
||||
@@ -6,7 +6,7 @@ import {
|
||||
getActiveWorkflowRecord,
|
||||
} from '@sim/workflow-authz'
|
||||
import { and, asc, eq, isNull, sql } from 'drizzle-orm'
|
||||
import type { PersistedMessage } from '@/lib/copilot/chat/persisted-message'
|
||||
import { type PersistedMessage, stripToolResultOutput } from '@/lib/copilot/chat/persisted-message'
|
||||
import {
|
||||
assertActiveWorkspaceAccess,
|
||||
checkWorkspaceAccess,
|
||||
@@ -84,7 +84,8 @@ export async function loadCopilotChatMessages(chatId: string): Promise<Persisted
|
||||
asc(copilotMessages.createdAt),
|
||||
asc(copilotMessages.id)
|
||||
)
|
||||
return rows.map((row) => row.content as PersistedMessage)
|
||||
// Also strip on read: rows written before the backfill still carry outputs.
|
||||
return rows.map((row) => stripToolResultOutput(row.content as PersistedMessage))
|
||||
}
|
||||
|
||||
type CopilotChatAuthRow = Pick<
|
||||
|
||||
@@ -26,6 +26,31 @@ const assistantMsg: PersistedMessage = {
|
||||
timestamp: '2026-01-01T00:00:01.000Z',
|
||||
}
|
||||
|
||||
const toolMsg: PersistedMessage = {
|
||||
id: 'msg-tool-1',
|
||||
role: 'assistant',
|
||||
content: '',
|
||||
timestamp: '2026-01-01T00:00:02.000Z',
|
||||
contentBlocks: [
|
||||
{
|
||||
type: 'tool',
|
||||
phase: 'call',
|
||||
toolCall: {
|
||||
id: 'tc-1',
|
||||
name: 'get_workflow_logs',
|
||||
state: 'error',
|
||||
params: { workflowId: 'wf-1' },
|
||||
result: { success: false, output: { huge: 'x'.repeat(5000) }, error: 'too big' },
|
||||
},
|
||||
},
|
||||
],
|
||||
}
|
||||
|
||||
/** The persisted `content` of the most recently inserted row at `index`. */
|
||||
function lastRowContent(index: number): PersistedMessage {
|
||||
return lastValuesRows()[index].content as PersistedMessage
|
||||
}
|
||||
|
||||
/** The first arg passed to the most recent `.values(...)` call. */
|
||||
function lastValuesRows() {
|
||||
const calls = dbChainMockFns.values.mock.calls
|
||||
@@ -131,6 +156,14 @@ describe('messages-store', () => {
|
||||
'connection lost'
|
||||
)
|
||||
})
|
||||
|
||||
it('strips tool-result output before persisting, keeping success/error', async () => {
|
||||
await appendCopilotChatMessages('chat-1', [toolMsg])
|
||||
|
||||
const toolCall = lastRowContent(0).contentBlocks?.[0].toolCall
|
||||
expect(toolCall?.result).toEqual({ success: false, error: 'too big' })
|
||||
expect(JSON.stringify(lastValuesRows())).not.toContain('huge')
|
||||
})
|
||||
})
|
||||
|
||||
describe('replaceCopilotChatMessages', () => {
|
||||
@@ -192,5 +225,13 @@ describe('messages-store', () => {
|
||||
|
||||
await expect(replaceCopilotChatMessages('chat-1', [userMsg])).rejects.toThrow('tx aborted')
|
||||
})
|
||||
|
||||
it('strips tool-result output before persisting, keeping success/error', async () => {
|
||||
await replaceCopilotChatMessages('chat-1', [toolMsg])
|
||||
|
||||
const toolCall = lastRowContent(0).contentBlocks?.[0].toolCall
|
||||
expect(toolCall?.result).toEqual({ success: false, error: 'too big' })
|
||||
expect(JSON.stringify(lastValuesRows())).not.toContain('huge')
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import { db } from '@sim/db'
|
||||
import { copilotMessages } from '@sim/db/schema'
|
||||
import { and, eq, notInArray, sql } from 'drizzle-orm'
|
||||
import type { PersistedMessage } from '@/lib/copilot/chat/persisted-message'
|
||||
import { type PersistedMessage, stripToolResultOutput } from '@/lib/copilot/chat/persisted-message'
|
||||
import type { DbOrTx } from '@/lib/db/types'
|
||||
|
||||
/**
|
||||
@@ -31,7 +31,7 @@ function toRow(
|
||||
chatId,
|
||||
messageId: message.id,
|
||||
role: message.role,
|
||||
content: message,
|
||||
content: stripToolResultOutput(message),
|
||||
seq,
|
||||
model: options?.chatModel ?? null,
|
||||
streamId: options?.streamId ?? null,
|
||||
|
||||
@@ -8,6 +8,8 @@ import {
|
||||
buildPersistedAssistantMessage,
|
||||
buildPersistedUserMessage,
|
||||
normalizeMessage,
|
||||
type PersistedMessage,
|
||||
stripToolResultOutput,
|
||||
} from './persisted-message'
|
||||
|
||||
describe('persisted-message', () => {
|
||||
@@ -234,3 +236,137 @@ describe('persisted-message', () => {
|
||||
expect(msg.contexts).toBeUndefined()
|
||||
})
|
||||
})
|
||||
|
||||
describe('stripToolResultOutput', () => {
|
||||
it('drops result.output but keeps success and error', () => {
|
||||
const message: PersistedMessage = {
|
||||
id: 'msg-1',
|
||||
role: 'assistant',
|
||||
content: '',
|
||||
timestamp: '2026-01-01T00:00:00.000Z',
|
||||
contentBlocks: [
|
||||
{
|
||||
type: 'tool',
|
||||
phase: 'call',
|
||||
toolCall: {
|
||||
id: 'tool-1',
|
||||
name: 'get_workflow_logs',
|
||||
state: 'error',
|
||||
params: { workflowId: 'wf-1' },
|
||||
display: { title: 'Reading logs' },
|
||||
result: { success: false, output: { huge: 'x'.repeat(1000) }, error: 'boom' },
|
||||
},
|
||||
},
|
||||
],
|
||||
}
|
||||
|
||||
const stripped = stripToolResultOutput(message)
|
||||
|
||||
expect(stripped.contentBlocks?.[0].toolCall).toEqual({
|
||||
id: 'tool-1',
|
||||
name: 'get_workflow_logs',
|
||||
state: 'error',
|
||||
params: { workflowId: 'wf-1' },
|
||||
display: { title: 'Reading logs' },
|
||||
result: { success: false, error: 'boom' },
|
||||
})
|
||||
expect(message.contentBlocks?.[0].toolCall?.result).toHaveProperty('output')
|
||||
})
|
||||
|
||||
it('omits error when the original result had none', () => {
|
||||
const message: PersistedMessage = {
|
||||
id: 'msg-1',
|
||||
role: 'assistant',
|
||||
content: '',
|
||||
timestamp: '2026-01-01T00:00:00.000Z',
|
||||
contentBlocks: [
|
||||
{
|
||||
type: 'tool',
|
||||
phase: 'call',
|
||||
toolCall: {
|
||||
id: 't',
|
||||
name: 'read',
|
||||
state: 'success',
|
||||
result: { success: true, output: [1, 2, 3] },
|
||||
},
|
||||
},
|
||||
],
|
||||
}
|
||||
|
||||
expect(stripToolResultOutput(message).contentBlocks?.[0].toolCall?.result).toEqual({
|
||||
success: true,
|
||||
})
|
||||
})
|
||||
|
||||
it('returns the same reference when there is nothing to strip', () => {
|
||||
const noBlocks: PersistedMessage = {
|
||||
id: 'u',
|
||||
role: 'user',
|
||||
content: 'hi',
|
||||
timestamp: '2026-01-01T00:00:00.000Z',
|
||||
}
|
||||
expect(stripToolResultOutput(noBlocks)).toBe(noBlocks)
|
||||
|
||||
const noOutput: PersistedMessage = {
|
||||
id: 'msg',
|
||||
role: 'assistant',
|
||||
content: 'done',
|
||||
timestamp: '2026-01-01T00:00:00.000Z',
|
||||
contentBlocks: [
|
||||
{ type: 'text', channel: 'assistant', content: 'done' },
|
||||
{ type: 'tool', phase: 'call', toolCall: { id: 't', name: 'read', state: 'pending' } },
|
||||
{
|
||||
type: 'tool',
|
||||
phase: 'call',
|
||||
toolCall: {
|
||||
id: 't2',
|
||||
name: 'read',
|
||||
state: 'error',
|
||||
result: { success: false, error: 'x' },
|
||||
},
|
||||
},
|
||||
],
|
||||
}
|
||||
expect(stripToolResultOutput(noOutput)).toBe(noOutput)
|
||||
})
|
||||
|
||||
it('strips every tool block while leaving text/thinking blocks intact', () => {
|
||||
const message: PersistedMessage = {
|
||||
id: 'msg',
|
||||
role: 'assistant',
|
||||
content: '',
|
||||
timestamp: '2026-01-01T00:00:00.000Z',
|
||||
contentBlocks: [
|
||||
{ type: 'text', channel: 'thinking', content: 'hmm' },
|
||||
{
|
||||
type: 'tool',
|
||||
phase: 'call',
|
||||
toolCall: {
|
||||
id: 'a',
|
||||
name: 'run_workflow',
|
||||
state: 'success',
|
||||
result: { success: true, output: { big: 1 } },
|
||||
},
|
||||
},
|
||||
{ type: 'text', channel: 'assistant', content: 'answer' },
|
||||
{
|
||||
type: 'tool',
|
||||
phase: 'call',
|
||||
toolCall: {
|
||||
id: 'b',
|
||||
name: 'read',
|
||||
state: 'success',
|
||||
result: { success: true, output: 'file contents' },
|
||||
},
|
||||
},
|
||||
],
|
||||
}
|
||||
|
||||
const blocks = stripToolResultOutput(message).contentBlocks ?? []
|
||||
expect(blocks[0]).toEqual({ type: 'text', channel: 'thinking', content: 'hmm' })
|
||||
expect(blocks[1].toolCall?.result).toEqual({ success: true })
|
||||
expect(blocks[2]).toEqual({ type: 'text', channel: 'assistant', content: 'answer' })
|
||||
expect(blocks[3].toolCall?.result).toEqual({ success: true })
|
||||
expect(JSON.stringify(blocks)).not.toContain('file contents')
|
||||
})
|
||||
})
|
||||
|
||||
@@ -79,6 +79,34 @@ export interface PersistedMessage {
|
||||
contexts?: PersistedMessageContext[]
|
||||
}
|
||||
|
||||
/**
|
||||
* Drop the `output` of every persisted tool result, keeping `success` and
|
||||
* `error`. Tool outputs are never rendered (the chat thread shows only the tool
|
||||
* name/title/status) and never replayed to the model (the upstream copilot
|
||||
* service owns conversation memory), so storing them only bloats
|
||||
* `copilot_messages.content` — a single `get_workflow_logs`/`run_workflow`
|
||||
* result can reach hundreds of MB and stall task loads.
|
||||
*
|
||||
* Applied on both the write path (so new rows never store outputs) and the read
|
||||
* path (so already-bloated rows still load fast). Returns the original
|
||||
* reference when there is nothing to strip, preserving memoized identity for
|
||||
* read-side consumers.
|
||||
*/
|
||||
export function stripToolResultOutput(message: PersistedMessage): PersistedMessage {
|
||||
if (!message.contentBlocks?.length) return message
|
||||
let changed = false
|
||||
const contentBlocks = message.contentBlocks.map((block) => {
|
||||
const toolCall = block.toolCall
|
||||
const result = toolCall?.result
|
||||
if (!toolCall || !result || typeof result !== 'object' || !('output' in result)) return block
|
||||
changed = true
|
||||
const strippedResult: { success: boolean; error?: string } = { success: result.success }
|
||||
if (result.error !== undefined) strippedResult.error = result.error
|
||||
return { ...block, toolCall: { ...toolCall, result: strippedResult } }
|
||||
})
|
||||
return changed ? { ...message, contentBlocks } : message
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Write: OrchestratorResult → PersistedMessage
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
Reference in New Issue
Block a user