From 9d10c45e3222af8244b158149c13180ca9ca9cec Mon Sep 17 00:00:00 2001 From: Krisli Dimo Date: Thu, 21 May 2026 10:22:46 +0000 Subject: [PATCH] fix(telegram): tighten table row-group spacing and drop redundant first bullet MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The GFM → Telegram-row-group rewriter previously joined every line in every row with a blank line ("\n\n".join(rendered_rows)), which made multi-column tables explode into one-bullet-per-paragraph walls on mobile. It also emitted the row heading twice when the table had no row-label column: once as the standalone bold heading and once again as the first labeled bullet (heading == headers[0] == data_cells[0]). This commit: * Uses single newlines between the heading and its bullets within a row-group, and a blank line only BETWEEN row-groups. * Skips any bullet whose value duplicates the heading text when the table has no row-label column (the heading already carries that information). Tables WITH a row-label column are unaffected since the heading comes from the label cell and never duplicates a header. Updated existing test assertions accordingly and added two regression tests: one that reproduces the screenshot bug (wide five-column "Plays" comparison table) and one that pins the row-label-column behavior so the dedup logic doesn't accidentally swallow real data. tests/gateway/test_telegram_format.py: 101 passed --- gateway/platforms/telegram.py | 24 ++++++-- tests/gateway/test_telegram_format.py | 80 ++++++++++++++++++++++++--- 2 files changed, 91 insertions(+), 13 deletions(-) diff --git a/gateway/platforms/telegram.py b/gateway/platforms/telegram.py index 64fbaee98db..300fc49c04f 100644 --- a/gateway/platforms/telegram.py +++ b/gateway/platforms/telegram.py @@ -240,7 +240,7 @@ def _render_table_block_for_telegram(table_block: list[str]) -> str: first_data_row = _split_markdown_table_row(table_block[2]) if len(table_block) > 2 else [] has_row_label_col = len(first_data_row) == len(headers) + 1 - rendered_rows: list[str] = [] + rendered_groups: list[str] = [] for index, row in enumerate(table_block[2:], start=1): cells = _split_markdown_table_row(row) if has_row_label_col: @@ -258,12 +258,24 @@ def _render_table_block_for_telegram(table_block: list[str]) -> str: elif len(data_cells) > len(headers): data_cells = data_cells[: len(headers)] - rendered_rows.append(f"**{heading}**") - rendered_rows.extend( - f"• {header}: {value}" for header, value in zip(headers, data_cells) - ) + # Build the bulleted lines for this row. Skip any bullet whose value + # duplicates the heading text -- when has_row_label_col is False the + # heading IS the first data cell, and emitting it twice (once as the + # bold heading, once as the first bullet) is visual noise. + bullets: list[str] = [] + for header, value in zip(headers, data_cells): + if not has_row_label_col and value == heading: + continue + bullets.append(f"• {header}: {value}") - return "\n\n".join(rendered_rows) + # Within a row-group: single newline between heading and its bullets, + # and between successive bullets. This keeps the row visually tight + # on Telegram instead of stretching each bullet into its own paragraph. + group_lines = [f"**{heading}**", *bullets] + rendered_groups.append("\n".join(group_lines)) + + # Between row-groups: blank line so each group reads as a distinct block. + return "\n\n".join(rendered_groups) def _wrap_markdown_tables(text: str) -> str: diff --git a/tests/gateway/test_telegram_format.py b/tests/gateway/test_telegram_format.py index 688bdc7269d..c8fb121a173 100644 --- a/tests/gateway/test_telegram_format.py +++ b/tests/gateway/test_telegram_format.py @@ -574,10 +574,15 @@ class TestWrapMarkdownTables: ) out = _wrap_markdown_tables(text) assert "**Alice**" in out - assert "• Player: Alice" in out + # The heading IS the Player cell — don't repeat it as a bullet. + assert "• Player: Alice" not in out assert "• Score: 150" in out assert "**Bob**" in out assert "• Score: 120" in out + # Heading and its bullet sit on consecutive lines (no blank between). + assert "**Alice**\n• Score: 150" in out + # Separate row groups ARE separated by a blank line. + assert "• Score: 150\n\n**Bob**" in out # Surrounding prose is preserved assert out.startswith("Scores:") assert out.endswith("End.") @@ -587,7 +592,8 @@ class TestWrapMarkdownTables: text = "head1 | head2\n--- | ---\na | b\nc | d" out = _wrap_markdown_tables(text) assert out.startswith("**a**") - assert "• head1: a" in out + # No duplicate first bullet — heading 'a' already shows the head1 value. + assert "• head1: a" not in out assert "• head2: b" in out assert "**c**" in out @@ -600,8 +606,12 @@ class TestWrapMarkdownTables: ) out = _wrap_markdown_tables(text) assert "**Ada**" in out + # 'Ada' is the heading (first cell); skip the redundant Name bullet. + assert "• Name: Ada" not in out assert "• Age: 30" in out assert "• City: NYC" in out + # All three lines pack tightly with single newlines. + assert "**Ada**\n• Age: 30\n• City: NYC" in out def test_two_consecutive_tables_rewritten_separately(self): text = ( @@ -616,8 +626,11 @@ class TestWrapMarkdownTables: out = _wrap_markdown_tables(text) assert out.count("**1**") == 1 assert out.count("**9**") == 1 - assert "• A: 1" in out - assert "• X: 9" in out + # Headings duplicate first cells (no row-label col) — skip those bullets. + assert "• A: 1" not in out + assert "• X: 9" not in out + assert "• B: 2" in out + assert "• Y: 8" in out def test_plain_text_with_pipes_not_wrapped(self): """A bare pipe in prose must NOT trigger wrapping.""" @@ -655,6 +668,56 @@ class TestWrapMarkdownTables: text = "| a |\n| - |\n| b |" assert _wrap_markdown_tables(text) == text + def test_row_group_uses_single_newlines_within_group(self): + """Regression: each bullet within a row-group must be separated by + a single newline, not a blank line. Telegram renders blank lines + as paragraph breaks, which previously left every bullet floating in + its own paragraph and made multi-column tables unreadable. + + Mirrors the exact pattern that produced the screenshot bug report: + a five-column comparison table with no row-label column. + """ + text = ( + "| Play | Capital | Build | $/day | Risk |\n" + "|---|---|---|---|---|\n" + "| A. Copy Hands (HK/SZ) | $5-10k | 2 wk | $30-70 | Low |\n" + "| B. NO-sweeper | $50-100k | 3 wk | $300-1000 | Med |" + ) + out = _wrap_markdown_tables(text) + + # No bullet sits inside its own paragraph: the substring "\n\n• " + # would mean a blank line precedes a bullet, which is the bug. + assert "\n\n• " not in out + + # The two row-groups DO have a paragraph break between them. + groups = [g for g in out.split("\n\n") if g.strip()] + assert len(groups) == 2 + # Heading + 4 bullets per group means each group is exactly 5 lines. + for group in groups: + line_count = group.count("\n") + 1 + assert line_count == 5, ( + "Each row-group should be 5 lines (heading + 4 bullets), " + f"got {line_count}:\n{group}" + ) + + def test_row_label_column_preserves_first_bullet(self): + """When the table has a row-label column (data rows have one more + cell than the header row), the heading comes from the label cell + and is distinct from any header — so every header→value bullet is + kept, including the first one.""" + text = ( + "| | Score | Rank |\n" + "|--------|-------|------|\n" + "| Alice | 150 | 1 |\n" + "| Bob | 120 | 2 |\n" + ) + out = _wrap_markdown_tables(text) + assert "**Alice**" in out + # No header to duplicate against — both bullets stay. + assert "• Score: 150" in out + assert "• Rank: 1" in out + assert "**Alice**\n• Score: 150\n• Rank: 1" in out + class TestFormatMessageTables: """End-to-end: pipe tables become readable Telegram-native text instead @@ -669,7 +732,8 @@ class TestFormatMessageTables: ) out = adapter.format_message(text) assert "*A*" in out - assert "• Col1: A" in out + # Heading 'A' duplicates the Col1 value — skip that bullet. + assert "• Col1: A" not in out assert "• Col2: B" in out assert "```" not in out assert "\\|" not in out @@ -688,7 +752,9 @@ class TestFormatMessageTables: # Exclamation outside fence is escaped assert "\\!" in out assert "*1*" in out - assert "• A: 1" in out + # Heading '1' is also the A-column value — skip the redundant bullet. + assert "• A: 1" not in out + assert "• B: 2" in out def test_multiple_tables_in_single_message(self, adapter): text = ( @@ -705,7 +771,7 @@ class TestFormatMessageTables: out = adapter.format_message(text) assert out.count("*1*") == 1 assert out.count("*9*") == 1 - assert "• X: 9" in out + assert "• Y: 8" in out @pytest.mark.asyncio