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
|
Warning Review limit reached
Next review available in: 47 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR adds project-scoped SQL preview/history/bookmarks, DBT run-history tracking, Monaco query and CTE execution, terminal panel integration, credential-aware dbt execution, chat tool-status normalization, and table/UI updates. ChangesProject query and DBT history
DBT tools and agent guidance
Chat and table UI updates
Estimated code review effort: 4 (Complex) | ~75 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
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
CodeRabbit fixes: - Fix projectAgent.ts phrasing contradiction (use "command: debug" not "dbt debug") - Handle empty env vars as missing in dbt.tools.ts (use existence check instead of truthiness) - Add shared @Keyframes spin to global App.css for HourglassEmptyIcon animation - Move ml: 'auto' from Tooltip to IconButton in DbtRunHistoryToolbar - Show error status icon for failed history items in ProjectQueryResultsPanel - Pass missing callback props to fullscreen instance in ProjectQueryResultsPanel - Clear terminal output when active project changes in terminal.tsx - Only attach run_results.json for commands that produce it (run, test, seed, snapshot) - Clear transient preview state when projectId changes in useProjectQueryResultsPanel - Thread limit parameter through query execution pipeline (useProjectSqlExecution, connectors.service, projectDetails) - Use compiledSql variable instead of querySql in catch block in useProjectSqlExecution Query history UX improvements: - Display model name instead of full SQL in history/bookmark titles - Add tooltips to show full SQL on hover - Add copy button to copy queries to clipboard
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/renderer/services/connectors.service.ts (1)
71-87: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider extracting the duplicated request body type.
The inline type
{ connection: ConnectionInput; query: string; projectName: string; queryId?: string; limit?: number }is defined identically in both the function parameter and theclient.postgeneric argument. Extracting a named interface would eliminate the duplication and keep future additions in sync.♻️ Proposed refactor
+interface QueryDataRequest { + connection: ConnectionInput; + query: string; + projectName: string; + queryId?: string; + limit?: number; +} + -export const queryData = async (body: { - connection: ConnectionInput; - query: string; - projectName: string; - queryId?: string; - limit?: number; -}): Promise<QueryResponseType> => { +export const queryData = async (body: QueryDataRequest): Promise<QueryResponseType> => { const { data } = await client.post< - { - connection: ConnectionInput; - query: string; - projectName: string; - queryId?: string; - limit?: number; - }, + QueryDataRequest, QueryResponseType >('connector:query', body); return data; };🤖 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/services/connectors.service.ts` around lines 71 - 87, Extract the duplicated request body shape used by queryData and its client.post generic into a single named interface or type, then reuse that symbol for both declarations. Preserve all existing fields and optionality while keeping queryData’s behavior unchanged.
🤖 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.
Nitpick comments:
In `@src/renderer/services/connectors.service.ts`:
- Around line 71-87: Extract the duplicated request body shape used by queryData
and its client.post generic into a single named interface or type, then reuse
that symbol for both declarations. Preserve all existing fields and optionality
while keeping queryData’s behavior unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 25bb5392-218d-46be-8fe3-c7a2a1423e22
📒 Files selected for processing (11)
src/main/services/ai/agents/projectAgent.tssrc/main/services/ai/tools/dbt.tools.tssrc/renderer/App.csssrc/renderer/components/dbtRunHistory/DbtRunHistoryToolbar.tsxsrc/renderer/components/projectQueryResults/ProjectQueryResultsPanel.tsxsrc/renderer/components/terminal/terminal.tsxsrc/renderer/hooks/useDbt.tssrc/renderer/hooks/useProjectQueryResultsPanel.tssrc/renderer/hooks/useProjectSqlExecution.tssrc/renderer/screens/projectDetails/index.tsxsrc/renderer/services/connectors.service.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- src/renderer/components/terminal/terminal.tsx
- src/main/services/ai/agents/projectAgent.ts
- src/main/services/ai/tools/dbt.tools.ts
- src/renderer/hooks/useDbt.ts
- src/renderer/hooks/useProjectSqlExecution.ts
- src/renderer/hooks/useProjectQueryResultsPanel.ts
- src/renderer/screens/projectDetails/index.tsx
- src/renderer/components/projectQueryResults/ProjectQueryResultsPanel.tsx
…t-query-results-and-run-history-funcionality
Summary by CodeRabbit
profiles.ymland stored credentials (including env vars).