feat: improvements for geoarrow deck layers in dashboard maps - #700

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/color-scales/src/ColorScaleLegend.tsx`:
- Around line 16-23: Add TSDoc documentation comments above the ColorScaleLegend
function definition to document the exported public API. Include a summary of
what the function does, document each prop parameter (legends, className, width,
swatchColumns, defaultCollapsed) with descriptions of their purpose and types,
and document the return type. This ensures the updated API contract including
the defaultCollapsed parameter is properly documented per coding guidelines.

In `@packages/deck/src/ai.ts`:
- Around line 346-353: The condition `if (!dashboard?.selectedTable)` prevents
deps.resolveTable from being called when a dashboard already has a selectedTable
set, which means the explicit tableName parameter passed by the caller is
ignored and the wrong dataset remains active. Remove the condition so that
deps.resolveTable(artifactId, params.tableName) is always called whenever
params.tableName is provided, regardless of whether dashboard.selectedTable is
already set.
- Around line 252-255: The current implementation of splitting
`source.tableName` using a simple `.split('.')` does not respect quoted
identifiers, causing already-quoted table name parts to be quoted again and
result in invalid SQL. Instead of splitting blindly by period, implement a
helper function that splits the table name by periods only when they occur
outside of double quotes, similar to the suggested
`splitIdentifierPathOutsideQuotes` function. Then use this function to split
`source.tableName` before applying `quoteDeckMapSqlIdentifier` to each part,
ensuring that parts that are already quoted are not re-quoted and that the full
qualified table name is handled correctly.

In `@packages/deck/src/dashboardConfig.ts`:
- Around line 129-134: The regex patterns at lines 129 and 133 in the
dashboardConfig.ts file use word boundary assertions `\b` after the dynamic
table name tokens, which fail to match quoted identifiers like "main"."events"
because word boundaries don't properly recognize the quotes and dots as
boundaries. Modify both regex patterns to replace the trailing `\b` word
boundary with a more flexible pattern that can handle both quoted and unquoted
table names, such as using a lookahead assertion or a character class that
explicitly matches whitespace, commas, semicolons, or closing parentheses that
typically follow table names in SQL queries.

In `@packages/deck/src/DeckJsonMap.tsx`:
- Around line 162-175: The button elements in the TripsTimeControl component at
lines 162, 173, and 197 lack explicit type attributes, which defaults them to
type="submit" and can trigger unintended form submission. Add type="button"
attribute to each of the three button elements in the TripsTimeControl component
to ensure they function as toggle/action buttons rather than form submission
buttons.
- Around line 518-523: The handleMapLoad callback function does not invoke the
consumer's onLoad handler from mapProps, causing it to be overridden when the
map load event fires. Modify the handleMapLoad function to call mapProps.onLoad
after executing the pending jump logic if onLoad exists, ensuring both the
internal jump behavior and any consumer-provided load handlers execute when the
map completes loading.
- Around line 430-433: The computedMaxTs variable is being overwritten with each
trips layer's maxTs value instead of keeping the maximum. Replace the assignment
`computedMaxTs = maxTs;` with `computedMaxTs = Math.max(computedMaxTs, maxTs);`
to ensure the timeline bounds correctly use the maximum timestamp across all
trips layers, preventing the timeline from shrinking based on layer order.

In `@packages/deck/src/json/createDeckJsonConfiguration.ts`:
- Around line 152-165: The promoted geometry source branch (when
layerData.source === 'promoted') is missing the table-consistency validation
that exists in the else branch. Add the same table mismatch validation check
that appears in the else block to the promoted branch, ensuring that when
layerData.source is 'promoted', the code also verifies that the table is not
combining promoted geometry columns from different prepared tables. This
validation should check if table is neither prepared.table nor layerData.table,
and if so, throw an error before assigning the geometryColumn to boundProps.

In `@packages/deck/src/json/layers/DeckH3HexagonLayer.ts`:
- Around line 62-66: The compiled accessor function calls at lines 62-66 and
105-109 pass a global row index `i` (from 0 to numRows-1) with only the first
batch `table.batches[0]`, but the GeoArrow accessor implementation expects a
batch-local index within that specific batch. For multi-batch tables, this
causes out-of-bounds access. Fix this by either determining which batch contains
the row at index i and computing the batch-local index accordingly before
calling the hexProp accessor, or by consolidating the table into a single batch
using appropriate table operations before processing the rows.

In `@packages/deck/src/json/layers/DeckTripsLayer.ts`:
- Around line 160-186: The code in the loop over Object.entries(restProps) only
handles float Arrow vectors for scalar binary accessors using
arrow.DataType.isFloat check, which causes integer vectors to be silently
skipped. Add an additional condition after the existing isFloat check to also
handle integer Arrow vectors by calling expandToCoords with chunkData.values
cast as TypedArray, using size 1, and relativeOffsets, then assigning the result
to binaryAttributes similar to the float case. Use arrow.DataType.isInteger or
the appropriate integer type checking method to identify integer vectors that
should be handled the same way as float vectors for scalar accessor promotion.

In `@packages/deck/src/mapConfigUtils.ts`:
- Around line 347-354: The return statement in the map configuration update
preserves the existing layer specs (which reference specific dataset IDs) but
replaces the datasets array with new ones, causing dataset ID mismatches and
broken layer bindings. Create two helper functions:
remapNextDatasetsToExistingIds to preserve the current dataset IDs while copying
over the updated source fields from the next config, and remapFitToDataDatasetId
to remap any dataset references in fitToData to use the preserved dataset IDs.
Then update the return statement to use these helpers so that layer bindings
remain valid when datasets are updated.

In `@packages/deck/src/prepare/detectGeometryColumn.ts`:
- Around line 54-67: The getFieldVector function can return an alternate vector
when the requested geometry column doesn't exist, but the calling code at line
170 still returns columnName: detectedGeometryColumn, creating a mismatch
between the returned vector and its column name. Modify getFieldVector to return
both the vector and the actual matched column name (including when a fallback
alternate is found), then update the return statement at lines 169-171 to use
the actual matched column name from the function result instead of always using
the original detectedGeometryColumn parameter.

In `@packages/deck/src/types.ts`:
- Around line 63-71: Add TSDoc comments to the DeckJsonMapHandle type definition
and its jumpTo method to document the public API contract. Include a brief
description of what DeckJsonMapHandle represents, and for the jumpTo method,
document each parameter (longitude, latitude, zoom, bearing, pitch) with their
types and purposes. Also apply the same TSDoc documentation approach to the
rendering-error callback contract defined in lines 73-93 as referenced in the
comment.

In `@packages/deck/src/useDeckMapDatasets.ts`:
- Around line 49-59: The current implementation only checks the individual
state's error status and clears the panel issue whenever any single dataset
updates without an error, even if other datasets still have errors in
multi-dataset panels. Refactor the error checking logic to examine all datasets
in datasetStates by iterating through Object.values(datasetStates) and finding
the first error that exists across any dataset. If an error is found, call
reportPanelIssue with that error; only call clearPanelIssue when no errors exist
across all datasets. Wrap this consolidated logic in a useEffect dependency that
responds to changes in datasetStates, dashboardId, panel.id, reportPanelIssue,
and clearPanelIssue to ensure proper tracking across all datasets.

In `@packages/duckdb-core/src/BaseDuckDbConnector.ts`:
- Around line 174-179: The catch block in the GeoJSON handling section (around
the loadSpatial function call) currently catches all errors indiscriminately and
silently falls back to load('auto', ...), which masks genuine GeoJSON parsing
failures. Instead of catching all errors, narrow the catch block to only catch
specific spatial-capability-related errors that indicate the spatial extension
is unavailable or doesn't support the operation, and allow other error types
(like parsing errors or invalid GeoJSON) to propagate up. This way, legitimate
failures in the GeoJSON file won't be silently converted to non-spatial data
loads.

In `@python/sqlrooms-server/sqlrooms/server/query.py`:
- Around line 102-114: The raw SQL variable is being directly wrapped in
subqueries at the wrapped query construction and cast_sql query construction
without stripping trailing semicolons, which makes the subqueries invalid.
Additionally, column names in the projections list may contain unescaped quotes
that could break the SQL. Fix this by stripping any trailing semicolons from the
sql variable before using it in both the wrapped and cast_sql constructions, and
ensure column names are properly quoted or escaped when building the projections
list to safely handle column names with special characters.

---

Outside diff comments:
In `@packages/color-scales/src/scale.ts`:
- Around line 176-192: The nullish checks in getSequentialDomain and
getDivergingDomain functions do not properly handle the case where extent
returns Infinity or -Infinity for empty input arrays. Update the guards on line
178 and line 190 to verify not only that min and max are not null but also that
they are finite numbers using isFinite or a similar check. This will ensure that
invalid domains with Infinity values are caught and default domains are returned
instead.

In `@packages/deck/src/dashboard.tsx`:
- Around line 358-467: Remove the duplicate function and type definitions from
dashboard.tsx that are already provided by the useDeckMapFitToBounds module.
Specifically, delete the implementations of createDeckMapBoundsQuery,
readBoundsFromExtentResult, fitViewStateToBounds,
deckMapDashboardFitRequestTarget, emitDeckMapDashboardFitRequest,
DeckMapDashboardFitState, and createInitialDeckMapDashboardFitState functions
shown in the diff. This will eliminate the dead code and resolve the shadowing
issue where the local emitDeckMapDashboardFitRequest definition conflicts with
the imported version from line 57, ensuring only the imported
emitDeckMapDashboardFitRequest is used throughout the file.

---

Nitpick comments:
In `@packages/deck/__tests__/createDeckJsonConfiguration.test.ts`:
- Around line 332-358: The current test suite only validates that the
getFillColor accessor is a function but does not test the new runtime branches
introduced in this PR, specifically waypoint auto-aggregation to LineString and
promoted-table consistency across multi-geometry bindings. Add separate focused
test cases to the test suite that verify these new behavior branches work
correctly during conversion, ensuring that the converter properly handles these
cases and that conversion correctness is maintained for multi-geometry bindings
and waypoint aggregation scenarios.

In `@packages/deck/__tests__/dashboard-ai-map.test.ts`:
- Around line 235-278: The existing tests in the dashboard-ai-map.test.ts file
only cover basic creation and update scenarios with unquoted table names and no
pre-selected dashboard table. You need to add two new test cases following the
same pattern as the existing it() tests: First, add a test that sets up deps
with a pre-existing selectedTable value, then calls execute on
createDeckMapDashboardTool with a different tableName to verify the table-switch
behavior works correctly. Second, add a test case where the dataset source uses
a quoted/qualified table name format like "local"."main"."events", then verify
through assertions that the normalized SQL remains valid and the panel is
created or updated correctly. Both tests should follow the same
arrange-act-assert pattern as the existing tests, using createDeps(),
createDeckMapDashboardTool(deps), and validating the results through expect()
assertions on llmResult.success, deps.panels, and relevant panel properties.

In `@packages/deck/src/mapLayerConfigUtils.ts`:
- Around line 134-173: Add tsdoc comments to all six exported functions in the
current code block (usesH3ColumnSetting, usesArcColumnSetting,
usesRadiusSetting, usesColumnRadiusSetting, usesTripsSettings, and
usesExtrusionSettings) and to setDeckMapLayerHexagonColumn and
setDeckMapLayerArcColumns functions mentioned at lines 273-307. Each function
should have a tsdoc comment above it that describes what the function does,
documents the layerType parameter (or relevant parameters for the
setDeckMapLayer functions), and describes the return type. Follow the existing
tsdoc style and format used elsewhere in the codebase to maintain consistency
with the project's coding guidelines for exported functions and types.

In `@packages/deck/src/useDeckMapFitToBounds.ts`:
- Around line 246-252: The exported function emitDeckMapDashboardFitRequest
lacks tsdoc documentation which is required for exported functions in packages.
Add tsdoc comments above the function definition to document what the function
does, including a description of the panelId parameter and the purpose of the
CustomEvent being dispatched. Use appropriate tsdoc tags like `@param` and
`@remarks` to provide clear documentation for developers using this function.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@packages/deck/src/dashboardConfig.ts`:
- Around line 205-227: The function `createDeckMapDashboardDatasetQuery` needs
defensive validation and documentation. First, add a guard clause at the
beginning of the function to validate that the source parameter contains at
least either a tableName or sqlQuery property, throwing an appropriate error if
neither is provided. This prevents invalid SQL generation when sourceExpr
becomes an empty string and sampleRows is specified. Second, add TSDoc comments
above the function to document all parameters (source with its nested
properties, filter, and options including sampleRows) and the return type to
meet library API documentation standards for exported functions in the packages
directory.

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/mosaic/src/charts/chart-types/base-types.ts`:
- Around line 203-206: The SQL table reference construction in the code block
where schema and table values are wrapped in double quotes does not escape
double quote characters within those identifier values. If schema or table
contain a double quote character, the generated SQL reference becomes invalid.
To fix this, apply proper escaping to both the schema and table variables before
constructing the SQL string by replacing any double quote characters with
escaped double quotes (using the standard SQL escaping method of doubling the
quote) before inserting them into the template string at line 205.