Skip to content

bug(slack): SlackFormatConverter.render_postable uses regex for PostableMarkdown instead of AST path (diverges from TS SDK) #81

Description

@patrick-chinchill

Summary

SlackFormatConverter.render_postable handles PostableMarkdown (and {"markdown": ...} dicts) via a private regex method _markdown_to_mrkdwn instead of the AST-based from_markdown path. The upstream TypeScript SDK uses fromAst(parseMarkdown(text)) for all markdown inputs. This is an undocumented divergence not present in docs/UPSTREAM_SYNC.md's Known Non-Parity table.

Root cause

# src/chat_sdk/adapters/slack/format_converter.py

def render_postable(self, message: Any) -> str:
    ...
    if "markdown" in message:
        return self._markdown_to_mrkdwn(message["markdown"])   # ← regex, not AST
    ...
    if hasattr(message, "markdown"):
        return self._markdown_to_mrkdwn(message.markdown)      # ← regex, not AST

The TS SDK equivalent routes through fromAst(parseMarkdown(text)). The base class render_postable also uses the correct from_markdown path. SlackFormatConverter overrides it to add Slack-specific mention conversion (correct for str/raw cases) but incorrectly also overrides the markdown case to use a simple regex.

Impact

_markdown_to_mrkdwn uses this link regex:

result = re.sub(r"\[([^\]]+)\]\(([^)]+)\)", r"<\2|\1>", result)

This fails on URLs with nested parentheses (e.g., https://en.wikipedia.org/wiki/Foo_(bar_(baz))), producing truncated or broken <url|text> output. The AST-based path (via the markdown parser) handles balanced parens correctly.

This is also the underlying cause of the PostableMarkdown streaming round-trip bug that caused chinchill-api to abandon PostableMarkdown in favour of PostableRaw + a local regex workaround (see chinchill-api#1002, chinchill-api#948). The streaming path in thread.py passes proper markdown text to PostableMarkdown, so fixing render_postable to use the AST path makes the SDK work correctly end-to-end.

Evidence from upstream TS SDK

packages/adapter-slack/src/markdown.test.ts has:

it("should convert links", () => {
  expect(converter.fromMarkdown("Check [this](https://example.com)")).toBe(
    "Check <https://example.com|this>"
  );
});

There is no equivalent test covering renderPostable(new PostableMarkdown(...)) directly — a test gap in both repos.

Fix

Change both markdown branches in render_postable to call from_markdown instead of _markdown_to_mrkdwn:

if "markdown" in message:
    return self.from_markdown(message["markdown"])
...
if hasattr(message, "markdown"):
    return self.from_markdown(message.markdown)

from_markdownfrom_ast(parse_markdown(text)) handles links, bold, italic, and mentions correctly through the AST, matching TS SDK behaviour. The str and raw branches of render_postable are unaffected — they correctly stay on _convert_mentions_to_slack.

Related

  • chinchill-api#1002 (Slack raw markdown links regression, root cause)
  • chinchill-api#948 (SDK integration — documents the PostableRaw workaround)
  • docs/UPSTREAM_SYNC.md Known Non-Parity table (divergence was never documented)

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions