Skip to content

Commit 5460133

Browse files
authored
fix(files): collapse blank-line runs to markdown standard (no empty-paragraph explosion / reflow) (#6198)
parseMarkdownToDoc now strips ALL top-level empty paragraphs (leading, interior, trailing), not just trailing. A run of blank lines between blocks is insignificant in markdown (CommonMark collapses it), but @tiptap/markdown reconstructs one empty paragraph per blank line — which made the mounted editor render vertical gaps that exist nowhere else the file is viewed (GitHub, download, our own static preview), let a pathological blank run explode into thousands of empty nodes, and caused the visible reflow on open (static preview collapses empty <p>; the live editor gives each a trailing-break line). Collapsing on parse keeps normal one-blank-line spacing, matches every standard renderer, and stays idempotent so the round-trip-safety probe still reaches a fixed point (files stay editable; existing files normalize on next cold-open + save). Serializer is intentionally NOT changed — a global blank-run collapse there would corrupt blank lines inside fenced code blocks.
1 parent 746756d commit 5460133

3 files changed

Lines changed: 78 additions & 84 deletions

File tree

apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/markdown-fidelity.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -172,9 +172,14 @@ function stripEmptyListItemLines(markdown: string): string {
172172
* Cleans up serializer output: drops empty list-item marker lines that would otherwise corrupt on
173173
* round-trip ({@link stripEmptyListItemLines}), restores callout markers the serializer
174174
* backslash-escapes (`> \[!NOTE\]` → `> [!NOTE]`), and collapses trailing blank lines to a single
175-
* newline. The table serializer's spurious surrounding blank lines are trimmed at the source
176-
* (PipeSafeTable), so no global leading-newline strip is needed here — avoiding clobbering content
177-
* that legitimately begins with whitespace.
175+
* newline. Interior blank runs are NOT collapsed here — blank lines inside a fenced code block (or a
176+
* verbatim raw-markdown-snippet) are significant, and a global collapse would corrupt them. Spurious
177+
* interior blank runs between top-level blocks are removed upstream instead, by
178+
* {@link parseMarkdownToDoc} stripping empty paragraphs, so a doc that has been through the editor
179+
* never serializes with an interior blank run outside code in the first place. The table serializer's
180+
* spurious surrounding blank lines are trimmed at the source (PipeSafeTable), so no global
181+
* leading-newline strip is needed here — avoiding clobbering content that legitimately begins with
182+
* whitespace.
178183
*/
179184
export function postProcessSerializedMarkdown(markdown: string): string {
180185
return collapseAutolinkedUrls(

apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/markdown-parse.test.ts

Lines changed: 43 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,10 @@ import { createMarkdownContentExtensions } from './extensions'
77
import { parseMarkdownToDoc, serializeMarkdownBody, splitMarkdownBlocks } from './markdown-parse'
88
import { isRoundTripSafe } from './round-trip-safety'
99

10+
/** Mirror of the production `isEmptyParagraph` (not exported): the shape a blank line reconstructs to. */
11+
const isEmptyPara = (n: { type?: string; content?: unknown[] }): boolean =>
12+
n.type === 'paragraph' && !n.content?.length
13+
1014
let editor: Editor | null = null
1115
afterEach(() => {
1216
editor?.destroy()
@@ -59,12 +63,6 @@ const CASES: Array<[string, string]> = [
5963
'1. First\n - sub bullet\n - another\n 1. deep ordered\n 2. item\n2. Second',
6064
],
6165
['heading-separated sections', '# A\n\nalpha\n\n## B\n\nbeta\n\n## C\n\ngamma'],
62-
// Blank-line spacing: `@tiptap/markdown` reconstructs empty paragraphs from runs of blank lines, so
63-
// the chunker must reinsert them or a saved blank line vanishes on reload. See the dedicated
64-
// "empty paragraphs" suite below for the exact whole-document-parser parity.
65-
['one empty paragraph between paragraphs', 'first\n\n\n\nsecond'],
66-
['two empty paragraphs between paragraphs', 'first\n\n\n\n\n\nsecond'],
67-
['empty paragraphs between headings and text', '# A\n\n\n\nalpha\n\n\n\n## B'],
6866
]
6967

7068
describe('parseMarkdownToDoc (chunked)', () => {
@@ -93,63 +91,62 @@ describe('parseMarkdownToDoc (chunked)', () => {
9391
expect(splitMarkdownBlocks('\n\n \n')).toEqual([])
9492
})
9593

96-
// The chunker used to drop empty paragraphs (visual blank lines between blocks) that the whole-document
97-
// parser preserves, so a saved blank line silently vanished on the next load. These assert the chunked
98-
// parse reconstructs the SAME empty-paragraph structure the whole-document parser does — at document
99-
// edges and between blocks, for one or many blank lines, and around lists.
100-
describe('empty paragraphs (blank-line spacing) match the whole-document parser', () => {
101-
/** Block-type shape of a doc, `∅` for an empty paragraph, normalized through the editor. */
102-
function shapeOf(md: string, parse: 'chunked' | 'whole'): string {
103-
editor = new Editor({ extensions: createMarkdownContentExtensions() })
104-
if (parse === 'whole') editor.commands.setContent(md, { contentType: 'markdown' })
105-
else editor.commands.setContent(parseMarkdownToDoc(md), { contentType: 'json' })
106-
const shape = (editor.getJSON().content ?? [])
107-
.map((n) => (n.type === 'paragraph' && !n.content?.length ? '∅' : n.type))
94+
// Asserts the collapse documented on `stripEmptyParagraphs` — at document edges, between blocks, for
95+
// one or many blank lines, and around lists. (Blank runs are insignificant in markdown, so a collapsed
96+
// file renders identically everywhere it's viewed; the pathological case is a run of thousands.)
97+
describe('collapses blank-line runs to markdown-standard spacing', () => {
98+
/** Block-type shape of a doc after `parseMarkdownToDoc`, `∅` for any surviving empty paragraph. */
99+
function shapeOf(md: string): string {
100+
return (parseMarkdownToDoc(md).content ?? [])
101+
.map((n) => (isEmptyPara(n) ? '∅' : n.type))
108102
.join(',')
109-
editor.destroy()
110-
editor = null
111-
return shape
112103
}
113104

114105
it.each([
115-
['one empty between paragraphs', 'a\n\n\n\nb'],
116-
['two empties between paragraphs', 'a\n\n\n\n\n\nb'],
117-
['three empties between paragraphs', 'a\n\n\n\n\n\n\n\nb'],
118-
['even blank-line gap (rounds down)', 'a\n\n\n\n\nb'],
119-
['leading empties', '\n\n\n\na'],
120-
['leading + between', '\n\n\na\n\n\n\nb'],
121-
['empties between a heading and text', '# H\n\n\n\ntext'],
122-
['empties after a tight list', '- a\n- b\n\n\n\ntext'],
123-
['empties before a tight list', 'text\n\n\n\n- a\n- b'],
124-
// Line-ending variants: the whole-vs-chunked routing must normalize first, or a `\r`-only body
125-
// skips the empty-paragraph guard and is chunked (dropping the empties this fix restores).
126-
['CRLF between empties', 'a\r\n\r\n\r\n\r\nb'],
127-
['CR-only (classic Mac) between empties', 'a\r\r\r\rb'],
128-
])('chunked matches whole-doc: %s', (_label, md) => {
129-
expect(shapeOf(md, 'chunked')).toBe(shapeOf(md, 'whole'))
106+
['one blank gap between paragraphs', 'a\n\n\n\nb', 'paragraph,paragraph'],
107+
['many blank lines between paragraphs', 'a\n\n\n\n\n\n\n\nb', 'paragraph,paragraph'],
108+
['leading blank lines', '\n\n\n\na', 'paragraph'],
109+
['leading + interior', '\n\n\na\n\n\n\nb', 'paragraph,paragraph'],
110+
['blank gap between a heading and text', '# H\n\n\n\ntext', 'heading,paragraph'],
111+
['blank gap after a tight list', '- a\n- b\n\n\n\ntext', 'bulletList,paragraph'],
112+
['blank gap before a tight list', 'text\n\n\n\n- a\n- b', 'paragraph,bulletList'],
113+
// Line-ending variants normalize first, so `\r`-only / CRLF blank runs collapse identically.
114+
['CRLF between blocks', 'a\r\n\r\n\r\n\r\nb', 'paragraph,paragraph'],
115+
['CR-only (classic Mac) between blocks', 'a\r\r\r\rb', 'paragraph,paragraph'],
116+
])('collapses to no empty paragraphs: %s', (_label, md, expected) => {
117+
expect(shapeOf(md)).toBe(expected)
118+
})
119+
120+
it('a pathological blank run does not explode into empty paragraph nodes', () => {
121+
// The production incident: an agent/paste artifact with a huge blank run became ~1959 empty
122+
// paragraphs baked into the doc. Collapsing on parse neutralizes any such source.
123+
const body = `Para A${'\n'.repeat(4000)}Para B`
124+
const content = parseMarkdownToDoc(body).content ?? []
125+
expect(content.filter(isEmptyPara).length).toBe(0)
126+
expect(content.map((n) => n.type)).toEqual(['paragraph', 'paragraph'])
130127
})
131128
})
132129

133-
// Regression: a file ending in a blank line (a trailing empty paragraph) must stay EDITABLE. Such an
134-
// empty paragraph can't be serialized stably (postProcess collapses trailing newlines), so the parser
135-
// strips it — keeping the doc round-trip-safe/idempotent instead of flipping the file read-only.
136-
describe('trailing blank lines stay editable (regression)', () => {
130+
// Regression: a file with blank lines (leading, interior, or trailing) must stay EDITABLE. Collapsing
131+
// blank runs keeps serialize→parse idempotent, so the round-trip-safety probe reaches a fixed point
132+
// instead of flipping the file read-only.
133+
describe('blank lines stay editable (regression)', () => {
137134
it.each([
138135
['plain paragraph', 'abc\n\n'],
139136
['heading + text', '# Title\n\nSome text\n\n'],
140137
['three trailing newlines', 'hello\n\n\n'],
141138
['two paragraphs', 'para one\n\npara two\n\n'],
142-
['interior empties + trailing', 'a\n\n\n\nb\n\n'],
143-
])('a file ending in a blank line is round-trip-safe: %s', (_label, md) => {
139+
['interior blank run + trailing', 'a\n\n\n\nb\n\n'],
140+
])('a file with blank lines is round-trip-safe: %s', (_label, md) => {
144141
expect(isRoundTripSafe(md)).toBe(true)
145142
})
146143

147-
it('strips the trailing empty paragraph but keeps interior ones', () => {
144+
it('removes only structurally-empty paragraphs — a paragraph with content survives', () => {
145+
// The shape suite above already proves leading/interior/trailing blank runs collapse to zero empty
146+
// paragraphs; this pins the complementary guarantee — a real (non-empty) paragraph is never dropped.
148147
const trailing = parseMarkdownToDoc('abc\n\n').content ?? []
149148
expect(trailing.at(-1)?.type).toBe('paragraph')
150-
expect(trailing.at(-1)?.content?.length ?? 0).toBeGreaterThan(0)
151-
const interior = parseMarkdownToDoc('a\n\n\n\nb').content ?? []
152-
expect(interior.some((n) => n.type === 'paragraph' && !n.content?.length)).toBe(true)
149+
expect(isEmptyPara(trailing.at(-1) ?? {})).toBe(false)
153150
})
154151
})
155152

apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/markdown-parse.ts

Lines changed: 27 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -47,20 +47,6 @@ const FENCE_CLOSE = /^ {0,3}(`{3,}|~{3,})[ \t]*$/
4747
const LIST_MARKER = /^[ ]{0,3}(?:[-*+]|\d+[.)])\s/
4848
const BLOCKQUOTE = /^[ ]{0,3}>/
4949

50-
/**
51-
* Blank-line spacing that `@tiptap/markdown` reconstructs as *interior* or *leading* empty paragraphs —
52-
* a run of two or more blank lines somewhere, or blank line(s) at the document's leading edge. `[^\S\n]`
53-
* matches horizontal whitespace, so a "blank" line may carry spaces/tabs. This is only ever tested
54-
* against the `\r`-normalized body ({@link parseMarkdownToDoc}), so no CRLF handling is needed here.
55-
*
56-
* A *single* trailing blank line is deliberately not matched — purely to avoid routing an otherwise-plain
57-
* file to the slower whole-document parser. Correctness does not depend on it: {@link parseMarkdownToDoc}
58-
* strips trailing empty paragraphs on *both* parse paths ({@link stripTrailingEmptyParagraphs}), so
59-
* serialize→parse stays idempotent regardless of which parser ran. (A trailing run of two or more blanks
60-
* still matches the interior alternative — harmless, since the strip cleans it either way.)
61-
*/
62-
const EMPTY_PARAGRAPH_SPACING = /\n[^\S\n]*\n[^\S\n]*\n|^[^\S\n]*\n[^\S\n]*\n/
63-
6450
/**
6551
* Split a markdown body into top-level blocks that can each be parsed independently and reassembled
6652
* without changing meaning. Blank lines separate candidate groups (fenced code blocks stay atomic),
@@ -135,21 +121,20 @@ export function splitMarkdownBlocks(body: string): string[] {
135121
* Documents whose constructs span blocks ({@link NON_CHUNKABLE}) parse whole, and any failure falls
136122
* back to a single whole-document parse, so correctness never depends on the splitter.
137123
*
138-
* Blank-line spacing ({@link EMPTY_PARAGRAPH_SPACING}) also parses whole: the chunker parses each block
139-
* stripped of the blank lines between them, so it drops the empty paragraphs `@tiptap/markdown` builds
140-
* from runs of blank lines — a saved visual blank line would silently vanish on reload. Whether a gap
141-
* yields an empty paragraph is a global, block-type-dependent decision (kept between two paragraphs,
142-
* dropped after a heading), so it can't be reconstructed block-locally; these documents parse whole for
143-
* exact fidelity. Ordinary single-blank-line separation still takes the fast chunked path.
124+
* Runs of blank lines take the fast chunked path too: the chunker parses each block stripped of the
125+
* blank lines between them, which drops the empty paragraphs `@tiptap/markdown` reconstructs from a
126+
* blank run — exactly what {@link stripEmptyParagraphs} does to the whole-parse output anyway. A blank
127+
* run between blocks is insignificant in markdown, so collapsing it is the intended normalization (see
128+
* {@link stripEmptyParagraphs}), and both parse paths converge on the same empty-paragraph-free result.
144129
*/
145130
export function parseMarkdownToDoc(body: string): JSONContent {
146131
const manager = markdownManager()
147-
// Normalize line endings up front so the routing guards see the same `\n` the chunker and parser
148-
// do — the guards' `\n`-anchored tests would otherwise miss a classic `\r`-only body (its blank
149-
// lines are `\r`), routing it to the chunker that then drops its empty paragraphs.
132+
// Normalize line endings up front so {@link NON_CHUNKABLE}'s `\n`-anchored tests see the same `\n`
133+
// the chunker and parser do — a classic `\r`-only body would otherwise slip past the reference-def /
134+
// block-HTML guard and be chunked, shattering a construct that must parse whole.
150135
const normalized = body.replace(/\r\n?/g, '\n')
151136
let doc: JSONContent
152-
if (NON_CHUNKABLE.test(normalized) || EMPTY_PARAGRAPH_SPACING.test(normalized)) {
137+
if (NON_CHUNKABLE.test(normalized)) {
153138
doc = manager.parse(normalized)
154139
} else {
155140
try {
@@ -163,7 +148,7 @@ export function parseMarkdownToDoc(body: string): JSONContent {
163148
doc = manager.parse(normalized)
164149
}
165150
}
166-
return stripTrailingEmptyParagraphs(doc)
151+
return stripEmptyParagraphs(doc)
167152
}
168153

169154
/** An empty paragraph node — the shape a blank line reconstructs to (no content, or `content: []`). */
@@ -172,19 +157,26 @@ function isEmptyParagraph(node: JSONContent): boolean {
172157
}
173158

174159
/**
175-
* Drop trailing empty paragraphs from a parsed doc. {@link postProcessSerializedMarkdown} collapses
176-
* trailing blank lines to a single newline, so a trailing empty paragraph can never round-trip — the
177-
* whole-document parser reconstructs one from a file ending in a blank line, but keeping it makes
178-
* serialize→parse non-idempotent, which flips the file read-only via the round-trip-safety probe.
179-
* Leading/interior empty paragraphs are untouched (postProcess never strips those). TipTap re-adds its
180-
* own trailing filler paragraph on `setContent`, so the editor still has a place to type.
160+
* Drop ALL top-level empty paragraphs from a parsed doc — leading, interior, and trailing. In markdown
161+
* a run of blank lines between blocks is insignificant (CommonMark collapses it), so `@tiptap/markdown`
162+
* reconstructing each blank as an empty paragraph node is not fidelity: it makes the editor render the
163+
* file differently from every standard renderer (GitHub, the download, our own static preview), and a
164+
* pathological blank run (an agent/paste artifact) explodes into thousands of empty nodes that persist
165+
* forever and reflow the doc on open. Collapsing them here keeps normal single-blank-line block spacing
166+
* while removing the spurious gaps, and stays idempotent so the round-trip-safety probe still passes: a
167+
* doc parsed this way has no empty paragraphs, so re-serializing it never re-emits an interior blank run
168+
* (the serializer is intentionally left alone — a blank line inside a fenced code block IS significant),
169+
* and a second parse is a fixed point. Only TOP-LEVEL paragraphs are touched, so blank lines that carry
170+
* meaning inside a construct (e.g. a loose list) are left to the block parser. TipTap re-adds its own
171+
* trailing filler paragraph on `setContent`, so the editor still has a place to type.
181172
*/
182-
function stripTrailingEmptyParagraphs(doc: JSONContent): JSONContent {
173+
function stripEmptyParagraphs(doc: JSONContent): JSONContent {
183174
const content = doc.content
184175
if (!content || content.length === 0) return doc
185-
let end = content.length
186-
while (end > 0 && isEmptyParagraph(content[end - 1])) end--
187-
return end === content.length ? doc : { ...doc, content: content.slice(0, end) }
176+
// The dominant (chunked) parse already emits no top-level empty paragraphs, so scan before allocating:
177+
// return the doc untouched — no array copy — unless there is actually something to strip.
178+
if (!content.some(isEmptyParagraph)) return doc
179+
return { ...doc, content: content.filter((node) => !isEmptyParagraph(node)) }
188180
}
189181

190182
/**

0 commit comments

Comments
 (0)