fix(sdk): transactional batch rollback, sorted coalesce key, root-priority unify

Round-2 review (Rames/Miguel) on the session layer:

- batch() is now transactional: on throw, accumulated inverse patches are
  replayed in reverse and the override-set snapshot restored — the model is
  exactly as it was at batch entry. Previously a throwing batch left the DOM
  partially mutated with no patch trail, no history entry, no recovery path.
  2 new tests (model unchanged + undo is no-op after throwing batch).
- history coalesce key sorts opTypes — same op-type set coalesces regardless
  of dispatch order within a batch.
- applyPatches comment documents that emitted PatchEvents carry an empty
  inversePatches array (hosts keep their own inverse log).
- document.ts extractDimensions/extractDuration now use the engine's
  findRoot — dimension extraction and mutations agree on the root element
  ([data-hf-root] > #stage > first child). Dimensions prefer the runtime's
  data-width/data-height forced-override attrs, falling back to inline style.
- ownText documented: snapshot .text is trimmed display text; setText writes
  verbatim.

Deferred to follow-up (acknowledged, not ship-blocking): persist-queue flush
error surfacing, debounce window, path default, history ring-buffer.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Author: vanceingalls

packages/sdk/src/document.ts

@@ -10,7 +10,7 @@
 
 import { parseHTML } from "linkedom";
 import { ensureHfIds } from "@hyperframes/core/hf-ids";
-import { getElementStyles } from "./engine/model.js";
+import { findRoot, getElementStyles } from "./engine/model.js";
 import type { HyperFramesElement, SdkDocument } from "./types.js";
 
 // Tags that carry no editable content and must not enter the element tree.
@@ -25,6 +25,9 @@ const EXCLUDED_TAGS = new Set([
   "head",
 ]);
 
+// Snapshot text is TRIMMED for display (markup indentation produces noisy
+// whitespace text nodes). setText writes verbatim — engine getOwnText/setOwnText
+// operate on raw text. el.text is a display value, not a round-trip identity.
 function ownText(el: Element): string | null {
   let text = "";
   el.childNodes.forEach((n) => {
@@ -105,21 +108,26 @@ function extractStyles(doc: Document): string | null {
   return styleEl ? styleEl.textContent : null;
 }
 
+// Root resolution delegates to the engine's findRoot so dimension extraction
+// and mutations agree on which element is the composition root.
 // fallow-ignore-next-line complexity
 function extractDimensions(doc: Document): { width: number | null; height: number | null } {
-  const stage = doc.getElementById("stage") ?? doc.querySelector("[data-hf-root]");
+  const stage = findRoot(doc);
   if (!stage) return { width: null, height: null };
+  // data-width/data-height are the runtime's forced override — prefer them.
+  const wAttr = stage.getAttribute("data-width");
+  const hAttr = stage.getAttribute("data-height");
   const style = (stage as HTMLElement).getAttribute?.("style") ?? "";
   const wm = /width:\s*(\d+)px/.exec(style);
   const hm = /height:\s*(\d+)px/.exec(style);
   return {
-    width: wm ? parseInt(wm[1] ?? "", 10) : null,
-    height: hm ? parseInt(hm[1] ?? "", 10) : null,
+    width: wAttr !== null ? parseInt(wAttr, 10) : wm ? parseInt(wm[1] ?? "", 10) : null,
+    height: hAttr !== null ? parseInt(hAttr, 10) : hm ? parseInt(hm[1] ?? "", 10) : null,
   };
 }
 
 function extractDuration(doc: Document): number | null {
-  const root = doc.querySelector("[data-hf-root]") ?? doc.body;
+  const root = findRoot(doc) ?? doc.body;
   const dur = root?.getAttribute("data-duration");
   return dur ? parseFloat(dur) : null;
 }

packages/sdk/src/history.ts

@@ -62,8 +62,13 @@ export function createHistory(session: Composition, opts: HistoryOptions = {}):
       .join("\n");
   }
 
+  function opTypesKey(opTypes: readonly string[]): string {
+    // Sorted: the same op-type SET coalesces regardless of dispatch order.
+    return [...opTypes].sort().join(",");
+  }
+
   function shouldCoalesce(entry: HistoryEntry, incoming: PatchEvent): boolean {
-    if (entry.opTypes.join(",") !== incoming.opTypes.join(",")) return false;
+    if (opTypesKey(entry.opTypes) !== opTypesKey(incoming.opTypes)) return false;
     if (entry.origin !== incoming.origin) return false;
     // Coalesce only when the SAME paths are touched (e.g. slider drag on one
     // property). Without this, rapid edits to different elements would merge

packages/sdk/src/session.test.ts

@@ -102,3 +102,39 @@ describe("override-set replay on open", () => {
     expect(comp.getOverrides()).toEqual(overrides);
   });
 });
+
+// ─── batch() transactional rollback ───────────────────────────────────────────
+
+describe("batch rollback on throw", () => {
+  it("reverts DOM mutations and override-set when the callback throws", async () => {
+    const comp = await openComposition(BASE_HTML);
+    const htmlBefore = comp.serialize();
+
+    expect(() =>
+      comp.batch(() => {
+        comp.setStyle("hf-title", { color: "#e63946" });
+        comp.setText("hf-sub", "changed");
+        throw new Error("user cancelled");
+      }),
+    ).toThrowError("user cancelled");
+
+    expect(comp.getElement("hf-title")?.inlineStyles["color"]).toBe("#fff");
+    expect(comp.getElement("hf-sub")?.text).toBe("subtitle");
+    expect(comp.serialize()).toBe(htmlBefore);
+    expect(comp.getOverrides()).toEqual({});
+  });
+
+  it("a throwing batch leaves no history entry — undo is a no-op", async () => {
+    const comp = await openComposition(BASE_HTML);
+    try {
+      comp.batch(() => {
+        comp.setStyle("hf-title", { color: "#e63946" });
+        throw new Error("boom");
+      });
+    } catch {
+      // expected
+    }
+    comp.undo();
+    expect(comp.getElement("hf-title")?.inlineStyles["color"]).toBe("#fff");
+  });
+});

packages/sdk/src/session.ts

@@ -77,6 +77,8 @@ class CompositionImpl implements Composition {
   private batchInverse: JsonPatchOp[] = [];
   private batchOpTypes: string[] = [];
   private batchOrigin: unknown = ORIGIN_LOCAL;
+  /** Override-set state at outermost batch entry — restored if the batch throws. */
+  private batchOverridesSnapshot: OverrideSet = {};
 
   constructor(parsed: ParsedDocument, opts: OpenCompositionOptions) {
     this.parsed = parsed;
@@ -237,6 +239,11 @@ class CompositionImpl implements Composition {
 
   /**
    * Coalesce multiple dispatches into one undo entry / one patch event.
+   *
+   * Transactional: if the callback throws, all DOM mutations applied so far
+   * are reverted (accumulated inverse patches replayed in reverse) and the
+   * override-set is restored — the model is exactly as it was at batch entry.
+   *
    * Note: a batch that produces no effective mutations still fires 'change'
    * handlers (parity with no-op dispatch) — subscribers must not assume
    * silence when wrapping speculative operations.
@@ -245,7 +252,10 @@ class CompositionImpl implements Composition {
   batch(fn: () => void, opts?: { origin?: unknown }): void {
     const origin = opts?.origin ?? ORIGIN_LOCAL;
     this.batchDepth++;
-    if (this.batchDepth === 1) this.batchOrigin = origin; // only set on outermost entry
+    if (this.batchDepth === 1) {
+      this.batchOrigin = origin; // only set on outermost entry
+      this.batchOverridesSnapshot = { ...this.overrides };
+    }
     let threw = false;
     try {
       fn();
@@ -262,24 +272,34 @@ class CompositionImpl implements Composition {
             this.batchOrigin,
             this.batchOpTypes,
           );
-          this.batchForward = [];
-          this.batchInverse = [];
-          this.batchOpTypes = [];
-          this.batchOrigin = ORIGIN_LOCAL;
+          this.resetBatchState();
           this.patchHandlers.forEach((h) => h(event));
           this.changeHandlers.forEach((h) => h());
         } else {
-          // threw OR empty: always reset; fire changeHandlers for no-op (parity with dispatch)
-          this.batchForward = [];
-          this.batchInverse = [];
-          this.batchOpTypes = [];
-          this.batchOrigin = ORIGIN_LOCAL;
+          if (threw && this.batchInverse.length > 0) {
+            // Roll back: the dispatches inside the batch already mutated the
+            // DOM. Without this, a throwing batch would leave the model in a
+            // partial state with no patch trail to undo it.
+            applyPatchesToDocument(this.parsed, [...this.batchInverse].reverse());
+            this.overrides = { ...this.batchOverridesSnapshot };
+            this.elementsCache = null;
+          }
+          this.resetBatchState();
+          // Empty no-op batch: fire changeHandlers (parity with dispatch)
           if (!threw) this.changeHandlers.forEach((h) => h());
         }
       }
     }
   }
 
+  private resetBatchState(): void {
+    this.batchForward = [];
+    this.batchInverse = [];
+    this.batchOpTypes = [];
+    this.batchOrigin = ORIGIN_LOCAL;
+    this.batchOverridesSnapshot = {};
+  }
+
   can(op: EditOp): boolean {
     return validateOp(this.parsed, op);
   }
@@ -339,9 +359,10 @@ class CompositionImpl implements Composition {
   applyPatches(patches: readonly JsonPatchOp[], opts?: { origin?: unknown }): void {
     const origin = opts?.origin ?? ORIGIN_APPLY_PATCHES;
 
-    // Compute inverse of the incoming patches (apply-then-revert = restore)
-    // For applyPatches the "inverse" is the current state before application.
-    // Emit a patch event so history and subscribers stay in sync.
+    // The emitted PatchEvent carries an EMPTY inversePatches array — hosts
+    // maintaining an external inverse log must compute inverses from their own
+    // state; applyPatches events never enter history (origin-guarded).
+    // Emit a patch event so subscribers stay in sync.
     applyPatchesToDocument(this.parsed, patches);
     this.elementsCache = null;