|
| 1 | +# Plan: patch-based editor undo/redo history (on Mutative) |
| 2 | + |
| 3 | +Status: proposed (not yet implemented). Supersedes the full-site-snapshot model in |
| 4 | +`src/admin/pages/site/store/slices/site/`, and migrates the editor state layer from |
| 5 | +Immer to **Mutative** first. |
| 6 | + |
| 7 | +## Library decision: Mutative |
| 8 | + |
| 9 | +The dominant win (full-site clone → patches) is delivered by Immer or Mutative |
| 10 | +equally. Mutative is chosen because, **specifically for the patches path**, it is |
| 11 | +~1.9× faster than Immer with auto-freeze on, and — unlike Immer — stays fast with |
| 12 | +auto-freeze **off** (Immer + patches + no-freeze ≈ 5.73 ops/sec, a collapse). That |
| 13 | +headroom matters most for the broad mutations (HTML import, framework reconcile) and |
| 14 | +large sites. General store updates via `zustand-mutative` are ~10× faster than |
| 15 | +`zustand/middleware/immer`. |
| 16 | + |
| 17 | +We do **not** adopt `zustand-travel`/`travels` (the ready-made Mutative undo/redo |
| 18 | +middleware): it tracks the whole store with no selective-state support, and our undo |
| 19 | +must exclude UI state (zoom/pan/selection — gated by an existing test). We use |
| 20 | +Mutative's **core** `create({ enablePatches: true })` + `apply()` with our own |
| 21 | +`site`-scoped wiring. |
| 22 | + |
| 23 | +Footprint: Immer is used in ~7 non-test files (one `middleware/immer`, rest |
| 24 | +`import type { Draft }`) + 6 test files (`produce`). Recipe bodies are unchanged — |
| 25 | +Mutative uses the identical mutate-the-draft style. State is plain JSON-ish data |
| 26 | +(no class instances), so no `markSimpleObject` marking is needed. |
| 27 | + |
| 28 | +API mapping: |
| 29 | +- `zustand/middleware/immer` → `zustand-mutative` (`mutative` middleware) |
| 30 | +- `produce(base, recipe)` → `create(base, recipe)` |
| 31 | +- `produceWithPatches(base, recipe)` → `create(base, recipe, { enablePatches: true })` → `[next, patches, inverse]` |
| 32 | +- `applyPatches(base, patches)` → `apply(base, patches)` |
| 33 | +- `import type { Draft } from 'immer'` → `import type { Draft } from 'mutative'` |
| 34 | +- Auto-freeze: Immer defaults on; Mutative defaults off. Keep **on in dev/test** |
| 35 | + (`{ enableAutoFreeze: true }`) for the accidental-mutation guard + parity, **off in |
| 36 | + prod** for speed. |
| 37 | + |
| 38 | +## Why |
| 39 | + |
| 40 | +Today every committed mutation runs `structuredClone(site)` (`snapshotCurrentSite()` |
| 41 | +in `helpers.ts`) and the undo stack stores up to 50 whole-`SiteDocument` clones. |
| 42 | +Benchmark (synthetic sites built with the real `createNode` factory) of the clone |
| 43 | +that runs **per mutation** plus the real RSS of a 50-deep history: |
| 44 | + |
| 45 | +| Site | Nodes | Clone/mutation (sync, main thread) | 50-deep history RSS | |
| 46 | +|------|-------|------|------| |
| 47 | +| 20 pg × 250 | 5,000 | 8.8 ms | ~142 MB | |
| 48 | +| 50 pg × 400 | 20,000 | 34 ms | ~436 MB | |
| 49 | +| 100 pg × 500 | 50,000 | 98 ms | ~759 MB | |
| 50 | +| 150 pg × 800 | 120,000 | 251 ms | ~1.6 GB | |
| 51 | + |
| 52 | +The 16 ms frame budget is blown by the clone alone at ~5–8k nodes (a realistic |
| 53 | +20–40 page site), and it is paid on **every** action (insert, move, delete, colour |
| 54 | +pick, toggle) — not just typing. Memory grows to crash territory. The |
| 55 | +keystroke-coalescing change already landed reduces snapshot *frequency*; this plan |
| 56 | +removes the *per-snapshot cost* and shrinks each entry from MBs to KB. |
| 57 | + |
| 58 | +## Target design |
| 59 | + |
| 60 | +Use Immer's `produceWithPatches` to capture **inverse patches** as a by-product of |
| 61 | +the mutation the store already performs. Undo = `applyPatches(site, entry.inverse)`; |
| 62 | +redo = `applyPatches(site, entry.forward)`. Only touched paths are drafted/copied, |
| 63 | +so cost is O(change), not O(site). |
| 64 | + |
| 65 | +### Data model (`types.ts`) |
| 66 | + |
| 67 | +Replace the two `SiteDocument[]` stacks with patch-pair entries: |
| 68 | + |
| 69 | +```ts |
| 70 | +import type { Patches } from 'mutative' // Patch[] is `Patches` |
| 71 | +
|
| 72 | +export interface HistoryEntry { |
| 73 | + /** Patches that revert this transaction (applied on undo). */ |
| 74 | + inverse: Patches |
| 75 | + /** Patches that re-apply this transaction (applied on redo). */ |
| 76 | + forward: Patches |
| 77 | + /** Coalescing identity, or null. Same role as the current _historyCoalesceKey. */ |
| 78 | + coalesceKey: string | null |
| 79 | +} |
| 80 | + |
| 81 | +_historyPast: HistoryEntry[] |
| 82 | +_historyFuture: HistoryEntry[] |
| 83 | +_historyCoalesceKey: string | null // kept |
| 84 | +``` |
| 85 | + |
| 86 | +All patches are rooted at `state.site` (the single undoable document — it already |
| 87 | +embeds `packageJson` and `runtime`). `packageJson` / `siteRuntime` top-level mirrors |
| 88 | +and `activePageId` validity are **derived** after apply, exactly as the current |
| 89 | +undo/redo already recompute them. |
| 90 | + |
| 91 | +### Capture (helpers.ts) |
| 92 | + |
| 93 | +Rewrite each `mutate*` helper to capture patches with Mutative's `create`: |
| 94 | + |
| 95 | +```ts |
| 96 | +import { create } from 'mutative' |
| 97 | + |
| 98 | +function mutateSite(fn, opts) { |
| 99 | + const cur = get().site |
| 100 | + if (!cur) return false |
| 101 | + let result: SiteMutationResult |
| 102 | + const [next, forward, inverse] = create(cur, (draft) => { |
| 103 | + result = fn(draft as SiteDocument) // recipe returns void here… |
| 104 | + }, { enablePatches: true }) |
| 105 | + // …the recipe must NOT return a value (that would replace site); capture |
| 106 | + // the no-op signal in `result` instead (use rawReturn() only if a real |
| 107 | + // replacement is ever needed). |
| 108 | + if (result === false || forward.length === 0) return false |
| 109 | + set((state) => { |
| 110 | + state.site = next |
| 111 | + commitHistory(state, { inverse, forward, coalesceKey: opts?.coalesceKey ?? null }) |
| 112 | + state.hasUnsavedChanges = true |
| 113 | + // updatedAt already set by recipe or here |
| 114 | + }) |
| 115 | + return true |
| 116 | +} |
| 117 | +``` |
| 118 | + |
| 119 | +`mutateActiveTree` / `mutateActiveTreeAndSite` read `activeDocument` / `activePageId` |
| 120 | +from `get()` first, then route to the page/VC tree **inside** the `create` recipe so |
| 121 | +slot reconciliation (`reconcileVCRefsForVc`) is captured in the same patch set. |
| 122 | +`mutateSiteState` keeps its editor-state writes in the outer `set` (those fields are |
| 123 | +not undoable today and stay that way — parity preserved). `mutateAllPagesAndSite` |
| 124 | +uses the same wrapper; its patch set is larger but still ≤ a full clone. |
| 125 | + |
| 126 | +`snapshotCurrentSite()` and the `structuredClone(site)` in `pushHistory()` are deleted. |
| 127 | + |
| 128 | +### Commit + coalescing (helpers.ts) |
| 129 | + |
| 130 | +```ts |
| 131 | +function commitHistory(state, entry: HistoryEntry) { |
| 132 | + const coalescing = |
| 133 | + entry.coalesceKey !== null && |
| 134 | + entry.coalesceKey === state._historyCoalesceKey && |
| 135 | + state._historyPast.length > 0 |
| 136 | + if (coalescing) { |
| 137 | + const top = state._historyPast[state._historyPast.length - 1] |
| 138 | + // Fold: one undo must revert the whole burst back to its pre-burst state. |
| 139 | + top.inverse = [...entry.inverse, ...top.inverse] // newest-undo first |
| 140 | + top.forward = [...top.forward, ...entry.forward] // oldest-redo first |
| 141 | + state._historyFuture = [] |
| 142 | + state.canRedo = false |
| 143 | + return |
| 144 | + } |
| 145 | + state._historyPast.push(entry) |
| 146 | + if (state._historyPast.length > MAX_HISTORY) state._historyPast.shift() |
| 147 | + state._historyFuture = [] |
| 148 | + state._historyCoalesceKey = entry.coalesceKey |
| 149 | + state.canUndo = true |
| 150 | + state.canRedo = false |
| 151 | +} |
| 152 | +``` |
| 153 | + |
| 154 | +Patch arrays for a text burst are tiny (1 patch/keystroke on one path), so the |
| 155 | +concatenation stays negligible. |
| 156 | + |
| 157 | +### Apply (undoRedoActions.ts) |
| 158 | + |
| 159 | +```ts |
| 160 | +import { apply } from 'mutative' |
| 161 | + |
| 162 | +undo() { |
| 163 | + const cur = get().site |
| 164 | + const entry = lastOf(_historyPast) |
| 165 | + if (!cur || !entry) return |
| 166 | + const restored = apply(cur, entry.inverse) |
| 167 | + const packageJson = clonePackageJson(restored.packageJson) |
| 168 | + const runtime = cloneSiteRuntimeConfig(restored.runtime) |
| 169 | + set((state) => { |
| 170 | + state.site = { ...restored, packageJson, runtime } |
| 171 | + state.packageJson = packageJson |
| 172 | + state.siteRuntime = runtime |
| 173 | + state._historyPast.pop() |
| 174 | + state._historyFuture.push(entry) |
| 175 | + state._historyCoalesceKey = null |
| 176 | + state.canUndo = state._historyPast.length > 0 |
| 177 | + state.canRedo = true |
| 178 | + state.hasUnsavedChanges = true |
| 179 | + if (!state.site.pages.find((p) => p.id === state.activePageId)) { |
| 180 | + state.activePageId = state.site.pages[0]?.id ?? null |
| 181 | + } |
| 182 | + }) |
| 183 | +} |
| 184 | +``` |
| 185 | + |
| 186 | +`redo()` is symmetric (`entry.forward`, push entry back onto `_historyPast`). This is |
| 187 | +the existing undo/redo body with the stored-snapshot swap replaced by `applyPatches`. |
| 188 | + |
| 189 | +## Result (implemented) |
| 190 | + |
| 191 | +Per-mutation wall time is now **flat at ~0.25–0.4 ms regardless of site size** |
| 192 | +(measured driving real `updateNodeProps` through the store), versus the old |
| 193 | +full-site `structuredClone`: |
| 194 | + |
| 195 | +| Nodes | Patch-based | structuredClone | Speedup | |
| 196 | +|-------|-------------|-----------------|---------| |
| 197 | +| 500 | 0.25 ms | 0.76 ms | 3× | |
| 198 | +| 5,000 | 0.28 ms | 8.8 ms | 31× | |
| 199 | +| 20,000 | 0.32 ms | 34 ms | 106× | |
| 200 | +| 50,000 | 0.40 ms | 98 ms | ~245× | |
| 201 | + |
| 202 | +A full 50-deep history holds ~240 small patches (KB) instead of 50 whole-site |
| 203 | +clones (hundreds of MB). O(change), not O(site). |
| 204 | + |
| 205 | +## Phases |
| 206 | + |
| 207 | +0. **[DONE] Migrate Immer → Mutative (behavior-preserving).** Add `mutative` + |
| 208 | + `zustand-mutative`; swap the `immer` middleware in `store.ts` (and |
| 209 | + `contentAgentStore.ts`) for `mutative` with `{ enableAutoFreeze: true }` in |
| 210 | + dev/test; swap `import type { Draft } from 'immer'` → `'mutative'` (6 files); |
| 211 | + swap `produce` → `create` in the 6 page-tree/component tests; remove the `immer` |
| 212 | + dependency; update `CLAUDE.md` stack line + any deps gate. No history changes yet |
| 213 | + — full suite must stay green. This isolates the library swap from the rewrite. |
| 214 | +1. **Data model.** Add `HistoryEntry`, change stack types in `types.ts` / |
| 215 | + `siteSlice.ts` initial state. Mutative patches are enabled per-call via |
| 216 | + `create(..., { enablePatches: true })`, so no global enable needed. |
| 217 | + Lifecycle resets already clear the stacks — keep. |
| 218 | +2. **Capture.** Rewrite the six `mutate*` helpers to `produceWithPatches`; delete |
| 219 | + `snapshotCurrentSite` + `structuredClone` paths. Port `pushHistory()` (external |
| 220 | + batch entry) to capture-and-commit (it brackets manual multi-mutation batches — |
| 221 | + confirm its few callers still get one undo step). |
| 222 | +3. **Commit + coalescing.** Implement `commitHistory` with patch-concat folding. |
| 223 | +4. **Apply.** Rewrite `undo` / `redo` with `applyPatches` + derived re-mirroring. |
| 224 | +5. **Test migration.** `undo-redo.test.ts` asserts `_historyPast.length` deltas and |
| 225 | + node counts — compatible. Audit any test that inspects entry *shape* (most only |
| 226 | + reset stacks to `[]` in `beforeEach`, which is fine). Add tests: deep-undo |
| 227 | + correctness on a multi-node tree, coalesced burst → single undo, redo after |
| 228 | + coalesced burst, VC-mode tree edits, `mutateAllPagesAndSite` one-step revert. |
| 229 | +6. **Validate perf.** Re-run the benchmark harness adapted to drive real mutations: |
| 230 | + assert per-mutation time is flat (O(change)) across site sizes and history RSS is |
| 231 | + bounded by patch size, not site size. |
| 232 | + |
| 233 | +## Correctness risks & handling |
| 234 | + |
| 235 | +- **Recipe `return false` vs producer replace semantics** — never return the recipe |
| 236 | + result from the `produceWithPatches` callback; capture it in a closure var and gate |
| 237 | + on `result !== false && forward.length > 0`. |
| 238 | +- **No-op mutations** — empty `forward` ⇒ no entry, no `updatedAt`/`hasUnsavedChanges` |
| 239 | + bump (parity with today's `recipeDidMutate`). |
| 240 | +- **Editor-only state in `mutateSiteState`** — not undoable today; keep it out of the |
| 241 | + patch set so behaviour is unchanged. |
| 242 | +- **`packageJson` / `siteRuntime` mirror identity** — re-derive (clone) from the |
| 243 | + restored site so `state.site.packageJson === state.packageJson` invariant holds. |
| 244 | +- **Slot reconcile / explorer reconcile** must run *inside* the producer so their |
| 245 | + writes are in the patch set; they only mutate bounded regions, so patch sets stay |
| 246 | + small. |
| 247 | +- **autofreeze** — Mutative defaults auto-freeze off. Enable it in dev/test |
| 248 | + (`{ enableAutoFreeze: true }`) to keep the accidental-external-mutation guard and |
| 249 | + match current Immer behaviour; leave it off in prod for speed. `apply()` and |
| 250 | + `create()` tolerate frozen bases, so assigning a produced `next` back in is fine. |
| 251 | +- **`rawReturn` / recipe replacement** — never return a value from the `create` |
| 252 | + recipe (Mutative treats a returned value as a replacement, same trap as Immer). |
| 253 | + Capture the no-op signal in a closure var; use `rawReturn()` only for a deliberate |
| 254 | + whole-site replacement (not needed by any current recipe). |
| 255 | + |
| 256 | +## Out of scope / unchanged |
| 257 | + |
| 258 | +- Keystroke coalescing keys (`props:`, `bp:`, `vcparam:`) — reused as-is. |
| 259 | +- `previewFrameworkChange` clone — not history, untouched. |
| 260 | +- Persistence — history is in-memory session state, never serialised. |
0 commit comments