Skip to content

Commit 5613e74

Browse files
authored
fix(chat): share thinking and preserve natural activity labels (#7810)
* fix(chat): use shared thinking and natural completion labels * fix(chat): share thinking across pending agent lanes * fix(chat): retain quiet gaps during streamed narration * docs(chat): explain pending activity ownership
1 parent 17e983e commit 5613e74

8 files changed

Lines changed: 339 additions & 62 deletions

File tree

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

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,12 @@ function items(tools: ToolCallData[]): AgentGroupItem[] {
1616
return tools.map((data) => ({ type: 'tool', data }))
1717
}
1818

19-
describe.each(['mothership', 'workflow', 'browser'])('%s activity cadence', (agentName) => {
19+
describe.each(['mothership', 'workflow', 'browser', 'deploy'])('%s activity', (agentName) => {
2020
let root: Root
2121
let container: HTMLDivElement
2222
beforeEach(() => {
2323
vi.useFakeTimers()
24+
vi.stubGlobal('matchMedia', vi.fn().mockReturnValue({ matches: false }))
2425
;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true
2526
container = document.createElement('div')
2627
document.body.appendChild(container)
@@ -30,6 +31,7 @@ describe.each(['mothership', 'workflow', 'browser'])('%s activity cadence', (age
3031
act(() => root.unmount())
3132
container.remove()
3233
vi.useRealTimers()
34+
vi.unstubAllGlobals()
3335
})
3436
const render = (tools: ToolCallData[], active = true) =>
3537
act(() =>
@@ -46,6 +48,34 @@ describe.each(['mothership', 'workflow', 'browser'])('%s activity cadence', (age
4648
const header = () => container.querySelector('[role="status"]')
4749
const advance = (ms: number) => act(() => vi.advanceTimersByTime(ms))
4850

51+
it('leaves empty lanes to the turn indicator and shows the first action immediately', () => {
52+
render([])
53+
expect(container.childElementCount).toBe(0)
54+
advance(100)
55+
render([tool('first')])
56+
expect(header()?.textContent).toBe('Reading first')
57+
const row = header()
58+
render([tool('first', 'success')], false)
59+
expect(header()?.textContent).toBe('Read first')
60+
expect(header()).toBe(row)
61+
})
62+
63+
it('preserves a successful model description in the header and expanded history', () => {
64+
const completed = {
65+
...tool('first', 'success'),
66+
activityDescription: 'Read the latest inbox emails',
67+
}
68+
render([completed], false)
69+
expect(header()?.textContent).toBe(completed.activityDescription)
70+
render([completed, tool('second', 'success')], false)
71+
const trigger = container.querySelector<HTMLElement>('[role="button"]')!
72+
act(() => trigger.click())
73+
expect(container.querySelector('[data-state="open"]')?.textContent).toContain(
74+
completed.activityDescription
75+
)
76+
expect(container.textContent).not.toContain('Completed:')
77+
})
78+
4979
it('shows the first action immediately and coalesces bursts without replaying a backlog', () => {
5080
render([])
5181
advance(100)
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
import type {
2+
AgentGroupItem,
3+
NestedAgentGroup,
4+
} from '@/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/agent-group-view'
5+
import type { ToolCallData } from '@/app/workspace/[workspaceId]/home/types'
6+
7+
/** Empty agent lanes share the turn's thinking indicator until they have output. */
8+
export function hasAgentGroupItemContent(item: AgentGroupItem): boolean {
9+
switch (item.type) {
10+
case 'tool':
11+
return true
12+
case 'text':
13+
return item.content.trim().length > 0
14+
case 'agent_group':
15+
return item.group.items.some(hasAgentGroupItemContent)
16+
}
17+
}
18+
19+
/** Finds empty live lanes at any depth whose wait belongs to the turn indicator. */
20+
export function hasPendingAgentGroup(
21+
group: Pick<NestedAgentGroup, 'items' | 'isOpen' | 'isDelegating'>
22+
): boolean {
23+
return (
24+
((group.isOpen || group.isDelegating) && !group.items.some(hasAgentGroupItemContent)) ||
25+
group.items.some((item) => item.type === 'agent_group' && hasPendingAgentGroup(item.group))
26+
)
27+
}
28+
29+
/**
30+
* Every tool in a group, in stream order, including those run by nested
31+
* agents. A parent's status line speaks for the whole subtree it delegated,
32+
* so a grandchild's work is what surfaces while the parent itself waits.
33+
*/
34+
export function collectGroupTools(items: AgentGroupItem[]): ToolCallData[] {
35+
const tools: ToolCallData[] = []
36+
const walk = (list: AgentGroupItem[]) => {
37+
for (const item of list) {
38+
if (item.type === 'tool') tools.push(item.data)
39+
else if (item.type === 'agent_group') walk(item.group.items)
40+
}
41+
}
42+
walk(items)
43+
return tools
44+
}

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

Lines changed: 19 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,15 @@
11
'use client'
22

33
import { type ComponentType, type ReactNode, useState } from 'react'
4+
import { ThinkingLoader } from '@/components/ui/thinking-loader'
45
import { isBrowserAgentAvailable } from '@/lib/browser-agent/transport'
56
import { RETIRED_BROWSER_REQUEST_TAKEOVER_ID } from '@/lib/copilot/tools/retired-tools'
67
import { getToolStatusDisplayTitle } from '@/lib/copilot/tools/tool-display'
78
import { ActivityStream } from '@/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/activity-stream'
9+
import {
10+
collectGroupTools,
11+
hasAgentGroupItemContent,
12+
} from '@/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/agent-group-content'
813
import { BrowserAgentIcon } from '@/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/browser-agent-icon'
914
import { renderInlineMarkdown } from '@/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/inline-markdown'
1015
import { MainAgentActivity } from '@/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/main-agent-activity'
@@ -67,23 +72,6 @@ function activeToolTitle(tool: ToolCallData): string {
6772
)
6873
}
6974

70-
/**
71-
* Every tool in a group, in stream order, including those run by nested
72-
* agents. A parent's status line speaks for the whole subtree it delegated,
73-
* so a grandchild's work is what surfaces while the parent itself waits.
74-
*/
75-
function collectGroupTools(items: AgentGroupItem[]): ToolCallData[] {
76-
const tools: ToolCallData[] = []
77-
const walk = (list: AgentGroupItem[]) => {
78-
for (const item of list) {
79-
if (item.type === 'tool') tools.push(item.data)
80-
else if (item.type === 'agent_group') walk(item.group.items)
81-
}
82-
}
83-
walk(items)
84-
return tools
85-
}
86-
8775
/** Reveal blocking interactions even when a parent group was manually collapsed. */
8876
function hasPendingInteraction(items: AgentGroupItem[]): boolean {
8977
return items.some((item) => {
@@ -162,12 +150,6 @@ export function AgentGroupView({
162150
renderBrowserTakeover,
163151
}: AgentGroupViewProps) {
164152
const AgentIcon = getAgentIcon(agentName)
165-
const agentIcon =
166-
agentName === 'browser' ? (
167-
<BrowserAgentIcon items={items} />
168-
) : (
169-
<AgentIcon className='size-full' />
170-
)
171153
const isMainAgent = agentName === 'mothership'
172154
const tools = isMainAgent ? [] : collectGroupTools(items)
173155
const statusTool = getActivityStatusTool(tools)
@@ -178,6 +160,14 @@ export function AgentGroupView({
178160
const nestedBrowserTakeover = browserAgentAvailable && hasNestedBrowserTakeover(items)
179161
const isWorking =
180162
!activeBrowserTakeover && ((isDelegating && !resolved) || (isStreaming && isLaneOpen))
163+
const agentIcon =
164+
isWorking && !statusTool ? (
165+
<ThinkingLoader size={14} startVariant='corners' />
166+
) : agentName === 'browser' ? (
167+
<BrowserAgentIcon items={items} />
168+
) : (
169+
<AgentIcon className='size-full' />
170+
)
181171

182172
const [manualExpanded, setManualExpanded] = useState(defaultExpanded)
183173
const [expandedTakeoverId, setExpandedTakeoverId] = useState<string | null>(null)
@@ -188,6 +178,9 @@ export function AgentGroupView({
188178
nestedBrowserTakeover ||
189179
(activeBrowserTakeover ? expandedTakeoverId === activeBrowserTakeover.id : manualExpanded)
190180

181+
const meaningfulItems = items.filter(hasAgentGroupItemContent)
182+
if (meaningfulItems.length === 0) return null
183+
191184
const toggleExpanded = () => {
192185
if (activeBrowserTakeover) {
193186
setExpandedTakeoverId(expanded ? null : activeBrowserTakeover.id)
@@ -229,6 +222,7 @@ export function AgentGroupView({
229222
/>
230223
)
231224
}
225+
if (!item.content.trim()) return null
232226
return (
233227
<NarrationText
234228
key={`text-${idx}`}
@@ -262,8 +256,8 @@ export function AgentGroupView({
262256
statusTool.status === ToolCallStatus.executing ||
263257
statusTool.status === ToolCallStatus.success)
264258
const collapsible =
265-
items.length > 1 ||
266-
items.some(
259+
meaningfulItems.length > 1 ||
260+
meaningfulItems.some(
267261
(item) =>
268262
item.type !== 'tool' ||
269263
needsToolInput(item.data) ||
Lines changed: 200 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,200 @@
1+
/**
2+
* @vitest-environment jsdom
3+
*/
4+
import { act } from 'react'
5+
import { QueryClient, QueryClientProvider } from '@tanstack/react-query'
6+
import { createRoot, type Root } from 'react-dom/client'
7+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
8+
import { MessageContent } from '@/app/workspace/[workspaceId]/home/components/message-content/message-content'
9+
import type { ContentBlock, ToolCallStatus } from '@/app/workspace/[workspaceId]/home/types'
10+
11+
vi.mock('@/lib/auth/auth-client', () => ({
12+
useSession: vi.fn(() => ({ data: null, isPending: false })),
13+
}))
14+
15+
function start(name: string, parentSpanId = 'main'): ContentBlock {
16+
return { type: 'subagent', content: name, spanId: name, parentSpanId, timestamp: 1 }
17+
}
18+
19+
function tool(spanId: string, status: ToolCallStatus = 'executing'): ContentBlock {
20+
return {
21+
type: 'tool_call',
22+
spanId,
23+
toolCall: {
24+
id: `${spanId}-read`,
25+
name: 'read',
26+
calledBy: spanId,
27+
status,
28+
activityDescription: `Reading ${spanId} notes`,
29+
},
30+
timestamp: 2,
31+
}
32+
}
33+
34+
describe('MessageContent shared thinking indicator', () => {
35+
let queryClient: QueryClient
36+
let root: Root
37+
let container: HTMLDivElement
38+
39+
beforeEach(() => {
40+
queryClient = new QueryClient()
41+
vi.useFakeTimers()
42+
vi.stubGlobal('matchMedia', vi.fn().mockReturnValue({ matches: false }))
43+
;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true
44+
container = document.createElement('div')
45+
document.body.appendChild(container)
46+
root = createRoot(container)
47+
})
48+
49+
afterEach(() => {
50+
act(() => root.unmount())
51+
container.remove()
52+
queryClient.clear()
53+
vi.useRealTimers()
54+
vi.unstubAllGlobals()
55+
})
56+
57+
const render = (blocks: ContentBlock[], isStreaming = true) =>
58+
act(() => {
59+
root.render(
60+
<QueryClientProvider client={queryClient}>
61+
<MessageContent blocks={blocks} fallbackContent='' isStreaming={isStreaming} isLast />
62+
</QueryClientProvider>
63+
)
64+
})
65+
const thinking = () => container.querySelectorAll('[aria-hidden="false"] svg')
66+
const groups = () => container.querySelectorAll('[data-agent-group]')
67+
68+
it('shares one indicator across parallel and nested empty agents', () => {
69+
render([start('workflow'), start('browser'), start('deploy', 'workflow')])
70+
expect(thinking()).toHaveLength(1)
71+
expect(groups()).toHaveLength(0)
72+
expect(container.querySelector('[role="status"]')).toBeNull()
73+
})
74+
75+
it('hands off immediately to meaningful activity without remounting existing rows', () => {
76+
const blocks = [start('workflow'), start('browser')]
77+
render(blocks)
78+
expect(thinking()).toHaveLength(1)
79+
render([...blocks, tool('workflow')])
80+
expect(thinking()).toHaveLength(0)
81+
expect(groups()).toHaveLength(1)
82+
const firstRow = container.querySelector('[role="status"]')
83+
expect(firstRow?.textContent).toBe('Reading workflow notes')
84+
render([...blocks, tool('workflow'), tool('browser')])
85+
expect(groups()).toHaveLength(2)
86+
expect(container.querySelector('[role="status"]')).toBe(firstRow)
87+
expect(thinking()).toHaveLength(0)
88+
})
89+
90+
it('shows the pending indicator after the only visible agent finishes', () => {
91+
const blocks = [start('workflow'), start('browser'), tool('workflow', 'success')]
92+
render(blocks)
93+
expect(thinking()).toHaveLength(0)
94+
render([...blocks, { type: 'subagent_end', spanId: 'workflow', timestamp: 3 }])
95+
expect(thinking()).toHaveLength(1)
96+
expect(groups()).toHaveLength(1)
97+
})
98+
99+
it('keeps a singleton action flat when its nested agent has no output', () => {
100+
render([start('workflow'), tool('workflow'), start('deploy', 'workflow')])
101+
expect(groups()).toHaveLength(1)
102+
expect(container.querySelectorAll('[role="status"]')).toHaveLength(1)
103+
expect(container.querySelector('[role="button"]')).toBeNull()
104+
expect(thinking()).toHaveLength(0)
105+
})
106+
107+
it('retains nested activity while another child is empty', () => {
108+
render([
109+
start('workflow'),
110+
start('deploy', 'workflow'),
111+
start('browser', 'workflow'),
112+
tool('deploy'),
113+
])
114+
const trigger = container.querySelector<HTMLElement>('[role="button"]')!
115+
act(() => trigger.click())
116+
expect(container.querySelector('[data-state="open"]')?.textContent).toContain(
117+
'Reading deploy notes'
118+
)
119+
expect(container.querySelectorAll('[role="status"]')).toHaveLength(2)
120+
expect(thinking()).toHaveLength(0)
121+
})
122+
123+
it('preserves narration and skips whitespace-only output', () => {
124+
const blocks: ContentBlock[] = [
125+
start('workflow'),
126+
{ type: 'subagent_text', spanId: 'workflow', content: ' ', timestamp: 2 },
127+
]
128+
render(blocks)
129+
expect(groups()).toHaveLength(0)
130+
expect(thinking()).toHaveLength(1)
131+
render(
132+
[
133+
...blocks,
134+
{ type: 'subagent_text', spanId: 'workflow', content: 'Checking the setup.', timestamp: 3 },
135+
],
136+
false
137+
)
138+
expect(groups()).toHaveLength(1)
139+
act(() => container.querySelector<HTMLElement>('[role="button"]')!.click())
140+
expect(container.querySelector('[data-state="open"]')?.textContent).toContain(
141+
'Checking the setup.'
142+
)
143+
})
144+
145+
it.each(['awaiting_approval', 'error', 'cancelled', 'rejected'] as const)(
146+
'keeps %s tool rows visible while another agent is pending',
147+
(status) => {
148+
render([start('workflow'), start('browser'), tool('workflow', status)])
149+
expect(groups()).toHaveLength(1)
150+
expect(container.querySelector('[role="status"]')?.textContent).toContain('workflow notes')
151+
expect(thinking()).toHaveLength(1)
152+
}
153+
)
154+
155+
it('keeps thinking hidden while prose streams and finishes revealing', () => {
156+
const blocks: ContentBlock[] = [
157+
start('browser'),
158+
{
159+
type: 'text',
160+
content: 'Here is the result of reviewing the project and checking its configuration.',
161+
timestamp: 3,
162+
},
163+
]
164+
render([start('browser'), { type: 'text', content: 'Here is', timestamp: 3 }])
165+
render(blocks)
166+
act(() => vi.advanceTimersByTime(100))
167+
expect(thinking()).toHaveLength(0)
168+
render(blocks, false)
169+
expect(thinking()).toHaveLength(0)
170+
})
171+
172+
it('waits for the normal quiet period between prose chunks while an agent is pending', () => {
173+
const prose = 'Here is the result of reviewing the project and checking its configuration.'
174+
const blocks: ContentBlock[] = [
175+
start('browser'),
176+
{ type: 'text', content: prose, timestamp: 3 },
177+
]
178+
render(blocks)
179+
expect(thinking()).toHaveLength(0)
180+
act(() => vi.advanceTimersByTime(1_499))
181+
expect(thinking()).toHaveLength(0)
182+
act(() => vi.advanceTimersByTime(1))
183+
expect(thinking()).toHaveLength(1)
184+
render([...blocks, { type: 'text', content: ' The configuration is valid.', timestamp: 4 }])
185+
expect(thinking()).toHaveLength(0)
186+
})
187+
188+
it('removes thinking when a turn stops or finishes without agent output', () => {
189+
const blocks = [start('workflow'), start('browser')]
190+
render(blocks)
191+
expect(thinking()).toHaveLength(1)
192+
render([...blocks, { type: 'stopped', timestamp: 3 }], false)
193+
expect(thinking()).toHaveLength(0)
194+
expect(groups()).toHaveLength(0)
195+
expect(container.textContent).toContain('Stopped')
196+
render(blocks, false)
197+
expect(thinking()).toHaveLength(0)
198+
expect(groups()).toHaveLength(0)
199+
})
200+
})

0 commit comments

Comments
 (0)