Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
118 changes: 118 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('keeps a streaming frontier table horizontal instead of the vertical list', () => {
// A tall cell trips the vertical fallback, which would render the table as
// a `label: value` list and then flip to a horizontal table once it
// completes (a visible jump). The live streaming frontier table must stay
// horizontal; the committed render may fall back to vertical.
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() ?? '';

expect(stripAnsi(streaming)).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,72 @@ 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('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
95 changes: 76 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,41 @@ 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
}
if (!insideCodeFence) {
const headerCells = splitMarkdownTableRow(
lines[start]!.match(tableRowRegex)?.[1] ?? '',
).filter((c) => c.length > 0).length;
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 headerCells > 0 && 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 +381,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 +412,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 +614,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 All @@ -587,6 +633,7 @@ const MarkdownDisplayInternal: React.FC<MarkdownDisplayProps> = ({
aligns={tableAligns}
enableInlineMath={renderVisualBlocks}
isPending={isPending}
isStreamingFrontier={true}
availableTerminalHeight={availableTerminalHeight}
/>,
);
Expand Down Expand Up @@ -901,6 +948,14 @@ interface RenderTableProps {
aligns?: ColumnAlign[];
enableInlineMath?: boolean;
isPending?: boolean;
/**
* True only for the table at the live streaming frontier — the one still
* being written at the end of a pending message. A table closed earlier in
* the same (still pending) message is already COMPLETE, so it must size its
* columns from every row immediately (isStreaming false) instead of staying
* frozen to the first row until the whole message finishes.
*/
isStreamingFrontier?: boolean;
availableTerminalHeight?: number;
}

Expand All @@ -911,6 +966,7 @@ const RenderTableInternal: React.FC<RenderTableProps> = ({
aligns,
enableInlineMath = false,
isPending = false,
isStreamingFrontier = false,
availableTerminalHeight,
}) => {
const maxHeight =
Expand All @@ -925,6 +981,7 @@ const RenderTableInternal: React.FC<RenderTableProps> = ({
aligns={aligns}
enableInlineMath={enableInlineMath}
maxHeight={maxHeight}
isStreaming={isPending && isStreamingFrontier}
/>
);
};
Expand Down
95 changes: 95 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,99 @@ 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('┌');
});

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 renderStreaming = (rows: string[][]) =>
renderWithProviders(
<TableRenderer
headers={headers}
rows={rows}
contentWidth={80}
isStreaming
/>,
).lastFrame() ?? '';

it('widens the streaming 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(renderStreaming(withWiderRow))).toBeGreaterThan(
topBorderWidth(renderStreaming(firstRowOnly)),
);
});

it('stays horizontal while streaming even when a row would wrap tall', () => {
// A cell tall enough to trip the vertical fallback (maxRowLines) must NOT
// flip a streaming table into the vertical `label: value` list — that
// brief list-then-table flip is a visible jump. contentWidth stays well
// above the narrow-terminal threshold so only the tall-row trigger differs.
const tall = [
[Array.from({ length: 80 }, (_, i) => `w${i}`).join(' '), 'y'],
];
const streaming =
renderWithProviders(
<TableRenderer
headers={headers}
rows={tall}
contentWidth={60}
isStreaming
/>,
).lastFrame() ?? '';
const completed =
renderWithProviders(
<TableRenderer headers={headers} rows={tall} contentWidth={60} />,
).lastFrame() ?? '';
// Streaming keeps the horizontal box; the completed render may fall back
// to the vertical list for the tall cell.
expect(stripAnsi(streaming)).toContain('┌');
expect(stripAnsi(completed)).not.toContain('┌');
});
});
});
Loading
Loading