Chat: render inline markdown in messages - #3484
Conversation
Bold, italic, strikethrough and inline code, rendered as nested Text. Block constructs are deliberately left alone: a message starting with '-' or '#' keeps reading the way it was typed, and the existing link, mention, emoji and image handling stays on its current code path. The risk here is not missing formatting, it is mangling ordinary chat, so every ambiguous case resolves toward leaving text untouched. Underscore emphasis does not fire inside a word (snake_case survives), a delimiter followed by a space does not open (2 * 3 * 4 survives), unterminated delimiters stay literal, and a span whose content is only delimiters does not format. URLs get a second, independent guard: anything inside a linkify range is passed through as a raw string, so a URL is never split into styled nodes and stays tappable via Hyperlink.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe PR adds inline Markdown parsing for chat text. It supports bold, italic, strikethrough, and code spans. Chat formatting applies styles while preserving URLs and mentions. Unit and integration tests cover syntax, nesting, invalid delimiters, link handling, and performance. ChangesChat Markdown rendering
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related issues
Poem
✨ 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: 8165e17a57
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| type Style = Omit<InlineSpan, 'text'>; | ||
|
|
||
| const isWordChar = (ch: string | undefined) => !!ch && /[A-Za-z0-9]/.test(ch); |
There was a problem hiding this comment.
For identifiers containing non-ASCII letters or consecutive underscores, this ASCII-only predicate lets _ open and close inside the identifier. For example, привет_мир_тест renders мир as italic, while foo__bar__baz renders bar as italic and removes two underscores, contradicting the parser's guarantee that ordinary variable names remain intact. Use a Unicode-aware identifier predicate that also treats adjacent underscores as word characters.
Useful? React with 👍 / 👎.
| if (!slice) { | ||
| return; | ||
| } | ||
| parseInlineMarkdown(slice).forEach((span, i) => { |
There was a problem hiding this comment.
When a markdown pair encloses a link, each side of the link is parsed independently, so neither side can find its matching delimiter. For example, **see https://example.com/foo now** displays the literal ** and applies no bold formatting. The URL can remain a raw string for Hyperlink, but delimiter matching must span across the protected link range.
Useful? React with 👍 / 👎.
| ); | ||
| } | ||
| return part.text; | ||
| return renderSegmentWithMarkdown(part.text, part.start, linkRanges, textStyle, `s${index}_`); |
There was a problem hiding this comment.
When formatting encloses a mention along with surrounding text, splitting the message into mention and non-mention parts prevents the delimiters from matching. For example, ~~ignore @bob please~~ leaves both ~~ markers visible and does not strike the text because the prefix and suffix are parsed separately. Parse the formatting across the complete message while treating the mention as a styled span.
Useful? React with 👍 / 👎.
| spans.push(...parse(content, style, depth + 1)); | ||
| } | ||
|
|
||
| spans.push(...parse(text.slice(match.end), inherited, depth)); |
There was a problem hiding this comment.
For a long message containing many sequential valid spans such as repeated *x* , this tail-recursive call adds one stack frame per span and reruns findMatch over every remaining suffix, producing quadratic work despite the nesting-depth guard. A roughly 16 KB message can already block the JS thread for close to a second, and longer server-supplied messages can overflow the stack while rendering; process sequential matches iteratively instead.
Useful? React with 👍 / 👎.
|
@coderabbitai review This merged without a review (rate limit). The inline markdown parser in |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== Pull request metadata and changed files =='
gh pr view 3484 --repo ecency/vision-mobile --json number,state,mergedAt,baseRefName,headRefName,files --jq '
"state=\(.state) mergedAt=\(.mergedAt) base=\(.baseRefName) head=\(.headRefName)",
(.files[] | "\(.path)")
echo
echo '== Candidate source and test files =='
fd -a -i 'inlineMarkdown' . || true
fd -a -i 'chat' . | head -100 || true
echo
echo '== Parser file outline =='
ast-grep outline src/screens/chats/utils/inlineMarkdown.ts --items all --view expanded 2>/dev/null || true
echo
echo '== Parser file =='
cat -n src/screens/chats/utils/inlineMarkdown.ts 2>/dev/null || true
echo
echo '== Direct parser and renderer references =='
rg -n -C 4 --glob '!node_modules/**' --glob '!dist/**' \
'\b(parseInlineMarkdown|inlineMarkdown)\b' src 2>/dev/null || trueLength of output: 1266 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== Pull request metadata =='
gh pr view 3484 --repo ecency/vision-mobile \
--json number,state,mergedAt,baseRefName,headRefName \
--jq '"state=\(.state) mergedAt=\(.mergedAt) base=\(.baseRefName) head=\(.headRefName)"'
echo
echo '== Pull request changed files =='
gh pr view 3484 --repo ecency/vision-mobile --json files --jq '.files[].path'
echo
echo '== Candidate source and test files =='
fd -a -i 'inlineMarkdown' . || true
echo
echo '== Parser file outline =='
ast-grep outline src/screens/chats/utils/inlineMarkdown.ts --items all --view expanded 2>/dev/null || true
echo
echo '== Parser file =='
cat -n src/screens/chats/utils/inlineMarkdown.ts 2>/dev/null || true
echo
echo '== Direct parser and renderer references =='
rg -n -C 4 --glob '!node_modules/**' --glob '!dist/**' \
'\b(parseInlineMarkdown|inlineMarkdown)\b' src 2>/dev/null || trueLength of output: 18355 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== Changed pull request diff =='
gh pr diff 3484 --repo ecency/vision-mobile -- \
src/screens/chats/utils/inlineMarkdown.ts \
src/screens/chats/utils/messageFormatters.tsx \
src/screens/chats/utils/inlineMarkdown.test.ts \
src/screens/chats/utils/messageMarkdown.test.tsx
echo
echo '== messageFormatters outline =='
ast-grep outline src/screens/chats/utils/messageFormatters.tsx --items all --view expanded 2>/dev/null || true
echo
echo '== Formatter range containing Markdown integration =='
sed -n '170,285p' src/screens/chats/utils/messageFormatters.tsx
echo
echo '== Parser tests =='
cat -n src/screens/chats/utils/inlineMarkdown.test.ts
echo
echo '== Renderer integration tests =='
cat -n src/screens/chats/utils/messageMarkdown.test.tsx
echo
echo '== Text component and hyperlink call sites in formatter =='
rg -n -C 6 '<(Text|Hyperlink)\b|renderTextWithBoldMentions\s*\(' \
src/screens/chats/utils/messageFormatters.tsxLength of output: 1355 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== Changed pull request diff =='
gh pr diff 3484 --repo ecency/vision-mobile
echo
echo '== Formatter range containing Markdown integration =='
sed -n '170,285p' src/screens/chats/utils/messageFormatters.tsx
echo
echo '== Parser tests =='
cat -n src/screens/chats/utils/inlineMarkdown.test.ts
echo
echo '== Renderer integration tests =='
cat -n src/screens/chats/utils/messageMarkdown.test.tsx
echo
echo '== Text component and hyperlink call sites in formatter =='
rg -n -C 6 '<(Text|Hyperlink)\b|renderTextWithBoldMentions\s*\(' \
src/screens/chats/utils/messageFormatters.tsx || trueLength of output: 38048
Use a Unicode-aware letter-or-number check for the underscore boundary logic. Add regression tests for accented Latin and Cyrillic examples. This preserves the stated rule that
|
1 participant
Part of #3468, scoped to the inline subset (see the issue comment for why block-level parity was deferred).
What renders now
**bold**,*italic*/_italic_,~~strike~~,`code`— as nested<Text>. Block constructs are deliberately untouched, so a message starting with-or#still reads the way it was typed, and the existing link, mention, emoji and image handling stays on its current code path.The design constraint
The risk is not missing formatting, it is mangling ordinary chat. Every ambiguous case resolves toward leaving text alone:
file_name_here_does not open inside a word2 * 3 * 4 = 24*hello there***`a **b** c`URLs get a second, independent guard
Anything inside a linkify range is passed through as a raw string, so a URL is never split into styled nodes and stays tappable via
<Hyperlink>. This matters because a URL containing_could otherwise be reformatted into something that no longer resolves.The two defences are independent, which I verified by removing the link-range guard: the underscore cases still passed on the parser rule alone, and only the
a*b*cURL case failed. Both are kept.Tests
23 new. The parser is covered directly; a separate integration spec covers the wiring, since the parser being right does not prove the renderer uses it correctly.
One regression is pinned specifically: an invalid opening delimiter must keep scanning the same rule rather than abandoning it, so
2 * 3 and *italic*still finds the real pair. I broke exactly this while removing acontinuefor the lint rule, and the test now catches it.Checks
typecheck ok: 0 errors (baseline 0). Full suite 790 passed / 55 suites. Lint clean on the chat directory (0 errors; 25 pre-existing warnings unchanged).Summary by CodeRabbit
New Features
Tests