feat(sdk): session API, optional history + persist-queue, adapters — Phase 3a complete

[vanceingalls]

Summary

Completes Phase 3a of the SDK — wires the engine layer into a full Composition session with history, persist queue, and event bus.

• session.ts — CompositionImpl with typed mutation methods, batch() with transactional rollback, override-set accumulation, can() validation, serialize(), applyPatches(), and on("change" | "patch" | "persist:error") event handlers. openComposition() factory wires history + persist-queue for standalone (T1/T2) mode; T3 embedded callers supply overrides.
• history.ts — undo/redo ring with configurable coalesce window; patch-path-keyed coalescing so rapid edits to the same property collapse into one undo entry
• persist-queue.ts — debounced async write queue with promise-chain mutex (one write in flight at a time, latest-wins coalescing)
• document.ts — buildRoots / flatElements for query API
• adapters/ — PersistAdapter and PreviewAdapter interfaces; Vite filesystem adapter

Key design decisions

• T3 origin sentinel — cross-realm Symbol-like string constant (ORIGIN_LOCAL, ORIGIN_APPLY_PATCHES) avoids instanceof Symbol failures across iframes
• Timeline per element — history coalesces by patch path, not op type, so setStyle({color}) + setStyle({color}) coalesces but setStyle({color}) + setStyle({fontSize}) does not
• Content-addressed SHA-256 storage — persist adapter uses hash of serialized HTML as ETag to deduplicate writes

Test plan

• bun test packages/sdk — all 51 session + history tests pass
• Manual T1 round-trip: open → edit → persist → reload → verify state preserved

🤖 Generated with Claude Code

[vanceingalls]

• fix(sdk,core): css tokenizer, override-set replay, setattribute safety, persist errors #1350
• fix(studio): watch external project dirs so preview ETag invalidates #1347
• fix(studio,core): persist manual position edits for GSAP-owned elements #1346
• feat(lint): add gsap_studio_edit_blocked rule for manual timeline + GSAP element targeting #1345
• feat(sdk): session API, optional history + persist-queue, adapters — Phase 3a complete #1325 👈 (View in Graphite)
• feat(sdk): scaffold @hyperframes/sdk — engine layer (model, RFC 6902 patches, mutate, apply-patches) #1324
• feat(core): expose hf-ids as subpath export for @hyperframes/sdk #1323
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[miguel-heygen]

Session API is well-composed: the batch depth-tracking, coalesce window, promise-chain mutex in the persist queue, and ORIGIN_APPLY_PATCHES guard all work correctly. The adapter contract test suite is the right pattern — parameterized so future adapters get coverage for free.

