From de596aca1c34b9c0f4b8b91660c46afb63469434 Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Fri, 24 Apr 2026 15:07:02 -0500 Subject: [PATCH] fix(tui): render tool trail before anchored inline diffs Inline diff segments were anchored relative to assistant narration, but the turn details pane still rendered after streamSegments. On completion that put the diff before the tool telemetry that produced it. When a turn has anchored diff segments, commit the accumulated thinking/tool trail as a pre-diff trail message, then render the diff and final summary. --- .../createGatewayEventHandler.test.ts | 50 ++++++++++--------- ui-tui/src/app/turnController.ts | 30 +++++++++-- ui-tui/src/components/messageLine.tsx | 16 ++++-- 3 files changed, 64 insertions(+), 32 deletions(-) diff --git a/ui-tui/src/__tests__/createGatewayEventHandler.test.ts b/ui-tui/src/__tests__/createGatewayEventHandler.test.ts index 43a17e669..f8d88a50f 100644 --- a/ui-tui/src/__tests__/createGatewayEventHandler.test.ts +++ b/ui-tui/src/__tests__/createGatewayEventHandler.test.ts @@ -175,14 +175,16 @@ describe('createGatewayEventHandler', () => { onEvent({ payload: { text: 'patch applied' }, type: 'message.complete' } as any) - // Three transcript messages: pre-tool narration → diff (kind='diff', - // so MessageLine gives it blank-line breathing room) → post-tool - // narration. The final message does NOT contain a diff. - expect(appended).toHaveLength(3) + // Four transcript messages: pre-tool narration → tool trail → diff + // (kind='diff', so MessageLine gives it blank-line breathing room) → + // post-tool narration. The final message does NOT contain a diff. + expect(appended).toHaveLength(4) expect(appended[0]?.text).toBe('Editing the file') - expect(appended[1]).toMatchObject({ kind: 'diff', text: block }) - expect(appended[2]?.text).toBe('patch applied') - expect(appended[2]?.text).not.toContain('```diff') + expect(appended[1]).toMatchObject({ kind: 'trail' }) + expect(appended[1]?.tools?.[0]).toContain('Patch') + expect(appended[2]).toMatchObject({ kind: 'diff', text: block }) + expect(appended[3]?.text).toBe('patch applied') + expect(appended[3]?.text).not.toContain('```diff') }) it('drops the diff segment when the final assistant text narrates the same diff', () => { @@ -209,12 +211,13 @@ describe('createGatewayEventHandler', () => { onEvent({ payload: { inline_diff: raw, summary: 'patched', tool_id: 'tool-1' }, type: 'tool.complete' } as any) onEvent({ payload: { text: 'done' }, type: 'message.complete' } as any) - // diff segment first (kind='diff'), final narration second - expect(appended).toHaveLength(2) - expect(appended[0]?.kind).toBe('diff') - expect(appended[0]?.text).not.toContain('┊ review diff') - expect(appended[0]?.text).toContain('--- a/foo.ts') - expect(appended[1]?.text).toBe('done') + // Tool trail first, then diff segment (kind='diff'), then final narration. + expect(appended).toHaveLength(3) + expect(appended[0]?.kind).toBe('trail') + expect(appended[1]?.kind).toBe('diff') + expect(appended[1]?.text).not.toContain('┊ review diff') + expect(appended[1]?.text).toContain('--- a/foo.ts') + expect(appended[2]?.text).toBe('done') }) it('drops the diff segment when assistant writes its own ```diff fence', () => { @@ -242,16 +245,17 @@ describe('createGatewayEventHandler', () => { } as any) onEvent({ payload: { text: 'done' }, type: 'message.complete' } as any) - // Two segments: the diff block (kind='diff', no tool row) and the final - // narration (tool row belongs here since pendingSegmentTools carries - // across the flushStreamingSegment call). - expect(appended).toHaveLength(2) - expect(appended[0]?.kind).toBe('diff') - expect(appended[0]?.text).toContain('```diff') - expect(appended[0]?.tools ?? []).toEqual([]) - expect(appended[1]?.text).toBe('done') - expect(appended[1]?.tools?.[0]).toContain('Review Diff') - expect(appended[1]?.tools?.[0]).not.toContain('--- a/foo.ts') + // Tool row is now placed before the diff, so telemetry does not render + // below the patch that came from that tool. + expect(appended).toHaveLength(3) + expect(appended[0]?.kind).toBe('trail') + expect(appended[0]?.tools?.[0]).toContain('Review Diff') + expect(appended[0]?.tools?.[0]).not.toContain('--- a/foo.ts') + expect(appended[1]?.kind).toBe('diff') + expect(appended[1]?.text).toContain('```diff') + expect(appended[1]?.tools ?? []).toEqual([]) + expect(appended[2]?.text).toBe('done') + expect(appended[2]?.tools ?? []).toEqual([]) }) it('shows setup panel for missing provider startup error', () => { diff --git a/ui-tui/src/app/turnController.ts b/ui-tui/src/app/turnController.ts index cbb03b444..f45cab241 100644 --- a/ui-tui/src/app/turnController.ts +++ b/ui-tui/src/app/turnController.ts @@ -33,6 +33,12 @@ const diffSegmentBody = (msg: Msg): null | string => { return m ? m[1]! : null } +const insertBeforeFirstDiff = (segments: Msg[], msg: Msg): Msg[] => { + const index = segments.findIndex(segment => segment.kind === 'diff') + + return index < 0 ? [...segments, msg] : [...segments.slice(0, index), msg, ...segments.slice(index)] +} + export interface InterruptDeps { appendMessage: (msg: Msg) => void gw: { request: (method: string, params?: Record) => Promise } @@ -292,16 +298,30 @@ class TurnController { return body === null || (!finalHasOwnDiffFence && !finalText.includes(body)) }) - const finalMessages = [...segments] + const hasDiffSegment = segments.some(msg => msg.kind === 'diff') + const detailsBelongBeforeDiff = hasDiffSegment && (tools.length > 0 || Boolean(savedReasoning)) + const finalMessages = detailsBelongBeforeDiff + ? insertBeforeFirstDiff(segments, { + kind: 'trail', + role: 'system', + text: '', + thinking: savedReasoning || undefined, + thinkingTokens: savedReasoning ? savedReasoningTokens : undefined, + toolTokens: savedToolTokens || undefined, + ...(tools.length && { tools }) + }) + : [...segments] if (finalText) { finalMessages.push({ role: 'assistant', text: finalText, - thinking: savedReasoning || undefined, - thinkingTokens: savedReasoning ? savedReasoningTokens : undefined, - toolTokens: savedToolTokens || undefined, - ...(tools.length && { tools }) + ...(!detailsBelongBeforeDiff && { + thinking: savedReasoning || undefined, + thinkingTokens: savedReasoning ? savedReasoningTokens : undefined, + toolTokens: savedToolTokens || undefined, + ...(tools.length && { tools }) + }) }) } diff --git a/ui-tui/src/components/messageLine.tsx b/ui-tui/src/components/messageLine.tsx index a73a4f01d..3fc40528a 100644 --- a/ui-tui/src/components/messageLine.tsx +++ b/ui-tui/src/components/messageLine.tsx @@ -31,11 +31,20 @@ export const MessageLine = memo(function MessageLine({ const thinkingMode = sectionMode('thinking', detailsMode, sections) const toolsMode = sectionMode('tools', detailsMode, sections) const activityMode = sectionMode('activity', detailsMode, sections) + const thinking = msg.thinking?.trim() ?? '' - if (msg.kind === 'trail' && msg.tools?.length) { - return toolsMode !== 'hidden' || activityMode !== 'hidden' ? ( + if (msg.kind === 'trail' && (msg.tools?.length || thinking)) { + return thinkingMode !== 'hidden' || toolsMode !== 'hidden' || activityMode !== 'hidden' ? ( - + ) : null } @@ -61,7 +70,6 @@ export const MessageLine = memo(function MessageLine({ } const { body, glyph, prefix } = ROLE[msg.role](t) - const thinking = msg.thinking?.trim() ?? '' const showDetails = (toolsMode !== 'hidden' && Boolean(msg.tools?.length)) ||