diff --git a/packages/cli/src/ui/utils/MarkdownDisplay.test.tsx b/packages/cli/src/ui/utils/MarkdownDisplay.test.tsx index efcb10b22b..f916c0a710 100644 --- a/packages/cli/src/ui/utils/MarkdownDisplay.test.tsx +++ b/packages/cli/src/ui/utils/MarkdownDisplay.test.tsx @@ -5,6 +5,7 @@ */ import { beforeEach, describe, expect, it, vi } from 'vitest'; +import stripAnsi from 'strip-ansi'; import { MarkdownDisplay } from './MarkdownDisplay.js'; import { LoadedSettings } from '../../config/settings.js'; import { renderWithProviders } from '../../test-utils/render.js'; @@ -329,6 +330,24 @@ Some text before. expect(output).toContain('│'); }); + it('holds back a frontier row that is closed but missing columns', () => { + // `| four | five |` on a 3-column table is an intermediate state of + // `| four | five | six |` still being typed: it matches the row regex but + // has too few cells, so it must not render (and fill in cell by cell) + // until every column has arrived. + const text = `| A | B | C | +|---|---|---| +| one | two | three | +| four | five |`.replace(/\n/g, eol); + const { lastFrame } = renderWithProviders( + , + ); + const output = lastFrame() ?? ''; + expect(output).toContain('one'); + expect(output).toContain('three'); + expect(output).not.toContain('four'); + }); + it('renders the previously-held frontier row once it terminates', () => { const text = `| A | B | |---|---| @@ -366,6 +385,39 @@ Some text before. expect(out).toContain('two'); }); + it('renders a tall-wrapping table the same way streaming and committed (no flip)', () => { + // The horizontal-vs-vertical decision is identical while pending and once + // committed, so a table never flips format mid-stream (which reads as a + // jump). A tall cell trips the vertical fallback in BOTH renders. + const tallCell = Array.from({ length: 80 }, (_, i) => `w${i}`).join(' '); + const text = `| Col | +|---| +| ${tallCell} |`.replace(/\n/g, eol); + + const streaming = + renderWithProviders( + , + ).lastFrame() ?? ''; + const committed = + renderWithProviders( + , + ).lastFrame() ?? ''; + + // Both vertical (no box border) — same format, no flip between the two. + expect(stripAnsi(streaming)).not.toContain('┌'); + expect(stripAnsi(committed)).not.toContain('┌'); + }); + it('does not hold back a partial row in committed (non-pending) output', () => { const text = `| A | B | |---|---| @@ -391,6 +443,248 @@ Done.`.replace(/\n/g, eol); expect(output).toContain('Done'); }); + it('holds back a forming table header until its separator arrives', () => { + // Header present but no separator yet — must not flash as raw `| a | b |` + // text (streaming in char by char) before the table box appears. + const text = `intro line +| Alpha | Beta |`.replace(/\n/g, eol); + const { lastFrame } = renderWithProviders( + , + ); + const output = lastFrame() ?? ''; + expect(output).toContain('intro line'); + expect(output).not.toContain('Alpha'); + }); + + it('holds back a multi-column header still being typed (no cell-by-cell flash)', () => { + // Incomplete header (no closing `|` yet) but already ≥2 columns: it must + // be held, not rendered as raw pipe text, so it does not flash in. + const text = `intro line +| Alpha | Bet`.replace(/\n/g, eol); + const { lastFrame } = renderWithProviders( + , + ); + const output = lastFrame() ?? ''; + expect(output).toContain('intro line'); + expect(output).not.toContain('Alpha'); + }); + + it('does not hold back non-table pipe-leading text', () => { + // A trailing pipe-line that is not a complete table header (e.g. a shell + // pipeline or pipe-prefixed log line) must still render, not vanish. + const text = `run: +| grep foo`.replace(/\n/g, eol); + const { lastFrame } = renderWithProviders( + , + ); + expect(lastFrame() ?? '').toContain('grep foo'); + }); + + it('releases multi-cell non-table pipe content once a non-separator line follows', () => { + // A log excerpt / multi-pipe shell output has ≥2 cells per line, so the + // header heuristic alone would hold it for the whole stream. But the line + // after the "header" is not a separator, so it is not a forming table and + // must render live, not vanish until commit. + const text = `logs: +| 200 | OK | GET /a +| 500 | ERR | GET /b`.replace(/\n/g, eol); + const { lastFrame } = renderWithProviders( + , + ); + const output = lastFrame() ?? ''; + expect(output).toContain('200'); + expect(output).toContain('500'); + }); + + it('releases an options table whose first data cell starts with a dash', () => { + // `| --verbose | … |` after a header looks separator-ish to a naive + // "starts with a dash" check and would be held all stream. It is not a + // real separator (trailing letters), so it must render live. + const text = `flags: +| Flag | Description | +| --verbose | Enable verbose output |`.replace(/\n/g, eol); + const { lastFrame } = renderWithProviders( + , + ); + expect(lastFrame() ?? '').toContain('--verbose'); + }); + + it('does not hold back a header with an empty-named column once its separator matches', () => { + // `| A || B |` is a 3-column table to the renderer (the empty middle cell + // counts). The hold-back must count columns the same way, or it never + // finds the matching 3-column separator and hides the table all stream. + const text = `intro line +| A || B | +| - | - | - | +| x || y |`.replace(/\n/g, eol); + const { lastFrame } = renderWithProviders( + , + ); + const output = lastFrame() ?? ''; + expect(output).toContain('x'); + expect(output).toContain('y'); + }); + + it('keeps holding the header while its separator is still being typed', () => { + // A partial separator whose column count does not yet match the header is + // not enough to recognize the table, so the header stays held. + const text = `intro line +| Alpha | Beta | +|--`.replace(/\n/g, eol); + const { lastFrame } = renderWithProviders( + , + ); + expect((lastFrame() ?? '').includes('Alpha')).toBe(false); + }); + + it('releases a complete separator whose column count differs from the header', () => { + // Header has 3 columns, the separator is complete (ends with `|`) but has + // only 2 — it can never become a matching separator, and the main parser + // treats it as plain text. So it must render, not be held for the stream. + const text = `| A | B | C | +| --- | --- |`.replace(/\n/g, eol); + const { lastFrame } = renderWithProviders( + , + ); + expect(lastFrame() ?? '').toContain('A'); + }); + + it('does not hold a pipe line inside a nested (longer) code fence', () => { + // A ```` fence is still open; an inner ``` (shorter) does NOT close it, so a + // `| … |` line after it is code content and must render. A naive fence + // toggle would treat the inner ``` as a close and hold the pipe line back. + const text = `\`\`\`\` +| code example | +\`\`\` +| ZZZ | YYY |`.replace(/\n/g, eol); + const { lastFrame } = renderWithProviders( + , + ); + expect(stripAnsi(lastFrame() ?? '')).toContain('ZZZ'); + }); + + it('does not let a backtick fence close an open tilde fence', () => { + // An open ~~~ fence must not be closed by an inner ``` (different char), so + // the `| … |` line after it stays code content. Guards the fence-char check + // for tilde fences (existing tests only cover backticks). + const text = `~~~ +| code | +\`\`\` +| ZZZ | YYY |`.replace(/\n/g, eol); + const { lastFrame } = renderWithProviders( + , + ); + expect(stripAnsi(lastFrame() ?? '')).toContain('ZZZ'); + }); + + it('does not hold a pipe line inside an open display-math block', () => { + // Inside a `$$ … $$` block the main parser pushes every line verbatim as + // math content, never as a table. The hold-back must mirror that: a `| … |` + // line (a norm/matrix row) while the math block is still open is NOT a + // forming table and must render, not be blanked until the block closes. + const text = `$$ +| ZZZ | YYY |`.replace(/\n/g, eol); + const { lastFrame } = renderWithProviders( + , + ); + expect(stripAnsi(lastFrame() ?? '')).toContain('ZZZ'); + }); + + it('renders the table once the separator matches the header columns', () => { + const text = `| Alpha | Beta | +|---|---| +| 1 | 2 |`.replace(/\n/g, eol); + const { lastFrame } = renderWithProviders( + , + ); + const output = lastFrame() ?? ''; + expect(output).toContain('Alpha'); + expect(output).toContain('│'); // drawn as a table box, not raw text + }); + + it('defers the table while it has no complete data row (no empty header box)', () => { + // Header + separator recognized but the first row is still being typed. A + // zero-row table can only render horizontally, so drawing the empty box now + // and flipping to vertical once a long first row lands is a visible format + // change. Defer instead: nothing is drawn until the first row completes. + const text = `| Alpha | Beta | +|---|---| +| 1`.replace(/\n/g, eol); + const { lastFrame } = renderWithProviders( + , + ); + const output = lastFrame() ?? ''; + expect(output).not.toContain('Alpha'); // table not drawn yet + expect(output).not.toContain('│'); + }); + + it('draws the table once its first row completes', () => { + // The deferred table appears — already in its final format — as soon as the + // first data row terminates. + const text = `| Alpha | Beta | +|---|---| +| 1 | 2 |`.replace(/\n/g, eol); + const { lastFrame } = renderWithProviders( + , + ); + const output = lastFrame() ?? ''; + expect(output).toContain('Alpha'); + expect(output).toContain('│'); + }); + + it('uses the final (all-rows) format for a completed mid-content table while streaming', () => { + // A table CLOSED by a following line is complete even while the message + // streams on, so its format is decided from all rows now — it must not + // render horizontal and then flip to vertical at commit. Short first row + + // a tall later row → vertical (no box border), not a horizontal grid. + const tall = Array.from({ length: 80 }, (_, i) => `w${i}`).join(' '); + const text = `| A | B | +|---|---| +| x | y | +| ${tall} | y | +trailing text`.replace(/\n/g, eol); + const { lastFrame } = renderWithProviders( + , + ); + const output = stripAnsi(lastFrame() ?? ''); + expect(output).toContain('trailing text'); // the table is mid-content + expect(output).not.toContain('┌'); // vertical, no horizontal box + }); + + it('does not hold back pipe lines inside a pending code block', () => { + // A `|`-leading line that is fenced code-block content must still render. + const text = '```\n| Alpha | Beta |'.replace(/\n/g, eol); + const { lastFrame } = renderWithProviders( + , + ); + expect(stripAnsi(lastFrame() ?? '')).toContain('Alpha'); + }); + it('renders a single-column table', () => { const text = ` | Name | diff --git a/packages/cli/src/ui/utils/MarkdownDisplay.tsx b/packages/cli/src/ui/utils/MarkdownDisplay.tsx index 4a4585576a..91472f0635 100644 --- a/packages/cli/src/ui/utils/MarkdownDisplay.tsx +++ b/packages/cli/src/ui/utils/MarkdownDisplay.tsx @@ -191,6 +191,123 @@ const MarkdownDisplayInternal: React.FC = ({ } } + // Hold back a still-forming table at the streaming frontier. A table is only + // recognized once its separator line (matching the header's column count) has + // arrived; until then the header — and any partial separator — would render as + // raw `| a | b |` text and stream in character by character before snapping + // into a table box. So while pending, trim a trailing run of pipe-lines that + // does not yet contain a matching separator: nothing renders for it until it + // can render as a table. (A run that IS a recognizable table is kept — the + // per-row hold-back below then handles its unterminated frontier row.) + if (isPending && renderVisualBlocks && lines.length > 0) { + let start = lines.length; + while (start > 0 && /^\s*\|/.test(lines[start - 1]!)) start--; + if (start < lines.length) { + // Don't touch pipe-lines that are actually fenced code-block OR display-math + // (`$$ … $$`) content: the main parser pushes those verbatim, never as a + // table (a `| a | b |` norm/matrix line inside `$$` would otherwise be held + // back as a forming table and blank until the block closes). Track the OPEN + // code fence's delimiter, not a naive toggle: a closing fence must use the + // same char and be at least as long (mirrors the main parser), or a nested + // fence (```` inside ```` ) mis-toggles and a real code line like `| A | B |` + // gets held back. Mirror the main parser's precedence — a code block wins, + // then a math block — so a `$$` inside a code fence does not open math. + let activeCodeFence = ''; + let insideMathBlock = false; + for (let i = 0; i < start; i++) { + const line = lines[i]!; + if (activeCodeFence) { + const fenceMatch = line.match(codeFenceRegex); + if ( + fenceMatch && + fenceMatch[1]!.startsWith(activeCodeFence[0]!) && + fenceMatch[1]!.length >= activeCodeFence.length + ) { + activeCodeFence = ''; + } + continue; + } + if (insideMathBlock) { + if (mathFenceRegex.test(line)) insideMathBlock = false; + continue; + } + const fenceMatch = line.match(codeFenceRegex); + if (fenceMatch) { + activeCodeFence = fenceMatch[1]!; + } else if (mathFenceRegex.test(line)) { + insideMathBlock = true; + } + } + const insideCodeFence = activeCodeFence !== ''; + // Only hold back a plausible forming TABLE, not arbitrary pipe text. A + // table header has ≥2 columns; a single-pipe line (an un-fenced shell + // pipeline `| grep foo`, a pipe-prefixed log line) has one cell and must + // render. Count cells on the first line whether or not it is closed yet, + // so a multi-column header held mid-type does not flash in cell by cell + // before its separator arrives. (A header still typing its very first + // cell — one cell so far — is indistinguishable from a single-pipe line, + // so it renders briefly until the second column appears; that is the + // narrowest flash we can allow without hiding real non-table text.) + // Count header cells the way the main table detector does (strip the + // outer pipes, split WITHOUT dropping empty cells) so an empty-named + // column like `| A || B |` — which the renderer treats as a real table — + // agrees between this hold-back and the renderer, instead of being held + // back for the whole stream. The trailing pipe is stripped only when + // present, so a still-forming header (`| A | B`) is counted mid-type. + let headerCells = 0; + if (!insideCodeFence && !insideMathBlock) { + let hdr = lines[start]!.replace(/^\s*\|/, ''); + if (/\|\s*$/.test(hdr)) hdr = hdr.replace(/\|\s*$/, ''); + headerCells = splitMarkdownTableRow(hdr).length; + } + if (headerCells >= 2) { + const rest = lines.slice(start + 1); + const hasMatchingSeparator = rest.some((l) => { + if (!tableSeparatorRegex.test(l)) return false; + const cols = splitMarkdownTableRow(l).filter( + (c) => c.length > 0, + ).length; + return cols === headerCells; + }); + // A markdown table's separator is the line IMMEDIATELY after the header. + // So once a line follows the header and it is not a (possibly still + // forming) separator row, this pipe run is decided: NOT a forming table — + // a multi-cell shell pipeline (`| grep foo | wc -l`), a log excerpt + // (`| 200 | OK | GET /x`), an options table whose first cell starts with + // a dash (`| --verbose | … |`), or an ASCII-art border. Release it rather + // than hiding it for the whole stream. `tableSeparatorRegex` matches a + // partial separator (`|--`) so a real header whose separator is still + // being typed stays held; it rejects a dash-led data cell like + // `--verbose` (trailing letters), which a looser "starts with a dash" + // test would wrongly hold. While only the header exists (no line after it + // yet) keep holding so a multi-column header does not flash in cell by + // cell before its separator arrives. + const lineAfterHeader = rest[0]; + let couldStillBeTable = + lineAfterHeader === undefined || + tableSeparatorRegex.test(lineAfterHeader); + // A COMPLETE separator (ends with `|`) whose column count already differs + // from the header will never become a valid table — the main parser + // treats it as plain text — so release it instead of holding the run for + // the rest of the stream. A still-forming separator (no closing `|`) can + // still gain columns, so keep holding it. + if ( + couldStillBeTable && + lineAfterHeader !== undefined && + /\|\s*$/.test(lineAfterHeader) + ) { + const sepCols = splitMarkdownTableRow(lineAfterHeader).filter( + (c) => c.length > 0, + ).length; + if (sepCols !== headerCells) couldStillBeTable = false; + } + if (!hasMatchingSeparator && couldStillBeTable) { + lines = lines.slice(0, start); + } + } + } + } + /** Parse column alignments from a markdown table separator like `|:---|:---:|---:|` */ const parseTableAligns = (line: string): ColumnAlign[] => splitMarkdownTableRow(line) @@ -346,6 +463,26 @@ const MarkdownDisplayInternal: React.FC = ({ } else if (inTable && tableSeparatorMatch) { // Parse alignment from separator line tableAligns = parseTableAligns(line); + } else if ( + isPending && + inTable && + index === lines.length - 1 && + tableHeaders.length > 0 && + /^\s*\|/.test(line) && + (!tableRowMatch || + splitMarkdownTableRow(tableRowMatch[1]).length < tableHeaders.length) + ) { + // Live streaming frontier: the final line is a row still being typed — + // either mid-cell (`| a | b` with no closing `|` yet) or closed but with + // fewer cells than the header (`| a |` while `| a | b | c |` is coming, an + // intermediate that itself matches the row regex). Both would otherwise + // render as a padded row that fills in cell by cell and flips as the + // closing `|`/columns arrive, jittering the frame. Hold the row back until + // it has all its columns: skip it so `inTable` stays set and the + // end-of-content handler renders only the COMPLETE rows as a live table. + // The whole row (border + all cells) then appears in one step. Guarded on + // `tableHeaders.length > 0` so the header + separator never blank out with + // a stray partial line beneath them while the first row is typed. } else if (inTable && tableRowMatch) { // Add table row const cells = splitMarkdownTableRow(tableRowMatch[1]); @@ -357,24 +494,6 @@ const MarkdownDisplayInternal: React.FC = ({ cells.length = tableHeaders.length; } tableRows.push(cells); - } else if ( - isPending && - inTable && - !tableRowMatch && - index === lines.length - 1 && - tableHeaders.length > 0 && - /^\s*\|/.test(line) - ) { - // Live streaming frontier: the final line is an unterminated table row or - // separator (`| a | b` with no closing `|` yet). Rendering it as a plain - // text line and then flipping it into the table once the closing `|` - // arrives makes the frame height and column widths oscillate on every - // token, which visibly jitters the footer/composer. Hold it back instead: - // skip it so `inTable` stays set and the end-of-content handler renders - // only the COMPLETE rows as a live table. A partial row never flips in; - // the table appears once its first row terminates and grows one complete - // row at a time. Until then the table is simply not drawn (no header + - // separator with a stray partial line beneath it). } else if (inTable && !tableRowMatch) { // End of table — a following line closes it, so this table is COMPLETE // and renders in full (the rendered-aware slice guarantees a completed @@ -390,6 +509,11 @@ const MarkdownDisplayInternal: React.FC = ({ aligns={tableAligns} enableInlineMath={renderVisualBlocks} isPending={isPending} + // A following non-table line closed this table: it is COMPLETE and + // will gain no more rows, so it is not the streaming frontier. Decide + // its format from all rows (not just the first) so it does not flip + // horizontal→vertical when the message finally commits. + isFrontier={false} availableTerminalHeight={availableTerminalHeight} />, ); @@ -577,6 +701,20 @@ const MarkdownDisplayInternal: React.FC = ({ // slice above keeps preceding content within budget, so it can never overflow // and lock the terminal. Renders in full once the message commits to // (isPending=false → no clamp). + // + // While PENDING, defer a table that has no COMPLETE data row yet. A zero-row + // table can only render horizontally (the vertical fallback needs rows to lay + // out), so drawing the empty header box and then flipping to the vertical + // `label: value` format once a long first row lands is a visible format change + // — and the format genuinely cannot be known from the header alone (column + // names are short; the width comes from the values). Waiting for the first + // complete row means the table first appears ALREADY in its final format, with + // no flip. Cost: the table area stays blank while the header + first row stream + // (the pre-loop trim already hid the header text, so this just extends that + // blank until the first row terminates). The `tableRows.length > 0` guard also + // matches the mid-content end-of-table handler above, so a degenerate zero-row + // table renders (or not) the same whether it ends the message or is followed by + // more text — pending or committed. if (inTable && tableHeaders.length > 0 && tableRows.length > 0) { addContentBlock( = ({ aligns={tableAligns} enableInlineMath={renderVisualBlocks} isPending={isPending} + // End of content: this table is at the streaming frontier and may still + // gain rows, so anchor its format to the first row while pending. + isFrontier={true} availableTerminalHeight={availableTerminalHeight} />, ); @@ -900,7 +1041,15 @@ interface RenderTableProps { contentWidth: number; aligns?: ColumnAlign[]; enableInlineMath?: boolean; + /** True while the whole message is still streaming — drives the height clamp. */ isPending?: boolean; + /** + * True only for the table at the streaming frontier (end of content, may still + * gain rows). A completed mid-content table passes false so its format is + * decided from all rows and does not flip when the message commits. Defaults + * true so a bare RenderTable behaves like the frontier. + */ + isFrontier?: boolean; availableTerminalHeight?: number; } @@ -911,8 +1060,15 @@ const RenderTableInternal: React.FC = ({ aligns, enableInlineMath = false, isPending = false, + isFrontier = true, availableTerminalHeight, }) => { + // The height clamp tracks whether the MESSAGE is streaming (overflow can grow + // on any tick). The format anchor tracks whether THIS TABLE is still streaming + // — only the frontier table anchors its format to the first row; a completed + // mid-content table measures all rows. Keeping the clamp on isPending (not + // isFrontier) means a mid-content table is still bounded, so the estimator's + // clamped cost still matches the render and cannot under-estimate. const maxHeight = isPending && availableTerminalHeight !== undefined ? Math.max(2, availableTerminalHeight - TABLE_PENDING_RESERVED_ROWS) @@ -924,6 +1080,7 @@ const RenderTableInternal: React.FC = ({ contentWidth={contentWidth} aligns={aligns} enableInlineMath={enableInlineMath} + isStreaming={isPending && isFrontier} maxHeight={maxHeight} /> ); diff --git a/packages/cli/src/ui/utils/TableRenderer.test.tsx b/packages/cli/src/ui/utils/TableRenderer.test.tsx index 28941b3d86..61eecb3cfa 100644 --- a/packages/cli/src/ui/utils/TableRenderer.test.tsx +++ b/packages/cli/src/ui/utils/TableRenderer.test.tsx @@ -864,4 +864,137 @@ describe('', () => { expect(stripAnsi(output)).toContain('more rows streaming'); }); }); + + describe('streaming table rendering (format stability)', () => { + it('renders a header-only box (no divider) when there are no data rows', () => { + // The live empty box shown while the first row streams: header + borders + // only. The header/body divider must be skipped so it does not stack on + // the bottom border and read as an empty second row. + const output = + renderWithProviders( + , + ).lastFrame() ?? ''; + const clean = stripAnsi(output); + expect(clean).toContain('Alpha'); + expect(clean).toContain('┌'); + expect(clean).toContain('└'); + expect(clean).not.toContain('├'); + }); + + it('keeps the zero-row header box horizontal on a narrow terminal', () => { + // The width trigger would otherwise force the vertical format, which with + // no rows renders an empty string — a blank box instead of the header. + const output = + renderWithProviders( + , + ).lastFrame() ?? ''; + const clean = stripAnsi(output); + expect(clean).toContain('Alpha'); + expect(clean).toContain('┌'); + }); + + it('keeps the zero-row box visible when it exceeds a very narrow terminal', () => { + // The maxLineWidth safety check is a second path to the vertical format; + // for a zero-row box that would render an empty string (blank). The header + // box must stay visible rather than vanish. + const output = + renderWithProviders( + , + ).lastFrame() ?? ''; + const clean = stripAnsi(output); + expect(clean).toContain('┌'); + expect(clean).toContain('C1'); + }); + + const headers = ['A', 'B']; + const firstRowOnly = [['x', 'y']]; + const withWiderRow = [ + ['x', 'y'], + ['x', 'a considerably wider streaming cell'], + ]; + const topBorderWidth = (frame: string) => { + const line = + stripAnsi(frame) + .split('\n') + .find((l) => l.includes('┌')) ?? ''; + return stringWidth(line.trim()); + }; + // Defaults to streaming (this block covers streaming behavior); pass false + // to exercise a completed table's all-rows format decision. + const render = (rows: string[][], isStreaming = true) => + renderWithProviders( + , + ).lastFrame() ?? ''; + + it('widens the table when a wider row is appended (redraw on wider)', () => { + // Widths always track the current rows: appending a wider row re-sizes + // (redraws) the whole table rather than staying frozen to the first row. + expect(topBorderWidth(render(withWiderRow))).toBeGreaterThan( + topBorderWidth(render(firstRowOnly)), + ); + }); + + it('picks the vertical format for a tall-wrapping first row', () => { + // A single row tall enough to trip the vertical fallback renders vertical + // — the same whether streaming or committed (its one row IS the first row). + const tall = [ + [Array.from({ length: 80 }, (_, i) => `w${i}`).join(' '), 'y'], + ]; + expect(stripAnsi(render(tall))).not.toContain('┌'); // vertical list + expect(stripAnsi(render(tall, false))).not.toContain('┌'); + }); + + it('does not flip to vertical when a tall row is appended after a short first row', () => { + // The vertical decision is anchored to the header + FIRST row. A short + // first row renders horizontal; appending a later tall-wrapping row must + // NOT flip the whole table to vertical mid-stream — it stays a (taller) + // horizontal grid. Guards the no-flip guarantee against later rows. + const tallLaterRow = Array.from({ length: 80 }, (_, i) => `w${i}`).join( + ' ', + ); + const output = stripAnsi( + render([ + ['x', 'y'], + [tallLaterRow, 'y'], + ]), + ); + expect(output).toContain('┌'); // still horizontal (box drawn) + }); + + it('DOES use the vertical format for the same table once committed', () => { + // A committed (non-pending) table has all rows and no flip concern, so it + // measures every row: a short first row + a tall later row goes vertical, + // which is more readable than a tall horizontal grid. + const tallLaterRow = Array.from({ length: 80 }, (_, i) => `w${i}`).join( + ' ', + ); + const output = stripAnsi( + render( + [ + ['x', 'y'], + [tallLaterRow, 'y'], + ], + false, + ), + ); + expect(output).not.toContain('┌'); // vertical list + }); + }); }); diff --git a/packages/cli/src/ui/utils/TableRenderer.tsx b/packages/cli/src/ui/utils/TableRenderer.tsx index 7212a5aa4e..0feed88d4c 100644 --- a/packages/cli/src/ui/utils/TableRenderer.tsx +++ b/packages/cli/src/ui/utils/TableRenderer.tsx @@ -63,6 +63,14 @@ interface TableRendererProps { /** Per-column alignment parsed from markdown separator line */ aligns?: ColumnAlign[]; enableInlineMath?: boolean; + /** + * True while THIS table is still streaming its rows (the frontier). The + * horizontal-vs-vertical decision is then anchored to the first row so the + * format cannot flip as later rows arrive; a completed table (false/undefined) + * — committed, or a mid-content table already closed by following text — + * measures every row for the most readable layout. + */ + isStreaming?: boolean; /** * Maximum rendered text lines the table may occupy. When set (streaming * preview) and the fully rendered table exceeds it, output is clipped to @@ -435,6 +443,7 @@ export const TableRenderer: React.FC = ({ contentWidth, aligns, enableInlineMath = false, + isStreaming = false, maxHeight, }) => { const colCount = headers.length; @@ -551,20 +560,35 @@ export const TableRenderer: React.FC = ({ } // ── Step 4: Check max row lines to decide vertical fallback ── + // While STREAMING (isStreaming), measure only the header + the FIRST data row. + // Using every row lets a later, taller row push maxRowLines over the threshold + // and flip an already-horizontal table to vertical mid-stream — a visible + // format change. The first row is representative for the common case, so + // anchoring to it keeps the format stable as rows stream in. A COMPLETED table + // (committed, or a mid-content table already closed by text) has all its rows + // and no flip concern, so it measures EVERY row for the most readable layout (a + // short first row followed by tall rows still goes vertical). Column WIDTHS + // always track all rows (redraw-on-wider is unchanged); only the + // horizontal-vs-vertical CHOICE is anchored to the first row while streaming. function calculateMaxRowLines(): number { let maxLines = 1; + const rowsToMeasure = isStreaming ? rowMetrics.slice(0, 1) : rowMetrics; for (let i = 0; i < colCount; i++) { - const wrapped = wrapText(headerMetrics[i]!.rendered, columnWidths[i]!, { - hard: needsHardWrap, - }); - maxLines = Math.max(maxLines, wrapped.length); + const headerWrapped = wrapText( + headerMetrics[i]!.rendered, + columnWidths[i]!, + { + hard: needsHardWrap, + }, + ); + maxLines = Math.max(maxLines, headerWrapped.length); } - for (const row of rowMetrics) { + for (const row of rowsToMeasure) { for (let i = 0; i < colCount; i++) { - const wrapped = wrapText(row[i]!.rendered, columnWidths[i]!, { + const cellWrapped = wrapText(row[i]!.rendered, columnWidths[i]!, { hard: needsHardWrap, }); - maxLines = Math.max(maxLines, wrapped.length); + maxLines = Math.max(maxLines, cellWrapped.length); } } return maxLines; @@ -581,8 +605,20 @@ export const TableRenderer: React.FC = ({ ABSOLUTE_MIN_HORIZONTAL_TABLE_WIDTH, colCount * MIN_COLUMN_WIDTH + borderOverhead + SAFETY_MARGIN, ); + // The horizontal-vs-vertical decision is the SAME while streaming and once + // committed, so a table never flips format mid-stream (which reads as a jump). + // A zero-row table is the live streaming header box: never vertical — the + // vertical fallback iterates the rows and with none would render an empty + // string (a blank box) on a narrow terminal where the width trigger fires. + // + // The vertical trigger is anchored to the header + first row (see + // calculateMaxRowLines) so appending rows does not flip the format. Residual + // corner: a very wide later row can force a proportional column shrink that + // re-wraps the first row taller; that is rare and only for genuinely + // overflowing tables, where vertical is the right call anyway. const useVerticalFormat = - contentWidth < minHorizontalTableWidth || maxRowLines > MAX_ROW_LINES; + rowMetrics.length > 0 && + (contentWidth < minHorizontalTableWidth || maxRowLines > MAX_ROW_LINES); // ── Helper: Get alignment for a column ── const getAlign = (colIndex: number): ColumnAlign => @@ -720,26 +756,35 @@ export const TableRenderer: React.FC = ({ const tableLines: string[] = []; tableLines.push(renderBorderLine('top')); tableLines.push(...renderRowLines(headerRendered, true)); - tableLines.push(renderBorderLine('middle')); - rowMetrics.forEach((row, rowIndex) => { - tableLines.push( - ...renderRowLines( - row.map((m) => m.rendered), - false, - ), - ); - if (rowIndex < rows.length - 1) { - tableLines.push(renderBorderLine('middle')); - } - }); + // With no data rows yet (a live table whose first row is still streaming), + // skip the header/body divider so the box reads as a clean header — otherwise + // the divider stacked directly on the bottom border looks like an empty row. + if (rowMetrics.length > 0) { + tableLines.push(renderBorderLine('middle')); + rowMetrics.forEach((row, rowIndex) => { + tableLines.push( + ...renderRowLines( + row.map((m) => m.rendered), + false, + ), + ); + if (rowIndex < rows.length - 1) { + tableLines.push(renderBorderLine('middle')); + } + }); + } tableLines.push(renderBorderLine('bottom')); // ── Safety check: verify no line exceeds content width ── const maxLineWidth = Math.max( ...tableLines.map((line) => getCachedStringWidth(stripAnsi(line))), ); - if (maxLineWidth > contentWidth - SAFETY_MARGIN) { - // Fallback to vertical format to prevent terminal resize flicker + if (rowMetrics.length > 0 && maxLineWidth > contentWidth - SAFETY_MARGIN) { + // Fallback to vertical format to prevent terminal resize flicker. Skipped + // for a zero-row streaming header box: the vertical format iterates the + // rows and would render an empty string (a blank box). Better to keep the + // horizontal header — even if it slightly overflows a very narrow terminal + // — than to make the box the PR draws immediately vanish. return ( {clampToMaxHeight(renderVerticalFormat())}