Skip to content

Commit 0bef1c3

Browse files
authored
fix(files): stop a cached collab snapshot resurrecting blank lines (#6293)
* fix(files): stop a cached collab snapshot resurrecting blank lines A collaborative markdown file's cold-start seed can come from a cached Yjs snapshot (workspace_file_collab_state.doc_state) rather than a fresh markdown re-parse. The snapshot is a raw CRDT binary, so it preserves top-level empty paragraphs that parseMarkdownToDoc/stripEmptyParagraphs strips from every parse target. The static placeholder always re-parses (clean); a warm seed replays the snapshot verbatim, so a stray blank line appears once the doc settles — and only intermittently, since a stale/cold cache falls through to the clean re-parse. Enforce the same no-top-level-empty-paragraph invariant on the Yjs side: - normalize.ts: stripEmptyTopLevelParagraphs(doc) shared helper. - seed.ts: repair the cached snapshot on read (self-heals legacy snapshots, preserving CRDT client ids; no data migration). - persist.ts: normalize before caching so new snapshots are clean by construction. * refactor(collab-doc): make COLLAB_DOC_FIELD a single canonical constant converter.ts had a duplicate 'default' fragment-name constant; import the now-exported one from normalize.ts so the value TipTap's Collaboration binding depends on lives in exactly one place. Fold the back-to-front loop note into the helper's TSDoc.
1 parent dcaa118 commit 0bef1c3

5 files changed

Lines changed: 199 additions & 9 deletions

File tree

apps/sim/lib/collab-doc/converter.ts

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {
1717
parseMarkdownToDoc,
1818
serializeDocToMarkdown,
1919
} from '@/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/markdown-parse'
20+
import { COLLAB_DOC_FIELD } from './normalize'
2021

2122
/**
2223
* Server-side conversion between a file's markdown and its collaborative Yjs document.
@@ -36,13 +37,6 @@ import {
3637
* 'server-only'` marker because this repo does not use that package.
3738
*/
3839

39-
/**
40-
* The Yjs `XmlFragment` name TipTap's Collaboration extension binds to. The client configures
41-
* `Collaboration.configure({ document })` with no explicit `field`, so it uses TipTap's default,
42-
* `'default'`. The server MUST target the same fragment or the client would sync an empty document.
43-
*/
44-
const COLLAB_DOC_FIELD = 'default'
45-
4640
let cachedSchema: Schema | null = null
4741

4842
/** The shared ProseMirror schema, built headlessly from the exact client extension set. */
Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { describe, expect, it } from 'vitest'
5+
import * as Y from 'yjs'
6+
import { COLLAB_DOC_FIELD, stripEmptyTopLevelParagraphs } from './normalize'
7+
8+
/** Build a top-level element with the given tag and optional text content. */
9+
function element(tag: string, text?: string): Y.XmlElement {
10+
const el = new Y.XmlElement(tag)
11+
if (text !== undefined) el.insert(0, [new Y.XmlText(text)])
12+
return el
13+
}
14+
15+
/** Recursively concatenate the visible text of a Yjs XML node. */
16+
function textOf(node: Y.XmlElement | Y.XmlText | Y.XmlHook): string {
17+
if (node instanceof Y.XmlText) return node.toString()
18+
if (node instanceof Y.XmlElement) {
19+
let text = ''
20+
for (let i = 0; i < node.length; i++) text += textOf(node.get(i))
21+
return text
22+
}
23+
return ''
24+
}
25+
26+
/** The ordered list of top-level `[tag, text]` pairs currently in a doc's body fragment. */
27+
function structure(doc: Y.Doc): Array<[string, string]> {
28+
const fragment = doc.getXmlFragment(COLLAB_DOC_FIELD)
29+
const out: Array<[string, string]> = []
30+
for (let i = 0; i < fragment.length; i++) {
31+
const node = fragment.get(i)
32+
out.push([node instanceof Y.XmlElement ? node.nodeName! : 'text', textOf(node)])
33+
}
34+
return out
35+
}
36+
37+
describe('stripEmptyTopLevelParagraphs', () => {
38+
it('removes interior empty paragraphs while preserving content and order (production repro)', () => {
39+
// Mirrors the persisted snapshot for random_data.md: a description paragraph, TWO consecutive empty
40+
// paragraphs (the reported "two spaces"), then a bullet list, then another interior empty paragraph.
41+
const doc = new Y.Doc()
42+
const fragment = doc.getXmlFragment(COLLAB_DOC_FIELD)
43+
fragment.insert(0, [
44+
element('paragraph', 'A small collection of sample data.'),
45+
element('paragraph'),
46+
element('paragraph'),
47+
element('bulletList', 'list'),
48+
element('paragraph'),
49+
element('paragraph', 'trailing content'),
50+
])
51+
52+
expect(stripEmptyTopLevelParagraphs(doc)).toBe(true)
53+
expect(structure(doc)).toEqual([
54+
['paragraph', 'A small collection of sample data.'],
55+
['bulletList', 'list'],
56+
['paragraph', 'trailing content'],
57+
])
58+
doc.destroy()
59+
})
60+
61+
it('is idempotent — a second pass finds nothing to remove', () => {
62+
const doc = new Y.Doc()
63+
doc
64+
.getXmlFragment(COLLAB_DOC_FIELD)
65+
.insert(0, [element('paragraph'), element('paragraph', 'body')])
66+
67+
expect(stripEmptyTopLevelParagraphs(doc)).toBe(true)
68+
expect(stripEmptyTopLevelParagraphs(doc)).toBe(false)
69+
expect(structure(doc)).toEqual([['paragraph', 'body']])
70+
doc.destroy()
71+
})
72+
73+
it('returns false and mutates nothing when there are no top-level empty paragraphs', () => {
74+
const doc = new Y.Doc()
75+
doc
76+
.getXmlFragment(COLLAB_DOC_FIELD)
77+
.insert(0, [element('heading', 'Title'), element('paragraph', 'body')])
78+
79+
expect(stripEmptyTopLevelParagraphs(doc)).toBe(false)
80+
expect(structure(doc)).toEqual([
81+
['heading', 'Title'],
82+
['paragraph', 'body'],
83+
])
84+
doc.destroy()
85+
})
86+
87+
it('leaves an empty paragraph nested inside another block untouched (only top-level is stripped)', () => {
88+
const doc = new Y.Doc()
89+
const listItem = new Y.XmlElement('listItem')
90+
listItem.insert(0, [new Y.XmlElement('paragraph')]) // an empty paragraph BELOW the fragment root
91+
const list = new Y.XmlElement('bulletList')
92+
list.insert(0, [listItem])
93+
doc.getXmlFragment(COLLAB_DOC_FIELD).insert(0, [list])
94+
95+
expect(stripEmptyTopLevelParagraphs(doc)).toBe(false)
96+
const nestedList = doc.getXmlFragment(COLLAB_DOC_FIELD).get(0) as Y.XmlElement
97+
const nestedItem = nestedList.get(0) as Y.XmlElement
98+
expect(nestedItem.get(0)).toBeInstanceOf(Y.XmlElement)
99+
expect((nestedItem.get(0) as Y.XmlElement).nodeName).toBe('paragraph')
100+
doc.destroy()
101+
})
102+
103+
it('survives an encode/decode round-trip preserving CRDT ids and the config map (seed-repair path)', () => {
104+
const original = new Y.Doc()
105+
original
106+
.getXmlFragment(COLLAB_DOC_FIELD)
107+
.insert(0, [element('paragraph', 'kept'), element('paragraph')])
108+
original.getMap('config').set('initialContentLoaded', true)
109+
original.getMap('config').set('frontmatter', 'title: x')
110+
const before = Y.encodeStateAsUpdate(original)
111+
original.destroy()
112+
113+
// Repair exactly as normalizeSeedUpdate does: apply → strip → re-encode.
114+
const repair = new Y.Doc()
115+
Y.applyUpdate(repair, before)
116+
expect(stripEmptyTopLevelParagraphs(repair)).toBe(true)
117+
const after = Y.encodeStateAsUpdate(repair)
118+
repair.destroy()
119+
120+
const seeded = new Y.Doc()
121+
Y.applyUpdate(seeded, after)
122+
expect(structure(seeded)).toEqual([['paragraph', 'kept']])
123+
expect(seeded.getMap('config').get('initialContentLoaded')).toBe(true)
124+
expect(seeded.getMap('config').get('frontmatter')).toBe('title: x')
125+
seeded.destroy()
126+
})
127+
})
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
import * as Y from 'yjs'
2+
3+
/**
4+
* The Yjs `XmlFragment` name TipTap's Collaboration extension binds to (its default `field`). The
5+
* client configures `Collaboration.configure({ document })` with no explicit `field`, so it uses
6+
* TipTap's default, `'default'`. Server-side conversion, seeding, and persistence MUST target the same
7+
* fragment or the client would sync an empty document — so this is the single canonical source consumed
8+
* by both bundles (it imports only `yjs`, making it safe from client and server alike).
9+
*/
10+
export const COLLAB_DOC_FIELD = 'default'
11+
12+
/**
13+
* Remove every top-level empty paragraph (a `paragraph` element with no children) from a collaborative
14+
* document's body fragment, returning whether it deleted any.
15+
*
16+
* The markdown parse pipeline strips these from EVERY parse target (see `stripEmptyParagraphs` in
17+
* `markdown-parse.ts`): in markdown a run of blank lines between blocks is insignificant, so the static
18+
* placeholder, the download, and every standard renderer show no interior blank. A cached Yjs snapshot,
19+
* however, is a raw CRDT binary that bypasses that parse — so it can preserve an empty-paragraph node the
20+
* re-parse would have dropped. When a warm room seeds from such a snapshot, the empty paragraph surfaces
21+
* as a stray blank line appearing once the doc settles, diverging from the placeholder that was shown
22+
* first. Enforcing the same no-top-level-empty-paragraph invariant on the Yjs side keeps the live
23+
* collaborative doc rendering identically to the markdown re-parse.
24+
*
25+
* Idempotent, and only TOP-LEVEL paragraphs are touched — blank lines that carry meaning inside a
26+
* construct (e.g. a loose list) live below the fragment root and are left alone. Runs its own Yjs
27+
* transaction so the deletions commit atomically, iterating the fragment back-to-front so a deletion
28+
* never shifts a not-yet-checked index.
29+
*/
30+
export function stripEmptyTopLevelParagraphs(doc: Y.Doc): boolean {
31+
const fragment = doc.getXmlFragment(COLLAB_DOC_FIELD)
32+
let removed = false
33+
doc.transact(() => {
34+
for (let i = fragment.length - 1; i >= 0; i--) {
35+
const node = fragment.get(i)
36+
if (node instanceof Y.XmlElement && node.nodeName === 'paragraph' && node.length === 0) {
37+
fragment.delete(i, 1)
38+
removed = true
39+
}
40+
}
41+
})
42+
return removed
43+
}

apps/sim/lib/collab-doc/persist.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {
88
} from '@/lib/uploads/contexts/workspace'
99
import { hashMarkdown, saveCollabDocState } from './collab-state'
1010
import { yDocToFileMarkdown } from './converter'
11+
import { stripEmptyTopLevelParagraphs } from './normalize'
1112

1213
const logger = createLogger('FileDocPersist')
1314

@@ -68,8 +69,14 @@ export async function persistFileDoc(
6869

6970
const ydoc = new Y.Doc()
7071
let markdownBuffer: Buffer
72+
// The Yjs snapshot cached below (`saveCollabDocState`) seeds a later cold room open directly, so it
73+
// must never carry structure the markdown re-parse would strip — a top-level empty paragraph left in
74+
// the snapshot resurfaces as a stray blank line when that warm doc settles, diverging from the static
75+
// placeholder. Normalize it out here so the cached binary matches the durable markdown by construction.
76+
let cachedDocState = docState
7177
try {
7278
Y.applyUpdate(ydoc, docState)
79+
if (stripEmptyTopLevelParagraphs(ydoc)) cachedDocState = Y.encodeStateAsUpdate(ydoc)
7380
markdownBuffer = Buffer.from(yDocToFileMarkdown(ydoc), 'utf-8')
7481
} finally {
7582
ydoc.destroy()
@@ -93,7 +100,7 @@ export async function persistFileDoc(
93100
// Cache the Yjs binary (tagged with the exact markdown just written) so a later cold room open loads
94101
// it directly instead of re-converting. Best-effort — the markdown is the durable source of truth.
95102
try {
96-
await saveCollabDocState(fileId, docState, hashMarkdown(markdownBuffer))
103+
await saveCollabDocState(fileId, cachedDocState, hashMarkdown(markdownBuffer))
97104
} catch (error) {
98105
logger.warn(`Failed to cache collab doc state for file ${fileId}`, {
99106
error: getErrorMessage(error),

apps/sim/lib/collab-doc/seed.ts

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { fetchWorkspaceFileBuffer, getWorkspaceFile } from '@/lib/uploads/contex
66
import { splitFrontmatter } from '@/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/markdown-fidelity'
77
import { hashMarkdown, loadFreshCollabDocState } from './collab-state'
88
import { markdownToYDoc } from './converter'
9+
import { stripEmptyTopLevelParagraphs } from './normalize'
910

1011
const logger = createLogger('FileDocSeed')
1112

@@ -26,6 +27,24 @@ export interface FileDocSeed {
2627
version: number
2728
}
2829

30+
/**
31+
* Repair a cached Yjs snapshot before it seeds a room: strip any top-level empty paragraphs the markdown
32+
* re-parse would drop (see {@link stripEmptyTopLevelParagraphs}), so a warm seed renders identically to
33+
* the static placeholder and never surfaces a stray blank line once the doc settles. Returns the original
34+
* bytes untouched when the snapshot is already clean (the common case) — no re-encode cost — and a fresh
35+
* encode (preserving the CRDT's client ids, only adding tombstones for the removed empties) when it
36+
* repaired a legacy snapshot baked before this normalization existed.
37+
*/
38+
function normalizeSeedUpdate(cached: Uint8Array): Uint8Array {
39+
const doc = new Y.Doc()
40+
try {
41+
Y.applyUpdate(doc, cached)
42+
return stripEmptyTopLevelParagraphs(doc) ? Y.encodeStateAsUpdate(doc) : cached
43+
} finally {
44+
doc.destroy()
45+
}
46+
}
47+
2948
/**
3049
* Build the server-side seed for a file's collaborative document: load the file's current markdown
3150
* and convert it — through the exact client engine (see {@link markdownToYDoc}) — into a Yjs update.
@@ -63,7 +82,7 @@ export async function buildFileDocSeed(
6382
// block the cold open — symmetric with persist's best-effort cache write.
6483
try {
6584
const cached = await loadFreshCollabDocState(fileId, hashMarkdown(buffer))
66-
if (cached) return { update: cached, version }
85+
if (cached) return { update: normalizeSeedUpdate(cached), version }
6786
} catch (error) {
6887
logger.warn(`Failed to read cached collab doc state for file ${fileId}`, {
6988
error: getErrorMessage(error),

0 commit comments

Comments
 (0)