From 8624ad15e0fde989e506806b8985fb7001e3ce01 Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Mon, 31 Aug 2026 18:47:37 +0300 Subject: [PATCH 01/23] fix(chat): preserve and render complete agent turn traces Co-authored-by: Medulla --- .../components/ChatToolParts.test.tsx | 44 ++++++ .../components/ChatToolParts.tsx | 145 +++++++++++++++++- .../components/SubagentActivityBlock.tsx | 44 +++++- .../__tests__/ToolTimelineBlock.test.tsx | 32 +++- app/src/providers/ChatRuntimeProvider.tsx | 8 + .../__tests__/assistantUiMessages.test.ts | 48 +++++- app/src/providers/assistantUiMessages.ts | 72 +++++++-- .../providers/useOpenHumanExternalStore.ts | 13 +- .../chatRuntimeSlice.derived.thunk.test.ts | 13 +- app/src/store/chatRuntimeSlice.ts | 99 +++++++----- .../specs/chat-harness-subagent.spec.ts | 121 ++++++++++++--- .../specs/chat-tool-call-flow.spec.ts | 16 ++ .../mock-api/routes/__tests__/llm.test.mjs | 47 ++++++ scripts/mock-api/routes/llm.mjs | 14 +- .../agent/harness/session/transcript.rs | 78 ++++++++-- .../agent/harness/session/transcript_tests.rs | 15 ++ .../threads/transcript_view/cache.rs | 10 +- .../threads/transcript_view/project.rs | 111 ++++++++++---- vendor/tinyagents | 2 +- 19 files changed, 783 insertions(+), 149 deletions(-) diff --git a/app/src/features/conversations/components/ChatToolParts.test.tsx b/app/src/features/conversations/components/ChatToolParts.test.tsx index c2c131b78d..63162031e8 100644 --- a/app/src/features/conversations/components/ChatToolParts.test.tsx +++ b/app/src/features/conversations/components/ChatToolParts.test.tsx @@ -1,4 +1,5 @@ import { render, screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; import { describe, expect, it } from 'vitest'; import type { SubagentActivity } from '../../../store/chatRuntimeSlice'; @@ -43,4 +44,47 @@ describe('ChatToolParts', () => { expect(screen.getByText('live delegation')).toBeVisible(); }); + + it('renders ordinary tools with rich input and output on the assistant-ui surface', async () => { + render( + {}} + resume={() => {}} + respondToApproval={() => {}} + /> + ); + + expect(screen.getByTestId('assistant-ui-tool-call')).toHaveTextContent('Searched the web'); + await userEvent.click(screen.getByRole('button', { name: /Searched the web/ })); + expect(screen.getByText(/Lean open conjectures/)).toBeInTheDocument(); + expect(screen.getByText('Found 12 candidate problems')).toBeInTheDocument(); + }); + + it('unwraps a single content field instead of showing a redundant title', async () => { + render( + {}} + resume={() => {}} + respondToApproval={() => {}} + /> + ); + + await userEvent.click(screen.getByRole('button', { name: /Fetched from the web/ })); + expect(screen.getByRole('strong')).toHaveTextContent('Example Domain'); + expect(screen.queryByText('Content', { exact: true })).not.toBeInTheDocument(); + }); }); diff --git a/app/src/features/conversations/components/ChatToolParts.tsx b/app/src/features/conversations/components/ChatToolParts.tsx index 69c06ec5fa..0f0097bf35 100644 --- a/app/src/features/conversations/components/ChatToolParts.tsx +++ b/app/src/features/conversations/components/ChatToolParts.tsx @@ -1,10 +1,9 @@ import type { ToolCallMessagePartComponent } from '@assistant-ui/react'; -import { CheckIcon, ChevronDownIcon, Loader2Icon, WorkflowIcon } from 'lucide-react'; +import { CheckIcon, ChevronDownIcon, Loader2Icon, WrenchIcon, WorkflowIcon } from 'lucide-react'; import type { FC, PropsWithChildren } from 'react'; import { cn } from '../../../components/assistant-ui/lib/utils'; import type { ThreadGroupPart } from '../../../components/assistant-ui/thread'; -import { ToolFallback } from '../../../components/assistant-ui/tool-fallback'; import { ToolGroupContent, ToolGroupRoot, @@ -16,6 +15,8 @@ import { CollapsibleTrigger, } from '../../../components/assistant-ui/ui/collapsible'; import type { SubagentActivity } from '../../../store/chatRuntimeSlice'; +import { formatToolName } from '../../../utils/toolTimelineFormatting'; +import { BubbleMarkdown } from './AgentMessageBubble'; import { SubagentActivityBlock } from './SubagentActivityBlock'; function asSubagentActivity(value: unknown): SubagentActivity | undefined { @@ -57,6 +58,7 @@ export const SubagentCall: ToolCallMessagePartComponent = ({ args, result }) => return ( ); }; -/** Route delegations to the rich renderer and ordinary tools to assistant-ui. */ +function friendlyLabel(key: string): string { + return key + .replace(/([a-z0-9])([A-Z])/g, '$1 $2') + .replace(/[_-]+/g, ' ') + .replace(/^./, char => char.toUpperCase()); +} + +function toolDisplayName(toolName: string, running: boolean): string { + if (toolName === 'web_fetch') return running ? 'Fetching from the web' : 'Fetched from the web'; + if (toolName === 'web_search_tool' || toolName === 'web_search') { + return running ? 'Searching the web' : 'Searched the web'; + } + return formatToolName(toolName); +} + +function parsedValue(value: unknown): unknown { + if (typeof value !== 'string') return value; + const trimmed = value.trim(); + if (!(trimmed.startsWith('{') || trimmed.startsWith('['))) return value; + try { + return JSON.parse(trimmed); + } catch { + return value; + } +} + +function hasDisplayValue(value: unknown): boolean { + if (value === undefined || value === null || value === '') return false; + if (Array.isArray(value)) return value.length > 0; + if (typeof value === 'object') return Object.keys(value as object).length > 0; + return true; +} + +function ToolDataView({ value }: { value: unknown }) { + const parsed = parsedValue(value); + if (Array.isArray(parsed)) { + return ( +
    + {parsed.map((item, index) => ( +
  • + +
    • + ))} +
+ ); + } + if (parsed && typeof parsed === 'object') { + const entries = Object.entries(parsed); + // Tool wrappers frequently add bookkeeping beside the actual payload + // (`tool_call_id`, success, timing). When a semantic output field exists, + // show that value directly and hide the wrapper entirely. + for (const key of ['content', 'output', 'result', 'message']) { + const semantic = entries.find(([candidate]) => candidate === key)?.[1]; + if (hasDisplayValue(semantic)) return ; + } + return ( +
+ {entries.map(([key, item]) => ( +
+
{friendlyLabel(key)}
+
+ +
+
+ ))} +
+ ); + } + if (typeof parsed === 'boolean') return {parsed ? 'Yes' : 'No'}; + if (typeof parsed === 'string') return ; + return {String(parsed ?? '')}; +} + +/** Rich assistant-ui-native renderer for an ordinary OpenHuman tool call. */ +export const OpenHumanToolCall: ToolCallMessagePartComponent = ({ + toolName, + args, + argsText, + result, +}) => { + const running = result === undefined; + const input = hasDisplayValue(args) ? args : parsedValue(argsText ?? ''); + const output = parsedValue(result); + return ( + + + + + {toolDisplayName(toolName, running)} + + {running ? ( + + + running + + ) : ( + + done + + )} + + + + {hasDisplayValue(input) ? ( +
+

Input

+
+ +
+
+ ) : null} + {hasDisplayValue(output) ? ( +
+

Output

+
+ +
+
+ ) : null} + + + ); +}; + +/** Route every call through an assistant-ui-native rich renderer. */ export const ChatToolFallback: ToolCallMessagePartComponent = props => - props.toolName === 'task' ? : ; + props.toolName === 'task' ? : ; -/** Keep a tool group open while any contained call is still running. */ +/** Keep the assistant-ui tool cards visible; each card owns its detail collapse. */ export const ChatToolGroup: FC> = ({ group, children, }) => { const running = group.status.type === 'running'; return ( - + {children} diff --git a/app/src/features/conversations/components/SubagentActivityBlock.tsx b/app/src/features/conversations/components/SubagentActivityBlock.tsx index a907f19351..d3b4d429d7 100644 --- a/app/src/features/conversations/components/SubagentActivityBlock.tsx +++ b/app/src/features/conversations/components/SubagentActivityBlock.tsx @@ -37,6 +37,38 @@ function toolCallTone(status: ToolTimelineEntryStatus): string { return 'text-coral-700 dark:text-coral-300'; } +function subagentToolLabel(name: string, status: ToolTimelineEntryStatus): string { + const running = status === 'running'; + if (name === 'web_fetch') return running ? 'Fetching from the web' : 'Fetched from the web'; + if (name === 'web_search_tool' || name === 'web_search') { + return running ? 'Searching the web' : 'Searched the web'; + } + return formatToolName(name); +} + +function readableToolOutput(value: unknown): string | null { + if (value === undefined || value === null) return null; + if (typeof value === 'string') { + const trimmed = value.trim(); + if (!trimmed) return null; + try { + return readableToolOutput(JSON.parse(trimmed)) ?? trimmed; + } catch { + return trimmed; + } + } + if (typeof value === 'object') { + const object = value as Record; + for (const key of ['content', 'output', 'message', 'result']) { + if (typeof object[key] === 'string' && object[key].trim()) return object[key].trim(); + } + return Object.entries(object) + .map(([key, item]) => `- **${key.replace(/[_-]+/g, ' ')}:** ${String(item)}`) + .join('\n'); + } + return String(value); +} + /** * Status pill for a tool-call row — a tinted "Done" / "Failed" / "Running" * tag instead of a bare ✓/✕ glyph, so the outcome reads at a glance. Built on @@ -82,8 +114,11 @@ export function ToolCallRow({ detail?: string; /** Structured why/next explanation for a FAILED child tool call (#4459). */ failure?: ToolFailureExplanation; + /** Child tool output, rendered as Markdown when present. */ + result?: unknown; }; }) { + const output = readableToolOutput(call.result); return (
@@ -91,7 +126,7 @@ export function ToolCallRow({ • - {call.displayName ?? formatToolName(call.toolName)} + {call.displayName ?? subagentToolLabel(call.toolName, call.status)} {/* The contextual arg (path / recipient / query) can be long, so it truncates to a single line and absorbs the row's spare width — the @@ -117,6 +152,13 @@ export function ToolCallRow({ ) : null}
{call.status === 'error' && call.failure ? : null} + {output ? ( +
+ +
+ ) : null}
); } diff --git a/app/src/features/conversations/components/__tests__/ToolTimelineBlock.test.tsx b/app/src/features/conversations/components/__tests__/ToolTimelineBlock.test.tsx index 19e13fb219..f263631005 100644 --- a/app/src/features/conversations/components/__tests__/ToolTimelineBlock.test.tsx +++ b/app/src/features/conversations/components/__tests__/ToolTimelineBlock.test.tsx @@ -82,7 +82,7 @@ describe('SubagentActivityBlock', () => { expect(calls).toHaveLength(3); // Human labels + timing, with status as a tinted "Done" / "Failed" / // "Running" tag instead of a bare ✓/✕ glyph or the raw lowercase word. - expect(calls[0].textContent).toContain('Searching the web'); + expect(calls[0].textContent).toContain('Searched the web'); expect(calls[0].textContent).toContain('Done'); expect(calls[0].textContent).toContain('312ms'); expect(calls[1].textContent).toContain('Composio Execute'); @@ -93,6 +93,32 @@ describe('SubagentActivityBlock', () => { expect(calls[2].textContent).toContain('50ms'); }); + it('renders subagent web output as Markdown instead of raw JSON', () => { + renderInStore( + + ); + + expect(screen.getByText('Searched the web')).toBeInTheDocument(); + expect(screen.getByTestId('subagent-tool-output')).toHaveTextContent('Formal Conjectures'); + expect(screen.getByRole('strong')).toHaveTextContent('Formal Conjectures'); + expect(screen.queryByText(/"content"/)).not.toBeInTheDocument(); + }); + it('labels cancelled / awaiting-user calls distinctly (not the green "Done" pill)', () => { renderInStore( { expect(rows[0]).toHaveAttribute('data-testid', 'subagent-thought'); expect(rows[0].textContent).toContain('I should search the web first'); expect(rows[1]).toHaveAttribute('data-testid', 'subagent-tool-call'); - expect(rows[1].textContent).toContain('Searching the web'); + expect(rows[1].textContent).toContain('Searched the web'); expect(rows[2]).toHaveAttribute('data-testid', 'subagent-thought'); expect(rows[2].textContent).toContain('Found three relevant results'); }); @@ -1350,7 +1376,7 @@ describe('ToolTimelineBlock — sub-agent activity survives the transcript path' expect(screen.getByTestId('processing-subagent')).toBeInTheDocument(); const calls = screen.getAllByTestId('subagent-tool-call'); expect(calls).toHaveLength(2); - expect(calls[0].textContent).toContain('Searching the web'); + expect(calls[0].textContent).toContain('Searched the web'); expect(calls[0].textContent).toContain('Done'); // Human label, not the raw `web_fetch` slug. expect(calls[1].textContent).toContain('Fetching'); diff --git a/app/src/providers/ChatRuntimeProvider.tsx b/app/src/providers/ChatRuntimeProvider.tsx index 87b97da7af..59ee25f279 100644 --- a/app/src/providers/ChatRuntimeProvider.tsx +++ b/app/src/providers/ChatRuntimeProvider.tsx @@ -39,6 +39,7 @@ import { clearProcessingForThread, clearStreamingAssistantForThread, endInferenceTurn, + fetchAndHydrateCompletedTurnState, fetchAndHydrateDerivedTranscript, markInferenceTurnStreaming, parseToolFailure, @@ -473,6 +474,13 @@ const ChatRuntimeProvider = ({ children }: { children: React.ReactNode }) => { await flushQueuedFollowups(event.thread_id); dispatch(endInferenceTurn({ threadId: event.thread_id })); dispatch(clearThreadInferenceActive(event.thread_id)); + // Socket reducers keep only the current iteration's prose in the live + // buffer. Once the turn settles, replace that partial projection with + // the core's completed snapshot, whose ordered transcript contains every + // parent and sub-agent event from the whole turn. Doing this here (after + // ending the live lifecycle) matters: `hydrateRuntimeFromSnapshot` + // intentionally refuses to overwrite an actively streaming turn. + await dispatch(fetchAndHydrateCompletedTurnState(event.thread_id)); // Live-turn seam: the turn just settled and its line was appended to the // append-only transcript. Invalidate/refresh the thread's derived // settled-turn trails so the next reopen is fresh. The just-finished turn diff --git a/app/src/providers/__tests__/assistantUiMessages.test.ts b/app/src/providers/__tests__/assistantUiMessages.test.ts index 6a975f51d6..ce4313ecaf 100644 --- a/app/src/providers/__tests__/assistantUiMessages.test.ts +++ b/app/src/providers/__tests__/assistantUiMessages.test.ts @@ -133,6 +133,25 @@ describe('buildRuntimeMessages', () => { expect(ids).toEqual(['a', STREAMING_TAIL_ID]); }); + it('does not keep a synthetic thinking/tool tail running after lifecycle completion', () => { + const projected = buildRuntimeMessages([msg({ id: 'answer', sender: 'agent' })], null, { + isRunning: false, + liveTimeline: [tool({ id: 'stale-tool', status: 'success' })], + liveTranscript: [ + { kind: 'thinking', round: 1, seq: 0, text: 'already finished thinking' }, + ], + }); + const ids = projected.map(message => message.id); + + expect(ids).toEqual(['answer']); + expect(ids).not.toContain(STREAMING_TAIL_ID); + expect(projected[0]?.content).toEqual([ + { type: 'reasoning', text: 'already finished thinking' }, + expect.objectContaining({ type: 'tool-call', toolCallId: 'stale-tool' }), + { type: 'text', text: 'hello' }, + ]); + }); + it('replays a settled turn reasoning and tool calls from its request id', () => { const answer = msg({ id: 'answer', @@ -143,7 +162,8 @@ describe('buildRuntimeMessages', () => { const timeline = [tool({ id: 'call-1', status: 'success', result: 'found it' })]; const transcript = [ { kind: 'thinking' as const, round: 1, seq: 0, text: 'need to search' }, - { kind: 'toolCall' as const, round: 1, seq: 1, callId: 'call-1' }, + { kind: 'narration' as const, round: 1, seq: 1, text: 'I will check the sources.' }, + { kind: 'toolCall' as const, round: 1, seq: 2, callId: 'call-1' }, ]; expect( @@ -153,6 +173,7 @@ describe('buildRuntimeMessages', () => { })[0]?.content ).toEqual([ { type: 'reasoning', text: 'need to search' }, + { type: 'text', text: 'I will check the sources.' }, expect.objectContaining({ type: 'tool-call', toolCallId: 'call-1', @@ -163,6 +184,31 @@ describe('buildRuntimeMessages', () => { ]); }); + it('chronologically anchors persisted trails to async agent messages without request ids', () => { + const acknowledgement = msg({ + id: 'ack', + sender: 'agent', + content: 'Accepted background work', + extraMetadata: {}, + }); + const content = buildRuntimeMessages([acknowledgement], null, { + isRunning: false, + turnTimelines: { 'request-async': [tool({ id: 'async-tool', status: 'success' })] }, + turnTranscripts: { + 'request-async': [ + { kind: 'thinking', round: 1, seq: 0, text: 'delegate this research' }, + { kind: 'toolCall', round: 1, seq: 1, callId: 'async-tool' }, + ], + }, + })[0]?.content; + + expect(content).toEqual([ + { type: 'reasoning', text: 'delegate this research' }, + expect.objectContaining({ type: 'tool-call', toolCallId: 'async-tool' }), + { type: 'text', text: 'Accepted background work' }, + ]); + }); + /** * The crash this guards: assistant-ui keys tool parts as `toolCallId-${id}` * and throws "Duplicate key … in useResources" on a repeat, taking the whole diff --git a/app/src/providers/assistantUiMessages.ts b/app/src/providers/assistantUiMessages.ts index b2a837174e..884fb49e08 100644 --- a/app/src/providers/assistantUiMessages.ts +++ b/app/src/providers/assistantUiMessages.ts @@ -140,9 +140,13 @@ function assistantParts( parts.push(toolPart(entry)); } } - // Narration is process commentary, not the final assistant answer. The - // legacy pane keeps it in the processing view; projecting it as ordinary - // text here would duplicate prose around the persisted final response. + if (item.kind === 'narration' && item.text.trim().length > 0) { + // Narration emitted before a tool call is assistant content in its own + // right. Keep it inline in assistant-ui's ordered part stream; the final + // answer is appended separately below, so this preserves the real turn + // sequence without relying on the removed legacy processing pane. + parts.push({ type: 'text', text: item.text }); + } } for (const entry of [...timeline].sort((a, b) => a.seq - b.seq)) { @@ -260,6 +264,8 @@ export function streamingTailMessage( } export type AssistantUiProjection = { + /** Whether the synthetic live tail has an active core turn driving it. */ + isRunning?: boolean; liveTimeline?: readonly ToolTimelineEntry[]; liveTranscript?: readonly ProcessingTranscriptItem[]; turnTimelines?: Readonly>; @@ -280,25 +286,71 @@ export function buildRuntimeMessages( projection: AssistantUiProjection = {} ): ThreadMessageLike[] { const out: ThreadMessageLike[] = []; + const claimedRequestIds = new Set( + messages.flatMap(message => + message.sender === 'agent' && typeof message.extraMetadata?.requestId === 'string' + ? [message.extraMetadata.requestId] + : [] + ) + ); + const projectedRequestIds = [ + ...new Set([ + ...Object.keys(projection.turnTimelines ?? {}), + ...Object.keys(projection.turnTranscripts ?? {}), + ]), + ].filter(requestId => !claimedRequestIds.has(requestId)); + let orphanRequestCursor = 0; + const lastVisibleAgentId = [...messages] + .reverse() + .find(message => message.sender === 'agent' && !message.extraMetadata?.hidden)?.id; for (const msg of messages) { if (msg.extraMetadata?.hidden) continue; const requestId = msg.sender === 'agent' && typeof msg.extraMetadata?.requestId === 'string' ? msg.extraMetadata.requestId : undefined; + // Async acknowledgements/background deliveries can be persisted without + // message-level request metadata. The transcript maps are chronological + // and request-keyed, so pair only unclaimed trails with unanchored agent + // messages in the same order instead of dropping them from assistant-ui. + const effectiveRequestId = + requestId ?? + (msg.sender === 'agent' ? projectedRequestIds[orphanRequestCursor++] : undefined); + const persistedTimeline = effectiveRequestId + ? projection.turnTimelines?.[effectiveRequestId] + : undefined; + const persistedTranscript = effectiveRequestId + ? projection.turnTranscripts?.[effectiveRequestId] + : undefined; + // `chat_done` clears the active lifecycle before the completed snapshot is + // indexed into the request maps. Keep the just-settled tools/reasoning on + // the final assistant message during that handoff; never mint a running + // synthetic tail for them. + const useSettledLiveFallback = + projection.isRunning === false && + msg.id === lastVisibleAgentId && + !persistedTimeline && + !persistedTranscript; out.push( toThreadMessageLike( msg, - requestId ? (projection.turnTimelines?.[requestId] ?? EMPTY_TIMELINE) : EMPTY_TIMELINE, - requestId ? (projection.turnTranscripts?.[requestId] ?? EMPTY_TRANSCRIPT) : EMPTY_TRANSCRIPT + persistedTimeline ?? + (useSettledLiveFallback ? (projection.liveTimeline ?? EMPTY_TIMELINE) : EMPTY_TIMELINE), + persistedTranscript ?? + (useSettledLiveFallback + ? (projection.liveTranscript ?? EMPTY_TRANSCRIPT) + : EMPTY_TRANSCRIPT) ) ); } - const tail = streamingTailMessage( - streaming, - projection.liveTimeline ?? EMPTY_TIMELINE, - projection.liveTranscript ?? EMPTY_TRANSCRIPT - ); + const tail = + projection.isRunning === false + ? null + : streamingTailMessage( + streaming, + projection.liveTimeline ?? EMPTY_TIMELINE, + projection.liveTranscript ?? EMPTY_TRANSCRIPT + ); if (tail) out.push(tail); return out; } diff --git a/app/src/providers/useOpenHumanExternalStore.ts b/app/src/providers/useOpenHumanExternalStore.ts index 60390f2fe1..ac93161d73 100644 --- a/app/src/providers/useOpenHumanExternalStore.ts +++ b/app/src/providers/useOpenHumanExternalStore.ts @@ -59,25 +59,26 @@ export function useOpenHumanExternalStore(threadId: string | null) { : EMPTY_TURN_MAP ); + // `started` and `streaming` are both in-flight. A completed turn can retain + // its tool/reasoning arrays while the persisted projection catches up; those + // arrays must not mint a forever-running assistant-ui tail. + const isRunning = lifecycle === 'started' || lifecycle === 'streaming'; + // Recomputed only when the settled transcript or the live tail changes. // Settled messages are converted through an identity-keyed cache, so a token // landing on the tail re-converts exactly one message, never the transcript. const runtimeMessages = useMemo( () => buildRuntimeMessages(messages, streaming, { + isRunning, liveTimeline, liveTranscript, turnTimelines, turnTranscripts, }), - [messages, streaming, liveTimeline, liveTranscript, turnTimelines, turnTranscripts] + [messages, streaming, isRunning, liveTimeline, liveTranscript, turnTimelines, turnTranscripts] ); - // `started` and `streaming` are both in-flight; the row is deleted on - // completion, so a present lifecycle (other than the cold-boot `interrupted` - // marker, which has no live driver) means a turn is running. - const isRunning = lifecycle === 'started' || lifecycle === 'streaming'; - const onNew = useCallback( async (message: AppendMessage) => { const surface = getChatSurface(threadId); diff --git a/app/src/store/__tests__/chatRuntimeSlice.derived.thunk.test.ts b/app/src/store/__tests__/chatRuntimeSlice.derived.thunk.test.ts index 07bf2c63e8..d575b6affa 100644 --- a/app/src/store/__tests__/chatRuntimeSlice.derived.thunk.test.ts +++ b/app/src/store/__tests__/chatRuntimeSlice.derived.thunk.test.ts @@ -46,7 +46,7 @@ beforeEach(() => { }); describe('fetchAndHydrateDerivedTranscript', () => { - it('hydrates settled-turn trails from the projection, skipping the newest turn', async () => { + it('hydrates every settled trail when no request is actively streaming', async () => { const store = configureStore({ reducer }); mockThreadApi.getDerivedTranscript.mockResolvedValueOnce( page( @@ -66,10 +66,9 @@ describe('fetchAndHydrateDerivedTranscript', () => { expect(mockThreadApi.getTurnStateHistory).not.toHaveBeenCalled(); const timelines = store.getState().turnTimelinesByThread['thread-1']; const transcripts = store.getState().turnTranscriptsByThread['thread-1']; - // Newest turn (req-new) is skipped — rendered by the live anchor. - expect(Object.keys(transcripts)).toEqual(['req-old']); + expect(Object.keys(transcripts)).toEqual(['req-old', 'req-new']); expect(timelines['req-old']).toHaveLength(1); - expect(transcripts['req-new']).toBeUndefined(); + expect(transcripts['req-new']).toHaveLength(1); expect(timelines['req-new']).toBeUndefined(); }); @@ -133,9 +132,9 @@ describe('fetchAndHydrateDerivedTranscript', () => { await store.dispatch(fetchAndHydrateDerivedTranscript('thread-1')); const transcripts = store.getState().turnTranscriptsByThread['thread-1']; - // req-new skipped (newest), req-mid skipped (streaming), req-old kept. - expect(Object.keys(transcripts)).toEqual(['req-old']); + // Only the genuinely streaming request is skipped. + expect(Object.keys(transcripts)).toEqual(['req-old', 'req-new']); expect(transcripts['req-mid']).toBeUndefined(); - expect(transcripts['req-new']).toBeUndefined(); + expect(transcripts['req-new']).toHaveLength(1); }); }); diff --git a/app/src/store/chatRuntimeSlice.ts b/app/src/store/chatRuntimeSlice.ts index 102f3691ba..2656796850 100644 --- a/app/src/store/chatRuntimeSlice.ts +++ b/app/src/store/chatRuntimeSlice.ts @@ -3,7 +3,7 @@ import debug from 'debug'; import { mapDisplayItems } from '../features/conversations/derived/mapDisplayItems'; import { threadApi } from '../services/api/threadApi'; -import type { DerivedDisplayItem, DerivedTranscriptPage } from '../types/derivedTranscript'; +import type { DerivedTranscriptPage } from '../types/derivedTranscript'; import type { ThreadMessage } from '../types/thread'; import type { AgentRun, @@ -870,6 +870,29 @@ function subagentTranscriptItemFromPersisted( } function subagentActivityFromPersisted(activity: PersistedSubagentActivity): SubagentActivity { + const toolCalls = activity.toolCalls.map(subagentToolCallFromPersisted); + const transcript = + activity.transcript && activity.transcript.length > 0 + ? activity.transcript.map(item => { + const mapped = subagentTranscriptItemFromPersisted(item); + if (mapped.kind !== 'tool') return mapped; + const call = toolCalls.find(candidate => candidate.callId === mapped.callId); + return call + ? { ...mapped, args: call.args, result: call.result, failure: call.failure } + : mapped; + }) + : toolCalls.map(call => ({ + kind: 'tool' as const, + iteration: call.iteration, + callId: call.callId, + toolName: call.toolName, + status: call.status, + elapsedMs: call.elapsedMs, + outputChars: call.outputChars, + args: call.args, + result: call.result, + failure: call.failure, + })); return { taskId: activity.taskId, agentId: activity.agentId, @@ -882,23 +905,12 @@ function subagentActivityFromPersisted(activity: PersistedSubagentActivity): Sub iterations: activity.iterations, elapsedMs: activity.elapsedMs, outputChars: activity.outputChars, - toolCalls: activity.toolCalls.map(subagentToolCallFromPersisted), + toolCalls, // Prefer the persisted prose transcript (reasoning/narration interleaved // with tools) so a settled / reloaded run replays its thoughts. Fall back // to a tool-only rebuild for snapshots written before sub-agent prose was // persisted (the `transcript` field is absent there). - transcript: - activity.transcript && activity.transcript.length > 0 - ? activity.transcript.map(subagentTranscriptItemFromPersisted) - : activity.toolCalls.map(call => ({ - kind: 'tool' as const, - iteration: call.iteration, - callId: call.callId, - toolName: call.toolName, - status: call.status, - elapsedMs: call.elapsedMs, - outputChars: call.outputChars, - })), + transcript, }; } @@ -2382,6 +2394,34 @@ export const fetchAndHydrateTurnState = createAsyncThunk( } ); +/** + * Wait briefly for the progress bridge to flush its terminal snapshot, then + * hydrate it. `chat_done` is delivered by the response presenter while the + * bridge may still be consuming the final `TurnCompleted` event; reading once + * at that boundary can otherwise install an intermediate, last-round-only + * transcript over the just-settled UI. + */ +export const fetchAndHydrateCompletedTurnState = createAsyncThunk( + 'chatRuntime/fetchAndHydrateCompletedTurnState', + async (threadId: string, { dispatch }) => { + for (let attempt = 0; attempt < 20; attempt += 1) { + try { + const snapshot = await threadApi.getTurnState(threadId); + if (snapshot?.lifecycle === 'completed') { + dispatch(hydrateRuntimeFromSnapshot({ snapshot })); + return snapshot; + } + } catch (error) { + turnStateLog('completed snapshot fetch failed thread=%s err=%O', threadId, error); + return null; + } + await new Promise(resolve => window.setTimeout(resolve, 50)); + } + turnStateLog('completed snapshot did not arrive thread=%s', threadId); + return null; + } +); + /** * Fetch the per-turn history for a thread and populate * {@link ChatRuntimeState.turnTimelinesByThread} so each *past* answer renders @@ -2466,31 +2506,14 @@ function readChatRuntimeState(state: unknown): ChatRuntimeState | undefined { } /** - * The request ids whose derived trail must NOT be hydrated: the newest turn - * (rendered as the live "agent insights" anchor from `toolTimelineByThread` / - * the socket stream, or the `turn_state` snapshot via - * {@link fetchAndHydrateTurnState}) and any turn currently streaming. Mirrors - * `fetchAndHydrateTurnHistory`'s `history.slice(1)` newest-turn skip. + * The request ids whose derived trail must NOT be hydrated: only turns that + * are provably active in this renderer. Do not infer "live" from whichever + * request happens to be newest in one transcript file: detached/background + * delivery runs in a separate session, so the root file's newest request can + * already be historical and must remain visible. */ -function liveRequestIdsToSkip( - state: unknown, - threadId: string, - items: DerivedDisplayItem[] -): Set { +function liveRequestIdsToSkip(state: unknown, threadId: string): Set { const skip = new Set(); - // Newest turn = first request id encountered walking newest-first. - for (const item of items) { - const rid = - item.kind === 'turnBoundary' - ? item.requestId - : 'requestId' in item - ? item.requestId - : undefined; - if (rid) { - skip.add(rid); - break; - } - } const runtime = readChatRuntimeState(state); const streamingRid = runtime?.streamingAssistantByThread[threadId]?.requestId; if (streamingRid) skip.add(streamingRid); @@ -2536,7 +2559,7 @@ export const fetchAndHydrateDerivedTranscript = createAsyncThunk( await dispatch(fetchAndHydrateTurnHistory(threadId)); return null; } - const skipRequestIds = liveRequestIdsToSkip(getState(), threadId, page.items); + const skipRequestIds = liveRequestIdsToSkip(getState(), threadId); const { timelines, transcripts } = mapDisplayItems(page.items, { skipRequestIds }); derivedLog( 'hydrated thread=%s items=%d timelines=%d transcripts=%d skip=%d hasMore=%s', diff --git a/app/test/playwright/specs/chat-harness-subagent.spec.ts b/app/test/playwright/specs/chat-harness-subagent.spec.ts index 65d0018f82..3f02fbe1da 100644 --- a/app/test/playwright/specs/chat-harness-subagent.spec.ts +++ b/app/test/playwright/specs/chat-harness-subagent.spec.ts @@ -3,6 +3,7 @@ import { expect, type Page, test } from '@playwright/test'; import { agentMessageText } from '../helpers/chat-locators'; import { bootAuthenticatedPage, + callCoreRpc, dismissWalkthroughIfPresent, waitForAppReady, } from '../helpers/core-rpc'; @@ -12,21 +13,49 @@ const USER_ID = 'pw-chat-subagent'; const PROMPT = 'Research the answer to life and tell me a marker phrase.'; const CANARY_FINAL = 'subagent-canary-final-7afe2'; const RESEARCHER_REPLY = 'The researcher answer is 42.'; +const PARENT_THINKING = 'Parent trace: delegate the factual lookup before synthesizing.'; +const PARENT_NARRATION = 'I am delegating the factual lookup now.'; +const CHILD_THINKING = 'Child trace: isolate the requested marker before replying.'; +const FINAL_THINKING = 'Parent trace: verify the delegated finding and answer.'; const KEYWORD_RESPONSES = [ { keyword: "Search the user's memory tree", content: 'No relevant memory.' }, { keyword: PROMPT, - content: '', - toolCalls: [ + streamScript: [ + { thinking: PARENT_THINKING }, + { text: PARENT_NARRATION }, { - id: 'call_research_1', - name: 'research', - arguments: JSON.stringify({ prompt: 'Tell me a marker phrase' }), + toolCall: { + id: 'call_research_1', + name: 'research', + arguments: JSON.stringify({ prompt: 'Tell me a marker phrase' }), + }, }, + { finish: 'tool_calls' }, + ], + }, + { + // `spawn_async_subagent` wraps the requested task in its background-run + // contract, so the latest child user message is the rendered handoff, not + // the raw tool argument. + keyword: 'Run this task without requiring attention from the parent or user', + streamScript: [ + { thinking: CHILD_THINKING }, + { text: RESEARCHER_REPLY }, + { finish: 'stop' }, + ], + }, + { + // Detached completion is delivered to the parent as a fresh background + // notification turn; match that stable framing rather than depending on + // exactly how the result body is quoted inside it. + keyword: 'background sub-agent finished while you were busy', + streamScript: [ + { thinking: FINAL_THINKING }, + { text: `Done. The result is: ${CANARY_FINAL}` }, + { finish: 'stop' }, ], }, - { keyword: 'Tell me a marker phrase', content: RESEARCHER_REPLY }, - { keyword: RESEARCHER_REPLY, content: `Done. The result is: ${CANARY_FINAL}` }, ]; interface MockRequest { @@ -155,6 +184,7 @@ interface DiagnosticsSnapshot { phase: string | null; toolTimelineNames: string[]; toolTimelineIds: string[]; + turnTranscriptTexts: Record; messageCount: number; lastAssistantText: string | null; }; @@ -216,6 +246,10 @@ async function diagnosticsSnapshot(page: Page): Promise { chatRuntime?: { inferenceStatusByThread?: Record; toolTimelineByThread?: Record>; + turnTranscriptsByThread?: Record< + string, + Record> + >; }; thread?: { messagesByThread?: Record>; @@ -233,6 +267,10 @@ async function diagnosticsSnapshot(page: Page): Promise { currentThreadId && state?.chatRuntime?.toolTimelineByThread?.[currentThreadId] ? state.chatRuntime.toolTimelineByThread[currentThreadId] : []; + const turnTranscripts = + currentThreadId && state?.chatRuntime?.turnTranscriptsByThread?.[currentThreadId] + ? state.chatRuntime.turnTranscriptsByThread[currentThreadId] + : {}; const messages = currentThreadId && state?.thread?.messagesByThread?.[currentThreadId] ? state.thread.messagesByThread[currentThreadId] @@ -242,6 +280,12 @@ async function diagnosticsSnapshot(page: Page): Promise { phase, toolTimelineNames: timeline.map(entry => entry?.name ?? ''), toolTimelineIds: timeline.map(entry => entry?.id ?? ''), + turnTranscriptTexts: Object.fromEntries( + Object.entries(turnTranscripts).map(([requestId, items]) => [ + requestId, + items.map(item => item.text ?? ''), + ]) + ), messageCount: messages.length, lastAssistantText: typeof lastAssistant?.content === 'string' ? lastAssistant.content.slice(0, 240) : null, @@ -258,6 +302,7 @@ function formatDiagnostics(snapshot: DiagnosticsSnapshot): string { `matchedKeywords=${JSON.stringify(snapshot.matchedKeywords)}`, `runtime.phase=${snapshot.runtime.phase ?? ''}`, `runtime.toolTimelineNames=${JSON.stringify(snapshot.runtime.toolTimelineNames)}`, + `runtime.turnTranscriptTexts=${JSON.stringify(snapshot.runtime.turnTranscriptTexts)}`, `runtime.messageCount=${snapshot.runtime.messageCount}`, `runtime.lastAssistantText=${JSON.stringify(snapshot.runtime.lastAssistantText)}`, `completionProbes=${JSON.stringify( @@ -292,7 +337,7 @@ test.describe('Chat Harness - Subagent', () => { } }); - test('delegates to a subagent and persists the final orchestrator text', async ({ page }) => { + test('renders and rehydrates the full delegated-turn trace', async ({ page }) => { test.setTimeout(150_000); await resetMock(); @@ -301,7 +346,7 @@ test.describe('Chat Harness - Subagent', () => { await setMockBehavior('llmStreamChunkDelayMs', '10'); await openChat(page); - await createNewThread(page); + const threadId = await createNewThread(page); await sendMessage(page, PROMPT); // Three LLM hits are expected: orchestrator-1 (delegates), researcher @@ -312,18 +357,52 @@ test.describe('Chat Harness - Subagent', () => { await expect.poll(completionRequestCount, { timeout: 90_000 }).toBeGreaterThanOrEqual(3); await expect(agentMessageText(page, CANARY_FINAL)).toBeVisible({ timeout: 30_000 }); - const runtimeSnapshot = await diagnosticsSnapshot(page); - expect( - runtimeSnapshot.runtime.phase === 'subagent' || - runtimeSnapshot.runtime.toolTimelineNames.some(name => name.startsWith('subagent:')) || - runtimeSnapshot.runtime.toolTimelineIds.some(id => id.includes(':subagent:')), - `expected runtime to show a subagent delegation, got:\n ${formatDiagnostics( - runtimeSnapshot - )}` - ).toBe(true); - - // Re-assert after the runtime probe so the persisted message survives the - // turn-completion store transition rather than only being visible mid-stream. + // Re-assert after completion so the persisted message survives the + // turn-settlement transition rather than only being visible mid-stream. await expect(agentMessageText(page, CANARY_FINAL)).toBeVisible({ timeout: 15_000 }); + + // The trace belongs to assistant-ui's message parts—there is no parallel + // legacy timeline surface. + const finalMessage = page.getByTestId('agent-message').filter({ hasText: CANARY_FINAL }).last(); + await expect(finalMessage).toBeVisible({ timeout: 15_000 }); + const finalReasoning = finalMessage.getByRole('button', { name: /Reasoning/ }); + if ((await finalReasoning.getAttribute('aria-expanded')) !== 'true') await finalReasoning.click(); + await expect(finalMessage.getByText(FINAL_THINKING, { exact: true })).toBeVisible(); + + // Reloading removes the live socket and Redux stream. The same visual + // trace must rehydrate from persisted transcript/turn-state data. + await page.reload(); + await waitForAppReady(page); + await page.goto('/#/chat'); + const restoredThread = page.getByTestId(`thread-row-${threadId}`); + await expect(restoredThread).toBeVisible({ timeout: 15_000 }); + await restoredThread.click({ force: true }); + await expect.poll(() => selectedThreadId(page), { timeout: 15_000 }).toBe(threadId); + const derived = await callCoreRpc('openhuman.threads_transcript_get', { + thread_id: threadId, + limit: 500, + }); + expect(JSON.stringify(derived)).toContain(PARENT_THINKING); + await expect + .poll( + async () => + JSON.stringify((await diagnosticsSnapshot(page)).runtime.turnTranscriptTexts), + { timeout: 20_000 } + ) + .toContain(PARENT_THINKING); + const restoredMessage = page + .getByTestId('agent-message') + .filter({ has: page.getByTestId('assistant-ui-subagent-call') }) + .last(); + await expect(restoredMessage).toBeVisible({ timeout: 20_000 }); + const restoredReasoning = restoredMessage.getByRole('button', { name: /Reasoning/ }).first(); + if ((await restoredReasoning.getAttribute('aria-expanded')) !== 'true') { + await restoredReasoning.click(); + } + await expect(restoredMessage.getByText(PARENT_THINKING, { exact: true })).toBeVisible(); + const subagentCall = restoredMessage.getByTestId('assistant-ui-subagent-call'); + await expect(subagentCall).toBeVisible(); + await expect(subagentCall.getByTestId('subagent-activity')).toContainText(CHILD_THINKING); + await expect(subagentCall.getByTestId('subagent-activity')).toContainText(RESEARCHER_REPLY); }); }); diff --git a/app/test/playwright/specs/chat-tool-call-flow.spec.ts b/app/test/playwright/specs/chat-tool-call-flow.spec.ts index 5d697dc318..f30cd802f0 100644 --- a/app/test/playwright/specs/chat-tool-call-flow.spec.ts +++ b/app/test/playwright/specs/chat-tool-call-flow.spec.ts @@ -172,6 +172,22 @@ test.describe('Chat Tool Call Flow', () => { await expect(agentMessageText(page, CANARY_FINAL)).toBeVisible({ timeout: 40_000 }); + // Regression: completed tool/reasoning arrays remain in Redux briefly, but + // they must not create a synthetic running tail after the final answer. + await expect(page.getByLabel('Assistant is working')).toHaveCount(0); + await expect(page.getByText('running', { exact: true })).toHaveCount(0); + + // Tool activity belongs to assistant-ui and renders as a readable card, + // never through the removed legacy Agentic task insights timeline. + await expect(page.getByTestId('agent-task-insights')).toHaveCount(0); + const toolCard = page.getByTestId('assistant-ui-tool-call').last(); + await expect(toolCard).toBeVisible(); + await expect(toolCard).not.toContainText('running'); + const toolTrigger = toolCard.getByRole('button'); + if ((await toolTrigger.getAttribute('aria-expanded')) !== 'true') await toolTrigger.click(); + await expect(toolCard.getByText('Output', { exact: true })).toBeVisible(); + await expect(toolCard.getByRole('link', { name: 'https://example.com/' })).toBeVisible(); + await expect .poll( async () => { diff --git a/scripts/mock-api/routes/__tests__/llm.test.mjs b/scripts/mock-api/routes/__tests__/llm.test.mjs index 5152105f11..b0d089c6c4 100644 --- a/scripts/mock-api/routes/__tests__/llm.test.mjs +++ b/scripts/mock-api/routes/__tests__/llm.test.mjs @@ -200,6 +200,53 @@ test("streams reasoning deltas for reasoning-family models", async () => { assert.match(ctx.res.body, /data: \[DONE\]/); }); +test("keyword stream scripts preserve reasoning and tool-event order", async () => { + setMockBehaviors( + { + llmKeywordRules: JSON.stringify([ + { + keyword: "trace this", + streamScript: [ + { thinking: "first reason" }, + { text: "then narrate" }, + { + toolCall: { + id: "call_trace_1", + name: "web_search", + arguments: '{"q":"trace"}', + }, + }, + { finish: "tool_calls" }, + ], + }, + ]), + llmStreamChunkDelayMs: "0", + }, + "replace", + ); + + const ctx = makeCtx({ + parsedBody: { + model: "e2e-mock-model", + stream: true, + messages: [{ role: "user", content: "please trace this turn" }], + }, + }); + + assert.equal(handleLlmCompletions(ctx), true); + // `safeDelayMs` deliberately normalizes zero to the default cadence, so + // wait for this tiny four-event script to finish rather than sampling it + // halfway through its SSE writes. + await new Promise((resolve) => setTimeout(resolve, 180)); + const thinking = ctx.res.body.indexOf("first reason"); + const narration = ctx.res.body.indexOf("then narrate"); + const tool = ctx.res.body.indexOf("call_trace_1"); + assert.ok(thinking >= 0, "keyword stream script should emit reasoning_content"); + assert.ok(narration > thinking, "narration should follow reasoning"); + assert.ok(tool > narration, "tool call should follow narration"); + assert.match(ctx.res.body, /data: \[DONE\]/); +}); + test("returns tool calls for agentic models and resolves follow-up turns", () => { const first = makeCtx({ parsedBody: { diff --git a/scripts/mock-api/routes/llm.mjs b/scripts/mock-api/routes/llm.mjs index 649b002709..5ed2c3f294 100644 --- a/scripts/mock-api/routes/llm.mjs +++ b/scripts/mock-api/routes/llm.mjs @@ -316,10 +316,16 @@ function handleStreamingCompletion({ if (!rule || typeof rule.keyword !== "string") continue; if (probe.includes(rule.keyword.toLowerCase())) { const rendered = applyDynamicPlaceholdersToResponse(rule, parsedBody); - script = defaultStreamScript({ - content: rendered.content, - toolCalls: rendered.toolCalls, - }); + // Keyword routes need the same control over event ordering as a + // global stream script. That lets browser tests exercise a real + // reasoning -> narration -> tool sequence without a FIFO script + // being consumed by an unrelated completion. + script = Array.isArray(rendered.streamScript) + ? rendered.streamScript + : defaultStreamScript({ + content: rendered.content, + toolCalls: rendered.toolCalls, + }); break; } } diff --git a/src/openhuman/agent/harness/session/transcript.rs b/src/openhuman/agent/harness/session/transcript.rs index f9d66a068d..51253ff6bf 100644 --- a/src/openhuman/agent/harness/session/transcript.rs +++ b/src/openhuman/agent/harness/session/transcript.rs @@ -528,6 +528,35 @@ fn build_message_line( Some((failed, detail)) => (failed, detail), None => (false, None), }; + // Every assistant model response can carry its own thinking metadata, + // especially an intermediate response that also opens tool calls. Turn + // usage is attached only to the final assistant row, so sourcing reasoning + // exclusively from it silently dropped all pre-tool thoughts from the + // append-only display transcript. + let message_reasoning = (msg.role == "assistant") + .then(|| { + extra_metadata + .as_ref() + .and_then(|meta| { + meta.get(crate::openhuman::agent::message_convert::REASONING_EXT_KEY) + }) + .and_then(serde_json::Value::as_str) + .map(str::to_string) + }) + .flatten(); + let native_envelope = (msg.role == "assistant") + .then(|| serde_json::from_str::(&msg.content).ok()) + .flatten(); + let envelope_reasoning = native_envelope + .as_ref() + .and_then(|value| value.get("reasoning_content")) + .and_then(serde_json::Value::as_str) + .map(str::to_string); + let envelope_tool_calls = native_envelope + .as_ref() + .and_then(|value| value.get("tool_calls")) + .and_then(|value| serde_json::from_value::>(value.clone()).ok()) + .filter(|calls| !calls.is_empty()); MessageLine { id: msg.id.clone(), role: msg.role.clone(), @@ -536,13 +565,17 @@ fn build_message_line( provider: assistant_usage.map(|tu| tu.provider.clone()), model: assistant_usage.map(|tu| tu.model.clone()), usage: assistant_usage.map(|tu| tu.usage.clone()), - reasoning_content: assistant_usage.and_then(|tu| tu.reasoning_content.clone()), - tool_calls: assistant_usage.and_then(|tu| { - if tu.tool_calls.is_empty() { - None - } else { - Some(tu.tool_calls.clone()) - } + reasoning_content: message_reasoning + .or(envelope_reasoning) + .or_else(|| assistant_usage.and_then(|tu| tu.reasoning_content.clone())), + tool_calls: envelope_tool_calls.or_else(|| { + assistant_usage.and_then(|tu| { + if tu.tool_calls.is_empty() { + None + } else { + Some(tu.tool_calls.clone()) + } + }) }), iteration: assistant_usage.map(|tu| tu.iteration), ts: assistant_usage.map(|tu| tu.ts.clone()), @@ -1187,10 +1220,21 @@ pub fn read_transcript_display(path: &Path) -> Result /// transcript without accidentally folding delegated worker transcripts /// into the main chat timeline. pub fn find_root_transcript_for_thread(workspace_dir: &Path, thread_id: &str) -> Option { - raw_session_dirs(workspace_dir) - .into_iter() - .filter_map(|raw_dir| find_root_transcript_for_thread_in_dir(&raw_dir, thread_id)) - .max_by(|left, right| left.file_name().cmp(&right.file_name())) + find_root_transcripts_for_thread(workspace_dir, thread_id).pop() +} + +/// Find every root transcript associated with a thread, oldest first. +/// +/// A detached/background completion can open a new root session for the same +/// chat thread. Display readers must merge those files; selecting only the +/// newest makes the continuation appear to erase the original tool turn. +pub fn find_root_transcripts_for_thread(workspace_dir: &Path, thread_id: &str) -> Vec { + let mut matches = Vec::new(); + for raw_dir in raw_session_dirs(workspace_dir) { + matches.extend(root_transcripts_for_thread_in_dir(&raw_dir, thread_id)); + } + matches.sort_by(|left, right| left.file_name().cmp(&right.file_name())); + matches } fn raw_session_dirs(workspace_dir: &Path) -> Vec { @@ -1212,12 +1256,18 @@ fn raw_session_dirs(workspace_dir: &Path) -> Vec { } pub fn find_root_transcript_for_thread_in_dir(raw_dir: &Path, thread_id: &str) -> Option { + root_transcripts_for_thread_in_dir(raw_dir, thread_id).pop() +} + +fn root_transcripts_for_thread_in_dir(raw_dir: &Path, thread_id: &str) -> Vec { let thread_id = thread_id.trim(); if thread_id.is_empty() { - return None; + return Vec::new(); } - let entries = fs::read_dir(raw_dir).ok()?; + let Ok(entries) = fs::read_dir(raw_dir) else { + return Vec::new(); + }; let mut matches: Vec = entries .flatten() .map(|entry| entry.path()) @@ -1241,7 +1291,7 @@ pub fn find_root_transcript_for_thread_in_dir(raw_dir: &Path, thread_id: &str) - .collect(); matches.sort(); - matches.pop() + matches } /// Aggregated token/cost usage for a chat thread, summed across **all** of the diff --git a/src/openhuman/agent/harness/session/transcript_tests.rs b/src/openhuman/agent/harness/session/transcript_tests.rs index 260ff125de..3d7d80ed53 100644 --- a/src/openhuman/agent/harness/session/transcript_tests.rs +++ b/src/openhuman/agent/harness/session/transcript_tests.rs @@ -57,6 +57,21 @@ fn sample_turn_usage() -> TurnUsage { } } +#[test] +fn intermediate_assistant_reasoning_is_lifted_without_turn_usage() { + let mut message = ChatMessage::assistant("I will inspect the repository."); + message.extra_metadata = Some(serde_json::json!({ + crate::openhuman::agent::message_convert::REASONING_EXT_KEY: + "First identify the relevant files." + })); + + let line = build_message_line(&message, None, Some("request-1"), false); + assert_eq!( + line.reasoning_content.as_deref(), + Some("First identify the relevant files.") + ); +} + #[test] fn round_trip_produces_byte_identical_messages() { let dir = TempDir::new().unwrap(); diff --git a/src/openhuman/threads/transcript_view/cache.rs b/src/openhuman/threads/transcript_view/cache.rs index f8b9016837..d1dcbfd418 100644 --- a/src/openhuman/threads/transcript_view/cache.rs +++ b/src/openhuman/threads/transcript_view/cache.rs @@ -70,8 +70,10 @@ impl TranscriptViewCache { workspace_dir: &Path, thread_id: &str, ) -> Option> { - let (root_path, sub_paths) = project::resolve_files(workspace_dir, thread_id)?; - let signature: Vec = std::iter::once(file_sig(&root_path)) + let (root_paths, sub_paths) = project::resolve_files(workspace_dir, thread_id)?; + let signature: Vec = root_paths + .iter() + .map(|path| file_sig(path)) .chain(sub_paths.iter().map(|p| file_sig(p))) .collect(); @@ -94,7 +96,9 @@ impl TranscriptViewCache { } let projected = Arc::new(project::project_from_files( - thread_id, &root_path, &sub_paths, + thread_id, + &root_paths, + &sub_paths, )); let mut inner = self.inner.lock().ok()?; diff --git a/src/openhuman/threads/transcript_view/project.rs b/src/openhuman/threads/transcript_view/project.rs index ea9aa125d1..411aa8e6a8 100644 --- a/src/openhuman/threads/transcript_view/project.rs +++ b/src/openhuman/threads/transcript_view/project.rs @@ -35,61 +35,87 @@ const CHANNEL_CONTEXT_PREFIX: &str = "[Channel context]"; /// project everything into display items. Returns `None` when the thread has /// no root transcript yet (brand-new thread / first turn not persisted). pub fn project_thread(workspace_dir: &Path, thread_id: &str) -> Option { - let (root_path, sub_paths) = resolve_files(workspace_dir, thread_id)?; - Some(project_from_files(thread_id, &root_path, &sub_paths)) + let (root_paths, sub_paths) = resolve_files(workspace_dir, thread_id)?; + Some(project_from_files(thread_id, &root_paths, &sub_paths)) } /// Resolve the on-disk file set backing a thread's transcript view: the root /// transcript path plus every sub-agent sibling file. `None` when the thread /// has no root transcript yet. Exposed so the cache can key on these paths /// (and their mtimes/lengths) without re-projecting. -pub fn resolve_files(workspace_dir: &Path, thread_id: &str) -> Option<(PathBuf, Vec)> { - let root_path = transcript::find_root_transcript_for_thread(workspace_dir, thread_id)?; - let root_stem = root_path.file_stem()?.to_str()?.to_string(); - let Some(raw_dir) = root_path.parent() else { - log::warn!( - "{LOG_PREFIX} resolved root has no parent thread={thread_id} root={}", - root_path.display() - ); +pub fn resolve_files( + workspace_dir: &Path, + thread_id: &str, +) -> Option<(Vec, Vec)> { + let root_paths = transcript::find_root_transcripts_for_thread(workspace_dir, thread_id); + if root_paths.is_empty() { return None; - }; - let sub_paths = discover_subagent_files(raw_dir, &root_stem); - Some((root_path, sub_paths)) + } + let mut sub_paths = Vec::new(); + for root_path in &root_paths { + let Some(root_stem) = root_path.file_stem().and_then(|stem| stem.to_str()) else { + continue; + }; + let Some(raw_dir) = root_path.parent() else { + continue; + }; + sub_paths.extend(discover_subagent_files(raw_dir, root_stem)); + } + sub_paths.sort(); + sub_paths.dedup(); + Some((root_paths, sub_paths)) } /// Project a thread from an already-resolved file set (root + sub-agent /// siblings). Missing/unreadable files degrade to empty rather than failing. pub fn project_from_files( thread_id: &str, - root_path: &Path, + root_paths: &[PathBuf], sub_paths: &[PathBuf], ) -> ProjectedTranscript { - let root_stem = root_path - .file_stem() - .and_then(|s| s.to_str()) - .unwrap_or_default() - .to_string(); - log::debug!( - "{LOG_PREFIX} projecting thread={thread_id} root={} subagent_files={}", - root_path.display(), + "{LOG_PREFIX} projecting thread={thread_id} roots={} subagent_files={}", + root_paths.len(), sub_paths.len() ); // Read the root display records once: they feed both the top-level items // and the per-turn timestamp ranges used to anchor sub-agent trails. - let (mut items, segments) = match transcript::read_transcript_display(root_path) { - Ok(d) => (project_records(&d.records), turn_segments(&d.records)), - Err(err) => { - log::warn!( - "{LOG_PREFIX} failed to read root transcript {}: {err}", - root_path.display() - ); - (Vec::new(), Vec::new()) + let mut items = Vec::new(); + let mut segments = Vec::new(); + for root_path in root_paths { + match transcript::read_transcript_display(root_path) { + Ok(display) => { + items.extend(project_records(&display.records)); + segments.extend(turn_segments(&display.records)); + } + Err(err) => { + log::warn!( + "{LOG_PREFIX} failed to read root transcript {}: {err}", + root_path.display() + ); + } } - }; + } - let subagents = build_subagent_items(sub_paths, &root_stem, 0, &segments); + let mut subagents = Vec::new(); + for root_path in root_paths { + let root_stem = root_path + .file_stem() + .and_then(|s| s.to_str()) + .unwrap_or_default(); + let prefix = format!("{root_stem}__"); + let siblings: Vec = sub_paths + .iter() + .filter(|path| { + path.file_stem() + .and_then(|stem| stem.to_str()) + .is_some_and(|stem| stem.starts_with(&prefix)) + }) + .cloned() + .collect(); + subagents.extend(build_subagent_items(&siblings, root_stem, 0, &segments)); + } log::debug!( "{LOG_PREFIX} projected thread={thread_id} top_level_items={} subagents={}", items.len(), @@ -384,10 +410,27 @@ fn project_assistant( .unwrap_or_default(); let interim = !tool_calls.is_empty(); + // Native tool-call turns are persisted as their provider envelope so they + // can be replayed byte-faithfully. The display projection needs only the + // envelope's visible `content`; rendering/sanitizing the whole JSON object + // makes the narration disappear (and risks showing raw tool JSON). + let visible_content = serde_json::from_str::(&msg.message.content) + .ok() + .and_then(|value| { + let object = value.as_object()?; + object.get("tool_calls")?.as_array()?; + match object.get("content") { + Some(serde_json::Value::String(content)) => Some(content.clone()), + Some(serde_json::Value::Null) | None => Some(String::new()), + _ => None, + } + }) + .unwrap_or_else(|| msg.message.content.clone()); + // The assistant's prose (if any) shows before its tool calls. - if !msg.message.content.trim().is_empty() { + if !visible_content.trim().is_empty() { items.push(DisplayItem::AssistantMessage { - content: msg.message.content.clone(), + content: visible_content, interim, request_id: msg.request_id.clone(), model: msg.turn_usage.as_ref().map(|tu| tu.model.clone()), diff --git a/vendor/tinyagents b/vendor/tinyagents index 954b3dcf58..1284a93f21 160000 --- a/vendor/tinyagents +++ b/vendor/tinyagents @@ -1 +1 @@ -Subproject commit 954b3dcf58038e8c11d674976fb779f1c72c6af4 +Subproject commit 1284a93f21876a5363ed14bc0554edba6a5ce917 From dedc62c46658a3b64559e6946aebf9a715d48007 Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Mon, 31 Aug 2026 18:57:02 +0300 Subject: [PATCH 02/23] fix(chat): render final assistant result once Co-authored-by: Medulla --- .../providers/__tests__/assistantUiMessages.test.ts | 13 +++++++++++++ app/src/providers/assistantUiMessages.ts | 10 +++++++--- .../playwright/specs/chat-tool-call-flow.spec.ts | 1 + 3 files changed, 21 insertions(+), 3 deletions(-) diff --git a/app/src/providers/__tests__/assistantUiMessages.test.ts b/app/src/providers/__tests__/assistantUiMessages.test.ts index ce4313ecaf..3f318f356d 100644 --- a/app/src/providers/__tests__/assistantUiMessages.test.ts +++ b/app/src/providers/__tests__/assistantUiMessages.test.ts @@ -209,6 +209,19 @@ describe('buildRuntimeMessages', () => { ]); }); + it('renders final streamed narration only once', () => { + const finalText = 'hey! what is up?'; + const answer = msg({ id: 'answer', sender: 'agent', content: finalText }); + const content = buildRuntimeMessages([answer], null, { + turnTranscripts: { + request: [{ kind: 'narration', round: 1, seq: 0, text: finalText }], + }, + turnTimelines: { request: [] }, + })[0]?.content; + + expect(content).toEqual([{ type: 'text', text: finalText }]); + }); + /** * The crash this guards: assistant-ui keys tool parts as `toolCallId-${id}` * and throws "Duplicate key … in useResources" on a repeat, taking the whole diff --git a/app/src/providers/assistantUiMessages.ts b/app/src/providers/assistantUiMessages.ts index 884fb49e08..117f1f46b0 100644 --- a/app/src/providers/assistantUiMessages.ts +++ b/app/src/providers/assistantUiMessages.ts @@ -140,11 +140,15 @@ function assistantParts( parts.push(toolPart(entry)); } } - if (item.kind === 'narration' && item.text.trim().length > 0) { + if ( + item.kind === 'narration' && + item.text.trim().length > 0 && + item.text.trim() !== text.trim() + ) { // Narration emitted before a tool call is assistant content in its own // right. Keep it inline in assistant-ui's ordered part stream; the final - // answer is appended separately below, so this preserves the real turn - // sequence without relying on the removed legacy processing pane. + // answer is appended separately below. A final-round narration is the + // same streamed bytes as that answer and must not render twice. parts.push({ type: 'text', text: item.text }); } } diff --git a/app/test/playwright/specs/chat-tool-call-flow.spec.ts b/app/test/playwright/specs/chat-tool-call-flow.spec.ts index f30cd802f0..5642446b50 100644 --- a/app/test/playwright/specs/chat-tool-call-flow.spec.ts +++ b/app/test/playwright/specs/chat-tool-call-flow.spec.ts @@ -171,6 +171,7 @@ test.describe('Chat Tool Call Flow', () => { await sendMessage(page, PROMPT); await expect(agentMessageText(page, CANARY_FINAL)).toBeVisible({ timeout: 40_000 }); + await expect(page.getByText(`Here is the fetched content: ${CANARY_FINAL}`, { exact: true })).toHaveCount(1); // Regression: completed tool/reasoning arrays remain in Redux briefly, but // they must not create a synthetic running tail after the final answer. From 6651d3dd4bc076b5845a0901c6ed003382497de0 Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Mon, 31 Aug 2026 20:05:34 +0300 Subject: [PATCH 03/23] fix(chat): keep responses whole and align composer controls Co-authored-by: Medulla --- app/src/components/assistant-ui/thread.tsx | 2 +- .../components/ChatToolParts.test.tsx | 1 + .../components/ChatToolParts.tsx | 2 +- .../__tests__/Conversations.render.test.tsx | 13 ++++++++---- src/openhuman/web_chat/presentation.rs | 16 +++++++------- ...nnels_bus_presentation_raw_coverage_e2e.rs | 21 ++++--------------- 6 files changed, 25 insertions(+), 30 deletions(-) diff --git a/app/src/components/assistant-ui/thread.tsx b/app/src/components/assistant-ui/thread.tsx index 852085b1fb..ddd4d29e47 100644 --- a/app/src/components/assistant-ui/thread.tsx +++ b/app/src/components/assistant-ui/thread.tsx @@ -600,7 +600,7 @@ const ComposerAction: FC<{ type="button" variant="default" size="icon" - className="aui-composer-cancel size-7 rounded-full" + className="aui-composer-cancel size-7 rounded-full bg-primary-500 text-content-inverted hover:bg-primary-600" data-testid="stop-generation-button" aria-label="Stop generating"> diff --git a/app/src/features/conversations/components/ChatToolParts.test.tsx b/app/src/features/conversations/components/ChatToolParts.test.tsx index 63162031e8..1e6b9283e9 100644 --- a/app/src/features/conversations/components/ChatToolParts.test.tsx +++ b/app/src/features/conversations/components/ChatToolParts.test.tsx @@ -64,6 +64,7 @@ describe('ChatToolParts', () => { expect(screen.getByTestId('assistant-ui-tool-call')).toHaveTextContent('Searched the web'); await userEvent.click(screen.getByRole('button', { name: /Searched the web/ })); expect(screen.getByText(/Lean open conjectures/)).toBeInTheDocument(); + expect(screen.queryByText('Query', { exact: true })).not.toBeInTheDocument(); expect(screen.getByText('Found 12 candidate problems')).toBeInTheDocument(); }); diff --git a/app/src/features/conversations/components/ChatToolParts.tsx b/app/src/features/conversations/components/ChatToolParts.tsx index 0f0097bf35..fef105e5b0 100644 --- a/app/src/features/conversations/components/ChatToolParts.tsx +++ b/app/src/features/conversations/components/ChatToolParts.tsx @@ -143,7 +143,7 @@ function ToolDataView({ value }: { value: unknown }) { // Tool wrappers frequently add bookkeeping beside the actual payload // (`tool_call_id`, success, timing). When a semantic output field exists, // show that value directly and hide the wrapper entirely. - for (const key of ['content', 'output', 'result', 'message']) { + for (const key of ['content', 'output', 'result', 'message', 'query', 'q']) { const semantic = entries.find(([candidate]) => candidate === key)?.[1]; if (hasDisplayValue(semantic)) return ; } diff --git a/app/src/pages/__tests__/Conversations.render.test.tsx b/app/src/pages/__tests__/Conversations.render.test.tsx index 090a836be4..aa391a27dd 100644 --- a/app/src/pages/__tests__/Conversations.render.test.tsx +++ b/app/src/pages/__tests__/Conversations.render.test.tsx @@ -654,7 +654,7 @@ describe('Conversations — smoke render (#1123 welcome-lock removal)', () => { }); // No past-turn tool call before hydration. - expect(screen.queryByText(/read_file/)).not.toBeInTheDocument(); + expect(screen.queryByTestId('assistant-ui-tool-call')).not.toBeInTheDocument(); // Hydrate the older turn's timeline (as fetchAndHydrateTurnHistory would). await act(async () => { @@ -669,8 +669,7 @@ describe('Conversations — smoke render (#1123 welcome-lock removal)', () => { }); // The past turn's tool call is projected into assistant-ui exactly once. - fireEvent.click(await screen.findByRole('button', { name: /1 tool call/ })); - expect(await screen.findByText('read_file')).toBeInTheDocument(); + expect(await screen.findByTestId('assistant-ui-tool-call')).toHaveTextContent('Read File'); }); it('keeps assistant message copy available through assistant-ui', async () => { @@ -955,7 +954,13 @@ describe('Conversations — smoke render (#1123 welcome-lock removal)', () => { // The send cleared the composer; with an empty composer mid-send the Send // button morphs into the Stop button, so there is no Send affordance left // to fire a duplicate send. - expect(screen.getByRole('button', { name: 'Stop generating' })).toBeInTheDocument(); + const stopButton = screen.getByRole('button', { name: 'Stop generating' }); + expect(stopButton).toBeInTheDocument(); + expect(stopButton).toHaveClass( + 'bg-primary-500', + 'text-content-inverted', + 'hover:bg-primary-600' + ); expect(screen.queryByRole('button', { name: 'Send message' })).not.toBeInTheDocument(); resolveSend?.(); }); diff --git a/src/openhuman/web_chat/presentation.rs b/src/openhuman/web_chat/presentation.rs index 3db2b671b0..1263affe33 100644 --- a/src/openhuman/web_chat/presentation.rs +++ b/src/openhuman/web_chat/presentation.rs @@ -44,12 +44,12 @@ fn usage_payload(usage: Option<&LastTurnUsage>) -> Option { }) } -/// Deliver an agent response to the frontend, applying local-model -/// presentation (segmentation + reaction) when the model is available. +/// Deliver one unmodified agent response to the frontend. /// -/// Always emits at least one `chat_done` event. When the response is -/// segmented, emits one `chat_segment` per bubble first, then a final -/// `chat_done` with the full text for deduplication. +/// Desktop/web chat owns Markdown layout inside one assistant message. Splitting +/// paragraphs into `chat_segment` messages duplicates tool/reasoning parts and +/// turns one answer into several bubbles, so this path always emits exactly one +/// `chat_done` with the model's original text. pub(crate) async fn deliver_response( client_id: &str, thread_id: &str, @@ -66,8 +66,10 @@ pub(crate) async fn deliver_response( let user_msg_owned = user_message.to_string(); let reaction_handle = tokio::spawn(async move { try_reaction(&user_msg_owned).await }); - // Segmentation is pure CPU work, runs immediately. - let segments = segment_for_delivery(full_response); + // Keep the response byte-for-byte in one assistant message. The legacy + // segmentation helpers remain available to channel-specific callers/tests, + // but the interactive web surface must not cut or reformat model output. + let segments = vec![full_response.to_string()]; // Await the reaction result (should already be done or nearly done). let reaction_emoji = reaction_handle.await.unwrap_or(None); diff --git a/tests/raw_coverage/channels_bus_presentation_raw_coverage_e2e.rs b/tests/raw_coverage/channels_bus_presentation_raw_coverage_e2e.rs index 14ff116460..6842d20320 100644 --- a/tests/raw_coverage/channels_bus_presentation_raw_coverage_e2e.rs +++ b/tests/raw_coverage/channels_bus_presentation_raw_coverage_e2e.rs @@ -79,16 +79,13 @@ async fn presentation_segments_text_and_delivers_single_bubble_with_citations() } #[tokio::test] -async fn presentation_delivers_segment_events_then_deduping_done_event() { +async fn presentation_delivers_one_unmodified_done_event() { let response = [ "First paragraph has enough natural language content to stand alone as a separate chat bubble.", "Second paragraph also contains enough prose to exercise segmented delivery and delay calculation.", "Third paragraph ensures the final chat_done event carries the complete response for deduplication.", ] .join("\n\n"); - let segments = presentation_test_support::segment_for_delivery_for_test(&response); - assert!(segments.len() >= 2, "expected segmented delivery"); - let mut rx = subscribe_web_channel_events(); presentation_test_support::deliver_response_for_test( "round20-client", @@ -100,30 +97,20 @@ async fn presentation_delivers_segment_events_then_deduping_done_event() { ) .await; - let mut seen_segments = 0_u32; let final_event = timeout(Duration::from_secs(10), async { loop { let event = rx.recv().await.expect("presentation event"); if event.request_id != "round20-segmented" { continue; } - match event.event.as_str() { - "chat_segment" => { - assert_eq!(event.segment_total, Some(segments.len() as u32)); - assert_eq!(event.segment_index, Some(seen_segments)); - assert!(event.full_response.as_deref().unwrap_or("").len() >= 40); - seen_segments += 1; - } - "chat_done" => break event, - other => panic!("unexpected presentation event {other}"), - } + assert_eq!(event.event, "chat_done"); + break event; } }) .await .expect("segmented delivery timeout"); - assert_eq!(seen_segments, segments.len() as u32); - assert_eq!(final_event.segment_total, Some(segments.len() as u32)); + assert_eq!(final_event.segment_total, None); assert_eq!( final_event.full_response.as_deref(), Some(response.as_str()) From f6fd9a1d436dacc80d509e602e65656d4c0eb84b Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Mon, 31 Aug 2026 20:09:26 +0300 Subject: [PATCH 04/23] fix(chat): infer descriptive labels for degraded tools Co-authored-by: Medulla --- .../components/ChatToolParts.test.tsx | 20 +++++++++++ .../components/ChatToolParts.tsx | 28 ++++++++++++--- .../components/SubagentActivityBlock.tsx | 34 ++++++++++++++++--- .../__tests__/ToolTimelineBlock.test.tsx | 22 ++++++++++++ 4 files changed, 96 insertions(+), 8 deletions(-) diff --git a/app/src/features/conversations/components/ChatToolParts.test.tsx b/app/src/features/conversations/components/ChatToolParts.test.tsx index 1e6b9283e9..4be0fe60b7 100644 --- a/app/src/features/conversations/components/ChatToolParts.test.tsx +++ b/app/src/features/conversations/components/ChatToolParts.test.tsx @@ -88,4 +88,24 @@ describe('ChatToolParts', () => { expect(screen.getByRole('strong')).toHaveTextContent('Example Domain'); expect(screen.queryByText('Content', { exact: true })).not.toBeInTheDocument(); }); + + it('infers web search labels when a persisted tool name degraded to tool', () => { + render( + {}} + resume={() => {}} + respondToApproval={() => {}} + /> + ); + + expect(screen.getByTestId('assistant-ui-tool-call')).toHaveTextContent('Searched the web'); + expect(screen.getByTestId('assistant-ui-tool-call')).not.toHaveTextContent(/^Tool done$/); + }); }); diff --git a/app/src/features/conversations/components/ChatToolParts.tsx b/app/src/features/conversations/components/ChatToolParts.tsx index fef105e5b0..cb6162a6c1 100644 --- a/app/src/features/conversations/components/ChatToolParts.tsx +++ b/app/src/features/conversations/components/ChatToolParts.tsx @@ -99,11 +99,31 @@ function friendlyLabel(key: string): string { .replace(/^./, char => char.toUpperCase()); } -function toolDisplayName(toolName: string, running: boolean): string { - if (toolName === 'web_fetch') return running ? 'Fetching from the web' : 'Fetched from the web'; - if (toolName === 'web_search_tool' || toolName === 'web_search') { +function toolDisplayName( + toolName: string, + running: boolean, + args: unknown, + result: unknown +): string { + const lowerName = toolName.toLowerCase(); + const parsedArgs = parsedValue(args); + const argKeys = + parsedArgs && typeof parsedArgs === 'object' && !Array.isArray(parsedArgs) + ? Object.keys(parsedArgs as object).map(key => key.toLowerCase()) + : []; + const renderedResult = typeof result === 'string' ? result : JSON.stringify(result ?? ''); + const looksLikeSearch = + lowerName.includes('search') || + argKeys.some(key => ['query', 'q', 'search_query'].includes(key)) || + /(?:^|\n)#?\s*search results\b/i.test(renderedResult); + const looksLikeFetch = + lowerName.includes('fetch') || + argKeys.some(key => ['url', 'uri'].includes(key)) || + /\bstatus=\d{3}\s+url=/i.test(renderedResult); + if (looksLikeSearch) { return running ? 'Searching the web' : 'Searched the web'; } + if (looksLikeFetch) return running ? 'Fetching from the web' : 'Fetched from the web'; return formatToolName(toolName); } @@ -187,7 +207,7 @@ export const OpenHumanToolCall: ToolCallMessagePartComponent = ({ - {toolDisplayName(toolName, running)} + {toolDisplayName(toolName, running, args, result)} {running ? ( diff --git a/app/src/features/conversations/components/SubagentActivityBlock.tsx b/app/src/features/conversations/components/SubagentActivityBlock.tsx index d3b4d429d7..6bc5bc6701 100644 --- a/app/src/features/conversations/components/SubagentActivityBlock.tsx +++ b/app/src/features/conversations/components/SubagentActivityBlock.tsx @@ -37,12 +37,31 @@ function toolCallTone(status: ToolTimelineEntryStatus): string { return 'text-coral-700 dark:text-coral-300'; } -function subagentToolLabel(name: string, status: ToolTimelineEntryStatus): string { +function subagentToolLabel( + name: string, + status: ToolTimelineEntryStatus, + args: unknown, + result: unknown +): string { const running = status === 'running'; - if (name === 'web_fetch') return running ? 'Fetching from the web' : 'Fetched from the web'; - if (name === 'web_search_tool' || name === 'web_search') { + const lowerName = name.toLowerCase(); + const argKeys = + args && typeof args === 'object' && !Array.isArray(args) + ? Object.keys(args as object).map(key => key.toLowerCase()) + : []; + const output = readableToolOutput(result) ?? ''; + const looksLikeSearch = + lowerName.includes('search') || + argKeys.some(key => ['query', 'q', 'search_query'].includes(key)) || + /(?:^|\n)#?\s*search results\b/i.test(output); + const looksLikeFetch = + lowerName.includes('fetch') || + argKeys.some(key => ['url', 'uri'].includes(key)) || + /\bstatus=\d{3}\s+url=/i.test(output); + if (looksLikeSearch) { return running ? 'Searching the web' : 'Searched the web'; } + if (looksLikeFetch) return running ? 'Fetching from the web' : 'Fetched from the web'; return formatToolName(name); } @@ -114,11 +133,18 @@ export function ToolCallRow({ detail?: string; /** Structured why/next explanation for a FAILED child tool call (#4459). */ failure?: ToolFailureExplanation; + /** Arguments supplied to the child tool, used for descriptive fallback labels. */ + args?: unknown; /** Child tool output, rendered as Markdown when present. */ result?: unknown; }; }) { const output = readableToolOutput(call.result); + const suppliedLabel = call.displayName?.trim(); + const label = + suppliedLabel && suppliedLabel.toLowerCase() !== 'tool' + ? suppliedLabel + : subagentToolLabel(call.toolName, call.status, call.args, call.result); return (
@@ -126,7 +152,7 @@ export function ToolCallRow({ • - {call.displayName ?? subagentToolLabel(call.toolName, call.status)} + {label} {/* The contextual arg (path / recipient / query) can be long, so it truncates to a single line and absorbs the row's spare width — the diff --git a/app/src/features/conversations/components/__tests__/ToolTimelineBlock.test.tsx b/app/src/features/conversations/components/__tests__/ToolTimelineBlock.test.tsx index f263631005..98cfe6e4a2 100644 --- a/app/src/features/conversations/components/__tests__/ToolTimelineBlock.test.tsx +++ b/app/src/features/conversations/components/__tests__/ToolTimelineBlock.test.tsx @@ -119,6 +119,28 @@ describe('SubagentActivityBlock', () => { expect(screen.queryByText(/"content"/)).not.toBeInTheDocument(); }); + it('infers a descriptive search label for a degraded subagent tool name', () => { + renderInStore( + + ); + + expect(screen.getByTestId('subagent-tool-call')).toHaveTextContent('Searched the web'); + }); + it('labels cancelled / awaiting-user calls distinctly (not the green "Done" pill)', () => { renderInStore( Date: Mon, 31 Aug 2026 20:33:25 +0300 Subject: [PATCH 05/23] fix(chat): coalesce persisted assistant turn segments Co-authored-by: Medulla --- .../__tests__/assistantUiMessages.test.ts | 69 ++++++++++- app/src/providers/assistantUiMessages.ts | 112 +++++++++++++++++- .../specs/chat-tool-call-flow.spec.ts | 13 +- .../threads/transcript_view/project.rs | 73 +++++++++--- .../transcript_view/transcript_view_tests.rs | 54 +++++++++ 5 files changed, 288 insertions(+), 33 deletions(-) diff --git a/app/src/providers/__tests__/assistantUiMessages.test.ts b/app/src/providers/__tests__/assistantUiMessages.test.ts index 3f318f356d..a8b891c8ed 100644 --- a/app/src/providers/__tests__/assistantUiMessages.test.ts +++ b/app/src/providers/__tests__/assistantUiMessages.test.ts @@ -137,9 +137,7 @@ describe('buildRuntimeMessages', () => { const projected = buildRuntimeMessages([msg({ id: 'answer', sender: 'agent' })], null, { isRunning: false, liveTimeline: [tool({ id: 'stale-tool', status: 'success' })], - liveTranscript: [ - { kind: 'thinking', round: 1, seq: 0, text: 'already finished thinking' }, - ], + liveTranscript: [{ kind: 'thinking', round: 1, seq: 0, text: 'already finished thinking' }], }); const ids = projected.map(message => message.id); @@ -213,15 +211,74 @@ describe('buildRuntimeMessages', () => { const finalText = 'hey! what is up?'; const answer = msg({ id: 'answer', sender: 'agent', content: finalText }); const content = buildRuntimeMessages([answer], null, { - turnTranscripts: { - request: [{ kind: 'narration', round: 1, seq: 0, text: finalText }], - }, + turnTranscripts: { request: [{ kind: 'narration', round: 1, seq: 0, text: finalText }] }, turnTimelines: { request: [] }, })[0]?.content; expect(content).toEqual([{ type: 'text', text: finalText }]); }); + it('coalesces legacy assistant segments into one bubble with one tool trail', () => { + const requestId = 'legacy-segmented-request'; + const intro = "Here's the crypto picture today:"; + const finalText = `${intro}\n\nBitcoin is trading around $77,000.`; + const messages = [ + msg({ id: 'user', content: 'What is happening with Bitcoin?' }), + msg({ + id: 'tool-envelope', + sender: 'agent', + content: JSON.stringify({ + content: null, + tool_calls: [ + { id: 'call-search', name: 'web_search_tool', arguments: '{"query":"bitcoin"}' }, + ], + }), + extraMetadata: { requestId }, + }), + msg({ id: 'intro', sender: 'agent', content: intro, extraMetadata: { requestId } }), + msg({ id: 'final', sender: 'agent', content: finalText, extraMetadata: { requestId } }), + ]; + + const projected = buildRuntimeMessages(messages, null, { + turnTimelines: { + [requestId]: [ + tool({ id: 'call-search', name: 'tool', status: 'success', result: 'market results' }), + ], + }, + turnTranscripts: { + [requestId]: [{ kind: 'toolCall', round: 1, seq: 0, callId: 'call-search' }], + }, + }); + + expect(projected).toHaveLength(2); + expect(projected[1]).toMatchObject({ id: 'final', role: 'assistant' }); + expect(projected[1]?.content).toEqual([ + expect.objectContaining({ + type: 'tool-call', + toolCallId: 'call-search', + toolName: 'web_search_tool', + }), + { type: 'text', text: finalText }, + ]); + }); + + it('does not coalesce adjacent assistant turns with different request ids', () => { + const projected = buildRuntimeMessages( + [ + msg({ id: 'first', sender: 'agent', content: 'first', extraMetadata: { requestId: 'r1' } }), + msg({ + id: 'second', + sender: 'agent', + content: 'second', + extraMetadata: { requestId: 'r2' }, + }), + ], + null + ); + + expect(projected.map(message => message.id)).toEqual(['first', 'second']); + }); + /** * The crash this guards: assistant-ui keys tool parts as `toolCallId-${id}` * and throws "Duplicate key … in useResources" on a repeat, taking the whole diff --git a/app/src/providers/assistantUiMessages.ts b/app/src/providers/assistantUiMessages.ts index 117f1f46b0..31a0dd7551 100644 --- a/app/src/providers/assistantUiMessages.ts +++ b/app/src/providers/assistantUiMessages.ts @@ -45,6 +45,8 @@ const conversionCache = new WeakMap(); const EMPTY_TIMELINE: readonly ToolTimelineEntry[] = []; const EMPTY_TRANSCRIPT: readonly ProcessingTranscriptItem[] = []; +const RECOVERED_TOOL_NAMES_KEY = 'assistantUiToolNames'; + /** Synthetic id for the live streaming tail. Stable so React reconciles it. */ export const STREAMING_TAIL_ID = '__openhuman_streaming_tail__'; @@ -168,6 +170,97 @@ function stringArray(value: unknown): string[] { : []; } +function requestIdOf(message: ThreadMessage): string | undefined { + const requestId = message.extraMetadata?.requestId; + return typeof requestId === 'string' && requestId.length > 0 ? requestId : undefined; +} + +function isGenericToolName(name: string): boolean { + return ['', 'tool', 'unknown', 'unknown_tool'].includes(name.trim().toLowerCase()); +} + +function recoverTimelineToolNames( + timeline: readonly ToolTimelineEntry[], + recoveredNames: readonly string[] +): readonly ToolTimelineEntry[] { + if (recoveredNames.length === 0 || !timeline.some(entry => isGenericToolName(entry.name))) { + return timeline; + } + let recoveredIndex = 0; + return timeline.map(entry => { + const recovered = recoveredNames[recoveredIndex]; + recoveredIndex += 1; + return recovered && isGenericToolName(entry.name) ? { ...entry, name: recovered } : entry; + }); +} + +function mergedAssistantText(messages: readonly ThreadMessage[]): string { + const texts = messages + .map(message => unwrapToolCallEnvelope(message.content ?? '').text) + .filter(text => text.trim().length > 0) + .filter((text, index, all) => all.indexOf(text) === index); + if (texts.length < 2) return texts[0] ?? ''; + + // Legacy web delivery persisted both paragraph-sized segments and the full + // final response. Prefer the complete response when it contains every + // segment; otherwise retain each distinct assistant emission in order. + const longest = [...texts].sort((left, right) => right.length - left.length)[0] ?? ''; + if (texts.every(text => longest.includes(text.trim()))) return longest; + return texts.join('\n\n'); +} + +function mergeAssistantRun(messages: readonly ThreadMessage[]): ThreadMessage { + if (messages.length === 1) return messages[0]; + const first = messages[0]; + const last = messages[messages.length - 1]; + const extraMetadata = Object.assign({}, ...messages.map(message => message.extraMetadata)); + const requestId = messages.map(requestIdOf).find(Boolean); + const toolNames = messages.flatMap( + message => unwrapToolCallEnvelope(message.content ?? '').toolNames + ); + if (requestId) extraMetadata.requestId = requestId; + if (toolNames.length > 0) extraMetadata[RECOVERED_TOOL_NAMES_KEY] = toolNames; + return { + ...last, + content: mergedAssistantText(messages), + createdAt: first.createdAt, + extraMetadata, + }; +} + +/** + * Collapse legacy paragraph/tool-envelope rows into one assistant turn. + * + * The old interactive-web delivery path persisted each segment as a separate + * agent message. Consecutive assistant rows cannot cross a user turn; when + * both rows carry request ids, a differing id is the explicit boundary. + */ +function coalesceAssistantSegments(messages: readonly ThreadMessage[]): ThreadMessage[] { + const out: ThreadMessage[] = []; + let run: ThreadMessage[] = []; + let runRequestId: string | undefined; + + const flush = () => { + if (run.length > 0) out.push(mergeAssistantRun(run)); + run = []; + runRequestId = undefined; + }; + + for (const message of messages) { + if (message.sender !== 'agent' || message.extraMetadata?.hidden) { + flush(); + out.push(message); + continue; + } + const requestId = requestIdOf(message); + if (run.length > 0 && runRequestId && requestId && runRequestId !== requestId) flush(); + run.push(message); + runRequestId ??= requestId; + } + flush(); + return out; +} + function mimeTypeFromDataUri(dataUri: string): string { return dataUri.match(/^data:([^;,]+)/i)?.[1] ?? 'application/octet-stream'; } @@ -218,13 +311,19 @@ export function toThreadMessageLike( const cached = conversionCache.get(msg); if (cached?.timeline === timeline && cached.transcript === transcript) return cached.converted; - const text = - msg.sender === 'agent' ? unwrapToolCallEnvelope(msg.content ?? '').text : (msg.content ?? ''); + const unwrapped = unwrapToolCallEnvelope(msg.content ?? ''); + const text = msg.sender === 'agent' ? unwrapped.text : (msg.content ?? ''); + const recoveredToolNames = [ + ...unwrapped.toolNames, + ...stringArray(msg.extraMetadata?.[RECOVERED_TOOL_NAMES_KEY]), + ]; + const effectiveTimeline = recoverTimelineToolNames(timeline, recoveredToolNames); const converted: ThreadMessageLike = { id: msg.id, role: msg.sender === 'agent' ? 'assistant' : 'user', - content: msg.sender === 'agent' ? assistantParts(text, timeline, transcript) : userParts(msg), + content: + msg.sender === 'agent' ? assistantParts(text, effectiveTimeline, transcript) : userParts(msg), createdAt: new Date(msg.createdAt), ...(msg.sender === 'agent' && msg.extraMetadata?.stopped === true ? { status: { type: 'incomplete' as const, reason: 'cancelled' as const } } @@ -289,9 +388,10 @@ export function buildRuntimeMessages( streaming: StreamingAssistantState | null, projection: AssistantUiProjection = {} ): ThreadMessageLike[] { + const coalescedMessages = coalesceAssistantSegments(messages); const out: ThreadMessageLike[] = []; const claimedRequestIds = new Set( - messages.flatMap(message => + coalescedMessages.flatMap(message => message.sender === 'agent' && typeof message.extraMetadata?.requestId === 'string' ? [message.extraMetadata.requestId] : [] @@ -304,10 +404,10 @@ export function buildRuntimeMessages( ]), ].filter(requestId => !claimedRequestIds.has(requestId)); let orphanRequestCursor = 0; - const lastVisibleAgentId = [...messages] + const lastVisibleAgentId = [...coalescedMessages] .reverse() .find(message => message.sender === 'agent' && !message.extraMetadata?.hidden)?.id; - for (const msg of messages) { + for (const msg of coalescedMessages) { if (msg.extraMetadata?.hidden) continue; const requestId = msg.sender === 'agent' && typeof msg.extraMetadata?.requestId === 'string' diff --git a/app/test/playwright/specs/chat-tool-call-flow.spec.ts b/app/test/playwright/specs/chat-tool-call-flow.spec.ts index 5642446b50..f5c1d96411 100644 --- a/app/test/playwright/specs/chat-tool-call-flow.spec.ts +++ b/app/test/playwright/specs/chat-tool-call-flow.spec.ts @@ -11,6 +11,8 @@ const MOCK_ADMIN_BASE = `http://127.0.0.1:${process.env.E2E_MOCK_PORT || '18473' const USER_ID = 'pw-chat-tool-call'; const PROMPT = 'Fetch the contents of https://example.com for me.'; const CANARY_FINAL = 'canary-tool-call-fetched-a1b2c3'; +const CANARY_SECOND_PARAGRAPH = 'The entire answer must stay in this same assistant bubble.'; +const FINAL_RESPONSE = `Here is the fetched content: ${CANARY_FINAL}\n\n${CANARY_SECOND_PARAGRAPH}`; const FORCED_RESPONSES = [ { content: '', @@ -22,7 +24,7 @@ const FORCED_RESPONSES = [ }, ], }, - { content: `Here is the fetched content: ${CANARY_FINAL}` }, + { content: FINAL_RESPONSE }, ]; interface MockRequest { @@ -171,7 +173,10 @@ test.describe('Chat Tool Call Flow', () => { await sendMessage(page, PROMPT); await expect(agentMessageText(page, CANARY_FINAL)).toBeVisible({ timeout: 40_000 }); - await expect(page.getByText(`Here is the fetched content: ${CANARY_FINAL}`, { exact: true })).toHaveCount(1); + const finalBubble = page.getByTestId('agent-message').filter({ hasText: CANARY_FINAL }); + await expect(finalBubble).toHaveCount(1); + await expect(finalBubble).toContainText(CANARY_SECOND_PARAGRAPH); + await expect(page.getByText(CANARY_SECOND_PARAGRAPH, { exact: true })).toHaveCount(1); // Regression: completed tool/reasoning arrays remain in Redux briefly, but // they must not create a synthetic running tail after the final answer. @@ -181,8 +186,10 @@ test.describe('Chat Tool Call Flow', () => { // Tool activity belongs to assistant-ui and renders as a readable card, // never through the removed legacy Agentic task insights timeline. await expect(page.getByTestId('agent-task-insights')).toHaveCount(0); - const toolCard = page.getByTestId('assistant-ui-tool-call').last(); + await expect(page.getByTestId('assistant-ui-tool-call')).toHaveCount(1); + const toolCard = page.getByTestId('assistant-ui-tool-call'); await expect(toolCard).toBeVisible(); + await expect(toolCard).toContainText('Fetched from the web'); await expect(toolCard).not.toContainText('running'); const toolTrigger = toolCard.getByRole('button'); if ((await toolTrigger.getAttribute('aria-expanded')) !== 'true') await toolTrigger.click(); diff --git a/src/openhuman/threads/transcript_view/project.rs b/src/openhuman/threads/transcript_view/project.rs index 411aa8e6a8..c1c7cc5a5a 100644 --- a/src/openhuman/threads/transcript_view/project.rs +++ b/src/openhuman/threads/transcript_view/project.rs @@ -403,28 +403,37 @@ fn project_assistant( } } - let tool_calls = msg + let persisted_tool_calls: Vec<(String, String, String)> = msg .turn_usage .as_ref() - .map(|tu| tu.tool_calls.as_slice()) + .map(|tu| { + tu.tool_calls + .iter() + .map(|call| (call.id.clone(), call.name.clone(), call.arguments.clone())) + .collect() + }) .unwrap_or_default(); + let native_envelope = parse_native_tool_envelope(&msg.message.content); + // Some native/tinyagents histories predate top-level `turn_usage` lifting + // and carry the invocation only inside the provider replay envelope. Use + // that canonical call shape rather than degrading the paired result to an + // orphan named "tool". + let tool_calls = if persisted_tool_calls.is_empty() { + native_envelope + .as_ref() + .map(|(_, calls)| calls.clone()) + .unwrap_or_default() + } else { + persisted_tool_calls + }; let interim = !tool_calls.is_empty(); // Native tool-call turns are persisted as their provider envelope so they // can be replayed byte-faithfully. The display projection needs only the // envelope's visible `content`; rendering/sanitizing the whole JSON object // makes the narration disappear (and risks showing raw tool JSON). - let visible_content = serde_json::from_str::(&msg.message.content) - .ok() - .and_then(|value| { - let object = value.as_object()?; - object.get("tool_calls")?.as_array()?; - match object.get("content") { - Some(serde_json::Value::String(content)) => Some(content.clone()), - Some(serde_json::Value::Null) | None => Some(String::new()), - _ => None, - } - }) + let visible_content = native_envelope + .map(|(content, _)| content) .unwrap_or_else(|| msg.message.content.clone()); // The assistant's prose (if any) shows before its tool calls. @@ -438,20 +447,48 @@ fn project_assistant( }); } - for call in tool_calls { - let args = parse_tool_args(&call.arguments); + for (call_id, name, arguments) in tool_calls { + let args = parse_tool_args(&arguments); items.push(DisplayItem::ToolCall { - call_id: call.id.clone(), - name: call.name.clone(), + call_id: call_id.clone(), + name, args, result: None, status: ToolCallStatus::Running, failure: None, }); - pending.push_back((call.id.clone(), items.len() - 1)); + pending.push_back((call_id, items.len() - 1)); } } +/// Decode the native provider replay envelope embedded in `ChatMessage.content`. +/// Returns visible assistant prose plus `(id, name, arguments)` calls. +fn parse_native_tool_envelope(raw: &str) -> Option<(String, Vec<(String, String, String)>)> { + let value = serde_json::from_str::(raw).ok()?; + let object = value.as_object()?; + let calls = object.get("tool_calls")?.as_array()?; + let content = match object.get("content") { + Some(serde_json::Value::String(content)) => content.clone(), + Some(serde_json::Value::Null) | None => String::new(), + _ => return None, + }; + let calls = calls + .iter() + .filter_map(|call| { + let call = call.as_object()?; + let id = call.get("id")?.as_str()?.to_string(); + let name = call.get("name")?.as_str()?.to_string(); + let arguments = match call.get("arguments") { + Some(serde_json::Value::String(arguments)) => arguments.clone(), + Some(arguments) => arguments.to_string(), + None => String::new(), + }; + Some((id, name, arguments)) + }) + .collect(); + Some((content, calls)) +} + fn project_tool_result( msg: &DisplayMessage, items: &mut Vec, diff --git a/src/openhuman/threads/transcript_view/transcript_view_tests.rs b/src/openhuman/threads/transcript_view/transcript_view_tests.rs index bdbb8cb950..2532c95420 100644 --- a/src/openhuman/threads/transcript_view/transcript_view_tests.rs +++ b/src/openhuman/threads/transcript_view/transcript_view_tests.rs @@ -123,6 +123,60 @@ fn projects_turn_with_tools_reasoning_and_sanitization() { } } +#[test] +fn recovers_tool_name_from_native_envelope_without_turn_usage() { + let dir = TempDir::new().unwrap(); + let envelope = serde_json::json!({ + "content": null, + "tool_calls": [{ + "id": "call-web-1", + "name": "web_fetch", + "arguments": { "url": "https://example.com" } + }] + }) + .to_string(); + let assistant = serde_json::json!({ + "role": "assistant", + "content": envelope, + "request_id": "req-native" + }) + .to_string(); + let result = + r#"{"role":"tool","content":"Example Domain","id":"call-web-1","request_id":"req-native"}"#; + let path = write_raw( + dir.path(), + "101_orchestrator", + "thr_native", + &[assistant.as_str(), result], + ); + let display = read_transcript_display(&path).unwrap(); + let items = project_records(&display.records); + + let tool = items + .iter() + .find_map(|item| match item { + DisplayItem::ToolCall { + name, + args, + result, + status, + .. + } => Some((name, args, result, status)), + _ => None, + }) + .expect("native envelope tool call projected"); + assert_eq!(tool.0, "web_fetch"); + assert_eq!( + tool.1 + .as_ref() + .and_then(|args| args.get("url")) + .and_then(serde_json::Value::as_str), + Some("https://example.com") + ); + assert_eq!(tool.2.as_deref(), Some("Example Domain")); + assert_eq!(*tool.3, ToolCallStatus::Success); +} + #[test] fn projects_compaction_and_interrupted_partial() { let dir = TempDir::new().unwrap(); From 50465317884ad45cbff1c2f0b5160560808a7fbd Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Mon, 31 Aug 2026 20:53:37 +0300 Subject: [PATCH 06/23] fix(auth): preserve local session during login handoff Co-authored-by: Medulla --- app/src/providers/CoreStateProvider.tsx | 61 ++++++++++++------- .../__tests__/CoreStateProvider.test.tsx | 39 ++++++++++++ 2 files changed, 77 insertions(+), 23 deletions(-) diff --git a/app/src/providers/CoreStateProvider.tsx b/app/src/providers/CoreStateProvider.tsx index 53fa0323c9..aabb4f2391 100644 --- a/app/src/providers/CoreStateProvider.tsx +++ b/app/src/providers/CoreStateProvider.tsx @@ -679,33 +679,46 @@ export default function CoreStateProvider({ children }: { children: ReactNode }) [refresh] ); + // The core switches credentials before the refreshed app snapshot reaches + // React. Keep the token being installed visible to the expiry handler during + // that gap; otherwise a late 401 from the previous/cloud surface can clear a + // newly stored local session even though the core correctly ignores it. + const sessionTokenBeingStoredRef = useRef(null); + const storeSessionToken = useCallback( async (token: string, user?: object) => { logoutGuardUntilRef.current = 0; - await storeSession(token, user ?? {}); + sessionTokenBeingStoredRef.current = token; try { - await syncMemoryClientToken(token); - memoryTokenRef.current = token; - } catch (error) { - console.warn('[core-state] memory client sync failed after session store:', error); - } - // refresh() drives refreshCore, which now owns identity-flip detection - // and dispatches handleIdentityFlip when both prev and next are - // authenticated and identities differ. The previous standalone - // restartApp call here was redundant and skipped the persist purge, - // letting redux-persist rehydrate the prior user's slices on launch - // (#900). Restart now happens inside handleIdentityFlip after purge. - // Swallow refresh failures here so a cold-boot `app_state_snapshot` - // timeout post-login doesn't surface as an unhandled rejection - // (OPENHUMAN-REACT-Z/Y) — the polling loop reconciles within - // `POLL_MS`. - await refresh().catch(err => { - log('refresh failed after session store: %O', sanitizeError(err)); - }); - if (!isLocalSessionToken(token)) { - await refreshTeams().catch(err => { - log('refreshTeams failed after session store: %O', sanitizeError(err)); + await storeSession(token, user ?? {}); + try { + await syncMemoryClientToken(token); + memoryTokenRef.current = token; + } catch (error) { + console.warn('[core-state] memory client sync failed after session store:', error); + } + // refresh() drives refreshCore, which now owns identity-flip detection + // and dispatches handleIdentityFlip when both prev and next are + // authenticated and identities differ. The previous standalone + // restartApp call here was redundant and skipped the persist purge, + // letting redux-persist rehydrate the prior user's slices on launch + // (#900). Restart now happens inside handleIdentityFlip after purge. + // Swallow refresh failures here so a cold-boot `app_state_snapshot` + // timeout post-login doesn't surface as an unhandled rejection + // (OPENHUMAN-REACT-Z/Y) — the polling loop reconciles within + // `POLL_MS`. + await refresh().catch(err => { + log('refresh failed after session store: %O', sanitizeError(err)); }); + if (!isLocalSessionToken(token)) { + await refreshTeams().catch(err => { + log('refreshTeams failed after session store: %O', sanitizeError(err)); + }); + } + } finally { + if (sessionTokenBeingStoredRef.current === token) { + sessionTokenBeingStoredRef.current = null; + } } }, [refresh, refreshTeams] @@ -782,7 +795,9 @@ export default function CoreStateProvider({ children }: { children: ReactNode }) // so re-registers are rare. useEffect(() => { const runReauth = async (method: string, source: string, reason: AuthExpiredReason) => { - if (isLocalSessionToken(getCoreStateSnapshot().snapshot.sessionToken)) { + const effectiveToken = + sessionTokenBeingStoredRef.current ?? getCoreStateSnapshot().snapshot.sessionToken; + if (isLocalSessionToken(effectiveToken)) { log('auth-expired ignored for local session (method=%s source=%s)', method, source); return; } diff --git a/app/src/providers/__tests__/CoreStateProvider.test.tsx b/app/src/providers/__tests__/CoreStateProvider.test.tsx index 6b3751857a..c48f850ed3 100644 --- a/app/src/providers/__tests__/CoreStateProvider.test.tsx +++ b/app/src/providers/__tests__/CoreStateProvider.test.tsx @@ -536,6 +536,45 @@ describe('CoreStateProvider — identity-change cache clearing', () => { expect(vi.mocked(tauriCommands.logout)).not.toHaveBeenCalled(); }); + it('does not clear a local session while its refreshed snapshot is still pending', async () => { + const localToken = `eyJhbGciOiJub25lIn0.${window.btoa(JSON.stringify({ sub: 'local' }))}.local`; + const stored = deferred(); + fetchSnapshot.mockResolvedValue(makeSnapshot({ userId: 'cloud-user', sessionToken: 'old' })); + listTeams.mockResolvedValue([]); + vi.mocked(tauriCommands.storeSession).mockReset(); + vi.mocked(tauriCommands.storeSession).mockReturnValue(stored.promise as never); + vi.mocked(tauriCommands.logout).mockReset(); + vi.mocked(tauriCommands.logout).mockResolvedValue(undefined as never); + + let ctx: CoreStateContextValue | undefined; + render( + + (ctx = next)} /> + + ); + await waitFor(() => expect(screen.getByTestId('ready').textContent).toBe('ready')); + + let storing!: Promise; + await act(async () => { + storing = ctx!.storeSessionToken(localToken, { id: 'local' }); + await Promise.resolve(); + window.dispatchEvent( + new CustomEvent('core-rpc-auth-expired', { + detail: { + method: 'openhuman.announcements_get_latest', + source: 'rpc', + reason: 'confirmed', + }, + }) + ); + await Promise.resolve(); + }); + + expect(vi.mocked(tauriCommands.logout)).not.toHaveBeenCalled(); + stored.resolve(); + await act(async () => storing); + }); + it('dispatching core-rpc-auth-expired triggers clearSession (and debounces repeated fires within 10s)', async () => { fetchSnapshot.mockResolvedValue(makeSnapshot({ userId: 'u1', sessionToken: 'tok1' })); listTeams.mockResolvedValue([]); From 2f4cc97c34e04ba2e931a987ee4f95c4c65ebcc8 Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Mon, 31 Aug 2026 20:53:38 +0300 Subject: [PATCH 07/23] refactor(chat): read settled transcripts directly from core Co-authored-by: Medulla --- .../features/conversations/Conversations.tsx | 7 -- .../components/ChatThreadView.tsx | 19 ++-- .../__tests__/Conversations.render.test.tsx | 40 ++++---- .../providers/AssistantUiRuntimeProvider.tsx | 12 +-- app/src/providers/ChatRuntimeProvider.tsx | 11 +-- .../AssistantUiRuntimeProvider.test.tsx | 67 ++++++++++++- .../providers/useOpenHumanExternalStore.ts | 94 +++++++++++++++---- 7 files changed, 175 insertions(+), 75 deletions(-) diff --git a/app/src/features/conversations/Conversations.tsx b/app/src/features/conversations/Conversations.tsx index ac50f5db3d..5596868489 100644 --- a/app/src/features/conversations/Conversations.tsx +++ b/app/src/features/conversations/Conversations.tsx @@ -81,7 +81,6 @@ import { clearRuntimeForThread, clearThreadSendPending, enqueueFollowup, - fetchAndHydrateDerivedTranscript, fetchAndHydrateTurnState, hydrateThreadUsage, markThreadSendPending, @@ -730,12 +729,6 @@ const Conversations = ({ if (selectedThreadId) { void dispatch(loadThreadMessages(selectedThreadId)); void dispatch(fetchAndHydrateTurnState(selectedThreadId)); - // Per-turn history: each past answer's own process trail. Phase C derives - // this from the append-only transcript projection - // (`threads_transcript_get`), auto-falling back to the legacy - // `turn_state_history` hydration when the derived path is off, errors, or - // the thread has no persisted transcript (legacy thread). - void dispatch(fetchAndHydrateDerivedTranscript(selectedThreadId)); void threadApi .getTaskBoard(selectedThreadId) .then(board => { diff --git a/app/src/features/conversations/components/ChatThreadView.tsx b/app/src/features/conversations/components/ChatThreadView.tsx index 1841ca7cac..003c384a0a 100644 --- a/app/src/features/conversations/components/ChatThreadView.tsx +++ b/app/src/features/conversations/components/ChatThreadView.tsx @@ -13,6 +13,7 @@ import { import { Conversation, ConversationContent } from '../../../components/ai-elements'; import { useStickToBottom } from '../../../hooks/useStickToBottom'; +import { useCoreTranscriptProjection } from '../../../providers/useOpenHumanExternalStore'; import type { ProcessingTranscriptItem, ToolTimelineEntry } from '../../../store/chatRuntimeSlice'; import { useAppDispatch, useAppSelector } from '../../../store/hooks'; import { persistReaction } from '../../../store/threadSlice'; @@ -196,10 +197,6 @@ export const ChatThreadView = forwardRef state.chatRuntime.toolTimelineByThread); - const turnTimelinesByThread = useAppSelector(state => state.chatRuntime.turnTimelinesByThread); - const turnTranscriptsByThread = useAppSelector( - state => state.chatRuntime.turnTranscriptsByThread - ); const interruptedAssistantByThread = useAppSelector( state => state.chatRuntime.interruptedAssistantByThread ); @@ -347,14 +344,16 @@ export const ChatThreadView = forwardRef { const anchors: Record = {}; const seen = new Set(); diff --git a/app/src/pages/__tests__/Conversations.render.test.tsx b/app/src/pages/__tests__/Conversations.render.test.tsx index aa391a27dd..15e9d02bc4 100644 --- a/app/src/pages/__tests__/Conversations.render.test.tsx +++ b/app/src/pages/__tests__/Conversations.render.test.tsx @@ -25,7 +25,6 @@ import chatRuntimeReducer, { setInferenceStatusForThread, setStreamingAssistantForThread, setToolTimelineForThread, - setTurnTimelinesForThread, } from '../../store/chatRuntimeSlice'; import layoutReducer from '../../store/layoutSlice'; import socketReducer from '../../store/socketSlice'; @@ -76,6 +75,15 @@ vi.mock('../../services/api/threadApi', () => ({ getThreadMessages: mockGetThreadMessages, getTurnState: vi.fn().mockResolvedValue(null), getTurnStateHistory: vi.fn().mockResolvedValue([]), + getDerivedTranscript: vi + .fn() + .mockResolvedValue({ + threadId: 'none', + items: [], + total: 0, + hasMore: false, + hasTranscript: false, + }), getTaskBoard: vi .fn() .mockResolvedValue({ threadId: 't-1', cards: [], updatedAt: '2026-05-04T10:00:00Z' }), @@ -641,9 +649,18 @@ describe('Conversations — smoke render (#1123 welcome-lock removal)', () => { mockGetThreads.mockResolvedValue({ threads: [thread], count: 1 }); mockGetThreadMessages.mockResolvedValue({ messages, count: messages.length }); - let store: ReturnType | undefined; + vi.mocked(threadApi.getDerivedTranscript).mockResolvedValueOnce({ + threadId: thread.id, + items: [ + { kind: 'toolCall', callId: 'tc-1', name: 'read_file', status: 'success' }, + { kind: 'turnBoundary', requestId: 'req-1' }, + ], + total: 2, + hasMore: false, + hasTranscript: true, + }); await act(async () => { - store = await renderConversations({ + await renderConversations({ thread: { ...selectedThreadState(thread), messagesByThreadId: { [thread.id]: messages }, @@ -653,22 +670,7 @@ describe('Conversations — smoke render (#1123 welcome-lock removal)', () => { }); }); - // No past-turn tool call before hydration. - expect(screen.queryByTestId('assistant-ui-tool-call')).not.toBeInTheDocument(); - - // Hydrate the older turn's timeline (as fetchAndHydrateTurnHistory would). - await act(async () => { - store!.dispatch( - setTurnTimelinesForThread({ - threadId: thread.id, - timelines: { - 'req-1': [{ id: 'tc-1', name: 'read_file', round: 0, seq: 0, status: 'success' }], - }, - }) - ); - }); - - // The past turn's tool call is projected into assistant-ui exactly once. + // The past turn's core transcript is projected into assistant-ui exactly once. expect(await screen.findByTestId('assistant-ui-tool-call')).toHaveTextContent('Read File'); }); diff --git a/app/src/providers/AssistantUiRuntimeProvider.tsx b/app/src/providers/AssistantUiRuntimeProvider.tsx index b35c7ba023..d03eec2c9f 100644 --- a/app/src/providers/AssistantUiRuntimeProvider.tsx +++ b/app/src/providers/AssistantUiRuntimeProvider.tsx @@ -8,15 +8,11 @@ import { useOpenHumanExternalStore } from './useOpenHumanExternalStore'; const debug = debugFactory('openhuman:assistant-ui'); /** - * Mounts assistant-ui's runtime over the existing Redux state, scoped to ONE - * thread. + * Mounts assistant-ui's runtime over one OpenHuman thread. * - * This is additive by design. Nothing below it is required to consume the - * runtime — the transcript, composer and tool timeline still render from Redux - * exactly as before — so mounting it cannot regress a surface that ignores it. - * What it provides is the runtime *context*: any component under it may use - * assistant-ui's hooks and primitives, and the two views of the conversation - * are guaranteed to agree because both read the same store. + * Settled process history (reasoning, tools and sub-agents) is read directly + * from the core transcript RPC, whose Rust projection cache is authoritative. + * Redux only supplies the existing message list and live socket deltas. * * ## Why the thread is a prop * diff --git a/app/src/providers/ChatRuntimeProvider.tsx b/app/src/providers/ChatRuntimeProvider.tsx index 59ee25f279..0565d8879a 100644 --- a/app/src/providers/ChatRuntimeProvider.tsx +++ b/app/src/providers/ChatRuntimeProvider.tsx @@ -40,7 +40,6 @@ import { clearStreamingAssistantForThread, endInferenceTurn, fetchAndHydrateCompletedTurnState, - fetchAndHydrateDerivedTranscript, markInferenceTurnStreaming, parseToolFailure, recordChatTurnUsage, @@ -78,7 +77,7 @@ import { setActiveThread, setSelectedThread, } from '../store/threadSlice'; -import { DERIVED_TRANSCRIPT_ENABLED, IS_PROD } from '../utils/config'; +import { IS_PROD } from '../utils/config'; import { AssistantUiRuntimeProvider } from './AssistantUiRuntimeProvider'; import { isProactiveConversationSurface, proactiveThreadPins } from './proactiveThreadPins'; @@ -481,14 +480,6 @@ const ChatRuntimeProvider = ({ children }: { children: React.ReactNode }) => { // ending the live lifecycle) matters: `hydrateRuntimeFromSnapshot` // intentionally refuses to overwrite an actively streaming turn. await dispatch(fetchAndHydrateCompletedTurnState(event.thread_id)); - // Live-turn seam: the turn just settled and its line was appended to the - // append-only transcript. Invalidate/refresh the thread's derived - // settled-turn trails so the next reopen is fresh. The just-finished turn - // is the newest, so the derived hydration skips it — this never fights the - // live anchor, and it does not touch any socket delta handler. - if (DERIVED_TRANSCRIPT_ENABLED) { - void dispatch(fetchAndHydrateDerivedTranscript(event.thread_id)); - } }; rtLog('subscribe_chat_events', { socket: socketStatus }); diff --git a/app/src/providers/__tests__/AssistantUiRuntimeProvider.test.tsx b/app/src/providers/__tests__/AssistantUiRuntimeProvider.test.tsx index f2f8e8e7dd..6ccc714038 100644 --- a/app/src/providers/__tests__/AssistantUiRuntimeProvider.test.tsx +++ b/app/src/providers/__tests__/AssistantUiRuntimeProvider.test.tsx @@ -9,16 +9,35 @@ */ import { useAui, useAuiState } from '@assistant-ui/react'; import { combineReducers, configureStore } from '@reduxjs/toolkit'; -import { act, render, screen } from '@testing-library/react'; +import { act, render, screen, waitFor } from '@testing-library/react'; import { Provider } from 'react-redux'; import { afterEach, describe, expect, it, vi } from 'vitest'; -import chatRuntimeReducer, { streamDeltaReceived } from '../../store/chatRuntimeSlice'; +import { threadApi } from '../../services/api/threadApi'; +import chatRuntimeReducer, { + beginInferenceTurn, + markInferenceTurnStreaming, + streamDeltaReceived, +} from '../../store/chatRuntimeSlice'; import threadReducer from '../../store/threadSlice'; import type { ThreadMessage } from '../../types/thread'; import { AssistantUiRuntimeProvider } from '../AssistantUiRuntimeProvider'; import { __resetChatSurfaces, registerChatSurface } from '../chatSurfaceHandlers'; +vi.mock('../../services/api/threadApi', () => ({ + threadApi: { + getDerivedTranscript: vi + .fn() + .mockResolvedValue({ + threadId: 't-aui', + items: [], + total: 0, + hasMore: false, + hasTranscript: false, + }), + }, +})); + const THREAD_ID = 't-aui'; function msg(id: string, sender: 'user' | 'agent', content: string): ThreadMessage { @@ -63,6 +82,15 @@ function RuntimeProbe() { .map(m => m.content.map(p => (p.type === 'text' ? p.text : '')).join('')) .join('|')}
+
+ {thread.messages + .flatMap(message => + message.role === 'assistant' + ? message.content.flatMap(part => (part.type === 'tool-call' ? [part.toolName] : [])) + : [] + ) + .join('|')} +
); } @@ -91,12 +119,14 @@ describe('AssistantUiRuntimeProvider', () => { expect(screen.getByTestId('count')).toHaveTextContent('0'); }); - it('surfaces the live stream as a running tail message', () => { + it('surfaces the live stream as a running tail message', async () => { const store = buildStore([msg('a', 'user', 'question')]); renderWith(store); expect(screen.getByTestId('count')).toHaveTextContent('1'); act(() => { + store.dispatch(beginInferenceTurn({ threadId: THREAD_ID })); + store.dispatch(markInferenceTurnStreaming({ threadId: THREAD_ID })); store.dispatch( streamDeltaReceived({ threadId: THREAD_ID, @@ -108,10 +138,39 @@ describe('AssistantUiRuntimeProvider', () => { ); }); - expect(screen.getByTestId('count')).toHaveTextContent('2'); + await waitFor(() => expect(screen.getByTestId('count')).toHaveTextContent('2')); expect(screen.getByTestId('text')).toHaveTextContent('question|partial answer'); }); + it('reads settled reasoning and tools directly from the core transcript RPC', async () => { + vi.mocked(threadApi.getDerivedTranscript).mockResolvedValueOnce({ + threadId: THREAD_ID, + // RPC pages are newest-first: reverse traversal sees the boundary first. + items: [ + { + kind: 'toolCall', + callId: 'call-web', + name: 'web_fetch', + args: { url: 'https://example.com' }, + result: 'Example Domain', + status: 'success', + }, + { kind: 'reasoning', text: 'I should fetch the source.' }, + { kind: 'turnBoundary', requestId: 'req-core' }, + ], + total: 3, + hasMore: false, + hasTranscript: true, + }); + const answer = msg('answer', 'agent', 'done'); + answer.extraMetadata = { requestId: 'req-core' }; + + renderWith(buildStore([msg('question', 'user', 'fetch it'), answer])); + + await waitFor(() => expect(screen.getByTestId('tools')).toHaveTextContent('web_fetch')); + expect(threadApi.getDerivedTranscript).toHaveBeenCalledWith(THREAD_ID, { limit: 500 }); + }); + it('forwards onNew to the surface that owns the thread', async () => { const send = vi.fn(async () => {}); registerChatSurface(THREAD_ID, { send }); diff --git a/app/src/providers/useOpenHumanExternalStore.ts b/app/src/providers/useOpenHumanExternalStore.ts index ac93161d73..aab1a80f00 100644 --- a/app/src/providers/useOpenHumanExternalStore.ts +++ b/app/src/providers/useOpenHumanExternalStore.ts @@ -1,6 +1,8 @@ import type { AppendMessage } from '@assistant-ui/react'; -import { useCallback, useMemo } from 'react'; +import { useCallback, useEffect, useMemo, useState } from 'react'; +import { mapDisplayItems } from '../features/conversations/derived/mapDisplayItems'; +import { threadApi } from '../services/api/threadApi'; import { useAppSelector } from '../store/hooks'; import type { ThreadMessage } from '../types/thread'; import { buildRuntimeMessages } from './assistantUiMessages'; @@ -11,6 +13,69 @@ const EMPTY_TIMELINE: never[] = []; const EMPTY_TRANSCRIPT: never[] = []; const EMPTY_TURN_MAP = {}; +type CoreTranscriptProjection = { + threadId: string | null; + timelines: ReturnType['timelines']; + transcripts: ReturnType['transcripts']; +}; + +const EMPTY_CORE_TRANSCRIPT: CoreTranscriptProjection = { + threadId: null, + timelines: EMPTY_TURN_MAP, + transcripts: EMPTY_TURN_MAP, +}; + +/** + * Read settled process history straight from the core's transcript projection. + * The Rust side owns a bounded, mtime-keyed LRU, so this hook deliberately does + * not establish a second Redux transcript store or duplicate cache policy. + */ +export function useCoreTranscriptProjection( + threadId: string | null, + revision: string, + liveRequestId: string | undefined +): CoreTranscriptProjection { + const [projection, setProjection] = useState(EMPTY_CORE_TRANSCRIPT); + + useEffect(() => { + if (!threadId) { + setProjection(EMPTY_CORE_TRANSCRIPT); + return; + } + // Defensive for narrow test/embedder shims that expose only a subset of + // threadApi. Production builds always provide this method. + if (typeof threadApi.getDerivedTranscript !== 'function') { + setProjection({ threadId, timelines: EMPTY_TURN_MAP, transcripts: EMPTY_TURN_MAP }); + return; + } + let cancelled = false; + void threadApi + .getDerivedTranscript(threadId, { limit: 500 }) + .then(page => { + if (cancelled) return; + if (!page.hasTranscript) { + setProjection({ threadId, timelines: EMPTY_TURN_MAP, transcripts: EMPTY_TURN_MAP }); + return; + } + const skipRequestIds = liveRequestId ? new Set([liveRequestId]) : undefined; + const mapped = mapDisplayItems(page.items, { skipRequestIds }); + setProjection({ threadId, timelines: mapped.timelines, transcripts: mapped.transcripts }); + }) + .catch(() => { + // A missing/older core has no settled process trail; message text and + // the live socket projection remain usable. Navigation must not fail. + if (!cancelled) { + setProjection({ threadId, timelines: EMPTY_TURN_MAP, transcripts: EMPTY_TURN_MAP }); + } + }); + return () => { + cancelled = true; + }; + }, [liveRequestId, revision, threadId]); + + return projection.threadId === threadId ? projection : EMPTY_CORE_TRANSCRIPT; +} + /** Flatten an assistant-ui append payload down to the plain text our core takes. */ function appendMessageText(message: AppendMessage): string { return message.content @@ -22,10 +87,9 @@ function appendMessageText(message: AppendMessage): string { /** * Build the `ExternalStoreAdapter` that backs `useExternalStoreRuntime`. * - * Read side: a pure projection of Redux. Write side: forwarded to the surface - * that owns the thread (see `chatSurfaceHandlers`). The runtime therefore gets - * a faithful, live view of the conversation and a working action API without - * holding any state of its own. + * Settled messages and live deltas remain in their existing UI stores, while + * reasoning/tool/sub-agent history comes directly from the core transcript + * projection. Redux is not a second transcript database. */ export function useOpenHumanExternalStore(threadId: string | null) { const messages = useAppSelector(state => @@ -48,15 +112,11 @@ export function useOpenHumanExternalStore(threadId: string | null) { ? (state.chatRuntime.processingByThread?.[threadId] ?? EMPTY_TRANSCRIPT) : EMPTY_TRANSCRIPT ); - const turnTimelines = useAppSelector(state => - threadId - ? (state.chatRuntime.turnTimelinesByThread?.[threadId] ?? EMPTY_TURN_MAP) - : EMPTY_TURN_MAP - ); - const turnTranscripts = useAppSelector(state => - threadId - ? (state.chatRuntime.turnTranscriptsByThread?.[threadId] ?? EMPTY_TURN_MAP) - : EMPTY_TURN_MAP + const settledRevision = `${messages.at(-1)?.id ?? ''}:${messages.at(-1)?.content.length ?? 0}:${lifecycle ?? ''}`; + const coreTranscript = useCoreTranscriptProjection( + threadId, + settledRevision, + streaming?.requestId ); // `started` and `streaming` are both in-flight. A completed turn can retain @@ -73,10 +133,10 @@ export function useOpenHumanExternalStore(threadId: string | null) { isRunning, liveTimeline, liveTranscript, - turnTimelines, - turnTranscripts, + turnTimelines: coreTranscript.timelines, + turnTranscripts: coreTranscript.transcripts, }), - [messages, streaming, isRunning, liveTimeline, liveTranscript, turnTimelines, turnTranscripts] + [messages, streaming, isRunning, liveTimeline, liveTranscript, coreTranscript] ); const onNew = useCallback( From cb1f26fca650d3f8185afd1e3f5c0359c2d0b600 Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Mon, 31 Aug 2026 21:15:55 +0300 Subject: [PATCH 08/23] refactor(chat): unify tool calls on assistant ui Co-authored-by: Medulla --- .../components/AssistantUiToolCall.tsx | 215 ++++++++++++++++++ .../components/ChatToolParts.test.tsx | 8 +- .../components/ChatToolParts.tsx | 158 +------------ .../components/SubagentActivityBlock.tsx | 199 ++-------------- .../components/SubagentToolCallRow.tsx | 102 ++------- .../__tests__/SubagentDrawer.test.tsx | 32 +-- .../__tests__/ToolTimelineBlock.test.tsx | 42 ++-- .../specs/chat-harness-subagent.spec.ts | 33 +-- 8 files changed, 305 insertions(+), 484 deletions(-) create mode 100644 app/src/features/conversations/components/AssistantUiToolCall.tsx diff --git a/app/src/features/conversations/components/AssistantUiToolCall.tsx b/app/src/features/conversations/components/AssistantUiToolCall.tsx new file mode 100644 index 0000000000..934f820455 --- /dev/null +++ b/app/src/features/conversations/components/AssistantUiToolCall.tsx @@ -0,0 +1,215 @@ +import type { ToolCallMessagePartComponent } from '@assistant-ui/react'; +import { CheckIcon, ChevronDownIcon, CircleXIcon, Loader2Icon, WrenchIcon } from 'lucide-react'; + +import { cn } from '../../../components/assistant-ui/lib/utils'; +import { + Collapsible, + CollapsibleContent, + CollapsibleTrigger, +} from '../../../components/assistant-ui/ui/collapsible'; +import type { + ToolFailureExplanation, + ToolTimelineEntryStatus, +} from '../../../store/chatRuntimeSlice'; +import { formatToolName } from '../../../utils/toolTimelineFormatting'; +import { BubbleMarkdown } from './AgentMessageBubble'; +import { ToolFailureLines } from './ToolFailureLines'; + +function friendlyLabel(key: string): string { + return key + .replace(/([a-z0-9])([A-Z])/g, '$1 $2') + .replace(/[_-]+/g, ' ') + .replace(/^./, char => char.toUpperCase()); +} + +function parsedValue(value: unknown): unknown { + if (typeof value !== 'string') return value; + const trimmed = value.trim(); + if (!(trimmed.startsWith('{') || trimmed.startsWith('['))) return value; + try { + return JSON.parse(trimmed); + } catch { + return value; + } +} + +function hasDisplayValue(value: unknown): boolean { + if (value === undefined || value === null || value === '') return false; + if (Array.isArray(value)) return value.length > 0; + if (typeof value === 'object') return Object.keys(value as object).length > 0; + return true; +} + +function ToolDataView({ value }: { value: unknown }) { + const parsed = parsedValue(value); + if (Array.isArray(parsed)) { + return ( +
    + {parsed.map((item, index) => ( +
  • + +
    • + ))} +
+ ); + } + if (parsed && typeof parsed === 'object') { + const entries = Object.entries(parsed); + for (const key of ['content', 'output', 'result', 'message', 'query', 'q']) { + const semantic = entries.find(([candidate]) => candidate === key)?.[1]; + if (hasDisplayValue(semantic)) return ; + } + return ( +
+ {entries.map(([key, item]) => ( +
+
{friendlyLabel(key)}
+
+ +
+
+ ))} +
+ ); + } + if (typeof parsed === 'boolean') return {parsed ? 'Yes' : 'No'}; + if (typeof parsed === 'string') return ; + return {String(parsed ?? '')}; +} + +function inferredToolLabel(toolName: string, running: boolean, args: unknown, result: unknown) { + const lowerName = toolName.toLowerCase(); + const parsedArgs = parsedValue(args); + const argKeys = + parsedArgs && typeof parsedArgs === 'object' && !Array.isArray(parsedArgs) + ? Object.keys(parsedArgs as object).map(key => key.toLowerCase()) + : []; + const renderedResult = typeof result === 'string' ? result : JSON.stringify(result ?? ''); + const looksLikeSearch = + lowerName.includes('search') || + argKeys.some(key => ['query', 'q', 'search_query'].includes(key)) || + /(?:^|\n)#?\s*search results\b/i.test(renderedResult); + const looksLikeFetch = + lowerName.includes('fetch') || + argKeys.some(key => ['url', 'uri'].includes(key)) || + /\bstatus=\d{3}\s+url=/i.test(renderedResult); + if (looksLikeSearch) return running ? 'Searching the web' : 'Searched the web'; + if (looksLikeFetch) return running ? 'Fetching from the web' : 'Fetched from the web'; + return formatToolName(toolName); +} + +export interface AssistantUiToolCallCardProps { + toolName: string; + args?: unknown; + argsText?: string; + result?: unknown; + status?: ToolTimelineEntryStatus; + displayName?: string; + detail?: string; + elapsedMs?: number; + failure?: ToolFailureExplanation; +} + +/** The single assistant-ui tool-call presentation used at every nesting level. */ +export function AssistantUiToolCallCard({ + toolName, + args, + argsText, + result, + status, + displayName, + detail, + elapsedMs, + failure, +}: AssistantUiToolCallCardProps) { + const running = status + ? status === 'running' || status === 'awaiting_user' + : result === undefined; + const input = hasDisplayValue(args) ? args : parsedValue(argsText ?? ''); + const output = result === '' && status && !running ? 'No output' : parsedValue(result); + const suppliedLabel = displayName?.trim(); + const label = + suppliedLabel && suppliedLabel.toLowerCase() !== 'tool' + ? suppliedLabel + : inferredToolLabel(toolName, running, args, result); + const statusLabel = + status === 'error' + ? 'failed' + : status === 'cancelled' + ? 'cancelled' + : status === 'awaiting_user' + ? 'awaiting input' + : running + ? 'running' + : 'done'; + const failed = status === 'error'; + + return ( + + + + {label} + {detail ? ( + + {detail} + + ) : null} + + {running ? ( + + ) : failed ? ( + + ) : ( + + )} + {statusLabel} + {elapsedMs != null && !running ? ( + + {elapsedMs >= 1000 ? `${(elapsedMs / 1000).toFixed(1)}s` : `${elapsedMs}ms`} + + ) : null} + + + + {failed && failure ? ( +
+ +
+ ) : null} + + {hasDisplayValue(input) ? ( +
+

Input

+
+ +
+
+ ) : null} + {hasDisplayValue(output) ? ( +
+

Output

+
+ +
+
+ ) : null} + + + ); +} + +export const OpenHumanToolCall: ToolCallMessagePartComponent = props => ( + +); diff --git a/app/src/features/conversations/components/ChatToolParts.test.tsx b/app/src/features/conversations/components/ChatToolParts.test.tsx index 4be0fe60b7..2149e2c218 100644 --- a/app/src/features/conversations/components/ChatToolParts.test.tsx +++ b/app/src/features/conversations/components/ChatToolParts.test.tsx @@ -14,7 +14,7 @@ const activity: SubagentActivity = { }; describe('ChatToolParts', () => { - it('renders a delegation with progress args and no result as running', () => { + it('renders a running delegation collapsed by default', async () => { render( { expect(screen.getByText('running')).toBeInTheDocument(); expect(screen.getByText('Researcher')).toBeInTheDocument(); + expect(screen.queryByText('Checking primary sources.')).not.toBeInTheDocument(); + expect(screen.getByTestId('assistant-ui-subagent-call')).toHaveAttribute( + 'data-state', + 'closed' + ); + await userEvent.click(screen.getByRole('button', { name: /Delegated to Researcher/i })); expect(screen.getByText('Checking primary sources.')).toBeInTheDocument(); }); diff --git a/app/src/features/conversations/components/ChatToolParts.tsx b/app/src/features/conversations/components/ChatToolParts.tsx index cb6162a6c1..f2d8c59a3a 100644 --- a/app/src/features/conversations/components/ChatToolParts.tsx +++ b/app/src/features/conversations/components/ChatToolParts.tsx @@ -1,5 +1,5 @@ import type { ToolCallMessagePartComponent } from '@assistant-ui/react'; -import { CheckIcon, ChevronDownIcon, Loader2Icon, WrenchIcon, WorkflowIcon } from 'lucide-react'; +import { CheckIcon, ChevronDownIcon, Loader2Icon, WorkflowIcon } from 'lucide-react'; import type { FC, PropsWithChildren } from 'react'; import { cn } from '../../../components/assistant-ui/lib/utils'; @@ -15,8 +15,7 @@ import { CollapsibleTrigger, } from '../../../components/assistant-ui/ui/collapsible'; import type { SubagentActivity } from '../../../store/chatRuntimeSlice'; -import { formatToolName } from '../../../utils/toolTimelineFormatting'; -import { BubbleMarkdown } from './AgentMessageBubble'; +import { OpenHumanToolCall } from './AssistantUiToolCall'; import { SubagentActivityBlock } from './SubagentActivityBlock'; function asSubagentActivity(value: unknown): SubagentActivity | undefined { @@ -57,9 +56,9 @@ export const SubagentCall: ToolCallMessagePartComponent = ({ args, result }) => return ( ); }; -function friendlyLabel(key: string): string { - return key - .replace(/([a-z0-9])([A-Z])/g, '$1 $2') - .replace(/[_-]+/g, ' ') - .replace(/^./, char => char.toUpperCase()); -} - -function toolDisplayName( - toolName: string, - running: boolean, - args: unknown, - result: unknown -): string { - const lowerName = toolName.toLowerCase(); - const parsedArgs = parsedValue(args); - const argKeys = - parsedArgs && typeof parsedArgs === 'object' && !Array.isArray(parsedArgs) - ? Object.keys(parsedArgs as object).map(key => key.toLowerCase()) - : []; - const renderedResult = typeof result === 'string' ? result : JSON.stringify(result ?? ''); - const looksLikeSearch = - lowerName.includes('search') || - argKeys.some(key => ['query', 'q', 'search_query'].includes(key)) || - /(?:^|\n)#?\s*search results\b/i.test(renderedResult); - const looksLikeFetch = - lowerName.includes('fetch') || - argKeys.some(key => ['url', 'uri'].includes(key)) || - /\bstatus=\d{3}\s+url=/i.test(renderedResult); - if (looksLikeSearch) { - return running ? 'Searching the web' : 'Searched the web'; - } - if (looksLikeFetch) return running ? 'Fetching from the web' : 'Fetched from the web'; - return formatToolName(toolName); -} - -function parsedValue(value: unknown): unknown { - if (typeof value !== 'string') return value; - const trimmed = value.trim(); - if (!(trimmed.startsWith('{') || trimmed.startsWith('['))) return value; - try { - return JSON.parse(trimmed); - } catch { - return value; - } -} - -function hasDisplayValue(value: unknown): boolean { - if (value === undefined || value === null || value === '') return false; - if (Array.isArray(value)) return value.length > 0; - if (typeof value === 'object') return Object.keys(value as object).length > 0; - return true; -} - -function ToolDataView({ value }: { value: unknown }) { - const parsed = parsedValue(value); - if (Array.isArray(parsed)) { - return ( -
    - {parsed.map((item, index) => ( -
  • - -
    • - ))} -
- ); - } - if (parsed && typeof parsed === 'object') { - const entries = Object.entries(parsed); - // Tool wrappers frequently add bookkeeping beside the actual payload - // (`tool_call_id`, success, timing). When a semantic output field exists, - // show that value directly and hide the wrapper entirely. - for (const key of ['content', 'output', 'result', 'message', 'query', 'q']) { - const semantic = entries.find(([candidate]) => candidate === key)?.[1]; - if (hasDisplayValue(semantic)) return ; - } - return ( -
- {entries.map(([key, item]) => ( -
-
{friendlyLabel(key)}
-
- -
-
- ))} -
- ); - } - if (typeof parsed === 'boolean') return {parsed ? 'Yes' : 'No'}; - if (typeof parsed === 'string') return ; - return {String(parsed ?? '')}; -} - -/** Rich assistant-ui-native renderer for an ordinary OpenHuman tool call. */ -export const OpenHumanToolCall: ToolCallMessagePartComponent = ({ - toolName, - args, - argsText, - result, -}) => { - const running = result === undefined; - const input = hasDisplayValue(args) ? args : parsedValue(argsText ?? ''); - const output = parsedValue(result); - return ( - - - - - {toolDisplayName(toolName, running, args, result)} - - {running ? ( - - - running - - ) : ( - - done - - )} - - - - {hasDisplayValue(input) ? ( -
-

Input

-
- -
-
- ) : null} - {hasDisplayValue(output) ? ( -
-

Output

-
- -
-
- ) : null} - - - ); -}; - /** Route every call through an assistant-ui-native rich renderer. */ export const ChatToolFallback: ToolCallMessagePartComponent = props => props.toolName === 'task' ? : ; diff --git a/app/src/features/conversations/components/SubagentActivityBlock.tsx b/app/src/features/conversations/components/SubagentActivityBlock.tsx index 6bc5bc6701..fa9cfc5463 100644 --- a/app/src/features/conversations/components/SubagentActivityBlock.tsx +++ b/app/src/features/conversations/components/SubagentActivityBlock.tsx @@ -1,191 +1,30 @@ -import Badge, { type BadgeVariant } from '../../../components/ui/Badge'; +import Badge from '../../../components/ui/Badge'; import WorktreeActions from '../../../components/worktree/WorktreeActions'; import { useT } from '../../../lib/i18n/I18nContext'; import type { SubagentActivity, - ToolFailureExplanation, - ToolTimelineEntryStatus, + SubagentToolCallEntry, + SubagentTranscriptItem, } from '../../../store/chatRuntimeSlice'; import { basename } from '../../../utils/pathUtils'; -import { formatToolName, stripToolCallEnvelopes } from '../../../utils/toolTimelineFormatting'; +import { stripToolCallEnvelopes } from '../../../utils/toolTimelineFormatting'; import { BubbleMarkdown } from './AgentMessageBubble'; -import { ToolFailureLines } from './ToolFailureLines'; +import { AssistantUiToolCallCard } from './AssistantUiToolCall'; -/** - * Map a child tool-call status to the shared {@link Badge} variant. The label - * itself comes from `useStatusTagLabel` below; keeping the two beside each - * other is what stops a status reading "Failed" in a success-toned pill. - */ -function statusTagVariant(status: ToolTimelineEntryStatus): BadgeVariant { - switch (status) { - case 'error': - return 'danger'; - case 'running': - case 'awaiting_user': - return 'warning'; - case 'cancelled': - return 'neutral'; - default: - return 'success'; - } -} - -/** Tone classes for a child tool-call row's bullet, keyed by lifecycle status. */ -function toolCallTone(status: ToolTimelineEntryStatus): string { - if (status === 'running') return 'text-amber-700 dark:text-amber-300'; - if (status === 'success') return 'text-sage-700 dark:text-sage-300'; - return 'text-coral-700 dark:text-coral-300'; -} - -function subagentToolLabel( - name: string, - status: ToolTimelineEntryStatus, - args: unknown, - result: unknown -): string { - const running = status === 'running'; - const lowerName = name.toLowerCase(); - const argKeys = - args && typeof args === 'object' && !Array.isArray(args) - ? Object.keys(args as object).map(key => key.toLowerCase()) - : []; - const output = readableToolOutput(result) ?? ''; - const looksLikeSearch = - lowerName.includes('search') || - argKeys.some(key => ['query', 'q', 'search_query'].includes(key)) || - /(?:^|\n)#?\s*search results\b/i.test(output); - const looksLikeFetch = - lowerName.includes('fetch') || - argKeys.some(key => ['url', 'uri'].includes(key)) || - /\bstatus=\d{3}\s+url=/i.test(output); - if (looksLikeSearch) { - return running ? 'Searching the web' : 'Searched the web'; - } - if (looksLikeFetch) return running ? 'Fetching from the web' : 'Fetched from the web'; - return formatToolName(name); -} - -function readableToolOutput(value: unknown): string | null { - if (value === undefined || value === null) return null; - if (typeof value === 'string') { - const trimmed = value.trim(); - if (!trimmed) return null; - try { - return readableToolOutput(JSON.parse(trimmed)) ?? trimmed; - } catch { - return trimmed; - } - } - if (typeof value === 'object') { - const object = value as Record; - for (const key of ['content', 'output', 'message', 'result']) { - if (typeof object[key] === 'string' && object[key].trim()) return object[key].trim(); - } - return Object.entries(object) - .map(([key, item]) => `- **${key.replace(/[_-]+/g, ' ')}:** ${String(item)}`) - .join('\n'); - } - return String(value); -} +type ChildToolCall = SubagentToolCallEntry | Extract; -/** - * Status pill for a tool-call row — a tinted "Done" / "Failed" / "Running" - * tag instead of a bare ✓/✕ glyph, so the outcome reads at a glance. Built on - * the shared {@link Badge} primitive, which publishes the tone as - * `data-variant` for tests to assert instead of a Tailwind class string. - */ -export function StatusTag({ status }: { status: ToolTimelineEntryStatus }) { - const { t } = useT(); - const label = - status === 'error' - ? t('conversations.agentTaskInsights.failed') - : status === 'running' - ? t('conversations.agentTaskInsights.running') - : status === 'cancelled' - ? t('conversations.agentTaskInsights.cancelled') - : status === 'awaiting_user' - ? t('conversations.agentTaskInsights.awaitingUser') - : t('conversations.agentTaskInsights.done'); +function ChildToolCallCard({ call }: { call: ChildToolCall }) { return ( - - {label} - - ); -} - -/** - * One child tool-call row in a sub-agent's inline activity. Shared by the - * ordered transcript (interleaved with {@link ThoughtBlock}) and the flat - * `toolCalls` fallback, so the row markup lives in exactly one place. - */ -export function ToolCallRow({ - call, -}: { - call: { - callId: string; - toolName: string; - status: ToolTimelineEntryStatus; - elapsedMs?: number; - iteration?: number; - /** Server-computed human label; preferred over the client formatter. */ - displayName?: string; - /** Server-computed contextual detail (path / recipient / query). */ - detail?: string; - /** Structured why/next explanation for a FAILED child tool call (#4459). */ - failure?: ToolFailureExplanation; - /** Arguments supplied to the child tool, used for descriptive fallback labels. */ - args?: unknown; - /** Child tool output, rendered as Markdown when present. */ - result?: unknown; - }; -}) { - const output = readableToolOutput(call.result); - const suppliedLabel = call.displayName?.trim(); - const label = - suppliedLabel && suppliedLabel.toLowerCase() !== 'tool' - ? suppliedLabel - : subagentToolLabel(call.toolName, call.status, call.args, call.result); - return ( -
-
- - • - - - {label} - - {/* The contextual arg (path / recipient / query) can be long, so it - truncates to a single line and absorbs the row's spare width — the - full value stays available on hover — instead of wrapping into a - multi-line box that knocks the name and status out of alignment. */} - {call.detail ? ( - - {call.detail} - - ) : null} - {/* Status reads as a tinted "Done" / "Failed" / "Running" tag. */} - - - - {call.elapsedMs != null && call.status !== 'running' ? ( - - {call.elapsedMs >= 1000 - ? `${(call.elapsedMs / 1000).toFixed(1)}s` - : `${call.elapsedMs}ms`} - - ) : null} -
- {call.status === 'error' && call.failure ? : null} - {output ? ( -
- -
- ) : null} -
+ ); } @@ -289,7 +128,7 @@ export function SubagentActivityBlock({
{transcript.map((item, i) => item.kind === 'tool' ? ( - + ) : ( ) @@ -298,7 +137,7 @@ export function SubagentActivityBlock({ ) : subagent.toolCalls.length > 0 ? (
{subagent.toolCalls.map(call => ( - + ))}
) : null} diff --git a/app/src/features/conversations/components/SubagentToolCallRow.tsx b/app/src/features/conversations/components/SubagentToolCallRow.tsx index a1da25ec8b..c2c030eefb 100644 --- a/app/src/features/conversations/components/SubagentToolCallRow.tsx +++ b/app/src/features/conversations/components/SubagentToolCallRow.tsx @@ -1,20 +1,10 @@ -import createDebug from 'debug'; -import { useState } from 'react'; - import Badge, { type BadgeVariant } from '../../../components/ui/Badge'; -import { - CollapsibleContent, - CollapsibleRoot, - CollapsibleTrigger, -} from '../../../components/ui/Collapsible'; import { useT } from '../../../lib/i18n/I18nContext'; import type { SubagentTranscriptItem, ToolTimelineEntryStatus, } from '../../../store/chatRuntimeSlice'; -import { ToolFailureLines } from './ToolFailureLines'; - -const log = createDebug('app:conversations:subagent-tool-call'); +import { AssistantUiToolCallCard } from './AssistantUiToolCall'; /** Human-readable elapsed time for a sub-agent run or one of its tool calls. */ export function formatElapsed(ms: number): string { @@ -77,87 +67,21 @@ export function formatArgs(args: unknown): string | null { } } -const DETAIL_PRE = - 'max-h-60 overflow-auto whitespace-pre-wrap wrap-break-word rounded bg-surface px-2 py-1.5 ' + - 'font-mono text-[11px] leading-relaxed text-content-secondary'; -const DETAIL_LABEL = 'mb-1 text-[10px] font-semibold uppercase tracking-wide text-content-faint'; - /** - * One child tool call in the drawer transcript, expandable to reveal exactly - * *what happened*: the input arguments the sub-agent passed and the raw output - * the tool returned. Collapsed by default to keep the transcript scannable; - * the disclosure is only enabled once there's detail to reveal (args present, - * or the call completed with a captured result). Reopened-from-memory - * transcripts carry no args/result, so those rows stay non-expandable. - * - * The disclosure is the shared Radix {@link CollapsibleRoot} rather than a - * hand-rolled `useState` + conditional render, so the trigger carries real - * `aria-expanded` / `aria-controls` wiring instead of an ad-hoc attribute. + * Drawer child calls use the exact assistant-ui card used by parent and inline + * tools. The drawer no longer owns a parallel tool-call component hierarchy. */ export function SubagentToolCallRow({ item }: { item: SubagentToolItem }) { - const { t } = useT(); - const [expanded, setExpanded] = useState(false); - - const statusLabel = useSubagentStatusLabel(item.status); - const argsText = formatArgs(item.args); - const hasOutput = item.result != null; - const expandable = argsText != null || hasOutput; - return ( - { - log('tool-call %s expanded=%s', item.toolName, next); - setExpanded(next); - }} - className="rounded-md border border-line bg-surface-muted text-xs" - data-testid="subagent-drawer-tool-call"> - - {expandable ? ( - - {expanded ? '▾' : '▸'} - - ) : ( - - )} - 🔧 - {item.toolName} - - {statusLabel} - - {item.elapsedMs != null && item.status !== 'running' ? ( - - {formatElapsed(item.elapsedMs)} - - ) : null} - - {item.status === 'error' && item.failure ? ( -
- -
- ) : null} - - {argsText != null ? ( -
-
{t('conversations.subagent.input')}
- 
{argsText}
-
- ) : null} - {hasOutput ? ( -
-
{t('conversations.subagent.output')}
- 
-              {item.result && item.result.length > 0
-                ? item.result
-                : t('conversations.subagent.noOutput')}
-
-
- ) : null} - - + ); } diff --git a/app/src/features/conversations/components/__tests__/SubagentDrawer.test.tsx b/app/src/features/conversations/components/__tests__/SubagentDrawer.test.tsx index e74f9a25ac..f885e4bdbb 100644 --- a/app/src/features/conversations/components/__tests__/SubagentDrawer.test.tsx +++ b/app/src/features/conversations/components/__tests__/SubagentDrawer.test.tsx @@ -1,4 +1,4 @@ -import { render, screen, waitFor } from '@testing-library/react'; +import { render, screen, waitFor, within } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { describe, expect, it, vi } from 'vitest'; @@ -49,7 +49,7 @@ describe('SubagentDrawer', () => { // thinking → text → tool → text — i.e. the tool sits between the two // text blocks, not in a separate section. const thinking = screen.getByTestId('subagent-transcript-thinking'); - const tool = screen.getByTestId('subagent-drawer-tool-call'); + const tool = screen.getByTestId('assistant-ui-tool-call'); const texts = screen.getAllByTestId('subagent-transcript-text'); expect(texts).toHaveLength(2); @@ -60,7 +60,7 @@ describe('SubagentDrawer', () => { expect(order(tool)).toBeLessThan(order(texts[1])); expect(thinking.textContent).toContain('comparing the two sources'); - expect(tool.textContent).toContain('web_search'); + expect(tool.textContent).toContain('Searched the web'); expect(tool.textContent).toContain('1.2s'); expect(texts[1].textContent).toContain('The answer is'); }); @@ -178,7 +178,7 @@ describe('SubagentDrawer', () => { await waitFor(() => expect(screen.getByTestId('subagent-parent-prompt').textContent).toContain('Research Q3') ); - expect(screen.getByTestId('subagent-drawer-tool-call').textContent).toContain('web_search'); + expect(screen.getByTestId('assistant-ui-tool-call').textContent).toContain('Searched the web'); expect(screen.getByTestId('subagent-transcript-text').textContent).toContain( 'Revenue grew 18%' ); @@ -292,15 +292,15 @@ describe('SubagentDrawer', () => { ); // Collapsed by default — neither input nor output is rendered yet. - expect(screen.queryByTestId('subagent-tool-call-input')).toBeNull(); - expect(screen.queryByTestId('subagent-tool-call-output')).toBeNull(); + expect(screen.queryByTestId('assistant-ui-tool-input')).toBeNull(); + expect(screen.queryByTestId('assistant-ui-tool-output')).toBeNull(); - await userEvent.click(screen.getByTestId('subagent-tool-call-toggle')); + await userEvent.click(within(screen.getByTestId('assistant-ui-tool-call')).getByRole('button')); - expect(screen.getByTestId('subagent-tool-call-input').textContent).toContain( + expect(screen.getByTestId('assistant-ui-tool-input').textContent).toContain( 'Q3 revenue drivers' ); - expect(screen.getByTestId('subagent-tool-call-output').textContent).toContain( + expect(screen.getByTestId('assistant-ui-tool-output').textContent).toContain( 'Found 3 results about revenue.' ); }); @@ -312,8 +312,8 @@ describe('SubagentDrawer', () => { render( {}} /> ); - await userEvent.click(screen.getByTestId('subagent-tool-call-toggle')); - expect(screen.getByTestId('subagent-tool-call-output').textContent?.toLowerCase()).toContain( + await userEvent.click(within(screen.getByTestId('assistant-ui-tool-call')).getByRole('button')); + expect(screen.getByTestId('assistant-ui-tool-output').textContent?.toLowerCase()).toContain( 'no output' ); }); @@ -326,7 +326,7 @@ describe('SubagentDrawer', () => { render( {}} /> ); - const rows = screen.getAllByTestId('subagent-drawer-tool-call'); + const rows = screen.getAllByTestId('assistant-ui-tool-call'); expect(rows[0].textContent?.toLowerCase()).toContain('cancelled'); expect(rows[0].textContent?.toLowerCase()).not.toContain('failed'); expect(rows[1].textContent?.toLowerCase()).toContain('awaiting'); @@ -339,9 +339,9 @@ describe('SubagentDrawer', () => { render( {}} /> ); - const toggle = screen.getByTestId('subagent-tool-call-toggle') as HTMLButtonElement; - expect(toggle.disabled).toBe(true); - expect(screen.queryByTestId('subagent-tool-call-input')).toBeNull(); - expect(screen.queryByTestId('subagent-tool-call-output')).toBeNull(); + const toggle = within(screen.getByTestId('assistant-ui-tool-call')).getByRole('button'); + expect(toggle).toHaveAttribute('aria-expanded', 'false'); + expect(screen.queryByTestId('assistant-ui-tool-input')).toBeNull(); + expect(screen.queryByTestId('assistant-ui-tool-output')).toBeNull(); }); }); diff --git a/app/src/features/conversations/components/__tests__/ToolTimelineBlock.test.tsx b/app/src/features/conversations/components/__tests__/ToolTimelineBlock.test.tsx index 98cfe6e4a2..0518f6514a 100644 --- a/app/src/features/conversations/components/__tests__/ToolTimelineBlock.test.tsx +++ b/app/src/features/conversations/components/__tests__/ToolTimelineBlock.test.tsx @@ -78,22 +78,22 @@ describe('SubagentActivityBlock', () => { }} /> ); - const calls = screen.getAllByTestId('subagent-tool-call'); + const calls = screen.getAllByTestId('assistant-ui-tool-call'); expect(calls).toHaveLength(3); // Human labels + timing, with status as a tinted "Done" / "Failed" / // "Running" tag instead of a bare ✓/✕ glyph or the raw lowercase word. expect(calls[0].textContent).toContain('Searched the web'); - expect(calls[0].textContent).toContain('Done'); + expect(calls[0].textContent?.toLowerCase()).toContain('done'); expect(calls[0].textContent).toContain('312ms'); expect(calls[1].textContent).toContain('Composio Execute'); - expect(calls[1].textContent).toContain('Running'); + expect(calls[1].textContent?.toLowerCase()).toContain('running'); expect(calls[1].textContent).not.toContain('·t2'); expect(calls[2].textContent).toContain('Reading file'); - expect(calls[2].textContent).toContain('Failed'); + expect(calls[2].textContent?.toLowerCase()).toContain('failed'); expect(calls[2].textContent).toContain('50ms'); }); - it('renders subagent web output as Markdown instead of raw JSON', () => { + it('renders subagent web output as Markdown instead of raw JSON', async () => { renderInStore( { ); expect(screen.getByText('Searched the web')).toBeInTheDocument(); - expect(screen.getByTestId('subagent-tool-output')).toHaveTextContent('Formal Conjectures'); + const call = screen.getByTestId('assistant-ui-tool-call'); + await userEvent.click(within(call).getByRole('button')); + expect(screen.getByTestId('assistant-ui-tool-output')).toHaveTextContent('Formal Conjectures'); expect(screen.getByRole('strong')).toHaveTextContent('Formal Conjectures'); expect(screen.queryByText(/"content"/)).not.toBeInTheDocument(); }); @@ -138,7 +140,7 @@ describe('SubagentActivityBlock', () => { /> ); - expect(screen.getByTestId('subagent-tool-call')).toHaveTextContent('Searched the web'); + expect(screen.getByTestId('assistant-ui-tool-call')).toHaveTextContent('Searched the web'); }); it('labels cancelled / awaiting-user calls distinctly (not the green "Done" pill)', () => { @@ -154,13 +156,13 @@ describe('SubagentActivityBlock', () => { }} /> ); - const calls = screen.getAllByTestId('subagent-tool-call'); + const calls = screen.getAllByTestId('assistant-ui-tool-call'); expect(calls).toHaveLength(2); // A cancelled / awaiting-user call must NOT read as a successful "Done" step. - expect(calls[0].textContent).toContain('Cancelled'); - expect(calls[0].textContent).not.toContain('Done'); - expect(calls[1].textContent).toContain('Awaiting input'); - expect(calls[1].textContent).not.toContain('Done'); + expect(calls[0].textContent?.toLowerCase()).toContain('cancelled'); + expect(calls[0].textContent?.toLowerCase()).not.toContain('done'); + expect(calls[1].textContent?.toLowerCase()).toContain('awaiting input'); + expect(calls[1].textContent?.toLowerCase()).not.toContain('done'); }); it('prefers the server-supplied label + contextual detail for a child tool call', () => { @@ -181,7 +183,7 @@ describe('SubagentActivityBlock', () => { }} /> ); - const row = screen.getByTestId('subagent-tool-call'); + const row = screen.getByTestId('assistant-ui-tool-call'); expect(row.textContent).toContain('Reading messages'); expect(row.textContent).toContain('steven@gmail.com'); // Never the raw snake_case slug. @@ -230,7 +232,7 @@ describe('SubagentActivityBlock', () => { // Order is preserved: thought → tool → thought. expect(rows[0]).toHaveAttribute('data-testid', 'subagent-thought'); expect(rows[0].textContent).toContain('I should search the web first'); - expect(rows[1]).toHaveAttribute('data-testid', 'subagent-tool-call'); + expect(rows[1]).toHaveAttribute('data-testid', 'assistant-ui-tool-call'); expect(rows[1].textContent).toContain('Searched the web'); expect(rows[2]).toHaveAttribute('data-testid', 'subagent-thought'); expect(rows[2].textContent).toContain('Found three relevant results'); @@ -908,7 +910,7 @@ describe('ToolTimelineBlock — coalescing repeated rows', () => { }); describe('ToolTimelineBlock — subagent rendering', () => { - it('expands a subagent row even without prompt detail and shows child tool calls', () => { + it('shows child tool calls after the collapsed subagent row is opened', () => { const entry: ToolTimelineEntry = { id: 'tid:subagent:sub-1:researcher', name: 'subagent:researcher', @@ -926,7 +928,7 @@ describe('ToolTimelineBlock — subagent rendering', () => { }; renderInStore(); - const calls = screen.getAllByTestId('subagent-tool-call'); + const calls = screen.getAllByTestId('assistant-ui-tool-call'); expect(calls).toHaveLength(1); expect(calls[0].textContent).toContain('Searching the web'); expect(screen.getByTestId('subagent-activity').textContent).toContain('turn 1/5'); @@ -1396,19 +1398,19 @@ describe('ToolTimelineBlock — sub-agent activity survives the transcript path' expect(screen.getByTestId('processing-transcript')).toBeInTheDocument(); // …and the nested child run is present, not collapsed to one line. expect(screen.getByTestId('processing-subagent')).toBeInTheDocument(); - const calls = screen.getAllByTestId('subagent-tool-call'); + const calls = screen.getAllByTestId('assistant-ui-tool-call'); expect(calls).toHaveLength(2); expect(calls[0].textContent).toContain('Searched the web'); - expect(calls[0].textContent).toContain('Done'); + expect(calls[0].textContent?.toLowerCase()).toContain('done'); // Human label, not the raw `web_fetch` slug. expect(calls[1].textContent).toContain('Fetching'); - expect(calls[1].textContent).toContain('Running'); + expect(calls[1].textContent?.toLowerCase()).toContain('running'); }); it('still renders child tool calls on the legacy row path (no transcript)', () => { renderInStore(); expect(screen.queryByTestId('processing-transcript')).toBeNull(); - expect(screen.getAllByTestId('subagent-tool-call')).toHaveLength(2); + expect(screen.getAllByTestId('assistant-ui-tool-call')).toHaveLength(2); }); // The nested child run must live INSIDE the windowed viewport, and must not diff --git a/app/test/playwright/specs/chat-harness-subagent.spec.ts b/app/test/playwright/specs/chat-harness-subagent.spec.ts index 3f02fbe1da..0655ee6720 100644 --- a/app/test/playwright/specs/chat-harness-subagent.spec.ts +++ b/app/test/playwright/specs/chat-harness-subagent.spec.ts @@ -39,11 +39,7 @@ const KEYWORD_RESPONSES = [ // contract, so the latest child user message is the rendered handoff, not // the raw tool argument. keyword: 'Run this task without requiring attention from the parent or user', - streamScript: [ - { thinking: CHILD_THINKING }, - { text: RESEARCHER_REPLY }, - { finish: 'stop' }, - ], + streamScript: [{ thinking: CHILD_THINKING }, { text: RESEARCHER_REPLY }, { finish: 'stop' }], }, { // Detached completion is delivered to the parent as a fresh background @@ -246,10 +242,7 @@ async function diagnosticsSnapshot(page: Page): Promise { chatRuntime?: { inferenceStatusByThread?: Record; toolTimelineByThread?: Record>; - turnTranscriptsByThread?: Record< - string, - Record> - >; + turnTranscriptsByThread?: Record>>; }; thread?: { messagesByThread?: Record>; @@ -366,7 +359,8 @@ test.describe('Chat Harness - Subagent', () => { const finalMessage = page.getByTestId('agent-message').filter({ hasText: CANARY_FINAL }).last(); await expect(finalMessage).toBeVisible({ timeout: 15_000 }); const finalReasoning = finalMessage.getByRole('button', { name: /Reasoning/ }); - if ((await finalReasoning.getAttribute('aria-expanded')) !== 'true') await finalReasoning.click(); + if ((await finalReasoning.getAttribute('aria-expanded')) !== 'true') + await finalReasoning.click(); await expect(finalMessage.getByText(FINAL_THINKING, { exact: true })).toBeVisible(); // Reloading removes the live socket and Redux stream. The same visual @@ -383,25 +377,18 @@ test.describe('Chat Harness - Subagent', () => { limit: 500, }); expect(JSON.stringify(derived)).toContain(PARENT_THINKING); - await expect - .poll( - async () => - JSON.stringify((await diagnosticsSnapshot(page)).runtime.turnTranscriptTexts), - { timeout: 20_000 } - ) - .toContain(PARENT_THINKING); const restoredMessage = page .getByTestId('agent-message') .filter({ has: page.getByTestId('assistant-ui-subagent-call') }) .last(); await expect(restoredMessage).toBeVisible({ timeout: 20_000 }); - const restoredReasoning = restoredMessage.getByRole('button', { name: /Reasoning/ }).first(); - if ((await restoredReasoning.getAttribute('aria-expanded')) !== 'true') { - await restoredReasoning.click(); - } - await expect(restoredMessage.getByText(PARENT_THINKING, { exact: true })).toBeVisible(); - const subagentCall = restoredMessage.getByTestId('assistant-ui-subagent-call'); + const subagentCall = page.getByTestId('assistant-ui-subagent-call').first(); await expect(subagentCall).toBeVisible(); + const subagentTrigger = subagentCall.getByRole('button').first(); + await expect(subagentTrigger).toHaveAttribute('aria-expanded', 'false'); + await expect(subagentCall.getByTestId('subagent-activity')).toHaveCount(0); + await subagentTrigger.click(); + await expect(subagentTrigger).toHaveAttribute('aria-expanded', 'true'); await expect(subagentCall.getByTestId('subagent-activity')).toContainText(CHILD_THINKING); await expect(subagentCall.getByTestId('subagent-activity')).toContainText(RESEARCHER_REPLY); }); From 579af5d9e78bf36f5076fed69c17f5c182e8f69c Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Mon, 31 Aug 2026 21:30:25 +0300 Subject: [PATCH 09/23] refactor(chat): remove legacy subagent renderers Co-authored-by: Medulla --- app/src/components/ai-elements/index.ts | 2 +- .../components/AgentProcessSourcePanel.tsx | 8 +- ...yBlock.tsx => AssistantUiSubagentCall.tsx} | 116 ++++++++++-------- .../components/ChatToolParts.tsx | 57 ++------- .../components/PastTurnInsights.tsx | 5 +- .../components/ProcessingTranscriptView.tsx | 4 +- .../components/SubagentDrawer.tsx | 40 ++++-- .../components/SubagentToolCallRow.tsx | 87 ------------- .../components/ToolTimelineBlock.tsx | 19 ++- .../__tests__/ToolTimelineBlock.test.tsx | 37 +++++- .../conversations/derived/mapDisplayItems.ts | 4 +- app/src/types/derivedTranscript.ts | 2 +- 12 files changed, 160 insertions(+), 221 deletions(-) rename app/src/features/conversations/components/{SubagentActivityBlock.tsx => AssistantUiSubagentCall.tsx} (62%) delete mode 100644 app/src/features/conversations/components/SubagentToolCallRow.tsx diff --git a/app/src/components/ai-elements/index.ts b/app/src/components/ai-elements/index.ts index 2a1f9ce6a6..c326bc0b89 100644 --- a/app/src/components/ai-elements/index.ts +++ b/app/src/components/ai-elements/index.ts @@ -17,7 +17,7 @@ * this product's real transcript surfaces and deleted rather than force-fitted: * * - `Reasoning` / `ChainOfThought` — this product renders the agent's thinking - * INLINE at the position it streamed (`ToolTimelineBlock`'s `ThoughtBlock`, + * INLINE at the position it streamed (the assistant-ui delegation transcript, * explicitly "no heading, no collapse"; `ProcessingTranscriptView`'s * interleaved narration). Both upstream components are whole-panel * collapsibles that hide that trail behind one "Thought for N seconds" diff --git a/app/src/features/conversations/components/AgentProcessSourcePanel.tsx b/app/src/features/conversations/components/AgentProcessSourcePanel.tsx index 4c5c94e5ff..326ba82287 100644 --- a/app/src/features/conversations/components/AgentProcessSourcePanel.tsx +++ b/app/src/features/conversations/components/AgentProcessSourcePanel.tsx @@ -11,8 +11,8 @@ import { formatTimelineEntry, } from '../../../utils/toolTimelineFormatting'; import { AgentSparkIcon } from './AgentTimelineRail'; +import { AssistantUiSubagentCall } from './AssistantUiSubagentCall'; import { ProcessingTranscriptView } from './ProcessingTranscriptView'; -import { SubagentActivityBlock } from './SubagentActivityBlock'; import { ToolTimelineBlock } from './ToolTimelineBlock'; const log = createDebug('app:conversations:agent-process-source'); @@ -176,7 +176,7 @@ export function AgentProcessSourcePanel({ {scopedEntry ? ( // Scoped to one step: show only that step's details. scopedEntry.subagent ? ( - + ) : scopedDetail ? ( 
                   {scopedDetail}
@@ -195,7 +195,7 @@ export function AgentProcessSourcePanel({
                }
+                renderSubagent={subagent => }
               />
             ) : entries.length > 0 ? (
               // Legacy snapshot (no transcript): fall back to the tool timeline,
@@ -223,7 +223,7 @@ export function AgentProcessSourcePanel({
                     

                       {formatTimelineEntry(entry).title}
                     

-                    
+
))} diff --git a/app/src/features/conversations/components/SubagentActivityBlock.tsx b/app/src/features/conversations/components/AssistantUiSubagentCall.tsx similarity index 62% rename from app/src/features/conversations/components/SubagentActivityBlock.tsx rename to app/src/features/conversations/components/AssistantUiSubagentCall.tsx index fa9cfc5463..783ee93fd3 100644 --- a/app/src/features/conversations/components/SubagentActivityBlock.tsx +++ b/app/src/features/conversations/components/AssistantUiSubagentCall.tsx @@ -1,3 +1,11 @@ +import { CheckIcon, ChevronDownIcon, Loader2Icon, WorkflowIcon } from 'lucide-react'; + +import { cn } from '../../../components/assistant-ui/lib/utils'; +import { + Collapsible, + CollapsibleContent, + CollapsibleTrigger, +} from '../../../components/assistant-ui/ui/collapsible'; import Badge from '../../../components/ui/Badge'; import WorktreeActions from '../../../components/worktree/WorktreeActions'; import { useT } from '../../../lib/i18n/I18nContext'; @@ -28,26 +36,9 @@ function ChildToolCallCard({ call }: { call: ChildToolCall }) { ); } -/** - * The agent's reasoning or visible narration, surfaced inline in the timeline - * as quoted/italic prose at the position it streamed — so a thought shows up - * wherever it occurred between tool calls. Shown directly (no "Thoughts" - * heading, no collapse). Both `thinking` and `text` transcript items render - * through here. Renders nothing for an all-whitespace delta so a half-streamed - * item never flashes an empty quote. - */ -export function ThoughtBlock({ text }: { text: string }) { - // Drop any inline `…` envelope the model emitted as - // text — the call already shows as its own row. Keep the original newlines - // (only trim the ends) so the markdown renderer can see headings, lists, - // code fences and emphasis instead of flattening them to one plain line. +function Thought({ text }: { text: string }) { const clean = stripToolCallEnvelopes(text).trim(); if (!clean) return null; - // Rendered through the shared `BubbleMarkdown` so a thought formats markdown - // (bold, code, lists) — but scaled back to the original quiet thought look: - // small (12px) and light/muted, not the larger, darker agent-bubble prose. - // Descendant overrides on `.prose` beat the typography plugin's base sizing; - // code keeps its accent colour so inline `tool_names` still read clearly. return (
void; }) { const { t } = useT(); @@ -82,13 +60,11 @@ export function SubagentActivityBlock({ if (subagent.mode) headerBits.push(subagent.mode); if (subagent.dedicatedThread) headerBits.push(t('conversations.toolTimeline.workerThread')); if (subagent.childIteration != null) { - if (subagent.childMaxIterations != null) { - headerBits.push( - `${t('conversations.toolTimeline.turn')} ${subagent.childIteration}/${subagent.childMaxIterations}` - ); - } else { - headerBits.push(`${t('conversations.toolTimeline.step')} ${subagent.childIteration}`); - } + headerBits.push( + subagent.childMaxIterations != null + ? `${t('conversations.toolTimeline.turn')} ${subagent.childIteration}/${subagent.childMaxIterations}` + : `${t('conversations.toolTimeline.step')} ${subagent.childIteration}` + ); } else if (subagent.iterations != null) { headerBits.push( subagent.iterations === 1 @@ -103,12 +79,6 @@ export function SubagentActivityBlock({ : `${subagent.elapsedMs}ms` ); } - - // The ordered transcript drives the inline activity: child tool-call rows - // and the agent's "Thoughts" (reasoning + visible narration) render in the - // exact order they streamed, so each thought appears wherever it occurred - // between tool calls. Falls back to the flat tool-call list when the prose - // transcript is absent (e.g. a rehydrated/interrupted snapshot). const transcript = subagent.transcript ?? []; return ( @@ -126,11 +96,11 @@ export function SubagentActivityBlock({ ) : null} {transcript.length > 0 ? (
- {transcript.map((item, i) => + {transcript.map((item, index) => item.kind === 'tool' ? ( ) : ( - + ) )}
@@ -155,7 +125,7 @@ export function SubagentActivityBlock({ {subagent.isDirty ? t('worktree.dirty') : t('worktree.clean')} - {subagent.changedFiles && subagent.changedFiles.length > 0 ? ( + {subagent.changedFiles?.length ? ( {subagent.changedFiles.length}{' '} {subagent.changedFiles.length === 1 @@ -179,3 +149,53 @@ export function SubagentActivityBlock({
); } + +export function AssistantUiSubagentCall({ + activity, + running = false, + description, + onView, + defaultOpen = false, +}: { + activity: SubagentActivity; + running?: boolean; + description?: string; + onView?: () => void; + defaultOpen?: boolean; +}) { + const name = activity.displayName ?? activity.agentId ?? 'subagent'; + return ( + + + + + Delegated to {name} + + {running ? ( + + running + + ) : ( + + + {activity.elapsedMs != null ? ( + {(activity.elapsedMs / 1000).toFixed(1)}s + ) : null} + + )} + + + + {description ?

{description}

: null} + + + + ); +} diff --git a/app/src/features/conversations/components/ChatToolParts.tsx b/app/src/features/conversations/components/ChatToolParts.tsx index f2d8c59a3a..606abd8490 100644 --- a/app/src/features/conversations/components/ChatToolParts.tsx +++ b/app/src/features/conversations/components/ChatToolParts.tsx @@ -1,22 +1,15 @@ import type { ToolCallMessagePartComponent } from '@assistant-ui/react'; -import { CheckIcon, ChevronDownIcon, Loader2Icon, WorkflowIcon } from 'lucide-react'; import type { FC, PropsWithChildren } from 'react'; -import { cn } from '../../../components/assistant-ui/lib/utils'; import type { ThreadGroupPart } from '../../../components/assistant-ui/thread'; import { ToolGroupContent, ToolGroupRoot, ToolGroupTrigger, } from '../../../components/assistant-ui/tool-group'; -import { - Collapsible, - CollapsibleContent, - CollapsibleTrigger, -} from '../../../components/assistant-ui/ui/collapsible'; import type { SubagentActivity } from '../../../store/chatRuntimeSlice'; +import { AssistantUiSubagentCall } from './AssistantUiSubagentCall'; import { OpenHumanToolCall } from './AssistantUiToolCall'; -import { SubagentActivityBlock } from './SubagentActivityBlock'; function asSubagentActivity(value: unknown): SubagentActivity | undefined { if (!value || typeof value !== 'object') return undefined; @@ -44,50 +37,18 @@ function readSubagentState( return { activity: progress, running: result === undefined }; } -/** Render a real OpenHuman `task` delegation using the existing activity view. */ +/** Adapt an assistant-ui `task` part onto the shared delegation card. */ export const SubagentCall: ToolCallMessagePartComponent = ({ args, result }) => { const { activity, running } = readSubagentState(args, result); const description = (args as { description?: string } | undefined)?.description; - const name = - activity?.displayName ?? - activity?.agentId ?? - (args as { subagent_type?: string } | undefined)?.subagent_type ?? - 'subagent'; - + const fallbackAgent = (args as { subagent_type?: string } | undefined)?.subagent_type; + const resolved = activity ?? { + taskId: 'pending-subagent', + agentId: fallbackAgent ?? 'subagent', + toolCalls: [], + }; return ( - - - - - Delegated to {name} - - {running ? ( - - - running - - ) : ( - - - {activity?.elapsedMs != null && ( - {(activity.elapsedMs / 1000).toFixed(1)}s - )} - - )} - - - - {description &&

{description}

} - {activity && } - - + ); }; diff --git a/app/src/features/conversations/components/PastTurnInsights.tsx b/app/src/features/conversations/components/PastTurnInsights.tsx index 28d2e5c6a2..6389ed70e5 100644 --- a/app/src/features/conversations/components/PastTurnInsights.tsx +++ b/app/src/features/conversations/components/PastTurnInsights.tsx @@ -1,7 +1,8 @@ import type { ProcessingTranscriptItem, ToolTimelineEntry } from '../../../store/chatRuntimeSlice'; import { formatTimelineEntry } from '../../../utils/toolTimelineFormatting'; +import { AssistantUiSubagentCall } from './AssistantUiSubagentCall'; import { ProcessingTranscriptView } from './ProcessingTranscriptView'; -import { SubagentActivityBlock, ToolTimelineBlock } from './ToolTimelineBlock'; +import { ToolTimelineBlock } from './ToolTimelineBlock'; /** * The collapsed process trail rendered above a PAST (settled) turn's answer on a @@ -50,7 +51,7 @@ export function PastTurnInsights({

{formatTimelineEntry(entry).title}

- + ))} diff --git a/app/src/features/conversations/components/ProcessingTranscriptView.tsx b/app/src/features/conversations/components/ProcessingTranscriptView.tsx index 4d2a680d01..b2ddf66801 100644 --- a/app/src/features/conversations/components/ProcessingTranscriptView.tsx +++ b/app/src/features/conversations/components/ProcessingTranscriptView.tsx @@ -35,7 +35,7 @@ export function ProcessingTranscriptView({ * Renders a delegated sub-agent's nested activity (its own child tool calls, * transcript and thoughts) under the row that spawned it. * - * Injected rather than imported because `SubagentActivityBlock` lives in + * Injected rather than imported because the assistant-ui delegation card lives in * `ToolTimelineBlock`, which imports THIS component for the inline rail — * importing it back would be a cycle. Without this, a `subagent:*` row * rendered as a bare one-line step and every child tool call it made was @@ -171,7 +171,7 @@ function ToolRow({ {/* A delegated sub-agent's own tool calls hang off the parent entry, so without this the whole child run collapsed into this single line. Rendered as a `
` SIBLING under the `
  • ` (indented past the - icon), not nested inside the label `` — SubagentActivityBlock + icon), not nested inside the label `` — the delegation card renders a `
    `, and `
    `-inside-`` is invalid nesting. */} {entry.subagent && renderSubagent ? (
    diff --git a/app/src/features/conversations/components/SubagentDrawer.tsx b/app/src/features/conversations/components/SubagentDrawer.tsx index 328c7feaf3..7df0210f1a 100644 --- a/app/src/features/conversations/components/SubagentDrawer.tsx +++ b/app/src/features/conversations/components/SubagentDrawer.tsx @@ -1,7 +1,7 @@ import createDebug from 'debug'; import { type ReactNode, useEffect, useState } from 'react'; -import Badge from '../../../components/ui/Badge'; +import Badge, { type BadgeVariant } from '../../../components/ui/Badge'; import Button from '../../../components/ui/Button'; import { SheetContent, SheetRoot, SheetTitle } from '../../../components/ui/Sheet'; import { useT } from '../../../lib/i18n/I18nContext'; @@ -14,15 +14,30 @@ import type { import type { ThreadMessage } from '../../../types/thread'; import { stripToolCallEnvelopes } from '../../../utils/toolTimelineFormatting'; import { BubbleMarkdown } from './AgentMessageBubble'; -import { - formatElapsed, - subagentStatusVariant, - SubagentToolCallRow, - useSubagentStatusLabel, -} from './SubagentToolCallRow'; +import { AssistantUiToolCallCard } from './AssistantUiToolCall'; const log = createDebug('app:conversations:subagent-drawer'); +function formatElapsed(ms: number): string { + return ms >= 1000 ? `${(ms / 1000).toFixed(1)}s` : `${ms}ms`; +} + +function subagentStatusVariant(status: ToolTimelineEntryStatus | undefined): BadgeVariant { + if (status === 'success') return 'success'; + if (status === 'error') return 'danger'; + if (status === 'cancelled') return 'neutral'; + return 'warning'; +} + +function useSubagentStatusLabel(status: ToolTimelineEntryStatus | undefined): string { + const { t } = useT(); + if (status === 'success') return t('conversations.subagent.statusCompleted'); + if (status === 'error') return t('conversations.subagent.statusFailed'); + if (status === 'cancelled') return t('conversations.subagent.statusCancelled'); + if (status === 'awaiting_user') return t('conversations.subagent.statusAwaitingUser'); + return t('conversations.subagent.statusRunning'); +} + /** * Rebuild a renderable transcript from a worker sub-thread's persisted * messages so a delegation can be reopened from memory after its live @@ -367,7 +382,16 @@ export function SubagentDrawer({ return ( - + ); })} diff --git a/app/src/features/conversations/components/SubagentToolCallRow.tsx b/app/src/features/conversations/components/SubagentToolCallRow.tsx deleted file mode 100644 index c2c030eefb..0000000000 --- a/app/src/features/conversations/components/SubagentToolCallRow.tsx +++ /dev/null @@ -1,87 +0,0 @@ -import Badge, { type BadgeVariant } from '../../../components/ui/Badge'; -import { useT } from '../../../lib/i18n/I18nContext'; -import type { - SubagentTranscriptItem, - ToolTimelineEntryStatus, -} from '../../../store/chatRuntimeSlice'; -import { AssistantUiToolCallCard } from './AssistantUiToolCall'; - -/** Human-readable elapsed time for a sub-agent run or one of its tool calls. */ -export function formatElapsed(ms: number): string { - return ms >= 1000 ? `${(ms / 1000).toFixed(1)}s` : `${ms}ms`; -} - -/** - * Map a sub-agent lifecycle status to the shared {@link Badge} variant used - * for it everywhere in the conversation panel, so the drawer header pill, the - * transcript tool rows and the inline timeline all read as one system. Kept in - * one place because a status that renders `warning` in one surface and - * `neutral` in another is a bug nobody notices until a screenshot diff. - */ -export function subagentStatusVariant(status: ToolTimelineEntryStatus | undefined): BadgeVariant { - switch (status) { - case 'success': - return 'success'; - case 'error': - return 'danger'; - case 'cancelled': - return 'neutral'; - default: - // running / awaiting_user — in flight, needs attention. - return 'warning'; - } -} - -/** Localised label for a sub-agent lifecycle status. */ -export function useSubagentStatusLabel(status: ToolTimelineEntryStatus | undefined): string { - const { t } = useT(); - switch (status) { - case 'success': - return t('conversations.subagent.statusCompleted'); - case 'error': - return t('conversations.subagent.statusFailed'); - case 'cancelled': - return t('conversations.subagent.statusCancelled'); - case 'awaiting_user': - return t('conversations.subagent.statusAwaitingUser'); - default: - return t('conversations.subagent.statusRunning'); - } -} - -type SubagentToolItem = Extract; - -/** - * Pretty-print a tool's input arguments for display. Objects/arrays are - * rendered as indented JSON; a string is shown verbatim. Returns `null` when - * there are no arguments to show (e.g. a tool called with no input, or a - * transcript reopened from memory where args weren't persisted). - */ -export function formatArgs(args: unknown): string | null { - if (args == null) return null; - if (typeof args === 'string') return args.length > 0 ? args : null; - try { - return JSON.stringify(args, null, 2); - } catch { - return String(args); - } -} - -/** - * Drawer child calls use the exact assistant-ui card used by parent and inline - * tools. The drawer no longer owns a parallel tool-call component hierarchy. - */ -export function SubagentToolCallRow({ item }: { item: SubagentToolItem }) { - return ( - - ); -} diff --git a/app/src/features/conversations/components/ToolTimelineBlock.tsx b/app/src/features/conversations/components/ToolTimelineBlock.tsx index 61a8e54e68..345356c7b3 100644 --- a/app/src/features/conversations/components/ToolTimelineBlock.tsx +++ b/app/src/features/conversations/components/ToolTimelineBlock.tsx @@ -15,8 +15,8 @@ import type { import { formatTimelineEntry, stripToolCallEnvelopes } from '../../../utils/toolTimelineFormatting'; import { parseWorkerThreadRef } from '../utils/workerThreadRef'; import { agentNameTone, AgentTimelineRail } from './AgentTimelineRail'; +import { AssistantUiSubagentCall } from './AssistantUiSubagentCall'; import { ProcessingTranscriptView } from './ProcessingTranscriptView'; -import { SubagentActivityBlock } from './SubagentActivityBlock'; import { coalesceTimelineEntries, normalizeToolBody, @@ -25,13 +25,6 @@ import { } from './toolTimelineRows'; import { WorkerThreadRefCard } from './WorkerThreadRefCard'; -/** - * Re-exported so the historical import path keeps resolving. The component - * itself moved to `./SubagentActivityBlock` when this file was split; it is a - * pure move, no behaviour changed. - */ -export { SubagentActivityBlock } from './SubagentActivityBlock'; - /** Tail of the parent's in-flight response shown in the processing panel. */ const RESPONSE_PREVIEW_CHARS = 320; @@ -426,8 +419,9 @@ export function ToolTimelineBlock({ transcript={transcript} entries={ordered} renderSubagent={subagent => ( - onViewSubagent(subagent) : undefined} /> )} @@ -536,8 +530,9 @@ export function ToolTimelineBlock({ ) : null} {subagent ? ( - onViewSubagent(subagent) : undefined} /> ) : null} diff --git a/app/src/features/conversations/components/__tests__/ToolTimelineBlock.test.tsx b/app/src/features/conversations/components/__tests__/ToolTimelineBlock.test.tsx index 0518f6514a..6939bacb0b 100644 --- a/app/src/features/conversations/components/__tests__/ToolTimelineBlock.test.tsx +++ b/app/src/features/conversations/components/__tests__/ToolTimelineBlock.test.tsx @@ -4,8 +4,19 @@ import { Provider } from 'react-redux'; import { describe, expect, it, vi } from 'vitest'; import { store } from '../../../../store'; -import type { ToolTimelineEntry } from '../../../../store/chatRuntimeSlice'; -import { SubagentActivityBlock, ToolTimelineBlock } from '../ToolTimelineBlock'; +import type { SubagentActivity, ToolTimelineEntry } from '../../../../store/chatRuntimeSlice'; +import { AssistantUiSubagentCall } from '../AssistantUiSubagentCall'; +import { ToolTimelineBlock } from '../ToolTimelineBlock'; + +function SubagentActivityBlock({ + subagent, + onView, +}: { + subagent: SubagentActivity; + onView?: () => void; +}) { + return ; +} // #1122 — guards the parent-thread live subagent rendering. The block // always expands subagent rows so the activity stays visible while the @@ -928,6 +939,10 @@ describe('ToolTimelineBlock — subagent rendering', () => { }; renderInStore(); + const subagent = screen.getByTestId('assistant-ui-subagent-call'); + const trigger = within(subagent).getByRole('button'); + expect(trigger).toHaveAttribute('aria-expanded', 'false'); + fireEvent.click(trigger); const calls = screen.getAllByTestId('assistant-ui-tool-call'); expect(calls).toHaveLength(1); expect(calls[0].textContent).toContain('Searching the web'); @@ -1033,7 +1048,7 @@ describe('ToolTimelineBlock — compact chat mode (onViewDetails)', () => { }, ]; - it('collapses finished steps to a "View details" link but keeps the running step expanded inline', () => { + it('collapses finished steps to a link and keeps the running delegation card inline', () => { const onViewDetails = vi.fn(); renderInStore(); @@ -1041,8 +1056,12 @@ describe('ToolTimelineBlock — compact chat mode (onViewDetails)', () => { const links = screen.getAllByTestId('view-details'); expect(links).toHaveLength(1); - // The currently-running sub-agent stays expanded inline in the main UI - // (its activity is visible) — and shows no "View details" link itself. + // The running delegation remains inline but its assistant-ui disclosure is + // collapsed by default like every other delegation card. + const subagent = screen.getByTestId('assistant-ui-subagent-call'); + const trigger = within(subagent).getByRole('button'); + expect(trigger).toHaveAttribute('aria-expanded', 'false'); + fireEvent.click(trigger); const activity = screen.getByTestId('subagent-activity'); expect(activity.textContent).toContain('pondering'); // The finished step SUCCEEDED, so its raw output is no longer duplicated @@ -1085,7 +1104,9 @@ describe('ToolTimelineBlock — compact chat mode (onViewDetails)', () => { it('still expands inline (no compact link) when onViewDetails is omitted (panel mode)', () => { renderInStore(); - // Panel/expandable path: sub-agent activity is shown, no "View details" link. + const subagent = screen.getByTestId('assistant-ui-subagent-call'); + fireEvent.click(within(subagent).getByRole('button')); + // Panel path uses the same delegation card, with no compact details link. expect(screen.getByTestId('subagent-activity')).toBeInTheDocument(); expect(screen.queryByTestId('view-details')).toBeNull(); }); @@ -1398,6 +1419,8 @@ describe('ToolTimelineBlock — sub-agent activity survives the transcript path' expect(screen.getByTestId('processing-transcript')).toBeInTheDocument(); // …and the nested child run is present, not collapsed to one line. expect(screen.getByTestId('processing-subagent')).toBeInTheDocument(); + const subagent = screen.getByTestId('assistant-ui-subagent-call'); + fireEvent.click(within(subagent).getByRole('button')); const calls = screen.getAllByTestId('assistant-ui-tool-call'); expect(calls).toHaveLength(2); expect(calls[0].textContent).toContain('Searched the web'); @@ -1410,6 +1433,8 @@ describe('ToolTimelineBlock — sub-agent activity survives the transcript path' it('still renders child tool calls on the legacy row path (no transcript)', () => { renderInStore(); expect(screen.queryByTestId('processing-transcript')).toBeNull(); + const subagent = screen.getByTestId('assistant-ui-subagent-call'); + fireEvent.click(within(subagent).getByRole('button')); expect(screen.getAllByTestId('assistant-ui-tool-call')).toHaveLength(2); }); diff --git a/app/src/features/conversations/derived/mapDisplayItems.ts b/app/src/features/conversations/derived/mapDisplayItems.ts index 1d5c3233ee..e3dab1b56f 100644 --- a/app/src/features/conversations/derived/mapDisplayItems.ts +++ b/app/src/features/conversations/derived/mapDisplayItems.ts @@ -4,7 +4,7 @@ * models, keyed by producing `requestId` — the exact shapes * `fetchAndHydrateTurnHistory` produces from the legacy `turn_state_history` * snapshot ring, so `PastTurnInsights` / `ProcessingTranscriptView` / - * `ToolTimelineBlock` / `SubagentActivityBlock` are reused unchanged. + * assistant-ui tool and delegation cards are reused unchanged. * * Division of labour (matches how `turnTimelinesByThread` / `PastTurnInsights` * anchor today): @@ -136,7 +136,7 @@ function stringifyArgs(args: unknown): string | undefined { * Build a {@link SubagentActivity} from a `subagent` display item's nested * items. The nested vocabulary (reasoning / assistantMessage / toolCall) * projects onto the sub-agent transcript (`thinking` / `text` / `tool`) plus a - * flat `toolCalls` list — exactly what `SubagentActivityBlock` reads. + * flat `toolCalls` list — exactly what the assistant-ui delegation card reads. */ function buildSubagentActivity(id: string, items: DerivedDisplayItem[]): SubagentActivity { const toolCalls: SubagentToolCallEntry[] = []; diff --git a/app/src/types/derivedTranscript.ts b/app/src/types/derivedTranscript.ts index e683ee6985..c4e5743739 100644 --- a/app/src/types/derivedTranscript.ts +++ b/app/src/types/derivedTranscript.ts @@ -6,7 +6,7 @@ * The Rust core projects the append-only `session_raw/*.jsonl` source of truth * into typed **display items** in the frontend's chat vocabulary. Phase C maps * these onto the existing settled-turn renderers (`PastTurnInsights` / - * `ProcessingTranscriptView` / `ToolTimelineBlock` / `SubagentActivityBlock`) + * `ProcessingTranscriptView` / assistant-ui tool and delegation cards) * via `features/conversations/derived/mapDisplayItems.ts`. * * Serde is camelCase on the wire, so every field here is camelCase and mirrors From ad8251e6ac1b2046c723fe39cebe26b43f7bf889 Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Mon, 31 Aug 2026 21:47:05 +0300 Subject: [PATCH 10/23] fix(chat): keep message paint stable while scrolling Co-authored-by: Medulla --- app/src/components/assistant-ui/thread.tsx | 8 +- .../__tests__/Conversations.render.test.tsx | 9 ++ .../specs/chat-scroll-stability.spec.ts | 95 +++++++++++++++++++ 3 files changed, 107 insertions(+), 5 deletions(-) create mode 100644 app/test/playwright/specs/chat-scroll-stability.spec.ts diff --git a/app/src/components/assistant-ui/thread.tsx b/app/src/components/assistant-ui/thread.tsx index ddd4d29e47..43944c72d5 100644 --- a/app/src/components/assistant-ui/thread.tsx +++ b/app/src/components/assistant-ui/thread.tsx @@ -638,7 +638,7 @@ const AssistantMessage: FC = () => { data-slot="aui_assistant-message-root" data-role="assistant" data-testid="agent-message" - className="fade-in slide-in-from-bottom-1 animate-in relative -mb-7.5 pb-7.5 duration-150 [contain-intrinsic-size:auto_200px] [content-visibility:auto]"> + className="fade-in slide-in-from-bottom-1 animate-in relative -mb-7.5 pb-7.5 duration-150"> {/* * One vertical rhythm for the whole message, rather than each part * bringing its own margin. Measured before this change the gaps ran @@ -809,7 +809,7 @@ const UserMessage: FC = () => { return ( @@ -852,9 +852,7 @@ const UserActionBar: FC = () => { const EditComposer: FC = () => { return ( - + { 'Long agent output with enough structure to prefer a text view.' ); expect(screen.getByText('Can you summarize this?')).toBeInTheDocument(); + // Message rows must retain their measured layout/paint while off-screen. + // `content-visibility:auto` plus a guessed intrinsic height makes WebKit + // reveal/re-size rows as they cross the viewport, producing scroll flicker. + const assistantRoot = screen.getByTestId('agent-message'); + const userRoot = document.querySelector('[data-slot="aui_user-message-root"]'); + expect(assistantRoot.className).not.toContain('content-visibility'); + expect(userRoot?.className).not.toContain('content-visibility'); + expect(assistantRoot.className).not.toContain('contain-intrinsic-size'); + expect(userRoot?.className).not.toContain('contain-intrinsic-size'); }); it("renders a past turn's process trail above the answer it produced (Phase 5)", async () => { diff --git a/app/test/playwright/specs/chat-scroll-stability.spec.ts b/app/test/playwright/specs/chat-scroll-stability.spec.ts new file mode 100644 index 0000000000..4195bbf9d4 --- /dev/null +++ b/app/test/playwright/specs/chat-scroll-stability.spec.ts @@ -0,0 +1,95 @@ +import { expect, test } from '@playwright/test'; + +import { + bootAuthenticatedPage, + callCoreRpc, + dismissWalkthroughIfPresent, + waitForAppReady, +} from '../helpers/core-rpc'; + +const USER_ID = 'pw-chat-scroll-stability'; +const TURN_COUNT = 18; + +test.describe('Chat scroll paint stability', () => { + test('keeps every message laid out while scrolling a long transcript', async ({ page }) => { + let transcriptRpcCount = 0; + page.on('request', request => { + if (request.postData()?.includes('openhuman.threads_transcript_get')) { + transcriptRpcCount += 1; + } + }); + await bootAuthenticatedPage(page, USER_ID, '/chat'); + + const created = await callCoreRpc<{ data: { id: string } }>('openhuman.threads_create_new'); + const threadId = created.data.id; + const base = Date.parse('2026-08-31T12:00:00.000Z'); + for (let turn = 0; turn < TURN_COUNT; turn += 1) { + for (const sender of ['user', 'agent'] as const) { + const index = turn * 2 + (sender === 'agent' ? 1 : 0); + await callCoreRpc('openhuman.threads_message_append', { + thread_id: threadId, + message: { + id: `scroll-${sender}-${turn}`, + sender, + type: 'text', + createdAt: new Date(base + index * 1000).toISOString(), + extraMetadata: {}, + content: + sender === 'user' + ? `Question ${turn}: explain this part of the long transcript.` + : `Answer ${turn}\n\nThis paragraph has deliberately varied content so its measured height is not a guessed placeholder.\n\n- detail one\n- detail two\n- detail three`, + }, + }); + } + } + + await page.reload(); + await waitForAppReady(page); + await page.goto('/#/chat'); + await dismissWalkthroughIfPresent(page); + const row = page.getByTestId(`thread-row-${threadId}`); + await expect(row).toBeVisible({ timeout: 20_000 }); + await row.click({ force: true }); + + const roots = page.locator( + '[data-slot="aui_assistant-message-root"], [data-slot="aui_user-message-root"]' + ); + await expect(roots).toHaveCount(TURN_COUNT * 2, { timeout: 20_000 }); + expect( + await roots.evaluateAll(elements => + elements.every( + element => + !element.className.includes('content-visibility') && + !element.className.includes('contain-intrinsic-size') + ) + ) + ).toBe(true); + + const viewport = page.locator('[data-slot="aui_thread-viewport"]'); + const initial = await viewport.evaluate(element => ({ + height: element.scrollHeight, + messages: element.querySelectorAll( + '[data-slot="aui_assistant-message-root"], [data-slot="aui_user-message-root"]' + ).length, + })); + const rpcCountBeforeScroll = transcriptRpcCount; + + for (const fraction of [0, 0.25, 0.5, 0.75, 1, 0.5, 0]) { + const metrics = await viewport.evaluate(async (element, nextFraction) => { + element.scrollTop = (element.scrollHeight - element.clientHeight) * nextFraction; + await new Promise(resolve => + requestAnimationFrame(() => requestAnimationFrame(() => resolve())) + ); + return { + height: element.scrollHeight, + messages: element.querySelectorAll( + '[data-slot="aui_assistant-message-root"], [data-slot="aui_user-message-root"]' + ).length, + }; + }, fraction); + expect(metrics).toEqual(initial); + await expect(page.getByText('Loading conversation…')).toHaveCount(0); + } + expect(transcriptRpcCount).toBe(rpcCountBeforeScroll); + }); +}); From 7049eb680ad7cdad39324d0d179fa0ccd7083472 Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Mon, 31 Aug 2026 21:56:27 +0300 Subject: [PATCH 11/23] chore(deps): point tinyagents at reasoning persistence Co-authored-by: Medulla --- vendor/tinyagents | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vendor/tinyagents b/vendor/tinyagents index d44f18843b..33586973ac 160000 --- a/vendor/tinyagents +++ b/vendor/tinyagents @@ -1 +1 @@ -Subproject commit d44f18843b88982e792fc8c9a4e056f6358831b6 +Subproject commit 33586973ac6ddf737429bee9dcc8150f89cb7aa4 From f630f8cdc540ef33104d8a52037fe147053b84ae Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Mon, 31 Aug 2026 22:04:19 +0300 Subject: [PATCH 12/23] fix(core): satisfy transcript delivery clippy gates Co-authored-by: Medulla --- src/openhuman/threads/transcript_view/project.rs | 5 ++++- src/openhuman/web_chat/presentation.rs | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/openhuman/threads/transcript_view/project.rs b/src/openhuman/threads/transcript_view/project.rs index c1c7cc5a5a..324f50d03f 100644 --- a/src/openhuman/threads/transcript_view/project.rs +++ b/src/openhuman/threads/transcript_view/project.rs @@ -31,6 +31,9 @@ const DATETIME_PREFIX: &str = "Current Date & Time:"; /// live injector currently only prepends [`DATETIME_PREFIX`]. const CHANNEL_CONTEXT_PREFIX: &str = "[Channel context]"; +type NativeToolCall = (String, String, String); +type NativeToolEnvelope = (String, Vec); + /// Resolve a thread's root transcript, discover its sub-agent siblings, and /// project everything into display items. Returns `None` when the thread has /// no root transcript yet (brand-new thread / first turn not persisted). @@ -463,7 +466,7 @@ fn project_assistant( /// Decode the native provider replay envelope embedded in `ChatMessage.content`. /// Returns visible assistant prose plus `(id, name, arguments)` calls. -fn parse_native_tool_envelope(raw: &str) -> Option<(String, Vec<(String, String, String)>)> { +fn parse_native_tool_envelope(raw: &str) -> Option { let value = serde_json::from_str::(raw).ok()?; let object = value.as_object()?; let calls = object.get("tool_calls")?.as_array()?; diff --git a/src/openhuman/web_chat/presentation.rs b/src/openhuman/web_chat/presentation.rs index 1263affe33..44e14076cb 100644 --- a/src/openhuman/web_chat/presentation.rs +++ b/src/openhuman/web_chat/presentation.rs @@ -69,7 +69,7 @@ pub(crate) async fn deliver_response( // Keep the response byte-for-byte in one assistant message. The legacy // segmentation helpers remain available to channel-specific callers/tests, // but the interactive web surface must not cut or reformat model output. - let segments = vec![full_response.to_string()]; + let segments = [full_response.to_string()]; // Await the reaction result (should already be done or nearly done). let reaction_emoji = reaction_handle.await.unwrap_or(None); From b6ab5ee948e51301c13d3830216401927eb71be9 Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Mon, 31 Aug 2026 22:12:38 +0300 Subject: [PATCH 13/23] chore(deps): update tinyagents reasoning fix Co-authored-by: Medulla --- vendor/tinyagents | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vendor/tinyagents b/vendor/tinyagents index 33586973ac..2b95b98ed0 160000 --- a/vendor/tinyagents +++ b/vendor/tinyagents @@ -1 +1 @@ -Subproject commit 33586973ac6ddf737429bee9dcc8150f89cb7aa4 +Subproject commit 2b95b98ed070d73a2ef44de8fbbf04bb4f51906c From 9d7d59558c040f662383e38f95514455f66899d2 Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Mon, 31 Aug 2026 22:37:39 +0300 Subject: [PATCH 14/23] test(chat): cover collapsed delegation consumers Co-authored-by: Medulla --- .../components/PastTurnInsights.test.tsx | 3 ++- .../__tests__/AgentProcessSourcePanel.test.tsx | 11 +++++++++-- .../derived/derivedRestore.render.test.tsx | 3 ++- app/src/providers/useOpenHumanExternalStore.ts | 2 +- 4 files changed, 14 insertions(+), 5 deletions(-) diff --git a/app/src/features/conversations/components/PastTurnInsights.test.tsx b/app/src/features/conversations/components/PastTurnInsights.test.tsx index ef0314b83b..31a5bb47cd 100644 --- a/app/src/features/conversations/components/PastTurnInsights.test.tsx +++ b/app/src/features/conversations/components/PastTurnInsights.test.tsx @@ -1,4 +1,4 @@ -import { render, screen } from '@testing-library/react'; +import { fireEvent, render, screen } from '@testing-library/react'; import { Provider } from 'react-redux'; import { describe, expect, it } from 'vitest'; @@ -53,6 +53,7 @@ describe('PastTurnInsights', () => { renderInStore(); const subagents = screen.getByTestId('past-turn-subagents'); + fireEvent.click(screen.getByTestId('assistant-ui-subagent-call').querySelector('button')!); expect(subagents.textContent).toContain('child reasoning trail'); }); diff --git a/app/src/features/conversations/components/__tests__/AgentProcessSourcePanel.test.tsx b/app/src/features/conversations/components/__tests__/AgentProcessSourcePanel.test.tsx index 9208cd39f5..fd0f5fd301 100644 --- a/app/src/features/conversations/components/__tests__/AgentProcessSourcePanel.test.tsx +++ b/app/src/features/conversations/components/__tests__/AgentProcessSourcePanel.test.tsx @@ -1,4 +1,4 @@ -import { render, screen } from '@testing-library/react'; +import { fireEvent, render, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { Provider } from 'react-redux'; import { describe, expect, it, vi } from 'vitest'; @@ -11,6 +11,10 @@ function renderPanel(ui: React.ReactNode) { return render({ui}); } +function openFirstSubagent(): void { + fireEvent.click(screen.getAllByTestId('assistant-ui-subagent-call')[0].querySelector('button')!); +} + const fetchEntry = (id: string, url: string): ToolTimelineEntry => ({ id, name: 'web_fetch', @@ -129,7 +133,8 @@ describe('AgentProcessSourcePanel', () => { onClose={() => {}} /> ); - // The subagent activity renders, but with no onView → no button. + // The assistant-ui delegation is collapsed until the reviewer opens it. + openFirstSubagent(); expect(screen.getByTestId('subagent-activity')).toBeInTheDocument(); expect(screen.queryByTestId('subagent-view-processing')).toBeNull(); }); @@ -187,6 +192,7 @@ describe('AgentProcessSourcePanel', () => { // …and the sub-agent's full activity (its thoughts) shows in the deep-dive, // with no redundant "view full processing" button (no onView). expect(screen.getByTestId('agent-source-subagent')).toBeInTheDocument(); + openFirstSubagent(); const activity = screen.getByTestId('subagent-activity'); expect(activity.textContent).toContain('planning the search'); expect(screen.queryByTestId('subagent-view-processing')).toBeNull(); @@ -222,6 +228,7 @@ describe('AgentProcessSourcePanel', () => { // Header shows the step's label, not the generic title. expect(screen.getByText('Researching')).toBeInTheDocument(); // Only the scoped step's activity renders… + openFirstSubagent(); expect(screen.getByTestId('subagent-activity').textContent).toContain('scoped thought'); // …and the whole-run transcript / other steps do NOT. expect(screen.queryByTestId('processing-transcript')).toBeNull(); diff --git a/app/src/features/conversations/derived/derivedRestore.render.test.tsx b/app/src/features/conversations/derived/derivedRestore.render.test.tsx index 02490e8ec1..af7a457a91 100644 --- a/app/src/features/conversations/derived/derivedRestore.render.test.tsx +++ b/app/src/features/conversations/derived/derivedRestore.render.test.tsx @@ -1,4 +1,4 @@ -import { render, screen } from '@testing-library/react'; +import { fireEvent, render, screen } from '@testing-library/react'; import { Provider } from 'react-redux'; import { describe, expect, it } from 'vitest'; @@ -73,6 +73,7 @@ describe('derived transcript restore (mapper → PastTurnInsights)', () => { expect(screen.getAllByTestId('processing-tool-row').length).toBeGreaterThan(0); // The sub-agent's own reasoning trail renders beneath. const subagents = screen.getByTestId('past-turn-subagents'); + fireEvent.click(screen.getByTestId('assistant-ui-subagent-call').querySelector('button')!); expect(subagents.textContent).toContain('child reasoning trail'); }); }); diff --git a/app/src/providers/useOpenHumanExternalStore.ts b/app/src/providers/useOpenHumanExternalStore.ts index aab1a80f00..92c88f80f4 100644 --- a/app/src/providers/useOpenHumanExternalStore.ts +++ b/app/src/providers/useOpenHumanExternalStore.ts @@ -112,7 +112,7 @@ export function useOpenHumanExternalStore(threadId: string | null) { ? (state.chatRuntime.processingByThread?.[threadId] ?? EMPTY_TRANSCRIPT) : EMPTY_TRANSCRIPT ); - const settledRevision = `${messages.at(-1)?.id ?? ''}:${messages.at(-1)?.content.length ?? 0}:${lifecycle ?? ''}`; + const settledRevision = `${messages.at(-1)?.id ?? ''}:${messages.at(-1)?.content?.length ?? 0}:${lifecycle ?? ''}`; const coreTranscript = useCoreTranscriptProjection( threadId, settledRevision, From 2c6c076a1126daa38b93cd27bf335ef77225d840 Mon Sep 17 00:00:00 2001 From: M3gA-Mind Date: Wed, 2 Sep 2026 07:16:28 +0530 Subject: [PATCH 15/23] fix(chat): report terminal tool and delegation states faithfully MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the review threads on #5885. Six defects, all in the direction of reporting a failure as a success — which matters more here than elsewhere, because this PR's whole subject is rendering a turn faithfully. - Sub-agent lifecycle was collapsed to a boolean in three places, producing opposite errors from the same cause: `AssistantUiSubagentCall`'s `running = false` default rendered a failed delegation with a success check, while `ToolTimelineBlock`'s `status !== 'completed'` gave the same row an endless spinner. `isActiveSubagentStatus` is now the single question all three call sites ask, and a failed or cancelled delegation renders with the `CircleXIcon` the tool card already uses for the same state. - A terminal tool status never reached the assistant-ui adapter: the part has no status field, so `OpenHumanToolCall` fell back to `result !== undefined` and labelled a failed tool "done". `toolPart` now carries the status for a failed or cancelled entry and the adapter unwraps it. The success path is byte-identical on purpose. - `recoverTimelineToolNames` advanced its cursor on every timeline entry even though `recoveredNames` only holds names for the generic rows, so a named row consumed the first recovered name and the last generic row kept `tool`. - Narration already contained in the merged final answer rendered twice, since `mergedAssistantText` prefers the longest text when it contains every segment and the guard tested equality rather than containment. - Root transcripts were ordered by file name. Modern `{unix_ts}_{agent}` stems sort the same either way, but a legacy `{agent}_{index}` root encodes no time and, digits sorting before letters, landed after every modern one regardless of age — reordering the view and able to attach a sub-agent trail to the wrong turn. Ordering is now by `meta.created`, with the path as tiebreak. - `ChatThreadView` read `content.length` behind a guard that only covered the message, though `TranscriptRow` already treats content as nullish. Test hygiene from the same review: two Playwright locators that could not fail (`Loading conversation…` never matches the rendered label, which has no ellipsis; `getByRole('button')` throws on strict mode if the card grows a second button), a `querySelector('button')!` that failed with an opaque TypeError at the click, and a single-microtask flush that could let a scheduled `logout` slip past a not-called assertion. Every new test was checked by reverting its fix and confirming it fails. --- .../components/AssistantUiSubagentCall.tsx | 36 ++++++++-- .../components/AssistantUiToolCall.tsx | 46 +++++++++--- .../components/ChatThreadView.tsx | 2 +- .../components/ChatToolParts.test.tsx | 72 +++++++++++++++++++ .../components/ChatToolParts.tsx | 7 +- .../components/ToolTimelineBlock.tsx | 4 +- .../__tests__/ToolTimelineBlock.test.tsx | 28 ++++++++ .../derived/derivedRestore.render.test.tsx | 9 ++- .../__tests__/CoreStateProvider.test.tsx | 5 +- .../__tests__/assistantUiMessages.test.ts | 68 ++++++++++++++++++ app/src/providers/assistantUiMessages.ts | 42 +++++++++-- .../specs/chat-scroll-stability.spec.ts | 5 +- .../specs/chat-tool-call-flow.spec.ts | 2 +- .../harness/session/transcript_part_02.rs | 35 +++++++-- .../session/transcript_tests_part_03_tests.rs | 55 ++++++++++++++ 15 files changed, 383 insertions(+), 33 deletions(-) diff --git a/app/src/features/conversations/components/AssistantUiSubagentCall.tsx b/app/src/features/conversations/components/AssistantUiSubagentCall.tsx index 783ee93fd3..fc0b29b3f7 100644 --- a/app/src/features/conversations/components/AssistantUiSubagentCall.tsx +++ b/app/src/features/conversations/components/AssistantUiSubagentCall.tsx @@ -1,4 +1,4 @@ -import { CheckIcon, ChevronDownIcon, Loader2Icon, WorkflowIcon } from 'lucide-react'; +import { CheckIcon, ChevronDownIcon, CircleXIcon, Loader2Icon, WorkflowIcon } from 'lucide-react'; import { cn } from '../../../components/assistant-ui/lib/utils'; import { @@ -150,9 +150,27 @@ function SubagentDetails({ ); } +/** + * Statuses that mean the delegation is still in flight. + * + * `SubagentActivity.status` carries `running` | `awaiting_user` | `completed` | + * `failed`, and collapsing that to a boolean is what produced two opposite + * rendering bugs: a caller that omitted `running` showed a *failed* delegation + * with a success check, while `status !== 'completed'` gave the same row an + * endless spinner. Both call sites now ask this one question. + */ +export function isActiveSubagentStatus(status: string | undefined): boolean { + return status === 'running' || status === 'awaiting_user'; +} + +/** Statuses that mean the delegation stopped without succeeding. */ +function isFailedSubagentStatus(status: string | undefined): boolean { + return status === 'failed' || status === 'cancelled'; +} + export function AssistantUiSubagentCall({ activity, - running = false, + running, description, onView, defaultOpen = false, @@ -164,27 +182,35 @@ export function AssistantUiSubagentCall({ defaultOpen?: boolean; }) { const name = activity.displayName ?? activity.agentId ?? 'subagent'; + // Default to the activity's own lifecycle rather than `false`: most call + // sites pass no `running` prop at all, and treating every non-running + // activity as finished-successfully is what rendered a failed delegation + // with a success check. + const active = running ?? isActiveSubagentStatus(activity.status); + const failed = !active && isFailedSubagentStatus(activity.status); return ( Delegated to {name} - {running ? ( + {active ? ( running ) : ( - + {failed ? : } + {failed ? {activity.status} : null} {activity.elapsedMs != null ? ( {(activity.elapsedMs / 1000).toFixed(1)}s ) : null} diff --git a/app/src/features/conversations/components/AssistantUiToolCall.tsx b/app/src/features/conversations/components/AssistantUiToolCall.tsx index 934f820455..dd0dfb19bd 100644 --- a/app/src/features/conversations/components/AssistantUiToolCall.tsx +++ b/app/src/features/conversations/components/AssistantUiToolCall.tsx @@ -205,11 +205,41 @@ export function AssistantUiToolCallCard({ ); } -export const OpenHumanToolCall: ToolCallMessagePartComponent = props => ( - -); +/** + * Terminal status carried inside a settled tool part's `result`. + * + * assistant-ui's tool-call part has no status field, so `toolPart` puts the + * status there for a tool that failed or was cancelled (`value` holds the real + * output when there was one). Without unwrapping it here the card fell back to + * `result !== undefined`, which reads as success — a failed tool rendered + * "done" with a check. + */ +function toolStatusEnvelope( + result: unknown +): + | { status: ToolTimelineEntryStatus; failure?: ToolFailureExplanation; value?: unknown } + | undefined { + if (!result || typeof result !== 'object' || Array.isArray(result)) return undefined; + const candidate = result as { status?: unknown; failure?: unknown; value?: unknown }; + return candidate.status === 'error' || candidate.status === 'cancelled' + ? { + status: candidate.status as ToolTimelineEntryStatus, + failure: candidate.failure as ToolFailureExplanation | undefined, + ...('value' in candidate ? { value: candidate.value } : {}), + } + : undefined; +} + +export const OpenHumanToolCall: ToolCallMessagePartComponent = props => { + const envelope = toolStatusEnvelope(props.result); + return ( + + ); +}; diff --git a/app/src/features/conversations/components/ChatThreadView.tsx b/app/src/features/conversations/components/ChatThreadView.tsx index 003c384a0a..e186bda89e 100644 --- a/app/src/features/conversations/components/ChatThreadView.tsx +++ b/app/src/features/conversations/components/ChatThreadView.tsx @@ -344,7 +344,7 @@ export const ChatThreadView = forwardRef { expect(screen.getByText('Checking primary sources.')).toBeInTheDocument(); }); + it('renders a failed delegation as failed, not as a completed one', () => { + // `SubagentActivity.status` carries `failed`, but a settled part was read + // as `running: false` and rendered with a success check — the transcript + // reported a failure as a success. + render( + {}} + resume={() => {}} + respondToApproval={() => {}} + /> + ); + + expect(screen.getByTestId('assistant-ui-subagent-call')).toHaveAttribute( + 'data-status', + 'failed' + ); + expect(screen.getByText('failed')).toBeInTheDocument(); + expect(screen.queryByText('running')).not.toBeInTheDocument(); + }); + + it('keeps a completed delegation reading as completed', () => { + render( + {}} + resume={() => {}} + respondToApproval={() => {}} + /> + ); + + expect(screen.getByTestId('assistant-ui-subagent-call')).toHaveAttribute( + 'data-status', + 'completed' + ); + expect(screen.queryByText('failed')).not.toBeInTheDocument(); + }); + + it('keeps a still-running delegation running when the part has already settled', () => { + // The tool-call status and the delegation status are separate fields, so a + // settled part can still carry an in-flight activity. Hard-coding + // `running: false` for any settled part froze that row into a success. + render( + {}} + resume={() => {}} + respondToApproval={() => {}} + /> + ); + + expect(screen.getByText('running')).toBeInTheDocument(); + }); + it('opens a group containing in-flight work on mount', () => { render( diff --git a/app/src/features/conversations/components/ChatToolParts.tsx b/app/src/features/conversations/components/ChatToolParts.tsx index 606abd8490..1069e6477d 100644 --- a/app/src/features/conversations/components/ChatToolParts.tsx +++ b/app/src/features/conversations/components/ChatToolParts.tsx @@ -8,7 +8,7 @@ import { ToolGroupTrigger, } from '../../../components/assistant-ui/tool-group'; import type { SubagentActivity } from '../../../store/chatRuntimeSlice'; -import { AssistantUiSubagentCall } from './AssistantUiSubagentCall'; +import { AssistantUiSubagentCall, isActiveSubagentStatus } from './AssistantUiSubagentCall'; import { OpenHumanToolCall } from './AssistantUiToolCall'; function asSubagentActivity(value: unknown): SubagentActivity | undefined { @@ -29,7 +29,10 @@ function readSubagentState( result: unknown ): { activity: SubagentActivity | undefined; running: boolean } { const completed = asSubagentActivity(result); - if (completed) return { activity: completed, running: false }; + // A settled part carries the activity, but "settled" is not "succeeded": + // ask the activity's own status so a `failed` delegation is not rendered as + // a completed one. + if (completed) return { activity: completed, running: isActiveSubagentStatus(completed.status) }; const progress = args && typeof args === 'object' ? asSubagentActivity((args as { progress?: unknown }).progress) diff --git a/app/src/features/conversations/components/ToolTimelineBlock.tsx b/app/src/features/conversations/components/ToolTimelineBlock.tsx index 345356c7b3..4975148cc4 100644 --- a/app/src/features/conversations/components/ToolTimelineBlock.tsx +++ b/app/src/features/conversations/components/ToolTimelineBlock.tsx @@ -15,7 +15,7 @@ import type { import { formatTimelineEntry, stripToolCallEnvelopes } from '../../../utils/toolTimelineFormatting'; import { parseWorkerThreadRef } from '../utils/workerThreadRef'; import { agentNameTone, AgentTimelineRail } from './AgentTimelineRail'; -import { AssistantUiSubagentCall } from './AssistantUiSubagentCall'; +import { AssistantUiSubagentCall, isActiveSubagentStatus } from './AssistantUiSubagentCall'; import { ProcessingTranscriptView } from './ProcessingTranscriptView'; import { coalesceTimelineEntries, @@ -421,7 +421,7 @@ export function ToolTimelineBlock({ renderSubagent={subagent => ( onViewSubagent(subagent) : undefined} /> )} diff --git a/app/src/features/conversations/components/__tests__/ToolTimelineBlock.test.tsx b/app/src/features/conversations/components/__tests__/ToolTimelineBlock.test.tsx index 6939bacb0b..4c68fa44cd 100644 --- a/app/src/features/conversations/components/__tests__/ToolTimelineBlock.test.tsx +++ b/app/src/features/conversations/components/__tests__/ToolTimelineBlock.test.tsx @@ -27,6 +27,34 @@ function renderInStore(ui: React.ReactNode) { } describe('SubagentActivityBlock', () => { + it('derives its lifecycle from the activity when no running prop is passed', () => { + // Most call sites (AgentProcessSourcePanel, PastTurnInsights, this block) + // pass no `running` prop at all. The old `running = false` default reported + // an in-flight delegation as finished, with a success check. + renderInStore( + + ); + + expect(screen.getByText('running')).toBeInTheDocument(); + }); + + it('marks a failed delegation as failed rather than complete', () => { + renderInStore( + + ); + + expect(screen.getByTestId('assistant-ui-subagent-call')).toHaveAttribute( + 'data-status', + 'failed' + ); + expect(screen.getByText('failed')).toBeInTheDocument(); + expect(screen.queryByText('running')).not.toBeInTheDocument(); + }); + it('renders mode + dedicated-thread + child-turn pills', () => { renderInStore( { expect(screen.getAllByTestId('processing-tool-row').length).toBeGreaterThan(0); // The sub-agent's own reasoning trail renders beneath. const subagents = screen.getByTestId('past-turn-subagents'); - fireEvent.click(screen.getByTestId('assistant-ui-subagent-call').querySelector('button')!); + // Assert the trigger before clicking: a bare `querySelector('button')!` + // throws an opaque TypeError at the click, pointing the stack at the click + // rather than at the element that was never rendered. + const subagentTrigger = screen + .getByTestId('assistant-ui-subagent-call') + .querySelector('button'); + expect(subagentTrigger).not.toBeNull(); + fireEvent.click(subagentTrigger as HTMLElement); expect(subagents.textContent).toContain('child reasoning trail'); }); }); diff --git a/app/src/providers/__tests__/CoreStateProvider.test.tsx b/app/src/providers/__tests__/CoreStateProvider.test.tsx index c48f850ed3..9204fb0162 100644 --- a/app/src/providers/__tests__/CoreStateProvider.test.tsx +++ b/app/src/providers/__tests__/CoreStateProvider.test.tsx @@ -567,7 +567,10 @@ describe('CoreStateProvider — identity-change cache clearing', () => { }, }) ); - await Promise.resolve(); + // Drain past the microtask queue: the handler may `await` before it can + // reach `logout`, and a single `Promise.resolve()` tick would let this + // assertion pass without that path having had a chance to run. + await new Promise(resolve => setTimeout(resolve, 0)); }); expect(vi.mocked(tauriCommands.logout)).not.toHaveBeenCalled(); diff --git a/app/src/providers/__tests__/assistantUiMessages.test.ts b/app/src/providers/__tests__/assistantUiMessages.test.ts index a8b891c8ed..0d0974b1ec 100644 --- a/app/src/providers/__tests__/assistantUiMessages.test.ts +++ b/app/src/providers/__tests__/assistantUiMessages.test.ts @@ -353,3 +353,71 @@ describe('buildRuntimeMessages', () => { parse.mockRestore(); }); }); + +describe('recovered tool names', () => { + it('does not consume a recovered name on an entry it does not rename', () => { + // `recoveredNames` comes from tool-call envelopes, so it only ever holds + // names for the *generic* rows. Advancing the cursor on every entry made a + // named row eat the first recovered name: the first generic row then took + // the second name and the last one kept the placeholder. + const converted = toThreadMessageLike( + msg({ + id: 'a', + sender: 'agent', + content: 'done', + extraMetadata: { assistantUiToolNames: ['web_search', 'web_fetch'] }, + }), + [ + tool({ id: 'c1', name: 'read_file', seq: 0, status: 'ok' }), + tool({ id: 'c2', name: 'tool', seq: 1, status: 'ok' }), + tool({ id: 'c3', name: 'tool', seq: 2, status: 'ok' }), + ] + ); + const names = (converted.content as { type: string; toolName?: string }[]) + .filter(part => part.type === 'tool-call') + .map(part => part.toolName); + expect(names).toEqual(['read_file', 'web_search', 'web_fetch']); + }); +}); + +describe('terminal tool status', () => { + it('carries a failed tool status through to the rendered part', () => { + // assistant-ui's tool-call part has no status field, so a failed tool that + // produced output used to arrive as a bare result and read as success. + const converted = toThreadMessageLike(msg({ id: 'a', sender: 'agent', content: 'done' }), [ + tool({ id: 'c1', name: 'web_search', seq: 0, status: 'error', result: 'boom' }), + ]); + const part = (converted.content as { type: string; result?: unknown }[]).find( + candidate => candidate.type === 'tool-call' + ); + expect(part?.result).toMatchObject({ status: 'error', value: 'boom' }); + }); + + it('leaves a successful tool result untouched', () => { + const converted = toThreadMessageLike(msg({ id: 'a', sender: 'agent', content: 'done' }), [ + tool({ id: 'c1', name: 'web_search', seq: 0, status: 'ok', result: 'the answer' }), + ]); + const part = (converted.content as { type: string; result?: unknown }[]).find( + candidate => candidate.type === 'tool-call' + ); + expect(part?.result).toBe('the answer'); + }); +}); + +describe('narration merged into the final answer', () => { + it('does not render narration that the final text already contains', () => { + // `mergedAssistantText` prefers the longest text when it *contains* every + // segment, so the duplicate is a substring rather than an exact match and + // the old equality guard let it through twice. + const finalText = 'I will check the sources. Here is what I found.'; + const converted = toThreadMessageLike( + msg({ id: 'a', sender: 'agent', content: finalText }), + [], + [{ kind: 'narration', round: 1, seq: 0, text: 'I will check the sources.' }] + ); + const texts = (converted.content as { type: string; text?: string }[]) + .filter(part => part.type === 'text') + .map(part => part.text); + expect(texts).toEqual([finalText]); + }); +}); diff --git a/app/src/providers/assistantUiMessages.ts b/app/src/providers/assistantUiMessages.ts index 31a0dd7551..ad39165b82 100644 --- a/app/src/providers/assistantUiMessages.ts +++ b/app/src/providers/assistantUiMessages.ts @@ -78,6 +78,30 @@ function toolArgs(entry: ToolTimelineEntry): Record { } } +/** + * The `result` payload for a settled non-sub-agent tool part. + * + * assistant-ui's tool-call part has no status field, so a terminal status has + * to travel inside `result` or not at all. It did not travel: the adapter fell + * back to `result !== undefined`, which reads as success, and a failed or + * cancelled tool rendered with a "done" label and a check. + * + * A tool that produced no output already reported `{ status, failure }` here, + * so only the failed-*with*-output case needed a shape — `value` carries the + * real output beside the status, and {@link isToolStatusEnvelope} unwraps it. + * The success path is byte-identical to before, deliberately: every reader of + * a successful result keeps seeing exactly what it saw. + */ +function toolResultPayload(entry: ToolTimelineEntry): unknown { + const terminalFailure = entry.status === 'error' || entry.status === 'cancelled'; + if (!terminalFailure) return entry.result ?? { status: entry.status, failure: entry.failure }; + return { + status: entry.status, + failure: entry.failure, + ...(entry.result !== undefined ? { value: entry.result } : {}), + }; +} + function toolPart(entry: ToolTimelineEntry): ThreadAssistantMessagePart { const running = entry.status === 'running' || entry.status === 'awaiting_user'; const isSubagent = entry.name.startsWith('subagent:') || entry.subagent !== undefined; @@ -99,7 +123,7 @@ function toolPart(entry: ToolTimelineEntry): ThreadAssistantMessagePart { ? { result: isSubagent ? (entry.subagent ?? { status: entry.status }) - : (entry.result ?? { status: entry.status, failure: entry.failure }), + : toolResultPayload(entry), } : {}), }; @@ -145,12 +169,15 @@ function assistantParts( if ( item.kind === 'narration' && item.text.trim().length > 0 && - item.text.trim() !== text.trim() + !text.trim().includes(item.text.trim()) ) { // Narration emitted before a tool call is assistant content in its own // right. Keep it inline in assistant-ui's ordered part stream; the final // answer is appended separately below. A final-round narration is the - // same streamed bytes as that answer and must not render twice. + // same streamed bytes as that answer and must not render twice — + // `includes`, not equality, because `mergedAssistantText` prefers the + // longest text when it *contains* every segment, so the duplicate is a + // substring rather than an exact match. parts.push({ type: 'text', text: item.text }); } } @@ -186,11 +213,18 @@ function recoverTimelineToolNames( if (recoveredNames.length === 0 || !timeline.some(entry => isGenericToolName(entry.name))) { return timeline; } + // Advance only when a name is actually consumed. `recoveredNames` comes from + // tool-call envelopes, so it is not positionally aligned with the whole + // timeline: incrementing on every entry made `[read_file, tool, tool]` + + // `[web_search, web_fetch]` mis-assign `web_fetch` to the first generic row + // and leave the second one named `tool`. let recoveredIndex = 0; return timeline.map(entry => { + if (!isGenericToolName(entry.name)) return entry; const recovered = recoveredNames[recoveredIndex]; + if (!recovered) return entry; recoveredIndex += 1; - return recovered && isGenericToolName(entry.name) ? { ...entry, name: recovered } : entry; + return { ...entry, name: recovered }; }); } diff --git a/app/test/playwright/specs/chat-scroll-stability.spec.ts b/app/test/playwright/specs/chat-scroll-stability.spec.ts index 4195bbf9d4..9f0a2bda7b 100644 --- a/app/test/playwright/specs/chat-scroll-stability.spec.ts +++ b/app/test/playwright/specs/chat-scroll-stability.spec.ts @@ -88,7 +88,10 @@ test.describe('Chat scroll paint stability', () => { }; }, fraction); expect(metrics).toEqual(initial); - await expect(page.getByText('Loading conversation…')).toHaveCount(0); + // The surface renders `Loading conversation` with no ellipsis; the old + // locator matched nothing, so `toHaveCount(0)` passed without ever + // checking the loading element. + await expect(page.getByText('Loading conversation')).toHaveCount(0); } expect(transcriptRpcCount).toBe(rpcCountBeforeScroll); }); diff --git a/app/test/playwright/specs/chat-tool-call-flow.spec.ts b/app/test/playwright/specs/chat-tool-call-flow.spec.ts index f5c1d96411..5cec18ff95 100644 --- a/app/test/playwright/specs/chat-tool-call-flow.spec.ts +++ b/app/test/playwright/specs/chat-tool-call-flow.spec.ts @@ -191,7 +191,7 @@ test.describe('Chat Tool Call Flow', () => { await expect(toolCard).toBeVisible(); await expect(toolCard).toContainText('Fetched from the web'); await expect(toolCard).not.toContainText('running'); - const toolTrigger = toolCard.getByRole('button'); + const toolTrigger = toolCard.getByRole('button').first(); if ((await toolTrigger.getAttribute('aria-expanded')) !== 'true') await toolTrigger.click(); await expect(toolCard.getByText('Output', { exact: true })).toBeVisible(); await expect(toolCard.getByRole('link', { name: 'https://example.com/' })).toBeVisible(); diff --git a/src/openhuman/agent/harness/session/transcript_part_02.rs b/src/openhuman/agent/harness/session/transcript_part_02.rs index 2066c4c278..8737aab9b2 100644 --- a/src/openhuman/agent/harness/session/transcript_part_02.rs +++ b/src/openhuman/agent/harness/session/transcript_part_02.rs @@ -464,7 +464,16 @@ pub fn find_root_transcripts_for_thread( for raw_dir in raw_session_dirs(workspace_dir) { matches.extend(root_transcripts_for_thread_in_dir(&raw_dir, thread_id)); } - matches.sort_by(|left, right| left.file_name().cmp(&right.file_name())); + // Already chronological within each directory (see + // `root_transcripts_for_thread_in_dir`); re-key across directories on the + // same `created` stamp rather than the file name. + matches.sort_by_cached_key(|path| { + let created = read_transcript(path) + .ok() + .map(|transcript| transcript.meta.created) + .unwrap_or_default(); + (created, path.clone()) + }); matches } @@ -499,7 +508,14 @@ fn root_transcripts_for_thread_in_dir(raw_dir: &Path, thread_id: &str) -> Vec = entries + // Keyed by `meta.created` so the order is chronological rather than + // lexicographic. Modern stems are `{unix_ts}_{agent_id}` and sort the same + // either way, but a legacy `{agent}_{index}` root encodes no time at all — + // and because digits sort before letters, every legacy root sorted *after* + // every modern one regardless of when it was written. `project_from_files` + // concatenates these in order, so that reordered the rendered view and + // could attach a sub-agent trail to the wrong turn. + let mut matches: Vec<(String, PathBuf)> = entries .flatten() .map(|entry| entry.path()) .filter(|path| { @@ -509,20 +525,25 @@ fn root_transcripts_for_thread_in_dir(raw_dir: &Path, thread_id: &str) -> Vec transcript.meta.thread_id.as_deref() == Some(thread_id), + .filter_map(|path| match read_transcript(&path) { + Ok(transcript) if transcript.meta.thread_id.as_deref() == Some(thread_id) => { + Some((transcript.meta.created.clone(), path)) + } + Ok(_) => None, Err(err) => { log::warn!( "[transcript] skipping unreadable root transcript candidate {}: {err}", path.display() ); - false + None } }) .collect(); - matches.sort(); - matches + // Path is the tiebreak so the order stays total and deterministic when two + // transcripts share a `created` stamp. + matches.sort_by(|left, right| left.0.cmp(&right.0).then_with(|| left.1.cmp(&right.1))); + matches.into_iter().map(|(_, path)| path).collect() } /// Aggregated token/cost usage for a chat thread, summed across **all** of the diff --git a/src/openhuman/agent/harness/session/transcript_tests_part_03_tests.rs b/src/openhuman/agent/harness/session/transcript_tests_part_03_tests.rs index 5baf739b9e..96f80b02a6 100644 --- a/src/openhuman/agent/harness/session/transcript_tests_part_03_tests.rs +++ b/src/openhuman/agent/harness/session/transcript_tests_part_03_tests.rs @@ -53,3 +53,58 @@ fn meta_version_stamped_and_optional() { "append writer must stamp the schema version on the meta header" ); } + +/// Root transcripts must come back in the order they were written, not in +/// filename order. +/// +/// Modern stems are `{unix_ts}_{agent}` and sort identically either way, but a +/// legacy `{agent}_{index}` root encodes no time at all — and because digits +/// sort before letters, every legacy root sorted *after* every modern one no +/// matter when it was written. `project_from_files` concatenates these in +/// order, so a mis-ordered list reorders the rendered view and can attach a +/// sub-agent trail to the wrong turn. +#[test] +fn root_transcripts_are_ordered_by_creation_not_file_name() { + let dir = TempDir::new().unwrap(); + let raw = dir.path().join("session_raw"); + std::fs::create_dir_all(&raw).unwrap(); + + let write_root = |stem: &str, created: &str| { + let mut meta = sample_meta(); + meta.thread_id = Some("thread-order".into()); + meta.created = created.into(); + write_transcript( + &raw.join(format!("{stem}.jsonl")), + &sample_messages(), + &meta, + None, + ) + .unwrap(); + }; + + // The legacy root is the OLDEST, but `orchestrator_1` sorts after any + // digit-led stem, so a filename sort puts it last. + write_root("orchestrator_1", "2026-04-10T09:00:00Z"); + write_root("1776211200_orchestrator", "2026-04-11T09:00:00Z"); + write_root("1776297600_orchestrator", "2026-04-12T09:00:00Z"); + + let ordered: Vec = find_root_transcripts_for_thread(dir.path(), "thread-order") + .into_iter() + .map(|path| { + path.file_stem() + .and_then(|stem| stem.to_str()) + .unwrap_or_default() + .to_string() + }) + .collect(); + + assert_eq!( + ordered, + vec![ + "orchestrator_1".to_string(), + "1776211200_orchestrator".to_string(), + "1776297600_orchestrator".to_string(), + ], + "roots must be chronological; a filename sort would put the legacy root last" + ); +} From da5c45de376be910db92213c4dc3636276af58c6 Mon Sep 17 00:00:00 2001 From: M3gA-Mind Date: Wed, 2 Sep 2026 12:09:36 +0530 Subject: [PATCH 16/23] fix(chat): keep a cancelled tool off the success icon, and type the new tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two follow-ups on the review of the previous commit. `AssistantUiToolCall` gated its non-success icon on `status === 'error'` alone. That was unreachable before the adapter started forwarding a status, so it had never been wrong; now a cancelled call reaches the card and rendered a check icon beside the word "cancelled". The icon question is wider than the failure-explanation block, which only an `error` carries, so `failed` still gates that block and a separate `terminalNonSuccess` gates the icon. The tests added in the previous commit also failed `tsc --noEmit`: `'ok'` is not a `ToolTimelineEntryStatus` (the settled-success value is `'success'`), and narrowing `ThreadMessageLike['content']` needs to go via `unknown` because assistant-ui's part union does not overlap the narrowed shape. Caught by CI, not locally, because `tsc -p app/tsconfig.json` from the repo root does not resolve the test files the way `tsc --noEmit` run from `app/` does — the latter is what CI runs and what I checked against this time. --- .../components/AssistantUiToolCall.tsx | 7 ++++- .../components/ChatToolParts.test.tsx | 27 +++++++++++++++++++ .../__tests__/assistantUiMessages.test.ts | 16 +++++------ 3 files changed, 41 insertions(+), 9 deletions(-) diff --git a/app/src/features/conversations/components/AssistantUiToolCall.tsx b/app/src/features/conversations/components/AssistantUiToolCall.tsx index dd0dfb19bd..4cf79e6bde 100644 --- a/app/src/features/conversations/components/AssistantUiToolCall.tsx +++ b/app/src/features/conversations/components/AssistantUiToolCall.tsx @@ -143,6 +143,11 @@ export function AssistantUiToolCallCard({ ? 'running' : 'done'; const failed = status === 'error'; + // `failed` gates the failure-explanation block, which only an `error` carries. + // The icon is a wider question: a cancelled call did not succeed either, and + // before the adapter forwarded a status this branch was unreachable, so the + // check icon sat next to the word "cancelled". + const terminalNonSuccess = failed || status === 'cancelled'; return ( {running ? ( - ) : failed ? ( + ) : terminalNonSuccess ? ( ) : ( diff --git a/app/src/features/conversations/components/ChatToolParts.test.tsx b/app/src/features/conversations/components/ChatToolParts.test.tsx index 9fba803e40..1aa5c2dd93 100644 --- a/app/src/features/conversations/components/ChatToolParts.test.tsx +++ b/app/src/features/conversations/components/ChatToolParts.test.tsx @@ -113,6 +113,33 @@ describe('ChatToolParts', () => { expect(screen.getByText('running')).toBeInTheDocument(); }); + it('does not show a success icon beside a cancelled tool', () => { + // The adapter forwards `cancelled` now, and the card gated its non-success + // icon on `error` alone — so the check icon sat next to the word + // "cancelled". `failed` still gates the failure-explanation block, which + // only an `error` carries. + const { container } = render( + {}} + resume={() => {}} + respondToApproval={() => {}} + /> + ); + + expect(screen.getByText('cancelled')).toBeInTheDocument(); + const card = screen.getByTestId('assistant-ui-tool-call'); + expect(card.querySelector('.lucide-circle-x')).not.toBeNull(); + expect(card.querySelector('.lucide-check')).toBeNull(); + expect(container).toBeTruthy(); + }); + it('opens a group containing in-flight work on mount', () => { render( diff --git a/app/src/providers/__tests__/assistantUiMessages.test.ts b/app/src/providers/__tests__/assistantUiMessages.test.ts index 0d0974b1ec..0f6252986a 100644 --- a/app/src/providers/__tests__/assistantUiMessages.test.ts +++ b/app/src/providers/__tests__/assistantUiMessages.test.ts @@ -368,12 +368,12 @@ describe('recovered tool names', () => { extraMetadata: { assistantUiToolNames: ['web_search', 'web_fetch'] }, }), [ - tool({ id: 'c1', name: 'read_file', seq: 0, status: 'ok' }), - tool({ id: 'c2', name: 'tool', seq: 1, status: 'ok' }), - tool({ id: 'c3', name: 'tool', seq: 2, status: 'ok' }), + tool({ id: 'c1', name: 'read_file', seq: 0, status: 'success' }), + tool({ id: 'c2', name: 'tool', seq: 1, status: 'success' }), + tool({ id: 'c3', name: 'tool', seq: 2, status: 'success' }), ] ); - const names = (converted.content as { type: string; toolName?: string }[]) + const names = (converted.content as unknown as { type: string; toolName?: string }[]) .filter(part => part.type === 'tool-call') .map(part => part.toolName); expect(names).toEqual(['read_file', 'web_search', 'web_fetch']); @@ -387,7 +387,7 @@ describe('terminal tool status', () => { const converted = toThreadMessageLike(msg({ id: 'a', sender: 'agent', content: 'done' }), [ tool({ id: 'c1', name: 'web_search', seq: 0, status: 'error', result: 'boom' }), ]); - const part = (converted.content as { type: string; result?: unknown }[]).find( + const part = (converted.content as unknown as { type: string; result?: unknown }[]).find( candidate => candidate.type === 'tool-call' ); expect(part?.result).toMatchObject({ status: 'error', value: 'boom' }); @@ -395,9 +395,9 @@ describe('terminal tool status', () => { it('leaves a successful tool result untouched', () => { const converted = toThreadMessageLike(msg({ id: 'a', sender: 'agent', content: 'done' }), [ - tool({ id: 'c1', name: 'web_search', seq: 0, status: 'ok', result: 'the answer' }), + tool({ id: 'c1', name: 'web_search', seq: 0, status: 'success', result: 'the answer' }), ]); - const part = (converted.content as { type: string; result?: unknown }[]).find( + const part = (converted.content as unknown as { type: string; result?: unknown }[]).find( candidate => candidate.type === 'tool-call' ); expect(part?.result).toBe('the answer'); @@ -415,7 +415,7 @@ describe('narration merged into the final answer', () => { [], [{ kind: 'narration', round: 1, seq: 0, text: 'I will check the sources.' }] ); - const texts = (converted.content as { type: string; text?: string }[]) + const texts = (converted.content as unknown as { type: string; text?: string }[]) .filter(part => part.type === 'text') .map(part => part.text); expect(texts).toEqual([finalText]); From 145c7a452befc8f67eac5e929262ff669a805a80 Mon Sep 17 00:00:00 2001 From: Shanu Date: Wed, 2 Sep 2026 22:35:33 +0530 Subject: [PATCH 17/23] fix(chat): keep scoped deliveries standalone and pair orphan trails only as a bijection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two review findings on the assistant-ui projection, both about rows that carry no request id. Coalescing: a background/autonomous delivery persisted by the core has no request id, exactly like a legacy answer segment, so an adjacent pair merged into one bubble with the delivery's text concatenated and its metadata overwriting the earlier row. Core writers stamp `extraMetadata.scope` on every such delivery and the legacy segmented path never did, so that marker is the positive signal: a scoped row is always its own turn and neither joins the run before it nor seeds the run after it. Orphan trails: positional pairing of unclaimed per-request trails with unanchored agent messages mis-attributes whenever there are more messages than trails — an earlier trail-less answer consumed a later tool-using answer's trail and the real answer rendered bare. Pair only when the two sets are the same size; otherwise render the trail nowhere, which is the lesser wrong. Timestamp correlation would attach those too, but needs the turn boundary to carry a timestamp on the wire. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_012jp2WUNkZ8dxDVmDtu3JxX --- .../__tests__/assistantUiMessages.test.ts | 78 +++++++++++++++++++ app/src/providers/assistantUiMessages.ts | 45 +++++++++-- 2 files changed, 117 insertions(+), 6 deletions(-) diff --git a/app/src/providers/__tests__/assistantUiMessages.test.ts b/app/src/providers/__tests__/assistantUiMessages.test.ts index 0f6252986a..4a409046e9 100644 --- a/app/src/providers/__tests__/assistantUiMessages.test.ts +++ b/app/src/providers/__tests__/assistantUiMessages.test.ts @@ -279,6 +279,84 @@ describe('buildRuntimeMessages', () => { expect(projected.map(message => message.id)).toEqual(['first', 'second']); }); + it('keeps a scoped standalone delivery out of the adjacent legacy runs', () => { + // Legacy segments carry no request id; a background delivery persisted by + // the core carries no request id either but is stamped with a `scope`. + // Without the marker the three rows read as one segmented answer and the + // delivery's text and metadata would be folded into its neighbours. + const projected = buildRuntimeMessages( + [ + msg({ id: 'seg-a', sender: 'agent', content: 'first paragraph' }), + msg({ + id: 'delivery', + sender: 'agent', + content: 'Background result: inbox digest', + extraMetadata: { scope: 'autonomous_task_result', success: true }, + }), + msg({ id: 'seg-b', sender: 'agent', content: 'a later paragraph' }), + ], + null + ); + + expect(projected.map(message => message.id)).toEqual(['seg-a', 'delivery', 'seg-b']); + expect(projected[1]?.content).toEqual([ + { type: 'text', text: 'Background result: inbox digest' }, + ]); + }); + + it('does not let a scoped delivery absorb the identified segment before it', () => { + const projected = buildRuntimeMessages( + [ + msg({ + id: 'answer', + sender: 'agent', + content: 'answer', + extraMetadata: { requestId: 'r1' }, + }), + msg({ + id: 'delivery', + sender: 'agent', + content: 'worker output', + extraMetadata: { scope: 'worker_thread', requestId: 'r-worker' }, + }), + ], + null + ); + + expect(projected.map(message => message.id)).toEqual(['answer', 'delivery']); + }); + + it('does not hand a later turn trail to an earlier trail-less answer', () => { + // Two unanchored answers, one unclaimed trail. Positional pairing would + // give the trail to `earlier` (which produced nothing) and leave `later` + // (which actually used the tool) bare — a wrong attribution, not a loss. + const projected = buildRuntimeMessages( + [ + msg({ id: 'ask-1', content: 'first question' }), + msg({ id: 'earlier', sender: 'agent', content: 'plain answer', extraMetadata: {} }), + msg({ id: 'ask-2', content: 'second question' }), + msg({ id: 'later', sender: 'agent', content: 'tool answer', extraMetadata: {} }), + ], + null, + { + isRunning: false, + turnTimelines: { 'request-later': [tool({ id: 'later-tool', status: 'success' })] }, + turnTranscripts: { + 'request-later': [{ kind: 'toolCall', round: 1, seq: 0, callId: 'later-tool' }], + }, + } + ); + + const toolBearing = projected + .filter( + message => + Array.isArray(message.content) && message.content.some(part => part.type === 'tool-call') + ) + .map(message => message.id); + expect(toolBearing).not.toContain('earlier'); + expect(projected[1]?.content).toEqual([{ type: 'text', text: 'plain answer' }]); + }); + /** * The crash this guards: assistant-ui keys tool parts as `toolCallId-${id}` * and throws "Duplicate key … in useResources" on a repeat, taking the whole diff --git a/app/src/providers/assistantUiMessages.ts b/app/src/providers/assistantUiMessages.ts index ad39165b82..a261c8a415 100644 --- a/app/src/providers/assistantUiMessages.ts +++ b/app/src/providers/assistantUiMessages.ts @@ -262,12 +262,26 @@ function mergeAssistantRun(messages: readonly ThreadMessage[]): ThreadMessage { }; } +/** + * A row the core persisted as a complete, self-contained delivery — an + * autonomous task result, a worker-thread hand-off, a workflow proposal. Core + * writers stamp `extraMetadata.scope` on every such row; the legacy segmented + * path never did. That positive marker is what tells an async delivery apart + * from a paragraph of the answer next to it, since neither carries a request + * id on the wire. + */ +function isStandaloneDelivery(message: ThreadMessage): boolean { + return typeof message.extraMetadata?.scope === 'string'; +} + /** * Collapse legacy paragraph/tool-envelope rows into one assistant turn. * * The old interactive-web delivery path persisted each segment as a separate * agent message. Consecutive assistant rows cannot cross a user turn; when - * both rows carry request ids, a differing id is the explicit boundary. + * both rows carry request ids, a differing id is the explicit boundary, and a + * scoped standalone delivery ({@link isStandaloneDelivery}) is always its own + * turn — it neither joins the run before it nor seeds the run after it. */ function coalesceAssistantSegments(messages: readonly ThreadMessage[]): ThreadMessage[] { const out: ThreadMessage[] = []; @@ -286,6 +300,12 @@ function coalesceAssistantSegments(messages: readonly ThreadMessage[]): ThreadMe out.push(message); continue; } + if (isStandaloneDelivery(message)) { + flush(); + run.push(message); + flush(); + continue; + } const requestId = requestIdOf(message); if (run.length > 0 && runRequestId && requestId && runRequestId !== requestId) flush(); run.push(message); @@ -437,6 +457,21 @@ export function buildRuntimeMessages( ...Object.keys(projection.turnTranscripts ?? {}), ]), ].filter(requestId => !claimedRequestIds.has(requestId)); + // Async acknowledgements/background deliveries can be persisted without + // message-level request metadata. The transcript maps are chronological and + // request-keyed, so unclaimed trails can be paired with unanchored agent + // messages in order — but only when the two sets are the same size. With a + // surplus of unanchored messages, positional pairing hands a later turn's + // tools to an earlier trail-less answer and leaves the real answer bare; + // rendering those trails nowhere is the lesser wrong. + const unanchoredAgentCount = coalescedMessages.filter( + message => + message.sender === 'agent' && + !message.extraMetadata?.hidden && + typeof message.extraMetadata?.requestId !== 'string' + ).length; + const pairOrphanTrails = + projectedRequestIds.length > 0 && projectedRequestIds.length === unanchoredAgentCount; let orphanRequestCursor = 0; const lastVisibleAgentId = [...coalescedMessages] .reverse() @@ -447,13 +482,11 @@ export function buildRuntimeMessages( msg.sender === 'agent' && typeof msg.extraMetadata?.requestId === 'string' ? msg.extraMetadata.requestId : undefined; - // Async acknowledgements/background deliveries can be persisted without - // message-level request metadata. The transcript maps are chronological - // and request-keyed, so pair only unclaimed trails with unanchored agent - // messages in the same order instead of dropping them from assistant-ui. const effectiveRequestId = requestId ?? - (msg.sender === 'agent' ? projectedRequestIds[orphanRequestCursor++] : undefined); + (msg.sender === 'agent' && pairOrphanTrails + ? projectedRequestIds[orphanRequestCursor++] + : undefined); const persistedTimeline = effectiveRequestId ? projection.turnTimelines?.[effectiveRequestId] : undefined; From b83d1aef51d08d910e4f08a1a2ffaa402c949b28 Mon Sep 17 00:00:00 2001 From: Shanu Date: Wed, 2 Sep 2026 22:35:33 +0530 Subject: [PATCH 18/23] fix(chat): page through the derived transcript instead of stopping at 500 items The core transcript projection fetched one page of 500 items and ignored `hasMore`, so a long thread silently lost its older reasoning, narration, tool calls and delegated activity, and a page that began mid-turn hid that turn's leading tool calls until its boundary was in view. Paint the newest page immediately, then walk the older pages through `nextCursor` and re-project once with the whole list (newest-first order is preserved by appending). Bounded at 20 pages; the turn-bounded RPC contract that removes the ceiling belongs with the transcript RPC. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_012jp2WUNkZ8dxDVmDtu3JxX --- .../useCoreTranscriptProjection.test.tsx | 79 +++++++++++++++++++ .../providers/useOpenHumanExternalStore.ts | 56 ++++++++++--- 2 files changed, 125 insertions(+), 10 deletions(-) create mode 100644 app/src/providers/__tests__/useCoreTranscriptProjection.test.tsx diff --git a/app/src/providers/__tests__/useCoreTranscriptProjection.test.tsx b/app/src/providers/__tests__/useCoreTranscriptProjection.test.tsx new file mode 100644 index 0000000000..412e3da815 --- /dev/null +++ b/app/src/providers/__tests__/useCoreTranscriptProjection.test.tsx @@ -0,0 +1,79 @@ +import { renderHook, waitFor } from '@testing-library/react'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import { threadApi } from '../../services/api/threadApi'; +import type { DerivedDisplayItem, DerivedTranscriptPage } from '../../types/derivedTranscript'; +import { useCoreTranscriptProjection } from '../useOpenHumanExternalStore'; + +vi.mock('../../services/api/threadApi', () => ({ threadApi: { getDerivedTranscript: vi.fn() } })); + +const THREAD = 'thread-long'; + +/** Newest-first items for one settled turn: a tool call after its boundary. */ +function turn(requestId: string, callId: string): DerivedDisplayItem[] { + return [ + { kind: 'toolCall', callId, name: 'web_search_tool', status: 'success', result: 'ok' }, + { kind: 'assistantMessage', content: `answer ${requestId}`, requestId, iteration: 1 }, + { kind: 'turnBoundary', requestId }, + ]; +} + +function page(over: Partial): DerivedTranscriptPage { + return { threadId: THREAD, items: [], total: 0, hasMore: false, hasTranscript: true, ...over }; +} + +describe('useCoreTranscriptProjection paging', () => { + beforeEach(() => { + vi.mocked(threadApi.getDerivedTranscript).mockReset(); + }); + + it('walks every older page and projects the whole history', async () => { + vi.mocked(threadApi.getDerivedTranscript) + .mockResolvedValueOnce( + page({ items: turn('r-new', 'call-new'), total: 6, hasMore: true, nextCursor: 'c1' }) + ) + .mockResolvedValueOnce(page({ items: turn('r-old', 'call-old'), total: 6, hasMore: false })); + + const { result } = renderHook(() => useCoreTranscriptProjection(THREAD, 'rev-1', undefined)); + + await waitFor(() => expect(Object.keys(result.current.timelines)).toHaveLength(2)); + expect(threadApi.getDerivedTranscript).toHaveBeenCalledTimes(2); + expect(threadApi.getDerivedTranscript).toHaveBeenLastCalledWith(THREAD, { + limit: 500, + cursor: 'c1', + }); + expect(result.current.timelines['r-old']?.[0]).toMatchObject({ id: 'call-old' }); + expect(result.current.timelines['r-new']?.[0]).toMatchObject({ id: 'call-new' }); + }); + + it('stops when the core reports no more pages', async () => { + vi.mocked(threadApi.getDerivedTranscript).mockResolvedValueOnce( + page({ items: turn('r-only', 'call-only'), total: 3, hasMore: false }) + ); + + const { result } = renderHook(() => useCoreTranscriptProjection(THREAD, 'rev-1', undefined)); + + await waitFor(() => expect(Object.keys(result.current.timelines)).toEqual(['r-only'])); + expect(threadApi.getDerivedTranscript).toHaveBeenCalledTimes(1); + }); + + it('drops a page that lands after the thread changed', async () => { + const older = new Promise(() => { + /* never resolves: the walk is still in flight when the thread switches */ + }); + vi.mocked(threadApi.getDerivedTranscript) + .mockResolvedValueOnce( + page({ items: turn('r-new', 'call-new'), total: 6, hasMore: true, nextCursor: 'c1' }) + ) + .mockReturnValueOnce(older); + + const { result, rerender } = renderHook( + ({ thread }) => useCoreTranscriptProjection(thread, 'rev-1', undefined), + { initialProps: { thread: THREAD as string | null } } + ); + await waitFor(() => expect(Object.keys(result.current.timelines)).toEqual(['r-new'])); + + rerender({ thread: null }); + expect(result.current.timelines).toEqual({}); + }); +}); diff --git a/app/src/providers/useOpenHumanExternalStore.ts b/app/src/providers/useOpenHumanExternalStore.ts index 92c88f80f4..47b88c1840 100644 --- a/app/src/providers/useOpenHumanExternalStore.ts +++ b/app/src/providers/useOpenHumanExternalStore.ts @@ -4,6 +4,7 @@ import { useCallback, useEffect, useMemo, useState } from 'react'; import { mapDisplayItems } from '../features/conversations/derived/mapDisplayItems'; import { threadApi } from '../services/api/threadApi'; import { useAppSelector } from '../store/hooks'; +import type { DerivedDisplayItem } from '../types/derivedTranscript'; import type { ThreadMessage } from '../types/thread'; import { buildRuntimeMessages } from './assistantUiMessages'; import { getChatSurface } from './chatSurfaceHandlers'; @@ -12,6 +13,15 @@ const EMPTY_MESSAGES: ThreadMessage[] = []; const EMPTY_TIMELINE: never[] = []; const EMPTY_TRANSCRIPT: never[] = []; const EMPTY_TURN_MAP = {}; +/** Items per derived-transcript RPC page; the core caps a page at this size. */ +const DERIVED_TRANSCRIPT_PAGE_LIMIT = 500; +/** + * Upper bound on pages walked for one thread (10k items). A thread longer than + * this is truncated at its oldest end rather than fetched without limit; the + * turn-bounded RPC contract that removes the ceiling altogether is tracked with + * the transcript RPC, not here. + */ +const DERIVED_TRANSCRIPT_MAX_PAGES = 20; type CoreTranscriptProjection = { threadId: string | null; @@ -49,25 +59,51 @@ export function useCoreTranscriptProjection( return; } let cancelled = false; - void threadApi - .getDerivedTranscript(threadId, { limit: 500 }) - .then(page => { + const skipRequestIds = liveRequestId ? new Set([liveRequestId]) : undefined; + const project = (items: DerivedDisplayItem[]) => { + const mapped = mapDisplayItems(items, { skipRequestIds }); + setProjection({ threadId, timelines: mapped.timelines, transcripts: mapped.transcripts }); + }; + void (async () => { + try { + const first = await threadApi.getDerivedTranscript(threadId, { + limit: DERIVED_TRANSCRIPT_PAGE_LIMIT, + }); if (cancelled) return; - if (!page.hasTranscript) { + if (!first.hasTranscript) { setProjection({ threadId, timelines: EMPTY_TURN_MAP, transcripts: EMPTY_TURN_MAP }); return; } - const skipRequestIds = liveRequestId ? new Set([liveRequestId]) : undefined; - const mapped = mapDisplayItems(page.items, { skipRequestIds }); - setProjection({ threadId, timelines: mapped.timelines, transcripts: mapped.transcripts }); - }) - .catch(() => { + // Paint the newest page immediately, then walk the older pages and + // re-project once with the whole history. A single page silently + // dropped everything older than 500 items on a long thread, and a + // page that begins mid-turn hides that turn's leading tool calls until + // its boundary is in view — both only resolve with the full list. + let items = first.items; + project(items); + let cursor = first.hasMore ? first.nextCursor : undefined; + let pages = 1; + while (cursor && pages < DERIVED_TRANSCRIPT_MAX_PAGES) { + const page = await threadApi.getDerivedTranscript(threadId, { + limit: DERIVED_TRANSCRIPT_PAGE_LIMIT, + cursor, + }); + if (cancelled) return; + // Pages are newest-first and each next page is older, so appending + // keeps the newest-first order `mapDisplayItems` expects. + items = [...items, ...page.items]; + pages += 1; + cursor = page.hasMore ? page.nextCursor : undefined; + } + if (pages > 1) project(items); + } catch { // A missing/older core has no settled process trail; message text and // the live socket projection remain usable. Navigation must not fail. if (!cancelled) { setProjection({ threadId, timelines: EMPTY_TURN_MAP, transcripts: EMPTY_TURN_MAP }); } - }); + } + })(); return () => { cancelled = true; }; From f456863fcb0b0444c392c2340c0ea5cf959533fa Mon Sep 17 00:00:00 2001 From: Shanu Date: Wed, 2 Sep 2026 22:35:33 +0530 Subject: [PATCH 19/23] fix(auth): do not clear the local-token marker on a pre-store snapshot `storeSessionToken` awaited `refresh()`, which dedupes onto any poll already in flight. A poll that began before `storeSession` resolved answers with the pre-store cloud snapshot; that answer was committed, the `finally` dropped the local-token marker on it, and a late confirmed 401 could then clear the local session that had just been stored. Wait the in-flight poll out, then require a refresh that began after the store committed before the marker clears. The regression test drives exactly that ordering and fails without the barrier (session token left at the stale cloud value). Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_012jp2WUNkZ8dxDVmDtu3JxX --- app/src/providers/CoreStateProvider.tsx | 9 ++++ .../__tests__/CoreStateProvider.test.tsx | 48 +++++++++++++++++++ 2 files changed, 57 insertions(+) diff --git a/app/src/providers/CoreStateProvider.tsx b/app/src/providers/CoreStateProvider.tsx index aabb4f2391..d534546f6f 100644 --- a/app/src/providers/CoreStateProvider.tsx +++ b/app/src/providers/CoreStateProvider.tsx @@ -707,6 +707,15 @@ export default function CoreStateProvider({ children }: { children: ReactNode }) // timeout post-login doesn't surface as an unhandled rejection // (OPENHUMAN-REACT-Z/Y) — the polling loop reconciles within // `POLL_MS`. + // `refresh()` dedupes onto any poll already in flight. A poll that + // began before `storeSession` resolved answers with the pre-store + // snapshot; awaiting that one here would commit stale cloud identity, + // then the `finally` below would drop the local-token marker on it and + // a late confirmed 401 could clear the session just stored. Wait the + // stale poll out, then require a refresh that began after the store. + if (refreshInFlightRef.current) { + await refreshInFlightRef.current.catch(() => undefined); + } await refresh().catch(err => { log('refresh failed after session store: %O', sanitizeError(err)); }); diff --git a/app/src/providers/__tests__/CoreStateProvider.test.tsx b/app/src/providers/__tests__/CoreStateProvider.test.tsx index 9204fb0162..d7b4ebf31b 100644 --- a/app/src/providers/__tests__/CoreStateProvider.test.tsx +++ b/app/src/providers/__tests__/CoreStateProvider.test.tsx @@ -578,6 +578,54 @@ describe('CoreStateProvider — identity-change cache clearing', () => { await act(async () => storing); }); + it('keeps a stored local session when a pre-store poll settles after the store', async () => { + const localToken = `eyJhbGciOiJub25lIn0.${window.btoa(JSON.stringify({ sub: 'local' }))}.local`; + const prestorePoll = deferred(); + // Bootstrap answers with the cloud identity, the poll started *before* the + // store is held open, and every poll after the store sees the local session. + fetchSnapshot + .mockResolvedValueOnce(makeSnapshot({ userId: 'cloud-user', sessionToken: 'old' })) + .mockReturnValueOnce(prestorePoll.promise as never) + .mockResolvedValue(makeSnapshot({ userId: 'local', sessionToken: localToken })); + listTeams.mockResolvedValue([]); + vi.mocked(tauriCommands.storeSession).mockReset(); + vi.mocked(tauriCommands.storeSession).mockResolvedValue(undefined as never); + vi.mocked(tauriCommands.logout).mockReset(); + vi.mocked(tauriCommands.logout).mockResolvedValue(undefined as never); + + let ctx: CoreStateContextValue | undefined; + render( + + (ctx = next)} /> + + ); + await waitFor(() => expect(screen.getByTestId('ready').textContent).toBe('ready')); + + let storing!: Promise; + await act(async () => { + // A poll is already in flight when the local session is stored... + void ctx!.refresh(); + storing = ctx!.storeSessionToken(localToken, { id: 'local' }); + await new Promise(resolve => setTimeout(resolve, 0)); + // ...and it settles with the stale cloud snapshot only after the store. + prestorePoll.resolve(makeSnapshot({ userId: 'cloud-user', sessionToken: 'old' })); + await storing; + }); + + expect(getCoreStateSnapshot().snapshot.sessionToken).toBe(localToken); + + await act(async () => { + window.dispatchEvent( + new CustomEvent('core-rpc-auth-expired', { + detail: { method: 'openhuman.team_get_usage', source: 'rpc', reason: 'confirmed' }, + }) + ); + await new Promise(resolve => setTimeout(resolve, 0)); + }); + + expect(vi.mocked(tauriCommands.logout)).not.toHaveBeenCalled(); + }); + it('dispatching core-rpc-auth-expired triggers clearSession (and debounces repeated fires within 10s)', async () => { fetchSnapshot.mockResolvedValue(makeSnapshot({ userId: 'u1', sessionToken: 'tok1' })); listTeams.mockResolvedValue([]); From 9097699a5db3bee6e2d545999caf3b8bff9aff21 Mon Sep 17 00:00:00 2001 From: Shanu Date: Wed, 2 Sep 2026 23:27:41 +0530 Subject: [PATCH 20/23] style(e2e): prettier-format five Playwright specs inherited from main MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Frontend Checks lane fails on this branch at `prettier --check` for five specs that arrived with the e2e backfills merged into main. They are unformatted on main itself — main's own Frontend Checks lane was skipped on those pushes — so this is the same whitespace-only fix main needs, applied here so the lane can run to the end. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_012jp2WUNkZ8dxDVmDtu3JxX --- .../specs/connections-tab-deeplinks.spec.ts | 11 ++--------- app/test/playwright/specs/core-rpc-bearer-401.spec.ts | 1 - .../playwright/specs/embeddings-setup-modal.spec.ts | 6 +++++- .../playwright/specs/settings-profiles-crud.spec.ts | 6 +----- .../playwright/specs/token-usage-load-failure.spec.ts | 6 +----- 5 files changed, 9 insertions(+), 21 deletions(-) diff --git a/app/test/playwright/specs/connections-tab-deeplinks.spec.ts b/app/test/playwright/specs/connections-tab-deeplinks.spec.ts index f4a5e89f9d..9eb94b9cc5 100644 --- a/app/test/playwright/specs/connections-tab-deeplinks.spec.ts +++ b/app/test/playwright/specs/connections-tab-deeplinks.spec.ts @@ -196,9 +196,7 @@ test.describe('Connections — the /skills back-compat redirect', () => { await expectSelectedTab(page, 'mcp'); }); - test('/skills?tab=mcp#fragment forwards the fragment as well as the query', async ({ - page, - }) => { + test('/skills?tab=mcp#fragment forwards the fragment as well as the query', async ({ page }) => { // `ForwardSearch` appends `hash` as well as `search`, and nothing here // exercised that half. `AppRoutes.skills.test.tsx` does assert // `loc.hash === '#section-mcp'`, but under `MemoryRouter`, which never @@ -207,12 +205,7 @@ test.describe('Connections — the /skills back-compat redirect', () => { // (`#/connections?tab=mcp#section-mcp`). That is the part only a browser // can answer, and it is the mechanism `/webhooks` relies on for // `#delivery-3`-style deep links. - await openRoute( - page, - 'pw-skills-hash-forward', - '/skills?tab=mcp#section-mcp', - '/connections' - ); + await openRoute(page, 'pw-skills-hash-forward', '/skills?tab=mcp#section-mcp', '/connections'); await expect.poll(() => currentHash(page), { timeout: 15_000 }).toContain('tab=mcp'); expect(await currentHash(page)).toContain('#section-mcp'); await expectSelectedTab(page, 'mcp'); diff --git a/app/test/playwright/specs/core-rpc-bearer-401.spec.ts b/app/test/playwright/specs/core-rpc-bearer-401.spec.ts index 57854a861b..eaabe1cae7 100644 --- a/app/test/playwright/specs/core-rpc-bearer-401.spec.ts +++ b/app/test/playwright/specs/core-rpc-bearer-401.spec.ts @@ -99,7 +99,6 @@ test.describe('Core RPC bearer 401 — recovery, not logout', () => { expect(seenAuth[1]).toContain(ROTATED); expect(seenAuth[1]).not.toEqual(seenAuth[0]); - // (3) The session survives. Without #5876 `clearSession()` wipes the auth // profile and the app falls back to the signed-out surface. Assert the // embeddings panel actually rendered rather than merely that the hash is diff --git a/app/test/playwright/specs/embeddings-setup-modal.spec.ts b/app/test/playwright/specs/embeddings-setup-modal.spec.ts index dfc2888621..bdd333c022 100644 --- a/app/test/playwright/specs/embeddings-setup-modal.spec.ts +++ b/app/test/playwright/specs/embeddings-setup-modal.spec.ts @@ -78,7 +78,11 @@ test.describe('Embeddings setup — Test connection for a custom endpoint', () = // and no unavailable-reason text is shown for a non-custom provider. await openEmbeddingsTab(page, 'pw-embed-openai'); - await page.getByRole('radio').filter({ hasText: /OpenAI/ }).first().click(); + await page + .getByRole('radio') + .filter({ hasText: /OpenAI/ }) + .first() + .click(); const testButton = page.getByRole('button', { name: TEST_CONNECTION }); await expect(testButton).toBeVisible({ timeout: 15_000 }); diff --git a/app/test/playwright/specs/settings-profiles-crud.spec.ts b/app/test/playwright/specs/settings-profiles-crud.spec.ts index fdbccf3247..2ddae4b777 100644 --- a/app/test/playwright/specs/settings-profiles-crud.spec.ts +++ b/app/test/playwright/specs/settings-profiles-crud.spec.ts @@ -248,11 +248,7 @@ test.describe('Agent profiles — a failing action shows the reason', () => { await route.fulfill({ status: 200, contentType: 'application/json', - body: JSON.stringify({ - jsonrpc: '2.0', - id: body.id, - error: { code: -32000, message }, - }), + body: JSON.stringify({ jsonrpc: '2.0', id: body.id, error: { code: -32000, message } }), }); return; } diff --git a/app/test/playwright/specs/token-usage-load-failure.spec.ts b/app/test/playwright/specs/token-usage-load-failure.spec.ts index e73eda9ff3..0a304506dc 100644 --- a/app/test/playwright/specs/token-usage-load-failure.spec.ts +++ b/app/test/playwright/specs/token-usage-load-failure.spec.ts @@ -43,11 +43,7 @@ async function failMethod(page: Page, method: string, message: string) { await route.fulfill({ status: 200, contentType: 'application/json', - body: JSON.stringify({ - jsonrpc: '2.0', - id: body.id, - error: { code: -32000, message }, - }), + body: JSON.stringify({ jsonrpc: '2.0', id: body.id, error: { code: -32000, message } }), }); return; } From 7d68ffe1372bf973371cbc31a9bebb9a6ddd8114 Mon Sep 17 00:00:00 2001 From: Shanu Date: Thu, 3 Sep 2026 00:20:21 +0530 Subject: [PATCH 21/23] test(observability): isolate the wallet paging tests on a private Sentry hub MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `paging::a_genuine_wallet_failure_still_pages` asserted one Sentry envelope for a real wallet error and got two, deterministically, under the product feature set — the only set in which the `paging` module compiles, which is why the contributor default run stays green. `captured_events_for` used `sentry::init`, which binds the client on the hub every test thread's hub is copied from. A sibling test's `report_error_or_expected` for the same genuine message, running on another thread outside the paging lock, therefore captured into the paging test's transport. Bind the client to a private hub that is current only inside `Hub::run` instead: nothing process-global is touched, the sibling's capture falls on a hub with no client, and the serialising lock is no longer needed. Verified under the product feature set: the full binary passes three parallel runs and one serial run; before the change every parallel run failed with `left: 2`. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_012jp2WUNkZ8dxDVmDtu3JxX --- tests/observability_wallet_expected_e2e.rs | 37 +++++++++++----------- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/tests/observability_wallet_expected_e2e.rs b/tests/observability_wallet_expected_e2e.rs index 640c33a458..222d21f28d 100644 --- a/tests/observability_wallet_expected_e2e.rs +++ b/tests/observability_wallet_expected_e2e.rs @@ -253,20 +253,17 @@ fn reporting_a_genuine_wallet_failure_still_emits_error() { #[cfg(feature = "crash-reporting")] mod paging { use super::*; - - /// Drive `report_error_or_expected` against an envelope-capturing Sentry - /// client and return how many events it actually sent. + /// Count the Sentry events one call to `report_error_or_expected` produces. /// - /// `sentry::init` mutates the process-global hub and cargo runs these - /// functions on parallel threads, so the critical section is serialized - /// here rather than by imposing `--test-threads=1` on the whole binary — - /// the same reasoning, and the same shape, as `observability_smoke.rs`. + /// The client is bound to a **private** hub that is current only inside + /// [`sentry::Hub::run`]. `sentry::init` instead binds it on the hub every + /// test thread's hub is copied from, so while a paging test held a client, + /// a sibling test's `report_error_or_expected` on another thread landed in + /// this transport too — `left: 2` for a genuine failure, deterministic + /// under the product feature set, invisible under the contributor default + /// set where this module does not compile. No process-global state is + /// touched, so the tests need no serialisation. fn captured_events_for(message: &str) -> usize { - static SENTRY_TEST_LOCK: Mutex<()> = Mutex::new(()); - let _guard = SENTRY_TEST_LOCK - .lock() - .unwrap_or_else(|poisoned| poisoned.into_inner()); - let transport = sentry::test::TestTransport::new(); let transport_for_factory = transport.clone(); let options = sentry::ClientOptions { @@ -281,13 +278,15 @@ mod paging { sample_rate: 1.0, ..sentry::ClientOptions::default() }; - let _sentry_guard = sentry::init(options); - - report_error_or_expected(message, "rpc", "invoke_method", &[]); - - sentry::Hub::current() - .client() - .map(|c| c.flush(Some(std::time::Duration::from_secs(2)))); + let client = Arc::new(sentry::Client::from_config(sentry::apply_defaults(options))); + let hub = Arc::new(sentry::Hub::new( + Some(Arc::clone(&client)), + Arc::new(sentry::Scope::default()), + )); + sentry::Hub::run(hub, || { + report_error_or_expected(message, "rpc", "invoke_method", &[]); + }); + client.flush(Some(std::time::Duration::from_secs(2))); transport.fetch_and_clear_envelopes().len() } From 1c402c8757d499f346d9b4ab276f999cc9a5d56c Mon Sep 17 00:00:00 2001 From: Shanu Date: Thu, 3 Sep 2026 00:58:40 +0530 Subject: [PATCH 22/23] style(e2e): prettier-format Playwright specs inherited from main MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The PR lane checks out the merge of this branch with main, and main keeps landing unformatted Playwright specs from the e2e backfills while its own Frontend Checks lane does not run for them — so the Prettier step here breaks on every re-run with no change on this branch. Whitespace-only, the same fix main needs. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_012jp2WUNkZ8dxDVmDtu3JxX --- .../specs/settings-theme-import-validation.spec.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/app/test/playwright/specs/settings-theme-import-validation.spec.ts b/app/test/playwright/specs/settings-theme-import-validation.spec.ts index 9679e37ded..89263c19a0 100644 --- a/app/test/playwright/specs/settings-theme-import-validation.spec.ts +++ b/app/test/playwright/specs/settings-theme-import-validation.spec.ts @@ -90,11 +90,7 @@ test.describe('Theme Studio — import validation', () => { } test('still accepts a theme carrying a single colour token', async ({ page }) => { - const valid = JSON.stringify({ - name: 'Minimal', - isDark: false, - colors: { surface: '1 2 3' }, - }); + const valid = JSON.stringify({ name: 'Minimal', isDark: false, colors: { surface: '1 2 3' } }); await attemptImport(page, valid); From 0fdc3f398b2bd2ee719d1033335ab35a4d9e5fb3 Mon Sep 17 00:00:00 2001 From: M3gA-Mind Date: Thu, 3 Sep 2026 05:26:18 +0530 Subject: [PATCH 23/23] test(observability): keep main's file-wide reporting lock alongside the private hub MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit My merge resolution in 77d1e1013 dropped `lock_reporting_state()` from `captured_events_for`, reasoning that a client bound to a private hub touches no process-global state so the guard was redundant. CI disproved it: `reporting_a_genuine_wallet_failure_still_emits_error` failed on the merge head with an empty capture — exactly the failure d0509bb17 added the file-wide lock to close. The private hub removes the *client* binding; it does not remove the need to serialise. `sentry-tracing`'s layer lives in the global subscriber stack, so while a client is current on the paging thread a `tracing::error!` raised by `capture_reporting` on another thread can be consumed by that layer instead of reaching its fmt subscriber. Both fixes are needed and both are now present: the private hub for the paging envelope count, main's file-wide lock for the capture. The doc comment that claimed no serialisation was needed is corrected rather than left contradicting the code. Worth recording that six green local runs preceded the CI failure, and six more followed this fix — for a race, a passing run is weak evidence either way. The reason to trust this one is that it restores the configuration d0509bb17 already validated, not the run count. --- tests/observability_wallet_expected_e2e.rs | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/tests/observability_wallet_expected_e2e.rs b/tests/observability_wallet_expected_e2e.rs index b8abd7ad93..368134c4a9 100644 --- a/tests/observability_wallet_expected_e2e.rs +++ b/tests/observability_wallet_expected_e2e.rs @@ -289,9 +289,23 @@ mod paging { /// a sibling test's `report_error_or_expected` on another thread landed in /// this transport too — `left: 2` for a genuine failure, deterministic /// under the product feature set, invisible under the contributor default - /// set where this module does not compile. No process-global state is - /// touched, so the tests need no serialisation. + /// set where this module does not compile. + /// + /// The private hub removes the *client* binding, but it does not remove the + /// need to serialise. `sentry-tracing`'s layer sits in the global + /// subscriber stack, so while a client is current on this thread a + /// `tracing::error!` raised by `capture_reporting` on ANOTHER thread can be + /// consumed by that layer instead of reaching its fmt subscriber — the + /// capture then comes back empty and + /// `reporting_a_genuine_wallet_failure_still_emits_error` fails for a + /// reason unrelated to the behaviour under test. That is `main`'s + /// `d0509bb17` finding, and it still holds here: an earlier revision of + /// this merge dropped the guard on the reasoning that a private hub made it + /// redundant, and CI reproduced exactly that failure. Both fixes are + /// needed — the private hub for the paging count, the file-wide lock for + /// the capture. fn captured_events_for(message: &str) -> usize { + let _guard = lock_reporting_state(); let transport = sentry::test::TestTransport::new(); let transport_for_factory = transport.clone(); let options = sentry::ClientOptions {