feat: add direct worksheet map blocks - #757
Conversation
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds an embeddable ChangesWorksheet Map Block Feature
Sequence Diagram(s)sequenceDiagram
participant User
participant WorksheetAgent
participant createWorksheetMapBlockTool
participant MosaicDashboard
participant WorksheetStore
participant DeckMapBlockRenderer
User->>WorksheetAgent: "Add a map of cities by population"
WorksheetAgent->>createWorksheetMapBlockTool: execute(title, nativeConfig, mapId?)
createWorksheetMapBlockTool->>WorksheetStore: validate artifact, ensure block document
createWorksheetMapBlockTool->>MosaicDashboard: ensureDashboard(mapId)
createWorksheetMapBlockTool->>MosaicDashboard: createDeckMapPanelFromNativeConfig(title, config)
alt new map (no mapId)
createWorksheetMapBlockTool->>MosaicDashboard: addPanel(deckMapPanel)
createWorksheetMapBlockTool->>WorksheetStore: createStatefulBlock(map, blockInstanceId)
else update existing (mapId provided)
createWorksheetMapBlockTool->>MosaicDashboard: updatePanel(existing deckMapPanel)
end
createWorksheetMapBlockTool-->>WorksheetAgent: {success: true, mapId}
WorksheetAgent-->>User: Map block created/updated
User->>DeckMapBlockRenderer: view worksheet with map
DeckMapBlockRenderer->>MosaicDashboard: ensureDeckMapBlockState(state, mapId, title)
DeckMapBlockRenderer->>MosaicDashboard: find deck-map panel
alt geospatial table available
DeckMapBlockRenderer->>DataTableSelector: show table picker
DataTableSelector-->>User: select table to populate map
else no data
DeckMapBlockRenderer->>DeckMapBlockRenderer: render empty map
end
DeckMapBlockRenderer-->>User: render Deck.gl map panel
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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: 7ced270153
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const mapId = params.mapId ?? createDefaultBlockDocumentBlockId(); | ||
| const title = params.title || 'Map'; | ||
| state.mosaicDashboard.ensureDashboard(mapId, title, 'grid'); |
There was a problem hiding this comment.
When mapId is supplied but is stale or not actually present as a map block in this worksheet, this call creates a backing dashboard for that arbitrary ID; because the later append is skipped whenever params.mapId is truthy, the tool can return success while no visible worksheet block exists. For update requests, verify that mapId belongs to an existing map stateful block in the target worksheet (or fail) before creating/updating dashboard state.
Useful? React with 👍 / 👎.
| const tableName = params.tableName; | ||
| if (tableName) { | ||
| state.db.findTable(tableName); | ||
| } |
There was a problem hiding this comment.
When the optional tableName is misspelled or no longer exists, db.findTable just returns undefined, but execution continues and the same value is stored as the map dashboard's selected table. The deck map renderer gives the selected table precedence over dataset sources, so a bad tableName can make an otherwise valid map config fail at query time instead of returning a tool error.
Useful? React with 👍 / 👎.
![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
🧹 Nitpick comments (1)
apps/sqlrooms-cli-ui/src/workspace/WorksheetMapBlockRenderer.tsx (1)
4-9: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winAdd TSDoc to the exported renderer component.
WorksheetMapBlockRendereris exported but undocumented in a TSX file.📝 Proposed fix
+/** + * Renders a worksheet `map` stateful block using the Deck embeddable map surface. + */ export const WorksheetMapBlockRenderer = ({ blockInstanceId, blockType, title, caption, }: BlockDocumentStatefulBlockRendererProps) => {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 `@apps/sqlrooms-cli-ui/src/workspace/WorksheetMapBlockRenderer.tsx` around lines 4 - 9, The exported function WorksheetMapBlockRenderer is missing TSDoc documentation. Add a TSDoc comment block above the function export that documents what the component does, describes the BlockDocumentStatefulBlockRendererProps parameter including its key properties (blockInstanceId, blockType, title, caption), and notes the return type. Use the standard TSDoc format with `@param` and `@returns` tags to provide comprehensive documentation for this 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 `@apps/sqlrooms-cli-ui/src/createWorksheetAgent.ts`:
- Around line 64-69: When params.mapId is supplied, validate that the provided
mapId actually belongs to a blockType: 'map' block in the target worksheet
before proceeding with the mosaicDashboard.ensureDashboard and
mosaicDashboard.setSelectedTable calls. Add a validation check that confirms the
mapId ownership by querying the worksheet state to verify the block exists and
has blockType set to 'map', and only execute the dashboard mutations if
validation succeeds. This same validation pattern should also be applied to the
other locations mentioned in the comment (lines 101-114).
In `@packages/mosaic/src/ai/worksheet/createWorksheetAgentTool.ts`:
- Around line 148-152: The map routing guidance contains contradictory
instructions across multiple locations. The conditional logic for
mapBlocksEnabled in the diff (lines 148-152) correctly routes to the direct
worksheet map tool when enabled, but other instruction surfaces at line 222 and
lines 291-304 still hardcode the dashboard-map workflow path regardless of the
mapBlocksEnabled flag. Update all other map-related instruction blocks (at line
222 and the tool description sections around lines 291-304) to include the same
mapBlocksEnabled conditional logic so that when mapBlocksEnabled is true, they
all direct users to the direct worksheet map tool instead of the dashboard path,
ensuring consistent agent behavior across all instruction surfaces.
---
Nitpick comments:
In `@apps/sqlrooms-cli-ui/src/workspace/WorksheetMapBlockRenderer.tsx`:
- Around line 4-9: The exported function WorksheetMapBlockRenderer is missing
TSDoc documentation. Add a TSDoc comment block above the function export that
documents what the component does, describes the
BlockDocumentStatefulBlockRendererProps parameter including its key properties
(blockInstanceId, blockType, title, caption), and notes the return type. Use the
standard TSDoc format with `@param` and `@returns` tags to provide comprehensive
documentation for this 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: e7e9159a-62bd-49ac-a472-757102597888
📒 Files selected for processing (15)
apps/sqlrooms-cli-ui/src/createWorksheetAgent.tsapps/sqlrooms-cli-ui/src/statefulBlockArtifactConfigs.tsapps/sqlrooms-cli-ui/src/store.tsapps/sqlrooms-cli-ui/src/workspace/WorksheetArtifact.tsxapps/sqlrooms-cli-ui/src/workspace/WorksheetMapBlockRenderer.tsxpackages/deck/README.mdpackages/deck/__tests__/dashboard-ai-map.test.tspackages/deck/src/ai.tspackages/deck/src/block.tsxpackages/deck/src/dashboard.tsxpackages/deck/src/index.tspackages/mosaic/__tests__/worksheet-ai.test.tspackages/mosaic/src/ai/worksheet/createListWorksheetBlocksTool.tspackages/mosaic/src/ai/worksheet/createWorksheetAgentTool.tspackages/mosaic/src/ai/worksheet/worksheet-types.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e87564a3eb
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (existingPanel) { | ||
| state.mosaicDashboard.updatePanel(mapId, existingPanel.id, { | ||
| title: panel.title, | ||
| config: panel.config, | ||
| }); |
There was a problem hiding this comment.
When mapId is supplied for an existing worksheet map and the caller changes the map title, this branch updates only the backing Deck panel. Newly created map blocks persist both title and caption, and DeckMapBlockRenderer displays caption || title || panel.title, so the worksheet block header continues to show the old title even though the tool reports that it updated the worksheet map. In the update path, also update the matching map stateful block attrs or stop letting the stale block caption override the panel title.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e5e3125bd5
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (tableName) { | ||
| state.mosaicDashboard.setSelectedTable(mapId, tableName); | ||
| } |
There was a problem hiding this comment.
When updating an existing worksheet map and the caller omits the optional top-level tableName, this leaves the backing dashboard's previous selectedTable in place even if params.config.datasets now points at a different table. resolveDeckMapDashboardDatasetSource gives dashboard.selectedTable precedence over dataset source.tableName, so an edit from old_table to new_table can keep querying old_table while the tool returns success. Mirror createDeckMapDashboardTool by falling back to the first dataset source table before validating/setting the selection, or require tableName for updates that change data sources.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 93f9be5edf
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const tableName = | ||
| params.tableName ?? getFirstDatasetSourceTableName(params.config); |
There was a problem hiding this comment.
When updating an existing worksheet map, this fallback still returns undefined for configs whose dataset source is only sqlQuery (a supported form and common for point maps), so the backing dashboard keeps its previous selectedTable. At render time resolveDeckMapDashboardDatasetSource rewrites SQL-only sources to the dashboard's selected table when one exists, so an edit from old_table to a SQL query over new_table can silently keep querying old_table unless the agent also supplied the optional top-level tableName. The fresh evidence is that SQL-only dataset sources still bypass this fallback because getFirstDatasetSourceTableName only reads source.tableName.
Useful? React with 👍 / 👎.
1 participant
Summary
mapstateful worksheet block in the CLI app.createDeckMapPanelFromNativeConfigfor host surfaces that need the same normalized Deck map panel shape as dashboard map tooling.Impact
Worksheet users can add or generate maps directly as worksheet blocks. Existing dashboard map panels continue to use the same renderer/config path.
Validation
pnpm buildpnpm --filter sqlrooms-cli-app typecheckpnpm --filter @sqlrooms/deck testpnpm --filter @sqlrooms/mosaic test -- worksheetknip,turbo typecheck,turbo testSummary by CodeRabbit