Skip to content

feat(feishu): send markdown tables as interactive cards - #20028

Open
yiyouguisu wants to merge 2 commits into
NousResearch:mainfrom
yiyouguisu:fix/feishu-sanitize-tables
Open

feat(feishu): send markdown tables as interactive cards#20028
yiyouguisu wants to merge 2 commits into
NousResearch:mainfrom
yiyouguisu:fix/feishu-sanitize-tables

Conversation

@yiyouguisu

@yiyouguisu yiyouguisu commented May 5, 2026

Copy link
Copy Markdown

Problem

Feishu's md renderer hides content inside Markdown tables — rows after the header/separator become invisible. Users reported that "标题下内容看不到" (content under headers is not visible).

Hermes only used the post / md tag for rich text, which does not support tables. Meanwhile the interactive card format renders tables correctly.

Solution

Add automatic card dispatch for content containing Markdown tables:

New functions:

  • _table_block_present() — detect real table blocks (≥2 contiguous |…| lines outside code blocks)
  • _build_markdown_card_payload() — wrap content in a Feishu interactive card ({"tag": "markdown", "content": …})
  • Regex patterns _TABLE_ROW_RE, _TABLE_SEP_RE for table detection

Message routing (_build_outbound_payload):

  1. Content has a table block → "interactive" (card) — tables render correctly
  2. Otherwise → "post" / "text" (existing logic)

Fallback chain (both send() and edit_message()):

  • interactive → post (table sanitization: converts to bullet lists)
  • post → plain text (existing fallback for invalid post)

This ensures content is never lost: best case is a rendered card, worst case is a bullet list.

Related

  • sanitize_markdown_table already existed in the Yuanbao adapter but was never ported to Feishu
  • This PR adds it as _sanitize_markdown_tables() (the post-pipeline safety net)
  • Additionally adds the card-based primary path that preserves table structure

Testing

  • Content with tables → card is sent
  • Content without tables → post/text (unchanged behaviour)
  • Tables inside code blocks → not detected as tables
  • Single pipe lines (e.g. inline |code|) → not detected (requires ≥2 rows)
  • Card failure → falls back to post with table-to-list sanitization

…to lists

Feishu interactive card format renders Markdown tables correctly,
unlike the post/md pipeline. Add automatic card dispatch:

- _table_block_present(): detect real table blocks outside code blocks
- _build_markdown_card_payload(): wrap content in a Feishu card
- _build_outbound_payload now checks for tables first, sends card
- Fallback: if card fails, fall back to post (which sanitizes tables
  to lists) so content is never lost
- Both send() and edit_message() have the same fallback chain:
  interactive -> post -> text

This works alongside the existing _sanitize_markdown_tables() which
serves as the safety net for the post pipeline.
@yiyouguisu
yiyouguisu force-pushed the fix/feishu-sanitize-tables branch from cae0d9d to 4174a92 Compare May 5, 2026 03:34
@yiyouguisu yiyouguisu changed the title fix(feishu): sanitize markdown tables in post payload feat(feishu): send markdown tables as interactive cards May 5, 2026
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists platform/feishu Feishu / Lark adapter comp/gateway Gateway runner, session dispatch, delivery labels May 5, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Related to #18756. Competes with #12114, #15956, #16194, #19038 — all addressing the same Feishu markdown table rendering problem with different approaches.

@alt-glitch

Copy link
Copy Markdown
Collaborator

Related to #18756. Competes with multiple PRs.

The PR branch referenced _sanitize_markdown_tables() but never defined
it — any post fallback path would crash with NameError.

Implementation: scan lines outside code fences, detect contiguous
table rows (^|...+|$), drop separator rows (|---|---|), and convert
data rows to '- |...|' bullet-list format so information remains
visible in Feishu's post/md channel.

This is the fallback for when the interactive card path fails.
@marsdream

Copy link
Copy Markdown

Confirmed. We hit this exact issue — Markdown tables sent via Hermes to Feishu render as garbled plain text. Our local patch using interactive (Card) + tag: markdown matches this PR's approach and verified working. Attaching a screenshot of the rendered table vs the broken fallback.

Local verification:

  • msg_type=interactive with Card schema 2.0 + tag: markdown renders tables correctly in Feishu
  • Tables detected via |.+\|[\r\n]+\|[-:| ]+\| regex pattern
  • Fallback: if Card send fails, tables degrade to bullet lists (never silently dropped)

This PR solves the problem. Adding P2 label seems appropriate given the severity. 👍

@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 preserving table content and documenting the card fallback approach. Current main now avoids the original blank-table symptom with text routing in plugins/platforms/feishu/adapter.py:4524-4534, so this should be treated as a structured-rendering enhancement rather than a direct restoration of the old path.

Problems

  • The advertised interactive → post → text chain is incomplete. After the interactive exception fallback at gateway/platforms/feishu.py:1828-1836, the response fallback at :1839-1851 still checks the original msg_type == "post"; it remains "interactive". A rejected post fallback therefore does not degrade to text. edit_message() likewise stops after interactive → post at :1900-1908.
  • The PR changes the legacy adapter path, while current main migrated Feishu to plugins/platforms/feishu/adapter.py (560010547; active routing at :4524-4534). The implementation and coverage need to target that active surface.

Suggested changes

  • Use a shared fallback helper/state transition so both send and edit retry post failure as text, including exception and unsuccessful-response cases.
  • Port the change to the plugin adapter and add focused gateway tests for the routing and both fallback stages.

Automated hermes-sweeper review.

)
elif msg_type == "interactive":
logger.warning("[Feishu] Interactive card rejected by API; falling back to post")
response = await self._feishu_send_with_retry(

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 post fallback is not followed by the post-to-text fallback below: msg_type remains "interactive", so the guard at lines 1839-1851 is skipped if this post response is rejected. Track the effective fallback type or route retries through a helper that can continue to text.

fallback_request = self._build_update_message_request(message_id=message_id, request_body=fallback_body)
fallback_response = await asyncio.to_thread(self._client.im.v1.message.update, fallback_request)
result = self._finalize_send_result(fallback_response, "update failed")
elif msg_type == "interactive":

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.

The edit path also stops after interactive → post; if that post update fails, it returns the failed result rather than the claimed final text fallback. Apply the same complete fallback state handling here.

@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 12, 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

Development

Successfully merging this pull request may close these issues.

4 participants