fix(comfyui): pin managed torch stack during install (EAI-8051) - #298
Conversation
ComfyUI dependency install runs `uv pip install` into the machine's managed ROCm runtime. Direct torch/torchvision/torchaudio specs are filtered out, but nothing scopes resolution, so a transitive dependency still resolves torch from PyPI and drags in a CUDA build plus nvidia-* wheels, displacing the runtime's ROCm torch and leaving a runtime with no AMD GPU support. Probe the runtime's installed torch-stack versions via importlib metadata (no native import) and pass a uv --constraint file pinning them to those exact versions. The resolver treats them as satisfied and never swaps in a CUDA build; ROCm torch has no nvidia-* deps, so none appear. A genuine version conflict now fails the install loudly instead of silently corrupting the runtime. Signed-off-by: fredespi <fredrik.espinoza@gmail.com>
r0x0r
left a comment
There was a problem hiding this comment.
Reviewed — the approach is sound. Pinning the runtime's torch stack to its exact installed versions (local +git… identifiers and all) via a uv --constraint file is the right lever: a transitive dep that wants a compatible torch is satisfied by what's already there (no reinstall), and a genuinely incompatible one fails loudly instead of silently swapping in a CUDA build. I checked the TORCH_STACK_VERSION_PROBE string — the Rust \-line-continuations strip the source indentation and the \x20 escapes re-supply it, so the emitted Python has valid, consistent block indentation. The pure helpers are unit-tested, and the end-to-end CUDA-rejection proof is the @requires-gpu @nightly comfyui-install-preserves-the-rocm-runtime scenario in #291 — a gated lane, named per AGENTS §3.
One substantive thread on the scope of the guarantee below.
Address review: make explicit why constraining only the installed torch-stack packages is safe. A managed runtime installs torch/torchvision/torchaudio atomically and refuses unless a mutually compatible set is found, and ComfyUI install only targets such a runtime — so the "torch present, torchvision/torchaudio absent" case in which an unpinned CUDA torchvision could slip in alongside the pinned ROCm torch cannot occur. Comment-only; no behavior change. Signed-off-by: fredespi <fredrik.espinoza@gmail.com>
…note (EAI-8051) Review nit: the resolver is `select_matching_pip_package_versions`, not `select_compatible_stack`. Comment-only. Signed-off-by: fredespi <fredrik.espinoza@gmail.com>
r0x0r
left a comment
There was a problem hiding this comment.
Approving. The transitive-torch defect (EAI-8051) is fixed at the right layer: the managed runtime's exact torch/torchvision/torchaudio versions are pinned via a uv --constraint file, so the resolver treats the ROCm stack as satisfied and can never swap in a CUDA build, while a genuine conflict fails loudly instead of silently corrupting the runtime. The \x20-escaped Python probe reconstructs to valid indentation, and the pure helpers are unit-tested. My one thread -- whether "constrain only what's installed" leaves a partial-stack gap -- is resolved: the follow-up commits document (and I verified) that therock installs the full trio atomically as a mutually-compatible set (select_matching_pip_package_versions bails otherwise) and ComfyUI's select_runtime only ever targets a managed wheel runtime, so the partial-stack case cannot arise. End-to-end CUDA-rejection proof is the @requires-gpu @nightly scenario in #291, named per AGENTS. 24/24 CI green.
The ComfyUI fix landed in 3e59435 (#298), pinning the managed runtime's torch stack through uv constraints. The scenario now asserts behavior main implements; remove its xfail row so a pass is not a fatal stale XPASS. Keep the scenario as the regression guard. Signed-off-by: fredespi <fredrik.espinoza@gmail.com>
Summary
rocm comfyui installinstalls ComfyUI's dependencies into the machine's managed ROCm runtime. It already filterstorch/torchvision/torchaudioout of ComfyUI's direct requirement specs, but nothing scopes package resolution — so a transitive ComfyUI dependency can still resolvetorchfrom PyPI, pull a CUDA build plusnvidia-*wheels, and displace the runtime's ROCm torch. The runtime the whole machine serves models with then becomes a CUDA build with no AMD GPU support: installing an optional app breaks the base runtime (and vLLM serving with it).This pins the managed runtime's existing torch stack to its exact installed versions so the resolver can never swap in a CUDA build.
Changes
uv pip installfor ComfyUI's deps, probe the managed runtime's currently-installedtorch/torchvision/torchaudioversions viaimportlib.metadata(no native import needed).--constraintfile and pass it to the install. The resolver treats the stack as already satisfied and never replaces it; because ROCm torch has nonvidia-*dependencies, none are pulled in.--constraintargument wiring.Test plan
cargo test -p rocm(Linux) — 477 tests pass, including 3 new ComfyUI tests.cargo clippy/cargo fmt --checkclean.@requires-gpu @nightlyscenariocomfyui-install-preserves-the-rocm-runtimefrom test(e2e): pin the driver-as-root and comfyui-runtime contracts (EAI-8071) #291 (EAI-8071), which runs on a GPU host after that contract lands.