feat: CLI: Mosaic dashboard - #408

apps/sqlrooms-cli-ui/src/store.ts (1)

219-222: Minor code duplication with DashboardSheet.tsx.

The getErrorMessage helper is duplicated between this file and DashboardSheet.tsx (lines 21-26). Consider extracting it to a shared utility module to maintain DRY principles.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/sqlrooms-cli-ui/src/store.ts` around lines 219 - 222, The
getErrorMessage function is duplicated in store.ts and DashboardSheet.tsx;
extract it into a shared utility module (e.g., create a new file exporting
getErrorMessage) and replace both local implementations with an import. Update
references to use the exported getErrorMessage function in store.ts and
DashboardSheet.tsx, ensure the utility exports the same signature
(getErrorMessage(error: unknown): string) and run type checks to confirm no
breaking imports.

packages/mosaic/src/MosaicSlice.ts (1)

349-376: Consider documenting the performance implications of the JSON fallback.

The adapter correctly falls back to JSON for both json and arrow query types when using non-WASM connectors. The comment explains the compatibility reason, but users should be aware that this may have performance implications for large datasets compared to native Arrow format.

The as any type assertions are acceptable here given the adapter pattern, but consider adding a brief JSDoc comment on createDuckDbMosaicConnector noting this trade-off.

📝 Suggested documentation

+/**
+ * Creates a Mosaic-compatible connector from a generic DuckDbConnector.
+ * Note: For non-WASM connectors, Arrow queries are materialized as JSON
+ * for compatibility, which may impact performance with large datasets.
+ */
 function createDuckDbMosaicConnector(connector: DuckDbConnector): Connector {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/mosaic/src/MosaicSlice.ts` around lines 349 - 376, Add a concise
JSDoc to createDuckDbMosaicConnector explaining that when the connector cannot
return native Arrow tables it falls back to JSON for both 'json' and 'arrow'
query types, that this materialization (using connector.queryJson and
Array.from) can have performance/memory implications for large datasets compared
to native Arrow, and that the use of `as any` in the Connector['query'] return
is an intentional adapter trade-off; include the JSDoc directly above the
createDuckDbMosaicConnector function and reference the query fallback behavior
so readers see the trade-off when inspecting that function.

apps/sqlrooms-cli-ui/src/components/DashboardSheet.tsx (2)

131-148: Effect dependency may cause unexpected behavior.

The effect depends on [schema], but the intent is to load the schema once when it's not available. Since setSchema is called inside the effect and schema is in the dependency array, this could theoretically cause issues if the schema object reference changes for other reasons. However, the early return on if (schema) return; mitigates this.

A clearer approach would be to use an empty dependency array with a separate loading flag, but the current implementation is functionally correct due to the early return guard.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/sqlrooms-cli-ui/src/components/DashboardSheet.tsx` around lines 131 -
148, The effect currently depends on [schema] which is confusing; change it to
run once on mount by replacing the dependency array with [] and keep the
cancellation guard to avoid state updates after unmount. Specifically, update
the useEffect that calls loadVgplotSchema so it no longer lists schema in
dependencies, retain the cancelled boolean (or use a ref like isMounted) and
continue to call setSchema(loadedSchema) and
setSchemaError(getErrorMessage(error)) only when not cancelled; remove the
initial if (schema) return; guard since the effect will only run once.

---

12-13: Module-level mutable state for schema caching.

The singleton caching pattern works for browser-only usage but may cause issues with SSR or testing scenarios where module state persists across tests. Consider whether this is acceptable for your use case.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/sqlrooms-cli-ui/src/components/DashboardSheet.tsx` around lines 12 - 13,
The module-level mutable variables vgplotSchemaCache and vgplotSchemaPromise
create a global singleton that can leak across SSR requests and tests; refactor
so the cache is not stored at module scope—either move the cache into
component-level state/ref (e.g., a useRef/useState inside the DashboardSheet
hook/component) or inject a cache provider passed into the loader hook so each
request/test can supply an isolated cache; alternatively, guard assignments with
a browser-only check (typeof window !== 'undefined') or add an explicit
reset/clear function used by tests—update all usages that read/write
vgplotSchemaCache and vgplotSchemaPromise to use the new per-instance cache API.

