29-s3-followup: module node-data writes become undoable and batched

src/lib/coalesce.js

@@ -0,0 +1,74 @@
+// R29 S2 — A CHANGE SIGNAL THAT CANNOT RUN PER STORE TICK. Imports NOTHING (a leaf,
+// vitest-covered).
+//
+// `api.flow.onChange` / `api.game.onChange` / `api.peerVars.onChange` exist so a module
+// toolbox can stop polling. The trap the plan wrote down: a handler that runs on every
+// store tick and then reads the graph is NOT cheaper than polling — one bulk edit is
+// dozens of flowGraphs writes, and edits ARRIVING from a peer are one message (one
+// task) each, thirty of them inside a few milliseconds. So the seam coalesces to ONE
+// FRAME: however many ticks land before the next frame, the handler runs once. A
+// microtask was tried first and MEASURED insufficient — it folds a local burst but not
+// thirty arriving messages (+30 handler calls for 30 edits). And because a frame never
+// comes in a background tab, a timer races it: a module's logic must not stall because
+// the window lost focus. Outside a browser (the unit tests) it falls back to a microtask.
+//
+// Svelte's subscribe also calls back SYNCHRONOUSLY with the current value; that is not
+// a change, so it is swallowed. And a flush already queued when the subscriber is torn
+// down must not run — a disabled module's handler firing once more is exactly the
+// "dead code still acting" shape the teardown journal exists to prevent.
+
+/** the timer that stands in for a frame a hidden tab never paints */
+export const FRAME_FALLBACK_MS = 100;
+
+/** run `cb` once, on the next frame or after FRAME_FALLBACK_MS, whichever comes first
+ * @param {() => void} cb */
+export function nextFrame(cb) {
+	if (typeof requestAnimationFrame !== 'function') {
+		queueMicrotask(cb);
+		return;
+	}
+	let done = false;
+	const run = () => {
+		if (done) return;
+		done = true;
+		clearTimeout(timer);
+		cb();
+	};
+	const timer = setTimeout(run, FRAME_FALLBACK_MS);
+	requestAnimationFrame(run);
+}
+
+/**
+ * Subscribe `fn` to every store in `stores`, coalesced: at most one call per burst.
+ * @param {{subscribe: (cb: (v: any) => void) => (() => void)}[]} stores
+ * @param {() => void} fn
+ * @param {(cb: () => void) => void} [schedule] defaults to `nextFrame`
+ * @returns {() => void} unsubscribe
+ */
+export function coalescedSubscribe(stores, fn, schedule) {
+	const later = schedule ?? nextFrame;
+	let live = true;
+	let queued = false;
+	let arming = true;
+	const flush = () => {
+		queued = false;
+		if (!live) return;
+		try {
+			fn();
+		} catch (error) {
+			// a module's handler must never take a core store write down with it
+			console.warn('[module onChange] handler threw', error);
+		}
+	};
+	const poke = () => {
+		if (arming || !live || queued) return;
+		queued = true;
+		later(flush);
+	};
+	const offs = stores.map((s) => s.subscribe(poke));
+	arming = false;
+	return () => {
+		live = false;
+		for (const off of offs) off();
+	};
+}

src/lib/flowGraphs.js

