diff --git a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/markdown-fidelity.ts b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/markdown-fidelity.ts index 94511d5ebad..43466f49861 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/markdown-fidelity.ts +++ b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/markdown-fidelity.ts @@ -172,9 +172,14 @@ function stripEmptyListItemLines(markdown: string): string { * Cleans up serializer output: drops empty list-item marker lines that would otherwise corrupt on * round-trip ({@link stripEmptyListItemLines}), restores callout markers the serializer * backslash-escapes (`> \[!NOTE\]` → `> [!NOTE]`), and collapses trailing blank lines to a single - * newline. The table serializer's spurious surrounding blank lines are trimmed at the source - * (PipeSafeTable), so no global leading-newline strip is needed here — avoiding clobbering content - * that legitimately begins with whitespace. + * newline. Interior blank runs are NOT collapsed here — blank lines inside a fenced code block (or a + * verbatim raw-markdown-snippet) are significant, and a global collapse would corrupt them. Spurious + * interior blank runs between top-level blocks are removed upstream instead, by + * {@link parseMarkdownToDoc} stripping empty paragraphs, so a doc that has been through the editor + * never serializes with an interior blank run outside code in the first place. The table serializer's + * spurious surrounding blank lines are trimmed at the source (PipeSafeTable), so no global + * leading-newline strip is needed here — avoiding clobbering content that legitimately begins with + * whitespace. */ export function postProcessSerializedMarkdown(markdown: string): string { return collapseAutolinkedUrls( diff --git a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/markdown-parse.test.ts b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/markdown-parse.test.ts index d59ef6afe6e..0067ef31f47 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/markdown-parse.test.ts +++ b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/markdown-parse.test.ts @@ -7,6 +7,10 @@ import { createMarkdownContentExtensions } from './extensions' import { parseMarkdownToDoc, serializeMarkdownBody, splitMarkdownBlocks } from './markdown-parse' import { isRoundTripSafe } from './round-trip-safety' +/** Mirror of the production `isEmptyParagraph` (not exported): the shape a blank line reconstructs to. */ +const isEmptyPara = (n: { type?: string; content?: unknown[] }): boolean => + n.type === 'paragraph' && !n.content?.length + let editor: Editor | null = null afterEach(() => { editor?.destroy() @@ -59,12 +63,6 @@ const CASES: Array<[string, string]> = [ '1. First\n - sub bullet\n - another\n 1. deep ordered\n 2. item\n2. Second', ], ['heading-separated sections', '# A\n\nalpha\n\n## B\n\nbeta\n\n## C\n\ngamma'], - // Blank-line spacing: `@tiptap/markdown` reconstructs empty paragraphs from runs of blank lines, so - // the chunker must reinsert them or a saved blank line vanishes on reload. See the dedicated - // "empty paragraphs" suite below for the exact whole-document-parser parity. - ['one empty paragraph between paragraphs', 'first\n\n\n\nsecond'], - ['two empty paragraphs between paragraphs', 'first\n\n\n\n\n\nsecond'], - ['empty paragraphs between headings and text', '# A\n\n\n\nalpha\n\n\n\n## B'], ] describe('parseMarkdownToDoc (chunked)', () => { @@ -93,63 +91,62 @@ describe('parseMarkdownToDoc (chunked)', () => { expect(splitMarkdownBlocks('\n\n \n')).toEqual([]) }) - // The chunker used to drop empty paragraphs (visual blank lines between blocks) that the whole-document - // parser preserves, so a saved blank line silently vanished on the next load. These assert the chunked - // parse reconstructs the SAME empty-paragraph structure the whole-document parser does — at document - // edges and between blocks, for one or many blank lines, and around lists. - describe('empty paragraphs (blank-line spacing) match the whole-document parser', () => { - /** Block-type shape of a doc, `∅` for an empty paragraph, normalized through the editor. */ - function shapeOf(md: string, parse: 'chunked' | 'whole'): string { - editor = new Editor({ extensions: createMarkdownContentExtensions() }) - if (parse === 'whole') editor.commands.setContent(md, { contentType: 'markdown' }) - else editor.commands.setContent(parseMarkdownToDoc(md), { contentType: 'json' }) - const shape = (editor.getJSON().content ?? []) - .map((n) => (n.type === 'paragraph' && !n.content?.length ? '∅' : n.type)) + // Asserts the collapse documented on `stripEmptyParagraphs` — at document edges, between blocks, for + // one or many blank lines, and around lists. (Blank runs are insignificant in markdown, so a collapsed + // file renders identically everywhere it's viewed; the pathological case is a run of thousands.) + describe('collapses blank-line runs to markdown-standard spacing', () => { + /** Block-type shape of a doc after `parseMarkdownToDoc`, `∅` for any surviving empty paragraph. */ + function shapeOf(md: string): string { + return (parseMarkdownToDoc(md).content ?? []) + .map((n) => (isEmptyPara(n) ? '∅' : n.type)) .join(',') - editor.destroy() - editor = null - return shape } it.each([ - ['one empty between paragraphs', 'a\n\n\n\nb'], - ['two empties between paragraphs', 'a\n\n\n\n\n\nb'], - ['three empties between paragraphs', 'a\n\n\n\n\n\n\n\nb'], - ['even blank-line gap (rounds down)', 'a\n\n\n\n\nb'], - ['leading empties', '\n\n\n\na'], - ['leading + between', '\n\n\na\n\n\n\nb'], - ['empties between a heading and text', '# H\n\n\n\ntext'], - ['empties after a tight list', '- a\n- b\n\n\n\ntext'], - ['empties before a tight list', 'text\n\n\n\n- a\n- b'], - // Line-ending variants: the whole-vs-chunked routing must normalize first, or a `\r`-only body - // skips the empty-paragraph guard and is chunked (dropping the empties this fix restores). - ['CRLF between empties', 'a\r\n\r\n\r\n\r\nb'], - ['CR-only (classic Mac) between empties', 'a\r\r\r\rb'], - ])('chunked matches whole-doc: %s', (_label, md) => { - expect(shapeOf(md, 'chunked')).toBe(shapeOf(md, 'whole')) + ['one blank gap between paragraphs', 'a\n\n\n\nb', 'paragraph,paragraph'], + ['many blank lines between paragraphs', 'a\n\n\n\n\n\n\n\nb', 'paragraph,paragraph'], + ['leading blank lines', '\n\n\n\na', 'paragraph'], + ['leading + interior', '\n\n\na\n\n\n\nb', 'paragraph,paragraph'], + ['blank gap between a heading and text', '# H\n\n\n\ntext', 'heading,paragraph'], + ['blank gap after a tight list', '- a\n- b\n\n\n\ntext', 'bulletList,paragraph'], + ['blank gap before a tight list', 'text\n\n\n\n- a\n- b', 'paragraph,bulletList'], + // Line-ending variants normalize first, so `\r`-only / CRLF blank runs collapse identically. + ['CRLF between blocks', 'a\r\n\r\n\r\n\r\nb', 'paragraph,paragraph'], + ['CR-only (classic Mac) between blocks', 'a\r\r\r\rb', 'paragraph,paragraph'], + ])('collapses to no empty paragraphs: %s', (_label, md, expected) => { + expect(shapeOf(md)).toBe(expected) + }) + + it('a pathological blank run does not explode into empty paragraph nodes', () => { + // The production incident: an agent/paste artifact with a huge blank run became ~1959 empty + // paragraphs baked into the doc. Collapsing on parse neutralizes any such source. + const body = `Para A${'\n'.repeat(4000)}Para B` + const content = parseMarkdownToDoc(body).content ?? [] + expect(content.filter(isEmptyPara).length).toBe(0) + expect(content.map((n) => n.type)).toEqual(['paragraph', 'paragraph']) }) }) - // Regression: a file ending in a blank line (a trailing empty paragraph) must stay EDITABLE. Such an - // empty paragraph can't be serialized stably (postProcess collapses trailing newlines), so the parser - // strips it — keeping the doc round-trip-safe/idempotent instead of flipping the file read-only. - describe('trailing blank lines stay editable (regression)', () => { + // Regression: a file with blank lines (leading, interior, or trailing) must stay EDITABLE. Collapsing + // blank runs keeps serialize→parse idempotent, so the round-trip-safety probe reaches a fixed point + // instead of flipping the file read-only. + describe('blank lines stay editable (regression)', () => { it.each([ ['plain paragraph', 'abc\n\n'], ['heading + text', '# Title\n\nSome text\n\n'], ['three trailing newlines', 'hello\n\n\n'], ['two paragraphs', 'para one\n\npara two\n\n'], - ['interior empties + trailing', 'a\n\n\n\nb\n\n'], - ])('a file ending in a blank line is round-trip-safe: %s', (_label, md) => { + ['interior blank run + trailing', 'a\n\n\n\nb\n\n'], + ])('a file with blank lines is round-trip-safe: %s', (_label, md) => { expect(isRoundTripSafe(md)).toBe(true) }) - it('strips the trailing empty paragraph but keeps interior ones', () => { + it('removes only structurally-empty paragraphs — a paragraph with content survives', () => { + // The shape suite above already proves leading/interior/trailing blank runs collapse to zero empty + // paragraphs; this pins the complementary guarantee — a real (non-empty) paragraph is never dropped. const trailing = parseMarkdownToDoc('abc\n\n').content ?? [] expect(trailing.at(-1)?.type).toBe('paragraph') - expect(trailing.at(-1)?.content?.length ?? 0).toBeGreaterThan(0) - const interior = parseMarkdownToDoc('a\n\n\n\nb').content ?? [] - expect(interior.some((n) => n.type === 'paragraph' && !n.content?.length)).toBe(true) + expect(isEmptyPara(trailing.at(-1) ?? {})).toBe(false) }) }) diff --git a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/markdown-parse.ts b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/markdown-parse.ts index d8255eef7e6..6cdeeabf4ae 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/markdown-parse.ts +++ b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/markdown-parse.ts @@ -47,20 +47,6 @@ const FENCE_CLOSE = /^ {0,3}(`{3,}|~{3,})[ \t]*$/ const LIST_MARKER = /^[ ]{0,3}(?:[-*+]|\d+[.)])\s/ const BLOCKQUOTE = /^[ ]{0,3}>/ -/** - * Blank-line spacing that `@tiptap/markdown` reconstructs as *interior* or *leading* empty paragraphs — - * a run of two or more blank lines somewhere, or blank line(s) at the document's leading edge. `[^\S\n]` - * matches horizontal whitespace, so a "blank" line may carry spaces/tabs. This is only ever tested - * against the `\r`-normalized body ({@link parseMarkdownToDoc}), so no CRLF handling is needed here. - * - * A *single* trailing blank line is deliberately not matched — purely to avoid routing an otherwise-plain - * file to the slower whole-document parser. Correctness does not depend on it: {@link parseMarkdownToDoc} - * strips trailing empty paragraphs on *both* parse paths ({@link stripTrailingEmptyParagraphs}), so - * serialize→parse stays idempotent regardless of which parser ran. (A trailing run of two or more blanks - * still matches the interior alternative — harmless, since the strip cleans it either way.) - */ -const EMPTY_PARAGRAPH_SPACING = /\n[^\S\n]*\n[^\S\n]*\n|^[^\S\n]*\n[^\S\n]*\n/ - /** * Split a markdown body into top-level blocks that can each be parsed independently and reassembled * without changing meaning. Blank lines separate candidate groups (fenced code blocks stay atomic), @@ -135,21 +121,20 @@ export function splitMarkdownBlocks(body: string): string[] { * Documents whose constructs span blocks ({@link NON_CHUNKABLE}) parse whole, and any failure falls * back to a single whole-document parse, so correctness never depends on the splitter. * - * Blank-line spacing ({@link EMPTY_PARAGRAPH_SPACING}) also parses whole: the chunker parses each block - * stripped of the blank lines between them, so it drops the empty paragraphs `@tiptap/markdown` builds - * from runs of blank lines — a saved visual blank line would silently vanish on reload. Whether a gap - * yields an empty paragraph is a global, block-type-dependent decision (kept between two paragraphs, - * dropped after a heading), so it can't be reconstructed block-locally; these documents parse whole for - * exact fidelity. Ordinary single-blank-line separation still takes the fast chunked path. + * Runs of blank lines take the fast chunked path too: the chunker parses each block stripped of the + * blank lines between them, which drops the empty paragraphs `@tiptap/markdown` reconstructs from a + * blank run — exactly what {@link stripEmptyParagraphs} does to the whole-parse output anyway. A blank + * run between blocks is insignificant in markdown, so collapsing it is the intended normalization (see + * {@link stripEmptyParagraphs}), and both parse paths converge on the same empty-paragraph-free result. */ export function parseMarkdownToDoc(body: string): JSONContent { const manager = markdownManager() - // Normalize line endings up front so the routing guards see the same `\n` the chunker and parser - // do — the guards' `\n`-anchored tests would otherwise miss a classic `\r`-only body (its blank - // lines are `\r`), routing it to the chunker that then drops its empty paragraphs. + // Normalize line endings up front so {@link NON_CHUNKABLE}'s `\n`-anchored tests see the same `\n` + // the chunker and parser do — a classic `\r`-only body would otherwise slip past the reference-def / + // block-HTML guard and be chunked, shattering a construct that must parse whole. const normalized = body.replace(/\r\n?/g, '\n') let doc: JSONContent - if (NON_CHUNKABLE.test(normalized) || EMPTY_PARAGRAPH_SPACING.test(normalized)) { + if (NON_CHUNKABLE.test(normalized)) { doc = manager.parse(normalized) } else { try { @@ -163,7 +148,7 @@ export function parseMarkdownToDoc(body: string): JSONContent { doc = manager.parse(normalized) } } - return stripTrailingEmptyParagraphs(doc) + return stripEmptyParagraphs(doc) } /** An empty paragraph node — the shape a blank line reconstructs to (no content, or `content: []`). */ @@ -172,19 +157,26 @@ function isEmptyParagraph(node: JSONContent): boolean { } /** - * Drop trailing empty paragraphs from a parsed doc. {@link postProcessSerializedMarkdown} collapses - * trailing blank lines to a single newline, so a trailing empty paragraph can never round-trip — the - * whole-document parser reconstructs one from a file ending in a blank line, but keeping it makes - * serialize→parse non-idempotent, which flips the file read-only via the round-trip-safety probe. - * Leading/interior empty paragraphs are untouched (postProcess never strips those). TipTap re-adds its - * own trailing filler paragraph on `setContent`, so the editor still has a place to type. + * Drop ALL top-level empty paragraphs from a parsed doc — leading, interior, and trailing. In markdown + * a run of blank lines between blocks is insignificant (CommonMark collapses it), so `@tiptap/markdown` + * reconstructing each blank as an empty paragraph node is not fidelity: it makes the editor render the + * file differently from every standard renderer (GitHub, the download, our own static preview), and a + * pathological blank run (an agent/paste artifact) explodes into thousands of empty nodes that persist + * forever and reflow the doc on open. Collapsing them here keeps normal single-blank-line block spacing + * while removing the spurious gaps, and stays idempotent so the round-trip-safety probe still passes: a + * doc parsed this way has no empty paragraphs, so re-serializing it never re-emits an interior blank run + * (the serializer is intentionally left alone — a blank line inside a fenced code block IS significant), + * and a second parse is a fixed point. Only TOP-LEVEL paragraphs are touched, so blank lines that carry + * meaning inside a construct (e.g. a loose list) are left to the block parser. TipTap re-adds its own + * trailing filler paragraph on `setContent`, so the editor still has a place to type. */ -function stripTrailingEmptyParagraphs(doc: JSONContent): JSONContent { +function stripEmptyParagraphs(doc: JSONContent): JSONContent { const content = doc.content if (!content || content.length === 0) return doc - let end = content.length - while (end > 0 && isEmptyParagraph(content[end - 1])) end-- - return end === content.length ? doc : { ...doc, content: content.slice(0, end) } + // The dominant (chunked) parse already emits no top-level empty paragraphs, so scan before allocating: + // return the doc untouched — no array copy — unless there is actually something to strip. + if (!content.some(isEmptyParagraph)) return doc + return { ...doc, content: content.filter((node) => !isEmptyParagraph(node)) } } /**