Skip to content

fix(feishu): render markdown tables via post instead of force-text downgrade - #36877

Open
banditburai wants to merge 2 commits into
NousResearch:mainfrom
banditburai:fix/feishu-markdown-tables
Open

fix(feishu): render markdown tables via post instead of force-text downgrade#36877
banditburai wants to merge 2 commits into
NousResearch:mainfrom
banditburai:fix/feishu-markdown-tables

Conversation

@banditburai

Copy link
Copy Markdown
Contributor

Summary

Markdown tables sent through the Feishu adapter arrived as raw |---| text, and any message that contained a table had its other markdown stripped as well. _build_outbound_payload (gateway/platforms/feishu.py:4310) routed any table-matching content straight to plain text — a workaround from before Feishu's post md elements rendered tables server-side. This removes the downgrade so table content goes through the existing post/tag:md pipeline, and normalizes CRLF before detection so \r\n tables are matched.

Change

  • _build_outbound_payload (feishu.py:4310): route content matching _MARKDOWN_TABLE_RE (:159) or _MARKDOWN_HINT_RE (:153) to post via _build_markdown_post_payload; everything else to text. Previously the table-matching branch returned text directly.
  • _build_outbound_payload (feishu.py:4310): content = content.replace("\r\n", "\n").replace("\r", "\n") before detection — _MARKDOWN_TABLE_RE and the post row-splitter match a literal \n, so a CRLF table was missed.
  • Unchanged: _MARKDOWN_HINT_RE (:153); the post→text fallback that triggers on a content format of the post type is incorrect rejection (_POST_CONTENT_INVALID_RE, :165); the shared payload call in send (:1785) and edit_message (:1839).

Behavior

Content Before After
GFM table, no other markdown text (raw |---|) post (tag:md)
Table + surrounding markdown text, surrounding markdown stripped post (tag:md)
Table with CRLF line endings text (detection missed \r\n) post (normalized, then tag:md)
Non-table markdown (headings, bold, lists) post post (unchanged)
Plain text; lone | (e.g. int | str) text text (unchanged)

Testing

uv run --extra dev --extra feishu python -m pytest tests/gateway/test_feishu.py — 206 passed. 6 failures in TestWebhookSecurity / url-verification predate this change and reproduce on origin/main. New tests:

  • test_send_routes_markdown_table_to_post — a bare table sends as post; pins the SDK call to create.
  • test_edit_message_routes_markdown_table_to_post — the same seam routes edit_message as post; pins the call to update.
  • test_outbound_keeps_post_for_prose_surrounding_a_table — mixed prose+table stays post.
  • test_outbound_normalizes_crlf_table_to_post — a \r\n table routes to post.
  • test_outbound_single_pipe_line_stays_plain_text — a lone pipe stays text.
  • test_table_detection_ignores_horizontal_rule_MARKDOWN_TABLE_RE does not match a --- rule.
  • test_table_inside_code_fence_stays_a_single_post_row — a fenced table stays one verbatim row.

The tests assert routing to post; the server-side render is not exercised by CI and was checked manually on feishu.cn.

Note

The post→text fallback triggers on a content format of the post type is incorrect rejection; it does not cover a payload the API accepts but renders empty. Table rendering is confirmed on feishu.cn and unconfirmed on larksuite.com — there, a non-rendering table would display as an empty message rather than raw text. A _domain_name == "lark" guard can route the international domain to text if a check shows that case.

Related

Other PRs targeting this area, by approach:


Fixes: #7310 #9549 #18704 #18756 #23938 #25452 #26658 #27529 #29245 #32607
Addresses: #21866
Refs: #6003 #6363 #16084 #19226 #21326 #26236 #27695 #7022 #21778

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery platform/feishu Feishu / Lark adapter labels Jun 1, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Duplicate of the long-standing Feishu markdown-table rendering cluster: #21778 (canonical bug), with multiple competing open fix PRs (#26108, #29552, #33641, #31056, #27739). Same root cause — _build_outbound_payload force-downgrades table content to text via _MARKDOWN_TABLE_RE. Maintainers should pick one PR from this cluster rather than merging several.

@Seryta

Seryta commented Jun 11, 2026

Copy link
Copy Markdown

any updates?

@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 isolating the table-routing behavior and covering both send and edit paths.

Problems

  • Current main moved the active adapter from gateway/platforms/feishu.py to plugins/platforms/feishu/adapter.py in 560010547. The current table downgrade remains at plugins/platforms/feishu/adapter.py:4524-4534, so this PR's legacy-path diff does not modify the runtime implementation.
  • The added tests import gateway.platforms.feishu (PR tests/gateway/test_feishu.py:32), while current tests import plugins.platforms.feishu.adapter (tests/gateway/test_feishu.py:489).
  • The new unconditional post route applies to both domains, but the adapter explicitly supports Lark via _domain_name (plugins/platforms/feishu/adapter.py:4718-4751) and the PR description leaves Lark rendering unconfirmed.

Suggested changes

  • Port the implementation and test imports/patch targets to plugins/platforms/feishu/adapter.py.
  • Keep the Lark path on text until it is validated, or add verified domain-specific coverage.

Automated hermes-sweeper review.

if _MARKDOWN_HINT_RE.search(content):
# _MARKDOWN_TABLE_RE and the post row-splitter match on literal "\n", so a
# CRLF table would slip through detection and leak raw pipes as plain text.
content = content.replace("\r\n", "\n").replace("\r", "\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.

Current main migrated the runtime adapter to plugins/platforms/feishu/adapter.py in 560010547; this legacy file no longer exists on HEAD. Please port this change to the plugin adapter so it affects live Feishu delivery.

def _capturing_adapter():
"""A FeishuAdapter whose message API records the last create/update request."""
from gateway.config import PlatformConfig
from gateway.platforms.feishu import FeishuAdapter

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.

Current tests import FeishuAdapter from plugins.platforms.feishu.adapter; update this helper and the added patch targets to the plugin module so the tests exercise the current runtime implementation.

# renders it natively. Feishu added server-side table support to post 'md'
# elements, so the old force-text table downgrade is obsolete (verified on
# feishu.cn; see docs/plans/2026-05-30-feishu-markdown-table-rendering.md).
if _MARKDOWN_TABLE_RE.search(content) or _MARKDOWN_HINT_RE.search(content):

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 routes both Feishu and Lark through post, but the adapter supports a distinct _domain_name == "lark" path and the PR description leaves Lark table rendering unconfirmed. Validate that domain or retain text routing there before enabling this universally.

@teknium1 teknium1 added sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 13, 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 P2 Medium — degraded but workaround exists platform/feishu Feishu / Lark adapter sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages type/bug Something isn't working

Projects

None yet

4 participants