@@ -206,9 +206,12 @@ registerHistoryKind('flowgraph', (entry, state) => {
 /**
  * Record an undoable flow-node mutation.
  * op 'create'/'delete' take {nodes, edges} (serialized); op 'data' takes
- * {items: [{id, before, after}]} of node-data patches.
+ * {items: [{id, before, after, graphId?}]} of node-data patches. An item's own
+ * `graphId` overrides the entry's (R29 S3: a module's group edit spans one graph per
+ * object, and it is still ONE undo step). `moduleId` attributes a module's write.
  * @param {{op: 'create'|'delete'|'data', graphId: string, nodes?: any[],
- *   edges?: any[], items?: {id: string, before: any, after: any}[]}} info
+ *   edges?: any[], items?: {id: string, before: any, after: any, graphId?: string}[],
+ *   moduleId?: string}} info
  */
 export function recordFlowNodesEntry(info) {
 	recordEntry({ kind: 'flownodes', ...info, before: 'before', after: 'after' });
@@ -220,10 +223,15 @@ registerHistoryKind('flownodes', (entry, state) => {
 	const peer = get(peers);
 	const graphId = entry.graphId;
 	if (entry.op === 'data') {
-		for (const item of entry.items ?? []) {
+		// undo walks the items BACKWARDS: a batch that writes one node twice recorded the
+		// second item's `before` AFTER the first write, so only the reverse order lands on
+		// the first item's `before` last
+		const items = entry.items ?? [];
+		for (const item of undoing ? [...items].reverse() : items) {
 			const data = undoing ? item.before : item.after;
-			updateFlowNodeData(item.id, data, graphId);
-			if (peer) peer.send({ type: 'nodedata', id: item.id, data, graphId });
+			const gid = item.graphId ?? graphId;
+			updateFlowNodeData(item.id, data, gid);
+			if (peer) peer.send({ type: 'nodedata', id: item.id, data, graphId: gid });
 		}
 		return true;
 	}

src/lib/flowLayout.js

@@ -0,0 +1,62 @@
+// R29 S1 — WHERE A BUILT GRAPH LANDS. Imports NOTHING (a leaf, vitest-covered).
+//
+// Anything that authors nodes on the user's behalf — a module recipe through
+// `api.flow.freeRegion`, the HUD editor's action bindings — has to ask one question:
+// "where can I put a block of nodes without landing on top of what is already there?"
+// `gameRecipes` answered it once (below everything), `hudActions` answered it again
+// (right of everything), and every module would have written a third copy. The rule
+// lives HERE now and nowhere else.
+//
+// It is deliberately DETERMINISTIC and cheap: a pure function of the node positions,
+// which are replicated, so two peers asking about the same graph get the same point.
+// Node SIZE is an estimate — a serialized node does not reliably carry its measured
+// box — and every card in the editor is `w-[150px]`, so width is known and height is
+// the one guess (measured when the node happens to carry it).
+
+/** every flow card is `w-[150px]` */
+export const NODE_W = 150;
+/** a typical card's height when the node carries no measured box */
+export const NODE_H = 150;
+/** clearance between the existing graph and the new block */
+export const GAP_Y = 40;
+export const GAP_X = 70;
+/** where the first block of an EMPTY graph goes */
+export const MARGIN = 40;
+
+/** @param {any} n */
+function heightOf(n) {
+	const h = Number(n?.measured?.height ?? n?.height);
+	return Number.isFinite(h) && h > 0 ? h : NODE_H;
+}
+
+/**
+ * A top-left point for a `w` x `h` block that overlaps nothing in `nodes`.
+ *
+ * `side: 'below'` (the default, the recipe rule): left-aligned with the graph's
+ * leftmost node, one gap under its lowest card. `side: 'right'` (the HUD bindings
+ * rule): one gap past the rightmost card, level with the top — with the empty graph
+ * read as a phantom card at x 0, which is what `hudActions` always did.
+ *
+ * Nothing is REJECTED: below/right of everything always has room, so `w`/`h` never
+ * change the answer today — they are part of the contract so a smarter packer can
+ * honour them without changing a caller.
+ * @param {any[]} nodes `{position: {x, y}}` (or `{x, y}`) records
+ * @param {{w?: number, h?: number, side?: 'below'|'right'}} [opts]
+ * @returns {{x: number, y: number, w: number, h: number}}
+ */
+export function freeRegion(nodes, opts = {}) {
+	const w = Math.max(0, Number(opts.w) || 0);
+	const h = Math.max(0, Number(opts.h) || 0);
+	const list = (Array.isArray(nodes) ? nodes : []).filter(Boolean);
+	const xOf = (/** @type {any} */ n) => Number(n.position?.x ?? n.x) || 0;
+	const yOf = (/** @type {any} */ n) => Number(n.position?.y ?? n.y) || 0;
+	if (opts.side === 'right') {
+		const right = list.reduce((max, n) => Math.max(max, xOf(n)), 0) + NODE_W + GAP_X;
+		const top = list.length ? Math.min(...list.map(yOf)) : MARGIN;
+		return { x: right, y: top, w, h };
+	}
+	if (!list.length) return { x: MARGIN, y: MARGIN, w, h };
+	const left = Math.min(...list.map(xOf));
+	const bottom = Math.max(...list.map((n) => yOf(n) + heightOf(n)));
+	return { x: left, y: bottom + GAP_Y, w, h };
+}

src/lib/hudActions.js

@@ -36,6 +36,8 @@ import { isInteractiveKind, isValuedKind } from './hudKinds';
 // reaches nodesHandler/flowGraphs — the history family — so moduleSDK cannot import it;
 // the leaf is where both sides can meet, the moduleToolboxes rule)
 import { moduleHudActionList } from './moduleHudKinds';
+// R29 S1: a leaf (imports nothing) — the one placement rule, shared with api.flow.freeRegion
+import { freeRegion } from './flowLayout';
 
 /** The HUD node types that READ an element (a display binding), by element kind. */
 // 21-E7.6: the PACK kinds map onto the SAME four display nodes rather than earning nodes
@@ -454,8 +456,9 @@ export function addBinding(elementId, actionKey) {
 	/** @type {any[]} */
 	const createdEdges = [];
 
-	// where to put them: past whatever is furthest right, in a column per binding
-	const baseX = nodes.reduce((max, n) => Math.max(max, Number(n.position?.x) || 0), 0) + 220;
+	// where to put them: past whatever is furthest right, in a column per binding (R29 S1:
+	// the rule is flowLayout's one copy now, shared with api.flow.freeRegion)
+	const baseX = freeRegion(nodes, { side: 'right' }).x;
 	const baseY = 40 + bindingsFor(elementId).length * 150;
 
 	if (action.role === 'value') {

src/lib/moduleSDK.js

@@ -4,7 +4,7 @@ import * as THREE from 'three';
 import { writable, get } from 'svelte/store';
 import { globalScene, objectsGroup, selectedObject, selectedObjects, globalCamera, isVRMode, isLocked } from '../stores/sceneStore';
 import { peers, showToast, modulesOpen, userdata } from '../stores/appStore';
-import { syncedAnimations, flowGraphs, flowValues, allNodes, findNodeAnyGraph, SCENE_GRAPH } from '../stores/flowStore';
+import { syncedAnimations, flowGraphs, flowValues, flowTriggers, allNodes, findNodeAnyGraph, SCENE_GRAPH } from '../stores/flowStore';
 import { customGeometryBuilders } from './customGeometries';
 // A1: moduleNodeIO imports NOTHING, so a static edge to it closes no cycle
 import {
@@ -32,8 +32,11 @@ import {
 // R3a: all three are LEAVES (svelte stores only — gameState/peerVars say so in their own
 // headers; nodesHandler reaches only flowStore + appStore), so none of these edges can
 // close the history cycle. flowGraphs/nodeCatalog are NOT leaves and stay primed below.
-import { roundCutoff, roundUnderway, gameVar, setGameVar } from './gameState';
-import { setPeerVar, myPeerVar, leaderboardRows } from './peerVars';
+import { roundCutoff, roundUnderway, gameVar, setGameVar, gameState } from './gameState';
+import { setPeerVar, myPeerVar, leaderboardRows, peerVarsMine, peerVarsRemote } from './peerVars';
+// R29 S1/S2: both leaves import NOTHING, so neither edge can close a cycle
+import { freeRegion as freeRegionIn } from './flowLayout';
+import { coalescedSubscribe } from './coalesce';
 import { createFlowNode, createFlowEdge, serializeNode, serializeEdge, setNodeData as sendNodeData } from './nodesHandler';
 import { APP_VERSION } from './version.js';
 import { ndcFromClient } from './canvasRect';
@@ -256,6 +259,35 @@ function makeApi(moduleId, moduleName = moduleId) {
 	const disposals = (moduleDisposals[moduleId] ??= []);
 	/** record an undo thunk deactivateModule runs at teardown (A2) @param {() => void} fn */
 	const onDispose = (fn) => disposals.push(fn);
+	/** A value frozen for the undo stack, so a module mutating its patch object later
+	 * cannot rewrite history. @param {any} v */
+	const frozen = (v) => {
+		try {
+			return structuredClone(v);
+		} catch {
+			return v;
+		}
+	};
+	/** One node-data write through the replicated `nodedata` path; returns the undo
+	 * item (the patched keys' previous values) or null when no node has the id.
+	 * @param {string} id @param {Record<string, any>} patch */
+	const writeNodeData = (id, patch) => {
+		const found = findNodeAnyGraph((n) => n.id === id);
+		if (!found) return null;
+		const after = frozen(patch && typeof patch === 'object' ? patch : {});
+		/** @type {Record<string, any>} */
+		const before = {};
+		for (const key of Object.keys(after)) before[key] = frozen(found.node.data?.[key]);
+		sendNodeData(id, patch && typeof patch === 'object' ? patch : {}, found.graphId);
+		return { id, graphId: found.graphId, before, after };
+	};
+	/** ONE `flownodes` data entry for these writes (a no-op until flowGraphs is primed,
+	 * which it is long before a module's UI can be pressed).
+	 * @param {{id: string, graphId: string, before: any, after: any}[]} items */
+	const recordNodeData = (items) => {
+		if (!items.length) return;
+		flowGraphsRef?.recordFlowNodesEntry({ op: 'data', graphId: items[0].graphId, items, moduleId });
+	};
 	/** input scopes this module still holds — released at teardown
 	 * @type {Set<'keys'|'locomotion'>} */
 	const claimedScopes = new Set();
@@ -965,6 +997,15 @@ function makeApi(moduleId, moduleName = moduleId) {
 			 * @param {string} name @param {number} value */
 			setVar(name, value) {
 				setGameVar(name, value);
+			},
+			/** R29 S2: `fn()` runs after the game singleton changes (state, round, a
+			 * variable) — COALESCED, at most once per frame however many writes land, never per
+			 * store tick; torn down with the module (or earlier, by calling what it returns).
+			 * Read what you need inside it. @param {() => void} fn @returns {() => void} off */
+			onChange(fn) {
+				const off = coalescedSubscribe([gameState], fn);
+				onDispose(off);
+				return off;
 			}
 		},
 		/**
@@ -987,6 +1028,14 @@ function makeApi(moduleId, moduleName = moduleId) {
 			 * @param {{order?: 'desc'|'asc'}=} opts */
 			all(name, opts) {
 				return leaderboardRows(name, opts);
+			},
+			/** R29 S2: `fn()` runs after ANY peer's row changes (mine or a remote one) —
+			 * coalesced to one call per frame, torn down with the module or by the returned
+			 * `off`. @param {() => void} fn @returns {() => void} off */
+			onChange(fn) {
+				const off = coalescedSubscribe([peerVarsMine, peerVarsRemote], fn);
+				onDispose(off);
+				return off;
 			}
 		},
 		/**
@@ -997,16 +1046,55 @@ function makeApi(moduleId, moduleName = moduleId) {
 		 */
 		flow: {
 			/** Every node (optionally one type) as plain snapshots:
-			 * `{id, type, graphId, data}` — graphId 'scene' or the owner object's uuid.
-			 * @param {string=} type @returns {any[]} */
+			 * `{id, type, graphId, x, y, data}` — graphId 'scene' or the owner object's uuid;
+			 * x/y the node's position in its graph (R29 S1, read-only — move a node in the
+			 * editor, never by writing these). @param {string=} type @returns {any[]} */
 			nodes(type) {
 				const out = [];
 				for (const n of allNodes()) {
 					if (type && n.type !== type) continue;
-					out.push({ id: n.id, type: n.type, graphId: n.__graph ?? SCENE_GRAPH, data: { ...(n.data ?? {}) } });
+					out.push({
+						id: n.id,
+						type: n.type,
+						graphId: n.__graph ?? SCENE_GRAPH,
+						x: Number(n.position?.x) || 0,
+						y: Number(n.position?.y) || 0,
+						data: { ...(n.data ?? {}) }
+					});
 				}
 				return out;
 			},
+			/**
+			 * R29 S1: where a `w` x `h` block of new nodes can land without covering what is
+			 * already in the graph — left-aligned under its lowest card (an empty graph gets
+			 * a margin from the origin). Pass the result's x/y to `addNodes`; ask again before
+			 * each block, since the answer moves as the graph grows. The rule is core's one
+			 * copy (`flowLayout.freeRegion`), shared with the HUD editor's bindings.
+			 * @param {{w?: number, h?: number, graphId?: string}=} opts
+			 * @returns {{x: number, y: number, w: number, h: number}}
+			 */
+			freeRegion(opts) {
+				const graphId = opts?.graphId ?? SCENE_GRAPH;
+				const nodes = get(flowGraphs)?.[graphId]?.nodes ?? [];
+				return freeRegionIn(nodes, { w: opts?.w, h: opts?.h });
+			},
+			/**
+			 * R29 S2: `fn()` runs after the flow graphs change — a node or edge created,
+			 * deleted, moved or re-parameterised, on this peer or arriving from one — or
+			 * after a node FIRES (the trigger log, which is what a latch, a counter or your
+			 * own `triggerStamp` read derives from, so a manager listing collected state
+			 * needs it as much as it needs the structure). It is
+			 * COALESCED to one frame: however many store writes a gesture or a stream of
+			 * arriving edits makes, the handler runs once, after them. Live VALUES (`nodeValue`) tick every frame
+			 * and deliberately do not fire it. Torn down with the module, or earlier by
+			 * calling the returned `off` (a toolbox that mounts and unmounts).
+			 * @param {() => void} fn @returns {() => void} off
+			 */
+			onChange(fn) {
+				const off = coalescedSubscribe([flowGraphs, flowTriggers], fn);
+				onDispose(off);
+				return off;
+			},
 			/** Every edge, graph-tagged: `{id, source, target, sourceHandle, targetHandle,
 			 * graphId}`. @returns {any[]} */
 			edges() {
@@ -1037,14 +1125,35 @@ function makeApi(moduleId, moduleName = moduleId) {
 				return flowRuntimeRef?.nodeTriggerStamp?.(id) ?? null;
 			},
 			/** Replicated node-data MERGE (the editor's own `nodedata` path — same message,
-			 * same merge). The manager toolbox's inline param edit. @param {string} id
+			 * same merge) that is ALSO one undo step: a `flownodes` data entry holding the
+			 * patched keys' previous values, attributed to this module (R29 S3 — before it,
+			 * a module's write left the stack untouched and the next Ctrl+Z undid whatever
+			 * came before). The manager toolbox's inline param edit. @param {string} id
 			 * @param {Record<string, any>} patch @returns {boolean} found */
 			setNodeData(id, patch) {
-				const found = findNodeAnyGraph((n) => n.id === id);
-				if (!found) return false;
-				sendNodeData(id, patch ?? {}, found.graphId);
+				const item = writeNodeData(id, patch);
+				if (!item) return false;
+				recordNodeData([item]);
 				return true;
 			},
+			/**
+			 * Many node-data writes as ONE undo step (R29 S3): a toolbox's group edit over
+			 * sixty collectibles is one Ctrl+Z, not sixty and not none. Each item is
+			 * validated exactly as `setNodeData` (an unknown id is skipped) and may live in
+			 * any graph. The wire is the ordinary per-node `nodedata` — there is no batched
+			 * type, so a peer on any build converges; the measured cost is ~0.1 ms/node.
+			 * @param {{id: string, patch: Record<string, any>}[]} list
+			 * @returns {number} how many nodes were written
+			 */
+			setNodesData(list) {
+				const items = [];
+				for (const entry of Array.isArray(list) ? list : []) {
+					const item = entry && writeNodeData(entry.id, entry.patch);
+					if (item) items.push(item);
+				}
+				recordNodeData(items);
+				return items.length;
+			},
 			/**
 			 * Create nodes (and edges) the way the editor does: replicated `nodecreate`/
 			 * `edgecreate` per item plus ONE `flownodes` undo entry for the batch — so what a