mirror of
https://github.com/simstudioai/sim.git
synced 2026-09-24 15:45:35 +08:00
fix(mothership): stop inlining full execution traces for the logs context (#5353)
* fix(mothership): stop inlining full execution traces for the logs context Tagging a run via "Troubleshoot in Chat" (or any @-mention of a logs context) resolved through processExecutionLogFromDb, which materialized the ENTIRE execution trace (every block's input/output, nested tool-call spans) and inlined it directly into the prompt. For any non-trivial run this repeatedly blew the context window, forcing multiple compactions and eventually auto-stopping the agent before it could investigate anything. Every other context resolver in this file already avoids this by sending a lightweight pointer instead of a full inline dump (workflow/blocks/ workflow_block contexts point into the VFS). Logs contexts have no VFS materialization to point at, but the equivalent lightweight mechanism already exists as a tool: query_logs supports incremental disclosure (overview for timing/cost, full for a scoped block's input/output, or pattern to grep the trace) and is already registered for the mothership agent. Now processExecutionLogFromDb sends a compact summary (id, workflow, level, trigger, timing, cost) plus a note pointing the model at query_logs with the executionId, instead of materializing and embedding the trace. Also drops the now-unused executionData column from the select projection, so resolving a logs context no longer fetches a potentially large JSONB blob it never reads. * improvement(mothership): send a bounded block overview instead of a bare tool pointer Follow-up to the previous commit's fix (stop inlining full execution traces). A pure text pointer telling the model to call query_logs made the agent's very first useful action against a tagged run contingent on it noticing and correctly acting on prose in a JSON blob it may only skim — every sibling resolver in this file instead returns a deterministic mechanism (a VFS path) the model reads on demand. There's no VFS materialization for individual execution logs, but the same deterministic signal is available cheaply: toOverview() (the exact projection query_logs's own "overview" view already returns) walks the raw trace spans and produces a compact tree — block name/type/status/ timing/cost, no input or output — without touching large-value refs at all. The summary now includes that tree, so the model can see which block failed on the first turn, and the note narrows to what still requires a tool call: a block's actual input/output/error, or a grep. materializeExecutionData is still called, but it's a no-op for the common inline case (it only unwraps a top-level object-storage pointer for runs whose whole trace was offloaded as one blob) and was needed to reach traceSpans at all for those heavier runs — exactly the runs most worth an overview. A serialized-size cap (mirroring query-logs.ts's own truncation fallback, scaled down since this lands in the prompt unconditionally) drops the overview if a pathological span count pushes it over budget, falling back to the note alone. Extends the tests: the happy path now asserts the overview tree is present and that no raw input/output payload leaks into the serialized summary, plus a new test for the size-cap fallback.
This commit is contained in:
@@ -2,12 +2,18 @@
|
||||
* @vitest-environment node
|
||||
*/
|
||||
|
||||
import { dbChainMock, dbChainMockFns, workflowAuthzMockFns } from '@sim/testing'
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import type { ChatContext } from '@/stores/panel'
|
||||
|
||||
const { getSkillById } = vi.hoisted(() => ({ getSkillById: vi.fn() }))
|
||||
|
||||
vi.mock('@/lib/workflows/skills/operations', () => ({ getSkillById }))
|
||||
/**
|
||||
* Overrides the global `@sim/db` mock: the logs-context tests below need
|
||||
* controllable row data, which the stable `dbChainMockFns.limit` provides.
|
||||
*/
|
||||
vi.mock('@sim/db', () => dbChainMock)
|
||||
|
||||
import { processContextsServer } from './process-contents'
|
||||
|
||||
@@ -67,3 +73,186 @@ describe('processContextsServer - skill contexts', () => {
|
||||
expect(result).toEqual([])
|
||||
})
|
||||
})
|
||||
|
||||
describe('processContextsServer - logs contexts', () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
})
|
||||
|
||||
it('resolves a tagged run to a compact summary with a block overview, never raw input/output', async () => {
|
||||
dbChainMockFns.limit.mockResolvedValueOnce([
|
||||
{
|
||||
id: 'log-1',
|
||||
workflowId: 'wf-1',
|
||||
workspaceId: 'ws-1',
|
||||
executionId: 'exec-1',
|
||||
level: 'error',
|
||||
trigger: 'manual',
|
||||
startedAt: new Date('2026-01-01T00:00:00.000Z'),
|
||||
endedAt: new Date('2026-01-01T00:00:01.000Z'),
|
||||
totalDurationMs: 1000,
|
||||
executionData: {
|
||||
traceSpans: [
|
||||
{
|
||||
id: 'span-1',
|
||||
blockId: 'block-1',
|
||||
name: 'Agent 1',
|
||||
type: 'agent',
|
||||
status: 'failed',
|
||||
duration: 500,
|
||||
input: { prompt: 'do the thing' },
|
||||
output: { error: '429 No active subscription' },
|
||||
},
|
||||
],
|
||||
},
|
||||
costTotal: '0.05',
|
||||
workflowName: 'My Flow',
|
||||
},
|
||||
])
|
||||
workflowAuthzMockFns.mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValueOnce({
|
||||
allowed: true,
|
||||
workflow: { workspaceId: 'ws-1' },
|
||||
})
|
||||
|
||||
const result = await processContextsServer(
|
||||
[{ kind: 'logs', executionId: 'exec-1', label: 'My Flow' } as ChatContext],
|
||||
'user-1',
|
||||
'hello',
|
||||
'ws-1'
|
||||
)
|
||||
|
||||
expect(result).toHaveLength(1)
|
||||
expect(result[0].type).toBe('logs')
|
||||
expect(result[0].tag).toBe('@My Flow')
|
||||
|
||||
const summary = JSON.parse(result[0].content)
|
||||
expect(summary).toMatchObject({
|
||||
executionId: 'exec-1',
|
||||
workflowId: 'wf-1',
|
||||
workflowName: 'My Flow',
|
||||
level: 'error',
|
||||
trigger: 'manual',
|
||||
totalDurationMs: 1000,
|
||||
cost: { total: 0.05 },
|
||||
overview: [
|
||||
{
|
||||
id: 'span-1',
|
||||
blockId: 'block-1',
|
||||
name: 'Agent 1',
|
||||
type: 'agent',
|
||||
status: 'failed',
|
||||
durationMs: 500,
|
||||
},
|
||||
],
|
||||
})
|
||||
const serialized = JSON.stringify(summary)
|
||||
expect(serialized).not.toContain('do the thing')
|
||||
expect(serialized).not.toContain('429 No active subscription')
|
||||
expect(summary.note).toContain('query_logs')
|
||||
expect(summary.note).toContain('exec-1')
|
||||
})
|
||||
|
||||
it('drops the overview (keeping the rest of the summary) when it exceeds the size cap', async () => {
|
||||
const traceSpans = Array.from({ length: 2000 }, (_, i) => ({
|
||||
id: `span-${i}`,
|
||||
blockId: `block-${i}`,
|
||||
name: `Block ${i}`,
|
||||
type: 'agent',
|
||||
status: 'success',
|
||||
duration: 10,
|
||||
}))
|
||||
dbChainMockFns.limit.mockResolvedValueOnce([
|
||||
{
|
||||
id: 'log-1',
|
||||
workflowId: 'wf-1',
|
||||
workspaceId: 'ws-1',
|
||||
executionId: 'exec-1',
|
||||
level: 'error',
|
||||
trigger: 'manual',
|
||||
startedAt: new Date('2026-01-01T00:00:00.000Z'),
|
||||
endedAt: null,
|
||||
totalDurationMs: null,
|
||||
executionData: { traceSpans },
|
||||
costTotal: null,
|
||||
workflowName: 'My Flow',
|
||||
},
|
||||
])
|
||||
workflowAuthzMockFns.mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValueOnce({
|
||||
allowed: true,
|
||||
workflow: { workspaceId: 'ws-1' },
|
||||
})
|
||||
|
||||
const result = await processContextsServer(
|
||||
[{ kind: 'logs', executionId: 'exec-1', label: 'My Flow' } as ChatContext],
|
||||
'user-1',
|
||||
'hello',
|
||||
'ws-1'
|
||||
)
|
||||
|
||||
const summary = JSON.parse(result[0].content)
|
||||
expect(summary.overview).toBeUndefined()
|
||||
expect(summary.executionId).toBe('exec-1')
|
||||
expect(summary.note).toContain('query_logs')
|
||||
})
|
||||
|
||||
it('drops a log context when the workflow is outside the current workspace', async () => {
|
||||
dbChainMockFns.limit.mockResolvedValueOnce([
|
||||
{
|
||||
id: 'log-1',
|
||||
workflowId: 'wf-1',
|
||||
workspaceId: 'ws-other',
|
||||
executionId: 'exec-1',
|
||||
level: 'error',
|
||||
trigger: 'manual',
|
||||
startedAt: new Date('2026-01-01T00:00:00.000Z'),
|
||||
endedAt: null,
|
||||
totalDurationMs: null,
|
||||
costTotal: null,
|
||||
workflowName: 'My Flow',
|
||||
},
|
||||
])
|
||||
workflowAuthzMockFns.mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValueOnce({
|
||||
allowed: true,
|
||||
workflow: { workspaceId: 'ws-other' },
|
||||
})
|
||||
|
||||
const result = await processContextsServer(
|
||||
[{ kind: 'logs', executionId: 'exec-1', label: 'My Flow' } as ChatContext],
|
||||
'user-1',
|
||||
'hello',
|
||||
'ws-1'
|
||||
)
|
||||
|
||||
expect(result).toEqual([])
|
||||
})
|
||||
|
||||
it('drops a log context the user is not authorized to read', async () => {
|
||||
dbChainMockFns.limit.mockResolvedValueOnce([
|
||||
{
|
||||
id: 'log-1',
|
||||
workflowId: 'wf-1',
|
||||
workspaceId: 'ws-1',
|
||||
executionId: 'exec-1',
|
||||
level: 'error',
|
||||
trigger: 'manual',
|
||||
startedAt: new Date('2026-01-01T00:00:00.000Z'),
|
||||
endedAt: null,
|
||||
totalDurationMs: null,
|
||||
costTotal: null,
|
||||
workflowName: 'My Flow',
|
||||
},
|
||||
])
|
||||
workflowAuthzMockFns.mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValueOnce({
|
||||
allowed: false,
|
||||
})
|
||||
|
||||
const result = await processContextsServer(
|
||||
[{ kind: 'logs', executionId: 'exec-1', label: 'My Flow' } as ChatContext],
|
||||
'user-1',
|
||||
'hello',
|
||||
'ws-1'
|
||||
)
|
||||
|
||||
expect(result).toEqual([])
|
||||
})
|
||||
})
|
||||
|
||||
@@ -6,6 +6,7 @@ import {
|
||||
getActiveWorkflowRecord,
|
||||
} from '@sim/platform-authz/workflow'
|
||||
import { and, eq, isNull, ne } from 'drizzle-orm'
|
||||
import { QueryLogs } from '@/lib/copilot/generated/tool-catalog-v1'
|
||||
import { normalizeVfsSegment } from '@/lib/copilot/vfs/normalize-segment'
|
||||
import {
|
||||
buildVfsFolderPathMap,
|
||||
@@ -18,6 +19,8 @@ import {
|
||||
encodeVfsSegment,
|
||||
} from '@/lib/copilot/vfs/path-utils'
|
||||
import { getAllowedIntegrationsFromEnv } from '@/lib/core/config/env-flags'
|
||||
import { toOverview } from '@/lib/logs/log-views'
|
||||
import type { TraceSpan } from '@/lib/logs/types'
|
||||
import { getTableById } from '@/lib/table/service'
|
||||
import { getWorkspaceFileFolderPath } from '@/lib/uploads/contexts/workspace/workspace-file-folder-manager'
|
||||
import { getWorkspaceFile } from '@/lib/uploads/contexts/workspace/workspace-file-manager'
|
||||
@@ -565,6 +568,31 @@ async function processWorkflowBlockFromDb(
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Cap on the serialized summary (including the block overview tree) sent for
|
||||
* a tagged run. `toOverview` already excludes every block's input/output, so
|
||||
* this is a safety net against pathological span counts, not the primary
|
||||
* defense — mirrors `MAX_FULL_RESULT_BYTES` in `query-logs.ts`, scaled down
|
||||
* since this lands in the prompt unconditionally rather than behind an
|
||||
* explicit tool call.
|
||||
*/
|
||||
const MAX_LOG_SUMMARY_BYTES = 64 * 1024
|
||||
|
||||
/**
|
||||
* Resolve a tagged run to a compact summary instead of its full execution
|
||||
* trace. A run's trace can carry every block's input/output plus nested
|
||||
* tool-call spans, which is unbounded and would repeatedly blow the context
|
||||
* window if inlined directly. The summary includes the block-level overview
|
||||
* tree (name/type/status/timing/cost, no input or output — the same
|
||||
* projection `query_logs`'s `overview` view returns) so the model can see
|
||||
* which block failed without a round trip, and points it at `query_logs` for
|
||||
* that block's actual input/output/error, or to grep the trace.
|
||||
*
|
||||
* `materializeExecutionData` only unwraps a top-level object-storage pointer,
|
||||
* for runs whose whole trace was offloaded as one blob — a no-op for the
|
||||
* common inline case. Individual span input/output stay as large-value refs;
|
||||
* `toOverview` never resolves those.
|
||||
*/
|
||||
async function processExecutionLogFromDb(
|
||||
executionId: string,
|
||||
userId: string | undefined,
|
||||
@@ -610,12 +638,14 @@ async function processExecutionLogFromDb(
|
||||
}
|
||||
}
|
||||
|
||||
// Heavy execution data may live in object storage; resolve the pointer.
|
||||
const { materializeExecutionData } = await import('@/lib/logs/execution/trace-store')
|
||||
const executionData = (await materializeExecutionData(
|
||||
log.executionData as Record<string, unknown> | null,
|
||||
{ workspaceId: log.workspaceId, workflowId: log.workflowId, executionId: log.executionId }
|
||||
)) as any
|
||||
)) as { traceSpans?: TraceSpan[] } | undefined
|
||||
const overview = executionData?.traceSpans?.length
|
||||
? toOverview(executionData.traceSpans)
|
||||
: undefined
|
||||
|
||||
const summary = {
|
||||
id: log.id,
|
||||
@@ -627,13 +657,13 @@ async function processExecutionLogFromDb(
|
||||
endedAt: log.endedAt?.toISOString?.() || (log.endedAt ? String(log.endedAt) : null),
|
||||
totalDurationMs: log.totalDurationMs ?? null,
|
||||
workflowName: log.workflowName || '',
|
||||
executionData: executionData
|
||||
? {
|
||||
traceSpans: executionData.traceSpans || undefined,
|
||||
errorDetails: executionData.errorDetails || undefined,
|
||||
}
|
||||
: undefined,
|
||||
cost: log.costTotal != null ? { total: Number(log.costTotal) } : undefined,
|
||||
overview,
|
||||
note: `For a block's input/output/error, or to grep the trace, call ${QueryLogs.id} with executionId: '${log.executionId}' — view: 'full' (scope with blockId or blockName), or pattern to grep.`,
|
||||
}
|
||||
|
||||
if (overview && JSON.stringify(summary).length > MAX_LOG_SUMMARY_BYTES) {
|
||||
summary.overview = undefined
|
||||
}
|
||||
|
||||
const content = JSON.stringify(summary)
|
||||
|
||||
Reference in New Issue
Block a user