Points feature filter (MVP step 2) + responsive off-thread loads

[xinaesthete]

Points feature filter (MVP step 2) + responsive off-thread loads

Follow-up to #80. Adds the Points feature filter on the @spatialdata/layers
PointsLayer composite and makes the geometry / catalog / codes loading
off-thread, so large transcripts layers stay responsive while loading,
filtering, and colouring. points-feature-filter → main.

The PR grew well past the original MVP-step-2 scope (the earlier "known
limitations" — whole-dataset scan, full catalog, memory-cap UI — are now all
done). It's at a mergeable checkpoint: correct, honest, and non-regressed,
with a deliberate stopping line before a larger redesign. Known rough edges are
inventoried in docs/plans/points-redesign-punchlist.md
rather than patched here (see Deferred below).

What's in it

• PointsDataEngine owns the feature catalog + per-row codes — a React-free,
  headless-tested cache in @spatialdata/layers with subscribe/notify.
• Off-thread geometry + features decode — decode runs in the points worker;
  the main thread only does async range-read fetches. This is what keeps loading
  and filtering responsive. Silent/misconfigured workers time out to a main-thread
  fallback instead of hanging.
• Whole-dataset feature-index scan — when a selection is active, scan the
  whole dataset for matching feature codes (parquet footer stats skip the
  row groups a selected feature can't live in) and load only matching rows up to
  the memory cap. So genes outside the resident preload window still render.
  Includes a Thrift-Compact footer-stats parser for row-group column min/max.
• Full-dataset catalog with counts — the panel lists all features (with
  counts), not just the resident preview; the instant resident subset is
  superseded by the full scan when it settles.
• Dictionary-only (wild-type) support — transcripts stores with a feature_name
  dict column and no *_codes column are matched by resolving each row's name
  against one authoritative code space (the full catalog).
• Colour-by-feature — per-point feature codes fed to deck as binary
  attributes; a GPU shader extension colours by an OKLCh golden-angle palette
  (single source for the L/C constants, JS swatch mirror in lockstep).
• Per-layer memory cap UI — pointsMemoryCap (1M–16M, default 4M); lowering
  sheds resident rows in memory (no refetch), raising reloads only to grow a
  truncated batch; an AbortController per load cancels superseded fetches.
• Reactive feature-state hook — usePointsFeatureState (one
  useSyncExternalStore over the engine) replaces the prop-drilled getters; the
  'use no memo' escape hatch is now scoped to two leaf panels instead of the
  whole canvas shell.
• Progressive matched-scan render — the scan streams matched chunks into a
  growing buffer that renders as an overlay while it loads (spike quality — see
  Deferred).

Verified

• Type-checks clean across core / layers / vis; tests core 146 · layers
  120 · vis 50.
• Live on a real Xenium transcripts_feature_then_morton layer: load + toggles
  stay responsive (worker path; prior main-thread path froze the tab); the 541-
  feature catalog appears and filters correctly; selecting a non-resident gene
  (e.g. MALL, 646k pts) loads and renders via the scan; colour matches the panel
  swatches.

On the vendored parquet-wasm

All browser parquet decoding uses the Vitessce CDN build of parquet-wasm
(Mark Keller's fork), copied into packages/core/vendor/parquet-wasm/ rather than
loaded from cdn.vitessce.io at runtime. That build exposes readMetadata /
readParquetRowGroup (the row-group APIs from
kylebarron/parquet-wasm#804)
which parquet-wasm@0.6.1 on npm lacks. docs/parquet-wasm-limitations.md
records the details. Key limitation: no per-column-chunk offsets, so we can't
do a projected fetch — we fetch all columns' bytes (async) and decode only the
projected columns off-thread. (readParquetRowGroup also mis-decodes dictionary
columns like feature_name, so those decode via whole parts using readParquet.)

Deferred — see the punch-list

Rather than keep patching, the remaining issues are tracked in
docs/plans/points-redesign-punchlist.md,
split fix-before-merge (done in this PR: F1–F4) vs defer-to-redesign. The deferred
set is mostly one root cause: PointsEntry is an imperative mutable record
mutated with side effects mid-flight, read through the monolithic useLayerData —
so "which points to show" and the stats are decided ad-hoc. The planned follow-up
(break up useLayerData, spike Effect / TanStack Query) fixes the class.
Highlights deferred there:

• Progressive-overlay visibility + flashing (D10) — the overlay still has
  visibility-logic problems and flashes, because each chunk rebuilds the render
  resource (deck recreates the layer per step); the real fix is a stable growing
  GPU buffer (preallocate to cap, append, bump a draw count).
• Progressive initial load (D3), progressive/active stats (D4),
  tiled/Morton viewport-driven loading (D5), multi-layer worker contention
  (D6), GeoArrow (D7), streaming cancellation (D8), removing the
  'use no memo' hatches (D9), and a precise per-selection matched count (F3 now
  reports the honest in-memory batch size instead).

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features
  • Added feature-based filtering for point layers, including progressive loading and filtering status.
  • Added optional categorical coloring by feature.
  • Added controls for point memory limits and clearer truncation/loading indicators.
  • Point data can now load and decode off the main thread for improved responsiveness.
• Bug Fixes
  • Preserved feature metadata when filtering, capping, and rendering point data.
  • Added safeguards so stalled background loading requests time out and recover gracefully.
• Documentation
  • Documented current Parquet and point-loading limitations, performance findings, and future improvements.

[coderabbitai]

Warning

Review limit reached

@xinaesthete, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 40 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: da8174d6-7710-442d-9e7c-74d51246382c

📥 Commits

Reviewing files that changed from the base of the PR and between ee58000 and ed87b18.

📒 Files selected for processing (1)

• docs/plans/points-redesign-punchlist.md

📝 Walkthrough

Walkthrough

Changes

This change adds feature-code-aware point loading, filtering, worker decoding, memory-cap management, GPU coloring, and SpatialCanvas controls. It also adds Parquet footer-stat parsing, worker timeout handling, tests, and documentation of parquet-wasm limitations.

Points data and workers

Layer / File(s)	Summary
Core feature loading and filtering packages/core/src/..., packages/core/tests/...	Point batches and load results now carry aligned feature codes and catalogs, with row-group statistics used to skip impossible feature groups.
Worker decoding and request lifecycle packages/core/src/workers/..., packages/core/tests/pointsWorker*	Workers decode geometry with feature metadata, return feature codes from scans, support dict-only mappings, transfer code buffers, and enforce request timeouts.
Memory-cap and feature-selection engine packages/layers/src/engine/..., packages/layers/tests/pointsDataEngine.spec.ts	PointsDataEngine tracks capped resident data, catalogs, row codes, matching scans, progressive results, cancellation, reuse, and code-space reconciliation.
Feature-coded rendering packages/layers/src/points*.ts, packages/layers/tests/pointsFeatureColor*.spec.ts, packages/layers/tests/pointsRenderAttributes.spec.ts	Rendering now builds memoized binary attributes, preserves feature codes through filtering and caps, and applies deterministic CPU/GPU feature colors.
SpatialCanvas integration packages/vis/src/SpatialCanvas/..., packages/vis/tests/pointsFeatureRowState.spec.ts	SpatialCanvas exposes feature state, memory-cap controls, feature filtering, matching-load status, row diagnostics, and worker-backed point rendering.
Parquet capability and roadmap documentation docs/...	Documents parquet-wasm metadata and decoding limitations, a worker catalog performance finding, and the points redesign punch list.

Estimated code review effort: 5 (Critical) | ~120 minutes

Sequence Diagram(s)

sequenceDiagram
  participant SpatialCanvas
  participant PointsDataEngine
  participant VPointsSource
  participant pointsWorkerClient
  participant points-worker
  SpatialCanvas->>PointsDataEngine: ensureMatchingFeaturesLoaded
  PointsDataEngine->>VPointsSource: loadPointsMatchingFeatureCodesByChunk
  VPointsSource->>pointsWorkerClient: scanParquetByFeatureCodesInWorker
  pointsWorkerClient->>points-worker: feature-code scan request
  points-worker-->>pointsWorkerClient: matched geometry, featureCodes, progress
  pointsWorkerClient-->>VPointsSource: scan chunk
  VPointsSource-->>PointsDataEngine: progressive partialResult
  PointsDataEngine-->>SpatialCanvas: matching resource and load state

Loading

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 29.49% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title is concise and accurately captures the main changes: points feature filtering and off-thread loading.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch points-feature-filter

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 5

🧹 Nitpick comments (9)

packages/vis/src/SpatialCanvas/useLayerData.ts (1)

559-559: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Leftover editing artifact.

//--- to be removed from here? reads as an unresolved note left in the source. Resolve or drop it before merge so the intent of the following engine block isn't ambiguous. Want me to open a tracking issue?

🤖 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/useLayerData.ts` at line 559, Remove the
leftover `//--- to be removed from here?` comment near the engine block in
`useLayerData`, or resolve it by replacing it with an intentional, descriptive
comment if clarification is required.

packages/layers/src/pointsScatterLayer.ts (1)

83-95: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Drop the as Float32Array by narrowing through a local.

colorByFeature already implies attributes.featureCodes !== undefined, but TS can't carry that through the derived boolean, so the assertion papers over it. Capturing the value in a local lets the compiler narrow it in the branch and removes the cast.

♻️ Suggested narrowing

-  const colorByFeature = props.colorByFeature !== false && attributes.featureCodes !== undefined;
+  const featureCodes = attributes.featureCodes;
+  const colorByFeature = props.colorByFeature !== false && featureCodes !== undefined;
@@
-        ...(colorByFeature
-          ? { getFeatureCode: { value: attributes.featureCodes as Float32Array, size: 1 } }
-          : {}),
+        ...(colorByFeature
+          ? { getFeatureCode: { value: featureCodes, size: 1 } }
+          : {}),

As per coding guidelines: "Avoid type assertions (as); use satisfies, 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/pointsScatterLayer.ts` around lines 83 - 95, Remove the
Float32Array assertion in the ScatterplotLayer construction by storing
attributes.featureCodes in a local variable and narrowing that value alongside
the colorByFeature condition. Update the conditional getFeatureCode attribute to
use the narrowed local directly, preserving the existing behavior without type
assertions.

Source: Coding guidelines

packages/layers/src/pointsFeatureColorExtension.ts (1)

69-71: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Add explanatory comments for deck.gl boundary type assertions.

Per the coding guidelines, type assertions at external boundaries should include a short comment explaining why the compiler cannot prove the type. The three assertions here are at deck.gl's extension API boundary and are likely unavoidable, but the comments don't satisfy the guideline:

• Line 71: The comment explains the runtime null/absence guard but not why the return type of super.getShaders() doesn't expose modules.
• Line 129: No comment explaining why highlightFeatureCode isn't on this.props (deck.gl's Layer type doesn't reflect custom defaultProps in TypeScript).
• Line 131: No comment explaining why setShaderModuleProps requires a double assertion (not in deck.gl's public type surface).

📝 Suggested comments

     // The base returns null, and the module list may be absent — guard both.
-    const shaders = (super.getShaders(extension) ?? {}) as { modules?: unknown[] };
+    // deck.gl's LayerExtension.getShaders return type doesn't expose `modules`
+    // as optional, so we assert the shape we know the runtime provides.
+    const shaders = (super.getShaders(extension) ?? {}) as { modules?: unknown[] };

   draw(this: Layer): void {
-    const highlight = (this.props as { highlightFeatureCode?: number }).highlightFeatureCode ?? -1;
+    // deck.gl's Layer props type doesn't reflect custom defaultProps in TS;
+    // assert the narrow shape we declared in defaultProps above.
+    const highlight = (this.props as { highlightFeatureCode?: number }).highlightFeatureCode ?? -1;

-    (this as unknown as { setShaderModuleProps(props: unknown): void }).setShaderModuleProps({
+    // setShaderModuleProps is a protected Layer method not in deck.gl's public
+    // type declarations; double-cast to reach it.
+    (this as unknown as { setShaderModuleProps(props: unknown): void }).setShaderModuleProps({

Also applies to: 128-134

🤖 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/pointsFeatureColorExtension.ts` around lines 69 - 71,
Update the assertions in getShaders and the related shader-property handling
around highlightFeatureCode and setShaderModuleProps with concise comments
explaining the deck.gl type-boundary limitations: super.getShaders() does not
expose modules despite the runtime shape, Layer.props omits custom defaultProps
such as highlightFeatureCode, and setShaderModuleProps is unavailable in
deck.gl’s public type surface, requiring the double assertion.

Source: Coding guidelines

packages/vis/src/SpatialCanvas/PointsFeatureFilterPanel.tsx (1)

287-295: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add an accessible label to the search input.

The search <input> at line 288 has only a placeholder, which is not a substitute for an accessible label. Screen readers may not announce the field's purpose. Adding aria-label="Search features" is a one-line fix.

♿ Proposed fix

       <input
         type="search"
         placeholder="Search features…"
         value={searchQuery}
         onChange={(event) => setSearchQuery(event.target.value)}
+        aria-label="Search features"
         style={searchStyle}
       />

🤖 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/PointsFeatureFilterPanel.tsx` around lines 287
- 295, Add an accessible label to the search input rendered by the showSearch
branch in PointsFeatureFilterPanel by adding aria-label="Search features"
alongside its existing type, placeholder, value, and onChange props.

packages/vis/src/SpatialCanvas/PointsLayerPanel.tsx (1)

71-99: 📐 Maintainability & Code Quality | 🔵 Trivial

Offer to help: t.loaded display semantics are known to be misleading.

The NOTE at lines 79–81 correctly flags that t.loaded is the covered-batch size, not the count matching the current selection. This means the "Showing X of Y" message can overstate what's actually rendered for a subset selection. Would you like me to open an issue or draft a fix that threads the actual matching count through getActiveTruncation?

🤖 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/PointsLayerPanel.tsx` around lines 71 - 99,
Update ShowMatchingPoints and the engine’s getActiveTruncation filtered branch
to expose the actual count of points matching the current selection, rather than
the covered-batch size in t.loaded; use that matching count for the rendered
“Showing” and “Loaded all” messages while preserving total and truncation
behavior, and remove the obsolete NOTE.

packages/layers/src/engine/PointsDataEngine.ts (1)

234-251: 📐 Maintainability & Code Quality | 🔵 Trivial

Type assertion in sliceArray would benefit from a short justifying comment.

The as unknown as { slice?: ... } cast is local and boundary-motivated (duck-typing an ArrayLike<number> that may or may not have a native .slice), but per guidelines it should carry a brief comment explaining why the compiler can't prove this instead of relying on the reader to infer it.

As per coding guidelines: "if an assertion is unavoidable at an external boundary, keep it local and add a short comment explaining why the compiler cannot prove it."

🤖 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/engine/PointsDataEngine.ts` around lines 234 - 251, The
local double assertion in sliceResidentBatch’s sliceArray helper lacks
justification. Add a brief comment immediately before the cast explaining that
ArrayLike<number> may optionally provide a native slice method, which TypeScript
cannot express or verify, while keeping the duck-typed assertion local.

Source: Coding guidelines

packages/core/src/pointsLimits.ts (1)

55-72: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Undocumented type assertions in capFeatureCodes.

Both the subarray cast and the slice.call(...) as ArrayLike<number> are reasonable here (no TS type captures "ArrayBufferView with subarray"), but per the coding guideline these boundary assertions should carry a short comment explaining why the compiler can't prove it.

♻️ Optional: document the assertions

   if (ArrayBuffer.isView(featureCodes) && 'subarray' in featureCodes) {
+    // TS's ArrayBufferView type doesn't expose `subarray` (only typed arrays
+    // do, not DataView), so the runtime `in` check can't be reflected in the
+    // type system without this local assertion.
     return (featureCodes as { subarray(begin: number, end: number): ArrayLike<number> }).subarray(
       0,
       count
     );
   }

As per coding guidelines: "if an assertion is unavoidable at an external boundary, keep it local and add a short comment explaining why the compiler cannot prove it."

🤖 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/src/pointsLimits.ts` around lines 55 - 72, Add brief comments
directly above both type assertions in capFeatureCodes, explaining that
TypeScript cannot express the narrowed ArrayBufferView with subarray and cannot
infer the ArrayLike<number> result of Array.prototype.slice.call; keep the
assertions local and unchanged.

Source: Coding guidelines

packages/core/src/models/VPointsSource.ts (1)

359-452: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Avoidable type assertion for featureKey.

configuredFeatureKey as string at line 367 can't be proven by the compiler because the narrowing check lives in a separate wantFeatures boolean rather than the ternary condition itself. Inlining the check removes the assertion entirely.

♻️ Proposed fix

-    const configuredFeatureKey = zattrs.spatialdata_attrs?.feature_key;
-    const wantFeatures =
-      options.includeFeatureCodes === true &&
-      typeof configuredFeatureKey === 'string' &&
-      configuredFeatureKey.length > 0;
-    const featureKey = wantFeatures ? (configuredFeatureKey as string) : undefined;
+    const configuredFeatureKey = zattrs.spatialdata_attrs?.feature_key;
+    const featureKey =
+      options.includeFeatureCodes === true &&
+      typeof configuredFeatureKey === 'string' &&
+      configuredFeatureKey.length > 0
+        ? configuredFeatureKey
+        : undefined;

Rest of the reworked preload/fallback logic (worker path, abort guard, catch handling) looks correct.

As per coding guidelines: "Avoid type assertions (as ...) in TypeScript when a library overload, local type guard... 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/src/models/VPointsSource.ts` around lines 359 - 452, Remove the
unnecessary configuredFeatureKey type assertion in the featureKey
initialization. Inline the includeFeatureCodes and nonempty-string checks
directly in the ternary so TypeScript narrows configuredFeatureKey to string,
while preserving the existing wantFeatures behavior used by the surrounding
preload logic.

Source: Coding guidelines

packages/core/src/models/VTableSource.ts (1)

438-438: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Cache loadParquetDatasetMetadata to eliminate the repeated 404 probing.

The comment correctly identifies the symptom (points.parquet/points.4.parquet 404s), but the root cause is that loadParquetDatasetMetadata itself isn't memoized — every call re-runs part discovery (loadParquetPartMetadata looped until a 404) from scratch. Given this PR adds many new call sites into points loading (loadPoints, loadPointsMatchingFeatureCodes, listPointsFeatures, tiling metadata, feature counts, etc.), the redundant round-trips compound. The class already has a precedent for this pattern (parquetTableCache: Record<string, Promise<ArrowTable>>).

Want me to add a Record<string, Promise<ParquetDatasetMetadata | null>> cache keyed by parquetPath (analogous to parquetTableCache), or open an issue to track it?

🤖 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/src/models/VTableSource.ts` at line 438, Memoize
loadParquetDatasetMetadata using a class-level Record<string,
Promise<ParquetDatasetMetadata | null>> keyed by parquetPath, following the
existing parquetTableCache pattern. Store and reuse the in-flight promise so
concurrent callers share one part-discovery sequence, including repeated 404
probing, while preserving the existing null/error behavior.

🤖 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 `@docs/parquet-wasm-limitations.md`:
- Around line 78-83: Revise the limitation describing column-chunk offsets to
clarify that the normalized wrapper exposes these offsets as a prerequisite for
a future sparse reader, but offsets alone do not enable projected fetching.
Retain that readParquetRowGroup currently requires contiguous row-group bytes
and that concatenating selected chunks invalidates footer offsets; apply the
same clarification to the corresponding repeated section.

In `@docs/plans/points-mvp-and-roadmap.md`:
- Around line 136-149: Update the roadmap section’s header/status to indicate
that implementation is in progress, or explicitly clarify that “not started”
applies only to the roadmap plan; ensure it no longer contradicts the documented
current worker behavior and ongoing performance work.

In `@packages/layers/src/engine/PointsDataEngine.ts`:
- Around line 694-709: Move the JSDoc describing resident distinct feature codes
from above hasFeatureCodeColumn to directly above getResidentFeatureCodes,
preserving its content. Keep the existing hasFeatureCodeColumn documentation
immediately above that method and ensure each comment documents only its
corresponding method.
- Around line 30-36: Update ensureRowFeatureCodes to accept a memory-cap
argument and pass entry.memoryCap from the fallback call site, ensuring it uses
the same resident window as ensureLoaded. Locate the relevant invocation in the
PointsDataEngine loading path and thread the value through all affected helper
signatures and calls.

In `@packages/vis/src/SpatialCanvas/featureRowState.ts`:
- Around line 89-111: Update the feature-row state logic so the selected,
non-scanning, non-resident, non-rendered, on-demand-load case does not use the
misleading “select it to fetch” reason. In the function containing the
selected/scanning and notLoaded branches, add a selected-specific pending-scan
branch with loading or equivalent pending tone/label and wording that indicates
the feature is already selected and its feature-index scan is about to begin;
keep the existing notLoaded message only for unselected features.

---

Nitpick comments:
In `@packages/core/src/models/VPointsSource.ts`:
- Around line 359-452: Remove the unnecessary configuredFeatureKey type
assertion in the featureKey initialization. Inline the includeFeatureCodes and
nonempty-string checks directly in the ternary so TypeScript narrows
configuredFeatureKey to string, while preserving the existing wantFeatures
behavior used by the surrounding preload logic.

In `@packages/core/src/models/VTableSource.ts`:
- Line 438: Memoize loadParquetDatasetMetadata using a class-level
Record<string, Promise<ParquetDatasetMetadata | null>> keyed by parquetPath,
following the existing parquetTableCache pattern. Store and reuse the in-flight
promise so concurrent callers share one part-discovery sequence, including
repeated 404 probing, while preserving the existing null/error behavior.

In `@packages/core/src/pointsLimits.ts`:
- Around line 55-72: Add brief comments directly above both type assertions in
capFeatureCodes, explaining that TypeScript cannot express the narrowed
ArrayBufferView with subarray and cannot infer the ArrayLike<number> result of
Array.prototype.slice.call; keep the assertions local and unchanged.

In `@packages/layers/src/engine/PointsDataEngine.ts`:
- Around line 234-251: The local double assertion in sliceResidentBatch’s
sliceArray helper lacks justification. Add a brief comment immediately before
the cast explaining that ArrayLike<number> may optionally provide a native slice
method, which TypeScript cannot express or verify, while keeping the duck-typed
assertion local.

In `@packages/layers/src/pointsFeatureColorExtension.ts`:
- Around line 69-71: Update the assertions in getShaders and the related
shader-property handling around highlightFeatureCode and setShaderModuleProps
with concise comments explaining the deck.gl type-boundary limitations:
super.getShaders() does not expose modules despite the runtime shape,
Layer.props omits custom defaultProps such as highlightFeatureCode, and
setShaderModuleProps is unavailable in deck.gl’s public type surface, requiring
the double assertion.

In `@packages/layers/src/pointsScatterLayer.ts`:
- Around line 83-95: Remove the Float32Array assertion in the ScatterplotLayer
construction by storing attributes.featureCodes in a local variable and
narrowing that value alongside the colorByFeature condition. Update the
conditional getFeatureCode attribute to use the narrowed local directly,
preserving the existing behavior without type assertions.

In `@packages/vis/src/SpatialCanvas/PointsFeatureFilterPanel.tsx`:
- Around line 287-295: Add an accessible label to the search input rendered by
the showSearch branch in PointsFeatureFilterPanel by adding aria-label="Search
features" alongside its existing type, placeholder, value, and onChange props.

In `@packages/vis/src/SpatialCanvas/PointsLayerPanel.tsx`:
- Around line 71-99: Update ShowMatchingPoints and the engine’s
getActiveTruncation filtered branch to expose the actual count of points
matching the current selection, rather than the covered-batch size in t.loaded;
use that matching count for the rendered “Showing” and “Loaded all” messages
while preserving total and truncation behavior, and remove the obsolete NOTE.

In `@packages/vis/src/SpatialCanvas/useLayerData.ts`:
- Line 559: Remove the leftover `//--- to be removed from here?` comment near
the engine block in `useLayerData`, or resolve it by replacing it with an
intentional, descriptive comment if clarification is required.

🪄 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

Run ID: 840748e6-c2f7-4cdc-a99a-062385e13dd7

📥 Commits

Reviewing files that changed from the base of the PR and between ab1b809 and e2de7d2.

📒 Files selected for processing (50)

• docs/parquet-wasm-limitations.md
• docs/plans/points-mvp-and-roadmap.md
• packages/core/src/index.ts
• packages/core/src/models/VPointsSource.ts
• packages/core/src/models/VTableSource.ts
• packages/core/src/models/index.ts
• packages/core/src/parquetFooterStats.ts
• packages/core/src/pointsFeatures.ts
• packages/core/src/pointsLimits.ts
• packages/core/src/pointsLoadOptions.ts
• packages/core/src/pointsLoader.ts
• packages/core/src/pointsTiling.ts
• packages/core/src/spatialViewFit.ts
• packages/core/src/workers/index.ts
• packages/core/src/workers/points-worker.ts
• packages/core/src/workers/pointsWorkerClient.ts
• packages/core/src/workers/pointsWorkerProtocol.ts
• packages/core/src/workers/pointsWorkerScan.ts
• packages/core/tests/parquetFooterStats.spec.ts
• packages/core/tests/pointsFeatures.spec.ts
• packages/core/tests/pointsLimits.spec.ts
• packages/core/tests/pointsTiling.spec.ts
• packages/core/tests/pointsWorker.spec.ts
• packages/core/tests/pointsWorkerScan.spec.ts
• packages/core/tests/remapRowFeatureCodes.spec.ts
• packages/layers/src/PointsLayer.ts
• packages/layers/src/engine/PointsDataEngine.ts
• packages/layers/src/index.ts
• packages/layers/src/pointsFeatureColor.ts
• packages/layers/src/pointsFeatureColorExtension.ts
• packages/layers/src/pointsLoader.ts
• packages/layers/src/pointsRenderAttributes.ts
• packages/layers/src/pointsScatterLayer.ts
• packages/layers/src/preloadedScatterStrategy.ts
• packages/layers/src/resolvePointsRenderResource.ts
• packages/layers/tests/pointsDataEngine.spec.ts
• packages/layers/tests/pointsFeatureColor.spec.ts
• packages/layers/tests/pointsFeatureColorExtension.spec.ts
• packages/layers/tests/pointsRenderAttributes.spec.ts
• packages/layers/tsconfig.json
• packages/vis/demo/src/main.tsx
• packages/vis/src/SpatialCanvas/PointsFeatureFilterPanel.tsx
• packages/vis/src/SpatialCanvas/PointsFeatureState.tsx
• packages/vis/src/SpatialCanvas/PointsLayerPanel.tsx
• packages/vis/src/SpatialCanvas/featureRowState.ts
• packages/vis/src/SpatialCanvas/index.tsx
• packages/vis/src/SpatialCanvas/public.ts
• packages/vis/src/SpatialCanvas/types.ts
• packages/vis/src/SpatialCanvas/useLayerData.ts
• packages/vis/tests/pointsFeatureRowState.spec.ts

[coderabbitai]

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Clarify that column offsets alone are not sufficient for projected fetching.

The runtime probe above says column offsets are already available, yet the current decoder still requires contiguous row-group bytes. Reword this item as exposing offsets through the normalized wrapper—a prerequisite for a future sparse reader, not something that independently unlocks projected fetching.

Suggested wording

-1. **Column-chunk offsets in the metadata** — expose `ColumnChunkMetaData`
+1. **Column-chunk offsets in the normalized metadata** — expose `ColumnChunkMetaData`
   (`file_offset`, `total_compressed_size`, `data_page_offset`,
-  `dictionary_page_offset`) so we can compute per-column byte ranges. This alone
-  unlocks projected fetching.
+  `dictionary_page_offset`) so callers can compute per-column byte ranges.
+  This is a prerequisite for projected fetching, but still requires a decoder
+  that accepts sparse column-chunk buffers.

Also applies to: 100-107

🤖 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 `@docs/parquet-wasm-limitations.md` around lines 78 - 83, Revise the limitation
describing column-chunk offsets to clarify that the normalized wrapper exposes
these offsets as a prerequisite for a future sparse reader, but offsets alone do
not enable projected fetching. Retain that readParquetRowGroup currently
requires contiguous row-group bytes and that concatenating selected chunks
invalidates footer offsets; apply the same clarification to the corresponding
repeated section.

[coderabbitai]

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Update the roadmap status now that implementation work is underway.

This section documents live behavior from the current worker implementation, which conflicts with the header’s “implementation not started” status. Change the status to “in progress” or explicitly scope that status to the roadmap plan rather than the implementation.

🤖 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 `@docs/plans/points-mvp-and-roadmap.md` around lines 136 - 149, Update the
roadmap section’s header/status to indicate that implementation is in progress,
or explicitly clarify that “not started” applies only to the roadmap plan;
ensure it no longer contradicts the documented current worker behavior and
ongoing performance work.

[coderabbitai]

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Locate relevant files
git ls-files 'packages/layers/src/engine/PointsDataEngine.ts' 'packages/core/src/models/VPointsSource.ts' 'packages/core/src/**' | sed -n '1,200p'

echo '--- PointsDataEngine outline ---'
ast-grep outline packages/layers/src/engine/PointsDataEngine.ts --view expanded || true

echo '--- VPointsSource outline ---'
ast-grep outline packages/core/src/models/VPointsSource.ts --view expanded || true

echo '--- Search for loadRowFeatureCodes/loadPointsRowFeatureCodes/ensureLoaded/memoryCap ---'
rg -n "loadRowFeatureCodes|loadPointsRowFeatureCodes|ensureLoaded|memoryCap" packages/layers/src/engine/PointsDataEngine.ts packages/core/src/models/VPointsSource.ts packages/core/src -g '!**/dist/**' -g '!**/build/**' || true

Repository: Taylor-CCB-Group/SpatialData.js

Length of output: 15036

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Read the relevant sections with line numbers
echo '--- PointsDataEngine.ts (selected lines) ---'
nl -ba packages/layers/src/engine/PointsDataEngine.ts | sed -n '1,220p'

echo '--- VPointsSource.ts (selected lines) ---'
nl -ba packages/core/src/models/VPointsSource.ts | sed -n '1,260p'

Repository: Taylor-CCB-Group/SpatialData.js

Length of output: 254

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
from pathlib import Path

files = [
    Path('packages/layers/src/engine/PointsDataEngine.ts'),
    Path('packages/core/src/models/VPointsSource.ts'),
]
for p in files:
    print(f'=== {p} ===')
    text = p.read_text()
    for needle in ['loadRowFeatureCodes', 'loadPointsRowFeatureCodes', 'ensureLoaded', 'memoryCap', 'loadPoints(']:
        if needle in text:
            print(f'-- hits for {needle} --')
            for i, line in enumerate(text.splitlines(), 1):
                if needle in line:
                    print(f'{i}: {line}')
PY

Repository: Taylor-CCB-Group/SpatialData.js

Length of output: 3310

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the concrete implementations of the relevant methods
rg -n "class VPointsSource|loadRowFeatureCodes|loadPointsRowFeatureCodes|loadPoints\(" packages/core/src/models/VPointsSource.ts packages/core/src -g '!**/dist/**' -g '!**/build/**' -A 12 -B 6 || true

Repository: Taylor-CCB-Group/SpatialData.js

Length of output: 13397

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
from pathlib import Path

def show(path, start, end):
    print(f'--- {path} {start}-{end} ---')
    lines = Path(path).read_text().splitlines()
    for i in range(start, min(end, len(lines)) + 1):
        print(f'{i:4d}: {lines[i-1]}')

show('packages/layers/src/engine/PointsDataEngine.ts', 540, 860)
show('packages/core/src/models/VPointsSource.ts', 335, 910)
PY

Repository: Taylor-CCB-Group/SpatialData.js

Length of output: 42047

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
from pathlib import Path

path = Path('packages/core/src/models/VPointsSource.ts')
lines = path.read_text().splitlines()

# Focus on the row-code loader internals.
for start, end in [(839, 910), (335, 410), (780, 830)]:
    print(f'--- {path} {start}-{end} ---')
    for i in range(start, min(end, len(lines)) + 1):
        print(f'{i:4d}: {lines[i-1]}')
PY

Repository: Taylor-CCB-Group/SpatialData.js

Length of output: 9387

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
from pathlib import Path

path = Path('packages/layers/src/engine/PointsDataEngine.ts')
lines = path.read_text().splitlines()

for start, end in [(545, 660), (820, 860)]:
    print(f'--- {path} {start}-{end} ---')
    for i in range(start, min(end, len(lines)) + 1):
        print(f'{i:4d}: {lines[i-1]}')
PY

Repository: Taylor-CCB-Group/SpatialData.js

Length of output: 8130

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
from pathlib import Path

def show(path, start, end):
    print(f'--- {path} {start}-{end} ---')
    lines = Path(path).read_text().splitlines()
    for i in range(start, min(end, len(lines)) + 1):
        print(f'{i:4d}: {lines[i-1]}')

show('packages/layers/src/engine/PointsDataEngine.ts', 545, 860)
show('packages/core/src/models/VPointsSource.ts', 335, 910)
PY

Repository: Taylor-CCB-Group/SpatialData.js

Length of output: 41694

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
from pathlib import Path

def show(path, start, end):
    print(f'--- {path} {start}-{end} ---')
    lines = Path(path).read_text().splitlines()
    for i in range(start, min(end, len(lines)) + 1):
        print(f'{i:4d}: {lines[i-1]}')

show('packages/layers/src/engine/PointsDataEngine.ts', 545, 860)
show('packages/core/src/models/VPointsSource.ts', 835, 910)
PY

Repository: Taylor-CCB-Group/SpatialData.js

Length of output: 19189

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
from pathlib import Path

for path in [
    'packages/core/src/pointsLoadOptions.ts',
    'packages/core/src/models/VPointsSource.ts',
    'packages/layers/src/engine/PointsDataEngine.ts',
]:
    text = Path(path).read_text().splitlines()
    print(f'--- {path} ---')
    for i, line in enumerate(text, 1):
        if 'resolvePointsMemoryCap' in line or 'DEFAULT_POINTS_MEMORY_CAP' in line or 'memoryCap' in line and ('resolve' in line or 'DEFAULT' in line):
            print(f'{i:4d}: {line}')
PY

Repository: Taylor-CCB-Group/SpatialData.js

Length of output: 704

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
from pathlib import Path

path = Path('packages/layers/src/engine/PointsDataEngine.ts')
lines = path.read_text().splitlines()
for start, end in [(730, 770), (834, 858), (545, 650)]:
    print(f'--- {path} {start}-{end} ---')
    for i in range(start, min(end, len(lines)) + 1):
        print(f'{i:4d}: {lines[i-1]}')
PY

Repository: Taylor-CCB-Group/SpatialData.js

Length of output: 8983

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
from pathlib import Path

for path in ['packages/core/src/pointsLoadOptions.ts', 'packages/core/src/models/VPointsSource.ts']:
    lines = Path(path).read_text().splitlines()
    print(f'--- {path} ---')
    for i, line in enumerate(lines, 1):
        if 'resolvePointsMemoryCap' in line or 'DEFAULT_POINTS_MEMORY_CAP' in line:
            print(f'{i:4d}: {line}')
PY

Repository: Taylor-CCB-Group/SpatialData.js

Length of output: 502

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
from pathlib import Path

path = Path('packages/layers/src/engine/PointsDataEngine.ts')
lines = path.read_text().splitlines()
for start, end in [(730, 780), (834, 860), (545, 660)]:
    print(f'--- {path} {start}-{end} ---')
    for i in range(start, min(end, len(lines)) + 1):
        print(f'{i:4d}: {lines[i-1]}')
PY

Repository: Taylor-CCB-Group/SpatialData.js

Length of output: 9890

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
from pathlib import Path

path = Path('packages/core/src/pointsLoadOptions.ts')
lines = path.read_text().splitlines()
print(f'--- {path} ---')
for i, line in enumerate(lines, 1):
    if 'resolvePointsMemoryCap' in line or 'DEFAULT_POINTS_MEMORY_CAP' in line or 'memoryCap' in line:
        print(f'{i:4d}: {line}')
PY

Repository: Taylor-CCB-Group/SpatialData.js

Length of output: 244

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
from pathlib import Path
import re

files = [Path('packages/layers/src/engine/PointsDataEngine.ts')] + list(Path('packages').rglob('*.ts')) + list(Path('packages').rglob('*.tsx'))
targets = ['ensureRowFeatureCodes(', 'getRowFeatureCodes(', 'hasRowFeatureCodes(']
for t in targets:
    print(f'=== {t} ===')
    for p in files:
        try:
            text = p.read_text()
        except Exception:
            continue
        if t in text:
            for i, line in enumerate(text.splitlines(), 1):
                if t in line:
                    print(f'{p}:{i}: {line.strip()}')
PY

Repository: Taylor-CCB-Group/SpatialData.js

Length of output: 3749

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
from pathlib import Path

for needle in ['ensureRowFeatureCodes(', 'getRowFeatureCodes(', 'hasRowFeatureCodes(']:
    print(f'=== {needle} ===')
    for p in Path('packages').rglob('*.ts'):
        try:
            lines = p.read_text().splitlines()
        except Exception:
            continue
        for i, line in enumerate(lines, 1):
            if needle in line:
                print(f'{p}:{i}: {line.strip()}')
PY

Repository: Taylor-CCB-Group/SpatialData.js

Length of output: 3290

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n "ensureRowFeatureCodes\\(|getRowFeatureCodes\\(|hasRowFeatureCodes\\(" packages -g '*.ts' -g '*.tsx' || true

Repository: Taylor-CCB-Group/SpatialData.js

Length of output: 3301

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
from pathlib import Path

path = Path('packages/vis/src/SpatialCanvas/useLayerData.ts')
lines = path.read_text().splitlines()
for start, end in [(1380, 1445), (1445, 1485)]:
    print(f'--- {path} {start}-{end} ---')
    for i in range(start, min(end, len(lines)) + 1):
        print(f'{i:4d}: {lines[i-1]}')
PY

Repository: Taylor-CCB-Group/SpatialData.js

Length of output: 6335

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
from pathlib import Path

path = Path('packages/vis/src/SpatialCanvas/useLayerData.ts')
lines = path.read_text().splitlines()
for i, line in enumerate(lines, 1):
    if 'ensureRowFeatureCodes' in line or 'ensureLoaded' in line or 'hasRowFeatureCodes' in line:
        print(f'{i:4d}: {line}')
PY

Repository: Taylor-CCB-Group/SpatialData.js

Length of output: 405

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
from pathlib import Path

path = Path('packages/vis/src/SpatialCanvas/useLayerData.ts')
lines = path.read_text().splitlines()
for start, end in [(1390, 1435), (1435, 1465)]:
    print(f'--- {path} {start}-{end} ---')
    for i in range(start, min(end, len(lines)) + 1):
        print(f'{i:4d}: {lines[i-1]}')
PY

Repository: Taylor-CCB-Group/SpatialData.js

Length of output: 4617

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
from pathlib import Path

path = Path('packages/vis/src/SpatialCanvas/useLayerData.ts')
lines = path.read_text().splitlines()
for start, end in [(830, 870), (1388, 1422)]:
    print(f'--- {path} {start}-{end} ---')
    for i in range(start, min(end, len(lines)) + 1):
        print(f'{i:4d}: {lines[i-1]}')
PY

Repository: Taylor-CCB-Group/SpatialData.js

Length of output: 4654

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
from pathlib import Path

path = Path('packages/core/src/pointsLimits.ts')
lines = path.read_text().splitlines()
for i, line in enumerate(lines, 1):
    if 'DEFAULT_POINTS_MEMORY_CAP' in line or 'resolvePointsMemoryCap' in line or 'exceedsPointsPreloadLimit' in line:
        print(f'{i:4d}: {line}')
PY

Repository: Taylor-CCB-Group/SpatialData.js

Length of output: 496

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
from pathlib import Path

path = Path('packages/vis/src/SpatialCanvas/useLayerData.ts')
lines = path.read_text().splitlines()
for start, end in [(840, 870), (1390, 1422)]:
    print(f'--- {path} {start}-{end} ---')
    for i in range(start, min(end, len(lines)) + 1):
        print(f'{i:4d}: {lines[i-1]}')
PY

Repository: Taylor-CCB-Group/SpatialData.js

Length of output: 4020

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
from pathlib import Path

path = Path('packages/vis/src/SpatialCanvas/useLayerData.ts')
lines = path.read_text().splitlines()
for start, end in [(850, 865)]:
    print(f'--- {path} {start}-{end} ---')
    for i in range(start, min(end, len(lines)) + 1):
        print(f'{i:4d}: {lines[i-1]}')
PY

Repository: Taylor-CCB-Group/SpatialData.js

Length of output: 1104

---

Thread entry.memoryCap into ensureRowFeatureCodes. ensureLoaded(target, cap) can use a layer-specific resident window, but this fallback still uses the loader default. Pass entry.memoryCap here to keep the row codes aligned with the resident batch.

🤖 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/engine/PointsDataEngine.ts` around lines 30 - 36, Update
ensureRowFeatureCodes to accept a memory-cap argument and pass entry.memoryCap
from the fallback call site, ensuring it uses the same resident window as
ensureLoaded. Locate the relevant invocation in the PointsDataEngine loading
path and thread the value through all affected helper signatures and calls.

[coderabbitai]

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Misplaced/duplicate JSDoc: this comment describes getResidentFeatureCodes, not hasFeatureCodeColumn.

The comment block at lines 694-701 ("distinct feature codes actually present in the resident batch… Memoized against the row-codes identity so the O(rows) scan runs once per batch") exactly matches getResidentFeatureCodes's behavior (line 754), which currently has no doc comment of its own. It's stranded above the unrelated hasFeatureCodeColumn, which already has its own correct doc directly below (702-708). This is confusing for future readers.

✏️ Proposed fix: move the stray doc to its rightful method

-  /**
-   * 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 {

and re-add the moved doc directly above getResidentFeatureCodes.

🤖 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/engine/PointsDataEngine.ts` around lines 694 - 709, Move
the JSDoc describing resident distinct feature codes from above
hasFeatureCodeColumn to directly above getResidentFeatureCodes, preserving its
content. Keep the existing hasFeatureCodeColumn documentation immediately above
that method and ensure each comment documents only its corresponding method.

[coderabbitai]

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

"Select it to fetch" message is misleading when the feature is already selected.

When selected && !scanning && !resident && !rendered && supportsOnDemandLoad, the function falls through to the notLoaded branch with reason "select it to fetch its points via the feature-index scan." But the feature IS already selected — this transient state occurs between selection and scan start. The message should reflect that a scan is pending, not that the user needs to select it.

The author noted "This is up for review" at line 46, so this is a known gap.

💡 Suggested fix

   if (selected && scanning) {
     return {
       tone: 'loading',
       greyed: true,
       label: 'loading',
       reason: 'Selected — its feature-index scan is in progress.',
     };
   }
+  if (selected && supportsOnDemandLoad) {
+    return {
+      tone: 'loading',
+      greyed: true,
+      label: 'queued',
+      reason: 'Selected — waiting for the feature-index scan to start.',
+    };
+  }
   if (!supportsOnDemandLoad) {

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	if (selected && scanning) {
	return {
	tone: 'loading',
	greyed: true,
	label: 'loading',
	reason: 'Selected — its feature-index scan is in progress.',
	};
	}
	if (!supportsOnDemandLoad) {
	return {
	tone: 'noIndex',
	greyed: true,
	label: 'not in sample',
	reason:
	'Beyond the resident window, and this dataset has no feature index, so it can’t be fetched on demand. Raise the memory cap or rewrite the dataset with an index.',
	};
	}
	return {
	tone: 'notLoaded',
	greyed: true,
	label: 'not loaded',
	reason: 'Beyond the resident window; select it to fetch its points via the feature-index scan.',
	};
	if (selected && scanning) {
	return {
	tone: 'loading',
	greyed: true,
	label: 'loading',
	reason: 'Selected — its feature-index scan is in progress.',
	};
	}
	if (selected && supportsOnDemandLoad) {
	return {
	tone: 'loading',
	greyed: true,
	label: 'queued',
	reason: 'Selected — waiting for the feature-index scan to start.',
	};
	}
	if (!supportsOnDemandLoad) {
	return {
	tone: 'noIndex',
	greyed: true,
	label: 'not in sample',
	reason:
	'Beyond the resident window, and this dataset has no feature index, so it can’t be fetched on demand. Raise the memory cap or rewrite the dataset with an index.',
	};
	}
	return {
	tone: 'notLoaded',
	greyed: true,
	label: 'not loaded',
	reason: 'Beyond the resident window; select it to fetch its points via the feature-index scan.',
	};

🤖 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/featureRowState.ts` around lines 89 - 111,
Update the feature-row state logic so the selected, non-scanning, non-resident,
non-rendered, on-demand-load case does not use the misleading “select it to
fetch” reason. In the function containing the selected/scanning and notLoaded
branches, add a selected-specific pending-scan branch with loading or equivalent
pending tone/label and wording that indicates the feature is already selected
and its feature-index scan is about to begin; keep the existing notLoaded
message only for unselected features.

[coderabbitai]

🧹 Nitpick comments (1)

docs/plans/points-redesign-punchlist.md (1)

49-53: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider paraphrasing the quoted profanity in D1.

The verbatim quote from PointsDataEngine.ts includes profanity ("shit-show"). While it reproduces an existing code comment, including it in documentation perpetulates unprofessional language. Consider paraphrasing (e.g., "given the difficulty of agent debugging…") or referencing the comment location without reproducing the exact wording.

🤖 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 `@docs/plans/points-redesign-punchlist.md` around lines 49 - 53, Update the D1
entry to remove the quoted profanity from the PointsDataEngine.ts reference,
paraphrasing it as the difficulty of agent debugging or referencing the comment
location without reproducing the exact wording; preserve the surrounding context
and intent.

🤖 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.

Nitpick comments:
In `@docs/plans/points-redesign-punchlist.md`:
- Around line 49-53: Update the D1 entry to remove the quoted profanity from the
PointsDataEngine.ts reference, paraphrasing it as the difficulty of agent
debugging or referencing the comment location without reproducing the exact
wording; preserve the surrounding context and intent.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 4d7dbecc-5bcd-4cd1-bbe0-548477d376cc

📥 Commits

Reviewing files that changed from the base of the PR and between e2de7d2 and ee58000.

📒 Files selected for processing (10)

• docs/plans/points-redesign-punchlist.md
• packages/core/src/models/VPointsSource.ts
• packages/core/src/models/index.ts
• packages/core/src/pointsLoadOptions.ts
• packages/layers/src/engine/PointsDataEngine.ts
• packages/layers/src/pointsScatterLayer.ts
• packages/vis/src/SpatialCanvas/PointsLayerPanel.tsx
• packages/vis/src/SpatialCanvas/renderers/index.ts
• packages/vis/src/SpatialCanvas/renderers/pointsRenderer.ts
• packages/vis/src/SpatialCanvas/useLayerData.ts

💤 Files with no reviewable changes (2)

• packages/vis/src/SpatialCanvas/renderers/pointsRenderer.ts
• packages/vis/src/SpatialCanvas/renderers/index.ts

🚧 Files skipped from review as they are similar to previous changes (5)

• packages/core/src/models/index.ts
• packages/layers/src/pointsScatterLayer.ts
• packages/core/src/models/VPointsSource.ts
• packages/layers/src/engine/PointsDataEngine.ts
• packages/vis/src/SpatialCanvas/PointsLayerPanel.tsx