fix(web): disable single-dollar inline math in remark-math#805
Open
heavygee wants to merge 1 commit into
Open
Conversation
Default remark-math configuration treats $...$ as inline LaTeX, which turns any prose containing two currency amounts (e.g. "save $400 vs the $200 plan") into a KaTeX block — paragraphs collapse, whitespace is stripped, the running text is re-rendered as math symbols. Pass `singleDollarTextMath: false` to remarkMath so single $ is plain text. Block math `$$...$$` (on its own line) still renders, matching GitHub-flavored markdown semantics. Single source of truth: MARKDOWN_PLUGINS is shared by MarkdownText, Reasoning, and MarkdownRenderer — fix lands in all three surfaces. Adds 3 regression tests that drive the unified pipeline end-to-end: prose with multiple "$N" amounts produces no `class="katex"` and no `<math>` element; `$$...$$` block math still does. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Findings
- [Minor] New test imports undeclared markdown packages — the added regression test now imports
unified,remark-parse,remark-rehype, andhast-util-to-htmldirectly, but none of those packages are declared inweb/package.jsonor the root/package manifests (checked repo manifests). The test currently relies on transitive dependencies from other markdown packages, so a valid dependency update can remove or move them and makebun run test:webfail even though this test still compiles conceptually. Evidence:web/src/components/assistant-ui/markdown-text.test.ts:2
Suggested fix:{ "devDependencies": { "hast-util-to-html": "^9.0.5", "remark-parse": "^11.0.0", "remark-rehype": "^11.1.2", "unified": "^11.0.5" } }
Summary
- Review mode: initial
- One dependency hygiene issue found in the new test. No correctness issue found in the
remarkMathoption change itself. Residual risk: I could not run the web test locally becausebunis not installed in this runner.
Testing
- Not run (automation):
buncommand not available in runner.
HAPI Bot
| @@ -1,7 +1,11 @@ | |||
| import { describe, expect, it } from 'vitest' | |||
| import { unified } from 'unified' | |||
There was a problem hiding this comment.
[Minor] This test now imports unified, remark-parse, remark-rehype, and hast-util-to-html directly, but the web package does not declare any of them. That makes bun run test:web depend on transitive dependencies from other markdown packages, so a valid dependency update can break this test without touching it.
Suggested fix:
{
"devDependencies": {
"hast-util-to-html": "^9.0.5",
"remark-parse": "^11.0.0",
"remark-rehype": "^11.1.2",
"unified": "^11.0.5"
}
}
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
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
Disables single-dollar inline math in
remark-mathso prose containing currency amounts no longer collapses into KaTeX output.$$...$$block math (on its own line) is unchanged.Fixes #800.
Problem
remark-mathis configured with defaults, which meanssingleDollarTextMath: true. Any pair of$characters in a paragraph becomes a math span, so messages like:render with everything between the first and last
$collapsed into a single<span class="katex">block — whitespace stripped, letters reflowed as italic math symbols, apostrophes turned into prime marks. The text is no longer readable as prose.Currency mentions are common in chat (pricing discussions, plan comparisons, billing), so the false-positive rate is high enough that day-to-day prose has been hitting it.
Approach
Pass
singleDollarTextMath: falsetoremarkMathinweb/src/components/assistant-ui/markdown-text.tsx. This is a single source of truth —MarkdownText,Reasoning, andMarkdownRendererall import the sameMARKDOWN_PLUGINSarray, so the fix lands in every markdown surface at once.Block math
$$...$$continues to render. This matches GitHub-flavored markdown semantics, which only treat$$...$$(or fenced```math) as math.Trade-off vs #237
#237 added KaTeX support and explicitly mentioned single-dollar form
$ E= mc^2 $as desired. This PR partially reverses that — single-dollar inline math goes away. Math-heavy users will need to switch to$$E = mc^2$$on its own line. The trade-off felt worth it because false positives from currency$are common in chat prose, while inline math is rare; happy to revisit if maintainers prefer a different policy.Testing
3 new regression tests in
web/src/components/assistant-ui/markdown-text.test.tsdrive the unified pipeline end-to-end and assert against the rendered HTML:$Namounts → noclass="katex", no<math>element, all amounts present as literal text$$E = mc^2$$block math → still produces a KaTeX blockRelated
Questions
Open to a different policy if maintainers prefer (e.g. only disable single-dollar when followed by a digit, to keep
$ E= mc^2 $working). Happy to iterate.