fix(telegram): render markdown as html#1667
Conversation
AI replies arrived as Markdown and were sent verbatim, so Telegram clients showed raw `**bold**`, `# heading`, and fenced code blocks. Add a local converter that maps the Markdown subset we emit to Telegram's HTML subset (`<b>`, `<i>`, `<s>`, `<code>`, `<pre>`, `<a>`, `<blockquote>`), thread `parseMode` through TelegramClient, and route every outbound chunk in telegramPoller through the converter with `parse_mode: 'HTML'`. Dangling fenced blocks at chunk boundaries are auto-closed so 4096-char splits stay parseable. Closes #1665
📝 WalkthroughWalkthroughThis PR implements Telegram markdown-to-HTML rendering for the remote control presenter. It extends TelegramClient with an optional parseMode parameter, adds a Markdown-to-Telegram-HTML converter, integrates the converter into TelegramPoller's message delivery flow, and updates all tests to verify parseMode propagation and conversion behavior. ChangesTelegram Markdown-to-HTML Rendering Feature
Sequence Diagram(s)sequenceDiagram
participant Poller as TelegramPoller
participant SendChunk as sendChunk helper
participant Converter as convertMarkdownToTelegramHtml
participant Client as TelegramClient
participant API as Telegram API
Poller->>SendChunk: sendChunk(markdownText, replyMarkup?)
SendChunk->>Converter: convert markdown
Converter-->>SendChunk: HTML text
SendChunk->>Client: sendMessage(html, undefined, {parseMode: 'HTML'})
Client->>API: POST /sendMessage with parse_mode: 'HTML'
API-->>Client: {ok: true, result: {message_id}}
Client-->>SendChunk: messageId
SendChunk-->>Poller: messageId
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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 |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/issues/telegram-message-markdown-render/plan.md (1)
1-20:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMove documentation to
docs/features/folder.According to the coding guidelines, feature documentation should be in
docs/features/<goal>/whiledocs/issues/<goal>/is reserved for bug fixes. Since issue#1665is marked as "[FEATURE]" and adds new Markdown rendering capability, this documentation set should be located at:
docs/features/telegram-message-markdown-render/plan.mddocs/features/telegram-message-markdown-render/spec.mddocs/features/telegram-message-markdown-render/tasks.mdAs per coding guidelines: Create specification-driven development documentation in kebab-case folders:
docs/features/<goal>/for new features,docs/issues/<goal>/for bug fixes,docs/architecture/<goal>/for refactors🤖 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 `@docs/issues/telegram-message-markdown-render/plan.md` around lines 1 - 20, Move the feature docs out of docs/issues and into the feature docs folder: create docs/features/telegram-message-markdown-render/ and relocate plan.md (and any related spec.md and tasks.md) there, update the front-matter/links if any refer to docs/issues paths, and ensure filenames and folder name use kebab-case as described in the guidelines; keep the content and headings the same but remove the docs/issues/telegram-message-markdown-render/* files to avoid duplication and comply with the docs/features/<goal>/ convention for feature work (issue `#1665`).src/main/presenter/remoteControlPresenter/telegram/telegramPoller.ts (1)
648-654:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftChunk Telegram messages after Markdown→HTML conversion to respect Telegram’s 4096-char limit
syncDeliverySegmentsplits withchunkTelegramTextusingTELEGRAM_OUTBOUND_TEXT_LIMIT = 4096on raw markdown (~648-654), butsendChunkthen converts each chunk viaconvertMarkdownToTelegramHtmland sends withparseMode: 'HTML'(~727-744). The HTML conversion can expand content (escaping + inserted tags like<pre>,<code>,<a>,<blockquote>, etc.) and there’s no post-conversion length guard intelegramMarkdown.ts, so a “safe” raw chunk can become oversized and trigger Telegram 400 send/edit failures. Fix by chunking based on the final HTML string length (or using an HTML-aware conservative limit) before callingsendMessage/editMessageText.🤖 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/main/presenter/remoteControlPresenter/telegram/telegramPoller.ts` around lines 648 - 654, syncDeliverySegment currently chunks raw markdown via chunkTelegramText (using TELEGRAM_OUTBOUND_TEXT_LIMIT) before calling sendChunk which runs convertMarkdownToTelegramHtml, but HTML conversion can expand length and exceed Telegram's 4096-char limit; update the flow so chunking happens on the final HTML-safe string (or apply a conservative HTML-aware limit) before calling sendChunk/sendMessage/editMessageText: convert each candidate chunk with convertMarkdownToTelegramHtml and then re-chunk by measuring the resulting HTML length (or reduce raw chunk size by a safe multiplier) to ensure each HTML string is <= TELEGRAM_OUTBOUND_TEXT_LIMIT, keeping logic around syncDeliverySegment, chunkTelegramText, sendChunk, convertMarkdownToTelegramHtml, and the sendMessage/editMessageText calls consistent.
🧹 Nitpick comments (1)
test/main/presenter/remoteControlPresenter/telegramClient.test.ts (1)
66-108: ⚡ Quick winAdd parse_mode coverage for
sendPhotoas well.
sendPhotowas updated to forwardparse_mode, but this suite only verifiessendMessageandeditMessageText. Please add one multipart-form assertion forsendPhoto(..., { parseMode: 'HTML' })to prevent regressions on that path.🤖 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 `@test/main/presenter/remoteControlPresenter/telegramClient.test.ts` around lines 66 - 108, Add a new test that calls TelegramClient.sendPhoto(..., { parseMode: 'HTML' }) and asserts the mocked fetch call hits the '/sendPhoto' endpoint and the multipart form body includes the caption and parse_mode fields; locate tests near sendMessage/editMessageText in telegramClient.test.ts, create a TelegramClient('token'), invoke sendPhoto with a caption containing HTML and parseMode: 'HTML', then inspect vi.mocked(fetch).mock.calls[0] and verify the body contains the multipart parts for name="caption" with the caption text and name="parse_mode" with value "HTML" so parse_mode forwarding is covered for sendPhoto.
🤖 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 `@test/main/presenter/remoteControlPresenter/telegramMarkdown.test.ts`:
- Around line 65-67: The test name is misleading: it claims to verify the
fallback branch when conversion throws but the input 'plain <tag>' doesn't cause
an exception; either rename the test to reflect normal escaping behavior (e.g.,
change the it description to "returns escaped text for plain input" while
keeping the assertion against convertMarkdownToTelegramHtml('plain <tag>')), or
modify the test to force an internal throw by mocking the underlying converter
used by convertMarkdownToTelegramHtml (stub the converter to throw and then
assert the function returns the escaped fallback 'plain <tag>'); reference
convertMarkdownToTelegramHtml and the test file telegramMarkdown.test.ts when
making the change.
---
Outside diff comments:
In `@docs/issues/telegram-message-markdown-render/plan.md`:
- Around line 1-20: Move the feature docs out of docs/issues and into the
feature docs folder: create docs/features/telegram-message-markdown-render/ and
relocate plan.md (and any related spec.md and tasks.md) there, update the
front-matter/links if any refer to docs/issues paths, and ensure filenames and
folder name use kebab-case as described in the guidelines; keep the content and
headings the same but remove the docs/issues/telegram-message-markdown-render/*
files to avoid duplication and comply with the docs/features/<goal>/ convention
for feature work (issue `#1665`).
In `@src/main/presenter/remoteControlPresenter/telegram/telegramPoller.ts`:
- Around line 648-654: syncDeliverySegment currently chunks raw markdown via
chunkTelegramText (using TELEGRAM_OUTBOUND_TEXT_LIMIT) before calling sendChunk
which runs convertMarkdownToTelegramHtml, but HTML conversion can expand length
and exceed Telegram's 4096-char limit; update the flow so chunking happens on
the final HTML-safe string (or apply a conservative HTML-aware limit) before
calling sendChunk/sendMessage/editMessageText: convert each candidate chunk with
convertMarkdownToTelegramHtml and then re-chunk by measuring the resulting HTML
length (or reduce raw chunk size by a safe multiplier) to ensure each HTML
string is <= TELEGRAM_OUTBOUND_TEXT_LIMIT, keeping logic around
syncDeliverySegment, chunkTelegramText, sendChunk,
convertMarkdownToTelegramHtml, and the sendMessage/editMessageText calls
consistent.
---
Nitpick comments:
In `@test/main/presenter/remoteControlPresenter/telegramClient.test.ts`:
- Around line 66-108: Add a new test that calls TelegramClient.sendPhoto(..., {
parseMode: 'HTML' }) and asserts the mocked fetch call hits the '/sendPhoto'
endpoint and the multipart form body includes the caption and parse_mode fields;
locate tests near sendMessage/editMessageText in telegramClient.test.ts, create
a TelegramClient('token'), invoke sendPhoto with a caption containing HTML and
parseMode: 'HTML', then inspect vi.mocked(fetch).mock.calls[0] and verify the
body contains the multipart parts for name="caption" with the caption text and
name="parse_mode" with value "HTML" so parse_mode forwarding is covered for
sendPhoto.
🪄 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: 6e4cc9a1-88ff-4b4c-b698-ece2e4b2605d
📒 Files selected for processing (9)
docs/issues/telegram-message-markdown-render/plan.mddocs/issues/telegram-message-markdown-render/spec.mddocs/issues/telegram-message-markdown-render/tasks.mdsrc/main/presenter/remoteControlPresenter/telegram/telegramClient.tssrc/main/presenter/remoteControlPresenter/telegram/telegramMarkdown.tssrc/main/presenter/remoteControlPresenter/telegram/telegramPoller.tstest/main/presenter/remoteControlPresenter/telegramClient.test.tstest/main/presenter/remoteControlPresenter/telegramMarkdown.test.tstest/main/presenter/remoteControlPresenter/telegramPoller.test.ts
| it('returns escaped text when conversion throws', () => { | ||
| expect(convertMarkdownToTelegramHtml('plain <tag>')).toBe('plain <tag>') | ||
| }) |
There was a problem hiding this comment.
This test name doesn’t match what it verifies.
The case does not force convertMarkdownToTelegramHtml to throw, so it doesn’t exercise the catch/fallback branch. Either rename it to plain escaping behavior or mock an internal throw path and assert fallback output.
🤖 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 `@test/main/presenter/remoteControlPresenter/telegramMarkdown.test.ts` around
lines 65 - 67, The test name is misleading: it claims to verify the fallback
branch when conversion throws but the input 'plain <tag>' doesn't cause an
exception; either rename the test to reflect normal escaping behavior (e.g.,
change the it description to "returns escaped text for plain input" while
keeping the assertion against convertMarkdownToTelegramHtml('plain <tag>')), or
modify the test to force an internal throw by mocking the underlying converter
used by convertMarkdownToTelegramHtml (stub the converter to throw and then
assert the function returns the escaped fallback 'plain <tag>'); reference
convertMarkdownToTelegramHtml and the test file telegramMarkdown.test.ts when
making the change.
2 participants
Summary
convertMarkdownToTelegramHtml(mirrors the existingfeishuMarkdown.tspattern) that maps the Markdown subset our AI replies emit into Telegram's HTML subset (<b>,<i>,<s>,<code>,<pre>,<a>,<blockquote>).parseModethroughTelegramClient.sendMessage/editMessageText/sendPhoto, and route every outbound chunk intelegramPollerthrough the converter withparse_mode: 'HTML'.Closes #1665
Test plan
pnpm test test/main/presenter/remoteControlPresenter/— 224/224 pass (incl. 12 newtelegramMarkdowncases + parseMode forwarding tests on client/poller).pnpm run typecheckclean.pnpm run lint0 warnings.pnpm run format/pnpm run i18nclean.**bold**, headings, a fenced ```ts code block, lists, and a link — verify rendering on a real Telegram client.Summary by CodeRabbit
New Features
Tests