[rominf]

Summary

• --version 10.1 stayed on the canonical 7.x index because it did not parse as major.minor.patch, then suggested --channel nightly, which refuses ROCm 10. A numeric major.minor pin now selects the newest stable publish in that line. A failed ROCm 10 pin no longer points at nightly. An exact --version 10.1.0 stays exact.
• Unpinned rocm install sdk is unchanged here and still installs ROCm 7.14.
• @id:therock-next-15-two-component-line-pin and @id:therock-next-16-unsatisfied-line-pin-skips-nightly cover the line pin and the failed-pin message. two_component_version_pin_routes_and_matches_its_release_line covers the resolver.

Test plan

• Therock tests passed locally (180), including two_component_version_pin_routes_and_matches_its_release_line.
• ensure_uv_venv_recreates_on_interpreter_tag_mismatch failed locally because it launches uv from /tmp (noexec). That is an environment limit.
• The two new scenarios are loopback fixture previews with no @requires-gpu tag, so the default e2e lane covers them. Full workspace cargo test and clippy were not run.
• This worktree was not rebuilt with cargo build --workspace --all-targets. A later build of the origin/main checkout does not prove this branch compiles.

[nowycondro]

🤖 Automated review (Claude Code) · a754cac

This automation never files a GitHub approval, so no approving review will appear here whatever the outcome — the merge decision stays with a human reviewer.

Review — needs work

Full review of the whole change (apps/rocm/src/therock.rs, apps/rocm/src/main.rs, README.md, tests/e2e-cucumber/features/therock_next_generation.feature, tests/e2e-cucumber/tests/e2e/therock_steps.rs), base 3dec0eb → head a754cac.
No ticket named. The PR description accurately describes the change's scope and honestly flags its own test-plan gaps (clippy/full cargo test not run locally, GPU lanes not exercised) rather than overclaiming — no mismatch found.

Blocking

• CI gate "Commit signatures + sign-off" is failing on the current head commit — this run
  cargo xtask verify-commits --base origin/main --require-verified reports: a754cac3 ... -> signature is not GitHub-"Verified". The commit carries a DCO Signed-off-by trailer but is not cryptographically signed / not GitHub-"Verified", which this repo's own AGENTS.md documents as a required, blocking gate ("a blocking CI gate (commit-signatures) re-checks every PR with GitHub 'Verified' status").
  Confidence 100 · mechanical · Fix: author signs the commit (SSH or GPG, with the key registered on GitHub) and re-pushes, per the job's own remediation text.

• published_in_stable_line can't recognize a stable release published with more than three dot-separated components, contradicting its own sibling function's documented contract — apps/rocm/src/therock.rs:212-219 (published_in_stable_line), via stable_release_line_pin (:199-204) and the strict parse_version (:7012-7047). version_request_matches's doc comment (:180-186) promises a two-component pin "selects every stable release in that major.minor line," but parse_version("7.2.4.70204") returns None (its splitn(3,'.') leaves .70204 as an unparseable stage suffix), and stable_release_line_pin's own fallback also returns None on the same string (rest after major/minor is .4.70204, not empty). This exact 4-component shape is not hypothetical: this file's own long-standing doc comment on VersionOrderKey/parse_version_for_ordering (:6805-6876) states these strings "come from a Python simple index" and calls 7.2.4.70204 "the shape ROCm's own packages are named with." Every caller that reaches a line pin through matches_version (select_rocm_version, select_matching_pip_package_versions, select_tarball_candidate, highest_candidate_needs_next_python) inherits this gap: a release published in that shape would be silently invisible to a major.minor pin aimed at its line.
  Confidence 75 · logic · Fix: have published_in_stable_line compare against the lenient parse_version_for_ordering release vector (release[0]==major && release[1]==minor && stage==Stable) instead of (or in addition to) the strict parse_version, the way compare_version_strings already does for ordering.

