Merge pull request #495 from bilawalsidhu/refactor/layer-lifecycle-presentation

refactor: separate layer lifecycle from presentation

CHANGELOG.md

@@ -1,5 +1,7 @@
 # Changelog
 
+- Separate layer lifecycle transactions from panel construction and render/detection reactions; retain existing transition and refresh behavior.
+
 - Let CLI tools, development launchers and the setup doctor use an explicit project directory while retaining their existing default paths.
 
 - Split application scene, controls, catalog, tools and HTML into reusable components; configure application request services and sources without changing global fetch. Preserve standalone markup and voice behavior. Explicit annotation navigation may resolve a distant named target.

docs/CODE-BOUNDARIES.md

@@ -448,3 +448,16 @@ location framing use the supplied operations. Voice shares the same boundary and
 floor services, with analyst memory scoped to the runner. Direct compatibility
 entrypoints retain default services; normal assembly does not configure their
 source slots.
+
+## Layer lifecycle and presentation
+
+`data/lifecycle` owns registrations, visibility intent, refresh transactions,
+parameters and teardown. Its package group contains one module and no external
+imports. Adding a panel, renderer or application dependency fails the boundary
+build, including unused imports.
+
+`app/layerPresentation` mounts the toggle panel and turns lifecycle activity into
+render requests and detection invalidation. It owns hidden-panel refresh and
+listener cleanup. Application data assembly constructs both owners explicitly;
+`data/manager` is the compatibility facade for direct callers. The ordinary
+state subscriptions retain their existing event contract.

docs/CURRENT-STATE.md

@@ -1,5 +1,14 @@
 # God's Eye View Current State
 
+Layer lifecycle transactions are owned by `data/lifecycle`, which has no panel,
+render-governor or detection imports. Application composition mounts a separate
+layer presentation owner that reacts to status, accepted visibility/parameter
+changes and non-throwing updates. Rejected partial updates still invalidate the
+rendered detection set. The legacy manager retains panel helper compatibility;
+registration sealing, restoration order and failed-transition rollback remain
+unchanged.
+
+
 Application scene setup owns request services, terrain/floor caches and annotation
 lookup state. Controls, layers and voice share those owners, and cancellation
 clears caches and removes map-stack listeners. Standalone setup supplies the

package.json

@@ -195,6 +195,7 @@
     "./application/catalog": "./src/app/catalog.js",
     "./standalone/catalog": "./src/standalone/catalog.js",
     "./application/layers": "./src/app/constructCatalog.js",
-    "./application/operations": "./src/app/operations.js"
+    "./application/operations": "./src/app/operations.js",
+    "./data/lifecycle": "./src/data/lifecycle.js"
   }
 }

scripts/format-scope.json

@@ -586,5 +586,8 @@
   "src/services/groundFloor.js",
   "src/services/meshFloorSampler.js",
   "src/services/groundSnap.js",
-  "src/app/operations.test.mjs"
+  "src/app/operations.test.mjs",
+  "src/data/lifecycle.js",
+  "src/data/manager.js",
+  "src/app/layerPresentation.js"
 ]

scripts/package-boundaries.json

@@ -1051,16 +1051,38 @@
       "src/annotations/annotationResolver.js",
       "src/annotations/hybridAnnotationRenderer.js",
       "src/annotations/index.js",
+      "src/annotations/resolver.js",
       "src/annotations/screenAnnotationRenderer.js",
       "src/annotations/worldAnnotationRenderer.js",
+      "src/app/catalog.js",
+      "src/app/constructCatalog.js",
       "src/app/controls.js",
       "src/app/data.js",
       "src/app/errors.js",
+      "src/app/layerPresentation.js",
+      "src/app/layers/aisLiveVessels.js",
+      "src/app/layers/alprCameras.js",
+      "src/app/layers/bikeshare.js",
+      "src/app/layers/cctv.js",
+      "src/app/layers/earthquakes.js",
+      "src/app/layers/firms.js",
+      "src/app/layers/flights.js",
+      "src/app/layers/militaryAwareness.js",
+      "src/app/layers/militaryFlights.js",
+      "src/app/layers/militaryInstallations.js",
+      "src/app/layers/overlayHost.js",
+      "src/app/layers/radio.js",
+      "src/app/layers/rocketLaunches.js",
+      "src/app/layers/satellites.js",
+      "src/app/layers/submarineCables.js",
+      "src/app/layers/traffic.js",
+      "src/app/operations.js",
       "src/app/scene.js",
       "src/app/sourceSlot.js",
       "src/app/sources.js",
       "src/app/startupChrome.js",
       "src/app/stateChannel.js",
