Repository navigation
Conversation
… message A streamed reply re-ran the whole Markdown-to-native conversion on every update. While a message is streaming, split its text at blank lines that close every open block, keep the previous update's converted blocks, and convert only the blocks that changed. Finished text, every other consumer and text with syntax that ties blocks together keep the whole-text conversion.
ApprovabilityVerdict: Approved at Macroscope's review found this PR approvable — This optimizes mobile streaming Markdown by reusing unchanged blocks. Finished messages and unsupported inputs retain whole-document conversion, keeping the exposure limited to transient rendering. You can add or adjust custom eligibility rules. Learn more. |
|
No actionable comments were generated in the recent review. Walkthrough
Merge Risk:
|
…streaming # Conflicts: # apps/mobile/src/features/threads/ThreadFeed.tsx # apps/mobile/src/native/EnrichedMarkdownText.tsx
|
Note Written by Merged current
No other changes. Main's whole-text conversion path is unchanged, so |
Problem
While an assistant reply streams,
MobileEnrichedMarkdownText(apps/mobile/src/native/EnrichedMarkdownText.tsx) re-runs the whole Markdown-to-native conversion over the full text on every update, because itsuseMemois keyed onprops.markdown. The thread feed always passesfileEmbedLabel, so assistant text takes the long branch: up to four remark parses per update (HTML tags and Windows link destinations, inline file-path links, file embeds, table breaks). The cost per update grows with the message, so a streamed reply costs more than linearly in its length.Host measurements, Node 26.13 (V8) and
node --jitless, a synthetic reply of headings, paragraphs with file-path spans, fenced code and lists, grown 500 characters per update; whole-stream conversion time, median of 5 alternating runs (minimum in brackets):--jitlessnow--jitlesswith changeFor scale, one conversion of the finished text (median of 11, dense in file paths): 5 KB 26 ms, 20 KB 83 ms, 50 KB 181 ms, 200 KB 877 ms on V8; 52, 238, 537 and 2397 ms under
--jitless. The other per-update passes the feed runs over the same text (citations, wide-block check,<details>split, Codex directives) cost under 0.4 ms at 200 KB, so the conversion is the part that grows.Nothing here was measured on a phone or on Hermes;
--jitlessis an interpreter-only stand-in, not Hermes. Replies of 10 KB and more are rare in chat, so most replies save little (see Verification, "Real documents").The web client fixed the same problem for its renderer (#11193, #27417,
apps/web/src/components/chat/markdownStreamBlocks.ts).Change
apps/mobile/src/lib/markdownStreamBlocks.ts(new). While a message streams and its text is at least 2048 characters,convertStreamingMarkdowncuts the text at blank lines that close every open block and converts each block on its own. It keeps the converted blocks of the previous update in a cache (keyed on block text, valid for onefileEmbedLabel) and converts only blocks that changed, normally the last one. The cache holds only the latest update's blocks and is dropped when streaming ends.null, and the caller converts the whole text as before, when the text is not streaming, is shorter than 2048 characters, or has syntax that can tie blocks together. The component's conversion is moved unchanged intoconvertNativeMarkdown, so the non-streaming path is main's code.streamingonSelectableMarkdownText. Only the assistant message passes it (message.streaming, throughAssistantMarkdownContentandAssistantMarkdownBlocks, including<details>folds). Finished messages, reasoning, user messages, file previews, pull request bodies and review comments never set it, so they never split.markdownDocument.tsgets an exportedmarkdownMayLinkFiles, an exportedmarkdownRenderParsed(the second half ofmarkdownForNativeRendering), and an optionallinkFilesargument onmarkdownWithInlineFileLinksand the internal transform. With the argument left out, behaviour is as before.]:anywhere outside a top-level fence (reference definitions, including in quotes, lists and over several lines, and footnote definitions); HTML at the start of a line, quote or list item; a line that starts with a backtick fence marker but has a backtick in its info string (a file-link rewrite can turn it into a fence); an unindented line inside an indented fence; carriage returns, U+FEFF and NUL. Only spaces and tabs count as blank, as in CommonMark.fileEmbedLabelkeeps one identity across streamed updates (React Compiler memoizes it inThreadFeed); if it ever changed per render the cache would miss and behave like main.Scope and approval
One defect (full re-conversion on every streamed update). It is not a "very small, focused fix for an obvious bug": it adds a heuristic module of about 230 lines, a prop, and touches
markdownDocument.ts. There is no triaged issue and no maintainer agreement yet. Under CONTRIBUTING.md this needs an Ideas discussion first, and this PR should not be opened before one agrees the direction. Open PRs touch the same files (markdownDocument.ts: #21278, #29355, #29657, #30021, #30280, #24735, #29739, #28093;EnrichedMarkdownText.tsx: #28523, #29657, #27984, #30021, #24735, #29739;ThreadFeed.tsx: #30021, #28093). #29657 and #30021 change which inline code becomes a file link, which is exactly the decision this change passes between blocks, so whichever lands second has to re-check it.Behaviour differences
streamingflag flips to false the text is converted once more, which costs as much as one streamed update of main.]:, line-start HTML, or the other refused syntax converts whole on every update and costs what it does today.message.streaming; a message whose flag were left true would keep the streamed conversion.Verification
Scripts, fuzz harness and raw rows: https://gist.github.com/Pyseph/fbc7b50d6a7be2d5670d5c5dbdeb1106.
Windows 11, Node 26 through
vp, scoped to the touched files.vp test run src/lib/markdownStreamBlocks.test.tsinapps/mobile: 14 tests pass. They compare every prefix of 32 hand-written documents (the shapes in the repo's markdown tests plus one case per failure found while building this: file link in a block without a path of its own, a fence opened by<br>or by a removed tag, definitions in quotes and lists and over several lines, image definitions, an HTML block inside a list item, a non-breaking space where a fence closes or a blank line ends a paragraph, U+FEFF, CRLF), and 250 generated documents at every prefix, against the whole-text conversion, through one shared cache and both branches (with and withoutfileEmbedLabel). Also: only changed blocks are converted, non-streaming and short text are never split, the cache is dropped when the owner changes, and the first path appearing late re-renders earlier blocks. Switching off the per-document file-link decision, the]:check, the rewritten-text re-split or the fence-info rule each makes tests fail. The module is new, so the tests cannot run on main.markdownHtml,markdownInlineFileLinks,markdownTableBreaks,enrichedLinkPresentation): pass.reactCompilerCoverage.test.ts -t "EnrichedMarkdownText|ThreadFeed|markdownStreamBlocks": pass.vp lint --report-unused-disable-directivesandvp fmt --checkon the six changed files: clean.tsc --noEmitinapps/mobile: no errors in the changed files (5 errors inpackages/client-runtime/src/state/review.tsin a file this change does not touch, from my scratch install).]:check 18, rewritten-text re-split 48, U+FEFF check 33,trim()for blank 12, closing fence withtrim()3, fence-info rule 1, HTML-in-list prefix 1 (needed a longer run), carriage-return check 0 (apparently redundant: a line ending in CR is never blank, so nothing is cut). 0 differences is evidence, not proof.node --jitlessin place of Hermes.Real documents (repo Markdown of 2.4 to 28 KB, 20 files spread over the size range, grown 200 characters per update, total conversion time for all 20 files): V8 4923 ms (min 3534) with main, 514 ms (min 394) with the change;
--jitless22894 ms (min 21814) and 2238 ms (min 1903). Per file the saving is small for most: median 15 ms on V8 (29 ms jitless), 90th percentile 827 ms (3965 ms), largest 2255 ms (11571 ms, a 28 KB document). Files with nothing to convert cost under 1 ms either way; 654 of the 900 updates took the block path, the rest fell back (for exampleCONTRIBUTING.mdstarts lines with<a id=...>, so it converts whole and gains nothing). Examples on V8 (main to change): 6.4 KB 172 to 28 ms, 12.9 KB 393 to 26 ms, 16.9 KB 873 to 46 ms, 28 KB 2336 to 81 ms. Repo documents are not chat replies; I have no measure of reply sizes in T3 Code mobile.Shapes that cannot gain: one 20 KB tight list or one numbered list with continuation paragraphs and indented fences never splits, because the list is a single block. 14 alternating runs each. V8: tight list 735 to 647 ms (min 458 to 486), numbered steps 488 to 496 ms (min 369 to 382).
--jitless: tight list 2621 to 3234 ms (min 2183 to 2351), numbered steps 1997 to 2042 ms (min 1498 to 1555). The extra cost is the failed split (0.04 to 0.5 ms per update at 20 KB) and the one extra whole conversion when streaming ends; run-to-run spread under--jitlesswas about 30% on this machine, so I cannot rule out a few percent of slowdown in those shapes.Not checked
message.streamingon every path that ends a reply (interrupt, failure, disconnect).Claude Sonnet 5.5 in T3 Code (Claude Code harness).