From a66063098692343395c234d2e67353595a8a5520 Mon Sep 17 00:00:00 2001 From: JasonFang1993 Date: Sat, 4 Jul 2026 10:28:20 +0800 Subject: [PATCH] fix(feishu): render markdown tables via post+md, not text downgrade MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolves issue #52786 (duplicate of #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 #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 44ddc552f5e054759a6970af8997ea588a9d81c9`` restores upstream main without the new test file. --- plugins/platforms/feishu/adapter.py | 31 +++-- tests/gateway/test_feishu_table_markdown.py | 133 ++++++++++++++++++++ 2 files changed, 155 insertions(+), 9 deletions(-) create mode 100644 tests/gateway/test_feishu_table_markdown.py diff --git a/plugins/platforms/feishu/adapter.py b/plugins/platforms/feishu/adapter.py index 96528646939..02b2b378adb 100644 --- a/plugins/platforms/feishu/adapter.py +++ b/plugins/platforms/feishu/adapter.py @@ -152,11 +152,24 @@ logger = logging.getLogger(__name__) # --------------------------------------------------------------------------- _MARKDOWN_HINT_RE = re.compile( - r"(^#{1,6}\s)|(^\s*[-*]\s)|(^\s*\d+\.\s)|(^\s*---+\s*$)|(```)|(`[^`\n]+`)|(\*\*[^*\n].+?\*\*)|(~~[^~\n].+?~~)|(.+?)|(\*[^*\n]+\*)|(\[[^\]]+\]\([^)]+\))|(^>\s)", + # Pipe table: any header line + separator line both starting with '|'. + r"(^\|.*\|\s*\n\|[-:|\s]+\|)" + # Headings, lists, code, bold/italic/strike/underline, links, blockquotes. + r"|(^#{1,6}\s)" + r"|(^\s*[-*]\s)" + r"|(^\s*\d+\.\s)" + r"|(^\s*---+\s*$)" + r"|(```)" + r"|(`[^`\n]+`)" + r"|(\*\*[^*\n].+?\*\*)" + r"|(~~[^~\n].+?~~)" + r"|(.+?)" + r"|(\*[^*\n]+\*)" + r"|(\[[^\]]+\]\([^)]+\))" + r"|(^>\s)", re.MULTILINE, ) -# Detect markdown tables: a line starting with | followed by a separator line. -# Feishu post-type 'md' elements do not render tables, so we force text mode. +# Backwards-compatible alias retained because external callers reference it. _MARKDOWN_TABLE_RE = re.compile(r"^\|.*\|\n\|[-|: ]+\|", re.MULTILINE) _MARKDOWN_LINK_RE = re.compile(r"\[([^\]]+)\]\(([^)]+)\)") _MARKDOWN_FENCE_OPEN_RE = re.compile(r"^```([^\n`]*)\s*$") @@ -4533,12 +4546,12 @@ class FeishuAdapter(BasePlatformAdapter): # ========================================================================= def _build_outbound_payload(self, content: str) -> tuple[str, str]: - # Feishu post-type 'md' elements do not render markdown tables; sending - # table content as post causes the message to appear blank on the client. - # Force plain text for anything that looks like a markdown table. - if _MARKDOWN_TABLE_RE.search(content): - text_payload = {"text": content} - return "text", json.dumps(text_payload, ensure_ascii=False) + # Empirically (issue #52786), current Feishu clients render markdown + # tables inside ``post``-type ``md`` elements natively. The previous + # table-downgrade branch forced any table-containing message to + # ``text``, which left Feishu readers seeing the raw pipe-and-dash + # source instead of a rendered table. Trust the common markdown path + # for table content too. if _MARKDOWN_HINT_RE.search(content): return "post", _build_markdown_post_payload(content) text_payload = {"text": content} diff --git a/tests/gateway/test_feishu_table_markdown.py b/tests/gateway/test_feishu_table_markdown.py new file mode 100644 index 00000000000..1ff9cbe3516 --- /dev/null +++ b/tests/gateway/test_feishu_table_markdown.py @@ -0,0 +1,133 @@ +"""Tests for Feishu adapter outbound markdown payload construction. + +Reproduces the bug tracked in hermes-agent issue #52786: +`_build_outbound_payload` was force-downgrading any message containing a +markdown pipe table to ``msg_type=text``, so Feishu clients rendered the raw +pipe-and-dash source instead of a table. Empirically current Feishu clients +render ``post``+``md`` tables natively, so the downgrade branch must be removed. + +These tests guard the fix. They invoke the real adapter via the project's +plugin-loader helper so that no ``sys.path`` / ``sys.modules`` games are +needed. +""" + +from __future__ import annotations + +import json + +from tests.gateway._plugin_adapter_loader import load_plugin_adapter + +_adapter = load_plugin_adapter("feishu") + + +def _call_build_outbound_payload(content: str) -> tuple[str, str]: + """Invoke ``_build_outbound_payload`` on a bare adapter instance. + + ``_build_outbound_payload`` is a method that only uses module-level + helpers (``_MARKDOWN_TABLE_RE``, ``_MARKDOWN_HINT_RE``, + ``_build_markdown_post_payload``) and never touches ``self.*``, so a bare + object is sufficient. + """ + inst = object.__new__(_adapter.FeishuAdapter) + return inst._build_outbound_payload(content) + + +def _md_texts_from_post_payload(payload_str: str) -> list[str]: + """Pull every ``{tag:'md', text:'...'}`` element out of a Feishu post payload. + + Real payload shape:: + + {"zh_cn": {"content": [[{"tag": "md", "text": "..."}], ...]}} + + Helpers and tests need to introspect the ``md`` blocks regardless of + locale, so we walk the structure generically. + """ + payload = json.loads(payload_str) + if not isinstance(payload, dict): + return [] + texts: list[str] = [] + for lang_val in payload.values(): + if not isinstance(lang_val, dict): + continue + content = lang_val.get("content", []) + if not isinstance(content, list): + continue + for block in content: + if isinstance(block, list): + candidates = block + else: + candidates = [block] + for el in candidates: + if isinstance(el, dict) and el.get("tag") == "md": + texts.append(el.get("text", "")) + return texts + + +def test_markdown_table_uses_post_not_text(): + """Regression test for issue #52786 (and its older sibling #23938). + + A message whose only markdown is a table must take the ``post`` path, + not be downgraded to plain text. + """ + content = ( + "| col A | col B |\n" + "| ----- | ----- |\n" + "| 1 | 2 |" + ) + msg_type, payload_str = _call_build_outbound_payload(content) + assert msg_type == "post", ( + f"expected 'post' for a markdown table (issue #52786), got {msg_type!r}; " + "the table-downgrade branch in _build_outbound_payload has been re-introduced" + ) + md_texts = _md_texts_from_post_payload(payload_str) + assert md_texts, f"post payload must include at least one md element; got {payload_str!r}" + joined = "".join(md_texts) + assert "col A" in joined and "|" in joined, ( + "table text was lost or reformatted when switching from text to post" + ) + + +def test_plain_text_without_markdown_still_uses_text(): + """Negative control: a message with no markdown hints and no table must + still go to plain text. Guards against accidentally promoting everything + to ``post``.""" + msg_type, _ = _call_build_outbound_payload("just a plain sentence with no markup") + assert msg_type == "text" + + +def test_existing_markdown_heading_still_uses_post(): + """Sanity: the existing ``post`` path (heading / list / code / bold / + link) must still work after the table downgrade is removed.""" + msg_type, payload_str = _call_build_outbound_payload("# hello world\n") + assert msg_type == "post" + md_texts = _md_texts_from_post_payload(payload_str) + assert md_texts, f"expected at least one md element; got {payload_str!r}" + assert any("hello world" in t for t in md_texts), ( + f"expected 'hello world' in md elements; got {md_texts!r}" + ) + + +def test_table_combined_with_other_markdown_does_not_downgrade(): + """A message that mixes a table with surrounding markdown must also + take the ``post`` path. + + The old ``_MARKDOWN_TABLE_RE`` branch returned ``text`` unconditionally + and stripped all the surrounding markdown formatting, so a Feishu + reader saw literal pipes and lost the prose framing the table. + """ + content = ( + "Here is the data:\n\n" + "| col A | col B |\n" + "| ----- | ----- |\n" + "| 1 | 2 |\n\n" + "Let me know." + ) + msg_type, payload_str = _call_build_outbound_payload(content) + assert msg_type == "post" + md_texts = _md_texts_from_post_payload(payload_str) + joined = "\n".join(md_texts) + assert "Here is the data" in joined, ( + "leading prose was lost when downgrading a mixed-table message" + ) + assert "col A" in joined, "table header was lost" + assert "Let me know" in joined, "trailing prose was lost"