Repository navigation
docs: LLM-friendly documentation - #375
Merged
Merged
Conversation
Co-authored-by: iboyandin <[email protected]>
|
Cursor Agent can help with this pull request. Just |
Contributor
|
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:
📝 WalkthroughWalkthroughUpdates example AI model defaults to the GPT‑5 series, restructures the docs build pipeline, removes a legacy custom-slice doc, and performs large README rewrites across many packages to emphasize store-centric patterns and adjusted public API surfaces; also adds llmstxt plugin options and minor UI and gitignore tweaks. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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: Ilya Boyandin <[email protected]>
Signed-off-by: Ilya Boyandin <[email protected]>
…qlrooms/sqlrooms into cursor/llm-friendly-documentation-d7cb
…qlrooms/sqlrooms into cursor/llm-friendly-documentation-d7cb
Signed-off-by: Ilya Boyandin <[email protected]>
…qlrooms/sqlrooms into cursor/llm-friendly-documentation-d7cb
Signed-off-by: Ilya Boyandin <[email protected]>
Signed-off-by: Ilya Boyandin <[email protected]>
ilyabo
marked this pull request as ready for review
February 23, 2026 12:16
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/duckdb/README.md (2)
278-308:⚠️ Potential issue | 🟡 Minor
connectormay benull/undefinedat first render — null guard missing
state.db.connectoraccessed synchronously viauseRoomStorecan be uninitialised when the component first renders, makingconnector.loadFile(...)andconnector.loadArrow(...)throw a TypeError in users' code copied from this example.Additionally, the
DataLoadersection above uses the safergetConnector()async pattern while this section uses a direct, unguarded sync reference — the inconsistency is likely to confuse readers.📝 Suggested fix
function AdvancedDataLoader() { - const connector = useRoomStore((state) => state.db.connector); + const getConnector = useRoomStore((state) => state.db.getConnector); const handleFileUpload = async (file: File) => { - // Load file directly using the connector - await connector.loadFile(file, 'uploaded_data', { + const connector = await getConnector(); + await connector.loadFile(file, 'uploaded_data', { method: 'auto', replace: true, temp: false, }); }; - const handleLoadArrowTable = async (arrowTable: arrow.Table) => { - // Load Arrow table directly - await connector.loadArrow(arrowTable, 'arrow_data'); + const handleLoadArrowTable = async (arrowTable: arrow.Table) => { + const connector = await getConnector(); + await connector.loadArrow(arrowTable, 'arrow_data'); };Based on learnings, direct connector access (
state.db.connector) is not recommended in React contexts; prefer custom Zustand selectors likeuseDuckDb()or thegetConnector()async accessor over raw store state access.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/duckdb/README.md` around lines 278 - 308, The example accesses useRoomStore((state) => state.db.connector) synchronously which can be null/undefined on first render; update the AdvancedDataLoader to guard against a missing connector or, better, switch to the async accessor pattern used elsewhere (getConnector) or the safer custom selector (useDuckDb). Specifically, replace direct uses of connector.loadFile and connector.loadArrow with logic that awaits getConnector() (or fetches via useDuckDb()) and only calls loadFile/loadArrow when the returned connector is non-null, and ensure the file input handler handles the async call and any errors.
206-222:⚠️ Potential issue | 🟡 MinorIncomplete snippet —
createTableFromQuery,dropTable, andcheckTableExistslack a sourceThe snippet calls three functions (
createTableFromQuery,dropTable,checkTableExists) that are never imported or obtained from the store, leaving readers with no way to reproduce the example. These are accessed from the store in theDatabaseManagercomponent above; the qualified-name example should either reuse that component's pattern or show a minimal store access setup.📝 Suggested fix
import {makeQualifiedTableName} from '@sqlrooms/duckdb'; +import {useRoomStore} from './store'; // Support for database.schema.table naming const qualifiedTable = makeQualifiedTableName({ database: 'mydb', schema: 'public', table: 'users', }); -// Use with table operations -await createTableFromQuery(qualifiedTable, 'SELECT * FROM source_table'); -await dropTable(qualifiedTable); -const tableExists = await checkTableExists(qualifiedTable); +// Obtain methods from the store +const {createTableFromQuery, dropTable, checkTableExists} = + useRoomStore.getState().db; + +await createTableFromQuery(qualifiedTable, 'SELECT * FROM source_table'); +await dropTable(qualifiedTable); +const tableExists = await checkTableExists(qualifiedTable);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/duckdb/README.md` around lines 206 - 222, The example using makeQualifiedTableName is incomplete because createTableFromQuery, dropTable, and checkTableExists are not imported or obtained from the store; update the snippet to show how to get these functions (e.g., from the same store/DatabaseManager pattern used above) or add explicit imports/initialization before calling them. Specifically, reference the makeQualifiedTableName usage and show obtaining createTableFromQuery, dropTable, and checkTableExists from the store (or demonstrate DatabaseManager instantiation/usage) so the example is runnable and self-contained.
🧹 Nitpick comments (8)
packages/ai-core/README.md (1)
9-9: Consider clarifying why@sqlrooms/uiis a direct install requirement.Neither code example imports from
@sqlrooms/uidirectly — Chat components come from@sqlrooms/ai-core. If it's a required peer dependency for the Chat UI to render correctly, a brief parenthetical note (e.g.,# peer dep for Chat UI) would help readers understand why it's listed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ai-core/README.md` at line 9, Clarify why `@sqlrooms/ui` is listed in the install command by adding a short parenthetical note after it explaining its role (for example: "(peer dependency required for Chat UI rendering)"), and mention that Chat components are exported from `@sqlrooms/ai-core` so readers know they typically import from `@sqlrooms/ai-core` while `@sqlrooms/ui` must be installed for styling/visual components to work; update the README line with this concise explanation referencing the package names `@sqlrooms/ai-core` and `@sqlrooms/ui`.packages/kepler-config/README.md (1)
9-12: Add usage context forKeplerMapSchemaexport.
KeplerMapSchemais listed in Exports but has no accompanying example explaining when or how to use it. A brief note clarifying that it defines the schema for individual map objects withinKeplerSliceConfigwould improve discoverability.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/kepler-config/README.md` around lines 9 - 12, Add a short usage note under the Exports section explaining that KeplerMapSchema describes the schema for individual map objects used inside KeplerSliceConfig and when to use it; mention that KeplerSliceConfig is the container for multiple KeplerMapSchema entries and that KeplerMapSchema should be used when constructing or validating each map object (e.g., defining layers, datasource, and viewport) to improve discoverability for users.packages/recharts/README.md (1)
65-81: The second code block is missing theRevenueChartimport, which could produce incomplete LLM-generated code.
RevenueChartis used on line 79 but not imported in this snippet. Since the README goal is LLM discoverability, each code block should be independently usable or include a clear import reference.📝 Suggested fix
import {useSql} from '@sqlrooms/duckdb'; +import {RevenueChart} from './RevenueChart'; // defined in the Basic usage example above function RevenueChartFromSql() {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/recharts/README.md` around lines 65 - 81, The snippet for RevenueChartFromSql uses RevenueChart but doesn't import it; update the example to include an explicit import for RevenueChart so the block is independently usable—e.g., add an import line referencing RevenueChart (the component used by RevenueChartFromSql) at the top of the snippet alongside the existing useSql import so readers/LLMs can run or copy the example without missing symbols.packages/layout-config/README.md (2)
44-53: Add an import comment noting the origin ofcreateRoomShellSlice.The snippet references
createRoomShellSlice,DataPanel, andMainPanelwithout context. For a "Typical integration" example, at least a comment noting the source package prevents readers from searching unnecessarily.📝 Proposed addition
+// createRoomShellSlice is from '@sqlrooms/room-shell' +// DataPanel / MainPanel are your own React components createRoomShellSlice({ layout: { config: twoPaneLayout,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/layout-config/README.md` around lines 44 - 53, Add a brief import comment above the example indicating where createRoomShellSlice (and the example components DataPanel and MainPanel) come from—e.g., note that createRoomShellSlice is exported by the layout-config package (or the actual module name) and that DataPanel/MainPanel are example components—so readers know which package/module to import from when using the "Typical integration" snippet referencing createRoomShellSlice, DataPanel, and MainPanel.
27-27:minimalLayoutis declared but never used in the example.The variable is assigned but plays no role in the rest of the snippet, making it dead code in the docs. Either use it (e.g., show it being passed to
LayoutConfig.parse) or remove it along with thecreateDefaultMosaicLayoutimport.📝 Option A — remove unused variable
-import { - LayoutConfig, - MAIN_VIEW, - MosaicLayoutConfig, - createDefaultMosaicLayout, -} from '@sqlrooms/layout-config'; - -const minimalLayout = createDefaultMosaicLayout(); +import { + LayoutConfig, + MAIN_VIEW, + MosaicLayoutConfig, +} from '@sqlrooms/layout-config';📝 Option B — demonstrate the minimal layout too
const minimalLayout = createDefaultMosaicLayout(); +const validatedMinimal: LayoutConfig = LayoutConfig.parse(minimalLayout);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/layout-config/README.md` at line 27, The README shows minimalLayout = createDefaultMosaicLayout() but never uses it; either remove the declaration and the createDefaultMosaicLayout import, or demonstrate its use by passing minimalLayout into LayoutConfig.parse (e.g., call LayoutConfig.parse(minimalLayout) or include it in the example flow) so the example is not dead code; update the example to reference the minimalLayout variable and/or remove the unused import and variable accordingly.packages/discuss/README.md (2)
40-47: Consider documenting the requiredroom.initialize()call.The README covers store creation but omits the mandatory
room.initialize()invocation at component mount. Readers who follow only this README would skip slice lifecycle initialization.A minimal addition to the store-setup section (or a separate "Initialization" snippet) would be:
import {useEffect} from 'react'; import {roomStore} from './store'; function App() { useEffect(() => { roomStore.getState().room.initialize(); }, []); // ... }Based on learnings: always call
room.initialize()during component mount to orchestrate automatic slice initialization with lifecycle hooks.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/discuss/README.md` around lines 40 - 47, Add documentation and an example showing that room.initialize() must be called after creating the store so slices run their lifecycle hooks; update the README around the store setup where createRoomShellSlice and createDiscussSlice are shown to include a short "Initialization" note and a minimal snippet that calls roomStore.getState().room.initialize() on component mount (e.g., inside useEffect) so readers know to invoke room.initialize() after store creation.
92-94: The second argument toaddDiscussionis opaque without a comment.
'airport-LAX'is used as an anchor ID, which is the core linking mechanism described in the package intro, but it reads like an accidental leftover from a real demo. A brief inline comment (e.g.// anchor/data-point ID) or a more generic placeholder like'data-point-id'would make the intent immediately clear to readers.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/discuss/README.md` around lines 92 - 94, The literal string 'airport-LAX' passed as the second argument to addDiscussion is opaque; update the example in the README where addDiscussion('Initial note', 'airport-LAX') is used to make intent clear by either replacing 'airport-LAX' with a generic placeholder like 'data-point-id' or adding an inline comment after the argument (e.g., // anchor/data-point ID) so readers know this parameter is the anchor/data-point ID used for linking; refer to the addDiscussion call site to apply the change.package.json (1)
34-36: LGTM — clean two-phase docs build.The split lets each phase be run independently for faster iteration. One minor note:
docs:build:typedocinvokespnpm --filter "@sqlrooms/*" typedocdirectly instead ofturbo typedoc, which bypasses turborepo's dependency-graph ordering and remote cache. This is fine for a purely source-reading typedoc pass, but aligns less with howtypedoc:watchis wired (turbo typedoc --concurrency=100). If any cross-package typedoc references are added later, the turbo-based invocation would be safer.🔧 Optional: keep turbo for consistency with typedoc:watch
- "docs:build:typedoc": "pnpm --filter \"@sqlrooms/*\" typedoc", + "docs:build:typedoc": "turbo typedoc --filter=\"@sqlrooms/*\"",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` around lines 34 - 36, The docs build currently runs typedoc directly via the "docs:build:typedoc" npm script which bypasses Turborepo's dependency graph and remote cache; update the "docs:build:typedoc" script to invoke Turbo (e.g., use "turbo typedoc" — optionally with the same flag used by watch like "--concurrency=100") so that "docs:build:typedoc" and the combined "docs:build" benefit from Turborepo ordering and caching.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/ai-core/README.md`:
- Line 9: The README's npm install snippet is missing the zod dependency
required by the import "z" used elsewhere; update the install command shown (the
line containing "npm install `@sqlrooms/ai-core` `@sqlrooms/room-store`
`@sqlrooms/ui`") to also include "zod" so consumers won't get a module-not-found
error when importing z from 'zod'.
In `@packages/ai/README.md`:
- Around line 126-136: The remote-endpoint example uses createAiSlice({...}) but
fails to invoke the returned slice factory with (set, get, store); update the
snippet so the result of createAiSlice (with tools, getInstructions,
chatEndPoint, chatHeaders) is called with (set, get, store) and its returned
state is spread where other examples do so — locate createAiSlice,
createDefaultAiTools, createDefaultAiInstructions, chatEndPoint and chatHeaders
in the snippet and replace the dangling factory reference with a proper
invocation: call the factory with (set, get, store) and spread its result into
the final state object.
In `@packages/canvas/README.md`:
- Around line 57-62: The README example still sets defaultModel to the retired
'gpt-4.1-mini'; update the example in the createCanvasSlice call to use a
currently supported model (e.g., 'gpt-5.2-mini' or your project's recommended
small model). Locate the createCanvasSlice({ ai: { getApiKey: ..., defaultModel:
... } })(set, get, store) snippet and replace the defaultModel value with the
chosen current model string, and optionally add a brief inline note indicating
it should match the project's configured recommended model.
In `@packages/cosmos/README.md`:
- Around line 34-36: Replace the deprecated dependency
`@cosmograph/cosmos`@2.0.0-beta.26 in the `@sqlrooms/cosmos` package.json with the
maintained package `@cosmos.gl/graph`, and update the README import line that
currently reads import {GraphConfigInterface} from '@cosmograph/cosmos' to
import {GraphConfigInterface} from '@cosmos.gl/graph'; ensure package.json lists
`@cosmos.gl/graph` as a regular dependency and the README example matches that new
package name.
In `@packages/discuss/README.md`:
- Around line 7-9: The README's install command omits the `@sqlrooms/utils`
package even though the example imports formatTimeRelative from `@sqlrooms/utils`;
update the npm install line to include `@sqlrooms/utils` so users won't get a
missing-module error, and ensure any example import of formatTimeRelative
matches the packages listed in the install snippet.
In `@packages/duckdb-config/README.md`:
- Line 1: The README is missing the top-level package name heading; add a H1
heading line "# `@sqlrooms/duckdb-config`" at the very top of
packages/duckdb-config/README.md so the file begins with the package name
(followed by the existing Status line), ensuring LLMs and registries can
identify the package; keep the rest of the content unchanged.
In `@packages/kepler/README.md`:
- Around line 53-77: MapPanel currently passes mapId (possibly undefined) to
KeplerMapContainer which expects a string; update MapPanel to guard render until
mapId is defined by checking mapId before returning the container (e.g., if
(!mapId) return null or a loading placeholder) so KeplerMapContainer always
receives a string, leaving the existing useEffect (which already guards on
mapId) unchanged; locate the check and return in the MapPanel function where
mapId is read from useRoomStore and adjust the render to only return
<KeplerMapContainer mapId={mapId} /> when mapId is a string.
In `@packages/recharts/README.md`:
- Line 51: The README's BarChart example omits the accessibilityLayer prop, so
update the example that renders <BarChart data={rows}> to include
accessibilityLayer (e.g., accessibilityLayer={true}) on the <BarChart> element
to enable keyboard and screen-reader support; change the example usage of
BarChart in the README (the <BarChart data={rows}> snippet) to include this prop
so generated code is accessible by default.
- Around line 42-44: The README example uses a fixed height class on
ChartContainer which prevents responsive behavior; update the ChartContainer
usage (look for the ChartContainer component and its className prop) to replace
the fixed "h-64" with a minimum-height utility like "min-h-[VALUE]" (e.g.,
min-h-[16rem] or another appropriate VALUE) while keeping "w-full" so the chart
is responsive in flexible layouts.
In `@packages/utils/README.md`:
- Around line 29-30: Update the examples for formatDate and formatDateTime to
show concrete output values instead of format descriptors: replace
formatDate(new Date()); // "YYYY-MM-DD" with a real example like formatDate(new
Date()); // "2026-02-23" and replace formatDateTime(new Date()); // "Wed
YYYY-MM-DD HH:MM AM/PM" with a real example like formatDateTime(new Date()); //
"Wed 2026-02-23 02:15 PM" (or another realistic date/time) so the README
demonstrates actual returned strings for the formatDate and formatDateTime
functions.
In `@packages/vega/README.md`:
- Around line 51-62: The compound component JSX is a bare expression and should
be wrapped in a functional component so it can be copied and used; create an
exported function (e.g., export function CompoundVegaChart() or similar) that
returns the <VegaLiteChart.Container ...>…</VegaLiteChart.Container> JSX, remove
the trailing semicolon after the closing tag, and ensure the example mentions
any necessary imports (e.g., VegaLiteChart) so readers can paste the snippet
directly into a file; reference the Container, Actions, Chart, SpecEditor and
SqlEditor symbols when locating the snippet to wrap.
- Around line 88-92: The README currently conflates createVegaChartTool
constructor options (e.g., editable, editorMode) with the runtime Zod schema
fields the LLM provides; update the documentation to separate those concerns:
keep the existing constructor/options heading for createVegaChartTool and its
args (editable, editorMode), and add a distinct “LLM invocation / Zod schema
fields” subsection that explicitly states sqlQuery, vegaLiteSpec, and reasoning
are fields supplied at runtime by the tool’s Zod schema (not parameters to pass
into createVegaChartTool), referencing createVegaChartTool, sqlQuery,
vegaLiteSpec, reasoning, and the Zod schema to make the distinction unambiguous.
---
Outside diff comments:
In `@packages/duckdb/README.md`:
- Around line 278-308: The example accesses useRoomStore((state) =>
state.db.connector) synchronously which can be null/undefined on first render;
update the AdvancedDataLoader to guard against a missing connector or, better,
switch to the async accessor pattern used elsewhere (getConnector) or the safer
custom selector (useDuckDb). Specifically, replace direct uses of
connector.loadFile and connector.loadArrow with logic that awaits getConnector()
(or fetches via useDuckDb()) and only calls loadFile/loadArrow when the returned
connector is non-null, and ensure the file input handler handles the async call
and any errors.
- Around line 206-222: The example using makeQualifiedTableName is incomplete
because createTableFromQuery, dropTable, and checkTableExists are not imported
or obtained from the store; update the snippet to show how to get these
functions (e.g., from the same store/DatabaseManager pattern used above) or add
explicit imports/initialization before calling them. Specifically, reference the
makeQualifiedTableName usage and show obtaining createTableFromQuery, dropTable,
and checkTableExists from the store (or demonstrate DatabaseManager
instantiation/usage) so the example is runnable and self-contained.
---
Nitpick comments:
In `@package.json`:
- Around line 34-36: The docs build currently runs typedoc directly via the
"docs:build:typedoc" npm script which bypasses Turborepo's dependency graph and
remote cache; update the "docs:build:typedoc" script to invoke Turbo (e.g., use
"turbo typedoc" — optionally with the same flag used by watch like
"--concurrency=100") so that "docs:build:typedoc" and the combined "docs:build"
benefit from Turborepo ordering and caching.
In `@packages/ai-core/README.md`:
- Line 9: Clarify why `@sqlrooms/ui` is listed in the install command by adding a
short parenthetical note after it explaining its role (for example: "(peer
dependency required for Chat UI rendering)"), and mention that Chat components
are exported from `@sqlrooms/ai-core` so readers know they typically import from
`@sqlrooms/ai-core` while `@sqlrooms/ui` must be installed for styling/visual
components to work; update the README line with this concise explanation
referencing the package names `@sqlrooms/ai-core` and `@sqlrooms/ui`.
In `@packages/discuss/README.md`:
- Around line 40-47: Add documentation and an example showing that
room.initialize() must be called after creating the store so slices run their
lifecycle hooks; update the README around the store setup where
createRoomShellSlice and createDiscussSlice are shown to include a short
"Initialization" note and a minimal snippet that calls
roomStore.getState().room.initialize() on component mount (e.g., inside
useEffect) so readers know to invoke room.initialize() after store creation.
- Around line 92-94: The literal string 'airport-LAX' passed as the second
argument to addDiscussion is opaque; update the example in the README where
addDiscussion('Initial note', 'airport-LAX') is used to make intent clear by
either replacing 'airport-LAX' with a generic placeholder like 'data-point-id'
or adding an inline comment after the argument (e.g., // anchor/data-point ID)
so readers know this parameter is the anchor/data-point ID used for linking;
refer to the addDiscussion call site to apply the change.
In `@packages/kepler-config/README.md`:
- Around line 9-12: Add a short usage note under the Exports section explaining
that KeplerMapSchema describes the schema for individual map objects used inside
KeplerSliceConfig and when to use it; mention that KeplerSliceConfig is the
container for multiple KeplerMapSchema entries and that KeplerMapSchema should
be used when constructing or validating each map object (e.g., defining layers,
datasource, and viewport) to improve discoverability for users.
In `@packages/layout-config/README.md`:
- Around line 44-53: Add a brief import comment above the example indicating
where createRoomShellSlice (and the example components DataPanel and MainPanel)
come from—e.g., note that createRoomShellSlice is exported by the layout-config
package (or the actual module name) and that DataPanel/MainPanel are example
components—so readers know which package/module to import from when using the
"Typical integration" snippet referencing createRoomShellSlice, DataPanel, and
MainPanel.
- Line 27: The README shows minimalLayout = createDefaultMosaicLayout() but
never uses it; either remove the declaration and the createDefaultMosaicLayout
import, or demonstrate its use by passing minimalLayout into LayoutConfig.parse
(e.g., call LayoutConfig.parse(minimalLayout) or include it in the example flow)
so the example is not dead code; update the example to reference the
minimalLayout variable and/or remove the unused import and variable accordingly.
In `@packages/recharts/README.md`:
- Around line 65-81: The snippet for RevenueChartFromSql uses RevenueChart but
doesn't import it; update the example to include an explicit import for
RevenueChart so the block is independently usable—e.g., add an import line
referencing RevenueChart (the component used by RevenueChartFromSql) at the top
of the snippet alongside the existing useSql import so readers/LLMs can run or
copy the example without missing symbols.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (40)
docs/.vitepress/config.tsdocs/custom-slice.mddocs/llms.mdexamples/ai-agent/src/agents/WeatherAgent.tsexamples/ai-agent/src/config.tsexamples/ai-core/src/config.tsexamples/ai-nextjs/config.tsexamples/ai-rag/src/config.tsexamples/ai/src/config.tsexamples/ai/src/example-sessions.jsonexamples/app-builder/src/config.tsexamples/canvas-sync/src/store.tsexamples/canvas/src/store.tspackage.jsonpackages/ai-config/README.mdpackages/ai-core/README.mdpackages/ai-core/src/chatTransport.tspackages/ai-settings/README.mdpackages/ai/README.mdpackages/canvas/README.mdpackages/cosmos/README.mdpackages/crdt/README.mdpackages/discuss/README.mdpackages/duckdb-config/README.mdpackages/duckdb/README.mdpackages/kepler-config/README.mdpackages/kepler/README.mdpackages/layout-config/README.mdpackages/layout/README.mdpackages/motherduck/README.mdpackages/recharts/README.mdpackages/room-config/README.mdpackages/room-shell/README.mdpackages/room-store/README.mdpackages/schema-tree/README.mdpackages/sql-editor-config/README.mdpackages/sql-editor/README.mdpackages/utils/README.mdpackages/vega/README.mdpython/.gitignore
💤 Files with no reviewable changes (2)
- docs/custom-slice.md
- packages/crdt/README.md
Comment on lines
+34
to
+36
| import {CosmosGraph, CosmosGraphControls, CosmosSimulationControls} from '@sqlrooms/cosmos'; | ||
| import {GraphConfigInterface} from '@cosmograph/cosmos'; | ||
|
|
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name package.json -path "*/cosmos/*" | head -20Repository: sqlrooms/sqlrooms
Length of output: 122
🏁 Script executed:
cat ./packages/cosmos/package.jsonRepository: sqlrooms/sqlrooms
Length of output: 946
🌐 Web query:
@cosmos.gl/graph @cosmograph/cosmos package migration rename
💡 Result:
@cosmograph/cosmos has been deprecated and moved to @cosmos.gl/graph. The npm deprecation notice for @cosmograph/cosmos explicitly says it was moved to @cosmos.gl/graph. [1]
Migration / rename
-
Replace the dependency
- Remove:
@cosmograph/cosmos - Add:
@cosmos.gl/graph[2]
- Remove:
-
Update imports
// before import { Graph } from '@cosmograph/cosmos' // after import { Graph } from '@cosmos.gl/graph'
(The
Graphentry point is shown in the@cosmos.gl/graphquick start.) [2] -
If you’re also upgrading across v2 changes
@cosmos.gl/graphv2+ highlights API changes like usingsetPointPositions()/setLinks()instead of older data-setting patterns; their README points to a “Migration Guide” for details. [2]
The project docs/wiki also distinguish latest (2.x) vs legacy (1.x) and include a migration guide section. [3]
Sources: npm package pages and project wiki. [1] [2] [3]
🏁 Script executed:
sed -n '1,50p' ./packages/cosmos/README.mdRepository: sqlrooms/sqlrooms
Length of output: 1293
Update @sqlrooms/cosmos to use the non-deprecated @cosmos.gl/graph package.
@cosmograph/cosmos has been deprecated and moved to @cosmos.gl/graph. The @sqlrooms/cosmos package.json currently lists @cosmograph/[email protected] as a regular dependency. This should be updated to @cosmos.gl/graph to use the maintained package going forward, and the README example should then import from @cosmos.gl/graph instead:
import {GraphConfigInterface} from '@cosmos.gl/graph';The install command on line 6 is correct as-is; regular dependencies like @cosmograph/cosmos are automatically installed when users install @sqlrooms/cosmos.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/cosmos/README.md` around lines 34 - 36, Replace the deprecated
dependency `@cosmograph/cosmos`@2.0.0-beta.26 in the `@sqlrooms/cosmos` package.json
with the maintained package `@cosmos.gl/graph`, and update the README import line
that currently reads import {GraphConfigInterface} from '@cosmograph/cosmos' to
import {GraphConfigInterface} from '@cosmos.gl/graph'; ensure package.json lists
`@cosmos.gl/graph` as a regular dependency and the README example matches that new
package name.
Contributor
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 (2)
packages/duckdb/README.md (2)
394-397:⚠️ Potential issue | 🟡 Minor
useRoomStoreis used but never imported in this code block.Every other
useRoomStoreexample in the file begins withimport {useRoomStore} from './store';. This block is the only one missing it.🐛 Proposed fix
+import {useRoomStore} from './store'; + function AdvancedOperations() { const executeSql = useRoomStore((s) => s.db.executeSql);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/duckdb/README.md` around lines 394 - 397, The AdvancedOperations example uses useRoomStore but doesn't import it; add the missing import for useRoomStore (the same as other examples) at the top of the snippet so AdvancedOperations can call useRoomStore to get executeSql, sqlSelectToJson and checkTableExists; specifically, insert the import { useRoomStore } from './store'; line so the useRoomStore references in AdvancedOperations resolve.
278-280:⚠️ Potential issue | 🟡 MinorFloating promise — missing
voidunlike theAdvancedDataLoaderexample below.
handleLoadCSVis async; calling it withoutvoiddrops the promise and is inconsistent with lines 324–326 which correctly writevoid handleFileUpload(...).🐛 Proposed fix
- if (e.target.files?.[0]) handleLoadCSV(e.target.files[0]); + if (e.target.files?.[0]) void handleLoadCSV(e.target.files[0]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/duckdb/README.md` around lines 278 - 280, The onChange handler is calling the async function handleLoadCSV without awaiting or prefixing with void, creating a floating promise; update the handler so the call is prefixed with void (i.e., void handleLoadCSV(e.target.files[0])) to mirror the AdvancedDataLoader pattern and intentionally ignore the returned promise, keeping handleLoadCSV as the invoked async function and preserving the existing null-check (e.target.files?.[0]).
♻️ Duplicate comments (6)
packages/kepler/README.md (1)
53-71:mapIdguard is now in place — LGTM.The
if (!mapId) return null;check at line 68 ensuresKeplerMapContaineralways receives astring, resolving the previously flagged type mismatch.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/kepler/README.md` around lines 53 - 71, MapPanel previously risked passing an undefined mapId into KeplerMapContainer; ensure the guard is present by checking mapId before rendering (as in the current MapPanel implementation) so KeplerMapContainer always receives a string; confirm the early return "if (!mapId) return null;" remains in the MapPanel function and that dependencies (mapId, isTableReady, addTableToMap) are used in the useEffect that calls addTableToMap.packages/vega/README.md (2)
92-105: ✅ Past issue resolved — constructor options and LLM schema fields are now clearly separated.The "### LLM invocation / Zod schema fields" sub-heading at line 97 makes the distinction explicit. No further action needed here.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/vega/README.md` around lines 92 - 105, The README correctly separates constructor options for createVegaChartTool from the runtime LLM-invoked Zod schema fields; no code changes are required—verify that the createVegaChartTool documentation (editable, editorMode) remains distinct from the LLM payload section (sqlQuery, vegaLiteSpec, reasoning) and keep the "LLM invocation / Zod schema fields" heading intact to prevent future regressions.
51-66: ✅ Past issue resolved — compound component is now properly wrapped.The previous bare JSX expression has been replaced with a valid exported function component
CompoundVegaChart. The trailing semicolon is also gone. No further action needed here.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/vega/README.md` around lines 51 - 66, The diff shows the JSX was wrapped into a proper exported React function component CompoundVegaChart that returns VegaLiteChart.Container with child components (VegaLiteChart.Actions, VegaLiteChart.Chart, VegaLiteChart.SpecEditor, VegaLiteChart.SqlEditor); no further code changes are required—keep the exported function CompoundVegaChart as-is and ensure the referenced symbols (VegaLiteChart.Container, initialSpec, and the onSpecChange/onSqlChange handlers) remain defined and imported elsewhere in the module.packages/ai-core/README.md (1)
9-9:zodinstall command is now correct.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ai-core/README.md` at line 9, The npm install line "npm install `@sqlrooms/ai-core` `@sqlrooms/room-store` `@sqlrooms/ui` zod" in the README is correct; do not change it—ensure the README.md maintains this exact install command so that zod is included with `@sqlrooms/ai-core`, `@sqlrooms/room-store`, and `@sqlrooms/ui`.packages/cosmos/README.md (1)
1-104: LGTM — README is accurate and well-structured.The
@cosmos.gl/graphimport (line 35) is consistent with the updated dependency. The Zustand selector pattern in lines 84–88 follows the recommended memoized per-field selector approach, and the store composition in lines 22–28 correctly uses thecreateRoomStorespread pattern. This addresses the prior review's concern about the deprecated package reference.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cosmos/README.md` around lines 1 - 104, The review comment is a duplicate approval (contains both [approve_code_changes] and [duplicate_comment]) and does not require code changes to packages/cosmos/README.md; remove the duplicate/extra reviewer tag and keep a single approval comment (or delete the duplicate comment) so the PR history is clean, referencing the README (packages/cosmos/README.md) as the file approved.packages/cosmos/package.json (1)
22-22: LGTM — correct migration away from the deprecated beta package.
@cosmograph/cosmoshas been officially deprecated and moved to@cosmos.gl/graph, and the switch from the old beta pin (2.0.0-beta.26) to a stable release range (^2.6.4) is the right move. This resolves the concern raised in the prior review.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cosmos/package.json` at line 22, Replace the deprecated dependency "@cosmograph/cosmos" with the new package "@cosmos.gl/graph" and ensure the version is set to "^2.6.4" in package.json (the dependency entry for "@cosmos.gl/graph"), then run your package manager (npm/yarn/pnpm) to update the lockfile and node_modules so the lockfile reflects the new package and remove any references to the old beta package from package.json and lockfiles.
🧹 Nitpick comments (3)
packages/vega/README.md (1)
1-1: Consider adding a top-level#heading.The file opens with plain prose rather than a Markdown heading. Without a title heading, the file won't render a proper title on npm or GitHub, and LLM context windows that chunk by heading will miss a title anchor.
📝 Proposed fix
-Vega-Lite chart components and AI chart tool integration for SQLRooms. +# `@sqlrooms/vega` + +Vega-Lite chart components and AI chart tool integration for SQLRooms.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/vega/README.md` at line 1, The README lacks a top-level Markdown heading; add a clear title line at the top (e.g., "# Vega" or "# Vega-Lite chart components and AI chart tool integration for SQLRooms") so the README renders with a proper title and can be used as a heading anchor for LLM chunking; update the first line of packages/vega/README.md to be that single-line H1 heading and keep the existing prose below it.packages/ai-core/README.md (1)
24-28: Consider usingimport typefor the type-only importBaseRoomStoreState.
BaseRoomStoreStateis used only in a type position. Combining it with value imports without thetypemodifier can cause issues underisolatedModules/verbatimModuleSyntaxwhen readers copy this snippet into a TypeScript project.📝 Proposed fix
import { - BaseRoomStoreState, createBaseRoomSlice, createRoomStore, } from '@sqlrooms/room-store'; +import type {BaseRoomStoreState} from '@sqlrooms/room-store';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ai-core/README.md` around lines 24 - 28, The import combines a type-only symbol with value imports; change the import so BaseRoomStoreState is imported using the type modifier (e.g., import type { BaseRoomStoreState } ... ) while leaving createBaseRoomSlice and createRoomStore as regular imports to avoid isolatedModules/verbatimModuleSyntax issues and make clear that BaseRoomStoreState is type-only.packages/duckdb/README.md (1)
401-404: Double-await pattern needs a clarifying comment.The inline comment says "await handle directly" but doesn't explain why
executeSqlreturns aQueryHandlerather than the final result, making this pattern look like a mistake to a first-time reader. A brief note on the return type ofexecuteSql(e.g.,Promise<QueryHandle | undefined>) would remove the ambiguity.📝 Suggested clarification
- // Cached execution with deduplication - const query = await executeSql('SELECT * FROM users LIMIT 10'); - if (query) { - const rows = await query; // await handle directly - console.log('Query result:', rows); - } + // executeSql returns Promise<QueryHandle | undefined>; undefined when already running. + const handle = await executeSql('SELECT * FROM users LIMIT 10'); + if (handle) { + const rows = await handle; // QueryHandle is thenable – resolves to Arrow Table + console.log('Query result:', rows); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/duckdb/README.md` around lines 401 - 404, The README example's "await handle directly" comment is ambiguous; update the text around the example to explicitly state that executeSql returns a Promise<QueryHandle | undefined> and that QueryHandle itself resolves to the final rows (so you first await the handle, then await the handle to get rows), and replace the inline comment on the line with `const rows = await query;` with a short clarifying note like "QueryHandle is a thenable that resolves to row results — first await executeSql for the handle, then await the handle for rows." Reference executeSql and QueryHandle in the explanation so readers understand the double-await pattern is intentional.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/ai/README.md`:
- Around line 100-118: The snippet incorrectly omits the object spread and uses
a semicolon terminator when integrating the AI slice: instead of discarding the
slice, spread the result of createAiSlice(...) into the enclosing tools object
and replace the terminating semicolon with a comma so the object literal remains
valid; specifically, update the place where createAiSlice({ tools: {
...createDefaultAiTools(store), echo: { ... }, }, getInstructions: ... })(set,
get, store) is used to apply "...createAiSlice(...)(set, get, store)" into the
tools object and ensure the trailing delimiter is a comma.
In `@packages/vega/README.md`:
- Line 95: Update the README's editorMode enum description to match the
TypeScript definition: replace the documented value 'vega' with 'spec' so the
list reads 'none' | 'sql' | 'spec' | 'both' to align with the type declared for
editorMode in packages/vega/src/editor/types.ts (the editorMode type).
---
Outside diff comments:
In `@packages/duckdb/README.md`:
- Around line 394-397: The AdvancedOperations example uses useRoomStore but
doesn't import it; add the missing import for useRoomStore (the same as other
examples) at the top of the snippet so AdvancedOperations can call useRoomStore
to get executeSql, sqlSelectToJson and checkTableExists; specifically, insert
the import { useRoomStore } from './store'; line so the useRoomStore references
in AdvancedOperations resolve.
- Around line 278-280: The onChange handler is calling the async function
handleLoadCSV without awaiting or prefixing with void, creating a floating
promise; update the handler so the call is prefixed with void (i.e., void
handleLoadCSV(e.target.files[0])) to mirror the AdvancedDataLoader pattern and
intentionally ignore the returned promise, keeping handleLoadCSV as the invoked
async function and preserving the existing null-check (e.target.files?.[0]).
---
Duplicate comments:
In `@packages/ai-core/README.md`:
- Line 9: The npm install line "npm install `@sqlrooms/ai-core`
`@sqlrooms/room-store` `@sqlrooms/ui` zod" in the README is correct; do not change
it—ensure the README.md maintains this exact install command so that zod is
included with `@sqlrooms/ai-core`, `@sqlrooms/room-store`, and `@sqlrooms/ui`.
In `@packages/cosmos/package.json`:
- Line 22: Replace the deprecated dependency "@cosmograph/cosmos" with the new
package "@cosmos.gl/graph" and ensure the version is set to "^2.6.4" in
package.json (the dependency entry for "@cosmos.gl/graph"), then run your
package manager (npm/yarn/pnpm) to update the lockfile and node_modules so the
lockfile reflects the new package and remove any references to the old beta
package from package.json and lockfiles.
In `@packages/cosmos/README.md`:
- Around line 1-104: The review comment is a duplicate approval (contains both
[approve_code_changes] and [duplicate_comment]) and does not require code
changes to packages/cosmos/README.md; remove the duplicate/extra reviewer tag
and keep a single approval comment (or delete the duplicate comment) so the PR
history is clean, referencing the README (packages/cosmos/README.md) as the file
approved.
In `@packages/kepler/README.md`:
- Around line 53-71: MapPanel previously risked passing an undefined mapId into
KeplerMapContainer; ensure the guard is present by checking mapId before
rendering (as in the current MapPanel implementation) so KeplerMapContainer
always receives a string; confirm the early return "if (!mapId) return null;"
remains in the MapPanel function and that dependencies (mapId, isTableReady,
addTableToMap) are used in the useEffect that calls addTableToMap.
In `@packages/vega/README.md`:
- Around line 92-105: The README correctly separates constructor options for
createVegaChartTool from the runtime LLM-invoked Zod schema fields; no code
changes are required—verify that the createVegaChartTool documentation
(editable, editorMode) remains distinct from the LLM payload section (sqlQuery,
vegaLiteSpec, reasoning) and keep the "LLM invocation / Zod schema fields"
heading intact to prevent future regressions.
- Around line 51-66: The diff shows the JSX was wrapped into a proper exported
React function component CompoundVegaChart that returns VegaLiteChart.Container
with child components (VegaLiteChart.Actions, VegaLiteChart.Chart,
VegaLiteChart.SpecEditor, VegaLiteChart.SqlEditor); no further code changes are
required—keep the exported function CompoundVegaChart as-is and ensure the
referenced symbols (VegaLiteChart.Container, initialSpec, and the
onSpecChange/onSqlChange handlers) remain defined and imported elsewhere in the
module.
---
Nitpick comments:
In `@packages/ai-core/README.md`:
- Around line 24-28: The import combines a type-only symbol with value imports;
change the import so BaseRoomStoreState is imported using the type modifier
(e.g., import type { BaseRoomStoreState } ... ) while leaving
createBaseRoomSlice and createRoomStore as regular imports to avoid
isolatedModules/verbatimModuleSyntax issues and make clear that
BaseRoomStoreState is type-only.
In `@packages/duckdb/README.md`:
- Around line 401-404: The README example's "await handle directly" comment is
ambiguous; update the text around the example to explicitly state that
executeSql returns a Promise<QueryHandle | undefined> and that QueryHandle
itself resolves to the final rows (so you first await the handle, then await the
handle to get rows), and replace the inline comment on the line with `const rows
= await query;` with a short clarifying note like "QueryHandle is a thenable
that resolves to row results — first await executeSql for the handle, then await
the handle for rows." Reference executeSql and QueryHandle in the explanation so
readers understand the double-await pattern is intentional.
In `@packages/vega/README.md`:
- Line 1: The README lacks a top-level Markdown heading; add a clear title line
at the top (e.g., "# Vega" or "# Vega-Lite chart components and AI chart tool
integration for SQLRooms") so the README renders with a proper title and can be
used as a heading anchor for LLM chunking; update the first line of
packages/vega/README.md to be that single-line H1 heading and keep the existing
prose below it.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (18)
package.jsonpackages/ai-core/README.mdpackages/ai/README.mdpackages/canvas/README.mdpackages/cosmos/README.mdpackages/cosmos/package.jsonpackages/cosmos/src/CosmosGraph.tsxpackages/cosmos/src/CosmosSlice.tspackages/cosmos/src/CosmosSliceConfig.tspackages/cosmos/src/hooks/useHoverState.tspackages/discuss/README.mdpackages/duckdb/README.mdpackages/kepler-config/README.mdpackages/kepler/README.mdpackages/layout-config/README.mdpackages/recharts/README.mdpackages/utils/README.mdpackages/vega/README.md
✅ Files skipped from review due to trivial changes (1)
- packages/cosmos/src/CosmosSlice.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/layout-config/README.md
- packages/utils/README.md
- packages/canvas/README.md
Comment on lines
+100
to
+118
| createAiSlice({ | ||
| tools: { | ||
| ...createDefaultAiTools(store), | ||
| echo: { | ||
| name: 'echo', | ||
| description: 'Return user text back to the chat', | ||
| parameters: z.object({ | ||
| text: z.string(), | ||
| }), | ||
| execute: async ({text}) => ({ | ||
| llmResult: { | ||
| success: true, | ||
| details: `Echo: ${text}`, | ||
| }, | ||
| }), | ||
| }, | ||
| }, | ||
| getInstructions: () => createDefaultAiInstructions(store), | ||
| })(set, get, store); |
Contributor
There was a problem hiding this comment.
Missing ... spread operator and wrong terminator in the custom tools snippet.
The snippet calls createAiSlice({...})(set, get, store) without spreading the result into the enclosing object literal, which means all AI slice state is silently discarded. It also uses a semicolon instead of a comma, making it invalid inside the object passed to createRoomStore. Both the Quick Start (line 53) and the remote-endpoint snippet (line 127) correctly use ... and a trailing comma.
🐛 Proposed fix
-createAiSlice({
+...createAiSlice({
tools: {
...createDefaultAiTools(store),
echo: {
name: 'echo',
description: 'Return user text back to the chat',
parameters: z.object({
text: z.string(),
}),
execute: async ({text}) => ({
llmResult: {
success: true,
details: `Echo: ${text}`,
},
}),
},
},
getInstructions: () => createDefaultAiInstructions(store),
-})(set, get, store);
+})(set, get, store),📝 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
| createAiSlice({ | |
| tools: { | |
| ...createDefaultAiTools(store), | |
| echo: { | |
| name: 'echo', | |
| description: 'Return user text back to the chat', | |
| parameters: z.object({ | |
| text: z.string(), | |
| }), | |
| execute: async ({text}) => ({ | |
| llmResult: { | |
| success: true, | |
| details: `Echo: ${text}`, | |
| }, | |
| }), | |
| }, | |
| }, | |
| getInstructions: () => createDefaultAiInstructions(store), | |
| })(set, get, store); | |
| ...createAiSlice({ | |
| tools: { | |
| ...createDefaultAiTools(store), | |
| echo: { | |
| name: 'echo', | |
| description: 'Return user text back to the chat', | |
| parameters: z.object({ | |
| text: z.string(), | |
| }), | |
| execute: async ({text}) => ({ | |
| llmResult: { | |
| success: true, | |
| details: `Echo: ${text}`, | |
| }, | |
| }), | |
| }, | |
| }, | |
| getInstructions: () => createDefaultAiInstructions(store), | |
| })(set, get, store), |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ai/README.md` around lines 100 - 118, The snippet incorrectly omits
the object spread and uses a semicolon terminator when integrating the AI slice:
instead of discarding the slice, spread the result of createAiSlice(...) into
the enclosing tools object and replace the terminating semicolon with a comma so
the object literal remains valid; specifically, update the place where
createAiSlice({ tools: { ...createDefaultAiTools(store), echo: { ... }, },
getInstructions: ... })(set, get, store) is used to apply
"...createAiSlice(...)(set, get, store)" into the tools object and ensure the
trailing delimiter is a comma.
| helps identify which product categories are driving the most revenue for your business. | ||
| ``` | ||
| - `editable`: whether users can edit SQL/spec in the chart UI | ||
| - `editorMode`: which editors to render (`'none' | 'sql' | 'vega' | 'both'`) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n "editorMode" --type ts -C 3Repository: sqlrooms/sqlrooms
Length of output: 5883
🏁 Script executed:
rg -n "type EditorMode|interface EditorMode" --type ts -A 5Repository: sqlrooms/sqlrooms
Length of output: 454
Update README editorMode value from 'vega' to 'spec'
The README documents 'none' | 'sql' | 'vega' | 'both', but the actual TypeScript type definition in packages/vega/src/editor/types.ts is 'spec' | 'sql' | 'both' | 'none'. Change 'vega' to 'spec' to match the implementation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/vega/README.md` at line 95, Update the README's editorMode enum
description to match the TypeScript definition: replace the documented value
'vega' with 'spec' so the list reads 'none' | 'sql' | 'spec' | 'both' to align
with the type declared for editorMode in packages/vega/src/editor/types.ts (the
editorMode type).
cursor
Bot
force-pushed
the
cursor/llm-friendly-documentation-d7cb
branch
from
February 23, 2026 13:47
085bb8b to
8937f7f
Compare
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/ai/README.md (1)
98-117:⚠️ Potential issue | 🟡 MinorFix the custom tools snippet to spread the slice result (and keep the object literal valid).
If this snippet is meant to sit inside thecreateRoomStorestate object, it needs...and a trailing comma like the other examples; otherwise it reads as a dangling factory call.📝 Proposed doc fix
-// inside createRoomStore(...): -createAiSlice({ +// inside createRoomStore(...): +...createAiSlice({ tools: { ...createDefaultAiTools(store), echo: { name: 'echo', description: 'Return user text back to the chat', parameters: z.object({ text: z.string(), }), execute: async ({text}) => ({ llmResult: { success: true, details: `Echo: ${text}`, }, }), }, }, getInstructions: () => createDefaultAiInstructions(store), -})(set, get, store); +})(set, get, store),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ai/README.md` around lines 98 - 117, The snippet currently calls createAiSlice(...) as a dangling factory instead of merging its returned slice into the room store; update the createRoomStore state to spread the slice result into the object (use ...createAiSlice({...})(set, get, store)) and include a trailing comma like other examples so the object literal remains valid; locate the createAiSlice call that builds tools (including createDefaultAiTools and the echo tool) and replace the standalone call with the spread expression and trailing comma.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/ai/README.md`:
- Around line 98-117: The snippet currently calls createAiSlice(...) as a
dangling factory instead of merging its returned slice into the room store;
update the createRoomStore state to spread the slice result into the object (use
...createAiSlice({...})(set, get, store)) and include a trailing comma like
other examples so the object literal remains valid; locate the createAiSlice
call that builds tools (including createDefaultAiTools and the echo tool) and
replace the standalone call with the spread expression and trailing comma.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
examples/ai-agent/src/components/MainView.tsxexamples/ai-core/src/components/MainView.tsxexamples/ai-rag/src/components/MainView.tsxexamples/ai/src/components/MainView.tsxpackages/ai-settings/README.mdpackages/ai/README.md
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (3)
packages/discuss/README.md (1)
7-9:@sqlrooms/utilsinclusion — previous issue resolved.The install command now includes
@sqlrooms/utils.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/discuss/README.md` around lines 7 - 9, The README install command should include `@sqlrooms/utils` as shown; verify the npm install line in the README (the code block with the install command) contains "@sqlrooms/utils" alongside "@sqlrooms/discuss", "@sqlrooms/room-shell" and "@sqlrooms/ui", and remove any duplicated review-note text so the README only contains the single corrected install command line.packages/cosmos/README.md (2)
34-35:@cosmograph/cosmos→@cosmos.gl/graphmigration is already applied.The past review comment flagging the deprecated
@cosmograph/cosmosimport has been resolved — line 35 now correctly imports from@cosmos.gl/graph.Verify that
GraphConfigInterfaceis still the correct export name and that the config properties used (simulationGravity,simulationRepulsion,simulationLinkSpring,simulationLinkDistance) are present on that type, and that all three components imported on line 34 (CosmosGraph,CosmosGraphControls,CosmosSimulationControls) are actually exported from@sqlrooms/cosmos.#!/bin/bash # Verify exported symbols from `@sqlrooms/cosmos` package echo "=== `@sqlrooms/cosmos` exports ===" fd "index" packages/cosmos/src --type f | head -10 | xargs cat 2>/dev/null echo "=== CosmosGraph, CosmosGraphControls, CosmosSimulationControls presence ===" rg -n "CosmosGraphControls|CosmosSimulationControls|CosmosGraph" packages/cosmos/src/ --type ts --type tsx -l 2>/dev/null || rg -n "CosmosGraphControls|CosmosSimulationControls|CosmosGraph" packages/cosmos/src/ echo "=== GraphConfigInterface in `@cosmos.gl/graph` or `@cosmograph/cosmos` ===" rg -n "GraphConfigInterface" packages/cosmos/src/ -A2 -B2GraphConfigInterface `@cosmos.gl/graph` exported type properties simulationGravity simulationRepulsion simulationLinkSpring simulationLinkDistance🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cosmos/README.md` around lines 34 - 35, Ensure the migration is correct by confirming the type and exports: verify that GraphConfigInterface (and its properties simulationGravity, simulationRepulsion, simulationLinkSpring, simulationLinkDistance) is actually exported from `@cosmos.gl/graph` and update the import if the exported name differs; also confirm that CosmosGraph, CosmosGraphControls, and CosmosSimulationControls are exported from `@sqlrooms/cosmos` and, if any are missing or renamed, update the imports or re-export them from the package so the module names CosmosGraph, CosmosGraphControls, CosmosSimulationControls resolve correctly where used.
34-35:@cosmograph/cosmos→@cosmos.gl/graphmigration already applied.The prior review concern about the deprecated
@cosmograph/cosmosimport has been addressed.simulationGravity,simulationRepulsion,simulationLinkSpring, andsimulationLinkDistanceare all valid properties onGraphConfigInterface, and@cosmograph/cosmoshas been moved to@cosmos.gl/graph— the import on line 35 now correctly reflects this.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cosmos/README.md` around lines 34 - 35, The README still imports CosmosGraph, CosmosGraphControls, and CosmosSimulationControls from '@sqlrooms/cosmos' while the migration moved exports to '@cosmos.gl/graph'; update the import so all Cosmos-related symbols (CosmosGraph, CosmosGraphControls, CosmosSimulationControls) are imported from '@cosmos.gl/graph' (or remove the '@sqlrooms/cosmos' import and consolidate to the single '@cosmos.gl/graph' import) so the README reflects the completed migration.
🧹 Nitpick comments (1)
packages/room-store/README.md (1)
22-60: Consider using type-only import forBaseRoomStoreStatein the doc example.
While this won't error in the repo's current TS config, usingtype BaseRoomStoreStatefollows best practices and matches the pattern used forStateCreatorin the same import statement.🔧 Suggested doc tweak
import { - BaseRoomStoreState, + type BaseRoomStoreState, createBaseRoomSlice, createRoomStore, createSlice, type StateCreator, } from '@sqlrooms/room-store';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/room-store/README.md` around lines 22 - 60, Update the import to use a type-only import for BaseRoomStoreState: change the import that brings in BaseRoomStoreState to use the TypeScript `type` modifier (matching how `StateCreator` is imported) so it becomes a type-only import; adjust the `import { BaseRoomStoreState, ... } from '@sqlrooms/room-store'` line to import `BaseRoomStoreState` as a type to keep consistency and follow best practices with the `createRoomStore`/`createBaseRoomSlice` example.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/duckdb-core/README.md`:
- Around line 150-151: The DatabaseManager example's import only includes Button
but the example calls useRoomStore in multiple places; update the import
statement to also import useRoomStore (e.g., add useRoomStore to the named
imports alongside Button) so the DatabaseManager example compiles and matches
the duckdb README; ensure both example blocks (the one around lines 153–161 and
the similar block at 188–191) import useRoomStore as well.
In `@packages/layout/README.md`:
- Around line 61-72: The example panel definitions under panels (entries named
data and main) are missing the required icon property; update the example so
each panel object includes an icon field alongside title, component and
placement (e.g., add an appropriate icon identifier for the data panel and main
panel), ensuring the DataPanel and MainPanel entries show the four required
properties: title, icon, component, and placement.
In `@packages/room-shell/README.md`:
- Around line 82-90: The Quick Start App example omits the required
room.initialize() call; update the App component example so that when mounting
it calls roomStore.room.initialize() (or room.initialize() on the store's room
instance) — e.g., invoke room.initialize() inside a mount effect in the App that
uses the provided roomStore, ensuring initialization runs and lifecycle hooks
for slices execute while keeping the existing RoomShell usage (RoomShell,
RoomShell.Sidebar, RoomShell.LayoutComposer, RoomShell.LoadingProgress).
In `@packages/sql-editor/README.md`:
- Around line 82-88: The example is ambiguous about why both createQueryTab and
parseAndRunQuery are called with the same SQL; update the README example to
either add a short inline comment clarifying their roles (createQueryTab
initializes a persistent UI tab with the given SQL, parseAndRunQuery executes
the provided SQL and stores results for the currently selected tab) or split
into two examples showing (A) creating a tab with createQueryTab('...') and (B)
executing a query with parseAndRunQuery('...'), and mention that
parseAndRunQuery runs the string passed (not necessarily the tab content) so
callers should keep them in sync when intended.
---
Duplicate comments:
In `@packages/cosmos/README.md`:
- Around line 34-35: Ensure the migration is correct by confirming the type and
exports: verify that GraphConfigInterface (and its properties simulationGravity,
simulationRepulsion, simulationLinkSpring, simulationLinkDistance) is actually
exported from `@cosmos.gl/graph` and update the import if the exported name
differs; also confirm that CosmosGraph, CosmosGraphControls, and
CosmosSimulationControls are exported from `@sqlrooms/cosmos` and, if any are
missing or renamed, update the imports or re-export them from the package so the
module names CosmosGraph, CosmosGraphControls, CosmosSimulationControls resolve
correctly where used.
- Around line 34-35: The README still imports CosmosGraph, CosmosGraphControls,
and CosmosSimulationControls from '@sqlrooms/cosmos' while the migration moved
exports to '@cosmos.gl/graph'; update the import so all Cosmos-related symbols
(CosmosGraph, CosmosGraphControls, CosmosSimulationControls) are imported from
'@cosmos.gl/graph' (or remove the '@sqlrooms/cosmos' import and consolidate to
the single '@cosmos.gl/graph' import) so the README reflects the completed
migration.
In `@packages/discuss/README.md`:
- Around line 7-9: The README install command should include `@sqlrooms/utils` as
shown; verify the npm install line in the README (the code block with the
install command) contains "@sqlrooms/utils" alongside "@sqlrooms/discuss",
"@sqlrooms/room-shell" and "@sqlrooms/ui", and remove any duplicated review-note
text so the README only contains the single corrected install command line.
---
Nitpick comments:
In `@packages/room-store/README.md`:
- Around line 22-60: Update the import to use a type-only import for
BaseRoomStoreState: change the import that brings in BaseRoomStoreState to use
the TypeScript `type` modifier (matching how `StateCreator` is imported) so it
becomes a type-only import; adjust the `import { BaseRoomStoreState, ... } from
'@sqlrooms/room-store'` line to import `BaseRoomStoreState` as a type to keep
consistency and follow best practices with the
`createRoomStore`/`createBaseRoomSlice` example.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
packages/ai-rag/docs/EXAMPLE.mdpackages/ai-settings/README.mdpackages/canvas/README.mdpackages/cosmos/README.mdpackages/discuss/README.mdpackages/duckdb-core/README.mdpackages/duckdb/README.mdpackages/layout/README.mdpackages/room-shell/README.mdpackages/room-store/README.mdpackages/s3-browser/README.mdpackages/sql-editor/README.mdpython/sqlrooms-rag/examples/SUMMARY.md
✅ Files skipped from review due to trivial changes (2)
- packages/ai-settings/README.md
- packages/canvas/README.md
Comment on lines
+150
to
+151
| import {Button} from '@sqlrooms/ui'; | ||
|
|
Contributor
There was a problem hiding this comment.
useRoomStore is missing from the import statement in the DatabaseManager example.
The code block uses useRoomStore across five call-sites (lines 153–161) but only Button is imported. The companion packages/duckdb/README.md already adds import {useRoomStore} from './store'; alongside Button in the equivalent block — duckdb-core/README.md should match.
📝 Proposed fix
+import {useRoomStore} from './store';
import {Button} from '@sqlrooms/ui';Also applies to: 188-191
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/duckdb-core/README.md` around lines 150 - 151, The DatabaseManager
example's import only includes Button but the example calls useRoomStore in
multiple places; update the import statement to also import useRoomStore (e.g.,
add useRoomStore to the named imports alongside Button) so the DatabaseManager
example compiles and matches the duckdb README; ensure both example blocks (the
one around lines 153–161 and the similar block at 188–191) import useRoomStore
as well.
Comment on lines
+61
to
+72
| panels: { | ||
| data: { | ||
| title: 'Data', | ||
| component: DataPanel, | ||
| placement: 'sidebar', | ||
| }, | ||
| main: { | ||
| title: 'Main', | ||
| component: MainPanel, | ||
| placement: 'main', | ||
| }, | ||
| }, |
Contributor
There was a problem hiding this comment.
Panel definitions are missing the required icon property.
Per the project conventions, panel definitions require four properties: title, icon, component, and placement. Both data and main panel definitions in the example omit icon, which could mislead users into creating panels without it.
📝 Proposed fix
panels: {
data: {
title: 'Data',
+ icon: SomeDataIcon,
component: DataPanel,
placement: 'sidebar',
},
main: {
title: 'Main',
+ icon: SomeMainIcon,
component: MainPanel,
placement: 'main',
},
},Based on learnings: "Structure panel definitions with required properties: title, icon, component, and placement."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/layout/README.md` around lines 61 - 72, The example panel
definitions under panels (entries named data and main) are missing the required
icon property; update the example so each panel object includes an icon field
alongside title, component and placement (e.g., add an appropriate icon
identifier for the data panel and main panel), ensuring the DataPanel and
MainPanel entries show the four required properties: title, icon, component, and
placement.
Comment on lines
+82
to
90
| export function App() { | ||
| return ( | ||
| <div> | ||
| <h2>My Items ({myFeatureData.length})</h2> | ||
| <button onClick={() => addItem({id: Date.now(), name: 'New Item'})}> | ||
| Add Item | ||
| </button> | ||
| {/* Render items */} | ||
| </div> | ||
| <RoomShell roomStore={roomStore} className="h-screen"> | ||
| <RoomShell.Sidebar /> | ||
| <RoomShell.LayoutComposer /> | ||
| <RoomShell.LoadingProgress /> | ||
| </RoomShell> | ||
| ); | ||
| } |
Contributor
There was a problem hiding this comment.
Missing room.initialize() call in the Quick Start App example
Per codebase conventions, room.initialize() must be called during component mount to orchestrate automatic slice initialization. Omitting it from the Quick Start will leave developers with a store that never runs its lifecycle hooks.
📝 Suggested addition to the App example
+import {useEffect} from 'react';
export function App() {
+ useEffect(() => {
+ void roomStore.getState().room.initialize();
+ }, []);
+
return (
<RoomShell roomStore={roomStore} className="h-screen">
<RoomShell.Sidebar />
<RoomShell.LayoutComposer />
<RoomShell.LoadingProgress />
</RoomShell>
);
}Based on learnings, always call room.initialize() during component mount to orchestrate automatic slice initialization with lifecycle hooks.
📝 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
| export function App() { | |
| return ( | |
| <div> | |
| <h2>My Items ({myFeatureData.length})</h2> | |
| <button onClick={() => addItem({id: Date.now(), name: 'New Item'})}> | |
| Add Item | |
| </button> | |
| {/* Render items */} | |
| </div> | |
| <RoomShell roomStore={roomStore} className="h-screen"> | |
| <RoomShell.Sidebar /> | |
| <RoomShell.LayoutComposer /> | |
| <RoomShell.LoadingProgress /> | |
| </RoomShell> | |
| ); | |
| } | |
| import {useEffect} from 'react'; | |
| export function App() { | |
| useEffect(() => { | |
| void roomStore.getState().room.initialize(); | |
| }, []); | |
| return ( | |
| <RoomShell roomStore={roomStore} className="h-screen"> | |
| <RoomShell.Sidebar /> | |
| <RoomShell.LayoutComposer /> | |
| <RoomShell.LoadingProgress /> | |
| </RoomShell> | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/room-shell/README.md` around lines 82 - 90, The Quick Start App
example omits the required room.initialize() call; update the App component
example so that when mounting it calls roomStore.room.initialize() (or
room.initialize() on the store's room instance) — e.g., invoke room.initialize()
inside a mount effect in the App that uses the provided roomStore, ensuring
initialization runs and lifecycle hooks for slices execute while keeping the
existing RoomShell usage (RoomShell, RoomShell.Sidebar,
RoomShell.LayoutComposer, RoomShell.LoadingProgress).
Comment on lines
+82
to
88
| const parseAndRunQuery = useRoomStore((state) => state.sqlEditor.parseAndRunQuery); | ||
| const createQueryTab = useRoomStore((state) => state.sqlEditor.createQueryTab); | ||
|
|
||
| // Use actions | ||
| const handleExecute = () => { | ||
| runQuery('SELECT * FROM users LIMIT 10'); | ||
| runQuery('SELECT * FROM users LIMIT 10'); | ||
| const run = async () => { | ||
| createQueryTab('SELECT COUNT(*) AS total FROM earthquakes'); | ||
| await parseAndRunQuery('SELECT COUNT(*) AS total FROM earthquakes'); | ||
| }; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, let's find the store implementation for SqlEditorSliceState
fd -t ts -t tsx "store" packages/sql-editor/src/ | head -20Repository: sqlrooms/sqlrooms
Length of output: 230
🏁 Script executed:
#!/bin/bash
# Search for parseAndRunQuery and createQueryTab definitions
rg -n "parseAndRunQuery|createQueryTab" packages/sql-editor/src/ -A 3 -B 1Repository: sqlrooms/sqlrooms
Length of output: 3778
🏁 Script executed:
#!/bin/bash
# Look for the SqlEditorSliceState or similar store definition
fd -t ts "slice\|store" packages/sql-editor/src/ --exec head -5 {}Repository: sqlrooms/sqlrooms
Length of output: 230
🏁 Script executed:
#!/bin/bash
# Read the parseAndRunQuery implementation to understand how it uses the query parameter
sed -n '422,460p' packages/sql-editor/src/SqlEditorSlice.tsxRepository: sqlrooms/sqlrooms
Length of output: 1470
🏁 Script executed:
#!/bin/bash
# Also read the full createQueryTab implementation
sed -n '209,240p' packages/sql-editor/src/SqlEditorSlice.tsxRepository: sqlrooms/sqlrooms
Length of output: 1103
Clarify the intent and relationship between createQueryTab and parseAndRunQuery.
The example calls both createQueryTab(sql) and parseAndRunQuery(sql) with identical SQL. While both calls are needed together, their purpose and interaction is unclear from the snippet:
createQueryTabcreates a persistent tab in the UI with the initial query contentparseAndRunQueryexecutes the passed query parameter and stores results for the currently selected tab
Since parseAndRunQuery executes the query it receives (not the tab's stored query), passing identical strings is correct but fragile—if they diverged, the tab would show different SQL than what was executed.
Add a brief explanatory comment in the example to clarify that createQueryTab initializes the tab and parseAndRunQuery executes the query, or restructure into separate examples for clarity.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/sql-editor/README.md` around lines 82 - 88, The example is ambiguous
about why both createQueryTab and parseAndRunQuery are called with the same SQL;
update the README example to either add a short inline comment clarifying their
roles (createQueryTab initializes a persistent UI tab with the given SQL,
parseAndRunQuery executes the provided SQL and stores results for the currently
selected tab) or split into two examples showing (A) creating a tab with
createQueryTab('...') and (B) executing a query with parseAndRunQuery('...'),
and mention that parseAndRunQuery runs the string passed (not necessarily the
tab content) so callers should keep them in sync when intended.
Signed-off-by: Ilya Boyandin <[email protected]>
Signed-off-by: Ilya Boyandin <[email protected]>
cursor Bot
pushed a commit
that referenced
this pull request
Feb 24, 2026
Signed-off-by: Ilya Boyandin <[email protected]> Co-authored-by: Cursor Agent <[email protected]>
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Improve LLM-friendly guidance and refresh package READMEs to enhance discoverability and accuracy.
The goal is to make the documentation more accessible and useful for Large Language Models, increasing the likelihood of SQLRooms being suggested for data analytics applications, and ensuring package-level READMEs are up-to-date with current APIs and practical examples.
Summary by CodeRabbit
Documentation
Chores
Defaults
Style