Studio: stop later code spans changing whether an earlier one is escaped by danielhanchen · Pull Request #9028 · unslothai/unsloth · GitHub
Skip to content

Studio: stop later code spans changing whether an earlier one is escaped - #9028

Merged
danielhanchen merged 2 commits into
mainfrom
fix-latex-code-regions
Aug 17, 2026
Merged

Studio: stop later code spans changing whether an earlier one is escaped#9028
danielhanchen merged 2 commits into
mainfrom
fix-latex-code-regions

Conversation

@danielhanchen

Copy link
Copy Markdown
Member

Problem

findCodeBlockRegions in studio/frontend/src/lib/latex.ts feeds isInRegion,
which binary-searches and therefore requires sorted, non-overlapping spans.
It does not always produce them.

The inline-code pass drops a span that sits inside a fence, but an inline span
can also contain a fence pair:

`~~~a~~~ $5`

The fenced scan matches ~~~a~~~ at [1, 8), the inline scan matches the whole
`~~~a~~~ $5` at [0, 12), and both are kept. The search then descends into
the inner span and misses the outer one, so whether a position is reported as
"inside code" depends on how many spans the rest of the reply happens to add.

The visible effect is that a finished code span gets rewritten by text that
arrives after it:

preprocessLaTeX("`~~~a~~~ $5`");        // "`~~~a~~~ $5`"      correct
preprocessLaTeX("`~~~a~~~ $5`\n\n`x`"); // "`~~~a~~~ \$5`\n\n`x`"  wrong

That contradicts the contract stated in the file's own doc comment, "Currency
inside code blocks/spans is untouched". The same lookup guards
convertLatexDelimiters, so a code sample showing \(x\) can also be turned
into real math once another code span appears later in the reply.

Because replies stream, "text that arrives after it" is the normal case: a code
span can render correctly and then change as generation continues.

The change

Collect the fenced and inline spans into two ascending lists and merge them into
their union, which is sorted and non-overlapping by construction. For input that
was already non-overlapping the merge reproduces the previous list exactly, so
the only behaviour that moves is the broken case. The merge also replaces the
sort.

Testing

npm test (2,940 pass), npm run typecheck, npm run build and npx eslint on
both changed files are clean. npm run biome:check reports the same 5 errors and
16 warnings for latex.ts as it does on main, so this adds none; biome is
already non-blocking in studio-frontend-ci.yml.

Differential fuzz

200,000 generated documents built from fragments mixing inline math, display
math, currency, bold-wrapped math, inline code, fenced code, tilde fences, links
with parentheses in the destination, tables, quotes and escaped backslashes, each
compared against the previous implementation:

documents output changed explained by overlapping spans unexplained
200,000 6,004 6,004 0

Every difference is a case where the old code emitted overlapping spans. Nothing
else moved.

New tests, and what each one catches

studio/frontend/tests/latex-code-regions.test.ts. There was previously no test
file for latex.ts at all; its only coverage was indirect, through the streaming
schedule tests.

Run against the unmodified tree, three of the four fail and one passes:

  • currency inside inline code survives unrelated later code spans - fails
    on main with `~~~a~~~ \$5`\n\n`x` against the expected
    `~~~a~~~ $5`\n\n`x`. This is the reported defect.
  • a code span's own text decides its escaping, whatever follows it - fails
    on main. Sweeps seven code-span shapes against one to four trailing spans,
    so it pins the general property rather than the one input, and would catch a
    fix that special-cased ~~~ inside backticks.
  • LaTeX inside inline code stays literal whatever follows it - fails on
    main. Covers the delimiter-conversion path, which uses the same lookup and
    would otherwise turn a code sample into a rendered formula.
  • ordinary code spans and fences are unchanged - passes on main as
    well.
    It is the regression guard for the merge itself, not a defect
    detector: it pins ten already-correct shapes so that a future change to the
    merge cannot move them. Stating this explicitly because a test that passes
    both ways proves nothing about the bug.

`findCodeBlockRegions` feeds `isInRegion`, which binary-searches and so needs
sorted, non-overlapping spans. The inline-code pass dropped a span nested inside
a fence, but an inline span can also contain one, as in `` `~~~a~~~ $5` ``. That
pushed two overlapping spans, and the search then descended into the inner one
and missed the outer, so the lookup answered according to how many spans the
rest of the reply happened to add.

