Skip to content

fix(feishu): convert markdown pipe tables to fenced text blocks befor… - #51458

Open
leeyo921 wants to merge 1 commit into
NousResearch:mainfrom
leeyo921:feat/feishu-markdown-table-codeblock-guard
Open

fix(feishu): convert markdown pipe tables to fenced text blocks befor…#51458
leeyo921 wants to merge 1 commit into
NousResearch:mainfrom
leeyo921:feat/feishu-markdown-table-codeblock-guard

Conversation

@leeyo921

@leeyo921 leeyo921 commented Jun 23, 2026

Copy link
Copy Markdown

…e send

Feishu post-type md elements do not render markdown tables reliably. Previously, pipe-table content was force-sent as plain text, leaving the raw |---| markup visible to users.

This change converts markdown pipe tables to fenced ```text blocks deterministically in the outbound payload builder — no model rewrite, no retry, no format scoring.

  • Skips tables already inside existing fenced code blocks.
  • Col-aligns cells and strips the separator row.
  • Preserves rich post delivery for mixed markdown + table content.
  • Adds three targeted tests (send path, skip-existing, payload shape).

What does this PR do?

Related Issue

Fixes #

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

How to Test

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:

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

For New Skills

  • This skill is broadly useful to most users (if bundled) — see Contributing Guide
  • SKILL.md follows the standard format (frontmatter, trigger conditions, steps, pitfalls)
  • No external dependencies that aren't already available (prefer stdlib, curl, existing Hermes tools)
  • I've tested the skill end-to-end: hermes --toolsets skills -q "Use the X skill to do Y"

Screenshots / Logs

…e send

Feishu post-type md elements do not render markdown tables reliably.
Previously, pipe-table content was force-sent as plain text, leaving the
raw |---| markup visible to users.

This change converts markdown pipe tables to fenced ```text blocks
deterministically in the outbound payload builder — no model rewrite,
no retry, no format scoring.

- Skips tables already inside existing fenced code blocks.
- Col-aligns cells and strips the separator row.
- Preserves rich post delivery for mixed markdown + table content.
- Adds three targeted tests (send path, skip-existing, payload shape).
@alt-glitch alt-glitch added type/bug Something isn't working comp/plugins Plugin system and bundled plugins platform/feishu Feishu / Lark adapter P3 Low — cosmetic, nice to have labels Jun 23, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related (saturated cluster): Feishu markdown-table rendering. This uses the fenced-code-block approach (same idea as closed #19015 / open #36877), competing with the interactive-card cluster tracked under canonical bug #21778 / PR #12114 (and #25453). Same goal, different mechanism — related, not a duplicate.

@leeyo921
leeyo921 marked this pull request as draft July 4, 2026 13:47
@leeyo921
leeyo921 marked this pull request as ready for review July 4, 2026 13:47
@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

@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 preserving rich-post delivery while reusing the existing fenced-code-block path. The underlying issue remains on current main: plugins/platforms/feishu/adapter.py:4524-4534 still routes detected tables to raw text.

Problems

  • plugins/platforms/feishu/adapter.py:574 checks _MARKDOWN_TABLE_RE before normalizing CRLF at :577. Since that regex requires a literal \n immediately after the header row, \r\n tables skip conversion and retain the current raw-text behavior.
  • plugins/platforms/feishu/adapter.py:558-562 uses len() and ljust() for visual alignment. CJK cells are wider than their code-point length; agent/markdown_tables.py:1-14,49-62 already documents and handles this with wcswidth.

Suggested changes

  • Normalize line endings before the detection guard and add a CRLF payload test.
  • Make code-block padding display-width-aware and cover a CJK alignment case.

This is an automated hermes-sweeper review.

The transform is intentionally local and deterministic: no model rewrite, no
prose changes, and no conversion inside existing fenced code blocks.
"""
if not content or not _MARKDOWN_TABLE_RE.search(content):

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.

Normalize line endings before this guard. _MARKDOWN_TABLE_RE requires |\n|, so a standard CRLF table fails here and never reaches the replace("\r\n", "\n") at line 577; it remains on the raw text fallback.


normalized_rows = [row + [""] * (column_count - len(row)) for row in rows]
widths = [
max(len(row[column]) for row in normalized_rows)

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.

len() measures code points rather than rendered columns, so CJK cells in the PR's target platform will not line up in the fenced block. Use display-cell width (as agent/markdown_tables.py does with wcswidth) for both width calculation and padding.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have 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.

3 participants