diff --git a/.changeset/clean-bots-wave.md b/.changeset/clean-bots-wave.md new file mode 100644 index 000000000..cf08f3234 --- /dev/null +++ b/.changeset/clean-bots-wave.md @@ -0,0 +1,5 @@ +--- +"chat": patch +--- + +Preserve markdown structural whitespace when extracting normalized message text. diff --git a/packages/adapter-github/src/index.test.ts b/packages/adapter-github/src/index.test.ts index 58ffa99f0..05c0cd05c 100644 --- a/packages/adapter-github/src/index.test.ts +++ b/packages/adapter-github/src/index.test.ts @@ -90,6 +90,7 @@ const mockLogger = { const WEBHOOK_SECRET = "test-secret"; const INSTALLATION_ERROR_PATTERN = /installation/i; +const TEST_BOT_MENTION_WITH_WHITESPACE_REGEX = /@test-bot\s+hi there/; function signPayload(body: string): string { return `sha256=${createHmac("sha256", WEBHOOK_SECRET).update(body).digest("hex")}`; @@ -1595,6 +1596,31 @@ describe("GitHubAdapter", () => { expect(message.author.isBot).toBe(false); }); + it("should preserve whitespace in newline-separated issue comment mentions", () => { + const raw = { + type: "issue_comment" as const, + comment: { + id: 100, + body: "@test-bot\nhi there", + user: { id: 1, login: "testuser", type: "User" as const }, + created_at: "2024-01-01T00:00:00Z", + updated_at: "2024-01-01T00:00:00Z", + html_url: "https://github.com/acme/app/pull/42#issuecomment-100", + }, + repository: { + id: 1, + name: "app", + full_name: "acme/app", + owner: { id: 10, login: "acme", type: "User" as const }, + }, + prNumber: 42, + }; + + const message = adapter.parseMessage(raw); + expect(message.text).toMatch(TEST_BOT_MENTION_WITH_WHITESPACE_REGEX); + expect(message.text).not.toContain("@test-bothi there"); + }); + it("should parse an issue_comment raw message from an issue thread", () => { const raw = { type: "issue_comment" as const, @@ -1681,6 +1707,35 @@ describe("GitHubAdapter", () => { expect(message.threadId).toBe("github:acme/app:42:rc:200"); }); + it("should preserve whitespace in newline-separated review comment mentions", () => { + const raw = { + type: "review_comment" as const, + comment: { + id: 200, + body: "@test-bot\nhi there", + user: { id: 2, login: "reviewer", type: "User" as const }, + created_at: "2024-01-01T00:00:00Z", + updated_at: "2024-01-01T00:00:00Z", + html_url: "https://github.com/acme/app/pull/42#discussion_r200", + path: "src/index.ts", + diff_hunk: "@@", + commit_id: "abc", + original_commit_id: "abc", + }, + repository: { + id: 1, + name: "app", + full_name: "acme/app", + owner: { id: 10, login: "acme", type: "User" as const }, + }, + prNumber: 42, + }; + + const message = adapter.parseMessage(raw); + expect(message.text).toMatch(TEST_BOT_MENTION_WITH_WHITESPACE_REGEX); + expect(message.text).not.toContain("@test-bothi there"); + }); + it("should parse a review_comment raw message (reply)", () => { const raw = { type: "review_comment" as const, diff --git a/packages/adapter-github/src/markdown.test.ts b/packages/adapter-github/src/markdown.test.ts index b1d705e2d..a4f3c57bf 100644 --- a/packages/adapter-github/src/markdown.test.ts +++ b/packages/adapter-github/src/markdown.test.ts @@ -1,6 +1,8 @@ import { describe, expect, it } from "vitest"; import { GitHubFormatConverter } from "./markdown"; +const TEST_BOT_MENTION_WITH_WHITESPACE_REGEX = /@test-bot\s+hi there/; + describe("GitHubFormatConverter", () => { const converter = new GitHubFormatConverter(); @@ -107,6 +109,11 @@ describe("GitHubFormatConverter", () => { expect(result).toContain("thanks"); }); + it("should preserve whitespace after newline-separated @mentions", () => { + const result = converter.extractPlainText("@test-bot\nhi there"); + expect(result).toMatch(TEST_BOT_MENTION_WITH_WHITESPACE_REGEX); + }); + it("should extract text from code blocks", () => { const result = converter.extractPlainText("```\ncode\n```"); expect(result).toContain("code"); diff --git a/packages/chat/src/chat.test.ts b/packages/chat/src/chat.test.ts index 69609eda9..9336aa490 100644 --- a/packages/chat/src/chat.test.ts +++ b/packages/chat/src/chat.test.ts @@ -220,6 +220,70 @@ describe("Chat", () => { expect(mockState.releaseLock).toHaveBeenCalled(); }); + it("should call onNewMention for newline-separated GitHub bot mentions", async () => { + const githubAdapter = { + ...createMockAdapter("github"), + userName: "test-bot", + }; + const state = createMockState(); + const githubChat = new Chat({ + userName: "fallback-bot", + adapters: { github: githubAdapter }, + state, + logger: mockLogger, + }); + + await githubChat.webhooks.github( + new Request("http://test.com", { method: "POST" }) + ); + + const handler = vi.fn().mockResolvedValue(undefined); + githubChat.onNewMention(handler); + + const message = createTestMessage("msg-1", "@test-bot\nhi there"); + + await githubChat.handleIncomingMessage( + githubAdapter, + "github:acme/app:42", + message + ); + + expect(handler).toHaveBeenCalled(); + const [, receivedMessage] = handler.mock.calls[0]; + expect(receivedMessage.isMention).toBe(true); + }); + + it("should not call onNewMention for concatenated GitHub bot mention text", async () => { + const githubAdapter = { + ...createMockAdapter("github"), + userName: "test-bot", + }; + const state = createMockState(); + const githubChat = new Chat({ + userName: "fallback-bot", + adapters: { github: githubAdapter }, + state, + logger: mockLogger, + }); + + await githubChat.webhooks.github( + new Request("http://test.com", { method: "POST" }) + ); + + const handler = vi.fn().mockResolvedValue(undefined); + githubChat.onNewMention(handler); + + const message = createTestMessage("msg-1", "@test-bothi there"); + + await githubChat.handleIncomingMessage( + githubAdapter, + "github:acme/app:42", + message + ); + + expect(handler).not.toHaveBeenCalled(); + }); + it("should call onSubscribedMessage handler for subscribed threads", async () => { const mentionHandler = vi.fn().mockResolvedValue(undefined); const subscribedHandler = vi.fn().mockResolvedValue(undefined); diff --git a/packages/chat/src/markdown.test.ts b/packages/chat/src/markdown.test.ts index 6f3ba28c5..9cfbafd55 100644 --- a/packages/chat/src/markdown.test.ts +++ b/packages/chat/src/markdown.test.ts @@ -52,6 +52,9 @@ import { walkAst, } from "./markdown"; +const BOT_MENTION_WITH_WHITESPACE_REGEX = /@test-bot\s+hi there/; +const LIST_ITEMS_WITH_WHITESPACE_REGEX = /one\s+two/; + // ============================================================================ // parseMarkdown Tests // ============================================================================ @@ -217,6 +220,32 @@ describe("toPlainText", () => { expect(result).toBe("link text"); }); + it("preserves soft line breaks as whitespace", () => { + const ast = parseMarkdown("@test-bot\nhi there"); + const result = toPlainText(ast); + expect(result).toMatch(BOT_MENTION_WITH_WHITESPACE_REGEX); + }); + + it("preserves paragraph boundaries as whitespace", () => { + const ast = parseMarkdown("@test-bot\n\nhi there"); + const result = toPlainText(ast); + expect(result).toMatch(BOT_MENTION_WITH_WHITESPACE_REGEX); + }); + + it("separates list items with whitespace", () => { + const ast = parseMarkdown("- one\n- two"); + const result = toPlainText(ast); + expect(result).toMatch(LIST_ITEMS_WITH_WHITESPACE_REGEX); + }); + + it("separates table cells and rows with structural whitespace", () => { + const ast = parseMarkdown( + "| Name | Role |\n| --- | --- |\n| **Ada** Lovelace | Engineer |" + ); + const result = toPlainText(ast); + expect(result).toBe("Name\tRole\nAda Lovelace\tEngineer"); + }); + it("handles empty AST", () => { const ast = root([]); const result = toPlainText(ast); @@ -239,6 +268,16 @@ describe("markdownToPlainText", () => { expect(result).toContain("Heading"); expect(result).toContain("Paragraph with code"); }); + + it("preserves whitespace after a newline-separated mention", () => { + const result = markdownToPlainText("@test-bot\nhi there"); + expect(result).toMatch(BOT_MENTION_WITH_WHITESPACE_REGEX); + }); + + it("preserves whitespace after a paragraph-separated mention", () => { + const result = markdownToPlainText("@test-bot\n\nhi there"); + expect(result).toMatch(BOT_MENTION_WITH_WHITESPACE_REGEX); + }); }); // ============================================================================ diff --git a/packages/chat/src/markdown.ts b/packages/chat/src/markdown.ts index 6201a575f..ca5000c45 100644 --- a/packages/chat/src/markdown.ts +++ b/packages/chat/src/markdown.ts @@ -285,11 +285,60 @@ export function stringifyMarkdown( return processor.stringify(ast); } +function nodeValue(node: Content | Root): string | null { + if ("value" in node && typeof node.value === "string") { + return node.value; + } + if ("alt" in node && typeof node.alt === "string") { + return node.alt; + } + return null; +} + +function childPlainText(node: Content | Root, separator: string): string { + if (!("children" in node && Array.isArray(node.children))) { + return ""; + } + + return (node.children as (Content | Root)[]) + .map((child) => plainTextNode(child)) + .filter((text) => text.length > 0) + .join(separator); +} + +function plainTextNode(node: Content | Root): string { + const value = nodeValue(node); + if (value !== null) { + return value; + } + + switch (node.type) { + case "root": + return childPlainText(node, "\n\n"); + case "list": + case "table": + return childPlainText(node, "\n"); + case "listItem": + case "blockquote": + return childPlainText(node, "\n"); + case "tableRow": + return childPlainText(node, "\t"); + case "break": + return "\n"; + case "thematicBreak": + return ""; + case "tableCell": + return childPlainText(node, ""); + default: + return childPlainText(node, ""); + } +} + /** * Extract plain text from an AST (strips all formatting). */ export function toPlainText(ast: Root): string { - return mdastToString(ast); + return plainTextNode(ast); } /** @@ -297,7 +346,7 @@ export function toPlainText(ast: Root): string { */ export function markdownToPlainText(markdown: string): string { const ast = parseMarkdown(markdown); - return mdastToString(ast); + return toPlainText(ast); } /** diff --git a/packages/chat/src/thread.test.ts b/packages/chat/src/thread.test.ts index 3a551e815..63134ba9a 100644 --- a/packages/chat/src/thread.test.ts +++ b/packages/chat/src/thread.test.ts @@ -529,9 +529,8 @@ describe("ThreadImpl", () => { const textStream = createTextStream(["hello.", "\n\n", "how are you?"]); const result = await thread.post(textStream); - // Plain text extraction from parsed markdown joins paragraphs without separator - // (mdast-util-to-string behavior). The formatted AST preserves paragraph structure. - expect(result.text).toBe("hello.how are you?"); + // Plain text extraction preserves paragraph boundaries from parsed markdown. + expect(result.text).toBe("hello.\n\nhow are you?"); expect(capturedChunks).toEqual(["hello.", "\n\n", "how are you?"]); });