The result was that a completed code span could be rewritten by text that
arrived after it:

    preprocessLaTeX("`~~~a~~~ $5`")          -> "`~~~a~~~ $5`"
    preprocessLaTeX("`~~~a~~~ $5`\n\n`x`")   -> "`~~~a~~~ \$5`\n\n`x`"

which contradicts the documented contract that currency inside code blocks and
spans is untouched. The same lookup guards the delimiter conversion, so a code
sample showing `\(x\)` could also be turned into real math.

Merge the two ascending lists into their union instead. For input that was
already non-overlapping the merge reproduces the previous list exactly, and it
replaces the sort as well.

Differential fuzz against the previous implementation over 200,000 generated
documents mixing inline math, display math, currency, fences, tilde fences,
links and escaped backslashes: 6,004 documents changed, and every one of them
had overlapping spans under the old code. Nothing else moved.
@danielhanchen

Copy link
Copy Markdown
Member Author

Verification round

Independent replication of the claims in the description, plus the surface the
description does not cover.

The defect is real, and narrower than the description implies

Instrumenting findCodeBlockRegions on the merge base:

input regions on main regions here
`~~~a~~~ $5` [[0,12],[1,8]] [[0,12]]
`~~~a~~~ $5`\n\n`x` [[0,12],[1,8],[14,17]] [[0,12],[14,17]]

With two entries the search's first probe is index 0 and lands on the outer span;
with three it probes [1,8], steps right on 9 >= 8, and never visits index 0.

