Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -127,4 +127,6 @@ tmp/
.venv
.codegraph
.qwen/computer-use/installed.json
# Auto-generated computer-use marker can also appear under nested packages.
**/.qwen/computer-use/
.playwright-mcp/
4 changes: 2 additions & 2 deletions packages/cli/src/ui/hooks/useGeminiStream.ts
Original file line number Diff line number Diff line change
Expand Up @@ -371,8 +371,8 @@ const STREAM_PENDING_ITEM_MAX_CHARS = 16_384;
// Rows kept in reserve below the commit budget so the incremental commit fires
// BEFORE MarkdownDisplay's safety-net clip (which reserves 2). Keeping the
// pending item's rendered height under the safety budget stops that clip from
// engaging and flickering "generating more" / hiding a table in step with the
// commit cycle.
// engaging and hiding a table (or slicing the tail) in step with the commit
// cycle.
const STREAM_PENDING_COMMIT_RESERVE_ROWS = 5;
// Conservative estimate of the rows the composer/footer occupy, used to derive
// a content-area height from terminalHeight before the live value is known.
Expand Down
109 changes: 92 additions & 17 deletions packages/cli/src/ui/utils/MarkdownDisplay.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -101,11 +101,11 @@ describe('<MarkdownDisplay />', () => {
/>,
);
const output = lastFrame() ?? '';
// Contiguous head + a "generating more" cue — NOT decimated (ink
// overflow="hidden" would drop interspersed rows) and NOT blank.
// Clipped to budget: head lines present, tail dropped. No "generating more"
// cue — incremental scrollback commit (PR #6170) streams content in
// real-time, so clipped content is "still streaming" not "delayed output".
expect(output).toContain('line 1');
expect(output).toContain('line 2');
expect(output).toContain('generating more');
// The tail is dropped (budget exceeded), so a late line is absent.
expect(output).not.toContain('line 60');
const lineCount = output.split('\n').length;
Expand Down Expand Up @@ -171,18 +171,14 @@ describe('<MarkdownDisplay />', () => {
);
const output = lastFrame() ?? '';
expect(output.split('\n').length).toBeLessThanOrEqual(10);
// The head-slice bounds code content to <= availableTerminalHeight - 2
// (= RenderCodeBlock's own inner budget), so the inner "generating more"
// never fires inside a slice — there is at most ONE cue, not two stacked.
expect(
(output.match(/generating more/g) ?? []).length,
).toBeLessThanOrEqual(1);
// No "generating more" cue — all cues removed (PR #6170 incremental commit
// handles real-time streaming).
expect(output.match(/generating more/g) ?? []).toHaveLength(0);
});

