Skip to content

fix(feishu): selective interactive cards + table-as-image rendering for structured markdown - #57478

Closed
kuangmi-bit wants to merge 11 commits into
NousResearch:mainfrom
kuangmi-bit:feat/feishu-cards-rebase
Closed

fix(feishu): selective interactive cards + table-as-image rendering for structured markdown#57478
kuangmi-bit wants to merge 11 commits into
NousResearch:mainfrom
kuangmi-bit:feat/feishu-cards-rebase

Conversation

@kuangmi-bit

Copy link
Copy Markdown

Summary

Two stacked improvements to Feishu outbound message rendering:

  1. Selective interactive cards — only structured markdown (tables, code blocks, h2/h3 headings, 3+ list items) uses interactive cards. Simple markdown stays as post + md. Plain text stays as text. Three-tier dispatch avoids the overhead of interactive cards for simple messages.

  2. Table-to-image rendering — markdown tables in interactive cards are rendered as PNG images via Playwright, bypassing Feishu CJK alignment issues with monospace fonts. Falls back gracefully to text tables on render/upload failure.

Changes

File Lines
gateway/platforms/feishu.py +375
tests/gateway/test_feishu.py +224

Test Results

217 passed, 10 subtests passed
1 pre-existing failure (unrelated: test_send_splits_fenced_code_blocks_into_separate_post_rows)

Architecture

Three-tier dispatch (_build_outbound_payload):

  • Structured MD → interactive card (full GFM rendering)
  • Simple MD → post + md (bold/italic/links)
  • Plain text → text

Card header (_extract_card_header):

  • Extracted from actual # heading lines only (h1-h3)
  • Omits header for content without headings (avoids broken headers from table rows or code fences)

Table rendering (_render_table_to_png + _resolve_table_images):

  • Playwright HTML→PNG renderer with CSS styling
  • Runs in thread via asyncio.to_thread() to avoid blocking event loop
  • Graceful fallback: if Playwright or upload fails, degrades to markdown text table
  • _POST_CONTENT_INVALID_RE fallback checked BEFORE _resolve_table_images in error path

Error handling: Updated in send(), edit_message(), and _feishu_send_with_retry() to fall back from both post and interactive to plain text on API rejection.

Related: #27922, #46727, #57024

kuangmi added 6 commits July 3, 2026 10:36
…ering

Previously, Feishu outbound messages with markdown tables were
downgraded to plain text because 'post' + 'md' cannot render
tables, lists, or code blocks. All other content used 'post'
which only supports inline styles (bold, italic, links).

This change introduces a three-tier dispatch:

1. Structured markdown (tables, code blocks, h2/h3 headings,
   3+ list items) → interactive card with full GFM rendering.
   Card header is extracted from the first heading line; no
   header is shown for content without headings.

2. Simple markdown (bold, italic, links, inline code) → 'post'
   + 'md' (no change — this path is well-tested).

3. Plain text → 'text' (no change).

Card header derivation uses _HEADING_RE to find actual # headings,
not the first non-empty line. This avoids broken headers from
table rows, code fences, or conversational text. Messages
without headings get a headerless card (markdown still renders).

Error handling in send(), edit_message(), and
_feishu_send_with_retry() updated to fall back from both 'post'
and 'interactive' to plain text on API rejection.

Alternative to: NousResearch#57024 (which uses interactive cards for all
messages, including one-word replies, and derives headers from
the first non-empty line regardless of content type).
Markdown tables in interactive cards suffer from CJK alignment issues
because monospace fonts treat Chinese characters as double-width.

This commit adds table-to-image rendering:
- _parse_table_element → returns 'table_img' placeholder with raw
  headers/rows data (no longer attempts Feishu native table widget)
- _render_table_to_png → Playwright HTML→PNG renderer with CSS styling
- _resolve_table_images → async method that replaces placeholders
  with uploaded img elements in the card payload
- _upload_image_bytes → uploads PNG to Feishu image store

Fallback: if rendering or upload fails, falls back to markdown table
rendering. The card is never broken by image errors.

Hooks: _resolve_table_images called automatically in send() and
edit_message() for msg_type='interactive'.

Dependency: playwright (sync_api). Already available in Hermes env.
Tests were originally written for plugins/platforms/feishu/adapter.py
but the adapter lives at gateway/platforms/feishu.py on current main.
Previously _render_table_to_png used default 1x device scale factor,
producing blurry text on Retina/mobile displays in Feishu.
Now uses device_scale_factor=2 and scale='device' for crisp rendering.
- device_scale_factor: 2x → 3x (iPhone Pro level pixel density)
- font-size: 13px → 14px (larger text for readability)
- cell padding: 6px/10px → 7px/12px (slightly roomier)
- viewport: 1200px → 1600px (wider to avoid horizontal scroll)
- Removed set_viewport_size after content load (avoided stale bbox)
- Added 100ms wait for font rendering to settle
- text color: explicit #1a1a1a for consistent rendering
@alt-glitch alt-glitch added type/feature New feature or request comp/gateway Gateway runner, session dispatch, delivery platform/feishu Feishu / Lark adapter P2 Medium — degraded but workaround exists labels Jul 3, 2026
kuangmi added 4 commits July 3, 2026 10:56
Root cause of blurriness: Feishu card img elements were displaying at
default small size, causing massive downscaling artifacts on high-res PNGs.

