From b4e10e8a90f4e9d0d7adffff83a229cd83d9c8fc Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 31 Jul 2026 04:20:39 -0700 Subject: [PATCH] fix: preserve markdown formatting around media attachments (#116786) Co-authored-by: Peter Steinberger --- src/infra/outbound/payloads.test.ts | 45 +++++++ src/media/parse.test.ts | 115 ++++++++++++++++++ src/media/parse.ts | 37 ++---- src/utils/directive-tags.test.ts | 16 +++ src/utils/directive-tags.ts | 21 ++-- ui/src/lib/chat/message-normalizer.test.ts | 80 ++++++++++++ ui/src/lib/chat/message-normalizer.ts | 3 +- .../chat/components/chat-message.test.ts | 15 +++ 8 files changed, 300 insertions(+), 32 deletions(-) diff --git a/src/infra/outbound/payloads.test.ts b/src/infra/outbound/payloads.test.ts index abfbca81ed1d..de13be4efd26 100644 --- a/src/infra/outbound/payloads.test.ts +++ b/src/infra/outbound/payloads.test.ts @@ -691,6 +691,51 @@ describe("OutboundPayloadPlan projections", () => { }, ]); }); + + it.each([ + { + name: "a MEDIA directive", + attachment: "MEDIA:https://example.com/config.png", + extractMarkdownImages: false, + }, + { + name: "an extracted Markdown image", + attachment: "![chart](https://example.com/config.png)", + extractMarkdownImages: true, + }, + ])("preserves formatted reply text when extracting $name", (testCase) => { + const visibleText = [ + "Here is the config.", + "", + "```yaml", + "server:", + " host: 0.0.0.0", + " ports:", + " - 80", + "```", + "", + "The service is ready.", + ].join("\n"); + const [planned] = createOutboundPayloadPlan( + [{ text: `${visibleText}\n\n${testCase.attachment}` }], + { extractMarkdownImages: testCase.extractMarkdownImages }, + ); + + expect(planned?.payload.text).toBe(visibleText); + expect(planned?.payload.mediaUrls).toEqual(["https://example.com/config.png"]); + }); + + it("preserves canonical code fences when reply directives and media share a payload", () => { + const code = ["```python", "value = 'a b'", "``` not a close", "other = 'c d'", "```"].join( + "\n", + ); + const [planned] = createOutboundPayloadPlan([ + { text: `[[reply_to_current]]\n${code}\nMEDIA:https://example.com/config.png` }, + ]); + + expect(planned?.payload.text).toBe(code); + expect(planned?.payload.mediaUrls).toEqual(["https://example.com/config.png"]); + }); }); describe("formatOutboundPayloadLog", () => { diff --git a/src/media/parse.test.ts b/src/media/parse.test.ts index b39a509d5d56..53ad9b154d2d 100644 --- a/src/media/parse.test.ts +++ b/src/media/parse.test.ts @@ -143,7 +143,122 @@ describe("splitMediaFromOutput", () => { ]); }); + it("preserves paragraph breaks in ordered media text segments", () => { + const result = splitMediaFromOutput( + "First paragraph\n\nSecond paragraph\nMEDIA:https://example.com/a.png", + ); + + expect(result.segments).toEqual([ + { type: "text", text: "First paragraph\n\nSecond paragraph" }, + { type: "media", url: "https://example.com/a.png" }, + ]); + }); + + it.each([ + ["before", "First paragraph\n\nMEDIA:https://example.com/a.png\nSecond paragraph"], + ["after", "First paragraph\nMEDIA:https://example.com/a.png\n\nSecond paragraph"], + ["around", "First paragraph\n\nMEDIA:https://example.com/a.png\n\nSecond paragraph"], + ["with spaces", "First paragraph\n \nMEDIA:https://example.com/a.png\n \nSecond paragraph"], + ["with tabs", "First paragraph\n\t\nMEDIA:https://example.com/a.png\n\t\nSecond paragraph"], + ])("preserves a paragraph separator %s an attachment", (_placement, input) => { + const result = splitMediaFromOutput(input); + + expect(result.segments).toEqual([ + { type: "text", text: "First paragraph\n" }, + { type: "media", url: "https://example.com/a.png" }, + { type: "text", text: "Second paragraph" }, + ]); + }); + + it.each([" ", "\t"])("does not emit a whitespace-only media caption: %j", (whitespace) => { + const result = splitMediaFromOutput(`${whitespace}\nMEDIA:https://example.com/a.png`); + + expect(result.text).toBe(""); + expect(result.segments).toEqual([{ type: "media", url: "https://example.com/a.png" }]); + }); + + it("drops separator-only lines before the caption after extracting leading media", () => { + expectParsedMediaOutputCase("MEDIA:https://example.com/a.png\n\nCaption", { + text: "Caption", + mediaUrls: ["https://example.com/a.png"], + }); + }); + + it.each([ + { + name: "a marker carrying trailing text", + lines: ["```python", "value = 'a b'", "``` not a close", "other = 'c d'", "```"], + }, + { + name: "an unclosed fence", + lines: ["```python", "value = 'a b'", "other = 'c d'"], + }, + { + name: "an indented closing fence", + lines: ["```python", "value = 'a b'", " ```"], + }, + ])("preserves canonical code fences with $name", ({ lines }) => { + const code = lines.join("\n"); + + expectParsedMediaOutputCase(`MEDIA:https://example.com/a.png\n${code}`, { + text: code, + mediaUrls: ["https://example.com/a.png"], + }); + expectParsedMediaOutputCase(`[[audio_as_voice]]\nMEDIA:https://example.com/a.png\n${code}`, { + text: code, + mediaUrls: ["https://example.com/a.png"], + audioAsVoice: true, + }); + }); + const extractMarkdownImages = { extractMarkdownImages: true } as const; + const formattedMediaReply = [ + "Here is the code.", + "", + "```python", + "def summarize(rows):", + " totals = {}", + " for row in rows:", + " totals[row] = 1", + " return totals", + "```", + "", + "The attachment is ready.", + ].join("\n"); + + it.each([ + { + name: "a MEDIA directive", + input: `${formattedMediaReply}\n\nMEDIA:https://example.com/config.png`, + mediaUrl: "https://example.com/config.png", + options: undefined, + audioAsVoice: undefined, + }, + { + name: "an extracted Markdown image", + input: `${formattedMediaReply}\n\n![chart](https://example.com/chart.png)`, + mediaUrl: "https://example.com/chart.png", + options: extractMarkdownImages, + audioAsVoice: undefined, + }, + { + name: "an audio directive and media", + input: `[[audio_as_voice]]\n${formattedMediaReply}\n\nMEDIA:https://example.com/recording.ogg`, + mediaUrl: "https://example.com/recording.ogg", + options: undefined, + audioAsVoice: true, + }, + ])("preserves code indentation and paragraph breaks with $name", (testCase) => { + expectParsedMediaOutputCase( + testCase.input, + { + text: formattedMediaReply, + mediaUrls: [testCase.mediaUrl], + ...(testCase.audioAsVoice ? { audioAsVoice: true } : {}), + }, + testCase.options, + ); + }); it("keeps markdown image urls as text by default", () => { const input = "Caption\n\n![chart](https://example.com/chart.png)"; diff --git a/src/media/parse.ts b/src/media/parse.ts index 5b002ffadf50..f9c934dd3cb5 100644 --- a/src/media/parse.ts +++ b/src/media/parse.ts @@ -477,11 +477,6 @@ function collectMarkdownImageSegments(params: { line: string; media: string[] }) }; } -// Check if a character offset is inside any fenced code block -function isInsideFence(fenceSpans: Array<{ start: number; end: number }>, offset: number): boolean { - return fenceSpans.some((span) => offset >= span.start && offset < span.end); -} - /** Splits tool/stdout text into visible text, media attachments, voice tags, and ordered segments. */ export function splitMediaFromOutput( raw: string, @@ -510,17 +505,20 @@ export function splitMediaFromOutput( const media: string[] = []; let foundMediaToken = false; const segments: ParsedMediaOutputSegment[] = []; + let lastTextSegment: Extract | undefined; const pushTextSegment = (text: string) => { - if (!text) { - return; - } const last = segments[segments.length - 1]; if (last?.type === "text") { - last.text = `${last.text}\n${text}`; - return; + last.text = `${last.text}\n${text.trim() ? text : ""}`; + } else if (!text.trim()) { + if (last?.type === "media" && lastTextSegment && !lastTextSegment.text.endsWith("\n")) { + lastTextSegment.text += "\n"; + } + } else { + lastTextSegment = { type: "text", text }; + segments.push(lastTextSegment); } - segments.push({ type: "text", text }); }; // Parse fenced code blocks to avoid extracting MEDIA tokens from inside them @@ -534,7 +532,7 @@ export function splitMediaFromOutput( let lineOffset = 0; // Track character offset for fence checking for (const line of lines) { // Fenced examples must remain text; extracting their MEDIA tokens would mutate transcripts. - if (hasFenceMarkers && isInsideFence(fenceSpans, lineOffset)) { + if (fenceSpans.some((span) => lineOffset >= span.start && lineOffset < span.end)) { keptLines.push(line); pushTextSegment(line); lineOffset += line.length + 1; // +1 for newline @@ -687,19 +685,10 @@ export function splitMediaFromOutput( lineOffset += line.length + 1; // +1 for newline } - let cleanedText = keptLines - .join("\n") - .replace(/[ \t]+\n/g, "\n") - .replace(/[ \t]{2,}/g, " ") - .replace(/\n{2,}/g, "\n") - .trim(); - - // Detect and strip [[audio_as_voice]] tag - const audioTagResult = parseAudioTag(cleanedText); + const visibleText = keptLines.join("\n").replace(/^(?:[ \t]*\n)+/, ""); + const audioTagResult = parseAudioTag(visibleText); + const cleanedText = audioTagResult.text.trimEnd(); const hasAudioAsVoice = audioTagResult.audioAsVoice; - if (audioTagResult.hadTag) { - cleanedText = audioTagResult.text.replace(/\n{2,}/g, "\n").trim(); - } if (media.length === 0) { const parsedText = foundMediaToken || hasAudioAsVoice ? cleanedText : trimmedRaw; diff --git a/src/utils/directive-tags.test.ts b/src/utils/directive-tags.test.ts index 23f201f4867a..009757420412 100644 --- a/src/utils/directive-tags.test.ts +++ b/src/utils/directive-tags.test.ts @@ -205,6 +205,22 @@ describe("parseInlineDirectives", () => { expect(result.text).toBe(["~~~python", " x = 1", " y = 2", "~~~"].join("\n")); }); + test.each([ + [ + "a false closing marker", + ["```python", "value = 'a b'", "``` not a close", "x = 'c d'", "```"], + ], + ["an unclosed fence", ["```python", "value = 'a b'", "x = 'c d'"]], + ["an indented closing fence", ["```python", "value = 'a b'", " ```"]], + ])("preserves canonical code fences with %s after removing directives", (_name, lines) => { + const code = lines.join("\n"); + const result = parseInlineDirectives(`[[reply_to_current]]\n[[audio_as_voice]]\n${code}`); + + expect(result.hasReplyTag).toBe(true); + expect(result.audioAsVoice).toBe(true); + expect(result.text).toBe(code); + }); + test("normalizes plain text without directives using code-fence awareness", () => { const input = "plain text with extra spaces\n\n```\n code preserved\n```"; const result = parseInlineDirectives(input); diff --git a/src/utils/directive-tags.ts b/src/utils/directive-tags.ts index 95524953a922..e9f2ddd7b5a4 100644 --- a/src/utils/directive-tags.ts +++ b/src/utils/directive-tags.ts @@ -1,6 +1,7 @@ import { expectDefined } from "@openclaw/normalization-core"; // Directive tag helpers parse inline directive tags from user text. import { normalizeOptionalString } from "@openclaw/normalization-core/string-coerce"; +import { parseFenceSpans } from "../../packages/markdown-core/src/fences.js"; export type InlineDirectiveParseResult = { text: string; @@ -47,13 +48,19 @@ function normalizeDirectiveWhitespace(text: string): string { const blockSentinel = createBlockSentinel(text); const blockPlaceholderRe = new RegExp(`${blockSentinel}(\\d+)${blockSentinel}`, "g"); const blocks: string[] = []; - const masked = text.replace( - /(`{3,}|~{3,})[^\n]*\n[\s\S]*?\n\1[^\n]*|(?:(?:^|\n)(?: |\t)[^\n]*)+/gm, - (block) => { - blocks.push(block); - return `${blockSentinel}${blocks.length - 1}${blockSentinel}`; - }, - ); + const fenceSpans = text.includes("```") || text.includes("~~~") ? parseFenceSpans(text) : []; + let masked = ""; + let cursor = 0; + // The canonical scanner keeps false closers, indented closers, and open fences intact. + for (const span of fenceSpans) { + blocks.push(text.slice(span.start, span.end)); + masked += `${text.slice(cursor, span.start)}${blockSentinel}${blocks.length - 1}${blockSentinel}`; + cursor = span.end; + } + masked = `${masked}${text.slice(cursor)}`.replace(/(?:(?:^|\n)(?: |\t)[^\n]*)+/gm, (block) => { + blocks.push(block); + return `${blockSentinel}${blocks.length - 1}${blockSentinel}`; + }); const normalized = masked .replace(/\r\n/g, "\n") diff --git a/ui/src/lib/chat/message-normalizer.test.ts b/ui/src/lib/chat/message-normalizer.test.ts index 75668e23a92a..dc4acc5e030b 100644 --- a/ui/src/lib/chat/message-normalizer.test.ts +++ b/ui/src/lib/chat/message-normalizer.test.ts @@ -432,6 +432,86 @@ describe("message-normalizer", () => { ]); }); + it("preserves paragraph breaks and code indentation before an assistant attachment", () => { + const text = [ + "Here is the code.", + "", + "```python", + "def run():", + " if ready:", + " return True", + "```", + "", + "The attachment is ready.", + ].join("\n"); + + expect( + normalizeMessage({ + role: "assistant", + content: `${text}\nMEDIA:https://example.com/image.png`, + }).content, + ).toEqual([ + { type: "text", text }, + { + type: "attachment", + attachment: { + url: "https://example.com/image.png", + kind: "image", + label: "image.png", + mimeType: "image/png", + }, + }, + ]); + }); + + it.each(["", " ", "\t"])( + "preserves a %j paragraph separator around an assistant attachment", + (whitespace) => { + expect( + normalizeMessage({ + role: "assistant", + content: `First paragraph\n${whitespace}\nMEDIA:https://example.com/image.png\n${whitespace}\nSecond paragraph`, + }).content, + ).toEqual([ + { type: "text", text: "First paragraph\n" }, + { + type: "attachment", + attachment: { + url: "https://example.com/image.png", + kind: "image", + label: "image.png", + mimeType: "image/png", + }, + }, + { type: "text", text: "Second paragraph" }, + ]); + }, + ); + + it("preserves canonical code fences after removing reply and audio directives", () => { + const code = ["```python", "value = 'a b'", "``` not a close", "other = 'c d'", "```"].join( + "\n", + ); + + expect( + normalizeMessage({ + role: "assistant", + content: `[[reply_to_current]]\n[[audio_as_voice]]\n${code}\nMEDIA:https://example.com/image.png`, + }).content, + ).toEqual([ + { type: "text", text: code }, + { + type: "attachment", + attachment: { + url: "https://example.com/image.png", + kind: "image", + label: "image.png", + mimeType: "image/png", + }, + }, + ]); + }); + it("marks media-only audio attachments as voice notes when audio_as_voice is present", () => { const result = normalizeMessage({ role: "assistant", diff --git a/ui/src/lib/chat/message-normalizer.ts b/ui/src/lib/chat/message-normalizer.ts index 50fc5c9d4423..40d87efc2e66 100644 --- a/ui/src/lib/chat/message-normalizer.ts +++ b/ui/src/lib/chat/message-normalizer.ts @@ -412,7 +412,8 @@ function expandTextContent(text: string): { replyTarget = { kind: "current" }; } if (directives.text) { - parts.push({ type: "text", text: directives.text }); + const normalizedText = directives.text + (segment.text.endsWith("\n") ? "\n" : ""); + parts.push({ type: "text", text: normalizedText }); } } for (const preview of extracted.previews) { diff --git a/ui/src/pages/chat/components/chat-message.test.ts b/ui/src/pages/chat/components/chat-message.test.ts index 1b4b5deb0c44..771c3fb8557f 100644 --- a/ui/src/pages/chat/components/chat-message.test.ts +++ b/ui/src/pages/chat/components/chat-message.test.ts @@ -622,6 +622,21 @@ afterEach(() => { }); describe("grouped chat rendering", () => { + it("preserves paragraph breaks around assistant attachments in rendered markdown", () => { + const container = document.createElement("div"); + + renderAssistantMessage(container, { + role: "assistant", + content: "First paragraph\n \nMEDIA:https://example.com/image.png\n\t\nSecond paragraph", + timestamp: 1000, + }); + + expect(markdownRenderMock).toHaveBeenCalledWith( + "First paragraph\n\nSecond paragraph", + expect.any(Object), + ); + }); + it("renders a compact count for collapsed duplicate messages", () => { const container = document.createElement("div"); renderAssistantMessageEntries(container, [