it('does not stack a double cue for a math block near the clip boundary', () => {
// RenderMathBlock reserves more rows (RESERVED_LINES=3) than a code block;
// the head-slice budget reserves 2 rows so a retained math block never
// hits its own inner truncation cue on top of the outer one.
// No "generating more" cue — all cues removed (PR #6170 incremental commit
// handles real-time streaming).
const text = [
'$$',
...Array.from({ length: 40 }, (_, i) => `x_{${i}} +`),
Expand All @@ -197,9 +193,7 @@ describe('<MarkdownDisplay />', () => {
/>,
);
const output = lastFrame() ?? '';
expect(
(output.match(/generating more/g) ?? []).length,
).toBeLessThanOrEqual(1);
expect(output.match(/generating more/g) ?? []).toHaveLength(0);
expect(output.split('\n').length).toBeLessThanOrEqual(8);
});

Expand All @@ -224,7 +218,7 @@ describe('<MarkdownDisplay />', () => {

it('applies the minimum floor at a degenerate budget of 1', () => {
// Math.max(MIN_PENDING_CONTENT_LINES, 1 - 2) = 1 (floored): keep one
// content line + the cue, never 0 or negative.
// content line, never 0 or negative. No "generating more" cue.
const text = Array.from({ length: 60 }, (_, i) => `line ${i + 1}`).join(
eol,
);
Expand All @@ -239,7 +233,7 @@ describe('<MarkdownDisplay />', () => {
const output = lastFrame() ?? '';
expect(output).toContain('line 1');
expect(output).not.toContain('line 2');
expect(output).toContain('generating more');
expect(output).not.toContain('generating more');
});

it('renders unordered lists with different markers', () => {
Expand Down Expand Up @@ -316,6 +310,87 @@ Some text before.
expect(lastFrame()).toMatchSnapshot();
});

it('holds back an unterminated trailing table row while streaming', () => {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] The four new table-streaming tests cover the main hold-back paths well, but none covers the tableRows.length > 0 guard edge case: a table with header+separator present but the first data row still partial (e.g. | A | B |\n|---|---|\n| one | tw). A test asserting current behavior (header/separator not rendered, partial row as plain text) would document this known limitation and prevent regressions if it's later changed.

— qwen3.7-max via Qwen Code /review

const text = `| A | B |
|---|---|
| one | two |
| three | fo`.replace(/\n/g, eol);
const { lastFrame } = renderWithProviders(
<MarkdownDisplay {...baseProps} text={text} isPending={true} />,
);
const output = lastFrame() ?? '';
// The completed row renders inside the table…
expect(output).toContain('one');
expect(output).toContain('two');
// …but the still-typing frontier row is held back until its closing `|`,
// so it never flips between a stray text line and a table row (the source
// of per-token footer jitter).
expect(output).not.toContain('three');
expect(output).toContain('│');
});

it('renders the previously-held frontier row once it terminates', () => {
const text = `| A | B |
|---|---|
| one | two |
| three | four |`.replace(/\n/g, eol);
const { lastFrame } = renderWithProviders(
<MarkdownDisplay {...baseProps} text={text} isPending={true} />,
);
const output = lastFrame() ?? '';
expect(output).toContain('three');
expect(output).toContain('four');
});

it('holds back a partial first row, then pops the table in once it terminates', () => {
// Header + separator present but the first data row is still being typed.
// The partial row is held back and the table is not drawn yet, so there is
// no header+separator with a stray partial line flashing beneath it.
const partial = `| A | B |
|---|---|
| one | tw`.replace(/\n/g, eol);
const { lastFrame: partialFrame } = renderWithProviders(
<MarkdownDisplay {...baseProps} text={partial} isPending={true} />,
);
expect(partialFrame() ?? '').not.toContain('one');

// Once the row terminates, the table appears complete.
const complete = `| A | B |
|---|---|
| one | two |`.replace(/\n/g, eol);
const { lastFrame: completeFrame } = renderWithProviders(
<MarkdownDisplay {...baseProps} text={complete} isPending={true} />,
);
const out = completeFrame() ?? '';
expect(out).toContain('one');
expect(out).toContain('two');
});

it('does not hold back a partial row in committed (non-pending) output', () => {
const text = `| A | B |
|---|---|
| one | two |
| three | fo`.replace(/\n/g, eol);
const { lastFrame } = renderWithProviders(
<MarkdownDisplay {...baseProps} text={text} isPending={false} />,
);
// Committed transcript is final, not a streaming frontier — render as-is.
expect(lastFrame() ?? '').toContain('three');
});

it('still closes a streaming table when a non-pipe line follows', () => {
const text = `| A | B |
|---|---|
| one | two |
Done.`.replace(/\n/g, eol);
const { lastFrame } = renderWithProviders(
<MarkdownDisplay {...baseProps} text={text} isPending={true} />,
);
const output = lastFrame() ?? '';
expect(output).toContain('one');
expect(output).toContain('Done');
});

it('renders a single-column table', () => {
const text = `
| Name |
Expand Down
102 changes: 59 additions & 43 deletions packages/cli/src/ui/utils/MarkdownDisplay.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,18 @@ import {
TABLE_SEPARATOR_RE,
CODE_FENCE_RE,
} from './pending-rendered-height.js';
// Minimum content lines to keep in a clipped live preview before the
// "generating more" cue (own constant — not coupled to MaxSizedBox's floor).
// Minimum content lines to keep in a clipped live preview (own constant — not
// coupled to MaxSizedBox's floor).
const MIN_PENDING_CONTENT_LINES = 1;

// Rows reserved from the viewport when clamping a streaming table's height:
// marginY 2 + one row of wrapped-cell safety headroom. Tables under-estimate
// their rendered height the most (a wrapped cell), so they keep one more
// reserved row than the other blocks. Shared by the render-side clamp
// (RenderTable's `maxHeight`) and the slice-side estimate (`tableClampRows`)
// so the two never diverge and let a table overflow the render cap.
const TABLE_PENDING_RESERVED_ROWS = 3;

interface MarkdownDisplayProps {
text: string;
isPending: boolean;
Expand Down Expand Up @@ -131,12 +139,13 @@ const MarkdownDisplayInternal: React.FC<MarkdownDisplayProps> = ({
// top lock"). The rendered-height-aware slice below keeps a CONTIGUOUS head of
// source lines whose RENDERED height fits the budget; a contiguous slice
// (rather than ink `overflow="hidden"`) avoids decimating interspersed rows
// and preserves a code block's own truncation cue. The full message still
// and preserves a code block's own truncation. The full message still
// renders once it commits to `<Static>`. Only while pending and when a budget
// is known (constrainHeight on — both non-VP and VP pending items pass one).
//
// Reserve 2 rows: 1 for the "generating more" cue, 1 so a retained code/math
// block's OWN inner truncation cue can't stack on top of the outer one.
// Reserve 2 rows of headroom below the viewport so the live frame stays bound
// even when a retained code/math block's own rendered height slightly exceeds
// the source-line estimate.
//
// This is a SAFETY NET on top of incremental scrollback commit: it guarantees
// the live frame never exceeds the viewport regardless of how the streaming
Expand Down Expand Up @@ -164,23 +173,20 @@ const MarkdownDisplayInternal: React.FC<MarkdownDisplayProps> = ({
// exceeds the viewport, so ink cannot fall into its from-top full-redraw path
// (the scroll-to-top lock). Note keptLines can be 0 when even the first
// line/table alone overflows (e.g. a single very wide/CJK line that wraps past
// the budget): render nothing plus the "generating more" cue rather than one
// oversized row that would bypass the bound.
// the budget): render nothing rather than an oversized row.
let lines = allLines;
let pendingClipped = false;
if (pendingRenderedBudget !== undefined) {
const tableClampRows =
availableTerminalHeight !== undefined
? Math.max(2, availableTerminalHeight - 3)
? Math.max(2, availableTerminalHeight - TABLE_PENDING_RESERVED_ROWS)
: Number.MAX_SAFE_INTEGER;
const { keptLines, clipped } = fitPendingSlice(
const { keptLines } = fitPendingSlice(
allLines,
contentWidth,
pendingRenderedBudget,
tableClampRows,
);
if (clipped) {
pendingClipped = true;
if (keptLines < allLines.length) {
lines = allLines.slice(0, keptLines);
}
}
Expand Down Expand Up @@ -351,6 +357,24 @@ 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 @@ -568,20 +592,12 @@ const MarkdownDisplayInternal: React.FC<MarkdownDisplayProps> = ({
);
}

// When the live message was clipped to a head slice (see pendingLineBudget
// above), add a cue that more is streaming. Code blocks retained in the head
// still render their own "... generating more ..." truncation.
if (pendingClipped) {
return (
<Box flexDirection="column">
{contentBlocks}
<Text color={theme.text.secondary} wrap="truncate">
... generating more ...
</Text>
</Box>
);
}

// Safety-net clip: when the pending preview exceeds the rendered-height
// budget, slice it to keep the live frame within the viewport. The clipping
// still happens (prevents scroll-to-top lock), but we no longer show the
// "... generating more ..." cue — incremental scrollback commit (PR #6170)
// already streams content to <Static> in real-time, so clipped content is
// not "delayed output" but rather "still streaming".
return <>{contentBlocks}</>;
};

Expand All @@ -608,8 +624,13 @@ const RenderCodeBlockInternal: React.FC<RenderCodeBlockProps> = ({
}) => {
const settings = useSettings();
const { renderMode } = useRenderMode();
const MIN_LINES_FOR_MESSAGE = 1; // Minimum lines to show before the "generating more" message
const RESERVED_LINES = 2; // Lines reserved for the message itself and potential padding
// Below this many usable rows there is no room for a meaningful code
// preview, so we fall back to the "... code is being written ..." notice.
const MIN_PREVIEW_LINES = 1;
// One row of headroom below availableTerminalHeight. This used to also hold a
// "... generating more ..." cue (removed with PR #6170's incremental commit),
// so the reclaimed row now shows an extra code line at the same total height.
const RESERVED_LINES = 1;

if (lang?.toLowerCase() === 'mermaid' && renderMode === 'render') {
if (isPending) {
Expand Down Expand Up @@ -642,8 +663,8 @@ const RenderCodeBlockInternal: React.FC<RenderCodeBlockProps> = ({
);

if (content.length > MAX_CODE_LINES_WHEN_PENDING) {
if (MAX_CODE_LINES_WHEN_PENDING < MIN_LINES_FOR_MESSAGE) {
// Not enough space to even show the message meaningfully
if (MAX_CODE_LINES_WHEN_PENDING < MIN_PREVIEW_LINES) {
// Not enough space to even show a truncated preview meaningfully
return (
<Box paddingLeft={CODE_BLOCK_PREFIX_PADDING}>
<Text color={theme.text.secondary}>
Expand All @@ -664,7 +685,6 @@ const RenderCodeBlockInternal: React.FC<RenderCodeBlockProps> = ({
return (
<Box paddingLeft={CODE_BLOCK_PREFIX_PADDING} flexDirection="column">
{colorizedTruncatedCode}
<Text color={theme.text.secondary}>... generating more ...</Text>
</Box>
);
}
Expand Down Expand Up @@ -702,10 +722,13 @@ interface RenderPendingMermaidBlockProps {
const RenderPendingMermaidBlockInternal: React.FC<
RenderPendingMermaidBlockProps
> = ({ content, availableTerminalHeight, contentWidth }) => {
// Reserve one row for the "Mermaid diagram is being written..." header. The
// second reserved row used to hold a "... generating more ..." cue (removed
// with PR #6170); reclaiming it shows an extra preview line at the same height.
const maxPreviewLines =
availableTerminalHeight === undefined
? 6
: Math.max(0, availableTerminalHeight - 2);
: Math.max(0, availableTerminalHeight - 1);
const previewLines = content.slice(0, maxPreviewLines);
return (
<Box
Expand All @@ -720,9 +743,6 @@ const RenderPendingMermaidBlockInternal: React.FC<
{line || ' '}
</Text>
))}
{content.length > previewLines.length && (
<Text color={theme.text.secondary}>... generating more ...</Text>
)}
</Box>
);
};
Expand All @@ -744,7 +764,10 @@ const RenderMathBlockInternal: React.FC<RenderMathBlockProps> = ({
isPending,
availableTerminalHeight,
}) => {
const RESERVED_LINES = 3;
// One row for the "LaTeX block · source:" header plus one row of headroom.
// The third row used to hold a "... generating more ..." cue (removed with PR
// #6170); reclaiming it shows an extra preview line at the same total height.
const RESERVED_LINES = 2;
if (isPending && availableTerminalHeight !== undefined) {
const maxPreviewLines = Math.max(
0,
Expand All @@ -767,7 +790,6 @@ const RenderMathBlockInternal: React.FC<RenderMathBlockProps> = ({
{line || ' '}
</Text>
))}
<Text color={theme.text.secondary}>... generating more ...</Text>
</Box>
);
}
Expand Down Expand Up @@ -882,12 +904,6 @@ interface RenderTableProps {
availableTerminalHeight?: number;
}

// Backstop only: the pending slice bounds a completed table's height assuming
// single-line cells, but a cell that WRAPS renders taller and could still
// overflow. While streaming, cap the table to the viewport (reserve 3 rows:
// marginY 2 + the outer cue) so it can never trigger the scroll-to-top lock.
const TABLE_PENDING_RESERVED_ROWS = 3;

const RenderTableInternal: React.FC<RenderTableProps> = ({
headers,
rows,
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/src/ui/utils/pending-rendered-height.ts
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ export interface PendingSliceResult {
/**
* Number of leading source lines whose combined RENDERED height fits within
* `budget`. May be 0 when even the first line/table alone overflows (the
* caller then renders nothing plus a "more" cue rather than an oversized row).
* caller then renders nothing rather than an oversized row).
*/
keptLines: number;
/** True when some trailing source lines were dropped to fit the budget. */
Expand Down
Loading