Skip to content

fix(feishu): try post-type before falling back to text for markdown tables - #34181

Open
wydty wants to merge 1 commit into
NousResearch:mainfrom
wydty:fix/feishu-table-rendering-try-post-first
Open

fix(feishu): try post-type before falling back to text for markdown tables#34181
wydty wants to merge 1 commit into
NousResearch:mainfrom
wydty:fix/feishu-table-rendering-try-post-first

Conversation

@wydty

@wydty wydty commented May 28, 2026

Copy link
Copy Markdown

Problem

_build_outbound_payload() pre-emptively forces text message type for any
content containing markdown tables, even when other markdown (bold, code,
lists) is also present. This sacrifices all formatting just because the
message happens to contain a table.

Feishu's post message type with md tag does support table rendering
on most clients, though behavior varies by client version.

Solution

Remove the pre-emptive table detection from _build_outbound_payload, letting
tables flow through the normal markdown detection logic:

  • Tables + other markdown (bold, code, etc.) → post type → md tag
    → Feishu renders the table on capable clients
  • Tables only (no other markdown) → text type (unchanged from before)
  • If Feishu API rejects the post payload → existing fallback in
    _send_message_chunks degrades to plain text automatically

The _MARKDOWN_TABLE_RE regex is retained as documentation but no longer
drives payload construction.

Tests

All 397 existing feishu-related tests pass.

…ables

Previously, _build_outbound_payload detected markdown tables and
pre-emptively forced text mode for any content containing table syntax.
This was a conservative fix for cases where Feishu's `md` tag rendered
tables as blank on some clients.

However, Feishu's Markdown renderer in the `post` message type does
support tables, and rendering varies by client version. The pre-emptive
force-to-text was overly conservative — it sacrificed all markdown
formatting (bold, code, lists, etc.) just because the message happened
to contain a table.

This change removes the pre-emptive table detection in
_build_outbound_payload, letting tables flow through the normal markdown
detection logic:

  - Tables + other markdown (bold, code, etc.) → `post` type → `md` tag
    → Feishu renders the table on capable clients
  - Tables only (no other markdown) → `text` type → raw markdown text
    (unchanged from before, since _MARKDOWN_HINT_RE won't match)
  - If the Feishu API rejects the post payload format → the existing
    fallback in _send_message_chunks degrades to plain text automatically

The _MARKDOWN_TABLE_RE regex is retained as documentation but no longer
drives payload construction.
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused table-rendering proposal. The underlying routing problem is still present on current main: plugins/platforms/feishu/adapter.py:4524-4534 returns text before evaluating _MARKDOWN_HINT_RE when _MARKDOWN_TABLE_RE matches.

Problems

  • The PR modifies the former gateway/platforms/feishu.py, but 560010547 moved the active adapter to plugins/platforms/feishu/adapter.py; the PR currently conflicts.
  • The stale diff also removes webhook authentication/config handling and approval-click authorization. Current main enforces approval state, operator authorization, and chat binding at plugins/platforms/feishu/adapter.py:2706-2744; those changes must not be included in a table-routing fix.
  • No table-routing regression test is added. Existing fallback coverage at tests/gateway/test_feishu.py:529-569 is generic and does not assert the table-plus-markdown decision.

Suggested changes

  • Salvage only the payload-routing change into plugins/platforms/feishu/adapter.py:_build_outbound_payload().
  • Add focused routing tests for a table combined with another markdown hint, plus the intended table-only behavior.

This is an automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data 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-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants