From 2258a181f01eb1616abfe8e8b20afe623d1a63ff Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Thu, 23 Apr 2026 19:11:59 -0500 Subject: [PATCH] fix(tui): give inline_diff segments blank-line breathing room Visual polish on top of the segment-anchor change: diff blocks were butting up against the narration around them. Tag diff-only segments with `kind: 'diff'` (extended on Msg) and give them `marginTop={1}` + `marginBottom={1}` in MessageLine, matching the spacing we already use for user messages. Also swaps the regex-based `diffSegmentBody` check for an explicit `kind === 'diff'` guard so the dedupe path is clearer. --- .../createGatewayEventHandler.test.ts | 18 ++++++++++-------- ui-tui/src/app/turnController.ts | 15 +++++++-------- ui-tui/src/components/messageLine.tsx | 9 +++++++-- ui-tui/src/types.ts | 2 +- 4 files changed, 25 insertions(+), 19 deletions(-) diff --git a/ui-tui/src/__tests__/createGatewayEventHandler.test.ts b/ui-tui/src/__tests__/createGatewayEventHandler.test.ts index 07721d441..289c9b7b2 100644 --- a/ui-tui/src/__tests__/createGatewayEventHandler.test.ts +++ b/ui-tui/src/__tests__/createGatewayEventHandler.test.ts @@ -165,19 +165,19 @@ describe('createGatewayEventHandler', () => { // Diff is already committed to segmentMessages as its own segment — // nothing is "pending" anymore. The pre-tool narration is also flushed. expect(appended).toHaveLength(0) - expect(turnController.segmentMessages.map(m => m.text)).toEqual([ - 'Editing the file', - `\`\`\`diff\n${cleaned}\n\`\`\`` + expect(turnController.segmentMessages).toEqual([ + { role: 'assistant', text: 'Editing the file' }, + { kind: 'diff', role: 'assistant', text: `\`\`\`diff\n${cleaned}\n\`\`\`` } ]) onEvent({ payload: { text: 'patch applied' }, type: 'message.complete' } as any) // Three messages in the transcript, in order: pre-tool narration → - // diff → post-tool narration. The final message does NOT contain - // `diff` content. + // 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) expect(appended[0]?.text).toBe('Editing the file') - expect(appended[1]?.text).toBe(`\`\`\`diff\n${cleaned}\n\`\`\``) + expect(appended[1]).toMatchObject({ kind: 'diff', text: `\`\`\`diff\n${cleaned}\n\`\`\`` }) expect(appended[2]?.text).toBe('patch applied') expect(appended[2]?.text).not.toContain('```diff') }) @@ -214,6 +214,7 @@ describe('createGatewayEventHandler', () => { // diff segment first, 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') @@ -247,9 +248,10 @@ describe('createGatewayEventHandler', () => { } as any) onEvent({ payload: { text: 'done' }, type: 'message.complete' } as any) - // Two segments: diff block (no tool row), final narration (tool row - // belongs here since pendingSegmentTools carries across the flush). + // Two segments: diff block (kind='diff', no tool row), final narration + // (tool row belongs here since pendingSegmentTools carries across the flush). 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') diff --git a/ui-tui/src/app/turnController.ts b/ui-tui/src/app/turnController.ts index 5bcbb05f2..31b65cb86 100644 --- a/ui-tui/src/app/turnController.ts +++ b/ui-tui/src/app/turnController.ts @@ -19,17 +19,16 @@ const INTERRUPT_COOLDOWN_MS = 1500 const ACTIVITY_LIMIT = 8 const TRAIL_LIMIT = 8 -// Matches segments produced by pushInlineDiffSegment — a bare ```diff fence -// wrapping the raw patch, no surrounding prose. Used at message.complete to -// dedupe against final assistant text that narrates the same patch. -const DIFF_SEGMENT_RE = /^```diff\n([\s\S]*?)\n```$/ - +// Extracts the raw patch from a diff-only segment produced by +// pushInlineDiffSegment. Used at message.complete to dedupe against final +// assistant text that narrates the same patch. Returns null for anything +// else so real assistant narration never gets touched. const diffSegmentBody = (msg: Msg): null | string => { - if (msg.role !== 'assistant' || msg.tools?.length) { + if (msg.kind !== 'diff') { return null } - const m = msg.text.match(DIFF_SEGMENT_RE) + const m = msg.text.match(/^```diff\n([\s\S]*?)\n```$/) return m ? m[1]! : null } @@ -226,7 +225,7 @@ class TurnController { return } - this.segmentMessages = [...this.segmentMessages, { role: 'assistant', text: block }] + this.segmentMessages = [...this.segmentMessages, { kind: 'diff', role: 'assistant', text: block }] patchTurnState({ streamSegments: this.segmentMessages }) } diff --git a/ui-tui/src/components/messageLine.tsx b/ui-tui/src/components/messageLine.tsx index 8d77a49e5..635c119a0 100644 --- a/ui-tui/src/components/messageLine.tsx +++ b/ui-tui/src/components/messageLine.tsx @@ -81,11 +81,16 @@ export const MessageLine = memo(function MessageLine({ return {msg.text} })() + // Diff segments (emitted by pushInlineDiffSegment between narration + // segments) need a blank line on both sides so the patch doesn't butt up + // against the prose around it. + const isDiffSegment = msg.kind === 'diff' + return ( {showDetails && ( diff --git a/ui-tui/src/types.ts b/ui-tui/src/types.ts index 63d6c6d4f..191e63900 100644 --- a/ui-tui/src/types.ts +++ b/ui-tui/src/types.ts @@ -102,7 +102,7 @@ export interface ClarifyReq { export interface Msg { info?: SessionInfo - kind?: 'intro' | 'panel' | 'slash' | 'trail' + kind?: 'diff' | 'intro' | 'panel' | 'slash' | 'trail' panelData?: PanelData role: Role text: string