Make a fill-colour column switch apply, and make its colours a property of the column

.changeset/apply-path-for-in-place-config-edits.md

@@ -0,0 +1,40 @@
+---
+'@spatialdata/core': patch
+'@spatialdata/vis': patch
+---
+
+Make a fill-colour column (or tooltip field) switch actually apply for a host that
+edits its layer configs in place.
+
+Two independent breaks sat between "the user picked a different column" and "the
+canvas shows it", and a host only hit them together. #119 fixed the third thing in
+that chain — the load-window blank — which is why the remaining two read as "the
+colours just never change".
+
+**The change never reached the resolver.** `useLayerData`'s reconcile effect is the
+one place a config change turns into a request, and it was keyed on the identity of
+`layers` and the configs inside it. That assumes the caller allocates a fresh config
+per edit; MDV's render-stack adapter deliberately does the opposite, keeping one
+`LayerConfig` per Stack Entry so a cosmetic edit does not look structural and
+re-enter geometry loads. Under that caller the effect never re-ran: the new column
+was never requested, `getShapeFillColorEntry` / `getLabelFillColorEntry` went on
+correctly serving last-good rows, and last-good was all there would ever be. The
+effect now also depends on `describeResolveInputs` — a value key over exactly the
+config fields each resolver's `plan()` reads, recomputed per render because a
+mutation is invisible to any memo. It holds scalars and short id lists only; a
+palette swap or an opacity drag does not move it, so nothing replans on a slider.
+
+**The settle never reached React.** `SpatialEntryStore` subscribed to its resolvers
+in its constructor and tore that bridge down in `dispose()` — which `useLayerData`
+calls from an effect cleanup. An effect cleanup is not "the end": StrictMode's dev
+double-mount runs cleanup and then re-runs the effect against the same memoised
+store, after which the store was permanently deaf to its own resolvers. Every async
+settle from then on was dropped, so rows that landed after a switch did not repaint
+until an unrelated re-render (a pan) came along. The bridge is now attached on the
+first listener and detached on the last, so it is exactly as long-lived as someone
+caring about it and survives any number of remounts. `getVersion()` became a derived
+sum of the resolvers' versions rather than a counter the bridge maintained, so it
+stays true whether or not anything is subscribed.
+
+No public API change. Verified against MDV driving only `fillColorByColumn` on a
+labels layer: switching to a column that has to be fetched now repaints on its own.

.changeset/column-colour-not-view-colour.md

@@ -0,0 +1,48 @@
+---
+'@spatialdata/layers': minor
+'@spatialdata/vis': minor
+---
+
+Make a column's colours a property of the column, not of the features that loaded.
+
+Three things decided the encoding from whatever happened to be in view, so two
+layers over one annotation could disagree about what a colour means — which reads
+as a data difference rather than as a bug:
+
+- Category indices were assigned in **first-seen feature order**. A shapes layer
+  walks the loader's geometry order and a labels layer walks the raster's ids, so
+  the same `cell_type` column rendered in two different schemes on the two kinds.
+  (`labelColorEncoding.spec.ts` claimed to cover this, but only pinned the indices
+  on one kind; it now actually builds the column through both.) Categories are now
+  ordered by value, with numeric-looking values ordered numerically so cluster 10
+  follows cluster 9 rather than cluster 1.
+- Positional palettes cannot survive a category being **absent from a view** at
+  all: `tumour` genuinely is the second category present when `stroma` is not.
+  `categoricalPalette` therefore also accepts `{ byValue: { Tumour: [200, 30, 30] } }`,
+  with an optional `fallback` for values it does not name (`'oklab'` by default, so
+  an unnamed category keeps its own hue instead of merging into one bucket). This
+  is the form to prefer in a saved stack, and the only form an embedding
+  application can use to make a layer agree with its own charts.
+- The continuous ramp measured its extent from the loaded features. `numericDomain`
+  pins it to the column's own range; values outside clamp rather than extrapolate.
+
+`numericRamp` also takes more than two stops now, spaced evenly across the domain,
+because the ramps people actually use are not two-stop — viridis, a diverging
+red/white/blue, or whatever a host has already chosen for the same column in its
+own UI. Approximating one by its endpoints loses the midpoint that made it
+meaningful. `numericScale: 'symlog'` goes with it: a counts or expression column
+whose mass sits near zero with a long tail collapses into the first stop under a
+linear position. Symmetric log rather than plain log, because these columns reach
+zero and below.
+
+`featureColorSchemeSignature` now takes the scheme as one object
+(`featureColorSchemeSignature(config.fillColorByColumn)`) rather than three
+positional arguments, so adding a term to the encoding cannot leave a call site
+silently keying on the old set — the failure mode there being a layer that keeps
+serving the previous colours after the scheme changed. Named palettes are
+serialised in sorted key order, since object key order is insertion order and a
+host rebuilding its palette each render need not insert in a stable one.
+
+**Colours will change** for existing categorical configs that relied on the
+implicit first-seen order. Pass `categoricalPalette: { byValue }` to fix a scheme
+in place.