A start-sorted list only mis-steers a binary search when one span contains
another; a partial overlap keeps each span's exclusive part on the correct side of
the pivot. In this function containment can only be an inline span containing a
tilde fence, because /`[^`\n]+`/ cannot span a backtick. That is worth
saying out loud, because it means the reachable input is a single inline code span
holding a complete ~~~...~~~ pair with a $<digits> or a \( after the inner
closer but still inside the span, and the right number of other spans after it. It
also flips on the count of trailing spans rather than monotonically: `~~~a~~~ $5`
differs at 1, 2, 5, 6, 7 and 8 trailing spans, and not at 0, 3 or 4.

Over 20,000 generated realistic-sentence replies, the region list overlapped in 1,552
of them and the output differed in none. I could not build a realistic model reply
that hits this. It is still worth merging: it removes a real invariant violation, it
can only ever protect more code and never less, and it is a landmine for whoever adds
a third region source to this function.

Differential against the merge base

Every difference classified, on the full document and on every streaming prefix:

corpus cases output changed explained by an overlapping span on main unexplained
12,067 streamed replies, 4 frame sizes 12,067 922 922 0
200,000 generated documents x 3 seeds, plus prefixes 600,000 5,128 / 5,218 / 5,201 all 0
1,500,000 hostile random strings, 5 alphabets - 19 19 0

Three difference classes, all fixes: currency inside such a code span; \(x\) inside
one, which reaches the delimiter-conversion path and turns a code sample into a
rendered formula; and the streaming-prefix forms of both. There is no fourth class.

The strongest check is a third build: the merge base's source with isInRegion
replaced by a linear scan, so it has main's exact region semantics with a correct
membership test. This branch agrees with that oracle on 24.9M prefix evaluations and
1.5M random strings, 0 mismatches
, which makes every difference provably "the binary
search returned a false negative" rather than a semantic change. Direction of error
over 5,091,155 membership probes: main gives 8 false negatives and 0 false positives,
this branch 0 and 0.

CommonMark

Per the 0.31.2 spec, block structure is resolved before inline structure (section 3.1)
and a code fence is a line-level construct (section 4.5), so a real parser can never
produce a genuinely overlapping code span and fenced block. Checked against micromark
with the GFM extension, which is what this app renders with: on every input tested,
this branch's merged coverage is byte-identical to main's raw coverage, so the merge
cannot move CommonMark fidelity in either direction. Where the heuristic over-covers,
it already over-covered.

Streaming monotonicity

preprocessLaTeX is not prefix-stable, which is the subject of #9017. This branch
makes it strictly less unstable and never more: over 1,398,087 prefix steps, main
violates on 107,296 and this branch on 106,142, with 1,154 removed and 0 added.

Cross engine and cross checks

  • The suite bundled and run in Chromium, Firefox and WebKit agrees case for case with
    Node and with itself, and the same 20 cases differ from main in all three. Playwright
    WebKit is a proxy for the webviews Desktop embeds, not those webviews.
  • mergeRegions properties over 900,000 random list pairs: output sorted,
    non-overlapping, coverage equal to the union, inputs not mutated, adjacent spans not
    fused. Its precondition is ascending by start; internal self-overlap is tolerated,
    but a non-ascending input silently drops regions, so that precondition is worth stating
    precisely in the doc comment since nothing enforces it. Both producers are provably
    ascending and internally disjoint: the regexes are declared inside the function so
    lastIndex is fresh, and neither can match empty.
  • Performance, interleaved, median of 5: code-heavy 100 KB 0.94x of main, 10,000 inline
    spans 0.85x, realistic 2 KB to 32 KB replies 0.93x to 1.00x. One worst case, a 129 KB
    document with 10,000 inline spans and no fenced block at all, is 1.08x to 1.13x,
    about +0.1 ms, because main's sort sees a single sorted run there while the merge
    always allocates. Not worth a fast path.
  • npm test 2,940 pass, npm run typecheck, npm run build and npx eslint clean.
  • The four new tests: three fail on main, one passes on both and says so in its own
    comment. Assertion level, 44 assertions and 7 fail on main, all of them inside
    a code span's own text decides its escaping, whatever follows it, which subsumes the
    other two detectors. That test sweeps 1 to 4 trailing spans and only 1 and 2 flip
    today, so narrowing the sweep later would silently disarm it.

Interaction with #9017

They are independent. #9017 changes only IncrementalMarkdownCache, this changes only
latex.ts; they merge cleanly. Streamed over the same 12,067 replies: this branch
differs from main in 922 cases, #9017 in 0, and the two merged differ in exactly the
same 922, with 0 difference between the merged tree and this branch alone. Neither is
ordered on the other.

@danielhanchen

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Keep them coming!

Reviewed commit: faa338862c

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

@danielhanchen

Copy link
Copy Markdown
Member Author

Before/after, from two isolated installs

Two Studios built with install.sh --local, one from this PR's merge base
c87fe20e3 and one from the head faa338862, each with its own
UNSLOTH_STUDIO_HOME. One reply, delivered to both as byte-identical synthetic SSE
through a fetch shim, so the two sides render the same characters and the only
variable left is the branch. Model unsloth/Qwen3.5-2B-MTP-GGUF:UD-Q4_K_XL at 4,096
context on both sides, CPU throttled 6x on both sides.

The reply contains the shape from the description: two inline code spans that each
wrap a ~~~...~~~ pair, one trailing code span, and ordinary math and prices around
them.

before and after

Read the last two lines of the reply. The model sent

The token `~~~a~~~ $5` marks a price inside code.
The token `~~~b~~~ \(x\)` marks math inside code.
BEFORE (merge base) AFTER (this PR)
first span ~~~a~~~ \$5 ~~~a~~~ $5
second span ~~~b~~~ $x$ ~~~b~~~ \(x\)

Both are visible because a backslash inside a code span is literal, so the escape the
preprocessor added shows on screen, and the delimiter conversion rewrote a code sample
into something the model never wrote.

Read from the DOM of the same servers that were photographed, so the picture and the
claim cannot disagree:

code_spans:       ["run --seed $1 --limit $2", "~~~a~~~ \\$5", "~~~b~~~ $x$", "x"]
             ->   ["run --seed $1 --limit $2", "~~~a~~~ $5",   "~~~b~~~ \\(x\\)", "x"]
currency_visible: ["$1,200", "$250", "$5", "\\$5"]  ->  ["$1,200", "$250", "$5"]
math_nodes:       ["r_i = y_i - \\hat{y}_i", "L(\\theta) = \\sum_i (y_i - \\theta x_i)^2"]
             ->   unchanged

Everything else on screen is unchanged: the same two KaTeX nodes, the same
$1,200 and $250 in the prose, the same shell fence.

The picture shows that the two differ; the character-level differential in the
verification comment above says which is right.

@danielhanchen
danielhanchen merged commit 8b033a1 into main Aug 17, 2026
40 checks passed
@danielhanchen
danielhanchen deleted the fix-latex-code-regions branch August 17, 2026 10:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant