Skip to content

fix(telegram): tighten table row-group spacing and drop redundant first bullet #29775

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
krislidimo wants to merge 1 commit into from
Closed

fix(telegram): tighten table row-group spacing and drop redundant first bullet #29775

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 18 additions & 6 deletions gateway/platforms/telegram.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,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:
Expand All @@ -256,12 +256,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}")

# 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))

return "\n\n".join(rendered_rows)
# 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:
Expand Down
80 changes: 73 additions & 7 deletions tests/gateway/test_telegram_format.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -574,10 +574,15 @@ def test_basic_table_rewritten_as_row_groups(self):
)
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.")
Expand All @@ -587,7 +592,8 @@ def test_bare_pipe_table_rewritten(self):
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

Expand All @@ -600,8 +606,12 @@ def test_alignment_separators(self):
)
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 = (
Expand All @@ -616,8 +626,11 @@ def test_two_consecutive_tables_rewritten_separately(self):
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."""
Expand Down Expand Up @@ -655,6 +668,56 @@ def test_single_column_separator_not_matched(self):
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
Expand All @@ -669,7 +732,8 @@ def test_table_rendered_as_bullets(self, adapter):
)
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
Expand All @@ -688,7 +752,9 @@ def test_text_after_table_still_formatted(self, adapter):
# 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 = (
Expand All @@ -705,7 +771,7 @@ def test_multiple_tables_in_single_message(self, adapter):
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
Expand Down