Configuration used: defaults

Review profile: CHILL

Plan: Pro

apps/sqlrooms-cli-ui/src/store.ts (1)

770-777: ⚠️ Potential issue | 🟠 Major

Remove the duplicate toolRenderers key.

The second object literal overwrites the first one, leaving dead code and a Biome noDuplicateObjectKeys error in this TS file.

As per coding guidelines, **/*.{js,ts,jsx,tsx}: Maintain lint baseline with warnings only; zero errors is the expected state.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/sqlrooms-cli-ui/src/store.ts` around lines 770 - 777, There are two
duplicate object keys named toolRenderers in the same object literal causing the
latter to overwrite the former and trigger a noDuplicateObjectKeys lint error;
remove the duplicate entry and ensure a single toolRenderers property merges
defaults with the chart renderer (use createDefaultAiToolRenderers() spread plus
chart: VegaChartToolResult) so only one toolRenderers exists in the object in
store.ts.

apps/sqlrooms-cli-ui/src/components/DashboardSheet.tsx (1)

196-225: Debounce the live preview compile loop.

This effect recompiles the preview on every valid keystroke. The cancelled flag only suppresses stale setStates, so the underlying work still runs in the background and can pile up quickly on larger specs. A short debounce before compiling would make typing much smoother.

⚙️ Small debounce around preview compilation

   const parsedAppliedSpec = React.useMemo(
     () => parseVgplotSpec(lastAppliedValue),
     [lastAppliedValue],
   );
   const previewSpec = parsedEditorSpec.parsed ?? parsedAppliedSpec.parsed;
+  const [debouncedPreviewSpec, setDebouncedPreviewSpec] =
+    React.useState<Spec | null>(previewSpec);
+
+  React.useEffect(() => {
+    const timeoutId = window.setTimeout(() => {
+      setDebouncedPreviewSpec(previewSpec);
+    }, 250);
+    return () => window.clearTimeout(timeoutId);
+  }, [previewSpec]);
+
   const previewError = parsedEditorSpec.error;
   const isDirty = editorValue !== lastAppliedValue;
   const canApply = Boolean(
     currentSheetId && parsedEditorSpec.formatted && isDirty,
   );
@@
-  React.useEffect(() => {
-    if (mosaicConnection.status !== 'ready' || !previewSpec) {
+  React.useEffect(() => {
+    if (mosaicConnection.status !== 'ready' || !debouncedPreviewSpec) {
       setCompiledPlot(null);
       setPlotCompileError(null);
       setIsCompilingPlot(false);
       return;
     }
@@
-        const ast = await parseSpec(previewSpec);
+        const ast = await parseSpec(debouncedPreviewSpec);
         const instantiated = await astToDOM(ast);
         if (cancelled) return;
         setCompiledPlot(instantiated.element);
@@
-  }, [mosaicConnection.status, previewSpec]);
+  }, [mosaicConnection.status, debouncedPreviewSpec]);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/sqlrooms-cli-ui/src/components/DashboardSheet.tsx` around lines 196 -
225, The effect in DashboardSheet.tsx (the React.useEffect watching
mosaicConnection.status and previewSpec) launches parseSpec/astToDOM on every
keystroke; add a short debounce (e.g., 200–400ms) so compilation only runs after
typing pauses: use a timer stored in a ref (or local timeout var) and
clearTimeout on subsequent effect runs/cleanup, start the async compile only
from the debounced callback, keep the existing cancelled flag semantics to avoid
stale setState, and ensure setIsCompilingPlot is set true when the debounced
compile starts and reset in the finally block; reference the useEffect,
previewSpec, parseSpec, astToDOM, setCompiledPlot, setPlotCompileError,
setIsCompilingPlot, and cancelled symbols when making the change.