fix(street-level): own the framing flight and the nearest-photo lookup

From the IC8 review of #768, each with tests that fail before:

- P1: closing a photo, switching the layer off or destroying it stops the
  1.6 s framing flight toward that photo. The flight is claimed with a
  token before flyToBoundingSphere and released by its complete/cancel
  callbacks, so cancelFraming() calls camera.cancelFlight() only while
  the flight is still Street Level's: a newer navigation flight (a search
  result, a scene) is never cancelled. Turning FOLLOW on also stops it, so
  the tween cannot overwrite the follow view.
- P2: an older nearest-photo lookup can no longer replace a newer choice.
  The core owns one AbortController per lookup, aborted by a newer lookup,
  a direct open or cone click, closing the viewer, disable and destroy;
  the signal reaches source.nearestImages through the provider, and an
  aborted lookup never surfaces as an error.

Co-Authored-By: Claude Opus 5.5 <[email protected]>

src/layers/streetLevel/cameraFollow.js

@@ -70,11 +70,26 @@ export function createCameraFollow({ state, parts }) {
     requestRender();
   }
 
+  /**
+   * The framing flight this module started, while it is still the camera's
+   * current flight. Cesium cancels it (calling `cancel`) when any other
+   * flight starts, so a stale token never cancels someone else's flight.
+   * @type {object|null}
+   */
+  let framing = null;
+
   /** Frame the current image from a short distance behind it. */
   function lookAtPosition() {
     const { position, bearing, altitude } = state.street;
     if (!position || !state.viewer) return;
     const ground = groundHeightAt(position.lon, position.lat, altitude);
+    // Owned before the call: starting it cancels the previous flight (ours
+    // included) synchronously, and a zero-length one completes at once.
+    const flight = {};
+    framing = flight;
+    const release = () => {
+      if (framing === flight) framing = null;
+    };
     state.viewer.camera.flyToBoundingSphere(
       new Cesium.BoundingSphere(
         Cesium.Cartesian3.fromDegrees(position.lon, position.lat, ground + 2),
@@ -87,15 +102,32 @@ export function createCameraFollow({ state, parts }) {
           140,
         ),
         duration: 1.6,
+        complete: release,
+        cancel: release,
       },
     );
   }
 
+  /**
+   * Stop the framing flight if it is still in the air (the image closed or
+   * the layer went off). A flight another feature started since is left be.
+   */
+  function cancelFraming() {
+    if (!framing) return;
+    // Give up ownership first: cancelFlight delivers `cancel` synchronously.
+    framing = null;
+    state.viewer?.camera?.cancelFlight?.();
+  }
+
   function setFollow(enabled) {
     state.street.follow = enabled === true && state.street.followAvailable;
-    if (state.street.follow) followCamera();
+    // The framing tween would keep overwriting the follow view until it lands.
+    if (state.street.follow) {
+      cancelFraming();
+      followCamera();
+    }
     state.notify?.();
   }
 
-  return { followCamera, lookAtPosition, setFollow };
+  return { followCamera, lookAtPosition, cancelFraming, setFollow };
 }

src/layers/streetLevel/cameraFollow.test.mjs

@@ -8,6 +8,40 @@ import { FOLLOW_EYE_HEIGHT_M } from './policy.js';
 const heightOf = (cartesian) =>
   Cesium.Cartographic.fromCartesian(cartesian).height;
 
