Skip to content

Commit ff859ac

Browse files
committed
fix(activity): keep unsuccessful tool labels neutral
1 parent bfb7ff3 commit ff859ac

7 files changed

Lines changed: 103 additions & 54 deletions

File tree

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

Lines changed: 43 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -127,16 +127,14 @@ describe.each(['mothership', 'workflow', 'browser', 'deploy'])('%s activity', (a
127127
advance(100)
128128
render([tool('first', 'success'), tool('second')])
129129
render([tool('first', 'success'), tool('second', status)])
130-
const prefix =
130+
const label =
131131
status === 'error' || status === 'rejected'
132-
? 'Failed'
133-
: status === 'skipped'
134-
? 'Skipped'
135-
: 'Stopped'
136-
expect(header()?.textContent).toBe(`${prefix} reading second`)
132+
? 'Reading second'
133+
: `${status === 'skipped' ? 'Skipped' : 'Stopped'} reading second`
134+
expect(header()?.textContent).toBe(label)
137135
expect(container.querySelector('[class*="shimmer"]')).toBeNull()
138136
advance(1500)
139-
expect(header()?.textContent).toBe(`${prefix} reading second`)
137+
expect(header()?.textContent).toBe(label)
140138
}
141139
)
142140

@@ -176,15 +174,51 @@ describe.each(['mothership', 'workflow', 'browser', 'deploy'])('%s activity', (a
176174
const trigger = container.querySelector<HTMLElement>('[role="button"]')!
177175
act(() => trigger.click())
178176
expect(container.querySelector('[data-state="open"]')?.textContent).toBe(
179-
'Failed reading firstReading second'
177+
'Reading firstReading second'
180178
)
181179
render([tool('first', 'error'), tool('second', 'success')], false)
182180
expect(header()?.textContent).toBe('Read files')
183181
expect(container.querySelector('[data-state="open"]')?.textContent).toBe(
184-
'Failed reading firstRead second'
182+
'Reading firstRead second'
185183
)
186184
})
187185

