fix: position remote text patches relative to unflushed local edits - #3020
Draft
ryanbonial wants to merge 8 commits into
Draft
fix: position remote text patches relative to unflushed local edits#3020ryanbonial wants to merge 8 commits into
ryanbonial wants to merge 8 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 4bedfee The changes in this PR will be included in the next version bump. This PR includes changesets to release 14 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
📦 Bundle Stats —
|
| Metric | Value | vs main (4352ef3) |
|---|---|---|
| Internal (raw) | 819.7 KB | +12.2 KB, +1.5% |
| Internal (gzip) | 157.6 KB | +2.4 KB, +1.6% |
| Bundled (raw) | 1.43 MB | +12.3 KB, +0.8% |
| Bundled (gzip) | 321.8 KB | +2.4 KB, +0.8% |
| Import time | 96ms | -2ms, -1.7% |
@portabletext/editor/behaviors
| Metric | Value | vs main (4352ef3) |
|---|---|---|
| Internal (raw) | 467 B | - |
| Internal (gzip) | 207 B | - |
| Bundled (raw) | 424 B | - |
| Bundled (gzip) | 171 B | - |
| Import time | 2ms | -0ms, -0.9% |
@portabletext/editor/plugins
| Metric | Value | vs main (4352ef3) |
|---|---|---|
| Internal (raw) | 2.7 KB | - |
| Internal (gzip) | 894 B | - |
| Bundled (raw) | 2.5 KB | - |
| Bundled (gzip) | 827 B | - |
| Import time | 7ms | -0ms, -0.6% |
@portabletext/editor/selectors
| Metric | Value | vs main (4352ef3) |
|---|---|---|
| Internal (raw) | 82.7 KB | - |
| Internal (gzip) | 15.4 KB | - |
| Bundled (raw) | 78.4 KB | - |
| Bundled (gzip) | 14.3 KB | - |
| Import time | 8ms | +0ms, +5.1% |
@portabletext/editor/traversal
| Metric | Value | vs main (4352ef3) |
|---|---|---|
| Internal (raw) | 28.1 KB | - |
| Internal (gzip) | 5.6 KB | - |
| Bundled (raw) | 28.1 KB | - |
| Bundled (gzip) | 5.5 KB | - |
| Import time | 6ms | +0ms, +3.8% |
@portabletext/editor/utils
| Metric | Value | vs main (4352ef3) |
|---|---|---|
| Internal (raw) | 30.6 KB | - |
| Internal (gzip) | 6.4 KB | - |
| Bundled (raw) | 28.2 KB | - |
| Bundled (gzip) | 6.1 KB | - |
| Import time | 6ms | -0ms, -0.2% |
🗺️ . · ./behaviors · ./plugins · ./selectors · ./traversal · ./utils · Artifacts
Details
- Import time regressions over 10% are flagged with
⚠️ - Sizes shown as raw / gzip 🗜️. Internal bytes = own code only. Total bytes = with all dependencies. Import time = Node.js cold-start median.
📦 Bundle Stats — @portabletext/markdown
Compared against main (4352ef3d)
| Metric | Value | vs main (4352ef3) |
|---|---|---|
| Internal (raw) | 53.8 KB | - |
| Internal (gzip) | 9.8 KB | - |
| Bundled (raw) | 348.9 KB | - |
| Bundled (gzip) | 96.3 KB | - |
| Import time | 38ms | -1ms, -2.3% |
🗺️ View treemap · Artifacts
Details
- Import time regressions over 10% are flagged with
⚠️ - Sizes shown as raw / gzip 🗜️. Internal bytes = own code only. Total bytes = with all dependencies. Import time = Node.js cold-start median.
…hes" The strict ok/ambiguous result plumbing and base-refresh timing did not survive end-to-end validation against the concurrency harness: the regressions it aimed at trace to value adoption and push consistency, not to base matching. Return to the simpler deterministic matching that the harness validates.
…and emptied documents Concurrency harness regressions relative to the published packages: delete-overlap and disjoint-blocks-format went SAFE to SILENT-LOSS and link-overlap-range persisted invalid Portable Text (an orphan mark with no markDef). Diagnosed against a real dataset with full value dumps and a mutation listener; three root causes, fixed client-side: - Editors reached semantically identical values with different JSON property order (locally created vs adopted from fetched documents) and could never converge byte-for-byte. Nodes are now rebuilt in canonical property order at the engine's mutation chokepoints. - Emptying a document set the field to [] and every editor re-minted placeholder keys on each adoption and repair pass, so empty documents diverged forever. Adopting "empty" into an empty editor is now a no-op, clearing empties the first text block in place keeping its keys, and the plugin treats the local placeholder and an empty document as the same state. - markDefs housekeeping pruned definitions on value adoption that a collaborator's in-flight edits still referenced, orphaning their marks at the document. The pruning is now cosmetic-only (local-edit fallout), the plugin never pushes a definition unset while the store, the editor, or the same flush still references it, re-inserts definitions the store lost when a flush references them, and heals stranded annotation references on confirmed repairs. Marks that never appeared in markDefs are untouched, so schema-less decorator marks still survive. Harness after the fix: delete-overlap and disjoint-blocks-format SAFE, no CORRUPT in any link trial (12 consecutive), link-same-range keeps its SILENT-LOSS improvement, and bold-overlap-range, block-split, and paste-vs-type improved to SAFE. No scenario scores worse than published.
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.
Description
Two bodies of work, validated end-to-end against the concurrency safety harness (two live sessions on a real dataset).
Position remote text patches relative to unflushed local edits
Two editors changing the same span concurrently could splice one editor's text into the middle of the other's. Incoming
diffMatchPatchpatches were fuzzy-matched against live text that already contained unresolved local edits, so repeated context could select the wrong position.The editor now stores the text before unresolved local edits for each span. Incoming patches are matched exactly against that base and the live text. Base matches are merged with the local branch using deterministic three-way text merging. A live-only match proves that the sender incorporated the local change and retires the pending base.
Concurrent insertions at the same boundary use stable text ordering. Deletions remove only characters from the shared base, so concurrent insertions survive overlapping deletes. The stored base advances after each remote merge and is cleared on acknowledgement, structural replacement, value replacement, or expiry.
The remote patch batcher also preserves earlier successful patches when a later patch is ignored, so normalization and change notification still run.
Converge concurrent editors on formatting, overlapping deletes, and emptied documents
Harness runs comparing this branch to the published packages surfaced three regressions:
delete-overlapanddisjoint-blocks-formatwent SAFE to SILENT-LOSS, andlink-overlap-rangepersisted invalid Portable Text (an orphan mark with nomarkDef). Diagnosing with full value dumps and a Content Lake mutation listener showed three root causes, all fixed client-side:{_type, _key, ...}while the same span adopted from a fetched document kept the server's alphabetized property order, so converged editors never became byte-identical. Nodes entering the engine are now rebuilt in a canonical property order (_type,_key, then the rest alphabetically) at the mutation chokepoints.[], and every editor then re-minted placeholder keys on each adoption and repair pass. Adopting "empty" into an already-empty editor is now a no-op, clearing empties the first text block in place (keeping its keys, the same shape a select-all-delete leaves), and the plugin treats the local placeholder and an empty document as the same state instead of repairing one into the other forever.markDefsare untouched, so decorators from schemas this editor doesn't know still survive.Harness results (TRIALS=2, ROUNDS=3, real dataset)
All typing scenarios stay SAFE, no scenario scores worse than published, and the link scenarios produced zero CORRUPT outcomes across 12 consecutive trials after the fix (previously roughly 1 in 3).
What to review
packages/editor/src/internal-utils/span-text-merge.ts: exact patch matching and deterministic three-way text merging.packages/editor/src/internal-utils/applyPatch.ts: base selection, acknowledgement, fuzzy fallback, and application to live text.packages/editor/src/internal-utils/pending-local-text-edits.ts: per-span base state and expiry.packages/editor/src/editor/subscriber.local-text-edits.ts: local, normalization, undo, redo, and structural lifecycle tracking.packages/editor/src/engine/utils/canonicalize-properties.tsplus its call sites inapply-operation.tsandmodify.ts: canonical property order.packages/editor/src/editor/sync-machine.ts: key-preservingclearEditorand the empty-adoption no-op.packages/editor/src/engine/core/normalize-node.tsandinternal-utils/validateValue.ts: markDefs housekeeping gated to local-edit fallout.packages/plugin-sdk-value/src/plugin.sdk-value.tsx: transaction-consistent markDefs decompose, stranded-mark healing, and empty-placeholder equivalence in the repair loop.Testing
pnpm test:unit(editor): 889 passedpnpm test:browser:chromium(editor): 1,761 passed, 1 skippedplugin-sdk-valueunit: 52 passed; chromium browser (incl. collision matrix and multi-round link overlap): 58 passedpnpm check:types,pnpm check:lint: passed in both packagesUpdated tests that pinned replaced behaviors: placeholder re-minting on clear (keys are now preserved), unused-markDef pruning on adoption (now kept until the next local edit), one literal property-order assertion, the multi-round link test (editors now converge on the server value verbatim), and the plugin's empty-store tests (no synthetic caret).
Notes for release
Fixes concurrent edits to the same span being merged inside each other or losing characters, concurrent formatting and overlapping deletes leaving sessions permanently diverged, and concurrent link formatting persisting marks without a matching
markDef. Sessions now converge on the same deterministic result, and documents stay valid Portable Text.