Skip to content

Commit 370c8a9

Browse files
YousefEDclaude
andcommitted
Fix reconcile loop when attribution marks are non-self-excluding
When an integrator declares the fixed `y-attributed-*` marks *non-self-excluding* (e.g. BlockNote, so user content marks may overlap them), `nodeToDelta` keyed them with the overlapping-mark `--<hash>` suffix while the attribution mapper and the PM->Y strip key them bare. The PM<->Y write diff and the Y->PM reconcile then disagreed on the format key and flapped between `y-attributed-delete` and `y-attributed-delete--<hash>` forever - a paragraph->heading suggestion froze the browser. Key the three fixed attribution mark names bare regardless of self-exclusion: the attribution manager already aggregates every author of a span into a single mark (its `userIds` is a list), so an attribution mark never needs the overlapping-mark hash to stay distinct. Adds a block-boundary regression test that reconcile-loops (hits a transaction guard) without the fix and converges with it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 66e4a86 commit 370c8a9

2 files changed

Lines changed: 87 additions & 1 deletion

File tree

src/sync-utils.js

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -192,14 +192,28 @@ const hashedMarkNameRegex = /(.*)(--[a-zA-Z0-9+/=]{8})$/
192192
*/
193193
export const yattr2markname = attrName => hashedMarkNameRegex.exec(attrName)?.[1] ?? attrName
194194

195+
/**
196+
* The three fixed attribution mark names (see {@link defaultMapAttributionToMark}).
197+
* They are always keyed by their bare name in the Y representation - never given
198+
* the overlapping-mark `--<hash>` suffix below - because the attribution mapper
199+
* and the PM->Y strip both key them bare, and the attribution manager already
200+
* aggregates every author of a span into a single mark (its `userIds` is a list),
201+
* so there is never more than one attribution mark of a kind on a span to
202+
* disambiguate. If an integrator declares these marks *non-self-excluding* (e.g.
203+
* so user content marks may overlap them, as BlockNote does), hashing them would
204+
* make {@link nodeToDelta}'s key disagree with the mapper's, and the PM<->Y
205+
* reconcile would flap between the two forms forever.
206+
*/
207+
const attributionMarkNameSet = new Set(['y-attributed-insert', 'y-attributed-delete', 'y-attributed-format'])
208+
195209
/**
196210
* Inverse of {@link yattr2markname}: the delta format key for a PM mark.
197211
*
198212
* @param {import('prosemirror-model').Mark} mark
199213
* @return {string}
200214
*/
201215
const markToYattrName = mark =>
202-
mark.type.excludes(mark.type)
216+
attributionMarkNameSet.has(mark.type.name) || mark.type.excludes(mark.type)
203217
? mark.type.name
204218
: `${mark.type.name}--${hashOfJSON(mark.toJSON())}`
205219

tests/block-boundary.test.js

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -331,3 +331,75 @@ export const testStrictContainerChildFlipReject = _tc => {
331331
assertDocJSON(viewer.state.doc, expected, 'viewer: original paragraph after reject')
332332
assertDocJSON(editor.state.doc, expected, 'editor: original paragraph after reject')
333333
}
334+
335+
/**
336+
* Regression: a boundary-raised type change must converge (no PM<->Y reconcile
337+
* loop) when the integrator declares the `y-attributed-*` attribution marks
338+
* *non-self-excluding* - as BlockNote does, so user content marks can overlap
339+
* them.
340+
*
341+
* The bug: `nodeToDelta` keys a non-self-excluding mark with the overlapping-mark
342+
* `--<hash>` suffix (`y-attributed-delete--<hash>`), but the attribution mapper
343+
* keys it bare (`y-attributed-delete`). The PM->Y diff and the Y->PM reconcile
344+
* then disagree on the key and flap between the two forms forever (the browser
345+
* demo froze on a paragraph->heading suggestion). The fix keys the three fixed
346+
* attribution mark names bare regardless of self-exclusion. This test loops
347+
* (hangs / hits the guard) without that fix.
348+
*
349+
* @param {t.TestCase} _tc
350+
*/
351+
export const testBoundaryRaiseConvergesWithOverlappingAttributionMarks = _tc => {
352+
// `excludes: ''` makes the mark non-self-excluding (multiple may coexist on a
353+
// span) - the configuration that triggered the hash/bare key reconcile loop.
354+
const overlappingMark = () => ({
355+
excludes: '',
356+
attrs: { userIds: { default: [] }, timestamp: { default: null } },
357+
toDOM () { return /** @type {import('prosemirror-model').DOMOutputSpec} */ (['span', 0]) }
358+
})
359+
const amark = 'y-attributed-insert y-attributed-delete y-attributed-format'
360+
const overlapSchema = new Schema({
361+
nodes: {
362+
doc: { content: 'blockGroup' },
363+
blockGroup: { content: 'blockContainer+', marks: amark, toDOM () { return ['div', 0] } },
364+
blockContainer: { content: 'blockContent blockGroup?', attrs: { id: { default: null } }, marks: amark, toDOM () { return ['div', 0] } },
365+
paragraph: { content: 'inline*', group: 'blockContent', marks: amark, toDOM () { return ['p', 0] } },
366+
heading: { content: 'inline*', group: 'blockContent', attrs: { level: { default: 1 } }, marks: amark, toDOM (n) { return ['h' + n.attrs.level, 0] } },
367+
text: { group: 'inline' }
368+
},
369+
marks: {
370+
'y-attributed-insert': overlappingMark(),
371+
'y-attributed-delete': overlappingMark(),
372+
'y-attributed-format': overlappingMark()
373+
}
374+
})
375+
const doc = new Y.Doc({ gc: false, guid: 'base' })
376+
const suggestionModeDoc = new Y.Doc({ isSuggestionDoc: true, gc: false, guid: 'se' })
377+
const am = Y.createAttributionManagerFromDiff(doc, suggestionModeDoc, { attrs: new Y.Attributions() })
378+
am.suggestionMode = true
379+
doc.get('prosemirror').applyDelta(delta.create().insert([
380+
delta.create('blockGroup', {}, [delta.create('blockContainer', { id: 'A' }, [delta.create('paragraph', {}, 'child')])])
381+
]).done())
382+
383+
let txCount = 0
384+
const view = new EditorView(
385+
{ mount: document.createElement('div') },
386+
{
387+
state: EditorState.create({ schema: overlapSchema, plugins: [YPM.syncPlugin({ matchNodes })] }),
388+
// Guard so a reconcile-loop regression fails cleanly instead of hanging CI.
389+
dispatchTransaction (tr) {
390+
if (++txCount > 100) throw new Error('reconcile loop: >100 transactions for a single flip')
391+
view.updateState(view.state.apply(tr))
392+
}
393+
}
394+
)
395+
YPM.configureYProsemirror({ ytype: suggestionModeDoc.get('prosemirror'), attributionManager: am })(view.state, view.dispatch)
396+
397+
txCount = 0
398+
view.dispatch(view.state.tr.setNodeMarkup(2, overlapSchema.nodes.heading, { level: 2 }))
399+
400+
t.assert(txCount <= 100, 'converged without a reconcile loop')
401+
const bg = view.state.doc.child(0)
402+
t.assert(bg.childCount === 2, 'boundary raised to two sibling blockContainers')
403+
t.assert(bg.child(0).child(0).type.name === 'paragraph', 'first block is the deleted paragraph')
404+
t.assert(bg.child(1).child(0).type.name === 'heading', 'second block is the inserted heading')
405+
}

0 commit comments

Comments
 (0)