Addresses the approve-with-comments review on PR #154:
- applyDocToFragment: hydrate PMNode.fromJSON in its OWN try so a hydration
failure (e.g. an unknown node type) is labelled "fromJSON" — the stage that
actually threw — instead of the misleading "updateYFragment". The diagnostic
comment on unstorableYjsError ("label names the stage that failed") is now
truthful.
- assertYjsEncodable: also rehearse PMNode.fromJSON(docmostSchema, …) so a doc
that would only fail in apply's hydration step is rejected at preview time too,
narrowing the preview/apply gap (review suggestion B). Still cheap — no live
fragment, no updateYFragment.
- Tests: relabel the diagnostic test to (fromJSON); add structural-diff edge
cases — neighbour deletion keeps the unchanged node's cursor anchor, doc->empty
clears the fragment without throwing, top-level node-type change diffs in
place — plus a preview-gate test for the new fromJSON rehearsal. 297/297 green.
build/ rebuilt for the changed lib module only (build/client.js left untouched
to avoid pulling in pre-existing unrelated src/build drift).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Review of #154 (Request changes) — all clean follow-ups, no defect in the fix:
1. Single source of the ProseMirror schema: export `docmostSchema` from
docmost-schema.ts (next to docmostExtensions); diff.ts and collaboration.ts
import it instead of each calling getSchema(docmostExtensions) — the schema
can no longer drift between call sites. Removed both local builds + the now
unused getSchema imports.
2. Doc fix: assertYjsEncodable's docstring and the client.ts comment no longer
claim "the same encoder as apply" — apply uses updateYFragment, the dry-run
uses toYdoc; both reject the same unstorable attrs but are NOT byte-identical.
Reworded to "independent encodability gate".
3+4+5. Extracted `unstorableYjsError(safe, label, e)` — buildYDoc and
applyDocToFragment now share one message template (label kept for diagnostics:
toYdoc vs updateYFragment), so the wording can't drift between dry-run/apply.
6. Test for applyDocToFragment's catch branch: an unknown node type makes the
schema-validated PMNode.fromJSON throw, and the function must re-throw it
wrapped with the (updateYFragment) diagnostic.
build/ rebuilt for the three changed lib modules; 293 package tests green.
(Left build/client.js untouched: rebuilding it would pull in a pre-existing,
unrelated src/build drift — a listSidebarPages slugId fix never rebuilt on
develop — and my client.ts change there is comment-only.)
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
mutatePageContent wrote agent edits back by DELETING the whole Yjs fragment and
re-applying a fresh Y.Doc. Yjs is a CRDT — the editor anchors its selection to
node ids — so wiping every id made an open editor's cursor lose its anchor and
snap to the end of the document on every agent write. It was most visible on
comment anchoring (issue #152): a comment changes no text, yet the cursor jumped.
(Before commit 4201f0a3 the anchoring silently no-op'd, so the destructive write
never ran for comments — hence the regression.)
Fix: write via `updateYFragment` (y-prosemirror) — the same routine the editor
uses to sync its own edits into Yjs. It structurally diffs the new doc against
the live fragment and touches only changed nodes, preserving the ids of unchanged
ones, so the cursor stays put. This improves ALL agent write tools (text edits,
node ops, comments, replace) — minimal diff instead of full replace: less collab
noise, stable block-ids, other users' cursors no longer disrupted.
- collaboration.ts: new `applyDocToFragment` (sanitize -> PMNode.fromJSON against
a memoized docmost schema -> updateYFragment in one transact), keeping the
`findUnstorableAttr` encode diagnostic; swap the destructive write-back for it.
- package.json: `y-prosemirror` promoted to a direct dependency (was transitive).
- test: comment-cursor-stability.test.mjs — a Yjs RelativePosition (the cursor
anchor) survives both a sibling edit and a comment-mark anchoring (the old
full-replace tombstoned it -> null). 292 package tests green.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