+      "src/app/surfaceServices.js",
       "src/app/tools.js",
       "src/app/viewer.js",
       "src/bloom.js",
@@ -1120,6 +1142,7 @@
       "src/data/issPass.js",
       "src/data/labelArbiter.js",
       "src/data/layerState.js",
+      "src/data/lifecycle.js",
       "src/data/localGeojson.js",
       "src/data/localGeojsonCore.js",
       "src/data/localGeojsonLod.js",
@@ -1152,6 +1175,7 @@
       "src/data/satellites.js",
       "src/data/scenePick.js",
       "src/data/spriteOrder.js",
+      "src/data/surfaceServices.js",
       "src/data/telegeographySubmarineCables.js",
       "src/data/terrainHeights.js",
       "src/data/tomtomTiles.js",
@@ -1413,13 +1437,19 @@
       "src/search/http.js",
       "src/search/placeSearch.js",
       "src/services/application.js",
+      "src/services/groundFloor.js",
+      "src/services/groundSnap.js",
+      "src/services/meshFloorSampler.js",
       "src/services/requests.js",
+      "src/services/terrainHeights.js",
       "src/sharelink.js",
       "src/sources/live/aircraft.js",
       "src/sources/live/contract.js",
       "src/sources/live/standalone.js",
       "src/sources/live/vessels.js",
       "src/splitFlap.js",
+      "src/standalone/catalog.js",
+      "src/standalone/layerSources.js",
       "src/styles/anime.js",
       "src/styles/noir.js",
       "src/styles/retro.js",
@@ -1501,35 +1531,7 @@
       "src/voice/realtimeController.js",
       "src/voice/voiceCost.js",
       "src/weatherEffectsMath.js",
-      "src/worldFocus.js",
-      "src/app/catalog.js",
-      "src/standalone/catalog.js",
-      "src/app/layers/militaryInstallations.js",
-      "src/app/layers/alprCameras.js",
-      "src/app/layers/traffic.js",
-      "src/app/layers/submarineCables.js",
-      "src/app/layers/militaryFlights.js",
-      "src/app/layers/bikeshare.js",
-      "src/app/layers/flights.js",
-      "src/app/layers/radio.js",
-      "src/app/layers/earthquakes.js",
-      "src/app/layers/cctv.js",
-      "src/app/layers/firms.js",
-      "src/app/layers/satellites.js",
-      "src/app/layers/militaryAwareness.js",
-      "src/app/layers/rocketLaunches.js",
-      "src/app/layers/overlayHost.js",
-      "src/app/layers/aisLiveVessels.js",
-      "src/app/constructCatalog.js",
-      "src/standalone/layerSources.js",
-      "src/app/surfaceServices.js",
-      "src/app/operations.js",
-      "src/data/surfaceServices.js",
-      "src/annotations/resolver.js",
-      "src/services/terrainHeights.js",
-      "src/services/groundFloor.js",
-      "src/services/meshFloorSampler.js",
-      "src/services/groundSnap.js"
+      "src/worldFocus.js"
     ],
     "external": [
       "@mapbox/vector-tile",
@@ -1904,5 +1906,10 @@
       "pbf",
       "mgrs"
     ]
+  },
+  "layer-lifecycle": {
+    "exports": ["./data/lifecycle"],
+    "modules": ["src/data/lifecycle.js"],
+    "external": []
   }
 }

src/app/data.js

