Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
142 changes: 142 additions & 0 deletions packages/cli/src/ui/utils/MarkdownDisplay.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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(
<MarkdownDisplay {...baseProps} text={text} isPending={true} />,
);
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 |
|---|---|
Expand Down Expand Up @@ -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(
<MarkdownDisplay
{...baseProps}
text={text}
isPending={true}
contentWidth={60}
/>,
).lastFrame() ?? '';
const committed =
renderWithProviders(
<MarkdownDisplay
{...baseProps}
text={text}
isPending={false}
contentWidth={60}
/>,
).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 |
|---|---|
Expand All @@ -391,6 +443,96 @@ 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(
<MarkdownDisplay {...baseProps} text={text} isPending={true} />,
);
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(
<MarkdownDisplay {...baseProps} text={text} isPending={true} />,
);
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(
<MarkdownDisplay {...baseProps} text={text} isPending={true} />,
);
expect(lastFrame() ?? '').toContain('grep foo');
});

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(
<MarkdownDisplay {...baseProps} text={text} isPending={true} />,
);
expect((lastFrame() ?? '').includes('Alpha')).toBe(false);
});

it('renders the table once the separator matches the header columns', () => {
const text = `| Alpha | Beta |
|---|---|
| 1 | 2 |`.replace(/\n/g, eol);
const { lastFrame } = renderWithProviders(
<MarkdownDisplay {...baseProps} text={text} isPending={true} />,
);
const output = lastFrame() ?? '';
expect(output).toContain('Alpha');
expect(output).toContain('│'); // drawn as a table box, not raw text
});

it('draws the empty table box once recognized, before the first row completes', () => {
// Header + separator recognized but the first row is still being typed:
// show the header box immediately (empty body) rather than a long blank
// while the first row streams (a stall in that window looked like a hang).
const text = `| Alpha | Beta |
|---|---|
| 1`.replace(/\n/g, eol);
const { lastFrame } = renderWithProviders(
<MarkdownDisplay {...baseProps} text={text} isPending={true} />,
);
const output = lastFrame() ?? '';
expect(output).toContain('Alpha'); // header box drawn with zero rows
expect(output).toContain('│');
});

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(
<MarkdownDisplay
{...baseProps}
text={text}
isPending={true}
availableTerminalHeight={20}
/>,
);
expect(stripAnsi(lastFrame() ?? '')).toContain('Alpha');
});

it('renders a single-column table', () => {
const text = `
| Name |
Expand Down
94 changes: 75 additions & 19 deletions packages/cli/src/ui/utils/MarkdownDisplay.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,51 @@ const MarkdownDisplayInternal: React.FC<MarkdownDisplayProps> = ({
}
}

// 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) {
Comment thread
MikeWang0316tw marked this conversation as resolved.
// Don't touch pipe-lines that are actually fenced code-block content.
let insideCodeFence = false;
for (let i = 0; i < start; i++) {
if (codeFenceRegex.test(lines[i]!)) insideCodeFence = !insideCodeFence;
Comment thread
MikeWang0316tw marked this conversation as resolved.
Outdated
}
// 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.)
const headerCells = insideCodeFence
? 0
: splitMarkdownTableRow(lines[start]!).filter((c) => c.length > 0)
Comment thread
MikeWang0316tw marked this conversation as resolved.
Outdated
.length;
if (headerCells >= 2) {
Comment thread
MikeWang0316tw marked this conversation as resolved.
const hasMatchingSeparator = lines.slice(start + 1).some((l) => {
if (!tableSeparatorRegex.test(l)) return false;
const cols = splitMarkdownTableRow(l).filter(
(c) => c.length > 0,
).length;
return cols === headerCells;
});
if (!hasMatchingSeparator) {
lines = lines.slice(0, start);
}
}
}
}

/** Parse column alignments from a markdown table separator like `|:---|:---:|---:|` */
const parseTableAligns = (line: string): ColumnAlign[] =>
splitMarkdownTableRow(line)
Expand Down Expand Up @@ -346,6 +391,26 @@ const MarkdownDisplayInternal: React.FC<MarkdownDisplayProps> = ({
} 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]);
Expand All @@ -357,24 +422,6 @@ const MarkdownDisplayInternal: React.FC<MarkdownDisplayProps> = ({
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
Expand Down Expand Up @@ -577,7 +624,16 @@ const MarkdownDisplayInternal: React.FC<MarkdownDisplayProps> = ({
// slice above keeps preceding content within budget, so it can never overflow
// and lock the terminal. Renders in full once the message commits to <Static>
// (isPending=false → no clamp).
if (inTable && tableHeaders.length > 0 && tableRows.length > 0) {
//
// No `tableRows.length > 0` guard: once the header + separator are recognized
// draw the empty table box right away (its data rows fill in as they complete
// via the per-row hold-back). Otherwise the whole table area stays blank from
// the moment it is recognized until the first row terminates — if generation
// stalls in that window it looks like a hang (no box, no cue). The header does
// NOT flash char by char: the pre-loop trim holds it until the separator lands,
// so the box appears atomically. Committed (isPending=false) tables always have
// rows, so this only affects the live frontier.
if (inTable && tableHeaders.length > 0) {
addContentBlock(
<RenderTable
key={`table-${contentBlocks.length}`}
Expand Down
90 changes: 90 additions & 0 deletions packages/cli/src/ui/utils/TableRenderer.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -864,4 +864,94 @@ describe('<TableRenderer />', () => {
expect(stripAnsi(output)).toContain('more rows streaming');
});
});

describe('streaming table rendering (isStreaming)', () => {
Comment thread
MikeWang0316tw marked this conversation as resolved.
Outdated
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(
<TableRenderer
headers={['Alpha', 'Beta']}
rows={[]}
contentWidth={80}
/>,
).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(
<TableRenderer
headers={['Alpha', 'Beta']}
rows={[]}
contentWidth={20}
/>,
).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(
<TableRenderer
headers={['C1', 'C2', 'C3', 'C4', 'C5', 'C6']}
rows={[]}
contentWidth={25}
/>,
).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());
};
const render = (rows: string[][]) =>
renderWithProviders(
<TableRenderer headers={headers} rows={rows} contentWidth={80} />,
).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 table (no format flip)', () => {
// The horizontal-vs-vertical decision must not depend on a streaming flag,
// so a table never flips format mid-stream. A cell tall enough to trip the
// vertical fallback renders vertical, identically at every point.
const tall = [
[Array.from({ length: 80 }, (_, i) => `w${i}`).join(' '), 'y'],
];
expect(stripAnsi(render(tall))).not.toContain('┌'); // vertical list
});
});
});
Loading
Loading