Skip to content

fix: run cosmetic span normalization only as fallout of local edits#3013

Merged
christianhg merged 1 commit into
mainfrom
skip-cosmetic-normalization-on-value-adoption
Jul 23, 2026
Merged

fix: run cosmetic span normalization only as fallout of local edits#3013
christianhg merged 1 commit into
mainfrom
skip-cosmetic-normalization-on-value-adoption

Conversation

@christianhg

@christianhg christianhg commented Jul 23, 2026

Copy link
Copy Markdown
Member

Opening a fragmented document and typing one character currently pushes a document-wide wave of structural rewrites: mount-time normalization merges every block's adjacent same-mark spans, parks the fallout on the pristine editor, and the first keystroke flushes it all, rewriting blocks the user never touched.

The cause is a nesting order. The sync machine's five apply sites wrap withoutNormalizing outside withRemoteChanges, so the normalize flush runs after the remote flag is restored and the cosmetic gate from #3001 never sees value adoption. The fix reorders the wrappers so withRemoteChanges is outermost. History and patching wrappers keep their positions relative to the flush, so repair emission and undo semantics are byte-identical.

Net behavior change: adopted span structure (initialValue, update value) is kept as the document has it, adjacent same-mark spans and empty sibling spans no longer merge away on load, and canonicalizes only as fallout of a local edit to its block. Scoped to the two rules #3001 classified as cosmetic: structural repairs still run everywhere, and markDef GC and empty-span annotation stripping are deliberately untouched (their classification is a tracked open decision). Four pins of the old behavior are re-homed rather than deleted: the merge contracts re-trigger via local edits, the selection pin flips to the stronger no-remap contract, and the sync-survival scenario builds its shape mismatch from a local merge. Full suites green.

Independent of #3011, whose round-trip fuzz surfaced the bug (mount-time merging forks the engine's base from the document, so diffed patches target dead keys); its seven test.fails seeds graduate when that branch rebases over this fix.

@vercel

vercel Bot commented Jul 23, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
portable-text-editor-documentation Ready Ready Preview, Comment Jul 23, 2026 8:27am
portable-text-example-basic Ready Ready Preview, Comment Jul 23, 2026 8:27am
portable-text-playground Ready Ready Preview, Comment Jul 23, 2026 8:27am

Request Review

@changeset-bot

changeset-bot Bot commented Jul 23, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 1e98388

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 14 packages
Name Type
@portabletext/editor Patch
@portabletext/plugin-character-pair-decorator Patch
@portabletext/plugin-dnd Patch
@portabletext/plugin-emoji-picker Patch
@portabletext/plugin-input-rule Patch
@portabletext/plugin-list-index Patch
@portabletext/plugin-markdown-shortcuts Patch
@portabletext/plugin-one-line Patch
@portabletext/plugin-paste-link Patch
@portabletext/plugin-sdk-value Patch
@portabletext/plugin-table Patch
@portabletext/plugin-typeahead-picker Patch
@portabletext/plugin-typography Patch
@portabletext/toolbar Patch

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

@github-actions

github-actions Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

📦 Bundle Stats — @portabletext/editor

Compared against main (149cc6f8)

@portabletext/editor

Metric Value vs main (149cc6f)
Internal (raw) 807.5 KB -
Internal (gzip) 155.1 KB +4 B, +0.0%
Bundled (raw) 1.42 MB -
Bundled (gzip) 319.4 KB +4 B, +0.0%
Import time 102ms -0ms, -0.1%

@portabletext/editor/behaviors

Metric Value vs main (149cc6f)
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 (149cc6f)
Internal (raw) 2.7 KB -
Internal (gzip) 894 B -
Bundled (raw) 2.5 KB -
Bundled (gzip) 827 B -
Import time 7ms -0ms, -0.8%

@portabletext/editor/selectors

Metric Value vs main (149cc6f)
Internal (raw) 82.7 KB -
Internal (gzip) 15.4 KB -
Bundled (raw) 78.4 KB -
Bundled (gzip) 14.3 KB -
Import time 8ms +0ms, +0.6%

@portabletext/editor/traversal

Metric Value vs main (149cc6f)
Internal (raw) 28.1 KB -
Internal (gzip) 5.6 KB -
Bundled (raw) 28.1 KB -
Bundled (gzip) 5.5 KB -
Import time 6ms +0ms, +2.8%

@portabletext/editor/utils

Metric Value vs main (149cc6f)
Internal (raw) 30.6 KB -
Internal (gzip) 6.4 KB -
Bundled (raw) 28.2 KB -
Bundled (gzip) 6.1 KB -
Import time 6ms +0ms, +1.3%

🗺️ . · ./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 (149cc6f8)

Metric Value vs main (149cc6f)
Internal (raw) 53.8 KB -
Internal (gzip) 9.8 KB -
Bundled (raw) 348.9 KB -
Bundled (gzip) 96.3 KB -
Import time 39ms -1ms, -2.6%

🗺️ 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.

The sync machine's five apply sites nested `withoutNormalizing`
outside `withRemoteChanges`, so the normalization flush at the
outermost exit ran after the remote flag was restored and cosmetic
rules (adjacent same-mark span merges, empty sibling drops) still ran
on value adoption: `initialValue` and `update value` canonicalized
content the user never touched, parking a document-wide mutation wave
that flushed with the first local edit.

Reordering the wrappers puts `withRemoteChanges` outermost at all
five sites, so the flush runs inside the flag and the cosmetic gate
covers every adoption path. History and patching wrappers keep their
positions relative to the flush, so repair emission and undo semantics
are byte-identical; the only delta is that cosmetic rules no longer
run on adopted content.

The engine-side diff round-trip fuzz on the revised-patch-pipeline
branch proved this necessary: seven seeds failed because mount-time
merging forked the engine's base from the document before any patch
arrived, and diffed patches hit dead keys. Those seeds are pinned
there as `test.fails` and will report unexpectedly-passing when that
branch rebases over this fix, graduating to its green set.

Four pins of the old adoption-merge behavior are deliberately
re-homed: the two normalization merge contracts re-trigger via local
edits (the order-independent marks comparison keeps its coverage), the
selection-preservation pin flips to the stronger no-remap contract
(the caret's span survives adoption untouched), and the stored-split-
shape sync-survival scenario creates its shape mismatch via a local
merge instead of a load-time one.
@christianhg
christianhg force-pushed the skip-cosmetic-normalization-on-value-adoption branch from b9a577e to 1e98388 Compare July 23, 2026 08:27
@christianhg christianhg changed the title fix: run cosmetic normalization only as fallout of local edits fix: run cosmetic span normalization only as fallout of local edits Jul 23, 2026
@christianhg
christianhg force-pushed the skip-cosmetic-normalization-on-value-adoption branch from 4c3f976 to de871f6 Compare July 23, 2026 08:51
@christianhg
christianhg marked this pull request as ready for review July 23, 2026 09:37
@christianhg
christianhg merged commit 68b5d17 into main Jul 23, 2026
17 checks passed
@christianhg
christianhg deleted the skip-cosmetic-normalization-on-value-adoption branch July 23, 2026 09:37
@ecoscript ecoscript Bot mentioned this pull request Jul 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant