Repository navigation
feat: dashboard maps improvements - #746
Conversation
Signed-off-by: Ihor Dykhta <[email protected]>
Signed-off-by: Ihor Dykhta <[email protected]>
Signed-off-by: Ihor Dykhta <[email protected]>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR adds overflow truncation with scrollable containers and "N more" indicators to ChangesLegend Item Overflow Truncation
Deck Map: Geometry Column Binding and Heatmap Color Defaults
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
Signed-off-by: Ihor Dykhta <[email protected]>
Signed-off-by: Ihor Dykhta <[email protected]>
Signed-off-by: Ihor Dykhta <[email protected]>
Signed-off-by: Ihor Dykhta <[email protected]>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b155af6e9d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/deck/src/mapLayerConfigUtils.ts (1)
261-278: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winKeep
fitToData.geometryColumnsynchronized with geometry-column edits.Updating only
datasets[datasetId].geometryColumnand_sqlroomsBinding.geometryColumnleavesconfig.fitToData.geometryColumnstale. That can make bounds queries run against the old column after users change geometry in Map Settings.Proposed fix
const updatedConfig = { ...config, datasets: { ...config.datasets, [datasetId]: { ...config.datasets[datasetId], geometryColumn, }, }, + fitToData: + config.fitToData && + config.fitToData.dataset === datasetId && + 'geometryColumn' in config.fitToData + ? { + ...config.fitToData, + geometryColumn, + } + : config.fitToData, };🤖 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/deck/src/mapLayerConfigUtils.ts` around lines 261 - 278, The current code updates datasets[datasetId].geometryColumn and _sqlroomsBinding.geometryColumn but leaves config.fitToData.geometryColumn out of sync, causing bounds queries to use stale geometry column references. When constructing the updatedConfig object, also add fitToData to the spread operation and update its geometryColumn property alongside the datasets update to keep all references synchronized.
🧹 Nitpick comments (8)
packages/color-scales/src/legend/CategoricalLegend.tsx (1)
8-14: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd TSDoc for the exported component.
Please add a short TSDoc block for
CategoricalLegendinputs/behavior to satisfy TS file documentation rules.As per coding guidelines "
**/*.{ts,tsx}: Add tsdoc to exported types and functions ".🤖 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/color-scales/src/legend/CategoricalLegend.tsx` around lines 8 - 14, The CategoricalLegend exported function is missing TSDoc documentation. Add a TSDoc comment block above the function definition that includes a brief description of what the component does and documents the parameters legend (describing it as a resolved categorical color legend) and columns (describing it as the number of columns to display). Follow the standard TSDoc format with `@param` tags for each parameter to satisfy the coding guideline requiring TSDoc for all exported functions in TypeScript files.Source: Coding guidelines
packages/color-scales/src/legend/SteppedLegend.tsx (1)
9-15: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd TSDoc for the exported component.
Please add TSDoc for
SteppedLegenddescribing props and truncation behavior.As per coding guidelines "
**/*.{ts,tsx}: Add tsdoc to exported types and functions ".🤖 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/color-scales/src/legend/SteppedLegend.tsx` around lines 9 - 15, The exported SteppedLegend component is missing TSDoc documentation as required by coding guidelines. Add a TSDoc comment block above the SteppedLegend function definition that includes a description of what the component does, documents each prop (legend and width) with param tags, and describes the truncation behavior. Ensure the documentation follows TypeScript documentation standards and provides clear guidance on the component's purpose and usage.Source: Coding guidelines
packages/deck/src/json/extractColorScaleLegends.ts (1)
75-79: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd TSDoc for the exported legend extractor.
The exported function now has heatmap-specific branching and fallback behavior that should be documented for callers.
As per coding guidelines,
**/*.{ts,tsx}: Add tsdoc to exported types and functions.🤖 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/deck/src/json/extractColorScaleLegends.ts` around lines 75 - 79, Add TSDoc comments above the exported function extractColorScaleLegends to document its purpose, parameters, return type, and behavior. Include documentation about the heatmap-specific branching logic and fallback behavior mentioned in the function implementation, so that callers understand what the function does and how it handles different cases. Follow the codebase's documentation standards for exported functions.Source: Coding guidelines
packages/deck/src/mapConfigUtils.ts (1)
199-211: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd TSDoc for the updated exported config builder.
This exported API gained new geometry-binding and geometry-
fitToDatabehavior but still has no TSDoc contract.As per coding guidelines,
**/*.{ts,tsx}: Add tsdoc to exported types and functions.🤖 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/deck/src/mapConfigUtils.ts` around lines 199 - 211, The exported function createDeckMapDashboardConfigForTable lacks TSDoc documentation despite being a public API with updated geometry-binding and geometry-fitToData behavior. Add a TSDoc comment block above the function that describes the purpose, documents all parameters (tableName, columns, tableReference, sourceSqlQuery, longitudeColumn, latitudeColumn, geometryColumn, geometryEncodingHint, pointRadius, fillColor, mapStyle), and describes the return type DeckMapDashboardPanelConfig. Include information about the geometry-binding and geometry-fitToData behavior in the documentation.Source: Coding guidelines
packages/deck/src/mapLayerConfigUtils.ts (1)
250-254: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd TSDoc for the exported geometry-column mutator.
This exported function now updates multiple config surfaces (dataset + layer binding), so the contract should be documented.
As per coding guidelines,
**/*.{ts,tsx}: Add tsdoc to exported types and functions.🤖 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/deck/src/mapLayerConfigUtils.ts` around lines 250 - 254, The exported function setDeckMapLayerGeometryColumn is missing TSDoc documentation which is required per coding guidelines. Add a TSDoc comment block above the function definition that documents what the function does, describing that it updates the geometry column configuration across both the dataset config and layer binding surfaces. Include parameter descriptions for config, layerIndex, and geometryColumn, along with documentation of the return type as DeckMapDashboardPanelConfig. This will clarify the function's contract and side effects for other developers.Source: Coding guidelines
packages/deck/src/json/createDeckJsonConfiguration.ts (2)
216-218: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd TSDoc for the exported JSON configuration factory.
This exported function now includes heatmap-default and trigger-rewrite behavior that should be documented at the API boundary.
As per coding guidelines,
**/*.{ts,tsx}: Add tsdoc to exported types and functions.🤖 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/deck/src/json/createDeckJsonConfiguration.ts` around lines 216 - 218, Add a TSDoc comment block above the createDeckJsonConfiguration exported function that documents its purpose and behavior. The documentation should describe what the function does, document the options parameter of type CreateDeckJsonConfigurationOptions, describe the return value, and specifically mention the heatmap-default and trigger-rewrite behaviors that the function now includes as these are important aspects of its API contract.Source: Coding guidelines
33-51: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the heatmap default palette into a shared constant/module.
This default range is duplicated with
extractColorScaleLegends.ts; drift here will desync rendered heatmap colors vs legend gradient.🤖 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/deck/src/json/createDeckJsonConfiguration.ts` around lines 33 - 51, The DEFAULT_HEATMAP_COLOR_RANGE constant in createDeckJsonConfiguration.ts is duplicated in extractColorScaleLegends.ts, which creates a risk of the two files becoming out of sync and causing rendering inconsistencies between the heatmap and legend. Extract the DEFAULT_HEATMAP_COLOR_RANGE constant into a new shared constants file (in a colors or utilities module), then update both createDeckJsonConfiguration.ts and extractColorScaleLegends.ts to import this constant from the shared location instead of maintaining separate copies.packages/deck/src/MapSettings.tsx (1)
175-179: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd TSDoc for the exported
MapSettingsPanelcomponent.This exported surface gained new geometry-column and heatmap-scheme behavior; document the contract and key props.
As per coding guidelines,
**/*.{ts,tsx}: Add tsdoc to exported types and functions.🤖 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/deck/src/MapSettings.tsx` around lines 175 - 179, Add TSDoc documentation comments above the MapSettingsPanel component export. Include a brief description of the component's purpose and mention the new geometry-column and heatmap-scheme behavior it supports. Document each of the key props (dashboardId, panel, onClose) with their types and purposes. Follow the coding guidelines that require TSDoc for all exported types and functions in .tsx files, ensuring the documentation clearly describes the contract and functionality of the exported component.Source: Coding guidelines
🤖 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/color-scales/src/legend/CategoricalLegend.tsx`:
- Around line 29-33: The maxHeight calculation in the scrollable container uses
a fixed height based on MAX_VISIBLE_ITEMS multiplied by item height, which
doesn't account for the number of columns. This causes the visible item count to
exceed the intended threshold when columns are present. Modify the maxHeight
style in the div with className "grid gap-x-3 gap-y-1.5" to calculate the height
based on the number of rows needed to fit MAX_VISIBLE_ITEMS across the given
number of columns, using the formula Math.ceil(MAX_VISIBLE_ITEMS / columns) *
itemHeight to ensure the scroll threshold is tied to total items rather than
grid rows.
In `@packages/deck/src/json/createDeckJsonConfiguration.ts`:
- Around line 362-371: The code in the `createDeckJsonConfiguration` function
unconditionally overwrites the `getWeight` property in `updateTriggers` without
preserving any existing value. Instead of directly assigning `getWeight:
JSON.stringify(rewritten.colorRange)` in the merge, check if
`existingTriggers.getWeight` already exists and preserve it. Combine both the
existing trigger and the new color-range based trigger so that both invalidation
signals survive the merge operation.
---
Outside diff comments:
In `@packages/deck/src/mapLayerConfigUtils.ts`:
- Around line 261-278: The current code updates
datasets[datasetId].geometryColumn and _sqlroomsBinding.geometryColumn but
leaves config.fitToData.geometryColumn out of sync, causing bounds queries to
use stale geometry column references. When constructing the updatedConfig
object, also add fitToData to the spread operation and update its geometryColumn
property alongside the datasets update to keep all references synchronized.
---
Nitpick comments:
In `@packages/color-scales/src/legend/CategoricalLegend.tsx`:
- Around line 8-14: The CategoricalLegend exported function is missing TSDoc
documentation. Add a TSDoc comment block above the function definition that
includes a brief description of what the component does and documents the
parameters legend (describing it as a resolved categorical color legend) and
columns (describing it as the number of columns to display). Follow the standard
TSDoc format with `@param` tags for each parameter to satisfy the coding guideline
requiring TSDoc for all exported functions in TypeScript files.
In `@packages/color-scales/src/legend/SteppedLegend.tsx`:
- Around line 9-15: The exported SteppedLegend component is missing TSDoc
documentation as required by coding guidelines. Add a TSDoc comment block above
the SteppedLegend function definition that includes a description of what the
component does, documents each prop (legend and width) with param tags, and
describes the truncation behavior. Ensure the documentation follows TypeScript
documentation standards and provides clear guidance on the component's purpose
and usage.
In `@packages/deck/src/json/createDeckJsonConfiguration.ts`:
- Around line 216-218: Add a TSDoc comment block above the
createDeckJsonConfiguration exported function that documents its purpose and
behavior. The documentation should describe what the function does, document the
options parameter of type CreateDeckJsonConfigurationOptions, describe the
return value, and specifically mention the heatmap-default and trigger-rewrite
behaviors that the function now includes as these are important aspects of its
API contract.
- Around line 33-51: The DEFAULT_HEATMAP_COLOR_RANGE constant in
createDeckJsonConfiguration.ts is duplicated in extractColorScaleLegends.ts,
which creates a risk of the two files becoming out of sync and causing rendering
inconsistencies between the heatmap and legend. Extract the
DEFAULT_HEATMAP_COLOR_RANGE constant into a new shared constants file (in a
colors or utilities module), then update both createDeckJsonConfiguration.ts and
extractColorScaleLegends.ts to import this constant from the shared location
instead of maintaining separate copies.
In `@packages/deck/src/json/extractColorScaleLegends.ts`:
- Around line 75-79: Add TSDoc comments above the exported function
extractColorScaleLegends to document its purpose, parameters, return type, and
behavior. Include documentation about the heatmap-specific branching logic and
fallback behavior mentioned in the function implementation, so that callers
understand what the function does and how it handles different cases. Follow the
codebase's documentation standards for exported functions.
In `@packages/deck/src/mapConfigUtils.ts`:
- Around line 199-211: The exported function
createDeckMapDashboardConfigForTable lacks TSDoc documentation despite being a
public API with updated geometry-binding and geometry-fitToData behavior. Add a
TSDoc comment block above the function that describes the purpose, documents all
parameters (tableName, columns, tableReference, sourceSqlQuery, longitudeColumn,
latitudeColumn, geometryColumn, geometryEncodingHint, pointRadius, fillColor,
mapStyle), and describes the return type DeckMapDashboardPanelConfig. Include
information about the geometry-binding and geometry-fitToData behavior in the
documentation.
In `@packages/deck/src/mapLayerConfigUtils.ts`:
- Around line 250-254: The exported function setDeckMapLayerGeometryColumn is
missing TSDoc documentation which is required per coding guidelines. Add a TSDoc
comment block above the function definition that documents what the function
does, describing that it updates the geometry column configuration across both
the dataset config and layer binding surfaces. Include parameter descriptions
for config, layerIndex, and geometryColumn, along with documentation of the
return type as DeckMapDashboardPanelConfig. This will clarify the function's
contract and side effects for other developers.
In `@packages/deck/src/MapSettings.tsx`:
- Around line 175-179: Add TSDoc documentation comments above the
MapSettingsPanel component export. Include a brief description of the
component's purpose and mention the new geometry-column and heatmap-scheme
behavior it supports. Document each of the key props (dashboardId, panel,
onClose) with their types and purposes. Follow the coding guidelines that
require TSDoc for all exported types and functions in .tsx files, ensuring the
documentation clearly describes the contract and functionality of the exported
component.
🪄 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: a1e8f191-1716-401a-b37f-9a5505fc3163
📒 Files selected for processing (9)
packages/color-scales/src/legend/CategoricalLegend.tsxpackages/color-scales/src/legend/SteppedLegend.tsxpackages/deck/__tests__/dashboard-ai-map.test.tspackages/deck/src/MapSettings.tsxpackages/deck/src/ai.tspackages/deck/src/json/createDeckJsonConfiguration.tspackages/deck/src/json/extractColorScaleLegends.tspackages/deck/src/mapConfigUtils.tspackages/deck/src/mapLayerConfigUtils.ts
Signed-off-by: Ihor Dykhta <[email protected]>
Signed-off-by: Ihor Dykhta <[email protected]>
Signed-off-by: Ihor Dykhta <[email protected]>
| */ | ||
| function extractSelectAliases(sqlQuery: string | undefined): string[] { | ||
| if (!sqlQuery) return []; | ||
| const selectMatch = sqlQuery.match(/\bSELECT\b(.+?)\bFROM\b/is); |
There was a problem hiding this comment.
oh my, is there no other way except for parsing SQL? this is very brittle.
e.g. if the query comes from the model maybe we could instruct it to provide the name as additional output
There was a problem hiding this comment.
AI suggested this, I haven't tested but maybe you can take a look:
#768
Yes, we can avoid it. I removed extractSelectAliases() from MapSettings.tsx and instead pass the loaded map datasetStates into MapSettingsPanel from dashboard.tsx. The settings panel now uses the actual resolved Arrow table schema for the active dataset, so computed columns like AS geom come from DuckDB’s query result rather than a regex over SQL text.
There was a problem hiding this comment.
I removed the generated column logic. It makes sense to implement it in a separate isolated PR. I tried several approaches, but each time I ran into issues and couldn't get everything working correctly.
Signed-off-by: Ihor Dykhta <[email protected]>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/deck/src/MapSettings.tsx (1)
190-199: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAlign geometry selector options with the same source as the selected value.
columnsWithVirtualis built fromactiveLayerDataset?.geometryColumn, but the selector value prefers_sqlroomsBinding.geometryColumn. That can produce a selected geometry column that is missing from options. Derive virtual columns from the same preferred source (binding first, dataset fallback).Suggested patch
+ const selectedGeometryColumn = + ((activeLayer?._sqlroomsBinding as Record<string, unknown>) + ?.geometryColumn as string | undefined) ?? + activeLayerDataset?.geometryColumn; + const columnsWithVirtual = useMemo( () => dataTable ? getColumnsWithVirtual( dataTable.columns, - activeLayerDataset?.geometryColumn, + selectedGeometryColumn, ) : [], - [dataTable, activeLayerDataset?.geometryColumn], + [dataTable, selectedGeometryColumn], );Also applies to: 659-666
🤖 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/deck/src/MapSettings.tsx` around lines 190 - 199, The columnsWithVirtual useMemo hook derives virtual columns from activeLayerDataset?.geometryColumn, but the geometry selector value uses _sqlroomsBinding.geometryColumn as the preferred source. This mismatch causes the selected geometry column to potentially not appear in the available options. Update the columnsWithVirtual useMemo hook to use the same source preference as the selector value: use _sqlroomsBinding.geometryColumn first, falling back to activeLayerDataset?.geometryColumn, and ensure both the getColumnsWithVirtual call and its dependency array reflect this preferred source. Apply the same fix to the other occurrence of this pattern mentioned at lines 659-666.
🤖 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.
Outside diff comments:
In `@packages/deck/src/MapSettings.tsx`:
- Around line 190-199: The columnsWithVirtual useMemo hook derives virtual
columns from activeLayerDataset?.geometryColumn, but the geometry selector value
uses _sqlroomsBinding.geometryColumn as the preferred source. This mismatch
causes the selected geometry column to potentially not appear in the available
options. Update the columnsWithVirtual useMemo hook to use the same source
preference as the selector value: use _sqlroomsBinding.geometryColumn first,
falling back to activeLayerDataset?.geometryColumn, and ensure both the
getColumnsWithVirtual call and its dependency array reflect this preferred
source. Apply the same fix to the other occurrence of this pattern mentioned at
lines 659-666.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 35d11284-fdb1-40ec-b442-e7968b9b8a6c
📒 Files selected for processing (2)
packages/deck/src/MapSettings.tsxpackages/deck/src/mapLayerConfigUtils.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3d07b245d0
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| getRadius: value, | ||
| radiusMinPixels: 1, |
There was a problem hiding this comment.
When the active scatterplot does not already have radiusUnits: 'pixels' (for example after changing a layer to Point or loading an external Deck spec), this pixel-labeled slider now writes getRadius while also lowering radiusMinPixels to 1. Scatterplot interprets getRadius in the layer's radius units, so choosing 4px can render as a map-unit radius instead of a 4-pixel point. Set radiusUnits: 'pixels' when writing getRadius here, or keep controlling the pixel minimum only.
Useful? React with 👍 / 👎.
| ? getColumnsWithVirtual( | ||
| dataTable.columns, | ||
| activeLayerDataset?.geometryColumn, | ||
| ) |
There was a problem hiding this comment.
When a layer's _sqlroomsBinding.geometryColumn points at a SQL-derived alias that differs from datasets[datasetId].geometryColumn, the selector below now reads the layer binding as its value, but this virtual-column list only appends the dataset-level column. Because dataTable.columns comes from the raw dashboard/source table, that bound alias is shown as missing and cannot be selected even though rendering uses it. Include the effective layer binding here (or include both columns) before building the ColumnsProvider.
Useful? React with 👍 / 👎.
Signed-off-by: Ihor Dykhta <[email protected]>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/deck/src/ai.ts (1)
41-41: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd TSDoc to the exported constant.
The
DECK_MAP_AI_INSTRUCTIONSexport lacks documentation. As per coding guidelines, exported types and functions in@sqlrooms/*packages should have TSDoc.📝 Proposed TSDoc addition
+/** + * AI agent instructions for authoring Deck.gl map configurations. + * + * Provides comprehensive guidance on layer selection, geometry handling, + * data binding, and basemap configuration for dashboard map creation. + */ export const DECK_MAP_AI_INSTRUCTIONS = `🤖 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/deck/src/ai.ts` at line 41, The exported constant DECK_MAP_AI_INSTRUCTIONS is missing TSDoc documentation. Add a TSDoc comment block above the export declaration that describes the purpose and usage of this AI instructions constant. The comment should follow the standard TSDoc format with appropriate tags and a clear description of what these instructions are used for in the deck mapping functionality.Source: Coding guidelines
🤖 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 `@packages/deck/src/ai.ts`:
- Line 41: The exported constant DECK_MAP_AI_INSTRUCTIONS is missing TSDoc
documentation. Add a TSDoc comment block above the export declaration that
describes the purpose and usage of this AI instructions constant. The comment
should follow the standard TSDoc format with appropriate tags and a clear
description of what these instructions are used for in the deck mapping
functionality.
Signed-off-by: Ihor Dykhta <[email protected]>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 17ca3f6ec3
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| @@ -108,7 +108,7 @@ function filterUnavailableLayers( | |||
| return false; | |||
There was a problem hiding this comment.
When this lets a status: 'loading' GeoArrowTripsLayer through, hasTripsLayer later sees the layer in availableSpec and starts the requestAnimationFrame loop, but the converter returns an empty/loading layer until the dataset is ready. Slow trip queries will therefore drive continuous React state updates with no renderable trips; gate trip animation on a ready dataset or on a computed max timestamp instead of keeping all non-error layers active here.
Useful? React with 👍 / 👎.
| (layer) => ({ | ||
| ...layer, | ||
| widthMinPixels: value, | ||
| getWidth: value, |
There was a problem hiding this comment.
When a trips layer does not already have widthUnits: 'pixels' (for example an older or external map spec), this pixel-labeled slider now writes getWidth, but the underlying Trips/Path layer interprets that value in widthUnits and defaults to meters. Selecting 3px can therefore become a 3-meter path width; set widthUnits: 'pixels' here or keep changing only widthMinPixels.
Useful? React with 👍 / 👎.
Signed-off-by: Ihor Dykhta <[email protected]>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a4d2319edd
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| ...(detectedGeometryColumn | ||
| ? {geometryColumn: detectedGeometryColumn.geometryColumn} | ||
| : {}), |
There was a problem hiding this comment.
When a generated map starts from a geometry column and the user later switches it to a point layer and uses the longitude/latitude selectors, regenerateMapConfigForTable preserves spec.layers but replaces the dataset with the synthesized __sqlrooms_geom source. This added layer-level geometryColumn still overrides the updated dataset geometry in resolveGeoArrowBindings, so the scatterplot keeps requesting the old native geometry column and fails even after valid coordinate columns are selected. Either omit this binding for generated single-geometry maps or clear/update it when regenerating coordinates.
Useful? React with 👍 / 👎.
Signed-off-by: Ihor Dykhta <[email protected]>
Signed-off-by: Ihor Dykhta <[email protected]>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ce5292254e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const lineWidthValue = isWidthInPixels | ||
| ? ((activeLayer?.widthMinPixels as number | undefined) ?? 1) | ||
| : ((activeLayer?.getWidth as number | undefined) ?? 1); |
There was a problem hiding this comment.
When an Arc/Trips layer already uses widthUnits: 'pixels' with the documented/generated shape of getWidth: 1–3 and no widthMinPixels, this branch displays and controls the fallback 1px instead of the actual getWidth. The first line-width slider adjustment can therefore overwrite an existing pixel width (for example getWidth: 3) with the fallback value; prefer reading getWidth ?? widthMinPixels for pixel-unit layers.
Useful? React with 👍 / 👎.
Signed-off-by: Ihor Dykhta <[email protected]>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 69e79a3fcf
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| ]} | ||
| max={isRadiusInPixels ? 50 : 10000} | ||
| step={isRadiusInPixels ? 1 : 10} | ||
| value={[pointRadiusValue]} |
There was a problem hiding this comment.
When a scatterplot uses a data-driven radius accessor such as getRadius: '@@=Magnitude' with radiusUnits: 'pixels' (a supported Deck JSON pattern in this repo), pointRadiusValue becomes that string and this passes it to Slider as if it were numeric. The settings panel can render an invalid radius control, and the first slider change overwrites the accessor with a constant getRadius, so guard on typeof getRadius === 'number' or keep this control on pixel min/max when the radius is accessor-based.
Useful? React with 👍 / 👎.
Signed-off-by: Ihor Dykhta <[email protected]>
Signed-off-by: Ihor Dykhta <[email protected]>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: efae4d6513
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| <div className="min-w-0" style={getRampStyle(width)}> | ||
| <div className="border-border/70 flex h-3 overflow-hidden rounded-[2px] border"> | ||
| {legend.items.map((item) => ( | ||
| {visibleItems.map((item) => ( |
There was a problem hiding this comment.
When a quantize/quantile/threshold legend has more than 50 bins, using visibleItems for the top color ramp drops every color after the cap while LegendTicks still represents the full domain. The right edge of the ramp can therefore show a mid-range color as if it were the maximum; keep the ramp based on legend.items and only truncate the scrollable label list.
Useful? React with 👍 / 👎.
Signed-off-by: Ihor Dykhta <[email protected]>
Summary
Improvements to dashboard map settings, legends, heatmap handling, and AI instructions.
Legend improvements
Heatmap color scheme
colorRangeis now YlOrRd (matching deck.gl's built-in default)colorRangeis absent, the rendering pipeline injects the default so the UI scheme selector and actual colors are always in syncupdateTriggersare set to force re-aggregation whencolorRangechangesMap settings panel
pixelsvsmeters), adjusting labels (px/m) and ranges (1–50 for pixels, 1–10000 for meters) accordingly — no more silent unit overridessequentialvscategoricalscale type based on column typeColumnSelector.Quantitative/ColumnSelector.Categoricalwith a genericColumnSelector, since type detection is now automaticclearDeckMapLayerColorScaleforgetFillColornow resets to default blue instead of deleting the accessorGeometry column and fitToData
_sqlroomsBinding.geometryColumnis now explicitly included in generated layer configsfitToDatais now set for geometry-column-backed layers (not just lon/lat)setDeckMapLayerGeometryColumnkeepsfitToData.geometryColumnin sync when the user changes the geometry column in settingsLayer availability
filterUnavailableLayersnow passes through layers when dataset status is noterror(previously requiredready), reducing layer flickering during cross-filteringAI instructions
geometryColumnrule (must match the exact WKB output alias)colorRange, let the default apply)fitToDataformat instruction (must be flat object, not nested by dataset ID)normalizeAiMapConfigto auto-fix malformed nestedfitToDatastructures from AIRemoved (dead import cleanup)
TableColumntype import from MapSettings