186+
it.each(['error', 'rejected'] as const)(
187+
'keeps a %s model description neutral in the header and expanded history',
188+
(status) => {
189+
render(
190+
[
191+
{
192+
...tool('first', status),
193+
displayTitle: 'Failed reading reference material',
194+
activityDescription: 'Locating reference material',
195+
},
196+
],
197+
false
198+
)
199+
expect(header()?.textContent).toBe('Locating reference material')
200+
expect(container.querySelector('[class*="shimmer"]')).toBeNull()
201+
render(
202+
[
203+
{
204+
...tool('first', status),
205+
displayTitle: 'Failed reading reference material',
206+
activityDescription: 'Failed: Locating reference material',
207+
},
208+
tool('second', 'success'),
209+
],
210+
false
211+
)
212+
const trigger = container.querySelector<HTMLElement>('[role="button"]')!
213+
act(() => trigger.click())
214+
expect(container.querySelector('[data-state="open"]')?.textContent).toBe(
215+
'Locating reference materialRead second'
216+
)
217+
expect(container.textContent).not.toContain('Failed')
218+
expect(container.querySelector('[class*="shimmer"]')).toBeNull()
219+
}
220+
)
221+
188222
it('shows three distinct actions and keeps the complete history available', () => {
189223
render(
190224
[

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -162,10 +162,10 @@ describe('AgentGroup inline main activity', () => {
162162
it.each([
163163
['executing', 'Reading notes'],
164164
['success', 'Read notes'],
165-
['error', 'Failed reading notes'],
165+
['error', 'Reading notes'],
166166
['cancelled', 'Stopped reading notes'],
167167
['skipped', 'Skipped reading notes'],
168-
['rejected', 'Failed reading notes'],
168+
['rejected', 'Reading notes'],
169169
['interrupted', 'Stopped reading notes'],
170170
] as const)('renders a single %s tool once without a disclosure', (status, expected) => {
171171
act(() =>

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ describe('getToolActivitySummary', () => {
8989
})
9090

9191
it.each([
92-
['rejected', 'Failed running checks'],
92+
['rejected', 'Running checks'],
9393
['skipped', 'Skipped running checks'],
9494
['interrupted', 'Stopped running checks'],
9595
] as const)('labels a single %s tool as finished', (status, expected) => {
@@ -132,9 +132,9 @@ describe('getToolActivitySummary', () => {
132132
).toBe('Read files, searched files, used the terminal +1 more · 1 stopped · 1 skipped')
133133
})
134134

135-
it('keeps individual failures explicit without adding aggregate failure badges', () => {
135+
it('keeps individual unsuccessful actions neutral without aggregate failure badges', () => {
136136
const rejected = { ...tool('terminal', 'rejected'), displayTitle: 'Running checks' }
137-
expect(getToolActivitySummary([rejected])).toBe('Failed running checks')
137+
expect(getToolActivitySummary([rejected])).toBe('Running checks')
138138
expect(getToolActivitySummary([rejected, tool('read', 'skipped')])).toBe(
139139
'Tool activity · 1 skipped'
140140
)

apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/tool-call-item.test.tsx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,9 +53,9 @@ describe('ToolCallItem', () => {
5353
it.each([
5454
['executing', 'Checking the invoice totals'],
5555
['success', 'Checked the invoice totals'],
56-
['error', 'Failed checking the invoice totals'],
56+
['error', 'Checking the invoice totals'],
5757
['cancelled', 'Stopped checking the invoice totals'],
58-
['rejected', 'Failed checking the invoice totals'],
58+
['rejected', 'Checking the invoice totals'],
5959
['skipped', 'Skipped checking the invoice totals'],
6060
] as const)(
6161
'projects %s from the actual tool status onto the model description',
@@ -130,7 +130,7 @@ describe('ToolCallItem', () => {
130130
/>
131131
)
132132

133-
expect(markup).toContain('Failed checking invoices')
133+
expect(markup).toContain('Checking invoices')
134134
expect(markup).not.toContain('Stopped checking invoices')
135135
})
136136

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -676,9 +676,9 @@ describe('completed tool titles', () => {
676676
expect(failures).toEqual([])
677677
})
678678

679-
it('keeps present tense while executing; failed rows say so', () => {
679+
it('keeps the action description for executing and unsuccessful rows', () => {
680680
expect(firstToolTitle([queryLogsCall('executing')])).toBe('Querying logs')
681-
expect(firstToolTitle([queryLogsCall('error')])).toBe('Failed querying logs')
681+
expect(firstToolTitle([queryLogsCall('error')])).toBe('Querying logs')
682682
})
683683
})
684684

apps/sim/lib/copilot/tools/tool-display.test.ts

Lines changed: 30 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -200,26 +200,19 @@ describe('getToolCompletedTitle', () => {
200200
expect(getToolCompletedTitle('Custom title from the model')).toBeUndefined()
201201
})
202202

203-
it('projects a terminal tense for every settled row, present tense only while running', () => {
203+
it('keeps unsuccessful actions neutral without rewriting them as completed', () => {
204204
expect(getToolStatusDisplayTitle('Comparing workflows', 'success')).toBe('Compared workflows')
205205
expect(getToolStatusDisplayTitle('Comparing workflows', 'executing')).toBe(
206206
'Comparing workflows'
207207
)
208-
// An errored row must not read as still running — the frozen present-tense
209-
// title ("Searching for X" forever) was reported as a stuck tool call.
210-
expect(getToolStatusDisplayTitle('Comparing workflows', 'error')).toBe(
211-
'Failed comparing workflows'
212-
)
208+
expect(getToolStatusDisplayTitle('Comparing workflows', 'error')).toBe('Comparing workflows')
213209
expect(getToolStatusDisplayTitle('Searching for admin mentions', 'error')).toBe(
214-
'Failed searching for admin mentions'
210+
'Searching for admin mentions'
215211
)
216212
expect(getToolStatusDisplayTitle('Comparing workflows', 'cancelled')).toBe(
217213
'Stopped comparing workflows'
218214
)
219-
// Non-gerund titles get a prefix rather than a bad rewrite.
220-
expect(getToolStatusDisplayTitle('Read recent emails', 'error')).toBe(
221-
'Failed: Read recent emails'
222-
)
215+
expect(getToolStatusDisplayTitle('Read recent emails', 'error')).toBe('Read recent emails')
223216
})
224217
})
225218

@@ -688,11 +681,28 @@ describe('terminal-title projection is idempotent', () => {
688681
expect(getToolStatusDisplayTitle(storeErrorLabel, 'rejected')).toBe(storeErrorLabel)
689682
})
690683

691-
it('never stacks a second Failed prefix', () => {
684+
it('removes historical failure prefixes idempotently', () => {
692685
const once = getToolStatusDisplayTitle('Reading table', 'error')
693-
expect(once).toBe('Failed reading table')
686+
expect(once).toBe('Reading table')
694687
expect(getToolStatusDisplayTitle(once, 'error')).toBe(once)
695-
expect(getToolStatusDisplayTitle('Failed: Something', 'error')).toBe('Failed: Something')
688+
expect(getToolStatusDisplayTitle('Failed: Something', 'error')).toBe('Something')
689+
})
690+
691+
it.each([
692+
['Failed: Failed reading notes', 'Reading notes'],
693+
['Failed: Locating reference material', 'Locating reference material'],
694+
['Failed', 'Tool activity'],
695+
['Reading failed runs', 'Reading failed runs'],
696+
['FailedJobs report', 'FailedJobs report'],
697+
['iPhone metadata', 'iPhone metadata'],
698+
['Failed: eBay metadata', 'eBay metadata'],
699+
['failed reading notes', 'Reading notes'],
700+
['Failed failed reading notes', 'Reading notes'],
701+
])('normalizes only leading outcome wording: %s', (title, expected) => {
702+
expect(getToolStatusDisplayTitle(title, 'error')).toBe(expected)
703+
expect(getToolStatusDisplayTitle(title, 'rejected')).toBe(expected)
704+
expect(getToolStatusDisplayTitle(expected, 'error')).toBe(expected)
705+
expect(getToolStatusDisplayTitle(expected, 'rejected')).toBe(expected)
696706
})
697707

698708
it('leaves a store-phrased skip label alone when cancelled', () => {
@@ -702,10 +712,8 @@ describe('terminal-title projection is idempotent', () => {
702712
expect(getToolStatusDisplayTitle(stopped, 'cancelled')).toBe(stopped)
703713
})
704714

705-
it('still projects an ordinary present-tense title', () => {
706-
expect(getToolStatusDisplayTitle('Searching Sim docs', 'error')).toBe(
707-
'Failed searching Sim docs'
708-
)
715+
it('leaves unsuccessful action wording intact and labels cancellation', () => {
716+
expect(getToolStatusDisplayTitle('Searching Sim docs', 'error')).toBe('Searching Sim docs')
709717
expect(getToolStatusDisplayTitle('Running workflow', 'cancelled')).toBe(
710718
'Stopped running workflow'
711719
)
@@ -854,10 +862,10 @@ describe('model-authored activity outcomes', () => {
854862
['success', 'Revisando facturas', 'Revisando facturas'],
855863
['success', 'Stopped checking invoices', 'Stopped checking invoices'],
856864
['success', 'Completed: Check invoices', 'Completed: Check invoices'],
857-
['error', 'Failed: Fetching invoices', 'Failed: Fetching invoices'],
858-
['error', 'Stopped checking invoices', 'Failed checking invoices'],
859-
['error', 'Completed checking invoices', 'Failed checking invoices'],
860-
['rejected', 'Failed checking invoices', 'Failed checking invoices'],
865+
['error', 'Failed: Fetching invoices', 'Fetching invoices'],
866+
['error', 'Stopped checking invoices', 'Checking invoices'],
867+
['error', 'Completed checking invoices', 'Checking invoices'],
868+
['rejected', 'Failed checking invoices', 'Checking invoices'],
861869
['cancelled', 'Stopped reading notes', 'Stopped reading notes'],
862870
['interrupted', 'Completed: Check invoices', 'Stopped: Check invoices'],
863871
['skipped', 'Failed: Checking invoices', 'Skipped: Checking invoices'],

apps/sim/lib/copilot/tools/tool-display.ts

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1419,16 +1419,7 @@ export function getToolCompletedTitle(title: string): string | undefined {
14191419
return past + title.slice(firstWord.length)
14201420
}
14211421

1422-
/**
1423-
* Titles that already say the work is over.
1424-
*
1425-
* Two layers project a terminal tense: the client tool store phrases its own
1426-
* error and skip labels ("Attempted to read X", "Skipped reading X"), and this
1427-
* module projects again at the render boundary. Re-projecting an
1428-
* already-projected title stacked prefixes — "Failed: Failed: Attempted to read
1429-
* metadata for thread_tracking" — and even a single pass over a store label
1430-
* reads as doubly hedged. Whichever layer spoke first wins.
1431-
*/
1422+
/** Recognize terminal wording already supplied by the tool store or persisted history. */
14321423
const TERMINAL_TITLE_PREFIXES = new Set(['Failed', 'Attempted', 'Skipped', 'Stopped'])
14331424

14341425
function firstWordOf(title: string): string {
@@ -1444,7 +1435,7 @@ function statesTerminalOutcome(title: string): boolean {
14441435
/** Apply one terminal outcome prefix while preserving already-resolved titles. */
14451436
function getToolOutcomeTitle(
14461437
title: string,
1447-
outcome: 'Failed' | 'Stopped' | 'Skipped',
1438+
outcome: 'Stopped' | 'Skipped',
14481439
preserveExistingOutcome: boolean
14491440
): string {
14501441
if (preserveExistingOutcome && statesTerminalOutcome(title)) return title
@@ -1462,10 +1453,26 @@ function getToolOutcomeTitle(
14621453
return `${outcome}: ${title}`
14631454
}
14641455

1456+
/** Error rows describe the action without failure badges or claims of completion. */
1457+
function getNeutralToolActionTitle(title: string): string {
1458+
let action = title
1459+
while (action) {
1460+
const firstWord = firstWordOf(action)
1461+
const prefix = firstWord.replace(/:$/, '').toLowerCase()
1462+
if (!['failed', 'stopped', 'skipped', 'completed'].includes(prefix)) break
1463+
action = action.slice(firstWord.length).trimStart()
1464+
}
1465+
if (!action) return 'Tool activity'
1466+
if (action === title) return title
1467+
const firstWord = firstWordOf(action)
1468+
const gerund = firstWord.charAt(0).toUpperCase() + firstWord.slice(1)
1469+
return COMPLETED_VERB_REWRITES[gerund] ? gerund + action.slice(firstWord.length) : action
1470+
}
1471+
14651472
/**
14661473
* Resolve a tool title at the rendering boundary. Successful calls use a known
14671474
* past-tense rewrite when available and otherwise preserve the wording.
1468-
* Failed, stopped, and skipped calls retain explicit outcome labels.
1475+
* Unsuccessful calls keep a neutral action; stopped and skipped calls retain their labels.
14691476
*/
14701477
export function getToolStatusDisplayTitle(
14711478
title: string,
@@ -1482,7 +1489,7 @@ export function getToolStatusDisplayTitle(
14821489
return getToolCompletedTitle(title) ?? title
14831490
}
14841491
if (status === 'error' || status === 'rejected') {
1485-
return getToolOutcomeTitle(title, 'Failed', !description)
1492+
return getNeutralToolActionTitle(title)
14861493
}
14871494
if (status === 'cancelled' || status === 'aborted' || status === 'interrupted') {
14881495
return getToolOutcomeTitle(title, 'Stopped', !description)

0 commit comments

Comments
 (0)