Skip to content

Commit 03377cd

Browse files
Sg312Sg312
committed
fix(chat): keep tool failures in expanded history
1 parent 97cfdb2 commit 03377cd

5 files changed

Lines changed: 152 additions & 47 deletions

File tree

‎apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/activity-stream.test.tsx‎

Lines changed: 30 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -121,26 +121,43 @@ describe.each(['mothership', 'workflow', 'browser', 'deploy'])('%s activity', (a
121121
expect(header()?.textContent).toBe('Reading third')
122122
})
123123

124-
it.each(['error', 'cancelled', 'interrupted', 'rejected', 'skipped'] as const)(
124+
it.each(['cancelled', 'interrupted', 'skipped'] as const)(
125125
'shows %s immediately and cancels a pending cosmetic update',
126126
(status) => {
127127
render([tool('first')])
128128
advance(100)
129129
render([tool('first', 'success'), tool('second')])
130130
render([tool('first', 'success'), tool('second', status)])
131-
const prefix =
132-
status === 'error' || status === 'rejected'
133-
? 'Failed'
134-
: status === 'skipped'
135-
? 'Skipped'
136-
: 'Stopped'
131+
const prefix = status === 'skipped' ? 'Skipped' : 'Stopped'
137132
expect(header()?.textContent).toBe(`${prefix} reading second`)
138133
expect(container.querySelector('[class*="shimmer"]')).toBeNull()
139134
advance(1500)
140135
expect(header()?.textContent).toBe(`${prefix} reading second`)
141136
}
142137
)
143138

139+
it.each(['error', 'rejected'] as const)(
140+
'keeps a %s attempt in history without promoting it into the summary',
141+
(status) => {
142+
render([tool('first')])
143+
advance(100)
144+
render([tool('first', 'success'), tool('second')])
145+
render([tool('first', 'success'), tool('second', status)])
146+
const label = agentName === 'mothership' ? 'Read first' : 'Reading first'
147+
expect(header()?.textContent).toBe(label)
148+
advance(1500)
149+
expect(header()?.textContent).toBe(label)
150+
render([tool('first', 'success'), tool('second', status)], false)
151+
expect(header()?.textContent).toBe(
152+
agentName === 'mothership' ? 'Read first + 1' : 'Read files'
153+
)
154+
act(() => container.querySelector<HTMLElement>('[role="button"]')?.click())
155+
expect(container.querySelector('[data-state="open"]')?.textContent).toContain(
156+
'Failed reading second'
157+
)
158+
}
159+
)
160+
144161
it('shows final completion immediately and never replays the held action', () => {
145162
render([tool('first')])
146163
advance(100)
@@ -161,22 +178,23 @@ describe.each(['mothership', 'workflow', 'browser', 'deploy'])('%s activity', (a
161178
render([tool('first'), tool('second', status)])
162179
const outcome =
163180
status === 'error' || status === 'rejected'
164-
? 'failed'
181+
? ''
165182
: status === 'skipped'
166183
? 'skipped'
167184
: 'stopped'
168-
expect(header()?.textContent).toBe(`Reading first · 1 ${outcome}`)
185+
const label = `Reading first${outcome ? ` · 1 ${outcome}` : ''}`
186+
expect(header()?.textContent).toBe(label)
169187
expect(container.querySelector('[class*="shimmer"]')).not.toBeNull()
170188
advance(1000)
171-
expect(header()?.textContent).toBe(`Reading first · 1 ${outcome}`)
189+
expect(header()?.textContent).toBe(label)
172190
}
173191
)
174192

175-
it('surfaces an earlier parallel failure while the latest call keeps working', () => {
193+
it('shows the latest parallel call without an earlier failure suffix', () => {
176194
render([tool('first'), tool('second')])
177195
advance(100)
178196
render([tool('first', 'error'), tool('second')])
179-
expect(header()?.textContent).toBe('Reading second · 1 failed')
197+
expect(header()?.textContent).toBe('Reading second')
180198
})
181199

182200
it('keeps narration from prematurely completing an open lane', () => {

‎apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/agent-group-view.tsx‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
'use client'
22

33
import { type ComponentType, type ReactNode, useState } from 'react'
4-
import type { ToolActivity } from '@/lib/mothership/generated/protocol'
54
import { ThinkingLoader } from '@/components/ui/thinking-loader'
65
import { isBrowserAgentAvailable } from '@/lib/browser-agent/transport'
6+
import type { ToolActivity } from '@/lib/mothership/generated/protocol'
77
import { RETIRED_BROWSER_REQUEST_TAKEOVER_ID } from '@/lib/mothership/tools/retired-tools'
88
import { getToolDisplayTitle, getToolStatusDisplayTitle } from '@/lib/mothership/tools/tool-display'
99
import { ActivityStream } from '@/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/activity-stream'
@@ -275,6 +275,8 @@ export function AgentGroupView({
275275
meaningfulItems.some(
276276
(item) =>
277277
item.type !== 'tool' ||
278+
item.data.status === ToolCallStatus.error ||
279+
item.data.status === ToolCallStatus.rejected ||
278280
needsToolInput(item.data) ||
279281
item.data.toolName === RETIRED_BROWSER_REQUEST_TAKEOVER_ID
280282
)

‎apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/agent-group.test.ts‎

Lines changed: 85 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -162,10 +162,8 @@ describe('AgentGroup inline main activity', () => {
162162
it.each([
163163
['executing', 'Reading notes'],
164164
['success', 'Read notes'],
165-
['error', 'Failed reading notes'],
166165
['cancelled', 'Stopped reading notes'],
167166
['skipped', 'Skipped reading notes'],
168-
['rejected', 'Failed reading notes'],
169167
['interrupted', 'Stopped reading notes'],
170168
] as const)('renders a single %s tool once without a disclosure', (status, expected) => {
171169
act(() =>
@@ -223,11 +221,11 @@ describe('AgentGroup inline main activity', () => {
223221

224222
it.each([
225223
['success', 'Checked search requirements'],
226-
['error', 'Failed searching'],
224+
['error', '1 tool call'],
227225
['cancelled', 'Stopped searching'],
228226
['skipped', 'Skipped searching'],
229227
['interrupted', 'Stopped searching'],
230-
['rejected', 'Failed searching'],
228+
['rejected', '1 tool call'],
231229
] as const)('uses an honest grouped activity label after %s', (status, expected) => {
232230
const item = tool(status)
233231
act(() =>
@@ -387,7 +385,7 @@ describe('AgentGroup inline main activity', () => {
387385
expect(container.textContent).not.toContain('Built API')
388386
})
389387

390-
it('retains earlier failed outcomes when completed activities collapse into one summary', () => {
388+
it('keeps earlier failures in expanded history when completed activities collapse', () => {
391389
act(() =>
392390
root.render(
393391
createElement(AgentGroup, {
@@ -420,11 +418,90 @@ describe('AgentGroup inline main activity', () => {
420418
})
421419
)
422420
)
423-
expect(container.querySelector('[role="status"]')?.textContent).toBe(
424-
'Checked inputs + 1 · 1 failed'
425-
)
421+
expect(container.querySelector('[role="status"]')?.textContent).toBe('Checked inputs + 1')
422+
act(() => container.querySelector<HTMLElement>('[role="button"]')?.click())
423+
expect(container.textContent).toContain('Failed reading first document')
426424
})
427425

426+
it.each(['mothership', 'workflow'])(
427+
'keeps a lone failed %s call accessible under a neutral disclosure',
428+
(agentName) => {
429+
act(() =>
430+
root.render(
431+
createElement(AgentGroup, {
432+
agentName,
433+
agentLabel: agentName,
434+
items: [
435+
{
436+
type: 'tool',
437+
data: {
438+
id: 'failed-read',
439+
toolName: 'read',
440+
displayTitle: 'Reading notes',
441+
status: 'error',
442+
},
443+
},
444+
],
445+
})
446+
)
447+
)
448+
expect(container.textContent).not.toContain('Failed')
449+
expect(container.textContent).toContain('1 tool call')
450+
const disclosure = container.querySelector<HTMLElement>('[role="button"]')
451+
expect(disclosure?.getAttribute('aria-expanded')).toBe('false')
452+
act(() => disclosure?.click())
453+
expect(container.textContent).toContain('Failed reading notes')
454+
}
455+
)
456+
457+
it.each(['executing', 'success'] as const)(
458+
'keeps failures out of a %s summary while retaining them in expanded history',
459+
(status) => {
460+
const items: AgentGroupItem[] = [
461+
{
462+
type: 'tool',
463+
data: {
464+
id: 'read-failed',
465+
toolName: 'read',
466+
displayTitle: 'Reading image.png',
467+
status: 'error',
468+
},
469+
},
470+
{
471+
type: 'tool',
472+
data: { id: 'view', toolName: 'read', displayTitle: 'Viewing image.png', status },
473+
},
474+
{
475+
type: 'tool',
476+
data: {
477+
id: 'later-failed',
478+
toolName: 'read',
479+
displayTitle: 'Reading notes',
480+
status: 'error',
481+
},
482+
},
483+
]
484+
act(() =>
485+
root.render(
486+
createElement(AgentGroup, {
487+
agentName: 'mothership',
488+
agentLabel: 'Sim',
489+
items,
490+
activity: { id: 'inspect', completedTitle: 'Inspected images' },
491+
isStreaming: status === 'executing',
492+
isLaneOpen: status === 'executing',
493+
})
494+
)
495+
)
496+
expect(container.textContent).toBe(
497+
status === 'executing' ? 'Viewing image.png' : 'Viewed image.png + 2'
498+
)
499+
act(() => container.querySelector<HTMLElement>('[role="button"]')?.click())
500+
expect(container.textContent).toContain('Failed reading image.png')
501+
expect(container.textContent).toContain('Failed reading notes')
502+
}
503+
)
504+
428505
it('paces the active status in place and expands the full completed history', () => {
429506
vi.useFakeTimers()
430507
const first: AgentGroupItem = {

‎apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/tool-activity-group.test.ts‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ describe('getToolActivitySummary', () => {
4646
tool('terminal_run', 'cancelled'),
4747
tool('browser_type', 'rejected'),
4848
])
49-
).toBe('Read files · 2 failed · 1 stopped')
49+
).toBe('Read files · 1 stopped')
5050
})
5151

5252
it('does not invent actions when all calls failed or were stopped', () => {
@@ -55,7 +55,7 @@ describe('getToolActivitySummary', () => {
5555
tool('apply_file_edit', 'error'),
5656
tool('terminal_run', 'interrupted'),
5757
])
58-
).toBe('Tool activity · 1 failed · 1 stopped')
58+
).toBe('2 tool calls · 1 stopped')
5959
})
6060

6161
it('keeps an individual tool’s descriptive title', () => {
@@ -65,7 +65,7 @@ describe('getToolActivitySummary', () => {
6565
})
6666

6767
it.each([
68-
['rejected', 'Failed running checks'],
68+
['rejected', '1 tool call'],
6969
['skipped', 'Skipped running checks'],
7070
['interrupted', 'Stopped running checks'],
7171
] as const)('labels a single %s tool as finished', (status, expected) => {
@@ -94,7 +94,7 @@ describe('getToolActivitySummary', () => {
9494
).toBe('Navigated pages, filled forms +5 more')
9595
})
9696

97-
it('keeps failure and interruption counts visible when action categories are capped', () => {
97+
it('omits failure counts while retaining interruption counts when action categories are capped', () => {
9898
expect(
9999
getToolActivitySummary([
100100
tool('read'),
@@ -105,14 +105,14 @@ describe('getToolActivitySummary', () => {
105105
tool('wait', 'interrupted'),
106106
tool('browser_type', 'skipped'),
107107
])
108-
).toBe('Read files, searched files +2 more · 1 failed · 1 stopped · 1 skipped')
108+
).toBe('Read files, searched files +2 more · 1 stopped · 1 skipped')
109109
})
110110

111-
it('uses the same outcome wording for rejected individual and grouped calls', () => {
111+
it('keeps rejected individual and grouped calls neutral in summaries', () => {
112112
const rejected = { ...tool('terminal', 'rejected'), displayTitle: 'Running checks' }
113-
expect(getToolActivitySummary([rejected])).toBe('Failed running checks')
113+
expect(getToolActivitySummary([rejected])).toBe('1 tool call')
114114
expect(getToolActivitySummary([rejected, tool('read', 'skipped')])).toBe(
115-
'Tool activity · 1 failed · 1 skipped'
115+
'2 tool calls · 1 skipped'
116116
)
117117
})
118118

‎apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/tool-activity-group.tsx‎

Lines changed: 26 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,19 @@ import { type ToolCallData, ToolCallStatus } from '@/app/workspace/[workspaceId]
1616

1717
const MAX_SUMMARY_ACTIONS = 2
1818

19+
function isFailedTool(tool: ToolCallData): boolean {
20+
return tool.status === ToolCallStatus.error || tool.status === ToolCallStatus.rejected
21+
}
22+
23+
function toolCountLabel(tools: ToolCallData[]): string {
24+
return `${tools.length} tool ${tools.length === 1 ? 'call' : 'calls'}`
25+
}
26+
1927
/** Summarize completed actions without describing failed or skipped work as successful. */
2028
export function getToolActivitySummary(tools: ToolCallData[]): string {
2129
if (tools.length === 1) {
2230
const tool = tools[0]
31+
if (isFailedTool(tool)) return toolCountLabel(tools)
2332
return getToolStatusDisplayTitle(
2433
tool.displayTitle,
2534
tool.status,
@@ -32,36 +41,31 @@ export function getToolActivitySummary(tools: ToolCallData[]): string {
3241
MAX_SUMMARY_ACTIONS
3342
)
3443
const summary = labels.join(', ')
35-
const summaryLabel = summary ? summary[0].toUpperCase() + summary.slice(1) : 'Tool activity'
44+
const summaryLabel = summary ? summary[0].toUpperCase() + summary.slice(1) : toolCountLabel(tools)
3645
return [
3746
additionalActions > 0 ? `${summaryLabel} +${additionalActions} more` : summaryLabel,
3847
...getToolActivityOutcomes(tools),
3948
].join(' · ')
4049
}
4150

4251
function getToolActivityOutcomes(tools: ToolCallData[]): string[] {
43-
let failed = 0
4452
let stopped = 0
4553
let skipped = 0
4654
for (const tool of tools) {
47-
if (tool.status === ToolCallStatus.error || tool.status === ToolCallStatus.rejected) failed++
48-
else if (tool.status === ToolCallStatus.cancelled || tool.status === ToolCallStatus.interrupted)
55+
if (tool.status === ToolCallStatus.cancelled || tool.status === ToolCallStatus.interrupted)
4956
stopped++
5057
else if (tool.status === ToolCallStatus.skipped) skipped++
5158
}
52-
return [
53-
...(failed ? [`${failed} failed`] : []),
54-
...(stopped ? [`${stopped} stopped`] : []),
55-
...(skipped ? [`${skipped} skipped`] : []),
56-
]
59+
return [...(stopped ? [`${stopped} stopped`] : []), ...(skipped ? [`${skipped} skipped`] : [])]
5760
}
5861

59-
/** Keep earlier parallel failures visible while the latest action continues. */
62+
/** Failed attempts belong in the expanded history, not the activity summary. */
6063
export function getActiveToolActivityTitle(
6164
label: string,
6265
tool: ToolCallData,
6366
tools: ToolCallData[]
6467
): string {
68+
if (isFailedTool(tool)) return toolCountLabel(tools)
6569
return tool.status === ToolCallStatus.executing || tool.status === ToolCallStatus.success
6670
? [label, ...getToolActivityOutcomes(tools)].join(' · ')
6771
: label
@@ -76,7 +80,9 @@ export function getActivityStatusTool(tools: ToolCallData[]): ToolCallData | und
7680
? tool
7781
: newest,
7882
undefined
79-
) ?? tools.at(-1)
83+
) ??
84+
tools.filter((tool) => !isFailedTool(tool)).at(-1) ??
85+
tools.at(-1)
8086
)
8187
}
8288

@@ -122,12 +128,14 @@ export function ToolActivityGroup({
122128
)
123129
const completedActivityLabel = groupedActivity?.completedTitle
124130
? failedActivityTool
125-
? getToolStatusDisplayTitle(
126-
failedActivityTool.displayTitle,
127-
failedActivityTool.status,
128-
failedActivityTool.toolName,
129-
failedActivityTool.activityDescription
130-
)
131+
? isFailedTool(failedActivityTool)
132+
? undefined
133+
: getToolStatusDisplayTitle(
134+
failedActivityTool.displayTitle,
135+
failedActivityTool.status,
136+
failedActivityTool.toolName,
137+
failedActivityTool.activityDescription
138+
)
131139
: groupedActivity.completedTitle
132140
: undefined
133141
const completedLabel = completedActivityLabel
@@ -166,7 +174,7 @@ export function ToolActivityGroup({
166174
}}
167175
activityKey={statusTool.id}
168176
attentionKey={attentionKey}
169-
collapsible={tools.length > 1}
177+
collapsible={tools.length > 1 || isFailedTool(statusTool)}
170178
expanded={expanded}
171179
onToggle={() => setExpanded(!expanded)}
172180
isStreaming={working && autoScrollActivity}

0 commit comments

Comments
 (0)