Repository navigation
feat(web): render TeX math in chat markdown - #81
Conversation
yaacovcorcos
left a comment
There was a problem hiding this comment.
Yishay, the overall architecture is strong and should remain:
- Math is owned under
apps/web/src/scient/math. - Integration with inherited T3 code is narrow and localized to
ChatMarkdown. - KaTeX is pinned, bundled locally, and lazily loaded.
- Rendering remains independent from the PDF reader and document-artifact architecture.
- Original TeX is preserved for copying.
- Literal fallback and bounded caching are the right design direction.
I am requesting changes because the current recognition and rendering behavior can corrupt ordinary Markdown and mishandle incomplete content.
1. Dollar parsing corrupts ordinary Markdown
With singleDollarTextMath: true, remark-math changes Markdown structure before ScientMath can apply its currency heuristic.
I reproduced these failures:
[_chat.$threadId.tsx](/tmp/_chat.$threadId.tsx:12)
$PATH$
$USD$
$HOME and $PATH.The file link is destroyed, and shell/environment identifiers are interpreted as math. The same parser-level problem affects some currency expressions. The component-level currency fallback cannot reconstruct Markdown that was already parsed incorrectly.
Please make dollar recognition token-aware and ensure ordinary dollars remain completely unchanged in:
- link labels and destinations;
- file paths and route parameters;
- shell/environment identifiers;
- currency;
- escaped dollars;
- unmatched delimiters.
If $...$ cannot be distinguished safely, it should not be globally enabled until it can be.
2. Backslash-delimiter normalization mutates literal code
The current raw-string transformation for \(...\) and \[...\] only protects simple backtick cases. I reproduced rewriting inside:
- tilde code fences;
- indented code blocks;
- raw
<code>regions.
Math normalization must only affect eligible Markdown text, never code or literal regions. The solution must preserve source positions so task-list editing and other offset-based behavior remain correct.
The current onTaskListChange === undefined gate also causes identical Markdown to support different delimiters depending on the rendering surface. Please remove that surface-dependent behavior as part of correcting the normalization boundary.
3. Incomplete streaming math renders prematurely
An unclosed display block such as:
$$
x + yis already interpreted and rendered as display math. Unclosed math fences behave similarly.
While streaming, incomplete math must stay literal until its closing delimiter arrives. Rendering should then begin normally. Please also bound the maximum TeX input size so malformed or extremely large model output cannot repeatedly trigger expensive KaTeX work.
4. Test the real ChatMarkdown integration
The current seam test primarily inspects source strings and a default react-markdown pipeline. It does not prove that the actual ChatMarkdown component map routes inline and display nodes correctly.
Please add integration-level regressions covering:
- inline math;
- display math;
- fenced math;
- unfinished streaming delimiters;
- links and paths containing
$; $PATH$,$USD$, currency, and escaped dollars;- inline, fenced, tilde-fenced, indented, and raw HTML code;
- task-list Markdown;
- malformed or unsupported TeX;
- copying the original TeX;
- both backslash and dollar delimiters.
The test should prove that inline math avoids <pre>, while display and fenced math use the display wrapper.
5. Prevent inherited chat wrapping from breaking equations
.chat-markdown applies overflow-wrap: anywhere and word-break: break-word. The KaTeX subtree currently does not reset those inherited rules.
Please prevent internal equation fragments from wrapping arbitrarily. Wide display equations should scroll within their math container rather than widening or breaking the conversation.
6. Make malformed-math fallback consistent
The documentation says unrenderable equations fall back to their original notation. However, throwOnError: false currently produces KaTeX's hard-coded red error markup for unsupported commands.
Please make the behavior consistent: malformed or unsupported TeX should display the original literal notation cleanly, without an unthemed red KaTeX error.
7. Finish dependency and asset hygiene
Before merge:
- Add KaTeX, its distributed fonts, and
remark-mathtoapps/web/THIRD_PARTY_NOTICES.md. - Use the KaTeX swap stylesheet or another explicit strategy that avoids blank math during cold font loading.
- Correct the documentation claiming only WOFF2 assets are emitted; the current stylesheet references WOFF2, WOFF, and TTF.
- Remove the incidental
unist-util-visitlockfile change if it is not required by the resolved dependency graph.
Final verification
After the fixes:
- Rebase onto current
main. - Run focused math and real ChatMarkdown tests, web typecheck, production build, formatting, lint, and
git diff --check. - Attach visual evidence for light/dark and narrow/wide chat, including one RTL conversation and a long display equation.
The PDF/document-artifact boundaries are already correct and do not need to change in this PR. The required work is to make the current math layer safe, deterministic, and proven through its real integration surface.
Alignment with existing Scient workBefore finalizing this PR, please review:
These references do not expand this PR into PDF export, LaTeX compilation, or broader document-platform work. They establish the boundaries this foundation should preserve:
Please also update The Scientific Document Platform Roadmap is proposed product direction, while the desktop PDF export plan is the repository implementation plan for its lane. Neither should be treated as permission to broaden this PR beyond the chat/Markdown math foundation. |
Three targeted additions to the requested changes
For equation wrapping, verify behavior with long inline and display fixtures: prevent arbitrary inherited word-breaking, retain KaTeX’s intentional inline break opportunities, and keep wide display math contained and horizontally scrollable. A regenerated lockfile may legitimately retain the |
Review changes for PR ScientFactory#81. Parser-level single-dollar math corrupted ordinary markdown (file links with $route params, $PATH, prices), so singleDollarTextMath is now off and $...$ spans are recognized by a Scient remark plugin on the parsed tree, where code, links, and raw HTML are structurally excluded and strict token rules apply: compact numeric spans ($42$, $1/2$, $12-15$) render as math while identifiers, spaced prose, price ranges, and escaped dollars stay text. Backslash-delimiter normalization is now length-preserving (every delimiter becomes exactly $$), so no source offset ever moves, the per-surface onTaskListChange gate is gone, and tilde fences, indented code, and raw HTML code regions are protected. A sole $$ span in its own paragraph is promoted to display math. Unclosed display blocks and math fences stay literal until their closing delimiter arrives (the streaming case), TeX is length-bounded, KaTeX runs with a finite maxSize and falls back to the literal source on parse errors instead of red error markup, equations no longer inherit the chat surface's word-breaking, KaTeX fonts load with font-display swap via a build-time stylesheet rewrite, and KaTeX and remark-math are recorded in THIRD_PARTY_NOTICES. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
617332e to
4822241
Compare
|
Thank you for the review — the recognition redesign it forced was the right call. All seven areas plus the follow-up additions are addressed in 1. Dollar parsing. 2. Normalization boundary. The rewrite is now length-preserving — each of 3. Streaming. The refinement plugin checks the raw source for each math node: an unclosed 4. Integration tests. The seam test now runs the review matrix through the real plugin chain and sanitize schema shape: inline/display/fenced math, unfinished streaming delimiters, links and paths with 5. Wrapping. 6. Malformed-math fallback. 7. Hygiene. KaTeX (with its WOFF2/WOFF/TTF fonts) and remark-math are in Verification: 65 focused tests, targeted typecheck clean for every changed file, Implemented and reviewed by Claude Fable 5 running in Claude Code, on Yishay's behalf. |
Superseded by the consolidated review of current head 4822241. Addressed items are no longer requested; the remaining current findings are recorded in the new review.
yaacovcorcos
left a comment
There was a problem hiding this comment.
Yishay, the redesign is a substantial improvement. Moving away from
singleDollarTextMath: true, keeping the implementation Scient-owned behind a
narrow ChatMarkdown seam, preserving literal fallback and copyable TeX, and
bounding streaming work are the right architectural decisions.
I rechecked the current head through the real plugin prefix. Three issues
remain in behavior this PR already implements:
- Single-dollar recognition is still unsafe and inconsistent.
echo $HOME/bin:$PATHturnsHOME/bin:into a math node;$a*b*c$and
$a~~b~~c$are fragmented; andliteral \$5 then valid $x+1$suppresses
the later valid expression. Please either move$...$recognition to a
parser/tokenizer boundary that retains source context or leave single-dollar
recognition disabled for this first release. - Backslash-delimiter normalization loses meaning and touches protected
text.\(x\)and\[x\]both become$$x$$, and raw HTML attributes can
be rewritten. Please preserve inline/display intent and structurally exclude
raw HTML, attributes, code, escapes, and other protected Markdown contexts. - The KaTeX font-display patch is ineffective. The emitted face contains
swapfollowed by the originalblock, soblockwins. Please replace the
original declaration or otherwise make the final emitted CSS unambiguous.
Please add the exact reproducers above to the focused real-plugin-chain tests.
The repository-wide cancelled Test job should be rerun through the repository
CI lane; it is not evidence of a math-specific flake.
I am not asking this PR to add PDF export, complete-document LaTeX, a broader
scientific renderer, or a new product-wide geometry policy.
4822241 to
fa2191b
Compare
Review changes for PR ScientFactory#81. Parser-level single-dollar math corrupted ordinary markdown (file links with $route params, $PATH, prices), so singleDollarTextMath is now off and $...$ spans are recognized by a Scient remark plugin on the parsed tree, where code, links, and raw HTML are structurally excluded and strict token rules apply: compact numeric spans ($42$, $1/2$, $12-15$) render as math while identifiers, spaced prose, price ranges, and escaped dollars stay text. Backslash-delimiter normalization is now length-preserving (every delimiter becomes exactly $$), so no source offset ever moves, the per-surface onTaskListChange gate is gone, and tilde fences, indented code, and raw HTML code regions are protected. A sole $$ span in its own paragraph is promoted to display math. Unclosed display blocks and math fences stay literal until their closing delimiter arrives (the streaming case), TeX is length-bounded, KaTeX runs with a finite maxSize and falls back to the literal source on parse errors instead of red error markup, equations no longer inherit the chat surface's word-breaking, KaTeX fonts load with font-display swap via a build-time stylesheet rewrite, and KaTeX and remark-math are recorded in THIRD_PARTY_NOTICES. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Addresses the second review round on ScientFactory#81, all three points verified by running reproducers. Single-dollar `$...$` recognition is removed rather than repaired: tree-level heuristics typeset shell fragments like `$HOME/bin:$PATH`, emphasis parsing fragments `$a*b*c$` before any tree pass can see the span, and the escaped-dollar opt-out silenced valid math in the same text run. Safe `$...$` needs tokenizer-level source context — future work. The supported spellings stay `$$...$$`, `\(...\)`, `\[...\]`, and math fences; literal and copy forms move to `$$...$$` so pasted math re-renders. Backslash pairs keep their authored intent. Normalization stays length-preserving, and that property now earns its keep twice: a new per-message hook threads the pre-normalization text to the refinement plugin, which reads the original delimiter at each math node's offset. `\[...\]` becomes display math wherever it appears — splitting its paragraph the way TeX does — and `\(...\)` is never promoted to display. Normalization also protects raw HTML tags and comments, so attribute text like an href is never rewritten; prose between tags stays eligible. The font-display build patch is deleted as ineffective: KaTeX's stylesheet already declares `font-display: block`, which beat the prepended swap by source order. The runtime now imports `katex/dist/katex-swap.min.css`, the variant KaTeX ships with swap in every face. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
All three points are addressed on the rebased head (fa2191b, on top of 5c1a63f), each verified by a running reproducer before and after the fix. 1. Single-dollar recognition. All three failures reproduced exactly as you described — and your diagnosis pointed at the fix. I implemented your first option: recognition moved to a guarded micromark text construct ( 2. Delimiter normalization. The rewrite stays length-preserving, and that property now recovers intent instead of erasing it: a per-message hook threads the pre-normalization text to the refinement plugin (the shared static plugin arrays are untouched when a message has no backslash delimiters), which reads the original delimiter at each math node's offset. 3. Font-display. Confirmed precisely: Verification: 106 focused math tests through the real plugin chain and sanitize schema shape; full Implemented and reviewed by Claude Fable 5 running in Claude Code, on Yishay's behalf. |
fa2191b to
5b94d72
Compare
yaacovcorcos
left a comment
There was a problem hiding this comment.
Re-reviewed at 5b94d72. The requested tokenizer, delimiter-normalization, and KaTeX font-loading corrections are implemented, the focused behavior is covered, all current hosted checks pass, and no review threads remain unresolved. Agent capability awareness is intentionally tracked as a separate follow-up rather than expanding this renderer PR.
Assistant responses full of LaTeX were unreadable: chat showed raw \(...\) and $$...$$ source instead of typeset math. A Scient-owned module (apps/web/src/scient/math) now normalizes model-emitted TeX delimiters, parses dollar math through remark-math, and renders it with a lazily loaded, locally bundled KaTeX — falling back to the literal source while loading, on render failure, and for currency-like text the parser cannot tell apart from math. The inherited-host seam is ChatMarkdown.tsx only (imports, one plugin entry per remark array, an offset-safe normalization gate, and two component branches), guarded by a static seam-audit test. Rendered markdown file previews inherit dollar-math rendering; mobile is deliberately untouched. See docs/internals/scient-math.md. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review findings on the math seam: the currency guard treated $$42$$ as money (display math is always intentional math — the guard is now inline-only); highlight-and-copy serialized KaTeX DOM into garbled text (math elements now carry data-markdown-copy with their dollar-form source, which markdown-clipboard returns verbatim); and the growing prefixes of an unclosed block during streaming each wrote an LRU cache entry (cache reads and writes now skip streaming renders, matching the Shiki highlight cache). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review changes for PR ScientFactory#81. Parser-level single-dollar math corrupted ordinary markdown (file links with $route params, $PATH, prices), so singleDollarTextMath is now off and $...$ spans are recognized by a Scient remark plugin on the parsed tree, where code, links, and raw HTML are structurally excluded and strict token rules apply: compact numeric spans ($42$, $1/2$, $12-15$) render as math while identifiers, spaced prose, price ranges, and escaped dollars stay text. Backslash-delimiter normalization is now length-preserving (every delimiter becomes exactly $$), so no source offset ever moves, the per-surface onTaskListChange gate is gone, and tilde fences, indented code, and raw HTML code regions are protected. A sole $$ span in its own paragraph is promoted to display math. Unclosed display blocks and math fences stay literal until their closing delimiter arrives (the streaming case), TeX is length-bounded, KaTeX runs with a finite maxSize and falls back to the literal source on parse errors instead of red error markup, equations no longer inherit the chat surface's word-breaking, KaTeX fonts load with font-display swap via a build-time stylesheet rewrite, and KaTeX and remark-math are recorded in THIRD_PARTY_NOTICES. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Addresses the second review round on ScientFactory#81, all three points verified by running reproducers. Single-dollar `$...$` recognition is removed rather than repaired: tree-level heuristics typeset shell fragments like `$HOME/bin:$PATH`, emphasis parsing fragments `$a*b*c$` before any tree pass can see the span, and the escaped-dollar opt-out silenced valid math in the same text run. Safe `$...$` needs tokenizer-level source context — future work. The supported spellings stay `$$...$$`, `\(...\)`, `\[...\]`, and math fences; literal and copy forms move to `$$...$$` so pasted math re-renders. Backslash pairs keep their authored intent. Normalization stays length-preserving, and that property now earns its keep twice: a new per-message hook threads the pre-normalization text to the refinement plugin, which reads the original delimiter at each math node's offset. `\[...\]` becomes display math wherever it appears — splitting its paragraph the way TeX does — and `\(...\)` is never promoted to display. Normalization also protects raw HTML tags and comments, so attribute text like an href is never rewritten; prose between tags stays eligible. The font-display build patch is deleted as ineffective: KaTeX's stylesheet already declares `font-display: block`, which beat the prepended swap by source order. The runtime now imports `katex/dist/katex-swap.min.css`, the variant KaTeX ships with swap in every face. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Dropping `$...$` entirely satisfied the review but not the product: model answers are the main source of chat math, and Claude-family and Gemini models emit single-dollar inline math constantly, so raw `$x^2$` in every reply loses the point of the feature. This implements the review's first option instead: recognition at a micromark text construct, the only altitude where `$...$` can be both safe and complete. The tokenizer sees raw source, so `$a*b*c$` arrives whole before emphasis fragments it; markdown's escape construct consumes `\$` first, so escapes work structurally instead of by text-run opt-out; and a rejected candidate unwinds cleanly, so links and prices cannot be corrupted the way parser-level `singleDollarTextMath` corrupts them. Guards: flanking rules (no word character before the opener, no whitespace at either edge, no digit or dollar after the closer), single-line spans capped at 300 characters, and content plausibility — shell identifiers and identifier paths stay text, spaced prose needs a TeX signal, a colon needs a strong signal, and the `](` link boundary is never math. A sole single-dollar paragraph stays inline; only `$$` forms promote to display. One micromark subtlety is load-bearing and documented: when several constructs share a character code, an attempt runs them all regardless of their individual `previous` hooks, so every guard lives inside `tokenize`. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
An adversarial verification pass (three lenses, 68 structure probes plus a
guard-bypass battery) confirmed fourteen inputs where routine chat rendered
as bogus math or lost structure. All shared one signature: implausible
content shape at the span edges.
New guards: content must end the way formulas end (alphanumeric, closing
bracket, factorial, prime, or percent), which rejects the glued-variable
class ($src/$dst, sed s/$old/$new/, $a=$b, awk $1==$NF); content starting
with an interpolation ( or { needs a TeX operator, protecting $(pwd),
${DIR}/${FILE}, $($user.Name), and jQuery chains while keeping $(a+b)^2;
markdown structure is never math content — the ][ reference-link boundary,
bare reference labels ([foo], while [0,1] intervals stay math), footnote
references ([^1], whose engulfment silently deleted footnote bodies),
HTML-tag and autolink shapes (a</b> left the block bold; <https://…>
swallowed the autolink); an asterisk no longer qualifies spaced content
(it is usually the author's emphasis delimiter, and *a $b* c$ destroyed
the emphasis); and bare domain-like content stays prose.
Also removes the this-alias in the tokenizer (both opener facts are
settled before the attempt starts, so they are captured as locals) and
scopes the one inherent oxlint warning with the plugin-contract reason.
The streaming lens and repo sweep passed as-is: every character prefix of
mixed-delimiter messages renders without premature math, refinements
interplay holds, and the full web unit suite stays green.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The refreshed adversarial round confirmed the previous fourteen bypasses
closed and every positive fixture intact, then found a further cluster,
all code glue by shape: quoted shell variables pairing across a flag
(`'$FILE' -C '$DEST'` — a quote after whitespace is shell quoting, not a
prime), bash default expansion (`${PREFIX:-app}`), command substitution
carrying flags (`$(date -u)`, `$(date +%F)`), dereference arrows
(`$x->{key}$y`), empty-paren calls (`$el.fadeOut()$next`), closers the
span never opened (`if ($a)$b`), and unescaped trailing `%` (a TeX
comment — KaTeX silently drops the percent, so such spans only ever
render wrong).
New rules: a trailing prime must follow a symbol; a trailing percent must
be escaped; arrows and empty parens without a TeX command reject; parens
and brackets must balance unless the span opens with one (half-open
`$[0,1)$` intervals survive); a brace-group start requires a backslash;
and a parenthesized start needs a real TeX operator — with whitespace
present, a shell flag's `-`/`+` no longer qualifies.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The convergence round confirmed both prior fix waves hold (byte-identical
to the no-plugin baseline on every guarded case, full web suite green)
and surfaced a final family: the backslash itself masquerading as a TeX
signal. In `echo -e $a\n$b` the `\n` satisfied every backslash-based
rule, even defeating the identifier guard for `$PATH\n$HOME`, and since
KaTeX rejects string escapes such spans could only ever display the
literal fallback — corruption with no upside.
A backslash now counts as TeX only when it starts a control word or
control symbol; string escapes and regex backreferences reject the span.
The brace-start rule requires a real control word (`${dir//\//_}` no
longer qualifies via its escaped slash), calls with multi-letter or
dotted names are code (`$el.fadeOut(200)$` dodged the empty-paren rule;
`f(x)` and named operators like `sin` stay math), and the closer never
abuts a `{` (`$dir${file}`).
One residual shape is documented as the accepted floor rather than
guarded: `echo $a$b` renders `a` as math, because `$a$` is byte-identical
to authored math and rejecting a letter after the closer would break the
legitimate `$n$th` idiom.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
5b94d72 to
f0a7c65
Compare
Assistant responses full of LaTeX were unreadable: chat showed raw
\(...\),$...$, and$$...$$source instead of typeset math.A Scient-owned module (
apps/web/src/scient/math) renders TeX in chat markdown with a lazily loaded, locally bundled KaTeX 0.16.47 — no CDN request; the swap-variant stylesheet (katex-swap.min.css) and woff2 fonts ship in the async chunk, so math paints with fallback glyphs during cold font loads. Recognition happens at three altitudes, each where its inputs are unambiguous:normalizeScientMathDelimitersrewrites model-emitted\(...\)/\[...\]into$$forms on the raw string, length-preserving (no character offset ever moves, so task-list toggling and other offset-based behavior stay correct on every surface). Fenced/indented/inline code, raw HTML<code>/<pre>regions, HTML tags (attribute text is never rewritten), and HTML comments are protected ranges.remarkScientSingleDollarMathrecognizes single-dollar$...$at a guarded micromark text construct — raw-source altitude, so spans arrive whole before emphasis runs,\$escapes are handled structurally, and rejected candidates unwind without corrupting links, prices, or shell text. Flanking rules plus content plausibility keep$HOME/bin:$PATH,${DIR}/${FILE},$a\n$b,$el.fadeOut(200)$, reference links, footnotes, and autolinks byte-identical to a math-free renderer, while$x^2$,$sin(2x)$,$a*b*c$,$100\%$, and$[0,1)$typeset. One residual glued-identifier shape is documented as the accepted floor in the internals doc.remarkScientMathRefinementsruns on the parsed tree: unclosed or oversized math stays literal while streaming; authored\[...\]becomes display math wherever it appears, splitting its paragraph the way TeX does;\(...\)and single-dollar spans keep inline intent; own-line$$x$$promotes to display. A per-message hook threads the pre-normalization text so the original delimiter at each node offset decides intent — possible precisely because normalization preserves offsets.Rendering falls back to the literal source while the chunk loads, on KaTeX failure (no red error markup —
throwOnErrorwith literal fallback), and for oversized input;maxSizecaps pathological dimensions. Rendered and fallback math carriesdata-markdown-copywith$$forms, so highlight-and-copy round-trips TeX. Math cache reads/writes skip streaming renders, mirroring the Shiki highlight cache. Rendered markdown file previews inherit the same treatment; mobile is deliberately untouched (native markdown module keeps raw TeX legible).The inherited-host seam is
ChatMarkdown.tsxonly: the math import block, three plugin entries per remark array, two hooks (useScientMathMarkdownText,useScientMathRemarkPlugins), and two component branches — guarded by the static seam-audit test. Maintenance contract:docs/internals/scient-math.md; shipped behavior:docs/user/math-in-chat.md.Verification: 106 focused unit tests (delimiter normalization, tokenizer guards and plausibility rules, authored-intent matrix, streaming prefixes, seam audit, chat-shaped pipeline through the real plugin chain and sanitizer, RTL fixture); full
apps/webunit project 2815/2815; three adversarial verification rounds structure-diffed against a no-plugin baseline with every discovered bypass encoded as a regression test; production Windows build packaged and manually exercised against live model output.Implemented and reviewed by Claude Fable 5 running in Claude Code.
🤖 Generated with Claude Code