@@ -1,4 +1,5 @@
-import { DataLayerManager } from '../data/manager.js';
+import { LayerLifecycle } from '../data/lifecycle.js';
+import { LayerPresentation } from './layerPresentation.js';
 /** Register the application layer catalog before allowing state restoration. */
 export function createApplicationData({
   scene: { viewer },
@@ -9,7 +10,7 @@ export function createApplicationData({
   defer,
 }) {
   // Initialize data layer manager
-  const dataManager = new DataLayerManager(viewer, {
+  const dataManager = new LayerLifecycle(viewer, {
     allowQaRegistration,
   });
   defer(async () => {
@@ -19,6 +20,8 @@ export function createApplicationData({
         `Data layers could not be destroyed: ${[...dataManager.layers.keys()].join(', ')}`,
       );
   });
+  const presentation = new LayerPresentation(dataManager);
+  defer(() => presentation.destroy());
   onData?.(dataManager);
   if (!catalog?.layers || !catalog?.metadata)
     throw new TypeError('An application layer catalog is required');
@@ -46,8 +49,8 @@ export function createApplicationData({
         delete window.__gevQaUnregisterLayer;
     });
   }
-  dataManager.buildTogglePanel(document.getElementById('data-toggles'));
+  presentation.mount(document.getElementById('data-toggles'));
   styleManager.attachDataManager(dataManager);
 
-  return { dataManager, catalog };
+  return { dataManager, catalog, presentation };
 }

src/app/layerPresentation.js

@@ -0,0 +1,85 @@
+import { LayerPanel } from '../ui/layers.js';
+import { governorRequestRender } from '../renderGovernor.js';
+import { markDetectionSourcesChanged } from '../data/detection.js';
+
+/** Own the layer panel and application reactions to lifecycle activity. */
+export class LayerPresentation {
+  constructor(
+    manager,
+    {
+      requestRender = governorRequestRender,
+      invalidateDetection = markDetectionSourcesChanged,
+    } = {},
+  ) {
+    this.manager = manager;
+    this._panel = null;
+    this.pendingVisible = false;
+    this._unsubscribe = manager.subscribeActivity((change) => {
+      if (change.type === 'status') this.refresh();
+      else if (change.type === 'destroy-all') this.destroy();
+      else {
+        const reason =
+          change.type === 'data-updated'
+            ? `layer-tick:${change.layerId}`
+            : change.type === 'visibility-settled'
+              ? 'layer-visibility'
+              : change.type === 'params-settled'
+                ? `layer-params:${change.layerId}`
+                : null;
+        if (!reason) return;
+        requestRender(reason);
+        if (change.type !== 'params-settled') invalidateDetection(reason);
+      }
+    });
+  }
+  get panel() {
+    if (!this._panel)
+      this._panel = new LayerPanel({
+        getLayers: () => this.manager.getAll(),
+        isEnabled: (id) => this.manager.isEnabled(id),
+        setEnabled: (id, enabled, options) =>
+          this.manager.setEnabled(id, enabled, options),
+        setLayerParams: (id, params, options) =>
+          this.manager.setLayerParams(id, params, options),
+        getRowControls: (id) => {
+          const module = this.manager.layers.get(id)?.module;
+          try {
+            return module?.getRowControls?.() || null;
+          } catch (error) {
+            console.warn(`[Data] ${id} getRowControls error:`, error);
+            return null;
+          }
+        },
+        hasRowControls: (id) =>
+          typeof this.manager.layers.get(id)?.module?.getRowControls ===
+          'function',
+        subscribeRowControls: (id, listener) => {
+          const module = this.manager.layers.get(id)?.module;
+          module?.setRowControlsListener?.(listener);
+          return () => module?.setRowControlsListener?.(null);
+        },
+        onHiddenRefresh: () => {
+          this.pendingVisible = true;
+        },
+      });
+    return this._panel;
+  }
+  mount(container) {
+    this.panel.mount(container);
+  }
+  refresh() {
+    this._panel?._refreshTogglePanel();
+  }
+  flushVisible() {
+    if (!this.pendingVisible) return;
+    this.pendingVisible = false;
+    this.refresh();
+  }
+  destroy() {
+    this._panel?.destroy();
+    this._panel = null;
+    this.pendingVisible = false;
+    this._unsubscribe?.();
+    this._unsubscribe = null;
+  }
+}

src/app/tools.js

@@ -69,10 +69,7 @@ export function createApplicationTools({
     viewer.useDefaultRenderLoop = !hidden;
     cockpitCloudEffects?.setSuspended?.(hidden);
     if (!hidden) {
-      if (dataManager._panelRefreshPendingOnVisible) {
-        dataManager._panelRefreshPendingOnVisible = false;
-        dataManager._refreshTogglePanel();
-      }
+      data.presentation.flushVisible();
       governorRequestRender('visibility-restore');
     }
   };

src/cockpitMarkup.test.mjs

@@ -31,7 +31,7 @@ const html = expandApplicationHtml(fs.readFileSync(path.join(ROOT, 'index.html')
 const ui = fs.readFileSync(path.join(ROOT, 'src', 'ui', 'applicationShell.js'), 'utf8');
 const css = readStylesheet(path.join(ROOT, 'style.css'));
 const sceneDirector = fs.readFileSync(path.join(ROOT, 'src', 'scenes', 'director.js'), 'utf8');
-const manager = fs.readFileSync(path.join(ROOT, 'src', 'data', 'manager.js'), 'utf8');
+const manager = fs.readFileSync(path.join(ROOT, 'src', 'data', 'lifecycle.js'), 'utf8');
 const contextLayer = readLayerSource(path.join(ROOT, 'src', 'data', 'militaryAwareness.js'), 'utf8');
 const voiceActions = fs.readFileSync(path.join(ROOT, 'src', 'voice', 'gevActions.js'), 'utf8');
 
@@ -764,7 +764,7 @@ test('Global Context uses its dedicated right rail without a duplicate Data Laye
   assert.match(contextLayer, /id:\s*'military-awareness'[\s\S]*?showInTogglePanel:\s*false/);
   const panel = fs.readFileSync(path.join(ROOT, 'src', 'ui', 'layerPanel.js'), 'utf8');
   assert.match(panel, /if \(!layer\.showInTogglePanel\) continue;/);
-  assert.match(manager, /getLayers: \(\) => this\.getAll\(\)/);
+  assert.match(fs.readFileSync(path.join(ROOT, 'src', 'app', 'layerPresentation.js'), 'utf8'), /getLayers: \(\) => this\.manager\.getAll\(\)/);
   assert.match(html, /id="global-context-panel"/);
   assert.match(html, /id="global-context-flights-btn"/);
   assert.match(html, /id="global-context-missions-btn"/);

src/data/detectionRenderDemand.test.mjs

@@ -359,14 +359,38 @@ test('a layer whose detectable set changed dirties the solve', async () => {
   assert.match(hook, /if \(_mode === MODE_OFF\) return;/,
     'and it stays inert while detection is off');
 
-  const manager = await readFile(new URL('./manager.js', import.meta.url), 'utf8');
-  assert.match(manager, /import \{ markDetectionSourcesChanged \} from '\.\/detection\.js';/);
-  // Both discrete events that can change the detectable set, next to the render
-  // request each already made.
-  assert.match(manager, /governorRequestRender\(`layer-tick:\$\{layerId\}`\);[\s\S]{0,700}?markDetectionSourcesChanged\(`layer-tick:\$\{layerId\}`\);/,
-    'a poll tick marks the solve dirty alongside its render request');
-  assert.match(manager, /governorRequestRender\('layer-visibility'\);[\s\S]{0,400}?markDetectionSourcesChanged\('layer-visibility'\);/,
-    'so does a layer appearing or disappearing');
+  const { LayerLifecycle } = await import('./lifecycle.js');
+  const { LayerPresentation } = await import('../app/layerPresentation.js');
+  const manager = new LayerLifecycle({});
+  const reactions = [];
+  const presentation = new LayerPresentation(manager, {
+    requestRender: (reason) => reactions.push(['render', reason]),
+    invalidateDetection: (reason) => reactions.push(['detection', reason]),
+  });
+  let result = true;
+  manager.register({ id: 'fixture', name: 'Fixture', updateInterval: 0,
+    init() {}, enable() {}, disable() {}, destroy() {},
+    update() { if (result instanceof Error) throw result; return result; },
+    getStats() { return { count: 1 }; },
+  });
+  await manager.setEnabled('fixture', true);
+  assert.deepEqual(reactions, [['render', 'layer-visibility'], ['detection', 'layer-visibility']]);
+  reactions.length = 0;
+  await manager.refreshLayer('fixture');
+  assert.deepEqual(reactions, [['render', 'layer-tick:fixture'], ['detection', 'layer-tick:fixture']]);
+  reactions.length = 0;
+  result = false;
+  await manager.refreshLayer('fixture');
+  assert.deepEqual(reactions, [['render', 'layer-tick:fixture'], ['detection', 'layer-tick:fixture']], 'partial/rejected updates still invalidate detection');
+  reactions.length = 0;
+  result = new Error('fixture update failure');
+  await manager.refreshLayer('fixture');
+  assert.deepEqual(reactions, [], 'throwing updates do not publish changed data');
+  result = true;
+  await manager.setEnabled('fixture', false);
+  assert.deepEqual(reactions, [['render', 'layer-visibility'], ['detection', 'layer-visibility']]);
+  presentation.destroy();
+  await manager.destroyAll();
 });
 
 test('the render-governor gate covers the parked case, with teeth on the painter', async () => {

src/tooling/packageBoundaries.test.mjs

@@ -107,3 +107,22 @@ test('a Node build export must be explicitly classified and scoped', async (t) =
   await writeFile(path.join(root, 'startup.js'), 'export const app = 2;');
   await assert.rejects(checkPackageBoundaries(root), /unowned module.*startup/);
 });
+
+test('a lifecycle owner cannot acquire a layer panel even through an unused import', async (t) => {
+  const root = await fixture(
+    t,
+    "import './panel.js'; export class LayerLifecycle {}",
+  );
+  await writeFile(path.join(root, 'panel.js'), 'export class LayerPanel {}');
+  await writeFile(
+    path.join(root, 'scripts/package-boundaries.json'),
+    JSON.stringify({
+      'layer-lifecycle': {
+        exports: ['./feature'],
+        modules: ['entry.js'],
+        external: [],
+      },
+    }),
+  );
+  await assert.rejects(checkPackageBoundaries(root), /unowned module.*panel/);
+});