From ff859ac8ddfb2bc1834e3d4de5c21f0abf04912f Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Thu, 17 Sep 2026 16:33:02 -0700 Subject: [PATCH] fix(activity): keep unsuccessful tool labels neutral --- .../agent-group/activity-stream.test.tsx | 52 +++++++++++++++---- .../agent-group/agent-group.test.ts | 4 +- .../agent-group/tool-activity-group.test.ts | 6 +-- .../agent-group/tool-call-item.test.tsx | 6 +-- .../message-content/message-content.test.ts | 4 +- .../lib/copilot/tools/tool-display.test.ts | 52 +++++++++++-------- apps/sim/lib/copilot/tools/tool-display.ts | 33 +++++++----- 7 files changed, 103 insertions(+), 54 deletions(-) diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/activity-stream.test.tsx b/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/activity-stream.test.tsx index dedae46db01..351a5a46b72 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/activity-stream.test.tsx +++ b/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/activity-stream.test.tsx @@ -127,16 +127,14 @@ describe.each(['mothership', 'workflow', 'browser', 'deploy'])('%s activity', (a advance(100) render([tool('first', 'success'), tool('second')]) render([tool('first', 'success'), tool('second', status)]) - const prefix = + const label = status === 'error' || status === 'rejected' - ? 'Failed' - : status === 'skipped' - ? 'Skipped' - : 'Stopped' - expect(header()?.textContent).toBe(`${prefix} reading second`) + ? 'Reading second' + : `${status === 'skipped' ? 'Skipped' : 'Stopped'} reading second` + expect(header()?.textContent).toBe(label) expect(container.querySelector('[class*="shimmer"]')).toBeNull() advance(1500) - expect(header()?.textContent).toBe(`${prefix} reading second`) + expect(header()?.textContent).toBe(label) } ) @@ -176,15 +174,51 @@ describe.each(['mothership', 'workflow', 'browser', 'deploy'])('%s activity', (a const trigger = container.querySelector('[role="button"]')! act(() => trigger.click()) expect(container.querySelector('[data-state="open"]')?.textContent).toBe( - 'Failed reading firstReading second' + 'Reading firstReading second' ) render([tool('first', 'error'), tool('second', 'success')], false) expect(header()?.textContent).toBe('Read files') expect(container.querySelector('[data-state="open"]')?.textContent).toBe( - 'Failed reading firstRead second' + 'Reading firstRead second' ) }) + it.each(['error', 'rejected'] as const)( + 'keeps a %s model description neutral in the header and expanded history', + (status) => { + render( + [ + { + ...tool('first', status), + displayTitle: 'Failed reading reference material', + activityDescription: 'Locating reference material', + }, + ], + false + ) + expect(header()?.textContent).toBe('Locating reference material') + expect(container.querySelector('[class*="shimmer"]')).toBeNull() + render( + [ + { + ...tool('first', status), + displayTitle: 'Failed reading reference material', + activityDescription: 'Failed: Locating reference material', + }, + tool('second', 'success'), + ], + false + ) + const trigger = container.querySelector('[role="button"]')! + act(() => trigger.click()) + expect(container.querySelector('[data-state="open"]')?.textContent).toBe( + 'Locating reference materialRead second' + ) + expect(container.textContent).not.toContain('Failed') + expect(container.querySelector('[class*="shimmer"]')).toBeNull() + } + ) + it('shows three distinct actions and keeps the complete history available', () => { render( [ diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/agent-group.test.ts b/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/agent-group.test.ts index 1b1075a60ce..39fb385c1e1 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/agent-group.test.ts +++ b/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/agent-group.test.ts @@ -162,10 +162,10 @@ describe('AgentGroup inline main activity', () => { it.each([ ['executing', 'Reading notes'], ['success', 'Read notes'], - ['error', 'Failed reading notes'], + ['error', 'Reading notes'], ['cancelled', 'Stopped reading notes'], ['skipped', 'Skipped reading notes'], - ['rejected', 'Failed reading notes'], + ['rejected', 'Reading notes'], ['interrupted', 'Stopped reading notes'], ] as const)('renders a single %s tool once without a disclosure', (status, expected) => { act(() => diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/tool-activity-group.test.ts b/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/tool-activity-group.test.ts index 53f44524306..f14ae20a0e2 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/tool-activity-group.test.ts +++ b/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/tool-activity-group.test.ts @@ -89,7 +89,7 @@ describe('getToolActivitySummary', () => { }) it.each([ - ['rejected', 'Failed running checks'], + ['rejected', 'Running checks'], ['skipped', 'Skipped running checks'], ['interrupted', 'Stopped running checks'], ] as const)('labels a single %s tool as finished', (status, expected) => { @@ -132,9 +132,9 @@ describe('getToolActivitySummary', () => { ).toBe('Read files, searched files, used the terminal +1 more · 1 stopped · 1 skipped') }) - it('keeps individual failures explicit without adding aggregate failure badges', () => { + it('keeps individual unsuccessful actions neutral without aggregate failure badges', () => { const rejected = { ...tool('terminal', 'rejected'), displayTitle: 'Running checks' } - expect(getToolActivitySummary([rejected])).toBe('Failed running checks') + expect(getToolActivitySummary([rejected])).toBe('Running checks') expect(getToolActivitySummary([rejected, tool('read', 'skipped')])).toBe( 'Tool activity · 1 skipped' ) diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/tool-call-item.test.tsx b/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/tool-call-item.test.tsx index 1692702faec..7f750517922 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/tool-call-item.test.tsx +++ b/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/tool-call-item.test.tsx @@ -53,9 +53,9 @@ describe('ToolCallItem', () => { it.each([ ['executing', 'Checking the invoice totals'], ['success', 'Checked the invoice totals'], - ['error', 'Failed checking the invoice totals'], + ['error', 'Checking the invoice totals'], ['cancelled', 'Stopped checking the invoice totals'], - ['rejected', 'Failed checking the invoice totals'], + ['rejected', 'Checking the invoice totals'], ['skipped', 'Skipped checking the invoice totals'], ] as const)( 'projects %s from the actual tool status onto the model description', @@ -130,7 +130,7 @@ describe('ToolCallItem', () => { /> ) - expect(markup).toContain('Failed checking invoices') + expect(markup).toContain('Checking invoices') expect(markup).not.toContain('Stopped checking invoices') }) diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/message-content/message-content.test.ts b/apps/sim/app/workspace/[workspaceId]/home/components/message-content/message-content.test.ts index 19543772721..a32fd27dae7 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/components/message-content/message-content.test.ts +++ b/apps/sim/app/workspace/[workspaceId]/home/components/message-content/message-content.test.ts @@ -676,9 +676,9 @@ describe('completed tool titles', () => { expect(failures).toEqual([]) }) - it('keeps present tense while executing; failed rows say so', () => { + it('keeps the action description for executing and unsuccessful rows', () => { expect(firstToolTitle([queryLogsCall('executing')])).toBe('Querying logs') - expect(firstToolTitle([queryLogsCall('error')])).toBe('Failed querying logs') + expect(firstToolTitle([queryLogsCall('error')])).toBe('Querying logs') }) }) diff --git a/apps/sim/lib/copilot/tools/tool-display.test.ts b/apps/sim/lib/copilot/tools/tool-display.test.ts index 39739a99d3e..bede6ec4b2e 100644 --- a/apps/sim/lib/copilot/tools/tool-display.test.ts +++ b/apps/sim/lib/copilot/tools/tool-display.test.ts @@ -200,26 +200,19 @@ describe('getToolCompletedTitle', () => { expect(getToolCompletedTitle('Custom title from the model')).toBeUndefined() }) - it('projects a terminal tense for every settled row, present tense only while running', () => { + it('keeps unsuccessful actions neutral without rewriting them as completed', () => { expect(getToolStatusDisplayTitle('Comparing workflows', 'success')).toBe('Compared workflows') expect(getToolStatusDisplayTitle('Comparing workflows', 'executing')).toBe( 'Comparing workflows' ) - // An errored row must not read as still running — the frozen present-tense - // title ("Searching for X" forever) was reported as a stuck tool call. - expect(getToolStatusDisplayTitle('Comparing workflows', 'error')).toBe( - 'Failed comparing workflows' - ) + expect(getToolStatusDisplayTitle('Comparing workflows', 'error')).toBe('Comparing workflows') expect(getToolStatusDisplayTitle('Searching for admin mentions', 'error')).toBe( - 'Failed searching for admin mentions' + 'Searching for admin mentions' ) expect(getToolStatusDisplayTitle('Comparing workflows', 'cancelled')).toBe( 'Stopped comparing workflows' ) - // Non-gerund titles get a prefix rather than a bad rewrite. - expect(getToolStatusDisplayTitle('Read recent emails', 'error')).toBe( - 'Failed: Read recent emails' - ) + expect(getToolStatusDisplayTitle('Read recent emails', 'error')).toBe('Read recent emails') }) }) @@ -688,11 +681,28 @@ describe('terminal-title projection is idempotent', () => { expect(getToolStatusDisplayTitle(storeErrorLabel, 'rejected')).toBe(storeErrorLabel) }) - it('never stacks a second Failed prefix', () => { + it('removes historical failure prefixes idempotently', () => { const once = getToolStatusDisplayTitle('Reading table', 'error') - expect(once).toBe('Failed reading table') + expect(once).toBe('Reading table') expect(getToolStatusDisplayTitle(once, 'error')).toBe(once) - expect(getToolStatusDisplayTitle('Failed: Something', 'error')).toBe('Failed: Something') + expect(getToolStatusDisplayTitle('Failed: Something', 'error')).toBe('Something') + }) + + it.each([ + ['Failed: Failed reading notes', 'Reading notes'], + ['Failed: Locating reference material', 'Locating reference material'], + ['Failed', 'Tool activity'], + ['Reading failed runs', 'Reading failed runs'], + ['FailedJobs report', 'FailedJobs report'], + ['iPhone metadata', 'iPhone metadata'], + ['Failed: eBay metadata', 'eBay metadata'], + ['failed reading notes', 'Reading notes'], + ['Failed failed reading notes', 'Reading notes'], + ])('normalizes only leading outcome wording: %s', (title, expected) => { + expect(getToolStatusDisplayTitle(title, 'error')).toBe(expected) + expect(getToolStatusDisplayTitle(title, 'rejected')).toBe(expected) + expect(getToolStatusDisplayTitle(expected, 'error')).toBe(expected) + expect(getToolStatusDisplayTitle(expected, 'rejected')).toBe(expected) }) it('leaves a store-phrased skip label alone when cancelled', () => { @@ -702,10 +712,8 @@ describe('terminal-title projection is idempotent', () => { expect(getToolStatusDisplayTitle(stopped, 'cancelled')).toBe(stopped) }) - it('still projects an ordinary present-tense title', () => { - expect(getToolStatusDisplayTitle('Searching Sim docs', 'error')).toBe( - 'Failed searching Sim docs' - ) + it('leaves unsuccessful action wording intact and labels cancellation', () => { + expect(getToolStatusDisplayTitle('Searching Sim docs', 'error')).toBe('Searching Sim docs') expect(getToolStatusDisplayTitle('Running workflow', 'cancelled')).toBe( 'Stopped running workflow' ) @@ -854,10 +862,10 @@ describe('model-authored activity outcomes', () => { ['success', 'Revisando facturas', 'Revisando facturas'], ['success', 'Stopped checking invoices', 'Stopped checking invoices'], ['success', 'Completed: Check invoices', 'Completed: Check invoices'], - ['error', 'Failed: Fetching invoices', 'Failed: Fetching invoices'], - ['error', 'Stopped checking invoices', 'Failed checking invoices'], - ['error', 'Completed checking invoices', 'Failed checking invoices'], - ['rejected', 'Failed checking invoices', 'Failed checking invoices'], + ['error', 'Failed: Fetching invoices', 'Fetching invoices'], + ['error', 'Stopped checking invoices', 'Checking invoices'], + ['error', 'Completed checking invoices', 'Checking invoices'], + ['rejected', 'Failed checking invoices', 'Checking invoices'], ['cancelled', 'Stopped reading notes', 'Stopped reading notes'], ['interrupted', 'Completed: Check invoices', 'Stopped: Check invoices'], ['skipped', 'Failed: Checking invoices', 'Skipped: Checking invoices'], diff --git a/apps/sim/lib/copilot/tools/tool-display.ts b/apps/sim/lib/copilot/tools/tool-display.ts index 6e3dbd905d1..8101ab909e2 100644 --- a/apps/sim/lib/copilot/tools/tool-display.ts +++ b/apps/sim/lib/copilot/tools/tool-display.ts @@ -1419,16 +1419,7 @@ export function getToolCompletedTitle(title: string): string | undefined { return past + title.slice(firstWord.length) } -/** - * Titles that already say the work is over. - * - * Two layers project a terminal tense: the client tool store phrases its own - * error and skip labels ("Attempted to read X", "Skipped reading X"), and this - * module projects again at the render boundary. Re-projecting an - * already-projected title stacked prefixes — "Failed: Failed: Attempted to read - * metadata for thread_tracking" — and even a single pass over a store label - * reads as doubly hedged. Whichever layer spoke first wins. - */ +/** Recognize terminal wording already supplied by the tool store or persisted history. */ const TERMINAL_TITLE_PREFIXES = new Set(['Failed', 'Attempted', 'Skipped', 'Stopped']) function firstWordOf(title: string): string { @@ -1444,7 +1435,7 @@ function statesTerminalOutcome(title: string): boolean { /** Apply one terminal outcome prefix while preserving already-resolved titles. */ function getToolOutcomeTitle( title: string, - outcome: 'Failed' | 'Stopped' | 'Skipped', + outcome: 'Stopped' | 'Skipped', preserveExistingOutcome: boolean ): string { if (preserveExistingOutcome && statesTerminalOutcome(title)) return title @@ -1462,10 +1453,26 @@ function getToolOutcomeTitle( return `${outcome}: ${title}` } +/** Error rows describe the action without failure badges or claims of completion. */ +function getNeutralToolActionTitle(title: string): string { + let action = title + while (action) { + const firstWord = firstWordOf(action) + const prefix = firstWord.replace(/:$/, '').toLowerCase() + if (!['failed', 'stopped', 'skipped', 'completed'].includes(prefix)) break + action = action.slice(firstWord.length).trimStart() + } + if (!action) return 'Tool activity' + if (action === title) return title + const firstWord = firstWordOf(action) + const gerund = firstWord.charAt(0).toUpperCase() + firstWord.slice(1) + return COMPLETED_VERB_REWRITES[gerund] ? gerund + action.slice(firstWord.length) : action +} + /** * Resolve a tool title at the rendering boundary. Successful calls use a known * past-tense rewrite when available and otherwise preserve the wording. - * Failed, stopped, and skipped calls retain explicit outcome labels. + * Unsuccessful calls keep a neutral action; stopped and skipped calls retain their labels. */ export function getToolStatusDisplayTitle( title: string, @@ -1482,7 +1489,7 @@ export function getToolStatusDisplayTitle( return getToolCompletedTitle(title) ?? title } if (status === 'error' || status === 'rejected') { - return getToolOutcomeTitle(title, 'Failed', !description) + return getNeutralToolActionTitle(title) } if (status === 'cancelled' || status === 'aborted' || status === 'interrupted') { return getToolOutcomeTitle(title, 'Stopped', !description)