• next_layout_unsatisfied_hint's version_selector = None branch is new behavior introduced by this PR with no test exercising it — apps/rocm/src/therock.rs:7153-7172, the final _ => arm. It's reachable via rocm update --apply: install_sdk_for_update (:1219) always passes version_selector = None and layout = Some(SourceLayout::from_generation(...)) (:1235), and that Some(layout) bypasses select_source_layout entirely in both resolve_pip_runtime_with_timeout and resolve_tarball_artifact_with_timeout. On a runtime previously installed under the Next layout, a later failed update-apply lands on wheel_resolution_failure_context/tarball_resolution_failure_context with layout = Next, version_selector = None, hitting this branch. The only unit test of this function (unsatisfied_rocm_10_line_pin_does_not_suggest_nightly) covers Some(line pin), Some(exact pin), and Canonical + None, but never Next + None; the two new BDD scenarios only drive the Some(Version) fresh-install-preview path. This is new, untested, user-observable CLI output on a real failure path.
  Confidence 75 · logic · Fix: add a direct unit test for next_layout_unsatisfied_hint(None) (and/or the two failure-context functions called with SourceLayout::Next and None), asserting the wording and that it still avoids suggesting nightly.

Non-blocking

None

Decisions for the author

• The "exact pin failed on Next layout" hint always appends "...and an exact GPU arch, for example --family gfx1200" — tradeoff/wording. By construction, select_source_layout only ever returns Ok(Next) after already confirming family_resolution.raw_arch is exact, so by the time this specific hint can fire for an exact version pin, the arch was already exact — the actual cause of failure is that the exact version itself isn't published. The advice isn't wrong, just not pointing at the real cause in that case. Confirm this one-size-fits-all phrasing (shared with the line-pin case, where it legitimately IS the needed fix) is the intended tradeoff rather than an oversight.
• The two-component line-pin fix applies uniformly to every ROCm major, not just ROCm 10+ — tradeoff, confirmed deliberate and tested (the older_line/"7.14" case in two_component_version_pin_routes_and_matches_its_release_line). The PR's title/description frame the motivating bug around ROCm 10, but the fix also changes behavior for pre-10 two-component pins (e.g. --version 7.14), which previously never matched anything and now resolves. This is a desirable broadening, not a problem — flagging only so the author can confirm it's the intended scope, since the description doesn't call it out explicitly.

Positive signals

• The select_tarball_candidate sort-order fix (by_version.then(by_mtime) vs by_mtime.then(by_version)) is genuinely exercised by release_line_pin_prefers_the_newest_stable_tarball_in_the_line: the fixture's 10.1.2 carries an older mtime than 10.1.0, so the test actually discriminates version-first from mtime-first ordering rather than passing either way.
• The whole feature funnels through the single matches_version/version_request_matches choke point — every caller (select_rocm_version, select_matching_pip_package_versions, highest_candidate_needs_next_python, select_tarball_candidate, requested_source_layout) was checked and picks up the new line-pin semantics consistently, with no caller left on a stale exact-match assumption.
• The two new BDD scenarios assert negative conditions as well as positive ones (e.g. scenario 16 explicitly asserts --channel nightly is not suggested and no install plan leaks out on failure), guarding against a vacuous pass.
• README.md and the main.rs clap doc-comment were updated consistently in both places that describe this behavior, with no contradiction between them.

Deployment notes

None beyond the CI gate above — no schema/migration change, no new dependency, no breaking change to an existing working flow (two-component pins were previously non-functional, so this only adds capability).

What this covered

Full read of all 5 changed files at a754cac vs merge-base 3dec0eb: apps/rocm/src/therock.rs (13,012 lines) was read in full by one independent worker focused on the core version-matching/layout-selection logic; README.md, apps/rocm/src/main.rs's relevant doc comment, tests/e2e-cucumber/features/therock_next_generation.feature, and tests/e2e-cucumber/tests/e2e/therock_steps.rs were read in full by a second independent worker cross-checking docs/CLI-help/e2e tests against the implementation. I independently traced the blast radius across every caller of matches_version/RuntimeVersionSelector, the rocm update --apply code path, and confirmed the CI check-run failure directly from its job log.

Did not run: an actual cargo test/cargo build myself for the two logic findings above — both are hand-traced against the source, not executed (scored 75, not 100). History pass (git blame on the modified region) found only this PR's own commit touching it, nothing notable. Prior-changes pass: this PR has zero existing reviews, nothing to reconcile. Agent-instruction adherence checked against the root AGENTS.md only (no directory-scoped AGENTS.md/CLAUDE.md applies to the touched paths). Not reconciled against any PR discussion (none exists yet).