Skip to content

fix(markdown): prevent currency values from being parsed as inline math - #463

Merged
xintaofei merged 4 commits into
xintaofei:mainfrom
Adam-Dalloul:fix/currency-parsed-as-inline-math
Aug 17, 2026
Merged

fix(markdown): prevent currency values from being parsed as inline math#463
xintaofei merged 4 commits into
xintaofei:mainfrom
Adam-Dalloul:fix/currency-parsed-as-inline-math

Conversation

@Adam-Dalloul

@Adam-Dalloul Adam-Dalloul commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Single $...$ was being treated as math, so prices and shell vars like $HOME looked broken.

This turns singleDollarTextMath off. Real math still works via $$...$$ and \(...\).

Follow-up after review:

  • multi-line \(...\) at the start of a block no longer drops the first line
  • parse tests for $9.99, $x$, $HOME, $1
  • lone $ does not load KaTeX

Disable singleDollarTextMath so that dollar signs in ordinary prose
(e.g. "$3.50" or "costs $9.99 ... $19.99 per month") are no longer
interpreted as KaTeX inline-math delimiters. This caused currency
values to render with italic styling and collapsed spacing.

Math content using $...$ syntax still works via $$...$$ (display)
and \(...\) / \[...\] (inline, normalized to $$..$$).
normalizeMathDelimiters now converts \(...\) to ... instead of
$...$ so inline math still works after singleDollarTextMath is
disabled. Update comments and add regression tests for currency
preservation and \(...\) math normalization.
@xintaofei

Copy link
Copy Markdown
Owner

Thanks for this, and for taking the time to write up the reasoning so clearly — the diagnosis is correct and I could reproduce it exactly. I parsed a few samples with the versions this repo actually has installed (remark-math@6.0.0 / micromark-extension-math@3.1.0 / mdast-util-math@3.0.0 / rehype-katex@7.0.1):

"The Pro plan costs $9.99 but the Team plan costs $19.99 per month."
  singleDollarTextMath: true  -> text("The Pro plan costs ")
                                 inlineMath("9.99 but the Team plan costs ")
                                 text("19.99 per month.")
  singleDollarTextMath: false -> one plain text node

One thing your description undersells: the same bug eats shell variables, which for a coding workbench are much more common than currency.

"Set $HOME and $PATH before running."   -> inlineMath("HOME and ")
"Use $1 and $2 as positional args."     -> inlineMath("1 and ")

That's honestly the strongest argument for the change here, and worth putting in the PR description.

I also want to give credit to the \(...\)$$...$$ half, because it isn't just a workaround — for single-line input it fixes real garbling that exists today:

"Total: $100. Also \(x\)."   old -> "Total: $100. Also $x$."   -> inlineMath("100. Also ")   [broken]
                             new -> "Total: $100. Also $$x$$." -> inlineMath("x")            [correct]
"Cost \(\$5\) here"          old -> inlineMath("\\") text("5$ here")                          [broken]
                             new -> inlineMath("\\$5")                                        [correct]

And it correctly stays inlinemdast-util-math tags every math-text node math-inline regardless of dollar count, and rehype-katex only enters display mode for math-display (or a fenced language-math block), so inline $$x$$ renders inline.

Local gates on your branch are all green: prettier, pnpm eslint src/components/ai-elements, tsc --noEmit, and the full pnpm test (305 files / 4083 tests). No conflicts with current main.

So — direction agreed. There's one thing I'd like fixed before merge, plus a couple of notes.


🔴 Needs a fix: multi-line \(...\) at the start of a block now loses content

$$ at the beginning of a line opens a math flow fence, and everything after it on that line is consumed as fence metadata and thrown away. So a \(...\) that wraps across a line break gets corrupted:

raw: "\(a\nb\)"
  old: "$a\nb$"   -> paragraph > inlineMath("a\nb")     ✅
  new: "$$a\nb$$" -> math(meta: "a", value: "b$$")      ❌ KaTeX error, "a" gone

raw: "\(a\nb\n\)"
  old:            -> inlineMath("a\nb")                 ✅
  new:            -> math(meta: "a", value: "b")        ❌ "a" silently dropped

