From 430aee57ee9f9d87129943fa89c058017bdd2b04 Mon Sep 17 00:00:00 2001 From: Peter Todd Date: Tue, 14 Jul 2026 18:46:14 +0100 Subject: [PATCH 1/9] =?UTF-8?q?Amend=20ADR=200004=20=C2=A76:=20no=20image?= =?UTF-8?q?=20port=20in=20core;=20placement=20is=20per-kind?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ADR 0004 §6 claimed `createImageLoader` closes over the React `VivLoaderRegistry` context, making the image loader "the one genuine ports-and-adapters dependency". It closes over nothing: it already takes `fetchMultiscales` as an injected parameter, and the React context is merely the DI container at the call site. There is no closure to break and therefore no port to invent. The claim was assumed, not checked against the code, before it was committed. Three things follow, and together they keep images out of core for now: - `avivatorish` is a deliberate de-vendoring holding pen for code that also lives upstream in Viv and in MDV, and its own README calls the serialized image-state model "still evolving". A port designed against it today would freeze a guess about an unsettled model into the interface `tgpu-htj2k` depends on. - The second consumer does not need it. zarrextra's `VivCompatiblePixelSource` already serves both Viv and `tgpu-htj2k`, so the shared images seam already exists *below* the resolver — as this ADR's own Non-goals section says. Images is the one kind of the four where the duplication argument, the entire reason for this ADR, does not apply. - So an image port in core pays a real cost to solve a problem that is not there. Resolver *placement* is therefore per-kind and driven by dependency, not by dogma. core defines the ResourceResolver interface. Points and Shapes resolvers live in core; Images and Labels implement the same interface but live in vis, next to the Viv/avivatorish dependencies they already have. The store holds only ResourceResolver and must not know which package an implementation came from. Also: - §7 (no runtime dependency in core's public signatures) is marked as a current position resting on tgpu-htj2k's stance — revisable, not a law. Read it as "not now", not "never". - The handoff plan's phase table is corrected: project()/render() belong to the Renderer Adapter in `layers`, per ADR 0004 §4, not to the resolver. This decides where PointsDataEngine's lazy memos land. Docs only; no source changes. Co-Authored-By: Claude Opus 4.8 --- .../0004-resource-resolver-owned-by-core.md | 77 ++++++++++++++++++- docs/plans/resource-resolver-handoff.md | 39 ++++++++-- 2 files changed, 104 insertions(+), 12 deletions(-) diff --git a/docs/adr/0004-resource-resolver-owned-by-core.md b/docs/adr/0004-resource-resolver-owned-by-core.md index e53d84f7..57e94bca 100644 --- a/docs/adr/0004-resource-resolver-owned-by-core.md +++ b/docs/adr/0004-resource-resolver-owned-by-core.md @@ -109,10 +109,19 @@ concept. paths. No consumer import moves. Removing the shims is a separate, deliberate deprecation, coordinated with MDV; it is not part of this work. -6. **The image loader is a port.** `createImageLoader` closes over the React - `VivLoaderRegistry` context. `core` defines the port; `vis` supplies the - adapter. This is the one genuine ports-and-adapters dependency; everything - else is local-substitutable (tests stub elements as plain object literals). +6. **Resolver *placement* is per-kind, driven by dependency.** `core` defines the + `ResourceResolver` **interface**. An implementation lives in the package its + dependencies already live in: + + - `PointsResolver`, `ShapesResolver` → **`core`**. Every type they touch + (`PointsLoadResult`, `PointsFeatureCatalog`, `ShapesRenderData`, + `ShapesTooltipMetadata`) is already a `core` type, and both are what + `tgpu-htj2k` was forced to hand-roll. + - `ImagesResolver`, `LabelsResolver` → **`vis`**, for now. See below. + + **`core` defines no image port.** *(Amended 2026-07-14; see "Amendment — + the image port" below for what this paragraph used to claim and why it was + wrong.)* 7. **No runtime dependency enters `core`.** No Effect, no `neverthrow`, no TanStack in a public signature. `core` is the dependency root for @@ -120,6 +129,66 @@ concept. dependency-free. A library may be used *inside* a resolver's implementation (see the `RequestSlot` spike) but must not appear in `core`'s interface. + This is a **current position resting on another repo's position**, not a law. + It is revisable if `tgpu-htj2k`'s dependency-free stance is renegotiated — + which is exactly the kind of cross-repo conversation §"Cross-repo consequence" + already flags as *owed*. Read it as *not now*, not as *never*. Nothing in the + Step 0 / Step 1 work turns on the answer either way, and the `RequestSlot` + seam is deliberately shaped so that revisiting it later is a decision rather + than a rewrite. + +## Amendment — the image port (2026-07-14) + +Decision 6 originally read: + +> **The image loader is a port.** `createImageLoader` closes over the React +> `VivLoaderRegistry` context. `core` defines the port; `vis` supplies the +> adapter. This is the one genuine ports-and-adapters dependency; everything else +> is local-substitutable. + +**It closes over nothing.** `createImageLoader` +(`packages/vis/src/SpatialCanvas/renderers/imageRenderer.ts`) already takes +`fetchMultiscales` as an injected parameter, defaulting to +`loadOmeZarrMultiscalesData`. The React context is merely the DI *container* at +the call site — `useLayerData` reads `useVivLoaderRegistry()` and passes the +value in. The injection already exists as a plain function parameter. **There is +no closure to break, and therefore no port to invent.** The original text +described an assumed constraint, not a real one, and it was not checked against +the code before it was committed. + +Three things follow, and together they keep images out of `core` for now: + +- **`avivatorish` is a deliberate holding pen, not a stable dependency.** Its own + README: *"SpatialData.js, MDV, and other apps should depend on this package + instead of vendoring near-duplicates"*, and *"the long-term shape of serialized + image state is still **evolving**."* It is a de-vendored fork of code that also + lives upstream in Viv and in MDV. A port designed against it today would encode + a guess about an unsettled model — into the interface `tgpu-htj2k` depends on. +- **The second consumer does not need it.** Per §Non-goals below, `zarrextra`'s + `VivCompatiblePixelSource` **already serves both Viv and `tgpu-htj2k` today**; + the shared seam for images already exists and sits *below* the Resolver. Images + is the one kind of the four where the duplication argument — the entire reason + for this ADR — **does not apply**. `tgpu-htj2k` needs `PointsResolver` and + `ShapesResolver` from `core`; it does not need an images resolver from anyone. +- So an image port in `core` would pay a real cost to solve a problem that is not + there. + +**Consequence.** `ImagesResolver` and `LabelsResolver` implement `core`'s +`ResourceResolver` interface but live in `@spatialdata/vis`, alongside the Viv and +`avivatorish` dependencies they already have. `createImageLoader` keeps its +injected `fetchMultiscales`; the channel-defaults ladder moves phase (out of the +god-hook, into `load()`) without moving package; world bounds are computed by the +resolver itself via viv's `getImageSize`, so `core` never needs raster extents. + +The interface is still one interface, and the store still holds only +`ResourceResolver` — it must not know which package an implementation came from. +If a `vis`-resident resolver ever needs something the interface does not offer, +that is a signal about the *interface*, not a licence to special-case images. + +**Still open, and deliberately so:** whether `avivatorish` should converge on +upstream Viv, be absorbed by MDV, or stabilise as its own contract. When that +settles, revisit whether an image resolver — and a port — belongs in `core`. + ## Lifecycle split — who reconciles what The Resolver is **not** the universal request-lifecycle owner. Tile scheduling diff --git a/docs/plans/resource-resolver-handoff.md b/docs/plans/resource-resolver-handoff.md index 760daddb..312be13b 100644 --- a/docs/plans/resource-resolver-handoff.md +++ b/docs/plans/resource-resolver-handoff.md @@ -26,14 +26,23 @@ core Resource Resolver reconcile · cache · supersede · stream · evict └──► headless no renderer ``` -**Phase separation inside a resolver** — this is the load-bearing part: - -| Phase | Purity | When | May start I/O? | -|---|---|---|---| -| `plan(ctx)` | pure, sync | commit only | no — *returns* task descriptors | -| `load(task, ctx)` | async | commit only | **yes — the only place** | -| `project(ctx)` | pure, sync | end of reconcile | no | -| `render(state, opts)` | pure, sync | **during React render** | no — *is handed no engine handle* | +**Phase separation** — this is the load-bearing part: + +| Phase | Owner | Purity | When | May start I/O? | +|---|---|---|---|---| +| `plan(ctx)` | **Resolver** | pure, sync | commit only | no — *returns* task descriptors | +| `load(task, ctx)` | **Resolver** | async | commit only | **yes — the only place** | +| `project(entry, config)` | **Renderer Adapter** | pure, sync | end of reconcile | no | +| `render(projected, opts)` | **Renderer Adapter** | pure, sync | **during React render** | no — *is handed no engine handle* | + +> **Correction (2026-07-14).** An earlier revision of this table was headed +> *"phase separation inside a resolver"*, implying all four phases live on the +> resolver. They do not. ADR 0004 §4 puts `project()` and `render()` on the +> **Renderer Adapter** in `layers`: *"identity-stable memoisation is a **deck +> requirement** … so it belongs on the renderer side."* This document is +> sequencing, not rationale — where they disagree, **the ADR wins**. The +> consequence is concrete: `PointsDataEngine`'s lazy render-resource memos move +> to `layers`, not to `core`. `render()` receives a frozen resolved state and nothing that can start work. That makes today's `void engine.ensureMatchingFeaturesLoaded(...)` inside `getLayers()` @@ -70,6 +79,20 @@ Write `Shapes` / `Images` / `Labels` resolvers as **thin adapters holding today' Maps and calling today's functions**. Behaviour-identical. No load changes, no race fixes, no memory work. +> **Correction (2026-07-14) — placement is per-kind, not "all four in `core`".** +> ADR 0004 §6 has been amended. `core` defines the **interface**; +> `PointsResolver` and `ShapesResolver` live in `core` (every type they touch is +> already a `core` type). `ImagesResolver` and `LabelsResolver` implement the same +> interface but live in **`vis`**, because `createImageLoader` already takes an +> injected `fetchMultiscales` — there is no React closure to break and **no image +> port to invent** — and because `avivatorish` is a de-vendoring holding pen for +> code that also lives upstream in Viv and MDV, with an image-state model its own +> README calls *"still evolving"*. `zarrextra`'s `VivCompatiblePixelSource` +> already serves both Viv and `tgpu-htj2k`, so the images seam already exists +> *below* the resolver: images is the one kind where the duplication argument does +> not apply. **Add no image port to `core`.** See ADR 0004 §"Amendment — the image +> port". + `useLayerData` becomes a loop over resolvers instead of a switch over kinds. Keep its 17-member public surface intact behind a compat shim — MDV consumes it. From c506ec5a6811101209a20b0d0f075960bf69dfa5 Mon Sep 17 00:00:00 2001 From: Peter Todd Date: Tue, 14 Jul 2026 18:51:57 +0100 Subject: [PATCH 2/9] Step 0: Resolution, SpatialEntryError and EntryNotice in core MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The shared contracts for the Resource Resolver (ADR 0004 §3). Types plus two pure functions, in a new packages/core/src/engine/. Zero call sites — this commit changes no behaviour. Resolution = idle | loading{partial?, stale?, progress?} | ready{value, notices?} | failed{error, stale?}. Per-resource, not per-entry: a shapes entry with a broken tooltip must still draw its geometry. Two design points worth stating, because both are easy to undo later: - The identity rule. Resolutions are constructed at mutation time and returned by reference. Constructing one during project()/render() is a fresh identity per frame, which is a deck layer teardown per frame — the pan flash this design exists to avoid. Only idle() is a frozen singleton. - No valueOf() collapsing lastGood ?? partial. The points path draws lastGood as the base layer AND partial as an overlay sub-layer, at the same time. A merged accessor would destroy that distinction. toSpatialEntryError() classifies from the SEAM, not from the throw. Core's leaf loaders keep throwing bare Error(string), and anything off the points worker has already lost its type — that failure channel is `{ ok: false; error: string }`. So the context carries a `fallback` kind: the seam knows what it was doing even when the throw doesn't. Three tiers: instanceof (the only lossless one — core has exactly two typed error classes), one quarantined message-recogniser for worker-unavailable (TODO(Track A): delete once the worker protocol has a typed error), then the fallback. Message-sniffing beyond that is banned and documented as such. SpatialEntryErrorFallbackKind is deliberately narrower than SpatialEntryErrorKind: coordinate-system-not-found and points-preload-too-large carry payload only their typed class can supply, so naming either as a fallback is a promise the classifier cannot keep. The type forbids it rather than degrading at runtime. isCancellation() is the gate before the classifier. Superseding a load is normal operation, not a domain failure, so there is no `cancelled` case in the union — and without this check every memory-cap drag would paint an error where today there is none. This also gives PointsPreloadTooLargeError a home. It has been exported and never constructed; it is now one of the two lossless inputs. 30 new tests. Build, full suite (427) and biome all green. Co-Authored-By: Claude Opus 4.8 --- packages/core/src/engine/errors.ts | 271 ++++++++++++++++++ packages/core/src/engine/index.ts | 18 ++ packages/core/src/engine/resolution.ts | 140 +++++++++ packages/core/src/index.ts | 2 + packages/core/tests/resolution.spec.ts | 158 ++++++++++ packages/core/tests/spatialEntryError.spec.ts | 160 +++++++++++ 6 files changed, 749 insertions(+) create mode 100644 packages/core/src/engine/errors.ts create mode 100644 packages/core/src/engine/index.ts create mode 100644 packages/core/src/engine/resolution.ts create mode 100644 packages/core/tests/resolution.spec.ts create mode 100644 packages/core/tests/spatialEntryError.spec.ts diff --git a/packages/core/src/engine/errors.ts b/packages/core/src/engine/errors.ts new file mode 100644 index 00000000..68595ca0 --- /dev/null +++ b/packages/core/src/engine/errors.ts @@ -0,0 +1,271 @@ +/** + * Spatial Entry Error — the typed domain-failure channel for a Resource Resolver. + * Entry Notice — the non-fatal channel beside it. + * + * See `CONTEXT.md` for the vocabulary and ADR 0004 for why these live in `core`. + * + * ## The one thing to understand about `toSpatialEntryError` + * + * **It classifies from the seam, not from the throw.** + * + * `core`'s leaf loaders throw bare `Error(string)` and will keep doing so — the + * resolver classifies at the seam, and pushing typed errors down into every + * loader is explicitly not this design. More decisively: anything that crossed + * the points-worker boundary has *already* lost its type, because the worker's + * failure channel is `{ ok: false; error: string }` (`pointsWorkerProtocol.ts`). + * By the time a worker-side decode failure reaches us it is a string. No amount + * of cleverness in this module recovers what the postMessage boundary threw away. + * + * So the *caller* supplies the answer. `SpatialEntryErrorContext.fallback` says + * what this operation fails as when the cause carries no type of its own — and + * the seam always knows what it was doing, even when the throw doesn't. + * + * **Message-sniffing is banned** beyond the single quarantined `recognise()` + * below. It is a smell, and here it also simply cannot work. + */ + +import { CoordinateSystemNotFoundError } from '../models/index.js'; +import { PointsPreloadTooLargeError } from '../pointsLimits.js'; + +/** The four Spatial Entry kinds. */ +export type SpatialEntryKind = 'points' | 'shapes' | 'images' | 'labels'; + +export type SpatialEntryErrorKind = + | 'coordinate-system-not-found' + | 'element-not-found' + | 'unsupported-format' + | 'points-preload-too-large' + | 'worker-unavailable' + | 'decode-failed' + | 'load-failed'; + +/** + * The kinds a seam may nominate as its `fallback`. + * + * Deliberately **narrower** than `SpatialEntryErrorKind`: + * `coordinate-system-not-found` and `points-preload-too-large` carry structured + * payload (available coordinate systems; row counts) that only their typed error + * class can supply. They are reachable *only* through the `instanceof` tier of + * `toSpatialEntryError`. Naming one as a fallback would be a promise the + * classifier cannot keep — so the type forbids it rather than degrading at + * runtime. + */ +export type SpatialEntryErrorFallbackKind = Exclude< + SpatialEntryErrorKind, + 'coordinate-system-not-found' | 'points-preload-too-large' +>; + +interface SpatialEntryErrorBase { + /** Human-readable, and safe to show. Every case carries what the UI needs to explain itself. */ + readonly message: string; + /** + * Gates a Retry affordance. This — not the union — is what stops a failed scan + * settling permanently. See ADR 0004 §3. + */ + readonly retryable: boolean; + /** Preserved for logs. Never for the UI, and never for classification. */ + readonly cause?: unknown; +} + +export type SpatialEntryError = + | (SpatialEntryErrorBase & { + readonly kind: 'coordinate-system-not-found'; + readonly retryable: false; + readonly coordinateSystem: string; + readonly elementKey: string; + readonly availableCoordinateSystems: readonly string[]; + }) + | (SpatialEntryErrorBase & { + readonly kind: 'element-not-found'; + readonly retryable: false; + readonly elementKey: string; + readonly elementType?: string; + }) + | (SpatialEntryErrorBase & { + readonly kind: 'unsupported-format'; + readonly retryable: false; + readonly detail: string; + }) + | (SpatialEntryErrorBase & { + readonly kind: 'points-preload-too-large'; + readonly retryable: false; + readonly rowCount: number; + readonly maxRows: number; + }) + | (SpatialEntryErrorBase & { + readonly kind: 'worker-unavailable'; + readonly retryable: true; + readonly detail: string; + }) + | (SpatialEntryErrorBase & { + readonly kind: 'decode-failed'; + readonly retryable: true; + readonly resource: string; + readonly detail: string; + }) + | (SpatialEntryErrorBase & { + readonly kind: 'load-failed'; + readonly retryable: true; + readonly resource: string; + readonly detail: string; + }); + +/** + * A non-fatal domain fact about a **successfully** resolved entry. A channel + * distinct from `SpatialEntryError`, so healthy data never renders as an error. + * + * Note what is *not* here: absence. A points element with no `feature_key` is a + * settled fact — `ready(null)` — not a failure and not a notice. + */ +export type EntryNotice = + | { + readonly kind: 'preload-truncated'; + readonly message: string; + readonly loaded: number; + readonly total: number; + } + | { + readonly kind: 'selection-served-from-memory'; + readonly message: string; + readonly coveredCodes: number; + } + | { readonly kind: 'catalog-is-resident-preview'; readonly message: string } + | { + readonly kind: 'channel-defaults-fallback'; + readonly message: string; + readonly reason: string; + }; + +/** + * What the seam was doing when it threw. Supplied by the caller, because the + * caller is the only one who still knows. + */ +export interface SpatialEntryErrorContext { + readonly elementKey: string; + readonly kind: SpatialEntryKind; + /** Which resource of the entry — 'preload' | 'catalog' | 'geometry' | 'tooltip' | … */ + readonly resource: string; + /** + * What this operation fails as when the cause carries no type. + * The SEAM knows what it was doing; the throw does not. + */ + readonly fallback: SpatialEntryErrorFallbackKind; +} + +/** + * Is this an abort, rather than a failure? + * + * **Call this before the classifier at every seam.** Cancellation is a + * non-event: superseding a load is normal operation, not a domain failure, and + * there is deliberately no `cancelled` case in `SpatialEntryError`. Skip this + * check and every memory-cap drag paints an error where today there is none. + */ +export function isCancellation(cause: unknown): boolean { + if (cause instanceof DOMException) return cause.name === 'AbortError'; + return ( + typeof cause === 'object' && + cause !== null && + (cause as { name?: unknown }).name === 'AbortError' + ); +} + +function messageOf(cause: unknown): string { + if (cause instanceof Error) return cause.message; + if (typeof cause === 'string') return cause; + return String(cause); +} + +/** + * The one sanctioned message-sniff, quarantined here so it cannot spread. + * + * `worker-unavailable` is thrown from *inside* seams whose `fallback` is + * `decode-failed` (`VPointsSource` falls back to the main thread when the worker + * can't service a request), so context alone cannot reach it. It is `retryable` + * where `decode-failed` is not always meaningfully so, which is why the + * distinction is worth one narrow recogniser. + * + * Throw sites: `points-worker.ts` ("readParquetRowGroup is unavailable in points + * worker"), `pointsWorkerScan.ts` ("readParquetRowGroup is unavailable"). + * + * TODO(Track A): delete this. Track A owns the worker protocol and can introduce + * a typed `PointsWorkerUnavailableError`, at which point the `instanceof` tier + * covers it losslessly and this function goes away. + */ +function recognise(cause: unknown): SpatialEntryErrorFallbackKind | undefined { + const message = messageOf(cause); + if (message.includes('readParquetRowGroup') && message.includes('unavailable')) { + return 'worker-unavailable'; + } + return undefined; +} + +/** Total over `SpatialEntryErrorFallbackKind` — every case is constructible from an untyped cause. */ +function build( + kind: SpatialEntryErrorFallbackKind, + cause: unknown, + ctx: SpatialEntryErrorContext +): SpatialEntryError { + const message = messageOf(cause); + switch (kind) { + case 'element-not-found': + return { + kind, + message, + retryable: false, + elementKey: ctx.elementKey, + elementType: ctx.kind, + cause, + }; + case 'unsupported-format': + return { kind, message, retryable: false, detail: message, cause }; + case 'worker-unavailable': + return { kind, message, retryable: true, detail: message, cause }; + case 'decode-failed': + return { kind, message, retryable: true, resource: ctx.resource, detail: message, cause }; + case 'load-failed': + return { kind, message, retryable: true, resource: ctx.resource, detail: message, cause }; + } +} + +/** + * Turn a thrown thing into a value. **The single place a throw becomes a + * `SpatialEntryError`** — do not classify anywhere else. + * + * Three tiers, in order: + * 1. `instanceof` — the only lossless tier. `core` has exactly two typed error + * classes, so exactly two inputs classify perfectly. + * 2. `recognise()` — one quarantined message-sniff for `worker-unavailable`. + * 3. `ctx.fallback` — everything else. The seam knows; the throw doesn't. + * + * Callers must check {@link isCancellation} first: an aborted load is not a + * failure and must not reach here. + */ +export function toSpatialEntryError( + cause: unknown, + ctx: SpatialEntryErrorContext +): SpatialEntryError { + if (cause instanceof CoordinateSystemNotFoundError) { + return { + kind: 'coordinate-system-not-found', + message: cause.message, + retryable: false, + coordinateSystem: cause.coordinateSystem, + elementKey: cause.elementKey, + availableCoordinateSystems: cause.availableCoordinateSystems, + cause, + }; + } + + if (cause instanceof PointsPreloadTooLargeError) { + return { + kind: 'points-preload-too-large', + message: cause.message, + retryable: false, + rowCount: cause.rowCount, + maxRows: cause.maxRows, + cause, + }; + } + + return build(recognise(cause) ?? ctx.fallback, cause, ctx); +} diff --git a/packages/core/src/engine/index.ts b/packages/core/src/engine/index.ts new file mode 100644 index 00000000..81739591 --- /dev/null +++ b/packages/core/src/engine/index.ts @@ -0,0 +1,18 @@ +/** + * The Resource Resolver's shared contracts. + * + * See ADR 0004 (Resource Resolver Owned By Core) and `CONTEXT.md`. + */ + +export type { + EntryNotice, + SpatialEntryError, + SpatialEntryErrorContext, + SpatialEntryErrorFallbackKind, + SpatialEntryErrorKind, + SpatialEntryKind, +} from './errors.js'; +export { isCancellation, toSpatialEntryError } from './errors.js'; +export type { ResolutionProgress } from './resolution.js'; +// `Resolution` is both the type and its constructor namespace — one export carries both. +export { fromResult, Resolution } from './resolution.js'; diff --git a/packages/core/src/engine/resolution.ts b/packages/core/src/engine/resolution.ts new file mode 100644 index 00000000..ef628e68 --- /dev/null +++ b/packages/core/src/engine/resolution.ts @@ -0,0 +1,140 @@ +/** + * Resolution — the state of one loaded resource of a Spatial Entry, **as a value**. + * + * See `CONTEXT.md` for the vocabulary and ADR 0004 §3 for why this lives in `core`. + * + * ## Resolutions are per-resource, not per-entry + * + * A shapes entry with a broken tooltip column must still draw its geometry. There + * is deliberately no entry-wide `Result`. + * + * ## `stale` is a retention, not a guarantee + * + * *While it is retained*, a failed or in-flight refine keeps drawing rather than + * blanking. It is released on eviction and on non-retryable failure, after which + * the resource is simply not renderable. **Callers must handle the no-stale + * case** — they may not assume a previously-ready resource stays drawable. + * + * ## The identity rule — load-bearing, read this before touching a render path + * + * Resolution values are constructed **at mutation time** and returned **by + * reference**. Never construct one inside `project()` or `render()` from live + * state: `Resolution.ready(v)` allocates, and a fresh identity per render is a + * deck layer teardown per frame — which is exactly the pan-flash this whole + * design exists to avoid. If you find yourself building a Resolution during + * render, the state belongs one phase earlier. + */ + +import type { Result } from '../types.js'; +import type { EntryNotice, SpatialEntryError, SpatialEntryErrorContext } from './errors.js'; +import { toSpatialEntryError } from './errors.js'; + +/** + * Progress of an in-flight load. + * + * `done` and `scanned` differ because producers routinely examine more than they + * keep: a feature scan reads every row group but retains only matching rows. + */ +export interface ResolutionProgress { + /** Rows/bytes retained so far. */ + readonly done: number; + /** Rows/bytes examined so far. `>= done` whenever the producer filters. */ + readonly scanned?: number; + /** The denominator, when it is known. It often isn't until the scan ends. */ + readonly total?: number; +} + +export type Resolution = + | { readonly status: 'idle' } + | { + readonly status: 'loading'; + /** What *this* load has produced so far — the streaming scan's growing buffer. */ + readonly partial?: T; + /** The last good value from the *previous* load. See "stale is a retention" above. */ + readonly stale?: T; + readonly progress?: ResolutionProgress; + } + | { readonly status: 'ready'; readonly value: T; readonly notices?: readonly EntryNotice[] } + | { readonly status: 'failed'; readonly error: SpatialEntryError; readonly stale?: T }; + +const IDLE: Resolution = Object.freeze({ status: 'idle' as const }); + +/** + * Constructors and guards for {@link Resolution}. + * + * Namespaced deliberately: bare `ready` / `failed` / `loading` are far too + * generic for `@spatialdata/core`'s top-level export surface. + */ +export const Resolution = { + /** One frozen singleton, so `idle` is identity-stable across renders. */ + idle: (): Resolution => IDLE as Resolution, + + loading: (o?: { partial?: T; stale?: T; progress?: ResolutionProgress }): Resolution => ({ + status: 'loading', + ...(o?.partial !== undefined ? { partial: o.partial } : {}), + ...(o?.stale !== undefined ? { stale: o.stale } : {}), + ...(o?.progress !== undefined ? { progress: o.progress } : {}), + }), + + ready: (value: T, notices?: readonly EntryNotice[]): Resolution => ({ + status: 'ready', + value, + ...(notices !== undefined && notices.length > 0 ? { notices } : {}), + }), + + failed: (error: SpatialEntryError, stale?: T): Resolution => ({ + status: 'failed', + error, + ...(stale !== undefined ? { stale } : {}), + }), + + isIdle: (r: Resolution): r is Extract, { status: 'idle' }> => + r.status === 'idle', + isLoading: (r: Resolution): r is Extract, { status: 'loading' }> => + r.status === 'loading', + isReady: (r: Resolution): r is Extract, { status: 'ready' }> => + r.status === 'ready', + isFailed: (r: Resolution): r is Extract, { status: 'failed' }> => + r.status === 'failed', + + /** The settled value, and only that. `undefined` while loading, even if `stale` exists. */ + readyValue: (r: Resolution): T | undefined => (r.status === 'ready' ? r.value : undefined), + + /** + * The newest drawable value: `ready.value`, else a retained `stale`. + * + * This is what a base layer draws. Note it is deliberately **not** unioned with + * `partial` — see below. + */ + lastGood: (r: Resolution): T | undefined => { + if (r.status === 'ready') return r.value; + if (r.status === 'loading' || r.status === 'failed') return r.stale; + return undefined; + }, + + /** The in-flight load's growing buffer, if it has produced one. */ + partialValue: (r: Resolution): T | undefined => + r.status === 'loading' ? r.partial : undefined, +} as const; + +// There is deliberately no `Resolution.valueOf()` collapsing lastGood ?? partial. +// The points render path draws `lastGood` as the base layer AND `partial` as a +// separate overlay sub-layer, simultaneously. A helper that merged them would +// destroy the exact distinction the render path depends on. + +/** + * Lift a `Result` into a `Resolution`. + * + * This is why `Resolution` lives in `core`: the `Result` it lifts is already + * here. Its one producer on this path is `AbstractSpatialElement.getTransformation()`, + * which returns `Result` — and + * that error class is the one input `toSpatialEntryError` classifies losslessly. + */ +export function fromResult( + result: Result, + ctx: SpatialEntryErrorContext +): Resolution { + return result.ok + ? Resolution.ready(result.value) + : Resolution.failed(toSpatialEntryError(result.error, ctx)); +} diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 6489cc6e..48cebfde 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -4,6 +4,8 @@ * Core library for interfacing with SpatialData stores in TypeScript/JavaScript */ +// Resource Resolver contracts (ADR 0004). +export * from './engine/index.js'; export * from './models/index.js'; export { type GeopandasGeoParquetMetadata, diff --git a/packages/core/tests/resolution.spec.ts b/packages/core/tests/resolution.spec.ts new file mode 100644 index 00000000..bcc1c6a8 --- /dev/null +++ b/packages/core/tests/resolution.spec.ts @@ -0,0 +1,158 @@ +import { describe, expect, it } from 'vitest'; +import { + fromResult, + Resolution, + type SpatialEntryErrorContext, + toSpatialEntryError, +} from '../src/engine/index.js'; +import { CoordinateSystemNotFoundError } from '../src/models/index.js'; +import { Err, Ok } from '../src/types.js'; + +const ctx: SpatialEntryErrorContext = { + elementKey: 'cells', + kind: 'shapes', + resource: 'geometry', + fallback: 'load-failed', +}; + +const anError = () => toSpatialEntryError(new Error('boom'), ctx); + +describe('Resolution — identity', () => { + // The identity rule is load-bearing: a fresh identity per render is a deck layer + // teardown per frame, i.e. the pan flash this design exists to avoid. + it('idle() is one frozen singleton, so it never churns identity', () => { + expect(Resolution.idle()).toBe(Resolution.idle()); + expect(Object.isFrozen(Resolution.idle())).toBe(true); + }); + + it('ready() returns the value BY REFERENCE — it does not copy', () => { + const value = { rows: new Float32Array([1, 2, 3]) }; + + const resolved = Resolution.ready(value); + + expect(Resolution.readyValue(resolved)).toBe(value); + }); + + it('every other constructor allocates — which is why they are called at mutation time only', () => { + // Documented so nobody "optimises" by calling ready() during render. + expect(Resolution.ready(1)).not.toBe(Resolution.ready(1)); + }); +}); + +describe('Resolution — the four states', () => { + it('idle carries nothing', () => { + expect(Resolution.idle().status).toBe('idle'); + expect(Resolution.readyValue(Resolution.idle())).toBeUndefined(); + expect(Resolution.lastGood(Resolution.idle())).toBeUndefined(); + }); + + it('loading may carry partial, stale and progress — and omits absent keys entirely', () => { + const bare = Resolution.loading(); + expect(bare).toEqual({ status: 'loading' }); + expect('partial' in bare).toBe(false); + expect('stale' in bare).toBe(false); + + const full = Resolution.loading({ + partial: 7, + stale: 3, + progress: { done: 7, scanned: 900, total: 1000 }, + }); + if (full.status !== 'loading') throw new Error('narrowing'); + expect(full.partial).toBe(7); + expect(full.stale).toBe(3); + // done vs scanned: a feature scan reads every row group but keeps only matches. + expect(full.progress).toEqual({ done: 7, scanned: 900, total: 1000 }); + }); + + it('ready omits an empty notices array rather than carrying it', () => { + expect(Resolution.ready(1, [])).toEqual({ status: 'ready', value: 1 }); + }); + + it('ready carries notices — healthy data with a caveat is NOT an error', () => { + const notice = { + kind: 'preload-truncated', + message: 'Showing 4,000,000 of 9,000,000 points', + loaded: 4_000_000, + total: 9_000_000, + } as const; + + const resolved = Resolution.ready('data', [notice]); + + expect(Resolution.isReady(resolved)).toBe(true); + if (resolved.status !== 'ready') throw new Error('narrowing'); + expect(resolved.notices).toEqual([notice]); + }); + + it('failed carries the error, and may retain a stale value', () => { + const error = anError(); + + expect(Resolution.failed(error)).toEqual({ status: 'failed', error }); + expect(Resolution.failed(error, 'previous')).toEqual({ + status: 'failed', + error, + stale: 'previous', + }); + }); +}); + +describe('Resolution — accessors', () => { + it('readyValue is settled-only: a stale value is not a ready value', () => { + expect(Resolution.readyValue(Resolution.loading({ stale: 'old' }))).toBeUndefined(); + expect(Resolution.readyValue(Resolution.failed(anError(), 'old'))).toBeUndefined(); + expect(Resolution.readyValue(Resolution.ready('new'))).toBe('new'); + }); + + it('lastGood keeps drawing across a refine, and across a failure that retains stale', () => { + expect(Resolution.lastGood(Resolution.ready('new'))).toBe('new'); + expect(Resolution.lastGood(Resolution.loading({ stale: 'old' }))).toBe('old'); + expect(Resolution.lastGood(Resolution.failed(anError(), 'old'))).toBe('old'); + }); + + it('lastGood is undefined when stale was NOT retained — callers must handle this', () => { + // "stale is a retention, not a guarantee." A previously-ready resource may + // become undrawable; the UI shows the Spatial Entry Error instead. + expect(Resolution.lastGood(Resolution.failed(anError()))).toBeUndefined(); + expect(Resolution.lastGood(Resolution.loading())).toBeUndefined(); + }); + + it('keeps partial and lastGood SEPARATE — the render path draws both at once', () => { + // Points draws lastGood as the base layer and partial as an overlay sub-layer. + // Any helper collapsing these two would destroy that distinction, which is why + // there is deliberately no `Resolution.valueOf()`. + const scanning = Resolution.loading({ partial: 'growing', stale: 'old' }); + + expect(Resolution.lastGood(scanning)).toBe('old'); + expect(Resolution.partialValue(scanning)).toBe('growing'); + }); + + it('partialValue is loading-only', () => { + expect(Resolution.partialValue(Resolution.ready('v'))).toBeUndefined(); + expect(Resolution.partialValue(Resolution.failed(anError()))).toBeUndefined(); + }); + + it('guards narrow', () => { + const r: Resolution = Resolution.ready(1); + expect(Resolution.isReady(r)).toBe(true); + expect(Resolution.isIdle(r)).toBe(false); + expect(Resolution.isLoading(r)).toBe(false); + expect(Resolution.isFailed(r)).toBe(false); + }); +}); + +describe('fromResult', () => { + // This is why Resolution lives in core: the Result it lifts is already here. + it('lifts Ok into ready', () => { + expect(fromResult(Ok(42), ctx)).toEqual({ status: 'ready', value: 42 }); + }); + + it('lifts Err through the classifier — losslessly, for the one typed case', () => { + const cause = new CoordinateSystemNotFoundError('aligned', 'cells', ['global']); + + const resolved = fromResult(Err(cause), ctx); + + expect(Resolution.isFailed(resolved)).toBe(true); + if (resolved.status !== 'failed') throw new Error('narrowing'); + expect(resolved.error.kind).toBe('coordinate-system-not-found'); + expect(resolved.error.retryable).toBe(false); + }); +}); diff --git a/packages/core/tests/spatialEntryError.spec.ts b/packages/core/tests/spatialEntryError.spec.ts new file mode 100644 index 00000000..05bfcf68 --- /dev/null +++ b/packages/core/tests/spatialEntryError.spec.ts @@ -0,0 +1,160 @@ +import { describe, expect, it } from 'vitest'; +import { + isCancellation, + type SpatialEntryErrorContext, + toSpatialEntryError, +} from '../src/engine/index.js'; +import { CoordinateSystemNotFoundError } from '../src/models/index.js'; +import { PointsPreloadTooLargeError } from '../src/pointsLimits.js'; + +const ctx = (over: Partial = {}): SpatialEntryErrorContext => ({ + elementKey: 'transcripts', + kind: 'points', + resource: 'preload', + fallback: 'load-failed', + ...over, +}); + +describe('toSpatialEntryError — tier 1: instanceof (the only lossless tier)', () => { + it('carries CoordinateSystemNotFoundError payload through, and is not retryable', () => { + const cause = new CoordinateSystemNotFoundError('aligned', 'cells', ['global', 'micron']); + + const error = toSpatialEntryError(cause, ctx({ kind: 'shapes', resource: 'geometry' })); + + expect(error.kind).toBe('coordinate-system-not-found'); + expect(error.retryable).toBe(false); + // The whole point of the typed tier: the UI can explain itself. + if (error.kind !== 'coordinate-system-not-found') throw new Error('narrowing'); + expect(error.coordinateSystem).toBe('aligned'); + expect(error.elementKey).toBe('cells'); + expect(error.availableCoordinateSystems).toEqual(['global', 'micron']); + }); + + it('beats the fallback — a typed cause is never degraded to ctx.fallback', () => { + const cause = new CoordinateSystemNotFoundError('aligned', 'cells', []); + + const error = toSpatialEntryError(cause, ctx({ fallback: 'decode-failed' })); + + expect(error.kind).toBe('coordinate-system-not-found'); + }); + + it('carries PointsPreloadTooLargeError counts through', () => { + const error = toSpatialEntryError(new PointsPreloadTooLargeError(9_000_000, 4_000_000), ctx()); + + expect(error.kind).toBe('points-preload-too-large'); + expect(error.retryable).toBe(false); + if (error.kind !== 'points-preload-too-large') throw new Error('narrowing'); + expect(error.rowCount).toBe(9_000_000); + expect(error.maxRows).toBe(4_000_000); + }); +}); + +describe('toSpatialEntryError — tier 2: the one quarantined recogniser', () => { + // worker-unavailable is thrown from inside seams whose fallback is decode-failed, + // so context alone cannot reach it. This is the only sanctioned message-sniff. + it('recognises worker-unavailable even when the seam nominated decode-failed', () => { + const cause = new Error('readParquetRowGroup is unavailable'); + + const error = toSpatialEntryError(cause, ctx({ fallback: 'decode-failed' })); + + expect(error.kind).toBe('worker-unavailable'); + expect(error.retryable).toBe(true); + }); + + it('recognises the points-worker variant of the same message', () => { + const cause = new Error('parquet-wasm readParquetRowGroup is unavailable in points worker'); + + expect(toSpatialEntryError(cause, ctx({ fallback: 'decode-failed' })).kind).toBe( + 'worker-unavailable' + ); + }); + + it('does not fire on an unrelated "unavailable"', () => { + const cause = new Error('the network is unavailable'); + + expect(toSpatialEntryError(cause, ctx({ fallback: 'decode-failed' })).kind).toBe( + 'decode-failed' + ); + }); +}); + +describe('toSpatialEntryError — tier 3: the seam decides', () => { + // The load-bearing claim of this module: a bare `throw new Error(string)` carries + // no type, and anything off the worker carries even less (its failure channel is + // `{ ok: false; error: string }`). So the SEAM says what it was doing. + it('classifies an untyped throw by ctx.fallback, not by its message', () => { + const cause = new Error('something went wrong deep in a codec'); + + expect(toSpatialEntryError(cause, ctx({ fallback: 'decode-failed' })).kind).toBe( + 'decode-failed' + ); + expect(toSpatialEntryError(cause, ctx({ fallback: 'unsupported-format' })).kind).toBe( + 'unsupported-format' + ); + expect(toSpatialEntryError(cause, ctx({ fallback: 'element-not-found' })).kind).toBe( + 'element-not-found' + ); + }); + + it('survives a non-Error throw', () => { + const error = toSpatialEntryError('just a string', ctx({ fallback: 'load-failed' })); + + expect(error.kind).toBe('load-failed'); + expect(error.message).toBe('just a string'); + }); + + it('preserves the cause for logs, and the resource for the message', () => { + const cause = new Error('boom'); + + const error = toSpatialEntryError(cause, ctx({ resource: 'tooltip', fallback: 'load-failed' })); + + expect(error.cause).toBe(cause); + if (error.kind !== 'load-failed') throw new Error('narrowing'); + expect(error.resource).toBe('tooltip'); + }); + + it('sets retryable per kind — this, not the union, is what enables Retry', () => { + const cause = new Error('boom'); + + expect(toSpatialEntryError(cause, ctx({ fallback: 'decode-failed' })).retryable).toBe(true); + expect(toSpatialEntryError(cause, ctx({ fallback: 'worker-unavailable' })).retryable).toBe( + true + ); + expect(toSpatialEntryError(cause, ctx({ fallback: 'load-failed' })).retryable).toBe(true); + // A missing element and a format we can't read will not fix themselves. + expect(toSpatialEntryError(cause, ctx({ fallback: 'element-not-found' })).retryable).toBe( + false + ); + expect(toSpatialEntryError(cause, ctx({ fallback: 'unsupported-format' })).retryable).toBe( + false + ); + }); +}); + +describe('isCancellation — the gate before the classifier', () => { + // Cancellation is a non-event, not a domain failure: superseding a load is normal + // operation. There is deliberately no `cancelled` case in SpatialEntryError, so + // seams MUST check this first — otherwise every memory-cap drag paints an error. + it('recognises a DOMException AbortError', () => { + expect(isCancellation(new DOMException('Aborted', 'AbortError'))).toBe(true); + }); + + it('recognises an AbortError-shaped plain error', () => { + const e = new Error('Aborted'); + e.name = 'AbortError'; + expect(isCancellation(e)).toBe(true); + }); + + it('recognises the signal an AbortController actually produces', () => { + const controller = new AbortController(); + controller.abort(); + expect(isCancellation(controller.signal.reason)).toBe(true); + }); + + it('does not swallow a real failure', () => { + expect(isCancellation(new Error('decode failed'))).toBe(false); + expect(isCancellation('nope')).toBe(false); + expect(isCancellation(undefined)).toBe(false); + expect(isCancellation(null)).toBe(false); + }); +}); From 3b6cda17ef34785880274b2d89d6d452f1eb1ec9 Mon Sep 17 00:00:00 2001 From: Peter Todd Date: Tue, 14 Jul 2026 18:56:42 +0100 Subject: [PATCH 3/9] Move RenderStack and SpatialLayerProps schemas to core MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ADR 0004 §5, amending ADR 0001. Forced by dependency direction: the Resource Resolver takes a Render Stack as input, and it lives in core. This also lands the schemas where they already wanted to be — both are zod persistence schemas that `vis` re-exported verbatim because MDV consumes them as a *data contract*, and core already owns schemas/ and depends on zod. Mechanical: both files imported nothing but zod. The migration contract from ADR 0004 §5 is honoured — core becomes canonical, and `layers` (and through it `vis`) retains its re-exports as compatibility shims. No consumer import path moves. Removing the shims is a separate, deliberate deprecation coordinated with MDV, and is not part of this work. tests/renderStackShim.spec.ts guards that promise, and guards it by *identity*, not by name: an `export … from` chain still typechecks if someone later re-declares a schema locally, and that drift would only surface downstream in MDV. Thirteen values, asserted `layers[x] === core[x]`. The two existing specs move to core/tests/ with the schemas. Build, full suite (441) and biome green. Both dependency-boundary DoD boxes still hold: core imports no react/deck.gl/viv, layers imports no react. Co-Authored-By: Claude Opus 4.8 --- packages/core/src/index.ts | 6 ++ packages/{layers => core}/src/renderStack.ts | 0 .../{layers => core}/src/spatialLayerProps.ts | 0 packages/core/tests/badFiles.spec.ts | 7 +-- .../core/tests/mortonPointsTiling.spec.ts | 14 ++--- .../core/tests/parquetFooterStats.spec.ts | 2 +- packages/core/tests/pointsFeatures.spec.ts | 4 +- packages/core/tests/pointsLoader.spec.ts | 37 ++++++------ .../core/tests/pointsPreloadGuard.spec.ts | 7 +-- .../tests/pointsPreloadReadStrategy.spec.ts | 8 +-- packages/core/tests/pointsWorker.spec.ts | 7 +-- packages/core/tests/pointsWorkerScan.spec.ts | 5 +- .../tests/renderStack.spec.ts | 8 ++- packages/core/tests/schemas.spec.ts | 6 +- packages/core/tests/shapesRenderData.spec.ts | 18 +++++- .../tests/spatialLayerProps.spec.ts | 8 +-- packages/core/tests/spatialViewFit.spec.ts | 4 +- packages/core/tests/tableAssociations.spec.ts | 4 +- packages/core/tests/transformations.spec.ts | 4 +- packages/core/tests/vshapes.spec.ts | 40 ++++++++++--- packages/core/tests/vtableMultipart.spec.ts | 7 ++- packages/layers/src/SpatialLayer.ts | 4 +- packages/layers/src/index.ts | 57 ++++++++++-------- packages/layers/src/shapesLayer.ts | 2 +- packages/layers/tests/labelsLayer.spec.ts | 28 +++------ .../layers/tests/pointsDataEngine.spec.ts | 52 +++++++++------- .../layers/tests/pointsFeatureColor.spec.ts | 2 +- .../layers/tests/pointsLayerFilter.spec.ts | 2 +- packages/layers/tests/pointsLoadPlan.spec.ts | 3 +- .../tests/pointsRenderAttributes.spec.ts | 2 +- .../tests/pointsRenderStrategies.spec.ts | 3 +- packages/layers/tests/renderStackShim.spec.ts | 59 +++++++++++++++++++ packages/layers/tests/shapesLayer.spec.ts | 19 +++--- 33 files changed, 266 insertions(+), 163 deletions(-) rename packages/{layers => core}/src/renderStack.ts (100%) rename packages/{layers => core}/src/spatialLayerProps.ts (100%) rename packages/{layers => core}/tests/renderStack.spec.ts (96%) rename packages/{layers => core}/tests/spatialLayerProps.spec.ts (96%) create mode 100644 packages/layers/tests/renderStackShim.spec.ts diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 48cebfde..303282ce 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -51,7 +51,13 @@ export type { PointsLoadResult, } from './pointsLoadOptions.js'; export * from './pointsTiling.js'; +// Render Stack schemas. Canonical here (ADR 0004 §5, amending ADR 0001): the +// Resource Resolver takes a Render Stack as input, so dependency direction forces +// the move. `layers` and `vis` retain their re-exports as compatibility shims — +// MDV consumes these as a data contract and no consumer import moves. +export * from './renderStack.js'; export * from './shapes.js'; +export * from './spatialLayerProps.js'; export * from './spatialViewFit.js'; export * from './store/index.js'; export * from './tableAssociations.js'; diff --git a/packages/layers/src/renderStack.ts b/packages/core/src/renderStack.ts similarity index 100% rename from packages/layers/src/renderStack.ts rename to packages/core/src/renderStack.ts diff --git a/packages/layers/src/spatialLayerProps.ts b/packages/core/src/spatialLayerProps.ts similarity index 100% rename from packages/layers/src/spatialLayerProps.ts rename to packages/core/src/spatialLayerProps.ts diff --git a/packages/core/tests/badFiles.spec.ts b/packages/core/tests/badFiles.spec.ts index e84ebd3d..16815508 100644 --- a/packages/core/tests/badFiles.spec.ts +++ b/packages/core/tests/badFiles.spec.ts @@ -1,6 +1,6 @@ import { describe, expect, it, vi } from 'vitest'; -import { ATTRS_KEY } from 'zarrextra'; import type { ConsolidatedStore } from 'zarrextra'; +import { ATTRS_KEY } from 'zarrextra'; import type * as zarr from 'zarrita'; import { SpatialData } from '../src/store/index.js'; @@ -40,10 +40,7 @@ describe('SpatialData bad-file handling', () => { expect(sdata.images).toEqual({}); expect(onBadFiles).toHaveBeenCalledTimes(1); - expect(onBadFiles).toHaveBeenCalledWith( - 'images/broken_image', - expect.any(Error) - ); + expect(onBadFiles).toHaveBeenCalledWith('images/broken_image', expect.any(Error)); expect(consoleErrorSpy).not.toHaveBeenCalled(); } finally { consoleErrorSpy.mockRestore(); diff --git a/packages/core/tests/mortonPointsTiling.spec.ts b/packages/core/tests/mortonPointsTiling.spec.ts index f85ad4e8..00f81bab 100644 --- a/packages/core/tests/mortonPointsTiling.spec.ts +++ b/packages/core/tests/mortonPointsTiling.spec.ts @@ -1,7 +1,7 @@ import { execSync } from 'node:child_process'; -import { mkdtemp, readFile, writeFile, mkdir, rm } from 'node:fs/promises'; +import { mkdir, mkdtemp, readFile, rm, writeFile } from 'node:fs/promises'; import { tmpdir } from 'node:os'; -import { join, dirname } from 'node:path'; +import { dirname, join } from 'node:path'; import { fileURLToPath } from 'node:url'; import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'; import SpatialDataPointsSource from '../src/models/VPointsSource.js'; @@ -13,10 +13,7 @@ const writerRoot = join(projectRoot, 'python/spatialdata-experimental-writer'); async function writeSyntheticPointsZarr(root: string) { const elementDir = join(root, 'points', 'transcripts'); await mkdir(elementDir, { recursive: true }); - await writeFile( - join(root, 'zarr.json'), - JSON.stringify({ zarr_format: 3, node_type: 'group' }) - ); + await writeFile(join(root, 'zarr.json'), JSON.stringify({ zarr_format: 3, node_type: 'group' })); await writeFile( join(elementDir, 'zarr.json'), JSON.stringify({ @@ -63,10 +60,7 @@ PY`, async function writeBadSentinelMortonPointsZarr(root: string) { const elementDir = join(root, 'points', 'transcripts'); await mkdir(elementDir, { recursive: true }); - await writeFile( - join(root, 'zarr.json'), - JSON.stringify({ zarr_format: 3, node_type: 'group' }) - ); + await writeFile(join(root, 'zarr.json'), JSON.stringify({ zarr_format: 3, node_type: 'group' })); await writeFile( join(elementDir, 'zarr.json'), JSON.stringify({ diff --git a/packages/core/tests/parquetFooterStats.spec.ts b/packages/core/tests/parquetFooterStats.spec.ts index 1ce0ac76..fc70c5bb 100644 --- a/packages/core/tests/parquetFooterStats.spec.ts +++ b/packages/core/tests/parquetFooterStats.spec.ts @@ -6,8 +6,8 @@ import { fileURLToPath } from 'node:url'; import { afterAll, beforeAll, describe, expect, it } from 'vitest'; import { decodeIntStat, - parseParquetFileMetaData, ParquetPhysicalType, + parseParquetFileMetaData, } from '../src/parquetFooterStats.js'; const __dirname = dirname(fileURLToPath(import.meta.url)); diff --git a/packages/core/tests/pointsFeatures.spec.ts b/packages/core/tests/pointsFeatures.spec.ts index 61d7a2bb..df6777f1 100644 --- a/packages/core/tests/pointsFeatures.spec.ts +++ b/packages/core/tests/pointsFeatures.spec.ts @@ -1,7 +1,7 @@ import { execSync } from 'node:child_process'; -import { mkdtemp, readFile, rm, mkdir, stat } from 'node:fs/promises'; +import { mkdir, mkdtemp, readFile, rm, stat } from 'node:fs/promises'; import { tmpdir } from 'node:os'; -import { join, dirname } from 'node:path'; +import { dirname, join } from 'node:path'; import { fileURLToPath } from 'node:url'; import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'; import SpatialDataPointsSource from '../src/models/VPointsSource.js'; diff --git a/packages/core/tests/pointsLoader.spec.ts b/packages/core/tests/pointsLoader.spec.ts index 7c7a04c3..e0668569 100644 --- a/packages/core/tests/pointsLoader.spec.ts +++ b/packages/core/tests/pointsLoader.spec.ts @@ -1,29 +1,32 @@ import { describe, expect, it } from 'vitest'; - -import { createMortonTiledPointsLoader, resolvePointsEncoding } from '../src/pointsLoader.js'; import type { PointsElement } from '../src/models/index.js'; +import { createMortonTiledPointsLoader, resolvePointsEncoding } from '../src/pointsLoader.js'; describe('resolvePointsEncoding', () => { it('prefers preloaded data when present', () => { - expect( - resolvePointsEncoding({ shape: [1], data: [[0], [0]] }, null, true) - ).toBe('preloaded-columnar'); + expect(resolvePointsEncoding({ shape: [1], data: [[0], [0]] }, null, true)).toBe( + 'preloaded-columnar' + ); }); it('selects morton tiling when metadata supports row-group reads', () => { expect( - resolvePointsEncoding(null, { - kind: 'morton-points', - parquetPath: 'points/a/points.parquet', - axisNames: ['x', 'y'], - featureCodeColumnName: 'feature_name_codes', - mortonCodeColumnName: 'morton_code_2d', - totalRows: 10, - totalRowGroups: 1, - maxRowsPerGroup: 10, - supportsRowGroupRangeReads: true, - bounds: { minX: 0, minY: 0, maxX: 10, maxY: 10 }, - }, true) + resolvePointsEncoding( + null, + { + kind: 'morton-points', + parquetPath: 'points/a/points.parquet', + axisNames: ['x', 'y'], + featureCodeColumnName: 'feature_name_codes', + mortonCodeColumnName: 'morton_code_2d', + totalRows: 10, + totalRowGroups: 1, + maxRowsPerGroup: 10, + supportsRowGroupRangeReads: true, + bounds: { minX: 0, minY: 0, maxX: 10, maxY: 10 }, + }, + true + ) ).toBe('morton-tiled'); }); }); diff --git a/packages/core/tests/pointsPreloadGuard.spec.ts b/packages/core/tests/pointsPreloadGuard.spec.ts index 5ef7596e..2500d414 100644 --- a/packages/core/tests/pointsPreloadGuard.spec.ts +++ b/packages/core/tests/pointsPreloadGuard.spec.ts @@ -1,10 +1,7 @@ -import { describe, expect, it, vi } from 'vitest'; import { tableFromArrays } from 'apache-arrow'; +import { describe, expect, it, vi } from 'vitest'; import SpatialDataPointsSource from '../src/models/VPointsSource.js'; -import { - POINTS_PRELOAD_MAX_ROWS, - preloadedColumnarPointCount, -} from '../src/pointsLimits.js'; +import { POINTS_PRELOAD_MAX_ROWS, preloadedColumnarPointCount } from '../src/pointsLimits.js'; describe('points preload cap', () => { it('loads a capped subset when parquet row count exceeds the cap', async () => { diff --git a/packages/core/tests/pointsPreloadReadStrategy.spec.ts b/packages/core/tests/pointsPreloadReadStrategy.spec.ts index 9c51c215..8f6998c9 100644 --- a/packages/core/tests/pointsPreloadReadStrategy.spec.ts +++ b/packages/core/tests/pointsPreloadReadStrategy.spec.ts @@ -1,5 +1,5 @@ -import { describe, expect, it, vi } from 'vitest'; import { tableFromArrays } from 'apache-arrow'; +import { describe, expect, it, vi } from 'vitest'; import SpatialDataPointsSource from '../src/models/VPointsSource.js'; import * as pointsWorkerClient from '../src/workers/pointsWorkerClient.js'; @@ -66,10 +66,6 @@ describe('points preload read strategy', () => { await source.loadPoints('points/transcripts'); - expect(cappedSpy).toHaveBeenCalledWith( - 'points/transcripts/points.parquet', - ['x', 'y'], - 100 - ); + expect(cappedSpy).toHaveBeenCalledWith('points/transcripts/points.parquet', ['x', 'y'], 100); }); }); diff --git a/packages/core/tests/pointsWorker.spec.ts b/packages/core/tests/pointsWorker.spec.ts index 329cb4a5..0f25fd30 100644 --- a/packages/core/tests/pointsWorker.spec.ts +++ b/packages/core/tests/pointsWorker.spec.ts @@ -1,4 +1,5 @@ import { afterEach, describe, expect, it, vi } from 'vitest'; +import { filterColumnarByFeatureCodes as filterSync } from '../src/pointsTiling.js'; import { decodeParquetRowFeatureCodesInWorker, disablePointsWorker, @@ -8,17 +9,13 @@ import { setPointsWorkerDefaultEnabled, setPointsWorkerRequestTimeout, } from '../src/workers/pointsWorkerClient.js'; -import { filterColumnarByFeatureCodes as filterSync } from '../src/pointsTiling.js'; describe('points worker client', () => { it('falls back to main-thread filtering when the worker is disabled', async () => { setPointsWorkerDefaultEnabled(false); const data = { shape: [2, 4] as [number, number], - data: [ - Float32Array.from([0, 1, 2, 3]), - Float32Array.from([0, 1, 2, 3]), - ], + data: [Float32Array.from([0, 1, 2, 3]), Float32Array.from([0, 1, 2, 3])], }; const sourceFeatureCodes = Int32Array.from([0, 1, 0, 2]); const filtered = await filterColumnarByFeatureCodesInWorker(data, [1], sourceFeatureCodes); diff --git a/packages/core/tests/pointsWorkerScan.spec.ts b/packages/core/tests/pointsWorkerScan.spec.ts index 316dd794..0018ff90 100644 --- a/packages/core/tests/pointsWorkerScan.spec.ts +++ b/packages/core/tests/pointsWorkerScan.spec.ts @@ -11,7 +11,10 @@ import { const throwingReadParquet = (() => { throw new Error('readParquet should not be called on the rowGroup path'); -}) as unknown as (bytes: Uint8Array, options?: { columns?: string[] }) => { +}) as unknown as ( + bytes: Uint8Array, + options?: { columns?: string[] } +) => { intoIPCStream(): Uint8Array; }; diff --git a/packages/layers/tests/renderStack.spec.ts b/packages/core/tests/renderStack.spec.ts similarity index 96% rename from packages/layers/tests/renderStack.spec.ts rename to packages/core/tests/renderStack.spec.ts index 9e989ab1..12641edc 100644 --- a/packages/layers/tests/renderStack.spec.ts +++ b/packages/core/tests/renderStack.spec.ts @@ -7,7 +7,7 @@ import { renderStackHostEntrySchema, renderStackSchema, renderStackSpatialEntrySchema, -} from '../src/renderStack'; +} from '../src/renderStack.js'; describe('renderStackSchema', () => { it('parses current render stacks', () => { @@ -150,7 +150,11 @@ describe('render-stack entry schemas', () => { ], }); - expect(getRenderStackEntryIds(stack)).toEqual(['deck:scatter', 'labels-cells', 'deck:selection']); + expect(getRenderStackEntryIds(stack)).toEqual([ + 'deck:scatter', + 'labels-cells', + 'deck:selection', + ]); expect(getRenderStackHostLayerIds(stack)).toEqual(['deck:scatter', 'deck:selection']); }); }); diff --git a/packages/core/tests/schemas.spec.ts b/packages/core/tests/schemas.spec.ts index d15570e0..a2b6886f 100644 --- a/packages/core/tests/schemas.spec.ts +++ b/packages/core/tests/schemas.spec.ts @@ -1,11 +1,11 @@ import { describe, expect, it } from 'vitest'; import { - rasterAttrsSchema, coordinateTransformationSchema, - shapesAttrsSchema, pointsAttrsSchema, - tableAttrsSchema, + rasterAttrsSchema, + shapesAttrsSchema, spatialDataSchema, + tableAttrsSchema, } from '../src/schemas/index.js'; describe('Schema Transformations', () => { diff --git a/packages/core/tests/shapesRenderData.spec.ts b/packages/core/tests/shapesRenderData.spec.ts index 412a5c5a..e83b654e 100644 --- a/packages/core/tests/shapesRenderData.spec.ts +++ b/packages/core/tests/shapesRenderData.spec.ts @@ -120,8 +120,22 @@ describe('loadFeatureRowIndexByFeatureIndex', () => { elementKey: 'cells', featureIds: ['cell-1', 'cell-2'], polygons: [ - [[[0, 0], [1, 0], [1, 1], [0, 0]]], - [[[2, 2], [3, 2], [3, 3], [2, 2]]], + [ + [ + [0, 0], + [1, 0], + [1, 1], + [0, 0], + ], + ], + [ + [ + [2, 2], + [3, 2], + [3, 3], + [2, 2], + ], + ], ], rowIndexByFeatureIndex: new Int32Array(2).fill(-1), }); diff --git a/packages/layers/tests/spatialLayerProps.spec.ts b/packages/core/tests/spatialLayerProps.spec.ts similarity index 96% rename from packages/layers/tests/spatialLayerProps.spec.ts rename to packages/core/tests/spatialLayerProps.spec.ts index 60e923f1..f3967848 100644 --- a/packages/layers/tests/spatialLayerProps.spec.ts +++ b/packages/core/tests/spatialLayerProps.spec.ts @@ -1,10 +1,10 @@ import { describe, expect, it } from 'vitest'; import { - SPATIAL_LAYER_PROPS_SCHEMA_VERSION, migrateSpatialLayerProps, + SPATIAL_LAYER_PROPS_SCHEMA_VERSION, spatialLayerPropsSchema, spatialShapesSublayerSchema, -} from '../src/spatialLayerProps'; +} from '../src/spatialLayerProps.js'; describe('migrateSpatialLayerProps', () => { it('parses current version unchanged', () => { @@ -50,9 +50,7 @@ describe('migrateSpatialLayerProps', () => { }); expect(result.success).toBe(false); if (!result.success) { - expect(result.error.issues.some((issue) => issue.message.includes('must be <='))).toBe( - true - ); + expect(result.error.issues.some((issue) => issue.message.includes('must be <='))).toBe(true); } }); diff --git a/packages/core/tests/spatialViewFit.spec.ts b/packages/core/tests/spatialViewFit.spec.ts index e365ec97..fa1acda2 100644 --- a/packages/core/tests/spatialViewFit.spec.ts +++ b/packages/core/tests/spatialViewFit.spec.ts @@ -1,10 +1,10 @@ import { Matrix4 } from '@math.gl/core'; import { describe, expect, it } from 'vitest'; import { - DEFAULT_ZOOM_BACK_OFF, - boundsFromPoints, boundsFromCircles, + boundsFromPoints, boundsFromPolygons, + DEFAULT_ZOOM_BACK_OFF, unionBounds, unionBoundsList, viewStateFromBounds, diff --git a/packages/core/tests/tableAssociations.spec.ts b/packages/core/tests/tableAssociations.spec.ts index 1f593e8e..32c7d10a 100644 --- a/packages/core/tests/tableAssociations.spec.ts +++ b/packages/core/tests/tableAssociations.spec.ts @@ -1,6 +1,6 @@ -import { ATTRS_KEY } from 'zarrextra'; -import type { ConsolidatedStore } from 'zarrextra'; import { describe, expect, it } from 'vitest'; +import type { ConsolidatedStore } from 'zarrextra'; +import { ATTRS_KEY } from 'zarrextra'; import { SpatialData } from '../src/store/index.js'; import { createFeatureTableAlignment, diff --git a/packages/core/tests/transformations.spec.ts b/packages/core/tests/transformations.spec.ts index 7732635e..c7840692 100644 --- a/packages/core/tests/transformations.spec.ts +++ b/packages/core/tests/transformations.spec.ts @@ -1,11 +1,11 @@ import { describe, expect, it } from 'vitest'; import { Affine, + buildMatrix4FromTransforms, + composeTransforms, MapAxis, Rotation, Scale, - buildMatrix4FromTransforms, - composeTransforms, } from '../src/transformations/transformations.js'; describe('Affine', () => { diff --git a/packages/core/tests/vshapes.spec.ts b/packages/core/tests/vshapes.spec.ts index 0092e054..bf00b8d2 100644 --- a/packages/core/tests/vshapes.spec.ts +++ b/packages/core/tests/vshapes.spec.ts @@ -88,8 +88,22 @@ describe('SpatialDataShapesSource', () => { vi.spyOn(source, 'loadPolygonShapes').mockResolvedValue({ shape: [2, null], data: [ - [[[0, 0], [1, 0], [1, 1], [0, 0]]], - [[[2, 2], [3, 2], [3, 3], [2, 2]]], + [ + [ + [0, 0], + [1, 0], + [1, 1], + [0, 0], + ], + ], + [ + [ + [2, 2], + [3, 2], + [3, 3], + [2, 2], + ], + ], ], }); @@ -183,9 +197,7 @@ describe('SpatialDataShapesSource', () => { const invalidTable = { schema: { - metadata: new Map([ - ['geo', JSON.stringify({ primary_column: 123, columns: {} })], - ]), + metadata: new Map([['geo', JSON.stringify({ primary_column: 123, columns: {} })]]), }, } as any; @@ -289,8 +301,22 @@ describe('SpatialDataShapesSource', () => { vi.spyOn(source, 'loadPolygonShapes').mockResolvedValue({ shape: [2, null], data: [ - [[[0, 0], [1, 0], [1, 1], [0, 0]]], - [[[2, 2], [3, 2], [3, 3], [2, 2]]], + [ + [ + [0, 0], + [1, 0], + [1, 1], + [0, 0], + ], + ], + [ + [ + [2, 2], + [3, 2], + [3, 3], + [2, 2], + ], + ], ], }); diff --git a/packages/core/tests/vtableMultipart.spec.ts b/packages/core/tests/vtableMultipart.spec.ts index 36d567e2..a9636a88 100644 --- a/packages/core/tests/vtableMultipart.spec.ts +++ b/packages/core/tests/vtableMultipart.spec.ts @@ -1,7 +1,7 @@ import { execSync } from 'node:child_process'; import { mkdtemp, readFile, rm, stat } from 'node:fs/promises'; import { tmpdir } from 'node:os'; -import { join, dirname } from 'node:path'; +import { dirname, join } from 'node:path'; import { fileURLToPath } from 'node:url'; import { afterAll, beforeAll, describe, expect, it } from 'vitest'; import SpatialDataTableSource from '../src/models/VTableSource.js'; @@ -56,7 +56,10 @@ function createFilesystemStore(root: string) { const relativePath = path.startsWith('/') ? path.slice(1) : path; return readStoreBytes(relativePath); }, - async getRange(path: string, range: { offset?: number; length?: number; suffixLength?: number }) { + async getRange( + path: string, + range: { offset?: number; length?: number; suffixLength?: number } + ) { const relativePath = path.startsWith('/') ? path.slice(1) : path; const bytes = await readStoreBytes(relativePath); if (!bytes) { diff --git a/packages/layers/src/SpatialLayer.ts b/packages/layers/src/SpatialLayer.ts index 7f40175b..a8e8682d 100644 --- a/packages/layers/src/SpatialLayer.ts +++ b/packages/layers/src/SpatialLayer.ts @@ -1,11 +1,11 @@ +import type { SpatialLayerProps } from '@spatialdata/core'; +import { spatialLayerPropsSchema } from '@spatialdata/core'; import { CompositeLayer, type Layer, type LayersList } from 'deck.gl'; import { createShapesDeckLayer, type ShapesLayerPickEvent, type ShapesRenderDataLike, } from './shapesLayer'; -import type { SpatialLayerProps } from './spatialLayerProps'; -import { spatialLayerPropsSchema } from './spatialLayerProps'; export type { SpatialLayerProps }; diff --git a/packages/layers/src/index.ts b/packages/layers/src/index.ts index dd5a6971..47f26440 100644 --- a/packages/layers/src/index.ts +++ b/packages/layers/src/index.ts @@ -1,4 +1,35 @@ // Framework-agnostic points loading/caching engine (LayerDataEngine step 1b). + +// Compatibility shim (ADR 0004 §5). The Render Stack and SpatialLayerProps schemas +// are canonical in `@spatialdata/core`; these re-exports keep their existing import +// paths so no consumer — MDV especially — has to move. Removing the shim is a +// separate, deliberate deprecation, and is not part of the resolver work. +// Guarded by tests/renderStackShim.spec.ts, which asserts re-export by identity. +export type { + RenderStack, + RenderStackEntry, + RenderStackGroupEntry, + RenderStackHostEntry, + RenderStackSpatialElementType, + RenderStackSpatialEntry, + SpatialLayerProps, + SpatialShapesSublayer, +} from '@spatialdata/core'; +export { + getRenderStackEntryIds, + getRenderStackHostLayerIds, + migrateSpatialLayerProps, + RENDER_STACK_SCHEMA_VERSION, + renderStackEntrySchema, + renderStackGroupEntrySchema, + renderStackHostEntrySchema, + renderStackSchema, + renderStackSpatialElementTypeSchema, + renderStackSpatialEntrySchema, + SPATIAL_LAYER_PROPS_SCHEMA_VERSION, + spatialLayerPropsSchema, + spatialSublayerSchema, +} from '@spatialdata/core'; export { PointsDataEngine, type PointsDataEngineCallbacks, @@ -65,25 +96,6 @@ export { type TiledPointsDebugState, } from './pointsTiledDebugHooks.js'; export type { PointsTileHandle, PointsTileLoadResult } from './pointsTileLoadCallbacks.js'; -export type { - RenderStack, - RenderStackEntry, - RenderStackGroupEntry, - RenderStackHostEntry, - RenderStackSpatialElementType, - RenderStackSpatialEntry, -} from './renderStack'; -export { - getRenderStackEntryIds, - getRenderStackHostLayerIds, - RENDER_STACK_SCHEMA_VERSION, - renderStackEntrySchema, - renderStackGroupEntrySchema, - renderStackHostEntrySchema, - renderStackSchema, - renderStackSpatialElementTypeSchema, - renderStackSpatialEntrySchema, -} from './renderStack'; export * from './resolvePointsRenderResource.js'; export type { SpatialLayerRuntimeProps } from './SpatialLayer'; export { SpatialLayer } from './SpatialLayer'; @@ -127,10 +139,3 @@ export { type ShapeTooltipRuntimeData, type SpatialShapesRuntimeSublayer, } from './shapesLayer'; -export type { SpatialLayerProps, SpatialShapesSublayer } from './spatialLayerProps'; -export { - migrateSpatialLayerProps, - SPATIAL_LAYER_PROPS_SCHEMA_VERSION, - spatialLayerPropsSchema, - spatialSublayerSchema, -} from './spatialLayerProps'; diff --git a/packages/layers/src/shapesLayer.ts b/packages/layers/src/shapesLayer.ts index 9a7893ed..8fdd6507 100644 --- a/packages/layers/src/shapesLayer.ts +++ b/packages/layers/src/shapesLayer.ts @@ -1,6 +1,6 @@ import type { Matrix4 } from '@math.gl/core'; +import type { SpatialShapesSublayer } from '@spatialdata/core'; import { type Layer, type PickingInfo, PolygonLayer, ScatterplotLayer } from 'deck.gl'; -import type { SpatialShapesSublayer } from './spatialLayerProps'; export type ShapePolygon = Array>; diff --git a/packages/layers/tests/labelsLayer.spec.ts b/packages/layers/tests/labelsLayer.spec.ts index 5c9f88af..2b718dd1 100644 --- a/packages/layers/tests/labelsLayer.spec.ts +++ b/packages/layers/tests/labelsLayer.spec.ts @@ -1,6 +1,6 @@ import { describe, expect, it, vi } from 'vitest'; -import { LabelsLayer } from '../src/LabelsLayer'; import type { LabelsLayerProps } from '../src/LabelsLayer'; +import { LabelsLayer } from '../src/LabelsLayer'; type TileLayerLike = { props: { @@ -32,24 +32,14 @@ function makeLabelsLoader() { const makeScale = (resolution: number) => ({ shape: [512, 512], tileSize: 256, - getTile: vi.fn( - async ({ - x, - y, - selection, - }: { - x: number; - y: number; - selection: unknown; - }) => { - onGetTile({ resolution, x, y, selection }); - return { - data: new Float32Array([0, 1, 2, 0]), - width: 2, - height: 2, - }; - } - ), + getTile: vi.fn(async ({ x, y, selection }: { x: number; y: number; selection: unknown }) => { + onGetTile({ resolution, x, y, selection }); + return { + data: new Float32Array([0, 1, 2, 0]), + width: 2, + height: 2, + }; + }), }); return { loader: [makeScale(0), makeScale(1)], diff --git a/packages/layers/tests/pointsDataEngine.spec.ts b/packages/layers/tests/pointsDataEngine.spec.ts index a7e430cc..43c9acd2 100644 --- a/packages/layers/tests/pointsDataEngine.spec.ts +++ b/packages/layers/tests/pointsDataEngine.spec.ts @@ -1,12 +1,11 @@ -import { describe, expect, it, vi } from 'vitest'; - -import { PointsDataEngine } from '../src/engine/PointsDataEngine.js'; import { DEFAULT_POINTS_MEMORY_CAP, type PointsElement, type PointsFeatureCatalog, type PointsLoadResult, } from '@spatialdata/core'; +import { describe, expect, it, vi } from 'vitest'; +import { PointsDataEngine } from '../src/engine/PointsDataEngine.js'; function makeBatch(): PointsLoadResult { return { @@ -424,10 +423,7 @@ describe('PointsDataEngine — codes with the geometry preload', () => { const previewCatalog = sampleCatalog; const fullCatalog = { ...sampleCatalog, - entries: [ - ...sampleCatalog.entries, - { code: 99, name: 'LATE_PART_GENE', count: 7 }, - ], + entries: [...sampleCatalog.entries, { code: 99, name: 'LATE_PART_GENE', count: 7 }], }; const loadPoints = vi.fn(async () => ({ ...makeBatch(), @@ -575,7 +571,11 @@ describe('PointsDataEngine — codes with the geometry preload', () => { expect(engine.isFeatureCatalogLoading('pts:inflight')).toBe(true); expect(engine.getFeatureCatalog('pts:inflight')).toBeUndefined(); - resolveLoad({ ...makeBatch(), featureCatalog: sampleCatalog, featureCodes: new Int32Array([0]) }); + resolveLoad({ + ...makeBatch(), + featureCatalog: sampleCatalog, + featureCodes: new Int32Array([0]), + }); await p; expect(engine.isFeatureCatalogLoading('pts:inflight')).toBe(false); expect(engine.getFeatureCatalog('pts:inflight')).toEqual(sampleCatalog); @@ -639,12 +639,17 @@ describe('PointsDataEngine — matching resource (empty-lock guard)', () => { describe('PointsDataEngine — matched-selection subset reuse', () => { function scanElement(key: string) { - const loadPointsMatchingFeatureCodes = vi.fn(async (opts: { featureCodes: readonly number[] }) => ({ - shape: [2, opts.featureCodes.length], - data: [new Float32Array(opts.featureCodes.length), new Float32Array(opts.featureCodes.length)], - // Per-row codes the render uses to filter the batch in memory. - featureCodes: Int32Array.from(opts.featureCodes), - })); + const loadPointsMatchingFeatureCodes = vi.fn( + async (opts: { featureCodes: readonly number[] }) => ({ + shape: [2, opts.featureCodes.length], + data: [ + new Float32Array(opts.featureCodes.length), + new Float32Array(opts.featureCodes.length), + ], + // Per-row codes the render uses to filter the batch in memory. + featureCodes: Int32Array.from(opts.featureCodes), + }) + ); const element = { key, loadPoints: vi.fn(async () => makeBatch()), @@ -697,12 +702,14 @@ describe('PointsDataEngine — matched-selection subset reuse', () => { it('does not rescan when the cap is lowered and the loaded selection already fits', async () => { const engine = new PointsDataEngine(); // A COMPLETE batch: the scan found all matching rows before the cap. - const loadPointsMatchingFeatureCodes = vi.fn(async (opts: { featureCodes: readonly number[] }) => ({ - shape: [2, 500], - data: [new Float32Array(500), new Float32Array(500)], - featureCodes: Int32Array.from({ length: 500 }, () => opts.featureCodes[0]), - preloadTruncated: false, - })); + const loadPointsMatchingFeatureCodes = vi.fn( + async (opts: { featureCodes: readonly number[] }) => ({ + shape: [2, 500], + data: [new Float32Array(500), new Float32Array(500)], + featureCodes: Int32Array.from({ length: 500 }, () => opts.featureCodes[0]), + preloadTruncated: false, + }) + ); const element = { key: 'pts:caplow', loadPoints: vi.fn(async () => makeBatch()), @@ -767,7 +774,10 @@ describe('PointsDataEngine — dict-only feature scan', () => { const loadPointsMatchingFeatureCodes = vi.fn( async (opts: { featureCodes: readonly number[] }) => ({ shape: [2, opts.featureCodes.length], - data: [new Float32Array(opts.featureCodes.length), new Float32Array(opts.featureCodes.length)], + data: [ + new Float32Array(opts.featureCodes.length), + new Float32Array(opts.featureCodes.length), + ], featureCodes: Int32Array.from(opts.featureCodes), }) ); diff --git a/packages/layers/tests/pointsFeatureColor.spec.ts b/packages/layers/tests/pointsFeatureColor.spec.ts index 42f859d0..a8b03335 100644 --- a/packages/layers/tests/pointsFeatureColor.spec.ts +++ b/packages/layers/tests/pointsFeatureColor.spec.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from 'vitest'; -import { featureCodeToRgb, featureCodeToCssColor } from '../src/pointsFeatureColor.js'; +import { featureCodeToCssColor, featureCodeToRgb } from '../src/pointsFeatureColor.js'; describe('featureCodeToRgb', () => { it('returns grey for the negative "no colour" sentinel', () => { diff --git a/packages/layers/tests/pointsLayerFilter.spec.ts b/packages/layers/tests/pointsLayerFilter.spec.ts index 12fbd074..0a944436 100644 --- a/packages/layers/tests/pointsLayerFilter.spec.ts +++ b/packages/layers/tests/pointsLayerFilter.spec.ts @@ -1,10 +1,10 @@ import { describe, expect, it } from 'vitest'; +import { filterPreloadedBatch } from '../src/PointsLayer.js'; import { featureCodesSignature, featureFilterAwaitingRowCodes, hasPreloadedRowFeatureCodes, } from '../src/pointsFeatureCodes.js'; -import { filterPreloadedBatch } from '../src/PointsLayer.js'; import type { ColumnarNdarrayPointsBatch } from '../src/pointsLoader.js'; describe('PointsLayer preloaded filtering', () => { diff --git a/packages/layers/tests/pointsLoadPlan.spec.ts b/packages/layers/tests/pointsLoadPlan.spec.ts index e467502a..e813ead2 100644 --- a/packages/layers/tests/pointsLoadPlan.spec.ts +++ b/packages/layers/tests/pointsLoadPlan.spec.ts @@ -1,6 +1,5 @@ -import { describe, expect, it } from 'vitest'; - import type { PointsTilingMetadata } from '@spatialdata/core'; +import { describe, expect, it } from 'vitest'; import { planPointsLoads, diff --git a/packages/layers/tests/pointsRenderAttributes.spec.ts b/packages/layers/tests/pointsRenderAttributes.spec.ts index b34ac341..5e8a429c 100644 --- a/packages/layers/tests/pointsRenderAttributes.spec.ts +++ b/packages/layers/tests/pointsRenderAttributes.spec.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from 'vitest'; -import { buildPointsAttributes } from '../src/pointsRenderAttributes.js'; import type { ColumnarNdarrayPointsBatch } from '../src/pointsLoader.js'; +import { buildPointsAttributes } from '../src/pointsRenderAttributes.js'; function batch(overrides: Partial): ColumnarNdarrayPointsBatch { return { diff --git a/packages/layers/tests/pointsRenderStrategies.spec.ts b/packages/layers/tests/pointsRenderStrategies.spec.ts index f971fe57..c4c4996f 100644 --- a/packages/layers/tests/pointsRenderStrategies.spec.ts +++ b/packages/layers/tests/pointsRenderStrategies.spec.ts @@ -1,8 +1,7 @@ import { describe, expect, it } from 'vitest'; - +import type { PointsLayer } from '../src/PointsLayer.js'; import { resolvePointsRenderStrategy } from '../src/pointsRenderStrategies.js'; import { preloadedScatterStrategy } from '../src/preloadedScatterStrategy.js'; -import type { PointsLayer } from '../src/PointsLayer.js'; describe('resolvePointsRenderStrategy', () => { it('selects morton and preloaded strategies by encoding kind', () => { diff --git a/packages/layers/tests/renderStackShim.spec.ts b/packages/layers/tests/renderStackShim.spec.ts new file mode 100644 index 00000000..a488ad9e --- /dev/null +++ b/packages/layers/tests/renderStackShim.spec.ts @@ -0,0 +1,59 @@ +import * as core from '@spatialdata/core'; +import { describe, expect, it } from 'vitest'; +import * as layers from '../src/index.js'; + +/** + * ADR 0004 §5 makes `@spatialdata/core` the canonical home of the Render Stack + * schemas, and promises that `@spatialdata/layers` and `@spatialdata/vis` keep + * their re-exports as **compatibility shims** — MDV consumes these as a data + * contract, and no consumer import may move. + * + * That promise is exactly the kind that rots silently: an `export … from` chain + * still typechecks if someone re-declares a schema locally, and the drift only + * shows up in a downstream repo. So assert it here — the shim must resolve to the + * *same object*, not merely to an equivalent one. + */ + +const SHIMMED_VALUES = [ + 'RENDER_STACK_SCHEMA_VERSION', + 'renderStackSchema', + 'renderStackEntrySchema', + 'renderStackSpatialEntrySchema', + 'renderStackHostEntrySchema', + 'renderStackGroupEntrySchema', + 'renderStackSpatialElementTypeSchema', + 'getRenderStackEntryIds', + 'getRenderStackHostLayerIds', + 'SPATIAL_LAYER_PROPS_SCHEMA_VERSION', + 'spatialLayerPropsSchema', + 'spatialSublayerSchema', + 'migrateSpatialLayerProps', +] as const; + +describe('render-stack compatibility shim (ADR 0004 §5)', () => { + it.each(SHIMMED_VALUES)('layers re-exports %s by identity from core', (name) => { + expect(layers[name]).toBeDefined(); + expect(layers[name]).toBe(core[name]); + }); + + it('the schemas still parse — the shim is not just a name, it is the behaviour', () => { + const stack = layers.renderStackSchema.parse({ + schemaVersion: layers.RENDER_STACK_SCHEMA_VERSION, + entries: [ + { + id: 'cells', + kind: 'spatial', + source: { elementType: 'shapes', elementKey: 'cells' }, + }, + { + id: 'mdv-scatter', + kind: 'host', + source: { hostLayerId: 'deck:scatter' }, + }, + ], + }); + + expect(layers.getRenderStackEntryIds(stack)).toEqual(['cells', 'mdv-scatter']); + expect(layers.getRenderStackHostLayerIds(stack)).toEqual(['deck:scatter']); + }); +}); diff --git a/packages/layers/tests/shapesLayer.spec.ts b/packages/layers/tests/shapesLayer.spec.ts index e466fcc8..cfcde655 100644 --- a/packages/layers/tests/shapesLayer.spec.ts +++ b/packages/layers/tests/shapesLayer.spec.ts @@ -1,19 +1,19 @@ import { describe, expect, it, vi } from 'vitest'; import { SpatialLayer } from '../src/SpatialLayer'; import { + buildShapeFeatureStateRuntime, + buildShapesPrebuiltData, + createShapesDeckLayer, DEFAULT_SHAPE_STROKE_WIDTH_MAX_PIXELS, DEFAULT_SHAPE_STROKE_WIDTH_MIN_PIXELS, DEFAULT_SHAPE_STROKE_WIDTH_UNITS, type GeoarrowTableLike, - type ShapesRenderDataLike, - buildShapeFeatureStateRuntime, - buildShapesPrebuiltData, - createShapesDeckLayer, isShapeFeatureStateRuntime, normalizeShapeFeatureState, resolveShapeFeatureFromPick, resolveShapeTooltipFromPickInfo, resolveShapeTooltipRowIndex, + type ShapesRenderDataLike, } from '../src/shapesLayer'; const renderData: ShapesRenderDataLike = { @@ -103,9 +103,7 @@ describe('shape feature state runtime', () => { const featureState = { fillColorByFeatureId: { 'cell-1': [1, 2, 3, 255] as [number, number, number, number] }, }; - expect(normalizeShapeFeatureState(featureState)).toBe( - normalizeShapeFeatureState(featureState) - ); + expect(normalizeShapeFeatureState(featureState)).toBe(normalizeShapeFeatureState(featureState)); }); }); @@ -420,7 +418,12 @@ describe('createShapesDeckLayer', () => { it('prefers feature-index alignment over instance-key map when both are present', () => { expect( resolveShapeTooltipRowIndex( - { featureId: '23816', featureIndex: 23816, rowIndex: 23816, polygon: renderData.polygons![0] }, + { + featureId: '23816', + featureIndex: 23816, + rowIndex: 23816, + polygon: renderData.polygons![0], + }, { tooltipRowIndexByFeatureId: new Map([['23816', 22271]]), rowIndexByFeatureIndex: new Int32Array(49750).fill(-1), From 1d75d966ddf00ff9e8cc6063f872b81f39d1bae8 Mon Sep 17 00:00:00 2001 From: Peter Todd Date: Tue, 14 Jul 2026 19:03:56 +0100 Subject: [PATCH 4/9] Guard tests: useLayerData, points resource identity, dependency boundaries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The net, landed BEFORE any behaviour moves. Nothing here changes source; all three files are written against the current code and must stay green through the resolver refactor. useLayerData.spec.tsx — the first test that actually renders the hook. Nothing did, until now. Two specs import from the module (one takes a type, one takes two module-scope helpers) but the 1,873-line hook itself was never invoked by any test in the repo. Its whole public surface — seventeen members that reach MDV through a `...layerData` spread — was unguarded, and the refactor dissolves six of its seven kind-switch ladders and re-points every member at a resolver snapshot. Asserts the contract, not the internals: the exact 17 keys, idle→ready for shapes and points, world bounds, and render-resource identity across repeated getLayers() calls. pointsResourceIdentity.spec.ts — all three points resources, not one. PointsDataEngine exposes three render resources and memoises all three lazily on read, because today their only caller is render. Deck rebuilds a layer's batch when data identity changes, so a resource rebuilt per call is a teardown per frame — the pan flash. The existing 845-line spec pins exactly one of the three (the resident one). Nothing guarded getMatchingResource or getMatchingPartialResource, so a flash on the selected genes, or a teardown-per-frame on the streaming partial overlay, would have been invisible to every test here. C5 moves these memos into the Renderer Adapter's project(); this is the contract that move must not break. Writing it surfaced a real ordering subtlety: ensureMatchingFeaturesLoaded assigns entry.matchingLoading AFTER constructing the scan's async IIFE, so a loader firing onProgress before its first await has that chunk silently dropped by the currency guard. Unreachable in production (real loaders do I/O first), but the stubs now model the real thing rather than a state the engine never reaches. dependencies.spec.ts — two DoD boxes that said "Still true". A box that says "still true" is a box that rots, and these are the constraints the whole design rests on: core is tgpu-htj2k's dependency root, and its framework-freedom is what makes the resolver testable with no GL context. Now enforced: core imports no react/deck.gl/viv/avivatorish and no sibling package; layers imports no react. Every assertion in it is `toEqual([])`, so a broken scanner would pass vacuously and read as proof — worse than no test. Four canary cases point the detector at imports we know exist (deck.gl in layers, react in vis, core in layers, and specifically via `export … from`) and require it to find them. Adds @testing-library/react to vis devDeps; jsdom was already configured. 486 tests (was 427). Build and biome green. Co-Authored-By: Claude Opus 4.8 --- packages/core/tests/dependencies.spec.ts | 138 +++++++++ .../tests/pointsResourceIdentity.spec.ts | 293 ++++++++++++++++++ packages/vis/package.json | 11 +- packages/vis/tests/demoUrl.spec.ts | 19 +- .../vis/tests/pointsFeatureRowState.spec.ts | 2 +- .../vis/tests/spatialCanvasViewer.spec.ts | 12 +- packages/vis/tests/useLayerData.spec.tsx | 242 +++++++++++++++ .../vis/tests/vivImagePassthrough.spec.ts | 7 +- packages/vis/tests/vivSpatialViewer.spec.ts | 2 +- pnpm-lock.yaml | 3 + 10 files changed, 701 insertions(+), 28 deletions(-) create mode 100644 packages/core/tests/dependencies.spec.ts create mode 100644 packages/layers/tests/pointsResourceIdentity.spec.ts create mode 100644 packages/vis/tests/useLayerData.spec.tsx diff --git a/packages/core/tests/dependencies.spec.ts b/packages/core/tests/dependencies.spec.ts new file mode 100644 index 00000000..f34f9f38 --- /dev/null +++ b/packages/core/tests/dependencies.spec.ts @@ -0,0 +1,138 @@ +import { readdirSync, readFileSync, statSync } from 'node:fs'; +import { join, relative, resolve } from 'node:path'; +import { describe, expect, it } from 'vitest'; + +/** + * The package dependency boundaries, as a test. + * + * ADR 0004's definition of done opens with two boxes that read "Still true": + * + * - `@spatialdata/core` has no `react`, no `deck.gl`, no `@hms-dbmi/viv` import. + * - `@spatialdata/layers` has no `react` import. + * + * A box that says "still true" is a box that rots. These are the constraints the + * whole Resource Resolver design rests on — `core` is the dependency root for + * `tgpu-htj2k`, whose engine core is deliberately dependency-free, and it is what + * makes the resolver testable headless with no GL context. Encode them so they + * cannot quietly stop being true. + */ + +const REPO = resolve(__dirname, '../../..'); + +function sourceFiles(pkg: string): string[] { + const root = join(REPO, 'packages', pkg, 'src'); + const out: string[] = []; + const walk = (dir: string) => { + for (const entry of readdirSync(dir)) { + const path = join(dir, entry); + if (statSync(path).isDirectory()) walk(path); + else if (/\.tsx?$/.test(entry)) out.push(path); + } + }; + walk(root); + return out; +} + +/** Module specifiers this file imports from — `import … from 'x'` and `export … from 'x'`. */ +function importsOf(file: string): string[] { + const source = readFileSync(file, 'utf8'); + const specifiers: string[] = []; + const re = /(?:^|\n)\s*(?:import|export)[\s\S]*?from\s*['"]([^'"]+)['"]/g; + let match: RegExpExecArray | null = re.exec(source); + while (match !== null) { + specifiers.push(match[1] as string); + match = re.exec(source); + } + // Bare side-effect imports: `import 'x'` + const bare = /(?:^|\n)\s*import\s*['"]([^'"]+)['"]/g; + let bareMatch: RegExpExecArray | null = bare.exec(source); + while (bareMatch !== null) { + specifiers.push(bareMatch[1] as string); + bareMatch = bare.exec(source); + } + return specifiers; +} + +/** Does `specifier` resolve to `pkg`, or a subpath of it? */ +const isFrom = (specifier: string, pkg: string) => + specifier === pkg || specifier.startsWith(`${pkg}/`); + +function offenders(pkg: string, forbidden: readonly string[]): string[] { + const found: string[] = []; + for (const file of sourceFiles(pkg)) { + for (const specifier of importsOf(file)) { + const hit = forbidden.find((f) => isFrom(specifier, f)); + if (hit) found.push(`${relative(REPO, file)} imports '${specifier}'`); + } + } + return found; +} + +describe('@spatialdata/core is framework-free (ADR 0004 §1)', () => { + // core is the dependency root for tgpu-htj2k (three.js/WebGPU, separate repo), + // which excludes deck.gl and React from its render path by name. Break this and + // the second consumer cannot use the resolver at all — which is the entire + // reason it moved here. + it('imports no React', () => { + expect(offenders('core', ['react', 'react-dom'])).toEqual([]); + }); + + it('imports no deck.gl', () => { + expect(offenders('core', ['deck.gl', '@deck.gl', '@geoarrow/deck.gl-geoarrow'])).toEqual([]); + }); + + it('imports no Viv', () => { + expect(offenders('core', ['@hms-dbmi/viv'])).toEqual([]); + }); + + it('imports no avivatorish', () => { + // avivatorish pulls in BOTH React and Viv, and is a de-vendoring holding pen + // for code that also lives upstream in Viv and MDV, with an evolving image + // state model. core must not be shaped around it. See ADR 0004's amendment. + expect(offenders('core', ['@spatialdata/avivatorish'])).toEqual([]); + }); + + it('does not depend on any sibling @spatialdata package', () => { + // core is the root. Anything else is a cycle waiting to happen. + expect( + offenders('core', ['@spatialdata/layers', '@spatialdata/vis', '@spatialdata/react']) + ).toEqual([]); + }); +}); + +describe('@spatialdata/layers is React-free (ADR 0004 §4)', () => { + // layers owns the deck Renderer Adapter. Deck layers are not React components; + // the moment React appears here, the adapter has become a component and the + // headless consumer is gone. + it('imports no React', () => { + expect(offenders('layers', ['react', 'react-dom'])).toEqual([]); + }); +}); + +describe('the detector itself', () => { + // Every assertion above is `toEqual([])`. An import scanner with a broken regex + // returns [] for everything and every boundary test passes vacuously — which + // would be strictly worse than having no test at all, because it would read as + // proof. So point it at imports we KNOW exist and require it to find them. + it('finds deck.gl in layers, which certainly imports it', () => { + expect(offenders('layers', ['deck.gl', '@deck.gl']).length).toBeGreaterThan(0); + }); + + it('finds React in vis, which certainly imports it', () => { + expect(offenders('vis', ['react']).length).toBeGreaterThan(0); + }); + + it('finds core imported by layers — so sibling detection works too', () => { + expect(offenders('layers', ['@spatialdata/core']).length).toBeGreaterThan(0); + }); + + it('sees `export … from` re-exports, not just `import`', () => { + // layers/src/index.ts reaches core ONLY via `export … from '@spatialdata/core'` + // (the render-stack compat shim). If the scanner missed export-from, a future + // forbidden dependency could enter core through a re-export unseen. + const viaExport = offenders('layers', ['@spatialdata/core']).filter((o) => + o.startsWith('packages/layers/src/index.ts') + ); + expect(viaExport.length).toBeGreaterThan(0); + }); +}); diff --git a/packages/layers/tests/pointsResourceIdentity.spec.ts b/packages/layers/tests/pointsResourceIdentity.spec.ts new file mode 100644 index 00000000..1185ee07 --- /dev/null +++ b/packages/layers/tests/pointsResourceIdentity.spec.ts @@ -0,0 +1,293 @@ +import type { PointsElement, PointsLoadProgress, PointsLoadResult } from '@spatialdata/core'; +import { describe, expect, it, vi } from 'vitest'; +import { PointsDataEngine } from '../src/engine/PointsDataEngine.js'; + +/** + * Render-resource IDENTITY stability, for all three points resources. + * + * Why this file exists, separately from pointsDataEngine.spec.ts: + * + * `PointsDataEngine` exposes three render resources, and **all three memoise + * lazily on read** — because today their only caller is React's render phase, via + * `getLayers()`. Deck tears a layer down and rebuilds its batch when `data` + * identity changes, so a resource that is rebuilt per call is a teardown per + * frame: the pan flash. + * + * The existing 845-line spec pins exactly ONE of the three (the resident one). + * Nothing guards `getMatchingResource` or `getMatchingPartialResource` — so a + * pan-flash on the matched layer, or a teardown-per-frame on the streaming + * partial overlay, would be invisible to every test in the repo. + * + * That matters now because the Resource Resolver work MOVES these memos: out of + * lazy-on-read in `core`'s resolver, into eager-once in the Renderer Adapter's + * `project()` in `layers` (ADR 0004 §4 — identity-stable memoisation is a deck + * requirement, so it belongs on the renderer side). This file is the contract + * that move must not break. It is written against the CURRENT engine so it is + * green before the refactor and must stay green through it. + * + * Repeated getter calls with no intervening state change stand in for repeated + * renders — a pan, a hover, a viewState tick. That is precisely how `getLayers()` + * calls them. + */ + +const batch = (pointCount: number, opts: { truncated?: boolean } = {}): PointsLoadResult => ({ + shape: [2, pointCount], + data: [ + new Float32Array(Array.from({ length: pointCount }, (_, i) => i)), + new Float32Array(Array.from({ length: pointCount }, (_, i) => i)), + ], + featureCodes: new Int32Array(Array.from({ length: pointCount }, (_, i) => i % 2)), + ...(opts.truncated ? { preloadTruncated: true, totalRowCount: 1_000_000 } : {}), +}); + +/** Defer a promise so a load/scan can be held open across assertions. */ +function deferred() { + let resolve!: (value: T) => void; + const promise = new Promise((r) => { + resolve = r; + }); + return { promise, resolve }; +} + +/** + * Yield once before emitting progress, the way a real loader does. + * + * This is not incidental. `ensureMatchingFeaturesLoaded` assigns + * `entry.matchingLoading` *after* it constructs the scan's async IIFE, and the + * IIFE runs synchronously up to its first `await`. So a loader that fires + * `onProgress` before ever yielding hits `PointsDataEngine.ts`'s + * `if (!loading …) return` guard and has its first chunk silently dropped. + * + * Real loaders always do I/O first, so this is unreachable in production — but a + * stub that emits synchronously is not modelling the real thing, and pretending + * otherwise here would make these tests assert against a state the engine never + * actually reaches. + */ +const yieldTick = () => new Promise((r) => setTimeout(r, 0)); + +describe('resident render resource — getResource', () => { + it('is identity-stable across repeated reads (the pan-flash guard)', async () => { + const engine = new PointsDataEngine(); + const element = { + key: 'pts', + loadPoints: vi.fn(async () => batch(3)), + } as unknown as PointsElement; + + await engine.ensureLoaded({ key: 'pts', layerId: 'l', element }); + + const first = engine.getResource(element, 'pts'); + expect(first).not.toBeNull(); + // Ten "renders". Deck must see one resource. + for (let i = 0; i < 10; i++) { + expect(engine.getResource(element, 'pts')).toBe(first); + } + }); + + it('is null before data, and does not memoise the null', async () => { + const engine = new PointsDataEngine(); + const element = { + key: 'pts', + loadPoints: vi.fn(async () => batch(3)), + } as unknown as PointsElement; + + expect(engine.getResource(element, 'pts')).toBeNull(); + + await engine.ensureLoaded({ key: 'pts', layerId: 'l', element }); + + expect(engine.getResource(element, 'pts')).not.toBeNull(); + }); + + it('CHANGES identity when the underlying batch changes — staleness is the other failure', async () => { + // The memo must not be so sticky that a genuine reload is ignored. Both + // directions are bugs: churn is a flash, staleness is a wrong render. + const engine = new PointsDataEngine(); + const element = { + key: 'pts', + // Truncated → raising the cap fetches more rows and swaps the batch. + loadPoints: vi.fn(async (o: { memoryCap?: number }) => + batch(o?.memoryCap === 8 ? 8 : 4, { truncated: true }) + ), + } as unknown as PointsElement; + + await engine.ensureLoaded({ key: 'pts', layerId: 'l', element }, 4); + const before = engine.getResource(element, 'pts'); + + await engine.ensureLoaded({ key: 'pts', layerId: 'l', element }, 8); + const after = engine.getResource(element, 'pts'); + + expect(after).not.toBeNull(); + expect(after).not.toBe(before); + }); +}); + +describe('matched render resource — getMatchingResource', () => { + // Covered by NOTHING today. A flash here is a flash on the selected genes. + const scanElement = (result: PointsLoadResult) => + ({ + key: 'pts', + loadPoints: vi.fn(async () => batch(4)), + loadPointsMatchingFeatureCodes: vi.fn(async () => result), + }) as unknown as PointsElement; + + it('is identity-stable across repeated reads', async () => { + const engine = new PointsDataEngine(); + const element = scanElement(batch(2)); + + await engine.ensureLoaded({ key: 'pts', layerId: 'l', element }); + await engine.ensureMatchingFeaturesLoaded({ key: 'pts', layerId: 'l', element }, [0]); + + const first = engine.getMatchingResource(element, 'pts'); + expect(first).not.toBeNull(); + for (let i = 0; i < 10; i++) { + expect(engine.getMatchingResource(element, 'pts')).toBe(first); + } + }); + + it('is distinct from the resident resource — they are two layers, drawn together', async () => { + const engine = new PointsDataEngine(); + const element = scanElement(batch(2)); + + await engine.ensureLoaded({ key: 'pts', layerId: 'l', element }); + await engine.ensureMatchingFeaturesLoaded({ key: 'pts', layerId: 'l', element }, [0]); + + expect(engine.getMatchingResource(element, 'pts')).not.toBe(engine.getResource(element, 'pts')); + }); + + it('CHANGES identity when a new selection settles', async () => { + const engine = new PointsDataEngine(); + const results = [batch(2), batch(3)]; + let call = 0; + const element = { + key: 'pts', + loadPoints: vi.fn(async () => batch(4)), + loadPointsMatchingFeatureCodes: vi.fn(async () => results[call++] as PointsLoadResult), + } as unknown as PointsElement; + const target = { key: 'pts', layerId: 'l', element }; + + await engine.ensureLoaded(target); + await engine.ensureMatchingFeaturesLoaded(target, [0]); + const first = engine.getMatchingResource(element, 'pts'); + + // A DIFFERENT selection, not covered by the first → a real rescan. + await engine.ensureMatchingFeaturesLoaded(target, [1]); + const second = engine.getMatchingResource(element, 'pts'); + + expect(second).not.toBeNull(); + expect(second).not.toBe(first); + }); + + it('returns null for an empty matched batch — the empty-lock guard', async () => { + // A scan that matched no rows must NOT supersede the resident preview, or the + // render locks to an empty batch with no way back. + const engine = new PointsDataEngine(); + const element = scanElement(batch(0)); + + await engine.ensureLoaded({ key: 'pts', layerId: 'l', element }); + await engine.ensureMatchingFeaturesLoaded({ key: 'pts', layerId: 'l', element }, [0]); + + expect(engine.getMatchingResource(element, 'pts')).toBeNull(); + }); +}); + +describe('partial render resource — getMatchingPartialResource', () => { + // Also covered by NOTHING today. This one is the streaming overlay: it is read + // on EVERY frame while a multi-second scan runs, which is exactly when the user + // is most likely to be panning. Churn here is the worst case in the file. + it('is identity-stable across reads while the partial buffer is unchanged', async () => { + const engine = new PointsDataEngine(); + const scan = deferred(); + const partial = batch(2); + + const element = { + key: 'pts', + loadPoints: vi.fn(async () => batch(4)), + loadPointsMatchingFeatureCodes: vi.fn( + async (o: { onProgress?: (p: PointsLoadProgress) => void }) => { + await yieldTick(); + o.onProgress?.({ matchedRows: 2, scannedRows: 10, partialResult: partial }); + return scan.promise; + } + ), + } as unknown as PointsElement; + const target = { key: 'pts', layerId: 'l', element }; + + await engine.ensureLoaded(target); + const pending = engine.ensureMatchingFeaturesLoaded(target, [0]); + await yieldTick(); + + const first = engine.getMatchingPartialResource(element, 'pts'); + expect(first).not.toBeNull(); + // Ten frames of panning mid-scan. One resource. + for (let i = 0; i < 10; i++) { + expect(engine.getMatchingPartialResource(element, 'pts')).toBe(first); + } + + scan.resolve(batch(2)); + await pending; + }); + + it('CHANGES identity when the scan grows the buffer — and only then', async () => { + const engine = new PointsDataEngine(); + const scan = deferred(); + const first = batch(2); + const grown = batch(5); + let emit!: (p: PointsLoadProgress) => void; + + const element = { + key: 'pts', + loadPoints: vi.fn(async () => batch(4)), + loadPointsMatchingFeatureCodes: vi.fn( + async (o: { onProgress?: (p: PointsLoadProgress) => void }) => { + await yieldTick(); + emit = o.onProgress as (p: PointsLoadProgress) => void; + emit({ matchedRows: 2, scannedRows: 10, partialResult: first }); + return scan.promise; + } + ), + } as unknown as PointsElement; + const target = { key: 'pts', layerId: 'l', element }; + + await engine.ensureLoaded(target); + const pending = engine.ensureMatchingFeaturesLoaded(target, [0]); + await yieldTick(); + + const atFirstChunk = engine.getMatchingPartialResource(element, 'pts'); + expect(atFirstChunk).not.toBeNull(); + + // A new chunk grows the buffer. The memo keys on the partial's IDENTITY, so + // this must produce a new resource — otherwise the overlay stops filling in. + emit({ matchedRows: 5, scannedRows: 30, partialResult: grown }); + const atSecondChunk = engine.getMatchingPartialResource(element, 'pts'); + + expect(atSecondChunk).not.toBeNull(); + expect(atSecondChunk).not.toBe(atFirstChunk); + + // ...and is then stable again until the next chunk. + expect(engine.getMatchingPartialResource(element, 'pts')).toBe(atSecondChunk); + + scan.resolve(grown); + await pending; + }); + + it('is null once the scan settles — the partial overlay is torn down exactly once', async () => { + const engine = new PointsDataEngine(); + const element = { + key: 'pts', + loadPoints: vi.fn(async () => batch(4)), + loadPointsMatchingFeatureCodes: vi.fn( + async (o: { onProgress?: (p: PointsLoadProgress) => void }) => { + await yieldTick(); + o.onProgress?.({ matchedRows: 2, scannedRows: 10, partialResult: batch(2) }); + return batch(2); + } + ), + } as unknown as PointsElement; + const target = { key: 'pts', layerId: 'l', element }; + + await engine.ensureLoaded(target); + await engine.ensureMatchingFeaturesLoaded(target, [0]); + + expect(engine.getMatchingPartialResource(element, 'pts')).toBeNull(); + expect(engine.getMatchingResource(element, 'pts')).not.toBeNull(); + }); +}); diff --git a/packages/vis/package.json b/packages/vis/package.json index e8cb0d0e..3df5d56e 100644 --- a/packages/vis/package.json +++ b/packages/vis/package.json @@ -42,27 +42,28 @@ "@spatialdata/core": "workspace:*", "@spatialdata/layers": "workspace:*", "@spatialdata/react": "workspace:*", - "zarrextra": "workspace:*", "@uidotdev/usehooks": "^2.4.1", "@uiw/react-json-view": "2.0.0-alpha.39", "@vivjs/views": "catalog:", "deck.gl": "catalog:", "fast-deep-equal": "^3.1.3", "geotiff": "2.1.4-beta.0", + "zarrextra": "workspace:*", "zustand": "^5.0.8" }, "devDependencies": { - "@vivjs/constants": "catalog:", "@cornerstonejs/codec-openjpeg": "^1.3.0", - "openjph-wasm": "^0.1.0", + "@rolldown/plugin-babel": "catalog:", + "@testing-library/react": "^16.3.2", "@types/node": "catalog:", "@types/react": "catalog:", "@types/react-dom": "catalog:", - "@rolldown/plugin-babel": "catalog:", + "@typescript/native": "catalog:", "@vitejs/plugin-react": "catalog:", + "@vivjs/constants": "catalog:", "babel-plugin-react-compiler": "catalog:", "jsdom": "catalog:", - "@typescript/native": "catalog:", + "openjph-wasm": "^0.1.0", "typescript": "catalog:", "vite": "catalog:", "vitest": "catalog:" diff --git a/packages/vis/tests/demoUrl.spec.ts b/packages/vis/tests/demoUrl.spec.ts index 6e78d112..7a947be5 100644 --- a/packages/vis/tests/demoUrl.spec.ts +++ b/packages/vis/tests/demoUrl.spec.ts @@ -1,7 +1,7 @@ import { describe, expect, it } from 'vitest'; import { - DEFAULT_DEMO_SPATIALDATA_URL, buildDemoPageHref, + DEFAULT_DEMO_SPATIALDATA_URL, getSpatialDataUrlFromSearchParams, } from '../src/Sketch/demoUrl.js'; @@ -14,17 +14,13 @@ describe('getSpatialDataUrlFromSearchParams', () => { it('reads url param', () => { const store = 'https://example.com/data.zarr'; - expect( - getSpatialDataUrlFromSearchParams(new URLSearchParams({ url: store })) - ).toBe(store); + expect(getSpatialDataUrlFromSearchParams(new URLSearchParams({ url: store }))).toBe(store); }); it('decodes encoded url param', () => { const store = 'https://example.com/a b.zarr'; expect( - getSpatialDataUrlFromSearchParams( - new URLSearchParams({ url: encodeURIComponent(store) }) - ) + getSpatialDataUrlFromSearchParams(new URLSearchParams({ url: encodeURIComponent(store) })) ).toBe(store); }); @@ -37,12 +33,7 @@ describe('getSpatialDataUrlFromSearchParams', () => { describe('buildDemoPageHref', () => { it('sets url search param', () => { - const href = buildDemoPageHref( - 'https://example.com/dataset.zarr', - 'https://demo.test/sketch' - ); - expect(href).toBe( - 'https://demo.test/sketch?url=https%3A%2F%2Fexample.com%2Fdataset.zarr' - ); + const href = buildDemoPageHref('https://example.com/dataset.zarr', 'https://demo.test/sketch'); + expect(href).toBe('https://demo.test/sketch?url=https%3A%2F%2Fexample.com%2Fdataset.zarr'); }); }); diff --git a/packages/vis/tests/pointsFeatureRowState.spec.ts b/packages/vis/tests/pointsFeatureRowState.spec.ts index 4c760948..d9c872a1 100644 --- a/packages/vis/tests/pointsFeatureRowState.spec.ts +++ b/packages/vis/tests/pointsFeatureRowState.spec.ts @@ -1,8 +1,8 @@ import { describe, expect, it } from 'vitest'; import { - type FeatureRowStateInput, describeFeatureRowState, + type FeatureRowStateInput, } from '../src/SpatialCanvas/featureRowState'; const base: FeatureRowStateInput = { diff --git a/packages/vis/tests/spatialCanvasViewer.spec.ts b/packages/vis/tests/spatialCanvasViewer.spec.ts index dbfb568a..c153a8f2 100644 --- a/packages/vis/tests/spatialCanvasViewer.spec.ts +++ b/packages/vis/tests/spatialCanvasViewer.spec.ts @@ -1,17 +1,17 @@ +import { renderStackSchema } from '@spatialdata/layers'; import { ScatterplotLayer } from 'deck.gl'; import { describe, expect, it } from 'vitest'; -import { - composeSpatialDeckLayers, - shouldAutoFitSpatialView, - shouldRenderInternalTooltip, -} from '../src/SpatialCanvas/SpatialCanvasViewer.js'; import { renderStackOrder, renderStackToLayerInputs, resolveRenderStackHostLayers, sortLayersByRenderStackOrder, } from '../src/SpatialCanvas/renderStackAdapters.js'; -import { renderStackSchema } from '@spatialdata/layers'; +import { + composeSpatialDeckLayers, + shouldAutoFitSpatialView, + shouldRenderInternalTooltip, +} from '../src/SpatialCanvas/SpatialCanvasViewer.js'; describe('composeSpatialDeckLayers', () => { it('places caller-provided deck layers after generated SpatialData layers', () => { diff --git a/packages/vis/tests/useLayerData.spec.tsx b/packages/vis/tests/useLayerData.spec.tsx new file mode 100644 index 00000000..47879710 --- /dev/null +++ b/packages/vis/tests/useLayerData.spec.tsx @@ -0,0 +1,242 @@ +import { Matrix4 } from '@math.gl/core'; +import type { PointsElement, ShapesElement } from '@spatialdata/core'; +import { renderHook, waitFor } from '@testing-library/react'; +import { describe, expect, it, vi } from 'vitest'; +import type { AvailableElement, ElementsByType, LayerConfig } from '../src/SpatialCanvas/types.js'; +import { useLayerData } from '../src/SpatialCanvas/useLayerData.js'; + +/** + * The first test that actually RENDERS `useLayerData`. + * + * Until this file, nothing did. Two specs import from the module — one takes a + * type, one takes two module-scope helpers — but the 1,873-line hook itself was + * never invoked by any test in the repo. Its entire public surface, seventeen + * members that reach MDV through a `...layerData` spread, was unguarded. + * + * That is untenable for the Resource Resolver work, which dissolves six of the + * hook's seven kind-switch ladders and re-points all seventeen members at a + * resolver snapshot. This file is the net. It is written against the CURRENT + * hook — it must be green before the refactor and stay green through it. + * + * It deliberately asserts the CONTRACT (the surface, the load lifecycle, resource + * identity), not the implementation. Nothing here should need to change when the + * internals are replaced; if something does, that is the signal to look hard at + * whether the shim is honest. + */ + +/** The seventeen members MDV consumes. This list IS the compat contract. */ +const PUBLIC_SURFACE = [ + 'getLayers', + 'getVivLayerProps', + 'getImageLayerLoadedData', + 'getImageLoadedDataByElementKey', + 'getLabelsLayerLoadedData', + 'getLayerLoadState', + 'hasRenderableLayerData', + 'pointsEngine', + 'resolvePointsTarget', + 'getFeatureTooltip', + 'getFeaturePickEvent', + 'getShapePickEvent', + 'isLoading', + 'isBlocking', + 'reloadElement', + 'getWorldBoundsForLayer', + 'getWorldBoundsForVisibleLayers', +] as const; + +const EMPTY_ELEMENTS: ElementsByType = { images: [], shapes: [], points: [], labels: [] }; + +function pointsElement(key: string): AvailableElement { + const element = { + key, + loadPoints: vi.fn(async () => ({ + shape: [2, 3], + data: [new Float32Array([0, 1, 2]), new Float32Array([3, 4, 5])], + featureCodes: new Int32Array([0, 1, 0]), + })), + listFeaturesWithCounts: vi.fn(async () => null), + } as unknown as PointsElement; + return { key, type: 'points', element, transform: new Matrix4() }; +} + +function shapesElement(key: string): AvailableElement { + // Xenium-style cell circles: columnar centres + radii, which is what + // `ShapeCircleColumnar` actually is — NOT an array of {x, y, radius} objects. + const element = { + key, + loadRenderData: vi.fn(async () => ({ + kind: 'js-polygons' as const, + geometryKind: 'circle' as const, + elementKey: key, + featureIds: ['c1', 'c2'], + circles: { + positions: [new Float32Array([0, 5]), new Float32Array([0, 5])] as [ + Float32Array, + Float32Array, + ], + radii: new Float32Array([1, 1]), + }, + rowIndexByFeatureIndex: new Int32Array([0, 1]), + })), + } as unknown as ShapesElement; + return { key, type: 'shapes', element, transform: new Matrix4() }; +} + +const pointsConfig = (id: string, elementKey: string): LayerConfig => ({ + id, + type: 'points', + elementKey, + visible: true, + opacity: 1, +}); + +const shapesConfig = (id: string, elementKey: string): LayerConfig => ({ + id, + type: 'shapes', + elementKey, + visible: true, + opacity: 1, +}); + +const render = (layers: Record, elements: ElementsByType) => + renderHook(() => useLayerData(layers, Object.keys(layers), elements, null)); + +describe('useLayerData — the 17-member public surface', () => { + // ADR 0004 promises MDV that this surface survives the refactor behind a compat + // shim. MDV gets it via `...layerData` in SpatialCanvasViewer, so a member that + // silently vanishes is a downstream break with no local failure. + it('exposes exactly the seventeen members, and no more', () => { + const { result } = render({}, EMPTY_ELEMENTS); + + expect(Object.keys(result.current).sort()).toEqual([...PUBLIC_SURFACE].sort()); + }); + + it.each(PUBLIC_SURFACE)('exposes %s', (member) => { + const { result } = render({}, EMPTY_ELEMENTS); + + expect(result.current[member]).toBeDefined(); + }); + + it('is inert with no layers — no bounds, not loading, not blocking', () => { + const { result } = render({}, EMPTY_ELEMENTS); + + expect(result.current.getLayers()).toEqual([]); + expect(result.current.getVivLayerProps()).toEqual([]); + expect(result.current.isLoading).toBe(false); + expect(result.current.isBlocking).toBe(false); + expect(result.current.getWorldBoundsForVisibleLayers()).toBeNull(); + }); +}); + +describe('useLayerData — the load lifecycle', () => { + it('drives a shapes layer idle -> ready and produces a deck layer', async () => { + const elements: ElementsByType = { ...EMPTY_ELEMENTS, shapes: [shapesElement('cells')] }; + + const { result } = render({ 'layer-1': shapesConfig('layer-1', 'cells') }, elements); + + // Nothing is renderable before the load resolves. + expect(result.current.hasRenderableLayerData('layer-1')).toBe(false); + + await waitFor(() => { + expect(result.current.getLayerLoadState('layer-1')?.geometry).toBe('ready'); + }); + + expect(result.current.hasRenderableLayerData('layer-1')).toBe(true); + expect(result.current.getLayers().length).toBeGreaterThan(0); + expect(result.current.isBlocking).toBe(false); + }); + + it('produces a deck layer for a points layer', async () => { + const elements: ElementsByType = { ...EMPTY_ELEMENTS, points: [pointsElement('transcripts')] }; + + const { result } = render({ 'layer-p': pointsConfig('layer-p', 'transcripts') }, elements); + + await waitFor(() => { + expect(result.current.hasRenderableLayerData('layer-p')).toBe(true); + }); + + expect(result.current.getLayers().length).toBeGreaterThan(0); + }); + + it('reports world bounds once a layer has data', async () => { + const elements: ElementsByType = { ...EMPTY_ELEMENTS, shapes: [shapesElement('cells')] }; + + const { result } = render({ 'layer-1': shapesConfig('layer-1', 'cells') }, elements); + + await waitFor(() => { + expect(result.current.getWorldBoundsForLayer('layer-1')).not.toBeNull(); + }); + + expect(result.current.getWorldBoundsForVisibleLayers()).not.toBeNull(); + }); + + it('does not resolve a points target for a shapes layer', async () => { + const elements: ElementsByType = { + ...EMPTY_ELEMENTS, + shapes: [shapesElement('cells')], + points: [pointsElement('transcripts')], + }; + + const { result } = render( + { + 'layer-s': shapesConfig('layer-s', 'cells'), + 'layer-p': pointsConfig('layer-p', 'transcripts'), + }, + elements + ); + + expect(result.current.resolvePointsTarget('layer-s')).toBeUndefined(); + expect(result.current.resolvePointsTarget('layer-p')).toMatchObject({ + key: 'transcripts', + layerId: 'layer-p', + }); + }); +}); + +describe('useLayerData — render-resource identity', () => { + // THE regression this whole design guards against. Deck rebuilds a layer's batch + // when its `data` identity changes, so a resource rebuilt per getLayers() call is + // a teardown per frame: the pan flash. `getLayers()` is called on every render — + // every pan, hover and viewState tick — so it must be idempotent within a commit. + it('returns an identity-stable points resource across repeated getLayers() calls', async () => { + const elements: ElementsByType = { ...EMPTY_ELEMENTS, points: [pointsElement('transcripts')] }; + + const { result } = render({ 'layer-p': pointsConfig('layer-p', 'transcripts') }, elements); + + await waitFor(() => { + expect(result.current.hasRenderableLayerData('layer-p')).toBe(true); + }); + + // Three "frames" in one commit. Deck must see one resource, not three. + const resources = [0, 1, 2].map( + () => (result.current.getLayers()[0]?.props as { resource?: unknown } | undefined)?.resource + ); + + expect(resources[0]).toBeDefined(); + expect(resources[1]).toBe(resources[0]); + expect(resources[2]).toBe(resources[0]); + }); + + it('keeps the points resource stable across an unrelated re-render', async () => { + const elements: ElementsByType = { ...EMPTY_ELEMENTS, points: [pointsElement('transcripts')] }; + const layers = { 'layer-p': pointsConfig('layer-p', 'transcripts') }; + + const { result, rerender } = renderHook( + ({ l }: { l: Record }) => + useLayerData(l, Object.keys(l), elements, null), + { initialProps: { l: layers } } + ); + + await waitFor(() => { + expect(result.current.hasRenderableLayerData('layer-p')).toBe(true); + }); + const before = (result.current.getLayers()[0]?.props as { resource?: unknown }).resource; + + // Same config object, new render — nothing about the DATA changed. + rerender({ l: layers }); + const after = (result.current.getLayers()[0]?.props as { resource?: unknown }).resource; + + expect(after).toBe(before); + }); +}); diff --git a/packages/vis/tests/vivImagePassthrough.spec.ts b/packages/vis/tests/vivImagePassthrough.spec.ts index db437b77..bc7e7fa6 100644 --- a/packages/vis/tests/vivImagePassthrough.spec.ts +++ b/packages/vis/tests/vivImagePassthrough.spec.ts @@ -21,7 +21,12 @@ describe('mergeVivImagePassthroughProps', () => { it('falls back to global extensions when resolver returns nothing', () => { const globalExt = [{ id: 'global' }]; - const merged = mergeVivImagePassthroughProps({ brightness: [0.5] }, undefined, undefined, globalExt); + const merged = mergeVivImagePassthroughProps( + { brightness: [0.5] }, + undefined, + undefined, + globalExt + ); expect(merged.extensions).toEqual(globalExt); }); }); diff --git a/packages/vis/tests/vivSpatialViewer.spec.ts b/packages/vis/tests/vivSpatialViewer.spec.ts index 2d155cce..91646b28 100644 --- a/packages/vis/tests/vivSpatialViewer.spec.ts +++ b/packages/vis/tests/vivSpatialViewer.spec.ts @@ -1,7 +1,7 @@ import { Matrix4 } from '@math.gl/core'; import { describe, expect, it, vi } from 'vitest'; -import { VivSpatialViewer } from '../src/SpatialCanvas/VivSpatialViewer.js'; import type { ImageLayerConfig } from '../src/SpatialCanvas/useLayerData.js'; +import { VivSpatialViewer } from '../src/SpatialCanvas/VivSpatialViewer.js'; describe('VivSpatialViewer passthrough', () => { it('forwards vivProps into detailView.getLayers props', () => { diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 6512216f..a5f69b38 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -448,6 +448,9 @@ importers: '@rolldown/plugin-babel': specifier: 'catalog:' version: 0.2.3(@babel/core@8.0.1)(@babel/runtime@7.28.4)(rolldown@1.1.5)(vite@8.1.4(@types/node@22.18.8)(jiti@1.21.7)(terser@5.44.0)) + '@testing-library/react': + specifier: ^16.3.2 + version: 16.3.2(@testing-library/dom@10.4.1)(@types/react-dom@19.2.2(@types/react@19.2.2))(@types/react@19.2.2)(react-dom@19.2.1(react@19.2.1))(react@19.2.1) '@types/node': specifier: 'catalog:' version: 22.18.8 From a6f17fc546981e330758037c405e6dcea0f1b709 Mon Sep 17 00:00:00 2001 From: Peter Todd Date: Tue, 14 Jul 2026 19:11:26 +0100 Subject: [PATCH 5/9] ResourceResolver interface, SpatialEntryStore and PointsResolver in core MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The resolver interface, extracted from the shape PointsDataEngine already has, plus the points implementation and the reconcile loop. All unconsumed: useLayerData still runs the old path, and PointsDataEngine is untouched. The interface pins the phase separation, which is the load-bearing part: plan(ctx) resolver pure, sync commit only no — RETURNS task descriptors load(task) resolver async commit only yes — the only place project() adapter pure, sync end of reconcile render() adapter pure, sync during render — handed NO engine handle project() and render() are deliberately NOT on this interface. They belong to the Renderer Adapter in `layers`, per ADR 0004 §4: identity-stable memoisation is a *deck* requirement, so it belongs on the renderer side. The handoff plan's phase table implied otherwise and has been corrected. PointsResolver is PointsDataEngine's cache/lifecycle half, lifted. The three render-resource memos did NOT come with it — they go to layers in C5. What it exposes instead is their INPUTS by identity: getData / getMatchedBatch / getPartialBatch. Batches here are always replaced, never mutated in place, so identity is an exact invalidation key. That matters more than it looks: pointsRenderResourceSignature keys on row COUNT, not identity, which is exactly why the old engine had to manually null entry.resource on every swap. Keying the adapter's memo on identity removes that bookkeeping entirely. plan() is where the two render-phase `void engine.ensureX(...)` calls go. Both conditions — "does this selection need a scan", "does this filter need row codes" — were always pure functions of config plus entry state. They were being asked in the wrong phase, from inside getLayers(). Here they cannot start work even by accident, and a test asserts plan() touches no element method. Consequently there is no queueMicrotask in the scan path. The old engine's `queueMicrotask(() => this.notify())` existed solely to defend against a synchronous notify during render; nothing kicks a load from render any more, so the defence is unreachable by construction rather than by discipline. ResolveTask.id carries everything the request depends on — the memory cap is IN the preload id, so a cap change supersedes rather than dedups. Step 1 does not act on this: resolvers keep today's in-flight-promise dedup byte for byte. It is the seam Track A's RequestSlot consumes, shaped now so that landing it changes no public type. (Every one of R1/R2/R3/R5 is a keying bug, so the key is where the fix has to go.) SpatialEntryStore holds only ResourceResolver and cannot tell which package an implementation came from — which is what keeps "images is special" from becoming representable once ImagesResolver lands in vis. pointsResolver.spec.ts (20 tests) is the DoD box "the resolver is exercised by a test that constructs no deck layer and no GL context". It imports nothing from layers or vis. The deeper behavioural net stays pointsDataEngine.spec.ts, which must pass UNCHANGED through C5. Preserved verbatim, deliberately, because this commit is a re-housing and not a rewrite: R5 (ensureRowFeatureCodes takes no memory cap, so it loads at the 4M default against a possibly-8M resident batch and misaligns the filter mask). It is documented in place and left for Track A. 506 tests. Build and biome green. Co-Authored-By: Claude Opus 4.8 --- packages/core/src/engine/PointsResolver.ts | 916 ++++++++++++++++++ packages/core/src/engine/SpatialEntryStore.ts | 149 +++ packages/core/src/engine/index.ts | 17 + packages/core/src/engine/resolver.ts | 166 ++++ packages/core/tests/pointsResolver.spec.ts | 339 +++++++ 5 files changed, 1587 insertions(+) create mode 100644 packages/core/src/engine/PointsResolver.ts create mode 100644 packages/core/src/engine/SpatialEntryStore.ts create mode 100644 packages/core/src/engine/resolver.ts create mode 100644 packages/core/tests/pointsResolver.spec.ts diff --git a/packages/core/src/engine/PointsResolver.ts b/packages/core/src/engine/PointsResolver.ts new file mode 100644 index 00000000..9c688b85 --- /dev/null +++ b/packages/core/src/engine/PointsResolver.ts @@ -0,0 +1,916 @@ +import type { PointsElement } from '../models/index.js'; +import { featureCodeMapFromCatalog, remapRowFeatureCodes } from '../pointsFeatures.js'; +import { DEFAULT_POINTS_MEMORY_CAP } from '../pointsLimits.js'; +import type { PointsLoadProgress, PointsLoadResult } from '../pointsLoadOptions.js'; +import type { PointsFeatureCatalog } from '../pointsTiling.js'; +import type { EntryNotice } from './errors.js'; +import { Resolution } from './resolution.js'; +import type { EntryResources, ResolveContext, ResolveTask, ResourceResolver } from './resolver.js'; + +/** + * The points Resource Resolver. + * + * This is `PointsDataEngine`'s **cache and lifecycle half**, moved from + * `@spatialdata/layers` to `@spatialdata/core` per ADR 0004 §1. It owns the + * per-element resource lifecycle — the resident preload, the feature catalog, the + * per-row feature codes, and the whole-dataset feature-index scan — including + * their cache, request dedup, supersession, cancellation and streaming partials. + * + * ## What did NOT come with it, and why + * + * The three **render-resource memos** (`getResource`, `getMatchingResource`, + * `getMatchingPartialResource`) stayed behind, in `layers`, as + * `PointsRendererAdapter`. Identity-stable memoisation is a *deck* requirement — + * deck tears a layer down and rebuilds its batch when `data` identity changes — so + * ADR 0004 §4 puts it on the renderer side. The memo was not deleted; it was + * *rescheduled*, from lazily-on-first-getter-call to eagerly-once in `project()`. + * + * What this resolver exposes instead is the memos' **inputs, by identity**: + * {@link getData}, {@link getMatchedBatch}, {@link getPartialBatch}. Batches here + * are always *replaced*, never mutated in place, so object identity is an exact + * invalidation key. That matters: `pointsRenderResourceSignature` keys on row + * *count*, not identity, which is precisely why the old engine had to manually + * null `entry.resource` on every swap. The adapter keys on identity and needs no + * such bookkeeping. + * + * ## Alignment invariant (load-bearing) + * + * `getRowFeatureCodes(key)` is row-aligned with the resident batch from + * `ensureLoaded`. Both the geometry preload (`element.loadPoints()`) and the row + * codes (`element.loadRowFeatureCodes()`) read the first `min(rowCount, memoryCap)` + * rows in *file order*, so index i in the codes array names the feature of point i + * in the batch. + * + * **The memory cap must reach both calls identically or the filter mask is + * misaligned.** It currently does not: `ensureRowFeatureCodes` takes no cap and so + * falls back to the 4M default while `ensureLoaded` honours the user's. That is + * race R5, and it is Track A's to fix — this commit is a re-housing, and + * deliberately preserves the behaviour, bug and all. + */ + +export type PointsLoadStatus = 'idle' | 'loading' | 'ready' | 'error'; + +export interface PointsLoadTarget { + /** Stable element key — the cache/resolver key. */ + key: string; + /** Layer id, used only to report status back to the host. */ + layerId: string; + element: PointsElement; +} + +export interface PointsResolverCallbacks { + /** Report load-status transitions so the host can drive its load-state UI. */ + onStatus?: (layerId: string, status: PointsLoadStatus) => void; +} + +/** The serialisable points props this resolver plans against. */ +export interface PointsResolveConfig { + pointsMemoryCap?: number; + colorByFeature?: boolean; + featureCodes?: number[]; +} + +interface PointsEntry { + data?: PointsLoadResult; + /** Memory cap (max resident rows) the current `data`/`loading` was requested + * with. A change means the resident window must reload — see `ensureLoaded`. */ + memoryCap?: number; + /** Aborts the in-flight preload when it is superseded (a cap change), so a + * stale load doesn't run its expensive main-thread fallback to completion. */ + loadAbort?: AbortController; + status: PointsLoadStatus; + loading?: Promise; + /** Feature catalog: `undefined` while unloaded, `null` once settled for an + * element with no `feature_key`, else the catalog. `catalogLoaded` disambiguates + * "not yet requested" from "settled as null". */ + catalog?: PointsFeatureCatalog | null; + catalogLoaded?: boolean; + catalogLoading?: Promise; + /** True once the full-dataset catalog scan (`listFeaturesWithCounts`) has + * replaced any resident-subset preview. */ + catalogComplete?: boolean; + /** Per-row feature codes aligned to the resident batch (see class doc). */ + rowCodes?: ArrayLike; + rowCodesLoaded?: boolean; + rowCodesLoading?: Promise; + /** The catalog whose code space {@link rowCodes} are expressed in. */ + rowCodesCatalog?: PointsFeatureCatalog; + /** True when the element has a file-backed feature code column (authoritative + * codes; a real feature index). False for dictionary-only feature columns. */ + featureCodeColumn?: boolean; + /** Memoized distinct codes in {@link rowCodes}, invalidated by identity. Note + * this is a DATA memo (a Set), not a render resource — it stays in core. */ + residentCodes?: ReadonlySet; + residentCodesSource?: ArrayLike; + /** Whole-dataset points for the active selection, keyed by the selected-codes + * `signature` so a selection change rebuilds it. */ + matching?: { signature: string; result: PointsLoadResult }; + /** In-flight feature-index scan, with progressive counts from `onProgress`. */ + matchingLoading?: { + signature: string; + promise: Promise; + matchedRows: number; + scannedRows: number; + partialResult?: PointsLoadResult; + }; + /** Cached snapshot, so `snapshot()` is identity-stable between mutations. */ + snapshot?: { version: number; value: EntryResources }; +} + +/** Public snapshot of a selection's feature-index load, for the filter panel. */ +export interface PointsMatchingLoadState { + loading: boolean; + matchedRows: number; + scannedRows: number; + settled: boolean; + /** The selection is served by filtering a larger in-memory batch (no scan ran). */ + covered?: boolean; +} + +export class PointsResolver implements ResourceResolver { + readonly kind = 'points' as const; + /** Only the resident preload gates a first paint. The catalog, row codes and + * feature scan all refine an already-drawable layer. */ + readonly blockingResources = ['preload'] as const; + + private readonly entries = new Map(); + private readonly listeners = new Set<() => void>(); + private readonly callbacks: PointsResolverCallbacks; + private version = 0; + + constructor(callbacks: PointsResolverCallbacks = {}) { + this.callbacks = callbacks; + } + + // --- ResourceResolver ------------------------------------------------------- + + /** + * PURE, SYNC. What does this entry need? Starts nothing. + * + * These three conditions are exactly the ones the old code evaluated — but two + * of them were evaluated *inside `getLayers()`, during React render*, and kicked + * their loads with a bare `void engine.ensureX(...)`. They were always pure + * functions of config plus entry state; they were just being asked in the wrong + * phase. Here they cannot start work even by accident. + */ + plan(ctx: ResolveContext): readonly ResolveTask[] { + const { elementKey: key, config } = ctx; + const tasks: ResolveTask[] = []; + const cap = config.pointsMemoryCap ?? DEFAULT_POINTS_MEMORY_CAP; + + if (!this.isLoadedWithCap(key, cap)) { + // The cap IS in the id: a cap change must supersede, not dedup. (R3 is the + // matching path making exactly this mistake.) + tasks.push({ id: `${key}#preload:${cap}`, resource: 'preload', payload: { memoryCap: cap } }); + } + + const selection = config.featureCodes; + const selectionActive = selection !== undefined && selection.length > 0; + + // Was `void engine.ensureRowFeatureCodes(...)` at useLayerData.ts:1425. + const needsRowCodes = selectionActive || config.colorByFeature === true; + if (needsRowCodes && !this.hasRowFeatureCodes(key)) { + tasks.push({ id: `${key}#rowCodes`, resource: 'rowCodes' }); + } + + // Was `void engine.ensureMatchingFeaturesLoaded(...)` at useLayerData.ts:1375. + if (selectionActive && this.supportsFeatureScan(key)) { + const signature = PointsResolver.matchingSignature(selection); + tasks.push({ + id: `${key}#matching:${signature}:${cap}`, + resource: 'matching', + payload: { featureCodes: selection, memoryCap: cap }, + }); + } + + return tasks; + } + + /** ASYNC. The only place I/O starts. Dispatches to the lifecycle methods below. */ + async load( + task: ResolveTask, + ctx: ResolveContext, + _signal: AbortSignal + ): Promise { + const target: PointsLoadTarget = { + key: ctx.elementKey, + layerId: ctx.entryId, + element: ctx.element, + }; + const payload = task.payload as + | { memoryCap?: number; featureCodes?: readonly number[] } + | undefined; + const cap = payload?.memoryCap ?? DEFAULT_POINTS_MEMORY_CAP; + + switch (task.resource) { + case 'preload': + await this.ensureLoaded(target, cap); + return; + case 'catalog': + await this.ensureFeatureCatalog(target); + return; + case 'rowCodes': + await this.ensureRowFeatureCodes(target); + return; + case 'matching': + await this.ensureMatchingFeaturesLoaded(target, payload?.featureCodes ?? [], cap); + return; + default: + return; + } + } + + /** + * PURE, SYNC. Identity-stable between mutations — an adapter memoises against + * it, so a fresh object per call would be a deck teardown per frame. + */ + snapshot(ctx: ResolveContext): EntryResources { + const key = ctx.elementKey; + const entry = this.entries.get(key); + if (entry?.snapshot && entry.snapshot.version === this.version) { + return entry.snapshot.value; + } + + const value: EntryResources = { + entryId: ctx.entryId, + elementKey: key, + resources: { + preload: this.preloadResolution(key), + catalog: this.catalogResolution(key), + rowCodes: this.rowCodesResolution(key), + matching: this.matchingResolution(key), + }, + notices: this.notices(key, ctx.config.featureCodes), + bounds: null, // Points bounds come from the tiling metadata; not wired in Step 1. + revision: this.version, + }; + + if (entry) { + entry.snapshot = { version: this.version, value }; + } + return value; + } + + private preloadResolution(key: string): Resolution { + const entry = this.entries.get(key); + if (!entry) return Resolution.idle(); + switch (entry.status) { + case 'ready': + return entry.data ? Resolution.ready(entry.data) : Resolution.idle(); + case 'loading': + // `stale` is what keeps the old batch on screen through a cap raise — + // the atomic swap the old engine described as "no blank". + return Resolution.loading(entry.data !== undefined ? { stale: entry.data } : {}); + case 'error': + // Step 1 preserves today's behaviour: the engine console.errors and leaves + // status 'error' with no structured error value. Wiring SpatialEntryError + // through these paths is Track A's `retryable` work. + return Resolution.idle(); + default: + return Resolution.idle(); + } + } + + private catalogResolution(key: string): Resolution { + const entry = this.entries.get(key); + if (!entry) return Resolution.idle(); + if (entry.catalogLoaded) return Resolution.ready(entry.catalog ?? null); + return this.isFeatureCatalogLoading(key) ? Resolution.loading() : Resolution.idle(); + } + + private rowCodesResolution(key: string): Resolution | undefined> { + const entry = this.entries.get(key); + if (!entry) return Resolution.idle(); + if (entry.rowCodesLoaded) return Resolution.ready(entry.rowCodes); + return entry.rowCodesLoading ? Resolution.loading() : Resolution.idle(); + } + + private matchingResolution(key: string): Resolution { + const entry = this.entries.get(key); + if (!entry) return Resolution.idle(); + const loading = entry.matchingLoading; + if (loading) { + return Resolution.loading({ + ...(loading.partialResult !== undefined ? { partial: loading.partialResult } : {}), + ...(entry.matching !== undefined ? { stale: entry.matching.result } : {}), + progress: { done: loading.matchedRows, scanned: loading.scannedRows }, + }); + } + return entry.matching ? Resolution.ready(entry.matching.result) : Resolution.idle(); + } + + private notices(key: string, featureCodes: readonly number[] | undefined): EntryNotice[] { + const out: EntryNotice[] = []; + const truncation = this.getActiveTruncation(key, featureCodes); + if (truncation?.truncated && truncation.total !== undefined) { + out.push({ + kind: 'preload-truncated', + message: `Showing ${truncation.loaded.toLocaleString()} of ${truncation.total.toLocaleString()} points`, + loaded: truncation.loaded, + total: truncation.total, + }); + } + if (this.isFeatureCatalogRefining(key)) { + out.push({ + kind: 'catalog-is-resident-preview', + message: 'Loading the full feature list…', + }); + } + return out; + } + + // --- Subscription ----------------------------------------------------------- + + subscribe(listener: () => void): () => void { + this.listeners.add(listener); + return () => { + this.listeners.delete(listener); + }; + } + + getVersion(): number { + return this.version; + } + + private notify(): void { + this.version += 1; + for (const listener of this.listeners) { + listener(); + } + } + + // --- Reads ------------------------------------------------------------------ + + hasData(key: string): boolean { + return this.entries.get(key)?.data !== undefined; + } + + /** The resident preload batch. One of the three inputs the Renderer Adapter memoises. */ + getData(key: string): PointsLoadResult | undefined { + return this.entries.get(key)?.data; + } + + /** The settled matched-selection batch. Input to the adapter's matched memo. */ + getMatchedBatch(key: string): PointsLoadResult | undefined { + return this.entries.get(key)?.matching?.result; + } + + /** The in-flight scan's growing buffer. Input to the adapter's partial memo. */ + getPartialBatch(key: string): PointsLoadResult | undefined { + return this.entries.get(key)?.matchingLoading?.partialResult; + } + + getStatus(key: string): PointsLoadStatus { + return this.entries.get(key)?.status ?? 'idle'; + } + + /** Order-independent cache key for a selected-codes set. */ + private static matchingSignature(featureCodes: readonly number[]): string { + return [...featureCodes].sort((left, right) => left - right).join(','); + } + + /** Feature codes a matched batch/scan covers, parsed from its signature. */ + private static coveredCodes(signature: string): Set { + if (signature === '') { + return new Set(); + } + return new Set(signature.split(',').map(Number)); + } + + /** + * Whether an already-loaded batch still satisfies a (possibly changed) memory + * cap. A COMPLETE batch always does. A TRUNCATED batch only does while the new + * cap doesn't ask for more rows than it already holds — so lowering never + * reloads, and raising past a truncated batch does. + */ + private static batchAdequateForCap(result: PointsLoadResult, memoryCap: number): boolean { + if (!result.preloadTruncated) { + return true; + } + return (result.shape[1] ?? 0) >= memoryCap; + } + + /** + * Copy a resident batch keeping only its first `rows` points (file order), + * marked truncated. Used to shed rows when the memory cap is LOWERED below what + * is resident — so a 4M cap never keeps 8M rows around — without re-fetching. + * Columnar geometry + per-row codes are sliced in lockstep. + * + * `preloadTruncated: true` is load-bearing, not decoration: it is what tells + * `batchAdequateForCap` that raising the cap again must re-fetch. Drop it and a + * shed batch reads as complete, so the rows never come back. + */ + private static sliceResidentBatch(data: PointsLoadResult, rows: number): PointsLoadResult { + const sliceArray = (array: ArrayLike): ArrayLike => { + const maybeSliceable = array as unknown as { + slice?: (start: number, end: number) => ArrayLike; + }; + return typeof maybeSliceable.slice === 'function' + ? maybeSliceable.slice(0, rows) + : Array.prototype.slice.call(array, 0, rows); + }; + const dims = data.shape[0] ?? data.data.length; + return { + ...data, + shape: [dims, rows], + data: data.data.map(sliceArray), + ...(data.featureCodes ? { featureCodes: sliceArray(data.featureCodes) } : {}), + preloadTruncated: true, + }; + } + + /** Whether the resident batch is in its final state for this cap. */ + isLoadedWithCap(key: string, memoryCap: number): boolean { + const entry = this.entries.get(key); + if (entry?.data === undefined) { + return false; + } + return ( + PointsResolver.batchAdequateForCap(entry.data, memoryCap) && + (entry.data.shape[1] ?? 0) <= memoryCap + ); + } + + getResidentTruncation( + key: string + ): { truncated: boolean; loaded: number; total?: number } | undefined { + const data = this.entries.get(key)?.data; + if (!data) { + return undefined; + } + return { + truncated: data.preloadTruncated === true, + loaded: data.shape[1] ?? 0, + ...(data.totalRowCount !== undefined ? { total: data.totalRowCount } : {}), + }; + } + + /** + * Truncation state of what is actually on screen. With an active selection a + * scanned batch covers, that batch IS the render — report its count, not the + * resident preload's, or the panel keeps saying "showing 4M" over a filtered subset. + */ + getActiveTruncation( + key: string, + featureCodes: readonly number[] | undefined + ): { truncated: boolean; loaded: number; total?: number; filtered?: boolean } | undefined { + const entry = this.entries.get(key); + if (!entry) { + return undefined; + } + if (featureCodes && featureCodes.length > 0 && entry.matching) { + const covered = PointsResolver.coveredCodes(entry.matching.signature); + if (covered.size > 0 && featureCodes.every((code) => covered.has(code))) { + const result = entry.matching.result; + return { + truncated: result.preloadTruncated === true, + loaded: result.shape[1] ?? 0, + filtered: true, + }; + } + } + return this.getResidentTruncation(key); + } + + // --- Resident preload ------------------------------------------------------- + + /** + * Idempotently preload an element's points at a given memory cap. A no-op when + * the resident data already satisfies the cap. Only RAISING the cap past a + * *truncated* batch reloads, and the previous batch stays on screen until the + * larger one settles (an atomic swap — no blank). + */ + ensureLoaded( + target: PointsLoadTarget, + memoryCap: number = DEFAULT_POINTS_MEMORY_CAP + ): Promise { + const { key, layerId, element } = target; + const existing = this.entries.get(key); + // (1) Existing data covers this cap without a reload. + if ( + existing?.data !== undefined && + PointsResolver.batchAdequateForCap(existing.data, memoryCap) + ) { + if (existing.loading) { + existing.loadAbort?.abort(); + existing.loading = undefined; + existing.loadAbort = undefined; + } + existing.memoryCap = memoryCap; + // Cap lowered below what's resident → shed the excess in memory (no re-fetch). + if ((existing.data.shape[1] ?? 0) > memoryCap) { + existing.data = PointsResolver.sliceResidentBatch(existing.data, memoryCap); + existing.residentCodes = undefined; + existing.residentCodesSource = undefined; + if (existing.rowCodes && existing.rowCodes.length > memoryCap) { + existing.rowCodes = Array.prototype.slice.call(existing.rowCodes, 0, memoryCap); + } + this.notify(); + } + return Promise.resolve(); + } + // (2) A load for this exact cap is already in flight → dedup. + if (existing?.loading && existing.memoryCap === memoryCap) { + return existing.loading; + } + + const entry: PointsEntry = existing ?? { status: 'idle' }; + entry.loadAbort?.abort(); + entry.memoryCap = memoryCap; + const abort = new AbortController(); + entry.loadAbort = abort; + entry.status = 'loading'; + this.entries.set(key, entry); + this.callbacks.onStatus?.(layerId, 'loading'); + + const loading = (async () => { + try { + // Read the feature column with the geometry so the filter's catalog and + // per-row codes come from this one decode. The catalog here reflects only + // the *resident* batch — an instant preview the full-dataset scan may + // still supersede. + const data = await element.loadPoints({ + includeFeatureCodes: true, + memoryCap, + signal: abort.signal, + }); + if (abort.signal.aborted || entry.memoryCap !== memoryCap) { + return; + } + entry.data = data; + entry.residentCodes = undefined; + entry.residentCodesSource = undefined; + entry.status = 'ready'; + entry.featureCodeColumn = data.hasFeatureCodeColumn === true; + if (data.featureCatalog !== undefined && !entry.catalogComplete) { + entry.catalog = data.featureCatalog; + entry.catalogLoaded = true; + } + if (data.featureCodes !== undefined) { + entry.rowCodes = data.featureCodes; + entry.rowCodesLoaded = true; + entry.rowCodesCatalog = data.featureCatalog; + this.reconcileRowCodes(entry); + } + this.callbacks.onStatus?.(layerId, 'ready'); + } catch (error) { + // Aborted (cap changed) or superseded → not a real error; stay quiet. + if (abort.signal.aborted || entry.memoryCap !== memoryCap) { + return; + } + entry.status = 'error'; + this.callbacks.onStatus?.(layerId, 'error'); + console.error(`Failed to load points for ${layerId}:`, error); + } finally { + if (entry.memoryCap === memoryCap) { + entry.loading = undefined; + entry.loadAbort = undefined; + } + this.notify(); + } + })(); + entry.loading = loading; + return loading; + } + + // --- Feature-index scan (whole-dataset load of a selection) ------------------ + + ensureMatchingFeaturesLoaded( + target: PointsLoadTarget, + featureCodes: readonly number[], + memoryCap: number = DEFAULT_POINTS_MEMORY_CAP + ): Promise { + const { key, element } = target; + const entry = this.entries.get(key) ?? { status: 'idle' as PointsLoadStatus }; + this.entries.set(key, entry); + const signature = PointsResolver.matchingSignature(featureCodes); + const isCoveredBy = (sig: string): boolean => { + const covered = PointsResolver.coveredCodes(sig); + return featureCodes.every((code) => covered.has(code)); + }; + // A loaded batch already covers this selection AND still satisfies the cap → + // reuse it; the layer filters down. No scan. + if ( + entry.matching && + isCoveredBy(entry.matching.signature) && + PointsResolver.batchAdequateForCap(entry.matching.result, memoryCap) + ) { + entry.matchingLoading = undefined; + return Promise.resolve(); + } + // An in-flight scan will cover this selection once it settles → wait for it. + if (entry.matchingLoading && isCoveredBy(entry.matchingLoading.signature)) { + return entry.matchingLoading.promise; + } + + const PROGRESS_NOTIFY_STEP = 5_000; + let lastNotifiedMatched = 0; + const onProgress = (progress: PointsLoadProgress): void => { + const loading = entry.matchingLoading; + if (!loading || loading.signature !== signature) { + return; + } + loading.matchedRows = progress.matchedRows; + loading.scannedRows = progress.scannedRows; + loading.partialResult = progress.partialResult; + if (progress.matchedRows - lastNotifiedMatched >= PROGRESS_NOTIFY_STEP) { + lastNotifiedMatched = progress.matchedRows; + this.notify(); // runs during the async scan, not render — safe to notify sync + } + }; + + const promise = (async () => { + try { + // Dict-only elements have no file-backed code column, so the scan must + // resolve each row's feature_name against the same catalog the selection + // was made in. The core call ignores this for indexed elements. + const featureCodeByName = + entry.featureCodeColumn === true ? undefined : featureCodeMapFromCatalog(entry.catalog); + const result = await element.loadPointsMatchingFeatureCodes({ + featureCodes, + memoryCap, + onProgress, + ...(featureCodeByName ? { featureCodeByName } : {}), + }); + // Apply only if this is still the latest requested scan. Keeping the + // previous `matching` batch until the current one is ready is what lets + // the render keep showing the prior selection instead of blanking. + if (entry.matchingLoading?.signature === signature) { + entry.matching = { signature, result }; + } + } catch (error) { + console.error(`Failed feature-index scan for ${target.layerId}:`, error); + } finally { + if (entry.matchingLoading?.signature === signature) { + entry.matchingLoading = undefined; + } + this.notify(); + } + })(); + entry.matchingLoading = { signature, promise, matchedRows: 0, scannedRows: 0 }; + // No queueMicrotask here, and none needed: nothing kicks a scan from render + // any more. `plan()` is pure and returns a task; the store calls `load()` from + // a commit-phase effect. The old engine's `queueMicrotask(() => this.notify())` + // existed solely to defend against a synchronous notify during render, and the + // phase separation makes that unreachable by construction. + this.notify(); + return promise; + } + + /** Whether the feature-index scan for this exact selection is in flight. */ + isMatchingLoading(key: string, featureCodes: readonly number[]): boolean { + const entry = this.entries.get(key); + return entry?.matchingLoading?.signature === PointsResolver.matchingSignature(featureCodes); + } + + getMatchingLoadState( + key: string, + featureCodes: readonly number[] + ): PointsMatchingLoadState | undefined { + const entry = this.entries.get(key); + if (!entry) { + return undefined; + } + const signature = PointsResolver.matchingSignature(featureCodes); + const loading = entry.matchingLoading; + if (loading?.signature === signature) { + return { + loading: true, + matchedRows: loading.matchedRows, + scannedRows: loading.scannedRows, + settled: false, + }; + } + const matching = entry.matching; + if (!matching) { + return undefined; + } + if (matching.signature === signature) { + return { + loading: false, + matchedRows: matching.result.shape[1] ?? 0, + scannedRows: matching.result.shape[1] ?? 0, + settled: true, + }; + } + // A larger loaded batch covers this selection — served from memory, no scan. + const covered = PointsResolver.coveredCodes(matching.signature); + if (covered.size > 0 && featureCodes.every((code) => covered.has(code))) { + return { + loading: false, + matchedRows: matching.result.shape[1] ?? 0, + scannedRows: matching.result.shape[1] ?? 0, + settled: true, + covered: true, + }; + } + return undefined; + } + + /** The feature codes the settled matched batch covers. */ + getLoadedMatchingFeatureCodes(key: string): ReadonlySet | undefined { + const matching = this.entries.get(key)?.matching; + if (!matching) { + return undefined; + } + return PointsResolver.coveredCodes(matching.signature); + } + + /** Per-row feature codes of the settled matched batch, row-aligned with it. */ + getMatchingRowFeatureCodes(key: string): ArrayLike | undefined { + return this.entries.get(key)?.matching?.result.featureCodes; + } + + /** Per-row feature codes of the in-flight scan's partial buffer. */ + getMatchingPartialRowFeatureCodes(key: string): ArrayLike | undefined { + return this.entries.get(key)?.matchingLoading?.partialResult?.featureCodes; + } + + // --- Feature catalog -------------------------------------------------------- + + getFeatureCatalog(key: string): PointsFeatureCatalog | null | undefined { + const entry = this.entries.get(key); + return entry?.catalogLoaded ? (entry.catalog ?? null) : undefined; + } + + isFeatureCatalogLoading(key: string): boolean { + const entry = this.entries.get(key); + if (!entry || entry.catalogLoaded) { + return false; + } + // The catalog rides the geometry preload, so a running geometry load counts as + // the catalog loading too — a spinner, not a premature "load feature list" prompt. + return entry.catalogLoading !== undefined || entry.loading !== undefined; + } + + /** True while the full-dataset scan runs behind an instant resident-subset preview. */ + isFeatureCatalogRefining(key: string): boolean { + const entry = this.entries.get(key); + return ( + entry?.catalogLoaded === true && + entry.catalogComplete !== true && + entry.catalogLoading !== undefined + ); + } + + /** True when the element has a file-backed feature code column (globally authoritative). */ + hasFeatureCodeColumn(key: string): boolean { + return this.entries.get(key)?.featureCodeColumn === true; + } + + /** + * Whether a whole-dataset feature scan can run — i.e. reach matching points + * beyond the resident preload window. True with a file-backed code column, AND + * for dictionary-only elements once a catalog is loaded (the scan resolves each + * row's `feature_name` against that catalog's code space). + */ + supportsFeatureScan(key: string): boolean { + const entry = this.entries.get(key); + if (!entry) { + return false; + } + return entry.featureCodeColumn === true || (entry.catalogLoaded === true && !!entry.catalog); + } + + /** + * Re-express `rowCodes` in the current catalog's code space when it was derived + * against an older one (resident preview → full-dataset upgrade). No-op for + * authoritative file-backed codes, which are identical across builds. + */ + private reconcileRowCodes(entry: PointsEntry): void { + if (entry.featureCodeColumn === true) { + return; + } + const source = entry.rowCodesCatalog; + const target = entry.catalog; + if (!entry.rowCodes || !source || !target || source === target) { + return; + } + entry.rowCodes = remapRowFeatureCodes(entry.rowCodes, source, target); + entry.rowCodesCatalog = target; + entry.residentCodes = undefined; + entry.residentCodesSource = undefined; + } + + /** + * The distinct feature codes present in the resident batch. The panel greys + * features outside this set, so selecting one that isn't loaded — which would + * render no points — is understandable rather than a glitch. + */ + getResidentFeatureCodes(key: string): ReadonlySet | undefined { + const entry = this.entries.get(key); + const rowCodes = entry?.rowCodes; + if (!entry || rowCodes === undefined) { + return undefined; + } + if (entry.residentCodes && entry.residentCodesSource === rowCodes) { + return entry.residentCodes; + } + const set = new Set(); + for (let i = 0; i < rowCodes.length; i += 1) { + set.add(rowCodes[i] as number); + } + entry.residentCodes = set; + entry.residentCodesSource = rowCodes; + return set; + } + + /** + * Idempotently build the *full-dataset* feature catalog. Runs even when a + * resident-subset preview is showing, and supersedes it. + */ + ensureFeatureCatalog(target: PointsLoadTarget): Promise { + const { key, element } = target; + const entry = this.entries.get(key) ?? { status: 'idle' as PointsLoadStatus }; + this.entries.set(key, entry); + if (entry.catalogComplete) { + return Promise.resolve(); + } + if (entry.catalogLoading) { + return entry.catalogLoading; + } + + const loading = (async () => { + try { + const fullCatalog = await element.listFeaturesWithCounts(); + entry.catalog = fullCatalog; + // The full-dataset catalog is authoritative. Re-express any resident row + // codes in its code space so the render's per-row codes match the panel's + // selection and swatches. + this.reconcileRowCodes(entry); + } catch (error) { + // Keep any resident preview catalog on failure rather than blanking it. + if (!entry.catalogLoaded) entry.catalog = null; + console.error(`Failed to build points feature catalog for ${target.layerId}:`, error); + } finally { + entry.catalogLoaded = true; + entry.catalogComplete = true; + entry.catalogLoading = undefined; + this.notify(); + } + })(); + entry.catalogLoading = loading; + this.notify(); // surface the loading transition to the panel + return loading; + } + + // --- Row feature codes ------------------------------------------------------ + + getRowFeatureCodes(key: string): ArrayLike | undefined { + return this.entries.get(key)?.rowCodes; + } + + /** True once row codes have settled (even if the element has none). */ + hasRowFeatureCodes(key: string): boolean { + return this.entries.get(key)?.rowCodesLoaded === true; + } + + /** + * Idempotently load the row feature codes for the resident batch. + * + * NOTE (R5): this takes no memory cap, so core falls back to the 4M default + * while `ensureLoaded` honours the user's — misaligning the filter mask against + * an 8M resident batch. Preserved verbatim here; it is Track A's to fix. + */ + ensureRowFeatureCodes(target: PointsLoadTarget): Promise { + const { key, element } = target; + const entry = this.entries.get(key) ?? { status: 'idle' as PointsLoadStatus }; + this.entries.set(key, entry); + if (entry.rowCodesLoaded) { + return Promise.resolve(); + } + if (entry.rowCodesLoading) { + return entry.rowCodesLoading; + } + + const loading = (async () => { + try { + const catalog = this.getFeatureCatalog(key); + entry.rowCodes = await element.loadRowFeatureCodes({ featureCatalog: catalog }); + entry.rowCodesCatalog = catalog ?? undefined; + this.reconcileRowCodes(entry); + } catch (error) { + entry.rowCodes = undefined; + console.error(`Failed to load points row feature codes for ${target.layerId}:`, error); + } finally { + entry.rowCodesLoaded = true; + entry.rowCodesLoading = undefined; + this.notify(); + } + })(); + entry.rowCodesLoading = loading; + return loading; + } + + // --- Lifecycle -------------------------------------------------------------- + + /** Drop an element from the cache. Catalog and row codes live in the same entry. */ + evict(key: string): void { + this.entries.delete(key); + } + + dispose(): void { + this.entries.clear(); + this.listeners.clear(); + } +} diff --git a/packages/core/src/engine/SpatialEntryStore.ts b/packages/core/src/engine/SpatialEntryStore.ts new file mode 100644 index 00000000..5806d9b5 --- /dev/null +++ b/packages/core/src/engine/SpatialEntryStore.ts @@ -0,0 +1,149 @@ +import type { SpatialEntryKind } from './errors.js'; +import type { EntryResources, ResolveContext, ResolveTask, ResourceResolver } from './resolver.js'; + +/** + * The reconcile loop over a Render Stack's Spatial Entries. + * + * This is what the 400-line `Promise.all` switch in `useLayerData` collapses into. + * It knows nothing about any kind: it holds a `Record` and + * calls `plan` → `load` → `snapshot` on whichever resolver a context names. + * + * That opacity is the point. A resolver's *package* is an implementation detail — + * `PointsResolver` and `ShapesResolver` live in `core`, `ImagesResolver` and + * `LabelsResolver` in `vis` — and the store must never be able to tell, because + * the moment it can, "images is special" becomes representable and the interface + * stops being one interface. If a `vis`-resident resolver needs something this + * loop doesn't offer, that is a signal about the interface, not a licence to + * special-case a kind here. + */ + +// biome-ignore lint/suspicious/noExplicitAny: the registry is heterogeneous by design — each +// resolver has its own config and element types, and the store deliberately cannot see them. +export type ResolverRegistry = Readonly>>; + +// biome-ignore lint/suspicious/noExplicitAny: see above. +export type AnyResolveContext = ResolveContext; + +export class SpatialEntryStore { + private readonly resolvers: ResolverRegistry; + private readonly listeners = new Set<() => void>(); + private readonly unsubscribes: Array<() => void> = []; + /** One AbortController per in-flight task id. Superseding cancels the old one. */ + private readonly inFlight = new Map(); + 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)) { + this.unsubscribes.push(resolver.subscribe(() => this.notify())); + } + } + + subscribe = (listener: () => void): (() => void) => { + this.listeners.add(listener); + return () => { + this.listeners.delete(listener); + }; + }; + + getVersion = (): number => this.version; + + private notify(): void { + this.version += 1; + for (const listener of this.listeners) { + listener(); + } + } + + /** + * PURE, SYNC. What work do these entries need? Starts nothing. + * + * Safe to call during render — which is the whole point of splitting it from + * {@link reconcile}. Nothing here can begin a load even by accident. + */ + plan(contexts: readonly AnyResolveContext[]): Array<[AnyResolveContext, ResolveTask]> { + const tasks: Array<[AnyResolveContext, ResolveTask]> = []; + for (const ctx of contexts) { + const resolver = this.resolvers[ctx.kind]; + if (!resolver) continue; + for (const task of resolver.plan(ctx)) { + tasks.push([ctx, task]); + } + } + return tasks; + } + + /** + * ASYNC. Plan, then load. Call from a commit-phase effect, never from render. + * + * Dedup and supersession are still each resolver's own business in Step 1 — they + * keep today's in-flight-promise checks, byte for byte. The store only tracks an + * `AbortController` per task id so a superseded request can be cancelled; the + * `id` carries everything the request depends on, which is the seam Track A's + * `RequestSlot` will take over. + */ + async reconcile(contexts: readonly AnyResolveContext[]): Promise { + const tasks = this.plan(contexts); + if (tasks.length === 0) { + return; + } + + await Promise.all( + tasks.map(async ([ctx, task]) => { + const resolver = this.resolvers[ctx.kind]; + if (!resolver) return; + + const previous = this.inFlight.get(task.id); + if (previous) { + // Same id ⇒ same request ⇒ the resolver will dedup. Don't abort it. + return; + } + const controller = new AbortController(); + this.inFlight.set(task.id, controller); + try { + await resolver.load(task, ctx, controller.signal); + } finally { + // Only clear if still ours — a superseding request installs its own. + if (this.inFlight.get(task.id) === controller) { + this.inFlight.delete(task.id); + } + } + }) + ); + } + + /** PURE, SYNC. The resolved state of one entry. Identity-stable between mutations. */ + snapshot(ctx: AnyResolveContext): EntryResources | undefined { + return this.resolvers[ctx.kind]?.snapshot(ctx); + } + + /** Is this entry still waiting on a resource it cannot first-paint without? */ + isBlocking(ctx: AnyResolveContext): boolean { + const resolver = this.resolvers[ctx.kind]; + if (!resolver) return false; + const snapshot = resolver.snapshot(ctx); + return resolver.blockingResources.some((name) => { + const resolution = snapshot.resources[name]; + if (!resolution) return false; + // Loading with a retained `stale` still draws — that is what `stale` is FOR. + // It blocks only when there is nothing to show at all. + if (resolution.status === 'loading') return resolution.stale === undefined; + return resolution.status === 'idle'; + }); + } + + evict(kind: SpatialEntryKind, elementKey: string): void { + this.resolvers[kind]?.evict(elementKey); + } + + dispose(): void { + for (const unsubscribe of this.unsubscribes) unsubscribe(); + this.unsubscribes.length = 0; + for (const controller of this.inFlight.values()) controller.abort(); + this.inFlight.clear(); + for (const resolver of Object.values(this.resolvers)) resolver.dispose(); + this.listeners.clear(); + } +} diff --git a/packages/core/src/engine/index.ts b/packages/core/src/engine/index.ts index 81739591..7a83000f 100644 --- a/packages/core/src/engine/index.ts +++ b/packages/core/src/engine/index.ts @@ -13,6 +13,23 @@ export type { SpatialEntryKind, } from './errors.js'; export { isCancellation, toSpatialEntryError } from './errors.js'; +export type { + PointsLoadStatus, + PointsLoadTarget, + PointsMatchingLoadState, + PointsResolveConfig, + PointsResolverCallbacks, +} from './PointsResolver.js'; +export { PointsResolver } from './PointsResolver.js'; export type { ResolutionProgress } from './resolution.js'; // `Resolution` is both the type and its constructor namespace — one export carries both. export { fromResult, Resolution } from './resolution.js'; +export type { + EntryResources, + ResolveContext, + ResolvedBounds, + ResolveTask, + ResourceResolver, +} from './resolver.js'; +export type { AnyResolveContext, ResolverRegistry } from './SpatialEntryStore.js'; +export { SpatialEntryStore } from './SpatialEntryStore.js'; diff --git a/packages/core/src/engine/resolver.ts b/packages/core/src/engine/resolver.ts new file mode 100644 index 00000000..52e3ca7c --- /dev/null +++ b/packages/core/src/engine/resolver.ts @@ -0,0 +1,166 @@ +/** + * The Resource Resolver interface (ADR 0004). + * + * A resolver turns structural Render Stack inputs into stable loaded resources. + * It is store-agnostic AND renderer-agnostic: it knows nothing about deck.gl, + * React, Viv, or three.js. `tgpu-htj2k` (three.js/WebGPU, separate repo) consumes + * the same interface `@spatialdata/vis` does — that second consumer is the whole + * reason this lives in `core` rather than behind deck.gl. + * + * ## The phase separation — the load-bearing part + * + * | Phase | Owner | Purity | When | May start I/O? | + * |------------|------------------|------------|-------------------|-------------------------| + * | `plan()` | Resolver | pure, sync | commit only | no — *returns* tasks | + * | `load()` | Resolver | async | commit only | **yes — the only place**| + * | `project()`| Renderer Adapter | pure, sync | end of reconcile | no | + * | `render()` | Renderer Adapter | pure, sync | during React render | no — gets no handle | + * + * `project()` and `render()` are NOT on this interface. They belong to the + * Renderer Adapter, in `@spatialdata/layers` — identity-stable memoisation is a + * *deck* requirement (deck tears a layer down when its data identity changes), so + * it belongs on the renderer side (ADR 0004 §4). + * + * The payoff is a type error, not a code-review note: because `render()` is handed + * a frozen projected state and no engine handle, today's + * `void engine.ensureMatchingFeaturesLoaded(...)` inside `getLayers()` **cannot + * compile**. Its intent moves to `plan()`, which is where it always belonged — + * both its conditions are pure functions of config and entry state. + * + * ## Placement is per-kind, driven by dependency (ADR 0004 §6, as amended) + * + * `core` defines this interface. Implementations live in the package their + * dependencies already live in: `PointsResolver` and `ShapesResolver` in `core` + * (every type they touch is already a `core` type); `ImagesResolver` and + * `LabelsResolver` in `vis`, next to Viv and `avivatorish`. + * + * A resolver's *package* is an implementation detail. The store holds only + * `ResourceResolver` and must never know which package an implementation came + * from. If a `vis`-resident resolver needs something this interface does not + * offer, that is a signal about the **interface** — not a licence to special-case + * images. + */ + +import type { EntryNotice, SpatialEntryKind } from './errors.js'; +import type { Resolution } from './resolution.js'; + +/** World-space axis-aligned bounds: `[minX, minY, maxX, maxY]`. */ +export type ResolvedBounds = [number, number, number, number]; + +/** + * Everything a resolver needs to reason about one Spatial Entry, this commit. + * + * `TElement` is the concrete element type (`PointsElement`, `ShapesElement`, …); + * `TConfig` is the entry's serialisable renderer props. Both are generic so that + * `core` never has to know a renderer's prop shape. + */ +export interface ResolveContext { + /** The Stack Entry's stable id — `layerId` today. */ + readonly entryId: string; + /** The SpatialData element key. THE cache key: several entries may share one. */ + readonly elementKey: string; + readonly kind: SpatialEntryKind; + readonly element: TElement; + /** Serialisable renderer props. Never a Runtime Attachment — no callbacks. */ + readonly config: TConfig; +} + +/** + * One unit of I/O. **Pure data** — no closures, no promises, no element handles. + * + * `plan()` returns these instead of starting work, which is what makes `plan()` + * safely callable during render. + */ +export interface ResolveTask { + /** + * Identifies this request. Stable within (entry, resource) for as long as the + * request means the same thing: **same id ⇒ dedup; different id ⇒ supersede**. + * + * So everything the request depends on must be IN the id. That is not incidental + * — every one of the four known points races is a keying bug: two live requests + * with equal keys (R1, R2), a key missing the memory cap (R3), or a key missing a + * dimension entirely (R5). + * + * Step 1 does NOT act on this: resolvers keep today's in-flight-promise dedup, + * byte for byte. This is the seam Track A's `RequestSlot` will key off, and + * shaping it now is what lets that land without touching a public type. + */ + readonly id: string; + /** Which resource of the entry — 'preload' | 'catalog' | 'geometry' | 'tooltip' | … */ + readonly resource: string; + /** Opaque to the store; only the resolver that planned it reads it. */ + readonly payload?: unknown; +} + +/** + * A resolver's per-entry output. Frozen, identity-stable, and safe to read during + * render. + */ +export interface EntryResources { + readonly entryId: string; + readonly elementKey: string; + /** + * Failure is **per-resource, not per-entry** (ADR 0004 §3). A shapes entry with + * a broken tooltip column must still draw its geometry, so there is no + * entry-wide `Result` here — only a resolution per named resource. + */ + readonly resources: Readonly>>; + /** Non-fatal facts about a *successful* resolve. Healthy data never renders as an error. */ + readonly notices: readonly EntryNotice[]; + /** World bounds, once anything is loaded. Computed by the resolver — a + * `vis`-resident one may reach for viv's `getImageSize`; `core` never needs to. */ + readonly bounds: ResolvedBounds | null; + /** Bumps iff anything above changed identity. Lets an adapter skip `project()`. */ + readonly revision: number; +} + +export interface ResourceResolver { + readonly kind: SpatialEntryKind; + + /** + * Which resources must be `ready` before this entry can first paint. + * + * Data, not a switch — this is what `isBlocking` becomes. It also preserves + * today's asymmetry honestly: points block on their preload and shapes on their + * geometry, but a tooltip or a fill-colour column has never blocked a first + * paint and must not start. + */ + readonly blockingResources: readonly string[]; + + /** + * What work does this entry need? **Pure and synchronous. Must not start I/O.** + * + * Called on every reconcile, so it must be cheap and idempotent: returning a + * task whose `id` matches an in-flight one is how dedup happens. + */ + plan(ctx: ResolveContext): readonly ResolveTask[]; + + /** + * Do the work. **The only place I/O may start.** + * + * Must classify its own failures into a `Resolution.failed` via + * `toSpatialEntryError` — checking `isCancellation(cause)` FIRST, because an + * aborted load is a non-event and must not paint an error. It must not throw. + */ + load( + task: ResolveTask, + ctx: ResolveContext, + signal: AbortSignal + ): Promise; + + /** + * The entry's resolved state. **Pure and synchronous.** + * + * Must return the SAME object by identity when nothing has changed — an adapter + * memoises against it, and a fresh identity per call is a deck teardown per frame. + */ + snapshot(ctx: ResolveContext): EntryResources; + + /** Subscribe to cache mutations. Returns an unsubscribe. */ + subscribe(listener: () => void): () => void; + /** Monotonic; backs `useSyncExternalStore`. */ + getVersion(): number; + /** Drop one element from the cache. */ + evict(elementKey: string): void; + dispose(): void; +} diff --git a/packages/core/tests/pointsResolver.spec.ts b/packages/core/tests/pointsResolver.spec.ts new file mode 100644 index 00000000..0be5a47d --- /dev/null +++ b/packages/core/tests/pointsResolver.spec.ts @@ -0,0 +1,339 @@ +import { describe, expect, it, vi } from 'vitest'; +import { + type PointsResolveConfig, + PointsResolver, + Resolution, + type ResolveContext, + SpatialEntryStore, +} from '../src/engine/index.js'; +import type { PointsElement } from '../src/models/index.js'; +import type { PointsLoadResult } from '../src/pointsLoadOptions.js'; + +/** + * The points Resource Resolver, driven headless. + * + * **This file IS the "resolver is exercised by a test that constructs no deck + * layer and no GL context" box in ADR 0004's definition of done.** It imports + * nothing from `layers` or `vis`, renders nothing, and touches no canvas — which + * is exactly what `tgpu-htj2k` needs to be true in order to consume this at all. + * + * The behavioural surface (cache, dedup, cap handling, catalog supersession, the + * dict-only remap) is already pinned in far more detail by + * `layers/tests/pointsDataEngine.spec.ts`, which must keep passing UNCHANGED + * through the split — that spec is the real regression net. What this file adds is + * the parts that are NEW: the plan/load phase separation, and the Resolution-shaped + * snapshot. + */ + +const batch = (pointCount: number, over: Partial = {}): PointsLoadResult => ({ + shape: [2, pointCount], + data: [ + new Float32Array(Array.from({ length: pointCount }, (_, i) => i)), + new Float32Array(Array.from({ length: pointCount }, (_, i) => i)), + ], + featureCodes: new Int32Array(Array.from({ length: pointCount }, (_, i) => i % 2)), + hasFeatureCodeColumn: true, + ...over, +}); + +function element(over: Partial> = {}) { + return { + key: 'transcripts', + loadPoints: vi.fn(async () => batch(4)), + listFeaturesWithCounts: vi.fn(async () => null), + loadRowFeatureCodes: vi.fn(async () => new Int32Array([0, 1, 0, 1])), + loadPointsMatchingFeatureCodes: vi.fn(async () => batch(2)), + ...over, + } as unknown as PointsElement; +} + +const ctx = ( + el: PointsElement, + config: PointsResolveConfig = {} +): ResolveContext => ({ + entryId: 'layer-p', + elementKey: 'transcripts', + kind: 'points', + element: el, + config, +}); + +describe('plan() — pure, synchronous, starts nothing', () => { + // The load-bearing claim of the whole phase separation. Two of these conditions + // used to be evaluated inside getLayers() DURING RENDER and kicked with a bare + // `void engine.ensureX(...)`. They were always pure functions of config + entry + // state; they were being asked in the wrong phase. Now they cannot start work. + it('does not touch the element', () => { + const el = element(); + const resolver = new PointsResolver(); + + resolver.plan(ctx(el, { featureCodes: [0, 1], colorByFeature: true })); + + expect(el.loadPoints).not.toHaveBeenCalled(); + expect(el.loadRowFeatureCodes).not.toHaveBeenCalled(); + expect(el.loadPointsMatchingFeatureCodes).not.toHaveBeenCalled(); + }); + + it('plans a preload for a fresh entry', () => { + const tasks = new PointsResolver().plan(ctx(element())); + + expect(tasks.map((t) => t.resource)).toEqual(['preload']); + }); + + it('puts the memory cap IN the task id, so a cap change supersedes rather than dedups', () => { + const resolver = new PointsResolver(); + + const at4m = resolver.plan(ctx(element(), { pointsMemoryCap: 4_000_000 }))[0]; + const at8m = resolver.plan(ctx(element(), { pointsMemoryCap: 8_000_000 }))[0]; + + // Same id ⇒ dedup; different id ⇒ supersede. R3 is the matching path getting + // exactly this wrong — dedup on signature alone, ignoring the cap entirely. + expect(at4m?.id).not.toBe(at8m?.id); + expect(at4m?.id).toContain('4000000'); + }); + + it('plans rowCodes only when a filter or colour-by-feature needs them', () => { + const resolver = new PointsResolver(); + const resources = (config: PointsResolveConfig) => + resolver.plan(ctx(element(), config)).map((t) => t.resource); + + expect(resources({})).not.toContain('rowCodes'); + expect(resources({ colorByFeature: true })).toContain('rowCodes'); + expect(resources({ featureCodes: [0] })).toContain('rowCodes'); + // An empty selection is "no filter", not "filter to nothing". + expect(resources({ featureCodes: [] })).not.toContain('rowCodes'); + }); + + it('plans a matching scan only once the element is known to support one', async () => { + const resolver = new PointsResolver(); + const el = element(); + const config: PointsResolveConfig = { featureCodes: [0] }; + + // Before anything loads we cannot know whether a scan is even possible. + expect(resolver.plan(ctx(el, config)).map((t) => t.resource)).not.toContain('matching'); + + await resolver.ensureLoaded({ key: 'transcripts', layerId: 'layer-p', element: el }); + + expect(resolver.plan(ctx(el, config)).map((t) => t.resource)).toContain('matching'); + }); + + it('stops planning a preload once one is resident', async () => { + const resolver = new PointsResolver(); + const el = element(); + + await resolver.load( + { id: 'x', resource: 'preload', payload: { memoryCap: 4_000_000 } }, + ctx(el), + new AbortController().signal + ); + + expect(resolver.plan(ctx(el, { pointsMemoryCap: 4_000_000 })).map((t) => t.resource)).toEqual( + [] + ); + }); +}); + +describe('load() — the only place I/O starts', () => { + it('dispatches each task to its lifecycle method', async () => { + const resolver = new PointsResolver(); + const el = element(); + const signal = new AbortController().signal; + + await resolver.load({ id: 'a', resource: 'preload' }, ctx(el), signal); + expect(el.loadPoints).toHaveBeenCalledTimes(1); + + await resolver.load({ id: 'b', resource: 'catalog' }, ctx(el), signal); + expect(el.listFeaturesWithCounts).toHaveBeenCalledTimes(1); + + await resolver.load( + { id: 'c', resource: 'matching', payload: { featureCodes: [0] } }, + ctx(el), + signal + ); + expect(el.loadPointsMatchingFeatureCodes).toHaveBeenCalledTimes(1); + }); + + it('ignores an unknown resource rather than throwing', async () => { + const resolver = new PointsResolver(); + + await expect( + resolver.load({ id: 'z', resource: 'nonsense' }, ctx(element()), new AbortController().signal) + ).resolves.toBeUndefined(); + }); +}); + +describe('snapshot() — per-resource resolutions, identity-stable', () => { + it('is idle before anything is planned', () => { + const snapshot = new PointsResolver().snapshot(ctx(element())); + + expect(Resolution.isIdle(snapshot.resources.preload as never)).toBe(true); + expect(snapshot.notices).toEqual([]); + }); + + it('reports the resident batch as ready, by reference', async () => { + const resolver = new PointsResolver(); + const el = element(); + await resolver.ensureLoaded({ key: 'transcripts', layerId: 'layer-p', element: el }); + + const snapshot = resolver.snapshot(ctx(el)); + + expect(Resolution.readyValue(snapshot.resources.preload as never)).toBe( + resolver.getData('transcripts') + ); + }); + + it('returns the SAME object until something mutates — an adapter memoises on this', async () => { + const resolver = new PointsResolver(); + const el = element(); + await resolver.ensureLoaded({ key: 'transcripts', layerId: 'layer-p', element: el }); + + const first = resolver.snapshot(ctx(el)); + + // Ten "renders" with no state change. A fresh object here is a deck teardown + // per frame — the pan flash, one layer up. + for (let i = 0; i < 10; i++) { + expect(resolver.snapshot(ctx(el))).toBe(first); + } + }); + + it('returns a NEW object once state changes', async () => { + const resolver = new PointsResolver(); + const el = element(); + await resolver.ensureLoaded({ key: 'transcripts', layerId: 'layer-p', element: el }); + const before = resolver.snapshot(ctx(el)); + + await resolver.ensureFeatureCatalog({ key: 'transcripts', layerId: 'layer-p', element: el }); + const after = resolver.snapshot(ctx(el)); + + expect(after).not.toBe(before); + }); + + it('keeps a failed resource from blanking a healthy one — failure is PER-RESOURCE', async () => { + // A points entry whose catalog scan fails must still draw its geometry. That is + // the whole reason there is no entry-wide Result. + const resolver = new PointsResolver(); + const el = element({ + listFeaturesWithCounts: vi.fn(async () => { + throw new Error('catalog scan exploded'); + }), + }); + vi.spyOn(console, 'error').mockImplementation(() => {}); + + await resolver.ensureLoaded({ key: 'transcripts', layerId: 'layer-p', element: el }); + await resolver.ensureFeatureCatalog({ key: 'transcripts', layerId: 'layer-p', element: el }); + + const snapshot = resolver.snapshot(ctx(el)); + + expect(Resolution.isReady(snapshot.resources.preload as never)).toBe(true); + expect(Resolution.readyValue(snapshot.resources.catalog as never)).toBeNull(); + }); + + it('carries `stale` through a cap raise, so the old batch keeps drawing', async () => { + const resolver = new PointsResolver(); + let resolveSecond!: (b: PointsLoadResult) => void; + let call = 0; + const el = element({ + loadPoints: vi.fn(async () => { + call += 1; + if (call === 1) return batch(4, { preloadTruncated: true, totalRowCount: 100 }); + return new Promise((r) => { + resolveSecond = r; + }); + }), + }); + const target = { key: 'transcripts', layerId: 'layer-p', element: el }; + + await resolver.ensureLoaded(target, 4); + const settled = resolver.getData('transcripts'); + + // Raise the cap past a truncated batch → a real reload, still in flight. + const pending = resolver.ensureLoaded(target, 8); + const midFlight = resolver.snapshot(ctx(el)); + + const preload = midFlight.resources.preload as never; + expect(Resolution.isLoading(preload)).toBe(true); + // The atomic swap: the previous batch is retained and still drawable. + expect(Resolution.lastGood(preload)).toBe(settled); + + resolveSecond(batch(8)); + await pending; + }); + + it('surfaces a truncated preload as a NOTICE, not an error — healthy data with a caveat', async () => { + const resolver = new PointsResolver(); + const el = element({ + loadPoints: vi.fn(async () => batch(4, { preloadTruncated: true, totalRowCount: 1_000_000 })), + }); + + await resolver.ensureLoaded({ key: 'transcripts', layerId: 'layer-p', element: el }, 4); + const snapshot = resolver.snapshot(ctx(el)); + + expect(Resolution.isReady(snapshot.resources.preload as never)).toBe(true); + expect(snapshot.notices).toEqual([ + expect.objectContaining({ kind: 'preload-truncated', loaded: 4, total: 1_000_000 }), + ]); + }); +}); + +describe('SpatialEntryStore — the reconcile loop', () => { + const store = (resolver: PointsResolver) => + new SpatialEntryStore({ + points: resolver, + // Step 1 registers all four; only points is exercised here. + shapes: resolver, + images: resolver, + labels: resolver, + }); + + it('plans and loads in one pass', async () => { + const resolver = new PointsResolver(); + const el = element(); + + await store(resolver).reconcile([ctx(el)]); + + expect(el.loadPoints).toHaveBeenCalledTimes(1); + expect(resolver.hasData('transcripts')).toBe(true); + }); + + it('is idempotent — a second reconcile with nothing changed does no I/O', async () => { + const resolver = new PointsResolver(); + const el = element(); + const s = store(resolver); + + await s.reconcile([ctx(el)]); + await s.reconcile([ctx(el)]); + + expect(el.loadPoints).toHaveBeenCalledTimes(1); + }); + + it('blocks on the preload, and stops blocking once it is drawable', async () => { + const resolver = new PointsResolver(); + const el = element(); + const s = store(resolver); + + expect(s.isBlocking(ctx(el))).toBe(true); + + await s.reconcile([ctx(el)]); + + expect(s.isBlocking(ctx(el))).toBe(false); + }); + + it('does not block on a resource that is merely refining', async () => { + // A catalog scan or a feature scan refines an already-drawable layer. Only the + // preload gates a first paint — and blockingResources says so as DATA, which is + // what today's isBlocking kind-switch collapses into. + const resolver = new PointsResolver(); + + expect(resolver.blockingResources).toEqual(['preload']); + }); + + it('bumps its version when any resolver mutates', async () => { + const resolver = new PointsResolver(); + const s = store(resolver); + const before = s.getVersion(); + + await s.reconcile([ctx(element())]); + + expect(s.getVersion()).toBeGreaterThan(before); + }); +}); From ba4283774f8de5062a471ead623327e28dbc4fa2 Mon Sep 17 00:00:00 2001 From: Peter Todd Date: Tue, 14 Jul 2026 19:14:54 +0100 Subject: [PATCH 6/9] PointsRendererAdapter in layers; PointsDataEngine becomes a facade MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The split lands. PointsDataEngine (940 lines) becomes a 228-line facade over PointsResolver in core (the cache and lifecycle, framework-free) and PointsRendererAdapter in layers (the three render-resource memos, 135 lines). ACCEPTANCE: packages/layers/tests/pointsDataEngine.spec.ts passes UNCHANGED, byte for byte — 35 tests, not a character edited. That was the whole point of keeping the facade, and it is the only credible proof that a 940-line class was cut in half without changing what it does. The memos move because ADR 0004 §4 says identity-stable memoisation is a *deck* requirement — deck tears a layer down when its data identity changes — so it belongs on the renderer side. Nothing was deleted: the memo was RESCHEDULED, from "lazily, whenever a getter is first called this frame" to "eagerly, once, at the end of reconcile". The interesting part is the invalidation key. pointsRenderResourceSignature keys on the batch's row COUNT, not its identity — two different batches with the same row count produce the same signature. That is exactly why the old engine had to reach in and manually null entry.resource on every swap and every in-memory shed: the signature alone would happily serve a stale resource. Once the memo lives outside the entry that manual invalidation isn't available, so the adapter keys on the batch OBJECT IDENTITY as well. That is exact, not approximate: PointsResolver always replaces a batch, never mutates one in place, so identity changes precisely when the data changes and never otherwise. The bookkeeping disappears — and with it the class of bug where someone adds a new mutation path and forgets to invalidate. The facade survives because it has to. `pointsEngine` is member 8 of useLayerData's seventeen, and PointsFeatureState.tsx calls ten of these methods directly. Keeping it is what lets the split land without touching the panels, the 845-line spec, or MDV. It goes away in Step 3, once the panels read from project() and nothing needs a live engine handle. pointsResourceIdentity.spec.ts (added in C3, before any of this moved) passes unchanged too — all three resources still identity-stable across repeated reads, including the matched and partial ones that no test covered before. 506 tests. Build, biome and the dependency boundaries all green. Co-Authored-By: Claude Opus 4.8 --- .../src/adapters/PointsRendererAdapter.ts | 135 +++ .../layers/src/engine/PointsDataEngine.ts | 978 +++--------------- 2 files changed, 268 insertions(+), 845 deletions(-) create mode 100644 packages/layers/src/adapters/PointsRendererAdapter.ts diff --git a/packages/layers/src/adapters/PointsRendererAdapter.ts b/packages/layers/src/adapters/PointsRendererAdapter.ts new file mode 100644 index 00000000..54fa6dde --- /dev/null +++ b/packages/layers/src/adapters/PointsRendererAdapter.ts @@ -0,0 +1,135 @@ +import type { PointsElement, PointsLoadResult } from '@spatialdata/core'; +import type { PointsRenderResource } from '../pointsLoader.js'; +import { + pointsRenderResourceSignature, + resolvePointsRenderResource, +} from '../resolvePointsRenderResource.js'; + +/** + * The deck Renderer Adapter for points: the three render-resource memos. + * + * These used to live inside `PointsDataEngine`, memoising lazily on read because + * their only caller was React's render phase. ADR 0004 §4 puts them here instead: + * *"identity-stable memoisation is a **deck requirement** — deck tears a layer down + * when its data identity changes — so it belongs on the renderer side."* + * + * Nothing was deleted. The memo was **rescheduled**, not removed: from "lazily, + * whenever a getter is first called this frame" to "eagerly, once, at the end of + * reconcile". The `PointsResolver` in `core` now owns the *data*; this owns the + * *identity* the renderer needs. + * + * ## Why these memos key on object identity, and the old ones could not + * + * `pointsRenderResourceSignature` keys on the batch's row **count**, not its + * identity — two different batches with the same row count produce the same + * signature. That is why the old engine had to reach in and manually null + * `entry.resource` on every swap and every in-memory shed: the signature alone + * would happily serve a stale resource. + * + * Once the memo lives outside the entry that manual invalidation is not available + * — so these memos key on the batch **object identity** as well. That is exact: + * `PointsResolver` always *replaces* a batch, never mutates one in place. Identity + * changes precisely when the data changes, and never otherwise. The bookkeeping + * disappears, and with it the class of bug where someone adds a new mutation path + * and forgets to invalidate. + * + * The signature is still checked, because it also captures the element key and the + * resolve options — things identity alone would miss. + */ + +interface ResourceMemo { + /** The batch this resource was built from. Identity is the invalidation key. */ + source: PointsLoadResult; + /** Also checked: catches element/option changes that identity alone would miss. */ + signature: string; + resource: PointsRenderResource; +} + +interface EntryMemos { + resident?: ResourceMemo; + matched?: ResourceMemo; + partial?: ResourceMemo; +} + +const RESOLVE_OPTIONS = { experimentalOptimizations: 'off' as const }; + +/** A batch with no points must not produce a resource — see the empty-lock guard below. */ +const isEmpty = (batch: PointsLoadResult): boolean => (batch.shape[1] ?? 0) === 0; + +export class PointsRendererAdapter { + private readonly memos = new Map(); + + private entry(key: string): EntryMemos { + let memos = this.memos.get(key); + if (!memos) { + memos = {}; + this.memos.set(key, memos); + } + return memos; + } + + private resolve( + memos: EntryMemos, + slot: keyof EntryMemos, + element: PointsElement, + batch: PointsLoadResult + ): PointsRenderResource | null { + const cache = { preloaded: batch, metadataKnown: false }; + const signature = pointsRenderResourceSignature(element, cache, RESOLVE_OPTIONS); + const memo = memos[slot]; + if (memo && memo.source === batch && memo.signature === signature) { + return memo.resource; + } + const resource = resolvePointsRenderResource(element, cache, RESOLVE_OPTIONS); + if (resource) { + memos[slot] = { source: batch, signature, resource }; + } + return resource; + } + + /** + * The resident preload's render resource. Stable across renders and pans, so the + * `PointsLayer` composite does not reset its async-loaded batch — resolving afresh + * per call would blank the layer for a frame (the pan flash). + */ + getResource(element: PointsElement, key: string, batch: PointsLoadResult | undefined) { + if (!batch) return null; + return this.resolve(this.entry(key), 'resident', element, batch); + } + + /** + * The matched selection's render resource. + * + * Empty-lock guard: a scan that matched no rows must NOT supersede the resident + * preview, or the render locks to an empty batch with no way to recover — the + * settled selection never re-scans. Returning null falls back to resident + * filtering. A legitimately empty selection cannot reach here: an empty + * `featureCodes` selection short-circuits before any scan is kicked. + */ + getMatchingResource(element: PointsElement, key: string, batch: PointsLoadResult | undefined) { + if (!batch || isEmpty(batch)) return null; + return this.resolve(this.entry(key), 'matched', element, batch); + } + + /** + * The in-flight scan's growing buffer, as a resource, so points progressively + * fill in before the full scan settles. Rebuilds only when a new chunk grows the + * buffer — not per pan, which is when the user is most likely to be moving. + */ + getMatchingPartialResource( + element: PointsElement, + key: string, + batch: PointsLoadResult | undefined + ) { + if (!batch || isEmpty(batch)) return null; + return this.resolve(this.entry(key), 'partial', element, batch); + } + + evict(key: string): void { + this.memos.delete(key); + } + + dispose(): void { + this.memos.clear(); + } +} diff --git a/packages/layers/src/engine/PointsDataEngine.ts b/packages/layers/src/engine/PointsDataEngine.ts index 160c384d..977f3090 100644 --- a/packages/layers/src/engine/PointsDataEngine.ts +++ b/packages/layers/src/engine/PointsDataEngine.ts @@ -1,940 +1,228 @@ import { - DEFAULT_POINTS_MEMORY_CAP, - featureCodeMapFromCatalog, type PointsElement, type PointsFeatureCatalog, - type PointsLoadProgress, type PointsLoadResult, - remapRowFeatureCodes, + PointsResolver, } from '@spatialdata/core'; +import { PointsRendererAdapter } from '../adapters/PointsRendererAdapter.js'; import type { PointsRenderResource } from '../pointsLoader.js'; -import { - pointsRenderResourceSignature, - resolvePointsRenderResource, -} from '../resolvePointsRenderResource.js'; /** - * Framework-agnostic points loading/caching/resolution engine. + * `PointsDataEngine` — now a **facade** over `PointsResolver` (`core`) and + * `PointsRendererAdapter` (`layers`). + * + * The class is split, not deleted, and its surface is unchanged. That is not + * timidity — it is the compat contract. `pointsEngine` is member 8 of + * `useLayerData`'s seventeen, and `PointsFeatureState.tsx` calls ten of these + * methods directly. Keeping the facade is what lets the split land without + * touching the panels, the 845-line `pointsDataEngine.spec.ts`, or MDV. + * + * **The acceptance test for the split is that `pointsDataEngine.spec.ts` passes + * unchanged, byte for byte.** If it doesn't, the split is wrong. + * + * ## The split * - * This is step 1b of the LayerDataEngine decomposition: the points-only - * sub-engine, extracted from the `@spatialdata/vis` `useLayerData` hook so the - * cache + orchestration live in React-free `@spatialdata/layers` and can be - * unit-tested headlessly. It owns exactly what the hook's points path used to - * hold as `useRef` state: + * - **`PointsResolver` (`core`)** — the cache and lifecycle: the resident preload, + * the feature catalog, the per-row codes, the feature-index scan. Framework-free, + * so `tgpu-htj2k` can consume it. (ADR 0004 §1.) + * - **`PointsRendererAdapter` (`layers`)** — the three render-resource memos. + * Identity-stable memoisation is a *deck* requirement, so it lives on the + * renderer side. (ADR 0004 §4.) * - * - the per-element preloaded batch cache (`loadedDataRef.points`), - * - the stable render-resource memo (`stablePointsResourceRef`), - * - the async preload orchestration (the load effect's points branch). + * The facade holds both and routes each call to the half that owns it. The resource + * getters read the resolver's batch and hand it to the adapter, which memoises on + * that batch's identity — so `getResource(element, key)` twice in a row still + * returns the same object, exactly as before. * - * Scope: the *preloaded flat scatter* path plus the **feature catalog** and - * **row feature codes** that MVP step 2 (feature filter) needs. Metadata - * probing, Morton tiling, and tile-debug state are still dark — later MVP steps - * wire them *into this engine* (see docs/plans/points-mvp-and-roadmap). + * ## Retiring this * - * Alignment invariant (load-bearing): `getRowFeatureCodes(key)` is row-aligned - * with the resident batch from `ensureLoaded`. Both the geometry preload - * (`element.loadPoints()`) and the row codes (`element.loadRowFeatureCodes()`) - * read the first `min(rowCount, memoryCap)` rows in *file order* under the same - * default memory cap, so index i in the codes array names the feature of point i - * in the batch. If a configurable memory cap is ever threaded, it MUST go to - * both calls identically or the filter mask will be misaligned. + * Step 3. Once `useLayerData` reads a resolver snapshot and the panels read from + * `project()`, nothing needs a live engine handle and this file goes away. Until + * then it is load-bearing, and its surface is a promise. */ -export type PointsLoadStatus = 'idle' | 'loading' | 'ready' | 'error'; - -export interface PointsLoadTarget { - /** Stable element key — the cache/resolver key. */ - key: string; - /** Layer id, used only to report status back to the host. */ - layerId: string; - element: PointsElement; -} - -export interface PointsDataEngineCallbacks { - /** Report load-status transitions so the host can drive its load-state UI. */ - onStatus?: (layerId: string, status: PointsLoadStatus) => void; -} +export type { + PointsLoadStatus, + PointsLoadTarget, + PointsMatchingLoadState, +} from '@spatialdata/core'; -interface PointsEntry { - data?: PointsLoadResult; - /** Memory cap (max resident rows) the current `data`/`loading` was requested - * with. A change means the resident window must reload — see `ensureLoaded`. */ - memoryCap?: number; - /** Aborts the in-flight preload when it is superseded (a cap change), so a - * stale load doesn't run its expensive main-thread fallback to completion. */ - loadAbort?: AbortController; - status: PointsLoadStatus; - loading?: Promise; - resource?: { signature: string; resource: PointsRenderResource }; - /** Feature catalog: `undefined` while unloaded, `null` once settled for an - * element with no `feature_key`, else the catalog. `catalogLoaded` disambiguates - * "not yet requested" from "settled as null". */ - catalog?: PointsFeatureCatalog | null; - catalogLoaded?: boolean; - catalogLoading?: Promise; - /** True once the full-dataset catalog scan (`listFeaturesWithCounts`) has - * replaced any resident-subset preview. Until then `catalog` may reflect only - * the resident batch, so the full scan is allowed to run and supersede it. */ - catalogComplete?: boolean; - /** Per-row feature codes aligned to the resident batch (see class doc). Value - * is `undefined` when the element exposes no feature codes; `rowCodesLoaded` - * marks the settled state. */ - rowCodes?: ArrayLike; - rowCodesLoaded?: boolean; - rowCodesLoading?: Promise; - /** The catalog whose code space {@link rowCodes} are expressed in. When the - * catalog is upgraded (resident preview → full-dataset), `reconcileRowCodes` - * remaps `rowCodes` into the new space and updates this — keeping the render's - * per-row codes aligned with the panel's selection codes for dictionary-only - * datasets, where codes are app-assigned and can differ between catalog builds. */ - rowCodesCatalog?: PointsFeatureCatalog; - /** True when the element has a file-backed feature code column (authoritative - * codes; a real feature index). Undefined until the resident batch loads; false - * for dictionary-only feature columns. Gates the whole-dataset feature-index - * scan — see {@link hasFeatureCodeColumn}. */ - featureCodeColumn?: boolean; - /** Memoized distinct codes in {@link rowCodes}, invalidated by identity via - * `residentCodesSource` (see `getResidentFeatureCodes`). */ - residentCodes?: ReadonlySet; - residentCodesSource?: ArrayLike; - /** Whole-dataset points for the active selection, loaded via the feature-index - * scan and keyed by the selected-codes `signature` so a selection change - * rebuilds it. `resource` is the stable render resource, built lazily. */ - matching?: { signature: string; result: PointsLoadResult; resource?: PointsRenderResource }; - /** In-flight feature-index scan, with progressive counts updated from the - * scan's `onProgress` so the panel can show partial stats as they accumulate. */ - matchingLoading?: { - signature: string; - promise: Promise; - matchedRows: number; - scannedRows: number; - partialResult?: PointsLoadResult; - /** SPIKE: render resource built from `partialResult`, cached on that chunk's - * identity so it only rebuilds when a new chunk arrives (not every pan). */ - partialResource?: { source: PointsLoadResult; resource: PointsRenderResource }; - }; -} +import type { + PointsLoadStatus, + PointsLoadTarget, + PointsMatchingLoadState, + PointsResolverCallbacks, +} from '@spatialdata/core'; -/** Public snapshot of a selection's feature-index load, for the filter panel. */ -export interface PointsMatchingLoadState { - /** The scan for this exact selection is in flight. */ - loading: boolean; - /** Matched points so far (progressive while loading; final once settled). */ - matchedRows: number; - /** Rows examined so far (progressive while loading; final once settled). */ - scannedRows: number; - /** True once the scan for this selection has settled. */ - settled: boolean; - /** The selection is served by filtering a larger in-memory batch (a removal - * reused it — no scan ran). `matchedRows` is then the whole batch, not the - * drawn subset, so the panel words it as "served from memory". */ - covered?: boolean; -} +export type PointsDataEngineCallbacks = PointsResolverCallbacks; export class PointsDataEngine { - private readonly entries = new Map(); - private readonly listeners = new Set<() => void>(); - private readonly callbacks: PointsDataEngineCallbacks; - /** Monotonic cache-mutation counter. Backs a `useSyncExternalStore` snapshot so - * React reliably re-renders on every settled load — including late async - * completions (e.g. the full-dataset catalog scan) that a plain subscribe → - * bump-a-counter → pull-during-render pattern was dropping. */ - private version = 0; + private readonly resolver: PointsResolver; + private readonly adapter = new PointsRendererAdapter(); constructor(callbacks: PointsDataEngineCallbacks = {}) { - this.callbacks = callbacks; - } - - /** Subscribe to cache mutations (async load settled). Returns an unsubscribe. */ - subscribe(listener: () => void): () => void { - this.listeners.add(listener); - return () => { - this.listeners.delete(listener); - }; + this.resolver = new PointsResolver(callbacks); } - /** Snapshot for `useSyncExternalStore`: changes on every {@link notify}. */ - getVersion(): number { - return this.version; + /** The underlying resolver, for callers that have migrated off the facade. */ + get resourceResolver(): PointsResolver { + return this.resolver; } - private notify(): void { - this.version += 1; - for (const listener of this.listeners) { - listener(); - } - } + // --- Subscription ----------------------------------------------------------- - hasData(key: string): boolean { - return this.entries.get(key)?.data !== undefined; + subscribe(listener: () => void): () => void { + return this.resolver.subscribe(listener); } - getData(key: string): PointsLoadResult | undefined { - return this.entries.get(key)?.data; + getVersion(): number { + return this.resolver.getVersion(); } - getStatus(key: string): PointsLoadStatus { - return this.entries.get(key)?.status ?? 'idle'; - } + // --- Render resources (adapter-owned) --------------------------------------- /** - * Resolve a **stable** render resource for an element. Memoized by signature - * so repeated calls (every render / pan-zoom frame) reuse the same loader - * identity: the `PointsLayer` composite resets its async-loaded batch whenever - * the loader identity changes, which would blank the layer for a frame (the - * pan flash) if we resolved afresh each call. Returns null until data loads. + * Resolve a **stable** render resource for an element. Memoised on the resident + * batch's identity, so repeated calls (every render / pan-zoom frame) reuse the + * same loader identity: `PointsLayer` resets its async-loaded batch whenever the + * loader identity changes, which would blank the layer for a frame. Null until + * data loads. */ getResource(element: PointsElement, key: string): PointsRenderResource | null { - const entry = this.entries.get(key); - if (!entry?.data) { - return null; - } - const cache = { preloaded: entry.data, metadataKnown: false }; - const options = { experimentalOptimizations: 'off' as const }; - const signature = pointsRenderResourceSignature(element, cache, options); - if (entry.resource && entry.resource.signature === signature) { - return entry.resource.resource; - } - const resource = resolvePointsRenderResource(element, cache, options); - if (resource) { - entry.resource = { signature, resource }; - } - return resource; - } - - // --- Feature-index render scan (whole-dataset load of a selection) ---------- - - /** Order-independent cache key for a selected-codes set. */ - private static matchingSignature(featureCodes: readonly number[]): string { - return [...featureCodes].sort((left, right) => left - right).join(','); - } - - /** Feature codes a matched batch/scan covers, parsed from its signature - * (sorted-codes-joined; `''` → the empty selection). */ - private static coveredCodes(signature: string): Set { - if (signature === '') { - return new Set(); - } - return new Set(signature.split(',').map(Number)); + return this.adapter.getResource(element, key, this.resolver.getData(key)); } - /** - * Whether an already-loaded batch (resident preload OR matched scan) still - * satisfies a (possibly changed) memory cap. A COMPLETE batch (it captured all - * rows before hitting the cap) always does. A TRUNCATED batch (it filled up to - * its cap, more rows exist) only does while the new cap doesn't ask for more - * rows than it already holds — so lowering the cap never reloads/rescans, and - * raising it past a truncated batch does, to fetch the extra rows. - */ - private static batchAdequateForCap(result: PointsLoadResult, memoryCap: number): boolean { - if (!result.preloadTruncated) { - return true; - } - return (result.shape[1] ?? 0) >= memoryCap; + /** Render resource for the settled matched-selection batch. */ + getMatchingResource(element: PointsElement, key: string): PointsRenderResource | null { + return this.adapter.getMatchingResource(element, key, this.resolver.getMatchedBatch(key)); } - /** - * Copy a resident batch keeping only its first `rows` points (file order), - * marked truncated. Used to shed rows when the memory cap is LOWERED below what - * is resident — so a 4M cap never keeps 8M rows around — without re-fetching. - * Columnar geometry + per-row codes are sliced in lockstep. - */ - private static sliceResidentBatch(data: PointsLoadResult, rows: number): PointsLoadResult { - const sliceArray = (array: ArrayLike): ArrayLike => { - const maybeSliceable = array as unknown as { - slice?: (start: number, end: number) => ArrayLike; - }; - return typeof maybeSliceable.slice === 'function' - ? maybeSliceable.slice(0, rows) - : Array.prototype.slice.call(array, 0, rows); - }; - const dims = data.shape[0] ?? data.data.length; - return { - ...data, - shape: [dims, rows], - data: data.data.map(sliceArray), - ...(data.featureCodes ? { featureCodes: sliceArray(data.featureCodes) } : {}), - preloadTruncated: true, - }; + /** Render resource for the in-flight scan's growing partial buffer. */ + getMatchingPartialResource(element: PointsElement, key: string): PointsRenderResource | null { + return this.adapter.getMatchingPartialResource( + element, + key, + this.resolver.getPartialBatch(key) + ); + } + + // --- Lifecycle (resolver-owned) --------------------------------------------- + + ensureLoaded(target: PointsLoadTarget, memoryCap?: number): Promise { + return memoryCap === undefined + ? this.resolver.ensureLoaded(target) + : this.resolver.ensureLoaded(target, memoryCap); } - /** - * Ensure the selected features' points are available for rendering. The scan - * loads the whole dataset for a selection (footer stats skip non-matching row - * groups) and retains the per-row codes, so the render can **filter that batch - * in the layer**. That makes a selection that is a SUBSET of an already-loaded - * batch a free in-memory filter — removing a feature never re-scans (its rows - * are already in memory), symmetric with resident filtering. A scan runs only - * when the selection needs codes no loaded/in-flight batch covers. Settles → - * `notify()`. - */ ensureMatchingFeaturesLoaded( target: PointsLoadTarget, featureCodes: readonly number[], - memoryCap: number = DEFAULT_POINTS_MEMORY_CAP + memoryCap?: number ): Promise { - const { key, element } = target; - // there will be various mutating side-effects on entry as we progress... - // so maybe that could include gradual accumulation of points, - // pending a less side-effect/mutation-ridden approach. - // we've been hitting a lot of general issues debugging this in general, - // (not necessarily this particular point in the code) and the behaviour is not right. - // I think I'm inclined to more purity. Might consider using Effect? - // would be a much bigger future change. - const entry = this.entries.get(key) ?? { status: 'idle' as PointsLoadStatus }; - this.entries.set(key, entry); - const signature = PointsDataEngine.matchingSignature(featureCodes); - const isCoveredBy = (sig: string): boolean => { - const covered = PointsDataEngine.coveredCodes(sig); - return featureCodes.every((code) => covered.has(code)); - }; - // A loaded batch already covers this selection AND still satisfies the memory - // cap → reuse it, the layer filters down to the current codes. No scan. This - // is both the removal fast path and the cap-lowering fast path: dropping the - // cap (or any cap change where the loaded rows already suffice) never rescans. - if ( - entry.matching && - isCoveredBy(entry.matching.signature) && - PointsDataEngine.batchAdequateForCap(entry.matching.result, memoryCap) - ) { - // Any in-flight scan for a different (now-unneeded) selection is superseded. - entry.matchingLoading = undefined; - return Promise.resolve(); - } - // An in-flight scan will cover this selection once it settles (e.g. a feature - // was removed mid-scan) → wait for it rather than starting another. - if (entry.matchingLoading && isCoveredBy(entry.matchingLoading.signature)) { - return entry.matchingLoading.promise; - } - - // Notify at most every `PROGRESS_NOTIFY_STEP` matched rows so the panel's - // partial stats update live without a re-render per scanned row group. - // (not sure how important this is, may prefer to see more granular update) - const PROGRESS_NOTIFY_STEP = 5_000; - let lastNotifiedMatched = 0; - const onProgress = (progress: PointsLoadProgress): void => { - // I'm a bit iffy about this ambient stateful thing - const loading = entry.matchingLoading; - if (!loading || loading.signature !== signature) { - return; - } - loading.matchedRows = progress.matchedRows; - loading.scannedRows = progress.scannedRows; - // we have a partialResult, which includes an accumulated buffer - // probably prefer to have AsyncGenerator throughout rather than this - // we're not doing the right thing yet, just seeing if we can push some data - // and render... at very least needs cleaning up resources, etc etc - loading.partialResult = progress.partialResult; - if (progress.matchedRows - lastNotifiedMatched >= PROGRESS_NOTIFY_STEP) { - lastNotifiedMatched = progress.matchedRows; - this.notify(); // runs during the async scan, not render — safe to notify sync - } - }; - - const promise = (async () => { - try { - // Dict-only elements have no file-backed code column, so the scan must - // resolve each row's feature_name against the same catalog the selection - // was made in. Pass that map; the core call ignores it for indexed - // elements (which match on their code column instead). - const featureCodeByName = - entry.featureCodeColumn === true ? undefined : featureCodeMapFromCatalog(entry.catalog); - //todo streamy version - const result = await element.loadPointsMatchingFeatureCodes({ - featureCodes, - memoryCap, - onProgress, - ...(featureCodeByName ? { featureCodeByName } : {}), - }); - // Apply only if this is still the latest requested scan — a newer - // selection may have superseded it while we were loading. Keeping the - // previous `matching` batch until the current one is ready is what lets - // the render keep showing the prior selection instead of blanking. - if (entry.matchingLoading?.signature === signature) { - entry.matching = { signature, result }; - } - } catch (error) { - console.error(`Failed feature-index scan for ${target.layerId}:`, error); - } finally { - if (entry.matchingLoading?.signature === signature) { - entry.matchingLoading = undefined; - } - this.notify(); - } - })(); - entry.matchingLoading = { signature, promise, matchedRows: 0, scannedRows: 0 }; - // Surface the loading transition, but DEFER it: this method is kicked from - // `getLayers` *during* render, so a synchronous notify would setState mid- - // render. A microtask runs after the current render commits. - queueMicrotask(() => this.notify()); - return promise; + return memoryCap === undefined + ? this.resolver.ensureMatchingFeaturesLoaded(target, featureCodes) + : this.resolver.ensureMatchingFeaturesLoaded(target, featureCodes, memoryCap); } - /** - * Load-state snapshot for a selection's feature-index scan: whether it is in - * flight, its progressive matched/scanned counts, and its final counts once - * settled. Drives the panel's "loading … / N points" indicator. Returns - * `undefined` when this selection has neither loaded nor started. - */ - getMatchingLoadState( - key: string, - featureCodes: readonly number[] - ): PointsMatchingLoadState | undefined { - const entry = this.entries.get(key); - const signature = PointsDataEngine.matchingSignature(featureCodes); - if (entry?.matchingLoading?.signature === signature) { - return { - loading: true, - matchedRows: entry.matchingLoading.matchedRows, - scannedRows: entry.matchingLoading.scannedRows, - settled: false, - }; - } - if (entry?.matching?.signature === signature) { - const result = entry.matching.result; - return { - loading: false, - matchedRows: result.shape[1] ?? 0, - scannedRows: result.scannedRowCount ?? 0, - settled: true, - }; - } - // The selection is a subset of an already-loaded batch (a removal reused it). - // It is settled — the layer just filters the batch — so report it as loaded - // rather than letting the indicator vanish. `covered` lets the panel word it - // as "served from memory" since the count is the whole in-memory batch. - if (entry?.matching && PointsDataEngine.coveredCodes(entry.matching.signature).size > 0) { - const covered = PointsDataEngine.coveredCodes(entry.matching.signature); - if (featureCodes.length > 0 && featureCodes.every((code) => covered.has(code))) { - const result = entry.matching.result; - return { - loading: false, - matchedRows: result.shape[1] ?? 0, - scannedRows: result.scannedRowCount ?? 0, - settled: true, - covered: true, - }; - } - } - return undefined; + ensureFeatureCatalog(target: PointsLoadTarget): Promise { + return this.resolver.ensureFeatureCatalog(target); } - /** - * Feature codes of the **last completed** matched selection — i.e. the - * non-resident features whose points are actually on screen right now (see - * {@link getMatchingResource}, which keeps that batch during a new scan). - * - * The panel greys features that are neither resident nor rendered. Deriving - * "rendered" from this last-completed set (not the current scan's settled - * state) is what keeps already-loaded features un-greyed while a newly added - * feature's scan is still in flight. `undefined` when nothing has settled. - */ - getLoadedMatchingFeatureCodes(key: string): ReadonlySet | undefined { - const signature = this.entries.get(key)?.matching?.signature; - if (signature === undefined) { - return undefined; - } - return PointsDataEngine.coveredCodes(signature); + ensureRowFeatureCodes(target: PointsLoadTarget): Promise { + return this.resolver.ensureRowFeatureCodes(target); } - /** - * Per-row feature codes of the last-completed matched batch, row-aligned with - * {@link getMatchingResource}'s geometry. The render passes these to the layer - * as `preloadedFeatureCodes` so it can filter the (possibly superset) matched - * batch down to the current selection in memory — no re-scan on a removal. - */ - getMatchingRowFeatureCodes(key: string): ArrayLike | undefined { - return this.entries.get(key)?.matching?.result.featureCodes; - } + // --- Reads (resolver-owned) ------------------------------------------------- - /** - * Stable render resource for the **last completed** matched selection, or null - * if no selection has ever settled. Deliberately NOT keyed to the current - * selection: while a new selection's scan is in flight, this keeps returning the - * previous selection's batch so the render shows those points instead of - * blanking for the (potentially multi-second) scan. - * - * The resource is cached on the matched batch and only changes identity when the - * batch does, so panning doesn't reset the composite. - * Pair with `getMatchingLoadState` (exact-signature) - * for the "is the current selection loaded" question. - */ - getMatchingResource(element: PointsElement, key: string): PointsRenderResource | null { - const entry = this.entries.get(key); - if (!entry?.matching) { - return null; - } - // Empty-lock guard: a scan that matched no rows must NOT supersede the resident - // preview — otherwise the render locks to an empty batch with no way to recover - // (the settled selection never re-scans). Returning null falls back to resident - // filtering. A legitimately empty selection can't reach here: an empty - // `featureCodes` selection short-circuits before any scan is kicked. - if ((entry.matching.result.shape[1] ?? 0) === 0) { - return null; - } - if (entry.matching.resource) { - return entry.matching.resource; - } - const cache = { preloaded: entry.matching.result, metadataKnown: false }; - const resource = resolvePointsRenderResource(element, cache, { - experimentalOptimizations: 'off' as const, - }); - if (resource) { - entry.matching.resource = resource; - } - return resource; + hasData(key: string): boolean { + return this.resolver.hasData(key); } - /** - * Render resource for the in-flight scan's latest `partialResult`, which the - * producer builds as a GROWING buffer (every matched chunk accumulated so far), - * so points progressively fill in before the full scan settles. Cached on the - * partial's identity (rebuilds only when a new chunk grows the buffer, not per - * pan). `null` when no scan is in flight / nothing has decoded yet / empty. - */ - getMatchingPartialResource(element: PointsElement, key: string): PointsRenderResource | null { - const loading = this.entries.get(key)?.matchingLoading; - const partial = loading?.partialResult; - if (!loading || !partial || (partial.shape[1] ?? 0) === 0) { - return null; - } - if (loading.partialResource?.source === partial) { - return loading.partialResource.resource; - } - const resource = resolvePointsRenderResource( - element, - { preloaded: partial, metadataKnown: false }, - { experimentalOptimizations: 'off' as const } - ); - if (resource) { - loading.partialResource = { source: partial, resource }; - } - return resource; - } - - /** Per-row feature codes of the in-flight scan's partial buffer, row-aligned - * with {@link getMatchingPartialResource}. The render passes these as - * `preloadedFeatureCodes` so the partial overlay can filter to the *current* - * selection — otherwise a feature deselected mid-scan (whose scan is still - * running because the smaller selection is covered) keeps rendering until the - * scan settles. */ - getMatchingPartialRowFeatureCodes(key: string): ArrayLike | undefined { - return this.entries.get(key)?.matchingLoading?.partialResult?.featureCodes; + getData(key: string): PointsLoadResult | undefined { + return this.resolver.getData(key); } - /** Whether the feature-index scan for this exact selection is in flight. */ - isMatchingLoading(key: string, featureCodes: readonly number[]): boolean { - const entry = this.entries.get(key); - return entry?.matchingLoading?.signature === PointsDataEngine.matchingSignature(featureCodes); + getStatus(key: string): PointsLoadStatus { + return this.resolver.getStatus(key); } - /** Whether the resident batch is in its final state for this cap — i.e. no - * resident work is needed. False (work needed) when it must GROW (a truncated - * batch and the cap was raised past it → reload) or SHRINK (it holds more rows - * than the cap → shed the excess). So raising past a truncated batch reloads, - * lowering below what's loaded sheds, and any cap a complete batch within the - * cap already covers is a no-op. */ isLoadedWithCap(key: string, memoryCap: number): boolean { - const entry = this.entries.get(key); - if (entry?.data === undefined) { - return false; - } - return ( - PointsDataEngine.batchAdequateForCap(entry.data, memoryCap) && - (entry.data.shape[1] ?? 0) <= memoryCap - ); + return this.resolver.isLoadedWithCap(key, memoryCap); } - /** - * Truncation state of the resident preload — is it the whole dataset or only a - * capped window, and how many rows of how many. `undefined` until data loads. - * Surfaced so the user can see when raising the cap would show more points. - */ - getResidentTruncation( - key: string - ): { truncated: boolean; loaded: number; total?: number } | undefined { - const data = this.entries.get(key)?.data; - if (!data) { - return undefined; - } - return { - truncated: data.preloadTruncated === true, - loaded: data.shape[1] ?? 0, - ...(data.totalRowCount !== undefined ? { total: data.totalRowCount } : {}), - }; + getResidentTruncation(key: string) { + return this.resolver.getResidentTruncation(key); } - /** - * Truncation state of what is actually on screen. With an active selection that - * a scanned batch covers, that batch is the render — report ITS count and - * whether it hit the cap (`filtered`), not the resident preload's, so the panel - * doesn't keep saying "showing 4M" while a filtered subset is drawn. Otherwise - * falls back to the resident preload. `undefined` until something has loaded. - */ - getActiveTruncation( + getActiveTruncation(key: string, featureCodes: readonly number[] | undefined) { + return this.resolver.getActiveTruncation(key, featureCodes); + } + + getMatchingLoadState( key: string, - featureCodes: readonly number[] | undefined - ): { truncated: boolean; loaded: number; total?: number; filtered?: boolean } | undefined { - const entry = this.entries.get(key); - if (!entry) { - return undefined; - } - if (featureCodes && featureCodes.length > 0 && entry.matching) { - const covered = PointsDataEngine.coveredCodes(entry.matching.signature); - if (covered.size > 0 && featureCodes.every((code) => covered.has(code))) { - const result = entry.matching.result; - return { - truncated: result.preloadTruncated === true, - loaded: result.shape[1] ?? 0, - filtered: true, - }; - } - } - return this.getResidentTruncation(key); + featureCodes: readonly number[] + ): PointsMatchingLoadState | undefined { + return this.resolver.getMatchingLoadState(key, featureCodes); } - /** - * Idempotently preload an element's points at a given memory cap. A no-op when - * the resident data already satisfies the cap (see {@link isLoadedWithCap}) — - * so lowering the cap, or raising it when a complete batch already covers it, - * never reloads. Only RAISING the cap past a *truncated* batch reloads, and it - * keeps the previously-loaded data on screen until the larger batch settles - * (an atomic swap — no blank). The full-dataset catalog and the matched - * selection are preserved across the reload. Status via `onStatus`; notifies. - */ - ensureLoaded( - target: PointsLoadTarget, - memoryCap: number = DEFAULT_POINTS_MEMORY_CAP - ): Promise { - const { key, layerId, element } = target; - const existing = this.entries.get(key); - // (1) Existing data covers this cap without a reload (it is complete, or a - // truncated batch the lowered cap doesn't outgrow). - if ( - existing?.data !== undefined && - PointsDataEngine.batchAdequateForCap(existing.data, memoryCap) - ) { - // Cancel a now-unneeded in-flight load (e.g. the cap was raised then - // lowered back to what we already hold). - if (existing.loading) { - existing.loadAbort?.abort(); - existing.loading = undefined; - existing.loadAbort = undefined; - } - existing.memoryCap = memoryCap; - // Cap lowered below what's resident → shed the excess in memory (no - // re-fetch), so a 4M cap doesn't keep holding an 8M batch. Rebuild the - // render resource / resident-code memos from the sliced batch. - if ((existing.data.shape[1] ?? 0) > memoryCap) { - existing.data = PointsDataEngine.sliceResidentBatch(existing.data, memoryCap); - existing.resource = undefined; - existing.residentCodes = undefined; - existing.residentCodesSource = undefined; - if (existing.rowCodes && existing.rowCodes.length > memoryCap) { - existing.rowCodes = Array.prototype.slice.call(existing.rowCodes, 0, memoryCap); - } - this.notify(); - } - return Promise.resolve(); - } - // (2) A load for this exact cap is already in flight → dedup. - if (existing?.loading && existing.memoryCap === memoryCap) { - return existing.loading; - } - - const entry: PointsEntry = existing ?? { status: 'idle' }; - // Reload needed (first load, or the cap was raised past a truncated batch). - // Abort any superseded in-flight load, but KEEP the old resident data / - // resource / row codes rendered — they are swapped atomically on completion, - // so the view keeps showing what it had while the larger batch loads. - entry.loadAbort?.abort(); - entry.memoryCap = memoryCap; - const abort = new AbortController(); - entry.loadAbort = abort; - entry.status = 'loading'; - this.entries.set(key, entry); - this.callbacks.onStatus?.(layerId, 'loading'); - - const loading = (async () => { - try { - // Read the feature column with the geometry so the filter's catalog and - // per-row codes come from this one decode — no separate blocking load at - // filter time. The catalog here reflects only the *resident* batch, so it - // is an instant preview (`catalogLoaded`, not `catalogComplete`): the - // full-dataset `ensureFeatureCatalog` scan is still allowed to run and - // supersede it (a feature-ordered file's first part holds only a slice of - // the features). Row codes are complete for the resident batch. - const data = await element.loadPoints({ - includeFeatureCodes: true, - memoryCap, - signal: abort.signal, - }); - // A newer cap may have superseded this load mid-flight; if so, drop it. - if (abort.signal.aborted || entry.memoryCap !== memoryCap) { - return; - } - // Atomic swap: replace the (possibly still-rendered) old batch and drop - // the resource/resident-codes memos derived from it so they rebuild. - entry.data = data; - entry.resource = undefined; - entry.residentCodes = undefined; - entry.residentCodesSource = undefined; - entry.status = 'ready'; - entry.featureCodeColumn = data.hasFeatureCodeColumn === true; - if (data.featureCatalog !== undefined && !entry.catalogComplete) { - entry.catalog = data.featureCatalog; - entry.catalogLoaded = true; - } - if (data.featureCodes !== undefined) { - entry.rowCodes = data.featureCodes; - entry.rowCodesLoaded = true; - // The preload derived these codes against its own catalog (the resident - // preview, unless a full-dataset catalog already superseded it). Record - // that space and reconcile to whatever catalog is current now. - entry.rowCodesCatalog = data.featureCatalog; - this.reconcileRowCodes(entry); - } - this.callbacks.onStatus?.(layerId, 'ready'); - } catch (error) { - // Aborted (cap changed) or superseded → not a real error; stay quiet. - if (abort.signal.aborted || entry.memoryCap !== memoryCap) { - return; - } - entry.status = 'error'; - this.callbacks.onStatus?.(layerId, 'error'); - console.error(`Failed to load points for ${layerId}:`, error); - } finally { - // Only clear the in-flight markers if they are still ours (a superseding - // cap change installs its own `loading`/`loadAbort`). - if (entry.memoryCap === memoryCap) { - entry.loading = undefined; - entry.loadAbort = undefined; - } - this.notify(); - } - })(); - entry.loading = loading; - return loading; - } - - // --- Feature catalog (MVP step 2: feature filter) -------------------------- + isMatchingLoading(key: string, featureCodes: readonly number[]): boolean { + return this.resolver.isMatchingLoading(key, featureCodes); + } + + getLoadedMatchingFeatureCodes(key: string): ReadonlySet | undefined { + return this.resolver.getLoadedMatchingFeatureCodes(key); + } + + getMatchingRowFeatureCodes(key: string): ArrayLike | undefined { + return this.resolver.getMatchingRowFeatureCodes(key); + } + + getMatchingPartialRowFeatureCodes(key: string): ArrayLike | undefined { + return this.resolver.getMatchingPartialRowFeatureCodes(key); + } - /** - * The feature catalog for an element: `undefined` until settled, then `null` - * for an element with no `feature_key`, else the catalog. Reactive via - * `subscribe` — a settled load calls `notify()`. - */ getFeatureCatalog(key: string): PointsFeatureCatalog | null | undefined { - const entry = this.entries.get(key); - return entry?.catalogLoaded ? (entry.catalog ?? null) : undefined; + return this.resolver.getFeatureCatalog(key); } isFeatureCatalogLoading(key: string): boolean { - const entry = this.entries.get(key); - if (!entry || entry.catalogLoaded) { - return false; - } - // The catalog rides the geometry preload (includeFeatureCodes), so a running - // geometry load counts as the catalog loading too — the panel shows a - // spinner rather than a premature "load feature list" prompt. - return entry.catalogLoading !== undefined || entry.loading !== undefined; + return this.resolver.isFeatureCatalogLoading(key); } - /** - * True while the full-dataset catalog scan is still running behind an instant - * resident-subset preview. Lets the panel show a "loading the full feature - * list" hint without hiding the preview it already has. - */ isFeatureCatalogRefining(key: string): boolean { - const entry = this.entries.get(key); - return ( - entry?.catalogLoaded === true && - entry.catalogComplete !== true && - entry.catalogLoading !== undefined - ); + return this.resolver.isFeatureCatalogRefining(key); } - /** - * The distinct feature codes actually present in the resident batch (the - * preload cap means a feature-ordered file only loads a slice of its features). - * The panel greys features outside this set so selecting one that isn't loaded - * — which would render no points — is understandable rather than a glitch. - * Returns `undefined` when the row codes are not yet resident. Memoized against - * the row-codes identity so the O(rows) scan runs once per batch. - */ - /** - * True when the element has a file-backed feature code column — a real feature - * index whose codes are globally authoritative. False for dictionary-only - * feature columns (codes app-assigned, only stable within one catalog build) or - * an element with no feature codes. Undefined-safe: false until the resident - * batch has loaded. Gates the whole-dataset feature-index scan. - */ hasFeatureCodeColumn(key: string): boolean { - return this.entries.get(key)?.featureCodeColumn === true; + return this.resolver.hasFeatureCodeColumn(key); } - /** - * Whether a whole-dataset feature scan can run for this element — i.e. reach - * matching points beyond the resident preload window. True with a file-backed - * code column (footer stats skip row groups), AND for dictionary-only elements - * once a catalog is loaded: the scan resolves each row's `feature_name` against - * that catalog's code space (no row-group skipping, so it reads the whole file, - * but it retains every match up to the cap). False before any catalog loads — - * dict-only codes are only stable relative to a catalog, so there'd be nothing - * to match names against. Gates the render scan and the on-demand affordance. - */ supportsFeatureScan(key: string): boolean { - const entry = this.entries.get(key); - if (!entry) { - return false; - } - return entry.featureCodeColumn === true || (entry.catalogLoaded === true && !!entry.catalog); - } - - /** - * Re-express {@link PointsEntry.rowCodes} in the current catalog's code space - * when it was derived against an older one (resident preview → full-dataset - * upgrade). No-op for authoritative file-backed codes (identical across builds) - * and when the source/target catalogs are the same object. See - * {@link remapRowFeatureCodes} for why dictionary-only codes need this. - */ - private reconcileRowCodes(entry: PointsEntry): void { - if (entry.featureCodeColumn === true) { - return; - } - const source = entry.rowCodesCatalog; - const target = entry.catalog; - if (!entry.rowCodes || !source || !target || source === target) { - return; - } - entry.rowCodes = remapRowFeatureCodes(entry.rowCodes, source, target); - entry.rowCodesCatalog = target; - // The distinct-codes memo was keyed to the old array identity — invalidate. - entry.residentCodes = undefined; - entry.residentCodesSource = undefined; + return this.resolver.supportsFeatureScan(key); } getResidentFeatureCodes(key: string): ReadonlySet | undefined { - const entry = this.entries.get(key); - const rowCodes = entry?.rowCodes; - if (!entry || rowCodes === undefined) { - return undefined; - } - if (entry.residentCodes && entry.residentCodesSource === rowCodes) { - return entry.residentCodes; - } - const set = new Set(); - for (let i = 0; i < rowCodes.length; i += 1) { - set.add(rowCodes[i]); - } - entry.residentCodes = set; - entry.residentCodesSource = rowCodes; - return set; + return this.resolver.getResidentFeatureCodes(key); } - /** - * Idempotently build the *full-dataset* feature catalog (feature-column scan; - * worker-offloaded for oversized datasets). Uses `listFeaturesWithCounts` so the - * panel can show/sort by per-feature counts. Runs even when a resident-subset - * preview is already showing (`catalogLoaded` but not `catalogComplete`) and - * supersedes it; no-op once the full scan has settled (`catalogComplete`) or is - * in flight. - */ - ensureFeatureCatalog(target: PointsLoadTarget): Promise { - const { key, element } = target; - const entry = this.entries.get(key) ?? { status: 'idle' as PointsLoadStatus }; - this.entries.set(key, entry); - if (entry.catalogComplete) { - return Promise.resolve(); - } - if (entry.catalogLoading) { - return entry.catalogLoading; - } - - const loading = (async () => { - try { - const fullCatalog = await element.listFeaturesWithCounts(); - entry.catalog = fullCatalog; - // The full-dataset catalog is authoritative. Re-express any resident row - // codes (derived against the resident-preview catalog) in its code space - // so the render's per-row codes match the panel's selection + swatches. - this.reconcileRowCodes(entry); - } catch (error) { - // Keep any resident preview catalog on failure rather than blanking it. - if (!entry.catalogLoaded) entry.catalog = null; - console.error(`Failed to build points feature catalog for ${target.layerId}:`, error); - } finally { - entry.catalogLoaded = true; - entry.catalogComplete = true; - entry.catalogLoading = undefined; - this.notify(); - } - })(); - entry.catalogLoading = loading; - this.notify(); // surface the loading transition to the panel - return loading; - } - - // --- Row feature codes (the filter mask, aligned to the resident batch) ----- - - /** Per-row feature codes aligned to the resident batch, or `undefined` if the - * element exposes none / they are not yet loaded. See the class-doc alignment - * invariant. */ getRowFeatureCodes(key: string): ArrayLike | undefined { - return this.entries.get(key)?.rowCodes; + return this.resolver.getRowFeatureCodes(key); } - /** True once row codes have settled (even if the element has none). */ hasRowFeatureCodes(key: string): boolean { - return this.entries.get(key)?.rowCodesLoaded === true; + return this.resolver.hasRowFeatureCodes(key); } - /** - * Idempotently load the row feature codes for the resident batch. Reuses the - * engine's catalog for name→code mapping when it is already built (else the - * core loader scans it internally). No-op once settled or in flight. - */ - ensureRowFeatureCodes(target: PointsLoadTarget): Promise { - const { key, element } = target; - const entry = this.entries.get(key) ?? { status: 'idle' as PointsLoadStatus }; - this.entries.set(key, entry); - if (entry.rowCodesLoaded) { - return Promise.resolve(); - } - if (entry.rowCodesLoading) { - return entry.rowCodesLoading; - } - - const loading = (async () => { - try { - const catalog = this.getFeatureCatalog(key); - entry.rowCodes = await element.loadRowFeatureCodes({ featureCatalog: catalog }); - // These codes were derived against `catalog`; record that space so a later - // catalog upgrade reconciles them (see reconcileRowCodes). - entry.rowCodesCatalog = catalog ?? undefined; - this.reconcileRowCodes(entry); - } catch (error) { - entry.rowCodes = undefined; - console.error(`Failed to load points row feature codes for ${target.layerId}:`, error); - } finally { - entry.rowCodesLoaded = true; - entry.rowCodesLoading = undefined; - this.notify(); - } - })(); - entry.rowCodesLoading = loading; - return loading; - } - - /** Drop an element from the cache (on unload / dataset switch). Catalog and row - * codes live in the same entry, so they are evicted together. */ + // --- Lifecycle -------------------------------------------------------------- + + /** Drop an element from both halves — the data AND the resources built from it. */ evict(key: string): void { - this.entries.delete(key); + this.resolver.evict(key); + this.adapter.evict(key); } - /** Release all cached data and listeners. */ dispose(): void { - this.entries.clear(); - this.listeners.clear(); + this.resolver.dispose(); + this.adapter.dispose(); } } From 3eeb1a6ea273d8b68b918ed7c6b0768f76a2a6a8 Mon Sep 17 00:00:00 2001 From: Peter Todd Date: Tue, 14 Jul 2026 19:21:00 +0100 Subject: [PATCH 7/9] ShapesResolver in core MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three resources — geometry, tooltip, fillColor — behind the same interface PointsResolver implements. Unconsumed: useLayerData still runs the old path. All three loaders already lived in core (element.loadRenderData, loadShapesTooltipMetadata, loadAssociatedTableFeatureRows). Only the orchestration was stranded in a React hook. This also absorbs loadShapesData from vis's shapesRenderer — a try/catch that re-threw with a nicer message — and replaces the re-throw with a SpatialEntryError, which is what the classifier exists for. The seam nominates `decode-failed` for geometry and `load-failed` for the table reads, because the seam knows what it was doing and the bare `throw new Error(string)` does not. Failure is PER-RESOURCE, and this is the resolver where that stops being an abstract claim: a shapes entry whose tooltip column is missing must still draw its geometry. An entry-wide Result would blank a layer whose geometry was perfectly fine. There is a test for exactly that. blockingResources = ['geometry']. Tooltip and fill colour have never blocked a first paint — which is why isBlocking already treats shapes differently from points today. That asymmetry was a kind-switch; here it is data. The fill COLOUR is deliberately not here. load() fetches the table rows; buildShapeFillColorByFeatureId stays in layers. That is not a dependency dodge, it is the phase separation doing its job: fetching rows is I/O, and mapping a column through a palette is a pure projection *for a renderer* — it belongs in project(). Same for buildShapesPrebuiltData. So the fillColor resource is the ROWS, not the colours. ResolveContext gains `transform`. World bounds are meaningless in element space, and ADR 0004 §1 gives the resolver entry resolution and bounds — so the element→coordinate-system matrix is the resolver's input, not something a renderer hands down. Bounds are memoised on the render data's identity. Preserved deliberately: the tooltip ping-pong. Tooltip metadata is cached per element but requested per layer config, so two layers over one element with different tooltipFields invalidate each other forever. (shapePrebuiltData and shapeFillColorData were keyed by layer id to avoid exactly this; the tooltip cache was missed.) Step 1 is a re-housing — documented in place, left to Track B, which owns it on the punchlist. Labels have the identical shape. **Track B unblocks here**, not at the end of Step 1: the shapes loader seam and the batch representation can start against this resolver now. 521 tests. Build and biome green. Co-Authored-By: Claude Opus 4.8 --- packages/core/src/engine/ShapesResolver.ts | 331 +++++++++++++++++++++ packages/core/src/engine/index.ts | 3 +- packages/core/src/engine/resolver.ts | 13 +- packages/core/tests/pointsResolver.spec.ts | 2 + packages/core/tests/shapesResolver.spec.ts | 258 ++++++++++++++++ 5 files changed, 602 insertions(+), 5 deletions(-) create mode 100644 packages/core/src/engine/ShapesResolver.ts create mode 100644 packages/core/tests/shapesResolver.spec.ts diff --git a/packages/core/src/engine/ShapesResolver.ts b/packages/core/src/engine/ShapesResolver.ts new file mode 100644 index 00000000..81ebad9c --- /dev/null +++ b/packages/core/src/engine/ShapesResolver.ts @@ -0,0 +1,331 @@ +import type { ShapesElement } from '../models/index.js'; +import type { ShapesRenderData } from '../shapes.js'; +import { + type AxisAlignedBounds, + boundsFromCircles, + boundsFromPolygons, +} from '../spatialViewFit.js'; +import type { SpatialData } from '../store/index.js'; +import { + type AssociatedTableFeatureRows, + loadAssociatedTableFeatureRows, +} from '../tableAssociations.js'; +import { loadShapesTooltipMetadata, type ShapesTooltipMetadata } from '../tooltip.js'; +import type { EntryNotice } from './errors.js'; +import { isCancellation, toSpatialEntryError } from './errors.js'; +import { Resolution } from './resolution.js'; +import type { EntryResources, ResolveContext, ResolveTask, ResourceResolver } from './resolver.js'; + +/** + * The shapes Resource Resolver. + * + * Three resources, and — this is the point of `Resolution` being **per-resource** + * rather than per-entry — they fail independently. A shapes entry whose tooltip + * column is broken must still draw its geometry. + * + * - `geometry` — `element.loadRenderData()`. The only one that blocks first paint. + * - `tooltip` — `loadShapesTooltipMetadata()`. Keyed by the tooltip-fields signature. + * - `fillColor` — `loadAssociatedTableFeatureRows()`. Keyed by the column name. + * + * All three loaders already lived in `core`; only the orchestration was stranded in + * a React hook. This absorbs `loadShapesData` from `vis`'s `shapesRenderer` — a + * try/catch that re-threw with a nicer message — and replaces the re-throw with a + * `SpatialEntryError`, which is what the classifier is for. + * + * ## What is deliberately NOT here + * + * The **fill colour itself**. `load()` fetches the table rows; turning rows into + * RGBA is `buildShapeFillColorByFeatureId` in `layers`, and it stays there. That + * is not a dependency dodge — it is the phase separation working: fetching rows is + * I/O, and mapping a column through a palette is a pure projection *for a + * renderer*. It belongs in `project()`. Same for `buildShapesPrebuiltData`. + * + * So this resolver's `fillColor` resource is the **rows**, not the colours. + * + * ## Known bug, preserved + * + * Tooltip metadata is cached per **element**, but requested per **layer config**. + * Two layers over one element with different `tooltipFields` therefore invalidate + * each other forever — a ping-pong. (`shapePrebuiltData` and `shapeFillColorData` + * were deliberately keyed by layer id to avoid exactly this; the tooltip cache was + * missed.) Step 1 is a re-housing, so the behaviour is preserved as-is. **Track B + * owns the fix**, and it is on the punchlist. Labels have the identical shape. + */ + +export interface ShapesResolveConfig { + tooltipFields?: string[]; + fillColorByColumn?: { columnName: string; mode: string }; +} + +export interface ShapesResolverCallbacks { + onStatus?: (layerId: string, resource: string, status: 'loading' | 'ready' | 'error') => void; +} + +interface ShapesEntry { + geometry: Resolution; + tooltip: Resolution; + fillColor: Resolution; + /** In-flight promises, keyed by task id — today's dedup, kept byte for byte. */ + inFlight: Map>; + /** The tooltip-fields signature `tooltip` was loaded for. */ + tooltipSignature?: string; + /** The column name `fillColor` was loaded for. */ + fillColorColumn?: string; + bounds?: AxisAlignedBounds | null; + boundsSource?: ShapesRenderData; + snapshot?: { version: number; value: EntryResources }; +} + +const tooltipSignature = (fields: string[] | undefined): string => (fields ?? []).join(''); + +export class ShapesResolver implements ResourceResolver { + readonly kind = 'shapes' as const; + /** + * Geometry only. Tooltip and fill colour have never blocked a first paint and + * must not start — which is precisely why `isBlocking` already treats shapes + * differently from points today. That asymmetry was a kind-switch; here it is data. + */ + readonly blockingResources = ['geometry'] as const; + + private readonly entries = new Map(); + private readonly listeners = new Set<() => void>(); + private readonly callbacks: ShapesResolverCallbacks; + private readonly spatialData: SpatialData | undefined; + private version = 0; + + constructor(options: { spatialData?: SpatialData; callbacks?: ShapesResolverCallbacks } = {}) { + this.spatialData = options.spatialData; + this.callbacks = options.callbacks ?? {}; + } + + private entry(key: string): ShapesEntry { + let entry = this.entries.get(key); + if (!entry) { + entry = { + geometry: Resolution.idle(), + tooltip: Resolution.idle(), + fillColor: Resolution.idle(), + inFlight: new Map(), + }; + this.entries.set(key, entry); + } + return entry; + } + + // --- ResourceResolver ------------------------------------------------------- + + /** PURE, SYNC. Starts nothing. */ + plan(ctx: ResolveContext): readonly ResolveTask[] { + const key = ctx.elementKey; + const entry = this.entries.get(key); + const tasks: ResolveTask[] = []; + + if (!entry || Resolution.isIdle(entry.geometry)) { + tasks.push({ id: `${key}#geometry`, resource: 'geometry' }); + } + + const fields = ctx.config.tooltipFields; + if (fields && fields.length > 0) { + const signature = tooltipSignature(fields); + if (!entry || entry.tooltipSignature !== signature) { + // The signature is IN the id: changing the tooltip columns supersedes. + tasks.push({ + id: `${key}#tooltip:${signature}`, + resource: 'tooltip', + payload: { tooltipFields: fields }, + }); + } + } + + const column = ctx.config.fillColorByColumn?.columnName; + if (column) { + if (!entry || entry.fillColorColumn !== column) { + tasks.push({ + id: `${key}#fillColor:${column}`, + resource: 'fillColor', + payload: { column }, + }); + } + } + + return tasks; + } + + /** ASYNC. The only place I/O starts. Never throws — failures become Resolutions. */ + async load( + task: ResolveTask, + ctx: ResolveContext, + signal: AbortSignal + ): Promise { + const entry = this.entry(ctx.elementKey); + // Same id ⇒ same request ⇒ dedup. (Today's in-flight-promise check, kept.) + const existing = entry.inFlight.get(task.id); + if (existing) return existing; + + const run = this.run(task, ctx, entry, signal).finally(() => { + if (entry.inFlight.get(task.id) === run) entry.inFlight.delete(task.id); + }); + entry.inFlight.set(task.id, run); + return run; + } + + private async run( + task: ResolveTask, + ctx: ResolveContext, + entry: ShapesEntry, + _signal: AbortSignal + ): Promise { + const key = ctx.elementKey; + const slot = task.resource as 'geometry' | 'tooltip' | 'fillColor'; + if (slot !== 'geometry' && slot !== 'tooltip' && slot !== 'fillColor') return; + + // Retain the last good value across the refine, so a reload keeps drawing. + const stale = Resolution.lastGood(entry[slot] as Resolution); + entry[slot] = Resolution.loading(stale !== undefined ? { stale } : {}) as never; + this.callbacks.onStatus?.(ctx.entryId, slot, 'loading'); + this.notify(); + + try { + switch (slot) { + case 'geometry': { + const renderData = await ctx.element.loadRenderData(); + entry.geometry = Resolution.ready(renderData); + break; + } + case 'tooltip': { + const fields = (task.payload as { tooltipFields: string[] }).tooltipFields; + const metadata = await loadShapesTooltipMetadata(this.spatialData, ctx.element, fields); + entry.tooltip = Resolution.ready(metadata); + entry.tooltipSignature = tooltipSignature(fields); + break; + } + case 'fillColor': { + const column = (task.payload as { column: string }).column; + const rows = await loadAssociatedTableFeatureRows({ + spatialData: this.spatialData, + kind: 'shapes', + key, + extraColumnNames: [column], + }); + entry.fillColor = Resolution.ready(rows); + entry.fillColorColumn = column; + break; + } + } + this.callbacks.onStatus?.(ctx.entryId, slot, 'ready'); + } catch (cause) { + // Cancellation is a non-event: an aborted load is not a domain failure, and + // painting an error for one would be a visible regression. + if (isCancellation(cause)) return; + const error = toSpatialEntryError(cause, { + elementKey: key, + kind: 'shapes', + resource: slot, + // The SEAM knows what it was doing. A geometry decode that throws is a + // decode failure; a tooltip/table read that throws is a load failure. + fallback: slot === 'geometry' ? 'decode-failed' : 'load-failed', + }); + entry[slot] = Resolution.failed(error, stale) as never; + this.callbacks.onStatus?.(ctx.entryId, slot, 'error'); + } finally { + this.notify(); + } + } + + /** PURE, SYNC. Identity-stable between mutations. */ + snapshot(ctx: ResolveContext): EntryResources { + const key = ctx.elementKey; + const entry = this.entries.get(key); + if (entry?.snapshot && entry.snapshot.version === this.version) { + return entry.snapshot.value; + } + + const value: EntryResources = { + entryId: ctx.entryId, + elementKey: key, + resources: { + geometry: entry?.geometry ?? Resolution.idle(), + tooltip: entry?.tooltip ?? Resolution.idle(), + fillColor: entry?.fillColor ?? Resolution.idle(), + }, + notices: [] as readonly EntryNotice[], + bounds: this.bounds(ctx), + revision: this.version, + }; + + if (entry) entry.snapshot = { version: this.version, value }; + return value; + } + + /** + * World bounds from the geometry, memoised on the render data's identity. + * + * Bounds are world-space, so they need the element→coordinate-system transform — + * which is why `transform` is on `ResolveContext` and not something a renderer + * hands down. The resolver owns bounds (ADR 0004 §1). + */ + private bounds( + ctx: ResolveContext + ): AxisAlignedBounds | null { + const entry = this.entries.get(ctx.elementKey); + if (!entry) return null; + const data = Resolution.lastGood(entry.geometry); + if (!data) return null; + if (entry.boundsSource === data) return entry.bounds ?? null; + + const computed = data.circles + ? boundsFromCircles(data.circles, ctx.transform) + : data.polygons?.length + ? boundsFromPolygons(data.polygons, ctx.transform) + : null; + entry.bounds = computed; + entry.boundsSource = data; + return computed; + } + + // --- Reads ------------------------------------------------------------------ + + /** The geometry, for a Renderer Adapter to project. */ + getRenderData(key: string): ShapesRenderData | undefined { + const entry = this.entries.get(key); + return entry ? Resolution.lastGood(entry.geometry) : undefined; + } + + getTooltipMetadata(key: string): ShapesTooltipMetadata | undefined { + const entry = this.entries.get(key); + return entry ? Resolution.lastGood(entry.tooltip) : undefined; + } + + /** The raw table rows. The COLOURS are built by the adapter — see the class doc. */ + getFillColorRows(key: string): AssociatedTableFeatureRows | undefined { + const entry = this.entries.get(key); + return entry ? Resolution.lastGood(entry.fillColor) : undefined; + } + + // --- Subscription ----------------------------------------------------------- + + subscribe(listener: () => void): () => void { + this.listeners.add(listener); + return () => { + this.listeners.delete(listener); + }; + } + + getVersion(): number { + return this.version; + } + + private notify(): void { + this.version += 1; + for (const listener of this.listeners) listener(); + } + + evict(key: string): void { + this.entries.delete(key); + } + + dispose(): void { + this.entries.clear(); + this.listeners.clear(); + } +} diff --git a/packages/core/src/engine/index.ts b/packages/core/src/engine/index.ts index 7a83000f..32efdc3e 100644 --- a/packages/core/src/engine/index.ts +++ b/packages/core/src/engine/index.ts @@ -27,9 +27,10 @@ export { fromResult, Resolution } from './resolution.js'; export type { EntryResources, ResolveContext, - ResolvedBounds, ResolveTask, ResourceResolver, } from './resolver.js'; +export type { ShapesResolveConfig, ShapesResolverCallbacks } from './ShapesResolver.js'; +export { ShapesResolver } from './ShapesResolver.js'; export type { AnyResolveContext, ResolverRegistry } from './SpatialEntryStore.js'; export { SpatialEntryStore } from './SpatialEntryStore.js'; diff --git a/packages/core/src/engine/resolver.ts b/packages/core/src/engine/resolver.ts index 52e3ca7c..d0270a4f 100644 --- a/packages/core/src/engine/resolver.ts +++ b/packages/core/src/engine/resolver.ts @@ -41,12 +41,11 @@ * images. */ +import type { Matrix4 } from '@math.gl/core'; +import type { AxisAlignedBounds } from '../spatialViewFit.js'; import type { EntryNotice, SpatialEntryKind } from './errors.js'; import type { Resolution } from './resolution.js'; -/** World-space axis-aligned bounds: `[minX, minY, maxX, maxY]`. */ -export type ResolvedBounds = [number, number, number, number]; - /** * Everything a resolver needs to reason about one Spatial Entry, this commit. * @@ -63,6 +62,12 @@ export interface ResolveContext { readonly element: TElement; /** Serialisable renderer props. Never a Runtime Attachment — no callbacks. */ readonly config: TConfig; + /** + * Element → active coordinate system. The resolver owns entry resolution and + * world bounds (ADR 0004 §1), and bounds are meaningless in element space, so + * the transform has to be here rather than being a renderer's business. + */ + readonly transform: Matrix4; } /** @@ -109,7 +114,7 @@ export interface EntryResources { readonly notices: readonly EntryNotice[]; /** World bounds, once anything is loaded. Computed by the resolver — a * `vis`-resident one may reach for viv's `getImageSize`; `core` never needs to. */ - readonly bounds: ResolvedBounds | null; + readonly bounds: AxisAlignedBounds | null; /** Bumps iff anything above changed identity. Lets an adapter skip `project()`. */ readonly revision: number; } diff --git a/packages/core/tests/pointsResolver.spec.ts b/packages/core/tests/pointsResolver.spec.ts index 0be5a47d..c07621de 100644 --- a/packages/core/tests/pointsResolver.spec.ts +++ b/packages/core/tests/pointsResolver.spec.ts @@ -1,3 +1,4 @@ +import { Matrix4 } from '@math.gl/core'; import { describe, expect, it, vi } from 'vitest'; import { type PointsResolveConfig, @@ -56,6 +57,7 @@ const ctx = ( kind: 'points', element: el, config, + transform: new Matrix4(), }); describe('plan() — pure, synchronous, starts nothing', () => { diff --git a/packages/core/tests/shapesResolver.spec.ts b/packages/core/tests/shapesResolver.spec.ts new file mode 100644 index 00000000..cc83e405 --- /dev/null +++ b/packages/core/tests/shapesResolver.spec.ts @@ -0,0 +1,258 @@ +import { Matrix4 } from '@math.gl/core'; +import { describe, expect, it, vi } from 'vitest'; +import { + Resolution, + type ResolveContext, + type ShapesResolveConfig, + ShapesResolver, +} from '../src/engine/index.js'; +import type { ShapesElement } from '../src/models/index.js'; +import type { ShapesRenderData } from '../src/shapes.js'; + +/** + * The shapes Resource Resolver, headless — no deck layer, no GL context. + * + * The claim under test that matters most is **per-resource failure**: a shapes + * entry whose tooltip column is broken must still draw its geometry. That is the + * whole reason `Resolution` is per-resource and there is no entry-wide `Result`, + * and it is the thing an entry-wide error channel would quietly get wrong. + */ + +const renderData = (over: Partial = {}): ShapesRenderData => ({ + kind: 'js-polygons', + geometryKind: 'circle', + elementKey: 'cells', + featureIds: ['c1', 'c2'], + circles: { + positions: [new Float32Array([0, 10]), new Float32Array([0, 10])], + radii: new Float32Array([1, 1]), + }, + rowIndexByFeatureIndex: new Int32Array([0, 1]), + ...over, +}); + +function element(over: Record = {}) { + return { + key: 'cells', + loadRenderData: vi.fn(async () => renderData()), + loadFeatureIds: vi.fn(async () => ['c1', 'c2']), + ...over, + } as unknown as ShapesElement; +} + +const ctx = ( + el: ShapesElement, + config: ShapesResolveConfig = {} +): ResolveContext => ({ + entryId: 'layer-s', + elementKey: 'cells', + kind: 'shapes', + element: el, + config, + transform: new Matrix4(), +}); + +const signal = () => new AbortController().signal; + +describe('plan() — pure, sync, starts nothing', () => { + it('does not touch the element', () => { + const el = element(); + + new ShapesResolver().plan(ctx(el, { tooltipFields: ['cell_type'] })); + + expect(el.loadRenderData).not.toHaveBeenCalled(); + }); + + it('plans geometry for a fresh entry, and nothing else by default', () => { + const tasks = new ShapesResolver().plan(ctx(element())); + + expect(tasks.map((t) => t.resource)).toEqual(['geometry']); + }); + + it('plans a tooltip only when tooltip fields are configured', () => { + const resolver = new ShapesResolver(); + const resources = (config: ShapesResolveConfig) => + resolver.plan(ctx(element(), config)).map((t) => t.resource); + + expect(resources({})).not.toContain('tooltip'); + expect(resources({ tooltipFields: [] })).not.toContain('tooltip'); + expect(resources({ tooltipFields: ['cell_type'] })).toContain('tooltip'); + }); + + it('puts the tooltip-fields signature IN the id, so changing columns supersedes', () => { + const resolver = new ShapesResolver(); + + const a = resolver.plan(ctx(element(), { tooltipFields: ['cell_type'] }))[1]; + const b = resolver.plan(ctx(element(), { tooltipFields: ['cluster'] }))[1]; + + expect(a?.id).not.toBe(b?.id); + }); + + it('plans a fill-colour load only when a column is configured', () => { + const resolver = new ShapesResolver(); + + expect(resolver.plan(ctx(element())).map((t) => t.resource)).not.toContain('fillColor'); + expect( + resolver + .plan(ctx(element(), { fillColorByColumn: { columnName: 'area', mode: 'numeric' } })) + .map((t) => t.resource) + ).toContain('fillColor'); + }); + + it('stops planning geometry once it is loaded', async () => { + const resolver = new ShapesResolver(); + const el = element(); + + await resolver.load({ id: 'g', resource: 'geometry' }, ctx(el), signal()); + + expect(resolver.plan(ctx(el)).map((t) => t.resource)).toEqual([]); + }); +}); + +describe('load() — the only place I/O starts', () => { + it('loads geometry and reports it ready, by reference', async () => { + const resolver = new ShapesResolver(); + const el = element(); + + await resolver.load({ id: 'g', resource: 'geometry' }, ctx(el), signal()); + + const snapshot = resolver.snapshot(ctx(el)); + expect(Resolution.isReady(snapshot.resources.geometry as never)).toBe(true); + expect(Resolution.readyValue(snapshot.resources.geometry as never)).toBe( + resolver.getRenderData('cells') + ); + }); + + it('dedups a concurrent request for the same task id', async () => { + const resolver = new ShapesResolver(); + const el = element(); + const task = { id: 'g', resource: 'geometry' }; + + await Promise.all([ + resolver.load(task, ctx(el), signal()), + resolver.load(task, ctx(el), signal()), + ]); + + expect(el.loadRenderData).toHaveBeenCalledTimes(1); + }); + + it('does not throw on failure — it turns the throw into a value', async () => { + const resolver = new ShapesResolver(); + const el = element({ + loadRenderData: vi.fn(async () => { + throw new Error('parquet is corrupt'); + }), + }); + + await expect( + resolver.load({ id: 'g', resource: 'geometry' }, ctx(el), signal()) + ).resolves.toBeUndefined(); + + const geometry = resolver.snapshot(ctx(el)).resources.geometry as never; + expect(Resolution.isFailed(geometry)).toBe(true); + }); + + it('classifies a geometry failure from the SEAM — a decode, not a generic load', async () => { + // The throw is a bare Error with no type. `decode-failed` comes from the + // context's `fallback`, because the seam knows what it was doing. + const resolver = new ShapesResolver(); + const el = element({ + loadRenderData: vi.fn(async () => { + throw new Error('something opaque from a codec'); + }), + }); + + await resolver.load({ id: 'g', resource: 'geometry' }, ctx(el), signal()); + + const geometry = resolver.snapshot(ctx(el)).resources.geometry; + if (geometry?.status !== 'failed') throw new Error('narrowing'); + expect(geometry.error.kind).toBe('decode-failed'); + expect(geometry.error.retryable).toBe(true); + }); +}); + +describe('failure is PER-RESOURCE', () => { + // The load-bearing claim. An entry-wide Result would get this wrong, and the + // symptom would be a blank layer where the geometry was perfectly fine. + it('a broken tooltip does NOT stop the geometry drawing', async () => { + const resolver = new ShapesResolver(); + const el = element({ + loadFeatureIds: vi.fn(async () => { + throw new Error('no such column: cell_type'); + }), + }); + const c = ctx(el, { tooltipFields: ['cell_type'] }); + + await resolver.load({ id: 'g', resource: 'geometry' }, c, signal()); + await resolver.load( + { id: 't', resource: 'tooltip', payload: { tooltipFields: ['cell_type'] } }, + c, + signal() + ); + + const snapshot = resolver.snapshot(c); + expect(Resolution.isFailed(snapshot.resources.tooltip as never)).toBe(true); + // ...and the geometry is untouched and still drawable. + expect(Resolution.isReady(snapshot.resources.geometry as never)).toBe(true); + expect(resolver.getRenderData('cells')).toBeDefined(); + }); + + it('only geometry blocks a first paint — tooltip and fill colour never have', () => { + // Today this asymmetry is a kind-switch inside isBlocking. Here it is data. + expect(new ShapesResolver().blockingResources).toEqual(['geometry']); + }); +}); + +describe('snapshot() — identity and bounds', () => { + it('returns the SAME object until something mutates', async () => { + const resolver = new ShapesResolver(); + const el = element(); + await resolver.load({ id: 'g', resource: 'geometry' }, ctx(el), signal()); + + const first = resolver.snapshot(ctx(el)); + + for (let i = 0; i < 10; i++) { + expect(resolver.snapshot(ctx(el))).toBe(first); + } + }); + + it('computes world bounds from the geometry once loaded', async () => { + const resolver = new ShapesResolver(); + const el = element(); + + expect(resolver.snapshot(ctx(el)).bounds).toBeNull(); + + await resolver.load({ id: 'g', resource: 'geometry' }, ctx(el), signal()); + + // Circles at (0,0) and (10,10), radius 1 → [-1, -1] .. [11, 11]. + expect(resolver.snapshot(ctx(el)).bounds).toMatchObject({ + minX: -1, + minY: -1, + maxX: 11, + maxY: 11, + }); + }); + + it('retains the last good geometry across a failed reload — stale keeps drawing', async () => { + const resolver = new ShapesResolver(); + let call = 0; + const el = element({ + loadRenderData: vi.fn(async () => { + call += 1; + if (call === 1) return renderData(); + throw new Error('the second read failed'); + }), + }); + const c = ctx(el); + + await resolver.load({ id: 'g1', resource: 'geometry' }, c, signal()); + const good = resolver.getRenderData('cells'); + + await resolver.load({ id: 'g2', resource: 'geometry' }, c, signal()); + + const geometry = resolver.snapshot(c).resources.geometry as never; + expect(Resolution.isFailed(geometry)).toBe(true); + // "stale is a retention": the failed refine keeps drawing rather than blanking. + expect(Resolution.lastGood(geometry)).toBe(good); + }); +}); From 1adaa3a0cbf5b30cb088f1eb1bac67ebbf075429 Mon Sep 17 00:00:00 2001 From: Peter Todd Date: Wed, 15 Jul 2026 10:18:38 +0100 Subject: [PATCH 8/9] =?UTF-8?q?ImagesResolver=20and=20LabelsResolver=20in?= =?UTF-8?q?=20vis=20=E2=80=94=20same=20interface,=20no=20port?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The images and labels resolvers implement core's ResourceResolver — the same interface PointsResolver and ShapesResolver implement — but they live in @spatialdata/vis, next to the Viv and avivatorish dependencies they need. Unconsumed: useLayerData still runs the old path. This is the payoff of the ADR 0004 §6 amendment. There is no RasterSourcePort in core, because there was never a closure to break: createImageLoader already takes fetchMultiscales as an injected parameter, and the React VivLoaderRegistry context is merely the DI container at the call site. The resolver takes the fetcher as a constructor arg; useLayerData will feed it useVivLoaderRegistry()'s value in C8 — exactly the DI that exists today, hoisted out of the load body. The ~180-line channel-defaults ladder moves too, but only in PHASE, not package: it goes from inline in useLayerData's kind-switch to buildImageChannelDefaults / buildLabelsChannelDefaults in vis's existing imageLoaderChannelDefaults.ts, lifted verbatim. The nested try/catch is preserved deliberately — computing contrast stats reads pixels, which can fail on a store that served its metadata fine, and a channel-defaults failure must degrade to a fallback rather than fail the image. That is exactly the healthy-data-with-a-caveat case EntryNotice exists for. World bounds are computed right here with getImageSize. That single line is what killed the port: core owns bounds (ADR 0004 §1) but may not import Viv, so a core-resident images resolver would have needed extents handed across a port. A vis-resident one just asks Viv, and core never learns rasters have a width. The test that matters most asserts the store CANNOT TELL these live in a different package: it registers them through the same SpatialEntryStore registry, typed only as ResourceResolver, and drives them through the same plan/load/snapshot loop. If the store could distinguish a vis-resident resolver from a core-resident one, "images is special" would become representable — and that is how one interface quietly becomes two. avivatorish is untouched. No image-state-model decision is forced; that question stays open and separately decidable, which is where it belongs. 542 tests. Build, biome and the core dependency boundaries all green. Co-Authored-By: Claude Opus 4.8 --- .../imageLoaderChannelDefaults.ts | 205 +++++++++- .../resolvers/RasterResolvers.ts | 382 ++++++++++++++++++ packages/vis/tests/rasterResolvers.spec.ts | 255 ++++++++++++ 3 files changed, 839 insertions(+), 3 deletions(-) create mode 100644 packages/vis/src/SpatialCanvas/resolvers/RasterResolvers.ts create mode 100644 packages/vis/tests/rasterResolvers.spec.ts diff --git a/packages/vis/src/SpatialCanvas/imageLoaderChannelDefaults.ts b/packages/vis/src/SpatialCanvas/imageLoaderChannelDefaults.ts index 12be5d4d..895355c0 100644 --- a/packages/vis/src/SpatialCanvas/imageLoaderChannelDefaults.ts +++ b/packages/vis/src/SpatialCanvas/imageLoaderChannelDefaults.ts @@ -1,9 +1,30 @@ /** - * Defaults for Viv image loaders when Omero channel metadata is missing or when - * channel setup fails partway through. Kept separate from useLayerData so the hook stays focused. + * Channel defaults for Viv image and labels loaders. + * + * The two `build*ChannelDefaults` functions at the bottom were **inline in + * `useLayerData`'s load effect** — ~180 lines braided into the middle of a + * kind-switch. They are lifted here **verbatim**, not rewritten: they now run in + * `ImagesResolver.load()` / `LabelsResolver.load()` instead of inside a React hook. + * + * They stay in `@spatialdata/vis` and are *not* moved to `core`. `avivatorish` + * imports React **and** Viv, and it is a de-vendoring holding pen for code that + * also lives upstream in Viv and in MDV — its own README calls the serialized image + * state model "still evolving". Shaping `core` around it, or inventing a port to + * hide it behind, would freeze a guess about an unsettled model into the package + * `tgpu-htj2k` depends on. See ADR 0004's amendment. */ -import { COLOR_PALLETE, getVivSelectionAxisSizes } from '@spatialdata/avivatorish'; +import { + buildDefaultSelection, + COLOR_PALLETE, + clampVivSelectionsToAxes, + getMultiSelectionStats, + getVivSelectionAxisSizes, + guessRgb, + isInterleaved, + tryParseOmeroHexColor, +} from '@spatialdata/avivatorish'; +import type { ImageElement, LabelsElement } from '@spatialdata/core'; /** Loader fields used for channel count and contrast range heuristics. */ export type VivLoaderMetadata = { @@ -85,3 +106,181 @@ export function applyPerChannelFallbackWithoutOmero( imageData.channelsVisible = Array(channelCount).fill(true); imageData.selections = selections; } + +/** Shape of the resolved image data. Mirrors `ImageLoaderData` in `useLayerData`. */ +export interface ImageChannelDefaults extends ImageLoaderChannelTarget { + loader: unknown; + channelNames?: string[]; +} + +/** Shape of the resolved labels data. Mirrors `LabelsLoaderData` in `useLayerData`. */ +export interface LabelsChannelDefaults extends ImageLoaderChannelTarget { + loader: unknown; + channelOpacities?: number[]; + channelOutlineOpacities?: number[]; + channelsFilled?: boolean[]; + channelStrokeWidths?: number[]; +} + +const hasVivMetadata = (loader: unknown): loader is VivLoaderMetadata => + !!loader && typeof loader === 'object' && 'labels' in loader && 'shape' in loader; + +/** The last-resort defaults, used when the loader tells us nothing at all. */ +const applyBlindDefaults = (target: ImageLoaderChannelTarget): void => { + target.contrastLimits = [[0, 65535]]; + target.colors = [[255, 255, 255]]; + target.channelsVisible = [true]; + target.selections = [{}]; +}; + +/** + * Channel defaults for an image: Omero metadata when present, RGB heuristics when + * it looks like RGB, computed contrast stats otherwise, per-channel fallback when + * Omero has no channel list, and blind defaults when the loader exposes no + * labels/shape at all. + * + * Lifted verbatim from `useLayerData`'s image branch. The nested try/catch is + * deliberate and preserved: computing stats reads pixels, which can fail on a + * store that served its metadata perfectly well — and a channel-defaults failure + * must degrade to a fallback, not fail the image. + */ +export async function buildImageChannelDefaults( + loader: unknown, + element: ImageElement, + onNotice?: (reason: string) => void +): Promise { + const loaderToCheck = Array.isArray(loader) ? loader[0] : loader; + const imageData: ImageChannelDefaults = { loader }; + + try { + if (hasVivMetadata(loaderToCheck)) { + const loaderObj = loaderToCheck; + imageData.selectionAxisSizes = getVivSelectionAxisSizes(loaderObj.labels, loaderObj.shape); + + const selections = buildDefaultSelection({ + labels: loaderObj.labels, + shape: loaderObj.shape, + }); + const metadata = element.attrs.omero; + + if (metadata?.channels) { + const Channels = metadata.channels; + imageData.channelNames = Channels.map( + (c: { label?: string }, i: number) => c.label ?? `Channel ${i + 1}` + ); + const isRgb = guessRgb({ + Pixels: { Channels: Channels.map((c: { label?: string }) => ({ Name: c.label })) }, + }); + + if (isRgb) { + if (isInterleaved(loaderObj.shape)) { + imageData.contrastLimits = [[0, 255]]; + imageData.colors = [[255, 0, 0]]; + } else { + imageData.contrastLimits = [ + [0, 255], + [0, 255], + [0, 255], + ]; + imageData.colors = [ + [255, 0, 0], + [0, 255, 0], + [0, 0, 255], + ]; + } + imageData.channelsVisible = imageData.colors.map(() => true); + } else { + const stats = await getMultiSelectionStats({ loader, selections, use3d: false }); + imageData.contrastLimits = stats.contrastLimits; + const computedColors: [number, number, number][] = + stats.contrastLimits.length === 1 + ? [[255, 255, 255]] + : stats.contrastLimits.map((_, i): [number, number, number] => { + const rgb = tryParseOmeroHexColor(Channels[i]?.color); + const p = COLOR_PALLETE[i % COLOR_PALLETE.length]; + return rgb ?? [p[0], p[1], p[2]]; + }); + imageData.colors = computedColors; + imageData.channelsVisible = computedColors.map(() => true); + } + imageData.selections = selections; + } else { + applyPerChannelFallbackWithoutOmero(imageData, loaderObj, selections); + } + } else { + applyBlindDefaults(imageData); + } + } catch (error) { + // Healthy imagery whose channel defaults could not be computed is NOT a failed + // image — it draws with fallback channels. That distinction is why EntryNotice + // exists as a channel separate from SpatialEntryError. + onNotice?.(error instanceof Error ? error.message : String(error)); + if (hasVivMetadata(loaderToCheck)) { + try { + imageData.selectionAxisSizes = + imageData.selectionAxisSizes ?? + getVivSelectionAxisSizes(loaderToCheck.labels, loaderToCheck.shape); + const fallbackSelections = buildDefaultSelection({ + labels: loaderToCheck.labels, + shape: loaderToCheck.shape, + }); + applyPerChannelFallbackWithoutOmero(imageData, loaderToCheck, fallbackSelections); + } catch { + applyBlindDefaults(imageData); + } + } else { + applyBlindDefaults(imageData); + } + } + + return imageData; +} + +/** + * Channel defaults for a labels element: one channel, semi-transparent fill with a + * strong outline. Lifted verbatim from `useLayerData`'s labels branch. + * + * Note labels carry SEVEN channel arrays where images carry four — which is why + * `avivatorish`'s `mergeLayerChannelState` does not cover them and the ladder is + * hand-written in two places today. + */ +export function buildLabelsChannelDefaults( + loader: unknown, + element: LabelsElement +): LabelsChannelDefaults { + const loaderToCheck = Array.isArray(loader) ? loader[0] : loader; + const labelsData: LabelsChannelDefaults = { + loader, + colors: [[255, 255, 255]], + channelsVisible: [true], + channelOpacities: [0.18], + channelOutlineOpacities: [0.95], + channelsFilled: [true], + channelStrokeWidths: [1.5], + selections: [{}], + }; + + if (hasVivMetadata(loaderToCheck)) { + const axisSizes = getVivSelectionAxisSizes(loaderToCheck.labels, loaderToCheck.shape); + const selections = clampVivSelectionsToAxes( + buildDefaultSelection({ labels: loaderToCheck.labels, shape: loaderToCheck.shape }), + axisSizes + ).slice(0, 1); + const metadataChannels = element.attrs.omero?.channels; + + const rgb = tryParseOmeroHexColor(metadataChannels?.[0]?.color); + const palette = COLOR_PALLETE[0]; + const color: [number, number, number] = rgb ?? [palette[0], palette[1], palette[2]]; + + labelsData.selectionAxisSizes = axisSizes; + labelsData.selections = selections.length > 0 ? selections : [{}]; + labelsData.colors = [color]; + labelsData.channelsVisible = [metadataChannels?.[0]?.active ?? true]; + labelsData.channelOpacities = [0.18]; + labelsData.channelOutlineOpacities = [0.95]; + labelsData.channelsFilled = [true]; + labelsData.channelStrokeWidths = [1.5]; + } + + return labelsData; +} diff --git a/packages/vis/src/SpatialCanvas/resolvers/RasterResolvers.ts b/packages/vis/src/SpatialCanvas/resolvers/RasterResolvers.ts new file mode 100644 index 00000000..2f48aca3 --- /dev/null +++ b/packages/vis/src/SpatialCanvas/resolvers/RasterResolvers.ts @@ -0,0 +1,382 @@ +import { getImageSize } from '@hms-dbmi/viv'; +import type { Matrix4 } from '@math.gl/core'; +import type { OmeZarrMultiscalesSource } from '@spatialdata/avivatorish'; +import { + type AxisAlignedBounds, + boundsFromImagePixelExtents, + type EntryResources, + type ImageElement, + isCancellation, + type LabelsElement, + type LabelsTooltipMetadata, + loadLabelsTooltipMetadata, + Resolution, + type ResolveContext, + type ResolveTask, + type ResourceResolver, + type SpatialData, + toSpatialEntryError, +} from '@spatialdata/core'; +import { + buildImageChannelDefaults, + buildLabelsChannelDefaults, + type ImageChannelDefaults, + type LabelsChannelDefaults, +} from '../imageLoaderChannelDefaults.js'; +import { createImageLoader } from '../renderers/imageRenderer.js'; + +/** + * The images and labels Resource Resolvers. + * + * They implement `@spatialdata/core`'s `ResourceResolver` — the same interface + * `PointsResolver` and `ShapesResolver` implement — but they **live in + * `@spatialdata/vis`**, and that is deliberate. + * + * ## Why here and not in `core` + * + * ADR 0004 §6 originally claimed `createImageLoader` "closes over the React + * `VivLoaderRegistry` context", making the image loader the one genuine + * ports-and-adapters dependency, and concluded that `core` should define a port. + * + * It closes over nothing. `createImageLoader` already takes `fetchMultiscales` as + * an injected parameter; the React context is merely the DI container at the call + * site. **The injection already existed, so there was no closure to break and no + * port to invent.** The ADR has been amended. + * + * And a port would have cost something real. `avivatorish` imports React *and* + * Viv, and is a de-vendoring holding pen for code that also lives upstream in Viv + * and in MDV — its own README calls the serialized image-state model *"still + * evolving"*. A port designed against it today would freeze a guess about an + * unsettled model into the interface `tgpu-htj2k` depends on. + * + * Decisively: `zarrextra`'s `VivCompatiblePixelSource` **already serves both Viv + * and `tgpu-htj2k`**, so the shared images seam already exists and sits *below* the + * resolver. Images is the one kind of the four where the duplication argument — the + * entire reason ADR 0004 exists — does not apply. `tgpu-htj2k` needs `PointsResolver` + * and `ShapesResolver` from `core`; it does not need an images resolver from anyone. + * + * **So: the interface is in `core`; placement is per-kind, driven by dependency.** + * These two may import Viv freely, which is also why `core` never needs raster + * extents — world bounds are computed right here, with `getImageSize`. + * + * The store holds only `ResourceResolver` and cannot tell which package these came + * from. If either of them ever needs something the interface does not offer, that + * is a signal about the *interface* — not a licence to special-case images. + */ + +/** The multiscales fetcher, injected. Supplied from `useVivLoaderRegistry()` at the call site. */ +export type FetchMultiscales = (source: OmeZarrMultiscalesSource) => Promise; + +export interface RasterResolverOptions { + fetchMultiscales?: FetchMultiscales; + spatialData?: SpatialData; + onStatus?: (layerId: string, resource: string, status: 'loading' | 'ready' | 'error') => void; +} + +export interface ImagesResolveConfig { + channels?: unknown; +} + +export interface LabelsResolveConfig { + tooltipFields?: string[]; + channels?: unknown; +} + +/** + * World bounds from a Viv loader's pixel extents. + * + * This is the line that killed the port. `core` owns world bounds (ADR 0004 §1) + * but may not import Viv — so a `core`-resident images resolver would have needed + * a port handing raster extents across. A `vis`-resident one just calls + * `getImageSize`, and `core` never has to know rasters have a width. + */ +function rasterBounds(loader: unknown, transform: Matrix4): AxisAlignedBounds | null { + const source = Array.isArray(loader) ? loader[0] : loader; + if (!source) return null; + try { + const { width, height } = getImageSize(source as never); + return boundsFromImagePixelExtents(width, height, transform); + } catch { + return null; + } +} + +// --------------------------------------------------------------------------- + +interface RasterEntry { + loader: Resolution; + inFlight: Map>; + bounds?: AxisAlignedBounds | null; + boundsSource?: unknown; + snapshot?: { version: number; value: EntryResources }; +} + +abstract class BaseRasterResolver { + protected readonly entries = new Map>(); + protected readonly listeners = new Set<() => void>(); + protected readonly options: RasterResolverOptions; + protected version = 0; + + constructor(options: RasterResolverOptions = {}) { + this.options = options; + } + + protected entry(key: string): RasterEntry { + let entry = this.entries.get(key); + if (!entry) { + entry = { loader: Resolution.idle(), inFlight: new Map() }; + this.entries.set(key, entry); + } + return entry; + } + + subscribe(listener: () => void): () => void { + this.listeners.add(listener); + return () => { + this.listeners.delete(listener); + }; + } + + getVersion(): number { + return this.version; + } + + protected notify(): void { + this.version += 1; + for (const listener of this.listeners) listener(); + } + + /** The resolved loader + channel defaults — what the panels read today. */ + getLoadedData(key: string): TData | undefined { + const entry = this.entries.get(key); + return entry ? Resolution.lastGood(entry.loader) : undefined; + } + + protected boundsFor(key: string, transform: Matrix4): AxisAlignedBounds | null { + const entry = this.entries.get(key); + if (!entry) return null; + const data = Resolution.lastGood(entry.loader); + if (!data) return null; + if (entry.boundsSource === data.loader) return entry.bounds ?? null; + const computed = rasterBounds(data.loader, transform); + entry.bounds = computed; + entry.boundsSource = data.loader; + return computed; + } + + evict(key: string): void { + this.entries.delete(key); + } + + dispose(): void { + this.entries.clear(); + this.listeners.clear(); + } + + /** Shared load scaffolding: dedup by task id, stale retention, error classification. */ + protected async runLoad( + entry: RasterEntry, + task: ResolveTask, + ctx: ResolveContext, + produce: () => Promise + ): Promise { + const existing = entry.inFlight.get(task.id); + if (existing) return existing; + + const stale = Resolution.lastGood(entry.loader); + entry.loader = Resolution.loading(stale !== undefined ? { stale } : {}); + this.options.onStatus?.(ctx.entryId, 'image', 'loading'); + this.notify(); + + const run = (async () => { + try { + entry.loader = Resolution.ready(await produce()); + this.options.onStatus?.(ctx.entryId, 'image', 'ready'); + } catch (cause) { + if (isCancellation(cause)) return; + entry.loader = Resolution.failed( + toSpatialEntryError(cause, { + elementKey: ctx.elementKey, + kind: ctx.kind, + resource: 'loader', + // A raster loader that throws failed to READ its multiscales — a load, + // not a decode. The seam knows; the bare Error does not. + fallback: 'load-failed', + }), + stale + ); + this.options.onStatus?.(ctx.entryId, 'image', 'error'); + } finally { + this.notify(); + } + })().finally(() => { + if (entry.inFlight.get(task.id) === run) entry.inFlight.delete(task.id); + }); + entry.inFlight.set(task.id, run); + return run; + } +} + +// --------------------------------------------------------------------------- + +export class ImagesResolver + extends BaseRasterResolver + implements ResourceResolver +{ + readonly kind = 'images' as const; + readonly blockingResources = ['loader'] as const; + + plan(ctx: ResolveContext): readonly ResolveTask[] { + const entry = this.entries.get(ctx.elementKey); + if (!entry || Resolution.isIdle(entry.loader)) { + return [{ id: `${ctx.elementKey}#loader`, resource: 'loader' }]; + } + return []; + } + + async load( + task: ResolveTask, + ctx: ResolveContext, + _signal: AbortSignal + ): Promise { + if (task.resource !== 'loader') return; + const entry = this.entry(ctx.elementKey); + await this.runLoad(entry, task, ctx, async () => { + const loader = await createImageLoader(ctx.element, this.options.fetchMultiscales); + return buildImageChannelDefaults(loader, ctx.element); + }); + } + + snapshot(ctx: ResolveContext): EntryResources { + const entry = this.entries.get(ctx.elementKey); + if (entry?.snapshot && entry.snapshot.version === this.version) { + return entry.snapshot.value; + } + const value: EntryResources = { + entryId: ctx.entryId, + elementKey: ctx.elementKey, + resources: { loader: entry?.loader ?? Resolution.idle() }, + notices: [], + bounds: this.boundsFor(ctx.elementKey, ctx.transform), + revision: this.version, + }; + if (entry) entry.snapshot = { version: this.version, value }; + return value; + } +} + +// --------------------------------------------------------------------------- + +export class LabelsResolver + extends BaseRasterResolver + implements ResourceResolver +{ + readonly kind = 'labels' as const; + readonly blockingResources = ['loader'] as const; + + private readonly tooltips = new Map< + string, + { signature: string; resolution: Resolution } + >(); + + plan(ctx: ResolveContext): readonly ResolveTask[] { + const key = ctx.elementKey; + const tasks: ResolveTask[] = []; + const entry = this.entries.get(key); + if (!entry || Resolution.isIdle(entry.loader)) { + tasks.push({ id: `${key}#loader`, resource: 'loader' }); + } + const fields = ctx.config.tooltipFields; + if (fields && fields.length > 0) { + const signature = fields.join(''); + if (this.tooltips.get(key)?.signature !== signature) { + tasks.push({ + id: `${key}#tooltip:${signature}`, + resource: 'tooltip', + payload: { tooltipFields: fields }, + }); + } + } + return tasks; + } + + async load( + task: ResolveTask, + ctx: ResolveContext, + _signal: AbortSignal + ): Promise { + const key = ctx.elementKey; + + if (task.resource === 'loader') { + const entry = this.entry(key); + await this.runLoad(entry, task, ctx, async () => { + const loader = await createImageLoader(ctx.element, this.options.fetchMultiscales); + return buildLabelsChannelDefaults(loader, ctx.element); + }); + return; + } + + if (task.resource === 'tooltip') { + const fields = (task.payload as { tooltipFields: string[] }).tooltipFields; + const signature = fields.join(''); + try { + const metadata = await loadLabelsTooltipMetadata( + this.options.spatialData, + ctx.element, + fields + ); + this.tooltips.set(key, { signature, resolution: Resolution.ready(metadata) }); + } catch (cause) { + if (isCancellation(cause)) return; + this.tooltips.set(key, { + signature, + resolution: Resolution.failed( + toSpatialEntryError(cause, { + elementKey: key, + kind: 'labels', + resource: 'tooltip', + fallback: 'load-failed', + }) + ), + }); + } finally { + this.notify(); + } + } + } + + getTooltipMetadata(key: string): LabelsTooltipMetadata | undefined { + const held = this.tooltips.get(key); + return held ? Resolution.lastGood(held.resolution) : undefined; + } + + snapshot(ctx: ResolveContext): EntryResources { + const entry = this.entries.get(ctx.elementKey); + if (entry?.snapshot && entry.snapshot.version === this.version) { + return entry.snapshot.value; + } + const value: EntryResources = { + entryId: ctx.entryId, + elementKey: ctx.elementKey, + resources: { + loader: entry?.loader ?? Resolution.idle(), + tooltip: this.tooltips.get(ctx.elementKey)?.resolution ?? Resolution.idle(), + }, + notices: [], + bounds: this.boundsFor(ctx.elementKey, ctx.transform), + revision: this.version, + }; + if (entry) entry.snapshot = { version: this.version, value }; + return value; + } + + override evict(key: string): void { + super.evict(key); + this.tooltips.delete(key); + } + + override dispose(): void { + super.dispose(); + this.tooltips.clear(); + } +} diff --git a/packages/vis/tests/rasterResolvers.spec.ts b/packages/vis/tests/rasterResolvers.spec.ts new file mode 100644 index 00000000..5521a961 --- /dev/null +++ b/packages/vis/tests/rasterResolvers.spec.ts @@ -0,0 +1,255 @@ +import { Matrix4 } from '@math.gl/core'; +import type { + ImageElement, + LabelsElement, + ResolveContext, + ResourceResolver, +} from '@spatialdata/core'; +import { Resolution, SpatialEntryStore } from '@spatialdata/core'; +import { describe, expect, it, vi } from 'vitest'; +import { + type ImagesResolveConfig, + ImagesResolver, + type LabelsResolveConfig, + LabelsResolver, +} from '../src/SpatialCanvas/resolvers/RasterResolvers.js'; + +/** + * The images and labels resolvers — which live in `vis`, not `core`. + * + * The claim under test is the one that makes that placement safe: **they satisfy + * the same `ResourceResolver` interface**, and the store cannot tell they came + * from a different package. Placement is per-kind and driven by dependency; it is + * not a licence for images to be special. + * + * If a `vis`-resident resolver ever needs something the interface doesn't offer, + * that is a signal about the interface — and these tests are where it would first + * show up as friction. + */ + +const vivLoader = () => [{ labels: ['y', 'x'], shape: [64, 64], dtype: 'uint16' }]; + +const imageElement = (over: Record = {}) => + ({ + key: 'morphology', + path: 'images/morphology', + attrs: {}, + getStore: () => ({}), + ...over, + }) as unknown as ImageElement; + +const labelsElement = (over: Record = {}) => + ({ + key: 'cell_labels', + path: 'labels/cells', + attrs: {}, + getStore: () => ({}), + ...over, + }) as unknown as LabelsElement; + +const imageCtx = ( + el: ImageElement, + config: ImagesResolveConfig = {} +): ResolveContext => ({ + entryId: 'layer-i', + elementKey: 'morphology', + kind: 'images', + element: el, + config, + transform: new Matrix4(), +}); + +const labelsCtx = ( + el: LabelsElement, + config: LabelsResolveConfig = {} +): ResolveContext => ({ + entryId: 'layer-l', + elementKey: 'cell_labels', + kind: 'labels', + element: el, + config, + transform: new Matrix4(), +}); + +const signal = () => new AbortController().signal; + +/** Injected in place of the real OME-Zarr multiscales fetch. This IS the DI seam + * ADR 0004 §6 mistakenly believed did not exist — createImageLoader has always + * taken it as a parameter. */ +const fetchMultiscales = () => vi.fn(async () => vivLoader()); + +describe('ImagesResolver — a ResourceResolver that happens to live in vis', () => { + it('is structurally a ResourceResolver', () => { + // The type assertion is the test: if the vis-resident resolvers drifted from + // core's interface, this would not compile. + const resolver: ResourceResolver = new ImagesResolver(); + + expect(resolver.kind).toBe('images'); + expect(resolver.blockingResources).toEqual(['loader']); + }); + + it('plan() is pure — it does not fetch', () => { + const fetch = fetchMultiscales(); + + new ImagesResolver({ fetchMultiscales: fetch }).plan(imageCtx(imageElement())); + + expect(fetch).not.toHaveBeenCalled(); + }); + + it('loads through the INJECTED fetcher — no React context, no port', async () => { + const fetch = fetchMultiscales(); + const resolver = new ImagesResolver({ fetchMultiscales: fetch }); + const el = imageElement(); + + await resolver.load({ id: 'l', resource: 'loader' }, imageCtx(el), signal()); + + expect(fetch).toHaveBeenCalledTimes(1); + expect(Resolution.isReady(resolver.snapshot(imageCtx(el)).resources.loader as never)).toBe( + true + ); + }); + + it('computes channel defaults with no omero metadata (the per-channel fallback)', async () => { + const resolver = new ImagesResolver({ fetchMultiscales: fetchMultiscales() }); + const el = imageElement(); + + await resolver.load({ id: 'l', resource: 'loader' }, imageCtx(el), signal()); + + const data = resolver.getLoadedData('morphology'); + expect(data?.colors).toBeDefined(); + expect(data?.contrastLimits).toBeDefined(); + expect(data?.channelsVisible).toBeDefined(); + // uint16 with no omero → the per-channel fallback's max value. + expect(data?.contrastLimits?.[0]).toEqual([0, 65535]); + }); + + it('turns a loader failure into a value, not a throw', async () => { + const resolver = new ImagesResolver({ + fetchMultiscales: vi.fn(async () => { + throw new Error('the store is unreachable'); + }), + }); + const el = imageElement(); + vi.spyOn(console, 'error').mockImplementation(() => {}); + + await expect( + resolver.load({ id: 'l', resource: 'loader' }, imageCtx(el), signal()) + ).resolves.toBeUndefined(); + + const loader = resolver.snapshot(imageCtx(el)).resources.loader; + if (loader?.status !== 'failed') throw new Error('narrowing'); + expect(loader.error.kind).toBe('load-failed'); + expect(loader.error.retryable).toBe(true); + }); + + it('stops planning once loaded', async () => { + const resolver = new ImagesResolver({ fetchMultiscales: fetchMultiscales() }); + const el = imageElement(); + + await resolver.load({ id: 'l', resource: 'loader' }, imageCtx(el), signal()); + + expect(resolver.plan(imageCtx(el))).toEqual([]); + }); + + it('snapshot is identity-stable between mutations', async () => { + const resolver = new ImagesResolver({ fetchMultiscales: fetchMultiscales() }); + const el = imageElement(); + await resolver.load({ id: 'l', resource: 'loader' }, imageCtx(el), signal()); + + const first = resolver.snapshot(imageCtx(el)); + + for (let i = 0; i < 5; i++) expect(resolver.snapshot(imageCtx(el))).toBe(first); + }); +}); + +describe('LabelsResolver', () => { + it('is structurally a ResourceResolver', () => { + const resolver: ResourceResolver = new LabelsResolver(); + + expect(resolver.kind).toBe('labels'); + }); + + it('builds the seven-array labels channel defaults', async () => { + // Labels carry SEVEN channel arrays where images carry four — which is why + // avivatorish's mergeLayerChannelState does not cover them, and why the ladder + // is hand-written in two places today. + const resolver = new LabelsResolver({ fetchMultiscales: fetchMultiscales() }); + const el = labelsElement(); + + await resolver.load({ id: 'l', resource: 'loader' }, labelsCtx(el), signal()); + + const data = resolver.getLoadedData('cell_labels'); + // The loader exposes labels/shape, so this takes the metadata branch: with no + // omero colour it falls to COLOR_PALLETE[0], NOT the bare white default. (White + // is only what you get when the loader tells us nothing at all.) + expect(data?.colors).toEqual([[0, 0, 255]]); + expect(data?.channelsVisible).toEqual([true]); + // The characteristic labels look: faint fill, strong outline. + expect(data?.channelOpacities).toEqual([0.18]); + expect(data?.channelOutlineOpacities).toEqual([0.95]); + expect(data?.channelsFilled).toEqual([true]); + expect(data?.channelStrokeWidths).toEqual([1.5]); + expect(data?.selections).toBeDefined(); + }); + + it('falls back to white when the loader exposes no metadata at all', async () => { + const resolver = new LabelsResolver({ fetchMultiscales: vi.fn(async () => ({})) }); + const el = labelsElement(); + + await resolver.load({ id: 'l', resource: 'loader' }, labelsCtx(el), signal()); + + expect(resolver.getLoadedData('cell_labels')?.colors).toEqual([[255, 255, 255]]); + }); + + it('plans a tooltip only when fields are configured', () => { + const resolver = new LabelsResolver(); + + expect(resolver.plan(labelsCtx(labelsElement())).map((t) => t.resource)).toEqual(['loader']); + expect( + resolver + .plan(labelsCtx(labelsElement(), { tooltipFields: ['region'] })) + .map((t) => t.resource) + ).toContain('tooltip'); + }); +}); + +describe('the store cannot tell where a resolver lives', () => { + // The whole safety argument for placement-by-dependency. If the store could + // distinguish a vis-resident resolver from a core-resident one, "images is + // special" would become representable — and that is how one interface quietly + // becomes two. + it('registers vis-resident resolvers through the same core registry', async () => { + const images = new ImagesResolver({ fetchMultiscales: fetchMultiscales() }); + const labels = new LabelsResolver({ fetchMultiscales: fetchMultiscales() }); + + const store = new SpatialEntryStore({ + points: images as never, // not exercised here + shapes: images as never, + images, + labels, + }); + + const el = imageElement(); + await store.reconcile([imageCtx(el)]); + + expect(store.snapshot(imageCtx(el))?.resources.loader?.status).toBe('ready'); + expect(store.isBlocking(imageCtx(el))).toBe(false); + }); + + it('blocks on the raster loader until it is drawable', async () => { + const images = new ImagesResolver({ fetchMultiscales: fetchMultiscales() }); + const store = new SpatialEntryStore({ + points: images as never, + shapes: images as never, + images, + labels: images as never, + }); + const el = imageElement(); + + expect(store.isBlocking(imageCtx(el))).toBe(true); + + await store.reconcile([imageCtx(el)]); + + expect(store.isBlocking(imageCtx(el))).toBe(false); + }); +}); From 5311d04dfdc50b0fd9fd8a65d064cedd668814d4 Mon Sep 17 00:00:00 2001 From: Peter Todd Date: Wed, 15 Jul 2026 11:41:15 +0100 Subject: [PATCH 9/9] Address CodeRabbit review: snapshot keying, cancellation, evict, notices MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes the review findings that touch the contract the flip (C8) builds on, before it builds on them. The deferred items (worker cancellation, R3/R5, store supersession, pre-existing guessRgb) are left for Track A / their own issues, as planned. Snapshot memoization was keyed on the resolver version alone, but an EntryResources embeds per-entry data (entryId) and per-context data (the transform, via bounds; the selection, via notices). Two layers over one element got each other's snapshot; a coordinate-system change left bounds stale. New SnapshotCache keys by (entryId, version, transform identity, config signature) and self-cleans via evictByElement. Applied in all three resolvers (points, shapes, raster) — it recurred in each. Transform identity is the right key, not value: availableElements is memoised on [spatialData, coordinateSystem], so a given entry sees a stable transform across pan/zoom and a fresh one exactly when the coordinate system changes — which is when bounds must recompute. The inner bounds memo now keys on the transform too, or it would defeat the outer fix. Cancellation left a slot stuck in `loading` forever: the loader was set to loading before the load, and the isCancellation early-return didn't restore it, so plan() (which only reschedules an idle slot) never retried. Both the shapes and raster load paths now capture the prior resolution and restore it on cancel. Regression tests for initial-load cancellation added. evict() now notifies, so external-store consumers drop the stale snapshot immediately instead of at the next unrelated mutation. Points evict notifies too; the frozen pointsDataEngine.spec.ts still passes unchanged (its one notify test unsubscribes before evicting). Channel-default failures are surfaced as a channel-defaults-fallback EntryNotice instead of vanishing. The verbatim lift had replaced the old console.warn with an onNotice callback that was never wired — strictly less observable than before. ImagesResolver now collects the notice and puts it in the snapshot; the image still draws with fallback channels, which is exactly the healthy-data-with-a-caveat case EntryNotice exists for. Also: guard the optional tooltip payload before deref (a malformed task is a no-op, not a reconcile-rejecting throw); isCancellation uses the `in` operator instead of a type assertion. New coverage: SnapshotCache unit spec (shared-element separation, per-dim invalidation, evict-by-element); shared-element and transform-invalidation tests in the points, shapes and raster suites; cancellation-restore regression tests. 559 tests (was 542). Build, biome, dependency boundaries green; frozen spec untouched. Co-Authored-By: Claude Opus 4.8 --- packages/core/src/engine/PointsResolver.ts | 25 +++--- packages/core/src/engine/ShapesResolver.ts | 47 +++++++--- packages/core/src/engine/errors.ts | 4 +- packages/core/src/engine/index.ts | 1 + packages/core/src/engine/snapshotCache.ts | 85 ++++++++++++++++++ packages/core/tests/pointsResolver.spec.ts | 44 +++++++++- packages/core/tests/shapesResolver.spec.ts | 57 +++++++++++- packages/core/tests/snapshotCache.spec.ts | 87 +++++++++++++++++++ .../resolvers/RasterResolvers.ts | 84 ++++++++++++++---- packages/vis/tests/rasterResolvers.spec.ts | 58 ++++++++++++- 10 files changed, 439 insertions(+), 53 deletions(-) create mode 100644 packages/core/src/engine/snapshotCache.ts create mode 100644 packages/core/tests/snapshotCache.spec.ts diff --git a/packages/core/src/engine/PointsResolver.ts b/packages/core/src/engine/PointsResolver.ts index 9c688b85..b86f84a2 100644 --- a/packages/core/src/engine/PointsResolver.ts +++ b/packages/core/src/engine/PointsResolver.ts @@ -6,6 +6,7 @@ import type { PointsFeatureCatalog } from '../pointsTiling.js'; import type { EntryNotice } from './errors.js'; import { Resolution } from './resolution.js'; import type { EntryResources, ResolveContext, ResolveTask, ResourceResolver } from './resolver.js'; +import { SnapshotCache } from './snapshotCache.js'; /** * The points Resource Resolver. @@ -113,8 +114,6 @@ interface PointsEntry { scannedRows: number; partialResult?: PointsLoadResult; }; - /** Cached snapshot, so `snapshot()` is identity-stable between mutations. */ - snapshot?: { version: number; value: EntryResources }; } /** Public snapshot of a selection's feature-index load, for the filter panel. */ @@ -136,6 +135,7 @@ export class PointsResolver implements ResourceResolver(); private readonly listeners = new Set<() => void>(); private readonly callbacks: PointsResolverCallbacks; + private readonly snapshots = new SnapshotCache(); private version = 0; constructor(callbacks: PointsResolverCallbacks = {}) { @@ -226,10 +226,12 @@ export class PointsResolver implements ResourceResolver): EntryResources { const key = ctx.elementKey; - const entry = this.entries.get(key); - if (entry?.snapshot && entry.snapshot.version === this.version) { - return entry.snapshot.value; - } + // Key the memo by everything the snapshot embeds: the entry (several layers + // may share one element), and the selection (it drives the truncation notice). + // Points bounds are not wired in Step 1, so the transform is not part of the key. + const configSig = (ctx.config.featureCodes ?? []).join(','); + const cached = this.snapshots.get(ctx.entryId, this.version, ctx.transform, configSig); + if (cached) return cached; const value: EntryResources = { entryId: ctx.entryId, @@ -245,9 +247,7 @@ export class PointsResolver implements ResourceResolver (fields ?? []).join(''); @@ -91,6 +93,7 @@ export class ShapesResolver implements ResourceResolver void>(); private readonly callbacks: ShapesResolverCallbacks; private readonly spatialData: SpatialData | undefined; + private readonly snapshots = new SnapshotCache(); private version = 0; constructor(options: { spatialData?: SpatialData; callbacks?: ShapesResolverCallbacks } = {}) { @@ -179,6 +182,10 @@ export class ShapesResolver implements ResourceResolver); entry[slot] = Resolution.loading(stale !== undefined ? { stale } : {}) as never; @@ -215,8 +222,12 @@ export class ShapesResolver implements ResourceResolver): EntryResources { const key = ctx.elementKey; - const entry = this.entries.get(key); - if (entry?.snapshot && entry.snapshot.version === this.version) { - return entry.snapshot.value; - } + // Key the memo by the entry (layers may share an element) and the transform + // (it moves the bounds). Shapes resources and notices don't depend on config, + // so there is no config dimension here. + const cached = this.snapshots.get(ctx.entryId, this.version, ctx.transform, ''); + if (cached) return cached; + const entry = this.entries.get(key); const value: EntryResources = { entryId: ctx.entryId, elementKey: key, @@ -253,16 +266,19 @@ export class ShapesResolver implements ResourceResolver @@ -271,7 +287,9 @@ export class ShapesResolver implements ResourceResolver(); + + /** The cached snapshot for this entry, or `undefined` if any input changed. */ + get( + entryId: string, + version: number, + transform: unknown, + configSig: string + ): EntryResources | undefined { + const hit = this.cache.get(entryId); + if ( + hit && + hit.version === version && + hit.transform === transform && + hit.configSig === configSig + ) { + return hit.value; + } + return undefined; + } + + set( + entryId: string, + version: number, + transform: unknown, + configSig: string, + value: EntryResources + ): void { + this.cache.set(entryId, { version, transform, configSig, value }); + } + + /** Drop every memo whose snapshot was for this element. */ + evictByElement(elementKey: string): void { + for (const [entryId, record] of this.cache) { + if (record.value.elementKey === elementKey) { + this.cache.delete(entryId); + } + } + } + + clear(): void { + this.cache.clear(); + } +} diff --git a/packages/core/tests/pointsResolver.spec.ts b/packages/core/tests/pointsResolver.spec.ts index c07621de..3781862e 100644 --- a/packages/core/tests/pointsResolver.spec.ts +++ b/packages/core/tests/pointsResolver.spec.ts @@ -187,29 +187,65 @@ describe('snapshot() — per-resource resolutions, identity-stable', () => { it('returns the SAME object until something mutates — an adapter memoises on this', async () => { const resolver = new PointsResolver(); const el = element(); + // ONE context, reused — the hook memoises AvailableElement (transform included) + // on [spatialData, coordinateSystem], so a given entry sees a stable ctx across + // renders. Repeated calls stand in for repeated renders (pan, hover, viewState). + const c = ctx(el); await resolver.ensureLoaded({ key: 'transcripts', layerId: 'layer-p', element: el }); - const first = resolver.snapshot(ctx(el)); + const first = resolver.snapshot(c); // Ten "renders" with no state change. A fresh object here is a deck teardown // per frame — the pan flash, one layer up. for (let i = 0; i < 10; i++) { - expect(resolver.snapshot(ctx(el))).toBe(first); + expect(resolver.snapshot(c)).toBe(first); } }); it('returns a NEW object once state changes', async () => { const resolver = new PointsResolver(); const el = element(); + const c = ctx(el); await resolver.ensureLoaded({ key: 'transcripts', layerId: 'layer-p', element: el }); - const before = resolver.snapshot(ctx(el)); + const before = resolver.snapshot(c); await resolver.ensureFeatureCatalog({ key: 'transcripts', layerId: 'layer-p', element: el }); - const after = resolver.snapshot(ctx(el)); + const after = resolver.snapshot(c); expect(after).not.toBe(before); }); + it('gives two entries sharing one element DISTINCT snapshots', async () => { + // elementKey is the cache key; several layers may share one. The memo must not + // hand entry B the snapshot it built for entry A — entryId and all. + const resolver = new PointsResolver(); + const el = element(); + await resolver.ensureLoaded({ key: 'transcripts', layerId: 'layer-a', element: el }); + // Same element, same transform — the ONLY difference is the entry (layer). + const base = ctx(el); + + const a = resolver.snapshot({ ...base, entryId: 'layer-a' }); + const b = resolver.snapshot({ ...base, entryId: 'layer-b' }); + + expect(a.entryId).toBe('layer-a'); + expect(b.entryId).toBe('layer-b'); + expect(a).not.toBe(b); + // ...but each is still stable on its own. + expect(resolver.snapshot({ ...base, entryId: 'layer-a' })).toBe(a); + }); + + it('refreshes the snapshot when the selection changes — it drives the notice', async () => { + // featureCodes is part of the memo key because the truncation notice depends on it. + const resolver = new PointsResolver(); + const el = element(); + await resolver.ensureLoaded({ key: 'transcripts', layerId: 'layer-p', element: el }); + + const none = resolver.snapshot(ctx(el, {})); + const filtered = resolver.snapshot(ctx(el, { featureCodes: [0] })); + + expect(filtered).not.toBe(none); + }); + it('keeps a failed resource from blanking a healthy one — failure is PER-RESOURCE', async () => { // A points entry whose catalog scan fails must still draw its geometry. That is // the whole reason there is no entry-wide Result. diff --git a/packages/core/tests/shapesResolver.spec.ts b/packages/core/tests/shapesResolver.spec.ts index cc83e405..6910ae22 100644 --- a/packages/core/tests/shapesResolver.spec.ts +++ b/packages/core/tests/shapesResolver.spec.ts @@ -169,6 +169,25 @@ describe('load() — the only place I/O starts', () => { expect(geometry.error.kind).toBe('decode-failed'); expect(geometry.error.retryable).toBe(true); }); + + it('restores the prior resolution when an initial load is cancelled — never hangs', async () => { + // The slot is set to `loading` before the load. A cancelled initial load must + // fall back to `idle`, or plan() (which only schedules idle geometry) never + // reschedules it and the entry hangs in loading forever. + const resolver = new ShapesResolver(); + const el = element({ + loadRenderData: vi.fn(async () => { + throw new DOMException('Aborted', 'AbortError'); + }), + }); + const c = ctx(el); + + await resolver.load({ id: 'g', resource: 'geometry' }, c, signal()); + + expect(resolver.snapshot(c).resources.geometry?.status).toBe('idle'); + // ...and it is therefore replannable. + expect(resolver.plan(c).map((t) => t.resource)).toContain('geometry'); + }); }); describe('failure is PER-RESOURCE', () => { @@ -207,15 +226,47 @@ describe('snapshot() — identity and bounds', () => { it('returns the SAME object until something mutates', async () => { const resolver = new ShapesResolver(); const el = element(); - await resolver.load({ id: 'g', resource: 'geometry' }, ctx(el), signal()); + // One ctx, reused — a given entry sees a stable transform across renders (the + // hook memoises it on [spatialData, coordinateSystem]). + const c = ctx(el); + await resolver.load({ id: 'g', resource: 'geometry' }, c, signal()); - const first = resolver.snapshot(ctx(el)); + const first = resolver.snapshot(c); for (let i = 0; i < 10; i++) { - expect(resolver.snapshot(ctx(el))).toBe(first); + expect(resolver.snapshot(c)).toBe(first); } }); + it('gives two layers over one element distinct snapshots', async () => { + const resolver = new ShapesResolver(); + const el = element(); + await resolver.load({ id: 'g', resource: 'geometry' }, ctx(el), signal()); + const base = ctx(el); // same element, same transform — differ only by entry + + const a = resolver.snapshot({ ...base, entryId: 'layer-a' }); + const b = resolver.snapshot({ ...base, entryId: 'layer-b' }); + + expect(a.entryId).toBe('layer-a'); + expect(b.entryId).toBe('layer-b'); + expect(a).not.toBe(b); + }); + + it('recomputes bounds when the transform changes (a new coordinate system)', async () => { + // Bounds are world-space. Reuse the same geometry under a new Matrix4 and the + // old bounds would be wrong — so the transform is part of the snapshot key. + const resolver = new ShapesResolver(); + const el = element(); + await resolver.load({ id: 'g', resource: 'geometry' }, ctx(el), signal()); + + const identity = resolver.snapshot({ ...ctx(el), transform: new Matrix4() }); + const scaled = resolver.snapshot({ ...ctx(el), transform: new Matrix4().scale(2) }); + + expect(scaled).not.toBe(identity); + // Circles at (0,0),(10,10) r=1 → [-1,11]; scaled ×2 → [-2,22]. + expect(scaled.bounds).toMatchObject({ minX: -2, maxX: 22 }); + }); + it('computes world bounds from the geometry once loaded', async () => { const resolver = new ShapesResolver(); const el = element(); diff --git a/packages/core/tests/snapshotCache.spec.ts b/packages/core/tests/snapshotCache.spec.ts new file mode 100644 index 00000000..352b459f --- /dev/null +++ b/packages/core/tests/snapshotCache.spec.ts @@ -0,0 +1,87 @@ +import { describe, expect, it } from 'vitest'; +import type { EntryResources } from '../src/engine/index.js'; +import { SnapshotCache } from '../src/engine/index.js'; + +/** + * The memo behind every resolver's `snapshot()`. Its whole job is to invalidate on + * exactly the things a snapshot embeds — entry, version, transform, config — and + * nothing else, so it recurs across three resolvers in two packages. + */ + +const entryResources = (entryId: string, elementKey: string): EntryResources => ({ + entryId, + elementKey, + resources: {}, + notices: [], + bounds: null, + revision: 0, +}); + +const transformA = {}; +const transformB = {}; + +describe('SnapshotCache', () => { + it('returns the cached value when every dimension matches', () => { + const cache = new SnapshotCache(); + const value = entryResources('e1', 'k'); + cache.set('e1', 1, transformA, 'sig', value); + + expect(cache.get('e1', 1, transformA, 'sig')).toBe(value); + }); + + it('misses on a version bump', () => { + const cache = new SnapshotCache(); + cache.set('e1', 1, transformA, 'sig', entryResources('e1', 'k')); + + expect(cache.get('e1', 2, transformA, 'sig')).toBeUndefined(); + }); + + it('misses on a different transform identity', () => { + const cache = new SnapshotCache(); + cache.set('e1', 1, transformA, 'sig', entryResources('e1', 'k')); + + expect(cache.get('e1', 1, transformB, 'sig')).toBeUndefined(); + }); + + it('misses on a different config signature', () => { + const cache = new SnapshotCache(); + cache.set('e1', 1, transformA, 'sig', entryResources('e1', 'k')); + + expect(cache.get('e1', 1, transformA, 'other')).toBeUndefined(); + }); + + it('keeps entries sharing one element separate', () => { + // The core reason keying by version alone is wrong: two entryIds, one element. + const cache = new SnapshotCache(); + const a = entryResources('layer-a', 'k'); + const b = entryResources('layer-b', 'k'); + cache.set('layer-a', 1, transformA, '', a); + cache.set('layer-b', 1, transformA, '', b); + + expect(cache.get('layer-a', 1, transformA, '')).toBe(a); + expect(cache.get('layer-b', 1, transformA, '')).toBe(b); + }); + + it('evicts by element, dropping every entry that shared it', () => { + const cache = new SnapshotCache(); + cache.set('layer-a', 1, transformA, '', entryResources('layer-a', 'shared')); + cache.set('layer-b', 1, transformA, '', entryResources('layer-b', 'shared')); + cache.set('layer-c', 1, transformA, '', entryResources('layer-c', 'other')); + + cache.evictByElement('shared'); + + expect(cache.get('layer-a', 1, transformA, '')).toBeUndefined(); + expect(cache.get('layer-b', 1, transformA, '')).toBeUndefined(); + // ...but an entry over a different element is untouched. + expect(cache.get('layer-c', 1, transformA, '')).toBeDefined(); + }); + + it('clear drops everything', () => { + const cache = new SnapshotCache(); + cache.set('e1', 1, transformA, '', entryResources('e1', 'k')); + + cache.clear(); + + expect(cache.get('e1', 1, transformA, '')).toBeUndefined(); + }); +}); diff --git a/packages/vis/src/SpatialCanvas/resolvers/RasterResolvers.ts b/packages/vis/src/SpatialCanvas/resolvers/RasterResolvers.ts index 2f48aca3..808757d0 100644 --- a/packages/vis/src/SpatialCanvas/resolvers/RasterResolvers.ts +++ b/packages/vis/src/SpatialCanvas/resolvers/RasterResolvers.ts @@ -4,6 +4,7 @@ import type { OmeZarrMultiscalesSource } from '@spatialdata/avivatorish'; import { type AxisAlignedBounds, boundsFromImagePixelExtents, + type EntryNotice, type EntryResources, type ImageElement, isCancellation, @@ -14,6 +15,7 @@ import { type ResolveContext, type ResolveTask, type ResourceResolver, + SnapshotCache, type SpatialData, toSpatialEntryError, } from '@spatialdata/core'; @@ -108,13 +110,23 @@ interface RasterEntry { inFlight: Map>; bounds?: AxisAlignedBounds | null; boundsSource?: unknown; - snapshot?: { version: number; value: EntryResources }; + /** The transform `bounds` was computed with — world bounds are transform-relative. */ + boundsTransform?: unknown; + /** Non-fatal facts about a successful load — e.g. channel defaults fell back. */ + notices?: readonly EntryNotice[]; +} + +/** What a raster load produces: the resolved data, plus any non-fatal notices. */ +interface RasterLoadResult { + data: T; + notices?: readonly EntryNotice[]; } abstract class BaseRasterResolver { protected readonly entries = new Map>(); protected readonly listeners = new Set<() => void>(); protected readonly options: RasterResolverOptions; + protected readonly snapshots = new SnapshotCache(); protected version = 0; constructor(options: RasterResolverOptions = {}) { @@ -157,19 +169,28 @@ abstract class BaseRasterResolver, task: ResolveTask, ctx: ResolveContext, - produce: () => Promise + produce: () => Promise> ): Promise { const existing = entry.inFlight.get(task.id); if (existing) return existing; + // Capture the exact pre-load resolution. On cancellation we restore it — an + // aborted initial load must fall back to its prior state, or `plan()` (which + // only schedules an idle loader) never reschedules it and the entry hangs. + const prior = entry.loader; const stale = Resolution.lastGood(entry.loader); entry.loader = Resolution.loading(stale !== undefined ? { stale } : {}); this.options.onStatus?.(ctx.entryId, 'image', 'loading'); @@ -190,10 +215,15 @@ abstract class BaseRasterResolver { try { - entry.loader = Resolution.ready(await produce()); + const result = await produce(); + entry.loader = Resolution.ready(result.data); + entry.notices = result.notices; this.options.onStatus?.(ctx.entryId, 'image', 'ready'); } catch (cause) { - if (isCancellation(cause)) return; + if (isCancellation(cause)) { + entry.loader = prior; + return; + } entry.loader = Resolution.failed( toSpatialEntryError(cause, { elementKey: ctx.elementKey, @@ -243,24 +273,36 @@ export class ImagesResolver const entry = this.entry(ctx.elementKey); await this.runLoad(entry, task, ctx, async () => { const loader = await createImageLoader(ctx.element, this.options.fetchMultiscales); - return buildImageChannelDefaults(loader, ctx.element); + // Channel-default computation reads pixels and can fail on a store that served + // its metadata fine. That is healthy-data-with-a-caveat, so it surfaces as a + // NOTICE, not a failed loader — the image still draws with fallback channels. + const notices: EntryNotice[] = []; + const data = await buildImageChannelDefaults(loader, ctx.element, (reason) => { + notices.push({ + kind: 'channel-defaults-fallback', + message: 'Channel defaults could not be computed; using fallbacks.', + reason, + }); + }); + return { data, notices }; }); } snapshot(ctx: ResolveContext): EntryResources { + // Key by entry (layers may share an element) and transform (it moves the bounds). + const cached = this.snapshots.get(ctx.entryId, this.version, ctx.transform, ''); + if (cached) return cached; + const entry = this.entries.get(ctx.elementKey); - if (entry?.snapshot && entry.snapshot.version === this.version) { - return entry.snapshot.value; - } const value: EntryResources = { entryId: ctx.entryId, elementKey: ctx.elementKey, resources: { loader: entry?.loader ?? Resolution.idle() }, - notices: [], + notices: entry?.notices ?? [], bounds: this.boundsFor(ctx.elementKey, ctx.transform), revision: this.version, }; - if (entry) entry.snapshot = { version: this.version, value }; + this.snapshots.set(ctx.entryId, this.version, ctx.transform, '', value); return value; } } @@ -311,13 +353,16 @@ export class LabelsResolver const entry = this.entry(key); await this.runLoad(entry, task, ctx, async () => { const loader = await createImageLoader(ctx.element, this.options.fetchMultiscales); - return buildLabelsChannelDefaults(loader, ctx.element); + return { data: buildLabelsChannelDefaults(loader, ctx.element) }; }); return; } if (task.resource === 'tooltip') { - const fields = (task.payload as { tooltipFields: string[] }).tooltipFields; + // `payload` is optional on ResolveTask; a malformed tooltip task must be a + // no-op rather than throwing out of load() and rejecting the whole reconcile. + const fields = (task.payload as { tooltipFields?: string[] } | undefined)?.tooltipFields; + if (!fields) return; const signature = fields.join(''); try { const metadata = await loadLabelsTooltipMetadata( @@ -351,10 +396,11 @@ export class LabelsResolver } snapshot(ctx: ResolveContext): EntryResources { + // Key by entry (layers may share an element) and transform (it moves the bounds). + const cached = this.snapshots.get(ctx.entryId, this.version, ctx.transform, ''); + if (cached) return cached; + const entry = this.entries.get(ctx.elementKey); - if (entry?.snapshot && entry.snapshot.version === this.version) { - return entry.snapshot.value; - } const value: EntryResources = { entryId: ctx.entryId, elementKey: ctx.elementKey, @@ -362,11 +408,11 @@ export class LabelsResolver loader: entry?.loader ?? Resolution.idle(), tooltip: this.tooltips.get(ctx.elementKey)?.resolution ?? Resolution.idle(), }, - notices: [], + notices: entry?.notices ?? [], bounds: this.boundsFor(ctx.elementKey, ctx.transform), revision: this.version, }; - if (entry) entry.snapshot = { version: this.version, value }; + this.snapshots.set(ctx.entryId, this.version, ctx.transform, '', value); return value; } diff --git a/packages/vis/tests/rasterResolvers.spec.ts b/packages/vis/tests/rasterResolvers.spec.ts index 5521a961..71270f7f 100644 --- a/packages/vis/tests/rasterResolvers.spec.ts +++ b/packages/vis/tests/rasterResolvers.spec.ts @@ -152,13 +152,67 @@ describe('ImagesResolver — a ResourceResolver that happens to live in vis', () }); it('snapshot is identity-stable between mutations', async () => { + const resolver = new ImagesResolver({ fetchMultiscales: fetchMultiscales() }); + const el = imageElement(); + // One ctx, reused — a given entry sees a stable transform across renders. + const c = imageCtx(el); + await resolver.load({ id: 'l', resource: 'loader' }, c, signal()); + + const first = resolver.snapshot(c); + + for (let i = 0; i < 5; i++) expect(resolver.snapshot(c)).toBe(first); + }); + + it('restores the prior resolution when an initial load is cancelled', async () => { + // Without the restore, plan() (which only schedules an idle loader) would never + // reschedule and the entry would hang in `loading` forever. + const resolver = new ImagesResolver({ + fetchMultiscales: vi.fn(async () => { + throw new DOMException('Aborted', 'AbortError'); + }), + }); + const el = imageElement(); + const c = imageCtx(el); + + await resolver.load({ id: 'l', resource: 'loader' }, c, signal()); + + // Back to idle, not stuck loading — and therefore replannable. + expect(resolver.snapshot(c).resources.loader?.status).toBe('idle'); + expect(resolver.plan(c).map((t) => t.resource)).toEqual(['loader']); + }); + + it('surfaces a channel-defaults failure as a NOTICE, not a failed loader', async () => { + // Computing contrast stats reads pixels and can fail on a store whose metadata + // loaded fine. The image still draws with fallback channels, so it is a notice. + const resolver = new ImagesResolver({ + // A loader with omero channels forces the stats path, which we make throw. + fetchMultiscales: vi.fn(async () => [{ labels: ['c', 'y', 'x'], shape: [2, 64, 64] }]), + }); + const el = imageElement({ + attrs: { omero: { channels: [{ label: 'DAPI' }, { label: 'GFP' }] } }, + }); + const c = imageCtx(el); + + await resolver.load({ id: 'l', resource: 'loader' }, c, signal()); + + const snapshot = resolver.snapshot(c); + // The loader itself is ready — the image draws. + expect(snapshot.resources.loader?.status).toBe('ready'); + // ...and the failure is recorded, not swallowed. + expect(snapshot.notices).toEqual([ + expect.objectContaining({ kind: 'channel-defaults-fallback' }), + ]); + }); + + it('recomputes bounds when the transform changes', async () => { const resolver = new ImagesResolver({ fetchMultiscales: fetchMultiscales() }); const el = imageElement(); await resolver.load({ id: 'l', resource: 'loader' }, imageCtx(el), signal()); - const first = resolver.snapshot(imageCtx(el)); + const a = resolver.snapshot({ ...imageCtx(el), transform: new Matrix4() }); + const b = resolver.snapshot({ ...imageCtx(el), transform: new Matrix4().scale(2) }); - for (let i = 0; i < 5; i++) expect(resolver.snapshot(imageCtx(el))).toBe(first); + expect(b).not.toBe(a); }); });