fix: resolve project context for untitled query files#1842
fix: resolve project context for untitled query files#1842shreyastelkar wants to merge 9 commits intomasterfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughPersist the selected dbt project when creating an untitled SQL file and use that persisted project to resolve project context for compiling/executing queries from untitled editors. RunModel now depends on QueryManifestService and workspace state is cleared when its selected project root is unregistered. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Index as "commands/index.ts"
participant Workspace as "Workspace State"
participant Editor as "Untitled Editor"
participant RunModel as "commands/runModel.ts"
participant QueryManifest as "QueryManifestService"
participant Container as "DBTProjectContainer"
User->>Index: New Query / pick project
Index->>Workspace: persist dbtPowerUser.projectSelected
Index->>Editor: open untitled SQL
User->>RunModel: Execute/Compile (untitled URI)
RunModel->>Workspace: read dbtPowerUser.projectSelected
alt no persisted project
RunModel->>QueryManifest: getOrPickProjectFromWorkspace()
QueryManifest-->>RunModel: selected project
RunModel->>Workspace: persist dbtPowerUser.projectSelected
end
RunModel->>Container: compile/execute with resolveProjectUri(untitled URI)
Container->>Workspace: read dbtPowerUser.projectSelected to resolve URI
Container-->>RunModel: compile/execute result
RunModel-->>User: show results
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| const query = window.activeTextEditor.document.getText(); | ||
| if (query !== undefined) { | ||
| this.compileDBTQuery(fullPath, query); |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/commands/runModel.ts`:
- Around line 48-60: In compileQueryOnActiveWindow(), ensure you check the
boolean result of ensureProjectForUntitledUri() and bail out if it returns false
(like executeQueryOnActiveWindow does) before calling compileDBTQuery; i.e.,
after detecting an untitled URI call ensureProjectForUntitledUri(), and if it
returns false return early so compileDBTQuery(fullPath, query) is not invoked
with an unresolved/untitled project.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1a1c6f19-df8c-47b1-b66f-c42c9d2dd036
📒 Files selected for processing (4)
src/commands/index.tssrc/commands/runModel.tssrc/dbt_client/dbtProjectContainer.tssrc/inversify.config.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/commands/runModel.ts (1)
82-84: Minor: Error message mentions "executing" but method is also used for compilation.The error message references "executing query" but
ensureProjectForUntitledUri()is called from bothcompileQueryOnActiveWindow()andexecuteQueryOnActiveWindow(). Consider a more generic message.Suggested fix
if (!project) { window.showErrorMessage( - "Unable to find dbt project for executing query.", + "Unable to find dbt project for this query.", ); return false; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/runModel.ts` around lines 82 - 84, The error message in ensureProjectForUntitledUri() is too specific ("Unable to find dbt project for executing query.") even though this helper is used by both compileQueryOnActiveWindow() and executeQueryOnActiveWindow(); update the message to be generic (e.g., "Unable to find dbt project for this operation.") by changing the string passed to window.showErrorMessage in ensureProjectForUntitledUri() so it accurately covers both compilation and execution calls.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/commands/runModel.ts`:
- Around line 82-84: The error message in ensureProjectForUntitledUri() is too
specific ("Unable to find dbt project for executing query.") even though this
helper is used by both compileQueryOnActiveWindow() and
executeQueryOnActiveWindow(); update the message to be generic (e.g., "Unable to
find dbt project for this operation.") by changing the string passed to
window.showErrorMessage in ensureProjectForUntitledUri() so it accurately covers
both compilation and execution calls.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fe004b6a-765d-4b24-9dd1-d687768704df
📒 Files selected for processing (1)
src/commands/runModel.ts
![setu-altimateai[bot]](https://avatars.githubusercontent.com/in/2889131?s=60&v=4){kind=small}
| this.executeSQL(modelPath, query, modelName); | ||
| } | ||
|
|
||
| runModelOnNodeTreeItem(type: RunModelType) { |
There was a problem hiding this comment.
nit: For untitled URIs, modelPath.fsPath returns something like "untitled:Untitled-1" on macOS/Linux, so path.basename(modelPath.fsPath, ".sql") produces "untitled:Untitled-1" as the modelName. This gets passed to executeSQLOnQueryPanel as the query tab name — probably shows up as an ugly label in the results panel.
Not a correctness issue (execution still works), but could be a minor UX nit. Consider a fallback like:
const modelName = modelPath.scheme === "untitled"
? "untitled"
: path.basename(modelPath.fsPath, ".sql");There was a problem hiding this comment.
Already addressed — see lines 129-132 in the current code:
const modelName =
modelPath.scheme === "untitled"
? "untitled"
: path.basename(modelPath.fsPath, ".sql");
| this.setToWorkspaceState("dbtPowerUser.projectSelected", undefined); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
thought: I verified that DBTProject.contains() uses uri.fsPath === this.projectRoot.fsPath || uri.fsPath.startsWith(this.projectRoot.fsPath + path.sep) — so an exact === comparison here is correct and consistent with how projects are keyed. Just documenting the verification in case future reviewers wonder why it's not startsWith.
There was a problem hiding this comment.
Good catch documenting this. The exact === match on fsPath is intentional and consistent with how DBTWorkspaceFolder.contains() keys projects.
There was a problem hiding this comment.
PR #1842 Review — Fix project context for untitled query files
Summary: Well-scoped fix for a real UX bug. The root cause analysis is correct (URI scheme mismatch → silent ?. no-op), the fix is layered correctly across the call stack, and the third commit's refactors (stale state cleanup, toProjectQuickPickItem extraction) show good follow-through on the review cycle. No blockers.
Files reviewed
src/commands/index.ts +11/-1 (persist project on createSqlFile)
src/commands/runModel.ts +51/-6 (ensureProjectForUntitledUri, async guards)
src/dbt_client/dbtProjectContainer.ts +51/-10 (resolveProjectUri, clearStaleProjectSelection)
src/inversify.config.ts +4/-1 (wire QueryManifestService into RunModel)
src/quickpick/projectQuickPick.ts +14/-7 (extract toProjectQuickPickItem)
What looks good
Root cause fix is correct. The untitled: scheme can't be matched by findDBTProject because DBTProject.contains() compares fsPath values — untitled URIs have no meaningful fsPath. Storing the selected project in workspace state at creation time and resolving it on demand is the right approach.
resolveProjectUri returning project root works. I checked DBTProject.contains() — it uses uri.fsPath === this.projectRoot.fsPath || uri.fsPath.startsWith(this.projectRoot.fsPath + sep), so passing the project root URI directly matches the exact-root case. Left a thought comment on this.
Uri.file(raw.fsPath || raw.path) reconstruction. Correct approach — workspace state deserializes Uri as plain JSON, losing the prototype chain. Reconstructing via Uri.file() ensures .fsPath works on retrieval.
clearStaleProjectSelection on unregister. Good defensive addition. The === comparison is correct given how project roots are keyed (verified against DBTProject.contains() impl).
compileQuery untitled guard. The second commit fixing the discarded return value from ensureProjectForUntitledUri() in compileQueryOnActiveWindow is a real bug catch — without it, a cancelled project picker would still proceed to compile.
toProjectQuickPickItem extraction. Clean DRY refactor. The {label, description, uri} shape was duplicated at 3 call sites — good to consolidate before this adds more.
DI wiring. RunModel → QueryManifestService → DBTProjectContainer is a clean DAG with no circular dependency, as noted in the PR description.
Issues
One nit inline on modelName for untitled URIs (see inline comment) — UX-only, not correctness.
Checklist
| Area | Status |
|---|---|
| Root cause addressed | Pass |
| Single-project workspace | Pass (auto-selects, no prompt) |
| Multi-project workspace | Pass (prompts on first use, persists) |
| createSqlFile path | Pass |
| File > New File + jinja-sql path | Pass (ensureProjectForUntitledUri fallback) |
| Compile path | Pass (+ second commit bail-out fix) |
| Stale state on project removal | Pass |
| DI wiring | Pass |
| No circular deps | Pass |
lgtm — ship it.
Untitled files created via 'New query' or manually silently failed to execute or compile because the selected project was never persisted to workspace state, and executeSQL/compileQuery could not resolve untitled URIs to a dbt project. Changes: - createSqlFile now stores the selected project in workspace state - Extract resolveProjectUri() helper with safe Uri reconstruction from deserialized workspace state; apply to both executeSQL and compileQuery - Add ensureProjectForUntitledUri() fallback in RunModel that validates stored project still exists (clears stale state) and falls back to getOrPickProjectFromWorkspace for untitled files not created via createSqlFile - Extract toProjectQuickPickItem() to eliminate duplicated shape across 3 call sites - Invalidate dbtPowerUser.projectSelected when a project is unregistered - Use clean 'untitled' modelName instead of raw fsPath for untitled URIs - Wire QueryManifestService into RunModel via DI Fixes #1827
shreyastelkar
force-pushed
the
fix/untitled-query-execution
branch
from
8c366b5 to
45646e1
Compare
…eview The 'Compiled dbt Preview' command (dbtPowerUser.sqlPreview) went through SqlPreviewContentProvider which assumed file: scheme for document lookup and fell back to readFileSync — causing ENOENT for untitled query files. Changes: - requestCompilation() now tries untitled: scheme when looking up the source document, uses resolveProjectUri for project resolution, and avoids disk reads for untitled files - onDidChangeTextDocument handler also checks untitled: scheme so live-update works when typing in untitled files with preview open - Make resolveProjectUri non-private so SqlPreviewContentProvider can use it for untitled URI resolution
Move query/URI capture after the ensureProjectForUntitledUri() await in both compileQueryOnActiveWindow and executeQueryOnActiveWindow. If the user switches editors while the project picker is open, we now read from whichever editor is active when the picker resolves.
This reverts commit 36c3bd0.
…mpile preview" This reverts commit a32285f.
Move untitled URI resolution inside findDBTProject so every caller gets untitled support automatically. resolveProjectUri stays private — no public API surface change. executeSQL and compileQuery simplified to plain findDBTProject calls.
Workspace state deserializes Uri as a plain JS object. The .fsPath getter lives on the Uri prototype and is lost during serialization, so raw.fsPath was always undefined. Use .path directly.
| if (!selectedProject?.uri) { | ||
| return; | ||
| } | ||
| if (selectedProject.uri.path === removedRoot.fsPath) { |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Valid catch. Fixed — now reconstructs via Uri.file(selectedProject.uri.path).fsPath before comparing to removedRoot.fsPath, so both sides use OS-native separators.
…Windows Uri.path uses forward slashes (/C:/Users/project) while Uri.fsPath uses OS-native separators (C:\Users\project). Reconstruct via Uri.file() before comparing to ensure cross-platform consistency.
Tests multi-project workspace scenarios: - untitled URI resolves to stored project from workspace state - file-scheme URIs bypass workspace state lookup - untitled URI returns undefined when no project stored - stale project selection is cleared on unregister
| // Persist project selection so untitled files can resolve | ||
| // project context during query execution and compilation | ||
| this.dbtProjectContainer.setToWorkspaceState( | ||
| "dbtPowerUser.projectSelected", |
There was a problem hiding this comment.
This already has some significance in other screens. It was not required before. Any way to avoid this?
I thought it worked like this before:
if single project. -> use that project, if multiple projects -> show dropdown
2 participants
Summary
Related issue: #1827
Fixes untitled query files silently failing in multi-project workspaces.
On multi-project workspace:
Before:
After (shows codelens to select project):
Broken flows
"Create new SQL file" → Cmd+Enter → nothing happens.
createSqlFilepicks a project but doesn't persist it to workspace state.executeSQLreadsdbtPowerUser.projectSelected→ empty →findDBTProject(untitledUri)can't match → silent no-op.File > New File → jinja-sql → Cmd+Enter → nothing happens. No project is persisted and no fallback exists to prompt the user.
In single-project workspaces both flows already work. In multi-project, they only work if the user previously clicked the "Project: ..." codelens (the only thing that writes
dbtPowerUser.projectSelectedonmain).Changes
createSqlFilepersists the selected project to workspace state after pickingensureProjectForUntitledUrifallback inRunModelfor the File > New File flow — auto-selects single project or prompts in multi-projectfindDBTProjectnow callsresolveProjectUriinternally — every caller gets untitled URI support automatically,resolveProjectUristays privateclearStaleProjectSelectionclears workspace state when a project is removed (defensive)Follow-up
Compiled dbt Preview also fails for untitled files in multi-project — tracked in #1845.