Skip to content

fix(feishu): use post(md) for messages containing markdown tables - #58391

Closed
liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:liuhao/cron-bugfix-58269-feishu-table-fallback
Closed

fix(feishu): use post(md) for messages containing markdown tables#58391
liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:liuhao/cron-bugfix-58269-feishu-table-fallback

Conversation

@liuhao1024

Copy link
Copy Markdown
Contributor

What does this PR do?

Removes the outdated _MARKDOWN_TABLE_RE guard that forced table-containing Feishu messages to plain text type. Feishu's post(md) element now supports GFM tables (CommonMark 0.31 + GFM), so the workaround is no longer needed and actively harms message quality — it strips all formatting (bold, lists, code blocks) and renders tables as raw pipe characters.

Table detection is merged into _MARKDOWN_HINT_RE so tables trigger post(md) mode like other markdown features.

Related Issue

Fixes #58269

Type of Change

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

Changes Made

  • plugins/platforms/feishu/adapter.py: Removed _MARKDOWN_TABLE_RE regex and its guard in _build_outbound_payload() that forced text type for table content. Added table pattern ^\|.*\|\n\|[-|: ]+\| to _MARKDOWN_HINT_RE so tables trigger post(md) mode.
  • tests/gateway/test_feishu.py: Added test_send_uses_post_for_markdown_table to verify table content is sent as post type with md tag.

How to Test

  1. Run pytest tests/gateway/test_feishu.py::TestAdapterBehavior::test_send_uses_post_for_markdown_table -xvs — should pass.
  2. Run pytest tests/gateway/test_feishu.py -q — all 206 tests should pass.
  3. On a live Feishu bot, send a message containing a markdown table (e.g. | Col A | Col B |\n|---|---|\n| 1 | 2 |). Observed result: the table renders with grid lines and other markdown (bold, lists) is preserved.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):)
  • 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/ -q and all tests pass
  • I've added tests for my changes
  • I've tested on my platform: macOS 15.4

Documentation & Housekeeping

  • I've updated relevant documentation — 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

Feishu's post-type 'md' elements now support GFM tables (CommonMark
0.31 + GFM).  The previous workaround that forced table-containing
messages to plain 'text' type stripped ALL formatting (bold, lists,
code blocks) and rendered the table itself as raw pipe characters.

Remove the _MARKDOWN_TABLE_RE guard and merge table detection into
_MARKDOWN_HINT_RE so tables trigger post(md) mode like other markdown
features.

Fixes NousResearch#58269
@alt-glitch alt-glitch added type/bug Something isn't working comp/gateway Gateway runner, session dispatch, delivery platform/feishu Feishu / Lark adapter sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages P3 Low — cosmetic, nice to have labels Jul 4, 2026

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for tracing this to the table-specific fallback. The premise remains present on current main: plugins/platforms/feishu/adapter.py:4528-4538 sends the reported pipe-bounded table shape as text, and Feishu’s current message-content documentation supports GFM through post/md.

Problems

  • The new alternative at plugins/platforms/feishu/adapter.py:155 requires both the header and separator row to begin with |. GFM allows outer pipes to be omitted, so A | B\n--- | ---\n1 | 2 still misses _MARKDOWN_HINT_RE and takes the text fallback at plugins/platforms/feishu/adapter.py:4535-4538.

Suggested changes

  • Make the outer pipes optional in the table detector and add a regression test for that valid GFM form, asserting post plus an md element.

Automated hermes-sweeper review.


_MARKDOWN_HINT_RE = re.compile(
r"(^#{1,6}\s)|(^\s*[-*]\s)|(^\s*\d+\.\s)|(^\s*---+\s*$)|(```)|(`[^`\n]+`)|(\*\*[^*\n].+?\*\*)|(~~[^~\n].+?~~)|(<u>.+?</u>)|(\*[^*\n]+\*)|(\[[^\]]+\]\([^)]+\))|(^>\s)",
r"(^#{1,6}\s)|(^\s*[-*]\s)|(^\s*\d+\.\s)|(^\s*---+\s*$)|(```)|(`[^`\n]+`)|(\*\*[^*\n].+?\*\*)|(~~[^~\n].+?~~)|(<u>.+?</u>)|(\*[^*\n]+\*)|(\[[^\]]+\]\([^)]+\))|(^>\s)|(^\|.*\|\n\|[-|: ]+\|)",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This requires outer pipes on both header and delimiter rows, but GFM permits them to be omitted. Please recognize A | B\n--- | ---\n1 | 2 as well and add a matching send-payload regression test; otherwise that valid table still falls through to text.

@teknium1 teknium1 added the sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform label Jul 15, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Closing — this was fixed on main by #68121, which removes the _MARKDOWN_TABLE_RE force-text downgrade and routes tables through post/md, the same direction as this PR. Your updated plugin-path version of #38834 — same mechanism as the landed fix. The salvage was based on #58019 (cleanest implementation vs current main, with tests) with #29552 credited as the earliest submitter against the live plugin adapter path; both authors' commits were cherry-picked with authorship preserved. Thanks for contributing a fix for this — it was one of the most-reported bugs in the tracker.

@teknium1 teknium1 closed this Jul 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery P3 Low — cosmetic, nice to have platform/feishu Feishu / Lark adapter sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[feishu] Remove outdated table-to-text fallback — post(md) now supports GFM tables

3 participants