P2 — getElements() re-parse is implemented here (same issue as #1324)

The concrete implementation is in session.ts:204:

getElements(): ElementSnapshot[] {
  return flatElements(buildDocument(serializeDocument(this.parsed)).roots);
}

serializeDocument → ensureHfIds → parseHTML → tree walk on every call. getElement() and find() both delegate here, so the headless agent calling:

const timedEls = comp.getElements().filter(...);
// + another filter in normalizeTiming

…does two full parses. This is the hot path for agent workloads.

Fix: cache the built element tree, set a private dirty = false flag, flip it in dispatch() / batch() finally block / applyPatches(), rebuild lazily in getElements(). Or — better — walk the live linkedom DOM directly in buildElement instead of round-tripping through HTML.

P3 — _doc parameter in constructor confirms the double-parse waste

openComposition() calls buildDocument(html) and parseMutable(html) — two parses of the same string. The doc result is immediately discarded (_doc parameter). Even with the cache fix above, you still want to derive the initial element tree from parseMutable's already-parsed DOM rather than re-parsing a second time.

P3 — empty batch() fires changeHandlers

An empty fn() (no dispatches inside) results in:

// batchForward.length === 0, !threw → fires change
if (!threw) this.changeHandlers.forEach((h) => h());

Consistent with the dispatch() no-op path, but this means comp.batch(() => {}) notifies all change subscribers. Likely fine for Phase 3b no-op ops, but worth a JSDoc note so consumers don't wrap speculative operations in a batch expecting silence on no-op.

[james-russo-rames-d-jusso]

Reviewed at the head of this PR (builds on #1324). Session API + history + persist-queue + adapters + examples. The session lifecycle and adapter contracts are well-thought-out; the persist queue and history modules are appropriately minimal. A couple of real semantic bugs worth fixing, plus interaction concerns with the embedded (T3) mode.

Strengths

• Three lifecycle modes (standalone T1/T2, embedded T3, headless) cleanly distinguished by OpenCompositionOptions shape. openComposition (session.ts:372-396) routes via isEmbedded flag derived from whether overrides is supplied.
• Adapter contract test pattern (persistAdapter.contract.test.ts) is the right shape — parameterized over implementations, runs against memory immediately, ready to extend to fs/S3/HTTP. "Write once, protect all" comment captures the intent perfectly.
• Single-in-flight write mutex in persist-queue via Promise chaining (persist-queue.ts:33,47) — correct pattern, avoids interleaved writes overwriting each other.
• coalesceMs-window history coalescing with sliding-timestamp semantics — sensible for the typing-into-text-field case where you want a single undo entry rather than per-character.
• Three end-to-end examples (headless-agent, react-embed, vanilla-editor) — demonstrates all three use modes. Good documentation pattern.

Blockers

1. batch() does NOT revert mid-batch DOM mutations on error (session.ts:233-269). When the callback throws, the batch:

   • Discards accumulated patches (line 261-264). ✓
   • But the DOM mutations from the dispatched ops already happened on this.parsed.document. ✗
   • The model is now in a partial state with no patch trail to undo it.

   So a host that does:

   comp.batch(() => {
     comp.setStyle('hf-a', { color: 'red' });
     throw new Error('user cancelled');
   });

   ends up with hf-a.style.color = 'red' permanently set in the model but no history entry, no patch emission, no way to revert. Worst-case-for-correctness.

   Fix options:

   • (a) Snapshot the live document at batch entry; restore on throw.
   • (b) Defer DOM mutations until batch commit — apply ops on batch exit, not during dispatches.
   • (c) Auto-apply inverse patches accumulated during the batch on throw.

   (c) is the smallest patch (pun unintended) — at line 240's catch, walk this.batchInverse.reverse() and apply each to the document before re-throwing. Tests for "batch throw → model unchanged" would pin this.

2. get/Element(id) and getElements() rebuild the entire SdkDocument from a re-serialization on every call (session.ts:146-150):

   getElements(): ElementSnapshot[] {
     return flatElements(buildDocument(serializeDocument(this.parsed)).roots);
   }

   This serializes the entire document to HTML, runs ensureHfIds again, parses it back into linkedom, walks the whole tree. For an editor UI that calls getElement(id) per-frame during drag preview (likely!), this is multi-millisecond on any non-trivial composition. Direct walk over this.parsed.document would be ~100× faster.

   Fix: implement getElement(id) via findById(this.parsed.document, id) directly, then build a single HyperFramesElement from the live DOM. Same for getElements() — walk this.parsed.document directly. The buildElement function in document.ts already does this; reuse it.

Concerns

3. openComposition calls ensureHfIds twice (session.ts:376-377):

   const doc = buildDocument(html);    // calls ensureHfIds internally
   const parsed = parseMutable(html);  // calls ensureHfIds internally

   Both calls produce identical output (content-keyed FNV-1a, no random — verified in core/parsers/hfIds.ts), so this is correct but wasteful. For large templates, ensureHfIds + linkedom parse is the hottest part of openComposition. Refactor to do it once:

   const stamped = ensureHfIds(html);
   const parsed = parseMutable_stamped(stamped);  // skip the internal ensureHfIds call
   const doc = buildDocument_from_parsed(parsed);

   Or have parseMutable accept an optional { skipStamp: true } flag.

4. SdkDocument is captured in CompositionImpl constructor but never used (session.ts:79). _doc: SdkDocument is the first constructor arg, prefixed with _ indicating "intentionally unused." If it's truly unused, drop it from the constructor signature and stop passing it from openComposition (line 378). If it's intended for future use, add a comment explaining what for.

5. Persist queue: error swallowed silently in doWrite (persist-queue.ts:51-53):

   try {
     await adapter.write(path, content);
   } catch {
     // error already surfaced via persist:error on the adapter
   }

   The contract relies on adapter.on('persist:error') being correctly wired by the adapter implementation. If an adapter forgets to emit persist:error on write failure (easy mistake), the SDK silently loses data with no signal to the host. Worth at least logging console.warn in the catch as a debug aid, or asserting in contract.test.ts that every adapter actually emits the error event (currently the contract test "skips" adapters without fault injection — see persistAdapter.contract.test.ts:81-93).

6. History coalescing key is brittle. shouldCoalesce (history.ts:58-63) joins opTypes with commas. If a batch contains ["setStyle", "setText"] and a subsequent single dispatch is ["setText", "setStyle"] (same set, different order), they DON'T coalesce because the joined strings differ. Sort the opTypes before join, or use a Set comparison.

7. applyPatches() emits empty inversePatches (session.ts:345):

   const event = buildPatchEvent(patches, [], origin, opTypes);

   Coherent with the design (applyPatches doesn't enter history via the isTracked filter), but if a host subscribes to 'patch' and tries to maintain its own inverse stack from emissions, applyPatches events break that stream. Document this in the applyPatches JSDoc explicitly: "PatchEvent.inversePatches is always empty for applyPatches-origin events; hosts maintaining an external inverse log must compute inverses from their own state."

Nits

• flush() in PersistQueueModule doesn't surface errors (persist-queue.ts:63-69). doWrite() swallows them, so await pq.flush() resolves even if the write failed. For an app-close handler ("flush before quit"), the host has no signal that the data didn't make it. Consider making flush() reject with the last error, or return a { ok: boolean, error?: ... } shape.

• createPersistQueue setTimeout(0) (persist-queue.ts:38) is a microtask-ish debounce. For high-frequency changes (drag preview at 60 FPS), this still queues ~16 writes per second since each change clears the prior timer but immediately schedules a new one. The intent seems to be "coalesce same-tick changes" but the actual behavior is "1-tick debounce." If the goal is "batch up writes during rapid edits," use a larger delay (50-200ms) with a max-wait ceiling. If the goal is genuinely "1-tick debounce," fine — comment explaining.

• dispose() doesn't await an in-flight write (persist-queue.ts:71-78). If pq.dispose() is called while a write is in flight, the disposed flag is set and the in-flight write completes (it'll log nothing to listeners since they're unsubscribed). For cleanup correctness this is probably fine; for tests that want to verify "no further writes after dispose," consider await writeChain in dispose.

• createPersistQueue path constructor option (persist-queue.ts:30) defaults to "composition.html". For an SDK that may be embedded in apps managing multiple compositions, a hardcoded default that overlaps with other instances is a footgun. Make it required, or surface a console.warn on default.

• createHistory maxEntries default 100 with undoStack.shift() (history.ts:89) — shift() is O(n). For 100 entries this doesn't matter; if maxEntries is ever bumped to 10000, swap to a ring-buffer pattern. Pre-emptive nit; not a real concern at the current defaults.

• session.on('persist:error') flow (session.ts:302-310) routes through the adapter's on('persist:error'). Good. But if the persist module is detached (attachPersistQueuenever called) AND a host registerson('persist:error'), the registration silently does nothing (the this.persist?.on(...)` chain is nullish). Worth a debug log or even a warn.

• adapters/headless.ts at 24 lines is presumably a no-op adapter for the headless mode. Worth a one-line comment in the file header noting "all methods are no-ops — for agent-only / non-persisting use cases."

What I verified

• Walked session.ts, history.ts, persist-queue.ts, adapters/types.ts, adapters/persistAdapter.contract.test.ts end-to-end.
• Traced the dispatch → mutate → patch → history → persist chain for a single setStyle op.
• Traced the batch() error path against the catch/finally in session.ts:240-267 and concluded blocker #1.
• Traced applyPatches() flow against isTracked in history.ts:52-56 — coherent (applyPatches origin skipped from history).
• Stamp consistency: ensureHfIds is deterministic (per core/parsers/hfIds.ts), so the double-call in openComposition is wasteful but not corrupting.
• CI mid-flight at review time; nothing failing yet.

What I didn't verify

• The three example files (vanilla-editor.ts, react-embed.ts, headless-agent.ts) — read them only for surface, didn't trace.
• Notion PRD / Shape Review (auth-gated).
• React-embed's interop with React's lifecycle — would want to verify dispose() is reachable from useEffect cleanup.
• Stress-test of the persist queue under high-frequency changes — concern #5 / nit #4 surface here.

Two real bugs (batch revert + getElement perf), several contract/doc gaps. The architectural shape is right — the SDK lifecycle is coherent, the three modes are well-distinguished, the adapter contract is honest. Worth a round-2 to address the blockers; the nits and concerns are accumulating-paint-points more than ship-stoppers.

— Review by Rames D Jusso

[vanceingalls]

All three addressed in cd85c20:

• P2 query re-parse: getElements() walks the live linkedom DOM directly (new buildRoots(document) in document.ts) with a lazily-built cache (elementsCache), invalidated in dispatch() (mutating path) and applyPatches(). Batched dispatches route through dispatch, so batches are covered. Returns a copy so callers can't alias the cache.
• P3 double parse: openComposition now calls only parseMutable(html); _doc constructor param removed.
• P3 empty batch: JSDoc on batch() documents that a no-mutation batch still fires change handlers (parity with no-op dispatch).

36/36 tests pass, lint/format clean.

[james-russo-rames-d-jusso]

Re-reviewed at 1f278807 (delta from f4199a8e — 4 files, +42/-38, plus rebases). Round-2 closes blocker #2 (live-DOM query), concern #3 (single-parse), nit #4 (unused _doc), Magi's P3 (style-parse dedup), and Magi's nit on batch() empty-fn behavior. One blocker still open (batch() doesn't revert DOM on throw).

Cleared

✅ Live-DOM query + invalidation cache (blocker #2). The new buildRoots(document) in document.ts:131-141 walks the live linkedom DOM directly — no serialize → ensureHfIds → re-parse round trip. CompositionImpl.getElements() (session.ts:147-150) caches the flattened snapshot in elementsCache: ElementSnapshot[] | null and invalidates on every dispatch (line 215) and applyPatches (line 346). UI doing per-frame getElement(id) during drag is now ~O(tree-walk) on first call after mutation, ~O(id-lookup) within the same mutation window. Significant perf improvement. Defensive copy via [...this.elementsCache] on read prevents external mutation of the cached array. Clean.

✅ Single parse in openComposition (concern #3). parseMutable(html) runs once; the SdkDocument-shaped derivation moved into the lazy query path. Saves a full ensureHfIds + linkedom parse on every session open. Comment at session.ts:389-390 documents the choice. ✓

✅ _doc: SdkDocument constructor parameter dropped (nit #4). CompositionImpl(parsed, opts) is the signature now — cleaner. ✓

✅ Magi P3 — style-parse dedup. document.ts no longer has its own parseInlineStyles; it imports getElementStyles from engine/model.ts. Single canonical kebab → camelCase implementation. Removes the drift risk between the two copies. ✓

✅ JSDoc note on batch() empty-fn behavior. Documents that "a batch that produces no effective mutations still fires 'change' handlers" — closes Magi's nit on speculative-op subscribers. ✓

Still open from round-1

1. batch() doesn't revert DOM mutations on throw (round-1 blocker #1). The catch path at session.ts:240-244 discards batchForward/batchInverse but the underlying applyOp calls have already mutated this.parsed.document. With the new elementsCache invalidation also firing inside dispatch (line 215), a subsequent getElements() correctly reflects the partial state — confirming the model and patch stream still diverge. Fix is still: walk this.batchInverse.reverse() and apply each via applyPatchesToDocument(this.parsed, ...) in the catch block before re-throwing. Round-trip test: comp.batch(() => { comp.setStyle('hf-a', { color: 'red' }); throw new Error(); }); expect(comp.getElement('hf-a').inlineStyles.color).toBeUndefined();.

Open lower-priority items

• Persist queue flush() swallows errors (round-1 nit). App-close handlers get no signal that the final write failed.
• createPersistQueue setTimeout(0) debounce (round-1 nit). 1-tick coalesce vs intended longer debounce.
• createPersistQueue path default "composition.html" (round-1 nit). Multi-composition apps will collide.
• History coalesce key uses unsorted opTypes.join(",") (round-1 concern #6). Same-set-different-order doesn't coalesce.

These are accumulation paint-points, not ship-stoppers — fine to land in a follow-up PR.

What I verified

• Round-2 diff vs round-1: document.ts (-21/+19), session.ts (-3/+15), index.ts (-1/+1), engine/model.ts (-1/+1).
• getElements() new path: buildRoots(this.parsed.document) → flatElements(...) → cached, defensive copy on read. No serialize/re-parse anywhere in the hot path.
• dispatch invalidates elementsCache BEFORE the override-set update (line 215 sits above line 217-225). Order is fine since neither path reads the cache.
• applyPatches invalidates cache too (line 346). ✓
• openComposition no longer calls buildDocument(html) — only parseMutable. ✓ The SdkDocument-shaped result is no longer materialized at open-time (only on demand via buildRoots).
• batch() control flow unchanged from round-1 — the JSDoc note doesn't alter behavior, just documents existing semantics. Concern #1 reproduces.

What I didn't verify

• Empirical perf: didn't measure getElements() before/after for a real-size composition.
• Whether the cache-invalidation-on-dispatch interacts cleanly with the batch() partial-revert fix when blocker #1 lands.
• Did not re-walk the three example files — assume unchanged unless flagged.

Big improvement on round-1 blocker #2 — the live-DOM walk is the right architecture and the cache is correctly invalidated. The batch() revert is still the one thing worth fixing before this lands on consumers; the rest are polish items.

— Review by Rames D Jusso

[miguel-heygen]

Building on Rames's review. The fix commit on this PR is housekeeping only (restore full public exports now session/document modules exist). None of the blockers are addressed yet.

✅ Fixed since last review

• index.ts export restoration — correct stack coordination, no semantic change

Still open — my findings

• P2 (getElements() re-parse hot path): session.ts:204 serializes + re-parses on every call. Resolves once the live DOM walk fix lands in #1324 and is threaded through here.
• P3 (double parse in openComposition): still fires buildDocument(html) + parseMutable(html) on the same string; _doc still discarded.
• P3 (empty batch() fires changeHandlers): worth a JSDoc note even if behavior is intentional.

Amplifying from Rames

Blocker 1 — batch() doesn't revert DOM mutations on throw is the one I'd call the most dangerous issue in this PR. The current error path:

1. DOM mutations already applied to this.parsed.document ✓ (they happen inside dispatch, which runs synchronously inside the callback)
2. Accumulated patches discarded ✓
3. No model restoration ✗

So comp.batch(() => { comp.setStyle(...); throw new Error(); }) permanently corrupts the in-memory model with no history entry, no patch emission, and no recovery path. This is nearly impossible to debug downstream because the symptom appears long after the throw site. Option (c) from Rames — apply accumulated inverse patches in the catch block before re-throwing — is the smallest correct fix and can be tested with a straightforward batch throw → getElements() state unchanged assertion.

Blocker 2 (getElements re-parse) is also confirmed — it's the same root issue as #1324 P2, just exposed here as a hot path. Rames's suggestion to call findById(this.parsed.document, id) directly for getElement() is right; buildElement already takes a live Element node, so this is a shallow refactor once the cache is in place.

[vanceingalls]

Round-3 pushed (f396300 + 4e3ece32). Against the open items:

Blocker 1 — batch rollback: fixed (4e3ece32). batch() is now transactional: on throw, accumulated inverse patches are replayed in reverse via applyPatchesToDocument and the override-set is restored from a snapshot taken at outermost batch entry; element cache invalidated. Exactly your option (c). Two tests pin it: batch throw → serialize() identical + getElement state unchanged + overrides empty, and undo after throwing batch is a no-op (no phantom history entry).

Concern 6 — coalesce key: fixed. opTypes sorted before join; also (from the adversarial review) coalescing now additionally requires identical patch paths — previously two rapid setStyle calls on different elements merged into one entry holding the second forward + first inverse, so undo reverted the wrong element. Tests cover same-path coalesce, cross-element no-coalesce, cross-property no-coalesce.

Concern 7 — applyPatches inversePatches: documented — comment now states emitted events carry an empty inversePatches array and hosts keep their own inverse log.

Also fixed since your review (adversarial pass findings):

• openComposition({ overrides }) now actually replays the override-set onto the base document (new keyToPath inverse mapping + applyOverrideSet). Previously overrides were stored but never applied — reopening an embedded composition showed the base template. 6 round-trip tests (style/text/attr/timing/removal-marker/restore-base).
• Root-priority unify (your feat(sdk): scaffold @hyperframes/sdk — engine layer (model, RFC 6902 patches, mutate, apply-patches) #1324 finding 6) + ownText trim doc (finding 7) landed here where document.ts lives.

Deferred to follow-up (agree not ship-stoppers, tracked in the last commit message): persist-queue flush() error surfacing, debounce window sizing, path default, history ring-buffer.

SDK suite: 51 tests green; tsc/lint/format/fallow clean.

[jking-lgtm]

Thanks Vance.. this is very close to what QA needs for the headless daily regression. The memory adapter + injectFault, the pure openComposition(html) seam, and the contract suite seem right.

From PR audit~~
Three small asks before we build against it:

1. Export adapter contract types from the package root
   PersistAdapter / PreviewAdapter (plus PersistVersionEntry, ElementAtPointResult, DraftProps) aren't reachable from @hyperframes/sdk. Without this, we can't type our own instrumented fakes.

Fix: Add the following to src/index.ts:
export type { PersistAdapter, PreviewAdapter, PersistVersionEntry, ElementAtPointResult, DraftProps } from "./adapters/types.js";

2. Expose an awaitable flush() on the session

openComposition({ persist }) stores the persist queue privately (session.ts:72) and Composition has no flush().

• The Issue: Tests can't deterministically await the debounced write and have to race a setTimeout(0).
• Fix: Add flush(): Promise<void> on Composition (as a no-op when the queue is empty).

3. Make history coalescing testable without the wall-clock

shouldCoalesce uses raw Date.now() (history.ts:78) with no injectable clock, and coalesceMs: 0 still coalesces same-millisecond bursts (<=).

• Fix: Either accept now?: () => number in HistoryOptions, or define coalesceMs <= 0 as "never coalesce".

---

4. (Optional, Phase 4) Promote runPersistAdapterContract

Move this into an exported testing entry (e.g., @hyperframes/sdk/testing) so future S3/HTTP adapters — and our fakes — can run the exact same contract suite.

[miguel-heygen]

Re-reviewing at dbc7da2 (fix commit from Vance). Building on Rames's rounds 1 & 2.

✅ Fixed since last review

• Blocker: batch() transactional rollback — the fix is correct. batchOverridesSnapshot snapped at outermost entry, inverse patches replayed in reverse in the catch block, elementsCache invalidated, resetBatchState() extracted cleanly. Two new tests pin the invariant (model unchanged, undo is no-op after throwing batch). Blocker closed. ✅
• History coalesce key sort (opTypesKey with .sort()) — closes concern #6 from round-1. Same-set, different-order batches now coalesce correctly. ✅
• flush() on Composition interface + CompositionImpl — flush() delegates to persistQueueModule?.flush(), no-ops cleanly when persist isn't attached. JSDoc on the interface is precise. ✅
• Public adapter type exports (PersistAdapter, PreviewAdapter, PersistVersionEntry) — callers can now write typed fakes without reaching into internals. ✅
• coalesceMs <= 0 disables coalescing — deterministic test scenario, correct short-circuit at the top of shouldCoalesce. ✅
• applyPatches JSDoc documenting empty inversePatches for applyPatches-origin events — closes concern #7 from round-1. ✅

✅ Already cleared in round-2 (confirming still in tree)

• Live-DOM walk + elementsCache (round-1 blocker #2) — buildRoots(this.parsed.document) with invalidation on dispatch/batch/applyPatches. ✅
• Single parse in openComposition — parseMutable(html) only. ✅
• _doc constructor param dropped. ✅

Remaining open items from round-1/2 (acknowledged as follow-up, not ship-blocking)

• persist-queue.ts flush() swallows errors — doWrite() catch only calls opts.onError; flush() resolves even on write failure. App-close handlers get no signal data didn't land. The onError callback added in this commit (opts.onError?.({ error: ... })) is wired on the #1350 branch but not here — this PR's persist-queue.ts still has the silent catch. Worth pulling that forward, but Rames flagged as nit and it was deferred explicitly in the commit message.
• setTimeout(0) 1-tick debounce — same as before.
• path default "composition.html" — multi-composition collision risk.

These match what the commit message explicitly deferred. Fine to land.

One new observation

resetBatchState() zeroes batchOverridesSnapshot = {}. This is correct — snapshot is consumed. But since it's a private field on CompositionImpl, the zero-value is harmless if the finally block runs before any outer batch re-enters. The nested-batch path (batchDepth > 1) only snapshots at depth === 1 and never writes to batchOverridesSnapshot again until the next outermost entry. Correct.

✅ Approving. All blockers resolved.

The base branch was changed.