Skip to content

fix(chat): stop inline-code CSS from leaking into fenced code blocks - #402

Merged
mbektas merged 1 commit into
plmbr:mainfrom
FelipeRamos-neuro:fix/inline-code-css-leaks-into-fenced-blocks
Aug 22, 2026
Merged

fix(chat): stop inline-code CSS from leaking into fenced code blocks#402
mbektas merged 1 commit into
plmbr:mainfrom
FelipeRamos-neuro:fix/inline-code-css-leaks-into-fenced-blocks

Conversation

@FelipeRamos-neuro

Copy link
Copy Markdown
Contributor

Summary

Fixes #401

  • style/base.css's :not(pre) > code selector (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's SyntaxHighlighter uses PreTag="div", so a highlighted block's <code> sits under a wrapper <div>, not directly under the outer <pre> (which still exists further up, from remark-rehype's default wrapping — only code is overridden, not pre).
  • The selector misfired and applied inline-code padding: 1px 4px to 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).
  • Fix: replace the parent-structure guess with an explicit .inline-code class assigned in JS (src/markdown-code-class.ts, new — kept dependency-free so it's unit-testable, see Test plan). react-markdown@9 no longer passes an inline prop (removed upstream, so the existing if (inline || !match) branch was already relying on !match alone), so the reliable signal is content shape: remark-rehype always appends a trailing \n to 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 — clean
  • Full Jest suite — 31 suites / 381 tests passing, no regressions
  • 5 new unit tests for resolveCodeClassName (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)
  • Synthetic jsdom test running the actual old (:not(pre) > code) vs. new (code.inline-code) selector text against the real captured DOM shape from the live app — confirms padding-left: 4px fires on the old selector for a highlighted block (the bug, reproduced exactly) and not on the new one
  • Manual verification in a running JupyterLab chat panel (not done — no way to launch the dev UI in this environment; Fenced code blocks render with a cosmetic first-line indent — :not(pre) > code misfires on highlighted blocks #401 has the live screenshot this fix addresses)

Note: a direct Jest-level render test of <MarkdownRenderer> itself isn't feasible under the current jest.config.js (no transformIgnorePatterns override, so importing react-markdown's ESM export syntax fails at parse time) — same constraint hit and accepted in #385's work. resolveCodeClassName was extracted into its own dependency-free module specifically to get real unit coverage despite that constraint.

🤖 Generated with Claude Code

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>
@FelipeRamos-neuro

Copy link
Copy Markdown
Contributor Author

@pjdoland @mbektas could one of you take a look when you have a chance?

@mbektas mbektas left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@mbektas
mbektas merged commit 7fe83de into plmbr:main Aug 22, 2026
3 of 4 checks passed

@pjdoland pjdoland left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

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.

Fenced code blocks render with a cosmetic first-line indent — :not(pre) > code misfires on highlighted blocks

3 participants