fix(viewer): route SDK isolate() through the assembly-expansion resolver (#3338)

.changeset/isolate-assembly-expansion.md

@@ -0,0 +1,7 @@
+---
+'@ifc-lite/viewer': patch
+---
+
+Fix the SDK/MCP `isolate()` call (scripts and the `viewer_isolate` tool) isolating a geometry-less `IfcElementAssembly` by its own id instead of its `IfcRelAggregates` parts, which showed an empty viewport (#3338).
+
+Assembly expansion has one shared implementation, `expandToGeometryBearingIds`, reached through `cameraCallbacks.resolveHighlightIds`. LensPanel, PropertiesPanel and both SearchModal isolate paths already route through it; `apps/viewer/src/sdk/adapters/visibility-adapter.ts`'s `isolate()` — reached by scripts and the MCP `viewer_isolate` tool — expanded spatial-structure refs (storey, building) but never routed its result through the same resolver, so isolating an assembly by ref left an id with no mesh in the isolation set. It now resolves through `cameraCallbacks.resolveHighlightIds` the same way the other channels do, falling back to the unresolved ids when no renderer has registered one yet.

apps/viewer/src/sdk/adapters/visibility-adapter.assembly.test.ts

@@ -0,0 +1,138 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
+
+import { describe, it } from 'node:test';
+import assert from 'node:assert/strict';
+import { createVisibilityAdapter } from './visibility-adapter.js';
+import type { StoreApi } from './types.js';
+import type { ViewerState } from '../../store/index.js';
+
+/**
+ * #3338: `expandToGeometryBearingIds` (assembly → geometry-bearing parts)
+ * has exactly one production call site the rest of the codebase knows
+ * about — `Viewport.tsx`'s `resolveHighlightIds`, wired into
+ * `cameraCallbacks.resolveHighlightIds` and used by LensPanel,
+ * PropertiesPanel and both SearchModal isolate paths.
+ *
+ * This SDK adapter is a FIFTH channel, not named in #3338: scripts and the
+ * MCP `viewer_isolate` tool call `ifc.isolate(refs)`, which reaches here.
+ * `isolate()` already expands a SPATIAL-structure ref (storey, building) to
+ * its contained elements (`expandSpatialRef`), but never routes the result
+ * through `cameraCallbacks.resolveHighlightIds` — so isolating a geometry-less
+ * `IfcElementAssembly` by ref isolates an id with no mesh, and the viewport
+ * shows nothing, exactly the #2532 failure mode #3338 describes, just in a
+ * channel nobody enumerated yet.
+ *
+ * `state.cameraCallbacks.resolveHighlightIds` is reachable from here — it
+ * lives on the same store `StoreApi` already reads — so nothing structural
+ * stops this adapter from using it; the only thing missing was remembering
+ * to call it, which is the exact "one call site every channel must
+ * remember to use" shape #3338 is about.
+ */
+describe('SDK visibility adapter: isolate() and #3338 assembly expansion', () => {
+  const MODEL_ID = 'm1';
+  const ASSEMBLY_EXPRESS_ID = 42;
+  const ASSEMBLY_GLOBAL_ID = 42; // idOffset 0
+  const PART_A_GLOBAL_ID = 9001;
+  const PART_B_GLOBAL_ID = 9002;
+
+  function makeStore(resolveHighlightIds?: (ids: number[]) => number[]): StoreApi {
+    const isolateEntities = (() => {
+      let calls: number[][] = [];
+      const fn = (ids: number[]) => { calls.push(ids); };
+      (fn as unknown as { calls: number[][] }).calls = calls;
+      return fn as unknown as ((ids: number[]) => void) & { calls: number[][] };
+    })();
+
+    const state = {
+      models: new Map([[MODEL_ID, {
+        id: MODEL_ID,
+        name: 'model',
+        ifcDataStore: null,
+        schemaVersion: 'IFC4',
+        fileSize: 0,
+        loadedAt: 0,
+        idOffset: 0,
+        maxExpressId: 1000,
+      }]]),
+      isolateEntities,
+      showAllInAllModels: () => {},
+      cameraCallbacks: {
+        ...(resolveHighlightIds ? { resolveHighlightIds } : {}),
+      },
+    } as unknown as ViewerState;
+
+    return {
+      getState: () => state,
+      subscribe: () => () => {},
+    };
+  }
+
+  /** Mirrors the real resolver's contract: the geometry-less assembly is
+   *  replaced by its geometry-bearing parts, never passed through as-is. */
+  const assemblyResolver = (ids: number[]) =>
+    ids.flatMap((id) => (id === ASSEMBLY_GLOBAL_ID ? [PART_A_GLOBAL_ID, PART_B_GLOBAL_ID] : [id]));
+
+  it('isolating a geometry-less assembly ref resolves to its geometry-bearing parts (RED without the fix)', () => {
+    const store = makeStore(assemblyResolver);
+    const adapter = createVisibilityAdapter(store);
+
+    adapter.isolate([{ modelId: MODEL_ID, expressId: ASSEMBLY_EXPRESS_ID }]);
+
+    const calls = (store.getState().isolateEntities as unknown as { calls: number[][] }).calls;
+    assert.equal(calls.length, 1, 'isolate() must call isolateEntities exactly once');
+    assert.deepEqual(
+      [...calls[0]].sort((a, b) => a - b),
+      // The resolved parts, unioned with the raw (pre-resolution) id — the
+      // same union every other selection channel (LensPanel, PropertiesPanel,
+      // SearchModal) performs, harmless here since the raw assembly id has
+      // no geometry of its own to draw.
+      [ASSEMBLY_GLOBAL_ID, PART_A_GLOBAL_ID, PART_B_GLOBAL_ID],
+      'isolate() must route through cameraCallbacks.resolveHighlightIds, the same aggregation ' +
+      'resolver every other selection channel (LensPanel, PropertiesPanel, SearchModal) uses, ' +
+      'instead of isolating the raw geometry-less assembly id',
+    );
+  });
+
+  it('proves the assertion is not vacuous: with no resolver wired, the un-fixed behaviour isolates the raw id', () => {
+    // Same scenario, but no cameraCallbacks.resolveHighlightIds registered
+    // (mirrors a renderer that has not mounted yet) — demonstrates what
+    // "forgetting to route through the resolver" actually looks like: the
+    // caller falls back to the unexpanded ids rather than isolating nothing.
+    const store = makeStore(undefined);
+    const adapter = createVisibilityAdapter(store);
+
+    adapter.isolate([{ modelId: MODEL_ID, expressId: ASSEMBLY_EXPRESS_ID }]);
+
+    const calls = (store.getState().isolateEntities as unknown as { calls: number[][] }).calls;
+    assert.equal(calls.length, 1);
+    assert.deepEqual(
+      calls[0],
+      [ASSEMBLY_GLOBAL_ID],
+      'without a resolver, isolate() falls back to the raw (unexpanded) id — the pre-fix shape',
+    );
+  });
+
+  it('a resolver that resolves to nothing must ALSO fall back to the raw ids, not isolate an empty set (#3338 follow-up)', () => {
+    // Viewport's resolveHighlightIds returns [] whenever geometryRef.current
+    // is null (the renderer-initialised-but-geometry-not-loaded window) or
+    // when every id resolves geometry-less. `??` only guards an ABSENT
+    // resolver, not one that runs and returns []: isolateEntities([]) hides
+    // the entire model, and it stays hidden after geometry finishes loading.
+    const emptyResolver = (_ids: number[]) => [];
+    const store = makeStore(emptyResolver);
+    const adapter = createVisibilityAdapter(store);
+
+    adapter.isolate([{ modelId: MODEL_ID, expressId: ASSEMBLY_EXPRESS_ID }]);
+
+    const calls = (store.getState().isolateEntities as unknown as { calls: number[][] }).calls;
+    assert.equal(calls.length, 1);
+    assert.deepEqual(
+      calls[0],
+      [ASSEMBLY_GLOBAL_ID],
+      'an empty resolver result must fall back to the raw ids, exactly like an absent resolver — ' +
+      'isolating [] would hide the entire model instead of the requested ref',
+    );
+  });
+});

apps/viewer/src/sdk/adapters/visibility-adapter.ts

@@ -91,7 +91,22 @@ export function createVisibilityAdapter(store: StoreApi): VisibilityBackendMetho
         }
       }
       if (globalIds.length > 0) {
-        state.isolateEntities?.(globalIds);
+        // #3338: this is a selection/isolation channel like LensPanel,
+        // PropertiesPanel and both SearchModal isolate paths, so it must
+        // resolve the same way they do — a geometry-less `IfcElementAssembly`
+        // ref (spatial expansion above leaves it untouched) has to become its
+        // geometry-bearing `IfcRelAggregates` parts, or the viewport isolates
+        // an id with nothing to render and shows an empty scene. Falling back
+        // to the unresolved ids when no renderer has registered
+        // `resolveHighlightIds` yet, OR when the resolver runs but resolves
+        // to nothing (the renderer-initialised-but-geometry-not-loaded
+        // window, or every id resolving geometry-less), matches every other
+        // channel's fallback — `??` alone only catches the former case, not
+        // the latter, and an empty isolation hides the entire model.
+        const resolved = state.cameraCallbacks.resolveHighlightIds?.(globalIds) ?? globalIds;
+        state.isolateEntities?.(
+          resolved.length > 0 ? [...new Set([...resolved, ...globalIds])] : globalIds,
+        );
       }
       return undefined;
     },