fix: setup page UX improvements and broken links#1841
Merged
Conversation
- Make wizard steps clickable for back/forth navigation - Remove fixed sidebar width (280px), size to content - Responsive layout: vertical sidebar → horizontal top bar on narrow screens - Add "Tutorials" button on landing page - Fix broken doc URLs (404) → point to `https://docs.myaltimate.com/` - Fresh install shows "Supercharge" landing page instead of jumping to prerequisites Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
|
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:
WalkthroughReplaces the onboarding help command, adds a PythonEnvironment refresh method and awaits it during DBT checks, switches the onboarding panel listener to DBT installation verification events, and includes multiple onboarding UI updates (responsive wizard, docs URL changes, tutorials/images expansion, and minor props/state adjustments). Changes
Sequence DiagramsequenceDiagram
participant Client as VSCode Command
participant DBT as DBTClient
participant PyEnv as PythonEnvironment
participant Panel as OnboardingPanel
participant Webview as Webview
Client->>DBT: checkAllInstalled()
DBT->>DBT: detectDBT()
DBT->>PyEnv: refreshPythonEnvironment()
PyEnv->>PyEnv: ensure Python extension active
PyEnv->>PyEnv: query environment details
PyEnv->>DBT: return updated python version/isPython3
DBT->>Panel: emit DBTInstallationVerificationEvent (inProgress true/false)
Panel->>Panel: ignore events with inProgress === true
Panel->>Webview: send { command: "pythonEnvironmentChanged" } after verification complete
Webview->>Webview: runDiagnostics(silent) / update UI
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 Tip Migrating from UI to YAML configuration.Use the |
- Tutorial GIFs now open in full-screen overlay on click - Added hint text: "Click any image to expand it" - Zoom-in cursor on images, zoom-out cursor on overlay Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Pass current `dbtIntegrationType` from `PrerequisitesStep` to `InstallDbtStep` - Install step now defaults to the already-chosen integration type instead of always "core" Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Check `dbtProjectContainer.dbtInstalled` after install completes - Show actionable error message pointing to Output panel if detection fails - Prevents misleading "Installation successful" flash when install silently fails Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add `refreshPythonVersion()` to `PythonEnvironment` to re-fetch version from the Python extension API - Call it in `getDiagnosticsStatus` before returning Python info - Previously `pythonVersion` was set once at init and never updated Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
mdesmet
approved these changes
Mar 19, 2026
| - Pre-select the correct integration type in the "Install dbt" step based on the already-chosen type | ||
| - Fix false "Installation successful" for dbt Fusion/Cloud — verify dbt is actually detected after install, show actionable error if not | ||
| - Fix Fusion CLI detection (in `altimate-dbt-integration` PR #35) — was finding dbt Core in Python venv instead of Fusion binary at `~/.local/bin/dbt` | ||
| - Fix Python version not updating in setup UI after changing interpreter — refresh version from Python extension API on each diagnostics check |
|
|
||
| // Refresh Python version in case interpreter changed since last check | ||
| await pythonEnvironment.refreshPythonVersion(); | ||
|
|
Contributor
There was a problem hiding this comment.
pythonEnvironment should always have the right version.
| Need help choosing?{" "} | ||
| <a | ||
| href="https://docs.myaltimate.com/setup/integrations/" | ||
| href="https://docs.myaltimate.com/" |
Contributor
There was a problem hiding this comment.
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
Comment on lines
+326
to
+331
| if (!this.dbtProjectContainer.dbtInstalled) { | ||
| throw new Error( | ||
| `dbt ${integrationType} could not be detected after installation. ` + | ||
| `Check the Output panel (View → Output → "dbt") for details.`, | ||
| ); | ||
| } |
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}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
webview_panels/src/modules/onboarding/TutorialsStep.tsx (1)
311-323: Add keyboard-accessible overlay dismissal.The image overlay is mouse-dismiss only right now. Please add dialog semantics and Escape-to-close to improve accessibility.
Suggested patch
+ const handleOverlayKeyDown = (e: React.KeyboardEvent<HTMLDivElement>) => { + if (e.key === "Escape") { + setExpandedImage(null); + } + }; + {expandedImage && ( <div - role="presentation" + role="dialog" + aria-modal="true" + tabIndex={-1} className={classes.imageOverlay} onClick={() => setExpandedImage(null)} + onKeyDown={handleOverlayKeyDown} > <img src={expandedImage.src} alt={expandedImage.alt} className={classes.imageOverlayImg} /> </div> )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@webview_panels/src/modules/onboarding/TutorialsStep.tsx` around lines 311 - 323, The overlay currently only supports mouse closing; make it keyboard-accessible by giving the overlay div dialog semantics (add role="dialog" and aria-modal="true"), make it focusable (tabIndex={0}), add an onKeyDown handler that calls setExpandedImage(null) when Escape is pressed, and ensure it receives focus when opened by focusing an overlayRef (use a ref and a useEffect tied to expandedImage). Update references in TutorialsStep.tsx: the overlay div using classes.imageOverlay, the image using classes.imageOverlayImg, and the setter setExpandedImage so Escape and keyboard focus close the overlay.src/dbt_client/pythonEnvironment.ts (1)
82-84: Don’t swallow Python version refresh failures silently.If refresh fails, diagnostics can look stale with no trace. Please emit a debug/error log in the catch block.
Suggested patch
} catch { - // Keep existing version if refresh fails + this.dbtTerminal.debug( + "pythonEnvironment:refreshPythonVersion", + "Failed to refresh Python version; keeping previous values", + ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/dbt_client/pythonEnvironment.ts` around lines 82 - 84, The catch block that currently swallows exceptions during the Python version refresh must log the failure instead of being silent; update the catch in the refresh routine (the try/catch around the Python version refresh call in pythonEnvironment.ts) to call the module's logger (e.g., processLogger.error or the existing logger used elsewhere in this module) and include a clear message plus the caught error/stack (e.g., "Failed to refresh Python version" + error). Ensure you preserve the existing behavior of keeping the existing version but do not swallow the error — log at debug/error level with the error object for diagnostics.webview_panels/src/modules/onboarding/SetupWizard.tsx (1)
102-108:getSnapshotcreates a newMediaQueryListon every call.
useSyncExternalStoremay callgetSnapshotfrequently (on every render and during hydration checks). Creating a newmatchMediaobject each time is inefficient.Consider caching the
MediaQueryListinstance:♻️ Proposed fix to cache MediaQueryList
const NARROW_QUERY = "(max-width: 700px)"; +const narrowMql = typeof window !== "undefined" ? window.matchMedia(NARROW_QUERY) : null; + const subscribe = (cb: () => void) => { - const mql = window.matchMedia(NARROW_QUERY); - mql.addEventListener("change", cb); - return () => mql.removeEventListener("change", cb); + narrowMql?.addEventListener("change", cb); + return () => narrowMql?.removeEventListener("change", cb); }; -const getSnapshot = () => window.matchMedia(NARROW_QUERY).matches; +const getSnapshot = () => narrowMql?.matches ?? false;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@webview_panels/src/modules/onboarding/SetupWizard.tsx` around lines 102 - 108, getSnapshot currently calls window.matchMedia(NARROW_QUERY) every time which allocates a new MediaQueryList on each render; instead create and reuse a single cached MediaQueryList (e.g. const mql = typeof window !== "undefined" ? window.matchMedia(NARROW_QUERY) : null) at module or hook initialization and use that same mql inside subscribe and getSnapshot so addEventListener/removeEventListener and .matches operate on the same object; also guard for SSR by handling mql === null in both subscribe and getSnapshot to avoid accessing window on the server.
🤖 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/dbt_client/pythonEnvironment.ts`:
- Around line 82-84: The catch block that currently swallows exceptions during
the Python version refresh must log the failure instead of being silent; update
the catch in the refresh routine (the try/catch around the Python version
refresh call in pythonEnvironment.ts) to call the module's logger (e.g.,
processLogger.error or the existing logger used elsewhere in this module) and
include a clear message plus the caught error/stack (e.g., "Failed to refresh
Python version" + error). Ensure you preserve the existing behavior of keeping
the existing version but do not swallow the error — log at debug/error level
with the error object for diagnostics.
In `@webview_panels/src/modules/onboarding/SetupWizard.tsx`:
- Around line 102-108: getSnapshot currently calls
window.matchMedia(NARROW_QUERY) every time which allocates a new MediaQueryList
on each render; instead create and reuse a single cached MediaQueryList (e.g.
const mql = typeof window !== "undefined" ? window.matchMedia(NARROW_QUERY) :
null) at module or hook initialization and use that same mql inside subscribe
and getSnapshot so addEventListener/removeEventListener and .matches operate on
the same object; also guard for SSR by handling mql === null in both subscribe
and getSnapshot to avoid accessing window on the server.
In `@webview_panels/src/modules/onboarding/TutorialsStep.tsx`:
- Around line 311-323: The overlay currently only supports mouse closing; make
it keyboard-accessible by giving the overlay div dialog semantics (add
role="dialog" and aria-modal="true"), make it focusable (tabIndex={0}), add an
onKeyDown handler that calls setExpandedImage(null) when Escape is pressed, and
ensure it receives focus when opened by focusing an overlayRef (use a ref and a
useEffect tied to expandedImage). Update references in TutorialsStep.tsx: the
overlay div using classes.imageOverlay, the image using classes.imageOverlayImg,
and the setter setExpandedImage so Escape and keyboard focus close the overlay.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7366597b-dd6c-4af5-af8f-3e753bd5879b
📒 Files selected for processing (11)
.github/meta/pr-fixes.mdsrc/dbt_client/dbtProjectContainer.tssrc/dbt_client/pythonEnvironment.tssrc/webview_provider/onboardingPanel.tswebview_panels/src/modules/onboarding/DbtIntegrationSetup.tsxwebview_panels/src/modules/onboarding/InstallDbtStep.tsxwebview_panels/src/modules/onboarding/Onboarding.tsxwebview_panels/src/modules/onboarding/PrerequisitesStep.tsxwebview_panels/src/modules/onboarding/SetupWizard.tsxwebview_panels/src/modules/onboarding/TutorialsStep.tsxwebview_panels/src/modules/onboarding/onboarding.module.scss
…fully - Update all onboarding `docs.myaltimate.com` links to `/setup/reqdConfig/` - Return `boolean` from `installDbt()` to distinguish cancellation from failure - Skip post-install verification when user cancels version/adapter quick pick - Remove `pr-fixes.md` scratch file Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Comment on lines
+80
to
+81
| this._pythonVersion = envDetails?.version?.join("."); | ||
| this.isPython3 = envDetails?.version[0] === "3"; |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/commands/walkthroughCommands.ts (1)
363-371:⚠️ Potential issue | 🟠 MajorAvoid firing async retry commands without awaiting them across all install paths.
The race condition is not limited to line 368. All three install functions (
installDbtFusion,installDbtCloud,installDbtCore) firecommands.executeCommand("dbtPowerUser.installDbt")without awaiting:
- Line 220 (
installDbtFusion)- Line 270 (
installDbtCloud)- Line 368 (
installDbtCore)This allows each function to complete and return to the caller while the retry command is still in flight, potentially racing downstream post-install verification. The issue is most acute in
installDbtCore(line 368) where an explicitreturn trueat line 371 gives the caller a false completion signal, but all three paths have the same underlying problem.Add
awaitbefore eachcommands.executeCommand("dbtPowerUser.installDbt")call (lines 220, 270, 368), and forinstallDbtCore, returnfalseafter the await to indicate the installation is still pending (lines 368-369).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/walkthroughCommands.ts` around lines 363 - 371, The three installer functions installDbtFusion, installDbtCloud, and installDbtCore currently call commands.executeCommand("dbtPowerUser.installDbt") without awaiting, causing a race; update each function to await commands.executeCommand("dbtPowerUser.installDbt") instead of firing it unawaited, and in installDbtCore specifically, after awaiting the retry command return false (not true) so the caller knows installation is still pending; locate the calls inside the installDbtFusion, installDbtCloud, and installDbtCore functions and add the await and adjusted return for installDbtCore.
🤖 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/walkthroughCommands.ts`:
- Around line 145-157: installDbt currently dispatches to
installDbtCore/installDbtFusion/installDbtCloud without passing the selected
workspace context, so environment resolution can pick the wrong interpreter in
multi-root workspaces; change installDbt to read the selected project from
workspace state key "dbtPowerUser.projectSelected", derive the WorkspaceFolder
via workspace.getWorkspaceFolder(pickedProject.uri), and pass that
workspaceFolder into the downstream installers (installDbtCore,
installDbtFusion, installDbtCloud) so they call
pythonEnvironment.getEnvironmentVariables(workspaceFolder) instead of relying on
a default or global workspace.
---
Outside diff comments:
In `@src/commands/walkthroughCommands.ts`:
- Around line 363-371: The three installer functions installDbtFusion,
installDbtCloud, and installDbtCore currently call
commands.executeCommand("dbtPowerUser.installDbt") without awaiting, causing a
race; update each function to await
commands.executeCommand("dbtPowerUser.installDbt") instead of firing it
unawaited, and in installDbtCore specifically, after awaiting the retry command
return false (not true) so the caller knows installation is still pending;
locate the calls inside the installDbtFusion, installDbtCloud, and
installDbtCore functions and add the await and adjusted return for
installDbtCore.
🪄 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: 2d742718-4040-47e3-a639-45ae47f8f35e
📒 Files selected for processing (6)
src/commands/walkthroughCommands.tssrc/webview_provider/onboardingPanel.tswebview_panels/src/modules/onboarding/DbtIntegrationSetup.tsxwebview_panels/src/modules/onboarding/InstallDbtStep.tsxwebview_panels/src/modules/onboarding/Onboarding.tsxwebview_panels/src/modules/onboarding/SetupWizard.tsx
✅ Files skipped from review due to trivial changes (2)
- webview_panels/src/modules/onboarding/Onboarding.tsx
- webview_panels/src/modules/onboarding/DbtIntegrationSetup.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- webview_panels/src/modules/onboarding/InstallDbtStep.tsx
- src/webview_provider/onboardingPanel.ts
- webview_panels/src/modules/onboarding/SetupWizard.tsx
Comment on lines
+145
to
+157
| async installDbt(): Promise<boolean> { | ||
| const dbtIntegration = workspace | ||
| .getConfiguration("dbt") | ||
| .get<string>("dbtIntegration", "core"); | ||
| switch (dbtIntegration) { | ||
| case "core": | ||
| return this.installDbtCore(); | ||
| case "fusion": | ||
| return this.installDbtFusion(); | ||
| await this.installDbtFusion(); | ||
| return true; | ||
| case "cloud": | ||
| return this.installDbtCloud(); | ||
| await this.installDbtCloud(); | ||
| return true; |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Pass selected project/workspace context through installDbt() dispatch.
installDbt() currently routes to install handlers without project/workspace context, so env resolution can still target the wrong interpreter in multi-root workspaces.
Based on learnings, in src/commands/walkthroughCommands.ts, the walkthrough install commands should resolve the active workspace folder from workspace state key "dbtPowerUser.projectSelected" using workspace.getWorkspaceFolder(pickedProject.uri) and pass it to pythonEnvironment.getEnvironmentVariables(workspaceFolder) instead of hardcoding/defaulting workspace selection.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/commands/walkthroughCommands.ts` around lines 145 - 157, installDbt
currently dispatches to installDbtCore/installDbtFusion/installDbtCloud without
passing the selected workspace context, so environment resolution can pick the
wrong interpreter in multi-root workspaces; change installDbt to read the
selected project from workspace state key "dbtPowerUser.projectSelected", derive
the WorkspaceFolder via workspace.getWorkspaceFolder(pickedProject.uri), and
pass that workspaceFolder into the downstream installers (installDbtCore,
installDbtFusion, installDbtCloud) so they call
pythonEnvironment.getEnvironmentVariables(workspaceFolder) instead of relying on
a default or global workspace.
Refresh `pythonEnvironment` cache in `checkAllInstalled()` after dbt detection completes — at that point the Python extension API has settled. Listen to `onDBTInstallationVerification` instead of the raw interpreter change event so the setup page reads fresh `dbtInstalled`, `pythonPath`, and `pythonVersion`. Silent refresh avoids UI flicker. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Comment on lines
223
to
231
| prev.map((check) => ({ ...check, status: "error" as CheckStatus })), | ||
| ); | ||
| } finally { | ||
| setChecking(false); | ||
| if (!silent) { | ||
| setChecking(false); | ||
| } | ||
| } | ||
| }; | ||
|
|
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/dbt_client/index.ts (1)
63-65:⚠️ Potential issue | 🟠 MajorDon't treat a swallowed refresh as fresh interpreter state.
src/dbt_client/pythonEnvironment.ts:70-84catches refresh failures and leaves the previouspythonVersion/isPython3values intact. If that happens after an interpreter switch, this path still completes verification as if the cache was refreshed, so the onboarding panel can render the old interpreter as current. Please haverefreshPythonVersion()clear stale fields or report failure back to this caller.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/dbt_client/index.ts` around lines 63 - 65, The refreshPythonVersion path currently swallows errors and leaves stale pythonVersion/isPython3 values; update pythonEnvironment.refreshPythonVersion so that on failure it clears those fields (set pythonVersion = undefined and isPython3 = undefined) and/or returns/throws a failure indicator, and then update the caller (the await this.pythonEnvironment.refreshPythonVersion() in dbt client) to check the returned boolean or catch the thrown error and treat the refresh as failed (so the onboarding UI won't assume a refreshed interpreter).
🤖 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/webview_provider/onboardingPanel.ts`:
- Around line 324-344: The code trusts this.dbtProjectContainer.dbtInstalled
after calling walkthroughCommands.installDbt(), but installDbt can return true
even when installation fails; update walkthroughCommands.installDbt to return
the actual post-install detection boolean (i.e., run a fresh detection after the
install attempt and return that result) or, alternatively, after the await
this.walkthroughCommands.installDbt() call explicitly trigger a fresh detection
on the project container (e.g., call a detect/refresh method on
dbtProjectContainer) and use that fresh result instead of the cached
this.dbtProjectContainer.dbtInstalled before sending success via
sendResponseToWebview.
---
Duplicate comments:
In `@src/dbt_client/index.ts`:
- Around line 63-65: The refreshPythonVersion path currently swallows errors and
leaves stale pythonVersion/isPython3 values; update
pythonEnvironment.refreshPythonVersion so that on failure it clears those fields
(set pythonVersion = undefined and isPython3 = undefined) and/or returns/throws
a failure indicator, and then update the caller (the await
this.pythonEnvironment.refreshPythonVersion() in dbt client) to check the
returned boolean or catch the thrown error and treat the refresh as failed (so
the onboarding UI won't assume a refreshed interpreter).
🪄 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: eb35a00b-1236-4da9-ac62-44978f4dc5a8
📒 Files selected for processing (4)
src/dbt_client/index.tssrc/dbt_client/pythonEnvironment.tssrc/webview_provider/onboardingPanel.tswebview_panels/src/modules/onboarding/PrerequisitesStep.tsx
✅ Files skipped from review due to trivial changes (1)
- webview_panels/src/modules/onboarding/PrerequisitesStep.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/dbt_client/pythonEnvironment.ts
Comment on lines
+324
to
+344
| // Call the installDbt command (swallows errors internally). | ||
| // Returns false if the user cancelled (e.g. dismissed quick pick). | ||
| const attempted = await this.walkthroughCommands.installDbt(); | ||
|
|
||
| if (!attempted) { | ||
| // User cancelled — don't report as failure | ||
| this.sendResponseToWebview({ | ||
| command: "response", | ||
| syncRequestId, | ||
| data: { success: false, cancelled: true }, | ||
| }); | ||
| break; | ||
| } | ||
|
|
||
| // Verify installation actually succeeded | ||
| if (!this.dbtProjectContainer.dbtInstalled) { | ||
| throw new Error( | ||
| `dbt ${integrationType} could not be detected after installation. ` + | ||
| `Check the Output panel (View → Output → "dbt") for details.`, | ||
| ); | ||
| } |
Contributor
There was a problem hiding this comment.
Don't trust cached dbtInstalled after a truthy install result.
At least the Core path in src/commands/walkthroughCommands.ts:275-372 catches install errors and still returns true, so this branch can end up reading whatever this.dbtProjectContainer.dbtInstalled was before the attempt. If Core was already detected and the reinstall fails, the webview still gets { success: true }. Please have installDbt() return the post-install detection result, or force a fresh detection before deciding success.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/webview_provider/onboardingPanel.ts` around lines 324 - 344, The code
trusts this.dbtProjectContainer.dbtInstalled after calling
walkthroughCommands.installDbt(), but installDbt can return true even when
installation fails; update walkthroughCommands.installDbt to return the actual
post-install detection boolean (i.e., run a fresh detection after the install
attempt and return that result) or, alternatively, after the await
this.walkthroughCommands.installDbt() call explicitly trigger a fresh detection
on the project container (e.g., call a detect/refresh method on
dbtProjectContainer) and use that fresh result instead of the cached
this.dbtProjectContainer.dbtInstalled before sending success via
sendResponseToWebview.
Refresh `pythonEnvironment` cache in `checkAllInstalled()` after dbt detection completes. Onboarding panel listens to `onDBTInstallationVerification` instead of the raw interpreter change event so all state is fresh. Reverts install flow changes (cancellation handling, boolean returns) to keep scope minimal. Fixes `?.version[0]` optional chaining bug. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Move `refreshPythonEnvironment()` (renamed from `refreshPythonVersion`) to run before `detectDBT()` with a brief delay, so `pythonPath` reflects the new interpreter when detection runs. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
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/dbt_client/pythonEnvironment.ts`:
- Around line 88-91: The current assignment unconditionally writes
this._pythonVersion and this.isPython3 from envDetails even when envDetails or
envDetails.version is missing; update the logic in pythonEnvironment.ts (around
the api.environment.getEnvironmentDetails call and the envDetails handling) to
only update this._pythonVersion when envDetails?.version is defined (e.g., set
this._pythonVersion = envDetails.version.join(".") only if envDetails?.version
exists) and only set this.isPython3 when envDetails?.version?.[0] is defined,
otherwise preserve the previous cached values; ensure no assignments write
undefined or false when the API returned an incomplete response.
🪄 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: 06ae1b40-3a3c-4de7-9181-0af4dd29d45e
📒 Files selected for processing (2)
src/dbt_client/index.tssrc/dbt_client/pythonEnvironment.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/dbt_client/index.ts
Comment on lines
+88
to
+91
| const envDetails = | ||
| await api.environment.getEnvironmentDetails(pythonPath); | ||
| this._pythonVersion = envDetails?.version?.join("."); | ||
| this.isPython3 = envDetails?.version?.[0] === "3"; |
Contributor
There was a problem hiding this comment.
Partial API response may overwrite valid cached values.
If envDetails or envDetails.version is undefined/null, lines 90-91 will overwrite _pythonVersion with undefined and isPython3 with false, contradicting the catch block's intent to "keep previous values." This could incorrectly signal Python 2 when the version simply couldn't be determined.
🛡️ Proposed fix to preserve previous values on incomplete response
const envDetails =
await api.environment.getEnvironmentDetails(pythonPath);
- this._pythonVersion = envDetails?.version?.join(".");
- this.isPython3 = envDetails?.version?.[0] === "3";
+ if (envDetails?.version) {
+ this._pythonVersion = envDetails.version.join(".");
+ this.isPython3 = envDetails.version[0] === "3";
+ }
} catch (e) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/dbt_client/pythonEnvironment.ts` around lines 88 - 91, The current
assignment unconditionally writes this._pythonVersion and this.isPython3 from
envDetails even when envDetails or envDetails.version is missing; update the
logic in pythonEnvironment.ts (around the api.environment.getEnvironmentDetails
call and the envDetails handling) to only update this._pythonVersion when
envDetails?.version is defined (e.g., set this._pythonVersion =
envDetails.version.join(".") only if envDetails?.version exists) and only set
this.isPython3 when envDetails?.version?.[0] is defined, otherwise preserve the
previous cached values; ensure no assignments write undefined or false when the
API returned an incomplete response.
…ter change" This reverts commit 188df8c.
ralphstodomingo
added a commit
that referenced
this pull request
Mar 24, 2026
Incorporates master fixes (PR #1841 setup page UX improvements) while preserving branch's Altimate setup step, MCP gating, substeps, and wizard navigation enhancements. Key integrations from master: - Responsive sidebar layout (`isNarrow` / `useSyncExternalStore`) - Clickable sidebar steps and contextual button labels - Image zoom/expand on tutorials page - Silent diagnostics refresh (no loading flicker) - `initialIntegrationType` prop pass-through - URL fixes, typo fix, optional chaining safety - `refreshPythonVersion()` utility method - Improved Python env listener (`onDBTInstallationVerification`) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Setup page UX improvements and bug fixes based on QA feedback.
Setup wizard UX
dbt installation flow
Python environment tracking
Other fixes
Test plan
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Minor