Labels filtering and colouring, mirroring shapes - #95
Conversation
📝 WalkthroughWalkthroughThis PR adds shared feature-color encoding for shapes and labels, declared table column kinds, missing-value policies, label feature-state LUT rendering, and host-provided color buffers. It also threads these features through ChangesFeature color data and encoding
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Host
participant SpatialCanvasViewer
participant useLayerData
participant TableResolver
participant FeatureColorEncoding
participant LabelsLayer
Host->>SpatialCanvasViewer: provide featureColorResolver and layer configuration
SpatialCanvasViewer->>useLayerData: forward resolver and render options
useLayerData->>TableResolver: load configured label column rows
TableResolver->>useLayerData: return values and declared column kinds
useLayerData->>FeatureColorEncoding: assign colors and build label LUT
FeatureColorEncoding->>LabelsLayer: provide stable feature colors
LabelsLayer->>LabelsLayer: upload LUT and render or discard labels
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/vis/src/SpatialCanvas/resolvers/RasterResolvers.ts (1)
332-364: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winKey label fill-colour rows by layer config, not element key.
LabelsResolverstores fill-colour rows infillColorskeyed byctx.elementKey, while two labels layers may reference the same element with differentfillColorByColumn.columnNamevalues. When these layers reconcile concurrently, each task schedules a column against the shared cache entry, then the last load updates that entry. The next pass invalidates the other layer’s cached column and re-requests it, so the same entries can keep ping-ponging.
getLabelFillColorEntry()then reads this sharedlabelsResolver.getFillColorRows(key), so the per-layerlabelFillColorDatacache cannot stabilize two layers on one element with different fill columns. KeyfillColorsby the layer/entry/config identity used byLabelsLayer, or store separate column rows per element.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/vis/src/SpatialCanvas/resolvers/RasterResolvers.ts` around lines 332 - 364, Update LabelsResolver’s fillColors storage and its accessors, including getLabelFillColorEntry(), so fill-colour rows are isolated by layer/config identity or by element plus column rather than only ctx.elementKey. Ensure plan() reuses the correct cached rows for each fillColorByColumn.columnName, allowing concurrent layers sharing an element to retain separate columns without invalidating and reloading each other’s entries.
🧹 Nitpick comments (5)
packages/core/tests/tableElement.spec.ts (1)
66-67: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDo not replace the private source through
any.This assertion bypasses the
TableElementsource contract. Spy on the source class method, or add a typed test factory for the source dependency, so this test fails when the source API changes.As per coding guidelines, avoid type assertions when a narrower API contract can express the same fact.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/tests/tableElement.spec.ts` around lines 66 - 67, Update the test setup around loadObsColumnKinds to avoid assigning table.tableSource through any. Spy on the actual source class method or use a typed test factory for the source dependency, while preserving the mocked resolved column kinds and ensuring the test remains coupled to the TableElement source contract.Source: Coding guidelines
packages/core/tests/tableAssociations.spec.ts (1)
257-259: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse a narrow association-loader dependency type.
loadAssociatedTableFeatureRowsonly needs the association lookup and table-loading methods. The double assertion hides differences between this mock and the production contract.Define a narrow input interface for the methods this function consumes. Type this mock against that interface instead of asserting it is
SpatialData.As per coding guidelines, avoid type assertions when a narrower API contract can express the same fact.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/tests/tableAssociations.spec.ts` around lines 257 - 259, Define a narrow input interface for the association-loader dependency used by loadAssociatedTableFeatureRows, containing only its association lookup and table-loading methods. Type the spatialData mock in tableAssociations.spec.ts against that interface and remove the double assertion, while leaving unrelated SpatialData contract requirements out of the mock.Source: Coding guidelines
packages/layers/src/LabelsLayer.ts (1)
348-352: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace the assertion with a declaration type.
channelColors?.[0] ?? [255, 255, 255]widens the fallback literal tonumber[], which forces theas LabelRgbColor. An annotation on the declaration expresses the same fact and keeps the tuple check on the fallback.♻️ Proposed change
- const defaultColor = (channelColors?.[0] ?? [255, 255, 255]) as LabelRgbColor; + const defaultColor: LabelRgbColor = channelColors?.[0] ?? [255, 255, 255];As per coding guidelines: "Avoid type assertions (
as); usesatisfies,as const, discriminated unions, and small helpers that return precise types."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/layers/src/LabelsLayer.ts` around lines 348 - 352, Update _resolveFeatureColorLut to replace the defaultColor `as LabelRgbColor` assertion with an explicit declaration type, preserving the existing channelColors fallback and tuple validation.Source: Coding guidelines
packages/layers/tests/labelColorEncoding.spec.ts (1)
96-178: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace the repeated
as Uint8Arrayassertions with a narrowing helper.Each assertion hides the case where
buildLabelColorLutreturnsundefined. A helper narrows once and fails with a clear message instead of reading properties ofundefined.♻️ Proposed helper
+function expectLut(lut: LabelColorLut | undefined): LabelColorLut { + expect(lut).toBeDefined(); + if (!lut) throw new Error('expected a lookup table'); + return lut; +}- expect(lut?.count).toBe(4); - expect(rgbaAt(lut?.colors as Uint8Array, 3)).toEqual([10, 20, 30, 255]); - expect(rgbaAt(lut?.colors as Uint8Array, 1)).toEqual([255, 255, 255, 255]); + const table = expectLut(lut); + expect(table.count).toBe(4); + expect(rgbaAt(table.colors, 3)).toEqual([10, 20, 30, 255]); + expect(rgbaAt(table.colors, 1)).toEqual([255, 255, 255, 255]);As per coding guidelines: "Avoid type assertions (
as); usesatisfies,as const, discriminated unions, and small helpers that return precise types."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/layers/tests/labelColorEncoding.spec.ts` around lines 96 - 178, Replace the repeated `as Uint8Array` assertions in the label colour lookup tests with a small narrowing helper that accepts the result of `buildLabelColorLut`, asserts it is defined with a clear failure message, and returns its colors as `Uint8Array`. Update the affected `rgbaAt` calls to use this helper while preserving the existing expectations.Source: Coding guidelines
packages/vis/src/SpatialCanvas/types.ts (1)
186-191: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExtract the shared feature-state fields.
LabelsLayerConfig.featureStaterepeatsShapesLayerConfig.featureStateminusstrokeColorByFeatureId. A shared base type keeps both in step when a field is added, in the same wayFillColorByColumn<TMode>now shares the colour configuration.♻️ Proposed shared type
+/** Per-feature filtering and colouring, shared by shapes and labels. */ +export interface FeatureStateConfig { + fillColorByFeatureId?: Record<string, [number, number, number, number]>; + hiddenFeatureIds?: string[]; + fadedFeatureIds?: string[]; + filteredOpacityMultiplier?: number; +}- featureState?: { - fillColorByFeatureId?: Record<string, [number, number, number, number]>; - hiddenFeatureIds?: string[]; - fadedFeatureIds?: string[]; - filteredOpacityMultiplier?: number; - }; + featureState?: FeatureStateConfig;
ShapesLayerConfigthen usesFeatureStateConfig & { strokeColorByFeatureId?: [number, number, number, number] extends never ? never : Record<string, [number, number, number, number]> }, or more simply an interface that extendsFeatureStateConfigand addsstrokeColorByFeatureId.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/vis/src/SpatialCanvas/types.ts` around lines 186 - 191, Extract the common fields of LabelsLayerConfig.featureState and ShapesLayerConfig.featureState into a shared FeatureStateConfig type, including fillColorByFeatureId, hiddenFeatureIds, fadedFeatureIds, and filteredOpacityMultiplier. Update both layer configurations to reuse this type, with ShapesLayerConfig additionally defining strokeColorByFeatureId.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/core/src/tableAssociations.ts`:
- Around line 187-195: Update the column-kind loading flow around
loadObsColumnKinds so a rejected or unavailable kind lookup preserves missing
metadata instead of defaulting to an empty array. Return undefined for
extraColumnKinds, or an undefined-filled array matching extraColumns.length,
while preserving positional alignment for successfully loaded kinds.
In `@packages/core/tests/spatialLayerProps.spec.ts`:
- Around line 117-129: Rename the test describing the labels stroke override to
state that the schema strips or removes the field rather than rejects it, and
revise the adjacent comment to describe successful parsing with the unsupported
property omitted. Keep the existing safeParse success and absence assertions
unchanged.
In `@packages/layers/src/featureColorEncoding.ts`:
- Around line 56-65: Update the validation guarding the color read in the
feature color lookup function to reject counts that exceed the available RGBA
rows, using Math.floor(buffer.colors.length / 4) as the effective row limit.
Ensure indexes are also rejected when they are outside that available-row limit,
so the returned FeatureRgbaColor tuple never contains undefined components.
In `@packages/layers/src/LabelsLayer.ts`:
- Line 374: Update the texture cleanup in the LabelsLayer methods around the
feature color texture and the additional release call sites to use the correct
texture destruction API consistently with the tile layer, rather than
optional-chaining potentially incorrect destroy/delete method names. Remove the
misleading optional method shape so cleanup invokes the established release
operation and cannot silently skip GPU resource disposal.
In `@packages/layers/src/shapesLayer.ts`:
- Around line 953-985: The featureColors fallback is inconsistent between
polygon and circle geometry paths. In packages/layers/src/shapesLayer.ts lines
953-985, update createPolygonDeckLayer’s fillAt to use TRANSPARENT_RGBA when
featureColorAt returns undefined with a buffer; apply the same fallback in
createCircleDeckLayer’s getFillColor at lines 1125-1136, while preserving
defaultFillColor behavior when no buffer is supplied.
---
Outside diff comments:
In `@packages/vis/src/SpatialCanvas/resolvers/RasterResolvers.ts`:
- Around line 332-364: Update LabelsResolver’s fillColors storage and its
accessors, including getLabelFillColorEntry(), so fill-colour rows are isolated
by layer/config identity or by element plus column rather than only
ctx.elementKey. Ensure plan() reuses the correct cached rows for each
fillColorByColumn.columnName, allowing concurrent layers sharing an element to
retain separate columns without invalidating and reloading each other’s entries.
---
Nitpick comments:
In `@packages/core/tests/tableAssociations.spec.ts`:
- Around line 257-259: Define a narrow input interface for the
association-loader dependency used by loadAssociatedTableFeatureRows, containing
only its association lookup and table-loading methods. Type the spatialData mock
in tableAssociations.spec.ts against that interface and remove the double
assertion, while leaving unrelated SpatialData contract requirements out of the
mock.
In `@packages/core/tests/tableElement.spec.ts`:
- Around line 66-67: Update the test setup around loadObsColumnKinds to avoid
assigning table.tableSource through any. Spy on the actual source class method
or use a typed test factory for the source dependency, while preserving the
mocked resolved column kinds and ensuring the test remains coupled to the
TableElement source contract.
In `@packages/layers/src/LabelsLayer.ts`:
- Around line 348-352: Update _resolveFeatureColorLut to replace the
defaultColor `as LabelRgbColor` assertion with an explicit declaration type,
preserving the existing channelColors fallback and tuple validation.
In `@packages/layers/tests/labelColorEncoding.spec.ts`:
- Around line 96-178: Replace the repeated `as Uint8Array` assertions in the
label colour lookup tests with a small narrowing helper that accepts the result
of `buildLabelColorLut`, asserts it is defined with a clear failure message, and
returns its colors as `Uint8Array`. Update the affected `rgbaAt` calls to use
this helper while preserving the existing expectations.
In `@packages/vis/src/SpatialCanvas/types.ts`:
- Around line 186-191: Extract the common fields of
LabelsLayerConfig.featureState and ShapesLayerConfig.featureState into a shared
FeatureStateConfig type, including fillColorByFeatureId, hiddenFeatureIds,
fadedFeatureIds, and filteredOpacityMultiplier. Update both layer configurations
to reuse this type, with ShapesLayerConfig additionally defining
strokeColorByFeatureId.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4efa4caa-19c5-4c85-94eb-4a2f96a769f3
📒 Files selected for processing (47)
.changeset/column-kinds-and-missing-values.md.changeset/feature-color-buffer-resolver.md.changeset/feature-colour-schemes.md.changeset/labels-feature-filtering-and-colouring.md.changeset/labels-sublayer-feature-state.md.changeset/nan-is-a-missing-value.mdpackages/core/src/models/VAnnDataSource.tspackages/core/src/models/index.tspackages/core/src/spatialLayerProps.tspackages/core/src/tableAssociations.tspackages/core/src/types.tspackages/core/tests/spatialLayerProps.spec.tspackages/core/tests/tableAssociations.spec.tspackages/core/tests/tableElement.spec.tspackages/layers/src/LabelsBitmaskTileLayer.tspackages/layers/src/LabelsLayer.tspackages/layers/src/featureColorEncoding.tspackages/layers/src/index.tspackages/layers/src/labelColorEncoding.tspackages/layers/src/labelsBitmaskLayerShaders.tspackages/layers/src/shapeColorEncoding.tspackages/layers/src/shapesLayer.tspackages/layers/tests/featureColorEncoding.spec.tspackages/layers/tests/labelColorEncoding.spec.tspackages/layers/tests/labelsLayer.spec.tspackages/layers/tests/shapeColorEncoding.spec.tspackages/layers/tests/shapesLayer.spec.tspackages/vis/demo/src/App.tsxpackages/vis/demo/src/HeadlessBlobsDemo.tsxpackages/vis/demo/src/VivContrastExtension.tspackages/vis/demo/src/main.tsxpackages/vis/src/SpatialCanvas/SpatialCanvasViewer.tsxpackages/vis/src/SpatialCanvas/featureColorResolver.tspackages/vis/src/SpatialCanvas/index.tsxpackages/vis/src/SpatialCanvas/labelsProjection.tspackages/vis/src/SpatialCanvas/public.tspackages/vis/src/SpatialCanvas/renderers/labelsRenderer.tspackages/vis/src/SpatialCanvas/renderers/shapesRenderer.tspackages/vis/src/SpatialCanvas/resolvers/RasterResolvers.tspackages/vis/src/SpatialCanvas/shapesProjection.tspackages/vis/src/SpatialCanvas/types.tspackages/vis/src/SpatialCanvas/useLayerData.tspackages/vis/src/index.tspackages/vis/tests/featureColorResolver.spec.tspackages/vis/tests/labelsProjection.spec.tspackages/vis/tests/rasterResolvers.spec.tspackages/vis/tests/useLayerData.spec.tsx
A labels layer now takes `fillColorByColumn` and a `featureState` with the same field names and meanings a shapes layer's takes — `fillColorByFeatureId`, `hiddenFeatureIds`, `fadedFeatureIds`, `filteredOpacityMultiplier` — keyed by the label's integer instance id as a string, which is the identity the tooltip path already resolves against the associated table. Shapes resolve per-feature colour from a texture indexed by feature index. Labels have no geometry — a label is a raster pixel value — so the analogue is a lookup table indexed by label id: the bitmask fragment shader samples the instance-id raster and looks the integer up. Same property that matters, that a feature-state change re-uploads only the small table and never the tiles. - featureColorEncoding: the palette, numeric ramp, 'auto' detection and category assignment, now shared. shapeColorEncoding and labelColorEncoding are addressing wrappers over it, so one column reads the same way on either kind over the same table. - labelColorEncoding: the feature-state runtime and the RGBA lookup table. RGB is the resolved colour; alpha is a modulation, not an opacity — 0 discards, anything else scales the channel's fill and outline opacities, so the channel sliders stay meaningful under a filter. Labels past the table's end keep the channel colour, as an uncoloured shape keeps the layer default. - LabelsLayer owns one LUT texture and hands it to every tile sublayer; per-tile textures would multiply a table that is already megabytes for a large segmentation. Picking consults the same table, so what is hidden can never be picked. - LabelsResolver gains a fillColor resource keyed by column name (rows only — colours stay a pure projection); labelsProjection caches the LUT with a stable identity so a hover or pan does not re-upload it. Verified against the blobs fixture: labels hide, recolour and fade per id, and clearing the state returns them to the plain uniform-colour path. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`fillColorByColumn` on shapes and labels now carries the scheme, not just the
column: `categoricalPalette` ('oklab' | 'classic' | an explicit RGB list) and
`numericRamp`. Both are JSON-serializable, so a headless host can set them in a
saved Render Stack entry rather than having to precompute colours itself.
The categorical default moves to the OkLab golden-angle scheme points already
uses for colour-by-feature. The old six-colour list cycled, so a cell-type
column with more than six categories drew two categories the same colour — a
failure that is invisible in the render and therefore survives review. The
OkLab scheme is a pure function of the category index and has no length, so it
cannot repeat; 'classic' keeps the old colours for configs that want them.
Resolving a palette now returns `categoryIndex → colour` rather than an array,
which is what lets a procedural scheme and a fixed list share one code path.
The scheme is folded into both projections' cache signatures — swapping a
palette changes every colour without touching the column, so a column-only key
would have kept serving the old table.
Tests that asserted the old palette now pin `'classic'` and say why, with new
coverage for the default and for the no-repeat property past six categories.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Colouring from data the config cannot carry — a computed column, an external
annotation, a live selection — previously had one route: a
`featureId → colour` dictionary in `featureState`. That makes a host stringify
integers it already had, then costs us a Map copy and (for labels) a regex
parse per entry, all to arrive at the buffer the renderer wanted in the first
place. For a 500k-cell element that is a 500k-key object and a million regex
tests per rebuild.
So take the buffer. `FeatureColorBuffer { colors: Uint8Array; count: number }`
is now the currency both kinds render from; `LabelColorLut` IS one
(`labelCount` → `count`), and `createShapesDeckLayer` accepts one across all
three shapes paths — verbatim into `FlatPolygonLayer` on the binary path, and
by a typed-array read per feature on the object and circle paths.
A buffer wins over `featureState` rather than merging: merging would mean a
per-feature dictionary lookup again, which is the cost this removes. A host
driving colour bakes hide and fade into the alpha instead.
The seam is `featureColorResolver` on `SpatialCanvasViewer`, in the same family
as `hostLayerResolver` and `vivImagePropsResolver` — deliberately NOT the
Render Stack, whose `props` are JSON that must survive being saved. The context
tells the host what its index means: for labels the raster's own pixel value,
for shapes the position in the loaded geometry, which is the loader's decision
and so is supplied as an explicit `featureIds` ordering to build against.
Identity is the invalidation signal, and `createFeatureColorStabilizer`
collapses a fresh wrapper around unchanged bytes onto the previous one — a host
that reuses its buffer but returns a new `{colors, count}` each render would
otherwise re-upload a multi-megabyte texture every frame while looking correct.
A short buffer is padded transparent rather than read past its end.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two loose ends from the labels work. `spatialLabelsSublayerSchema` gains `featureState` with the same field names and meanings shapes already had, so the older `SpatialLayerProps` serialization surface can express per-label filtering and colouring too — the last place a labels layer could not say what a shapes layer could. It omits `strokeColorByFeatureId`, because a label's outline is derived from its fill in the shader and there is no per-label stroke to set. `SpatialLabelsSublayer` is exported alongside `SpatialShapesSublayer`. The `'classic'` categorical palette is removed. It existed to preserve the pre-scheme default, but nothing real depends on those colours, and keeping a named alias for a six-colour cycle invites exactly the repetition the OkLab default was chosen to avoid. `FeatureCategoricalPaletteSpec` is now `'oklab'` or an explicit list; tests that needed pinned colours declare their own palette inline, which also says plainly that their subject is row alignment rather than colour choice. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`'auto'` mode decides continuous vs categorical by asking whether EVERY non-empty value parses as a finite number. A non-finite number stringified to "NaN" — non-empty, does not parse — so one cell was enough to flip the verdict. The visible result was worse than a wrong ramp. A `UMAP1` column with a single failed embedding became categorical, and categorical mode gives every distinct value its own category, so half a million distinct floats became half a million hues: the layer rendered as static. One bad cell, and the whole element was noise. A non-finite `number` now normalises to '' like `null` already did, so it is excluded from the mode decision and the cell falls back to the layer default instead of being coloured as a category of its own. Kept deliberately typed rather than textual: only an actual `number` is treated this way. The STRING "NaN" in a string column is left alone, because there is no way to tell a missing float from a category spelled that way, and dropping a real category would be the same class of bug in the other direction. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two follow-ups to the NaN fix, both aimed at the same root cause: we were
inferring what a column IS from what its decoded values look like, and that
inference is not recoverable in either direction.
The loader always knew. `_loadColumn` has to distinguish an AnnData categorical
(codes plus a categories array) from a `string-array` from a plain typed array
in order to decode at all, and it threw the answer away. It now records a
`TableColumnKind` per column path, `TableElement.loadObsColumnKinds` exposes it
sharing the same per-column cache, and `loadAssociatedTableFeatureRows` carries
it beside the values. `'auto'` trusts it: numeric is a ramp, everything else is
a palette, and `bool` is two levels rather than a 0..1 gradient. Value sniffing
survives only where no kind is on offer.
Kinds are best-effort at the association boundary — a table source that cannot
report them still serves values, degrading to the same "no kind" path a column
that failed to resolve already takes.
Missing values become the caller's choice, because the part we cannot infer is
store-specific: `treatAsMissing` names the sentinel STRINGS a given pipeline
writes ('NA', 'unknown'), and `render` says whether a feature with no value
keeps the layer default, hides, or takes an explicit colour — the "grey out the
unmeasured cells" request. `null` and `NaN` stay unconditionally missing; those
are the domain's own spelling of no-value and a config claiming otherwise could
only ever be a bug.
Sentinels resolve once, before the mode decision, the numeric extent and the
category set. A sentinel that reached the category set would become a category
and consume a palette slot; one that reached the extent would drag the ramp.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… keys Six findings verified against the code and fixed; four skipped, noted below. - `loadAssociatedTableFeatureRows` returned `[]` when kinds were unavailable, which claims "no column has a kind" rather than "we do not know", and broke the positional alignment the field documents. It now returns `undefined`, and builds the array from `uniqueExtra` so its length matches `extraColumns` even if a source answers short. - `featureColorAt` bounded reads on `count` alone. `count` is a caller's claim about its own buffer, so an over-stated one returned a tuple of `undefined`s typed as a colour — malformed attribute data rather than a visible error. Now bounded by the bytes actually present. - The shapes object and circle paths fell back to `defaultFillColor` for a feature the buffer did not cover, while the binary path pads with transparent. One buffer therefore rendered differently depending on which geometry representation the element loaded as. All three are transparent now. - `LabelsLayer` disposed its LUT texture through `destroy?.()` behind a structural type with an OPTIONAL method, so a wrong shape would silently skip disposal and leak a texture nothing reports. `destroy` is now required in the type and called unconditionally. The fallback texture moves off `delete()`, which luma 9 deprecates; the pre-existing channel-texture cleanup in that file is left alone. - `LabelsResolver.fillColors` was keyed by element alone, so two layers colouring one element by different columns evicted each other on every plan — a reload ping-pong that never settles. Keyed by element AND column, with the snapshot cache keyed by column too. (The tooltip caches keep the shape `ShapesResolver`'s class doc records for Track B; not made worse here.) - A schema test named "rejects" asserted successful parsing with the field stripped. Renamed, and the comment corrected to describe what it does. Also taken: `FeatureStateConfig` extracted so both layer configs express their shared fields once, a narrowing helper in place of repeated `as Uint8Array`, and an explicit declaration type in place of an assertion. Skipped: replacing `(table as any).tableSource` and the mock double-assertion in two core specs — both match the surrounding file's established pattern, and diverging for one new test would be inconsistent rather than safer. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…type Two things the review called for. `getObsColumnKinds` replaces `loadObsColumnKinds` and is synchronous. Opening a store already reads every node's attributes and array metadata into the tree — the same tree `getObsColumnNames` has always read names from — so the `encoding-type` that distinguishes a categorical from a string array, and the dtype that distinguishes a float from a bool, are in memory before anyone asks for values. Deriving the kind during `_loadColumn` and caching it in a side map was work to recover something we already had. Being sync is not just tidier. A caller can now ask what a column IS before deciding whether to load it, which is what a "colour by" UI wants in order to offer the right affordance up front — an async accessor could not give it that at any price. `_loadColumn` returns to its previous form, and the side map, the extra await and the best-effort catch all go. The classifier reads both zarr generations, because both reach the tree: v3 spells dtypes out (`float64`, `bool`, `string`), v2 uses numpy typestrings (`<f8`, `|b1`, `|O`). Unit tests cover the classifier against a mock tree; an integration assertion covers the assumption underneath it — that the tree really carries this metadata — across all three fixture versions, so a zarrextra change that stopped populating it would fail rather than quietly degrade every column to "unknown". `LabelsLayer` types its LUT texture as luma's `Texture` instead of a local structural type, with `@luma.gl/core` added as a direct dependency. The package already depends on `@luma.gl/engine` and cannot realistically be used without luma core, so restating a GPU type locally bought nothing and could drift. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
cdcbc7c to
bc4f1df
Compare
The docs site described the shapes-only world this branch replaced. Six pages were stale in ways a reader would act on. `headless-viewer` had "Shapes: table-driven colour and filter state" as the one place the API is written down. Labels now take the same `fillColorByColumn` and the same `featureState`, so the section covers both, and says the one thing that is not symmetric: a labels feature id is the label's integer instance id as a string. The scheme fields (`categoricalPalette`, `numericRamp`, `missingValues`) had no documentation at all, and the categorical default changing to OkLab is a behaviour change a reader needs told rather than left to discover — it is called out in an admonition next to the field that controls it. `featureColorResolver` gets its own section under the runtime-attachment list, because which list it is in *is* the point: a Uint8Array cannot live in a serialisable config. `layer-prop-flow` documents the render path and its invalidation rules. Its "feature state = table column to buffer" section described the shapes texture; labels reuse that primitive with the one substitution their data forces, so the labels LUT is documented beside it, with the anti-patterns that actually bit during review — per-tile LUTs, unstable buffer identity, and a colour resource keyed by element alone. `feature-table-associations` carried a checklist whose item 2 asked for exactly what this branch implements; it is marked done for the encode path and honest that row-resolution parity is still item 1. Item 3 is annotated as partly advanced rather than done, since column kinds are the discovery metadata but direct shape annotations and matrix-backed values remain. `core/elements` gains `getObsColumnKinds()`, including why it is synchronous — that is the property a "colour by" UI depends on, not an implementation detail. `layers/overview` describes the shared encoding core rather than implying each kind has its own. `mdv-release-checklist` had labels as "render when configured", which now understates them. Verified with `pnpm build` in docs/: the site builds and every new cross-page anchor resolves. Two broken anchors remain, both pre-existing on main and in files untouched here (`#issue-56-channel--extension-apis`). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Annotated
labelselements can now be filtered and coloured through the same API asshapes, as originally asked for. The work then pulled in three things it turned out to depend on: a shared colour-encoding core, a precomputed-buffer escape hatch, and the column dtype.Labels filtering and colouring (
37fd9ea)A labels layer takes
fillColorByColumnand afeatureStatewith shapes' field names and meanings —fillColorByFeatureId/hiddenFeatureIds/fadedFeatureIds/filteredOpacityMultiplier— keyed by the label's integer instance id as a string, which is the identity the tooltip already resolves against the associated table.Shapes resolve per-feature colour from a texture indexed by feature index. A label has no geometry — it is a raster pixel value — so the analogue is a lookup table indexed by label id: the bitmask fragment shader samples the instance-id raster and looks the integer up. Same primitive, same property that matters: a feature-state change re-uploads only the small table, never the tiles.
LabelsLayerand shared across tile sublayers; per-tile textures would multiply a table that is already megabytes for a large segmentation.LabelsResolvergains afillColorresource (rows only — colours stay a pure projection, as for shapes).Verified in the browser against the blobs fixture: labels hide, recolour and fade per id, and clearing the state returns them to the plain uniform-colour path.
Colour schemes, OkLab default (
b150e86) — behaviour changefillColorByColumnnow carries the scheme, not just the column:categoricalPalette('oklab'or your own RGB list) andnumericRamp, both JSON-serializable so they survive a saved Render Stack.The categorical default changes. It was a six-colour list that cycled, so a cell-type column with more than six categories drew two categories the same colour — a failure invisible in the render. The default is now the unbounded golden-angle OkLab scheme
@spatialdata/layersalready used for points, which is a pure function of the category index and cannot repeat. Pass an explicit list to pin specific colours.Precomputed colour buffers (
de1738b)For colour driven by data a config cannot carry — a computed column, an external annotation, a live selection — the only route was a
featureId → colourdictionary. That makes a host stringify integers it already had, then costs a Map copy and (for labels) a parse per entry, to arrive at the buffer the renderer wanted anyway.FeatureColorBuffer { colors: Uint8Array; count: number }is now the currency both kinds render from, supplied via afeatureColorResolverprop onSpatialCanvasViewer— a runtime attachment alongsidehostLayerResolver/vivImagePropsResolver, deliberately not the Render Stack, whose props are JSON that must survive being saved.The resolver context says what the index means: for labels the raster's own pixel value; for shapes the position in the loaded geometry, which is the loader's decision, so the ordering is supplied explicitly to build against. A buffer wins over
featureStaterather than merging — merging would reinstate the per-feature lookup this removes.Schema symmetry (
ddab672)spatialLabelsSublayerSchemagainsfeatureState, so the olderSpatialLayerPropssurface can express per-label filtering too. It omitsstrokeColorByFeatureId: a label's outline is derived from its fill in the shader. The now-unreachable'classic'palette name goes with it.Colouring correctness (
bc2104e,a75b283)Two defects in how a column was read, both from inferring what a column is from what its decoded values look like.
NaNstringified to"NaN"— non-empty and unparseable — so a single failed embedding made a wholeUMAP1column categorical, and categorical mode then gave every distinct float its own colour. The layer rendered as static. A non-finite number now normalises as missing, the waynullalready did.More fundamentally, the loader already had to distinguish an AnnData categorical from a
string-arrayfrom a typed array in order to decode, and threw the answer away. It now reports aTableColumnKind, andloadAssociatedTableFeatureRowscarries it beside the values.'auto'trusts it — which also fixes the mirror-image bug where integer cluster codes read as a continuum. Value sniffing remains only where no kind is on offer, and kinds are best-effort so a source that cannot report them still serves values.Missing-value handling becomes configurable for the part that genuinely cannot be inferred:
missingValues.treatAsMissingnames the sentinel strings a given pipeline writes ('NA','unknown'), andmissingValues.renderchooses whether a feature with no value keeps the layer default, hides, or takes an explicit colour.nullandNaNstay unconditionally missing. Sentinels resolve before the mode decision, the numeric extent and the category set, so one can never become a category or drag a ramp.Review follow-ups (
e07dcab,bc4f1df)Findings from review, verified against the code and fixed.
Correctness, in
e07dcab:loadAssociatedTableFeatureRowsreturned[]when kinds were unavailable, claiming "no column has a kind" rather than "we do not know", and breaking the positional alignment the field documents — it returnsundefinednow, and builds the array so its length matchesextraColumnseven if a source answers short.featureColorAtbounded reads oncount, a caller's claim about its own buffer, so an over-stated one returned a tuple ofundefineds typed as a colour; it is bounded by the bytes actually present. The shapes object and circle paths fell back todefaultFillColorfor a feature the buffer did not cover while the binary path padded transparent, so one buffer rendered differently depending on which geometry representation the element loaded as — all three are transparent now.LabelsLayerdisposed its LUT texture through an optionaldestroy?.(), so a wrong shape would silently leak a texture nothing reports;destroyis required and called unconditionally.LabelsResolver.fillColorswas keyed by element alone, so two layers colouring one element by different columns evicted each other on every plan — a reload ping-pong that never settled; it is keyed by element and column.Then
bc4f1dfreplacesloadObsColumnKindswith a synchronousgetObsColumnKinds. Opening a store already reads every node's attributes and array metadata into the tree — the same treegetObsColumnNamesreads names from — so theencoding-typethat distinguishes a categorical from a string array, and the dtype that distinguishes a float from a bool, are in memory before anyone asks for values. Deriving the kind during_loadColumnand caching it in a side map was work to recover something we already had. Being sync also lets a caller ask what a column is before deciding whether to load it, which is what a "colour by" UI wants in order to offer the right affordance up front — an async accessor could not give it that at any price. The classifier reads both zarr generations (v3 spells dtypes out, v2 uses numpy typestrings), with an integration assertion across all three fixture versions covering the assumption underneath it, so a zarrextra change that stopped populating the tree would fail rather than quietly degrade every column to "unknown".LabelsLayeralso types its LUT texture as luma'sTexturerather than a local structural type that could drift.Docs (
0fd5ab8)The docs site described the shapes-only world this branch replaces.
headless-vieweris where the API is written down, so its colour/filter section now covers both kinds, documents the scheme fields (categoricalPalette,numericRamp,missingValues) that had no coverage at all, and calls out the OkLab default change in an admonition next to the field that controls it.featureColorResolveris documented under the runtime-attachment list, because which list it is in is the point.layer-prop-flowgains the labels LUT beside the shapes colour texture it mirrors, with the anti-patterns that actually bit during review.feature-table-associationshad a checklist whose item 2 asked for exactly this work — marked done for the encode path, honest that row-resolution parity is still item 1.core/elementsgainsgetObsColumnKinds()and why it is synchronous;layers/overviewdescribes the shared encoding core;mdv-release-checklisthad labels as "render when configured", which now understates them.The site builds and every new cross-page anchor resolves. Two broken anchors remain, both pre-existing on
mainand in files untouched here.Testing
Full workspace build, both lint gates (
lint:biome,lint:react) clean, and the whole suite green: 365 core, 206 layers, 119 vis, 38 avivatorish, 7 react, plus 25 root integration tests. New coverage for the label colour LUT, the LUT prop flow, the vis projections, the buffer resolver and its identity contract, the column-kind classifier over both zarr generations, and the missing-value policy.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
NaNand other non-finite numeric values during color assignment.