From 1b58ede8e7948c8a99955acb131d4115f815f4a7 Mon Sep 17 00:00:00 2001 From: MikeWang0316tw Date: Sun, 5 Jul 2026 23:56:48 +0800 Subject: [PATCH] =?UTF-8?q?fix(cli):=20smoother=20live=20streaming=20previ?= =?UTF-8?q?ew=20=E2=80=94=20drop=20"generating=20more"=20cue,=20hold=20bac?= =?UTF-8?q?k=20partial=20table=20rows=20(#6340)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(cli): drop redundant "generating more" cue from the live preview In non-VP mode the live markdown preview is clipped to a rendered-height budget so the frame never overflows the viewport and triggers ink's scroll-to-top full redraw. It used to append a "... generating more ..." cue (and code/math/mermaid blocks appended their own) to signal that the clipped tail was still coming. Since #6170 landed the incremental scrollback commit, that tail is streamed into in real time — clipped content is "still streaming" and reappears within a commit cycle, not "delayed output". The cue is therefore redundant noise that flickers in step with the commit cycle, so remove all four occurrences (outer preview clip, code block, mermaid block, math block). The row each cue used to occupy is reclaimed for content, so the total rendered height is unchanged: the code/math/mermaid RESERVED_LINES drop by one and the outer slice trigger switches from the (now-inlined) `clipped` flag to `keptLines < allLines.length`. The TableRenderer "… more rows streaming …" clamp is intentionally kept — an in-progress oversized table is not yet in scrollback, so that cue still carries information. Also gitignore the nested `.qwen/computer-use/` marker so the auto-generated artifact stops showing up as untracked. Co-Authored-By: Claude Opus 4.8 * fix(cli): hold back the unterminated table row while streaming While a markdown table streams, the frontier line is often a half-typed row like `| a | b` with no closing `|` yet. Because TABLE_ROW_RE requires both a leading and trailing pipe, that partial line does not match, so the parser closed the table and rendered the partial as a plain text line below it — then, once the closing `|` arrived, flipped it into the table. This per-token flip changed the frame height and re-ran column autosizing on every keystroke, jittering the live table. Hold the partial row back instead: when pending, if the final line is an unterminated table row and at least one complete row already exists, skip it so `inTable` stays set and the end-of-content handler keeps rendering the accumulated rows as a live table. The row appears the moment it terminates. The `tableRows.length > 0` guard keeps the header + separator from blanking out while the very first row is still being typed. Note: this smooths the table content itself; it does not change the streaming repaint frequency, so the fixed bottom controls still repaint on each tick (that is the domain of the flicker-reduction work, e.g. #5396). Co-Authored-By: Claude Opus 4.8 * docs(cli): fix two stale "generating more" cue references in comments Review follow-up: two comments still referenced the removed outer cue. - TABLE_PENDING_RESERVED_ROWS: reword "marginY 2 + the outer cue" to "marginY 2 + one row of wrapped-cell safety headroom". The reserve stays at 3 on purpose — tables under-estimate their rendered height the most (wrapped cells), so they keep one more backstop row than the other blocks; lowering it would shrink that safety margin. - pending-rendered-height PendingSliceResult.keptLines JSDoc: drop the "plus a 'more' cue" phrasing — the caller now renders nothing rather than an oversized row. Comment-only; no behaviour change. 155 tests pass. Co-Authored-By: Claude Opus 4.8 * fix(cli): hold back the partial first table row too; de-dup reserve constant Review follow-up on the streaming table hold-back. - The `tableRows.length > 0` guard skipped the hold-back for the FIRST data row: a partial first row fell through to the table-closing branch, which also requires a row, so the header + separator were dropped and the partial rendered as a stray text line — the same per-token flip the change is meant to remove, just for the first row. Relax the guard to `tableHeaders.length > 0` so an unterminated first row/separator is held back too; the table is simply not drawn until its first row terminates, then pops in complete and grows one row at a time. Comment corrected to describe the actual behaviour. - Add a test for that edge case (partial first row held back, table appears once the row terminates). - De-duplicate the magic `3`: the slice-side `tableClampRows` estimate now references `TABLE_PENDING_RESERVED_ROWS` (moved to the top-of-file constants) instead of a literal, so the estimate and RenderTable's render-side `maxHeight` cap can never diverge. 157 tests pass. Co-Authored-By: Claude Opus 4.8 --------- Co-authored-by: Claude Opus 4.8 --- .gitignore | 2 + packages/cli/src/ui/hooks/useGeminiStream.ts | 4 +- .../cli/src/ui/utils/MarkdownDisplay.test.tsx | 109 +++++++++++++++--- packages/cli/src/ui/utils/MarkdownDisplay.tsx | 102 +++++++++------- .../src/ui/utils/pending-rendered-height.ts | 2 +- 5 files changed, 156 insertions(+), 63 deletions(-) diff --git a/.gitignore b/.gitignore index 0e4afaca61..e7e956c2c7 100644 --- a/.gitignore +++ b/.gitignore @@ -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/ diff --git a/packages/cli/src/ui/hooks/useGeminiStream.ts b/packages/cli/src/ui/hooks/useGeminiStream.ts index 02a3174bff..c83d9a018d 100644 --- a/packages/cli/src/ui/hooks/useGeminiStream.ts +++ b/packages/cli/src/ui/hooks/useGeminiStream.ts @@ -372,8 +372,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. diff --git a/packages/cli/src/ui/utils/MarkdownDisplay.test.tsx b/packages/cli/src/ui/utils/MarkdownDisplay.test.tsx index c3c7f7604d..efcb10b22b 100644 --- a/packages/cli/src/ui/utils/MarkdownDisplay.test.tsx +++ b/packages/cli/src/ui/utils/MarkdownDisplay.test.tsx @@ -101,11 +101,11 @@ describe('', () => { />, ); 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; @@ -171,18 +171,14 @@ describe('', () => { ); 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}} +`), @@ -197,9 +193,7 @@ describe('', () => { />, ); 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); }); @@ -224,7 +218,7 @@ describe('', () => { 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, ); @@ -239,7 +233,7 @@ describe('', () => { 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', () => { @@ -316,6 +310,87 @@ Some text before. expect(lastFrame()).toMatchSnapshot(); }); + it('holds back an unterminated trailing table row while streaming', () => { + const text = `| A | B | +|---|---| +| one | two | +| three | fo`.replace(/\n/g, eol); + const { lastFrame } = renderWithProviders( + , + ); + 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( + , + ); + 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( + , + ); + 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( + , + ); + 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( + , + ); + // 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( + , + ); + const output = lastFrame() ?? ''; + expect(output).toContain('one'); + expect(output).toContain('Done'); + }); + 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 997da2b037..4a4585576a 100644 --- a/packages/cli/src/ui/utils/MarkdownDisplay.tsx +++ b/packages/cli/src/ui/utils/MarkdownDisplay.tsx @@ -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; @@ -131,12 +139,13 @@ const MarkdownDisplayInternal: React.FC = ({ // 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 ``. 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 @@ -164,23 +173,20 @@ const MarkdownDisplayInternal: React.FC = ({ // 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); } } @@ -351,6 +357,24 @@ 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 @@ -568,20 +592,12 @@ const MarkdownDisplayInternal: React.FC = ({ ); } - // 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 ( - - {contentBlocks} - - ... generating more ... - - - ); - } - + // 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 in real-time, so clipped content is + // not "delayed output" but rather "still streaming". return <>{contentBlocks}; }; @@ -608,8 +624,13 @@ const RenderCodeBlockInternal: React.FC = ({ }) => { 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) { @@ -642,8 +663,8 @@ const RenderCodeBlockInternal: React.FC = ({ ); 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 ( @@ -664,7 +685,6 @@ const RenderCodeBlockInternal: React.FC = ({ return ( {colorizedTruncatedCode} - ... generating more ... ); } @@ -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 ( ))} - {content.length > previewLines.length && ( - ... generating more ... - )} ); }; @@ -744,7 +764,10 @@ const RenderMathBlockInternal: React.FC = ({ 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, @@ -767,7 +790,6 @@ const RenderMathBlockInternal: React.FC = ({ {line || ' '} ))} - ... generating more ... ); } @@ -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 = ({ headers, rows, diff --git a/packages/cli/src/ui/utils/pending-rendered-height.ts b/packages/cli/src/ui/utils/pending-rendered-height.ts index 2d7a9d81eb..cadaca4b15 100644 --- a/packages/cli/src/ui/utils/pending-rendered-height.ts +++ b/packages/cli/src/ui/utils/pending-rendered-height.ts @@ -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. */