Skip to content

fix(feishu): remove outdated markdown table fallback, always use post format - #38834

Closed
liuhao1024 wants to merge 2 commits into
NousResearch:mainfrom
liuhao1024:fix/feishu-md-table-rendering
Closed

fix(feishu): remove outdated markdown table fallback, always use post format#38834
liuhao1024 wants to merge 2 commits into
NousResearch:mainfrom
liuhao1024:fix/feishu-md-table-rendering

Conversation

@liuhao1024

Copy link
Copy Markdown
Contributor

What does this PR do?

Removes the outdated _MARKDOWN_TABLE_RE fallback in the Feishu adapter that forced table-containing messages to plain text mode. Feishu's post-type md elements now natively support all markdown syntax including pipe tables, so the fallback causes tables to render as raw source code instead of formatted output.

Related Issue

Fixes #38755

Type of Change

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

Changes Made

  • gateway/platforms/feishu.py: Removed _MARKDOWN_TABLE_RE regex definition (line 157-159) and its check in _build_outbound_payload() (lines 4311-4313). All markdown content including tables now routes through the post format with md elements.
  • tests/gateway/test_feishu_outbound_payload.py: Added 10 tests covering outbound payload routing — plain text, headings, tables with headings, pure tables, code blocks, bold, links, empty content, mixed table+code, and verification that _MARKDOWN_TABLE_RE no longer exists.

How to Test

  1. Run pytest tests/gateway/test_feishu_outbound_payload.py -v — all 10 tests should pass
  2. Run pytest tests/gateway/test_feishu.py -v — all 205 existing tests should pass (no regression)
  3. Run python -m ruff check gateway/platforms/feishu.py tests/gateway/test_feishu_outbound_payload.py — all checks passed

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • 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 (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — 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
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

Code Intelligence

⚠️ GitNexus unavailable — grep-based fallback used.

  • Checked: _MARKDOWN_TABLE_RE used only in definition (line 159) and _build_outbound_payload (line 4314) — both removed
  • Checked: _build_outbound_payload called from 2 send paths (line 1785, 4307) — both already have text fallback on post failure
  • Checked: _build_markdown_post_payload unchanged — handles code blocks and plain content correctly
  • Blast radius: LOW (2 call sites, both with existing error recovery)

… format

Feishu's post-type 'md' elements now natively support all markdown syntax
including pipe tables. The old _MARKDOWN_TABLE_RE fallback that forced
table-containing messages to plain text mode is no longer needed and
causes tables to render as raw source code instead of formatted output.

Remove the _MARKDOWN_TABLE_RE regex and its check in _build_outbound_payload()
so all markdown content (including tables) is routed through the post format
with 'md' elements for proper rendering.

Fixes NousResearch#38755
@alt-glitch alt-glitch added type/bug Something isn't working comp/gateway Gateway runner, session dispatch, delivery platform/feishu Feishu / Lark adapter P2 Medium — degraded but workaround exists labels Jun 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 the focused Feishu fix. The underlying force-to-text behavior is still present on current main, but this branch needs a targeted salvage.

Problems

  • The active adapter moved from gateway/platforms/feishu.py to plugins/platforms/feishu/adapter.py in 560010547; current routing remains at plugins/platforms/feishu/adapter.py:4524-4534. The new test imports the removed module at tests/gateway/test_feishu_outbound_payload.py:20.
  • The unchanged _MARKDOWN_HINT_RE does not match a bare pipe table. The new test explicitly keeps that case in text mode at tests/gateway/test_feishu_outbound_payload.py:66-77, so the reported bare-table path remains unfixed.

Suggested changes

  • Port the focused change and test to plugins/platforms/feishu/adapter.py.
  • Route a pure table to post and assert the resulting md payload.

This is an automated hermes-sweeper review.


def _make_adapter():
"""Create a minimal FeishuAdapter skeleton for testing _build_outbound_payload."""
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 main migrated the active adapter to plugins/platforms/feishu/adapter.py in 5600105, so this import targets a removed module after salvage. Please import FeishuAdapter from the bundled plugin path.

"| foo | 42 |"
)
msg_type, payload = adapter._build_outbound_payload(table_content)
assert msg_type == "text"

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 locks in the reported defect for a bare table: _MARKDOWN_HINT_RE does not match pipe-table syntax, so the proposed code still sends this case as text. Route this case to post and assert the md payload instead.

@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 14, 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 PR targeted the since-removed gateway/platforms/feishu.py path; the landed fix applies the same removal to the plugin adapter. 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.

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.

fix(feishu): remove outdated markdown table fallback, always use post format

3 participants