Skip to content

Review 5407

Cindy Zhang edited this page Aug 24, 2026 · 1 revision

Review 5407 — the streaming Markdown cache stops collapsing

#5407 · cixzhang · internal (written by this loop, so the bar goes up — R23) Head reviewed 1b0fab89661 (gate 1) → 9dd1d2de161 (gate 2) → c43b16d61ba (gate 3) Verdict gate 1 request-changes · gate 2 approve, one nit · gate 3 merge — nothing posted, three self-gates Outcome merged 2026-08-24 as cbb991e91e6, squash, closing #5378

Problem

A chat or docs surface streaming a Markdown answer pays parse work that grows with how much of the answer has already arrived. parseMarkdownIncremental re-parses only the unsettled tail — except on two paths that threw the settled cache away. Streaming the perf fixture at 50-char chunks, blocks rebuilt by object identity: 1,513 at 50 paragraphs, 126,756 at 500, 497,878 at 1000. Twenty times the document, 329 times the work. Not a dropped frame — aggregate main-thread CPU that keeps growing while the answer streams.

Solution

One decision. The settled boundary may never move backwards, and both defects moved it backwards: a blank last line settled a line the next chunk took back, and an open code fence collapsed the boundary to -1. Neither half alone reaches the invariant (worst chunk at 50/200/500: fence half only 3/215/565, blank-line half only 59/234/584, both 3/3/3), so they shipped together, with structural trimming skipped while a fence is open — without that a streamed TypeScript union blanks its own code block.

Impact

Everyone on the next release, while a Markdown answer streams. Blocks rebuilt per chunk goes flat at 1.4 for any document length; wall clock is roughly 1.5–3x less, and moves that much between identical runs on one laptop, which is why the assertion counts blocks. Two behaviour changes rode along, both toward the one-shot parse: the streamed final AST now equals parseMarkdown across 168 document × chunk-size × option combinations (main differed on 8 — a list came out loose), and an open fence's tail is whitespace-trimmed like every other tail. Not fixed, and named in the body: every chunk still re-scans the whole input four ways — #5406.

API

No API change. findSettledBoundary is module-private; its return went from number to {boundary, openFence}. Nothing reachable from the barrel moved.

Theme targets

No new theme targets. Nothing in the diff renders.

Ossification

n/a — no API, no theme targets.

Breaking

  • API — no. packages/core is published and has real consumers; none can see this.
  • Visual — no in the final state, verified as AST equality across 168 combinations rather than asserted; two transient mid-stream deltas, both improvements (a table header no longer flashes as raw text for one chunk; no half-typed indent at the bottom of a streaming code block).
  • Theme — no.

Judgement

The assertion moved with the fix, which was the point of the exercise: #5379's p95 bound was chosen because stronger ones were red on main, and it is replaced by the worst chunk — 3/3/3 at 50/200/500 paragraphs, on the default fenced fixture. Watched fail three ways; the row that matters is the blank-line-only break, which passes the median, the fence-free p95 and the fenced p95 and fails only the new bound.

What the gates changed before merge

This is the part worth keeping.

  1. Gate 1, blocking. The test written to pin the in-fence trimming skip passed with the skip deleted — the fixture ended on - still code, which trimUnsettledStructural treats as complete, so nothing was ever trimmed. Fixed by ending it on the union line; now content === '' without the guard. A test nobody had watched fail.
  2. Gate 1, non-blocking. The body's wall-clock pair (4617.8ms → 1190.0ms) did not reproduce, and "janky" was unsupported — no chunk on main is anywhere near a dropped frame. Both rewritten. Gate 2 then contradicted gate 1's counter-number with a fourth non-overlapping band, which settled the real finding: on this machine wall clock is not evidence at this resolution.
  3. Gate 1, non-blocking. The break table's p95 column read "—" where the measured answer is "pass" — understating the case. Filled in for all three broken parsers, fence-free and fenced.
  4. Gate 2. The test comment quoted the issue's "2% / 31%" estimate and called both invisible to p95. Measured 1.4% and 24.5%, and the 24.5% one does fail a fenced p95. The comment now states only what was measured here.
  5. Gate 3. Code clean; found pr-a11y red on a story this diff cannot touch — filed as #5411 after the same commit went red, red, green.

Three passes, five corrections, none of them to the mechanism — which two passes independently called right.

Clone this wiki locally