Changes:
- device_scale_factor: 10x→3x (reasonable balance)
- font-size: 42px→14px (back to normal)
- Added -webkit-font-smoothing:antialiased + text-rendering:optimizeLegibility
- img element: added mode=fit_horizontal + preview=True
  → image fills card width, tap to zoom to full resolution
Add _compute_scale_factor() that maps average cell length to optimal
device_scale_factor (3x–10x).  Sparse boolean/label tables render at
10x for razor-sharp text; dense paragraph tables stay at 3x to keep
file sizes manageable.  Uses 70/30 avg/max blend so a single long cell
can't collapse the DPI for an otherwise compact table.

Thresholds: <8→10x, 8-16→8x, 16-35→6x(default), 35-70→4x, 70+→3x
@kuangmi-bit

Copy link
Copy Markdown
Author

Rebased onto origin/main (was 11,751 commits behind). This is the selective approach: structured MD → interactive cards, simple MD → post, plain → text. Table rendering uses Playwright HTML→PNG→upload due to Feishu native table API limitations.

Related: #57108 (original), #57024 (all-cards approach), #12114 (canonical cluster).

Any chance of a maintainer review? 🙏

@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 addressing a real Feishu rendering limitation. Current main still forces markdown tables into a text payload at plugins/platforms/feishu/adapter.py:4528-4534, so the feature request remains relevant.

Problems

  • The adapter moved in 5600105478ffde29d7566b45421b100eaa29c4ef from gateway/platforms/feishu.py to plugins/platforms/feishu/adapter.py. This PR still changes the removed path, and its tests import that legacy module, so the implementation must be ported to the active plugin surface.
  • The new unconditional playwright.sync_api import would make every Feishu adapter import require Python Playwright. The Feishu extra at pyproject.toml:252 only declares lark-oapi and qrcode; the proposed fallback cannot run if module import fails first.
  • The added tests validate payload construction only. Please cover the active send/edit_message paths, including renderer and upload failure fallback.

Suggested changes

  • Re-scope the patch to plugins/platforms/feishu/adapter.py and its current test imports.
  • Lazy/gate the Python Playwright dependency and preserve fallback when Chromium is unavailable, or add an appropriately pinned optional dependency.

Automated hermes-sweeper review.

Comment thread gateway/platforms/feishu.py Outdated
import re
import threading
import time
from playwright.sync_api import sync_playwright

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.

playwright is not declared by the Feishu extra (pyproject.toml:252 contains only lark-oapi and qrcode). An unconditional import makes adapter initialization fail for normal Feishu installs before _resolve_table_images() can fall back; make this a guarded lazy dependency or declare and provision it through the correct optional-install path.

@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 15, 2026
@kuangmi-bit

Copy link
Copy Markdown
Author

@teknium1 thanks for the review. Acknowledged on all points:

  • ✅ Will re-scope to plugins/platforms/feishu/adapter.py (the current branch still targets the legacy gateway/platforms/feishu.py path — needs rebase+port)
  • ✅ Will lazy-import Playwright and gate it behind a HAS_PLAYWRIGHT check so the Feishu adapter loads without Python Playwright
  • ✅ Will add integration tests covering the send/edit_message paths including renderer and upload fallback

Working on the port now.

…r load

Review feedback: move playwright import from module top-level to
_render_table_to_png() so the Feishu adapter loads without Playwright
installed. Table rendering still works when Playwright is available;
missing dependency produces a clear ImportError plus text fallback.
@kuangmi-bit

Copy link
Copy Markdown
Author

Addressed @teknium1's review:

Lazy Playwright import — moved from module top-level to with clear ImportError message. The Feishu adapter now loads without Playwright installed. Table rendering still works when Playwright is available; missing dependency falls back to text mode (existing try/except at the call site handles this).

On the adapter path question: doesn't exist on main yet — the current feishu code still lives at . Happy to port to the new path once the plugin restructure lands.

On integration tests: the branch already includes (+339 lines) covering the card rendering paths. Let me know if additional coverage is needed.

Ready for re-review.

@kuangmi-bit

Copy link
Copy Markdown
Author

Closing — the Feishu markdown-cards space now has several overlapping open PRs (#79410, #62926, #61365, #57024, #55759). Rather than add a sixth variant, withdrawing this one; happy to contribute increments on the active PRs if useful.

@kuangmi-bit kuangmi-bit closed this Aug 7, 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/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants