Repository navigation
🤖 fix: keep a loose list whole in chunked streaming rows - #5668
Merged
Merged
Conversation
Do not let the last block (or the blank lines before it) start a new chunk, so a partial list marker cannot seal the list. Remount a chunk when it is sealed and when the stream ends, so Streamdown's position-keyed memo cannot keep a stale first list item. Refs #5664
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d1bbee09c7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
A closed oversized block followed by a growing paragraph is sealed again, so the paragraph renders synchronously (Codex review).
This was referenced Oct 5, 2026
yermakoffivan
pushed a commit
to yermakoffivan/mux
that referenced
this pull request
Oct 6, 2026
…er#5705) ## Summary Partial fix for one case of coder#5666: a huge **list** in a chunked switch-back row. The chunker now cuts that list at its top-level item starts into chunks of about 2,000 chars. When you switch back to a workspace that streams a long list, the newest items appear about 50 ms after the click instead of after about 5 s, and the list keeps growing without multi-second stalls. Tables, fresh rows and paragraphs are not changed. Refs coder#5666 ## Background Rows longer than 20k chars that mount mid-stream render through `ChunkedStreamingMarkdown` (coder#5647). Its chunker groups whole Streamdown blocks, so one huge list stayed one open chunk. That chunk was larger than the 20k synchronous-render cap, so it used Streamdown's deferred render. While the stream was live, that render was starved, so the row stayed empty for about 5 s and then grew in multi-second steps. ## Implementation 1. `listItemRanges(block, maxChars)` (new `streamingMarkdownBlocks.ts`) lexes one list block with `marked` and groups its top-level items into ranges that join back to the block. - `marked` is a new direct dependency, pinned exactly to 17.0.1, the version Streamdown's block splitter uses (`bun.lock`: `streamdown/marked`). Streamdown's only public lexer, `parseMarkdownIntoBlocks`, returns a whole list as one block, so it cannot give item bounds. mermaid keeps its own nested `marked@16.4.2`. - Item bounds come from the lexer, so a nested list, an indented fence, a lazy continuation line and an HTML block stay inside their item. - An item longer than a range is a range on its own. There is no cut inside an item. - If the block is not exactly one list, or the lexer's raw text does not map back to the block, the block stays whole, as before. - A list that holds a reference definition (marked's `links`) or a footnote (`[^x]`) also stays whole. Those resolve across the whole document, so a cut could put a use and its definition in different chunks. 2. `MarkdownChunker` gives each range of a cut list its own chunk. A range is sealed once a later item follows it, like any other block, and later frames lex only the text after the sealed prefix. 3. `completedChunks()` joins each cut list back into one chunk. The row uses it once the stream ends, so the completed row renders each list as one list, as a single render does. 4. Chunk keys are now the chunk's source offset and length (the open chunk keeps the `open-` prefix). A joined list gets a new key and remounts. It must not reuse its first range's key: Streamdown memoizes list items by source position, so a range that was tight would keep its stale items in a list that is loose as a whole (the coder#5664 bug). 5. The row tracks its oldest mounted chunk by source offset, not by index. Completion can join a cut list while older chunks are still mounting, and every chunk after the list then moves to a lower index. The row now mounts from the chunk that contains that offset, so an offset inside the joined list keeps the whole list mounted. ## Visible changes While the row streams: - **List margins at the cuts.** Each range renders as its own `<ul>`/`<ol>`, so a list margin shows at every cut. - **Ordered numbers.** An ordered range takes its `start` from its own first marker. Sequentially numbered lists keep their numbers. A list that repeats one marker (`1.` `1.` `1.`) restarts at 1 at each cut while streaming. Completion merges it into one correctly numbered list (test: `repeated-marker`). - **Looseness per range.** A range whose items have no blank line between them renders tight, even if the list as a whole is loose. Completion renders it as one list, as a single render does. At completion: - **DOM-state loss.** The cut list remounts as one list. A text selection inside it clears, and a "copied" state on a code block inside it resets. Chunks outside the cut list keep their DOM nodes. - **Completion cost.** The end-of-stream commit renders the joined list once: 103–111 ms at 30k and 157–166 ms at 50k (main: 77–104 ms and 156–198 ms). One more task follows: 87–92 ms at 30k and 129–136 ms at 50k (main: 104–152 ms and 129–171 ms). Completion cost is out of scope for this PR. Out of scope: tables, fresh rows (rows that mount at stream start), paragraphs and the completion stall. ## Validation Tests (each new rendering and chunker test fails on main): - `streamingMarkdownBlocks.test.ts` - `listItemRanges > %s: ranges join back to every prefix and never split an item` (`bullet`, `ordered` (numbered from 7), `loose`, `task`, `nested`, `fence` (holds a `- x` line), `lazy`, `html`) - `keeps an item longer than a range whole, as a range of its own` - `returns null for blocks that are not one list, or whose text the lexer normalizes` - `ChunkedStreamingMarkdown.test.tsx` - `cuts a huge streaming list at item starts, so the open chunk stays small` (warm chunker over prefixes = cold chunker; completed = one chunk) - `completed chunks join only the cut list and keep the other chunks` - `a huge list that mounts mid-stream shows its newest items in the first commit` - `ordered ranges keep their numbers while the list streams` - `a %s list cut while streaming renders as one list like a single render once complete` (bullet, ordered from 7, loose, nested, task, repeated-marker): one top-level list, same `ol/ul/li/li > p/input[checked]` outline and `start` as a single render, same text - `completion remounts the cut list but keeps the other chunks' DOM` - `completion during the backfill, with the oldest mounted chunk %s, unmounts nothing` (`after the cut list`, `inside the cut list`): the stream ends between backfill frames; every chunk that was visible stays visible, and the sealed chunks after the list keep their DOM nodes. Both cases fail on `48dd9359fd`. - The coder#5668 loose-list tests are unchanged and pass. ### Fixture gate (production build, switch-back list, 5 runs each) Phases follow the plan's definitions: - **cold:** from the click to the first visible text, including the mount commit; - **backfill:** the first 500 ms after the first text; - **incremental:** from there to the server's stream end; - **completion:** tasks that start at or after the server's stream end. "Before" is main `9373461eaa` (post-coder#5669, before coder#5702, which touched only highlighting). "After" is this head (`c1a58ac305`). | list | build | cold: first text p50 / p95 | cold: tasks >50 ms before first text, per run (max) | backfill: tasks >50 ms per run | incremental: longest gap p50 / max | incremental: tasks >50 ms per run | |---|---|---:|---|---|---:|---| | 30k | main | 5178 / 5187 ms | 50, 46, 44, 47, 43 (90 ms) | 4, 4, 3, 3, 3 | 4619 / 5208 ms | 49, 47, 42, 41, 36 | | 30k | this PR | **49 / 57 ms** | 1, 0, 0, 0, 0 (57 ms) | **0 in every run** | **153 / 174 ms** | **0 in every run** | | 50k | main | 5411 / 5707 ms | 14, 17, 8, 18, 16 (230 ms) | 1, 0, 1, 0, 0 | 7419 / 7601 ms | 2, 0, 2, 0, 1 | | 50k | this PR | **53 / 54 ms** | 1, 1, 1, 1, 0 (53 ms) | **0 in every run** | **202 / 212 ms** | **0 in every run** | The stream is not free of long tasks: - The switch-back mount commit is a 51–60 ms task in some runs (in this run: 1 of 5 at 30k, max 57 ms, and 4 of 5 at 50k, max 53 ms; in earlier runs of this branch, up to 4 of 5 at 50k, 51–64 ms). It ends before the first text appears, so it counts in the cold phase. - Part of that commit is a second lex: Streamdown's block splitter lexes the text and returns only strings, so `listItemRanges` lexes the list block again. In V8, the first `MarkdownChunker.update` on a 41k list takes 12.0 ms (10.4–14.4) on this branch and 6.5 ms (6.4–6.9) on main. Reusing the splitter's tokens would mean reimplementing it, so this PR keeps the second lex. - The completion tasks are listed under "Visible changes". Other fixture checks: - coder#5668 loose-list fixture (2 runs): 1 `<ol>` in both the chunked and the single render, 319/319 items with a `<p>`, 0 outline and 0 text diff lines. - Dense 60k fresh row, alternating main and this branch, 3 runs each (on `85c4fbcfd2`): completion longest task 240–256 ms on main and 239–245 ms here. Both rendered 97.4–97.5% while live, with an 83–84 ms longest gap and 0 live tasks. On this head (3 runs, not alternated): 97.5%, 77–84 ms longest gap, 0 live tasks, completion 257–259 ms. ### Bench: `MarkdownChunker.update`, last 120 frames (+30 chars each) of a 50k list, one chunker | runtime | method | main `3469017c97` | this PR | delta | |---|---|---:|---:|---:| | Node 22 (V8), primary | scratch harness, separate installs, 10 alternating rounds | 6.71 ms/frame (6.47–6.90) | 0.28 ms/frame (0.27–0.37) | −95.8% | | Bun, secondary | `make bench-compare BENCH=ChunkedStreamingMarkdown RUNTIME=bun ROUNDS=10` | 148.04 ms | 1.25 ms | −99.2% (95% CI −99.2% to −99.1%) | `make bench RUNTIME=node` cannot load this bench: the import chain reaches `MarkdownCore`, which imports KaTeX's `.css`, and node stops with `ERR_UNKNOWN_FILE_EXTENSION`. The bench runner is unchanged. The V8 row comes from a scratch harness that bundles the same loop with esbuild. Its CSS stub lives only in the harness. <details> <summary>V8 harness: commands, source and evidence path</summary> Each tree (main in a separate clone with its own `bun install`, and this branch) is bundled and run alternately: ```bash bun nodebench/build.ts <tree> <tree>/build/nodebench/b.mjs for i in $(seq 1 10); do for t in <main-tree> <pr-tree>; do node --expose-gc $t/build/nodebench/b.mjs 5; done; done ``` Evidence (on the measurement host): `$XUM_SCRATCH_DIR/nodebench/final/` (`per-frame-rounds.txt`, `cold-update-rounds.txt`, `bench-compare-bun.log`, and the harness sources). `build.ts`: ```ts import { build } from "esbuild"; import * as path from "node:path"; const [root, out] = process.argv.slice(2); await build({ absWorkingDir: root, entryPoints: [path.join(process.env.XUM_SCRATCH_DIR!, "nodebench/entry.ts")], outfile: out, bundle: true, platform: "node", format: "esm", target: "node22", tsconfig: path.join(root, "tsconfig.json"), packages: "external", logLevel: "error", nodePaths: [path.join(root, "node_modules")], plugins: [{ name: "css-stub", setup(b) { b.onResolve({ filter: /\.css$/ }, (a) => ({ path: a.path, namespace: "css-stub" })); b.onLoad({ filter: /.*/, namespace: "css-stub" }, () => ({ contents: "", loader: "js" })); } }], }); ``` `entry.ts`: ```ts // Node (V8) per-frame cost of MarkdownChunker.update over the last 120 frames of a 50k list. import { MarkdownChunker } from "@/browser/features/Messages/ChunkedStreamingMarkdown"; import { CHUNKED_STREAMING_CHUNK_CHARS } from "@/constants/streaming"; const FRAMES = 120; let list = ""; for (let k = 1; list.length < 50_000; k++) list += `- item ${k} with **bold ${k}** and *em* and \`c${k}\` [l](http://x/${k})\n`; const frames = Array.from({ length: FRAMES }, (_, i) => list.slice(0, list.length - (FRAMES - 1 - i) * 30)); const passes = Number(process.argv[2] ?? 5); const perPass: number[] = []; for (let p = 0; p < passes + 1; p++) { const chunker = new MarkdownChunker(CHUNKED_STREAMING_CHUNK_CHARS); const t0 = performance.now(); let sink = 0; for (const f of frames) sink += chunker.update(f).length; const dt = (performance.now() - t0) / FRAMES; if (p > 0) perPass.push(dt); // first pass is warmup if (sink < 0) console.log(sink); } perPass.sort((a, b) => a - b); console.log(JSON.stringify({ medianMsPerFrame: perPass[Math.floor(perPass.length / 2)], min: perPass[0], max: perPass.at(-1) })); ``` </details> ## Risks Medium-low. The change affects only rows longer than 20k chars that mounted mid-stream (switch-back), and only lists longer than 2,000 chars in them. Completed rows render whole-block chunks as before. A lexer disagreement between `marked` and remark on item bounds would show only while streaming, because completion joins the list and renders it once. --- _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `high` • Cost: `$123.22`_ <!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=high costs=123.22 -->
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A chunked streaming row (#5663) could split one loose list into several lists, and the split stayed after the reply completed. This PR keeps the list whole and makes the completed row render the same structure as a single static render.
Refs #5664
Background
The #5664/#5666 measurement found it: a 30k loose ordered list that mounted chunked rendered as 4
<ol>(start=253/277/301) after completion, and one item lost its<p>. A reload (single render) showed 1<ol>.Two causes:
…\n\n301before the.arrives.parseMarkdownIntoBlocksthen returns[list, "\n\n", "301"]. The chunker started a new group at the blank-line or301block, sealed the list, and301.then started a second list in the open chunk.<p>. The unchunked row is correct after completion, but a chunked row stays in static mode and never remounts, so the stale item stays.Implementation
MarkdownChunker: a tail that is only blank lines or bare digits (a partial ordered-list marker before its.) never starts a group, together with the blank lines before it. Every other last block still seals the block before it, as on main.ChunkedStreamingMarkdown: while streaming, the open chunk is keyedopen-<index>. It mounts fresh when it is sealed and when the stream ends. A sealed chunk keeps its key, soMarkdownCore's memo still skips it.Cost: each seal does one fresh render of the sealed chunk (about 2,000 chars), and completion does one fresh render of the open chunk. Before #5663, completion re-rendered the whole reply.
Validation
New test
ChunkedStreamingMarkdown > a loose %s list streamed across chunks renders like one static render(ordered and bullet). It streams 60 items, with one frame per partial marker (\n\n3,\n\n30,\n\n-), then comparesol/ul/li/li > pstructure and text with a singleMarkdownCorerender. Both cases fail on main (ordered: 38 outline entries differ, bullet: 2) and pass here.Local browser harness (production build, loopback mock provider, switch back at 22k emitted chars, 30k loose ordered list, then compare with a reload):
<ol>count31616698f2, 2 runsc0f8c67ad1), 2 runsA reference-link and footnote reply stays identical to the single render (0 lines differ).
React Compiler:
ChunkedStreamingMarkdownhas the same single bailout (??=on the ref) before and after.Not in this PR
[^…]inside code). A correct fix needs code-span and escape parsing or a block re-parse on every completed render. A wrong strip would hide a real footnote, which is worse than today's extra render. It stays on 🤖 Chunked streaming rows: bound oversized blocks without a transition, narrow document-scoped detection #5664.Risks
Low. The change affects only rows that mount chunked mid-stream (above 20k chars). The worst case is one extra synchronous render of the open chunk at completion. For a trailing block close to 20k chars, that render can take more than 50 ms.
Generated with
xum• Model:anthropic:claude-opus-5-5• Thinking:high• Cost:$33.41