From f9459bf462e80e2c49665b831879d6055178a99e Mon Sep 17 00:00:00 2001 From: pppobear Date: Wed, 15 Jul 2026 20:01:39 +0800 Subject: [PATCH 1/2] fix(tui): stabilize streamed table scrollback --- packages/coding-agent/CHANGELOG.md | 1 + .../modes/components/transcript-container.ts | 32 ++- .../test/streaming-output-scrollback.test.ts | 86 ++++++ packages/tui/CHANGELOG.md | 4 + packages/tui/src/components/markdown.ts | 246 ++++++++++++++++-- packages/tui/src/tui.ts | 30 ++- packages/tui/test/markdown.test.ts | 189 ++++++++++++++ 7 files changed, 559 insertions(+), 29 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 71e86facb..fe8ebd8f7 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -13,6 +13,7 @@ ### Fixed +- Fixed long streamed table responses duplicating in terminal scrollback when later rows widened an earlier column. - Fixed Bash internal URLs remaining unresolved when used as unquoted arguments inside command substitutions ([#5535](https://github.com/can1357/oh-my-pi/issues/5535)). - Fixed the built-in `fd` printing `fd: Broken pipe (os error 32)` when a downstream pipeline reader exited early (e.g. `fd … | head`); it now exits silently with 141 (128+SIGPIPE), matching real fd. - Fixed prewalk repeatedly continuing after a bash-only task such as `commit` had already completed ([#5551](https://github.com/can1357/oh-my-pi/issues/5551)). diff --git a/packages/coding-agent/src/modes/components/transcript-container.ts b/packages/coding-agent/src/modes/components/transcript-container.ts index 576dec064..33c5e44ec 100644 --- a/packages/coding-agent/src/modes/components/transcript-container.ts +++ b/packages/coding-agent/src/modes/components/transcript-container.ts @@ -79,6 +79,10 @@ function sealCommittedSnapshot(child: Component): void { if (block.isDisplaceableBlock?.()) block.seal?.(); } +function setBlockCommittedRows(child: Component, rows: number): void { + (child as Component & Partial).setNativeScrollbackCommittedRows?.(rows); +} + // A "plain blank" row is empty or whitespace-only with no ANSI bytes. It marks // separation padding (a `Spacer`, or a no-background `paddingY` row) as opposed // to a background-colored padding row, whose escape sequences contain `\S` and @@ -208,11 +212,35 @@ export class TranscriptContainer this.#replayPending = false; } - setNativeScrollbackCommittedRows(rows: number): void { + override setNativeScrollbackCommittedRows(rows: number): void { this.#committedRows = Number.isFinite(rows) ? Math.max(0, Math.trunc(rows)) : 0; + for (let i = this.#compactedChildStart; i < this.children.length; i++) { + const child = this.children[i]!; + const segment = this.#segments[i]; + if (segment === undefined || segment.component !== child) continue; + const committedContribution = Math.min( + segment.contribution.length, + Math.max(0, this.#committedRows - segment.startRow - segment.sep), + ); + if (committedContribution === 0) { + setBlockCommittedRows(child, 0); + continue; + } + // Transcript assembly strips plain blank edges from each block. Map the + // committed contribution back into the child's raw render coordinates so + // nested containers can split the prefix against their exact child rows. + let leadingTrimmedRows = 0; + while (leadingTrimmedRows < segment.rawRef.length && isPlainBlank(segment.rawRef[leadingTrimmedRows]!)) { + leadingTrimmedRows++; + } + setBlockCommittedRows(child, Math.min(segment.rawRef.length, leadingTrimmedRows + committedContribution)); + } } - prepareNativeScrollbackReplay(): void { + override prepareNativeScrollbackReplay(): void { + // Replay retires the old terminal tape, so descendants may discard layout + // locks whose only purpose was keeping that immutable history byte-stable. + super.prepareNativeScrollbackReplay(); if (this.#compactedChildStart === 0) return; this.#compactedChildStart = 0; this.#replayPending = true; diff --git a/packages/coding-agent/test/streaming-output-scrollback.test.ts b/packages/coding-agent/test/streaming-output-scrollback.test.ts index 3305cb413..95371d85a 100644 --- a/packages/coding-agent/test/streaming-output-scrollback.test.ts +++ b/packages/coding-agent/test/streaming-output-scrollback.test.ts @@ -295,6 +295,92 @@ describe("streaming tool output never sprays duplicate scrollback banners", () = } }, 30_000); + test("finalizes a width-growing streamed table exactly once", async () => { + const rows = 8; + stubStdoutRows(rows); + const term = new VirtualTerminal(80, rows); + Object.defineProperty(term, "isNativeViewportAtBottom", { configurable: true, value: () => undefined }); + const scheduler = makeDrainableScheduler(); + const tui = new TUI(term, undefined, { renderScheduler: scheduler }); + // Exercise the default append-only path directly; erase-and-replay would + // hide the duplicate-history regression this test is meant to catch. + tui.setScrollbackRebuild(false); + const transcript = new TranscriptContainer(); + const assistant = new AssistantMessageComponent(undefined, false); + transcript.addChild(assistant); + tui.addChild(transcript); + + const entries = [ + "alpha", + "beta-entry", + "gamma-component", + "delta-module", + "epsilon-adapter", + "zeta-runner", + "eta-service", + "theta-provider", + "iota-component-with-later-width-growth", + "kappa-component-with-later-width-growth", + ]; + const markers = entries.map((_, index) => `R${index.toString().padStart(3, "0")}`); + const table = (count: number): string => + [ + "| Entry | Col A | Col B | Col C | Col D |", + "| --- | --- | --- | --- | --- |", + ...entries.slice(0, count).map((entry, index) => `| ${entry} | - | ${markers[index]} | - | - |`), + ].join("\n"); + + try { + tui.start(); + scheduler.flush(); + await term.flush(); + + for (let count = 1; count <= 8; count++) { + assistant.updateContent(makeAssistantMessage([{ type: "text", text: table(count) }]), { + transient: true, + }); + tui.requestRender(); + scheduler.flush(); + await term.flush(); + } + + const midRows = plainScrollBuffer(term); + const headerIndex = midRows.findIndex(row => row.includes("Entry")); + expect(headerIndex).toBeGreaterThanOrEqual(0); + expect(headerIndex).toBeLessThan(term.getBufferPosition().baseY); + expect(midRows.filter(row => row.includes("Entry"))).toHaveLength(1); + + for (let count = 9; count <= entries.length; count++) { + assistant.updateContent(makeAssistantMessage([{ type: "text", text: table(count) }]), { + transient: true, + }); + tui.requestRender(); + scheduler.flush(); + await term.flush(); + } + + assistant.updateContent(makeAssistantMessage([{ type: "text", text: table(entries.length) }]), { + transient: false, + }); + assistant.markTranscriptBlockFinalized(); + for (let i = 0; i < 2; i++) { + tui.requestRender(); + scheduler.flush(); + await term.flush(); + } + + const finalRows = plainScrollBuffer(term); + expect(finalRows.filter(row => row.includes("Entry"))).toHaveLength(1); + expect(markers.map(marker => finalRows.filter(row => row.includes(marker)).length)).toEqual( + markers.map(() => 1), + ); + } finally { + assistant.dispose(); + tui.stop(); + await term.flush(); + } + }, 30_000); + test("expanded live eval output records painted rows without spraying after settle", async () => { const rows = 8; stubStdoutRows(rows); diff --git a/packages/tui/CHANGELOG.md b/packages/tui/CHANGELOG.md index 771416726..9a6896f1f 100644 --- a/packages/tui/CHANGELOG.md +++ b/packages/tui/CHANGELOG.md @@ -7,6 +7,10 @@ - Display LaTeX now renders `\underbrace`/`\overbrace` (and the bracket/paren variants) as drawn horizontal braces with centered labels, and stacks `\overset`/`\underset`/`\stackrel` annotations above/below the base instead of falling back to flat inline glyphs. - Display LaTeX renders multi-letter script words (`N_{turns}`) as raised/lowered blocks instead of ragged per-character Unicode sub/superscript glyphs; single letters and digits keep the compact Unicode forms. +### Fixed + +- Fixed streamed Markdown tables reflowing rows already written to native scrollback when later cells widen a column. + ## [16.5.2] - 2026-07-14 ### Fixed diff --git a/packages/tui/src/components/markdown.ts b/packages/tui/src/components/markdown.ts index 1ce564d4c..35acba17e 100644 --- a/packages/tui/src/components/markdown.ts +++ b/packages/tui/src/components/markdown.ts @@ -4,7 +4,7 @@ import { latexToBlock } from "../latex-block"; import { inlineMathSpanEnd, isBareMathEnvironment, latexToUnicode } from "../latex-to-unicode"; import type { SymbolTheme } from "../symbols"; import { TERMINAL } from "../terminal-capabilities"; -import type { Component } from "../tui"; +import type { Component, NativeScrollbackCommittedRows, NativeScrollbackReplay } from "../tui"; import { applyBackgroundToLine, Ellipsis, @@ -607,11 +607,17 @@ const RENDER_CACHE_MAX = 256; // sane cap: ~256 distinct message × width combos const RENDER_CACHE_MAX_SIZE = 512 * 1024; const RENDER_CACHE_MAX_ENTRY_SIZE = 32 * 1024; const EMPTY_RENDER_LINES: readonly string[] = []; -const renderCache = new LRUCache({ + +interface RenderCacheEntry { + lines: readonly string[]; + tables: readonly RenderedTableLayout[]; +} + +const renderCache = new LRUCache({ max: RENDER_CACHE_MAX, maxSize: RENDER_CACHE_MAX_SIZE, maxEntrySize: RENDER_CACHE_MAX_ENTRY_SIZE, - sizeCalculation: renderedLinesCacheSize, + sizeCalculation: renderCacheEntrySize, }); function renderedLinesCacheSize(lines: readonly string[]): number { @@ -620,6 +626,12 @@ function renderedLinesCacheSize(lines: readonly string[]): number { return Math.max(1, size); } +function renderCacheEntrySize(entry: RenderCacheEntry): number { + let size = renderedLinesCacheSize(entry.lines); + for (const table of entry.tables) size += table.key.length + table.columnWidths.length + 4; + return size; +} + // A reference-link definition (`[label]: dest`) resolves across the whole // document, so a split lex cannot reproduce it — disable the streaming fast path // when one is present (rare in streamed output). The label may contain @@ -951,6 +963,7 @@ interface StreamPrefixLineCache extends RenderSignature { text: string; tokenCount: number; lines: readonly string[]; + tables: readonly TableRenderSpec[]; } interface StreamingDiffLineCache extends RenderSignature { lang: string | undefined; @@ -958,7 +971,25 @@ interface StreamingDiffLineCache extends RenderSignature { lines: readonly string[]; } -export class Markdown implements Component { +interface TableLayoutLock { + availableWidth: number; + columnWidths: readonly number[]; +} + +interface TableRenderSpec extends TableLayoutLock { + key: string; + lineCount: number; + startRow: number; + endRow: number; +} + +interface RenderedTableLayout extends TableLayoutLock { + key: string; + startRow: number; + endRow: number; +} + +export class Markdown implements Component, NativeScrollbackCommittedRows, NativeScrollbackReplay { #text: string; #paddingX: number; // Left/right padding #paddingY: number; // Top/bottom padding @@ -1005,10 +1036,19 @@ export class Markdown implements Component { #renderingFrozenPrefix = false; #streamingDiffLineCache?: StreamingDiffLineCache; #activeRenderSignature?: RenderSignature; + // Streaming tables may grow naturally while wholly repaintable. Once any + // physical row of a table enters native scrollback, its current column widths + // are locked for the rest of this append-only text lineage: future wider cells + // wrap inside those columns instead of reflowing immutable history above. + #tableLayoutWidth?: number; + #lockedTableLayouts = new Map(); + #lastRenderedTableLayouts: RenderedTableLayout[] = []; + #activeTableRenderSpecs?: TableRenderSpec[]; #ignoreTight = false; setIgnoreTight(ignore: boolean): this { + if (this.#ignoreTight !== ignore) this.#clearTableLayouts(); this.#ignoreTight = ignore; this.invalidate(); return this; @@ -1037,6 +1077,7 @@ export class Markdown implements Component { // full lex + wrap runs per re-emit — one of the top CPU hotspots during // streaming (issue #4353). Mirrors `Text.setText`'s guard. if (text === this.#text) return false; + if (!text.startsWith(this.#text)) this.#clearTableLayouts(); this.#text = text; if (!text.trim()) { // Blank replacement: render() early-returns before #lexTokens can see @@ -1080,6 +1121,41 @@ export class Markdown implements Component { return this.#lastRenderSettledRows; } + /** + * Freeze every table whose first physical row is already part of the native + * scrollback prefix. The recorded widths came from the exact frame that was + * just emitted, so the next streamed delta cannot retroactively widen it. + */ + setNativeScrollbackCommittedRows(rows: number): void { + const committed = Number.isFinite(rows) ? Math.max(0, Math.trunc(rows)) : 0; + let changed = false; + for (const table of this.#lastRenderedTableLayouts) { + if (table.startRow >= committed || this.#lockedTableLayouts.has(table.key)) continue; + this.#lockedTableLayouts.set(table.key, { + availableWidth: table.availableWidth, + columnWidths: table.columnWidths.slice(), + }); + changed = true; + } + if (changed) this.invalidate(); + } + + /** A destructive replay removes the immutable tape this layout was guarding. */ + prepareNativeScrollbackReplay(): void { + this.#clearTableLayouts(); + this.#tableLayoutWidth = undefined; + this.invalidate(); + } + + #clearTableLayouts(): void { + this.#lockedTableLayouts.clear(); + this.#lastRenderedTableLayouts = []; + this.#activeTableRenderSpecs = undefined; + // Same-width replay/non-append rewrites could otherwise reuse physical + // prefix lines rendered with the retired locked widths. + this.#streamPrefixLineCache = undefined; + } + // Lex `text` into block tokens, reusing the frozen stable prefix when the text // only grew (the streaming path). Falls back to a full lex whenever the prefix // is no longer a prefix (non-append edit), the text carries reference-link @@ -1159,6 +1235,11 @@ export class Markdown implements Component { } render(width: number): readonly string[] { + if (this.#tableLayoutWidth !== undefined && this.#tableLayoutWidth !== width) { + this.#clearTableLayouts(); + this.invalidate(); + } + this.#tableLayoutWidth = width; // L1: per-instance cache — fastest path for repeated renders of the same // instance at the same width (e.g. resize debounce, repeated redraws). // Returning the cached reference is load-bearing: parents memoize their @@ -1200,21 +1281,30 @@ export class Markdown implements Component { // theme.heading is used as the representative theme probe — it's required // by MarkdownTheme and is one of the most styling-sensitive entries. let cacheKey: string | undefined; - if (!this.transientRenderCache) { + if (!this.transientRenderCache && this.#lockedTableLayouts.size === 0) { cacheKey = this.#renderCacheKey(normalizedText, signature); const cached = renderCache.get(cacheKey); if (cached !== undefined) { + // Restore both the rendered rows and the geometry metadata that produced + // them. A later scrollback publication must never lock widths from an + // older transient frame against rows served from this cache entry. + this.#lastRenderedTableLayouts = cached.tables.map(table => ({ + ...table, + columnWidths: table.columnWidths.slice(), + })); // Populate L1 so subsequent calls from this instance are O(1) map lookup. this.#cachedText = this.#text; this.#cachedWidth = width; - this.#cachedLines = cached; - return cached; + this.#cachedLines = cached.lines; + return cached.lines; } } // Parse markdown to HTML-like tokens const tokens = this.#lexTokens(normalizedText); let contentLines: string[]; + const tableRenderSpecs: TableRenderSpec[] = []; + this.#activeTableRenderSpecs = tableRenderSpecs; this.#activeRenderSignature = signature; try { contentLines = this.transientRenderCache @@ -1222,7 +1312,9 @@ export class Markdown implements Component { : this.#renderContentLines(tokens, 0, tokens.length, contentWidth, signature); } finally { this.#activeRenderSignature = undefined; + this.#activeTableRenderSpecs = undefined; } + this.#lastRenderedTableLayouts = this.#resolveRenderedTableLayouts(tableRenderSpecs, signature.paddingY); const emptyLines = this.#renderEmptyPaddingLines(signature); // Combine top padding, content, and bottom padding @@ -1239,7 +1331,13 @@ export class Markdown implements Component { // Update L2 module-level LRU so future instances with the same key skip // the marked.lexer + highlightCode (Rust FFI) work entirely. if (cacheKey !== undefined) { - renderCache.set(cacheKey, result); + renderCache.set(cacheKey, { + lines: result, + tables: this.#lastRenderedTableLayouts.map(table => ({ + ...table, + columnWidths: table.columnWidths.slice(), + })), + }); } return result; @@ -1284,6 +1382,7 @@ export class Markdown implements Component { let renderedUntil = 0; if (reusablePrefix && reusablePrefix.tokenCount <= frozenTokenCount) { contentLines.push(...reusablePrefix.lines); + this.#activeTableRenderSpecs?.push(...reusablePrefix.tables); renderedUntil = reusablePrefix.tokenCount; } @@ -1293,7 +1392,14 @@ export class Markdown implements Component { this.#renderingFrozenPrefix = true; try { contentLines.push( - ...this.#renderContentLines(tokens, renderedUntil, frozenTokenCount, contentWidth, signature), + ...this.#renderContentLines( + tokens, + renderedUntil, + frozenTokenCount, + contentWidth, + signature, + contentLines.length, + ), ); } finally { this.#renderingFrozenPrefix = false; @@ -1306,6 +1412,7 @@ export class Markdown implements Component { text: frozenText, tokenCount: frozenTokenCount, lines: contentLines.slice(), + tables: this.#activeTableRenderSpecs?.slice() ?? [], }; // Settled exposure (hard-monotone): these rows are declared final to @@ -1322,7 +1429,16 @@ export class Markdown implements Component { } if (renderedUntil < tokens.length) { - contentLines.push(...this.#renderContentLines(tokens, renderedUntil, tokens.length, contentWidth, signature)); + contentLines.push( + ...this.#renderContentLines( + tokens, + renderedUntil, + tokens.length, + contentWidth, + signature, + contentLines.length, + ), + ); } return contentLines; @@ -1356,23 +1472,53 @@ export class Markdown implements Component { end: number, contentWidth: number, signature: RenderSignature, + rowOffset = 0, ): string[] { - const renderedLines: string[] = []; + const wrappedLines: string[] = []; + let sourceOffset = 0; + for (let i = 0; i < start; i++) sourceOffset += tokens[i]!.raw.length; for (let i = start; i < end; i++) { const token = tokens[i]; const nextToken = tokens[i + 1]; - renderedLines.push(...this.#renderToken(token, contentWidth, nextToken?.type)); - } - - const wrappedLines: string[] = []; - for (const line of renderedLines) { - // Skip wrapping for image protocol lines and OSC 66 sized headings - // (would corrupt escape sequences / split the indivisible sized span). - if (TERMINAL.isImageLine(line) || isOsc66Line(line)) { - wrappedLines.push(line); - } else { - wrappedLines.push(...wrapTextWithAnsi(line, contentWidth)); + const tableSpecStart = this.#activeTableRenderSpecs?.length ?? 0; + const tokenRowStart = rowOffset + wrappedLines.length; + const renderedTokenLines = this.#renderToken( + token, + contentWidth, + nextToken?.type, + undefined, + `offset:${sourceOffset}`, + ); + for (const line of renderedTokenLines) { + // Skip wrapping for image protocol lines and OSC 66 sized headings + // (would corrupt escape sequences / split the indivisible sized span). + if (TERMINAL.isImageLine(line) || isOsc66Line(line)) { + wrappedLines.push(line); + } else { + wrappedLines.push(...wrapTextWithAnsi(line, contentWidth)); + } } + const tokenRowEnd = rowOffset + wrappedLines.length; + const tableSpecs = this.#activeTableRenderSpecs; + if (tableSpecs !== undefined) { + for (let specIndex = tableSpecStart; specIndex < tableSpecs.length; specIndex++) { + const spec = tableSpecs[specIndex]!; + if (token.type === "table") { + // A top-level table's own rows are already width-bounded, so none + // wrap here. Exclude the optional inter-block blank from its span. + spec.startRow = tokenRowStart; + spec.endRow = Math.min(tokenRowEnd, tokenRowStart + spec.lineCount); + } else { + // Tables nested in a blockquote inherit the enclosing token's span. + // This is conservative (it may lock within the quote's prose head) + // but remains structural and can never confuse unrelated text for + // a table border. + spec.startRow = tokenRowStart; + spec.endRow = tokenRowEnd; + } + } + } + sourceOffset += token.raw.length; } const leftMargin = padding(signature.paddingX); @@ -1416,6 +1562,21 @@ export class Markdown implements Component { return contentLines; } + #resolveRenderedTableLayouts(specs: readonly TableRenderSpec[], topPadding: number): RenderedTableLayout[] { + const layouts: RenderedTableLayout[] = []; + for (const spec of specs) { + if (spec.startRow < 0 || spec.endRow <= spec.startRow) continue; + layouts.push({ + key: spec.key, + availableWidth: spec.availableWidth, + columnWidths: spec.columnWidths.slice(), + startRow: topPadding + spec.startRow, + endRow: topPadding + spec.endRow, + }); + } + return layouts; + } + #renderCodeBodyLines(token: Token, codeIndent: string): string[] { const bodyLines: string[] = []; const tokenText = "text" in token && typeof token.text === "string" ? token.text : ""; @@ -1626,7 +1787,13 @@ export class Markdown implements Component { }; } - #renderToken(token: Token, width: number, nextTokenType?: string, styleContext?: InlineStyleContext): string[] { + #renderToken( + token: Token, + width: number, + nextTokenType?: string, + styleContext?: InlineStyleContext, + tokenKey = "root", + ): string[] { const lines: string[] = []; // Display math block (own-line `$$…$$` / `\[…\]`): stack `\frac` vertically @@ -1728,7 +1895,7 @@ export class Markdown implements Component { } case "table": { - const tableLines = this.#renderTable(token as TableToken, width, nextTokenType, styleContext); + const tableLines = this.#renderTable(token as TableToken, width, nextTokenType, styleContext, tokenKey); lines.push(...tableLines); break; } @@ -1746,7 +1913,13 @@ export class Markdown implements Component { const quoteToken = quoteTokens[i]; const nextQuoteToken = quoteTokens[i + 1]; renderedQuoteLines.push( - ...this.#renderToken(quoteToken, quoteContentWidth, nextQuoteToken?.type, quoteInlineStyleContext), + ...this.#renderToken( + quoteToken, + quoteContentWidth, + nextQuoteToken?.type, + quoteInlineStyleContext, + `${tokenKey}/quote:${i}`, + ), ); } @@ -2149,6 +2322,7 @@ export class Markdown implements Component { availableWidth: number, nextTokenType?: string, styleContext?: InlineStyleContext, + tableKey = "table", ): string[] { const lines: string[] = []; const numCols = token.header.length; @@ -2263,6 +2437,17 @@ export class Markdown implements Component { } } + const lockedLayout = this.#lockedTableLayouts.get(tableKey); + if ( + lockedLayout !== undefined && + lockedLayout.availableWidth === availableWidth && + lockedLayout.columnWidths.length === numCols && + lockedLayout.columnWidths.every(width => Number.isFinite(width) && width >= 1) && + lockedLayout.columnWidths.reduce((total, width) => total + width, borderOverhead) <= availableWidth + ) { + columnWidths = lockedLayout.columnWidths.slice(); + } + const t = this.#theme.symbols.table; const h = t.horizontal; const v = t.vertical; @@ -2316,7 +2501,16 @@ export class Markdown implements Component { // Render bottom border const bottomBorderCells = columnWidths.map(w => h.repeat(w)); - lines.push(`${t.bottomLeft}${h}${bottomBorderCells.join(`${h}${t.teeUp}${h}`)}${h}${t.bottomRight}`); + const bottomBorder = `${t.bottomLeft}${h}${bottomBorderCells.join(`${h}${t.teeUp}${h}`)}${h}${t.bottomRight}`; + lines.push(bottomBorder); + this.#activeTableRenderSpecs?.push({ + key: tableKey, + availableWidth, + columnWidths: columnWidths.slice(), + lineCount: lines.length, + startRow: -1, + endRow: -1, + }); if (nextTokenType && nextTokenType !== "space") { lines.push(""); // Add spacing after table diff --git a/packages/tui/src/tui.ts b/packages/tui/src/tui.ts index ca10e6041..a66a55db8 100644 --- a/packages/tui/src/tui.ts +++ b/packages/tui/src/tui.ts @@ -473,7 +473,7 @@ export interface OverlayHandle { /** * Container - a component that contains other components */ -export class Container implements Component { +export class Container implements Component, NativeScrollbackCommittedRows, NativeScrollbackReplay { children: Component[] = []; // Memoized concatenation of the children's latest renders. Children are @@ -543,6 +543,34 @@ export class Container implements Component { } } + /** + * Split the committed prefix from the container's most recently rendered + * rows across its children. The memoized child arrays are the exact geometry + * that produced that frame; when the child list was invalidated or rebuilt, + * there is no safe old-to-new coordinate mapping, so propagation waits for + * the next render/post-emit publication. + */ + setNativeScrollbackCommittedRows(rows: number): void { + const refs = this.#memoChildLines; + if (this.#memoLines === undefined || refs.length !== this.children.length) return; + const committed = Number.isFinite(rows) ? Math.max(0, Math.trunc(rows)) : 0; + let offset = 0; + for (let i = 0; i < this.children.length; i++) { + const childRows = refs[i]; + if (childRows === undefined) return; + setNativeScrollbackCommittedRows( + this.children[i]!, + Math.min(childRows.length, Math.max(0, committed - offset)), + ); + offset += childRows.length; + } + } + + /** Recursively discard layout locks that are meaningful only to the old tape. */ + prepareNativeScrollbackReplay(): void { + for (const child of this.children) prepareNativeScrollbackReplay(child); + } + render(width: number): readonly string[] { width = Math.max(1, width); const children = this.children; diff --git a/packages/tui/test/markdown.test.ts b/packages/tui/test/markdown.test.ts index bad86807d..6b2276309 100644 --- a/packages/tui/test/markdown.test.ts +++ b/packages/tui/test/markdown.test.ts @@ -446,6 +446,195 @@ describe("Markdown component", () => { expect(dataLine, "Should have data row").toBeTruthy(); }); + it("locks streamed table widths only after the table enters native scrollback", () => { + const initial = `| Entry | Value | +| --- | --- | +| short-entry | R000 |`; + const beforeCommit = `${initial} +| medium-width-entry | R001 |`; + const afterCommit = `${beforeCommit} +| much-longer-entry-that-arrives-after-commit | R002 |`; + const markdown = new Markdown(initial, 0, 0, defaultMarkdownTheme); + markdown.transientRenderCache = true; + + const topBorder = (lines: readonly string[]): string => { + const plain = lines.map(line => stripVTControlCharacters(line).trimEnd()); + const border = plain.find(line => line.startsWith("+")); + expect(border).toBeDefined(); + return border!; + }; + + const initialBorder = topBorder(markdown.render(80)); + markdown.setText(beforeCommit); + const growingLines = markdown.render(80); + const growingBorder = topBorder(growingLines); + // Wholly-live tables retain today's natural-width behavior. + expect(growingBorder).not.toBe(initialBorder); + + const tableStart = growingLines.findIndex(line => stripVTControlCharacters(line).trimStart().startsWith("+")); + markdown.setNativeScrollbackCommittedRows(tableStart + 1); + markdown.setText(afterCommit); + const lockedLines = markdown.render(80); + expect(topBorder(lockedLines)).toBe(growingBorder); + expect(lockedLines.some(line => stripVTControlCharacters(line).includes("R002"))).toBe(true); + + // Finalization must not swap in a canonical full-content layout from L2. + markdown.transientRenderCache = false; + expect(topBorder(markdown.render(80))).toBe(growingBorder); + + // A destructive replay has no immutable old tape to protect and may + // recompute the natural width from the complete table. + markdown.prepareNativeScrollbackReplay(); + expect(topBorder(markdown.render(80))).not.toBe(growingBorder); + }); + + it("keeps layout locks independent across streamed tables", () => { + const first = `| First table column | Value | +| --- | --- | +| medium-width-entry | A |`; + const second = `${first} + +| Entry | Value | +| --- | --- | +| short | R000 |`; + const widenedSecond = `${second} +| much-longer-entry-that-arrives-after-commit | R001 |`; + const markdown = new Markdown(first, 0, 0, defaultMarkdownTheme); + markdown.transientRenderCache = true; + markdown.render(80); + markdown.setNativeScrollbackCommittedRows(1); + + markdown.setText(second); + const secondLines = markdown.render(80); + const borders = secondLines + .map(line => stripVTControlCharacters(line).trimEnd()) + .filter(line => line.startsWith("+")); + expect(borders).toHaveLength(6); + const secondTop = secondLines.findIndex( + (line, index) => index > 0 && stripVTControlCharacters(line).trimEnd() === borders[3], + ); + expect(secondTop).toBeGreaterThan(0); + expect(borders[3]).not.toBe(borders[0]); + + markdown.setNativeScrollbackCommittedRows(secondTop + 1); + markdown.setText(widenedSecond); + const widenedBorders = markdown + .render(80) + .map(line => stripVTControlCharacters(line).trimEnd()) + .filter(line => line.startsWith("+")); + expect(widenedBorders[0]).toBe(borders[0]); + expect(widenedBorders[3]).toBe(borders[3]); + }); + + it("recomputes a locked streamed table after resize or non-append replacement", () => { + const short = `| Entry | Value | +| --- | --- | +| short | R000 |`; + const wide = `${short} +| much-longer-entry-that-arrives-after-commit | R001 |`; + const topBorder = (lines: readonly string[]): string => { + const border = lines + .map(line => stripVTControlCharacters(line).trimEnd()) + .find(line => line.startsWith("+")); + expect(border).toBeDefined(); + return border!; + }; + const markdown = new Markdown(short, 0, 0, defaultMarkdownTheme); + markdown.transientRenderCache = true; + const shortBorder = topBorder(markdown.render(80)); + markdown.setNativeScrollbackCommittedRows(1); + markdown.setText(wide); + expect(topBorder(markdown.render(80))).toBe(shortBorder); + + // A width change starts fresh geometry; the complete source can widen. + expect(topBorder(markdown.render(100))).not.toBe(shortBorder); + + const replacement = `| New | Value | +| --- | --- | +| x | R100 |`; + const expandedReplacement = `${replacement} +| replacement-column-can-grow | R101 |`; + markdown.setText(replacement); + const replacementBorder = topBorder(markdown.render(80)); + markdown.setText(expandedReplacement); + expect(topBorder(markdown.render(80))).not.toBe(replacementBorder); + }); + + it("does not lock a table when earlier code prints an identical border", () => { + const table = `| Entry | Value | +| --- | --- | +| short | R000 |`; + const probe = new Markdown(table, 0, 0, defaultMarkdownTheme); + const narrowBorder = probe + .render(80) + .map(line => stripVTControlCharacters(line).trimEnd()) + .find(line => line.startsWith("+")); + expect(narrowBorder).toBeDefined(); + + const source = `\`\`\` +${narrowBorder} +\`\`\` + +${table}`; + const markdown = new Markdown(source, 0, 0, defaultMarkdownTheme); + markdown.transientRenderCache = true; + const initialLines = markdown.render(80); + const plainInitialLines = initialLines.map(line => stripVTControlCharacters(line).trimEnd()); + const codeBorderRow = plainInitialLines.indexOf(narrowBorder!); + const tableHeaderRow = plainInitialLines.findIndex(line => line.includes("Entry") && line.includes("Value")); + expect(codeBorderRow).toBeGreaterThanOrEqual(0); + expect(tableHeaderRow).toBeGreaterThan(codeBorderRow); + const actualTableStart = tableHeaderRow - 1; + expect(plainInitialLines[actualTableStart]!).toBe(narrowBorder!); + + // Commit through the code block, but stop immediately before the real + // table. Textual border scanning used to mistake the code row for it. + markdown.setNativeScrollbackCommittedRows(actualTableStart); + markdown.setText(`${source} +| much-longer-entry-that-arrives-after-commit | R001 |`); + const widenedBorder = markdown + .render(80) + .map(line => stripVTControlCharacters(line).trimEnd()) + .filter(line => line.startsWith("+")) + .at(-1); + expect(widenedBorder).toBeDefined(); + expect(widenedBorder).not.toBe(narrowBorder); + }); + + it("restores table layout metadata when finalization hits the shared render cache", () => { + clearRenderCache(); + const short = `| Entry | Value | +| --- | --- | +| short | R000 |`; + const wide = `${short} +| much-longer-entry-that-arrives-after-commit | R001 |`; + const topBorder = (lines: readonly string[]): string => { + const border = lines + .map(line => stripVTControlCharacters(line).trimEnd()) + .find(line => line.startsWith("+")); + expect(border).toBeDefined(); + return border!; + }; + + const markdown = new Markdown(short, 0, 0, defaultMarkdownTheme); + markdown.transientRenderCache = true; + const narrowBorder = topBorder(markdown.render(80)); + + // Pre-warm the canonical final render after this instance has retained + // metadata from its narrower transient frame. + const cachedWideBorder = topBorder(new Markdown(wide, 0, 0, defaultMarkdownTheme).render(80)); + markdown.setText(wide); + markdown.transientRenderCache = false; + expect(topBorder(markdown.render(80))).toBe(cachedWideBorder); + + // The frame served by L2 is now in native scrollback. Locking it must + // preserve the wide cached geometry, not the earlier transient geometry. + markdown.setNativeScrollbackCommittedRows(1); + expect(topBorder(markdown.render(80))).toBe(cachedWideBorder); + expect(cachedWideBorder).not.toBe(narrowBorder); + clearRenderCache(); + }); + it("should respect paddingX when calculating table width", () => { const markdown = new Markdown( `| Column One | Column Two | From 8570d80f25ddbf0a1720c4b9ab3e9d437049b2bb Mon Sep 17 00:00:00 2001 From: pppobear Date: Wed, 15 Jul 2026 21:24:01 +0800 Subject: [PATCH 2/2] fix(tui): preserve streamed table coordinates --- packages/tui/src/components/markdown.ts | 97 +++++++++++++------ .../test/markdown-stream-prefix-cache.test.ts | 38 ++++++++ packages/tui/test/markdown.test.ts | 38 ++++++++ 3 files changed, 146 insertions(+), 27 deletions(-) diff --git a/packages/tui/src/components/markdown.ts b/packages/tui/src/components/markdown.ts index 35acba17e..f7ec27b83 100644 --- a/packages/tui/src/components/markdown.ts +++ b/packages/tui/src/components/markdown.ts @@ -1309,7 +1309,7 @@ export class Markdown implements Component, NativeScrollbackCommittedRows, Nativ try { contentLines = this.transientRenderCache ? this.#renderStreamingContentLines(tokens, normalizedText, signature, contentWidth) - : this.#renderContentLines(tokens, 0, tokens.length, contentWidth, signature); + : this.#renderContentLines(tokens, 0, tokens.length, contentWidth, signature, 0, 0); } finally { this.#activeRenderSignature = undefined; this.#activeTableRenderSpecs = undefined; @@ -1374,16 +1374,18 @@ export class Markdown implements Component, NativeScrollbackCommittedRows, Nativ const frozenText = this.#streamPrefixText; const frozenTokenCount = this.#streamPrefixTokens?.length ?? 0; if (frozenText === undefined || frozenTokenCount === 0 || !normalizedText.startsWith(frozenText)) { - return this.#renderContentLines(tokens, 0, tokens.length, contentWidth, signature); + return this.#renderContentLines(tokens, 0, tokens.length, contentWidth, signature, 0, 0); } const contentLines: string[] = []; const reusablePrefix = this.#matchingStreamPrefixLineCache(normalizedText, frozenText, signature); let renderedUntil = 0; + let renderedSourceOffset = 0; if (reusablePrefix && reusablePrefix.tokenCount <= frozenTokenCount) { contentLines.push(...reusablePrefix.lines); this.#activeTableRenderSpecs?.push(...reusablePrefix.tables); renderedUntil = reusablePrefix.tokenCount; + renderedSourceOffset = reusablePrefix.text.length; } if (renderedUntil < frozenTokenCount) { @@ -1399,6 +1401,7 @@ export class Markdown implements Component, NativeScrollbackCommittedRows, Nativ contentWidth, signature, contentLines.length, + renderedSourceOffset, ), ); } finally { @@ -1437,6 +1440,7 @@ export class Markdown implements Component, NativeScrollbackCommittedRows, Nativ contentWidth, signature, contentLines.length, + frozenText.length, ), ); } @@ -1472,16 +1476,17 @@ export class Markdown implements Component, NativeScrollbackCommittedRows, Nativ end: number, contentWidth: number, signature: RenderSignature, - rowOffset = 0, + rowOffset: number, + startingSourceOffset: number, ): string[] { const wrappedLines: string[] = []; - let sourceOffset = 0; - for (let i = 0; i < start; i++) sourceOffset += tokens[i]!.raw.length; + let sourceOffset = startingSourceOffset; for (let i = start; i < end; i++) { const token = tokens[i]; const nextToken = tokens[i + 1]; const tableSpecStart = this.#activeTableRenderSpecs?.length ?? 0; - const tokenRowStart = rowOffset + wrappedLines.length; + const tokenWrappedRowStart = wrappedLines.length; + const tokenRowStart = rowOffset + tokenWrappedRowStart; const renderedTokenLines = this.#renderToken( token, contentWidth, @@ -1489,6 +1494,7 @@ export class Markdown implements Component, NativeScrollbackCommittedRows, Nativ undefined, `offset:${sourceOffset}`, ); + const tokenLineOffsets = [0]; for (const line of renderedTokenLines) { // Skip wrapping for image protocol lines and OSC 66 sized headings // (would corrupt escape sequences / split the indivisible sized span). @@ -1497,25 +1503,27 @@ export class Markdown implements Component, NativeScrollbackCommittedRows, Nativ } else { wrappedLines.push(...wrapTextWithAnsi(line, contentWidth)); } + tokenLineOffsets.push(wrappedLines.length - tokenWrappedRowStart); } - const tokenRowEnd = rowOffset + wrappedLines.length; const tableSpecs = this.#activeTableRenderSpecs; if (tableSpecs !== undefined) { for (let specIndex = tableSpecStart; specIndex < tableSpecs.length; specIndex++) { const spec = tableSpecs[specIndex]!; + let relativeStart: number; + let relativeEnd: number; if (token.type === "table") { - // A top-level table's own rows are already width-bounded, so none - // wrap here. Exclude the optional inter-block blank from its span. - spec.startRow = tokenRowStart; - spec.endRow = Math.min(tokenRowEnd, tokenRowStart + spec.lineCount); + // Exclude the optional inter-block blank from a top-level table's span. + relativeStart = 0; + relativeEnd = Math.min(renderedTokenLines.length, spec.lineCount); } else { - // Tables nested in a blockquote inherit the enclosing token's span. - // This is conservative (it may lock within the quote's prose head) - // but remains structural and can never confuse unrelated text for - // a table border. - spec.startRow = tokenRowStart; - spec.endRow = tokenRowEnd; + // Container renderers express nested table spans relative to their + // returned lines. Preserve that exact span through this final wrap. + if (spec.startRow < 0 || spec.endRow <= spec.startRow) continue; + relativeStart = Math.min(renderedTokenLines.length, spec.startRow); + relativeEnd = Math.min(renderedTokenLines.length, spec.endRow); } + spec.startRow = tokenRowStart + tokenLineOffsets[relativeStart]!; + spec.endRow = tokenRowStart + tokenLineOffsets[relativeEnd]!; } } sourceOffset += token.raw.length; @@ -1908,26 +1916,59 @@ export class Markdown implements Component, NativeScrollbackCommittedRows, Nativ const quoteContentWidth = Math.max(1, width - 2); const quoteTokens = token.tokens || []; const renderedQuoteLines: string[] = []; + const blockquoteSpecStart = this.#activeTableRenderSpecs?.length ?? 0; for (let i = 0; i < quoteTokens.length; i++) { const quoteToken = quoteTokens[i]; const nextQuoteToken = quoteTokens[i + 1]; - renderedQuoteLines.push( - ...this.#renderToken( - quoteToken, - quoteContentWidth, - nextQuoteToken?.type, - quoteInlineStyleContext, - `${tokenKey}/quote:${i}`, - ), + const quoteTokenRowStart = renderedQuoteLines.length; + const quoteSpecStart = this.#activeTableRenderSpecs?.length ?? 0; + const quoteTokenLines = this.#renderToken( + quoteToken, + quoteContentWidth, + nextQuoteToken?.type, + quoteInlineStyleContext, + `${tokenKey}/quote:${i}`, ); + renderedQuoteLines.push(...quoteTokenLines); + + const tableSpecs = this.#activeTableRenderSpecs; + if (tableSpecs !== undefined) { + for (let specIndex = quoteSpecStart; specIndex < tableSpecs.length; specIndex++) { + const spec = tableSpecs[specIndex]!; + if (spec.startRow < 0) { + // Direct child tables initially have no row coordinates. Their + // structural line count excludes any inter-block blank. + spec.startRow = quoteTokenRowStart; + spec.endRow = quoteTokenRowStart + Math.min(quoteTokenLines.length, spec.lineCount); + } else { + // A nested blockquote already mapped the table into its own + // returned rows; translate those rows into this quote's input. + spec.startRow += quoteTokenRowStart; + spec.endRow += quoteTokenRowStart; + } + } + } } while (renderedQuoteLines.length > 0 && renderedQuoteLines[renderedQuoteLines.length - 1] === "") { renderedQuoteLines.pop(); } - lines.push(...this.#applyQuoteBorder(renderedQuoteLines, width)); + const quoteRowOffsets: number[] = []; + const borderedQuoteLines = this.#applyQuoteBorder(renderedQuoteLines, width, quoteRowOffsets); + const tableSpecs = this.#activeTableRenderSpecs; + if (tableSpecs !== undefined) { + for (let specIndex = blockquoteSpecStart; specIndex < tableSpecs.length; specIndex++) { + const spec = tableSpecs[specIndex]!; + if (spec.startRow < 0 || spec.endRow <= spec.startRow) continue; + const relativeStart = Math.min(renderedQuoteLines.length, spec.startRow); + const relativeEnd = Math.min(renderedQuoteLines.length, spec.endRow); + spec.startRow = quoteRowOffsets[relativeStart]!; + spec.endRow = quoteRowOffsets[relativeEnd]!; + } + } + lines.push(...borderedQuoteLines); if (nextTokenType && nextTokenType !== "space") { lines.push(""); // Add spacing after blockquotes (unless space token follows) } @@ -1974,7 +2015,7 @@ export class Markdown implements Component, NativeScrollbackCommittedRows, Nativ * Wrap already-rendered lines in the blockquote border and quote styling. * `width` is the full content width; the border reserves two cells. */ - #applyQuoteBorder(renderedLines: string[], width: number): string[] { + #applyQuoteBorder(renderedLines: string[], width: number, sourceRowOffsets?: number[]): string[] { const quoteStyle = (text: string) => this.#theme.quote(this.#theme.italic(text)); const quoteStylePrefix = this.#getStylePrefix(quoteStyle); const applyQuoteStyle = (line: string): string => { @@ -1986,11 +2027,13 @@ export class Markdown implements Component, NativeScrollbackCommittedRows, Nativ }; const quoteContentWidth = Math.max(1, width - 2); const lines: string[] = []; + sourceRowOffsets?.push(0); for (const quoteLine of renderedLines) { const styledLine = applyQuoteStyle(quoteLine); for (const wrappedLine of wrapTextWithAnsi(styledLine, quoteContentWidth)) { lines.push(this.#theme.quoteBorder(`${this.#theme.symbols.quoteBorder} `) + wrappedLine); } + sourceRowOffsets?.push(lines.length); } return lines; } diff --git a/packages/tui/test/markdown-stream-prefix-cache.test.ts b/packages/tui/test/markdown-stream-prefix-cache.test.ts index 845b16819..dcf242634 100644 --- a/packages/tui/test/markdown-stream-prefix-cache.test.ts +++ b/packages/tui/test/markdown-stream-prefix-cache.test.ts @@ -1,4 +1,5 @@ import { describe, expect, it } from "bun:test"; +import { stripVTControlCharacters } from "node:util"; import { clearRenderCache, Markdown, type MarkdownTheme } from "@oh-my-pi/pi-tui/components/markdown"; import { defaultMarkdownTheme } from "./test-themes.js"; @@ -90,4 +91,41 @@ describe("Markdown streaming prefix render cache", () => { expect(streamingLines).toEqual(renderCold(prefix, defaultMarkdownTheme)); }); + + it("keeps table layout keys stable when rendering after a cached prefix", () => { + const prefix = `| Archived entry | Code | +| --- | --- | +| prefix-column-is-deliberately-wide | P000 | + +`; + const initialText = `${prefix}| Live entry | Value | +| --- | --- | +| short | R000 |`; + const expandedText = `${initialText} +| tail-column-is-even-longer-than-before | R001 |`; + const md = new Markdown(initialText, 0, 0, defaultMarkdownTheme); + md.transientRenderCache = true; + + const tableAt = (lines: readonly string[], marker: string): { startRow: number; border: string } => { + const plain = lines.map(line => stripVTControlCharacters(line).trimEnd()); + const headerRow = plain.findIndex(line => line.includes(marker)); + expect(headerRow).toBeGreaterThan(0); + return { startRow: headerRow - 1, border: plain[headerRow - 1]! }; + }; + + const initialLines = md.render(WIDTH); + const prefixTable = tableAt(initialLines, "Archived entry"); + const tailTable = tableAt(initialLines, "Live entry"); + expect(prefixTable.border).not.toBe(tailTable.border); + md.setNativeScrollbackCommittedRows(tailTable.startRow + 1); + + md.setText(expandedText); + const expandedLines = md.render(WIDTH); + const expandedPrefix = tableAt(expandedLines, "Archived entry"); + const expandedTail = tableAt(expandedLines, "Live entry"); + expect(expandedPrefix.border).toBe(prefixTable.border); + expect(expandedTail.border).toBe(tailTable.border); + expect(expandedTail.border).not.toBe(expandedPrefix.border); + expect(expandedLines.some(line => stripVTControlCharacters(line).includes("R001"))).toBe(true); + }); }); diff --git a/packages/tui/test/markdown.test.ts b/packages/tui/test/markdown.test.ts index 6b2276309..0fad6dff7 100644 --- a/packages/tui/test/markdown.test.ts +++ b/packages/tui/test/markdown.test.ts @@ -526,6 +526,44 @@ describe("Markdown component", () => { expect(widenedBorders[3]).toBe(borders[3]); }); + it("does not lock a quoted table until the table itself enters native scrollback", () => { + const initial = `> > Intro sentence deliberately long enough to wrap across several physical quote rows before the table. +> > +> > | Entry | Value | +> > | --- | --- | +> > | short | R000 |`; + const beforeCommit = `${initial} +> > | medium-width-entry | R001 |`; + const afterCommit = `${beforeCommit} +> > | entry-that-is-even-wider-than-the-locked-layout | R002 |`; + const markdown = new Markdown(initial, 0, 0, defaultMarkdownTheme); + markdown.transientRenderCache = true; + + const tableGeometry = (lines: readonly string[]): { start: number; border: string } => { + const plain = lines.map(line => stripVTControlCharacters(line).trimEnd()); + const header = plain.findIndex(line => line.includes("Entry") && line.includes("Value")); + expect(header).toBeGreaterThan(0); + return { start: header - 1, border: plain[header - 1]! }; + }; + + const initialLines = markdown.render(48); + const initialTable = tableGeometry(initialLines); + expect(initialTable.start).toBeGreaterThan(2); + // Commit only the quote prose; the nested table remains wholly live. + markdown.setNativeScrollbackCommittedRows(initialTable.start); + + markdown.setText(beforeCommit); + const growingLines = markdown.render(48); + const growingTable = tableGeometry(growingLines); + expect(growingTable.border).not.toBe(initialTable.border); + + markdown.setNativeScrollbackCommittedRows(growingTable.start + 1); + markdown.setText(afterCommit); + const lockedLines = markdown.render(48); + expect(tableGeometry(lockedLines).border).toBe(growingTable.border); + expect(lockedLines.some(line => stripVTControlCharacters(line).includes("R002"))).toBe(true); + }); + it("recomputes a locked streamed table after resize or non-append replacement", () => { const short = `| Entry | Value | | --- | --- |