fix: codify space-detection commit visibility

Fixes #586

Author: mvanhorn

packages/core/src/lib/space-detection.test.ts

@@ -3,6 +3,8 @@ import { BuildingNode, CeilingNode, LevelNode, SlabNode, WallNode, ZoneNode } fr
 import type { AnyNode, AnyNodeId } from '../schema/types'
 import { resolveCeilingHeight } from '../services/level-height'
 import { getCeilingClampBound } from '../services/storey'
+import { type SceneCommit, subscribeSceneCommits } from '../store/history-control'
+import useScene, { clearSceneHistory } from '../store/use-scene'
 import {
   detectSpacesForLevel,
   initSpaceDetectionSync,
@@ -15,6 +17,16 @@ import {
 import { encodeTerrainField } from './terrain-codec'
 import { applyHeightPatch, createTerrainField, flattenPatch } from './terrain-field'
 
+type RafFn = (callback: (time: number) => void) => number
+;(globalThis as unknown as { requestAnimationFrame?: RafFn }).requestAnimationFrame ??= (
+  callback,
+) => {
+  callback(0)
+  return 0
+}
+;(globalThis as unknown as { cancelAnimationFrame?: (id: number) => void }).cancelAnimationFrame ??=
+  () => {}
+
 const square: Array<[number, number]> = [
   [0, 0],
   [4, 0],
@@ -43,6 +55,111 @@ function slab(elevation: number) {
   })
 }
 
+describe('space detection scene commit boundary', () => {
+  test('includes room reconciliation in the closing wall commit and undo step', () => {
+    const buildingId = 'building_space_commit' as AnyNodeId
+    const levelId = 'level_space_commit' as AnyNodeId
+    const walls = [
+      WallNode.parse({
+        id: 'wall_space_commit_bottom',
+        parentId: levelId,
+        start: [0, 0],
+        end: [4, 0],
+      }),
+      WallNode.parse({
+        id: 'wall_space_commit_right',
+        parentId: levelId,
+        start: [4, 0],
+        end: [4, 3],
+      }),
+      WallNode.parse({
+        id: 'wall_space_commit_top',
+        parentId: levelId,
+        start: [4, 3],
+        end: [0, 3],
+      }),
+      WallNode.parse({
+        id: 'wall_space_commit_left',
+        parentId: levelId,
+        start: [0, 3],
+        end: [0, 0],
+      }),
+    ]
+    const initialWalls = walls.slice(0, 3)
+    const building = BuildingNode.parse({
+      id: buildingId,
+      children: [levelId],
+    })
+    const level = LevelNode.parse({
+      id: levelId,
+      parentId: buildingId,
+      children: initialWalls.map((wall) => wall.id),
+      level: 0,
+      height: 2.5,
+    })
+    const initialNodes = Object.fromEntries(
+      [building, level, ...initialWalls].map((node) => [node.id, node]),
+    ) as Record<AnyNodeId, AnyNode>
+
+    useScene.setState({
+      nodes: initialNodes,
+      rootNodeIds: [buildingId],
+      dirtyNodes: new Set<AnyNodeId>(),
+      collections: {},
+      materials: {},
+      installedPlugins: [],
+      readOnly: false,
+    } as never)
+    clearSceneHistory()
+
+    const commits: SceneCommit[] = []
+    const stopDetection = initSpaceDetectionSync(useScene, createEditorStoreStub())
+    const stopCommits = subscribeSceneCommits((commit) => commits.push(commit))
+
+    try {
+      const closingWall = walls[3]!
+      useScene.getState().createNode(closingWall, levelId)
+
+      const liveNodes = useScene.getState().nodes
+      const autoSlab = Object.values(liveNodes).find(
+        (node): node is SlabNode => node.type === 'slab' && node.autoFromWalls,
+      )
+      const autoCeiling = Object.values(liveNodes).find(
+        (node): node is CeilingNode => node.type === 'ceiling' && node.autoFromWalls,
+      )
+      expect(autoSlab).toBeDefined()
+      expect(autoCeiling).toBeDefined()
+
+      const localCommits = commits.filter((commit) => commit.origin === 'local')
+      expect(localCommits).toHaveLength(1)
+      const currentNodes = localCommits[0]!.current.nodes
+      expect(Object.keys(currentNodes).sort()).toEqual(Object.keys(liveNodes).sort())
+
+      const committedLevel = currentNodes[levelId] as LevelNode
+      expect(committedLevel.children).toEqual(
+        expect.arrayContaining([closingWall.id, autoSlab!.id, autoCeiling!.id]),
+      )
+      for (const wall of walls) {
+        const committedWall = currentNodes[wall.id] as WallNode
+        expect(committedWall.frontSide).toBe('interior')
+        expect(committedWall.backSide).toBe('exterior')
+      }
+      expect(useScene.temporal.getState().pastStates).toHaveLength(1)
+
+      useScene.temporal.getState().undo()
+
+      const undoneNodes = useScene.getState().nodes
+      expect(undoneNodes[closingWall.id]).toBeUndefined()
+      expect(undoneNodes[autoSlab!.id]).toBeUndefined()
+      expect(undoneNodes[autoCeiling!.id]).toBeUndefined()
+    } finally {
+      stopCommits()
+      stopDetection()
+      clearSceneHistory()
+    }
+  })
+})
+
 describe('planAutoCeilingsForLevel', () => {
   test('creates auto ceilings height-less so they follow the level top', () => {
     const created = planAutoCeilingsForLevel([roomPolygon()], [], {

packages/core/src/lib/space-detection.ts

@@ -1633,6 +1633,11 @@ export function initSpaceDetectionSync(sceneStore: any, editorStore: any): () =>
   const previousSnapshots = levelStructureSnapshots(sceneStore.getState().nodes)
   let isProcessing = false
 
+  // Keep reconciliation in this synchronous store subscription. Zundo emits
+  // the originating local SceneCommit only after subscribers return, so the
+  // history-paused derived writes below join that commit's current snapshot
+  // and undo step. Running from subscribeSceneCommits would cross the snapshot
+  // boundary, and the paused writes would emit no replacement commit.
   const unsubscribe = sceneStore.subscribe((state: any) => {
     if (isProcessing) return
     if (getSceneHistoryPauseDepth() > 0) return

wiki/architecture/README.md

@@ -23,6 +23,7 @@ Canonical rules for code that touches `packages/core`, `packages/viewer`, `packa
 | [spatial-queries](spatial-queries.md) | Placement validation (`canPlaceOnFloor`/`Wall`/`Ceiling`) for tools |
 | [node-schemas](node-schemas.md) | Zod schema pattern for node types, `createNode`, `updateNode` |
 | [vertical-model](vertical-model.md) | Stored level heights, plane-bound wall/ceiling tops, slab placement + thickness, support hosts, clamp rules, and the load migration |
+| [space-detection](space-detection.md) | Commit and replication contract for wall-driven room reconciliation |
 | [events](events.md) | Typed event bus — emitting and listening to node and grid events |
 | [creating-rules](creating-rules.md) | How to add or update a page in this folder |
 

wiki/architecture/space-detection.md

@@ -0,0 +1,33 @@
+# Space Detection
+
+*Commit and replication contract for wall-driven room reconciliation.*
+
+Applies to: `packages/core/src/lib/space-detection.ts`, `packages/core/src/store/**`, and collaboration consumers of `SceneCommit`.
+
+Space detection derives room state from wall geometry. Reconciliation updates wall side classifications, creates or updates automatic slabs and ceilings, and updates their level's `children`. Those derived writes are part of the wall edit that triggered them, not a later background operation.
+
+## Local commit boundary
+
+`initSpaceDetectionSync` must remain a synchronous scene-store subscriber. A local wall mutation and all reconciliation it triggers must finish before zundo emits the mutation's `SceneCommit` snapshot.
+
+Reconciliation pauses scene history while applying derived writes. This keeps the triggering edit and its generated state in one undo step, while the outer tracked mutation still captures the final reconciled graph in `SceneCommit.current`. The emitted snapshot must therefore contain:
+
+- the triggering wall edit;
+- reconciled `frontSide` and `backSide` values;
+- generated or updated automatic slabs and ceilings; and
+- the corresponding level `children` updates.
+
+Do not schedule reconciliation from `subscribeSceneCommits`. Commit listeners run after the snapshot boundary. Because reconciliation writes are history-paused, moving the work there would neither amend the emitted snapshot nor produce a second local commit, leaving collaboration consumers unable to transmit the generated state.
+
+## Host patch consumption
+
+The originating client is the only client that reconciles a local wall edit and mints IDs for generated room surfaces. Collaboration transports the resulting before/current difference, including the generated nodes and parent updates.
+
+Receiving clients apply that transmitted graph as a host patch. Host application is history-paused and may run while the scene is read-only, so space detection must not regenerate the room locally. The receiver consumes the originator's slab and ceiling IDs and records no local undo entry or local commit for the host change.
+
+This two-sided contract prevents peers from independently minting different IDs for the same room:
+
+1. Local wall edit → synchronous reconciliation → one complete local commit and one undo step.
+2. Host patch → apply the transmitted generated state → no local reconciliation or local history entry.
+
+Changes to space-detection scheduling, history pausing, scene commit delivery, or host patch application must preserve both sides of this contract.