Repository navigation
refactor: Extract reusable dashboard AI tools - #650
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (11)
💤 Files with no reviewable changes (2)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughExtracts dashboard AI tooling into ChangesDashboard AI Extraction to Reusable Library
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/createDashboardAgent.ts`:
- Around line 12-22: The getModel implementation always uses
createOpenAICompatible which breaks Anthropic; update getModel in
createDashboardAgent.ts to first attempt provider-specific model creation (e.g.,
call the same getCustomModel hook used in chatTransport.ts) using the resolved
provider and baseURL from
state.ai.getBaseUrlFromSettings()/state.ai.getApiKeyFromSettings(), and only if
that returns null/undefined fall back to createOpenAICompatible(...). Use the
existing variables currentSession, provider and modelId to pass into the
provider-specific hook and maintain the existing fallback behavior.
In `@packages/mosaic/src/ai.ts`:
- Around line 136-138: The createQueryTools contract allows arbitrary tool keys
which can break callers that expect a stable "query" tool
(DASHBOARD_AGENT_INSTRUCTIONS and calculateAgentMetadata hardcode "query" and
queriesRun relies on it); update the type/signature of createQueryTools to
require an object with a "query" Tool key (e.g., make it return { query: Tool }
or Record<'query', Tool>), update any call sites to supply the "query" key, and
adjust related code paths that reference createQueryTools to use the guaranteed
"query" tool rather than arbitrary names.
- Around line 612-678: Declare dashboardId in the outer scope before the try
block (e.g., let dashboardId = '') so it can be referenced in the catch, assign
it the value returned by getOrCreateDashboard inside the try, and in the catch
return that dashboardId instead of an empty string; update the catch return
object (success: false) to include dashboardId (and optionally any partial
metadata if available) so callers retain the handle to partially created/updated
dashboards.
🪄 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: e675ca33-bcdf-43c8-933c-653ce03ee285
📒 Files selected for processing (9)
apps/sqlrooms-cli-ui/src/createDashboardAgent.tsapps/sqlrooms-cli-ui/src/createDashboardAiTools.tsapps/sqlrooms-cli-ui/src/createDashboardToolDeps.tsapps/sqlrooms-cli-ui/src/dashboardAgentInstructions.tspackages/mosaic/README.mdpackages/mosaic/__tests__/dashboard-ai.test.tspackages/mosaic/package.jsonpackages/mosaic/src/ai.tspackages/mosaic/src/chart-types/remove-panel-tool.ts
💤 Files with no reviewable changes (1)
- apps/sqlrooms-cli-ui/src/dashboardAgentInstructions.ts
| getModel: ({state}) => { | ||
| const currentSession = state.ai.getCurrentSession(); | ||
| const provider = currentSession?.modelProvider || 'openai'; | ||
| const modelId = currentSession?.model || 'gpt-4.1'; | ||
|
|
||
| return createOpenAICompatible({ | ||
| apiKey: state.ai.getApiKeyFromSettings(), | ||
| name: provider || '', | ||
| baseURL: | ||
| state.ai.getBaseUrlFromSettings() || 'https://api.openai.com/v1', | ||
| }).chatModel(modelId); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== modelProvider usages and AI settings helpers ==="
rg -n -C 3 --type ts --type tsx '\bmodelProvider\b|\bgetBaseUrlFromSettings\s*\(|\bgetApiKeyFromSettings\s*\('
echo
echo "=== model factory usage across the repo ==="
rg -n -C 3 --type ts --type tsx 'createOpenAICompatible|createAnthropic|createGoogle|createOpenRouter|chatModel\(|languageModel\('Repository: sqlrooms/sqlrooms
Length of output: 160
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Explore repository structure ==="
fd -t f "createDashboardAgent" --max-one-result
echo
echo "=== Search for modelProvider, getBaseUrlFromSettings, getApiKeyFromSettings ==="
rg -n -C 3 "modelProvider|getBaseUrlFromSettings|getApiKeyFromSettings"
echo
echo "=== Search for provider factory patterns ==="
rg -n "createOpenAICompatible|createAnthropic|createGoogle|createOpenRouter"
echo
echo "=== Look for state.ai structure and provider options ==="
rg -n "state\.ai\.|getCurrentSession|\.model|\.modelProvider" | head -50Repository: sqlrooms/sqlrooms
Length of output: 350
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Find createDashboardAgent file ==="
fd "createDashboardAgent"
echo
echo "=== Search for modelProvider and settings functions ==="
rg -n "modelProvider|getBaseUrlFromSettings|getApiKeyFromSettings" | head -40
echo
echo "=== Search for provider factory patterns ==="
rg -n "createOpenAICompatible|createAnthropic|createGoogle" | head -40
echo
echo "=== Search for AI state structure and getCurrentSession ==="
rg -n "getCurrentSession|state\.ai" | head -50Repository: sqlrooms/sqlrooms
Length of output: 11000
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== View getBaseUrlFromSettings implementation in AiSlice.ts ==="
rg -n -A 20 "getBaseUrlFromSettings\s*:\s*\(" packages/ai-core/src/AiSlice.ts
echo
echo "=== View chatTransport.ts provider routing logic ==="
sed -n '270,300p' packages/ai-core/src/chatTransport.ts
echo
echo "=== Check how createOpenAICompatible is used in chatTransport.ts ==="
rg -n -B 5 -A 10 "createOpenAICompatible" packages/ai-core/src/chatTransport.ts | head -50Repository: sqlrooms/sqlrooms
Length of output: 4354
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Find supported provider definitions ==="
rg -n "providers\[" packages/ai-core/src/AiSlice.ts | head -20
echo
echo "=== Look for Anthropic-specific model creation ==="
rg -n "anthropic|createAnthropic" --ignore-case
echo
echo "=== Check AiSlice test for all provider scenarios ==="
sed -n '50,120p' packages/ai-core/__tests__/AiSlice.modelSelection.test.ts
echo
echo "=== View more of getBaseUrlFromSettings implementation ==="
sed -n '891,930p' packages/ai-core/src/AiSlice.tsRepository: sqlrooms/sqlrooms
Length of output: 8788
This code will fail for Anthropic providers because createOpenAICompatible() is incompatible with Anthropic's API.
While getBaseUrlFromSettings() correctly resolves provider-specific base URLs, createDashboardAgent.ts always instantiates the model through createOpenAICompatible(...). Anthropic is a supported provider (as shown in tests), but it requires a different factory. Compare this to chatTransport.ts (line 286–296), which uses a getCustomModel hook to allow provider-specific model creation before falling back to createOpenAICompatible. The createDashboardAgent needs the same pattern.
🤖 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/createDashboardAgent.ts` around lines 12 - 22, The
getModel implementation always uses createOpenAICompatible which breaks
Anthropic; update getModel in createDashboardAgent.ts to first attempt
provider-specific model creation (e.g., call the same getCustomModel hook used
in chatTransport.ts) using the resolved provider and baseURL from
state.ai.getBaseUrlFromSettings()/state.ai.getApiKeyFromSettings(), and only if
that returns null/undefined fall back to createOpenAICompatible(...). Use the
existing variables currentSession, provider and modelId to pass into the
provider-specific hook and maintain the existing fallback behavior.
795b06c to
e14d3d5
Compare
Summary by CodeRabbit
Refactor
Bug Fixes
Tests
Documentation
Chores