Feature/dbt query results and run history funcionality#327
Conversation
Addresses Plan 48a to bring a native, IDE-like query result panel into the selected project screen, mirroring the workflow of dbt Power User. * Added a top-level `QUERY RESULTS` bottom tab to the `TerminalLayout`. * Built the `ProjectQueryResultsPanel` featuring inner tabs for Preview, SQL, History, and Bookmarks. * Implemented `useProjectQueryResultsPanel` hook to manage query states and safely persist history and bookmarks to `localStorage` (stripping large result payloads to avoid quota limits). * Updated `CustomTable` and `QueryResult` components to support headerless rendering for a cleaner embedded UI. * Integrated `Execute Query` and `Execute CTE` Monaco CodeLens providers to route SQL directly into the new bottom panel. * Fixed implicit `any` TypeScript errors in the `localStorage` parsing logic for query history and bookmarks.
…t-query-results-and-run-history-funcionality
…ation Implement Plan 48b by adding a project-level dbt Run History surface and wiring dbt command execution into a persisted renderer-side history model. Add the dbt run history UI: - Add DbtRunHistoryPanel with toolbar, run rows, and result detail rows - Add run-history prompt builders for AI-assisted failure triage - Add shared run-history types and component exports - Persist recent dbt invocations per project with useDbtRunHistory - Capture command, timing, status summaries, invocation metadata, and child result details from dbt run_results.json Wire dbt commands into run history: - Update model and project dbt split buttons to record dbt run/test/build/etc. executions - Extend useDbt so command completion can feed run-history entries - Preserve existing dbt execution behavior while adding history capture Improve selected-project terminal tabs: - Add Run History as a bottom terminal panel tab - Keep Query Results and Run History available alongside Terminal, Lineage, and PID Server - Add terminal layout ref support so callers can switch directly to a target bottom-panel tab - Auto-focus Query Results when project SQL execution produces new results - Improve PID Server tab behavior so the tab can be closed even after stopping or killing a docs/server process - Fix minimized terminal layout so the restore bar remains visible inside the project editor area Improve project SQL/query navigation: - Wire ProjectQueryResultsPanel into the selected-project terminal layout - Allow SQL history items to rerun into the Query Results tab - Allow opening saved/history SQL back into the editor - Keep bottom-panel navigation consistent when running model SQL, dbt commands, or project queries
📝 WalkthroughWalkthroughThis PR adds project-scoped SQL query preview/history/bookmarks UI backed by new hooks, persistent DBT run history tracking with dedicated UI components, Monaco CTE-detection/execution wiring, terminal layout tab restructuring, DBT subprocess credential/environment handling, chat tool-call status normalization, and minor table/UI cleanups. ChangesQuery Preview & DBT Run History Feature
DBT Tool Environment and Agent Instructions
Chat Tool Call Status Normalization
Table and Minor UI Cleanup
Estimated code review effort: 4 (Complex) | ~75 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant ModelSplitButton
participant useProjectSqlExecution
participant dbtCompileModel
participant queryData
participant useProjectQueryResultsPanel
participant ProjectQueryResultsPanel
User->>ModelSplitButton: click preview/run
ModelSplitButton->>useProjectSqlExecution: executeProjectSql(payload)
useProjectSqlExecution->>dbtCompileModel: compile model (if compileModel)
dbtCompileModel-->>useProjectSqlExecution: compiledSql
useProjectSqlExecution->>queryData: execute SQL against connection
queryData-->>useProjectSqlExecution: result or error
useProjectSqlExecution->>useProjectQueryResultsPanel: completePreview/failPreview
useProjectQueryResultsPanel->>ProjectQueryResultsPanel: update state.result/error
ProjectQueryResultsPanel-->>User: render Preview/SQL/History tab
sequenceDiagram
participant useDbt
participant useDbtRunHistory
participant localStorage
participant DbtRunHistoryPanel
participant DbtRunHistoryRunRow
useDbt->>useDbtRunHistory: recordCommandStart(project, command)
useDbtRunHistory->>localStorage: persist running entry
useDbt->>useDbt: run dbt command
useDbt->>useDbtRunHistory: recordCommandFinished/recordCommandFailed
useDbtRunHistory->>localStorage: update entry, dispatch change event
localStorage-->>DbtRunHistoryPanel: refresh history via event listener
DbtRunHistoryPanel->>DbtRunHistoryRunRow: render entry
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (5)
src/renderer/components/queryResult/index.tsx (1)
476-527: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider using theme-aware color for NULL placeholder.
The hardcoded
#999color for the NULL cell text (line 492) won't adapt to light/dark theme variations. Using a theme token liketext.disabledortext.secondarywould ensure consistent contrast across modes.♻️ Suggested theme-aware approach
render: (value) => { const cellValue = value[column]; if (cellValue === null || cellValue === undefined) { return ( <div style={{ whiteSpace: 'nowrap', minHeight: '24px', display: 'flex', alignItems: 'center', - color: '`#999`', + color: 'var(--mui-palette-text-disabled, `#999`)', fontStyle: 'italic', }} > NULL </div> ); }Alternatively, wrap the div in a MUI
Boxwithsx={{ color: 'text.disabled' }}to leverage the theme directly.🤖 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 `@src/renderer/components/queryResult/index.tsx` around lines 476 - 527, The NULL placeholder in the cell renderer uses a hardcoded gray color that won’t adapt to theme changes. Update the render path in queryResult/index.tsx where the NULL block is returned so it uses a theme-aware text color token instead of the fixed hex value, ideally through the same render logic in the columns.map cell renderer. Use a token such as text.disabled or text.secondary, or apply it via a themed Box/sx wrapper, so the NULL display stays consistent in light and dark modes.src/renderer/components/projectQueryResults/index.ts (1)
2-7: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExport
ProjectQueryBookmarkfrom the barrel.
ProjectQueryBookmarkis defined in./typesbut not re-exported here, while all sibling types are. External consumers needing the bookmark type must import directly from./types, bypassing the barrel.♻️ Proposed fix
export type { ProjectQueryHistoryItem, ProjectQueryPanelState, ProjectQueryPreviewPayload, ProjectQueryResultsTab, + ProjectQueryBookmark, } from './types';🤖 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 `@src/renderer/components/projectQueryResults/index.ts` around lines 2 - 7, The barrel export in projectQueryResults is missing ProjectQueryBookmark even though it is defined alongside the other types in ./types. Update the export list in the index barrel to re-export ProjectQueryBookmark together with ProjectQueryHistoryItem, ProjectQueryPanelState, ProjectQueryPreviewPayload, and ProjectQueryResultsTab so consumers can import it from the barrel consistently.src/renderer/components/projectQueryResults/ProjectQueryResultsPanel.tsx (1)
47-47: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
onOpenSqlInEditoris declared and destructured but never used.The prop is typed in
Props, destructured from the component parameters, and forwarded to the fullscreen instance, but no code path ever calls it. Either wire it up or remove it to avoid dead API surface.Also applies to: 486-486
🤖 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 `@src/renderer/components/projectQueryResults/ProjectQueryResultsPanel.tsx` at line 47, `onOpenSqlInEditor` is dead API surface in ProjectQueryResultsPanel: it’s declared in Props and destructured but never invoked. Either remove the prop from the Props type and the component parameter list, or wire it into the relevant SQL result/UI action in ProjectQueryResultsPanel and the fullscreen instance so it is actually called when the user opens SQL in the editor.src/main/services/ai/tools/studio/cli.tools.ts (1)
59-59: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDrop the
as anycast oninputSchema.tool()already accepts this Zod schema shape here, and the cast only hides future type drift.🤖 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 `@src/main/services/ai/tools/studio/cli.tools.ts` at line 59, The `inputSchema` assignment in `tool()` should no longer use the `as any` cast because it is masking type safety and future schema drift. Update the `studioCliRunDbtInputSchema` usage in `studioCliRunDbtTool` so it is passed directly with its proper inferred type, and verify the `tool()` call still satisfies the expected Zod schema shape without the cast.src/renderer/components/dbtModelButtons/ModelSplitButton.tsx (1)
164-316: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftPreview flow duplicates
useProjectSqlExecution.
handlePreviewModelreimplements the exact compile → execute → lifecycle-callback sequence now provided byuseProjectSqlExecution.executeProjectSql(compile model, empty-SQL guard, missing-connection guards,queryData,onStart/onSuccess/onErrorwithdurationMs). Maintaining both risks the two paths drifting. Consider driving the preview throughexecuteProjectSql(withcompileModel: true) and keeping only the local modal state here.🤖 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 `@src/renderer/components/dbtModelButtons/ModelSplitButton.tsx` around lines 164 - 316, The preview logic in `handlePreviewModel` duplicates the compile-and-run flow already implemented by `useProjectSqlExecution.executeProjectSql`. Refactor `ModelSplitButton` to call `executeProjectSql` with `compileModel: true` and let the hook handle `dbtCompileModel`, empty SQL checks, connection validation, `queryData`, and `onQueryPreviewStart/onQueryPreviewSuccess/onQueryPreviewError` timing/callback payloads. Keep only the local preview modal state and result handling in `ModelSplitButton`, and remove the duplicated lifecycle/error handling from `handlePreviewModel`.
🤖 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 `@src/main/services/ai/agents/projectAgent.ts`:
- Around line 140-145: The guidance in projectAgent should be made consistent so
it never tells the agent to pass “dbt debug” as the studio_cli_run_dbt command.
Update the conditional branch in the dbt failure-handling instructions to use
the same “command: debug” wording as the earlier steps, and remove any phrasing
that implies the tool should receive “dbt debug” directly. Keep the existing
context about invalid credentials and connection metadata, but ensure the
referenced workflow matches the allowed command set enforced by cli.tools.ts and
PHASE_19_ALLOWED_COMMANDS.
In `@src/main/services/ai/tools/dbt.tools.ts`:
- Around line 61-96: The env lookup in buildDbtProcessEnv is treating empty
strings as present values, so SecureStorageService fallback is skipped for blank
env vars. Update the envVarName check to test whether the variable exists in env
rather than relying on truthiness, so profiles.yml env_var entries still get
hydrated when the current value is ''.
In `@src/renderer/components/dbtRunHistory/DbtRunHistoryRunRow.tsx`:
- Around line 29-36: The running-state HourglassEmptyIcon in DbtRunHistoryRunRow
relies on a spin animation that is only defined in other component-local styles,
so add the shared `@keyframes` spin to a global/shared renderer style source that
is always mounted. Update the styling used by DbtRunHistoryRunRow so its
animation can resolve regardless of which other components are rendered, and
reference the existing animation usage on HourglassEmptyIcon to keep the naming
consistent.
In `@src/renderer/components/dbtRunHistory/DbtRunHistoryToolbar.tsx`:
- Around line 83-96: The fullscreen control layout is using `ml: 'auto'` on
`Tooltip`, but `Tooltip` is not the flex item that needs to be pushed right.
Move the auto left margin to the `IconButton` in `DbtRunHistoryToolbar` (or wrap
it in a flex `Box`) so the fullscreen button is actually aligned to the far
right while keeping the `onToggleFullscreen` and `isFullscreen` behavior
unchanged.
In `@src/renderer/components/projectQueryResults/ProjectQueryResultsPanel.tsx`:
- Around line 268-270: The history item icon in ProjectQueryResultsPanel is
hardcoded to a success state, so failed queries are not visually distinguished.
Update the rendering for ProjectQueryHistoryItem to inspect its status field and
choose an error-colored icon when status is error, while keeping the current
success icon styling for successful items. Use the existing history item render
path around CodeIcon in ProjectQueryResultsPanel to wire this conditional
status-based icon choice.
- Around line 733-750: The fullscreen recursive ProjectQueryResultsPanel is
missing several action callbacks, so bookmark and history actions break inside
the Dialog. Update the fullscreen instance to pass through onAddBookmark,
onDeleteBookmark, and onRunHistoryItem alongside the existing props, using the
same callback values already available in ProjectQueryResultsPanel so
AddBookmarkModal save, bookmark deletion, and history/bookmark execution work in
fullscreen mode.
In `@src/renderer/components/terminal/terminal.tsx`:
- Around line 38-42: The Terminal component’s clearOutput effect only runs once
on initial mount, so switching projects leaves old terminal output visible.
Update the React.useEffect in terminal.tsx to depend on the active project
identity (such as project.id or the project signal) instead of an empty
dependency array, and keep calling clearOutput from that effect so each project
switch resets the terminal.
In `@src/renderer/hooks/useDbt.ts`:
- Around line 350-354: The `recordCommandFinished` call in `useDbt` is always
attaching `${project.path}/target/run_results.json`, which lets commands like
`deps`, `clean`, `debug`, and `docs:serve` reuse stale artifacts and corrupt the
current run history. Update the logic around `recordCommandFinished` to only
read and attach the artifact for commands that actually produce
`run_results.json`, or verify the parsed `invocationId` matches the current run
before saving it, so unrelated prior results are not recorded.
In `@src/renderer/hooks/useProjectQueryResultsPanel.ts`:
- Around line 38-61: When `projectId` changes in `useProjectQueryResultsPanel`,
the load effect should reset transient preview fields instead of only merging in
`history` and `bookmarks`. Update the `React.useEffect` that reads from
`localStorage` so the state passed to `setState` also clears `result`, `error`,
`rawSql`, `compiledSql`, `filePath`, `modelName`, and `lastDurationMs` back to
their empty/default values for the new project. Keep the existing parsing and
fallback behavior, but ensure stale query panel data cannot carry over between
projects.
In `@src/renderer/hooks/useProjectSqlExecution.ts`:
- Around line 143-158: The catch block in useProjectSqlExecution is reporting
the wrong SQL value in the error payload. Update the onError call to use the
already-computed compiledSql variable instead of querySql so failures after
compileModel preserve the actual compiled SQL. Keep the rest of the error
handling in executeSqlQuery unchanged.
- Around line 104-108: Project SQL execution is ignoring the user-selected
limit, so the result set is always unbounded. Thread the selected limit through
useProjectSqlExecution into the queryData call, then extend queryData and the
backend query executor path to accept and apply that limit when running the SQL.
Make sure the limit is passed along with connection, query, and projectName so
project queries respect the selected cap end to end.
---
Nitpick comments:
In `@src/main/services/ai/tools/studio/cli.tools.ts`:
- Line 59: The `inputSchema` assignment in `tool()` should no longer use the `as
any` cast because it is masking type safety and future schema drift. Update the
`studioCliRunDbtInputSchema` usage in `studioCliRunDbtTool` so it is passed
directly with its proper inferred type, and verify the `tool()` call still
satisfies the expected Zod schema shape without the cast.
In `@src/renderer/components/dbtModelButtons/ModelSplitButton.tsx`:
- Around line 164-316: The preview logic in `handlePreviewModel` duplicates the
compile-and-run flow already implemented by
`useProjectSqlExecution.executeProjectSql`. Refactor `ModelSplitButton` to call
`executeProjectSql` with `compileModel: true` and let the hook handle
`dbtCompileModel`, empty SQL checks, connection validation, `queryData`, and
`onQueryPreviewStart/onQueryPreviewSuccess/onQueryPreviewError` timing/callback
payloads. Keep only the local preview modal state and result handling in
`ModelSplitButton`, and remove the duplicated lifecycle/error handling from
`handlePreviewModel`.
In `@src/renderer/components/projectQueryResults/index.ts`:
- Around line 2-7: The barrel export in projectQueryResults is missing
ProjectQueryBookmark even though it is defined alongside the other types in
./types. Update the export list in the index barrel to re-export
ProjectQueryBookmark together with ProjectQueryHistoryItem,
ProjectQueryPanelState, ProjectQueryPreviewPayload, and ProjectQueryResultsTab
so consumers can import it from the barrel consistently.
In `@src/renderer/components/projectQueryResults/ProjectQueryResultsPanel.tsx`:
- Line 47: `onOpenSqlInEditor` is dead API surface in ProjectQueryResultsPanel:
it’s declared in Props and destructured but never invoked. Either remove the
prop from the Props type and the component parameter list, or wire it into the
relevant SQL result/UI action in ProjectQueryResultsPanel and the fullscreen
instance so it is actually called when the user opens SQL in the editor.
In `@src/renderer/components/queryResult/index.tsx`:
- Around line 476-527: The NULL placeholder in the cell renderer uses a
hardcoded gray color that won’t adapt to theme changes. Update the render path
in queryResult/index.tsx where the NULL block is returned so it uses a
theme-aware text color token instead of the fixed hex value, ideally through the
same render logic in the columns.map cell renderer. Use a token such as
text.disabled or text.secondary, or apply it via a themed Box/sx wrapper, so the
NULL display stays consistent in light and dark modes.
🪄 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: e5c3f9ce-4393-4337-b0eb-58a2e36baab8
📒 Files selected for processing (33)
src/main/services/ai/agents/projectAgent.tssrc/main/services/ai/tools/dbt.tools.tssrc/main/services/ai/tools/studio/cli.tools.tssrc/renderer/components/chat/MessageRenderer.tsxsrc/renderer/components/chat/ToolCallRow.tsxsrc/renderer/components/customTable/index.tsxsrc/renderer/components/customTable/types.tssrc/renderer/components/dbtModelButtons/ModelSplitButton.tsxsrc/renderer/components/dbtModelButtons/ProjectDbtSplitButton.tsxsrc/renderer/components/dbtRunHistory/DbtRunHistoryPanel.tsxsrc/renderer/components/dbtRunHistory/DbtRunHistoryResultRow.tsxsrc/renderer/components/dbtRunHistory/DbtRunHistoryRunRow.tsxsrc/renderer/components/dbtRunHistory/DbtRunHistoryToolbar.tsxsrc/renderer/components/dbtRunHistory/dbtRunHistoryPromptBuilders.tssrc/renderer/components/dbtRunHistory/index.tssrc/renderer/components/dbtRunHistory/types.tssrc/renderer/components/editor/index.tsxsrc/renderer/components/lineage/LineageView.tsxsrc/renderer/components/projectQueryResults/ProjectQueryResultsPanel.tsxsrc/renderer/components/projectQueryResults/index.tssrc/renderer/components/projectQueryResults/types.tssrc/renderer/components/queryResult/index.tsxsrc/renderer/components/terminal/index.tsxsrc/renderer/components/terminal/styles.tssrc/renderer/components/terminal/terminal.tsxsrc/renderer/hooks/index.tssrc/renderer/hooks/useDbt.tssrc/renderer/hooks/useDbtRunHistory.tssrc/renderer/hooks/useProjectQueryResultsPanel.tssrc/renderer/hooks/useProjectSqlExecution.tssrc/renderer/screens/projectDetails/index.tsxsrc/renderer/utils/sql/cteDetection.tssrc/types/dbtRunHistory.ts
| 3. If needed, run \`studio_cli_run_dbt\` with \`command: "debug"\` to verify the failure mode. Do not pass \`"dbt debug"\` as the command; the tool adds the dbt executable. | ||
| 4. Use \`getDbtLogs\` and read-only file inspection to explain the likely cause. | ||
| 5. Ask the user to fix the connection in the DBT Studio Connections UI, secure keytar-backed credentials, or the intended connection-management workflow. | ||
| 6. Wait for user confirmation that the connection has been corrected before continuing with dbt execution. | ||
|
|
||
| If the failure appears to be caused by invalid credentials, unreachable host, wrong port, missing network access, expired token, missing env vars, or bad connection metadata, do not rewrite \`profiles.yml\`. Report the issue clearly and direct the user to fix the connection configuration. | ||
| If the failure appears to be caused by invalid credentials, unreachable host, wrong port, missing network access, expired token, missing env vars, or bad connection metadata, run \`dbt debug\` through \`studio_cli_run_dbt\` when it can add evidence, then do not rewrite \`profiles.yml\`. Report the exact missing env var or failed connection field clearly and direct the user to fix the connection configuration. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Line 145 phrasing contradicts the Line 140 guidance.
Line 140 correctly instructs to use command: "debug" and explicitly warns against passing "dbt debug". However, Line 145 still says run \dbt debug` through `studio_cli_run_dbt`, which could lead the agent to pass "dbt debug"as the command parameter. Per the upstream schema incli.tools.ts, PHASE_19_ALLOWED_COMMANDSincludes"debug"but not"dbt debug"`, so the latter would be rejected by validation.
💡 Suggested fix for consistency
-If the failure appears to be caused by invalid credentials, unreachable host, wrong port, missing network access, expired token, missing env vars, or bad connection metadata, run `dbt debug` through `studio_cli_run_dbt` when it can add evidence, then do not rewrite `profiles.yml`. Report the exact missing env var or failed connection field clearly and direct the user to fix the connection configuration.
+If the failure appears to be caused by invalid credentials, unreachable host, wrong port, missing network access, expired token, missing env vars, or bad connection metadata, run `studio_cli_run_dbt` with `command: "debug"` when it can add evidence, then do not rewrite `profiles.yml`. Report the exact missing env var or failed connection field clearly and direct the user to fix the connection configuration.📝 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.
| 3. If needed, run \`studio_cli_run_dbt\` with \`command: "debug"\` to verify the failure mode. Do not pass \`"dbt debug"\` as the command; the tool adds the dbt executable. | |
| 4. Use \`getDbtLogs\` and read-only file inspection to explain the likely cause. | |
| 5. Ask the user to fix the connection in the DBT Studio Connections UI, secure keytar-backed credentials, or the intended connection-management workflow. | |
| 6. Wait for user confirmation that the connection has been corrected before continuing with dbt execution. | |
| If the failure appears to be caused by invalid credentials, unreachable host, wrong port, missing network access, expired token, missing env vars, or bad connection metadata, do not rewrite \`profiles.yml\`. Report the issue clearly and direct the user to fix the connection configuration. | |
| If the failure appears to be caused by invalid credentials, unreachable host, wrong port, missing network access, expired token, missing env vars, or bad connection metadata, run \`dbt debug\` through \`studio_cli_run_dbt\` when it can add evidence, then do not rewrite \`profiles.yml\`. Report the exact missing env var or failed connection field clearly and direct the user to fix the connection configuration. | |
| 3. If needed, run `studio_cli_run_dbt` with `command: "debug"` to verify the failure mode. Do not pass `"dbt debug"` as the command; the tool adds the dbt executable. | |
| 4. Use `getDbtLogs` and read-only file inspection to explain the likely cause. | |
| 5. Ask the user to fix the connection in the DBT Studio Connections UI, secure keytar-backed credentials, or the intended connection-management workflow. | |
| 6. Wait for user confirmation that the connection has been corrected before continuing with dbt execution. | |
| If the failure appears to be caused by invalid credentials, unreachable host, wrong port, missing network access, expired token, missing env vars, or bad connection metadata, run `studio_cli_run_dbt` with `command: "debug"` when it can add evidence, then do not rewrite `profiles.yml`. Report the exact missing env var or failed connection field clearly and direct the user to fix the connection configuration. |
🤖 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 `@src/main/services/ai/agents/projectAgent.ts` around lines 140 - 145, The
guidance in projectAgent should be made consistent so it never tells the agent
to pass “dbt debug” as the studio_cli_run_dbt command. Update the conditional
branch in the dbt failure-handling instructions to use the same “command: debug”
wording as the earlier steps, and remove any phrasing that implies the tool
should receive “dbt debug” directly. Keep the existing context about invalid
credentials and connection metadata, but ensure the referenced workflow matches
the allowed command set enforced by cli.tools.ts and PHASE_19_ALLOWED_COMMANDS.
| async function buildDbtProcessEnv( | ||
| projectPath: string, | ||
| ): Promise<Record<string, string | undefined>> { | ||
| const env = { ...process.env }; | ||
| const profilesPath = path.join(projectPath, 'profiles.yml'); | ||
|
|
||
| if (!fs.existsSync(profilesPath)) { | ||
| return env; | ||
| } | ||
|
|
||
| const profilesContent = await fs.promises.readFile(profilesPath, 'utf8'); | ||
| const envVarRegex = /env_var\(\s*["']([^"']+)["']/g; | ||
| const envVarNames = new Set<string>(); | ||
|
|
||
| for ( | ||
| let match = envVarRegex.exec(profilesContent); | ||
| match !== null; | ||
| match = envVarRegex.exec(profilesContent) | ||
| ) { | ||
| envVarNames.add(match[1]); | ||
| } | ||
|
|
||
| await Promise.all( | ||
| Array.from(envVarNames).map(async (envVarName) => { | ||
| if (env[envVarName]) return; | ||
|
|
||
| const storedValue = await SecureStorageService.getCredential(envVarName); | ||
| if (storedValue) { | ||
| env[envVarName] = storedValue; | ||
| } | ||
| }), | ||
| ); | ||
|
|
||
| return env; | ||
| } | ||
|
|
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect how credentials are stored/keyed to confirm getCredential(envVarName) resolves them.
rg -nP '\b(setCredential|saveCredential|getCredential|setPassword)\b' src/main --type=ts -C3Repository: rosettadb/dbt-studio
Length of output: 30486
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== dbt.tools.ts =="
cat -n src/main/services/ai/tools/dbt.tools.ts | sed -n '1,180p'
echo
echo "== secureStorage.service.ts relevant methods =="
cat -n src/main/services/secureStorage.service.ts | sed -n '1,180p'
echo
echo "== search for dbt-specific secure-storage usage =="
rg -n "profiles\.yml|env_var\(|SecureStorageService\.(getCredential|setCredential)\(" src/main/services/ai src/main/services -C2Repository: rosettadb/dbt-studio
Length of output: 47821
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== connectors.service.ts env_var generation / env extraction =="
sed -n '1210,1360p' src/main/services/connectors.service.ts | cat -n
echo
echo "== projects.service.ts env var extraction =="
sed -n '1290,1345p' src/main/services/projects.service.ts | cat -nRepository: rosettadb/dbt-studio
Length of output: 8053
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
text = Path('src/main/services/connectors.service.ts').read_text()
for pat in [
r'env_var\("db-[^"]+"\)',
r'getCredential\(`db-[^`]+`\)',
r'setCredential\(`db-[^`]+`\)',
]:
print(f"PATTERN {pat}")
for m in re.finditer(pat, text):
start = max(0, m.start()-200)
end = min(len(text), m.end()+200)
print(text[start:end].replace('\n', '\n'))
print('---')
break
PYRepository: rosettadb/dbt-studio
Length of output: 700
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== buildDbtProcessEnv call sites =="
rg -n "buildDbtProcessEnv\(" src/main/services/ai/tools/dbt.tools.ts -C3
echo
echo "== minimal runtime semantics for empty string guard =="
node - <<'JS'
const env = { FOO: '' };
console.log('FOO in env:', 'FOO' in env);
console.log('truthy check:', !!env.FOO);
console.log('should fallback on existence-only check:', env.FOO !== undefined);
JSRepository: rosettadb/dbt-studio
Length of output: 1057
Handle empty env vars as missing here.
if (env[envVarName]) return; skips the secure-storage fallback when the variable exists but is set to '', so dbt still gets a blank env var. Check for existence instead of truthiness.
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 70-70: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.promises.readFile(profilesPath, 'utf8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename-typescript)
🪛 OpenGrep (1.23.0)
[ERROR] 76-76: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
[ERROR] 78-78: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
🤖 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 `@src/main/services/ai/tools/dbt.tools.ts` around lines 61 - 96, The env lookup
in buildDbtProcessEnv is treating empty strings as present values, so
SecureStorageService fallback is skipped for blank env vars. Update the
envVarName check to test whether the variable exists in env rather than relying
on truthiness, so profiles.yml env_var entries still get hydrated when the
current value is ''.
| case 'running': | ||
| return ( | ||
| <HourglassEmptyIcon | ||
| color="action" | ||
| fontSize="small" | ||
| sx={{ animation: 'spin 2s linear infinite' }} | ||
| /> | ||
| ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether a global "spin" keyframes animation is defined anywhere in the renderer.
rg -n "`@keyframes` spin" src/renderer
rg -n "keyframes\(" src/renderer -A3 | rg -n "spin" -B3Repository: rosettadb/dbt-studio
Length of output: 253
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the keyframes definition and all "spin" animation usages in the renderer.
sed -n '70,120p' src/renderer/components/lineage/LineageToolbar.tsx
printf '\n---\n'
rg -n "animation: .*spin|`@keyframes` spin|spin 2s linear infinite" src/rendererRepository: rosettadb/dbt-studio
Length of output: 1529
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Look for shared/global definitions of the "spin" keyframes and all usages.
printf '--- keyframe definitions ---\n'
rg -n --hidden --glob '!node_modules' --glob '!dist' --glob '!build' "`@keyframes` spin|spin\s*:\s*\{" src
printf '\n--- animation usages ---\n'
rg -n --hidden --glob '!node_modules' --glob '!dist' --glob '!build' "animation:\s*.*spin" src
printf '\n--- theme/global style entry points that might inject keyframes ---\n'
rg -n --hidden --glob '!node_modules' --glob '!dist' --glob '!build' "GlobalStyles|CssBaseline|ThemeProvider|createTheme|Global" src/renderer srcRepository: rosettadb/dbt-studio
Length of output: 249
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- keyframe definitions ---'
rg -n --hidden --glob '!node_modules' --glob '!dist' --glob '!build' "`@keyframes` spin|spin\s*:\s*\{" src || true
echo
echo '--- animation usages ---'
rg -n --hidden --glob '!node_modules' --glob '!dist' --glob '!build' "animation:\s*.*spin" src || true
echo
echo '--- theme/global style entry points ---'
rg -n --hidden --glob '!node_modules' --glob '!dist' --glob '!build' "GlobalStyles|CssBaseline|ThemeProvider|createTheme|Global" src/renderer src || true
echo
echo '--- RepositoryHeader context ---'
sed -n '410,450p' src/renderer/components/sourceControl/RepositoryHeader.tsxRepository: rosettadb/dbt-studio
Length of output: 7451
Add a shared spin keyframe HourglassEmptyIcon uses animation: 'spin 2s linear infinite', but the renderer only defines @keyframes spin inside other component-local styles, so this icon can stay static when those components aren’t mounted.
🤖 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 `@src/renderer/components/dbtRunHistory/DbtRunHistoryRunRow.tsx` around lines
29 - 36, The running-state HourglassEmptyIcon in DbtRunHistoryRunRow relies on a
spin animation that is only defined in other component-local styles, so add the
shared `@keyframes` spin to a global/shared renderer style source that is always
mounted. Update the styling used by DbtRunHistoryRunRow so its animation can
resolve regardless of which other components are rendered, and reference the
existing animation usage on HourglassEmptyIcon to keep the naming consistent.
| {onToggleFullscreen && ( | ||
| <Tooltip | ||
| title={isFullscreen ? 'Exit fullscreen' : 'Fullscreen'} | ||
| sx={{ ml: 'auto' }} | ||
| > | ||
| <IconButton size="small" onClick={onToggleFullscreen}> | ||
| {isFullscreen ? ( | ||
| <FullscreenExitIcon fontSize="small" /> | ||
| ) : ( | ||
| <FullscreenIcon fontSize="small" /> | ||
| )} | ||
| </IconButton> | ||
| </Tooltip> | ||
| )} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Does applying the sx prop directly on MUI v6 Tooltip style the popper or the child trigger element?
💡 Result:
Applying the sx prop directly to the MUI v6 Tooltip component does not style the underlying popper or the tooltip surface itself [1][2][3]. The sx prop on the Tooltip component is typically passed down to the trigger element (the child) rather than the internal popper component [2][3]. To apply custom styles to the tooltip content (the popper), you must use the slotProps prop to target the specific sub-component [1][2][3]. Recommended approach: Use slotProps to provide an sx object to the tooltip or popper slots [1][2]: <Tooltip title="My Tooltip" slotProps={{ tooltip: { sx: { backgroundColor: 'common.black', fontSize: '0.8rem', }, }, popper: { sx: { // Styles for the popper container }, }, }} > Hover me This method provides granular control over the different parts of the Tooltip (such as the tooltip surface, the popper, the arrow, and the transition) [4][1][2].
Citations:
- 1: [material-ui][Tooltip] Tooltip/popper styles are not applying through direct sx styles in Tooltip component mui/material-ui#43134
- 2: [Tooltip] Override styles using sx property not working! mui/material-ui#28679
- 3: [Tooltip] Override styles using sx property not working! mui/material-ui#28679
- 4: https://v6.mui.com/material-ui/api/tooltip/
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== package versions =="
git ls-files 'package.json' 'pnpm-lock.yaml' 'yarn.lock' 'package-lock.json' | xargs -r -I{} sh -c 'echo "--- {}"; sed -n "1,220p" "{}" | rg -n "\"`@mui/material`\"|\"`@mui/icons-material`\"|\"react\"" -n || true'
echo
echo "== file outline =="
ast-grep outline src/renderer/components/dbtRunHistory/DbtRunHistoryToolbar.tsx --view expanded || true
echo
echo "== relevant source =="
cat -n src/renderer/components/dbtRunHistory/DbtRunHistoryToolbar.tsx | sed -n '1,180p'
echo
echo "== Tooltip usage elsewhere =="
rg -n --hidden --glob '!node_modules' --glob '!dist' --glob '!build' "<Tooltip|slotProps=.*tooltip|slotProps=.*popper|sx=.*ml: 'auto'|sx=.*ml:\"auto\"" src || true
echo
echo "== MUI docs references in repo =="
rg -n --hidden --glob '!node_modules' "slotProps|Tooltip" README.md docs src | sed -n '1,120p' || trueRepository: rosettadb/dbt-studio
Length of output: 34096
🌐 Web query:
MUI Tooltip sx prop forwarded to child trigger element or root slot? Tooltip sx ml:auto flex layout child
💡 Result:
The sx prop on the MUI Tooltip component is not automatically forwarded to the child trigger element [1][2]. In MUI, the Tooltip is a portal component; its content (the tooltip itself) is rendered outside the DOM hierarchy of its children [2]. Applying an sx prop directly to the Tooltip component does not style the trigger child, nor does it necessarily style the tooltip popper element in all versions [1][3][2]. To style the child trigger element, you should apply the sx prop or other styling methods directly to the child component itself [2]. If your goal is to control the layout of the child element (e.g., using ml:auto within a flex container), you must apply those layout properties to the child element, not the Tooltip wrapper [4][5]. If the child element is a custom component, ensure it is wrapped in React.forwardRef and spreads props correctly, as the Tooltip relies on this to attach event listeners [6]. If you intended to style the tooltip content (the bubble) rather than the trigger, use the slotProps prop instead of the sx prop [1][3]: <Tooltip slotProps={{ tooltip: { sx: { // Your styles for the tooltip bubble here } } }} title="Your content" > <div style={{ marginLeft: 'auto' }}>Trigger This approach provides granular control over the internal Tooltip and Popper components [1][3]. For alignment issues within flex containers, verify that the child element's CSS is properly configured for the flex layout [4][5].
Citations:
- 1: [material-ui][Tooltip] Tooltip/popper styles are not applying through direct sx styles in Tooltip component mui/material-ui#43134
- 2: [system] Can't add custom styles to the Tooltip with
sxprop mui/material-ui#32422 - 3: [Tooltip] Override styles using sx property not working! mui/material-ui#28679
- 4: Tooltip alignment inside a flex container mui/material-ui#15194
- 5: https://mui.com/material-ui/react-tooltip.md
- 6: https://mui.com/material-ui/react-tooltip/
Move ml: 'auto' off Tooltip.
Tooltip isn’t the flex item here, so the margin won’t push the fullscreen button to the right. Put ml: 'auto' on the IconButton (or a wrapping Box) instead.
🤖 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 `@src/renderer/components/dbtRunHistory/DbtRunHistoryToolbar.tsx` around lines
83 - 96, The fullscreen control layout is using `ml: 'auto'` on `Tooltip`, but
`Tooltip` is not the flex item that needs to be pushed right. Move the auto left
margin to the `IconButton` in `DbtRunHistoryToolbar` (or wrap it in a flex
`Box`) so the fullscreen button is actually aligned to the far right while
keeping the `onToggleFullscreen` and `isFullscreen` behavior unchanged.
| <CodeIcon | ||
| sx={{ fontSize: 16, color: 'success.main', mr: 2, flexShrink: 0 }} | ||
| /> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Show error status icon for failed history items.
ProjectQueryHistoryItem has a status: 'success' | 'error' field, but the history tab always renders a green CodeIcon. Failed queries should use an error-colored icon to give users immediate visual feedback.
🐛 Proposed fix
<CodeIcon
- sx={{ fontSize: 16, color: 'success.main', mr: 2, flexShrink: 0 }}
+ sx={{
+ fontSize: 16,
+ color: item.status === 'error' ? 'error.main' : 'success.main',
+ mr: 2,
+ flexShrink: 0,
+ }}
/>📝 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.
| <CodeIcon | |
| sx={{ fontSize: 16, color: 'success.main', mr: 2, flexShrink: 0 }} | |
| /> | |
| <CodeIcon | |
| sx={{ | |
| fontSize: 16, | |
| color: item.status === 'error' ? 'error.main' : 'success.main', | |
| mr: 2, | |
| flexShrink: 0, | |
| }} | |
| /> |
🤖 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 `@src/renderer/components/projectQueryResults/ProjectQueryResultsPanel.tsx`
around lines 268 - 270, The history item icon in ProjectQueryResultsPanel is
hardcoded to a success state, so failed queries are not visually distinguished.
Update the rendering for ProjectQueryHistoryItem to inspect its status field and
choose an error-colored icon when status is error, while keeping the current
success icon styling for successful items. Use the existing history item render
path around CodeIcon in ProjectQueryResultsPanel to wire this conditional
status-based icon choice.
| // Clear output when mounting for a new project (project.id changes → key changes → remount) | ||
| React.useEffect(() => { | ||
| clearOutput(); | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, []); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm whether Terminal/TerminalLayout is keyed by project id anywhere, or whether
# ProjectDetails remounts on project switch.
rg -n '<Terminal\b' -A2 -B2 src/renderer/components/terminal/index.tsx
rg -n '<TerminalLayout\b' -A5 -B2 src/renderer/screens/projectDetails/index.tsx
rg -n 'key=\{.*project' src/renderer/screens/projectDetails/index.tsx src/renderer/components/terminal/index.tsxRepository: rosettadb/dbt-studio
Length of output: 727
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- terminal.tsx around the effect ---'
sed -n '1,120p' src/renderer/components/terminal/terminal.tsx | cat -n
echo
echo '--- projectDetails index around TerminalLayout and project change handling ---'
sed -n '1120,1225p' src/renderer/screens/projectDetails/index.tsx | cat -n
echo
echo '--- search for project-id keyed remounts in relevant files ---'
rg -n 'key=\{[^}]*project|key=.*projectId|key=.*project\.id|<Terminal\b|<TerminalLayout\b|previousProjectPathRef|reset\(\)' \
src/renderer/components/terminal/terminal.tsx \
src/renderer/components/terminal/index.tsx \
src/renderer/screens/projectDetails/index.tsxRepository: rosettadb/dbt-studio
Length of output: 10285
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- projectDetails reset logic around previousProjectPathRef ---'
sed -n '480,530p' src/renderer/screens/projectDetails/index.tsx | cat -n
echo
echo '--- projectDetails component signature / project source ---'
sed -n '1,260p' src/renderer/screens/projectDetails/index.tsx | cat -n | sed -n '1,220p'
echo
echo '--- terminal component usage in parent file ---'
sed -n '580,610p' src/renderer/components/terminal/index.tsx | cat -nRepository: rosettadb/dbt-studio
Length of output: 11617
Clear terminal output when the active project changes
Terminal isn’t remounted on project switch, so clearOutput() in a [] effect only runs once on the initial mount. Use [project.id] (or the active project signal) so each project starts with an empty terminal.
🤖 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 `@src/renderer/components/terminal/terminal.tsx` around lines 38 - 42, The
Terminal component’s clearOutput effect only runs once on initial mount, so
switching projects leaves old terminal output visible. Update the
React.useEffect in terminal.tsx to depend on the active project identity (such
as project.id or the project signal) instead of an empty dependency array, and
keep calling clearOutput from that effect so each project switch resets the
terminal.
| await recordCommandFinished( | ||
| runHistoryId, | ||
| project.id, | ||
| `${project.path}/target/run_results.json`, | ||
| ); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect how executeCommand dispatches per command and confirm the artifact path is unconditional.
fd -t f 'useDbt.ts' src/renderer/hooks --exec sed -n '260,400p' {}Repository: rosettadb/dbt-studio
Length of output: 3982
🌐 Web query:
which dbt commands write target/run_results.json (deps clean debug docs serve)
💡 Result:
The target/run_results.json file is generated by dbt commands that execute, compile, or catalog nodes within a DAG, recording timing and status information for each completed invocation [1][2]. Of the commands you listed, the behavior regarding target/run_results.json is as follows: - dbt clean: Does not generate this file. It is a utility command specifically designed to delete artifacts and directories (such as the target/ directory) specified in your configuration [3][4]. - dbt deps: Does not generate this file. Its purpose is to download dependencies specified in packages.yml [3][4]. - dbt debug: Does not generate run_results.json by default. While the dbt debug command documentation references a --no-write-json flag [5], this refers to its ability to toggle the generation of artifacts generally; however, debug itself is intended to check connections and system setup rather than execute model nodes [4]. - dbt docs generate: Does not generate run_results.json. It generates catalog.json and index.html files [2][6]. - dbt docs serve: Does not generate this file. It is a web server command that serves documentation files already generated by dbt docs generate [6][4]. In contrast, the primary commands that produce run_results.json are those that interact with the DAG and execute operations, such as: - dbt run [7][8] - dbt test [7][8] - dbt build [7][9] - dbt seed [7][8] - dbt snapshot [8][10] For these commands, the generation of JSON artifacts is controlled by the --write-json flag (which defaults to True) [11][12].
Citations:
- 1: https://docs.getdbt.com/reference/artifacts/run-results-json
- 2: https://docs.getdbt.com/reference/artifacts/dbt-artifacts.md
- 3: https://docs.getdbt.com/reference/dbt-commands
- 4: https://docs.getdbt.com/category/list-of-commands
- 5: https://docs.getdbt.com/reference/commands/debug
- 6: https://docs.getdbt.com/reference/commands/cmd-docs.md
- 7: https://docs.getdbt.com/reference/node-selection/configure-state.md
- 8: https://guitton.co/posts/dbt-artifacts
- 9: https://docs.getdbt.com/reference/commands/build
- 10: https://tomasfarias.dev/articles/working-with-dbts-run-results/
- 11: https://docs.getdbt.com/reference/global-configs/json-artifacts
- 12: https://docs.getdbt.com/reference/global-configs/about-global-configs.md
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the run-history helpers and command mapping around useDbt.ts.
FILE="src/renderer/hooks/useDbt.ts"
echo "=== recordCommandFinished / related helpers ==="
ast-grep outline "$FILE" --view expanded | sed -n '1,220p'
echo
echo "=== Relevant source slice around recordCommandFinished and executeCommand ==="
sed -n '120,240p' "$FILE"
echo
sed -n '300,380p' "$FILE"
echo
echo "=== Command type / builder references in repository ==="
rg -n "type DbtCommandType|DbtCommandType|buildCommand\\(" src/renderer -SRepository: rosettadb/dbt-studio
Length of output: 9474
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="src/renderer/hooks/useDbt.ts"
echo "=== buildCommand and run-history helper bodies ==="
sed -n '220,300p' "$FILE"
echo
sed -n '1,120p' "$FILE"
echo
echo "=== recordCommandFinished implementation if defined elsewhere ==="
rg -n "recordCommandFinished|invocationId|run_results\\.json" src -SRepository: rosettadb/dbt-studio
Length of output: 7318
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="src/renderer/hooks/useDbtRunHistory.ts"
echo "=== file size ==="
wc -l "$FILE"
echo
echo "=== AST outline ==="
ast-grep outline "$FILE" --view expanded | sed -n '1,240p'
echo
echo "=== recordCommandFinished body and adjacent helpers ==="
sed -n '1,260p' "$FILE"Repository: rosettadb/dbt-studio
Length of output: 8255
Avoid attaching run_results.json for commands that don’t emit it. recordCommandFinished always reads ${project.path}/target/run_results.json, so deps/clean/debug/docs:serve can pick up a stale artifact from a prior run and overwrite this entry with unrelated results; if that file carries an old invocation_id, the dedupe step can also delete the original run entry. Only load the artifact for commands that actually write it, or skip it unless the parsed invocationId matches the current run.
🤖 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 `@src/renderer/hooks/useDbt.ts` around lines 350 - 354, The
`recordCommandFinished` call in `useDbt` is always attaching
`${project.path}/target/run_results.json`, which lets commands like `deps`,
`clean`, `debug`, and `docs:serve` reuse stale artifacts and corrupt the current
run history. Update the logic around `recordCommandFinished` to only read and
attach the artifact for commands that actually produce `run_results.json`, or
verify the parsed `invocationId` matches the current run before saving it, so
unrelated prior results are not recorded.
| React.useEffect(() => { | ||
| if (!projectId) return; | ||
| try { | ||
| const storedHistory = localStorage.getItem(getHistoryKey(projectId)); | ||
| const storedBookmarks = localStorage.getItem(getBookmarkKey(projectId)); | ||
|
|
||
| const savedHistory: ProjectQueryHistoryItem[] = storedHistory | ||
| ? JSON.parse(storedHistory) | ||
| : []; | ||
| const savedBookmarks: ProjectQueryBookmark[] = storedBookmarks | ||
| ? JSON.parse(storedBookmarks) | ||
| : []; | ||
|
|
||
| setState((current) => ({ | ||
| ...current, | ||
| history: savedHistory, | ||
| bookmarks: savedBookmarks, | ||
| })); | ||
| } catch (e) { | ||
| // eslint-disable-next-line no-console | ||
| console.warn('Failed to parse query state from localStorage', e); | ||
| setState((current) => ({ ...current, history: [], bookmarks: [] })); | ||
| } | ||
| }, [projectId]); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Clear transient preview state when projectId changes.
The load effect only replaces history and bookmarks but preserves result, error, rawSql, compiledSql, filePath, modelName, and lastDurationMs from the previous project. Switching projects shows stale query results until a new query is run.
🛡️ Proposed fix
React.useEffect(() => {
if (!projectId) return;
try {
const storedHistory = localStorage.getItem(getHistoryKey(projectId));
const storedBookmarks = localStorage.getItem(getBookmarkKey(projectId));
const savedHistory: ProjectQueryHistoryItem[] = storedHistory
? JSON.parse(storedHistory)
: [];
const savedBookmarks: ProjectQueryBookmark[] = storedBookmarks
? JSON.parse(storedBookmarks)
: [];
setState((current) => ({
...current,
+ activeTab: 'preview',
+ isRunning: false,
+ result: undefined,
+ error: undefined,
+ rawSql: undefined,
+ compiledSql: undefined,
+ filePath: undefined,
+ modelName: undefined,
+ lastDurationMs: undefined,
history: savedHistory,
bookmarks: savedBookmarks,
}));📝 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.
| React.useEffect(() => { | |
| if (!projectId) return; | |
| try { | |
| const storedHistory = localStorage.getItem(getHistoryKey(projectId)); | |
| const storedBookmarks = localStorage.getItem(getBookmarkKey(projectId)); | |
| const savedHistory: ProjectQueryHistoryItem[] = storedHistory | |
| ? JSON.parse(storedHistory) | |
| : []; | |
| const savedBookmarks: ProjectQueryBookmark[] = storedBookmarks | |
| ? JSON.parse(storedBookmarks) | |
| : []; | |
| setState((current) => ({ | |
| ...current, | |
| history: savedHistory, | |
| bookmarks: savedBookmarks, | |
| })); | |
| } catch (e) { | |
| // eslint-disable-next-line no-console | |
| console.warn('Failed to parse query state from localStorage', e); | |
| setState((current) => ({ ...current, history: [], bookmarks: [] })); | |
| } | |
| }, [projectId]); | |
| React.useEffect(() => { | |
| if (!projectId) return; | |
| try { | |
| const storedHistory = localStorage.getItem(getHistoryKey(projectId)); | |
| const storedBookmarks = localStorage.getItem(getBookmarkKey(projectId)); | |
| const savedHistory: ProjectQueryHistoryItem[] = storedHistory | |
| ? JSON.parse(storedHistory) | |
| : []; | |
| const savedBookmarks: ProjectQueryBookmark[] = storedBookmarks | |
| ? JSON.parse(storedBookmarks) | |
| : []; | |
| setState((current) => ({ | |
| ...current, | |
| activeTab: 'preview', | |
| isRunning: false, | |
| result: undefined, | |
| error: undefined, | |
| rawSql: undefined, | |
| compiledSql: undefined, | |
| filePath: undefined, | |
| modelName: undefined, | |
| lastDurationMs: undefined, | |
| history: savedHistory, | |
| bookmarks: savedBookmarks, | |
| })); | |
| } catch (e) { | |
| // eslint-disable-next-line no-console | |
| console.warn('Failed to parse query state from localStorage', e); | |
| setState((current) => ({ ...current, history: [], bookmarks: [] })); | |
| } | |
| }, [projectId]); |
🤖 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 `@src/renderer/hooks/useProjectQueryResultsPanel.ts` around lines 38 - 61, When
`projectId` changes in `useProjectQueryResultsPanel`, the load effect should
reset transient preview fields instead of only merging in `history` and
`bookmarks`. Update the `React.useEffect` that reads from `localStorage` so the
state passed to `setState` also clears `result`, `error`, `rawSql`,
`compiledSql`, `filePath`, `modelName`, and `lastDurationMs` back to their
empty/default values for the new project. Keep the existing parsing and fallback
behavior, but ensure stale query panel data cannot carry over between projects.
| const result = await queryData({ | ||
| connection: connection.connection, | ||
| query: executableSql, | ||
| projectName: project.name, | ||
| }); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check queryData implementation for limit support
# Find and inspect the queryData function signature and implementation
rg -n -A 20 'export.*queryData|async.*queryData' --type=ts src/renderer/services/connectors.service.tsRepository: rosettadb/dbt-studio
Length of output: 761
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the hook and related types/usages for limit handling.
echo '--- useProjectSqlExecution.ts ---'
ast-grep outline src/renderer/hooks/useProjectSqlExecution.ts --view expanded || true
echo
rg -n -A 8 -B 8 'limit|queryData\(|executableSql|ProjectQueryHistoryItem|executeProjectSql' src/renderer/hooks/useProjectSqlExecution.ts src/renderer -g '!**/*.map'Repository: rosettadb/dbt-studio
Length of output: 50376
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the connector query endpoint and any server-side handling of query limits.
rg -n -A 20 -B 8 "connector:query|queryData|limit" src -g '!**/*.map'Repository: rosettadb/dbt-studio
Length of output: 50377
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the IPC handler for connector:query and inspect the request forwarded to the backend.
rg -n -A 25 -B 10 "connector:query|queryData\(|execute_sql_request|execute_sql\(" src/main src/renderer -g '!**/*.map' | sed -n '1,220p'Repository: rosettadb/dbt-studio
Length of output: 16350
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the backend connector query implementation and the request shape it receives.
ast-grep outline src/main/utils/connectors.ts --view expanded || true
echo
rg -n -A 40 -B 20 'connector:query|queryData|execute_sql_request|execute_sql\(' src/main/utils/connectors.ts src/main -g '!**/*.map' | sed -n '1,260p'Repository: rosettadb/dbt-studio
Length of output: 16914
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the main-process handler for connector:query and inspect which query executor it calls.
rg -n -A 25 -B 12 "'connector:query'|\"connector:query\"|executeKineticaQuery|executePostgresQuery|executeSnowflakeQuery|executeDatabricksQuery|executeBigQueryQuery|executeDuckDBQuery|executeRedshiftQuery" src/main src -g '!**/*.map' | sed -n '1,260p'Repository: rosettadb/dbt-studio
Length of output: 16234
Thread the selected limit into project query execution src/renderer/hooks/useProjectSqlExecution.ts:104
The limit control is ignored here: queryData only forwards { connection, query, projectName, queryId }, and the backend query executors run the SQL without any limit parameter. The selected limit has no effect, so project queries return the full result set.
🤖 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 `@src/renderer/hooks/useProjectSqlExecution.ts` around lines 104 - 108, Project
SQL execution is ignoring the user-selected limit, so the result set is always
unbounded. Thread the selected limit through useProjectSqlExecution into the
queryData call, then extend queryData and the backend query executor path to
accept and apply that limit when running the SQL. Make sure the limit is passed
along with connection, query, and projectName so project queries respect the
selected cap end to end.
| } catch (error) { | ||
| const errorMessage = | ||
| error instanceof Error ? error.message : 'Unknown error'; | ||
| onError?.({ | ||
| projectId: project.id, | ||
| projectName: project.name, | ||
| filePath, | ||
| modelName, | ||
| rawSql, | ||
| compiledSql: querySql, | ||
| durationMs: Date.now() - startedAt, | ||
| errorMessage, | ||
| }); | ||
| toast.error(`Query failed: ${errorMessage}`); | ||
| return undefined; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use compiledSql variable instead of querySql in the catch block.
When compileModel is true and compilation succeeds but a later step throws (e.g., queryData fails), the catch block reports compiledSql: querySql instead of the actual compiled SQL stored in the compiledSql variable. This loses valuable debugging context in the error payload.
🐛 Proposed fix
} catch (error) {
const errorMessage =
error instanceof Error ? error.message : 'Unknown error';
onError?.({
projectId: project.id,
projectName: project.name,
filePath,
modelName,
rawSql,
- compiledSql: querySql,
+ compiledSql,
durationMs: Date.now() - startedAt,
errorMessage,
});📝 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.
| } catch (error) { | |
| const errorMessage = | |
| error instanceof Error ? error.message : 'Unknown error'; | |
| onError?.({ | |
| projectId: project.id, | |
| projectName: project.name, | |
| filePath, | |
| modelName, | |
| rawSql, | |
| compiledSql: querySql, | |
| durationMs: Date.now() - startedAt, | |
| errorMessage, | |
| }); | |
| toast.error(`Query failed: ${errorMessage}`); | |
| return undefined; | |
| } | |
| } catch (error) { | |
| const errorMessage = | |
| error instanceof Error ? error.message : 'Unknown error'; | |
| onError?.({ | |
| projectId: project.id, | |
| projectName: project.name, | |
| filePath, | |
| modelName, | |
| rawSql, | |
| compiledSql, | |
| durationMs: Date.now() - startedAt, | |
| errorMessage, | |
| }); | |
| toast.error(`Query failed: ${errorMessage}`); | |
| return undefined; | |
| } |
🤖 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 `@src/renderer/hooks/useProjectSqlExecution.ts` around lines 143 - 158, The
catch block in useProjectSqlExecution is reporting the wrong SQL value in the
error payload. Update the onError call to use the already-computed compiledSql
variable instead of querySql so failures after compileModel preserve the actual
compiled SQL. Keep the rest of the error handling in executeSqlQuery unchanged.
1 participant
Summary by CodeRabbit
New Features
Bug Fixes