fix(desktop): keep plain-text fences copyable - #49972
Conversation
|
Additional source-backed validation / reproduction: CommonMark defines the text after an opening fence as the fence’s info string; the code block content starts on the subsequent lines. So when a fenced block explicitly uses the info string I also checked #57540 and #57542. They describe the same Useful acceptance boundary:
This PR looks like the cleaner consolidation target versus #57542 because it is the earlier open PR for the same mechanism/symptom. Source checked: CommonMark fenced code block semantics — https://spec.commonmark.org/0.31.2/#fenced-code-blocks |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for tracing this through both markdown stages. The premise is verified on current main: apps/desktop/src/lib/markdown-code.ts:301-308 classifies explicit plain-text fences with prose bodies as prose, and apps/desktop/src/components/chat/shiki-highlighter.tsx:133-155 independently uses isLikelyProseCodeBlock() to choose a prose div instead of the copyable CodeCard.
Problems
- The added tests cover only preprocessing and only
text(apps/desktop/src/components/assistant-ui/markdown-text.test.ts:73-100). They do not protect the renderer-sideisLikelyProseCodeBlock()change, nor the statedplainandplaintextvariants.
Suggested changes
- Add table-driven helper coverage in
apps/desktop/src/lib/markdown-code.test.tsfortext,plain, andplaintext, asserting prose content remains a code block. - Parameterize the closed and streaming preprocessing cases over those same tags.
Automated hermes-sweeper review.
| expect(output).toContain('const value = 1;') | ||
| }) | ||
|
|
||
| it('keeps explicit text fences copyable instead of leaking the language label into prose', () => { |
There was a problem hiding this comment.
Suggestion: parameterize this regression over text, plain, and plaintext, then add direct isLikelyProseCodeBlock() coverage. The renderer makes a separate copy-card decision through that helper (apps/desktop/src/components/chat/shiki-highlighter.tsx:133), so the current preprocessing-only text case does not cover the full stated contract.
Summary
text,plain, andplaintextfenced blocks in the desktop markdown pipeline.text Hallo....Verification
npm exec vitest run src/components/assistant-ui/markdown-text.test.ts -- --environment nodenpm run typecheckNote
npm run test:ui -- src/components/assistant-ui/markdown-text.test.tsis currently blocked in this checkout by a jsdom dependency ESM/CJS incompatibility inhtml-encoding-snifferrequiring@exodus/bytes/encoding-lite.js; the focused test passes under the node environment because this preprocessor test does not need jsdom.