feat(chat): render LaTeX math with KaTeX (closes #312)#356
feat(chat): render LaTeX math with KaTeX (closes #312)#356kimbotai1337 wants to merge 6 commits into
Conversation
Adds inline ($..$) and block ($$..$$) math rendering through the shared MarkdownRenderer via remark-math and rehype-katex. KaTeX runs with trust=false, strict=ignore, and throwOnError=false so malformed input fails gracefully without crashing the message bubble. The katex.min.css import lives at the app entry to avoid layout shift on first paint; theme-aware overrides in index.css make math text inherit foreground color and route .katex-error through the destructive token. All four MarkdownRenderer consumers (chat, kanban, beads viewer, file browser markdown) gain math rendering automatically. Closes #312
Six safe_auto fixes from the multi-agent ce-code-review pass: - drop dead `throwOnError: false` rehype-katex option (the plugin catches parse errors internally; the option is omitted from its Options type and is ignored at runtime) - add `errorColor: 'currentColor'` so KaTeX's inline error style yields to the surrounding theme cascade instead of locking in #cc0000 - add `maxSize: 100, maxExpand: 1000` to bound rendered element size and macro expansion against UI-DoS via untrusted LaTeX - drop redundant `.markdown-document-content .katex*` CSS selectors (MarkdownRenderer always emits `.markdown-content`, which already covers every consumer) - hoist remarkPlugins and rehypePlugins arrays to module-scope constants so the plugin pipeline does not re-initialize per render (matters most for streaming chat messages) - strengthen the malformed-LaTeX test with a secondary assertion that either a .katex-error span or the raw source remains visible (R4) All 67 markdown tests pass; lint and typecheck clean.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds KaTeX math rendering to the Markdown pipeline: new dependencies, remark/rehype plugin wiring in MarkdownRenderer (with fenced-math handling), global KaTeX CSS and theme overrides, and tests for inline/display math, code-block exemptions, fraction rendering, and malformed-LaTeX fallback. ChangesKaTeX Math Rendering in Markdown
Sequence Diagram(s)sequenceDiagram
participant Markdown as Markdown input
participant Remark as remark-math
participant Rehype as rehype-katex
participant KaTeX as KaTeX renderer
participant ReactMD as ReactMarkdown DOM
Markdown->>Remark: parse inline/display math delimiters ($, $$)
Remark->>Rehype: emit math AST nodes
Rehype->>KaTeX: convert math nodes to HTML
KaTeX->>ReactMD: inject .katex/.katex-display elements
ReactMD->>ReactMD: apply `.markdown-content` CSS rules
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8d66ec9b7d
ℹ️ 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".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/features/markdown/MarkdownRenderer.test.tsx (1)
936-942: ⚡ Quick winAdd one currency-prose regression test to lock expected non-math behavior.
Given the documented single-dollar trade-off, adding a minimal assertion for a non-ambiguous currency sentence (for example,
Price is $5 today.) would protect R1 behavior against future parser/config drift.Suggested test addition
describe('math rendering', () => { + it('does not parse a non-ambiguous single currency amount as math', () => { + render(<MarkdownRenderer content="Price is $5 today." />); + expect(document.querySelector('.katex')).toBeNull(); + }); + it('renders inline math delimited by single dollars', () => { render(<MarkdownRenderer content="The identity $x^2 + y^2 = z^2$ holds." />); const katex = document.querySelector('.katex');🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/markdown/MarkdownRenderer.test.tsx` around lines 936 - 942, Add a regression test in MarkdownRenderer.test.tsx that verifies single-dollar currency prose is NOT parsed as math: render a simple sentence like "Price is $5 today." through the same test helper used elsewhere (e.g., the renderer or renderMarkdown/renderComponent utilities in this test file) and assert the output contains the raw "$5" text (or that no math node/element is produced). This locks the current single-dollar behavior and will fail if remark-math/MarkdownRenderer changes to treat that sentence as inline math.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/plans/2026-05-22-001-feat-math-rendering-katex-plan.md`:
- Around line 84-89: Update the D4 section text to match the actual KaTeX
configuration by removing the claim that we pass throwOnError: false (or
explicitly noting that throwOnError is intentionally omitted in the runtime
config); specifically edit the D4 paragraph that lists KaTeX options (mentions
trust, strict, throwOnError) so it no longer states we pass throwOnError: false
and ensure the later mention around line 110 is also corrected to reflect the
implemented behavior of not passing throwOnError. Reference the KaTeX option
name throwOnError and the D4 heading so future readers see the accurate,
consistent documentation.
---
Nitpick comments:
In `@src/features/markdown/MarkdownRenderer.test.tsx`:
- Around line 936-942: Add a regression test in MarkdownRenderer.test.tsx that
verifies single-dollar currency prose is NOT parsed as math: render a simple
sentence like "Price is $5 today." through the same test helper used elsewhere
(e.g., the renderer or renderMarkdown/renderComponent utilities in this test
file) and assert the output contains the raw "$5" text (or that no math
node/element is produced). This locks the current single-dollar behavior and
will fail if remark-math/MarkdownRenderer changes to treat that sentence as
inline math.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ec0b1b62-4c40-459b-b3b0-37589438103e
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (6)
docs/plans/2026-05-22-001-feat-math-rendering-katex-plan.mdpackage.jsonsrc/features/markdown/MarkdownRenderer.test.tsxsrc/features/markdown/MarkdownRenderer.tsxsrc/index.csssrc/main.tsx
- preserve ```math fenced code blocks as code instead of typesetting them as equations: rehype-katex targets <code class="language-math|math-display| math-inline">, which pulls in both legit math (remark-math sets all three classes) and plain ```math fences (markdown sets only language-math). A small rehype plugin runs before rehype-katex and strips language-math only when math-inline/math-display are absent. New test locks the behavior. - align plan D4 with the implemented options: drop the throwOnError mention (rehype-katex hardcodes the error catch internally and the option is silently ignored), and document the errorColor / maxSize / maxExpand additions from the autofix pass alongside the High-Level diagram. Validation: vitest src/features/markdown/MarkdownRenderer.test.tsx 68/68 pass; tsc -b clean; eslint clean.
|
Round 1 - pushed P0 fixed (0) P1 fixed (2):
Deferred to follow-up issues (0) Open for human input (0) Validation: speaks bot, ships code @coderabbitai full review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3f7eaa4b46
ℹ️ 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".
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain
🏁 Script executed: Length of output: 79242
✅ What's solid
🔴 P1 —
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/features/markdown/MarkdownRenderer.tsx (1)
590-591:⚠️ Potential issue | 🟠 Major | ⚡ Quick winTreat KaTeX/MathML output as opaque before applying
processChildren()highlighting/linkification.
processChildren()currently recurses into all child React elements for paragraphs/headings/lists (only skippingcode/pre/a) and highlights any string children viarenderInlinePathReferences(). Withrehype-katexconfigured forhtmlAndMathml, KaTeX includes the original LaTeX inside MathML<annotation encoding="application/x-tex">, sosearchQuery="frac"can inject<mark className="search-highlight">inside the rendered equation subtree (.katex). If path linkification is enabled, similar mutations can also occur inside those string nodes.Short-circuit recursion for KaTeX subtrees (e.g. descendants of
.katexand/or MathML<annotation>/<semantics>) and add a regression test:searchQuery="frac"with$\\frac{1}{2}$should render no<mark>under.katex.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/markdown/MarkdownRenderer.tsx` around lines 590 - 591, processChildren() currently recurses into all element children and lets renderInlinePathReferences() mutate text nodes inside KaTeX/MathML output; update processChildren (in MarkdownRenderer.tsx) to detect and short-circuit traversal for KaTeX subtrees by skipping descendants where element.className includes "katex" or where tagName is "annotation" or "semantics" (or an element with annotation encoding "application/x-tex"), so no highlighting/linkification runs inside those nodes; ensure this check is applied before calling renderInlinePathReferences() and add a regression test that renders "$\\frac{1}{2}$" with searchQuery="frac" and asserts there are no <mark> elements under the .katex subtree.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/features/markdown/MarkdownRenderer.tsx`:
- Around line 44-45: When removing 'language-math' in the block that checks
isRemarkMath, ensure node.properties.className is never set to an empty array so
codeClassName remains truthy and fenced math blocks still follow the app's
CodeBlock path; update the branch in MarkdownRenderer (the isRemarkMath / list
handling) to filter out 'language-math' but if the resulting array is empty
assign a fallback class (e.g. ['language-text'] or another non-KaTeX marker)
instead of leaving className empty.
---
Outside diff comments:
In `@src/features/markdown/MarkdownRenderer.tsx`:
- Around line 590-591: processChildren() currently recurses into all element
children and lets renderInlinePathReferences() mutate text nodes inside
KaTeX/MathML output; update processChildren (in MarkdownRenderer.tsx) to detect
and short-circuit traversal for KaTeX subtrees by skipping descendants where
element.className includes "katex" or where tagName is "annotation" or
"semantics" (or an element with annotation encoding "application/x-tex"), so no
highlighting/linkification runs inside those nodes; ensure this check is applied
before calling renderInlinePathReferences() and add a regression test that
renders "$\\frac{1}{2}$" with searchQuery="frac" and asserts there are no <mark>
elements under the .katex subtree.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c897e10e-88d3-47b3-a9cc-9edfd45523c3
📒 Files selected for processing (3)
docs/plans/2026-05-22-001-feat-math-rendering-katex-plan.mdsrc/features/markdown/MarkdownRenderer.test.tsxsrc/features/markdown/MarkdownRenderer.tsx
✅ Files skipped from review due to trivial changes (1)
- docs/plans/2026-05-22-001-feat-math-rendering-katex-plan.md
Plan artifacts are kept locally for the implementer's reference but do not belong in the shipped PR. The implementation, tests, and inline comments in MarkdownRenderer.tsx remain the source of truth for D1-D6 decisions.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7f4e574f5b
ℹ️ 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".
CR (Minor) + Codex (P2 x2): the round-1 plugin stripped language-math from fenced ```math blocks but left the className empty, which caused the downstream code component to misclassify them as inline code (falling out of the CodeBlock path) and lose the block UI/lang label. Rename language-math -> language-latex on fenced math nodes instead of stripping. LaTeX is a real hljs language and ```math content is LaTeX-shaped, so the user gets a proper code block with the "latex" label and syntax highlighting. rehype-katex still does not match language-latex, so equation typesetting remains gated to the remark-math output path (where math-inline/math-display also live). Test updated to assert the .code-block-wrapper + .code-lang="latex" path rather than the empty-className regression. Validation: vitest 68/68 pass; tsc -b clean; eslint clean.
|
Round 2 - pushed P0 fixed (0) P1 fixed (3 - all cross-corroborated on the same regression):
Out of scope (handled separately) (1):
Deferred to follow-up issues (0) Open for human input (0) Validation: I argue with the rabbits so you can ship @coderabbitai full review |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 57906
✅ Round-2 changes verified
🔴 P1 carry-over —
|
Both items raised by CR's full-review pass on 2232c7b: - (P1) .katex-error theme color never paints. KaTeX writes an inline `style="color: ..."` attribute on every .katex-error span, and inline styles beat class-selector rules by specificity. The errorColor: 'currentColor' I had passed in the autofix pass did NOT let the cascade win, it only changed what KaTeX wrote inline. Drop the errorColor option (so KaTeX uses its default #cc0000 inline) and add !important to the .katex-error CSS rule, so the themed destructive token wins over the inline attribute. - (P3 -> P1 in our rubric) processChildren walked KaTeX subtrees on every render of a chat message containing equations, paying the path-link / search-highlight scan over hundreds of inner spans for zero benefit (the spans are pure presentation, no path text or search-matchable content). Skip any child whose className contains "katex" (or equals "math" for the wrapper div remark-math emits). Validation: vitest 68/68 pass; tsc -b clean; eslint clean.
|
Round 3 - pushed Both items from your full-review pass on P1 fixed (1):
P1 fixed (1 - flagged as 🟡 P3 by you, treated as P1 in our rubric):
P0 fixed (0) Deferred to follow-up issues (0) Open for human input (0) Validation: noise in, signal out @coderabbitai full review |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 25755
✅ Round-3 changes verified
One micro-observation (non-blocking)The Summary: both P1 fixes are correct and tight. The PR is clean across all three review rounds — plugin ordering, safety options, fenced- (๑˃ᴗ˂)ﻌﻌ♡ [approve] |
|
Bot cycle complete. No remaining P0/P1 findings after 3 round(s). CodeRabbit's full review of The one micro-observation in the round-3 verification (" Ready for human review. fewer threads at dusk than at dawn |
2 participants
Summary
Renders LaTeX math in chat messages via the shared
MarkdownRenderer, addressing issue #312. Inline$..$and block$$\n..\n$$math now typeset with KaTeX instead of showing as raw source.Implementation:
remark-math^6,rehype-katex^7,katex^0.16MarkdownRenderer.tsxwires the plugins; KaTeX runs withtrust: false,strict: 'ignore',errorColor: 'currentColor',maxSize: 100,maxExpand: 1000main.tsximportskatex.min.cssonce at app entry to avoid layout shift on first paintindex.cssadds theme-aware overrides so math text inherits the foreground color and.katex-errorroutes through the destructive tokenAll four
MarkdownRendererconsumers (chat, kanban task drawer, beads viewer, file-browser markdown) gain math rendering automatically.Closes #312
Test plan
MarkdownRenderer.test.tsx: 67/67 pass (7 new math cases)npm run lint: cleannpx tsc -b --noEmit: cleannpm run build: succeeds; markdown lazy chunk grew to 178 KB / 56 KB gzip; entry chunk only gained the ~24 KB KaTeX CSS$FoM = \frac{S}{FWHM_n + FWHM_\gamma}$and a block equation; confirm typeset rendering$5still renders as code, not mathBaseline note: 9 pre-existing test failures in
server/lib/{files,upload-reference,beads}.test.tsexist on master at312e273and are not introduced by this PR.Summary by CodeRabbit
New Features
Tests
Style
Residual Review Findings
Surfaced by the multi-agent ce-code-review pass (run
20260522-090830-a5438a5d). Six fixes were already applied as commit8d66ec9; the items below were not auto-applied. None are blockers for this PR — they are concrete test-coverage gaps and small follow-ups, kept here rather than filed as separate issues to avoid backlog churn.Manual follow-ups (testing — concrete fix, defer to next pass):
src/features/markdown/MarkdownRenderer.test.tsx— Add a pinning test for the documented D2 single-dollar false-positive (Bought it for $5 yesterday and $10 today) so any future flip tosingleDollarTextMath: falseis visible. (testing, conf 100)src/features/markdown/MarkdownRenderer.test.tsx— Inline and block math tests do not assert any rendered content; the existing.katexqueries would still pass on an empty span. Add positive content assertions mirroring the issue-312 example test. (testing, conf 75)src/features/markdown/MarkdownRenderer.test.tsx— Add a defense-in-depth test that$\href{javascript:alert(1)}{X}$produces no<a>element, locking the R6trust: falsecontract against future regressions. (kieran-typescript, security, conf 75)src/features/markdown/MarkdownRenderer.test.tsx— Add a test for headings containing math (## The $E=mc^2$ identity) to lock the heading-id walker's behavior on inline-math nodes. (correctness, conf 75)src/features/markdown/MarkdownRenderer.test.tsx— Add a test for nested inline code + math in the same paragraph to lock theprocessChildrenskip-list vsrehype-katexhast replacement interaction. (correctness, conf 75)Advisory (report-only — no action this PR):
maxSize: 100/maxExpand: 1000defaults added in the autofix commit.$$x^2$$(whichmicromark-extension-mathparses as math-text, not display). Documented in the plan; current tests use the multi-line block form.Pre-existing (not introduced by this PR):
react-markdownrebuilds its processor on every render; for streaming chat this re-runsremark-mathandrehype-katexper token. The pattern predates this PR and is amplified by adding math plugins; mitigation (memoize-by-content or debounce streaming re-renders) is best handled as a separate performance pass onMarkdownRenderer.Full per-reviewer findings:
/tmp/compound-engineering/ce-code-review/20260522-090830-a5438a5d/(9 reviewer artifacts +synthesis.md+metadata.json).