fix(install): keep a torch that runs GPU kernels, realign one that does not

[tomastola]

Reading the diff. Most of the change lands in apps/rocm/src/main.rs, and the default diff algorithm mis-pairs large blocks in a file that size: it reports 8,155 insertions / 4,037 deletions, while git diff --histogram gives the real 4,272 / 154. Use git diff --histogram when reviewing locally. GitHub also declines to serve a patch for that file for the same reason, so inline comments on it have to be anchored to a hunk in another file with the real location named in the comment text.

The symptom

A fresh rocm install sdk on a gfx94X host produces a managed runtime that cannot serve. The install exits 0 and every health surface reports ready, but the first serve dies with:

RuntimeError: Failed to infer device type, please set the environment variable
VLLM_LOGGING_LEVEL=DEBUG to turn on verbose logging to help debug the issue.

Earlier in the same log, more plainly: Triton is installed but 0 active driver(s) found (expected 1).

What is actually wrong

Two installers write torch into the same environment. install sdk writes TheRock's build; the engine install that follows writes the build from its own index, pinned to an exact version, replacing it:

SDK phase:  + torch==2.11.0+rocm7.13.0     (hip 7.13.99004)
end state:    torch-2.11.0+gitd0c8b1f      (hip 7.2.53211)
              vllm-0.26.0+rocm723

That torch imports cleanly against the installed SDK and then enumerates no devices, so vLLM resolves UnspecifiedPlatform and refuses to start.

Measured by swapping only torch — same environment, SDK, driver and vLLM:

torch	device_count()	serve
2.11.0+gitd0c8b1f	0	fails
2.11.0+rocm7.13.0	8	serves a real completion

vLLM itself is fine. The served response carried system_fingerprint: vllm-0.26.0-d391b761, so VLLM_PINNED_SPEC does not need to change.

Why neither obvious fix is correct

Both directions have been observed failing:

installed by SDK	engine's pin	letting which side win breaks it
2.9.1+rocm7.14.0a…	2.10.0+git8514f05	the SDK's — a torch release the engine does not accept (the case #264 fixed)
2.11.0+rocm7.13.0	2.11.0+gitd0c8b1f	the engine's — a torch that cannot open a device

So "the engine's pin always wins" and "the SDK's build always wins" each re-create the other bug. Pinning a different vLLM release is not an option either: wheels.vllm.ai/rocm/<version>/ publishes only the rocm723 ABI for 0.23.0 through 0.27.0, so working and broken environments alike run a rocm723 build. The variable is the torch each release pins, not the ABI tag.

The rule

Because both directions fail in the field, compatibility is tested rather than assumed, in two stages.

First, a torch that already runs a GPU kernel with this SDK is kept exactly as it is. Not merely one that imports, and not merely one that enumerates a device — one that has launched a kernel. Two builds are allowed to stand this way: the engine's own exact pin, and the SDK's build of the release the engine pins. Both are states this tool produces deliberately, and each is a fixed point, so a rerun over either keeps it and installs nothing. That is what stops --reinstall from oscillating between the two package sources and re-downloading multiple gigabytes to arrive where it started.

Everything else is repaired: take the torch release from the engine's pin, and the build from the SDK. The release is what the engine was built against; the build is what the installed libraries belong to. Applied to the table above, both rows resolve correctly, and #264's guarantee is preserved rather than reverted.

Where the SDK publishes no build of that release, the engine's own is kept rather than failing a multi-gigabyte install, and the device check reports the outcome.

The kernel probe is what makes the first stage safe. Enumeration alone is not enough to justify keeping a torch: a build can report a device and still hold no kernel image for this exact target, failing on the first tensor operation instead of at startup. Device enumeration and kernel launch are therefore reported as distinct outcomes, and only a launched kernel earns retention — every other verdict, including no answer at all, goes to the repair path.

Why the build comes from the manifest, not the environment

This is worth calling out because the obvious implementation is wrong.

Reading "the SDK's torch" from the environment just before the engine install seems natural, and it works — exactly once. On any later install the environment already holds the engine's build, so that reading mistakes it for the SDK's, concludes the runtime is already correct, and leaves it broken permanently. Repeat installs are the common case, not the edge case: a refresh, a re-run, or a user reaching for engines install --reinstall because something already looks wrong.

The SDK's torch is therefore recorded in the runtime manifest at install time and read back from there. Manifests written before this change fall back to deriving the build from the recorded SDK version, including dated nightlies, where the date is part of the build identifier and not decoration on it. Both paths are covered by tests, and the fallback was exercised end to end against a runtime that was already stuck on the engine's build — it recovered.

Opting out

ROCM_CLI_DISABLE_TORCH_ALIGNMENT keeps whatever torch the user installed. It is for the case where someone is deliberately running a hand-built torch and needs this tool to leave it alone.

This is documented in docs/vllm.md alongside the alignment rule itself, so the escape hatch is discoverable without reading the source or tripping over the log line.

The variable is read only once a replacement is actually due, so it reports the install it stopped rather than standing in for a runtime that needed nothing. The opt-out is then honoured consistently by the dependency check as well: a torch the user chose is not a violation, and it is not offered a reinstall — which would install the engine's build over exactly the torch the opt-out exists to keep. Only torch is spared; every other unmet requirement still reports as a violation.

One unrelated violation must not undo the alignment

The dependency check reports uv pip check output, and a deliberate torch divergence has to be excused there or every install ends on a violation. The excusing was applied to the run rather than to each line, so a single unrelated violation — numpy, say — demoted the whole result and the block then advised rocm engines install <engine> --reinstall. That remedy repairs numpy and reinstates the engine's own build of torch, leaving a runtime that cannot open a device: a worse defect, adopted on the strength of one that had nothing to do with torch.

Each line is now judged on its own subject, and a mixed result carries both sets — violations under the existing key, divergences under divergence: — and says to repair the violations without reinstalling. With no divergence in play the advice is unchanged. The subject is parsed by shared code rather than a substring test, so the CLI and the engine cannot disagree about what a line is about, and a line whose shape does not parse stays a violation rather than being assumed intentional.

Why a device check, and why it looks the way it does

A satisfied dependency check does not mean a usable environment. In the broken state the engine's requirements are fully satisfied — its own pinned torch is installed — and the runtime still cannot serve. That is why this went unnoticed until first serve.

The check reports four outcomes rather than a boolean: usable, no devices, a kernel failure after successful enumeration, and not verified. The last is distinct on purpose — a host with no GPU at all must not be reported as a broken runtime, and a probe that could not run must not be read as a healthy one.

The block reports the runtime after the torch is settled. On a realignment that alone cannot say whether the alignment broke something that worked or repaired something already broken, and those want opposite responses. The realigned outcome therefore also carries the verdict the probe returned beforehand, in the printed block and in the audit line both — the log especially, because it is read long after the torch that produced the earlier verdict has been replaced. Only the realigned outcome carries it; every other one left the runtime alone, so a before/after pair there would invite reading a change into an outcome that made none.

Verification

On hardware

Cold install sdk on a clean runtime	torch realigned, device_check: usable (8 device(s), torch 2.11.0+rocm7.13.0), serves a real completion
engines install --reinstall afterwards	re-settles torch instead of re-breaking the runtime
A runtime already stuck on the engine's build	recovers, exercising the pre-change-manifest fallback
SDK index unreachable during rocm engines install	fails with the intended error instead of exiting 0
The nightly e2e scenarios	pass

The third row matters because it is the state real machines are already in — the fix has to repair them, not just avoid creating new ones.

Unit tests

The suite covers the decision surface rather than one happy path:

• Both field failures from the table — the engine's build replaced by the SDK's build of the same release, and the SDK's release corrected to the one the engine pins. Getting either backwards re-creates the other bug, so both directions are pinned.
• Retention as a fixed point — that each of the two blessed builds is kept when a kernel has run, and that a rerun over either installs nothing.
• Only a launched kernel earns retention — no devices, a kernel failure, and no answer at all each route to the repair path.
• The cases where nothing should happen — an already-correct runtime, a loose (non-pinned) requirement, an unidentified SDK build.
• The pre-change manifest fallback, including a dated nightly, where trimming the date would name a build belonging to a different SDK.
• Failure reporting — that a missing wheel and a DNS or permission failure do not get described the same way.
• The divergence/violation split, in both directions: expected on the realigned or retained package, still a violation on any other.
• The opt-out — that it is read only when a replacement was due, that the dependency check spares torch and nothing else, and that no reinstall is advised over a kept torch.
• The device check's outcomes, including "not verified" being distinct from both healthy and broken.
• The exit code, through the command's real completion path rather than the predicate behind it.

