docs(architecture): codify live-overrides drag protocol + floorplan per-node perf

Capture the durable patterns from the placement-interaction overhaul so reviews
and new devs don't regress them:

- tools.md: new "Data-driven live drag" section — kinds whose geometry is
  recomputed from fields (wall/opening/endpoint) preview via useLiveNodeOverrides
  (merged by getEffectiveWall/getEffectiveNode), store written once on commit;
  per-tick useScene.updateNodes is the documented anti-pattern (churns the nodes
  ref → app-wide re-render flood). Plus "Floorplan registry: per-node
  subscriptions" — each entry subscribes to its own live slice, memo'd with
  stable props, sibling-epoch invalidation; widening to the whole Map / dropping
  memo is a regression. Plus a note that the HUD snapping chip renders for any
  snap-context tool, not only those with def.toolHints.
- review SKILL.md: matching blockers in §C (data-driven drag / no per-tick store
  write) and §D (per-node list subscriptions).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Wassim SAMAD
2026-06-27 21:19:39 -04:00
co-authored by Claude Opus 4.8
parent dc468a084b
commit a2f1ef1683
2 changed files with 21 additions and 2 deletions
@@ -120,6 +120,7 @@ If the PR adds or modifies a node kind, check against `wiki/architecture/node-de
- New state added to `useViewer` must be presentation-only (selection, camera, level mode, display toggles). Editor-only state (active tool, phase, edit mode, paint preview, floorplan state) goes in `useEditor`.
- **Node code does not import `useScene` directly.** A kind's geometry / system / tool should read and write through `SceneApi` (passed in by the framework) or `GeometryContext`. Direct `useScene.getState()` calls inside `packages/nodes/src/<kind>/` are a smell — they bypass the registry's IoC point and make the code harder to test.
- **Live drag motion is imperative, not store-driven.** Tools must not call `useLiveTransforms.set(...)` per `grid:move` tick to animate registered parametric kinds — the selector path doesn't reliably re-render and the mesh visibly disappears mid-drag. Use `sceneRegistry.nodes.get(node.id)?.position.set(x, y, z)` instead, and commit once at the end via `useScene.temporal.getState().resume() → updateNode → pause()`. The reference implementation is `MoveRegistryNodeTool`. This is the *only* sanctioned use of imperative mesh transforms by a tool; flag any other location that does the same.
- **Data-driven drags preview via `useLiveNodeOverrides`, never per-tick `useScene`.** A kind whose geometry is recomputed from data fields (wall `start`/`end`, opening host-cut, endpoint reshape) previews by publishing field patches to `useLiveNodeOverrides` (merged by `getEffectiveWall` / `getEffectiveNode`), writing the scene store **once on commit**. A tool that calls `useScene.updateNodes`/`updateNode` on `grid:move` (or any per-pointer-move tick) is a **blocker** — it swaps the `nodes` map ref and re-renders every `useScene(s => s.nodes)` subscriber app-wide each frame (`markDirty` per tick is fine). Grep tell: `updateNode(s)?(` in an `onGridMove`/`onMove`/`applyPreview` path under `packages/nodes/src/<kind>/`. See `wiki/architecture/tools.md` § "Data-driven live drag".
### D. Selector performance
@@ -127,6 +128,7 @@ If the PR adds or modifies a node kind, check against `wiki/architecture/node-de
- Selectors that return new object or array references each call (e.g. `s => ({ a: s.a, b: s.b })`, `s => s.items.filter(...)`) without a custom equality function (shallow or custom) are re-render hazards.
- Prefer subscribing by ID deep in the tree (one node per renderer) over subscribing to the full collection high up.
- Inside a `<XxxPanel>` (legacy or `parametrics.customPanel`-mounted), avoid `useScene(s => s.nodes[selectedId])` as a callback dep — it changes every tick and pushes `useCallback` into infinite-loop territory. The recipe is in `plans/editor-node-registry.md` under "Panel slider-drag fix recipe".
- **Per-node list renderers subscribe per-node, not to the whole live Map.** A list that draws one child per node (`FloorplanRegistryLayer``FloorplanRegistryEntry`) must have each child subscribe to its **own** slice (`useLiveTransforms(s => s.transforms.get(id))` / `overrides.get(id)`) and be `memo`'d with referentially stable props; the parent subscribes only to the stable id list. Subscribing the parent or a child to the whole `transforms`/`overrides` Map, dropping a `memo`, or passing unstable props re-renders all N children every drag tick — a flood that type-checks and passes tests. Sibling invalidation goes through a per-node epoch, not a whole-layer re-render. See `wiki/architecture/tools.md` § "Floorplan registry: per-node subscriptions".
### E. Separation of concerns
+19 -2
View File
@@ -83,8 +83,11 @@ export function MyTool() {
`modifiers.shiftKey` to bypass snapping, and never apply a grid step that isn't gated on
`isGridSnapActive()` (`const step = isGridSnapActive() ? gridSnapStep : 0`). A snappable kind declares
`NodeDefinition.snapProfile` (`'item' | 'structural'`) so its context, mode-set, and chip fall out
with no per-kind switch. See [interaction-scope](interaction-scope.md) § "Snapping mode & modifiers"
and `lib/snapping-mode.ts`.
with no per-kind switch. The contextual HUD renders the snapping chip for **any** tool that resolves
to a snap context — `helper-manager` gates the generic `RegisteredToolHelper` on `snapContext` (or
`continuationContext`), not on the presence of hand-written `def.toolHints`, so a snappable draft tool
with no bespoke hints (e.g. `zone`) still advertises the Shift = cycle control it already honors. See
[interaction-scope](interaction-scope.md) § "Snapping mode & modifiers" and `lib/snapping-mode.ts`.
- **Constraints and guides can be decoupled.** When a stronger constraint owns the proposal —
a wall segment's 45° lock while in `angles` mode — the tool may still publish passive dashed
alignment/proximity guides as long as it does not apply the guide snap delta. Use this for chained
@@ -151,6 +154,20 @@ The store name suggests a uniform contract; the writes in practice are not. Docu
Anything that subscribes to `useLiveTransforms` to inform 2D rendering needs to handle these frames explicitly. The `FloorplanRegistryLayer` override currently branches by kind: `item` / `shelf` / `column` are treated as world-plan (it copies `live.position` onto the effective node and forces `parentId: null` so the resolver skips the parent-chain transform), while `slab` / `ceiling` / `zone` are treated as a polygon **delta** (it translates the polygon vertices by `live.position`). Each kind added to the live-drag path grows this consumer-side switch; the preferred long-term fix is to standardise the frame at the writer so the consumer stops branching by `node.type`.
## Data-driven live drag: `useLiveNodeOverrides`, never per-tick `useScene`
`useLiveTransforms` (above) carries a rigid position/rotation offset — right when the renderer can preview the move by transforming the node's group. It's **wrong** when the geometry is *recomputed from data fields* (a wall re-miters from its `start`/`end`, an opening re-cuts its host wall, an endpoint drag reshapes the segment and cascades to linked walls): the shape itself changes, so there's no rigid offset to apply. Those preview via **`useLiveNodeOverrides`** (`@pascal-app/core`) — the tool publishes the changed fields per tick (`set(id, patch)` / `setMany(...)`) and the geometry systems merge them (`getEffectiveWall` in 3D, the floor-plan sibling-override merge in 2D, `getEffectiveNode` in panels). The scene store stays untouched during the drag; on commit the tool clears overrides and writes it **once** (`resumeSceneHistory → updateNodes([...]) → pauseSceneHistory`), so the gesture is a single undo step. Esc/unmount just clears overrides — cancel is free.
**Writing `useScene.updateNodes`/`updateNode` per `grid:move` tick is a blocker:** it replaces the `nodes` map ref, so every `useScene(s => s.nodes)` subscriber app-wide (panels, HUD, tooltips, floor plan, catalog) re-renders each frame → FPS collapse. (`markDirty` per tick is fine — it never calls `set()`.) Reference: `packages/nodes/src/wall/{move-tool,move-endpoint-tool}.tsx`.
## Floorplan registry: per-node subscriptions, stable props
`FloorplanRegistryLayer` draws one `FloorplanRegistryEntry` per node. The perf invariant — a live drag must re-render only the changed node(s), not all ~150 entries — rests on three things, and breaking any of them is a re-render-flood regression that still type-checks and passes tests (see `floorplan-registry-layer.tsx`):
- Each entry subscribes to **its own slice**`useLiveTransforms(s => s.transforms.get(id))` / `useLiveNodeOverrides(s => s.overrides.get(id))`, never the whole Map. This works because the live stores write a fresh value only for the changed node (the Map is cloned but unchanged value refs are reused), so an unchanged node's selector stays identity-stable and Zustand skips it. The parent subscribes only to the stable id list.
- `FloorplanRegistryEntry` and `InteractiveGeometry` are `memo`'d, so the parent must pass **referentially stable props** (hoisted styles, `useCallback` handlers, memoized descriptors) — a fresh inline object/handler per entry defeats the memo.
- Sibling-dependent geometry (wall miters, opening cuts) invalidates via a **per-node sibling epoch** bumped from a store `subscribe` (`computeAffectedSiblingIds`), not a whole-layer re-render.
## Wall-attached node rotations must be wall-local
`door` / `window` / wall-attached `item` are children of the wall mesh in 3D. The wall's `mesh.rotation.y = -atan2(dy, dx)`. The child node's `rotation.y` therefore lives in the wall's local frame and composes with the wall's rotation at render time.