review-architecture: cover new registry capabilities (#338)
Add bullets to §4.B for surfaces that landed after the 2026-05-20 skill update: - def.capabilities.paint (PaintCapability dispatch for paint mode) - def.capabilities.floorPlaced (generic FloorElevationSystem) - def.surfaceRole (Solid/Rendered/Clay render-mode pipeline) Also add a verb-not-host-kind rule and mark capabilities.roofAccessory as documented tech debt — extending the *Accessory / *Hosted shape is now a blocker rather than a precedent. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.7
parent
814d94336b
commit
6b0fe5b99c
@@ -99,6 +99,10 @@ If the PR adds or modifies a node kind, check against `wiki/architecture/node-de
|
||||
- **Previews must clone cached materials.** If `def.preview` calls the geometry builder and then sets `material.opacity = 0.5`, but the builder caches materials at module scope (most do, keyed on `material` / `materialPreset`), the mutation leaks into every committed instance. Clone, mutate the clone, reassign `mesh.material`, dispose only the clone on unmount. Reference: `nodes/src/shelf/preview.tsx`.
|
||||
- **Host kinds need `children` on the schema.** If `def.relations.hosts` is set, the schema must declare `children: z.array(z.string()).default([])` (and `migrateNodes` must patch existing scenes). Otherwise `useScene.createNode(child, parentId)` writes a `parent.children` entry into nothing and the host never sees the new child.
|
||||
- **Movable opt-in.** `MoveTool` dispatches to `MoveRegistryNodeTool` only when `def.capabilities.movable` is set. Kinds with bespoke move semantics (wall endpoint drag with linked-wall cascade, slab vertex edit, etc.) deliberately omit `movable` and supply `def.affordanceTools.move` instead. Force-routing a bespoke-move kind through generic dispatch (`nodeRegistry.has(kind)` instead of `def.capabilities.movable`) is a regression — call it out. The bug history is documented in `plans/editor-node-registry.md` ("Capability-driven move dispatch").
|
||||
- **Paint dispatch lives on `def.capabilities.paint`.** A paintable kind declares `resolveRole` / `buildPatch` / `applyPreview` (+ optional `getEffectiveMaterial`) on `PaintCapability`; the editor's selection-manager routes hover / click / preview through the generic dispatcher. A PR that adds an `if (node.type === '<kind>')` arm to paint-mode handling, paint-preview application, or material picker resolution is a regression — the behaviour belongs on the kind's `paint` capability. See `packages/core/src/registry/types.ts` (`PaintCapability`).
|
||||
- **Floor elevation lives on `def.capabilities.floorPlaced`.** Kinds that rest on a level and lift over overlapping slabs declare a `footprint` (and optional `applies` predicate) on `FloorPlacedConfig`; the generic `<FloorElevationSystem>` writes `slabElevation + node.position[1]` onto the registered mesh on each dirty mark. A new per-kind `useEffect` / per-kind system that recomputes Y from slab overlap is a regression — the per-kind block was lifted out of `ItemSystem` in Phase 6.1. See `packages/core/src/registry/types.ts` (`FloorPlacedConfig`).
|
||||
- **Render-mode behaviour goes through `def.surfaceRole`.** Solid / Rendered / Clay viewer modes look up the kind's `surfaceRole` on `NodeDefinition` to pick the right material strategy (clay overrides, edge passes, theme overlays). New `if (node.type === '<kind>')` arms inside the render-mode pipeline or theme application are a regression — the behaviour belongs on `def.surfaceRole`. See `packages/core/src/registry/types.ts:529`.
|
||||
- **Capability names must describe verbs, not host kinds.** `movable` / `paint` / `floorPlaced` / `cuttable` / `selectable` describe *what the node does*. A new capability named after a host kind (`slabAccessory`, `wallAccessory`, `siteAccessory`, anything `Xaccessory` / `Xhosted` shaped) couples the registry's type surface to one specific host and reads as precedent for the next reviewer. Blocker. Push back: generalise into a paired *host-side* capability ("I merge subtractive accessories from my children") + *accessory-side* capability ("I provide a cut geometry, cascade my dirty mark to my host's parent"). The single existing case — `capabilities.roofAccessory` (`packages/core/src/registry/types.ts:791`, consumed by `packages/viewer/src/systems/roof/roof-system.tsx`) — is documented tech debt; do not extend the pattern.
|
||||
- **Per-kind files in legacy locations are a regression.** New `viewer/src/components/renderers/<kind>/*`, `viewer/src/systems/<kind>-system.tsx`, `editor/src/components/tools/<kind>/*`, `editor/src/components/ui/panels/<kind>-panel.tsx`, `editor/src/components/ui/helpers/<kind>-helper.tsx`, or inline `useMemo` floor-plan entry-builders inside `editor/src/components/editor/floorplan-panel.tsx` — all of these were systematically deleted at Phase 6. The behavior belongs on the kind's `NodeDefinition` (`def.renderer` / `def.system` / `def.geometry` / `def.tool` / `def.affordanceTools` / `parametrics.customPanel` / `def.toolHints` / `def.floorplan`).
|
||||
- **Floor-plan output via `def.floorplan`.** New per-kind floor-plan rendering must return `FloorplanGeometry` from `def.floorplan(node, ctx)` and be rendered by `<FloorplanRegistryLayer>`. New inline branches in `floorplan-panel.tsx` are a blocker.
|
||||
- **Plugin contract surface.** A PR that extends the v1 plugin surface — adding `plugin.materials`, `plugin.systems`, `plugin.panels`, or making plugins extend host stores (`useScene` / `useEditor` / `useViewer`) — is out of scope for the v1 contract documented in `wiki/architecture/plugin-authoring.md`. Either the change belongs as a new field on `NodeDefinition` (additive, doesn't bump `apiVersion`) or it needs its own plan.
|
||||
|
||||
Reference in New Issue
Block a user