From 90c696d00ac6dfcad6d8eb18d3683848770bfca6 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Thu, 6 Aug 2026 21:53:19 -0700 Subject: [PATCH] Stop transcript headings flickering while text below them streams MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The markdown renderer's default block mode merges a heading into the same raw chunk as the paragraph that follows it. Every keystroke of that paragraph changes the merged chunk's raw text, so the heading's already-settled markup re-highlights too, flickering while the rest of the message keeps streaming in. Render a streaming row's markdown body as two stacked renderers when a heading has closed: a "frozen" one holding everything through the last such heading, marked non-streaming and never handed new content again, and a "live" one holding the still-growing tail. Most rows have no closed heading yet, so they keep painting through a single renderer as before; the split only ever falls at a settled heading boundary, never at a paragraph, list, or fence boundary, so non-heading layout is unchanged. Checked whether `@opentui/core` exposes a public surface for this boundary instead of scanning lines by hand: `MarkdownRenderable`'s own block/token state (`_parseState`, `_blockStates`) is underscore-prefixed and not part of its declared public API, and the module that builds it (`renderables/markdown-parser.js`) has no subpath in the package's `exports` map, so it cannot be imported at all through the supported entry points. No such surface exists, so the boundary is derived locally, tracking fence state (both fence characters, matching-or-longer closers only, a closing line may carry no trailing text per CommonMark) so a `#` line inside a fenced code block — a shell or Python comment, for instance — is never read as a heading. Adds a mid-stream span-sampling test with no settle wait that reproduces the shake directly against the unfixed renderer, plus regression tests pinning list spacing, ordered-list marker width, and fence/heading edge cases (unmatched fence lengths and characters, a closing fence with trailing text, indentation limits, an indented heading) across the change. --- src/tui-opentui/markdown-rows.test.ts | 325 +++++++++++++++++++++++++- src/tui-opentui/shell.ts | 195 ++++++++++++++-- 2 files changed, 497 insertions(+), 23 deletions(-) diff --git a/src/tui-opentui/markdown-rows.test.ts b/src/tui-opentui/markdown-rows.test.ts index d816769b7..65e5d9da8 100644 --- a/src/tui-opentui/markdown-rows.test.ts +++ b/src/tui-opentui/markdown-rows.test.ts @@ -4,8 +4,15 @@ */ import { describe, expect, test } from "bun:test" +import { MarkdownRenderable, BoxRenderable, type CapturedSpan } from "@opentui/core" import { withTestRenderer, type Harness } from "./harness" -import { appendStreamRow, createAppShell, replaceStreamRowAt } from "./shell" +import { + appendStreamRow, + createAppShell, + createStreamRowRenderable, + replaceStreamRowAt, + splitAtSettledHeading, +} from "./shell" import { isMarkdownRow } from "./stream" const WIDE = { width: 80, height: 24 } as const @@ -15,11 +22,17 @@ const shellOpts = { wireKeys: false, } as const -/** Markdown blocks highlight asynchronously; settle before capturing a frame. */ +/** + * Markdown blocks highlight asynchronously; settle before capturing a frame. + * A row with several top-level blocks (heading, list, fence, link) resolves + * its highlight promises one render at a time, so a fixed couple of ticks + * that was enough for one block is not enough for several. + */ async function settle(h: Harness): Promise { - await new Promise((resolve) => setTimeout(resolve, 250)) - await h.renderOnce() - await h.renderOnce() + for (let i = 0; i < 8; i += 1) { + await new Promise((resolve) => setTimeout(resolve, 50)) + await h.renderOnce() + } return h.captureCharFrame() } @@ -144,4 +157,306 @@ describe("markdown transcript rows", () => { expect(next).not.toContain("#### Title") }, WIDE) }) + + test("a row with no settled heading paints through a single renderer, not a wasted split", async () => { + // Most rows never have a settled heading behind their tail (no heading at + // all, or the only one is still being typed). Building the frozen/live + // pair unconditionally would double every markdown row's renderer count + // for no benefit in the common case. + await withTestRenderer(async (h) => { + const shell = createAppShell(h.renderer, shellOpts) + const node = createStreamRowRenderable(shell, { + role: "assistant", + text: "Just a paragraph, no heading at all.", + }) + expect(node).toBeInstanceOf(BoxRenderable) + const [, bodyNode] = (node as BoxRenderable).getChildren() + expect(bodyNode).toBeInstanceOf(MarkdownRenderable) + }, WIDE) + }) + + test("a closed heading renders in its own settled renderer, separate from the prose after it", async () => { + // The library's default block mode merges a heading into the same raw + // chunk as the paragraph that follows it, so every keystroke of that + // paragraph re-highlights the heading's already-settled text too — the + // heading's markers and styling visibly flicker while the rest of the + // message keeps streaming in. Splitting the body at the heading gives it + // its own renderer, marked non-streaming, that the live (still-growing) + // half never shares — so it is never asked to re-highlight again. + await withTestRenderer(async (h) => { + const shell = createAppShell(h.renderer, shellOpts) + const node = createStreamRowRenderable(shell, { + role: "assistant", + streaming: true, + text: ["### Title", "", "Some body text."].join("\n"), + }) + expect(node).toBeInstanceOf(BoxRenderable) + const [, bodyNode] = (node as BoxRenderable).getChildren() + expect(bodyNode).toBeInstanceOf(BoxRenderable) + const [frozenNode, liveNode] = (bodyNode as BoxRenderable).getChildren() + expect(frozenNode).toBeInstanceOf(MarkdownRenderable) + expect(liveNode).toBeInstanceOf(MarkdownRenderable) + expect((frozenNode as MarkdownRenderable).content).toContain("### Title") + expect((frozenNode as MarkdownRenderable).streaming).toBe(false) + expect((liveNode as MarkdownRenderable).content).toBe("Some body text.") + expect((liveNode as MarkdownRenderable).streaming).toBe(true) + }, WIDE) + }) + + test("the settled heading renderer is never rewritten while the prose after it keeps streaming", async () => { + await withTestRenderer(async (h) => { + const shell = createAppShell(h.renderer, shellOpts) + appendStreamRow(shell, { + role: "assistant", + streaming: true, + text: ["### Title", "", "Some"].join("\n"), + }) + const children = shell.transcript.getChildren().slice(1) + const [, bodyNode] = (children[0] as BoxRenderable).getChildren() + const [frozenNode] = (bodyNode as BoxRenderable).getChildren() + const before = (frozenNode as MarkdownRenderable).content + + replaceStreamRowAt(shell, shell.streamLog.length - 1, { + role: "assistant", + streaming: true, + text: ["### Title", "", "Some body text that keeps growing and growing."].join("\n"), + }) + const childrenAfter = shell.transcript.getChildren().slice(1) + const [, bodyNodeAfter] = (childrenAfter[0] as BoxRenderable).getChildren() + const [frozenNodeAfter] = (bodyNodeAfter as BoxRenderable).getChildren() + + expect(frozenNodeAfter).toBe(frozenNode) + expect((frozenNodeAfter as MarkdownRenderable).content).toBe(before) + }, WIDE) + }) + + test("a list directly under a paragraph, with no blank line, keeps that shape after the split", async () => { + // Regression guard: the split must never fall at a list boundary — only + // at a settled heading — so paragraph/list spacing stays byte-identical + // to the unsplit renderer's own default layout. + await withTestRenderer(async (h) => { + const shell = createAppShell(h.renderer, shellOpts) + appendStreamRow(shell, { + role: "assistant", + text: ["### Title", "", "Here is the list:", "- alpha", "- beta"].join("\n"), + }) + const frame = await settle(h) + const lines = frame.split("\n").map((line) => line.trimEnd()) + const listLine = lines.findIndex((line) => line.includes("Here is the list:")) + expect(listLine).toBeGreaterThan(-1) + // No blank row inserted between the paragraph and the list beneath it. + expect(lines[listLine + 1]).toContain("alpha") + }, WIDE) + }) + + test("a ten-item ordered list under a heading keeps unpadded markers", async () => { + await withTestRenderer(async (h) => { + const shell = createAppShell(h.renderer, shellOpts) + const items = Array.from({ length: 10 }, (_, i) => `${i + 1}. item ${i + 1}`) + appendStreamRow(shell, { + role: "assistant", + text: ["### Steps", "", ...items].join("\n"), + }) + const frame = await settle(h) + expect(frame).toContain("1. item 1") + expect(frame).toContain("10. item 10") + }, WIDE) + }) + + /** The heading's own painted span, wherever it lands in the current frame. */ + function headingSpan(h: Harness): CapturedSpan | null { + for (const line of h.captureSpans().lines) { + for (const span of line.spans) { + if (span.text.includes("Title")) return span + } + } + return null + } + + test("a settled heading's painted span never changes while the prose after it keeps streaming", async () => { + // The shake this fixes is a transient re-highlight, not a settled-frame + // difference — a snapshot taken only after several idle ticks (as every + // other test in this file does) cannot see it, because the async + // highlight pass has always finished by then. This test instead samples + // the heading's span on every delta, immediately after a single render + // with no settle wait, which is the one place the flicker would show up. + await withTestRenderer(async (h) => { + const shell = createAppShell(h.renderer, shellOpts) + const full = "Some body text that keeps growing and growing more and more and even more." + appendStreamRow(shell, { + role: "assistant", + streaming: true, + text: ["### Title", "", full.slice(0, 1)].join("\n"), + }) + // Warm up once: the first highlight pass in a process loads the + // tree-sitter grammar and is not itself part of what this test samples. + const baseline = await settle(h).then(() => headingSpan(h)) + expect(baseline).not.toBeNull() + + for (let i = 2; i <= full.length; i += 1) { + replaceStreamRowAt(shell, shell.streamLog.length - 1, { + role: "assistant", + streaming: true, + text: ["### Title", "", full.slice(0, i)].join("\n"), + }) + await h.renderOnce() + const span = headingSpan(h) + expect(span).not.toBeNull() + expect(span!.text).toBe(baseline!.text) + expect(span!.fg).toEqual(baseline!.fg) + expect(span!.attributes).toBe(baseline!.attributes) + } + }, WIDE) + }) + + describe("splitAtSettledHeading never splits a fenced code block", () => { + test("a `#` shell comment inside a fence is not read as a heading boundary", () => { + const text = [ + "```bash", + "# this is a comment, not a heading", + "echo hi", + "```", + "", + "more prose streaming in", + ].join("\n") + const split = splitAtSettledHeading(text) + // No real heading anywhere in this text, fenced or not: no split at all. + expect(split).toBeNull() + }) + + test("a real heading before an open fence still splits, and the fence stays whole", () => { + const text = [ + "### Title", + "", + "```bash", + "# comment, not a heading", + "echo hi", + "```", + "", + "more prose streaming in", + ].join("\n") + const split = splitAtSettledHeading(text) + expect(split).not.toBeNull() + // The fence opens and closes on the same side of the split. + expect(split!.frozen).toBe("### Title") + expect(split!.live).toContain("```bash") + expect(split!.live).toContain("```\n") + }) + + test("a fence opened before a heading keeps the heading out of the boundary search until it closes", () => { + const text = [ + "```py", + "# looks like a heading but is not", + "```", + "", + "### Real Title", + "", + "body text", + ].join("\n") + const split = splitAtSettledHeading(text) + expect(split).not.toBeNull() + expect(split!.frozen).toContain("### Real Title") + expect(split!.frozen).not.toContain("body text") + expect(split!.live).toBe("body text") + }) + + test("a fenced `#` comment renders inside a matched fence, not split across two renderers", async () => { + await withTestRenderer(async (h) => { + const shell = createAppShell(h.renderer, shellOpts) + appendStreamRow(shell, { + role: "assistant", + streaming: true, + text: [ + "```bash", + "# this is a comment, not a heading", + "echo hi", + "```", + "", + "more prose streaming in", + ].join("\n"), + }) + const frame = await settle(h) + expect(frame).toContain("# this is a comment, not a heading") + expect(frame).toContain("echo hi") + expect(frame).toContain("more prose streaming in") + }, WIDE) + }) + + test("a closing-fence-shaped line carrying trailing text does not close the fence", () => { + // CommonMark: the closing delimiter may contain only the fence + // characters and trailing whitespace. "```stillcode" is more fence + // content, not a closer, so the `#` after the real closer is still the + // first heading — not the "```stillcode" line before it. + const text = [ + "```bash", + "echo hi", + "```stillcode", + "# should still be inside fence per CommonMark", + "```", + "", + "### Title", + "", + "body", + ].join("\n") + const split = splitAtSettledHeading(text) + expect(split).not.toBeNull() + expect(split!.frozen).toContain("### Title") + expect(split!.frozen).toContain("```stillcode") + expect(split!.frozen).toContain("# should still be inside fence per CommonMark") + expect(split!.live).toBe("body") + }) + + test("adversarial fence pairings: length and character must both match, indentation is bounded", () => { + // Four backticks are not closed by three — the fence stays open, so + // "### Title" is fence content too and there is no heading at all. + expect( + splitAtSettledHeading( + ["````", "# not a heading", "```", "### Title", "", "body"].join("\n"), + ), + ).toBeNull() + // The same shape, properly closed by a run of 4+: now it is a heading. + expect( + splitAtSettledHeading( + ["````", "# not a heading", "```", "### not a heading either", "````", "", "### Title", "", "body"].join( + "\n", + ), + )!.frozen, + ).toContain("### Title") + // Three backticks are closed by four (a longer run of the same char). + expect( + splitAtSettledHeading( + ["```", "# not a heading", "````", "", "### Title", "", "body"].join("\n"), + )!.frozen, + ).toContain("### Title") + // A tilde run never closes a backtick fence, or vice versa. + expect( + splitAtSettledHeading( + ["```", "~~~", "# not a heading", "```", "### Title", "", "body"].join("\n"), + )!.frozen, + ).toContain("### Title") + // Up to 3 spaces of indent still opens/closes a fence. + expect( + splitAtSettledHeading( + [" ```", "# not a heading", " ```", "### Title", "", "body"].join("\n"), + )!.frozen, + ).toContain("### Title") + // 4 spaces is indented code, not a fence — the `#` line is still inside + // it as indented code, never a heading boundary on its own. + expect( + splitAtSettledHeading([" ```", " # not a heading", "body"].join("\n")), + ).toBeNull() + // An unclosed fence at end of input, with a `#` line inside it and no + // real heading anywhere: nothing to split at. + expect( + splitAtSettledHeading(["```", "# still fence content, not a heading", "still going"].join("\n")), + ).toBeNull() + }) + }) + + test("an indented heading (CommonMark allows up to 3 leading spaces) still closes the split", () => { + const split = splitAtSettledHeading([" ### Title", "", "body"].join("\n")) + expect(split).not.toBeNull() + expect(split!.frozen).toBe(" ### Title") + expect(split!.live).toBe("body") + }) }) diff --git a/src/tui-opentui/shell.ts b/src/tui-opentui/shell.ts index 432d9f872..03d7aa12f 100644 --- a/src/tui-opentui/shell.ts +++ b/src/tui-opentui/shell.ts @@ -2150,18 +2150,37 @@ function retextStreamRowBody( if (!(node instanceof BoxRenderable) || !isMarkdownRow(row)) return false const [gutterNode, bodyNode] = node.getChildren() - if ( - !(gutterNode instanceof TextRenderable) || - !(bodyNode instanceof MarkdownRenderable) - ) { - return false - } + if (!(gutterNode instanceof TextRenderable)) return false const gutter = streamRowGutter(row, layout) gutterNode.content = gutter.content gutterNode.width = stringWidth(gutter.content) - bodyNode.width = markdownBodyColumns(gutter, layout) - bodyNode.content = markdownContent(row) - bodyNode.streaming = row.streaming === true + const width = markdownBodyColumns(gutter, layout) + const content = markdownContent(row) + const split = splitAtSettledHeading(content) + + // No settled heading behind the tail: a lone renderer, same as an unsplit + // body. A shape change (a heading just closed, or one just left the window + // a full rebuild trimmed) falls through to the caller's rebuild. + if (split === null) { + if (!(bodyNode instanceof MarkdownRenderable)) return false + bodyNode.width = width + bodyNode.content = content + bodyNode.streaming = row.streaming === true + return true + } + + if (!(bodyNode instanceof BoxRenderable)) return false + const [frozenNode, liveNode] = bodyNode.getChildren() + if (!(frozenNode instanceof MarkdownRenderable) || !(liveNode instanceof MarkdownRenderable)) { + return false + } + bodyNode.width = width + frozenNode.width = width + frozenNode.content = split.frozen + liveNode.width = width + liveNode.content = split.live + liveNode.streaming = row.streaming === true + liveNode.marginTop = split.gapRows return true } @@ -2312,6 +2331,99 @@ function markdownContent(row: StreamRow): string { return row.text.replace(/(^|\n)#{1,6}[ \t]*$/, "$1") } +/** + * An ATX heading line (`#` through `######`) with a title, not a bare marker. + * CommonMark allows the marker up to 3 spaces in; a 4th makes it indented code + * instead, which this line still has to reject. + */ +const HEADING_LINE_RE = /^ {0,3}#{1,6}[ \t]+\S.*$/ + +/** + * A fenced code block's opening delimiter: three or more backticks or tildes, + * optionally indented up to three spaces (CommonMark's limit before a fence + * counts as indented code instead), followed by anything (an info string, + * e.g. the "bash" in ` ```bash `). + */ +const FENCE_OPEN_RE = /^ {0,3}(`{3,}|~{3,})/ + +/** + * A fenced code block's closing delimiter. Unlike the opener, CommonMark + * requires the closing line to contain nothing but the fence run and + * trailing whitespace — "```stillcode" does not close a fence, it is more + * fence content — so this is deliberately not just `FENCE_OPEN_RE` again. + */ +const FENCE_CLOSE_RE = /^ {0,3}(`{3,}|~{3,})[ \t]*$/ + +/** + * Lines that are inside a fenced code block, where a leading `#` is a shell + * comment or similar and never a heading. A closer needs the same character + * as the opener and a run at least as long — a shorter run, a run of the + * other character, or a closing-shaped line carrying trailing text is just + * more fence content, per CommonMark. + */ +function fencedLineMask(lines: readonly string[]): boolean[] { + const inside = new Array(lines.length).fill(false) + let opener: { char: string; length: number } | null = null + for (let i = 0; i < lines.length; i += 1) { + if (opener === null) { + const match = lines[i]!.match(FENCE_OPEN_RE) + if (match) { + inside[i] = true + opener = { char: match[1]![0]!, length: match[1]!.length } + } + continue + } + inside[i] = true + const close = lines[i]!.match(FENCE_CLOSE_RE) + if (close && close[1]![0] === opener.char && close[1]!.length >= opener.length) { + opener = null + } + } + return inside +} + +/** + * A markdown body split at the last heading that already has content behind + * it: everything through that heading, and everything after it. + * + * The renderer's own incremental parser only reuses a block whose raw text is + * unchanged; the default block mode merges a heading into the same raw chunk + * as the paragraph that follows it, so every keystroke of that paragraph + * changes the merged chunk's raw text and forces the heading's already-settled + * markup to re-highlight too — visibly flickering while the rest of the + * message keeps streaming in. Rendering the two halves as separate + * `MarkdownRenderable`s keeps the heading's renderer untouched once it is no + * longer the one growing, without changing how paragraphs, lists or tables + * inside either half are laid out (both halves still use the library's + * default block mode). + */ +export type MarkdownSplit = { + readonly frozen: string + readonly live: string + /** Blank source lines between the heading and what follows it (0 or 1). */ + readonly gapRows: number +} + +export function splitAtSettledHeading(text: string): MarkdownSplit | null { + const lines = text.split("\n") + const insideFence = fencedLineMask(lines) + let boundary = -1 + for (let i = 0; i < lines.length; i += 1) { + if (!insideFence[i] && HEADING_LINE_RE.test(lines[i]!)) boundary = i + } + // No heading, or the last one is still the open tail: nothing to freeze. + if (boundary === -1 || boundary >= lines.length - 1) return null + const rest = lines.slice(boundary + 1) + const firstContent = rest.findIndex((line) => line.trim().length > 0) + // Heading closed but nothing has started under it yet. + if (firstContent === -1) return null + return { + frozen: lines.slice(0, boundary + 1).join("\n"), + live: rest.slice(firstContent).join("\n"), + gapRows: firstContent > 0 ? 1 : 0, + } +} + /** * Build the row-shaped paint node: a MarkdownRenderable body next to a plain * gutter for markdown-bearing rows (assistant replies), a TextTableRenderable @@ -2380,19 +2492,66 @@ function buildRowNode( const gutter = streamRowGutter(row, layout) const wrapper = new BoxRenderable(ctx, { flexDirection: "row", width: "100%" }) wrapper.add(gutterNode(ctx, gutter)) - wrapper.add( - new MarkdownRenderable(ctx, { - content: markdownContent(row), - syntaxStyle: transcriptSyntaxStyle(), - fg: gutter.fg, - width: markdownBodyColumns(gutter, layout), - flexShrink: 0, - tableOptions: TRANSCRIPT_TABLE_OPTIONS, + wrapper.add(createMarkdownBody(ctx, row, gutter, layout)) + return wrapper +} + +/** Shared construction options for a transcript markdown body's renderer. */ +function markdownBodyOptions(gutter: PaintedStreamLine, width: number) { + return { + syntaxStyle: transcriptSyntaxStyle(), + fg: gutter.fg, + width, + flexShrink: 0, + tableOptions: TRANSCRIPT_TABLE_OPTIONS, + } as const +} + +/** + * A markdown row's body. Most rows have no settled heading yet (no heading at + * all, or the only one is still the open tail), and paint through a single + * renderer, same as before this fix existed. Once a heading closes, the body + * becomes a settled `frozen` renderer — everything through that heading, + * never streaming, never handed new content while the tail keeps growing, so + * it is never asked to re-highlight once written — stacked above the still + * `live` one, which carries the row's own streaming flag. Both halves use the + * library's default block mode, so paragraphs, lists and tables inside either + * one lay out exactly as a single unsplit body would. + */ +function createMarkdownBody( + ctx: CliRenderer, + row: StreamRow, + gutter: PaintedStreamLine, + layout: RowLayout, +): MarkdownRenderable | BoxRenderable { + const width = markdownBodyColumns(gutter, layout) + const content = markdownContent(row) + const split = splitAtSettledHeading(content) + if (split === null) { + return new MarkdownRenderable(ctx, { + ...markdownBodyOptions(gutter, width), + content, // Native incremental block stability: only the trailing block is unstable. streaming: row.streaming === true, + }) + } + const column = new BoxRenderable(ctx, { flexDirection: "column", width }) + column.add( + new MarkdownRenderable(ctx, { + ...markdownBodyOptions(gutter, width), + content: split.frozen, + streaming: false, }), ) - return wrapper + column.add( + new MarkdownRenderable(ctx, { + ...markdownBodyOptions(gutter, width), + content: split.live, + streaming: row.streaming === true, + marginTop: split.gapRows, + }), + ) + return column } /**