It only triggers when the \( starts the block — text \(a\nb\) tail is unaffected. Models do wrap inline math across lines often enough that I'd rather not ship a silent content-loss path.

One warning, since it cost me some time: the obvious fix — collapsing the newlines inside the \(...\) body — is not safe. It operates on raw Markdown, so:

  • container prefixes leak into the formula: > \(a\n> b\) collapses to > $$a > b$$inlineMath("a > b")
  • TeX % comments are newline-terminated: KaTeX renders a % comment\nb + c as ab+c, but the collapsed one-liner renders just a

So it needs to be Markdown-aware and TeX-aware, or handled at the parser level rather than by string rewriting. Happy to take this one off your hands if you'd rather not go down that rabbit hole — just say the word.

🟡 This reverts b23f6a5, and I'd like that pinned by a test

git log -S singleDollarTextMath -- src/ returns exactly two commits: e0b677af (which just relocated it) and b23f6a5a"fix: enable inline math formula rendering with single dollar signs". So $x$ will stop rendering, and that was deliberate at the time. Relevant here: codeg hosts DeepSeek / Kimi / Qwen-family agents that lean toward $...$ for inline math, where Claude prefers \(...\).

I'm inclined to accept the trade — $VAR and prices in agent prose are more common than inline LaTeX, and false is @streamdown/math's own default — but I'd like it to be a recorded decision rather than something the next person flips back. A short comment at the createMathPlugin call, plus a test pinning that $x$ is now literal text, would do it.

🟡 The new currency test doesn't actually test the fix

it("preserves currency values as plain text", () => {
  const text = "Costs $25 direct and $13 elsewhere."
  expect(normalizeMathDelimiters(text)).toBe(text)
})

normalizeMathDelimiters only rewrites \[...\] / \(...\) and never touches $, so this assertion passes identically on main — I ran it against both. It reads like currency regression coverage but protects nothing. The real behavior change is guarded only by expect(createMathPlugin).toHaveBeenCalledWith({ singleDollarTextMath: false }), which asserts the argument we pass, not what renders.

A parse-level test (same remark-parse + remark-math option) asserting that $9.99 … $19.99 produces no inlineMath node would cover the actual fix — and it's the natural home for the $x$ case above too.

🟢 Optional follow-up (fine to skip in this PR)

detectHeavyPlugins still treats a lone $ as a math trigger:

math: text.includes("$") || text.includes("\\[") || text.includes("\\(")

After this change a single $ can never produce math on its own, so every message mentioning a price, $PATH, or $1 still eagerly imports KaTeX (~4.3 MB) — exactly what that module exists to avoid.

Careful if you do touch it though: tightening to $$ alone is not a safe superset, because rehype-katex also renders ```math fences via language-math with no $ anywhere. Price $5 followed by a ```math fence currently loads KaTeX only by accident of the stray $5. Any tightening has to detect math fences too. (Side note for me, not you: a ```math fence with no $ anywhere already fails to load KaTeX on main today — pre-existing, I'll file it separately.)

Nits

  • streamdown-plugins.ts:169\[…\] got mangled into \[…]\ in the comment
  • same commit reworded "keying the effect and memo""and the memo"; unrelated to the fix
  • streamdown-plugins.ts:34 still says "remark-math only transforms $…$", which is no longer accurate
  • worth a line noting that single-line $$...$$ inside a paragraph stays inline math, so nobody "fixes" it into display later

Sorry for the long list — the underlying change is small but it sits on a surface that's easy to get subtly wrong, so I wanted to give you the measurements rather than just opinions. Fix the multi-line case and add the two tests and I'm happy to merge this. Thanks again for the contribution! 🙏

Do not collapse inner newlines. Pad a start-of-block multi-line paren
formula with a ZWSP so $$ is not a fence, and keep the closer on the
last content line. Parse-level tests pin currency, $x$, $HOME, and the
multi-line cases. Lone $ no longer warms KaTeX.
@Adam-Dalloul

Copy link
Copy Markdown
Contributor Author

Pushed a follow-up.

The multi-line \(a\nb\) case at the start of a block was dropping the first line. I didn't flatten the newlines, that wrecks TeX comments and blockquotes. If $$ would land at the start of a line I prefix a ZWSP so it can't become a fence. A normal space isn't enough. Trailing newlines inside the parens move after the closer.

$x$ staying as plain text is on purpose. Comment on the plugin call mentions the old commit. Parse tests cover $9.99, $HOME, $1, and $x$.

Lone $ no longer loads KaTeX. Detection is $$, \(/\[, and math fences.

Comments you flagged are cleaned up.

@xintaofei

Copy link
Copy Markdown
Owner

Thanks for turning this around so fast — and for not taking the shortcut. You adopted every point, and the two things I was most worried about are handled correctly:

"\(a % comment\nb + c\)"  -> ​$$a % comment\nb + c$$  -> inlineMath("a % comment\nb + c")   ✅ newline kept
"> \(a\n> b\)"            -> > ​$$a\n> b$$            -> inlineMath("a\nb")                 ✅ prefix intact
"text \(a\nb\) tail"      -> not padded                                                     ✅ no stray char

Blockquotes, -/* bullets, ordered lists, nested > -, and the single-line paths all check out. The parse-level tests are real coverage now, and the detector work is genuinely good — it even fixes a pre-existing bug I was going to file separately (a ```math fence with no $ anywhere never loaded KaTeX on main). Gates are green here too: prettier, eslint, tsc, and the full suite (306 files / 4091 tests).

So the approach is right. But I ran the position logic across the rest of the container grammar and there's more surface than either of us thought. Everything below is measured against main, using your normalizeMathDelimiters.

🔴 R1 — the closer needs the same treatment as the opener

Only the opening $$ gets padded. When the trailing content is a container continuation prefix (\n> , \n ) rather than a bare \n, the peel doesn't catch it, so the closing $$ still lands alone at a line start:

"> \(a\n> b\n> \)"
  main -> inlineMath("a\nb\n")
  head -> text("​$$a\nb")  +  math(meta=null, value="")

That renders as a literal $$a b with an invisible ZWSP, followed by an empty math block — the formula is destroyed rather than just mangled. Same for the list form:

"- Note:\n  \(a\n  b\n  \) holds."  ->  math(meta="a", value="b\n$$ holds.")

🔴 R2 — the opener prefix grammar is narrower than CommonMark's

"+ \(a\n  b\)"     -> math(meta="a", value="b$$")   # `+` is a valid bullet; class is [*-]
" \(a\nb\)"        -> math(meta="a", value="b$$")   # 1-space indent
"   \(a\nb\)"      -> math(meta="a", value="b$$")   # 3-space indent (fences allow 0–3)
"-  \(a\n   b\)"   -> math(meta="a", value="b$$")   # two spaces after the marker
">  \(a\n>  b\)"   -> math(meta="a", value="b$$")   # two spaces after `>`
"- Note that\n  \(a + b\n  = c\) holds."
                   -> math(meta="a + b", value="= c$$ holds.")   # list continuation line

The last one is the realistic shape — a bulleted point whose formula wraps. The common thread: the regex only accepts prefixes built from container markers with exactly one trailing space, so bare indentation is never recognised, and that's exactly what a continuation line looks like.

🔴 R3 — CR / CRLF

"\(a\r\n\)"   main -> inlineMath("a\r\n")   head -> math(meta="a", value="")
"\(a\rb\)"    main -> inlineMath("a\rb")    head -> math(meta="a", value="b$$")

/\n+$/ leaves the \r behind and lastIndexOf("\n") can't see a lone-CR line start. This one reaches beyond chat: file-workspace-panel.tsx:2088 runs the same function over real .md file contents, which are often CRLF.

🔴 R4 — the prefix regex backtracks exponentially

Measured on normalizeMathDelimiters itself with "> ".repeat(n) + "x \(a\nb\)":

n 18 20 22 24 26
ms 3.5 14.3 57.3 228.6 909.6

Clean doubling every 2 markers. In /^(?:[ \t]{0,3}(?:>[ \t]?|…))*$/, [ \t]{0,3} and >[ \t]? can both consume the same space, so a prefix that ultimately fails blows up. This function runs in a useMemo keyed on the message text — once per streaming batch — so it's on the render hot path, and the file preview runs it over whatever the user opens. It needs both the multi-line body and the deep prefix to trigger, so it isn't trivially reachable, but making the regex unambiguous costs nothing.

🟡 One note on the ZWSP

It's a reasonable trick and I'm not asking you to drop it, but it isn't inert — it's a real character every downstream matcher sees. Emphasis is fine in the shapes you pad (I checked * \(a\nb\) specifically, since * is both a bullet and an emphasis marker), but link reference identifiers aren't:

> [foo
> \(a
> b\)]
>
> [foo \(a
> b\)]: /dest

Only the line-start occurrence is padded, so the label normalises to foo ​$$a b$$ and the definition to foo $$a b$$ — they stop matching and the link degrades to text. Genuinely exotic; I mention it only so the blast radius is written down next to the injection site.


On the approach — I need to walk back my round-1 suggestion

I suggested "do it at the parser level with a remark plugin visiting text nodes." That doesn't work, and I should have checked before suggesting it. CommonMark eats the escaping backslashes, so by the time a text node exists there is nothing left to key on:

raw "\(a\nb\)"  -> text("(a\nb)")
raw  "(a\nb)"   -> text("(a\nb)")     # byte-identical

And markdown-active bodies shred across nodes — \(a *b* c\) becomes text + emphasis + text, where your pre-parse rewrite gets it right as one inlineMath("a *b* c"). (I also said "that's how remark-math works" — it isn't; remark-math registers micromark syntax extensions, which see raw bytes before escapes resolve.)

A micromark syntax extension for \( / \[ would be sound, and I shouldn't imply Streamdown prevents it — 2.2.0 exposes mode="static" and parseMarkdownIntoBlocksFn, so its block split is configurable. But that's a much bigger lift than what you've built.

So: your layer is the right layer for how this app currently drives Streamdown. It's incomplete, not wrong. What's left is R1–R4: canonicalise CR/CRLF before any offset logic, give the closer the same handling as the opener, and make the position test both complete and linear.

Totally fine if that's more than you signed up for — say the word and I'll take it from here and credit you on the commit. If you'd rather finish it, I'll review promptly. Either way, thanks for the care you've put into this one. 🙏

Move a prefix-only last line after $$ so the closer cannot open a
flow fence. Walk CommonMark prefixes in one pass (+, indent,
continuation). Fold CR/CRLF to LF first.
@Adam-Dalloul

Copy link
Copy Markdown
Contributor Author

Pushed another follow-up.

R1: ZWSP before the closer ended up inside the formula, and after it still opened a fence. If the last line is only a container prefix I move that line after $$ instead.

R2: the prefix walk now accepts +, 0-3 spaces of indent, extra spaces after the marker, and list continuation indent.

R3: CR and CRLF get folded to LF before any offset work.

R4: that walk is a single pass now, no regex.

I left a note on the opener ZWSP about link reference labels.

@xintaofei

Copy link
Copy Markdown
Owner

Thanks — and genuinely, this round is good work. R1–R4 are all properly fixed, and I verified each one shape by shape against main:

"> \(a\n> b\n> \)"        -> inlineMath("a\nb")        R1 ✅ (formula intact; KaTeX HTML byte-identical)
"+ \(a\n  b\)"  " \(a\nb\)"  "   \(a\nb\)"
"-  \(a\n   b\)"  ">  \(a\n>  b\)"
"- Note that\n  \(a + b\n  = c\) holds."               R2 ✅ all six, math nodes identical to main
"\(a\r\n\)"  "\(a\rb\)"                                R3 ✅ content preserved
"> ".repeat(26)  ->  0.12ms  (was 909.6ms)             R4 ✅ linear — 1.31ms at n=2000

scanContainerPrefix really is O(n) — i strictly advances on every continue, including the ordered-marker branch. And I re-checked 16 previously-working shapes; none regressed. The peelPrefixOnlyLastLine idea is a genuinely clever way out of the closer problem.

Gates are green: prettier, eslint, tsc, your three test files (37 tests), and the full suite at 4097/4098 — the one failure is an unrelated 5s-timeout flake in logs-settings.test.tsx. (An earlier run of mine reported 22 failures; that was my own bad measurement — the machine was at load ~295 and had picked up a stray scratch file. Disregard it.)

I owe you a correction before anything else: I was about to suggest two one-line fixes, and I checked them first — both are wrong. Removing the indent < 3 cap breaks indented marker-looking TeX that your capped version currently gets right; and dropping the peeled tail turns the problem below from expulsion into outright deletion. So please don't spend time on either.


What's still open

N1 — peelPrefixOnlyLastLine pushes marker-looking TeX out of the formula. It classifies the formula's last line with the same scanner used for container prefixes, so TeX that happens to resemble a list marker is treated as structure:

"Before \(a\n2. \) after"
  main -> t("Before ")  inlineMath("a\n2. ")  t(" after")
  head -> t("Before ")  inlineMath("a")       t("\n2.  after")     <- "2. " is no longer in the formula

N2 — the trailing-newline peel splits paragraphs when prose follows. The peeled \n goes into after, then the source's own newline after \) follows it, so you get a blank line:

"\(a\nb\n\)\nafter"  ->  "​$$a\nb$$\n\nafter"
  main -> one paragraph
  head -> two paragraphs

Different mechanism from R5 — this newline lives in after, so it isn't fixed by touching prefixTail.

N3 — the CR fix introduced an inline-code regression. The fold runs before the code masking, and the inline-code mask is /`[^`\n]+`/ — it excludes LF but allows CR. Folding CR→LF first un-masks multiline inline code:

"`\(a\rb\)`"
  main -> inlineCode("\(a\rb\)")   (masked, untouched)
  head -> inlineCode("$$a\nb$$")   (contents rewritten)

R5 (still open) — the moved closer line leaves a line of only the container prefix, i.e. a blank line, so > \(a\n> b\n> \)\n> more text goes from one paragraph to two, and the list form flips LI(spread=false)spread=true (a tight list item becomes loose while its sibling stays tight).

B1 (still open) — container prefixes ≥ 4 columns still corrupt: ordered item ≥ 10, 2+ level nested lists.


Where I think this leaves us

Round 1 found one defect. Fixing it surfaced four. Fixing those surfaced five. And the two obvious one-liners for this round are both unsafe. I don't think that's carelessness on your part — I think it's the layer. normalizeMathDelimiters has to answer block-structure questions (is this a fence position? is this line a container prefix? is this span code?) by pattern-matching raw bytes, without the container stack that only the parser has. Raw preprocessing isn't inherently incapable of getting this right, but it is brittle here, and I haven't found a bounded fix either.

So my suggestion: cut the multi-line case out of this PR and let's land the rest.

Concretely — rewrite \(...\) only when the body contains no line break, and leave multi-line bodies alone. That removes the ZWSP, the prefix scan, the tail peel, and the CR fold, and retires N1, N2, N3, R5 and B1 in one move. I measured it:

identical to main : "Also \(x\)."   "\(a *b* c\)"   "\[\n a \n\]"   and the N3 CR inline-code case
better than main  : "Total: $100. Also \(x\)."   "Cost \(\$5\) here"    (both garbled on main today)

Two things to be precise about, because I checked and my first read was too rosy:

  • A skipped body is not guaranteed to render as literal text — it goes back to being ordinary Markdown, and can pick up Markdown semantics. \(x\n[a](b)\ny\) yields a real link; \(a\n*b*\) yields emphasis; a TeX \\ row separator loses a backslash. So this is a real trade-off, not a free win — it's just a much smaller and more visible one than silent content loss and restructured lists.
  • The multi-line test has to be "contains CR or LF", not just LF. With the global CR fold gone, an LF-only check would treat \(a\rb\) as single-line and produce $$a\rb$$math("b$$"). /[\r\n]/ leaves it alone correctly.

(For the record: multi-line \[a\nb\] loses its first line too, but that's pre-existing on main and this scoping leaves it byte-identical, so it's out of scope here — worth its own issue.)

That keeps everything this PR is actually for: singleDollarTextMath: false for currency and $HOME/$1, correct single-line \(...\), and the detector work — which also fixes a pre-existing bug where a ```math fence with no $ never loaded KaTeX.

If you'd rather not do another push, say the word and I'll do the descope myself and land it with you credited. You've put four solid rounds into this and I'd like it to ship. 🙏

Whatever lands, it should carry AST-shape regressions rather than only value checks — the current parse tests never assert paragraph or list-spread structure, and the linearity test is a single wall-clock < 50ms at n=40, which is timing-sensitive on a loaded machine.

@xintaofei

Copy link
Copy Markdown
Owner

I went ahead and fixed the remaining five myself rather than sending you round five — sorry for the length of this thread. 🙏

Adam-Dalloul#1 (opened against your branch; "Allow edits by maintainers" is off, so I couldn't push directly). Merging it updates this PR in place and I'll merge #463 straight after.

The short version: every remaining defect was the same question wearing a different hat — is this line a container prefix, or is it TeX? — and a single line can't answer it. 2. is a list marker or a formula term. A tab-indented > is a marker or a relation. A lazily continued blockquote carries no marker at all. I sharpened that classifier three times; each pass fixed the named shape and broke a new one, which is exactly what happened to you.

So the fix stops asking. The closing $$ gets a zero-width pad unconditionally — it lands inside the formula, so there's no decision to make — and the opener keeps your conditional pad, because that one sits in the surrounding Markdown where U+200B is a real character to emphasis and link-reference matching.

Your ZWSP instinct was right, by the way. The part neither of us checked: KaTeX doesn't ignore U+200B. It lexes as a textord, so a trailing pad changes the spacing class of a terminal operator — a+ keeps mbin and gains 0.2222em — and warns on every streaming rerender. So the pad is stripped back out after parsing, before rendering, by wrapping the math plugin's remark half. That also covers the file preview, which passes no remarkPlugins of its own.

Everything you built stays: singleDollarTextMath: false, the \(...\) rewrite, the detector work (which also fixed a pre-existing ```math-fence bug on main), and your parse-level tests. containerPrefixEnd is your single-pass scan, kept as-is for the opener.

Full suite green, 307 files / 4109 tests.

Thanks for sticking with four rounds of increasingly obscure markdown trivia — the $VAR/currency fix underneath is genuinely worth having, and it wouldn't have got here without you pushing it.

@xintaofei
xintaofei merged commit 5cf9619 into xintaofei:main Aug 17, 2026
7 checks passed
@xintaofei

Copy link
Copy Markdown
Owner

Landed — this is done. ✅

Correcting my earlier note: rather than merging the follow-up into your branch first, I merged this PR as-is and then applied the remaining fixes directly on top, so Adam-Dalloul/codeg#1 is closed as superseded rather than merged. Nothing further needed from you.

Final state on main: the currency / $HOME / $1 fix, your \(...\) rewrite, the detector work, and the follow-up guards. Full suite green at 314 files / 4226 tests.

The one thing deliberately left alone, pre-existing and unchanged by any of this: \(...\) inside an indented code block still gets rewritten, since indented code isn't masked and four spaces means "code" at top level but "content column" inside a nested list. That gets its own issue rather than more guessing.

Thanks for the patience across four rounds — good contribution. 🙏

xintaofei added a commit that referenced this pull request Aug 18, 2026
Follow-up to #463. The `\(...\)` -> `$$...$$` rewrite still had to keep
`$$` off the start of a line's block content, and the guards for that were
asking a question a single line cannot answer — container prefix, or TeX?
Marker-shaped TeX was pushed out of formulas, prefix-only closing lines
split paragraphs and loosened list items, CR/CRLF corrupted inline code,
and >= 4-column prefixes were unguarded.

Replaces the classifier: the closing `$$` is padded unconditionally (that
pad lands inside the formula, so no decision is needed) and stripped back
out after parsing, before KaTeX — which lexes U+200B as a `textord` and
would otherwise shift the spacing of a terminal operator.
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.

2 participants