-
Notifications
You must be signed in to change notification settings - Fork 202
Handle NVIDIA PyTorch updates #1525
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 7 commits
8f76581
d9c89cf
290d8d4
4dbd3b9
c3290ee
273942b
6f59baf
ecbe8fd
a4b4925
29c7dbd
3c7b54f
6e96626
30388eb
a64ef0e
8393245
477fb95
14c9f7e
2b26142
c38e2b4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -5,7 +5,14 @@ import { promisify } from 'node:util'; | |||||
|
|
||||||
| import { strictIpcMain as ipcMain } from '@/infrastructure/ipcChannels'; | ||||||
|
|
||||||
| import { IPC_CHANNELS, InstallStage, ProgressStatus } from '../constants'; | ||||||
| import { | ||||||
| IPC_CHANNELS, | ||||||
| InstallStage, | ||||||
| NVIDIA_TORCHVISION_VERSION, | ||||||
| NVIDIA_TORCH_RECOMMENDED_VERSION, | ||||||
| NVIDIA_TORCH_VERSION, | ||||||
| ProgressStatus, | ||||||
| } from '../constants'; | ||||||
| import { PythonImportVerificationError } from '../infrastructure/pythonImportVerificationError'; | ||||||
| import { useAppState } from '../main-process/appState'; | ||||||
| import type { AppWindow } from '../main-process/appWindow'; | ||||||
|
|
@@ -91,7 +98,7 @@ export class InstallationManager implements HasTelemetry { | |||||
| // Convert from old format | ||||||
| if (state === 'upgraded') installation.upgradeConfig(); | ||||||
|
|
||||||
| // Install updated manager requirements | ||||||
| // Install updated requirements | ||||||
| if (installation.needsRequirementsUpdate) await this.updatePackages(installation); | ||||||
|
|
||||||
| // Resolve issues and re-run validation | ||||||
|
|
@@ -382,14 +389,120 @@ export class InstallationManager implements HasTelemetry { | |||||
| await installation.virtualEnvironment.installComfyUIRequirements(callbacks); | ||||||
| await installation.virtualEnvironment.installComfyUIManagerRequirements(callbacks); | ||||||
| await this.warnIfNvidiaDriverTooOld(installation); | ||||||
| await installation.virtualEnvironment.ensureRecommendedNvidiaTorch(callbacks); | ||||||
| await this.maybeUpdateNvidiaTorch(installation, callbacks); | ||||||
| await installation.validate(); | ||||||
| } catch (error) { | ||||||
| log.error('Error auto-updating packages:', error); | ||||||
| await this.appWindow.loadPage('server-start'); | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| private async maybeUpdateNvidiaTorch(installation: ComfyInstallation, callbacks: ProcessCallbacks): Promise<void> { | ||||||
| const virtualEnvironment = installation.virtualEnvironment; | ||||||
| if (virtualEnvironment.selectedDevice !== 'nvidia') return; | ||||||
|
|
||||||
| const config = useDesktopConfig(); | ||||||
| const updatePolicy = config.get('torchUpdatePolicy'); | ||||||
| if (updatePolicy === 'pinned') { | ||||||
| log.info('Skipping NVIDIA PyTorch update because updates are pinned.'); | ||||||
| return; | ||||||
| } | ||||||
|
|
||||||
| const installedVersions = await virtualEnvironment.getInstalledTorchPackageVersions(); | ||||||
| if (!installedVersions) { | ||||||
| log.warn('Skipping NVIDIA PyTorch update because installed versions could not be read.'); | ||||||
| return; | ||||||
| } | ||||||
|
|
||||||
| const isOutOfDate = await virtualEnvironment.isNvidiaTorchOutOfDate(installedVersions); | ||||||
| if (!isOutOfDate) return; | ||||||
|
|
||||||
| const recommendedVersion = NVIDIA_TORCH_RECOMMENDED_VERSION; | ||||||
| const lastPromptedVersion = config.get('torchLastPromptedVersion'); | ||||||
|
|
||||||
| if (config.get('torchOutOfDateRecommendedVersion') !== recommendedVersion) { | ||||||
| config.set('torchOutOfDateRecommendedVersion', recommendedVersion); | ||||||
| config.set('torchOutOfDatePackages', installedVersions); | ||||||
| } | ||||||
|
|
||||||
| if (updatePolicy === 'defer' && lastPromptedVersion === recommendedVersion) { | ||||||
| log.info('Skipping NVIDIA PyTorch update because updates are deferred for this version.'); | ||||||
| return; | ||||||
| } | ||||||
|
|
||||||
| const autoPolicy = updatePolicy === undefined || updatePolicy === 'auto'; | ||||||
| const updateApproved = autoPolicy && lastPromptedVersion === recommendedVersion; | ||||||
|
|
||||||
| if (!updateApproved) { | ||||||
| const currentTorch = installedVersions.torch ?? 'unknown'; | ||||||
| const currentTorchaudio = installedVersions.torchaudio ?? 'unknown'; | ||||||
| const currentTorchvision = installedVersions.torchvision ?? 'unknown'; | ||||||
|
|
||||||
| const { response } = await this.appWindow.showMessageBox({ | ||||||
| type: 'question', | ||||||
| title: 'Update PyTorch?', | ||||||
| message: | ||||||
| 'Your NVIDIA PyTorch build is out of date. We can update to the recommended build for performance improvements. This may change memory use and/or compatibility with custom nodes', | ||||||
|
||||||
| 'Your NVIDIA PyTorch build is out of date. We can update to the recommended build for performance improvements. This may change memory use and/or compatibility with custom nodes', | |
| 'Your NVIDIA PyTorch build is out of date. We can update to the recommended build for performance improvements. This may change memory use and/or compatibility with custom nodes.', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
rg -n "isUsingCustomTorchMirror|ensureRecommendedNvidiaTorch|torchMirror" src/ -t tsRepository: Comfy-Org/desktop
Length of output: 2404
🏁 Script executed:
sed -n '694,740p' src/virtualEnvironment.tsRepository: Comfy-Org/desktop
Length of output: 2001
🏁 Script executed:
grep -n "isUsingCustomTorchMirror" src/install/installationManager.tsRepository: Comfy-Org/desktop
Length of output: 43
🏁 Script executed:
grep -rn "isUsingCustomTorchMirror" src/ -t tsRepository: Comfy-Org/desktop
Length of output: 171
🏁 Script executed:
rg -n "isUsingCustomTorchMirror" src/Repository: Comfy-Org/desktop
Length of output: 130
Add early return for custom torch mirrors.
The method isUsingCustomTorchMirror() exists but is unused in the torch update flow. Without a guard at the start of maybeUpdateNvidiaTorch(), users with custom mirrors will still see update prompts and attempts, contradicting the PR objective to skip updates when a custom mirror is configured.
Add the check after the device check:
if (virtualEnvironment.isUsingCustomTorchMirror()) {
log.info('Skipping NVIDIA PyTorch update because a custom torch mirror is configured.');
return;
}
🤖 Prompt for AI Agents
In `@src/install/installationManager.ts` around lines 400 - 504, The
maybeUpdateNvidiaTorch function lacks a guard for custom torch mirrors; after
the existing device check (virtualEnvironment.selectedDevice !== 'nvidia') add a
check using virtualEnvironment.isUsingCustomTorchMirror() and if true log a
message like "Skipping NVIDIA PyTorch update because a custom torch mirror is
configured." and return; update references: maybeUpdateNvidiaTorch,
virtualEnvironment.isUsingCustomTorchMirror(), and keep the rest of the flow
unchanged so prompts and installs are skipped when a custom mirror is set.
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -14,8 +14,11 @@ import { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| LEGACY_NVIDIA_TORCH_MIRROR, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| NVIDIA_TORCHVISION_VERSION, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| NVIDIA_TORCH_PACKAGES, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| NVIDIA_TORCH_RECOMMENDED_VERSION, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| NVIDIA_TORCH_VERSION, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| TorchMirrorUrl, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| TorchPinnedPackages, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| TorchUpdatePolicy, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } from './constants'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { PythonImportVerificationError } from './infrastructure/pythonImportVerificationError'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { useAppState } from './main-process/appState'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -89,13 +92,7 @@ function getPipInstallArgs(config: PipInstallConfig): string[] { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return installArgs; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Returns the default torch mirror for the given device. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @param device The device type | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @returns The default torch mirror | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| function getDefaultTorchMirror(device: TorchDeviceType): string { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| log.debug('Falling back to default torch mirror'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| function getDeviceDefaultTorchMirror(device: TorchDeviceType): string { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| switch (device) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| case 'mps': | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return TorchMirrorUrl.NightlyCpu; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -140,6 +137,9 @@ export class VirtualEnvironment implements HasTelemetry, PythonExecutor { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| readonly pythonMirror?: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| readonly pypiMirror?: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| readonly torchMirror?: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| torchUpdatePolicy?: TorchUpdatePolicy; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| torchPinnedPackages?: TorchPinnedPackages; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| torchUpdateDecisionVersion?: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+143
to
+145
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick | 🔵 Trivial Public mutable properties warrant careful consideration. These properties are public and mutable, meaning external code can modify them directly. While this aligns with the current usage via 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| uvPty: pty.IPty | undefined; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** The environment variables to set for uv. */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -184,13 +184,19 @@ export class VirtualEnvironment implements HasTelemetry, PythonExecutor { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| pythonMirror, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| pypiMirror, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| torchMirror, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| torchUpdatePolicy, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| torchPinnedPackages, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| torchUpdateDecisionVersion, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }: { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| telemetry: ITelemetry; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| selectedDevice?: TorchDeviceType; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| pythonVersion?: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| pythonMirror?: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| pypiMirror?: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| torchMirror?: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| torchUpdatePolicy?: TorchUpdatePolicy; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| torchPinnedPackages?: TorchPinnedPackages; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| torchUpdateDecisionVersion?: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| this.basePath = basePath; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -200,6 +206,9 @@ export class VirtualEnvironment implements HasTelemetry, PythonExecutor { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| this.pythonMirror = pythonMirror; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| this.pypiMirror = pypiMirror; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| this.torchMirror = fixDeviceMirrorMismatch(selectedDevice!, torchMirror); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| this.torchUpdatePolicy = torchUpdatePolicy; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| this.torchPinnedPackages = torchPinnedPackages; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| this.torchUpdateDecisionVersion = torchUpdateDecisionVersion; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // uv defaults to .venv | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| this.venvPath = path.join(basePath, '.venv'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -263,6 +272,34 @@ export class VirtualEnvironment implements HasTelemetry, PythonExecutor { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return primary; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| updateTorchUpdatePolicy( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| policy: TorchUpdatePolicy | undefined, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| pinnedPackages?: TorchPinnedPackages, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| decisionVersion?: string | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| this.torchUpdatePolicy = policy; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (pinnedPackages !== undefined) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| this.torchPinnedPackages = pinnedPackages; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } else if (policy !== 'pinned') { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| this.torchPinnedPackages = undefined; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (decisionVersion !== undefined) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| this.torchUpdateDecisionVersion = decisionVersion; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+287
to
+301
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: # First, let's check the file and locate the class definition and the method in question
head -300 src/virtualEnvironment.ts | tail -100Repository: Comfy-Org/desktop Length of output: 3646 🏁 Script executed: # Also check the beginning of the file to see class export
head -50 src/virtualEnvironment.tsRepository: Comfy-Org/desktop Length of output: 1725 🏁 Script executed: # Check if the class is exported
grep -n "export.*class\|class.*{" src/virtualEnvironment.ts | head -5Repository: Comfy-Org/desktop Length of output: 138 🏁 Script executed: # Search for all public methods in VirtualEnvironment class to check consistency
rg -A 2 "^\s+(async\s+)?[a-zA-Z]+\(" src/virtualEnvironment.ts | head -80Repository: Comfy-Org/desktop Length of output: 2875 🏁 Script executed: # More targeted: find methods and check for return type annotations
ast-grep --pattern 'class VirtualEnvironment {
$$$
$_($_) {
$$$
}
$$$
}'Repository: Comfy-Org/desktop Length of output: 43 🏁 Script executed: # Let's get a cleaner view of the class methods and their signatures
sed -n '124,400p' src/virtualEnvironment.ts | grep -E "^\s+(async\s+)?[a-zA-Z]+.*\(" | head -20Repository: Comfy-Org/desktop Length of output: 1222 Add explicit return type to this public method. The 🔧 Proposed fix updateTorchUpdatePolicy(
policy: TorchUpdatePolicy | undefined,
pinnedPackages?: TorchPinnedPackages,
decisionVersion?: string
- ) {
+ ): void {📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| isUsingCustomTorchMirror(): boolean { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!this.torchMirror) return false; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return this.torchMirror !== getDeviceDefaultTorchMirror(this.selectedDevice); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private shouldSkipNvidiaTorchUpgrade(): boolean { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private shouldSkipNvidiaTorchUpgrade(): boolean { | |
| private shouldSkipNvidiaTorchUpgrade(): boolean { | |
| if (this.isUsingCustomTorchMirror()) return true; |
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for call sites of isUsingCustomTorchMirror and torchUpdatePolicy
echo "=== Looking for isUsingCustomTorchMirror usage ==="
rg -nP --type=ts -C2 'isUsingCustomTorchMirror\s*\('
echo ""
echo "=== Looking for shouldSkipNvidiaTorchUpgrade usage ==="
rg -nP --type=ts -C2 'shouldSkipNvidiaTorchUpgrade\s*\('
echo ""
echo "=== Looking for NVIDIA torch upgrade decision points ==="
rg -nP --type=ts -C3 'torchUpdatePolicy|shouldSkipNvidia' | head -100Repository: Comfy-Org/desktop
Length of output: 9079
🏁 Script executed:
#!/bin/bash
# Get full context of ensureRecommendedNvidiaTorch method
echo "=== ensureRecommendedNvidiaTorch implementation ==="
sed -n '694,750p' src/virtualEnvironment.ts
echo ""
echo "=== needsNvidiaTorchUpgrade implementation ==="
sed -n '1041,1100p' src/virtualEnvironment.ts
echo ""
echo "=== Check if custom mirror is checked anywhere in upgrade flow ==="
rg -nP --type=ts 'isUsingCustomTorchMirror' src/virtualEnvironment.tsRepository: Comfy-Org/desktop
Length of output: 4652
🏁 Script executed:
#!/bin/bash
# Check if tests verify custom mirror blocking NVIDIA upgrades
echo "=== Test coverage for custom mirror and NVIDIA upgrades ==="
sed -n '300,330p' tests/unit/virtualEnvironment.test.ts
echo ""
echo "=== Search for test cases related to shouldSkipNvidiaTorchUpgrade ==="
rg -nP --type=ts 'shouldSkipNvidiaTorchUpgrade|ensureRecommendedNvidiaTorch' tests/
echo ""
echo "=== Check if custom mirror is tested with upgrade logic ==="
rg -nP --type=ts -B5 -A5 'isUsingCustomTorchMirror.*true' tests/ | head -50Repository: Comfy-Org/desktop
Length of output: 5535
🏁 Script executed:
#!/bin/bash
# Check if custom mirror skip is mentioned in any documentation or PR-related files
echo "=== Search for PR objectives or documentation about custom mirror ==="
find . -type f \( -name "*.md" -o -name "*.txt" \) | head -20 | xargs grep -l "custom.*mirror\|torch.*mirror" 2>/dev/null
echo ""
echo "=== Look for any references to the PR or CHANGELOG ==="
git log --oneline --all -S "isUsingCustomTorchMirror" | head -10
echo ""
echo "=== Check if shouldSkipNvidiaTorchUpgrade should check custom mirror ==="
echo "Looking for all places shouldSkipNvidiaTorchUpgrade is called:"
rg -nP --type=ts 'shouldSkipNvidiaTorchUpgrade' src/Repository: Comfy-Org/desktop
Length of output: 595
🏁 Script executed:
#!/bin/bash
# Check the full implementation and see if custom mirror should prevent upgrades
echo "=== Verify the relationship: custom mirror usage context ==="
sed -n '708,720p' src/virtualEnvironment.ts
echo ""
echo "=== Check if installationManager has any custom mirror checks ==="
rg -nP --type=ts 'isUsingCustomTorchMirror|torchMirror' src/install/installationManager.ts | head -20
echo ""
echo "=== Summary: All places where NVIDIA torch updates are gated ==="
rg -nP --type=ts -B2 'ensureRecommendedNvidiaTorch|needsNvidiaTorchUpgrade' src/ --type-list | grep -E '\.(ts|tsx):' | head -30Repository: Comfy-Org/desktop
Length of output: 1268
🏁 Script executed:
#!/bin/bash
# Final verification: confirm custom mirror skip is NOT implemented
echo "=== Direct check: Does shouldSkipNvidiaTorchUpgrade call isUsingCustomTorchMirror? ==="
sed -n '296,301p' src/virtualEnvironment.ts
echo ""
echo "=== Do ensureRecommendedNvidiaTorch or needsNvidiaTorchUpgrade check custom mirror? ==="
grep -n "isUsingCustomTorchMirror" src/virtualEnvironment.ts
echo ""
echo "=== Log message context at upgrade skip point ==="
sed -n '694,700p' src/virtualEnvironment.tsRepository: Comfy-Org/desktop
Length of output: 893
Custom mirror skip is not enforced in NVIDIA upgrade paths.
The isUsingCustomTorchMirror() helper exists but is unused. The shouldSkipNvidiaTorchUpgrade() method only checks policy ('pinned' or 'defer') and does not consult the custom mirror check. Both ensureRecommendedNvidiaTorch() and needsNvidiaTorchUpgrade() rely solely on this incomplete gate, meaning custom mirror configurations do not prevent NVIDIA PyTorch upgrades. Add isUsingCustomTorchMirror() to the skip logic in shouldSkipNvidiaTorchUpgrade().
🤖 Prompt for AI Agents
In `@src/virtualEnvironment.ts` around lines 291 - 301, The skip logic in
shouldSkipNvidiaTorchUpgrade() must also honor custom mirrors: update
shouldSkipNvidiaTorchUpgrade() to return true if isUsingCustomTorchMirror() is
true in addition to the existing checks (torchUpdatePolicy === 'pinned' or the
'defer' + torchUpdateDecisionVersion === NVIDIA_TORCH_RECOMMENDED_VERSION case)
so that ensureRecommendedNvidiaTorch() and needsNvidiaTorchUpgrade() will not
trigger NVIDIA PyTorch upgrades when a custom torchMirror (checked via
isUsingCustomTorchMirror(), which compares this.torchMirror against
getDeviceDefaultTorchMirror(this.selectedDevice)) is configured.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Add brief JSDoc for new torch policy types.
These are exported public types; a short description helps consumers and aligns with the repo’s documentation expectations. As per coding guidelines, please add concise JSDoc.
✍️ Suggested doc additions
📝 Committable suggestion
🤖 Prompt for AI Agents