From a30fd5f10ec3c8405381d25f14e7676027f156c1 Mon Sep 17 00:00:00 2001 From: Aymeric Rabot Date: Tue, 22 Sep 2026 12:19:11 -0400 Subject: [PATCH 1/2] fix(export): keep the buildings on a hidden Site in visible-only exports A layout whose root Site was saved with visible: false exported as an empty file: pruneHiddenSceneNodes inherited visibility up the parentId chain, so the Site's flag hid every building, level and node beneath it. The viewer never applies that flag (the Site renderer ignores it and the Site row has no eye toggle), so the editor showed the full house while the GLB held one empty scene-renderer node. The Site's own visibility now stops at the Site: hosted nodes keep their own flag, and a hidden Site drops only its ground fill and boundary. The declaredSiteParents map existed solely to cascade Site visibility to detached children, so it goes too. Co-Authored-By: Claude Fable 5.1 --- packages/editor/src/lib/glb-export.test.ts | 75 +++++++++++++++++----- packages/editor/src/lib/glb-export.ts | 39 +++++++---- 2 files changed, 85 insertions(+), 29 deletions(-) diff --git a/packages/editor/src/lib/glb-export.test.ts b/packages/editor/src/lib/glb-export.test.ts index 31ca44cb45..e0b16ed1db 100644 --- a/packages/editor/src/lib/glb-export.test.ts +++ b/packages/editor/src/lib/glb-export.test.ts @@ -590,19 +590,64 @@ describe('prepareSceneForExport', () => { expect(animations).toHaveLength(0) }) - test('inherits hidden Site visibility for detached declared children and their descendants', async () => { + test('keeps the buildings on a hidden Site and drops only the Site ground', () => { + // A layout authored outside the editor can hide the root Site (the + // renderer ignores that flag) while every node on it stays visible. The + // Site's own flag must stop at the Site: it is the parcel reference, not + // a container the building inherits visibility from. + const root = new THREE.Group() + const siteGroup = new THREE.Group() + const siteGround = meshWithNodeMaterial(nodeMaterial()) + const buildingGroup = new THREE.Group() + const levelGroup = new THREE.Group() + const itemGroup = new THREE.Group() + itemGroup.add(meshWithNodeMaterial(nodeMaterial())) + levelGroup.add(itemGroup) + buildingGroup.add(levelGroup) + siteGroup.add(buildingGroup, siteGround) + root.add(siteGroup) + + const siteId = 'site_hidden' + const buildingId = 'building_on_hidden_site' + const levelId = 'level_on_hidden_site' + const itemId = 'item_on_hidden_site' + sceneRegistry.nodes.set(siteId, siteGroup) + sceneRegistry.nodes.set(buildingId, buildingGroup) + sceneRegistry.nodes.set(levelId, levelGroup) + sceneRegistry.nodes.set(itemId, itemGroup) + const node = (id: string, type: string, parentId: string | null, visible: boolean) => + ({ object: 'node', id, type, parentId, visible }) as unknown as AnyNode + const nodes: Record = { + [siteId]: node(siteId, 'site', null, false), + [buildingId]: node(buildingId, 'building', siteId, true), + [levelId]: node(levelId, 'level', buildingId, true), + [itemId]: node(itemId, 'item', levelId, true), + } + + const { scene } = prepareSceneForExport(root, nodes) + + const meshes: THREE.Mesh[] = [] + scene.traverse((object) => { + if ((object as THREE.Mesh).isMesh) meshes.push(object as THREE.Mesh) + }) + expect(scene.getObjectByName(buildingId)).toBeDefined() + expect(scene.getObjectByName(itemId)).toBeDefined() + expect(meshes).toHaveLength(1) + expect(meshes[0]?.parent?.name).toBe(itemId) + }) + + test('keeps the nodes a hidden Site hosts, declared or detached, unless hidden themselves', async () => { const restoreRegistry = nodeRegistry._snapshot() try { - const kind = 'test:detached-site-visibility' + const kind = 'test:hidden-site-host' const childId = 'detached_site_child' const descendantId = 'detached_site_descendant' const explicitId = 'explicit_site_child' - const unownedId = 'unowned_site_child' + const hiddenChildId = 'hidden_site_child' const hiddenSite = SiteNode.parse({ visible: false, - children: [childId, explicitId], + children: [childId, explicitId, hiddenChildId], }) - const visibleSite = SiteNode.parse({ visible: true }) registerNode({ kind, schemaVersion: 1, @@ -615,17 +660,19 @@ describe('prepareSceneForExport', () => { } as AnyNodeDefinition) const nodes = { [hiddenSite.id]: hiddenSite, - [visibleSite.id]: visibleSite, [childId]: { id: childId, type: kind, parentId: null, visible: true }, [descendantId]: { id: descendantId, type: kind, parentId: childId, visible: true }, - [explicitId]: { id: explicitId, type: kind, parentId: visibleSite.id, visible: true }, - [unownedId]: { id: unownedId, type: kind, parentId: null, visible: true }, + [explicitId]: { id: explicitId, type: kind, parentId: hiddenSite.id, visible: true }, + [hiddenChildId]: { id: hiddenChildId, type: kind, parentId: hiddenSite.id, visible: false }, } as unknown as Record - const allIds = [childId, descendantId, explicitId, unownedId] + const allIds = [childId, descendantId, explicitId, hiddenChildId] const root = new THREE.Group() - for (const id of [hiddenSite.id, visibleSite.id, ...allIds]) { + const siteObject = new THREE.Group() + root.add(siteObject) + sceneRegistry.nodes.set(hiddenSite.id, siteObject) + for (const id of allIds) { const object = new THREE.Group() - root.add(object) + siteObject.add(object) sceneRegistry.nodes.set(id, object) } const exportedIds = async (onlyVisible?: boolean) => { @@ -637,12 +684,8 @@ describe('prepareSceneForExport', () => { } } - expect(await exportedIds()).toEqual([explicitId, unownedId]) + expect(await exportedIds()).toEqual([childId, descendantId, explicitId]) expect(await exportedIds(false)).toEqual(allIds) - nodes[hiddenSite.id] = { ...hiddenSite, visible: true } - expect(await exportedIds()).toEqual(allIds) - nodes[hiddenSite.id] = hiddenSite - expect(await exportedIds()).toEqual([explicitId, unownedId]) } finally { restoreRegistry() } diff --git a/packages/editor/src/lib/glb-export.ts b/packages/editor/src/lib/glb-export.ts index 5cf2c102ba..b270ec9781 100644 --- a/packages/editor/src/lib/glb-export.ts +++ b/packages/editor/src/lib/glb-export.ts @@ -715,16 +715,6 @@ function pruneHiddenSceneNodes( registryEntries: readonly RegistryEntry[], ) { const visibility = new Map() - const declaredSiteParents = new Map() - for (const node of Object.values(nodes)) { - if (node.type !== 'site' || !('children' in node) || !Array.isArray(node.children)) continue - for (const childId of node.children) { - const child = nodes[childId] - if (child && !child.parentId && !declaredSiteParents.has(childId)) { - declaredSiteParents.set(childId, node.id) - } - } - } const isVisible = (id: string, path: Set): boolean => { const cached = visibility.get(id) @@ -736,8 +726,11 @@ function pruneHiddenSceneNodes( visibility.set(id, false) return false } - const parentId = node.parentId || declaredSiteParents.get(id) - if (!parentId || path.has(id)) { + const parentId = node.parentId + // A Site is the parcel reference, not a container: its renderer ignores + // `visible`, so a hidden Site must not take the buildings on it out of the + // export the way a hidden level takes its furniture. + if (!parentId || path.has(id) || nodes[parentId]?.type === 'site') { visibility.set(id, true) return true } @@ -749,12 +742,32 @@ function pruneHiddenSceneNodes( return visible } + const nodeClones = new Set() + for (const [, original] of registryEntries) { + const clone = cloneByOriginal.get(original) + if (clone) nodeClones.add(clone) + } + for (const [id, original] of registryEntries) { if (isVisible(id, new Set())) continue - cloneByOriginal.get(original)?.removeFromParent() + const clone = cloneByOriginal.get(original) + if (!clone) continue + if (nodes[id]?.type !== 'site') { + clone.removeFromParent() + continue + } + // Drop the hidden Site's own ground fill and boundary; keep what it hosts. + for (const child of [...clone.children]) { + if (!hostsSceneNode(child, nodeClones)) child.removeFromParent() + } } } +function hostsSceneNode(object: THREE.Object3D, nodeClones: Set): boolean { + if (nodeClones.has(object)) return true + return object.children.some((child) => hostsSceneNode(child, nodeClones)) +} + function retainedClones( root: THREE.Object3D, cloneByOriginal: Map, From f4dcdc2b45e42cda4725a95de1c1e970cd7d6251 Mon Sep 17 00:00:00 2001 From: Aymeric Rabot Date: Tue, 22 Sep 2026 13:22:45 -0400 Subject: [PATCH 2/2] fix(visibility): stop a hidden Site at the Site in the 2D plan and validators The GLB fix left the same ancestor walk in two 2D paths: the shared viewer's FloorplanPreview rejected every node under a hidden Site (an empty plan for the same layout), and isFloorplanHierarchyVisible gated site-scoped plugin kinds on the Site's flag for the PDF export and the live 2D view. One rule now lives in core: hidesDescendants(node) is false only for a Site, whose flag hides its own ground fill and boundary and nothing beneath it. The exporter, the plan preview and the 2D hierarchy gate read it. validate_scene returns a warnings list and flags a hidden Site with that note; Load Build (validateBuildJson) warns the same way, so an author who hides the Site learns what the flag does. Co-Authored-By: Claude Fable 5.1 --- packages/core/src/index.ts | 1 + packages/core/src/lib/node-visibility.ts | 16 ++++++++ .../validation/validate-build-json.test.ts | 16 +++++++- .../src/validation/validate-build-json.ts | 12 ++++++ .../floorplan-registry-layer.test.ts | 39 +++++++++++++++++++ .../renderers/floorplan-registry-layer.tsx | 16 +++++++- .../floorplan-preview-visibility.test.ts | 35 +++++++++++++++++ .../viewer/floorplan-preview-visibility.ts | 19 +++++++++ .../components/viewer/floorplan-preview.tsx | 13 +------ .../lib/floorplan/floorplan-export.test.ts | 4 +- packages/editor/src/lib/glb-export.ts | 10 ++--- packages/mcp/src/bridge/scene-bridge.ts | 15 +++++-- packages/mcp/src/tools/validate-scene.test.ts | 16 ++++++++ packages/mcp/src/tools/validate-scene.ts | 17 ++++---- wiki/architecture/node-schemas.md | 2 + 15 files changed, 199 insertions(+), 32 deletions(-) create mode 100644 packages/core/src/lib/node-visibility.ts create mode 100644 packages/editor/src/components/viewer/floorplan-preview-visibility.test.ts create mode 100644 packages/editor/src/components/viewer/floorplan-preview-visibility.ts diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 7df199bbfb..8c49e3fd5e 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -126,6 +126,7 @@ export { remapMeasurementAnchors, remapMeasurementReferences, } from './lib/measurement-geometry' +export { HIDDEN_SITE_NOTE, hidesDescendants } from './lib/node-visibility' export { type Point2D as PolygonPoint2D, pointInPolygon as pointInPolygon2D, diff --git a/packages/core/src/lib/node-visibility.ts b/packages/core/src/lib/node-visibility.ts new file mode 100644 index 0000000000..bc7e994d27 --- /dev/null +++ b/packages/core/src/lib/node-visibility.ts @@ -0,0 +1,16 @@ +import type { AnyNode } from '../schema' + +/** + * Whether a node's `visible: false` also hides the nodes beneath it. + * + * A Site is the parcel reference, not a container: hiding it hides its own + * ground fill and boundary only, and the buildings on it keep their own flag. + * Every other kind hides its whole subtree. Exports, the 2D plan and the + * validators all read this one rule so a hidden Site can never empty a scene. + */ +export function hidesDescendants(node: Pick): boolean { + return node.type !== 'site' +} + +export const HIDDEN_SITE_NOTE = + 'A hidden Site hides only its own ground fill and boundary; the buildings on it stay visible in the viewport, the 2D plan and every export.' diff --git a/packages/core/src/validation/validate-build-json.test.ts b/packages/core/src/validation/validate-build-json.test.ts index 3f0488dce5..9dee2ca1c3 100644 --- a/packages/core/src/validation/validate-build-json.test.ts +++ b/packages/core/src/validation/validate-build-json.test.ts @@ -2,7 +2,7 @@ import { afterEach, beforeEach, describe, expect, test } from 'bun:test' import { z } from 'zod' import { nodeRegistry, registerNode } from '../registry' import type { AnyNodeDefinition } from '../registry/types' -import { LevelNode, WallNode } from '../schema' +import { LevelNode, SiteNode, WallNode } from '../schema' import { validateBuildJson } from './validate-build-json' function makeScene() { @@ -31,6 +31,20 @@ describe('validateBuildJson', () => { expect(result.schemaIssueCount).toBe(0) }) + test('warns that a hidden site keeps the buildings on it visible', () => { + const scene = makeScene() + const site = SiteNode.parse({ id: 'site_test', visible: false }) + const result = validateBuildJson({ + ...scene, + nodes: { ...scene.nodes, [site.id]: site }, + rootNodeIds: [site.id, ...scene.rootNodeIds], + }) + expect(result.ok).toBe(true) + const warning = result.warnings.find((w) => w.code === 'site_hidden') + expect(warning?.message).toContain('site_test') + expect(warning?.message).toContain('stay visible') + }) + test('plugin-typed children do not hard-fail their parent level', () => { // Exports from projects with plugins carry nodes like `trees:tree` // whose ids sit in level.children. The static children id union would diff --git a/packages/core/src/validation/validate-build-json.ts b/packages/core/src/validation/validate-build-json.ts index 0f9390c87f..dc58df25b9 100644 --- a/packages/core/src/validation/validate-build-json.ts +++ b/packages/core/src/validation/validate-build-json.ts @@ -1,3 +1,4 @@ +import { HIDDEN_SITE_NOTE } from '../lib/node-visibility' import { nodeRegistry } from '../registry' import type { Collection } from '../schema/collections' import { SceneMaterial } from '../schema/scene-material' @@ -267,6 +268,17 @@ export function validateBuildJson(input: unknown): ValidateBuildJsonResult { }) } + // Hand-authored files hide the Site expecting the parcel to disappear; the + // flag is accepted but reaches nothing beneath it, so say so at import. + for (const [key, value] of Object.entries(nodes)) { + if (!isPlainObject(value) || value.type !== 'site' || value.visible !== false) continue + warnings.push({ + severity: 'warning', + code: 'site_hidden', + message: `Site "${typeof value.id === 'string' ? value.id : key}" is hidden. ${HIDDEN_SITE_NOTE}`, + }) + } + // Ids of nodes whose type falls outside the static schema union — plugin // kinds (`trees:tree`) or genuinely unknown types. The scene store accepts // them on load (they already round-trip through the DB fine) and they're diff --git a/packages/editor/src/components/editor-2d/renderers/floorplan-registry-layer.test.ts b/packages/editor/src/components/editor-2d/renderers/floorplan-registry-layer.test.ts index 40af99c705..7e7d4d3f5d 100644 --- a/packages/editor/src/components/editor-2d/renderers/floorplan-registry-layer.test.ts +++ b/packages/editor/src/components/editor-2d/renderers/floorplan-registry-layer.test.ts @@ -26,6 +26,7 @@ import { floorplanEntryYieldsToTool, floorplanHandleDoubleClickAffordance, InteractiveGeometry, + isFloorplanHierarchyVisible, isFloorplanOpeningPlacementState, resolveFloorplanHandleUnitsPerPixel, siteToFloorplanTransform, @@ -790,3 +791,41 @@ describe('floorplan entry routing while a tool is active', () => { expect(floorplanEntryYieldsToTool({ mode: 'delete', openingPlacement: false })).toBe(false) }) }) + +describe('isFloorplanHierarchyVisible', () => { + const node = (id: string, type: string, parentId: string | null, visible = true) => + ({ object: 'node', id, type, parentId, visible, metadata: {} }) as unknown as AnyNode + const noOverrides = new Map() + const visibleUnder = (nodes: Record, rootId: string, id: string) => + isFloorplanHierarchyVisible(nodes[id]!, nodes, noOverrides, rootId as AnyNodeId) + + test('a hidden Site root keeps the nodes on it, linked or detached', () => { + const nodes: Record = { + site_a: node('site_a', 'site', null, false), + building_a: node('building_a', 'building', 'site_a'), + level_a: node('level_a', 'level', 'building_a'), + wall_a: node('wall_a', 'wall', 'level_a'), + wall_b: node('wall_b', 'wall', 'level_a', false), + tree_a: node('tree_a', 'trees:tree', null), + } + expect(visibleUnder(nodes, 'site_a', 'wall_a')).toBe(true) + expect(visibleUnder(nodes, 'site_a', 'tree_a')).toBe(true) + expect(visibleUnder(nodes, 'site_a', 'wall_b')).toBe(false) + expect(visibleUnder(nodes, 'site_a', 'site_a')).toBe(false) + }) + + test('a hidden building or level root still hides what it hosts', () => { + const nodes: Record = { + site_a: node('site_a', 'site', null), + building_a: node('building_a', 'building', 'site_a', false), + level_a: node('level_a', 'level', 'building_a'), + wall_a: node('wall_a', 'wall', 'level_a'), + elevator_a: node('elevator_a', 'elevator', null), + } + expect(visibleUnder(nodes, 'site_a', 'wall_a')).toBe(false) + expect(visibleUnder(nodes, 'building_a', 'elevator_a')).toBe(false) + // A level plan is scoped to its level: the walk stops at the root and + // never consults the building above it. + expect(visibleUnder(nodes, 'level_a', 'wall_a')).toBe(true) + }) +}) diff --git a/packages/editor/src/components/editor-2d/renderers/floorplan-registry-layer.tsx b/packages/editor/src/components/editor-2d/renderers/floorplan-registry-layer.tsx index 2c711dd105..87826e4e94 100644 --- a/packages/editor/src/components/editor-2d/renderers/floorplan-registry-layer.tsx +++ b/packages/editor/src/components/editor-2d/renderers/floorplan-registry-layer.tsx @@ -12,6 +12,7 @@ import { type FloorplanPoint, type FloorplanScope, type GeometryContext, + hidesDescendants, isNodeKindEnabled, isRegistryMovable, kindsWithFloorplanScope, @@ -3368,15 +3369,26 @@ export function isFloorplanHierarchyVisible( liveOverrides: Map, rootId: AnyNodeId, ): boolean { + // The root is checked on its own because a site-scoped node can be declared + // on the Site without a `parentId` link. A Site's flag never reaches the + // nodes on it; see `hidesDescendants`. const root = nodes[rootId] - if (root && !isFloorplanNodeVisible(root, liveOverrides.get(root.id))) return false + if ( + root && + root.id !== node.id && + hidesDescendants(root) && + !isFloorplanNodeVisible(root, liveOverrides.get(root.id)) + ) { + return false + } let current: AnyNode | undefined = node const seen = new Set() while (current) { if (seen.has(current.id)) return true seen.add(current.id) - if (!isFloorplanNodeVisible(current, liveOverrides.get(current.id))) return false + const reaches = current.id === node.id || hidesDescendants(current) + if (reaches && !isFloorplanNodeVisible(current, liveOverrides.get(current.id))) return false if (current.id === rootId) return true const parentId = current.parentId as AnyNodeId | null if (!parentId) return true diff --git a/packages/editor/src/components/viewer/floorplan-preview-visibility.test.ts b/packages/editor/src/components/viewer/floorplan-preview-visibility.test.ts new file mode 100644 index 0000000000..b7b25e6109 --- /dev/null +++ b/packages/editor/src/components/viewer/floorplan-preview-visibility.test.ts @@ -0,0 +1,35 @@ +import { describe, expect, test } from 'bun:test' +import type { AnyNode } from '@pascal-app/core' +import { isVisibleInFloorplan } from './floorplan-preview-visibility' + +const node = (id: string, type: string, parentId: string | null, visible = true) => + ({ object: 'node', id, type, parentId, visible, metadata: {} }) as unknown as AnyNode + +describe('isVisibleInFloorplan', () => { + test('a hidden Site keeps the buildings on it in the plan', () => { + const nodes: Record = { + site_a: node('site_a', 'site', null, false), + building_a: node('building_a', 'building', 'site_a'), + level_a: node('level_a', 'level', 'building_a'), + wall_a: node('wall_a', 'wall', 'level_a'), + wall_b: node('wall_b', 'wall', 'level_a', false), + } + expect(isVisibleInFloorplan(nodes.wall_a!, nodes)).toBe(true) + expect(isVisibleInFloorplan(nodes.wall_b!, nodes)).toBe(false) + expect(isVisibleInFloorplan(nodes.site_a!, nodes)).toBe(false) + }) + + test('a hidden building or level still hides its subtree', () => { + const nodes: Record = { + site_a: node('site_a', 'site', null), + building_a: node('building_a', 'building', 'site_a', false), + level_a: node('level_a', 'level', 'building_a'), + wall_a: node('wall_a', 'wall', 'level_a'), + building_b: node('building_b', 'building', 'site_a'), + level_b: node('level_b', 'level', 'building_b', false), + wall_b: node('wall_b', 'wall', 'level_b'), + } + expect(isVisibleInFloorplan(nodes.wall_a!, nodes)).toBe(false) + expect(isVisibleInFloorplan(nodes.wall_b!, nodes)).toBe(false) + }) +}) diff --git a/packages/editor/src/components/viewer/floorplan-preview-visibility.ts b/packages/editor/src/components/viewer/floorplan-preview-visibility.ts new file mode 100644 index 0000000000..2e894acfc4 --- /dev/null +++ b/packages/editor/src/components/viewer/floorplan-preview-visibility.ts @@ -0,0 +1,19 @@ +import { type AnyNode, hidesDescendants } from '@pascal-app/core' + +/** + * A node draws in the plan unless it, or an ancestor whose flag reaches its + * descendants, is hidden. A hidden Site keeps its buildings on the plan; see + * `hidesDescendants`. + */ +export function isVisibleInFloorplan(node: AnyNode, nodes: Record): boolean { + if (node.visible === false) return false + const seen = new Set([node.id]) + let current: AnyNode | undefined = node.parentId ? nodes[node.parentId] : undefined + while (current) { + if (seen.has(current.id)) return true + seen.add(current.id) + if (current.visible === false && hidesDescendants(current)) return false + current = current.parentId ? nodes[current.parentId] : undefined + } + return true +} diff --git a/packages/editor/src/components/viewer/floorplan-preview.tsx b/packages/editor/src/components/viewer/floorplan-preview.tsx index fc2d98e425..f7edb42e65 100644 --- a/packages/editor/src/components/viewer/floorplan-preview.tsx +++ b/packages/editor/src/components/viewer/floorplan-preview.tsx @@ -62,6 +62,7 @@ import { rotateFloorplanPoint, visibleFloorplanViewWidth, } from './floorplan-preview-navigation' +import { isVisibleInFloorplan } from './floorplan-preview-visibility' const READ_ONLY_PALETTE: FloorplanPalette = { selectedStroke: '#4f46e5', @@ -193,18 +194,6 @@ export function normalizeFloorplanPreviewNodes( return normalized } -function isVisibleInFloorplan(node: AnyNode, nodes: Record): boolean { - const seen = new Set() - let current: AnyNode | undefined = node - while (current) { - if (seen.has(current.id)) return true - seen.add(current.id) - if (current.visible === false) return false - current = current.parentId ? nodes[current.parentId] : undefined - } - return true -} - function buildFloorplanGeometries( nodes: Record, installedPlugins: readonly string[] | undefined, diff --git a/packages/editor/src/lib/floorplan/floorplan-export.test.ts b/packages/editor/src/lib/floorplan/floorplan-export.test.ts index 7f00ae63c3..e08377c9cb 100644 --- a/packages/editor/src/lib/floorplan/floorplan-export.test.ts +++ b/packages/editor/src/lib/floorplan/floorplan-export.test.ts @@ -774,7 +774,9 @@ describe('collectFloorplanGeometry', () => { 'finished-faces', [enabledPluginId], ) - expect(hiddenSite.map(({ id }) => id)).toEqual(['level_architecture']) + // A hidden Site hides only its own ground and boundary; the site-scoped + // nodes on it keep their own flag (see `hidesDescendants`). + expect(hiddenSite.map(({ id }) => id)).toEqual(['site_overlay', 'level_architecture']) const structure = collectFloorplanGeometry( nodes, diff --git a/packages/editor/src/lib/glb-export.ts b/packages/editor/src/lib/glb-export.ts index b270ec9781..5366e6111b 100644 --- a/packages/editor/src/lib/glb-export.ts +++ b/packages/editor/src/lib/glb-export.ts @@ -7,6 +7,7 @@ import { findLevelAncestorId, type GeometryContext, getLevelDisplayName, + hidesDescendants, isNodeKindEnabled, isOperationDoorType, itemClipRegistry, @@ -727,10 +728,8 @@ function pruneHiddenSceneNodes( return false } const parentId = node.parentId - // A Site is the parcel reference, not a container: its renderer ignores - // `visible`, so a hidden Site must not take the buildings on it out of the - // export the way a hidden level takes its furniture. - if (!parentId || path.has(id) || nodes[parentId]?.type === 'site') { + const parent = parentId ? nodes[parentId] : undefined + if (!parentId || path.has(id) || (parent && !hidesDescendants(parent))) { visibility.set(id, true) return true } @@ -752,7 +751,8 @@ function pruneHiddenSceneNodes( if (isVisible(id, new Set())) continue const clone = cloneByOriginal.get(original) if (!clone) continue - if (nodes[id]?.type !== 'site') { + const node = nodes[id] + if (!node || hidesDescendants(node)) { clone.removeFromParent() continue } diff --git a/packages/mcp/src/bridge/scene-bridge.ts b/packages/mcp/src/bridge/scene-bridge.ts index 32d174646d..27227af8cb 100644 --- a/packages/mcp/src/bridge/scene-bridge.ts +++ b/packages/mcp/src/bridge/scene-bridge.ts @@ -1,6 +1,6 @@ // Side-effect import MUST come first: installs RAF polyfill before core loads. import './node-shims' - +import { HIDDEN_SITE_NOTE } from '@pascal-app/core' import type { SceneGraph } from '@pascal-app/core/clone-scene-graph' import type { AnyNode } from '@pascal-app/core/schema' import { @@ -14,7 +14,12 @@ import useScene from '@pascal-app/core/store' import type { SceneMeta } from '../storage/types' export type ValidationError = { nodeId: string; path: string; message: string } -export type ValidationResult = { valid: boolean; errors: ValidationError[] } +export type ValidationResult = { + valid: boolean + errors: ValidationError[] + /** Advisories that do not fail validation, such as a hidden Site. */ + warnings: ValidationError[] +} export type CreatePatch = { op: 'create'; node: AnyNode; parentId?: AnyNodeId } export type UpdatePatch = { op: 'update'; id: AnyNodeId; data: Partial } @@ -475,8 +480,12 @@ export class SceneBridge { */ validateScene(): ValidationResult { const errors: ValidationError[] = [] + const warnings: ValidationError[] = [] const nodes = useScene.getState().nodes for (const [id, node] of Object.entries(nodes)) { + if (node.type === 'site' && node.visible === false) { + warnings.push({ nodeId: id, path: 'visible', message: HIDDEN_SITE_NOTE }) + } const res = AnyNodeSchema.safeParse(node) if (res.success) continue for (const issue of res.error.issues) { @@ -487,7 +496,7 @@ export class SceneBridge { }) } } - return { valid: errors.length === 0, errors } + return { valid: errors.length === 0, errors, warnings } } /** diff --git a/packages/mcp/src/tools/validate-scene.test.ts b/packages/mcp/src/tools/validate-scene.test.ts index aac1a3bdf0..1cede60b85 100644 --- a/packages/mcp/src/tools/validate-scene.test.ts +++ b/packages/mcp/src/tools/validate-scene.test.ts @@ -2,6 +2,7 @@ import { beforeEach, describe, expect, test } from 'bun:test' import { Client } from '@modelcontextprotocol/sdk/client/index.js' import { InMemoryTransport } from '@modelcontextprotocol/sdk/inMemory.js' import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js' +import { SiteNode } from '@pascal-app/core/schema' import { SceneBridge } from '../bridge/scene-bridge' import { registerValidateScene } from './validate-scene' @@ -29,6 +30,21 @@ describe('validate_scene', () => { const parsed = JSON.parse((result.content as Array<{ type: string; text: string }>)[0]!.text) expect(parsed.valid).toBe(true) expect(Array.isArray(parsed.errors)).toBe(true) + expect(parsed.warnings).toEqual([]) + }) + + test('warns that a hidden site keeps the buildings on it visible', async () => { + const site = SiteNode.parse({ visible: false }) + bridge.setScene({ [site.id]: site }, [site.id]) + const result = await client.callTool({ + name: 'validate_scene', + arguments: {}, + }) + const parsed = JSON.parse((result.content as Array<{ type: string; text: string }>)[0]!.text) + expect(parsed.valid).toBe(true) + expect(parsed.warnings).toHaveLength(1) + expect(parsed.warnings[0]).toMatchObject({ nodeId: site.id, path: 'visible' }) + expect(parsed.warnings[0].message).toContain('stay visible') }) test('reports structured errors', async () => { diff --git a/packages/mcp/src/tools/validate-scene.ts b/packages/mcp/src/tools/validate-scene.ts index a6f2c64df1..538cda1fff 100644 --- a/packages/mcp/src/tools/validate-scene.ts +++ b/packages/mcp/src/tools/validate-scene.ts @@ -5,15 +5,16 @@ import { READ_ONLY_TOOL_ANNOTATIONS } from './annotations' export const validateSceneInput = {} +const validationIssue = z.object({ + nodeId: z.string(), + path: z.string(), + message: z.string(), +}) + export const validateSceneOutput = { valid: z.boolean(), - errors: z.array( - z.object({ - nodeId: z.string(), - path: z.string(), - message: z.string(), - }), - ), + errors: z.array(validationIssue), + warnings: z.array(validationIssue), } export function registerValidateScene(server: McpServer, bridge: SceneOperations): void { @@ -22,7 +23,7 @@ export function registerValidateScene(server: McpServer, bridge: SceneOperations { title: 'Validate scene', description: - 'Run Zod validation against every node in the scene. Returns `{ valid, errors }` where each error has `{ nodeId, path, message }`.', + 'Run Zod validation against every node in the scene. Returns `{ valid, errors, warnings }` where each entry has `{ nodeId, path, message }`. Warnings do not fail validation; for example a hidden Site, whose flag hides only its own ground and boundary while the buildings on it stay visible and exported.', inputSchema: validateSceneInput, outputSchema: validateSceneOutput, annotations: READ_ONLY_TOOL_ANNOTATIONS, diff --git a/wiki/architecture/node-schemas.md b/wiki/architecture/node-schemas.md index 7006d77e37..a3b237ac39 100644 --- a/wiki/architecture/node-schemas.md +++ b/wiki/architecture/node-schemas.md @@ -24,6 +24,8 @@ Every node shares these fields: } ``` +`visible: false` takes the node and everything beneath it out of the 2D plan and every export. `site` is the one exception: it is the parcel reference, so hiding it hides only its own ground fill and boundary, and the buildings on it keep their own flag. The rule lives in `hidesDescendants` (`packages/core/src/lib/node-visibility.ts`); `validate_scene` and Load Build warn when a Site is hidden. + ## Defining a New Node Type ```ts