fix(telegram): repair model-collapsed/pre-escaped pipe tables (#53632) - #54302
fix(telegram): repair model-collapsed/pre-escaped pipe tables (#53632)#54302youngting520 wants to merge 1 commit into
Conversation
Competing fix for #53632 alongside #53661. This PR normalizes collapsed/pre-escaped pipe tables in the Telegram adapter ( |
|
Hourly commander review on head What I verified locally in a detached worktree:
|
|
Thanks for the automated triage and verification passes above. A few notes to help with the "canonical layer" decision: Why this lives in the adapter It repairs the input shape, not just the escaping. The root payload is both pre-escaped ( How this relates to the open rich-table work. This change is an input normalization step, not another renderer. #45327 (preserve raw pipe syntax on the rich path) and #49258 (convert pipe tables to native HTML) both operate on an already-well-formed table and reuse the existing separator-on-its-own-line detection — so a collapsed/pre-escaped payload like the one in #53632 isn't recognized by either of them today. This PR repairs that input shape before the rich/legacy split, which is what lets those native-rendering paths actually receive a table to render. It composes with either of them rather than overlapping, so it can stand alone or land underneath whichever rendering layer a maintainer settles on. Scope. Two files, no unrelated changes: one normalizer plus a single call site, then it hands off to the existing table renderer (no second rendering path). It's idempotent and a no-op for any content without a pipe-bounded table row — prose, arithmetic, and fenced code are left verbatim, with negative-control tests covering those. Happy to rebase onto or fold this into whichever layer is chosen as canonical. |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for isolating the collapsed/pre-escaped delivery shape. Current main still has the reported detection gap: rich routing requires a separator on its own line (plugins/platforms/telegram/adapter.py:1405), and the legacy converter requires that separator to be the next line (gateway/platforms/helpers.py:386-396).
Problems
plugins/platforms/telegram/adapter.py:300unconditionally turns every\|into|within a pipe-bounded row.\|is also valid GFM syntax for a literal pipe inside a cell. On the raw rich path,send()forwards that altered markdown beforeformat_message()(plugins/platforms/telegram/adapter.py:3592;_rich_message_payload()at:1511-1518), changing cell boundaries and potentially losing content.
Suggested changes
- Detect an all-structural pre-escaped table shape before unescaping; preserve literal-pipe cell escapes.
- Add a regression for a well-formed table containing a
\|cell value on both rich and legacy paths.
Automated hermes-sweeper review.
ee51f16 to
87f0ba9
Compare
87f0ba9 to
9a2192b
Compare
9a2192b to
85f4d28
Compare
85f4d28 to
062ce34
Compare
…#53632) Normalize only structurally verified pre-escaped tables, preserve valid escaped-pipe cells across rich and legacy delivery, and apply the same repair to finalized streaming edits. Keep fenced and indented code untouched and reject collapsed tables whose separator is not the second row.
062ce34 to
8781dee
Compare
Summary
Repairs Telegram pipe tables that arrive with both of these model-generated defects:
\| Date \| Event \|); andThe repair runs before Telegram chooses the rich or legacy delivery path, and also runs when a streaming edit is finalized. Well-formed GFM tables, literal escaped pipes inside cells, prose, and code blocks remain unchanged.
This revision is rebased directly onto current
mainat9712b8f0c. It keeps the adapter-local normalization boundary and preserves main's newer shared link-formatting extraction.Issue status
Addresses #53632.
The reporter closed that issue after confirming the documented workaround—requesting plain GFM with one row per line and no pre-escaped structural pipes. The underlying adapter defect was not fixed by that workaround, and a later comment confirmed the same reproduction in a regular Telegram DM outside cron delivery. This PR provides the product-level fix.
Root cause
The existing Telegram paths only recognize a GFM table when the separator is on its own line. A collapsed payload therefore bypasses both rich-table routing and the legacy table-to-bullets converter. Generic MarkdownV2 escaping then escapes the already escaped bars again, exposing literal
\|text to the recipient.A separate but related problem exists in the shared table realigner:
split_table_row()uses plainstr.split("|"), which treats a legal escaped pipe inside a cell as a column boundary.Fix
agent.markdown_tables.split_table_row()respect Markdown backslash parity: an odd run escapes a literal pipe, while an even run leaves a structural delimiter.TelegramAdapter.send()and finalizededit_message()delivery, while leaving intermediate plain-text previews unchanged.The repaired table feeds the existing rich renderer or legacy bullet conversion; this does not add another rendering path.
Review feedback addressed
The original implementation unconditionally changed every
\|inside a bounded row. In response to review, this revision separates pre-escaped structure recognition from legal escaped-pipe cell parsing and adds rich, legacy, Discord, final-edit, ambiguity, atomicity, protected-code, and backslash-parity regressions.Files changed
agent/markdown_tables.py— escaped-pipe-aware row splitting and safe horizontal re-rendering.plugins/platforms/telegram/adapter.py— conservative table normalization before rich/legacy routing and on finalized edits.tests/agent/test_markdown_tables.py— shared parser and realigner contracts.tests/gateway/test_table_helpers.py— legacy table conversion coverage.tests/gateway/test_telegram_format.py— normalization, legacy delivery, final-edit, protected-region, ambiguity, and atomicity coverage.tests/gateway/test_telegram_rich_messages.py— rich delivery regressions.tests/gateway/test_discord_format.py— downstream escaped-pipe regression coverage.Validation
The focused run covers Telegram formatting and delivery, Telegram rich messages and rich-newline handling, shared markdown-table behavior, legacy table helpers, and Discord formatting.
Risk
The normalizer is deliberately conservative. It requires a complete, structurally consistent table and performs an atomic whole-block conversion. Ambiguous escaping, inconsistent widths, misplaced or duplicate separators, valid literal-pipe cells, prose, and protected code regions are preserved unchanged.