fix(chat): stop inline-code CSS from leaking into fenced code blocks - #402
Conversation
The `:not(pre) > code` selectors in base.css assumed a fenced code block's <code> is always a direct child of a real <pre>. That breaks for syntax-highlighted blocks: SyntaxHighlighter's PreTag="div" puts a wrapper <div> between <code> and the outer <pre>, so the selector misfires and applies inline-code padding to highlighted blocks too. Since <code> is an inline element spanning multiple lines, left padding only paints on the first line fragment per the CSS box model - visible as a first-line-only indent, invisible in copied text (pure paint, not a text difference). Replace the parent-structure guess with an explicit marker set in JS: resolveCodeClassName (new, dependency-free module so it's unit testable under the current Jest/ESM constraint - react-markdown's ESM export syntax can't be parsed without a jest.config.js change) tags genuine inline code with .inline-code based on content shape (no trailing newline), since react-markdown@9 no longer passes an `inline` prop to distinguish it directly. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
pjdoland
left a comment
There was a problem hiding this comment.
Thank you for tracking this down, and for the write-up in the module comment. I confirmed the premise rather than taking it on faith: SyntaxHighlighter is rendered with PreTag="div", so a highlighted block's <code> sits inside a div, not a pre. :not(pre) > code therefore matched it and dressed fenced code in the inline pill treatment. Classifying by content shape instead of DOM nesting is the right move, since the nesting is exactly the thing the highlighter changes out from under you.
I checked the heuristic against the real remark-rehype pipeline rather than reasoning about it:
| input | code text | classified |
|---|---|---|
inline `x = 1` |
"x = 1" |
inline |
| fenced with language | "x = 1\n" |
block |
| fenced without language | "x = 1\n" |
block |
| indented code block | "indented = 1\n" |
block |
| inline spanning a source newline | "multi word" |
inline |
That last row is the one I most expected to break the rule, since the source contains a newline; remark normalizes it to a space before it reaches the code node, so it still classifies correctly.
Keeping markdown-code-class.ts free of React and react-markdown imports is a good call, and the comment explaining why (the ESM parse failure that forced the MarkdownRenderer tests to be reverted in #385) is the kind of note that saves the next person an afternoon. Both :not(pre) > code selectors are converted with none left behind, and .nbi-form-hint code is correctly untouched since it renders through plain JSX rather than the markdown renderer.
Verified on your branch: tsc --noEmit clean, jest green at 381 across 31 suites, so the five new cases are running.
One thing worth fixing
A fenced block with no content classifies as inline and picks up the pill styling:
resolveCodeClassName("", undefined) -> "inline-code"
mdast-util-to-hast's code handler is node.value ? node.value + '\n' : '', so an empty code node yields '' rather than '\n'. The "has a newline" test then comes up false and a real <pre><code> gets .inline-code, which is the same misfire the PR is fixing, just reached a different way.
I initially wrote this off as a rare literal empty fence. That undersells it, and the streaming path is the reason. MarkdownPart content re-renders on every partial chunk, so a fence arrives incrementally and passes through a bare ``` state before its language token lands. I simulated a typical response arriving character by character and every fenced block hits that state exactly once:
'Here:\n\n```' -> code content '' -> class "inline-code"
It flips to the highlighter branch as soon as '```p' streams in, so it is transient, but it fires on essentially every response containing a code block rather than on an unusual input. A brief pill flash on each block during generation is the visible symptom.
The one-line version closes both the streaming flash and the literal empty fence:
const isTrulyInline = String(children).length > 0 && !String(children).includes('\n');I ran all five existing tests against that change and they stay green.
Optional
The test file covers the main cases well, and the gap above is exactly what it does not reach: every case uses non-empty content. A resolveCodeClassName('') case would pin the fix, and an inline-spanning-a-source-newline case would lock in the normalization behavior that makes this heuristic safe in the first place.
The diagnosis and the approach are both right, and this fixes a real and visible bug. I am requesting changes only for the empty-content case, since without it a pill still flashes on every streamed code block, which is the same class of misfire the PR sets out to remove. It is a one-line change and the existing tests stay green, so I would be glad to re-review as soon as you have pushed it.
Summary
Fixes #401
style/base.css's:not(pre) > codeselector (two occurrences) assumed a fenced code block's<code>is always the direct child of a real<pre>. That breaks for syntax-highlighted blocks:markdown-renderer.tsx'sSyntaxHighlighterusesPreTag="div", so a highlighted block's<code>sits under a wrapper<div>, not directly under the outer<pre>(which still exists further up, fromremark-rehype's default wrapping — onlycodeis overridden, notpre).padding: 1px 4pxto highlighted blocks too. Since<code>is a CSS inline element spanning multiple lines via literal\n+white-space: pre, left padding on a wrapped inline box only paints on the first line fragment per the CSS box model — visible as a first-line-only indent, invisible in copied text (pure paint, not a text/DOM content difference — confirmed via direct DOM inspection and copy/paste testing)..inline-codeclass assigned in JS (src/markdown-code-class.ts, new — kept dependency-free so it's unit-testable, see Test plan).react-markdown@9no longer passes aninlineprop (removed upstream, so the existingif (inline || !match)branch was already relying on!matchalone), so the reliable signal is content shape:remark-rehypealways appends a trailing\nto fenced code (even single-line), while true inline code never contains one.See #401 for the full root-cause writeup, screenshot, and reproduction.
Test plan
tsc --noEmit— cleanresolveCodeClassName(tests/ts/markdown-code-class.test.ts) covering: true inline code, inline code with an existing className, a highlighted fenced block, a fenced block with no/unrecognized language (multi- and single-line):not(pre) > code) vs. new (code.inline-code) selector text against the real captured DOM shape from the live app — confirmspadding-left: 4pxfires on the old selector for a highlighted block (the bug, reproduced exactly) and not on the new oneNote: a direct Jest-level render test of
<MarkdownRenderer>itself isn't feasible under the currentjest.config.js(notransformIgnorePatternsoverride, so importingreact-markdown's ESMexportsyntax fails at parse time) — same constraint hit and accepted in #385's work.resolveCodeClassNamewas extracted into its own dependency-free module specifically to get real unit coverage despite that constraint.🤖 Generated with Claude Code