Revert "PageScene: coalesce burst undo entries via UndoableAction.tryMerge"

This reverts commit 1155b12b32.

frontend/apps/artcraft/app/src/pages/PageScene/actions/setObjectColor.ts

@@ -1,17 +1,14 @@
 import type Editor from "../engine/editor";
 import { ColorAction } from "../engine/editor/actions/ColorAction";
 
-// Apply a color change and record it for undo.
+// One-shot apply: use when the change is atomic (e.g. a button click
+// or programmatic setter). Records a single ColorAction.
 //
-// Burst inputs (the native <input type="color"> dialog spams onChange
-// per slider pixel) call this on every onChange. HistoryManager.record
-// coalesces consecutive ColorActions for the same uuid into one undo
-// entry via ColorAction.tryMerge — so the caller doesn't need
-// session-state.
-//
-// Reads the before-state from the live engine object so the helper is
-// self-contained and stays correct even after undo/redo (where the
-// engine state diverges from any local React state the view may hold).
+// For interactive flows where a single user gesture fires many
+// intermediate values (the native <input type="color"> dialog spams
+// onChange per slider pixel), use beginColorSession instead — it
+// applies each intermediate value to the engine for visual feedback
+// but records only ONE ColorAction on commit.
 export function setObjectColor(
   editor: Editor,
   uuid: string,
@@ -24,3 +21,38 @@ export function setObjectColor(
   editor.activeScene.setColor(uuid, color);
   editor.history.record(new ColorAction(editor, uuid, before, color));
 }
+
+// Session-style color edit: snapshots the before-state at construction
+// (focus / picker open), applies each intermediate value to the engine
+// (visual feedback during slider drag), and records exactly one
+// ColorAction on commit (focus loss / picker close). No-op on commit
+// if the final color matches the before-state.
+export type ColorSession = {
+  apply(color: string): void;
+  commit(): void;
+};
+
+export function beginColorSession(
+  editor: Editor,
+  uuid: string,
+): ColorSession {
+  const startObj = editor.activeScene.scene.getObjectByProperty(
+    "uuid",
+    uuid,
+  );
+  const before = (startObj?.userData.color as string) ?? "#ffffff";
+  return {
+    apply(color: string) {
+      editor.activeScene.setColor(uuid, color);
+    },
+    commit() {
+      const endObj = editor.activeScene.scene.getObjectByProperty(
+        "uuid",
+        uuid,
+      );
+      const after = (endObj?.userData.color as string) ?? before;
+      if (before === after) return;
+      editor.history.record(new ColorAction(editor, uuid, before, after));
+    },
+  };
+}

frontend/apps/artcraft/app/src/pages/PageScene/actions/transformObject.ts

@@ -1,39 +1,39 @@
 import type Editor from "../engine/editor";
 import { TransformAction } from "../engine/editor/actions/TransformAction";
-import {
-  snapshotTransform,
-  transformsEqual,
-} from "../engine/editor/actions/snapshots";
-import type { XYZ } from "../datastructures/common";
 
-// Apply a transform change and record it for undo.
+// A transform "session" — opened on the first edit of a multi-input
+// editing flow (e.g. Object Panel position/rotation/scale text fields),
+// committed at the end of the session (selection change, panel
+// unmount, focus moving away from the panel).
 //
-// Burst inputs (panel keystrokes, scrub drags) call this on every
-// engine update. HistoryManager.record coalesces consecutive
-// TransformActions for the same uuid into one undo entry, so the
-// caller doesn't need session-state — `tryMerge` on TransformAction
-// folds successive entries into the most recent one.
+// The session captures the before-state at construction. On commit it
+// captures the after-state, pushes a single TransformAction onto the
+// undo stack only if the transform actually changed, and re-syncs the
+// Zustand `objectPanel` slice from the engine so the panel's view of
+// the selection matches the engine state.
 //
-// Snapshots before and after so the recorded action holds the actual
-// engine state delta (not the panel's local React state). Also
-// re-syncs the Zustand objectPanel slice so the panel's
-// currentSceneObject reflects the post-update transform.
-export function transformObject(
+// Engine-level concerns (capture / apply / revert) stay in
+// `engine/editor/actions/TransformAction.ts`; this file is the
+// orchestration wrapper that the action layer (UI code) uses so the
+// view never has to construct an UndoableAction directly.
+export type TransformSession = {
+  commit(): void;
+};
+
+export function beginTransformSession(
   editor: Editor,
   uuid: string,
-  position: XYZ,
-  rotation: XYZ,
-  scale: XYZ,
-): void {
-  const obj = editor.activeScene.scene.getObjectByProperty("uuid", uuid);
-  if (!obj) return;
-  const before = snapshotTransform(obj);
-
-  editor.sceneManager?.updateSelectedTransform(position, rotation, scale);
-
-  const after = snapshotTransform(obj);
-  if (!transformsEqual(before, after)) {
-    editor.history.record(new TransformAction(editor, uuid, before, after));
-  }
-  editor.selection.updateSelectedUI();
+): TransformSession {
+  const action = new TransformAction(editor, uuid);
+  return {
+    commit() {
+      if (action.commit()) {
+        editor.history.record(action);
+      }
+      // Push the engine's post-edit transform back into the Zustand
+      // objectPanel slice so the panel's currentSceneObject vectors
+      // are in sync with what the user just committed. Idempotent.
+      editor.selection.updateSelectedUI();
+    },
+  };
 }

frontend/apps/artcraft/app/src/pages/PageScene/comps/ControlPanelSceneObject/ControlPanelSceneObject.tsx

@@ -3,6 +3,7 @@ import {
   useContext,
   useEffect,
   useId,
+  useRef,
   useState,
 } from "react";
 import { Transition } from "@headlessui/react";
@@ -24,8 +25,14 @@ import { objectMismatch } from "~/pages/PageScene/comps/ControlPanelSceneObject/
 import { XYZ } from "~/pages/PageScene/datastructures/common";
 import { pageHeight } from "~/signals";
 import { DraggablePrecisionMutator } from "./DraggablePrecisionMutator";
-import { setObjectColor } from "~/pages/PageScene/actions/setObjectColor";
-import { transformObject } from "~/pages/PageScene/actions/transformObject";
+import {
+  beginColorSession,
+  ColorSession,
+} from "~/pages/PageScene/actions/setObjectColor";
+import {
+  beginTransformSession,
+  TransformSession,
+} from "~/pages/PageScene/actions/transformObject";
 
 // TODO this will be useful later to fix the bug on leading zeros
 // const formatNumber = (input: string): number => {
@@ -65,11 +72,34 @@ export const ControlPanelSceneObject = () => {
 
   const [color, setColor] = useState("#ffffff");
 
-  // No session machinery here — burst inputs (keystrokes, scrub drags,
-  // native color-picker per-pixel onChange) record per engine update.
-  // HistoryManager coalesces consecutive same-target same-kind actions
-  // into one undo entry via tryMerge, so each editing run collapses to
-  // a single Ctrl+Z step automatically.
+  // Pending transform session — opened on first panel edit, committed
+  // when the *selection's uuid* actually changes (NOT on every per-frame
+  // updateSelectedUI re-push that just creates a new currentSceneObject
+  // reference for the same uuid). lastUuidRef tracks the previous uuid
+  // so the commit effect can distinguish a real selection change from
+  // a same-uuid resync.
+  const transformSessionRef = useRef<TransformSession | null>(null);
+  const lastUuidRef = useRef<string | null>(null);
+
+  const beginPanelTransform = () => {
+    if (transformSessionRef.current) return;
+    if (!editorEngine || !currentSceneObject) return;
+    transformSessionRef.current = beginTransformSession(
+      editorEngine,
+      currentSceneObject.object_uuid,
+    );
+  };
+
+  const commitPanelTransform = () => {
+    transformSessionRef.current?.commit();
+    transformSessionRef.current = null;
+  };
+
+  // Color picker session — opened on input focus (picker dialog opens),
+  // committed on input blur (picker dialog closes). Records exactly one
+  // ColorAction per pick regardless of how many onChange events the
+  // native picker fires while the user drags the slider.
+  const colorSessionRef = useRef<ColorSession | null>(null);
 
   const colorInputId = useId();
 
@@ -93,32 +123,37 @@ export const ControlPanelSceneObject = () => {
   }
 
   useEffect(() => {
-    if (!inputsUpdated || !editorEngine || !currentSceneObject) {
+    if (!inputsUpdated || !editorEngine) {
       return;
     }
 
     setInputsUpdated(false);
-    transformObject(
-      editorEngine,
-      currentSceneObject.object_uuid,
+    editorEngine.sceneManager?.updateSelectedTransform(
       localToEngine(localPosition),
       localToEngine(localRotation),
       localToEngine(localScale),
     );
-  }, [
-    inputsUpdated,
-    localPosition,
-    localRotation,
-    localScale,
-    editorEngine,
-    currentSceneObject,
-  ]);
+  }, [inputsUpdated, localPosition, localRotation, localScale, editorEngine]);
 
   useEffect(() => {
     if (!editorEngine || !currentSceneObject) {
       return;
     }
 
+    // The per-frame `updateSelectedUI` in the engine pushes a fresh
+    // `currentSceneObject` object into the store every frame the
+    // selected obj's transform changes. That re-fires this effect with
+    // the *same* uuid but new vectors. Only an actual selection change
+    // (different uuid) is a real session boundary — commit the pending
+    // session for the previous uuid before we resync local state.
+    if (
+      lastUuidRef.current !== null &&
+      lastUuidRef.current !== currentSceneObject.object_uuid
+    ) {
+      commitPanelTransform();
+    }
+    lastUuidRef.current = currentSceneObject.object_uuid;
+
     const vectors = currentSceneObject.objectVectors;
 
     // local state relies on strings
@@ -128,8 +163,15 @@ export const ControlPanelSceneObject = () => {
 
     setLocked(editorEngine.selection.isObjectLocked(editorEngine?.selected?.uuid || ""));
     setColor(editorEngine?.selected?.userData.color);
+    // No cleanup function — uuid-change commit lives in the body
+    // above. Unmount commit is handled by the separate effect below.
   }, [currentSceneObject, editorEngine]);
 
+  // Final commit on panel unmount.
+  useEffect(() => {
+    return () => commitPanelTransform();
+  }, []);
+
   if (!currentSceneObject) {
     return null;
   }
@@ -149,6 +191,7 @@ export const ControlPanelSceneObject = () => {
     }
     const cleanXyz = sanitize(xyz);
     if (objectMismatch(localPosition, cleanXyz)) {
+      beginPanelTransform();
       setInputsUpdated(true);
     }
     setLocalPosition(xyz);
@@ -161,6 +204,7 @@ export const ControlPanelSceneObject = () => {
     }
     const cleanXyz = sanitize(xyz);
     if (objectMismatch(localRotation, cleanXyz)) {
+      beginPanelTransform();
       setInputsUpdated(true);
     }
     setLocalRotation(xyz);
@@ -182,6 +226,7 @@ export const ControlPanelSceneObject = () => {
     }
     const cleanXyz = sanitize(xyz);
     if (objectMismatch(localScale, cleanXyz)) {
+      beginPanelTransform();
       setInputsUpdated(true);
     }
     setLocalScale(xyz);
@@ -245,18 +290,31 @@ export const ControlPanelSceneObject = () => {
             type="color"
             value={color}
             disabled={locked}
-            onChange={(e: ChangeEvent<HTMLInputElement>) => {
-              // The native picker fires onChange per slider pixel.
-              // setObjectColor records each one as a small ColorAction;
-              // HistoryManager.record uses ColorAction.tryMerge to
-              // collapse the burst into a single undo entry.
-              const after = e.target.value;
+            onFocus={() => {
+              // Picker is opening — open a session that captures the
+              // before-state once. apply()/commit() handle visual
+              // feedback during the drag and the final undo entry on
+              // close.
               const uuid = editorEngine?.selected?.uuid;
               if (uuid && editorEngine) {
-                setObjectColor(editorEngine, uuid, after);
+                colorSessionRef.current = beginColorSession(
+                  editorEngine,
+                  uuid,
+                );
               }
+            }}
+            onChange={(e: ChangeEvent<HTMLInputElement>) => {
+              // Native color picker fires per-pixel during slider
+              // drag. apply() updates the engine for visual feedback
+              // but does NOT record. Recording happens once on blur.
+              const after = e.target.value;
+              colorSessionRef.current?.apply(after);
               setColor(after);
             }}
+            onBlur={() => {
+              colorSessionRef.current?.commit();
+              colorSessionRef.current = null;
+            }}
           />
           <Button
             className="cursor-pointer p-3.5"

frontend/apps/artcraft/app/src/pages/PageScene/engine/editor.ts

@@ -25,11 +25,6 @@ import { CameraController } from "./editor/CameraController";
 import { HistoryManager } from "./editor/HistoryManager";
 import { DeleteAction } from "./editor/actions/DeleteAction";
 import { TransformAction } from "./editor/actions/TransformAction";
-import {
-  snapshotTransform,
-  transformsEqual,
-  type TransformSnap,
-} from "./editor/actions/snapshots";
 
 import Stats from "three/examples/jsm/libs/stats.module.js";
 import { SparkRenderer } from "@sparkjsdev/spark";
@@ -91,10 +86,9 @@ class Editor {
   // engine/editor/actions/ encapsulates its own apply/revert.
   history: HistoryManager;
 
-  // Captured at gizmo dragstart so dragend can build the right
-  // TransformAction (before-snapshot + uuid). Null whenever no drag
-  // is in progress.
-  private activeDragStart: { uuid: string; before: TransformSnap } | null = null;
+  // Holds the in-flight transform action between gizmo dragstart and
+  // dragend. Null whenever no drag is in progress.
+  private activeTransform: TransformAction | null = null;
 
   // Forwarding getter — ControlPanelSceneObject reads `editor.selected`.
   get selected(): THREE.Object3D | undefined {
@@ -394,38 +388,19 @@ class Editor {
             new THREE.Vector3(-99999, -99999, -99999),
           );
           this.focused = !dragging;
-          // Gizmo drag boundary → TransformAction.
-          // Dragstart: snapshot the live obj's transform.
-          // Dragend: snapshot again; record TransformAction(before,after)
-          // if anything changed. HistoryManager.record will coalesce
-          // this with any preceding transform of the same object.
+          // Gizmo drag boundary → TransformAction. Begin captures the
+          // pre-drag transform in the constructor; end commits the diff
+          // (or drops a no-op move).
           if (dragging) {
             const target = this.sceneManager?.selected_objects?.[0];
             if (target) {
-              this.activeDragStart = {
-                uuid: target.uuid,
-                before: snapshotTransform(target),
-              };
+              this.activeTransform = new TransformAction(this, target.uuid);
             }
-          } else if (this.activeDragStart) {
-            const target = this.activeScene.scene.getObjectByProperty(
-              "uuid",
-              this.activeDragStart.uuid,
-            );
-            if (target) {
-              const after = snapshotTransform(target);
-              if (!transformsEqual(this.activeDragStart.before, after)) {
-                this.history.record(
-                  new TransformAction(
-                    this,
-                    this.activeDragStart.uuid,
-                    this.activeDragStart.before,
-                    after,
-                  ),
-                );
-              }
+          } else if (this.activeTransform) {
+            if (this.activeTransform.commit()) {
+              this.history.record(this.activeTransform);
             }
-            this.activeDragStart = null;
+            this.activeTransform = null;
           }
         },
       },

frontend/apps/artcraft/app/src/pages/PageScene/engine/editor/HistoryManager.ts

@@ -18,12 +18,6 @@ export interface UndoableAction {
   readonly label: string;
   apply(): Promise<void> | void; // do or redo
   revert(): Promise<void> | void; // undo
-  // Optional: try to absorb a freshly-recorded same-kind same-target
-  // action into this one. Used to coalesce burst inputs (panel input
-  // keystrokes / scrub drags / native color-picker per-pixel onChange)
-  // into a single undo entry. Return true if absorbed (caller drops
-  // the new action); false to push the new action as a separate entry.
-  tryMerge?(other: UndoableAction): boolean;
 }
 
 export interface HistoryManagerOptions {
@@ -47,16 +41,6 @@ export class HistoryManager {
 
   record(action: UndoableAction): void {
     if (this.isReplaying) return;
-    // Try to coalesce into the most-recent action so burst inputs
-    // (panel keystrokes, scrub drags, color-picker per-pixel onChange)
-    // become one undo entry. Action classes opt in by implementing
-    // tryMerge; the manager itself stays generic.
-    const prev = this.past[this.past.length - 1];
-    if (prev?.tryMerge?.(action)) {
-      this.future.length = 0;
-      this.notifyChange();
-      return;
-    }
     this.future.length = 0;
     this.past.push(action);
     if (this.past.length > this.capacity) this.past.shift();

frontend/apps/artcraft/app/src/pages/PageScene/engine/editor/actions/ColorAction.ts

@@ -2,20 +2,16 @@ import type Editor from "../../editor";
 import type { UndoableAction } from "../HistoryManager";
 
 // Records a color change. apply() and revert() both call
-// activeScene.setColor — the engine handles material magic.
-//
-// Coalescing: tryMerge absorbs a follow-up ColorAction whose `before`
-// matches this one's `after`, so the native <input type="color">
-// per-pixel onChange spam (and any other burst color source) collapses
-// into one undo entry without session machinery on the caller.
+// activeScene.setColor — the engine handles material magic. The action
+// captures the uuid + before/after hex pair at construction time.
 export class ColorAction implements UndoableAction {
   readonly label = "Color";
 
   constructor(
     private readonly editor: Editor,
-    public readonly uuid: string,
+    private readonly uuid: string,
     private readonly before: string,
-    private after: string,
+    private readonly after: string,
   ) {}
 
   apply(): void {
@@ -25,12 +21,4 @@ export class ColorAction implements UndoableAction {
   revert(): void {
     this.editor.activeScene.setColor(this.uuid, this.before);
   }
-
-  tryMerge(other: UndoableAction): boolean {
-    if (!(other instanceof ColorAction)) return false;
-    if (other.uuid !== this.uuid) return false;
-    if (this.after !== other.before) return false;
-    this.after = other.after;
-    return true;
-  }
 }

frontend/apps/artcraft/app/src/pages/PageScene/engine/editor/actions/TransformAction.ts

@@ -2,48 +2,57 @@ import type Editor from "../../editor";
 import type { UndoableAction } from "../HistoryManager";
 import {
   TransformSnap,
+  snapshotTransform,
   transformsEqual,
   writeTransform,
 } from "./snapshots";
 
-// Records a transform change. Constructor takes the before/after
-// snapshots directly; callers are responsible for snapshotting the
-// live object before the engine write and after.
+// Records a transform change. Caller holds the instance between the
+// drag/edit boundary callbacks (gizmo dragstart/dragend, panel input
+// focus/blur) and calls commit() at the end. If the transform didn't
+// actually change, commit() returns false and the caller drops the
+// action without recording.
 //
-// Coalescing: tryMerge absorbs a follow-up TransformAction whose
-// `before` matches this one's `after` (chain continuity), extending
-// `after` to the new endpoint. This collapses panel keystroke /
-// scrub-drag / gizmo-drag bursts into a single undo entry without
-// any session-state on the caller — HistoryManager.record handles it.
+// Begin/end transaction state lives on the action itself — no need for
+// a per-manager pendingTransform field.
 export class TransformAction implements UndoableAction {
   readonly label = "Transform";
-  private before: TransformSnap;
-  private after: TransformSnap;
+  private readonly before: TransformSnap;
+  private after?: TransformSnap;
 
   constructor(
     private readonly editor: Editor,
-    public readonly uuid: string,
-    before: TransformSnap,
-    after: TransformSnap,
+    private readonly uuid: string,
   ) {
-    this.before = before;
-    this.after = after;
+    const obj = editor.activeScene.scene.getObjectByProperty("uuid", uuid);
+    if (!obj) {
+      this.before = {
+        position: { x: 0, y: 0, z: 0 },
+        rotation: { x: 0, y: 0, z: 0 },
+        scale: { x: 1, y: 1, z: 1 },
+      };
+      return;
+    }
+    this.before = snapshotTransform(obj);
+  }
+
+  // Capture the after-state from the live object. Returns true if it
+  // differs from the before-state (worth recording).
+  commit(): boolean {
+    const obj = this.editor.activeScene.scene.getObjectByProperty(
+      "uuid",
+      this.uuid,
+    );
+    if (!obj) return false;
+    this.after = snapshotTransform(obj);
+    return !transformsEqual(this.before, this.after);
   }
 
   apply(): void {
-    writeTransform(this.editor, this.uuid, this.after);
+    if (this.after) writeTransform(this.editor, this.uuid, this.after);
   }
 
   revert(): void {
     writeTransform(this.editor, this.uuid, this.before);
   }
-
-  tryMerge(other: UndoableAction): boolean {
-    if (!(other instanceof TransformAction)) return false;
-    if (other.uuid !== this.uuid) return false;
-    // Continuation: the new edit must start where our current end is.
-    if (!transformsEqual(this.after, other.before)) return false;
-    this.after = other.after;
-    return true;
-  }
 }