Studio: give a CRLF reply the same incremental rendering as an LF one - #9088
Conversation
IncrementalMarkdownCache measures every boundary as an offset into the text it is handed, but compares against blocks Streamdown returns with their line endings already normalised. On a CRLF reply the two disagree one block in: findCommitBoundary sees "para" where the source holds "para\r", the exact prefix check fails at the first separator, nothing is ever committed, and the sticky full-document path repairs and lexes the whole reply on every frame. Normalise the line endings on the way in. CommonMark counts a line feed, a lone carriage return, and a carriage return followed by a line feed as the same line ending and reference parsers normalise before parsing, so this cannot change what is rendered; a lone carriage return is included because a CRLF pair can be split across two frames. Measured, paired in one process, 24 character frames: a 34,080 character CRLF reply goes from 20,177 ms to 701 ms, against 521 ms for the same reply in LF. Retention over a 454 reply corpus goes from 281 characters to 7,591, which is exactly what the LF corpus retains. A reply with no carriage return in it takes the same path it took before, byte for byte.
|
@codex review |
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
The comment named indexOf; the code calls includes. Replace the primitive with the measurement it was standing in for: 0.4 us per frame on an 88,000 character reply.
|
@codex review |
Independent re-review: no change needed to the fixRe-reviewed against the strict bar. No genuine bug found; one comment corrected in What was checked beyond the description:
One thing this surfaced that is not in scope hereCross-platform CI on a staging repo runs the frontend suite on Windows, which the |

Problem
IncrementalMarkdownCachemeasures every boundary as an offset into the text it ishanded. It compares those offsets against the blocks
parseMarkdownIntoBlocksreturns, and Streamdown hands those back with their line endings already
normalised. On a CRLF reply the two disagree one block in:
findCommitBoundarymatches block 0 at offset 0, advances to 8, then asks whetherthe tail continues
"\n\n"where it actually holds"\r\n\r\n". The check fails,repairBrokeis set, andupdatetakes the sticky full-document path. Fromthat point the whole reply is repaired and lexed on every frame, for the rest of
the reply.
A user whose provider or platform emits CRLF gets no incremental rendering at all.
It is not only a cost bug. Streamed at every prefix over a 454 reply corpus, a
CRLF reply's block list already differs from the same reply in LF in 11 of 454
replies on
main, and the block count differs in one. After this change it does notdiffer in any. So
mainrenders some CRLF replies into a different block list thanthe identical LF reply, which is a correctness difference, not a performance trade.
The rest of the difference is time, and nothing in the output says so, which is why
this has survived.
The change
Normalise the line endings at the entry to the cache.
Per the CommonMark spec, "a line ending is a
line feed (U+000A), a carriage return (U+000D) not followed by a line feed, or a
carriage return and a following line feed", and reference parsers normalise to LF
before parsing, so this cannot change what is rendered. A lone carriage return
is included deliberately: a CRLF pair can be split across two frames, and treating
only the pair would leave that CR in place for one frame and delete it the next,
which is one more retro-edit for the cache to absorb.
The scan is a native
includes, and the replace runs only for a reply that actuallycarries a carriage return.
Measurement
Paired A/B in a single process, both versions of the cache imported side by side and
run alternately so host load lands on both arms, 24 character frames, median of 5
paired ratios.
mainThe ratio grows with the reply because the discarded work is the whole reply each
time. After the change a CRLF reply sits within 1.35x of the same reply in LF; the
gap left is the per-frame preprocessing, not the cache.
Retention over a 454 reply corpus streamed at every prefix:
mainmainExact parity with the LF corpus.
Rendered output is unchanged
454 replies streamed at every prefix in both line endings, comparing the mdast text
and math nodes of the whole document, of every block on its own, and the block count:
mainmaintodayRow 1 is the guarantee for the common case: an LF reply is not touched.
Row 2 is the goal, reached: a CRLF reply now renders as the identical block list to
the same reply in LF. The
documentTextanddocumentMathcolumns in that row arenot this cache: that fingerprint is a direct whole-document parse of the CRLF
text, bypassing the cache entirely, and row 3 shows the same 1 and 85 on
main.That is a pre-existing property of parsing CRLF markdown with micromark, unchanged
here in either direction.
Row 3 is the correctness point above, in the table: on
maina CRLF reply's blocklist diverges from the LF one in 11 replies and the block count in 1. Row 2 shows
both at 0.
The oracle, the cache's block list against a whole-document split at every prefix of
353 CRLF replies: 0 mismatches, comparing against a parse of the normalised text,
which is what a CommonMark parser sees.
Tests
studio/frontend/tests/streaming-crlf-retention.test.ts, three tests. Being explicitabout which are detectors, since a test that passes on the broken tree proves nothing
about the bug:
a CRLF reply retains as much as the same reply in LF- fails onmainwith
CRLF retained 0 characters against LF's 10518. This is the defect. It alsopins the block list, so retention that returned different blocks would not pass.
a CRLF reply matches a whole-document split at every prefix- passes onmainas well. It is the output guard, including a reply ending in a lone carriage
return, not a defect detector.
an LF reply is untouched by the line-ending handling- passes onmainaswell, by design: it is the regression guard for the common path.
npm test(3,590 pass),npm run typecheck,npm run buildandnpx eslintonboth changed files are clean.
Scope
Found while verifying #9017, and deliberately not folded into it: it is a separate,
pre-existing defect that predates that PR, and #9017's own Codex round raised a CRLF
issue in the blank-line scan whose stated CPU consequence turned out to be this,
rather than anything #9017 introduced.