From dfba3f3e519717efc6405c6b9c35a90dc279efb1 Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Tue, 2 Jun 2026 21:04:21 -0500 Subject: [PATCH] fix(tui): clear selection on right-click copy + group transcript blocks Two TUI polish fixes. (1) Right-click copy now clears the highlight. The right-click handler copied an active selection via onCopySelectionNoClear (the copy-on-select variant that keeps the highlight during a drag) and never cleared it, so after right-click-to-copy the selection stayed lit with no confirmation and a follow-up right-click re-copied the stale range instead of pasting. A successful right-click copy now clears the selection and notifies; if the copy fails (no clipboard path) the highlight survives and we fall back to the right-click paste handler, exactly as before. (2) Group transcript blocks so boundaries read clearly. Model replies, reasoning/tool trails, and system/error notes rendered with no vertical separation, so distinct block types butted together and were hard to scan. Group adjacent blocks by kind: one blank line opens only where the visual group changes (model prose <-> reasoning/tool trails <-> notes), while a run of same-kind blocks renders flush. The rule lives in domain/blockLayout.ts (messageGroup + hasLeadGap) and is applied intrinsically in MessageLine via a `prev` prop, which fixes the things ad-hoc per-block margins kept breaking: - Streaming stability: the gap is derived from the stable predecessor, never the live block's own changing text, so the actively-streaming reply computes the same gap while it streams as the settled segment does once it flushes. No reflow/jump. - Transparent empty trails: a trail hidden by /details, or one carrying only a token tally (the finalDetails segment message.complete appends), renders nothing and is transparent to grouping (prevRenderedMsg skips it), so there are no floating gaps, no doubled gap after a prompt, and no padded space above the final reply. In the default/collapsed modes content-bearing trails always render, so the grouping is a no-op there. The virtual-height estimator counts the group-boundary line so scroll math stays accurate before Yoga remeasures. ui-tui/src/domain/blockLayout.ts (new), components/messageLine.tsx, components/streamingAssistant.tsx, components/appLayout.tsx, lib/virtualHeights.ts, app/useMainApp.ts. Tests: blockLayout.test.ts (grouping + hidden/empty-trail visibility), virtualHeights leadGap, app-mouse.test.ts copy behavior. Full ui-tui suite green apart from 3 pre-existing local/env failures (cursorDrift, ink-resize, virtualHeights user-prompt-width) unchanged from main. --- .../hermes-ink/src/ink/app-mouse.test.ts | 35 ++++- .../hermes-ink/src/ink/components/App.tsx | 12 +- ui-tui/src/__tests__/blockLayout.test.ts | 122 +++++++++++++++ ui-tui/src/__tests__/virtualHeights.test.ts | 8 + ui-tui/src/app/useMainApp.ts | 12 ++ ui-tui/src/components/appLayout.tsx | 7 + ui-tui/src/components/messageLine.tsx | 28 +++- ui-tui/src/components/streamingAssistant.tsx | 112 +++++++------- ui-tui/src/domain/blockLayout.ts | 146 ++++++++++++++++++ ui-tui/src/lib/virtualHeights.ts | 13 ++ 10 files changed, 437 insertions(+), 58 deletions(-) create mode 100644 ui-tui/src/__tests__/blockLayout.test.ts create mode 100644 ui-tui/src/domain/blockLayout.ts diff --git a/ui-tui/packages/hermes-ink/src/ink/app-mouse.test.ts b/ui-tui/packages/hermes-ink/src/ink/app-mouse.test.ts index a4c63d3eb..b3d453624 100644 --- a/ui-tui/packages/hermes-ink/src/ink/app-mouse.test.ts +++ b/ui-tui/packages/hermes-ink/src/ink/app-mouse.test.ts @@ -1,7 +1,7 @@ import { describe, expect, it, vi } from 'vitest' import { handleMouseEvent } from './components/App.js' -import { createSelectionState, startSelection, updateSelection } from './selection.js' +import { createSelectionState, hasSelection, startSelection, updateSelection } from './selection.js' const makeApp = () => { const selection = createSelectionState() @@ -39,6 +39,39 @@ describe('handleMouseEvent right-click selection behavior', () => { expect(app.clickCount).toBe(0) }) + it('clears the highlight after a successful right-click copy', async () => { + const app = makeApp() + + startSelection(app.props.selection, 0, 0) + updateSelection(app.props.selection, 4, 0) + expect(hasSelection(app.props.selection)).toBe(true) + + handleMouseEvent(app, { action: 'press', button: 2, col: 3, kind: 'mouse', row: 1, sequence: '' }) + await Promise.resolve() + await Promise.resolve() + + // Deliberate copy clears the selection (visual confirmation + a follow-up + // right-click on empty space pastes rather than re-copying a stale range). + expect(hasSelection(app.props.selection)).toBe(false) + expect(app.props.onSelectionChange).toHaveBeenCalled() + }) + + it('keeps the highlight when right-click copy fails (no clipboard path)', async () => { + const app = makeApp() + app.props.onCopySelectionNoClear.mockResolvedValue('') + + startSelection(app.props.selection, 0, 0) + updateSelection(app.props.selection, 4, 0) + + handleMouseEvent(app, { action: 'press', button: 2, col: 3, kind: 'mouse', row: 1, sequence: '' }) + await Promise.resolve() + await Promise.resolve() + + // Copy didn't land, so the highlight must survive (and we fall back to the + // right-click paste handler instead). + expect(hasSelection(app.props.selection)).toBe(true) + }) + it('falls back to right-click handlers when selection copy has no clipboard path', async () => { const app = makeApp() app.props.onCopySelectionNoClear.mockResolvedValue('') diff --git a/ui-tui/packages/hermes-ink/src/ink/components/App.tsx b/ui-tui/packages/hermes-ink/src/ink/components/App.tsx index 81d3a689f..ced4bcfc5 100644 --- a/ui-tui/packages/hermes-ink/src/ink/components/App.tsx +++ b/ui-tui/packages/hermes-ink/src/ink/components/App.tsx @@ -17,7 +17,7 @@ import { parseMultipleKeypresses } from '../parse-keypress.js' import reconciler from '../reconciler.js' -import { finishSelection, hasSelection, type SelectionState, startSelection } from '../selection.js' +import { clearSelection, finishSelection, hasSelection, type SelectionState, startSelection } from '../selection.js' import { getTerminalFocused, setTerminalFocused } from '../terminal-focus-state.js' import { TerminalQuerier, xtversion } from '../terminal-querier.js' import { isXtermJs, setXtversionName, supportsExtendedKeys } from '../terminal.js' @@ -648,7 +648,15 @@ export function handleMouseEvent(app: App, m: ParsedMouse): void { void app.props .onCopySelectionNoClear() .then(text => { - if (!text) { + if (text) { + // Right-click copy is a deliberate action (unlike copy-on-select + // during a drag, which keeps the highlight so the drag can + // continue). Clear the highlight so the user gets visual + // confirmation the copy landed and a follow-up right-click on + // empty space pastes instead of re-copying the stale range. + clearSelection(sel) + app.props.onSelectionChange() + } else { app.props.onMouseDownAt(col, row, baseButton) } }) diff --git a/ui-tui/src/__tests__/blockLayout.test.ts b/ui-tui/src/__tests__/blockLayout.test.ts new file mode 100644 index 000000000..525254ceb --- /dev/null +++ b/ui-tui/src/__tests__/blockLayout.test.ts @@ -0,0 +1,122 @@ +import { describe, expect, it } from 'vitest' + +import { blockRenders, hasLeadGap, messageGroup, prevRenderedMsg } from '../domain/blockLayout.js' +import type { Msg } from '../types.js' + +const m = (over: Partial): Msg => ({ role: 'assistant', text: '', ...over }) + +describe('messageGroup', () => { + it('classifies each block kind into its visual band', () => { + expect(messageGroup(m({ role: 'assistant' }))).toBe('model') + expect(messageGroup(m({ role: 'assistant', kind: 'diff' }))).toBe('diff') + expect(messageGroup(m({ role: 'system', kind: 'trail' }))).toBe('trail') + expect(messageGroup(m({ role: 'system' }))).toBe('note') + expect(messageGroup(m({ role: 'user' }))).toBe('user') + expect(messageGroup(m({ role: 'user', kind: 'slash' }))).toBe('slash') + expect(messageGroup(m({ role: 'system', kind: 'intro' }))).toBe('intro') + expect(messageGroup(m({ role: 'system', kind: 'panel' }))).toBe('intro') + }) +}) + +describe('hasLeadGap', () => { + const trail = m({ role: 'system', kind: 'trail' }) + const model = m({ role: 'assistant' }) + const note = m({ role: 'system' }) + const user = m({ role: 'user' }) + const diff = m({ role: 'assistant', kind: 'diff' }) + const slash = m({ role: 'user', kind: 'slash' }) + + it('opens a gap only at a boundary between working-area groups', () => { + expect(hasLeadGap(trail, model)).toBe(true) + expect(hasLeadGap(model, trail)).toBe(true) + expect(hasLeadGap(model, note)).toBe(true) + expect(hasLeadGap(note, model)).toBe(true) + }) + + it('keeps same-group neighbours flush (the grouping)', () => { + expect(hasLeadGap(trail, trail)).toBe(false) + expect(hasLeadGap(model, model)).toBe(false) + expect(hasLeadGap(note, note)).toBe(false) + }) + + it('never gaps the first block (no predecessor)', () => { + expect(hasLeadGap(undefined, model)).toBe(false) + expect(hasLeadGap(undefined, trail)).toBe(false) + }) + + it('suppresses the gap after blocks that already paint a trailing line', () => { + // user and diff carry their own marginBottom — the following block must + // not add a second blank line on top of it. + expect(hasLeadGap(user, trail)).toBe(false) + expect(hasLeadGap(user, model)).toBe(false) + expect(hasLeadGap(diff, model)).toBe(false) + }) + + it('still gaps after a slash echo (it has no trailing margin)', () => { + expect(hasLeadGap(slash, model)).toBe(true) + expect(hasLeadGap(slash, trail)).toBe(true) + }) + + it('lets user / slash / diff own their spacing (never managed here)', () => { + expect(hasLeadGap(model, user)).toBe(false) + expect(hasLeadGap(model, slash)).toBe(false) + expect(hasLeadGap(model, diff)).toBe(false) + }) +}) + +describe('blockRenders', () => { + const trail: Msg = { role: 'system', kind: 'trail', text: '', tools: ['Edit foo.ts'] } + const model: Msg = { role: 'assistant', text: 'hi' } + const todos: Msg = { role: 'system', kind: 'trail', text: '', todos: [{ content: 'a', id: '1', status: 'pending' }] } + + it('always renders non-trail blocks', () => { + expect(blockRenders(model, { detailsMode: 'hidden', commandOverride: true })).toBe(true) + }) + + it('renders a content-bearing trail unless every section is hidden', () => { + expect(blockRenders(trail, { detailsMode: 'collapsed' })).toBe(true) + expect(blockRenders(trail, { detailsMode: 'expanded' })).toBe(true) + // /details hidden routes through commandOverride, which hides every section. + expect(blockRenders(trail, { detailsMode: 'hidden', commandOverride: true })).toBe(false) + }) + + it('does not render a content-less trail (e.g. finalDetails with only a token tally)', () => { + const tally: Msg = { role: 'system', kind: 'trail', text: '', toolTokens: 40 } + + expect(blockRenders(tally, { detailsMode: 'expanded' })).toBe(false) + }) + + it('keeps todo trails visible even when details are hidden', () => { + expect(blockRenders(todos, { detailsMode: 'hidden', commandOverride: true })).toBe(true) + }) +}) + +describe('prevRenderedMsg', () => { + const hiddenCtx = { commandOverride: true, detailsMode: 'hidden' as const } + const shownCtx = { detailsMode: 'collapsed' as const } + + const rows: Msg[] = [ + { role: 'user', text: 'q' }, // 0 + { role: 'system', kind: 'trail', text: '', tools: ['Edit foo.ts'] }, // 1 + { role: 'assistant', text: 'first' }, // 2 + { role: 'system', kind: 'trail', text: '', tools: ['Edit bar.ts'] }, // 3 + { role: 'assistant', text: 'second' } // 4 + ] + const at = (i: number) => rows[i] + + it('returns the literal predecessor when everything renders', () => { + expect(prevRenderedMsg(at, 2, shownCtx)).toBe(rows[1]) + expect(prevRenderedMsg(at, 4, shownCtx)).toBe(rows[3]) + }) + + it('skips hidden trails so grouping sees the nearest visible block', () => { + // With trails hidden, the prose at index 2 groups against the user (not the + // invisible trail) and the prose at index 4 groups against the prose at 2. + expect(prevRenderedMsg(at, 2, hiddenCtx)).toBe(rows[0]) + expect(prevRenderedMsg(at, 4, hiddenCtx)).toBe(rows[2]) + }) + + it('returns undefined at the top of the transcript', () => { + expect(prevRenderedMsg(at, 0, shownCtx)).toBeUndefined() + }) +}) diff --git a/ui-tui/src/__tests__/virtualHeights.test.ts b/ui-tui/src/__tests__/virtualHeights.test.ts index 37cb9c009..9819a7214 100644 --- a/ui-tui/src/__tests__/virtualHeights.test.ts +++ b/ui-tui/src/__tests__/virtualHeights.test.ts @@ -24,6 +24,14 @@ describe('virtual height estimates', () => { expect(estimatedMsgHeight(msg, 26, { compact: false, details: false, userPrompt: 'Ψ >' })).toBe(4) }) + it('adds one row for a group-boundary lead gap', () => { + const msg: Msg = { role: 'assistant', text: 'reply' } + + expect(estimatedMsgHeight(msg, 80, { compact: false, details: false, leadGap: true })).toBe( + estimatedMsgHeight(msg, 80, { compact: false, details: false, leadGap: false }) + 1 + ) + }) + it('includes detail sections when visible', () => { const msg: Msg = { role: 'assistant', text: 'ok', thinking: 'line 1\nline 2', tools: ['Tool A', 'Tool B'] } diff --git a/ui-tui/src/app/useMainApp.ts b/ui-tui/src/app/useMainApp.ts index bb703e134..6c48c56f9 100644 --- a/ui-tui/src/app/useMainApp.ts +++ b/ui-tui/src/app/useMainApp.ts @@ -4,6 +4,7 @@ import { useCallback, useEffect, useMemo, useRef, useState } from 'react' import { STARTUP_RESUME_ID } from '../config/env.js' import { MAX_HISTORY, WHEEL_SCROLL_STEP } from '../config/limits.js' +import { hasLeadGap, prevRenderedMsg } from '../domain/blockLayout.js' import { SECTION_NAMES, sectionMode } from '../domain/details.js' import { attachedImageNotice, imageTokenMeta } from '../domain/messages.js' import { fmtCwdBranch, shortCwd } from '../domain/paths.js' @@ -350,6 +351,14 @@ export function useMainApp(gw: GatewayClient) { estimatedMsgHeight(virtualRows[index]!.msg, cols, { compact: ui.compact, details: detailsVisible, + leadGap: hasLeadGap( + prevRenderedMsg(i => virtualRows[i]?.msg, index, { + commandOverride: ui.detailsModeCommandOverride, + detailsMode: ui.detailsMode, + sections: ui.sections + }), + virtualRows[index]!.msg + ), thinkingVisible: thinkingDetailsVisible, toolsVisible: toolsDetailsVisible, userPrompt: ui.theme.brand.prompt, @@ -362,6 +371,9 @@ export function useMainApp(gw: GatewayClient) { thinkingDetailsVisible, toolsDetailsVisible, ui.compact, + ui.detailsMode, + ui.detailsModeCommandOverride, + ui.sections, ui.theme.brand.prompt, virtualRows ] diff --git a/ui-tui/src/components/appLayout.tsx b/ui-tui/src/components/appLayout.tsx index f4204551c..113735c49 100644 --- a/ui-tui/src/components/appLayout.tsx +++ b/ui-tui/src/components/appLayout.tsx @@ -8,6 +8,7 @@ import { $isBlocked, $overlayState, patchOverlayState } from '../app/overlayStor import { $uiState } from '../app/uiStore.js' import { INLINE_MODE, SHOW_FPS, TERMUX_TUI_MODE } from '../config/env.js' import { PLACEHOLDER } from '../content/placeholders.js' +import { prevRenderedMsg } from '../domain/blockLayout.js' import { COMPOSER_PROMPT_GAP_WIDTH, composerPromptWidth, @@ -125,6 +126,11 @@ const TranscriptPane = memo(function TranscriptPane({ detailsMode={ui.detailsMode} detailsModeCommandOverride={ui.detailsModeCommandOverride} msg={row.msg} + prev={prevRenderedMsg( + i => transcript.virtualRows[i]?.msg, + row.index, + { commandOverride: ui.detailsModeCommandOverride, detailsMode: ui.detailsMode, sections: ui.sections } + )} sections={ui.sections} t={ui.theme} /> @@ -141,6 +147,7 @@ const TranscriptPane = memo(function TranscriptPane({ compact={ui.compact} detailsMode={ui.detailsMode} detailsModeCommandOverride={ui.detailsModeCommandOverride} + prevMsg={transcript.historyItems[transcript.historyItems.length - 1]} progress={progress} sections={ui.sections} /> diff --git a/ui-tui/src/components/messageLine.tsx b/ui-tui/src/components/messageLine.tsx index 2a7f0bbba..18658a253 100644 --- a/ui-tui/src/components/messageLine.tsx +++ b/ui-tui/src/components/messageLine.tsx @@ -3,6 +3,7 @@ import { memo, useState } from 'react' import { TERMUX_TUI_MODE } from '../config/env.js' import { LONG_MSG } from '../config/limits.js' +import { hasLeadGap } from '../domain/blockLayout.js' import { sectionMode } from '../domain/details.js' import { userDisplay } from '../domain/messages.js' import { ROLE } from '../domain/roles.js' @@ -33,6 +34,7 @@ export const MessageLine = memo(function MessageLine({ detailsModeCommandOverride = false, isStreaming = false, msg, + prev, sections, t, tools = [] @@ -49,6 +51,14 @@ export const MessageLine = memo(function MessageLine({ const activityMode = sectionMode('activity', detailsMode, sections, detailsModeCommandOverride) const thinking = msg.thinking?.trim() ?? '' + // One blank line above this block iff it opens a new visual group relative + // to the block directly above it (`prev`) — the flex-grouping rule. Applied + // intrinsically on each *rendered* element (not via an outer wrapper) so a + // block that renders nothing — e.g. a tool trail hidden by /details — emits + // no floating gap. Streaming-safe: the gap is derived from the stable + // predecessor, never this block's own live content. See domain/blockLayout. + const leadGap = hasLeadGap(prev, msg) + // Collapse toggle for long system messages const systemIsLong = msg.role === 'system' && msg.text.length > SYSTEM_COLLAPSE_CHARS const [systemOpen, setSystemOpen] = useState(false) @@ -66,7 +76,7 @@ export const MessageLine = memo(function MessageLine({ if (msg.kind === 'trail' && (msg.tools?.length || tools.length || thinking)) { return thinkingMode !== 'hidden' || toolsMode !== 'hidden' || activityMode !== 'hidden' ? ( - + {showDetails && ( @@ -231,6 +249,10 @@ interface MessageLineProps { detailsModeCommandOverride?: boolean isStreaming?: boolean msg: Msg + // The block rendered directly above this one. Drives the group-boundary + // lead gap (see domain/blockLayout.ts::hasLeadGap). Undefined at the top of + // the transcript or when spacing is irrelevant. + prev?: Msg sections?: SectionVisibility t: Theme tools?: ActiveTool[] diff --git a/ui-tui/src/components/streamingAssistant.tsx b/ui-tui/src/components/streamingAssistant.tsx index d691138bc..3f4c500c7 100644 --- a/ui-tui/src/components/streamingAssistant.tsx +++ b/ui-tui/src/components/streamingAssistant.tsx @@ -4,8 +4,9 @@ import { memo } from 'react' import type { AppLayoutProgressProps } from '../app/interfaces.js' import { toggleTodoCollapsed, useTurnSelector } from '../app/turnStore.js' import { $uiState } from '../app/uiStore.js' +import { blockRenders } from '../domain/blockLayout.js' import { appendToolShelfMessage } from '../lib/liveProgress.js' -import type { DetailsMode, Msg, SectionVisibility } from '../types.js' +import type { ActiveTool, DetailsMode, Msg, SectionVisibility } from '../types.js' import { MessageLine } from './messageLine.js' import { TodoPanel } from './todoPanel.js' @@ -13,11 +14,19 @@ import { TodoPanel } from './todoPanel.js' const groupedSegments = (segments: Msg[]): Msg[] => segments.reduce((acc, msg) => appendToolShelfMessage(acc, msg), []) +interface LiveBlock { + isStreaming?: boolean + key: string + msg: Msg + tools?: ActiveTool[] +} + export const StreamingAssistant = memo(function StreamingAssistant({ cols, compact, detailsMode, detailsModeCommandOverride, + prevMsg, progress, sections }: StreamingAssistantProps) { @@ -32,62 +41,60 @@ export const StreamingAssistant = memo(function StreamingAssistant({ return null } + // Flatten the live area into one ordered list so each block's leading gap + // can be derived from the block directly above it — including the boundary + // back into settled history (prevMsg). Tracking the predecessor rather than + // the live text is what keeps the streaming block from jumping when it + // flushes into a settled segment. + const blocks: LiveBlock[] = groupedSegments(streamSegments).map((msg, i) => ({ key: `seg:${i}`, msg })) + + if (activeTools.length) { + blocks.push({ key: 'active-tools', msg: { kind: 'trail', role: 'system', text: '' }, tools: activeTools }) + } + + if (showStreamingArea) { + blocks.push({ + isStreaming: true, + key: 'streaming', + msg: { role: 'assistant', text: streaming, ...(streamPendingTools.length && { tools: streamPendingTools }) } + }) + } else if (streamPendingTools.length) { + blocks.push({ key: 'pending-tools', msg: { kind: 'trail', role: 'system', text: '', tools: streamPendingTools } }) + } + + const detailsCtx = { commandOverride: detailsModeCommandOverride, detailsMode, sections } + let prev = prevMsg + return ( <> - {groupedSegments(streamSegments).map((msg, i) => ( - - ))} + {blocks.map(block => { + const node = ( + + ) - {!!activeTools.length && ( - - )} + // Advance the grouping predecessor only past blocks that actually + // paint, so a trail hidden by /details stays transparent here too + // (active tools live in the prop, so fold them into the check). + const checkMsg = block.tools?.length ? { ...block.msg, tools: block.tools.map(tool => tool.name) } : block.msg - {showStreamingArea && ( - - )} + if (blockRenders(checkMsg, detailsCtx)) { + prev = block.msg + } - {!showStreamingArea && !!streamPendingTools.length && ( - - )} + return node + })} ) }) @@ -105,6 +112,7 @@ interface StreamingAssistantProps { compact?: boolean detailsMode: DetailsMode detailsModeCommandOverride: boolean + prevMsg?: Msg progress: AppLayoutProgressProps sections?: SectionVisibility } diff --git a/ui-tui/src/domain/blockLayout.ts b/ui-tui/src/domain/blockLayout.ts new file mode 100644 index 000000000..1fad02246 --- /dev/null +++ b/ui-tui/src/domain/blockLayout.ts @@ -0,0 +1,146 @@ +import type { DetailsMode, Msg, SectionVisibility } from '../types.js' + +import { sectionMode } from './details.js' + +/** + * Visual group a transcript block belongs to. Blocks in the same group render + * flush; a single blank line opens at each group boundary. So a run of tool + * trails (or model paragraphs) reads as one section and the eye only catches a + * gap where the *kind* of content actually changes — the boundary between + * reasoning/tool trails, model prose, and notes/errors. + * + * user — the human turn (owns its separator + margins in MessageLine) + * model — assistant prose, the model's voice + * trail — reasoning + tool-call trails (the agent's working area) + * note — system notes and errors (a quieter band) + * diff — inline patch segments (an island, owns its own margins) + * slash — slash-command echoes (owns its margin) + * intro — banner / panels (rendered out-of-band, never gapped here) + */ +export type BlockGroup = 'diff' | 'intro' | 'model' | 'note' | 'slash' | 'trail' | 'user' + +export const messageGroup = (msg: Pick): BlockGroup => { + switch (msg.kind) { + case 'intro': + case 'panel': + return 'intro' + case 'slash': + return 'slash' + case 'diff': + return 'diff' + case 'trail': + return 'trail' + } + + if (msg.role === 'user') { + return 'user' + } + + // Assistant prose is the model's voice; system notes/errors are their own + // band. (No runtime block uses role 'tool' — tool *results* fold into + // trails — so a stray 'tool' falls through to the note band harmlessly.) + return msg.role === 'assistant' ? 'model' : 'note' +} + +// Groups whose leading gap is already owned by their own chrome in +// MessageLine (the turn separator + top margin for user, the top margin for +// slash, the top+bottom margins for diff) or that are painted out-of-band +// (intro). The grouping primitive only spaces the model working area — +// model prose, reasoning/tool trails, and notes/errors. +const SELF_SPACED: ReadonlySet = new Set(['diff', 'intro', 'slash', 'user']) + +// Groups that already paint a trailing blank line beneath themselves +// (marginBottom in MessageLine), so the block that follows must not add its +// own leading gap or the single boundary would become a double gap. +const PAINTS_TRAILING_GAP: ReadonlySet = new Set(['diff', 'user']) + +/** + * Whether `cur` renders one blank line above it, given the block rendered + * directly above it (`prev`). True only where the visual group changes, and + * only for the model-working-area bands (model / trail / note) — user, slash, + * diff, and intro keep their existing spacing. + * + * Streaming-safe by construction: the result depends on the *predecessor's* + * group, never on `cur`'s own (live, changing) content. The actively-streaming + * assistant block therefore computes the same gap while it streams as the + * settled segment does once it flushes, so the live area never jumps. + */ +export const hasLeadGap = ( + prev: Pick | undefined, + cur: Pick +): boolean => { + const group = messageGroup(cur) + + if (SELF_SPACED.has(group)) { + return false + } + + if (!prev) { + return false + } + + const prevGroup = messageGroup(prev) + + return prevGroup !== group && !PAINTS_TRAILING_GAP.has(prevGroup) +} + +export interface DetailsCtx { + commandOverride?: boolean + detailsMode: DetailsMode + sections?: SectionVisibility +} + +const trailAllHidden = (ctx: DetailsCtx): boolean => + sectionMode('thinking', ctx.detailsMode, ctx.sections, ctx.commandOverride) === 'hidden' && + sectionMode('tools', ctx.detailsMode, ctx.sections, ctx.commandOverride) === 'hidden' && + sectionMode('activity', ctx.detailsMode, ctx.sections, ctx.commandOverride) === 'hidden' + +/** + * Whether a settled transcript block paints anything. A trail renders nothing + * when it has no reasoning/tools/todos to show (e.g. the finalDetails segment + * that carries only a token tally) or when every section it does have is hidden + * (`/details hidden`); every other block draws at least one row. A block that + * renders nothing is *transparent* to grouping: the block below it draws its + * boundary against the nearest visible block instead (see prevRenderedMsg), so + * a hidden or content-less trail never leaves a floating blank line, doubles + * the gap after a user prompt, or pads the space above the final reply. In the + * default/collapsed modes content-bearing trails always render, so this is a + * no-op there. + */ +export const blockRenders = (msg: Pick, ctx: DetailsCtx): boolean => { + if (msg.kind !== 'trail') { + return true + } + + if (msg.todos?.length) { + return true + } + + if (!(msg.tools?.length || msg.thinking?.trim())) { + return false + } + + return !trailAllHidden(ctx) +} + +/** + * The nearest block above `index` that actually renders, resolved through a + * lazy accessor so it works over either the virtualized history rows or the + * live block list. This is the grouping predecessor — using it (instead of the + * literal previous row) keeps hidden trails from interrupting the rhythm. + */ +export const prevRenderedMsg = ( + msgAt: (i: number) => Msg | undefined, + index: number, + ctx: DetailsCtx +): Msg | undefined => { + for (let i = index - 1; i >= 0; i--) { + const candidate = msgAt(i) + + if (candidate && blockRenders(candidate, ctx)) { + return candidate + } + } + + return undefined +} diff --git a/ui-tui/src/lib/virtualHeights.ts b/ui-tui/src/lib/virtualHeights.ts index 4ae2ee3f7..e1760cf86 100644 --- a/ui-tui/src/lib/virtualHeights.ts +++ b/ui-tui/src/lib/virtualHeights.ts @@ -72,6 +72,7 @@ export const estimatedMsgHeight = ( { compact, details, + leadGap = false, thinkingVisible = details, toolsVisible = details, userPrompt = '', @@ -79,6 +80,7 @@ export const estimatedMsgHeight = ( }: { compact: boolean details: boolean + leadGap?: boolean thinkingVisible?: boolean toolsVisible?: boolean userPrompt?: string @@ -129,11 +131,22 @@ export const estimatedMsgHeight = ( } if (msg.role === 'user' || msg.kind === 'diff') { + // Top + bottom blank line. h += 2 } else if (msg.kind === 'slash') { h++ } + // Group-boundary blank line owned by BlockSlot: model prose, reasoning/tool + // trails, and notes/errors each start a new visual group when the block + // above them is a different kind. The caller resolves the boundary against + // the previous row (see domain/blockLayout.ts::hasLeadGap) and passes the + // result here so the estimate matches the rendered marginTop before Yoga + // remeasures. user / diff / slash never set this — they own their margins. + if (leadGap) { + h++ + } + // Inter-turn separator above non-first user messages (1 rule row + 1 // top-margin row). The render-side gate is in appLayout.tsx; we trust // the caller to pass `withSeparator` only when it matches that gate.