Skip to content
Merged
Show file tree
Hide file tree
Changes from 10 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
202 changes: 202 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,156 @@ 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('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(
<MarkdownDisplay {...baseProps} text={text} isPending={true} />,
);
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(
<MarkdownDisplay {...baseProps} text={text} isPending={true} />,
);
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(
<MarkdownDisplay {...baseProps} text={text} isPending={true} />,
);
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(
<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('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(
<MarkdownDisplay {...baseProps} text={text} isPending={true} />,
);
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(
<MarkdownDisplay {...baseProps} text={text} isPending={true} />,
);
const output = lastFrame() ?? '';
expect(output).toContain('Alpha');
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
127 changes: 108 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,77 @@ 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.)
// 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) {
let hdr = lines[start]!.replace(/^\s*\|/, '');
if (/\|\s*$/.test(hdr)) hdr = hdr.replace(/\|\s*$/, '');
headerCells = splitMarkdownTableRow(hdr).length;
}
if (headerCells >= 2) {
Comment thread
MikeWang0316tw marked this conversation as resolved.
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];
const couldStillBeTable =
Comment thread
MikeWang0316tw marked this conversation as resolved.
Outdated
lineAfterHeader === undefined ||
tableSeparatorRegex.test(lineAfterHeader);
if (!hasMatchingSeparator && couldStillBeTable) {
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 +417,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 +448,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 +650,23 @@ 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) {
//
// 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). Committed (isPending=false) tables
// always have rows, so `!isPending` keeps their behavior unchanged.
if (
inTable &&
tableHeaders.length > 0 &&
(tableRows.length > 0 || !isPending)
Comment thread
MikeWang0316tw marked this conversation as resolved.
Outdated
) {
addContentBlock(
<RenderTable
key={`table-${contentBlocks.length}`}
Expand Down
Loading
Loading