+/**
+ * A camera with Cesium's flight bookkeeping: starting a flight cancels the
+ * current one (calling its `cancel`), `cancelFlight` cancels whichever flight
+ * is current, whoever started it, and `land()` completes it. `cancelled`
+ * counts the flights `cancelFlight` stopped.
+ */
+function flightCamera(flights, views) {
+  let current = null;
+  function stop() {
+    const flight = current;
+    current = null;
+    flight?.options.cancel?.();
+    return Boolean(flight);
+  }
+  const camera = {
+    cancelled: 0,
+    flyToBoundingSphere(sphere, options = {}) {
+      stop();
+      current = { sphere, options };
+      flights.push(current);
+    },
+    cancelFlight() {
+      if (stop()) camera.cancelled++;
+    },
+    land() {
+      const flight = current;
+      current = null;
+      flight?.options.complete?.();
+    },
+    setView: (view) => views.push(view),
+  };
+  return camera;
+}
+
 /**
  * A viewer whose scene sample is `sampled` (the Austin case: −14,886 m before
  * the tiles under the photo had loaded) and whose bare earth is `dem`.
@@ -31,11 +65,7 @@ function setup({ sampled, dem = null, globe = null, altitude = 149 }) {
         sampleHeight: () => sampled,
         globe: { getHeight: () => globe },
       },
-      camera: {
-        flyToBoundingSphere: (sphere, options) =>
-          flights.push({ sphere, options }),
-        setView: (view) => views.push(view),
-      },
+      camera: flightCamera(flights, views),
     },
   };
   const parts = {
@@ -87,3 +117,44 @@ test('following stands the camera at eye height above the checked ground', () =>
       0.01,
   );
 });
+
+test('cancelFraming stops the framing flight while it is still ours (review IC8 P1)', () => {
+  const { follow, state } = setup({ sampled: 121, dem: 117 });
+  const { camera } = state.viewer;
+  follow.lookAtPosition();
+  follow.cancelFraming();
+  assert.equal(camera.cancelled, 1, 'the flight toward the closed photo stops');
+  follow.cancelFraming();
+  assert.equal(camera.cancelled, 1, 'and only once');
+
+  // Re-framing (the next photo) supersedes the first flight, not the claim.
+  follow.lookAtPosition();
+  follow.lookAtPosition();
+  follow.cancelFraming();
+  assert.equal(camera.cancelled, 2);
+});
+
+test('cancelFraming leaves a landed flight and a newer navigation flight alone (review IC8 P1)', () => {
+  const { follow, state, flights } = setup({ sampled: 121, dem: 117 });
+  const { camera } = state.viewer;
+  follow.lookAtPosition();
+  camera.land();
+  follow.cancelFraming();
+  assert.equal(camera.cancelled, 0, 'landed: nothing to stop');
+
+  follow.lookAtPosition();
+  // A search result flies the globe elsewhere; Cesium cancels ours first.
+  camera.flyToBoundingSphere(null, {});
+  assert.equal(flights.length, 3);
+  follow.cancelFraming();
+  assert.equal(camera.cancelled, 0, 'the newer flight keeps going');
+});
+
+test('turning FOLLOW on stops the framing flight so it cannot fight the follow view', () => {
+  const { follow, state, views } = setup({ sampled: 121, dem: 117 });
+  const { camera } = state.viewer;
+  follow.lookAtPosition();
+  follow.setFollow(true);
+  assert.equal(camera.cancelled, 1, 'the framing tween stops');
+  assert.equal(views.length, 1, 'and the camera stands at the photo');
+});

src/layers/streetLevel/index.js

@@ -239,7 +239,21 @@ export function createStreetLevelLayer({
     parts.credits.hide(state.viewer, entry.def);
   }
 
+  /**
+   * The nearest-image lookup in flight. Anything newer the user asks for (a
+   * later lookup, a picked image, closing, the layer going off) aborts it,
+   * so its late answer can never replace that choice.
+   * @type {AbortController|null}
+   */
+  let nearestLookup = null;
+
+  function abortNearest() {
+    nearestLookup?.abort();
+    nearestLookup = null;
+  }
+
   function openImage(providerId, imageId) {
+    abortNearest();
     return parts.viewerHost.open(providerId, imageId);
   }
 
