Repository navigation
🐛 Stop diff() emitting lone surrogates after semantic cleanup - #116
Open
alecgibson wants to merge 1 commit into
Open
alecgibson wants to merge 1 commit into
alecgibson wants to merge 1 commit into
Conversation
Fixes slab#115 `diff()` runs `fast-diff` with semantic cleanup enabled. `fast-diff` only applies its unicode-aware merge in the pass that runs *before* cleanup: `diff_cleanupSemantic()` then merges again without the flag, re-splitting the surrogate pairs the first pass protected. The result is ops holding half an astral character. Composing them still gives the right document, since the halves rejoin across op boundaries, but a lone surrogate has no UTF-8 encoding, so anything that serialises an op on its own to bytes mangles or rejects it. This change falls back to the uncleaned diff when the cleaned one splits a pair, since the plain diff is surrogate-safe by construction. Those rare diffs then come back character-aligned rather than word-aligned. Fixing this in `fast-diff` would be preferable, but it hasn't had a release since May 2023. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
alecgibson
force-pushed
the
diff-lone-surrogates
branch
from
September 9, 2026 07:25
2275581 to
02ec62b
Compare
alecgibson
added a commit
to reedsy/delta
that referenced
this pull request
Sep 9, 2026
Refs reedsy/reedsy-editor#14888 `Delta#diff()` calls `fast-diff` with semantic cleanup enabled. Under the hood, `fast-diff` runs `diff_cleanupMerge()` with its `fix_unicode` flag, which shaves stray surrogates off the boundaries of equalities so that no tuple ever ends up holding half an astral character. However, it only does this in the merge pass that runs *before* the optional semantic cleanup, and `diff_cleanupSemantic()` calls `diff_cleanupMerge()` again without the flag, happily re-splitting the pairs the first pass protected. The result is that a small fraction of diffs between documents containing emoji come back with lone surrogates in their ops, e.g.: ``` new Delta().insert('x🌀').diff(new Delta().insert('x🏆🌀')) // [{retain: 2}, {insert: '\uDFC6\uD83C'}] ``` Composing such a delta still produces the right document — the halves rejoin across op boundaries — so this doesn't corrupt anything locally. But a lone surrogate is legal in a JS string while having no UTF-8 encoding, so anything that serialises the op to bytes either mangles it to U+FFFD or rejects it. In our case ShareDB stores it happily in Mongo, and then BigQuery's Storage Write API rejects the archived row, which aborts the commit and blocks the user's save. This change checks the cleaned-up diff for lone surrogates and, if it finds any, re-runs the diff without semantic cleanup. `fast-diff`'s plain diff is surrogate-safe by construction, so the fallback can never split a pair. The only cost is that these rare diffs come out character-aligned rather than word-aligned, which is cosmetic for us: our callers use the result to derive ops, not to show them to a human. The proper fix would be for `fast-diff` to run a final unicode-aware merge after semantic cleanup, but it hasn't had a release since 2023, so we work around it here rather than forking it. The bug is [reported][1] and the same workaround [proposed][2] upstream, so we can drop ours if it lands there. Semantic cleanup only splits a pair for particular alignments of the two documents, which is far too sparse a target to hit by hand, so the tests include a seeded fuzz over a few thousand random emoji-laden document pairs alongside the hand-written cases. [1]: slab#115 [2]: slab#116 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
alecgibson
marked this pull request as ready for review
September 9, 2026 07:37
alecgibson
added a commit
to reedsy/delta
that referenced
this pull request
Sep 9, 2026
Refs reedsy/reedsy-editor#14888 `fast-diff` shaves stray surrogates off its equality boundaries, but only in the merge pass that runs *before* semantic cleanup, which then happily re-splits the pairs that pass protected. A lone surrogate is legal in a JS string while having no UTF-8 encoding, so diffs of emoji-laden documents come back with ops that ShareDB stores happily in Mongo and BigQuery's Storage Write API then rejects, aborting the commit and blocking the user's save. So when the cleaned-up diff contains a lone surrogate, we re-run the diff without semantic cleanup, which is surrogate-safe by construction. Those rare diffs then come out character- rather than word-aligned, which is cosmetic for us: we derive ops from the result, we never show it to anyone. `fast-diff` should really run a final unicode-aware merge itself, but it hasn't had a release since 2023, so we work around it here rather than forking it. The bug is [reported][1] and this workaround [proposed][2] upstream, so we can drop ours if it lands there. [1]: slab#115 [2]: slab#116 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
alecgibson
added a commit
to reedsy/delta
that referenced
this pull request
Sep 9, 2026
Refs reedsy/reedsy-editor#14888 `fast-diff` shaves stray surrogates off its equality boundaries, but only in the merge pass that runs *before* semantic cleanup, which then happily re-splits the pairs that pass protected: ``` new Delta().insert('x🌀').diff(new Delta().insert('x🏆🌀')) // [{retain: 2}, {insert: '\uDFC6\uD83C'}] ``` A lone surrogate is legal in a JS string while having no UTF-8 encoding, so ShareDB stores these ops happily in Mongo and BigQuery's Storage Write API then rejects the archived row, aborting the commit and blocking the user's save. So when the cleaned-up diff contains a lone surrogate, we re-run the diff without semantic cleanup, which is surrogate-safe by construction. Those rare diffs then come out character- rather than word-aligned, which is cosmetic for us: we derive ops from the result, we never show it to anyone. `fast-diff` should really run a final unicode-aware merge itself, but it hasn't had a release since 2023, so we work around it here rather than forking it. The bug is [reported][1] and this workaround [proposed][2] upstream, so we can drop ours if it lands there. [1]: slab#115 [2]: slab#116 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
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.
Fixes #115
diff()runsfast-diffwith semantic cleanup enabled.fast-diffonly applies its unicode-aware merge in the pass that runs before cleanup:diff_cleanupSemantic()then merges again without the flag, re-splitting the surrogate pairs the first pass protected.The result is ops holding half an astral character. Composing them still gives the right document, since the halves rejoin across op boundaries, but a lone surrogate has no UTF-8 encoding, so anything that serialises an op on its own to bytes mangles or rejects it.
This change falls back to the uncleaned diff when the cleaned one splits a pair, since the plain diff is surrogate-safe by construction. Those rare diffs then come back character-aligned rather than word-aligned. Fixing this in
fast-diffwould be preferable, but it hasn't had a release since May 2023.🤖 Generated with Claude Code
Co-Authored-By: Claude noreply@anthropic.com