Skip to content

fix(discord): wrap markdown tables in code fences for readable rendering - #35707

Closed
liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:fix/discord-markdown-table-code-fence
Closed

fix(discord): wrap markdown tables in code fences for readable rendering#35707
liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:fix/discord-markdown-table-code-fence

Conversation

@liuhao1024

Copy link
Copy Markdown
Contributor

What does this PR do?

Wraps GFM-style markdown tables in triple-backtick code fences when sending messages to Discord, so pipe characters render as readable monospaced text instead of garbage.

Related Issue

Fixes #21168

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • plugins/platforms/discord/adapter.py: Added _wrap_tables_in_code_fence() helper that detects markdown table patterns (header row + separator row + data rows) and wraps them in ``` fences. Tables already inside code blocks are left untouched. Updated format_message() to call this helper instead of being a no-op passthrough.
  • tests/gateway/test_discord_table_wrap.py: Added 11 tests covering basic wrapping, surrounding text preservation, tables inside existing code fences (untouched), no-table passthrough, pipe-without-separator, empty input, tables without leading pipes, multiple tables, alignment rows, single-column separators, and format_message integration.

How to Test

  1. Run python3 -m pytest tests/gateway/test_discord_table_wrap.py -v — all 11 tests should pass
  2. Run python3 -m pytest tests/gateway/test_discord_system_messages.py tests/gateway/test_discord_bot_filter.py tests/gateway/test_discord_connect.py -v — all 32 existing Discord tests should still pass (no regression)
  3. Send a message via Discord that produces a markdown table (e.g., "compare models") and verify the table renders as a code block instead of garbage text

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/gateway/test_discord_table_wrap.py tests/gateway/test_discord_system_messages.py tests/gateway/test_discord_bot_filter.py tests/gateway/test_discord_connect.py -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

Code Intelligence

  • Analyzed: plugins/platforms/discord/adapter.pyformat_message() (callers: 3 in send/send_multiple_images methods), _wrap_tables_in_code_fence() (new helper)
  • Blast radius: LOW — change is confined to Discord adapter's message formatting; no cross-module impact
  • Related patterns: Telegram adapter's _wrap_markdown_tables() in gateway/platforms/telegram.py (same table detection logic, different output format — Telegram converts to bullet groups, Discord wraps in code fences)

Discord does not render GFM pipe tables natively — raw pipe characters
display as garbage text.  Detect markdown table patterns (header row +
separator row + data rows) and wrap them in triple-backtick code fences
so they render as readable monospaced text.

Tables already inside fenced code blocks are left untouched.

Fixes NousResearch#21168
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists platform/discord Discord bot adapter comp/plugins Plugin system and bundled plugins labels May 31, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Duplicate of #4401 and #16903 — all three PRs implement the same fix (wrapping markdown tables in code fences for Discord). See also #21170 (closed earlier duplicate) and #29593 (box-drawing variant). The parent issue is #21168.

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Summary

Verdict: Approved

Overview

Well-crafted fix wrapping GFM markdown tables in code fences for Discord rendering. Discord lacks native table support, so pipe tables render as garbage — wrapping them in triple backticks gives readable monospaced output.

Looks Good

  • Correct detection: Header row + separator row pattern matching, avoids false positives on pipes-in-text
  • Nested fences: Tables already inside ``` blocks are left untouched (tracked via in_fence state)
  • Edge cases: No pipes, no separator, pipes without separator, empty input, multiple tables, alignment rows, tables without leading/trailing pipes — all handled
  • Comprehensive tests: 11 test cases covering all edge cases
  • Clean integration: format_message called on every Discord send, so all messages benefit
  • No debug artifacts found: Clean diff

Reviewed by Hermes Agent

@liuhao1024

Copy link
Copy Markdown
Contributor Author

@alt-glitch Acknowledged. Both competing PRs (#4401, #16903) have CI failures (no CI run or 2 test failures), while this PR passes all checks. If maintainers prefer the duplicate flag to take precedence, feel free to close this. Otherwise, I believe this PR is ready for merge.

@DiscountDarcy

Copy link
Copy Markdown

We experimented with this PR this morning and got a table like so:

| Artifact | Path | Purpose |
|----------|------|---------|
| Prefab | `Assets/Resources/Prefabs/CombatNetSync.prefab` | NetworkObject + CombatNetSync with serialized empty lists |
| DPO entry | `Assets/Resources/DefaultPrefabObjects.asset` | Registers prefab for FishNet codegen + spawn pipeline |
| Code patch | `Assets/Scripts/Network/NetworkManager.cs` | `SpawnCombatSync()` → `Load` + `Instantiate` + `Spawn` |

That is marginally more readable than without this PR, but it doesn't actually solve the problem of readability.

It seems like it should render an ascii box table (converting pipes to aligned +---+) -- unless the goal here is just to treat the table as text to shield from Discord's parser versus trying to render it 'correctly'

@kshitijk4poor

Copy link
Copy Markdown
Collaborator

Closing as the issue #21168 has been resolved via #53284, which takes the same bullet-group conversion approach Telegram already uses (matching your code-fence wrapping would have been an alternative approach, but the bullet-group pattern was chosen for consistency with the existing Telegram implementation and better mobile readability).

Thanks for the contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/plugins Plugin system and bundled plugins P2 Medium — degraded but workaround exists platform/discord Discord bot adapter type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Discord markdown tables render as garbage — need auto code-fence wrapping

5 participants