fix(ai): operations on collaborative documents - #2952
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📝 WalkthroughWalkthroughThe PR rebinds ProseMirror Yjs synchronization state during document fork and merge operations. Position mapping now uses the bound Y.Doc. New tests cover tracked positions and AI updates in local and collaborative editors. ChangesYjs position synchronization
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Editor
participant ForkYDoc
participant ySync
participant RelativePositionMapping
Editor->>ForkYDoc: Fork collaborative document
ForkYDoc->>ySync: Bind forked fragment and Y.Doc
RelativePositionMapping->>RelativePositionMapping: Resolve position from bound Y.Doc
Editor->>ForkYDoc: Merge forked changes
ForkYDoc->>ySync: Restore original fragment and Y.Doc
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/xl-ai/src/api/formats/html-blocks/collabUpdate.test.ts`:
- Around line 55-59: Update the returned fork function around the yForkDoc
extension lookup to fail explicitly when ForkYDocExtension is unavailable
instead of returning undefined through optional chaining. Preserve the existing
extension.fork() behavior when the extension is present, and ensure
collaborative tests cannot silently skip the forked Y.Doc path.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: fe468dca-4bf7-4407-839d-708a56da3637
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (5)
packages/core/src/yjs/extensions/ForkYDoc.test.tspackages/core/src/yjs/extensions/ForkYDoc.tspackages/core/src/yjs/extensions/RelativePositionMapping.tspackages/xl-ai/package.jsonpackages/xl-ai/src/api/formats/html-blocks/collabUpdate.test.ts
Comment on lines
+55
to
+59
| return { | ||
| editor, | ||
| fork: () => | ||
| editor.getExtension<typeof ForkYDocExtension>("yForkDoc")?.fork(), | ||
| }; |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Fail when the fork extension is unavailable.
Optional chaining makes fork() a no-op when yForkDoc is missing. The collaborative tests can then pass without testing the forked Y.Doc path. Throw if the extension is unavailable.
Proposed fix
return {
editor,
- fork: () =>
- editor.getExtension<typeof ForkYDocExtension>("yForkDoc")?.fork(),
+ fork: () => {
+ const forkYDoc = editor.getExtension<typeof ForkYDocExtension>("yForkDoc");
+ if (!forkYDoc) {
+ throw new Error("yForkDoc extension is not available");
+ }
+ forkYDoc.fork();
+ },
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| return { | |
| editor, | |
| fork: () => | |
| editor.getExtension<typeof ForkYDocExtension>("yForkDoc")?.fork(), | |
| }; | |
| return { | |
| editor, | |
| fork: () => { | |
| const forkYDoc = editor.getExtension<typeof ForkYDocExtension>("yForkDoc"); | |
| if (!forkYDoc) { | |
| throw new Error("yForkDoc extension is not available"); | |
| } | |
| forkYDoc.fork(); | |
| }, | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/xl-ai/src/api/formats/html-blocks/collabUpdate.test.ts` around lines
55 - 59, Update the returned fork function around the yForkDoc extension lookup
to fail explicitly when ForkYDocExtension is unavailable instead of returning
undefined through optional chaining. Preserve the existing extension.fork()
behavior when the extension is present, and ensure collaborative tests cannot
silently skip the forked Y.Doc path.
@blocknote/ariakit
@blocknote/code-block
@blocknote/core
@blocknote/mantine
@blocknote/react
@blocknote/server-util
@blocknote/shadcn
@blocknote/xl-ai
@blocknote/xl-docx-exporter
@blocknote/xl-email-exporter
@blocknote/xl-multi-column
@blocknote/xl-odt-exporter
@blocknote/xl-pdf-exporter
commit: |
4 tasks
nperez0111
changed the title
`AIExtension.invokeAI` forks the Y.Doc before a request starts, so AI changes don't reach other collaborators until they're accepted. Forking swaps the `ySync` plugin, which reconfigures the ProseMirror state — and ProseMirror carries over the state of plugins that share a key rather than re-initializing them. The new plugin's `binding` still ends up on the forked fragment (it's set from the plugin's view, via a transaction), but `type` and `doc` keep pointing at the fragment the editor was bound to before. That left the plugin state split across two Y.Docs. `RelativePositionMappingExtension` resolved tracked positions with the stale `doc` and the new `binding.type`, so the decoded type was never part of the bound fragment and every lookup returned `null`. The `update` tool tracks the selection across the LLM round-trip, so any AI request on a selection in a collaborative document failed with "Position not found, cannot track positions". Re-point the plugin state's `type`/`doc` at the newly bound fragment on both fork and merge, and resolve relative positions against the doc that owns the bound type.
nperez0111
force-pushed
the
fix/ai-collab-position-tracking
branch
from
August 10, 2026 09:59
edbeefb to
e867078
Compare
|
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
1 participant
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
Fixes #2946
Running any AI request on a selection in a Yjs-collaborative document fails with
Position not found, cannot track positions.Rationale
AIExtension.invokeAIforks the Y.Doc before a request starts, so AI changes don't reach other collaborators until they're accepted. Forking swaps theySyncplugin, which reconfigures the ProseMirror state — and ProseMirror carries over the state of plugins that share a key instead of re-initializing them.The new plugin's
bindingstill ends up on the forked fragment (it's set from the plugin's view, via a transaction), buttypeanddockeep pointing at the fragment the editor was bound to before. The plugin state ends up split across two Y.Docs:RelativePositionMappingExtension.mapPositionthen resolved with the staledocand the newbinding.type. y-prosemirror decodes the position againstdoc, notices the resulting type isn't part ofdocumentType, and returnsnull— every single lookup, not just garbage-collected ones.createUpdateBlockToolcallstrackPositionforupdateSelection.from/toto follow the selection across the LLM round-trip, so it threw on the first chunk of the response. The LLM call itself succeeded (chat.status === "ready"), which is why the AI menu showed a generic error despite a perfectly valid tool call.Changes
ForkYDocExtension: re-point theySyncplugin state'stype/docat the newly bound fragment, on bothfork()andmerge().RelativePositionMappingExtension: resolve relative positions against the doc that owns the currently bound type, rather than the plugin state'sdoc.Impact
Fixing the plugin state at the source also covers other readers of
ySyncPluginKey.getState(...).type/.docwhile forked, including y-prosemirror's own sync plugin, which wraps local ProseMirror changes in a transaction onpluginState.doc.The mapping change is a no-op when not forked:
binding.type.docandpluginState.docare the same Y.Doc.Positions tracked before a fork keep resolving after it — forking copies the Y.Doc, so item IDs are preserved on both sides — which the second test below covers.
Testing
ForkYDoc.test.ts: tracking a position while forked, and tracking one across fork → edit → merge. Both fail onmainwithPosition not found, cannot track positions.collabUpdate.test.ts: runs theupdatestream tool end-to-end against a forked collaborative editor, parameterized over local/collaborative so the two are held to the same result. Runs fully offline — the tool call is fed straight intoStreamToolExecutor, no LLM.packages/core716 passed / 9 skipped,packages/xl-ai204 passed / 260 skipped (skips are the API-key-gated model suites, unchanged).Checklist
Summary by CodeRabbit
Bug Fixes
Tests