feat: render suggestions as decorations with step-based delta sync - #264
Draft
nperez0111 wants to merge 8 commits into
Draft
feat: render suggestions as decorations with step-based delta sync#264nperez0111 wants to merge 8 commits into
nperez0111 wants to merge 8 commits into
Conversation
Move suggestion attribution out of the ProseMirror document and into a decoration overlay instead. The PM doc now always mirrors clean Y content (no y-attributed-* marks, no inline deleted text); attribution is read separately via ytype.toDeltaDeep(am) and rendered as decorations. This removes the attributed-variant node machinery, attribution mappers, and delta-stripping from sync-plugin/sync-utils/commands. Add modules: y-attribution-to-diffset.js (ydeltaToDiffSet: attributed delta -> DiffSet), diff-decorations.js (buildDiffDecorationSet / suggestionDiffPlugin / renderDeletedContent), and suggestion-decoration-plugin.js (ySuggestionDecorationPlugin). Fix block-split rendering: a CRDT records a split as delete-tail + insert-new-block, so toDeltaDeep reported the moved text twice, drawing a phantom strikethrough plus duplicated text. ydeltaToDiffSet now suppresses the redundant delete half of a split. Reorganize the suggestion tests into three layers and remove cross-layer duplication: y-attribution-to-diffset.test.js (unit: the transform), suggestion-decoration-plugin.test.js (integration: the live plugin), suggestions.test.js (e2e: multi-view sync + convergence). Update demos to decoration mode; remove unused imports; fix tsc (TS2345/TS2589) and standard lint errors.
The sync-plugin computes diffs in clean coordinates (where suggested deletions are invisible), but applyDelta with a DiffAttributionManager navigates Y items using contentLength which counts suggested deletions at their full length. This caused retain counts to land at wrong positions when prior suggested deletions existed in the document. Fix: wrap the AM in a Proxy that overrides contentLength and readContent to use clean counting (deleted items = 0), passing everything else through to the real AM. The proxy is a distinct identity from noAttributionsManager, keeping attribution-aware code paths active in applyDelta. Attribution recording is unaffected since the DiffAttributionManager's event listener on the Y.Doc fires based on the transaction, not the AM passed to applyDelta. Also fixes deletion widget rendering: - side: -1 → side: 1 so cursor appears before the ghost (natural for backspace-then-type flow) - Widget key includes content size to force re-render when consecutive deletions grow the ghost content
- Remove stale AttributionMapper and AttributedNodesPredicate global typedefs (old mark-based approach, no longer referenced) - Remove vestigial `change` field from sync plugin state updates (always set to null, never read) - Remove dead mapAttributionToMark option from test helper
Global type declarations leak into consumers' type namespaces. Inline the remaining references (ProsemirrorDelta, SyncPluginState, Transaction) as local JSDoc typedefs and import() annotations.
Replace the full-document diff in the sync plugin's write path with step-based delta generation. PM transaction steps are converted directly to Y-compatible deltas and applied, skipping the O(doc) walk and diff for common single-keystroke edits. - Accumulate pending transactions in plugin state between view.update() calls to handle appendTransaction batches - Single-step transactions use stepToDelta (O(change)) - Multi-step transactions use docDiffToDelta (O(doc) but correct) - Full-doc diff retained as fallback for attribution-manager paths (step-based deltas create different Y item structures that propagate incorrectly through the AM chain to remote peers) - Fix pmToDeltaPath bug: use child index (not byte offset) at block boundaries inside non-inline containers like blockquotes - Fix cross-block AddMarkStep/RemoveMarkStep: use block-range diff when mark range spans multiple textblocks - Handle custom/unknown step types via getMap() fallback - Enable and fix tests/tr.test.js (was disabled): fix import path, export testBuilders from complexSchema, rename tes* → test* - Add new test coverage: mark steps, attr steps, block wrapping, cross-block operations, appendTransaction stress tests, deterministic blockquote sync divergence reproduction
- Remove unused `lib0/error` import from sync-utils - Remove unused `callCount` variable in delta test - Remove unused `findDivergences` import in simulation test - Add JSDoc param types for filter callback, StepMap.forEach params - Add @ts-ignore for excessively deep type instantiation in matcher - Split combined AddMarkStep/RemoveMarkStep handler to avoid TS narrowing to `never` on the union - Add 'wrapInBlockquote' and 'deleteBlock' to TracedOp type union - Add @PARAM JSDoc for new test functions
…eted blocks - Add `decorationMode` option to `syncPlugin()`. When true, the PM doc contains clean content (no attribution marks, no suggestion-deleted text). Attribution is rendered as decorations by the separate `ySuggestionDecorationPlugin`. - Add `stripDeletesFromAttributedDelta` to filter suggestion-deleted content from the attributed delta before syncing to PM. Without this, suggestion-deleted items (which are not Y-deleted, only AM-marked) would reappear in the PM doc after every reconcile. - Add `createNavAM` proxy for clean-coordinate navigation when applying diffs through the AM in decoration mode. - Upgrade block-delete ghost rendering in `diff-decorations.js`: when the editor has registered node views for the deleted content, render via a live ghost `EditorView` (with `spec.destroy` cleanup) instead of `DOMSerializer`. This preserves block structure (e.g. checkbox items) that `toDOM` can express. Falls back to `DOMSerializer` when no node views are registered. - Update `configureYProsemirror` in `commands.js` to hydrate from clean content in decoration mode. - Add CSS for ghost content strikethrough inheritance in the BlockNote demo. - Enable `decorationMode: true` in the BlockNote demo's sync plugin. sync-utils.js is unchanged from master (mark-based code remains as backward-compatible exports). The mark-based rendering path in sync-plugin.js is preserved as the default.
✅ Deploy Preview for y-prosemirror-demo canceled.
|
nperez0111
marked this pull request as draft
June 8, 2026 11:13
Member
|
In terms of changed lines of code this comes close to my PR with option A. But the changes really are not straight-forward. bi-directional transformers (from one presentation to another) should not be underestimated - this is super hard to get right. From a performance perspective: This PR implements a complete transformation pipeline of the complete document whenever the document is changed. |
There was a problem hiding this comment.
Pull request overview
This PR migrates suggestion/attribution rendering from ProseMirror marks to a decoration overlay, keeping the ProseMirror document model “clean” while reading attribution from toDeltaDeep(am) and rendering insert/delete/update diffs as decorations. It also introduces step-based delta sync (stepToDelta) to avoid full-document diffs for single-step edits, and reorganizes/expands tests to cover the new pipeline (diff extraction + decoration rendering + nested structures + fuzz).
Changes:
- Replace mark-based suggestion attribution with decoration-based rendering (
ydeltaToDiffSet→buildDiffDecorationSet→ySuggestionDecorationPlugin). - Add step-based PM→Y delta generation for O(change) sync in common cases; keep diff-based fallback.
- Update demos and test suite to use the decoration plugin and validate nested-structure + convergence behavior.
Reviewed changes
Copilot reviewed 34 out of 39 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| yhub-tiptap-demo/src/main.js | Remove attribution marks integration; enable suggestion decoration extension. |
| yhub-tiptap-demo/src/extensions.js | Wrap ySuggestionDecorationPlugin() as a Tiptap extension; simplify sync extension. |
| yhub-tiptap-demo/package-lock.json | Lockfile normalization (removes libc fields for optional deps). |
| yhub-demo/schema.js | Remove attribution mark support from the demo schema. |
| yhub-demo/package-lock.json | Bump @y/prosemirror and @y/y versions in the lockfile. |
| yhub-demo/demo.js | Switch demo to ySuggestionDecorationPlugin() and clean-doc sync. |
| yhub-blocknote-demo/src/yhub.js | Remove mark-mapper/debug plumbing; use exported accept/reject commands. |
| yhub-blocknote-demo/src/Editor.jsx | Add decoration plugin extension alongside sync + cursor. |
| yhub-blocknote-demo/package-lock.json | Bump @y/prosemirror; react/react-dom patch bump. |
| yhub-blocknote-demo/index.html | Update CSS to style suggestion decorations via data-diff-type/--author-color. |
| tsconfig.json | Stop including deleted global.d.ts. |
| tests/y-attribution-to-diffset.test.js | New unit tests for attributed-delta → DiffSet transform + fuzz invariants. |
| tests/tr.test.js | Expand delta sync coverage incl. stepToDelta roundtrips across more step types. |
| tests/suggestion-simulation.test.js | Add stress tests for position mapping + deterministic divergence regression. |
| tests/suggestion-decoration-plugin.test.js | New integration tests for syncPlugin + decoration plugin behavior. |
| tests/nested-node-suggestions.test.js | New e2e tests for nested block structures + suggestion accept flow. |
| tests/index.node.js | Register new test modules; remove attributed-nodes suite; re-enable tr tests. |
| tests/index.js | Register new test modules for browser test entrypoint. |
| tests/delta.test.js | Add appendTransaction sync tests to validate chained/derived transactions. |
| tests/complexSchema.js | Remove attribution marks; re-enable prosemirror-test-builder builders export. |
| tests/commands.test.js | Update assertions to validate decorations instead of attribution marks. |
| tests/cohort.js | Always include decoration plugin in PM views; add traced ops for wrap/deleteBlock. |
| tests/attributed-nodes.test.js | Remove old attributed-node-variant based tests (feature removed). |
| src/y-attribution-to-diffset.js | New module: attributed delta → DiffSet mapping in clean-doc coordinates. |
| src/sync-utils.js | Remove mark-based attribution utilities; add broader stepToDelta support + path fixes. |
| src/sync-plugin.js | Clean-doc sync; add step-based PM→Y application path + AM navigation proxy. |
| src/suggestion-decoration-plugin.js | New plugin: compute and render attribution-based decorations. |
| src/keys.js | Add ySuggestionDecorationPluginKey and suggestionDiffPluginKey. |
| src/index.js | Export new diff/decorations APIs and ySuggestionDecorationPlugin. |
| src/diff-decorations.js | New module: DiffSet → DecorationSet + customizable mapping hooks. |
| src/commands.js | Update configure/commands to operate on clean deltas (no attribution marks). |
| global.d.ts | Remove global ambient types now that JSDoc imports are used. |
| demo/schema.js | Remove attribution marks from the main demo schema. |
| demo/prosemirror.js | Wire decoration plugin; update accept/reject UI to act on selected diff decoration. |
| demo/prosemirror.html | Update CSS from mark styling to decoration styling. |
| CAVEATS.md | Update caveats to reflect decoration-based attribution approach. |
| ATTRIBUTION.md | Rewrite attribution docs for decoration pipeline + new APIs. |
| ARCHITECTURE.md | New architecture doc describing the decoration-based design and tradeoffs. |
Files not reviewed (3)
- yhub-blocknote-demo/package-lock.json: Language not supported
- yhub-demo/package-lock.json: Language not supported
- yhub-tiptap-demo/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+56
to
+69
| const ySyncMeta = tr.getMeta('y-sync-transaction') | ||
| const configMeta = tr.getMeta(ySyncPluginKey) | ||
| const metaOverride = configMeta || ySyncMeta | ||
| if (metaOverride) { | ||
| const baseSync = ySyncPluginKey.getState(oldState) || ySyncPluginKey.getState(newState) | ||
| const ystate = Object.assign({}, baseSync, metaOverride) | ||
| if (ystate?.attributionManager && ystate.attributionManager !== Y.noAttributionsManager) { | ||
| return computeDecorations( | ||
| newState.doc, newState.schema, ystate.ytype, ystate.attributionManager, opts | ||
| ) | ||
| } | ||
| } | ||
| if (tr.docChanged) return prev.map(tr.mapping, tr.doc) | ||
| return prev |
Comment on lines
61
to
66
| apply: (tr, prevPluginState) => { | ||
| const stateUpdate = $maybeSyncPluginStateUpdate.expect(tr.getMeta(ySyncPluginKey) || null) | ||
| const prev = /** @type {any} */ (prevPluginState) | ||
| const isSyncTr = tr.getMeta('y-sync-transaction') != null || tr.getMeta(ySyncPluginKey) != null | ||
| const pendingTrs = isSyncTr ? [] : [...(prev.pendingTrs || []), tr] | ||
| if (!stateUpdate) { |
Member
There was a problem hiding this comment.
@copilot how does the yjs to prosemirror transformation work? How are diffs (including decorations) computed?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Replaces the mark-based suggestion attribution approach with a decoration overlay. The ProseMirror document now always mirrors clean Yjs content; suggestion attribution is read via
toDeltaDeepand rendered as ProseMirror decorations (inline highlights for insertions, widgets for deletions).Adds step-based delta sync (
stepToDelta) so single-keystroke edits skip the O(doc) diff and apply in O(change) time. Multi-step and attribution-manager paths fall back to full-document diff.New modules:
y-attribution-to-diffset.js,diff-decorations.js,suggestion-decoration-plugin.js. Tests reorganized into unit/integration/e2e layers with new coverage for nested nodes, step conversion, and decoration rendering.Test plan
npm testpassesnpm run lintpassesdemo/prosemirror.html) renders suggestions as decorations