@@ -358,6 +372,7 @@ export function createStreetLevelLayer({
 
     disable() {
       state.enabled = false;
+      abortNearest();
       unwatchSurface();
       parts.viewerHost.unmount();
       for (const entry of state.providers.values()) entry.instance.deactivate();
@@ -461,6 +476,10 @@ export function createStreetLevelLayer({
       const view = point || viewCentre(state.viewer);
       if (!Number.isFinite(view?.lat) || !Number.isFinite(view?.lon))
         return false;
+      abortNearest();
+      const lookup = new AbortController();
+      nearestLookup = lookup;
+      const { signal } = lookup;
       state.street.loading = true;
       state.street.error = null;
       notify();
@@ -468,20 +487,25 @@ export function createStreetLevelLayer({
       for (const entry of activeEntries()) {
         let imageId = null;
         try {
-          imageId = await entry.instance.nearestImage({
-            lat: view.lat,
-            lon: view.lon,
-          });
+          imageId = await entry.instance.nearestImage(
+            { lat: view.lat, lon: view.lon },
+            { signal },
+          );
         } catch (error) {
+          // Overtaken: whatever replaced it owns the panel, errors included.
+          if (signal.aborted) return false;
           // One provider failing (no key, offline) must not hide the others.
           lastError = error;
           continue;
         }
+        if (signal.aborted) return false;
         if (!state.enabled) break;
         // This provider was switched off while it looked: try the next.
         if (!imageId || !entry.on) continue;
+        nearestLookup = null; // answered: nothing left to abort
         return openImage(entry.def.id, imageId);
       }
+      if (nearestLookup === lookup) nearestLookup = null;
       // Switched off while it looked: nothing to open, nothing to report.
       if (state.enabled && activeEntries().length)
         state.street.error =
@@ -493,6 +517,7 @@ export function createStreetLevelLayer({
     },
     /** Close the image and deselect it everywhere on the map. */
     closeViewer() {
+      abortNearest();
       parts.viewerHost.close();
       parts.clearSequences();
     },

src/layers/streetLevel/index.test.mjs

@@ -3,6 +3,7 @@ import test from 'node:test';
 import * as Cesium from 'cesium';
 import { createStreetLevelLayer } from './index.js';
 import { MAPILLARY_CREDIT_HTML } from './providers/mapillary/policy.js';
+import { createMapillaryProvider } from './providers/mapillary/index.js';
 import { fakeStreetLevelProvider as fakeProvider } from '../../testSupport/streetLevelFakes.mjs';
 
 /**
@@ -11,6 +12,15 @@ import { fakeStreetLevelProvider as fakeProvider } from '../../testSupport/stree
  */
 function fakeViewer() {
   const credits = [];
+  /** Cesium's flight bookkeeping: a new flight cancels the current one. */
+  let flight = null;
+  const flights = { started: 0, cancelled: 0 };
+  const stopFlight = () => {
+    const current = flight;
+    flight = null;
+    current?.cancel?.();
+    return Boolean(current);
+  };
   const canvas = Object.assign(new EventTarget(), {
     style: {},
     // Keep Cesium's handler on the canvas; there is no real document here.
@@ -19,11 +29,27 @@ function fakeViewer() {
   });
   return {
     credits,
-    scene: { canvas, primitives: { add: (p) => p, remove() {} } },
+    flights,
+    scene: {
+      canvas,
+      primitives: { add: (p) => p, remove() {} },
+      // Enough of a scene for the position marker to clamp to the ground.
+      frameState: { mode: Cesium.SceneMode.SCENE3D },
+      updateHeight: () => () => {},
+      getHeight: () => undefined,
+    },
     camera: {
       changed: new Cesium.Event(),
       moveStart: new Cesium.Event(),
       moveEnd: new Cesium.Event(),
+      flyToBoundingSphere(sphere, options = {}) {
+        stopFlight();
+        flight = options;
+        flights.started++;
+      },
+      cancelFlight() {
+        if (stopFlight()) flights.cancelled++;
+      },
     },
     creditDisplay: {
       addStaticCredit: (credit) => credits.push(credit),
@@ -36,7 +62,11 @@ function fakeViewer() {
 /** A layer over `providers`, initialised and enabled on a stand-in viewer. */
 async function enabledLayer(t, providers = [fakeProvider()]) {
   const saved = globalThis.document;
-  globalThis.document = new EventTarget();
+  // A pose moves the position marker, whose glyph is drawn on a canvas.
+  const drawing = new Proxy({}, { get: () => () => ({ addColorStop() {} }) });
+  globalThis.document = Object.assign(new EventTarget(), {
+    createElement: () => ({ getContext: () => drawing }),
+  });
   const viewer = fakeViewer();
   const layer = createStreetLevelLayer({ providers });
   layer.init(viewer);
@@ -53,10 +83,50 @@ const settle = () => new Promise((resolve) => setTimeout(resolve, 0));
 
 function deferred() {
   let resolve;
-  const promise = new Promise((done) => {
+  let reject;
+  const promise = new Promise((done, fail) => {
     resolve = done;
+    reject = fail;
   });
-  return { promise, resolve };
+  return { promise, resolve, reject };
+}
+
+/** A provider whose viewer reports a pose for every image it opens. */
+function posingProvider() {
+  const provider = fakeProvider();
+  const create = provider.create;
+  provider.create = (context) => {
+    const instance = create(context);
+    let emit = null;
+    instance.viewer.onPose = (listener) => {
+      emit = listener;
+      return () => {
+        emit = null;
+      };
+    };
+    instance.viewer.open = async (imageId) => {
+      provider.calls.open.push(imageId);
+      emit?.({
+        providerId: 'mapillary',
+        imageId,
+        position: { lon: -121.49, lat: 38.58 },
+        bearing: 90,
+      });
+    };
+    return instance;
+  };
+  return provider;
+}
+
+/** nearestImage lookups that answer when the test says, with their signals. */
+function slowLookups() {
+  const lookups = [];
+  const nearestImage = (point, { signal } = {}) => {
+    const answer = deferred();
+    lookups.push({ point, signal, answer });
+    return answer.promise;
+  };
+  return { lookups, nearestImage };
 }
 
 /** A map stack controller that can be switched between stacks. */
@@ -227,3 +297,125 @@ test('a provider withdraws only the error it reported (review P3)', async (t) =>
   actions.reportError(null);
   assert.match(layer.getUIState().street.error, /Open the Street Level panel/);
 });
+
+test('closing the photo stops the globe flying to it (review IC8 P1)', async (t) => {
+  const { layer, viewer } = await enabledLayer(t, [posingProvider()]);
+  layer.attachViewerHost({});
+  assert.equal(await layer.openImage('mapillary', 'img1'), true);
+  assert.equal(viewer.flights.started, 1, 'the photo is framed');
+  layer.closeViewer();
+  assert.equal(viewer.flights.cancelled, 1);
+});
+
+test('switching the layer off or destroying it stops the framing flight (review IC8 P1)', async (t) => {
+  const { layer, viewer } = await enabledLayer(t, [posingProvider()]);
+  layer.attachViewerHost({});
+  await layer.openImage('mapillary', 'img1');
+  layer.disable();
+  assert.equal(viewer.flights.cancelled, 1, 'layer off');
+
+  layer.enable(viewer);
+  await layer.openImage('mapillary', 'img2');
+  layer.destroy();
+  assert.equal(viewer.flights.cancelled, 2, 'destroyed');
+});
+
+test('closing the photo leaves a newer navigation flight alone (review IC8 P1)', async (t) => {
+  const { layer, viewer } = await enabledLayer(t, [posingProvider()]);
+  layer.attachViewerHost({});
+  await layer.openImage('mapillary', 'img1');
+  assert.equal(viewer.flights.started, 1, 'the photo is being framed');
+  // A search result flies the globe elsewhere before the framing lands.
+  viewer.camera.flyToBoundingSphere(null, {});
+  layer.closeViewer();
+  assert.equal(viewer.flights.cancelled, 0, 'the search flight keeps going');
+});
+
+test('an older nearest lookup that answers late cannot replace a newer one (review IC8 P2)', async (t) => {
+  const { lookups, nearestImage } = slowLookups();
+  const provider = fakeProvider({ nearestImage });
+  const { layer } = await enabledLayer(t, [provider]);
+  layer.attachViewerHost({});
+  const older = layer.openNearest({ lat: 38.58, lon: -121.49 });
+  const newer = layer.openNearest({ lat: 38.59, lon: -121.48 });
+  lookups[1].answer.resolve('newer');
+  assert.equal(await newer, true);
+  lookups[0].answer.resolve('older');
+  assert.equal(await older, false);
+  assert.deepEqual(provider.calls.open, ['newer']);
+  assert.equal(lookups[0].signal?.aborted, true, 'its request was aborted');
+  assert.equal(lookups[1].signal?.aborted, false);
+});
+
+test('an image picked during a nearest lookup wins over its late answer (review IC8 P2)', async (t) => {
+  const { lookups, nearestImage } = slowLookups();
+  const provider = fakeProvider({ nearestImage });
+  const { layer } = await enabledLayer(t, [provider]);
+  layer.attachViewerHost({});
+  const lookup = layer.openNearest({ lat: 38.58, lon: -121.49 });
+  // A cone click goes through the provider's openImage action.
+  assert.equal(await provider.context().actions.openImage('picked'), true);
+  lookups[0].answer.resolve('nearest');
+  assert.equal(await lookup, false);
+  assert.deepEqual(provider.calls.open, ['picked']);
+  assert.equal(lookups[0].signal?.aborted, true);
+});
+
+test('closing the viewer or switching the layer off retires a nearest lookup (review IC8 P2)', async (t) => {
+  const { lookups, nearestImage } = slowLookups();
+  const provider = fakeProvider({ nearestImage });
+  const { layer, viewer } = await enabledLayer(t, [provider]);
+  layer.attachViewerHost({});
+
+  const closed = layer.openNearest({ lat: 38.58, lon: -121.49 });
+  layer.closeViewer();
+  // The aborted fetch rejects, as fetch does; that is not the user's error.
+  lookups[0].answer.reject(new DOMException('aborted', 'AbortError'));
+  assert.equal(await closed, false);
+  let { street } = layer.getUIState();
+  assert.deepEqual(
+    [street.open, street.loading, street.error],
+    [false, false, null],
+  );
+  assert.equal(lookups[0].signal?.aborted, true, 'closing aborts the request');
+
+  const disabled = layer.openNearest({ lat: 38.58, lon: -121.49 });
+  layer.disable();
+  layer.enable(viewer);
+  lookups[1].answer.resolve('img1');
+  assert.equal(await disabled, false, 'nor does it open once back on');
+  assert.deepEqual(provider.calls.open, []);
+  ({ street } = layer.getUIState());
+  assert.deepEqual(
+    [street.open, street.loading, street.error],
+    [false, false, null],
+  );
+  assert.equal(lookups[1].signal?.aborted, true, 'layer off aborts it too');
+});
+
+test('the Mapillary provider hands the lookup signal to its source (review IC8 P2)', async () => {
+  const requests = [];
+  const source = {
+    hasToken: () => true,
+    getStatus: async () => ({ configured: true }),
+    getTile: async () => new Uint8Array(0),
+    getSequenceImages: async () => [],
+    nearestImages: async (query, options) => {
+      requests.push(options);
+      return [{ id: 7, is_pano: false, captured_at: 10 }];
+    },
+  };
+  const instance = createMapillaryProvider({ source }).create({
+    services: {},
+    getFilter: () => ({ pano: 'all', sinceMs: null }),
+    isActive: () => true,
+    notify() {},
+    actions: { openImage() {}, reportError() {} },
+  });
+  const { signal } = new AbortController();
+  assert.equal(
+    await instance.nearestImage({ lat: 1, lon: 2 }, { signal }),
+    '7',
+  );
+  assert.equal(requests[0]?.signal, signal);
+});

src/layers/streetLevel/providers/mapillary/index.js

@@ -180,14 +180,15 @@ export function createMapillaryProvider({ source }) {
           loading: state.sequence.loading,
         }),
 
-        /** Nearest image that passes the imagery filter, or null. */
-        async nearestImage({ lat, lon }) {
-          const images = await source.nearestImages({
-            lat,
-            lon,
-            radius: NEAREST_RADIUS_M,
-            limit: NEAREST_LIMIT,
-          });
+        /**
+         * Nearest image that passes the imagery filter, or null. The core
+         * aborts `signal` once a newer request overtakes the lookup.
+         */
+        async nearestImage({ lat, lon }, { signal } = {}) {
+          const images = await source.nearestImages(
+            { lat, lon, radius: NEAREST_RADIUS_M, limit: NEAREST_LIMIT },
+            { signal },
+          );
           // The API returns the images in the radius in no particular order.
           const metres = (record) => {
             const [lon2, lat2] = (record.computed_geometry || record.geometry)

src/layers/streetLevel/registry.js

@@ -35,7 +35,7 @@
  * @property {(sequenceId: string) => Promise<void>} [selectSequence]
  * @property {() => void} [clearSequence]
  * @property {() => {selectedId: string|null, images: number, loading: boolean}} [sequenceStats]
- * @property {(point: {lat: number, lon: number}) => Promise<string|null>} nearestImage
+ * @property {(point: {lat: number, lon: number}, options?: {signal?: AbortSignal}) => Promise<string|null>} nearestImage   An aborted lookup gives up (it may reject with an AbortError).
  * @property {ViewerAdapter} viewer
  *
  * @typedef {object} GroundCaster

src/layers/streetLevel/viewerHost.js

@@ -153,9 +153,13 @@ export function createViewerHost({ state, parts }) {
     return true;
   }
 
-  /** Close the image; the mounted adapter stays warm for the next open. */
+  /**
+   * Close the image; the mounted adapter stays warm for the next open. The
+   * globe stops flying toward it too (unmount closes, so layer off does).
+   */
   function close() {
     openSeq++;
+    parts.follow.cancelFraming();
     active?.adapter.close();
     Object.assign(state.street, freshStreet());
     parts.marker.clear();

src/layers/streetLevel/viewerHost.test.mjs

@@ -39,11 +39,16 @@ function harness(adapter) {
   };
   const def = { id: 'mapillary', name: 'Mapillary', label: 'MAPILLARY' };
   state.providers.set('mapillary', { def, instance: { viewer: adapter } });
+  const framing = { started: 0, cancelled: 0 };
   const parts = {
     marker: { set() {}, clear() {} },
-    follow: { followCamera() {}, lookAtPosition() {} },
+    follow: {
+      followCamera() {},
+      lookAtPosition: () => framing.started++,
+      cancelFraming: () => framing.cancelled++,
+    },
   };
-  return { state, host: createViewerHost({ state, parts }) };
+  return { state, framing, host: createViewerHost({ state, parts }) };
 }
 
 /**
@@ -318,3 +323,17 @@ test('closing the photo stops sequence playback in the hidden viewer (review P2-
   assert.equal(viewer.playback.stops, 1, 'playback stopped with the photo');
   assert.equal(viewers.live, 1, 'the viewer itself stays warm');
 });
+
+test('closing the photo or switching the layer off stops its framing flight (review IC8 P1)', async () => {
+  const adapter = fakeAdapter();
+  const { framing, host } = harness(adapter);
+  await host.open('mapillary', 'a');
+  assert.equal(framing.started, 1, 'the open framed the photo');
+  host.close();
+  assert.equal(framing.cancelled, 1, 'closed: the globe stops flying to it');
+  await host.open('mapillary', 'b');
+  host.unmount('panoramax');
+  assert.equal(framing.cancelled, 1, 'another provider leaves it alone');
+  host.unmount();
+  assert.equal(framing.cancelled, 2, 'layer off: likewise');
+});