From 24b44d56a3f989dbdfdceaf346ec07e3d8e9e281 Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Sat, 18 Jul 2026 23:42:10 -0400 Subject: [PATCH] refactor(tui): tighten StreamingMd comments and fence folding Condense the header rationale to the load-bearing invariants and inline the single-line math-fence guards. No behavior change; scanner logic and public surface (createScanState / advanceScan / findStableBoundary) unchanged. --- ui-tui/src/components/streamingMarkdown.tsx | 144 +++++++------------- 1 file changed, 46 insertions(+), 98 deletions(-) diff --git a/ui-tui/src/components/streamingMarkdown.tsx b/ui-tui/src/components/streamingMarkdown.tsx index 1acda990b781..992bf9a67b65 100644 --- a/ui-tui/src/components/streamingMarkdown.tsx +++ b/ui-tui/src/components/streamingMarkdown.tsx @@ -1,60 +1,32 @@ // StreamingMd — incremental markdown renderer for in-flight assistant text. // -// Naive approach (render ) re-tokenizes the entire message -// on every stream delta. At 20-char batches over a 3 KB response that's 150 -// full re-parses. +// Rendering per delta re-tokenizes the whole message every +// time (O(total) × deltas). The prior stable-prefix split fixed the per-delta +// cost but not the per-block cliff: each advanced boundary re-tokenized the +// entire prefix from scratch — O(blocks²) — plus an O(total) fence rescan. // -// The previous incarnation split `text` at the last stable top-level block -// boundary into ONE memoized stable-prefix plus a re-parsed tail . -// That removed the per-delta cost but kept a per-block cliff: every time the -// boundary advanced, the prefix string changed, its memo key missed, and the -// ENTIRE prefix re-tokenized from scratch — O(blocks²) work across a long -// reply, plus an O(total) fence scan on every delta to find the boundary. +// This is fully incremental. A forward scanner keeps fence/math state + scan +// position in a ref across deltas, so each delta touches only newly-arrived +// complete lines. Settled top-level blocks (split on "\n\n" outside a fence) +// are frozen into an append-only array, each its own memoized on text +// that never changes — every block tokenizes exactly once. Only the in-flight +// tail re-parses per delta (O(tail)). // -// This version is fully incremental: -// settled blocks — an append-only array of top-level block strings. Each -// block renders as its own , memoized on its exact -// text, which never changes once committed. Every block -// is tokenized exactly once for the life of the stream. -// unstable tail — the in-flight block(s) after the last committed -// boundary. A single re-parses just this tail on -// every delta (O(tail) vs. O(total)). -// scanner state — fence/math open-state and scan position persist across -// deltas in a ref, so each delta only scans the newly -// arrived complete lines instead of the whole text. +// Invariants that keep it correct: +// · Only newline-terminated lines are scanned; a partial trailing line may +// yet become a fence opener, so it stays in the tail. +// · Blank-line boundaries can't be retroactively merged (a setext underline +// only binds the contiguous line above it, never across a committed "\n\n"). +// · An unmatched `$$` / `\[` opener is treated as open forever — more +// conservative than markdown.tsx's full-text fallback, because a committed +// block is frozen and can't be un-decided once the closer streams in. +// · State only advances (idempotent under StrictMode). The component +// unmounts between turns; if `text` stops extending `scanned` (turn reuse, +// or boundedLiveRenderText front-trimming a huge reply) the scanner resets +// and 's LRU absorbs the re-parse. // -// A block boundary is a literal "\n\n" outside any fenced code or display -// math block. Only complete lines (terminated by "\n") are scanned — a -// partial trailing line could still grow into a fence opener, so it stays -// in the tail until its newline arrives. Blank-line boundaries also make -// the split immune to retroactive block merges (e.g. a setext underline -// only attaches to the contiguous paragraph above it — it cannot reach -// across a committed "\n\n"). -// -// The scanner treats an unmatched `$$` / `\[` opener as open forever. This -// is INTENTIONALLY more conservative than `markdown.tsx`'s parser, which -// falls back to paragraph rendering when an opener has no closer. The -// renderer can do that safely because it always sees the full text on -// every call. The streaming chunker cannot — once a block is committed it -// is frozen, so prematurely deciding "this `$$` is just prose" would -// permanently commit a paragraph rendering that becomes wrong the instant -// the closer streams in. Parking the boundary keeps everything from the -// opener onward in the mutable tail until the closer arrives (or the -// stream ends and the non-streaming takes over, at which point the -// renderer's fallback kicks in correctly). -// -// State is stored in a ref and only ever advances — idempotent under -// StrictMode double-render. The component unmounts between turns -// (isStreaming flips off → message moves to history and renders via a -// single ), so the ref resets naturally. If `text` stops extending the -// scanned prefix anyway (turn reuse, or the bounded live-render window -// trimming the front of a very long reply), the scanner resets and the -// per-text LRU inside still absorbs most of the re-parse. -// -// Layout: the subtrees MUST render stacked (column). The parent -// container in messageLine.tsx is a default `flexDirection: 'row'` Box -// (Ink's default), so returning a bare Fragment of sibling s would lay -// them out side-by-side — the "two jumbled columns while streaming" bug. +// Layout: the subtrees MUST stack in a column — the messageLine.tsx +// parent is a default row Box, so bare siblings render side-by-side. import { Box } from '@hermes/ink' import { memo, useRef } from 'react' @@ -66,18 +38,14 @@ import { Md } from './markdown.js' export interface StreamScanState { /** Settled top-level block strings, in order. Append-only. */ blocks: string[] - /** True while inside an unclosed ``` / ~~~ fence at the scan position. */ + /** Inside an unclosed ``` / ~~~ fence at the scan position. */ codeOpen: boolean - /** Non-null while inside an unclosed display-math block at the scan position. */ + /** Non-null inside an unclosed display-math block at the scan position. */ mathOpener: '$$' | '\\[' | null - /** - * The prefix of the text whose complete lines have been scanned — - * text.slice(0, scanPos), with scanPos === scanned.length. Kept as a - * string (not just an index) so the reset guard can verify the scanned - * region still matches, fence state included. - */ + /** Prefix whose complete lines have been scanned (kept as text so the reset + * guard can confirm the scanned region — and thus fence state — still holds). */ scanned: string - /** Length of the committed prefix — blocks.join('').length. */ + /** Length of the committed prefix (blocks.join('').length). */ settledLen: number } @@ -89,12 +57,9 @@ export const createScanState = (): StreamScanState => ({ settledLen: 0 }) -// Apply one complete (newline-terminated) line to the fence/math state. -// Mirrors the toggle rules the old fenceOpenAt() used: ``` / ~~~ toggle the -// code fence; `$$` / `\[` open display math only when the line doesn't also -// close it; closers only count when the matching opener is pending. Math -// markers inside an open code fence are ignored (a `$$` example in a code -// block must not open math). +// Fold one complete line into the fence/math state: ``` / ~~~ toggle the code +// fence; `$$` / `\[` open display math unless the line also closes it; closers +// count only against a pending opener; math inside an open code fence is inert. const applyLine = (state: StreamScanState, line: string) => { if (/^(?:`{3,}|~{3,})/.test(line)) { state.codeOpen = !state.codeOpen @@ -107,18 +72,10 @@ const applyLine = (state: StreamScanState, line: string) => { } if (!state.mathOpener) { - if (/^\$\$/.test(line)) { - const singleLine = line.length >= 4 && /\$\$$/.test(line) - - if (!singleLine) { - state.mathOpener = '$$' - } - } else if (/^\\\[/.test(line)) { - const singleLine = /\\\]$/.test(line) - - if (!singleLine) { - state.mathOpener = '\\[' - } + if (/^\$\$/.test(line) && !(line.length >= 4 && /\$\$$/.test(line))) { + state.mathOpener = '$$' + } else if (/^\\\[/.test(line) && !/\\\]$/.test(line)) { + state.mathOpener = '\\[' } } else if (state.mathOpener === '$$' && /\$\$$/.test(line)) { state.mathOpener = null @@ -127,11 +84,10 @@ const applyLine = (state: StreamScanState, line: string) => { } } -// Consume newly arrived COMPLETE lines from the scan position, committing a -// block at every "\n\n" boundary reached outside fences. Whitespace-only -// candidates (runs of 3+ newlines) are left for the next block rather than -// committed as empty s. Mutates `state`; calling again with the same -// text is a no-op (idempotent). +// Consume newly-arrived complete lines, committing a settled block at every +// "\n\n" outside a fence. Whitespace-only runs stay with the next block, never +// committed as empty s. Mutates `state`; re-calling with the same text is +// a no-op (idempotent). export const advanceScan = (text: string, state: StreamScanState) => { const start = state.scanned.length @@ -141,15 +97,11 @@ export const advanceScan = (text: string, state: StreamScanState) => { const nl = text.indexOf('\n', i) if (nl < 0) { - // Partial trailing line — could still become a fence opener. Leave it - // (and its whole block) in the unstable tail. - break + break // partial trailing line — could still open a fence; keep in tail } if (nl === i) { - // Empty line. If it's the second half of a "\n\n" pair (i.e. not the - // very first character) and no fence is open, the text before it is a - // settled top-level block. + // Second half of a "\n\n" outside any fence → prior text is a block. if (i > 0 && !state.codeOpen && !state.mathOpener) { const block = text.slice(state.settledLen, nl + 1) @@ -170,10 +122,8 @@ export const advanceScan = (text: string, state: StreamScanState) => { } } -// Compatibility shim over the incremental scanner: index just past the last -// committed block boundary in `text`, or -1 when nothing has settled yet. -// Kept for boundary-semantics tests; StreamingMd itself keeps scanner state -// across deltas instead of recomputing from scratch. +// Index just past the last committed boundary, or -1 if nothing has settled. +// Thin wrapper over the scanner for boundary-semantics tests. export const findStableBoundary = (text: string) => { const state = createScanState() @@ -187,10 +137,8 @@ export const StreamingMd = memo(function StreamingMd({ cols, compact, t, text }: let state = scanRef.current - // Reset when the text no longer extends the scanned prefix: a new turn - // reusing the mounted component, or boundedLiveRenderText trimming the - // front of a very long reply. Comparing the full scanned region (not just - // the settled blocks) guarantees the persisted fence state is still valid. + // Reset if `text` no longer extends the scanned prefix (turn reuse, or a + // front-trim), which would invalidate the persisted fence state. if (!text.startsWith(state.scanned)) { state = scanRef.current = createScanState() }