That last one is worth explaining. install sdk takes its engine auto-install as a parameter, so a test can drive both outcomes and assert what the command actually returns, without a multi-gigabyte install behind it. One test pins that an unusable runtime fails it (and that the SDK's own audit record is still written first), one that an ordinary engine-install failure does not, and one that the distinction survives being wrapped in context on the way up — flattening it into a plain message is how this would quietly regress.

To confirm that test is load-bearing rather than merely green, the catch was reverted to warning-and-continue and the test was observed to fail. A regression test that cannot fail is decoration.

End-to-end

Two nightly GPU scenarios, both functional rather than string matches:

• The scenario covering a second SDK install asserted the wording of the dependency check, which a settled runtime no longer produces. It now asserts the alignment reached a settled outcome and that the runtime can still open a GPU — the outcome it was always protecting — while still failing on any genuine unmet requirement.
• A new scenario covers the opt-out: that the CLI reports it instead of rewriting torch, that it does not then offer to reinstall the engine over the kept torch, and that device health is still reported.

The pre-warm also repairs an active-runtime pointer that names a runtime no longer on disk. A tree in that state died nine seconds in, before any test ran, and because the cache is shared across runs one interrupted or hand-edited tree kept failing every subsequent job on that runner until somebody fixed it by hand. Both pointers are rewritten — the active.json marker and config.json's active_runtime_key — since different callers read different ones and repairing only the marker leaves the engine install still following the dead key. A pointer that resolves is left alone, and a tree with no runtimes at all still fails: there is nothing to repoint it at, and inventing one would hide an install that produced nothing.

The pre-warm used by these lanes installs the engine on a reused runtime as well as a freshly installed one. Without that, a warm shared tree could serve every GPU scenario a runtime with no engine — and, because the engine install is where the alignment settles, a warm tree would never exercise this change at all.

Known limitation

Only torch is realigned. torchvision, torchaudio and triton are left as the engine's install leaves them. That combination served correctly in testing, but the SDK resolves those four as a coordinated set, so the mixed stack has not been validated against the full matrix of supported combinations. Worth a follow-up if the engine's pins and the SDK's ever diverge further than a build identifier.

---

• If this PR fixes a bug, searched tests/e2e-cucumber/expectations.toml for the fixed ticket ID and removed/narrowed any now-stale xfail rows. (No xfail row covered this; the affected scenario is a passing GPU scenario that this PR strengthens.)

[volen-silo]

Reviewed at bc0477d. The diagnosis here is the best writing I've seen on this problem in the repo — separating "which torch release" from "which build of that release", instead of picking a winner between the two installers, is the insight that was missing from every previous attempt. "Read the manifest, not the environment" is the right principle and the reasoning for it is correct.

Requesting changes on one thing: the change applies that principle on the CLI side but not on the engine side, and the two mechanisms now work against each other. Two smaller defects below that.

1. This leaves two merged mechanisms fighting, and the loop does not converge

3426a858 (#264) changed vLLM's install short-circuit from "vLLM resolves" to "vLLM's own Requires-Dist are met". This PR deliberately and permanently leaves one of those requirements unmet. Nothing reconciles the two.

The chain:

1. crates/rocm-core/src/uv.rs:288-296 — violations_requiring filters by violation.requiring, i.e. the requirer (vllm), not the required package. The intended torch divergence therefore lands squarely inside vLLM's own violation set.
2. engines/vllm/src/lib.rs:541-566 — repair_from_violations returns needed: true for it.
3. engines/vllm/src/lib.rs:422-461 — needed: true skips the short-circuit and runs a full install_vllm_with_uv, which rewrites torch back to the engine's build. That rewrite isn't incidental; it's the mechanism #264 relies on, per the comment at lib.rs:417-421.
4. apps/rocm/src/main.rs:7971-7998 — settle_engine_install then realigns torch back to the SDK's build.
5. The steady state is once again "vLLM's torch pin violated", so the next invocation repeats step 1.

The clearest evidence is in the vLLM crate's own suite. engines/vllm/src/lib.rs:2585-2604, the_whole_replaced_torch_stack_is_reported_one_finding_per_line, encodes this PR's exact steady state — requires torch==2.11.0+gitd0c8b1f, installed 2.11.0+rocm7.13.0 — and asserts assessment.needed. a_replaced_pinned_torch_forces_a_reinstall (lib.rs:2565) does the same. Both are merged and currently passing, and both pin the behaviour this PR needs not to happen. The diff doesn't touch engines/vllm/src/lib.rs.

Reachability, which is narrower than it first looks:

Path	Churns?
rocm engines install vllm (no --reinstall), main.rs:3684	every time
rocm install sdk auto-installing vLLM, main.rs:7297	every time
rocm engines shell first-time setup, main.rs:4223	first time only
rocm serve	no

I specifically tried to make this worse and couldn't: serve doesn't call EngineMethod::Install per invocation — it goes resolve_engine_selection → validate_engine_selection_runtime → EngineMethod::ResolveModel, and resolve_engine_env has exactly one caller (engine_shell, main.rs:4079) gated behind interactive_terminal(). So there's no per-serve churn.

Cost is two full torch-stack flips per affected invocation instead of one. uv's wheel cache blunts that to extract-and-link churn after the first round trip, but on cache-cold hosts — CI, ephemeral containers, pruned disks — it's a repeated multi-gigabyte download. And by both this PR's framing and #264's, rocm install sdk re-runs are the common case rather than the edge case.

It's also user-visible in a way that undercuts the change: every affected run prints warning: the runtime environment did not satisfy vLLM's pinned dependencies; vLLM was reinstalled to restore them (engines/vllm/src/lib.rs:546-547). Under this PR that sentence is false — the divergence is intended, which is the entire reason ExpectedDivergence exists. The CLI learns to say "this divergence is expected" while the engine underneath keeps saying "this divergence is a defect I just repaired".

Suggested direction, staying off the rocm-engine-protocol contract surface: teach assess_runtime_repair / repair_from_violations the same distinction classify_dependency_details already makes on the CLI side — a torch-only violation whose installed version matches the runtime manifest's recorded sdk_torch build is not a defect and must not set needed = true; anything else still is. The engine already reads TheRockRuntimeManifest in collect_managed_runtime_candidates (lib.rs:1136), so it can read sdk_torch directly without a protocol change. That applies this PR's own "manifest, not environment" principle to the one place it didn't reach. The two tests above would need to move with it.

Verification boundary, stated plainly: I did not run uv. Steps 1, 2, 4 and 5 are confirmed by reading source and by the two tests named above. Step 3 — that uv pip install vllm==X without --reinstall actually rewrites an out-of-spec transitive torch — rests on #264's stated design intent and its code comments, which would make that commit a no-op if it weren't true. If that inference is wrong, this finding collapses, and you're better placed than I am to say so.

2. Lemonade is run through the torch and device probes with a non-Python executable

The description says "Environments rocm-cli does not own are left untouched." The guard implementing that doesn't cover Lemonade.

• apps/rocm/src/main.rs:7977 — the only guard is if response.managed_env == Some(false) { return Ok(()); }
• engines/lemonade/src/lib.rs:416, :456 — Lemonade reports managed_env: Some(true), so it isn't skipped
• engines/lemonade/src/lib.rs:453 — Lemonade's python_executable is manifest.lemonade, the Lemonade CLI binary. There's no Python interpreter and no torch anywhere in a Lemonade runtime
• main.rs:7980 takes that as python and hands it to probe_torch_alignment and probe_runtime_devices, both of which do Command::new(python).arg(<probe.py>)

Reached from rocm engines install lemonade, and — because ensure_self_managed_engine_ready is called from serve() at main.rs:4884 — from rocm serve on any fresh Lemonade install or version bump (main.rs:3905). The result is two pointless spawns of the Lemonade binary with a .py argument, and user-facing nonsense on the serve path: torch_alignment: not_applicable (failed to launch torch alignment probe via …) and device_check: not_verified (…), plus audit entries claiming a torch check was attempted for an engine that has no torch.

Not a false failure — NotApplicable and NotVerified sit outside the fatal conjunction — but it's wrong output on a hot command and it contradicts the invariant the PR states. Gating on !engine_manages_own_runtime(engine) (main.rs:3757) would cover it.

3. The doc comment carrying the change's central invariant is on the wrong function

apps/rocm/src/main.rs:7858-7871. Two doc blocks are merged with no separator, so the text describing settle_engine_install's contract —

Every path that installs an engine into a managed runtime must call this. Skipping it anywhere lets the engine's own torch win silently… External environments are left alone: rocm-cli does not own them.

— is followed immediately by /// Whether an install finished having produced a runtime that cannot serve. and lands on const fn install_left_runtime_unusable (main.rs:7871). settle_engine_install (main.rs:7971) has no doc comment at all.

Ordinarily a nit, but this is the most important invariant the change introduces, and finding 2 is precisely the failure it warns about — which suggests the misplacement isn't harmless.

Worth answering, not necessarily fixing here

No escape hatch. If the SDK ever publishes a build of the engine's pinned release that's ABI-incompatible with the engine, alignment reports Realigned, the device check passes (torch opens devices fine), and the install exits 0 with an engine that may not work — every check is about torch, not about the engine. I grepped for an opt-out and there's no flag, env var, or config key. The description says anyone wanting a different torch can install it directly, but alignment runs on every engine-install path, so a hand-installed torch is overwritten on the next engines install, install sdk, or engines shell. Given the PR's own known limitation about the mixed stack not being validated against the supported matrix, an env-var opt-out seems cheap insurance.

A third torch policy now exists. vLLM pins to its own index; this PR takes release-from-engine and build-from-manifest, explicitly because reading the environment is a trap; ComfyUI's probe_torch_stack_versions reads the live environment and constrains to whatever it finds. Install vLLM then ComfyUI and ComfyUI pins to the engine's build — the one this PR documents as enumerating zero devices. Not this PR's job, but its own reasoning implies the bug, and comfyui.rs here only gains sdk_torch: None in three fixtures.

rocm update --apply bypasses all of this — it calls therock::install_sdk directly (main.rs:14978-15069) and never reaches finish_sdk_install or settle_engine_install. Pre-existing and not introduced here, but it's a hole in "every path that installs an engine" and looks like a follow-up.

Smaller items

• sdk_torch_build_for_key (main.rs:7842) has no test coverage — grep returns the definition and its single call site at :7981, nothing else. The description says "Both paths are covered by tests" of the manifest-fallback story; the hardware row is real but that half isn't. This is the function the "repair already-broken machines" case rests on, and its fallback is a string reconstruction. (I did chase the fallback's correctness and it holds: package_rocm_suffix, therock.rs:1472-1478, rejects any torch version without a literal +rocm segment, so a recorded sdk_torch always carries a usable local segment. Sound, just untested.)
• main.rs:26351 and main.rs:26621 are byte-identical tests — same arguments to plan_torch_alignment, same assertion, only the doc comments differ. The second's stated point (build comes from the manifest, not the environment) is sdk_torch_build_for_key's job, and plan_torch_alignment takes sdk_build as an opaque parameter, so nothing in it demonstrates that. It's counted as a distinct decision-surface row but isn't one — and the facet it claims to cover is the untested one above.
• report_torch_alignment's doc (main.rs:7856-7860) describes a different function — it's a near-verbatim copy of deliberately_diverged_package's doc (main.rs:7676), where it's correct.
• The device probe discards its real error text. main.rs:7742-7744: when torch.cuda.device_count() raises — a genuine ROCm failure mode, HIP init errors — the probe returns the real exception in error, and the None arm replaces it with the generic "torch imported but did not report a device count". That's exactly the diagnostic the probe exists to capture; probe.error.unwrap_or_else(…) keeps it.
• NotVerified is never fatal despite its doc saying "never assume healthy" (main.rs:7706 vs :7871-7882, which fires only on NoDevices). A runtime where torch can't import at all exits 0. The conjunction is defensible — you can't fail a multi-gigabyte install because a probe couldn't run — but the enum doc promises more than the predicate delivers.
• capture_python_stdout_with_env (therock.rs:3062+) uses Stdio::piped() on every platform, while its sibling capture_python_stdout (:3101+) branches on runtime_is_windows() and uses a runpy file-redirect workaround elaborate enough to have been written against a real problem. The new helper knows about the split (it uses it to pick the temp dir, :3072-3076) and then ignores it for capture. If that workaround is load-bearing, both new probes degrade to NotApplicable/NotVerified on Windows and the fix silently no-ops there — and per the previous point, nothing fails. I have no Windows host, so this is a question rather than a confirmed break.
• install_pinned_package (therock.rs:2825-2850) uses --extra-index-url, leaving PyPI in the candidate set, and has no --no-deps or constraint file, so transitive deps can drift during what's meant to be a surgical "put this exact build back". 3e59435a solved the same class of problem in comfyui.rs with a --constraint file — worth matching or noting why not.
• Realigned and AlreadyAligned — the two states a healthy run actually produces — are unasserted anywhere. render_torch_alignment is tested on Unavailable, InstallFailed and NotApplicable only, and grepping tests/e2e-cucumber/ for torch_alignment or expected_divergence returns nothing.
• torchvision/torchaudio divergences are now invisible rather than merely unfixed: because violations_requiring filters by requirer, a torchvision → torch complaint carries requiring: "torchvision" and is dropped entirely, so a mixed stack yields neither a violation nor a divergence. Pre-existing filter behaviour, but the known limitation makes the mixed stack the expected steady state, which changes what that silence costs.

Verification

Ran on this branch: cargo fmt --check clean; cargo clippy --locked --workspace --all-targets -- -D warnings clean, zero warnings; cargo test -p rocm --all-targets → 500 passed, 0 failed, 1 ignored, matching the description exactly. All 18 new tests run individually and pass; the count is exact and none are vacuous. One workspace failure in rocm-dash-daemon::gen_tps_expiry_boundary_held_then_unavailable traced to a timing flake in a crate this PR doesn't touch — passes in isolation. The anyhow claim about is::<T>() surviving .context() was confirmed in a standalone repro; it holds.

The expectations.toml checklist claim holds — grepped the full file for the scenario id, torch, device_check and dependency_check; zero hits, nothing stale. The e2e change itself is correct: device_check: usable matches the renderer byte-for-byte including the two leading spaces, and the old step and scenario title are fully removed repo-wide.

Liked

The Unavailable vs InstallFailed split — refusing to claim "the index has no such build" unless the resolver actually said so, and degrading to the honest answer otherwise — is the kind of restraint that usually gets skipped. render_engine_dependency_check's ExpectedDivergence arm deliberately withholding the reinstall remedy (main.rs:8074-8080), because that remedy is what produces the broken state, is a genuinely thoughtful piece of UX. Using device_count() rather than is_available(), and probing with the runtime's recorded library paths rather than initialize_process(), are both non-obvious and both backed by measurement. And moving the e2e assertion from a string match on a diagnostic to "can the runtime open a GPU" is strictly stronger than what it replaces.

[rominf]

I read through this one carefully. The core idea is right and the reasoning in the doc comments is genuinely good — the alignment/device-check split, the unsettled && NoDevices conjunction, and the decision to keep a successful exit for an ordinary engine-install failure but not for a runtime left unable to open a device are all well argued, and the 18 new unit tests cover the interesting branches. My concerns are about how the new code is wired in, not about the design.

Two things I'd want fixed before merge:

1. The new torch-realignment install runs uv without the environment every other uv call in the tree applies. That loses both the long HTTP timeout and the managed cache directory, and the cache one has a real cost that crates/rocm-core/src/uv.rs explicitly warns about.
2. settle_engine_install is now on the lemonade path, where the "python executable" is actually a native binary. The repo already has engine_manages_own_runtime() for exactly this distinction and it isn't used here.

The rest are smaller — a doc comment that looks like it got merged with its neighbour during an edit, an index flag that disagrees with how the SDK's own installs are done, and some output ordering. Details inline.

A few things I chased and decided were fine, for what it's worth: the install_left_runtime_unusable conjunction can't be tripped by NotApplicable, so a probe that simply can't answer won't fail an install; engine_auto_install_failure_is_fatal correctly survives .context() wrapping because anyhow's is::<T>() walks the chain; and classify_dependency_details does keep saying "violated" when there's an unrelated violation alongside the deliberate torch divergence.

Unrelated to the code: the Read the Docs check is red on this head (build 4363268 — the link is on the checks tab). This PR touches no docs, so it's probably not yours, but a red X will stall the merge — worth a re-run or a look.

I reviewed by reading the diff and the surrounding code at the PR head; I didn't run the GPU e2e scenario, so the behavioural claims below are from reading the call paths, not from execution.

[rominf]

This is a solid, well-explained fix for the real bug (torch build vs. release getting overwritten on a shared runtime) — the writeup, the manifest field, and the alignment/device-check logic all hold up under a close read, and I traced it against the recently-merged install-root fix (#317) with no conflict there.

One thing worth fixing before merge: settle_engine_install now runs unconditionally for every engine on every install path, including lemonade, which manages its own runtime and was never part of the shared-Python-venv problem this PR fixes. Every sibling helper that branches on engine type (resolve_engine_install_runtime_id, env_root_for_runtime, env_root_for_engine_install, ensure_self_managed_engine_ready itself) checks engine_manages_own_runtime(engine) first and skips lemonade — settle_engine_install doesn't, and its only early-return guard is response.managed_env == Some(false), which lemonade's adapter never sets (it always reports managed_env: Some(true)).

Concretely: for a lemonade install, response.python_executable is the lemonade CLI binary, not a Python interpreter. settle_engine_install still passes it to probe_torch_alignment/probe_runtime_devices, which launch it as <binary> <probe-script>.py. That will not behave like a real Python probe and should surface as a probe failure. In practice it degrades gracefully to torch_alignment: not_applicable, dependency_check: not_verified, and device_check: not_verified rather than a hard error — I confirmed install_left_runtime_unusable can't fire off a NotApplicable alignment, so this doesn't break the exit code — but every rocm engines install lemonade (both the direct CLI command and the auto-triggered install before serving) now prints three lines of noise that don't apply to it and don't mean anything for that engine. Since lemonade never registers a runtime manifest under runtime_manifest_for_selector, sdk_torch_build_for_key also always misses for it, which is a second symptom of the same root cause.

I don't see a test (unit or e2e) that currently exercises this path for lemonade, so nothing in CI should catch it. The fix looks like adding if engine_manages_own_runtime(engine) { return Ok(()); } at the top of settle_engine_install, mirroring the guard already used everywhere else this distinction matters.

Everything else — the manifest field, the alignment/device probes, the divergence-vs-violation classification, the fatal-vs-warning split for install sdk, and the updated e2e scenario — looks correct and well-tested for the vLLM path this was written for.

[rominf]

Re-reviewed at 700f185.

Prior blocking finding (lemonade probed as if it were a Python interpreter): fixed.

settle_engine_install() now delegates its skip decision to a new settles_runtime_torch(engine, managed_env):

fn settles_runtime_torch(engine: &str, managed_env: Option<bool>) -> bool {
    managed_env != Some(false) && !engine_manages_own_runtime(engine)
}

This adds the missing engine_manages_own_runtime(engine) guard alongside the existing managed_env == Some(false) check, matching the pattern already used by resolve_engine_install_runtime_id and env_root_for_runtime. Verified against the current lemonade adapter (engines/lemonade/src/lib.rs): it still reports managed_env: Some(true) and still hands back the native lemonade binary as python_executable, so without this guard it would still be probed. All 4 call sites are covered: the three that call settle_engine_install now benefit from the guard inside it, and the fourth (ensure_self_managed_engine_ready) never called it in the first place (it only runs for self-managed engines by construction) and now carries a comment explaining why. There's a direct unit test for the lemonade case (an_engine_that_manages_its_own_runtime_is_left_alone) and for the general managed/external cases.

I also checked the other individual review comments left on 700f185 (the --extra-index-url vs --index-url PyPI-fallback issue, the missing uv_command_env on the realignment install, the doc-block merge, the sanitize_log_value no-op on a literal, and the settle_engine_install ordering relative to config bookkeeping) — all of these read as genuinely fixed in the current diff, with the fix matching what was asked.

New pass over the rest of the diff (bug-scan): nothing new at >=70 confidence. The alignment/plan/device-check state machine (plan_torch_alignment, align_runtime_torch, install_left_runtime_unusable, is_intended_torch_divergence, violation_subject) is consistent between apps/rocm/src/main.rs, apps/rocm/src/therock.rs, crates/rocm-core/src/uv.rs and engines/vllm/src/lib.rs, the manifest fallback for sdk_torch is mirrored correctly on both the CLI and vLLM sides, and the new/changed unit tests exercise the branches described in the PR body.

Looks safe to approve after final CI/hardware validation.

[tomastola]

Thanks both — this was a genuinely useful pair of reviews. Everything raised is addressed in 700f185; the six inline threads have individual replies. This covers the findings that lived in the review bodies, plus one thing that happened along the way.

The two mechanisms fighting (@volen-silo finding 1)

Fixed, and the chain was real. I verified each step rather than taking it on trust:

• violations_requiring filters by requirer, so the intended torch divergence lands in vLLM's own set (uv.rs:283-295)
• repair_from_violations returns needed: true for it, needed: true skips the short-circuit, and install_vllm_with_uv rewrites torch back
• settle_engine_install then realigns it, and the next invocation repeats

On your step 3, the one you flagged as resting on inference: it is stated as design intent in-tree, at engines/vllm/src/lib.rs:417-421 — "short-circuit on 'vllm's own requirements are met' instead, so the install below restores them." The finding does not collapse.

One correction to the framing, because it changes the severity rather than the conclusion: each individual invocation does end in the right state, since settling runs last. What oscillates is the steady state across invocations. So the cost is the churn, the cache-cold re-download, and a warning that says the opposite of what the CLI says — not a runtime left broken.

The fix is the direction you suggested, with one addition. repair_from_violations now takes the SDK torch build from the runtime manifest and treats a violation as intended only when all three hold: it is about torch, the installed release is the one the engine pins, and the installed build is the SDK's. The release condition is load-bearing — matching on the build alone would have accepted the #264 case, where the SDK installed a torch release the engine cannot use, and that one must still reinstall. So the engine now applies the same "release from the pin, build from the SDK" rule as the CLI, from the same source of truth.

The two tests you named moved with it. a_replaced_pinned_torch_forces_a_reinstall became a_torch_of_the_wrong_release_still_forces_a_reinstall — same scenario, now pinning why it still repairs. the_whole_replaced_torch_stack_is_reported_one_finding_per_line keeps its point and now asserts two violations plus one expected divergence. New rows cover the steady state, a torch belonging to neither side, and an unidentifiable SDK build falling back to the old behaviour.

I checked both regression tests are load-bearing by reverting the predicate to false and confirming they fail. They do.

The parsing needed by both sides now lives once in rocm-core (violation_subject, split_local_version), rather than a third copy of "what does this uv line mean".

Smaller items

All fixed: the device probe keeps its real exception; sdk_torch_build_for_key is split so the manifest decision is testable and now covers the recorded value, the reconstruction older manifests need, and the no-version case; the byte-identical test at :26621 is replaced by three that exercise the facet it claimed; Realigned and AlreadyAligned have renderer assertions; report_torch_alignment's doc no longer describes deliberately_diverged_package.

On install_pinned_package — both of your points landed. --index-url (not --extra-index-url) so a build the SDK index does not publish cannot quietly resolve against PyPI, which was undermining the Unavailable check; and --no-deps, since this is a same-release build swap into an already-resolved environment and re-resolving is free to move torchvision/torchaudio. I checked the --index-url risk before making the change: install_therock_runtime already installs torch from this same index with --index-url and no --no-deps, so that is an exercised path.

On the Windows capture question — I could not settle it either, no Windows host. Left as-is and worth a separate look; run_command_with_env at least pipes stderr on every platform now, which removes the classifier's Windows blind spot.

On torchvision/torchaudio being invisible — still true, and your point about the known limitation making the mixed stack the expected steady state is the sharper version of it. Not changed here.

Deferred, tracked

The four "worth answering, not necessarily fixing" items are deliberately not in this change: the missing escape hatch, ComfyUI's third torch policy reading the live environment, rocm update --apply bypassing finish_sdk_install, and NotVerified never being fatal despite its doc. Three are pre-existing and the fourth is a design question worth its own discussion.

Housekeeping

The branch had gone CONFLICTING against main (#317 touched the same region of engines/vllm/src/lib.rs), which is why no checks appeared on 700f185 at first — GitHub cannot build the merge ref, so it creates no pull_request runs. I merged main in rather than rebasing, specifically to avoid rewriting the commit you both reviewed; the repo squash-merges, so the merge commit will not survive landing. describe_skipped_managed_runtimes from #317 is kept exactly as it landed.

The torch_alignment: realigned line now names the engine, so the sample output in the description reads ... the release vllm pins rather than ... the release the engine pins.

Verification boundary, stated plainly

cargo fmt --check clean, cargo clippy --locked --workspace --all-targets -- -D warnings clean, and cargo test --workspace --all-targets green post-merge — 525 in the CLI crate, 56 in the vLLM engine, 323 in rocm-core, zero failures. @volen-silo, the rocm-dash-daemon timing flake you hit did not reproduce here.

None of this is verified on GPU hardware. The hardware evidence in the description predates these changes, and two of them alter what a real install does: the --index-url/--no-deps swap changes the resolution, and the engine change alters when a reinstall happens at all. Both are exactly the kind of thing a unit test cannot confirm. A hardware re-run of the cold install, the --reinstall, and the already-stuck-runtime scenarios is warranted before this merges, and I would not claim the earlier results carry over.

[rominf]

Re-reviewed at 6ca96d0 (moved from 700f185).

What changed since the last review: nothing authored. This push is a merge commit (Merge remote-tracking branch 'origin/main' into review-fixes/torch-alignment) bringing in 13 unrelated commits from main (#324, #323, #311, #302, #295, #304, #306, #307, #317, #316, #261, #235, #249), with one conflict in engines/vllm/src/lib.rs resolved between this PR's torch-alignment work and main's stale_interpreter_hint/describe_skipped_managed_runtimes additions.

Re-verified: settles_runtime_torch() (the guard from the last review) is byte-identical before and after this push — diffed both the function body and its call site in settle_engine_install(), confirmed unchanged. Its four unit tests (a_managed_python_runtime_is_settled, an_external_runtime_is_left_alone, an_engine_that_manages_its_own_runtime_is_left_alone, an_already_correct_runtime_is_left_alone) are present and unchanged, including the lemonade exclusion. No regression.

Merge conflict resolution: checked line-by-line — both feature sets from the conflicting sides landed correctly with nothing dropped or duplicated. engines/vllm/src/lib.rs test count went from 50 (main) + this PR's new tests to 56, consistent with a clean union. No leftover conflict markers.

CI: the self-hosted GPU E2E lane fails on this head, but the same 4 scenarios (bench-load-real-serve, chat-end-to-end-local-model, serve-vllm-inference, serve-vllm-default-on-instinct) fail with the identical no active ROCm runtime is configured error on main's own latest self-hosted run (33126563205) — pre-existing infra flakiness unrelated to this PR, not a regression it introduced.

No new findings. Still looks safe to approve pending final CI/hardware validation.

[tomastola]

CI status on 6ca96d0

Everything green except E2E tests (GPU), which I do not believe is this PR — but the evidence is circumstantial rather than conclusive, so here it is in full.

The failure is the harness one, seven times over:

Error: device_policy: gpu_required; no active ROCm runtime is configured;
run `rocm runtimes list` and `rocm runtimes activate <runtime_key>`
no service records: /tmp/rocm-e2e-*/data/services (No such file or directory)

That fires during pre-warm/activation, before any code path this PR touches. #322 — "activate a runtime explicitly in the shared pre-warm tree" — is open and unmerged and targets exactly this signature. #322 and #308 both fail the same lane on their own heads, so it is not specific to this branch. The lane's last green run here (2026-08-26, bc0477d) predates the break.

The same run also shows a rocm serve … --engine lemonade --managed scenario failing with "exited 0, but … never served". That matches the EAI-7423 rows already in expectations.toml ("Lemonade managed serve reaches ready then shuts down immediately", effective_engine = "lemonade", os = "linux") rather than anything here — the Lemonade change in this PR only removes read-only probes that returned Ok, so it cannot affect whether the engine stays up.

The honest limit: the GPU lane cannot pass on this branch until #322 lands, so I cannot show this PR green there. It needs a re-run after #322.

docs/readthedocs.com is now green, incidentally — merging main picked up the Sphinx site from #311, which was the repo-wide cause of that red X.

Strix Halo (Ubuntu/Windows) and rad3 are still queued on shared runners as I write this.

---

Unrelated to the review, but worth knowing before this merges

#308 appears to contain this PR's feature independently. Its branch has a commit 46a5967 fix(install): always keep the SDK's build of the torch an engine pins — same subject, substantially the same change — and git merge-base --is-ancestor says the two branches share no ancestry, so it is duplicated work rather than a stack. #308 touches the same three files (main.rs, therock.rs, engines/vllm/src/lib.rs) including settle_engine_install, TorchAlignment, report_torch_alignment and unusable_runtime_error.

It also has something this PR does not: a third device-check outcome, device_check: kernel_failed, for torch that finds a GPU but cannot execute a kernel — AcceleratorError: CUDA error: device kernel image is invalid / hipErrorInvalidImage, observed on 2.11.0+rocm7.14.0. This PR's RuntimeDeviceCheck has only Usable/NoDevices/NotVerified, so that state currently reads as Usable here. That looks like a real gap rather than scope creep.

I have deliberately not tried to reconcile them — that is a call for the two authors, and re-implementing kernel_failed here would be duplicating someone else's measured work. Flagging it so whichever merges second is not surprised by the conflict.

[tomastola]

E2E tests (GPU) is green on 0669185

Follow-up to my CI-status note above. The lane passes now, and the reason it could not before is more specific than "#322 needs to land".

Why it never exercised this PR. In xtask/src/e2e_prewarm.rs, decide() returns Decision::Reuse whenever the installed runtime is up to date with the channel index, and run() then returns early — before both the install sdk and the update --apply arms. The cache is keyed on SDK freshness alone, with no notion of the CLI version that populated the tree. So the shared tree kept serving a torch installed by an older build, and every run printed:

pre-warm: reusing the shared release runtime (runtime is up to date with the channel index)

There are zero occurrences of torch in those logs — settle_engine_install never ran, so nothing in this PR could execute there either way.

How it was tested. I moved the shared pre-warm data/runtimes aside on the GPU runner so decide() would return Install, and re-ran. The reinstall resolved to the same SDK (release-wheel-gfx94x-dcgpu-7-13-0), so it is a like-for-like comparison, and it left one runtime instead of three. The runtime's torch went from 2.11.0+gitd0c8b1f to 2.11.0+rocm7.13.0 — same release the pin asks for, now the SDK's build — and the settle path rendered:

torch_alignment: realigned
  2.11.0+gitd0c8b1f -> 2.11.0+rocm7.13.0 (the SDK's build of the release vllm pins)
dependency_check: expected_divergence
divergence: The package `vllm` requires `torch==2.11.0+gitd0c8b1f`, but `2.11.0+rocm7.13.0` is installed
action: none; the SDK's build is intended here (see torch_alignment above)
device_check: usable (1 device(s), torch 2.11.0+rocm7.13.0)

84 scenarios (79 passed, 5 failed)
Reconciliation: 5 xfail (failed as expected), 0 XPASS (0 flaky, 0 stale), 0 unexpected failure(s).

bench-load-real-serve, chat-end-to-end-local-model and serve-vllm-inference all pass, and both Failed to infer device type and no active ROCm runtime is configured are absent from the log entirely.

Correcting myself: I said above that the lane could not pass until #322 landed. #322 was necessary but not sufficient — with the stale tree still in place, the pre-warm would have kept reusing regardless.

On poisoning — not the cause here, and no detection needed for this PR. The runner this landed on had a clean tree (every install_root valid) and still reused, so poisoning was never the blocker. One thing worth recording separately, though: repair_poisoned_runtimes calls list_runtimes, which bails when rocm runtimes list exits non-zero — and a poisoned manifest is precisely what makes it exit non-zero. It then prints "leaving the shared tree untouched" and returns Ok(()), while its own doc comment says "Failure to repair is fatal, unlike the rest of this module". With #317 stopping new poisoning and #322 filtering dead entries at selection, the remaining gap is narrow, and it belongs in its own issue rather than here.

[tomastola]

E2E tests (GPU) is green on 0669185

Follow-up to my CI-status note above. The lane passes now, and the reason it could not before is more specific than "#322 needs to land".

Why it never exercised this PR. In xtask/src/e2e_prewarm.rs, decide() returns Decision::Reuse whenever the installed runtime is up to date with the channel index, and run() then returns early — before both the install sdk and the update --apply arms. The cache is keyed on SDK freshness alone, with no notion of the CLI version that populated the tree. So the shared tree kept serving a torch installed by an older build, and every run printed:

pre-warm: reusing the shared release runtime (runtime is up to date with the channel index)

There are zero occurrences of torch in those logs — settle_engine_install never ran, so nothing in this PR could execute there either way.

How it was tested. I moved the shared pre-warm data/runtimes aside on the GPU runner so decide() would return Install, and re-ran. The reinstall resolved to the same SDK (release-wheel-gfx94x-dcgpu-7-13-0), so it is a like-for-like comparison, and it left one runtime instead of three. The runtime's torch went from 2.11.0+gitd0c8b1f to 2.11.0+rocm7.13.0 — same release the pin asks for, now the SDK's build — and the settle path rendered:

torch_alignment: realigned
  2.11.0+gitd0c8b1f -> 2.11.0+rocm7.13.0 (the SDK's build of the release vllm pins)
dependency_check: expected_divergence
divergence: The package `vllm` requires `torch==2.11.0+gitd0c8b1f`, but `2.11.0+rocm7.13.0` is installed
action: none; the SDK's build is intended here (see torch_alignment above)
device_check: usable (1 device(s), torch 2.11.0+rocm7.13.0)

84 scenarios (79 passed, 5 failed)
Reconciliation: 5 xfail (failed as expected), 0 XPASS (0 flaky, 0 stale), 0 unexpected failure(s).

bench-load-real-serve, chat-end-to-end-local-model and serve-vllm-inference all pass, and both Failed to infer device type and no active ROCm runtime is configured are absent from the log entirely.

Correcting myself: I said above that the lane could not pass until #322 landed. #322 was necessary but not sufficient — with the stale tree still in place, the pre-warm would have kept reusing regardless.

The fix also repairs an existing runtime in place — the wipe is not required.

The green run above was a fresh tree, which leaves open whether an already-broken runtime heals. It does. I restored a tree whose manifests predate this PR and ran rocm engines install vllm against it:

torch_alignment: realigned
  2.11.0+gitd0c8b1f -> 2.11.0+rocm7.14.0 (the SDK's build of the release vllm pins)
dependency_check: expected_divergence
device_check: usable (1 device(s), torch 2.11.0+rocm7.14.0)

A second run is idempotent — torch_alignment: already_aligned (2.11.0+rocm7.14.0), plus the 0669185 guard so the engine install no longer undoes it.

This exercises the branch @volen-silo flagged as untested. sdk_torch is introduced by this PR (git log -S sdk_torch returns only 9308505), so every pre-existing manifest lacks it and takes the format!("rocm{version}") fallback — that is the path that derived rocm7.14.0 above, and rocm7.13.0 on a second runtime. One correction to something I said earlier in review: I described that fallback as having a finite lifetime. It does not. sdk_torch is written in exactly one place (therock.rs:988, fresh SDK install); the repair does not backfill it, and I confirmed it is still None after a successful realign. The fallback retires only on reinstall, so it should be treated as a permanent path, not a migration shim.

Worth being explicit about the scope of the self-heal, since the pre-warm discussion above can make it look CI-shaped: Decision::Reuse returns before all three settle_engine_install entry points (engines(), resolve_engine_env via engine_shell, and maybe_auto_install_sdk_preferred_engine — rocm serve is not among them), so the alignment does not fire in the shared-tree CI path at all. Its audience is user machines, where the next engines install / install sdk / engines shell heals them.

And this is not a one-off migration: vLLM's metadata pins torch==2.11.0+gitd0c8b1f, and uv satisfies that from PyPI on every install. The fresh green tree above was installed by the fixed binary and still needed the realign.

On poisoning — not the cause here, and no detection needed for this PR. The runner this landed on had a clean tree (every install_root valid) and still reused, so poisoning was never the blocker. One thing worth recording separately, though: repair_poisoned_runtimes calls list_runtimes, which bails when rocm runtimes list exits non-zero — and a poisoned manifest is precisely what makes it exit non-zero. It then prints "leaving the shared tree untouched" and returns Ok(()), while its own doc comment says "Failure to repair is fatal, unlike the rest of this module". With #317 stopping new poisoning and #322 filtering dead entries at selection, the remaining gap is narrow, and it belongs in its own issue rather than here.

---

Review follow-ups — three fixes pushed

Thanks @volen-silo. Three of the points are addressed in 2492bfc, 22fb6d8 and 01fff76.

1. No escape hatch (2492bfc). Agreed, and this was the strongest of the three. The alignment rewrites a package the user may have installed deliberately, on every path that installs an engine, so a hand-installed torch was replaced again by the next engines install / install sdk / engines shell with no way to say no. That matters more than it would for a cosmetic correction, because this PR's own known limitation admits the resulting stack is not validated against the supported matrix — "the SDK's build does not work on my machine" needed an exit that was not "stop using the CLI".

ROCM_CLI_DISABLE_TORCH_ALIGNMENT skips the rewrite. Presence, not value, matching ROCM_CLI_DISABLE_STARTUP_UPDATE_CHECK. The check runs before the probe — someone reaches for this precisely when their machine does not probe cleanly, so it cannot depend on a clean probe first. The dependency and device checks still run: it suppresses the correction, not the diagnosis.

The two tests are a pair on purpose. Set, the call returns the opt-out reason even though it points at a Python that does not exist, which is what proves the gate precedes the probe. Unset, the same call has to fail at the probe instead, so the opt-out cannot appear to work for an unrelated reason.

2. NotVerified (22fb6d8) — doc only, deliberately. You read it as a case where an unimportable torch should fail the install, and the old one-liner did not explain why it does not. It fires for benign causes too — a runtime whose Python could not be located, a probe that could not launch — so making it fatal would fail multi-gigabyte installs because a probe did not run. Only NoDevices is acted on. The cost is real and is now stated rather than implied: a runtime whose torch is present but unimportable is reported rather than failed. Separating the two cases means splitting the variant, which is a behaviour change and wants its own PR.

3. update --apply — real, but narrower than described. It does call install_sdk directly without finish_sdk_install/settle_engine_install. But reading the whole of apply_runtime_update, it never installs an engine, so torch cannot end up misaligned through it. The actual inconsistency is that install sdk auto-installs and settles a preferred engine while update --apply does not. Worth fixing, not here.

4. Test coverage for the sdk_torch fallback — already there. Your grep was accurate at bc0477d but is stale at 0669185: the unit tests came in with that commit, and the hardware runs above exercise the fallback twice (7.13.0 and 7.14.0), because every pre-existing manifest lacks sdk_torch and takes it.

5. No E2E coverage of the alignment (01fff76). Fair, and it was the last gap I could close inside this PR. Scenario 4 asserted the outcome that matters — the runtime can still open a GPU after a second install sdk — but nothing asserted that the alignment itself fired, and those are not the same check. A runtime whose torch was never touched can still open a device, so device_check: usable passes whether settle_engine_install reached the engine or not. That left the gate in front of the settle step — the part most likely to be widened or narrowed by a later change — with no coverage at all: silently skipping the alignment looked identical to doing it correctly.

The new step reads the torch_alignment: block, and asserts it is present, that it reached one of the two healthy outcomes, and — separately, negatively — that it reached none of the three unhealthy ones. The negative half is not redundant: the positive check would pass on a run that also printed a failed second block. not_applicable is the one that would otherwise slip past, because it is what a manifest yielding no SDK build produces, which is exactly the repair path for every pre-sdk_torch runtime.

Both healthy outcomes are accepted rather than pinning one. Whether the reinstall rewrites torch or finds it already correct depends on what the shared tree held when the scenario started, and the rule held either way. The divergence assertion is conditional for the same reason: a divergence is today's steady state, but a future pair could agree and then there is nothing left to classify. What must never happen is the CLI reporting a divergence and calling it a defect, and that is what is asserted.

One clarification, since the pre-warm discussion above could be read as making this scenario moot: the Decision::Reuse early return is in the xtask that populates the shared tree. The scenario itself invokes rocm install sdk directly, which goes through finish_sdk_install and therefore the settle path, so it is not subject to that bypass. @requires-gpu @requires-engine:vllm @nightly, and no expectations.toml entry — it is expected to pass.

You were right about the torch stack, and it is worse than three

Your point that this treats torch as if it were the whole stack — while ComfyUI (torch_stack_constraints) treats the trio as the unit — is correct, and I measured it rather than reasoning about it. On the GPU runner's tree after this PR's alignment has run:

package	version	origin
torch	2.11.0+rocm7.13.0	SDK — realigned by this PR
triton	3.6.0+rocm7.13.0	SDK
torchvision	0.24.1+d801a34	engine's PyPI build
torchaudio	2.9.0+eaa9e4e	engine's PyPI build

So the stack is genuinely mixed, and it is four packages, not three. It works for vLLM — device_check: usable, 79 scenarios pass — which is exactly what makes it easy to miss. torchvision-0.24.1+d801a34 is character-for-character the string in EAI-8051's reproduction, so the provenance is not in doubt.

Two things fall out of that:

• ComfyUI's TORCH_STACK_PACKAGES names three packages. triton appears nowhere in the Rust sources (grep -rn triton --include='*.rs' apps/rocm/src/ is empty), yet it ships at +rocm7.13.0 and EAI-8051/EAI-8363 both show it being swapped for a generic build. Three of the four managed packages are protected by name; the fourth is not.
• Extending the realignment to torchvision/torchaudio is cheap, because the resolver already computes them and even carries a compatibility_key — we simply drop them, persisting only sdk_torch. Triton is not carried at all and needs a resolver change.

I have not widened this PR to cover it: it changes what the alignment is allowed to touch, and it deserves its own review rather than being appended to one that already has approval blocked. Filed as EAI-8427 and linked to EAI-8294.

[volen-silo]

Re-reviewed at 01fff76a.

The previous blockers are genuinely closed. I checked each against source rather than against the summary, and the substantive ones hold up: is_intended_torch_divergence (engines/vllm/src/lib.rs:585-604) implements the three-condition rule rather than hardcoding a winner, and both named tests are load-bearing — they fail if the predicate is pinned either way. The Lemonade guard (settles_runtime_torch, main.rs:8146) covers all three real call sites and is still load-bearing, since the adapter reports managed_env: Some(true) with a native binary. The misplaced docs, the sdk_torch_build_from_manifest tests, the duplicate test, the preserved device-probe error text, and the --index-url / --no-deps / uv_command_env flags are all closed. Both deferrals check out too: apply_runtime_update (main.rs:15166-15268) genuinely never installs an engine, and repair_poisoned_runtimes lives in xtask/, which this PR does not touch.

The problem is that the opt-out in 2492bfc2 introduces two new defects, and the first re-opens the exact failure this PR exists to close.

---

Blocking

1. Under the opt-out, the CLI recommends the command that produces the broken runtime

main.rs:7655 → :7832 → :8216-8232, rendered at :8250-8272

With the var set, align_runtime_torch returns TorchAlignment::NotApplicable. deliberately_diverged_package(&NotApplicable) then returns None (:7832), so classify_dependency_details(details, None) short-circuits to Violated (:8224) — and the Violated arm prints action: rocm engines install <engine> --reinstall. That is precisely the remedy the ExpectedDivergence arm exists to withhold, with a comment saying so.

The engine side does not read the var, so a single invocation contradicts itself:

warning: the runtime holds the SDK's build of the torch vLLM pins;
         that divergence is intended and a reinstall would undo it
  torch_alignment: not_applicable (torch alignment is disabled by ROCM_CLI_DISABLE_TORCH_ALIGNMENT)
  dependency_check: violated
  violation: The package `vllm` requires `torch==2.11.0+gitd0c8b1f`, but `2.11.0+rocm7.13.0` is installed
  action: rocm engines install vllm --reinstall

Reproduction, on an already correctly-aligned runtime:

export ROCM_CLI_DISABLE_TORCH_ALIGNMENT=1
rocm engines install vllm          # prints the above
rocm engines install vllm --reinstall   # following the advice

The second command restores the engine's build, skips realignment, and yields device_check: no_devices. Because NotApplicable is not part of the fatal conjunction (install_left_runtime_unusable, main.rs:8038, requires Unavailable|InstallFailed and NoDevices), it exits 0 — the silent-success behaviour the PR was written to eliminate, now reachable through the PR's own escape hatch. No test covers this path.

Either deliberately_diverged_package should still return Some("torch") when the opt-out produced the NotApplicable, or the opt-out should carry its own TorchAlignment variant instead of reusing one that means four other things.

2. The opt-out does not do what its doc promises

main.rs:7631-7646 (doc) vs engines/vllm/src/lib.rs:585-604 and :1195-1216

The doc justifies the var on the grounds that a hand-installed torch is otherwise "replaced again by the next engines install, install sdk, or engines shell". But is_intended_torch_divergence whitelists only the SDK's build. A user's own build is neither the engine's pin nor the SDK's, so repair_from_violations returns needed: true, and install_vllm_with_uv runs uv pip install vllm==<pin>, which pulls torch==2.11.0+gitd0c8b1f over the top of it.

So a user in the documented situation — "the SDK's build does not work on my machine" — sets the var, installs a working torch by hand, runs engines install for any routine reason, and loses it. Alignment is skipped by the opt-out, dependency_check now reports satisfied because torch matches the pin exactly, device_check: no_devices, exit 0. The hatch fails on its first intended use.

The engine needs to honour the same opt-out, or the doc should stop promising this.

3. New user-observable behaviour with no scenario and no stated reason — AGENTS.md §3

main.rs:7644-7646

ROCM_CLI_DISABLE_TORCH_ALIGNMENT changes command output and which torch ends up installed, so §3 applies. grep -r DISABLE_TORCH_ALIGNMENT tests/ returns nothing, and the PR description contains no mention of the var, the opt-out, or an escape hatch — the description was not updated after 2492bfc2. §3 wants either a scenario or an explicit note saying why one is not needed; neither is present.

Worth noting the interaction: the var forces torch_alignment: not_applicable, which is one of the three strings the new step in 01fff76a asserts must never appear.

For contrast, the new non-zero exit path has no scenario either, but the description's "What is not covered" section explains why end-to-end reproduction is impractical. That one reads as disclosed.

---

Still open from the previous round

Windows stdio capture

therock.rs:3083-3125

capture_python_stdout_with_env, new in 93085058 and the sole transport for both new probes, uses Stdio::piped() unconditionally. Its two siblings in the same file — capture_python_stdout (:3127-3191) and capture_command_output_with_temp_files (~:3048) — both branch on runtime_is_windows() and use a runpy.run_path plus file-redirect workaround. The new function is aware of the split; it calls runtime_is_windows() at :3072-3076 to pick the temp dir, then ignores it for capture.

If the workaround exists for the reason its shape suggests, then on Windows both probes degrade to NotApplicable and NotVerified. Neither is in the fatal conjunction, so rocm engines install vllm exits 0 having realigned nothing, and the fix is inert on a platform AGENTS.md §6 treats as first-class.

I could not confirm this — no Windows host, and git log -S "runpy" -- apps/rocm/src/therock.rs traces only to the squashed initial import, so the original motivation is not recoverable. Flagging it as plausible rather than proven, but it is the one item that could make the whole fix a no-op on a supported platform, and it has been open across five commits.

---

Worth fixing, not blocking

The sdk_torch fallback is untested against the version form it will actually meet. main.rs:7998-8030, engines/vllm/src/lib.rs:1330-1345, test at :3081-3095. The fallback is format!("rocm{version}") where version comes from rocm_sdk.__version__, but the value it reconstructs is derived from the index version. Those agree on the release channel. This repo's own fixtures carry rocm_sdk_version values like "7.13.0a20260423" (therock.rs:5428, :5492), and the description's evidence table shows a real SDK torch of 2.9.1+rocm7.14.0a…. The only test uses "7.13.0". If the forms disagree on a dated-alpha channel the result is Unavailable + NoDevices — the fatal conjunction — so both engines install and install sdk exit non-zero on a machine that was previously only broken at serve. One test with the dated-alpha form would settle it; preferring the index-derived provenance would remove the question.

The fallback's scope is wider than "pre-existing manifests". sdk_torch is written in exactly one place (therock.rs:988, wheel install) and set to None at :1117 (tarball install) and in adopt_runtime_from_probe. Newly created tarball and adopted runtimes take the fallback permanently too. For adopted runtimes it is a guess — an externally-built environment's torch has no necessary relation to the probed SDK version — though that degrades to Unavailable rather than a wrong install.

NotApplicable now carries four meanings. main.rs:7660-7663, :7770-7776. It covers the genuine no-ops, a probe launch failure, and now the opt-out. That conflation is the mechanism behind blocker 1, it excludes probe failures from the fatal conjunction, and it logs a failed probe at audit level info.

The fatal predicate is inverted for the more-broken case. main.rs:8038 fires on Unavailable|InstallFailed + NoDevices. A runtime whose torch cannot import at all yields NotVerified and exits 0, while the strictly less broken "imports but sees zero devices" fails the install. 22fb6d82 is honest about NotVerified being non-fatal, but describes the cost as "reported rather than failed" — not as the worse state exiting more successfully than the better one. Worth a sentence.

Presence-not-value only half matches the repo. ROCM_CLI_DISABLE_STARTUP_UPDATE_CHECK is presence-based, but ROCM_CLI_DISABLE_MANAGED_PYTHON_BOOTSTRAP (therock.rs:3338) is value-based (1/true/yes/on). This picks the looser one, so ROCM_CLI_DISABLE_TORCH_ALIGNMENT=0 disables alignment. Nothing in .github/, xtask/, tests/ or docs/ sets a ROCM_CLI_DISABLE_* var to 0, so it is not live — but it is a real inconsistency.

The conditional divergence assertion is vacuous today. tests/e2e-cucumber/tests/e2e/runtime_steps.rs:237-242. The only emitter of a divergence: line is the ExpectedDivergence arm, which unconditionally prints dependency_check: expected_divergence in the same block, so the if body cannot be false. Reasonable as forward-looking insurance; the comment claims more than it currently delivers.

A diagnostic hint is hidden in a case it describes. engines/vllm/src/lib.rs:660-663. The "if this recurs after rocm install sdk" hint is gated on sdk_torch_build.is_none(), but in the wrong-release defect the build is known and does match the SDK's. Cosmetic — needed: true still repairs correctly.

crates/rocm-core/src/uv.rs:301-303 says "Both the CLI and the vLLM engine need the same two fields", but main.rs never calls violation_subject / ViolationSubject; the CLI decides from a direct Python probe. Only the engine consumes it.

Two claims in the description are stale. The sample output at line 94 still reads (the SDK's build of the release the engine pins), while render_torch_alignment (main.rs:7778-7793) emits the release vllm pins. The "500 passing in the CLI crate" figure is now 527.

---

Checked and clean

The e2e negative assertion in 01fff76a holds: I constructed the two-block output (one realigned, one not_applicable) and walked runtime_steps.rs:203-243 — str::contains scans the whole haystack, so the not_applicable assertion fails on that text as intended, and there is no substring overlap between the healthy and unhealthy literals. settle_engine_install is also called at most once per invocation from all three sites, so two blocks are not reachable today regardless. The step fails loudly when the block is absent rather than skipping.

main.rs's +1347 hides nothing unrelated — roughly 740 production and 475 test lines; every production hunk outside the main new-code block (:2444, :3812, :4001, :4336, :7389, :8199, :8559) is on-topic, with the only incidental edits being borrow changes forced by the new call sites. Scenario tags and IDs are unique and consistent with siblings, orphan no step definition, and need no expectations.toml row. sdk_torch is Option<String> with #[serde(default)] and no deny_unknown_fields, so it is compatible in both directions and no schema constant needed bumping.

cargo fmt --all -- --check and cargo clippy --workspace --all-targets -- -D warnings are both clean.

One thing I could not pin down: my first cargo test --workspace --all-targets reported 526 passed; 1 failed in -p rocm --bin rocm, and 24 subsequent runs were green. I did not capture the name. ScopedTestEnv serialises only tests that take its mutex, while std::env::set_var is process-global, so any concurrent test that merely reads an env var can race — a pre-existing hazard this PR widens slightly by adding two more env-mutating tests. Recording it rather than dropping it, since I could not reproduce it to confirm.

[michaelroy-amd]

Integrated the kernel-execution safety and addressed the opt-out blockers in d616d14b.

Key changes since 01fff76a:

• device health now separates import/enumeration errors from a synchronized GPU kernel failure; device_count() alone is no longer considered usable
• torch retention is a three-state decision: exact engine pin, manifest SDK build, or realignment
• a detected GPU with NoDevices or KernelFailed now fails the install; no-GPU/unknown-host cases remain non-fatal
• ROCM_CLI_DISABLE_TORCH_ALIGNMENT now has a distinct Disabled outcome, never recommends the reinstall that would undo the choice, and vLLM repair honors the same opt-out for torch only while still repairing unrelated defects
• added a nightly GPU Cucumber scenario for the opt-out and strengthened the reinstall scenario

Local verification on the actual updated PR tree:

• cargo test --workspace --all-targets: 2,376 passed, 8 ignored
• cargo clippy --workspace --all-targets -- -D warnings: clean
• formatter, smoke, license-header, DCO/signature hooks: clean
• three independent focused reviews: PASS

@volen-silo @rominf please re-review the current head. I have not resolved any of your threads.

[michaelroy-amd]

Follow-up 74a157a6 fixes the self-hosted prewarm false negative exposed by the prior run: the Reuse path now ensures the active runtime default engine before returning. The prior MI300X job reused a current 7.14 runtime with no vLLM installed, so all vLLM scenarios failed before exercising #314. Local workspace tests now pass 2,380/2,380; the fresh hardware matrix is running.

[michaelroy-amd]

Fresh hardware evidence at 74a157a6:

• The prewarm Reuse defect is closed: the log now shows pre-warm: ensuring the vllm engine is installed, and vLLM is installed into the active release-wheel-gfx94x-dcgpu-7-14-0 runtime.
• The new kernel probe then catches the real incompatibility instead of reporting a false green:
  • torch_alignment: realigned
  • device_check: kernel_failed
  • AcceleratorError: ... hipErrorInvalidImage
• The runner identifies as gfx943. The current stable and nightly aggregate indexes do not publish a rocm-sdk-device-gfx943 package; using the available gfx942 build is therefore rejected at the first synchronized kernel rather than failing later during serve.
• Strix Halo Ubuntu, Windows, WSL2, and R9700 lanes pass. Required build/test/clippy/signature/license/docs checks pass.

The MI300X-labelled lane remains red because its actual reported target is gfx943 and no compatible published device package is available. This is now an explicit, early unsupported-artifact failure—not the original silent success. Run: https://github.com/ROCm/rocm-cli/actions/runs/33427086701/job/99603389430

The separate CodeQL app check lists 17 pre-existing cleartext-key alerts in unrelated provider-key code because this PR changes the large main.rs; the CodeQL workflow itself passes.

[tomastola]

Why the GPU lane was red — two stacked problems, both now cleared

The failure had two independent causes. One is a real defect in this PR's area and is fixed here; the other was state on the self-hosted runners and has been cleaned up.

1. settle_engine_install settled the wrong runtime's torch

It derived the target torch build and the device-probe library paths from a caller-supplied selector, but installed into the environment's own interpreter. Two of the three call sites pass a runtime_id — and runtime_id is shared by every side-by-side install of one channel and family:

runtime_key: release-wheel-gfx94x-dcgpu-7-13-0   version 7.13.0
runtime_key: release-wheel-gfx94x-dcgpu-7-14-0   version 7.14.0
runtime_id : therock-release:gfx94X-dcgpu        <- both

So with two runtimes installed side by side the selector can name a different runtime than the one the engine env actually lives in, and the alignment then installs one tree's SDK torch build into the other tree's python. That is exactly what the runners were doing.

Fixed in da2ea4c: the owning runtime is resolved from response.python_executable — longest containing install_root wins, compared both verbatim and canonicalized (the CLI stores install_root canonicalized, while an engine adapter echoes back whatever path it was handed, so a single-form compare fails silently on a symlinked runtimes directory). The selector stays as the fallback for interpreters outside every install root, i.e. external/self-managed envs. Three unit tests cover the shared-runtime_id case, an interpreter outside every root, and nested roots.

runtime_id is ambiguous by construction here, so keying anything per-runtime on it is unsound; runtime_key is the unique one.

2. The shared pre-warm cache held a runtime this branch cannot compose

With the fix in place the lane still failed, for a different reason. The pre-warm cache on the self-hosted MI300X runners contained a second runtime:

7-13-0  version 7.13.0  index_url https://repo.amd.com/rocm/whl/gfx94X-dcgpu
7-14-0  version 7.14.0  index_url https://repo.amd.com/rocm/whl-multi-arch

That second index_url cannot be produced by main: therock_index_urls builds whl/{family} and whl-multi-arch/{family}, and the latter 403s, so the only index main can reach for this family tops out at 7.13.0. The flat whl-multi-arch path is reachable only from #308, which redefines THEROCK_RELEASE_PIP_INDEX_BASE to it. The pre-warm cache is shared and retained across jobs, so a 7.14.0 multi-arch runtime installed by one branch's run is then picked up by every other branch's run.

It is fatal because torch==2.11.0+rocm7.14.0 is published only under whl-multi-arch, and multi-arch wheels are shipped stripped of device code by design — kernels live in .kpack archives carried by the aggregate device package. Measured on the runner:

	.hip_fatbin	libtorch_hip.so	offload targets	device check
2.11.0+rocm7.14.0 (multi-arch)	NOBITS, 0 bytes	136 MB	none	hipErrorInvalidImage
2.11.0+rocm7.13.0 (family index)	PROGBITS	188 MB	gfx942	usable, 1 device

This is not a 7.14 regression and the wheel is not corrupt — on-disk size matches the published wheel exactly, and +rocm7.13.0 under whl-multi-arch is stripped the same way. It is simply that resolving a version published only in the multi-arch channel into a family-specific tree always yields a kernel-less torch. That is the bug class #308's WheelRuntimeComposition / required_aggregate_device_target machinery exists to prevent — so #308 is the structural fix and this PR is the targeted one.

Worth calling out: unusable_runtime_error fired on both variants — no_devices on one tree, kernel_failed on the other. The device check worked exactly as designed; it caught a real cross-wiring rather than a false positive. It should not be weakened to get the lane green.

What was done to the runners

The 7.14.0 multi-arch runtime and its registry entry were removed from the pre-warm cache on both runners, and the active-runtime pointer on the one that referenced it was repointed at 7.13.0. The 7.13.0 tree is left warm and intact, so Decision::Reuse still fires and the lane still exercises the ensure_default_engine path on a warm tree — the case that surfaced this bug in the first place. main cannot re-create the 7.14.0 runtime, so this is not a treadmill; it will recur only when #308's hardware matrix next runs.

Follow-ups, deliberately not in this PR

• The engine env id is built from runtime_id, so it drops the version: the active runtime is not necessarily the one an engine ends up using. This PR makes the used runtime work; it does not make the right runtime get used. Wants its own change.
• Pre-warm runs on different branches share one retained cache, so a branch that can compose a runtime main cannot leaves that runtime behind for everyone else. Worth an isolation or version-fencing change independent of fix(install): resolve canonical ROCm streams (EAI-8268) #308.

[juhovainio]

Reviewed at da2ea4c. The design here is sound and the problem is real: a shared Python env that two installers both write torch into, with every health surface reporting ready afterwards, is exactly the kind of silent failure worth this much machinery. Separating kernel launch from device enumeration is the right signal, and the pure decision functions with their unit tests make the whole thing reviewable.

First, one thing I withdraw. I initially read the hipErrorInvalidImage on the GPU lane as evidence that "the SDK's build of the release the engine pins" is a bad premise. It is not, and your diagnosis checks out: the existing test in therock.rs confirms multi-arch is always built as {base}/{family}, so the flat whl-multi-arch index on that cached runtime cannot come from main. The device check caught a real cross-wiring rather than a false positive, and I agree it should not be weakened to get the lane green.

That said, the lane is red again at 9 seconds, and it is worth looking at before merge:

pre-warm: reusing the shared release runtime (runtime is up to date with the channel index)
pre-warm: ensuring the vllm engine is installed
Error: runtime selector `release-wheel-gfx94x-dcgpu-7-14-0` ... is not an exact usable runtime:
       installed runtime not found: release-wheel-gfx94x-dcgpu-7-14-0

The runtime is gone but an engine record still names it. Partly incomplete cleanup, but it also shows the new prewarm step has no tolerance for that state, and this PR is what put a fatal step on every warm reuse. Details inline.

Six comments below. The one on the prewarm is the one I would want fixed before merge; the rest are smaller, and two of them are one-liners.

A note on where the comments landed. GitHub does not serve a patch for apps/rocm/src/main.rs on this PR (the diff is too large for it to render), so I could not anchor anything there. Four of the six comments are about main.rs code and are attached to the nearest related hunk in another file, with the real location named in the comment text. Related: the default git diff reports 16.8k/13.2k across the ten files, while git diff --histogram gives the real 3,771/145. Worth putting in the PR description, since it affects how anyone reads this.

On CodeQL: the 17 high-severity alerts are false positives. All seventeen point at resolve_provider_api_key / set_provider_api_key / clear_provider_api_key, and this PR touches none of that code (grepping the diff for provider_api_key returns nothing). CodeQL says as much in its own summary: adding 2.2k lines to a 29k-line file shifted the line numbers past its change-detection threshold. Those want dismissing rather than fixing.

Two smaller things I checked that did not turn into comments: install_pinned_package using --index-url rather than --extra-index-url is the safer choice and is explained, and the Windows PATH versus LD_LIBRARY_PATH split in the probe is correct. One genuine nit not worth its own thread: detect_host_gpu() runs ExamineSummary::gather() at the top of settle_engine_install, but the result is only read in the last four lines, so it could move into the branch that needs it.

Also worth saying: ensure_default_engine is a good fix, and running it before the Reuse early return is what surfaced all of this in the first place.

[juhovainio]

This is where the lane is failing now, and I think it has a claim on this PR rather than only on the follow-up list.

The cleanup removed the 7.14.0 runtime, but an engine record still names it, so resolve_runtime_selector_to_exact_key bails and takes the whole job down 9 seconds in. repair_poisoned_runtimes already heals one flavour of stale state (a runtime recording an install root outside the tree), but nothing heals the inverse: an engine pointing at a runtime that no longer exists.

This matters more after this PR than before it, because this PR is what makes the engine check run on every warm reuse and be fatal on failure. The doc comment says fatal is deliberate, and for "the engine is missing" it clearly is. For "the engine names a runtime that was deleted out from under it" the prewarm can just fix it, and the shared retained cache means it will keep happening: any branch that can compose a runtime main cannot leaves it behind for everyone, which is your own second follow-up bullet.

Either extending repair_poisoned_runtimes to drop engine records whose runtime is gone, or catching this specific error here and reinstalling, would stop the next occurrence needing a human on the runners.

[tomastola]

Done in 83586fb — the pre-warm now repairs this instead of dying on it.

One correction on the mechanism, which does not change your conclusion: the dead name is not held by an engine record. resolve_engine_install_runtime_id takes its selector from config.active_runtime_key, falling back to config.default_runtime_id, and hands that to resolve_runtime_selector_to_exact_key. So the state that killed the lane was a config pointer at a runtime that had been removed, and an engine record never enters it. rocm runtimes list surfaces it as active_status: missing manifest for <field>=<selector>, which is what the repair keys off.

dangling_active_runtime reads that line, activation_candidates lists the managed runtimes the tree could be pointed at instead, and repair_dangling_active_runtime repoints. Both pointers are rewritten — the active.json marker and config.json's active_runtime_key — because they are read by different callers, and repairing only one leaves the engine install still following the dead key. That asymmetry cost me an hour on the runners.

Two deliberate non-repairs. Read-only entries are excluded as candidates: runtimes adopt records a folder the pre-warm does not own, and silently making somebody's external ROCm install the default for every GPU scenario is a bigger decision than fixing a pointer. And a tree with no runtimes at all still fails as before — there is nothing to repoint it at, and inventing one would hide an install that produced nothing.

You are right that this has a claim on this PR rather than the follow-up list, and for the reason you give: this PR is what makes the engine check run on every warm reuse and be fatal. Six tests cover it.

[juhovainio]

This comment is about classify_retained_torch in main.rs, anchored here because GitHub will not serve a patch for that file. This scenario's prose is the closest thing to an acknowledgement of it.

Retention requires the installed torch to equal the engine pin or the SDK build. A torch that is Usable, meaning it has been proven to run a GPU kernel, but is a third build (a user's own working install) falls through to Realign. If the replacement then fails the kernel probe, the install exits 1 and the working torch is gone, with no rollback.

The comment right here says the target stack "is not validated against the supported matrix", which is exactly the condition under which replacing a kernel-proven torch is the wrong trade. ROCM_CLI_DISABLE_TORCH_ALIGNMENT only protects someone who already knew to set it before the first run.

Two options, either seems fine: retain any Usable torch and report the divergence rather than correcting it, or keep the current behaviour but restore the previous torch when the post-realign probe fails. The second keeps the intent and removes the one-way door.

[tomastola]

Real, and I am not changing the behaviour in this PR. Reasoning, then what I am doing instead.

You have the mechanism right: retention requires equality with the engine pin or the SDK build, so a third build that is Usable falls through to Realign, and if the replacement fails the kernel probe the install exits 1 with the working torch gone. One-way door, accurately named.

Of your two options I would take the first — retain any Usable torch and report the divergence — not the rollback. A kernel-proven torch is the strongest evidence the tool has about that runtime, stronger than either name it matches against, and classify_retained_torch already treats "it runs a kernel" as decisive for the two builds it recognises. Extending that to any build makes the rule simpler: proven beats named, full stop. The rollback keeps the current intent but adds a second failure mode to reason about, since restoring a torch is another multi-gigabyte install that can itself fail, and it leaves the tool still preferring a name over a proof.

But that is a behaviour change to the retention rule, on a PR whose whole subject is the retention rule, landing after two rounds of review on the current one. It also widens TorchRetention into a state neither reviewer has seen — a divergence that is neither the engine's build nor the SDK's, reported and kept — and the reporting side of that is what makes it safe rather than silent. That deserves its own review, not a late commit here.

What this PR now does carry, and what changes the stakes:

• The pre-realignment verdict is printed under torch_alignment: realigned (#3903676438, 19f2da0). So when this fires, the output says outright that a usable runtime was replaced by a kernel_failed one, rather than showing only the after state. The one-way door is at least no longer an unlit one.
• ROCM_CLI_DISABLE_TORCH_ALIGNMENT is documented (#3903676458, 05fb20b), including the "locally built wheel / stack pinned for a reproduction" case, which is exactly the user you describe. Your objection that it "only protects someone who already knew to set it" still stands for the first run — documentation narrows that, it does not close it.

Filing the retention change as a follow-up with your first option as the proposal, and linking it here.

[tomastola]

Filed as EAI-8479, with your first option as the proposal: retain any torch whose kernel probe returns Usable — whatever installed it — and report the divergence, instead of installing a third build over a working one with no rollback.

The ticket records the argument you made here (the probe is stronger evidence than the provenance test the retention rule currently applies, so overriding it discards the better evidence), and notes the four things whoever picks it up has to decide: that broadening retention also makes an accidentally-odd torch a permanent fixed point, that a kernel-proven third build still is not a validated combination and has to say so, that NotVerified must stay firmly distinct from Usable if retention rests entirely on the probe, and that only torch is aligned today so this widens the set of reachable mixed stacks.

Leaving it out of this PR because it is a behaviour change rather than a fix for the bug, and it deserves to be decided on its own merits rather than widened in under a bug fix.

[juhovainio]

This function is the right abstraction, and engines/vllm uses it properly. classify_dependency_details in main.rs does not, which is the one place in the PR that hand-rolls what this parses:

let marker = format!("`{package}==");
if details.iter().all(|detail| detail.contains(&marker)) { ExpectedDivergence(details) }
else { Violated(details) }

Two consequences:

• contains does not check that torch is the subject of the violation rather than the requirer. That distinction is precisely what violation_subject exists to make.
• .all(...) is all or nothing. One unrelated violation, say numpy, demotes the whole set back to Violated, which renders action: rocm engines install <engine> --reinstall over the torch that was deliberately installed. That is the exact instruction assert_no_reinstall_remedy asserts must never appear, and it would only be caught there in the opt-out scenario.

The vLLM side already partitions intended divergences from real defects rather than deciding for the whole set at once. Doing the same here, through this function, would make both sides agree by construction instead of by review.

[tomastola]

Done in 7b6f99f, including the numpy case, which was the real defect — the hand-rolled matcher was the mechanism.

classify_dependency_details now partitions per line rather than judging the set:

rocm_core::violation_subject(detail)
    .is_some_and(|subject| subject.package.eq_ignore_ascii_case(package))

so an unrelated numpy violation stays a violation and the deliberate torch divergence is still reported as one, instead of either demoting the other. EngineDependencyCheck::Violated now carries both lists (violations and expected), and the renderer prints the expected ones as divergence: lines.

That also fixed a worse consequence I had not seen: when the set was demoted, the report advised rocm engines install <engine> --reinstall, which would have undone the divergence the alignment had just put there on purpose. Where any expected divergence is present, the advice is now to repair the real violations without reinstalling.

Two tests cover the boundary — an unparsable line stays a violation rather than being silently swallowed, and a divergence on any other package is still a violation.

[volen-silo]

Re-reviewed at da2ea4cd. Gates in a clean worktree: cargo fmt --all -- --check clean, cargo clippy --workspace --all-targets -- -D warnings clean, cargo test --workspace --all-targets 2384 passed / 0 failed / 8 ignored. The one-off test failure I could not reproduce last round did not recur across this round either.

Real progress since 01fff76a. The Disabled variant is the right shape and its doc comment does the valuable thing of explaining why it is not NotApplicable; install_left_runtime_unusable becoming device-only closes the exit-0 half of my first blocker outright; e57e5e4b turns a tautological assertion into a falsifiable one and writes down why, which is exactly the right instinct; and classify_retained_torch comparing the running interpreter's reported version rather than distribution metadata avoids a real trap deliberately.

All three of my blockers are nonetheless partially closed — each has a residual that is reachable, not theoretical — and the five new commits introduce four more. Detail below, most severe first.

---

New blocking issues

N1. 74a157a6 makes the prewarm fatal on all three decisions, against two precedents in this repo

xtask/src/e2e_prewarm.rs:376

ensure_default_engine(&rocm, prewarm_dir)? sits after the match &decision block, so it runs for Install and Update as well as Reuse. Every failure inside it (:435-450) takes the whole job down. The commit narrative and the doc comment at :431-434 frame this entirely around Reuse; the new fatal dependency on the other two paths is not discussed anywhere.

Two precedents in this repo say that is too broad:

• decide()'s own stated bias (:68-72): "the bias is deliberately conservative… A GPU lane must not be turned red… because the package index was briefly unreachable." It returns Reuse specifically when rocm update reports status=error (:97-105). ensure_default_engine then runs with no equivalent tolerance, so the exact transient that justified Reuse kills the job one call later.
• finish_sdk_install (apps/rocm/src/main.rs:8432-8469) does the functionally identical thing in the product and treats engine-install failure as non-fatal, with install_sdk_survives_an_engine_install_failure (:27554) commenting: "Failing here would throw away a multi-gigabyte install over an unreachable engine index."

Because ensure_default_engine passes no --runtime-id (:447-449), the CLI falls back to runtime_id.or(active_runtime_key).or(default_runtime_id) (main.rs:3857-3859). Six pre-existing states now become hard failures, each traced through resolve_engine_install_runtime_id → select_runtime_manifest (:9124-9158):

1. A stale global pointer naming a deleted runtime — bail!("installed runtime not found: …") at :9146, which is the currently observed 9-second failure verbatim.
2. No runtime configured even with exactly one ready runtime on disk — bail! at :3861-3863, though single_ready_runtime_key (:18548) exists and is used elsewhere.
3. An ambiguous selector matching multiple retained installs — bail! at :9153. Retention keeps up to --keep N per channel/format/family (storage.rs:36, :286-328), so this is reachable by design.
4. Cross-channel pointer clobbering. active_runtime_key / previous_runtime_key / default_runtime_id are a single global triple (crates/rocm-core/src/lib.rs:5209-5217) while the module doc says the shared tree holds multiple channels (e2e_prewarm.rs:39). A second channel's lane overwrites the pointer and demotes the first channel's in-use runtime out of two of its three retention protections (storage.rs:160-268). This is the most plausible mechanism for the observed "selector names an already-superseded version".
5. A transient index or network failure during the engine install — bail! at :441/:449, with no analogue of probe()'s Err → Reuse fallback (:325-340).
6. Disk-full. Not new in kind, but newly reachable on Reuse, which previously touched nothing beyond mkdirs.

Suggested split: "the selector genuinely cannot identify a usable runtime" stays fatal, since that is real misconfiguration; "the install itself failed" warns, as finish_sdk_install already does. The smallest concrete fix is at the call site — pass an explicit --runtime-id derived from the runtime this same run() just confirmed, rather than letting the CLI fall back to global config that may be stale.

N2. da2ea4cd hand-rolls path containment while the correct helper is already imported into the same file

apps/rocm/src/main.rs:3969-3986

fn both_forms(path: &Path) -> Vec<PathBuf> {
    let verbatim = path.to_path_buf();
    match path.canonicalize() {
        Ok(resolved) if resolved != verbatim => vec![verbatim, resolved],
        _ => vec![verbatim],
    }
}
…  .any(|root| pythons.iter().any(|python| python.starts_with(root)))
   .max_by_key(|manifest| manifest.install_root.as_os_str().len())

runtime_path_is_same_or_inside (crates/rocm-core/src/runtime.rs:434-443, via runtime_paths_equivalent at :418-431) already does separator-normalized, case-insensitive-on-Windows, existence-independent containment — and is already imported at main.rs:44. disk_space.rs:144-157 is a second precedent for this same "longest containing prefix wins" pattern and documents why: "Windows filesystems are case-insensitive, so a volume mounted at C:\Data must still match a path spelled C:\data\...", alongside strip_verbatim_prefix (:122-141) for the \\?\ prefix canonicalize adds on Windows.

Three silent fallbacks all land on the ambiguous selector (:8519), reopening precisely the bug this commit fixes: a Windows case difference between the CLI's stored install_root and the adapter's echoed python_executable (Path::starts_with is case-sensitive, so it only matches if canonicalize rescues both sides); canonicalize() failing; and runtime_key_for_python (:3945-3948) swallowing a registry read failure via load_runtime_manifests(paths).ok()?.

This now feeds a fatal path. A wrong runtime_key gives a wrong sdk_torch_build_for_key, so classify_retained_torch compares the installed torch against another runtime's SDK build, returns Realign, installs the wrong tree's torch build, fails the post-realign probe with KernelFailed, and exits 1 via unusable_runtime_error (:8571).

Concretely: side-by-side release-wheel-gfx94x-dcgpu-7-13-0 and -7-14-0 sharing runtime_id therock-release:gfx94X-dcgpu, engine env in 7-13-0, registry read transiently fails → owned = None → runtime_key = selector → where the selector resolves to the active 7-14-0, that runtime's SDK torch build is installed into 7-13-0's python and the install exits 1.

One thing I checked and can rule out: the classic /a/runtimes/foo versus /a/runtimes/foobar prefix bug is not present. Path::starts_with is component-wise, and trailing / and . components normalize away. That part is correct.

N3. da2ea4cd's three unit tests never exercise the mechanism the commit is built around

apps/rocm/src/main.rs:27835-27849

side_by_side_runtimes() uses /runtimes/release-wheel-gfx94x-dcgpu-7-13-0 and similar, and the outside-root test uses /opt/somewhere-else/bin/python3. None of these exist on the test machine, so path.canonicalize() fails for every path in every test and both_forms only ever takes the _ => vec![verbatim] branch. The "compared both verbatim and canonicalized" logic — the whole justification in the doc comment at :3960-3964, and the stated reason a single-form compare fails on a symlinked runtimes directory — has zero coverage.

Worth adding: a real-tempdir test where canonicalize succeeds, a symlinked-install-root test, a dangling-interpreter test, and a foo/foobar sibling test to pin behaviour that is currently correct only by accident of Path::starts_with's semantics.

Related: max_by_key (:3985) ranks by the verbatim configured install_root length regardless of which form-combination produced the match. Where two manifests both match — one root a symlink alias into another's tree — the winner is whichever string is longer, not the one that owns the resolved directory.

N4. The new non-zero exit fires on a bare except Exception, on a sensor that cannot see the failure modes it gates, with no override

apps/rocm/src/main.rs:8381

install_left_runtime_unusable is HostGpu::Detected && (NoDevices | KernelFailed). detect_host_gpu (:8338) returns Detected iff ExamineSummary::gather() yields detected_gfx_target.is_some(). Gather only errors when AppPaths::discover() fails (crates/rocm-core/src/lib.rs:1975, :1564), every detector under it swallows I/O errors with .ok()?, and it reads only /sys/class/kfd/kfd/topology/nodes (:4637) — world-readable, and entirely independent of whether the runtime's Python can open /dev/kfd. So NotVerified is effectively unreachable and Detected is true in cases where torch legitimately sees nothing:

host state	detect_host_gpu	device check	previously 0, now non-zero
container with host /sys but no --device=/dev/kfd --device=/dev/dri	Detected	NoDevices	yes
user not in render / video group	Detected	NoDevices	yes
ambient HIP_VISIBLE_DEVICES / ROCR_VISIBLE_DEVICES inherited by the probe (therock.rs:3117)	Detected	NoDevices	yes
AMD iGPU registered with kfd, discrete GPU not enumerated	Detected	NoDevices	yes
8-GPU host, GPU 0 faulty — the probe only touches the default device, never loops over device_count (therock.rs:2727-2763)	Detected	KernelFailed	yes
GPU fully claimed by another tenant, HIP OOM on the trivial alloc	Detected	KernelFailed	yes

The probe does call torch.cuda.synchronize(), so the async-launch false negative is correctly avoided — that part is right. But it wraps the launch in except Exception, and classify_runtime_device_probe (:8017) turns any non-empty kernel_error unconditionally into KernelFailed, which is unconditionally fatal. An unrelated AttributeError, an OOM, or an ECC/driver-reset message all become a hard install failure.

There is no override: no --force, no --skip-device-check, and torch_alignment_disabled() is explicitly documented as not suppressing it (:7695-7700). unusable_runtime_error (:8404-8409) advises installing "a torch build that suits both this SDK and this GPU", which is right for the real defect and misleading for every row above.

detect_host_gpu — the sensor the whole predicate rests on — has no test against any of these conditions, and the two e2e scenarios assert only that no_devices / kernel_failed are absent.

N5. The opt-out does not escape the new fatal exit, and the two compose badly

Under the opt-out, retention is Realign by definition, align_runtime_torch returns Disabled and installs nothing, and devices = probed — the pre-existing verdict — via the _ arm at :8547-8552. On a GPU-detected host whose torch enumerates zero devices, the install exits 1 having deliberately declined to change anything. Someone who sets the variable precisely because the correction is wrong for their machine gets a hard failure and no way out. It is documented as intentional at :7695-7700, but it still means the PR's only escape hatch does not escape the PR's only new failure mode. Combined with N4's false positives, this is the pairing I would most want an override for.

---

My earlier blockers — what remains

B1 — partially closed

Fixed and verified: TorchAlignment::Disabled { wanted, kept } (:7619-7627) is produced only once a replacement is actually due (:7745-7747), deliberately_diverged_package maps it to Some("torch") (:7937-7945), and the ExpectedDivergence arm prints action: none; this torch is kept on purpose (:8648-8662). The exit-0 half is closed too, since install_left_runtime_unusable is now device-only. Both new tests are load-bearing — they construct Disabled directly, so reverting the variant breaks compilation.

Residual: classify_dependency_details still gates on .all():

// main.rs:8617-8618
let marker = format!("`{package}==");
if details.iter().all(|detail| detail.contains(&marker)) {

One unrelated violation demotes the whole set to Violated, whose arm prints action: rocm engines install <engine> --reinstall (:8638-8646). Reachable state: opt-out set, a hand-built torch==2.9.0+cpu, plus a torchvision mismatch — the normal companion state for a hand-built torch, not a contrived one. Output then reads torch_alignment: disabled … keeping 2.9.0+cpu immediately followed by dependency_check: violated and the reinstall remedy. The contradiction is now conditional on a co-occurring violation rather than unconditional, which is an improvement, but it is the same contradiction.

B2 — partially closed

Fixed: is_intended_torch_divergence short-circuits to true for any torch pin violation when the opt-out is set (engines/vllm/src/lib.rs:618-620), and the doc no longer overclaims.

Residual: the classification is honoured, the mechanism is not. When any non-torch pin is also violated, repair_from_violations returns needed: true — its own test an_opted_out_custom_torch_still_repairs_an_unrelated_defect (:3208) uses a torchvision companion — which calls install_vllm_with_uv (:1247-1281):

uv pip install vllm==<pin> --extra-index-url <url>

No --no-deps, no constraint. vllm==<pin> declares an exact torch==<engine-pin> dependency the environment by construction does not satisfy, so uv resolves and overwrites the user's torch. Nothing asserts the invocation preserves torch, and scenario 9 covers only the single-divergence case.

B3 — partially closed

Closed: the scenario is real and correctly wired — tests/e2e-cucumber/features/runtime_setup.feature:73-80, @id:runtime-torch-alignment-opt-out @requires-gpu @requires-engine:vllm @nightly, unique id, tags matching sibling scenario 4, no orphan steps, no expectations.toml row needed, and the variable genuinely reaches the subprocess (tests/e2e.rs:719-740).

Open: the opt-out-specific assertions are conditional on channel state the scenario does not control, and on the shared warm tree they will not fire. torch_alignment_disabled() is read only inside align_runtime_torch, which runs only on TorchRetention::Realign (:8534). classify_retained_torch (:8218-8248) short-circuits to EngineBuild / SdkBuild whenever the installed torch already ran a kernel and matches either build — and scenario 4's own comment says that is what happens on this fixture. Scenario 9 is scenario 4 plus the variable, so Disabled is never produced, and the only assertion naming the variable is guarded by if verdict == "disabled". The step's doc comment is honest that disabled "cannot be demanded unconditionally", but that describes the gap rather than closing it. Net over scenario 4, this adds two negative string assertions. Deterministic coverage needs a Given that plants a third build first.

Also open: ROCM_CLI_DISABLE_TORCH_ALIGNMENT is in neither docs/ nor README.md, which between them document around 21 other ROCM_CLI_* variables; docs/vllm.md:18-24 is the natural home. The PR body discusses the opt-out but never cites the @id:, which §3 requires. And the new non-zero exit path has no scenario at all and no §3 justification — the body explains why the exit code is unit-tested, but §3 says a unit test does not discharge this and asks for either a named gated lane or a stated reason.

Windows stdio capture — unchanged, still unproven

capture_python_stdout_with_env (therock.rs:3117-3154) still uses unconditional Stdio::piped() at :3138-3140 while selecting a Windows-versus-Linux temp dir at :3123-3127. Both siblings still use the file-redirect workaround. None of the five new commits touched it; d616d14b is the only one editing therock.rs and only adds kernel_error. git log -S "runpy" still resolves to the squashed initial import, so the original motivation remains unrecoverable.

One mitigating finding: the consequences are bounded. probe_torch_alignment Err gives NotApplicable (:7724-7725), probe_runtime_devices Err gives NotVerified (:8000), and neither is in the fatal set. So the worst case is the fix silently no-opping on Windows rather than a broken runtime reported healthy. runtime_is_windows() is cfg!(windows), which is the correct condition.

Cheapest way to settle it without a Windows host: one round-trip test calling capture_python_stdout_with_env on a trivial print(json.dumps(...)), run by the existing windows-latest job.

---

On the concurrent review

I looked at all six of the inline comments in the other outstanding review and agree with all six, with two corrections worth recording.

On the prewarm: the suggested remedy of extending repair_poisoned_runtimes will not work. It runs first (:321) and is correctly ordered, but it only talks to rocm runtimes list and reasons about install_root: text — it has no engine visibility, and there is no subcommand to surgically clear a stale per-engine pointer. The only available lever is engines install --reinstall, which duplicates ensure_default_engine. Passing an explicit --runtime-id at the call site is the smaller change.

On classify_dependency_details: the all-or-nothing .all() consequence is real and is B1's residual above. The subject-versus-requirer consequence, though, cannot arise on this path — violations_requiring(&violations, engine) (uv.rs:288-296, called at main.rs:8590) has already filtered details to requiring == engine, and uv renders the requirer as `name` followed by ` requires, never `name==. Building the suggested counter-example confirms `torch== is not a substring of it. Separately, assert_no_reinstall_remedy would not catch the .all() case either, since it runs only in scenario 9, which has no engineered second violation.

On the third-party-torch realignment, of the two proposed options the second — restore the previous torch when the post-realign probe fails — is the right one. Retaining any Usable torch would reopen the earlier defect, because an SDK torch release the engine will not accept can still run a kernel perfectly well.

---

Smaller items

• Dated-alpha fallback — closed on the construction side. main.rs:27801-27818 uses a genuine "7.14.0a20260812" and asserts the full Some("rocm7.14.0a20260812"), load-bearing against a date trim since production is a naive format!("rocm{version}") (:8185-8190). Whether that build string resolves on a real SDK index remains untested, which is inherent rather than a regression.
• Fallback scope — unchanged. Tarball installs (therock.rs:1117) and adopt_runtime_from_probe (main.rs:8955) still write sdk_torch: None, so newly created runtimes take the fallback permanently, not only pre-existing manifests.
• NotApplicable conflation — down from four meanings to two. The opt-out has its own variant now, but a genuine probe launch failure still returns NotApplicable (:7724-7725) and is still logged at audit level info (:7910-7915).
• Inverted fatal predicate — unchanged but now disclosed. On a GPU host, a torch that cannot import at all exits 0 while the strictly less broken imports-but-sees-nothing exits 1. The 22fb6d82 doc (:7979-7989) now names the cost explicitly, which satisfies what I asked for last round; the asymmetry itself remains.
• Presence-not-value parsing — unchanged, not live. Both copies agree with each other. The repo majority is value-based: truthy_env (therock.rs:1831) and env_flag (rocm-core/src/lib.rs:1764) back five variables against two presence-based ones. Nothing in .github/, xtask/, tests/, docs/ or scripts/ sets such a variable to 0.
• Vacuous divergence assertion — closed. e57e5e4b replaced it with an unconditional, falsifiable !output.contains("dependency_check: violated"), and the comment explains why the old form could never fail. No new vacuous guards in e57e5e4b or 74a157a6's four tests.
• crates/rocm-core/src/uv.rs:301-303 still says both the CLI and the engine need violation_subject, but main.rs still never calls it.
• PR body has no mention of da2ea4cd. The HEAD commit changes which runtime gets settled and adds a whole new decision with tests at :27827-27920, but the "Unit tests" list — which presents itself as covering the decision surface rather than one happy path — omits it, and the "On hardware" table predates it. Both staleness items from my last review are resolved: the test-count figure is gone and the sample-output wording now matches render_torch_alignment (:7799).
• CodeQL. Agreed these are false positives, with one refinement to the evidence: grepping the diff for provider_api_key under git's default algorithm returns four lines, not none. They are an identical -/+ pair, i.e. alignment noise. Under --histogram it returns zero, and the diff sizes as 3,771/145 rather than 16,811/13,185. That --histogram figure is worth putting in the description, since it is also why GitHub will not render a patch for main.rs.

[tomastola]

E2E tests (GPU) is green at 05fb20bd

Run 33514333081, job 99877780346, on mi300x-1. The log carries the whole decision chain this PR adds, on hardware:

warning: expected divergence: The package `vllm` requires `torch==2.11.0+gitd0c8b1f`, but `2.11.0+rocm7.13.0` is installed
torch_alignment: retained_sdk_build (2.11.0+rocm7.13.0)
divergence: The package `vllm` requires `torch==2.11.0+gitd0c8b1f`, but `2.11.0+rocm7.13.0` is installed
device_check: usable (1 device(s), torch 2.11.0+rocm7.13.0)
84 scenarios (79 passed, 5 failed)
Reconciliation: 5 xfail (failed as expected), 0 XPASS (0 flaky, 0 stale), 0 unexpected failure(s).

Three things to read off that:

• retained_sdk_build, not realigned. The torch already in the tree executes a kernel against the installed SDK, so it stays exactly as it is and the engine's exact pin is deliberately not restored over it. That is the whole point of the change.
• The mismatch is reported as divergence:, not as a violation that demotes the runtime. An engine pinning 2.11.0+gitd0c8b1f against an SDK build of the same release is an expected divergence, not a reason to refuse to serve — the per-line subject judgement decides which is which rather than treating every unsatisfied requirement the same way.
• device_check: usable is a real kernel, not device_count(). That distinction is what caught the defect below in the first place.

One runner still carried the cross-wired torch

Removing the 7.14.0 multi-arch runtime (previous comment, §2) was necessary but not sufficient, and the residue is worth recording because it is the surviving evidence of §1.

mi300x-1's 7.13.0 tree had a matching +rocm7.13.0 torch, which is why the run above landed cleanly. mi300x-0's did not — its 7.13.0 tree still had torch-2.11.0+rocm7.14.0 in site-packages. Running that tree's own interpreter:

UserWarning: The program was compiled against a ROCm version matching '7\.14\.0' but the
installed ROCm version in this Python environment is 7.13.0.
2.11.0+rocm7.14.0

Deleting the 7.14.0 tree does not undo this: the wrong build was written into the surviving tree, so it outlives the tree it came from. That runner is being restored to the SDK build in place so it stops false-failing unrelated PRs — any red GPU lane on another branch in the last day is likely this, not that branch.

Follow-up, not for this PR: a warm tree only has its torch settled when an engine install actually runs. On the Decision::Reuse path with the engine already present, nothing re-examines a torch that is already cross-wired. This PR prevents the cross-wiring; it does not sweep one that has already happened.

CodeQL

The CodeQL check is red with 17 alerts attributed to this PR. All 17 are pre-existing on main — a strict subset of the alerts already open there — and none are in code this PR adds. GitHub's own check summary gives the reason: "Alerts not introduced by this pull request might have been detected because the code changes were too large." Not addressed here; they belong to a separate pass over main.

Remaining lanes

E2E tests (Strix Halo, Ubuntu), (Strix Halo, Windows) and (rad3 R9700) are still running at the time of writing. Everything else on 05fb20bd is green.

feedback addressed