mirror of
https://github.com/simstudioai/sim.git
synced 2026-10-13 15:48:31 +08:00
fix(chat): name stackless error subclasses and compact stored unfinished tool params (#8928)
This commit is contained in:
@@ -118,6 +118,16 @@ throw e
|
||||
Node.js v22.19.0
|
||||
`
|
||||
|
||||
/** A stackless error whose class keeps its parent's `name`, annotated by Node. */
|
||||
const STACKLESS_SUBCLASS_ERROR = `node:internal/modules/run_main:123
|
||||
triggerUncaughtException(
|
||||
^
|
||||
|
||||
[QuotaExceeded [Error]: over quota] { code: 'E_Q' }
|
||||
|
||||
Node.js v22.19.0
|
||||
`
|
||||
|
||||
const STACKLESS_ERROR_WITH_INLINE_PROPERTIES = `file:///home/user/.sim-function-abc.mjs:1
|
||||
throw e
|
||||
^
|
||||
@@ -168,6 +178,7 @@ describe('processCodeFailure', () => {
|
||||
],
|
||||
['a stackless error with no properties', STACKLESS_RANGE_ERROR, 'RangeError', 'limit hit'],
|
||||
['a stackless plain Error', STACKLESS_PLAIN_ERROR, 'Error', 'plain failure'],
|
||||
['a stackless error subclass', STACKLESS_SUBCLASS_ERROR, 'QuotaExceeded [Error]', 'over quota'],
|
||||
[
|
||||
'a stackless error with inline properties',
|
||||
STACKLESS_ERROR_WITH_INLINE_PROPERTIES,
|
||||
|
||||
@@ -2,15 +2,17 @@ import type { SandboxCodeResult, SandboxCommandResult } from '@/lib/execution/re
|
||||
|
||||
/**
|
||||
* A top-level error headline in a Python traceback or Node crash report: `Name: message`,
|
||||
* Node's `Name [CODE]: message`, or `[NameError: message]` (any inline properties after it)
|
||||
* for an error printed without a stack. It starts the line, so the properties Node prints indented beneath an error
|
||||
* (`code: 'X'`, `bytesRead: 0`) never match, and a space follows the colon, so
|
||||
* source-location headers (`node:internal/x:12`, `file:///x.mjs:3`) don't either. The
|
||||
* bracketed form needs an error-type name, since Node brackets other thrown values the
|
||||
* same way (`[Object: null prototype] {`, `[Function: handler]`).
|
||||
* Node's `Name [CODE]: message` (or `Class [name]: message` for an error class that keeps
|
||||
* its parent's name), or the same bracketed with any inline properties after it
|
||||
* (`[NameError: message] { … }`) for an error printed without a stack. It starts the line,
|
||||
* so the properties Node prints indented beneath an error (`code: 'X'`, `bytesRead: 0`)
|
||||
* never match, and a space follows the colon, so source-location headers
|
||||
* (`node:internal/x:12`, `file:///x.mjs:3`) don't either. The bracketed form needs an
|
||||
* error-type name or that annotation, since Node brackets other thrown values the same
|
||||
* way (`[Object: null prototype] {`, `[Function: handler]`).
|
||||
*/
|
||||
const ERROR_HEADLINE =
|
||||
/^(?:\[(?:[A-Za-z_$][\w.$]*)?(?:Error|Exception)|[A-Za-z_$][\w.$]*(?: \[[\w.$-]+\])?):(?:\s|$)/
|
||||
/^(?:\[(?:[A-Za-z_$][\w.$]*)?(?:Error|Exception)|\[?[A-Za-z_$][\w.$]* \[[\w.$-]+\]|[A-Za-z_$][\w.$]*):(?:\s|$)/
|
||||
|
||||
/**
|
||||
* Converts a failed code command into a code result, naming the error from the
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import type { PersistedContentBlock } from '@/lib/api/contracts/copilot-messages'
|
||||
import { getMothershipAttachmentPreviewUrl } from '@/lib/mothership/chat/attachment-preview'
|
||||
import { isLiveAssistantMessageId } from '@/lib/mothership/chat/live-message-id'
|
||||
import { isStoredAssistantMessage } from '@/lib/mothership/chat/live-message-id'
|
||||
import type { PersistedMessage } from '@/lib/mothership/chat/persisted-message'
|
||||
import {
|
||||
copyPersistedMessageContext,
|
||||
@@ -216,7 +216,7 @@ export function toDisplayMessage(msg: PersistedMessage): ChatMessage {
|
||||
}
|
||||
|
||||
if (msg.contentBlocks && msg.contentBlocks.length > 0) {
|
||||
const stored = msg.role === 'assistant' && !isLiveAssistantMessageId(msg.id)
|
||||
const stored = isStoredAssistantMessage(msg)
|
||||
const displayBlocks = msg.contentBlocks
|
||||
.map((block) => toDisplayBlock(block, stored))
|
||||
.filter((block): block is ContentBlock => !!block)
|
||||
|
||||
@@ -4,10 +4,19 @@ export function getLiveAssistantMessageId(streamId: string): string {
|
||||
|
||||
/**
|
||||
* True for the synthetic id of a streaming/just-streamed assistant message.
|
||||
* These ids exist only in the client's effective transcript — never in the
|
||||
* persisted one — so message-scoped server actions (e.g. fork) must not be
|
||||
* offered until the transcript refetch swaps in the persisted message id.
|
||||
* These ids exist only in the effective transcript built for the client —
|
||||
* never in the persisted one — so message-scoped server actions (e.g. fork)
|
||||
* must not be offered until the transcript refetch swaps in the persisted
|
||||
* message id.
|
||||
*/
|
||||
export function isLiveAssistantMessageId(messageId: string): boolean {
|
||||
return messageId.startsWith('live-assistant:')
|
||||
}
|
||||
|
||||
/**
|
||||
* True for a persisted assistant turn. Its turn has ended, so a tool row it left
|
||||
* pending, executing or awaiting approval did not finish and never runs again.
|
||||
*/
|
||||
export function isStoredAssistantMessage(message: { role: string; id: string }): boolean {
|
||||
return message.role === 'assistant' && !isLiveAssistantMessageId(message.id)
|
||||
}
|
||||
|
||||
@@ -2,6 +2,7 @@ import { describe, expect, it } from 'vitest'
|
||||
import { copilotChatStopBodySchema } from '@/lib/api/contracts/copilot'
|
||||
import type { PersistedToolState } from '@/lib/api/contracts/copilot-messages'
|
||||
import { toDisplayMessage } from '@/lib/mothership/chat/display-message'
|
||||
import { getLiveAssistantMessageId } from '@/lib/mothership/chat/live-message-id'
|
||||
import type { OrchestratorResult } from '@/lib/mothership/request/types'
|
||||
import { getToolDisplayTitle } from '@/lib/mothership/tools/tool-display'
|
||||
import { resolveMessageCitations } from '@/app/workspace/[workspaceId]/home/components/message-content/resolve-citations'
|
||||
@@ -635,22 +636,25 @@ describe('compactToolCallParams', () => {
|
||||
})
|
||||
|
||||
it.each(['pending', 'executing', 'awaiting_approval'] as const)(
|
||||
'keeps a %s call whole: it can still run or await approval from these params',
|
||||
'keeps a live %s call whole: it can still run or await approval from these params',
|
||||
(state) => {
|
||||
const message = sandboxCall(state)
|
||||
const message = { ...sandboxCall(state), id: getLiveAssistantMessageId('stream-1') }
|
||||
expect(compactToolCallParams(message)).toBe(message)
|
||||
}
|
||||
)
|
||||
|
||||
it('bounds an interrupted call, which no reload re-runs or asks to approve', () => {
|
||||
const compacted = compactToolCallParams(sandboxCall('interrupted'))
|
||||
const stdout = (
|
||||
compacted.contentBlocks?.[0].toolCall?.params?.request as {
|
||||
invocation: { stdout: string }
|
||||
}
|
||||
).invocation.stdout
|
||||
expect(stdout.length).toBeLessThanOrEqual(1024)
|
||||
})
|
||||
it.each(['pending', 'executing', 'awaiting_approval', 'interrupted'] as const)(
|
||||
'bounds a stored %s call, which renders as interrupted and never re-runs or asks to approve',
|
||||
(state) => {
|
||||
const compacted = compactToolCallParams(sandboxCall(state))
|
||||
const stdout = (
|
||||
compacted.contentBlocks?.[0].toolCall?.params?.request as {
|
||||
invocation: { stdout: string }
|
||||
}
|
||||
).invocation.stdout
|
||||
expect(stdout.length).toBeLessThanOrEqual(1024)
|
||||
}
|
||||
)
|
||||
|
||||
it('is idempotent and keeps the reference once compacted', () => {
|
||||
const compacted = compactToolCallParams(sandboxCall('error'))
|
||||
|
||||
@@ -7,6 +7,7 @@ import type {
|
||||
PersistedToolState,
|
||||
} from '@/lib/api/contracts/copilot-messages'
|
||||
import { buildMothershipErrorTag } from '@/lib/mothership/chat/error-tag'
|
||||
import { isStoredAssistantMessage } from '@/lib/mothership/chat/live-message-id'
|
||||
import { compactRetrievalCitations } from '@/lib/mothership/chat/retrieval-citations'
|
||||
import {
|
||||
mergeAndRedactPersistedBlocks,
|
||||
@@ -228,10 +229,12 @@ function compactParamRecord(record: Record<string, unknown>): Record<string, unk
|
||||
}
|
||||
|
||||
/**
|
||||
* Truncate long strings inside settled tool calls' params, keeping every key,
|
||||
* array entry and short scalar. A pending, executing or awaiting-approval call
|
||||
* keeps its params whole: a reloaded executing workflow tool re-runs from them
|
||||
* and the permission card previews them.
|
||||
* Truncate long strings inside tool calls' params, keeping every key, array
|
||||
* entry and short scalar. On the live assistant message a pending, executing or
|
||||
* awaiting-approval call keeps its params whole: a reloaded executing workflow
|
||||
* tool re-runs from them and the permission card previews them. A stored turn
|
||||
* has ended, so its unfinished calls render as interrupted and are compacted
|
||||
* like settled ones.
|
||||
*
|
||||
* A projection for the transcript served to the chat UI only. Stored rows keep
|
||||
* full params: data drains export them verbatim, and fork and import copy
|
||||
@@ -240,10 +243,11 @@ function compactParamRecord(record: Record<string, unknown>): Record<string, unk
|
||||
*/
|
||||
export function compactToolCallParams(message: PersistedMessage): PersistedMessage {
|
||||
if (!message.contentBlocks?.length) return message
|
||||
const stored = isStoredAssistantMessage(message)
|
||||
let changed = false
|
||||
const contentBlocks = message.contentBlocks.map((block) => {
|
||||
const toolCall = block.toolCall
|
||||
if (!toolCall?.params || isUnsettledToolState(toolCall.state)) return block
|
||||
if (!toolCall?.params || (!stored && isUnsettledToolState(toolCall.state))) return block
|
||||
const params = compactParamRecord(toolCall.params)
|
||||
if (params === toolCall.params) return block
|
||||
changed = true
|
||||
|
||||
Reference in New Issue
Block a user