.changeset/publish-sourcemaps-and-survive-bad-schemes.md

@@ -0,0 +1,21 @@
+---
+'@spatialdata/layers': patch
+'@spatialdata/vis': patch
+'@spatialdata/avivatorish': patch
+'@spatialdata/react': patch
+---
+
+Publish sourcemaps, and survive a colour scheme that does not match its own type.
+
+`core` shipped `index.js.map`; `layers`, `vis`, `avivatorish` and `react` did not.
+A crash inside one of them reached a consumer as
+`Le (…/.vite/deps/@spatialdata_layers.js:396)` — an esbuild-minified name with
+nothing to map it back to. An embedding application has only the built artifact to
+debug against, so it has to carry a map.
+
+`resolveCategoricalPalette` and the ramp sampler now always return a colour. A
+scheme arrives from a saved Render Stack, so its type is a claim about JSON rather
+than a guarantee: a palette object with no `byValue`, a list with a hole in it, or
+a ramp with fewer than two stops all used to return `undefined` and fail several
+frames later in the arithmetic that reads `rgb[0]`. Wrong colours can be seen and
+reported; that `TypeError` cannot.

packages/core/src/engine/SpatialEntryStore.ts

@@ -29,28 +29,63 @@ export class SpatialEntryStore {
   private readonly unsubscribes: Array<() => void> = [];
   /** One AbortController per in-flight task id. Superseding cancels the old one. */
   private readonly inFlight = new Map<string, AbortController>();
-  private version = 0;
 
   constructor(resolvers: ResolverRegistry) {
     this.resolvers = resolvers;
-    // The store's version is the sum of its parts: any resolver mutating is a
-    // reason for React to re-read.
-    for (const resolver of Object.values(resolvers)) {
+  }
+
+  /**
+   * The store's version is the sum of its parts: any resolver mutating is a reason
+   * for React to re-read. The bridge that carries that is tied to HAVING LISTENERS,
+   * not to construction — because the store outlives the effect that consumes it.
+   *
+   * Subscribing in the constructor and tearing down in `dispose` looks equivalent
+   * and is not. `dispose()` runs from a React effect cleanup, and an effect cleanup
+   * is not "the end": StrictMode's dev double-mount runs cleanup and then re-runs
+   * the effect against the SAME memoised store. A constructor-time bridge cannot be
+   * rebuilt, so from that moment the store was permanently deaf to its own resolvers
+   * — every async settle after mount was dropped, and a fill-colour column whose
+   * rows landed after the switch never repainted until an unrelated re-render (a
+   * pan) happened to come along. Attaching on the first listener and detaching on
+   * the last makes the bridge exactly as long-lived as someone caring about it, and
+   * survives any number of remounts.
+   */
+  private attachResolvers(): void {
+    if (this.unsubscribes.length > 0) return;
+    for (const resolver of Object.values(this.resolvers)) {
       this.unsubscribes.push(resolver.subscribe(() => this.notify()));
     }
   }
 
+  private detachResolvers(): void {
+    for (const unsubscribe of this.unsubscribes) unsubscribe();
+    this.unsubscribes.length = 0;
+  }
+
   subscribe = (listener: () => void): (() => void) => {
     this.listeners.add(listener);
+    if (this.listeners.size === 1) this.attachResolvers();
     return () => {
       this.listeners.delete(listener);
+      if (this.listeners.size === 0) this.detachResolvers();
     };
   };
 
-  getVersion = (): number => this.version;
+  /**
+   * The sum of its parts, DERIVED rather than counted.
+   *
+   * A counter incremented from the notification bridge would only be correct while
+   * something was subscribed — and the bridge is now listener-driven, so that is not
+   * always. Summing the resolvers' own versions makes this a pure read of the state
+   * it describes: true with a listener, without one, and across a dispose.
+   */
+  getVersion = (): number => {
+    let version = 0;
+    for (const resolver of Object.values(this.resolvers)) version += resolver.getVersion();
+    return version;
+  };
 
   private notify(): void {
-    this.version += 1;
     for (const listener of this.listeners) {
       listener();
     }
@@ -137,9 +172,13 @@ export class SpatialEntryStore {
     this.resolvers[kind]?.evict(elementKey);
   }
 
+  /**
+   * Release everything this store owns. Not a one-way door: a later `subscribe`
+   * re-attaches the resolver bridge, which is what lets a StrictMode remount (or any
+   * effect that re-runs against the same store) recover instead of going silent.
+   */
   dispose(): void {
-    for (const unsubscribe of this.unsubscribes) unsubscribe();
-    this.unsubscribes.length = 0;
+    this.detachResolvers();
     for (const controller of this.inFlight.values()) controller.abort();
     this.inFlight.clear();
     for (const resolver of Object.values(this.resolvers)) resolver.dispose();

packages/core/tests/spatialEntryStoreSubscription.spec.ts

@@ -0,0 +1,134 @@
+import { describe, expect, it, vi } from 'vitest';
+import {
+  type EntryResources,
+  Resolution,
+  type ResolveTask,
+  SpatialEntryStore,
+} from '../src/engine/index.js';
+
+/**
+ * The store's bridge to its resolvers, under the lifecycle React actually gives it.
+ *
+ * `SpatialEntryStore` is memoised by `useLayerData` and disposed from an effect
+ * cleanup — and an effect cleanup is not "the end". StrictMode's dev double-mount
+ * runs cleanup and then re-runs the effect against the same store instance. When the
+ * bridge was built in the constructor and torn down in `dispose`, that sequence left
+ * the store permanently deaf: resolvers went on loading and settling, and nothing
+ * downstream ever heard about it. The symptom was a fill-colour column that loaded
+ * and then never painted until an unrelated re-render came along.
+ *
+ * These tests drive that sequence directly, with a resolver stub whose only job is to
+ * emit one settle.
+ */
+
+/** A resolver that does nothing but let a test fire its settle notification. */
+function notifyingResolver() {
+  const listeners = new Set<() => void>();
+  return {
+    resolver: {
+      kind: 'labels' as const,
+      blockingResources: [] as const,
+      plan: (): readonly ResolveTask[] => [],
+      load: async () => {},
+      snapshot: (): EntryResources => ({
+        entryId: 'e',
+        elementKey: 'k',
+        resources: {},
+        notices: [],
+        bounds: null,
+        revision: 0,
+      }),
+      evict: () => {},
+      dispose: () => {
+        listeners.clear();
+      },
+      subscribe: (listener: () => void) => {
+        listeners.add(listener);
+        return () => {
+          listeners.delete(listener);
+        };
+      },
+      getVersion: () => 0,
+    },
+    /** Stand in for a load settling — what `finally { this.notify() }` does. */
+    settle: () => {
+      for (const listener of listeners) listener();
+    },
+    listenerCount: () => listeners.size,
+  };
+}
+
+function storeWith(labels: ReturnType<typeof notifyingResolver>['resolver']) {
+  const inert = { ...labels, subscribe: () => () => {} };
+  return new SpatialEntryStore({
+    points: inert,
+    shapes: inert,
+    images: inert,
+    labels,
+  });
+}
+
+describe('SpatialEntryStore — the resolver notification bridge', () => {
+  it('forwards a resolver settle to its listeners', () => {
+    const labels = notifyingResolver();
+    const store = storeWith(labels.resolver);
+    const onChange = vi.fn();
+    store.subscribe(onChange);
+
+    labels.settle();
+
+    expect(onChange).toHaveBeenCalledTimes(1);
+  });
+
+  it('still forwards a settle after dispose + resubscribe (the StrictMode remount)', () => {
+    // THE regression. React runs cleanup then re-runs the effect against the same
+    // memoised store; nothing about that says the store is finished.
+    const labels = notifyingResolver();
+    const store = storeWith(labels.resolver);
+    const onChange = vi.fn();
+
+    const unsubscribe = store.subscribe(onChange);
+    unsubscribe();
+    store.dispose();
+    store.subscribe(onChange);
+
+    labels.settle();
+
+    expect(onChange).toHaveBeenCalledTimes(1);
+  });
+
+  it('holds no resolver subscription while nobody is listening', () => {
+    // The bridge exists for listeners. With none, it must not pin the resolver —
+    // otherwise a discarded store keeps a live edge into a resolver it no longer owns.
+    const labels = notifyingResolver();
+    const store = storeWith(labels.resolver);
+
+    expect(labels.listenerCount()).toBe(0);
+
+    const unsubscribe = store.subscribe(vi.fn());
+    expect(labels.listenerCount()).toBe(1);
+
+    unsubscribe();
+    expect(labels.listenerCount()).toBe(0);
+  });
+
+  it('attaches once for many listeners, and detaches only when the last one goes', () => {
+    const labels = notifyingResolver();
+    const store = storeWith(labels.resolver);
+    const first = vi.fn();
+    const second = vi.fn();
+
+    const unsubscribeFirst = store.subscribe(first);
+    const unsubscribeSecond = store.subscribe(second);
+    expect(labels.listenerCount()).toBe(1);
+
+    unsubscribeFirst();
+    expect(labels.listenerCount()).toBe(1);
+
+    labels.settle();
+    expect(second).toHaveBeenCalledTimes(1);
+
+    unsubscribeSecond();
+    expect(labels.listenerCount()).toBe(0);
+  });
+});