fix: emit structural repair patches immediately and make engine normalization the sole repairer - #3246
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
Bundle Stats✅ No significant changes. All scenario measurements (7)🗺️
Significant means at least 1.0 KB and 1% gzip, or at least 5 ms and 10% import time. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
christianhg
force-pushed
the
emit-repair-patches-immediately
branch
from
September 8, 2026 12:32
225ddef to
ff45522
Compare
🦋 Changeset detectedLatest commit: 0db1d62 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 |
christianhg
force-pushed
the
emit-repair-patches-immediately
branch
from
September 8, 2026 13:13
ff45522 to
30ff4fe
Compare
christianhg
force-pushed
the
emit-repair-patches-immediately
branch
from
September 9, 2026 07:48
918ffda to
bbace4a
Compare
christianhg
force-pushed
the
emit-repair-patches-immediately
branch
from
September 9, 2026 07:54
bbace4a to
f2d5db0
Compare
christianhg
force-pushed
the
emit-repair-patches-immediately
branch
from
September 9, 2026 07:57
f2d5db0 to
11039dd
Compare
christianhg
changed the title
christianhg
force-pushed
the
emit-repair-patches-immediately
branch
from
September 15, 2026 07:04
28d25c8 to
5756762
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 09a0708. Configure here.
…lization the sole repairer
When a value entered the editor with structurally invalid content
(blocks or children without a `_key`, missing or empty `children`
arrays, duplicate keys), the engine repaired it right away but the
repair patches were parked: the editor machine's `pristine` state
deferred `internal.patch` and `mutation` events produced under
normalization until the first local edit flipped it to `dirty`.
Parked patches go stale under concurrent remote traffic (a sibling
insert shifts the indices a parked repair addresses, so the flush
re-keys the wrong node). The mutation batcher additionally dropped
pending work on a read-only unmount, `validateValue` repaired some
of the same defects before ingestion with its own patch shapes (its
insert branch never reported them), and a block missing its `_key`
failed validation into the invalid-value flow.
The parking machinery is deleted rather than gated: `writing` emits
`internal.patch`/`mutation` directly (the `pristine`/`dirty` split,
the normalization guard, and the conflict-discard actions are gone),
and events deferred during setup flush FIFO when setup completes.
Nothing keys on document pristineness anymore. The mutation batcher
keeps one gate, and it is a different kind: `patch` events relay
immediately regardless of read-only state, but mutation bulks hold
while the editor is read-only and flush on the first tick after it
becomes editable. Studio and Canvas flip the editor read-only when
the connection drops and both throw on mutations delivered while
read-only, so an ungated flush loses the edit typed just before the
flip (pinned by a test whose host listener rejects mutations while
read-only, red on the ungated batcher). The cleanup flush is
unconditional, so pending work is handed over on unmount instead of
dropped. `isDeferringMutations` counts only bulks holding a patch
accumulated while the editor was editable and not stamped as an
intake repair (below): repair bulks held by a
read-only editor do not latch the sync machine's busy guard, so a
read-only viewer of a malformed document keeps applying incoming
value updates (pinned red on the latching version). Held bulks
without editable-time patches are culled when a value sync settles
(signalled via an emit ordered after the pass's own repair emissions;
a plain call misordered under reentrant streamed syncs and lost the
fresh re-mint, pinned red; the emit carries the pass's echoed block
keys and is skipped when the pass aborts before completing its walk,
below): a full snapshot either already contains
the held repair or the pass re-minted it, so a superseded repair is
dropped rather than delivered against state that moved past it.
Remote patch batches never cull: a patch batch can change engine state
without superseding a held repair, and the engine's already-repaired
tree re-mints nothing under normalization, so culling there orphaned
the store's only copy of the fix (pinned red on the culling version).
A batch that genuinely supersedes a held repair (another client's key
mint) instead lets the stale repair flush and converge by last write,
the same convergence as any concurrent mint. The batch's `changed`
flag accumulates across patches instead of reflecting only the last
one, gating `normalize` and `onChange` on the whole batch. One
consequence: an operation whose patches span an inbound application
splits into two mutation events.
`validateValue`'s `autoResolve` resolutions are deleted with their
reporting machinery (`applyAutoResolution`, the sync actor's
out-of-band `patch` channel); validation passes mechanically fixable
shapes through and engine normalization repairs them on intake,
emitting engine-shaped patches with `origin: 'local'` (minimal `set`
on the minted `_key`, placeholder-span `insert` before
`children[0]`). `updateBlock` gains a wholesale-set fallback for raw
input: children arrays containing keyless children, or a raw empty
`children` replacing populated children, are set wholesale instead
of reconciled per child, so no `{_key: undefined}` path segment is
ever applied; normalization then mints the keys.
Immediate emission makes host lag observable, and two mechanisms
defend against it. First, a repair journal: `update value` snapshots
routinely arrive from before the host persisted an intake repair, and
without correlation `syncBlock` reads the engine's repaired block and
the snapshot's pre-repair block as genuinely different content,
rewrites the pre-repair shape (for `children: []` a `withoutPatching`
wholesale set), and normalization re-mints with a fresh key: content
grows one placeholder span per echo and every echo persists a
mutation. `editor.repairJournal` (FIFO-capped at 100) tracks intake
repairs keyed by the owning top-level block's post-repair `_key`. An
after-phase operation subscriber (`subscriber.repair-journal.ts`)
records entries from `event.beforeValue` (not the operation inverse:
inverses are skipped inside remote frames) whenever an operation
fires under `isInNormalization(event.context) &&
hasRemoteFrame(event.context)`, the signature of a repair replaying
remote content and never of a local edit's normalization. Chained
repairs on one block extend the entry's `afterShape` so it always
spans back to the original shape; any other operation touching the block
retires the entry. Retirement resolves the touched block by the
operation's own identity, not a positional lookup, at a numeric first
path segment: an `insert` there retires by the inserted node's own
`_key`, because `replaceBlock`'s unset-then-insert sequence shifts the
next sibling into the vacated index before the insert applies, and
`beforeValue[index]` at that point names the sibling, not the replaced
block; `unset` and `set` at a numeric segment keep resolving through
`beforeValue`, since the index still names the touched block when they
apply. `syncBlock` consults the journal before
`isEqualBlocks`: an inbound block matching `afterShape` acknowledges
the repair (entry retired), one matching `beforeShape` while the
engine holds exactly `afterShape` is the stale echo (no write,
`blockChanged: false`, the block's own `_key` returned as its
echoed-block marker), anything else retires the entry and diffs
normally. Comparisons use `deepEqualJson` (order-insensitive object
keys, order-sensitive arrays), not `isEqualBlocks`, which compares
only known text-block fields and would miss a stale `children: []`.
An echo pass proves the snapshot does not yet reflect that block's held
repair, so tracking is per block, not pass-global: `updateValue`
collects every echoing block's key into an `echoedBlockKeys` set and
`inbound state applied` fires on every settle that completed its walk,
carrying that set; a pass that stops early on an invalid block skips the
emission entirely, since an unexamined block authorizes nothing. The
mutation batcher gives each intake-repair bulk a synthetic identity
(`intakeRepairBlockKey`, derived from its patches' first path segment)
alongside `operationId`, so repairs for different blocks never share a
bulk, and `dropSupersededRepairs` takes the echoed set in place of a
boolean: it keeps a bulk holding editable-time work, a bulk from the
pass that just ran (current generation), or an intake-repair bulk whose
block key is in the echoed set, and drops any other stale intake-repair
bulk.
A numeric first path segment only resolves to a block key by indexing
into the bulk's own value snapshot, so a repair addressing a block that
is itself still keyless at that moment (normalization's missing-`_type`
arm runs before its missing-`_key` arm, so a block missing both defects
gets a `_type` patch before its own key exists) can't resolve one.
`handlePatch` never merges across that resolution boundary in either
direction: a resolved-key patch starts a fresh bulk rather than
backfilling onto an unresolved one, and vice versa, since the unresolved
bulk could belong to a different block under interleaving. Consecutive
unresolved patches still share one bulk, while a later resolved patch
for the same block opens a second. `dropSupersededRepairs` exempts an
unresolved bulk outright: it can never echo-match a block key, so there
is no way to prove it superseded, only ways to wrongly assume it. It
flushes on cadence instead, at worst one redundant mutation, never a
silently dropped repair.
The generation itself bumps at pass start, not at the cull that ends
a pass: a new engine callback, `notifyInboundSyncStarted`, wired
through `types/editor-engine.ts` and `create-editor.ts` exactly like
the settle signal above, fires from an `emit` in the sync machine's
`syncing` entry (also re-fired on the reentrant `done syncing`
transition, since reentry starts a new pass) and lands before the
pass's own invoked sync can mint anything, because XState resolves a
state's `entry` actions before spawning its `invoke`d actors (both
run on entry, entry first). Bumping at the end instead let a repair
minted between passes (normalization firing inside the `patches`
remote frame) inherit whatever generation number the next pass was
about to claim, exempting it from that pass's cull as though it were
the pass's own mint, and letting it flush stale onto a document the
pass's own snapshot had already superseded. Tagging pass membership
at the start means a between-pass repair keeps the generation current
before the next pass began, so it falls to the same echoed-set
judgment as any other held repair instead of masquerading as a fresh
mint.
Second, provenance classification: the editor machine defers
`internal.patch` during `setup`, so the batcher receives startup
repairs after the editor is already editable; receipt-time state
would classify them as user work, latch `isDeferringMutations`, and
exempt them from the supersede cull, letting the editor's queued
repair overwrite a host that repaired the same defect first (Sanity's
backend mints array keys at draft creation).
`subscriber.patch-generation.ts` stamps each `internal.patch` with
`intakeRepair` (the journal's predicate, computed at emission), the
deferred-events replay re-emits event objects verbatim so the field
survives, and `handlePatch` computes `accumulatedWhileEditable` as
`!isReadOnly() && !event.intakeRepair`. A repair bulk then never
parks the sync machine, a host's own repaired value applies on
arrival, the journal reads it as divergence (neither echo nor ack),
and the settle culls the superseded bulk.
The invalid-value flow remains for defects that need a human (a
non-object block, an unknown `_type`, a span whose `text` is not a
string), and its resolutions address the defective block truthfully:
block anchors resolve through `nodeSegment` (numeric index when the
key is unusable, instead of `{_key: undefined}`, which keyed
matching would resolve to the first keyless sibling), and
`validateValue` takes the block's base index so paths and
descriptions carry the block's document position rather than its
position in the one-block slice `syncBlock` validates (previously
always 0: applying such a resolution rewrote the first block).
The same truthfulness extends to children: a keyless child's
description and i18n params now carry its index, the same
fallback its resolution patch addresses via `nodeSegment`,
instead of interpolating an undefined `_key`.
Deliberate changes: orphaned `markDefs` are no longer pruned at
intake (the engine's prune is local-only by the two-class model);
they go when a local edit next dirties the block, as a `set` of the
filtered array. Sync `insert` operations carry the raw block, and
normalization repairs follow as their own operations. A snapshot
echoing a still-keyless copy of an already-repaired block re-mints
on the next sync: whole-value sync has no key to correlate an echo
by. Cosmetic normalization (span merging, `markDefs`/`style`/`marks`
defaults) still materializes at the first local edit.
`InvalidValueResolution.autoResolve` is removed, not deprecated: no producer exists and the type lands in a major.
A flushed `mutation` event's `value` now reads
`editorEngine.snapshot.context.value` at flush time instead of the
value captured off the last patch to join the bulk. A bulk held
read-only can sit for an arbitrary stretch, during which unrelated
engine state (a remote patch batch, a later value sync) moves on; the
old bulk-formation-time value stamped the mutation with whatever the
engine looked like at accumulation, stale by the time the hold lifts
and the mutation actually flushes. Reading the engine's current value
at the point of flush instead ties the event to what the mutation is
actually being sent alongside, closing that window. Pinned by a unit
test that changes engine state between a patch joining a bulk and the
bulk's flush, red on the bulk-captured value.
`createTestEditors` (the dual-editor test harness) relayed each mutation
to the other editor as both `patches` and a same-value `update value`,
assuming that value described only that mutation's own patches. Several
bulks flushed in one synchronous pass now carry the identical
current-at-flush value (they're delivered in the same instant), so
sending it after every one jumped the other editor past bulks whose
`patches` hadn't relayed yet, leaving those patches nothing to apply
against. The relay now defers the `update value` send to a microtask,
coalescing repeat sends within one flush pass to the last (and only, by
then identical) value, after every bulk's `patches` in that pass has
already relayed. Two dual-editor tests exercising this path needed no
further changes; the collaboration failures they'd otherwise show are
gone once the relay stops racing itself.
Two single-editor tests simulating a host that mirrors `mutation.value`
straight back into `update value` relied on that mirror being stale by
construction (the old bulk-formation-time value, sent synchronously,
already lagged reality). A synchronous mirror of a current-at-flush
value can never be stale, so both now delay the mirrored send and, where
needed, space out the local edits enough for a separate flush to land in
between, reproducing the same host-latency race against the new
contract instead of one the old value shape produced for free.
…-only
The plugin's `patch` listener marked every patch as an in-flight local
write: it set `unflushedEdits` and sent 'patch emitted', both reset
only by the 'mutation flushed' that follows a mutation. The editor now
emits intake-repair patches while read-only but holds their mutation
until the editor becomes editable, so a read-only session on a
structurally invalid document parked the machine in 'local
write'/'pending sync' (where 'remote value changed' applies nothing)
and left `unflushedEdits` set (re-arming every repair confirmation
indefinitely): whole-value store updates stopped reaching the editor
for the rest of the read-only session.
An intake-repair patch no longer latches either mechanism, regardless
of read-only state at receipt: `intakeRepair` is a provenance flag the
editor stamps on the patch at emission (normalization firing inside a
remote frame), not a read-only inference, so it survives `setup`'s
deferred-events replay landing after read-only has already lifted.
Its held mutation still pushes to the store once it flushes: 'mutation
flushed' is handled in every machine state. Pinned by a test whose
read-only editor receives a store update after an intake repair, red
on the latching listener.
Skipping the latch left `applySync`'s idle-quiescent pass with no way
to tell a held repair from the store having drifted: it diffed the
repaired engine against the still-unrepaired store and shipped patches
undoing the repair. A held read-only repair now records the store
value it was diffed away from; a divergence against that exact value
is recognized as the repair still waiting to flush and left alone,
and the record clears once the store moves on, whether from the
eventual push or a genuinely new remote value. Pinned by a test whose
read-only editor's minted key survives a quiescent sync pass and
flushes once the editor becomes editable, red on the unrecorded held
repair.
Gating on `intakeRepair` instead of read-only state also matters for a
patch the read-only edit-mode state still admits: a handful of
Behavior events (`select`, `mouse.click`, `clipboard.copy`, and the
serialization events) run their actions even while read-only, and a
mutating Behavior on one of those produces a patch that is genuine
user work, not a repair. The old read-only-at-receipt gate mistook it
for a held repair (never latching `unflushedEdits`), so the background
repair-diff pass could read the user's own edit as store divergence
and revert it. Pinned by a test whose read-only `select` Behavior
deletes a block and survives a store-driven sync pass, red on the
read-only-at-receipt gate. The recorded store-value baseline
(`heldReadOnlyRepairBaselines`) now compares with `deepEqualJson`
(order-insensitive object keys) instead of `JSON.stringify`, matching
the comparison the engine itself uses, so a remote echo with
differently ordered keys isn't mistaken for a genuinely new value.
A `patches`-only reconciliation (`applySync`'s confirmed repair diff,
and the SDK patch channel's `'apply remote patches'`) never gave a
held repair the chance `dropSupersededRepairs` needs: that cull only
runs on a completed `update value` pass, so a held repair a newer
store transaction had already superseded survived to the editable
flip and flushed anyway, overwriting the transaction's key (pushed
through `pushPatches`) or, on a host without a patch channel, pushing
the pre-transaction content as a whole value. Both call sites now
follow a successful patches-based reconciliation, while a repair is
still held read-only, with `editor.send({type: 'update value', value})`
using the snapshot just applied: the engine already matches it, so the
resulting sync pass settles without emitting anything of its own, but
its completion still runs the batcher's supersede judgment against the
newer state. Pinned by browser tests covering both the patch-channel
and whole-value-only hosts, red on the unjudged held repair.
…eyless spans Adds coverage for the same normalize-node repair arms already pinned for flat blocks, applied to editable containers: the missing/empty child-array repair (`normalizeNode`'s container arm, unconditional on remote frame) and the generic missing-`_key` repair reaching a span nested three container levels down (table > row > cell > block > span). Both repairs journal and cull at the top-level container's own key, same as a flat block: a stale echo of the pre-repair container is swallowed without re-minting, and two sibling containers each carrying their own deep defect open two separate per-block repair bulks in the mutation batcher, so a host fixing one container's defect supersedes only that container's bulk while the other container's still-echoing repair still flushes.
The rig arrives from `main` pinning the stable line's contract: loading a structurally broken document mutates nothing. On this line repairs persist immediately when a value is applied, so `repair-on-load.spec.ts` flips to the matching assertion: exactly one editor-emitted repair transaction on load (alongside Content Lake's own server-side key enrichment during draft creation), then silence, with the repaired draft re-fetched and every child keyed. The README's contract section swaps accordingly. Each line's copy of the suite guards its own intended behavior, so an accidental cross-port of either policy turns the other branch's CI red.
This branch was successfully deployed
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.
The editor repairs structural defects in any value passed to it, initial or updated: it generates a
_keyfor a block or child that has none or duplicates a sibling's, and it inserts an empty span into a text block with no children. Until now those repairs stayed in the editor's memory and the fixing patches were only emitted once the user made their first edit. Now they are emitted as soon as the value is applied. The practical consequence: loading a broken document can produce amutationto persist before any user action.patchevents and theirmutationright away, instead of at the user's first edit.patchevents right away too, but holds themutationuntil it becomes editable, since hosts reject writes to read-only documents. Previously, the pristine/dirty machinery that gated this deferred everypatch/mutation, not only repairs, until the document's first local edit, and a read-only document can never make one; that staging is now deleted outright. The one thing still deferred to a local edit is cosmetic normalization (span merging, defaultmarkDefs/style/marks, orphanedmarkDefspruning), which never runs on intake at all.mutationis pending normally discards the outdated repair, so a host that already fixed the same defect with its own key wins regardless of arrival order. Two exceptions, deliberate: a repair whose owning block can't yet be key-resolved is never dropped, only flushed on schedule, occasionally redundantly; a repair superseded by another client's own patch batch, rather than a whole-value snapshot, still flushes and converges by last write. A value still showing a block's exact pre-repair shape reads as host lag, not a fresh defect, so it's neither recomputed nor dropped, but that recognition expires once the block's tracking entry retires (another operation touches the block, or the 100-entry journal cycles it out): a still-stale value re-mints under a new key from there.mutationevents instead of one.mutationlisteners for any host: previously a read-only unmount dropped them without emitting them at all. A host that rejects mutations while read-only still loses them there, as before; a host that doesn't reject now actually receives them, where before it silently didn't._keyno longer triggers the "invalid value" prompt; it's repaired like the other defects. The prompt still appears for defects that need a human decision (an unknown_type, a non-object block), and its resolution patches now address the defective block's own position, with keyless children named by index instead of anundefinedkey.InvalidValueResolution.autoResolveis removed. The editor repairs mechanically fixable defects itself on intake, so no resolution is ever auto-resolvable; hosts that branched on the flag can delete that path.Every patch normalization produces while replaying remote content is now stamped
intakeRepair, and that provenance flag, not read-only state at receipt, is what the supersede logic above keys on. That matters beyond repairs: a handful of Behavior events (select,mouse.click,clipboard.copy, the serialization events) still run their actions while read-only, and a mutating Behavior on one of those produces genuine user work. Never stampedintakeRepair, that patch is never mistaken for a held repair and never at risk from the supersede cull, whatever the read-only state was when it landed.@portabletext/plugin-sdk-valueships a companion fix keyed on that same flag. Its value-sync machine used to treat every incoming patch as an unsaved local write, latching until a mutation flush cleared it; a read-only repair patch can't get that flush until the editor turns editable, so the plugin stopped applying remote store updates for the rest of the read-only session. It now readsintakeRepairinstead of read-only state, keeps applying store updates while a repair sits unflushed, and pushes it once the editor goes editable. A held repair also used to get misread as store drift by a background sync pass and reverted; the plugin now recognizes it as still-pending and leaves it alone. The same fix protects a read-only Behavior's own mutation the same way, tracking it like any other unpushed edit instead of risking it to reversion.Studio rig contract
The Studio integration rig (
apps/studio, landed onmainand inherited here through the base branch) pins each line's load contract: onmainthe suite asserts that loading a broken document mutates nothing, and this PR flips this branch's copy to the matching v9 assertion, exactly one editor-emitted repair transaction on load, then silence, so an accidental cross-port of either policy turns the other branch's CI red. The full suite runs against this PR via thetrigger: studio-e2elabel.Note
High Risk
Major changes to value sync, mutation batching, and persistence on load affect every host integrating patches/mutations and collaborative read-only flows.
Overview
Structural repairs now persist on load. Missing or duplicate
_keys and empty text-blockchildrenare fixed by engine normalization when a value is applied, and the resultingpatchevents fire immediately (mutationbatches on the usual debounce). Hosts can see a repairmutationbefore any user edit; Studio e2e now expects exactly one editor repair transaction when opening a broken draft.The old “pristine until first edit” gating is gone from the editor machine and mutation batcher.
intakeRepairtags normalization fixes during remote intake so they are not treated as unsaved user work. A repair journal plus sync-machineinbound sync started/inbound state applied(with per-block echoed keys) lets the batcher drop superseded held repairs when a newer snapshot arrives, while still protecting unresolved-key repairs, concurrent remote patches, and read-only Behavior mutations on events likeselect.Invalid-value handling narrows: missing
_keyis no longer a prompt—normalization repairs it.InvalidValueResolution.autoResolveis removed; human-needed defects get resolution patches keyed by block/child index when keys are missing. OrphanmarkDefsare no longer stripped at validation time.Read-only editors relay patches immediately but hold mutations until editable; unmount can flush held mutations even while read-only.
MutationEvent.valueis taken at flush time, not when patches were queued.@portabletext/plugin-sdk-value(changeset): value-sync no longer latches on read-only repair patches and avoids reverting held repairs during background sync.Reviewed by Cursor Bugbot for commit 0db1d62. Bugbot is set up for automated code reviews on this repo. Configure here.