feat: run a saved model as its real node when previewing (#2028) - #2055
feat: run a saved model as its real node when previewing (#2028)#2055ralphstodomingo wants to merge 8 commits into
Conversation
Execute SQL sends the editor contents as `dbt show --inline`, which compiles them as an anonymous node. Jinja that reads the current node's identity — `model.name`, `this`, `model.config`, or a macro derived from them — resolves against a placeholder, so the model fails to compile. Route the existing Execute SQL path instead of adding a command. In order: - a selection means the fragment is the point, so run it inline, unchanged - an untitled buffer has no node, so run inline, unchanged - a saved file with no unsaved edits runs as the real node. The buffer is byte-identical to disk, so the SQL is the same and only identity resolution changes - a file with unsaved edits runs inline, because the edits are nearly always what was meant. Only if that run fails for lack of node identity is the saved-node alternative offered, since that failure is the one reliable signal it would have behaved differently Whether an integration can supply the node as context comes from `supportsExecuteModel()` rather than from the configured mode: which modes qualify belongs to the library and changes as integrations gain it. The Python bridge reports false today, so those users always run inline and never see the offer. The offer saves the document only after the user chooses to, and never silently.
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
Docker E2E — routing verified in the running extensionBuilt from this branch with AltimateAI/altimate-dbt-integration#148 linked, deployed to Every case below is evidenced by dbt's own invocation log, not inferred from the UI, so each shows the exact command the extension produced. Case 3 — saved, no unsaved edits → runs as the real nodeThis is the reported bug (#2028), and it is gone.
Case 1 — selection → still inline, unchangedWith lines 2-4 selected (excluding the macro call):
That run also had unsaved edits present, so it doubles as proof that rule 1 takes precedence over rule 4: a selection runs inline and raises no offer. Case 4 — unsaved edits → inline, then offerRuns inline as intended, and fails on identity: The offer then appears:
The button works. Clicking it, with the file dirty:
It saves the document and re-runs as the real node — only after the user chooses it. Nothing is ever saved silently. Case 5 — integration without node-context supportThe Two findings worth a reviewer's attention1. A real bug, found and fixed here. The identity marker only attached when dbt reported the compile error on stdout. dbt-core reports it on stderr, where 2. A timing caveat worth being deliberate about. Immediately after a window reload or container restart, previews route inline until the manifest is parsed into the extension's memory — Suite310 library tests, 642 extension tests, typecheck clean, 0 lint errors. Note on merge orderThis PR is red until AltimateAI/altimate-dbt-integration#148 merges and a release cuts — it compiles only against Note Case 4's quoted prompt above is superseded. Review on AltimateAI/altimate-dbt-integration#148 found that the marker this hint keys off reports that the failed run was an inline preview, not why it failed — so the old wording asserted node identity as the cause even when a preview died on an unreachable warehouse. The prompt now reads:
Routing is unchanged, so cases 1-3 and the case-4 trigger below still hold as evidenced. The re-run confirming the new wording renders is pending and this note will be replaced with it. |
…aved-model-as-node
The library renamed the API this routing consumes, so that "execute a model" no longer reads as something any dbt version can do: - `executeModelWithLimit` -> `executeSqlWithNodeContextWithLimit` - `supportsExecuteModel` -> `canInjectNodeContextInSqlSnippetExecution` Local helpers and the telemetry label follow suit. Naming only; the routing rules and their tests are unchanged.
The library folded node-context execution into `executeSQL` options rather than
a separate method, so the routing now passes `{ useNodeContext: true }` instead
of calling a second entry point. `query` is empty there because dbt reads the
node from disk.
Routing rules and their tests are unchanged.
…-node' into feat/2028-execute-saved-model-as-node
The preview-failure hint asserted a cause: "did not compile because previewing unsaved SQL runs it as an anonymous query". The marker it keys off reports that the failed run was an inline preview, not why it failed, so a preview that died on an unreachable warehouse produced that sentence too — telling the user their connection error was a node-identity problem. Reworded to state the failure, explain the limitation, and offer the alternative, without claiming to know which one bit. Tracks the library rename to `isInlinePreviewError` / `InlinePreviewError`. 642 tests passing, typecheck clean.



Closes #1875. Fixes #2028. Replaces #1894, which added a separate command; this takes the approach @mdesmet proposed in review instead.
Problem
Execute SQL sends the editor contents as
dbt show --inline, which compiles them as an anonymous node (inline_<hash>). Any Jinja that reads the current node's identity —model.name,this,model.config, or a macro derived from them — resolves against a placeholder rather than the model in front of you, so the model fails to compile.Same root cause, two reported symptoms: #2028 (a
model.nameguard macro rejects the model) and #1875 (--inlineleavesselected_resourcesempty, so packages likeupstream-prodmisbehave).Approach
The earlier attempt added an opt-in
Execute dbt Modelcommand plus a hint pointing at it. That works, but the user still has to fail once and then press something else. Review preferred routing the existing path, which is what this does — no new command, no new keybinding, nothing to discover. For the reported bug,Cmd+Enternow simply works.Execute SQL decides in this order:
dbt show --select). No prompt: the buffer is byte-identical to disk, so the SQL is the same either way and the only difference is that identity now resolves.Case 3 is the only silent behaviour change, and it is safe precisely because the content is identical.
Integration capability
Whether an integration can supply the real node as context comes from
canInjectNodeContextInSqlSnippetExecution()on the integration, not from testingdbt.dbtIntegration. Which modes qualify belongs to the library and will change as integrations gain the capability, so the extension does not hardcode mode strings.The Python bridge reports
falsetoday — it compiles in-process under a temporary id and has no API for supplying the node. Those users always run inline and never see the offer. AI-8139 tracks fixing that at the source, after which it stops being an exception with no change here.Saving
We never save silently. Case 4's offer is a prompt; the document is written only after the user picks Save and run as model, and a failed save is reported rather than swallowed. Every other path leaves the file untouched.
Requires
AltimateAI/altimate-dbt-integration#148, which supplies the three things this routing needs: the
useNodeContextoption onexecuteSQL, thecanInjectNodeContextInSqlSnippetExecution()capability check, and the compile-error surfacing that produces the marker case 4 keys on. This PR is red until that merges and a release cuts, since it compiles only against those APIs. Verified locally against a build of that branch: typecheck clean, 642 tests passing.Tests
6 new unit tests, plus updates to two container assertions for the widened signature.
canRunAsModel: allowed for a known model on a supporting integration; refused when the integration cannot supply node context; refused when the file is not a manifest nodeVerification
The behaviour was verified end-to-end in code-server against a project whose macro asserts on
model.name, on thecorecommandpath — before, an inline preview of that model died with a bareTypeError; after, it returnsnode_name = stg_identity_probewith identity resolved. A scripted fixture lives intest-fixtures/inline-preview-loses-node-identity/.