Skip to content

Warn about outdated NVIDIA drivers during fresh install#1581

Merged
christian-byrne merged 1 commit intomainfrom
warn-nvidia-driver-early
Feb 4, 2026
Merged

Warn about outdated NVIDIA drivers during fresh install#1581
christian-byrne merged 1 commit intomainfrom
warn-nvidia-driver-early

Conversation

@benceruleanlu
Copy link
Member

@benceruleanlu benceruleanlu commented Feb 3, 2026

Summary

  • show the NVIDIA driver warning immediately after install options are chosen
  • reuse the same warning logic for updatePackages with the selected device

The bug was that this wasn't shown early enough in the install process that new users who do not have the latest NVIDIA driver for pytorch 2.10 is that they would crash during install without a good error code.

Summary by CodeRabbit

  • Refactor
    • Optimized internal device handling in the installation management system for improved code maintainability and clarity.

┆Issue is synchronized with this Notion page by Unito

@benceruleanlu benceruleanlu requested a review from a team as a code owner February 3, 2026 20:40
Copilot AI review requested due to automatic review settings February 3, 2026 20:40
@dosubot dosubot bot added size:XS This PR changes 0-9 lines, ignoring generated files. enhancement New feature or request labels Feb 3, 2026
@coderabbitai
Copy link

coderabbitai bot commented Feb 3, 2026

📝 Walkthrough

Walkthrough

The warnIfNvidiaDriverTooOld method in InstallationManager was refactored to accept a device parameter directly instead of a ComfyInstallation object. Call sites were updated to pass the selected device value, simplifying the method's dependency on the installation object structure.

Changes

Cohort / File(s) Summary
Nvidia Driver Warning Refactor
src/install/installationManager.ts
Updated warnIfNvidiaDriverTooOld method signature to accept device parameter instead of ComfyInstallation object. Call sites in freshInstall and updatePackages modified to pass the device directly. Behavior remains gated by Windows platform and 'nvidia' device value.

Poem

A rabbit hops through code so neat, 🐰
Passing devices, sleek and sweet,
No more objects, just what's true,
Parameters refined and new! ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately reflects the main change: refactoring warnIfNvidiaDriverTooOld to show the warning during fresh install by accepting a device parameter instead of installation object.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch warn-nvidia-driver-early

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves the user experience by warning users about outdated NVIDIA drivers earlier in the installation process, right after they select their installation options. The change also refactors the warning method to accept a device parameter instead of the full installation object, making it reusable in multiple contexts.

Changes:

  • Added early NVIDIA driver warning call immediately after installation options are selected
  • Refactored warnIfNvidiaDriverTooOld to accept device parameter instead of installation object
  • Updated the method call in updatePackages to pass the device from the virtual environment

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Feb 4, 2026
@christian-byrne christian-byrne merged commit e365b39 into main Feb 4, 2026
27 of 28 checks passed
@christian-byrne christian-byrne deleted the warn-nvidia-driver-early branch February 4, 2026 01:24
christian-byrne pushed a commit that referenced this pull request Feb 4, 2026
## Summary
- bump desktop app version to 0.8.1

DO NOT MERGE UNTIL THESE TWO PRS ARE MERGED
1. #1581
2. #1583

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Chores**
  * Patch version update (0.8.1).

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-1584-Bump-desktop-version-to-0-8-1-2fc6d73d365081ae830dd16019dd7479)
by [Unito](https://www.unito.io)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request lgtm This PR has been approved by a maintainer size:XS This PR changes 0-9 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants