fix(feishu): render markdown tables via post+md, not text downgrade
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.
This commit is contained in:
JasonFang1993
Teknium
parent
5e999b98cb
commit
a660630986
|
|
@ -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].+?~~)|(<u>.+?</u>)|(\*[^*\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"|(<u>.+?</u>)"
|
||||
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}
|
||||
|
|
|
|||
|
|
@ -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"
|
||||
Loading…
Reference in New Issue