Skip to content

fix(feishu): render markdown tables via post+md, not text downgrade - #58019

Closed
JasonFang1993 wants to merge 1 commit into
NousResearch:mainfrom
JasonFang1993:fix/feishu-table-markdown-rendering
Closed

fix(feishu): render markdown tables via post+md, not text downgrade#58019
JasonFang1993 wants to merge 1 commit into
NousResearch:mainfrom
JasonFang1993:fix/feishu-table-markdown-rendering

Conversation

@JasonFang1993

@JasonFang1993 JasonFang1993 commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #52786 and fixes #23938, closing the duplicate cluster around #31056.

_build_outbound_payload in plugins/platforms/feishu/adapter.py was force-downgrading any message containing a markdown pipe table to msg_type=text, so Feishu readers rendered the raw pipe-and-dash source instead of a table. Empirically current Feishu clients render markdown tables inside post-type md elements natively, so the downgrade branch had to go.

Changes

  1. _MARKDOWN_HINT_RE now also matches a pipe-table header + separator pair, so a table-only message is recognised as "has markdown" and takes the post path.
  2. _build_outbound_payload no longer special-cases _MARKDOWN_TABLE_RE before the hint check. Table content now flows through the same _build_markdown_post_payload path as every other markdown structure (headings, lists, code, bold, links, etc.).

_MARKDOWN_TABLE_RE itself is retained as a module-level constant for backwards compatibility with external callers.

Tests

New file: tests/gateway/test_feishu_table_markdown.py (4 regression tests).

  • test_markdown_table_uses_post_not_text — pure-table content → post, with table text inside an md element (issue Feishu adapter incorrectly downgrades markdown tables to plain text #52786 scenario)
  • test_table_combined_with_other_markdown_does_not_downgrade — prose + table + prose message keeps its surrounding markdown
  • test_existing_markdown_heading_still_uses_post — heading path unchanged
  • test_plain_text_without_markdown_still_uses_text — negative control

The new tests use the projects tests/gateway/_plugin_adapter_loader.load_plugin_adapter() helper, as required by the conftest plugin-adapter anti-pattern guard.

Verification

pytest tests/gateway/test_feishu.py tests/gateway/test_feishu_table_markdown.py

209 passed (205 existing + 4 new), three consecutive runs.

Rollback

git reset --hard 44ddc552f5e054759a6970af8997ea588a9d81c9

restores upstream main without the new test file.

Related

Resolves issue NousResearch#52786 (duplicate of NousResearch#23938):

The `_build_outbound_payload` shortcut forced any message containing a
pipe table to ``msg_type=text``.  Feishu readers then rendered the raw
pipe-and-dash source instead of a table.  Empirically current Feishu
clients render markdown tables inside ``post``-type ``md`` elements
natively, so the downgrade branch had to go.

Two changes:

1. ``_MARKDOWN_HINT_RE`` now also matches a pipe-table header+separator
   pair, so a table-only message is recognised as "has markdown" and
   takes the ``post`` path.  All previously recognised hints (headings,
   lists, code, bold/italic/strike/underline, links, blockquotes, hr)
   still match — verified by the existing 205 test_feishu.py cases plus
   the new regression tests below.

2. ``_build_outbound_payload`` no longer special-cases `_MARKDOWN_TABLE_RE`
   before the hint check.  The hint check now routes table content to
   `_build_markdown_post_payload`, which is the same path any other
   markdown structure takes.

``_MARKDOWN_TABLE_RE`` itself is retained as a module-level constant for
external callers (import-path-sensitive tests, third-party consumers of
the adapter module) and continues to work for its existing uses.

Tests
-----
New: ``tests/gateway/test_feishu_table_markdown.py`` — four regression
tests:

- ``test_markdown_table_uses_post_not_text`` — pure-table content
  reaches ``post`` (issue NousResearch#52786 scenario).
- ``test_table_combined_with_other_markdown_does_not_downgrade`` —
  prose + table + prose message keeps its surrounding markdown.
- ``test_existing_markdown_heading_still_uses_post`` — sanity guard:
  the heading path is unchanged.
- ``test_plain_text_without_markdown_still_uses_text`` — negative
  control: pure prose still goes to ``text``.

Verification
------------
``pytest tests/gateway/test_feishu.py
tests/gateway/test_feishu_table_markdown.py`` passes 209/209 (205
existing + 4 new), three consecutive runs.

Rollback
--------
``git reset --hard 44ddc55``
restores upstream main without the new test file.
@JasonFang1993

Copy link
Copy Markdown
Contributor Author

Hi! Submitting this as a fix for the table-downgrade branch in
_build_outbound_payload.

A quick note on where this PR sits in the existing cluster of "feishu
markdown table" reports:

I deliberately did not touch any of the in-flight
feat/feishu-markdown-* PRs (#52046 etc.) because the right place for
this fix is the post + md payload-builder path, not the
interactive-card path those PRs pivot toward — the two are not
equivalent, and trying to merge them would create a behaviour split
between "card-rendering clients" and "post-rendering clients" that
would be painful to undo later. Happy to coordinate if a maintainer
prefers the card route, but my read of #52786's reproduction (text +
table on the same line) is that the post path is the lowest-risk
fix.

Quick verification locally (with this branch):

$ pytest tests/gateway/test_feishu.py          tests/gateway/test_feishu_table_markdown.py -q
209 passed, 2 warnings in ~12s

(205 pre-existing + 4 new regression tests.)

Happy to iterate on the test surface or refactor any of the
inline re.compile blocks into a more readable shape — just point.

@alt-glitch alt-glitch added type/bug Something isn't working comp/plugins Plugin system and bundled plugins platform/feishu Feishu / Lark adapter P3 Low — cosmetic, nice to have sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages duplicate This issue or pull request already exists labels Jul 4, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Duplicate of #52790 — same code site (_build_outbound_payload in plugins/platforms/feishu/adapter.py) and same mechanism: remove the _MARKDOWN_TABLE_RE text-downgrade and add a pipe-table pattern to _MARKDOWN_HINT_RE so table-only messages still take the post+md path. #52790 (also "Closes #52786") is the earliest open PR with this exact approach. Related competing fixes in the Feishu-table cluster use a different mechanism: #57024 (guard-delete only), #31056 (downgrade-removal only), #53453 (adds a streaming text→post edit fallback). Anchor issue: #9549.

@teknium1

Copy link
Copy Markdown
Contributor

Merged via #68121 — your commit was cherry-picked onto current main with your authorship preserved in git log. Your implementation was the cleanest of the 9 competing fixes (current plugin path, hint-regex extension, regression tests using the required loader helper) and served as the base of the salvage. Thanks!

@teknium1 teknium1 closed this Jul 20, 2026
This was referenced Jul 20, 2026
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 duplicate This issue or pull request already exists 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

3 participants