ROCMAI-813: split rocm-core diagnose.rs by platform family and extract the checker catalog by jussielo-amd · Pull Request #890 · ROCm/rocm-cli

jussielo-amd · 2026-11-04T19:54:47Z

Summary

crates/rocm-core/src/diagnose.rs had grown to about 3,430 production lines plus a 5,400-line test module, so a reviewer could not open it without scrolling past a dozen unrelated failure modes, and it was the one file over the 1,500-production-line ceiling that the repository is about to enforce. 83% of it was a single closed failure catalog, and the catalog already tags every check with the platform families it applies to, so the file splits cleanly along platform lines.

This PR splits it into child modules under diagnose/ and changes nothing else. diagnose.rs stays the module root (public types, the two score thresholds, diagnose, the mod declarations and the re-exports), so every rocm_core::diagnose:: path and the lib.rs re-exports keep resolving, and the file the docs, the AGENTS.md citation and the xtask workspace walk point at is still there. The catalog is not touched: CHECKERS has the same entries, order, ids, platform tags and scores (ties still break by catalog order), no keyword, score, message or fix plan changed, and doctor-catalog.json is byte-identical (cargo xtask catalog --check). The 175 diagnose #[test] functions (174 compile on Linux; the 175th is the Windows-only fix-9 verify test) moved with the code they exercise and keep their names; the leaf-name test list is identical before and after, with and without --features e2e-test-hooks.

Production lines now (test modules excluded, brace-matched): diagnose.rs 235, catalog.rs 211, scoring.rs 510, report.rs 192, linux.rs 645, windows.rs 337, wsl.rs 405, host.rs 545, engine.rs 534, plus the test-only test_support.rs 78. The largest file is linux.rs at 645, so every file is well under 1,500.

Completes ROCMAI-813.

What moved

Module	What it holds
`diagnose.rs` (root)	`MIN_SCORE_FOR_MATCH`, `HIGH_CONFIDENCE`, `Fix`, `Diagnosis`, `Route`, `DiagnoseReport`, `any_cleared_threshold`, `diagnose`, the two `vllm_*_symptom_is_diagnosable` classifiers (they run the whole catalog, so they sit in the root, above the modules that list the checkers), the `mod` declarations, and the `pub use` re-exports of `render_report_text`, `upstream_route`, `dkms_driver_plan_is_supported`, `VLLM_OOM_CANONICAL_SYMPTOM` and `VLLM_IMPORT_CANONICAL_SYMPTOM`
`scoring.rs`	`KeywordTable`, every `KEYWORDS_*` table (including the two WSL ones), `keyword_score`, `keyword_score_collapsing_overlaps`, `keyword_score_impl`, `symptom_matches`, `amd_gfx_targets`, `amd_gpu_count`, `zero`, `finalize`
`report.rs`	`upstream_tracker`, `upstream_route`, `route_when_no_match`, `is_advice_only`, `render_report_text`
`linux.rs`	`check_3`, `check_4`, `check_5`, `check_7`, `check_10`, `check_11`, `check_12`, `refresh_command_for_family`, `repo_pkg_family`, `is_repo_native_installed`, `dkms_driver_plan_is_supported`
`windows.rs`	`check_13`, `check_14`, `check_15`, `check_21`, `check_22`, `msvc_redist_older_than`, `MSVC_REDIST_MIN_VERSION`, `MSVC_REDIST_OUTDATED_SCORE`, `DISPLAY_DRIVER_OLD_SCORE`
`wsl.rs`	`check_wsl_1` to `check_wsl_7`, `wsl_facts`, `WSL_DOCS_URL` (only the WSL checks use it), and the section comment explaining the parallel WSL catalog
`host.rs`	`check_1`, `check_2`, `check_6`, `check_8`, `check_9`, `check_20`, `notes_1_arch`, `major_version` (checks that run on more than one platform, grouped by what they inspect)
`engine.rs`	`check_16`, `check_17`, `check_18`, `check_19`, `vllm_anchored_lines`, `scores_on_every_engine_host`, `VLLM_ANCHOR_PATTERN`, `SHM_MIN_BYTES`, `SHM_RECOMMENDED`, and the two canonical symptom constants (`scores_on_every_engine_host` is the helper the root's classifiers call, so it is `pub(super)`)
`catalog.rs`	`Checker`, `CHECKERS`, `WSL_ONLY`, `platform_family`, `run_all_checks`, `catalog_covers`, `uncovered_platform_message`, `take_applicability_from_the_catalog`, and the comment above the checkers, rewritten for the new layout (a new checker goes in the child for its platform and at the end of `CHECKERS`)
`test_support.rs`	test-only `Examination` builders used by more than one module's tests: `linux_base`, `wsl_base`, `igpu_dgpu_host`, `windows_driver_exam`, names and signatures unchanged

Private items became pub(super) only where another module needs them; everything else stays private to its child.

Module edges after the split, listed from each file's use lines and from every name it uses that the root imports or re-exports from a child (so indirect edges through the root are included). Production code and test code are listed separately.

Module	Production code uses	Its tests also use
`scoring`	root types (`Diagnosis`, `Fix`)	root thresholds
`report`	root types and thresholds	root `diagnose`, `test_support`
`linux`, `windows`, `wsl`, `host`	root types, `scoring`	root `diagnose`, `render_report_text` (the root's re-export of `report`), `test_support`
`engine`	root types, `scoring`	root `diagnose`, `test_support`
`catalog`	root `Diagnosis`; every checker in `linux`, `windows`, `wsl`, `host`, `engine`	root `diagnose`, `render_report_text`, constants in `engine` and `windows`, `test_support`
root	`catalog` (`platform_family`, `run_all_checks`, `take_applicability_from_the_catalog`, `uncovered_platform_message`), `report` (`route_when_no_match`), `engine` (`scores_on_every_engine_host`, the canonical constants)	`engine`, `test_support`
`test_support`	nothing from the module

No production code in a child uses another child, directly or through the root, except catalog, which names every checker, so there is no module cycle in production code. A first version of this split had one: vllm_oom_symptom_is_diagnosable and its import sibling run the whole registry, so they called run_all_checks from engine while catalog listed the engine checkers, and the call went through the root's private import, which hid the edge. They now live in the root, which already imports run_all_checks, and their public paths (rocm_core::diagnose:: and the lib.rs re-export) are unchanged. Their three tests (the two canonical-symptom tests and the engine-host guard) moved with them, because their subject is the wrappers and the registry they run. The only remaining loop-like edge is in test code: the end-to-end tests in each child call the root's diagnose, which runs the catalog.

Verification

Ran, on the final commit (each of the eight commits was also verified on its own: cargo fmt --all --check, clippy with -D warnings for rocm-core on Linux, with --features e2e-test-hooks, and cross-checked for x86_64-pc-windows-msvc, cargo test -p rocm-core --lib --no-fail-fast, the leaf-name diff, cargo xtask catalog --check, manifest --check and check-crate-edges):

cargo test -p rocm-core --lib --no-fail-fast: 1007 passed, 3 ignored, same as origin/main.
Leaf-name test list (cargo test -p rocm-core --lib -- --list | grep ': test$' | sed 's/: test$//; s/.*:://' | sort) against origin/main: empty diff, with and without --features e2e-test-hooks. 174 tests under diagnose:: on Linux, before and after. Counted by attribute, the old diagnose.rs held 175 #[test] functions (174 on Linux, because one is cfg(windows)); afterwards the root keeps 8 and the children hold 167 (catalog 13, engine 35, host 22, linux 37, report 8, scoring 3, windows 32, wsl 17), 175 again.
A pure-move check that parses origin/main's diagnose.rs and the new files into items and compares them after normalising visibility and whitespace: no item lost, and no item changed except the ten commented below.
cargo test -p rocm advised_commands (20 passed; includes exclusions_still_match_something), cargo test -p xtask ascii (27 passed), cargo test -p e2e-cucumber --test skill_reference (3 passed) and cargo test -p e2e-cucumber --lib (196 passed). skills/rocm-doctor/reference.md, its SKILL.md and every feature file's scenarios are unmodified; only comments that name a path changed.
Mutation checks for what this PR edits: re-keying an advised_commands.rs pin back to diagnose.rs makes exclusions_still_match_something fail; adding a hand-set auto_applicable: true to a Linux checker makes no_checker_hand_sets_auto_applicable fail (see below for why that test was edited). Both were restored.
cargo doc -p rocm-core --document-private-items has no unresolved-link warning in diagnose/ (six links needed an explicit path after their targets moved).

Not run: cargo xtask e2e, scripts/smoke_local.py, the full workspace test and clippy gates, and cargo xtask coverage (CI carries them; no test was added, removed or changed, so no coverage floor is expected to move). The Windows target was checked with cargo clippy -p rocm-core --all-targets --target x86_64-pc-windows-msvc, with the C build step of a dependency stubbed out on this Linux host, so it type-checks the cfg(windows) test and the cfg(unix) helpers and their imports but runs nothing; the Windows CI lane is the real check for those tests.

User-visible behaviour

None. This is a pure internal move: no command output, exit code, file or engine selection changes, so no new scenario applies. The scenarios that exercise the catalog end to end (diagnose.feature, examine.feature, rocm_doctor_skill.feature) are unmodified and unaffected.

Notes for reviewers

Read it commit by commit; each is a pure move that builds and passes clippy and the tests on its own.

part 1: scoring.rs and the test-only test_support.rs (the shared builders first, so the later commits never touch them).
part 2: report.rs, with two advised_commands.rs pins re-keyed to the new file.
part 3: linux.rs, plus the re-export of dkms_driver_plan_is_supported.
part 4: windows.rs.
part 5: wsl.rs.
part 6: host.rs.
part 7: engine.rs, the two comgr-conflict pins re-keyed, and the engine re-exports.
part 8: catalog.rs and the catalog-level tests, the last pin, the xtask module doc, and the docs and comments that cite diagnose.rs.
Follow-up commits from review: repoint the comments, assertion messages and docs that still named diagnose.rs for moved code (and make the AGENTS.md and e2e README citations links); move the two vLLM classifiers to the root to remove the engine/catalog loop; make the guard test scan the directory.

Things worth a look:

Ten items differ from origin/main beyond visibility and imports, and nine of them differ only in comments: the section comment above the checkers (rewritten in part 8 and moved next to CHECKERS), the WSL section comment (now at the top of wsl.rs), and the neighbouring items it was attached to, plus five items whose doc comments linked to a moved item by bare name ([MIN_SCORE_FOR_MATCH] and similar) and now use an explicit path so the links keep resolving. The tenth is the guard test: no_checker_hand_sets_auto_applicable used include_str!("diagnose.rs"), which would have gone vacuous as checkers moved out, so it now walks the diagnose/ directory recursively (skipping only the test-only test_support.rs), so a new child module, nested or not, is scanned without being listed. It was checked in four ways: a hand-set auto_applicable in an existing child turns it red, so does one in a new file dropped into the directory, so does one in a nested file such as diagnose/linux/zz_scratch.rs, and it fails if it scanned no child at all.
Tests moved with the code they exercise. Judgment calls: tests that name two checkers from different modules (fix_13_still_answers_a_pasted_hip_error_when_no_amd_gpu_was_detected, the fix-catalog cross-checks, the PowerShell text rule) went to the catalog.rs tests, which may name every module. sub_threshold_causes_leave_has_match_false_and_route_upstream stays in diagnose.rs because tests/e2e-cucumber/tests/e2e/skill_steps.rs cites it as diagnose::tests::. The old-display-driver tests that render reports are with windows.rs, the Windows-only fix-9 verify test and the cfg(unix) shell helpers are with host.rs.
Comments and assertion messages in fix.rs that named diagnose.rs for the fix-3, fix-7 and fix-12 checkers now name diagnose/linux.rs; the needs_reboot message, which is about literals spread over several modules, names "the diagnose modules".
rocm_doctor_skill.feature and skill_steps.rs still cite diagnose.rs, which is right: the test they point at did not move.
Commit signatures show U locally because this machine has no allowed-signers file; they are signed with the configured SSH key.

Open PRs that touch the same lines (checked with a trial merge against each PR head; none of them conflicts with this branch beyond what is listed):

ROCMAI-762: split rocm-core fix.rs, automation.rs, net.rs and openmpi.rs by domain #886 (split of fix.rs and friends) edits about 14 comment lines in diagnose.rs that name fix.rs, plus AGENTS.md, docs/architecture.md, docs/testing.md and the same feature and step files. The comment edits will have to be re-applied in the child file that now holds each line: parts 3 to 8 conflict in diagnose.rs, fix.rs and docs/architecture.md, and part 8 also in AGENTS.md and tests/e2e-cucumber/tests/e2e/diagnose_steps.rs. Whichever lands second rebases; the contents are small comment hunks.
fix(examine): keep the rocminfo agent name past its ISA entries #394 (rocminfo agent name) edits the imports, amd_gfx_targets and check_1_arch_not_in_wheel; after this branch those hunks belong in scoring.rs and host.rs. It already conflicts with main in examine.rs.
test(e2e): describe GPU hosts with simulated machines (ROCMAI-677) #517 touches the comment in examine.feature that names the CHECKERS gate.

Dependencies

Depends on nothing. It is independent of #886 apart from the small rebase above.

…yword tables and scoring Move every KEYWORDS_* table, KeywordTable, keyword_score and its two variants, symptom_matches, the Examination accessors amd_gfx_targets and amd_gpu_count, and the zero/finalize constructors into diagnose/scoring.rs. The three keyword-score tests move with them. The Examination builders that tests in more than one later module share (linux_base, wsl_base, igpu_dgpu_host, windows_driver_exam) move into a test-only diagnose/test_support.rs with their names and signatures unchanged, so the later moves do not touch them. This is a pure move. diagnose.rs stays the module root and imports what it still uses. no_checker_hand_sets_auto_applicable read only diagnose.rs; it now reads every file that can hold a checker so it keeps guarding the checkers as they move out. Signed-off-by: Jussi Elo <jussi.elo@amd.com>

…uting and the text report Move upstream_tracker, upstream_route, route_when_no_match, render_report_text and is_advice_only into diagnose/report.rs, together with the report-rendering tests and the routing tests. The routing entry point, its tracker table and the hand-maintained framework list in routing_targets_cover_every_framework_the_probe_reports stay in one file, because the comment on route_when_no_match describes a three-place edit. diagnose.rs re-exports render_report_text and upstream_route, so every rocm_core::diagnose:: path and the lib.rs re-export keep resolving. The two report headings that advised_commands.rs excludes by file are re-keyed to the new file. This is a pure move: no behaviour, text or score changes. Signed-off-by: Jussi Elo <jussi.elo@amd.com>

…nux-only checks Move check_3_rocm_kernel_unsupported, check_4_render_group, check_5_amdgpu_blacklisted, check_7_stale_repos, check_10_container_devices, check_11_iommu_hang and check_12_repo_native_broken into diagnose/linux.rs, with their helpers refresh_command_for_family, repo_pkg_family, is_repo_native_installed and dkms_driver_plan_is_supported, and the tests that exercise them. diagnose.rs re-exports dkms_driver_plan_is_supported, which the driver planner calls as rocm_core::diagnose::dkms_driver_plan_is_supported, and imports the seven checkers for CHECKERS, which stays where it is until the last part. The one comment in fix.rs that names check_3_rocm_kernel_unsupported now names its new file. This is a pure move: no behaviour, text or score changes. Signed-off-by: Jussi Elo <jussi.elo@amd.com>

…ndows-only checks Move check_13_hip_sdk_missing, check_14_adrenalin_too_old, check_15_msvc_redist, check_21_msvc_redist_outdated and check_22_display_driver_old into diagnose/windows.rs, with msvc_redist_older_than, MSVC_REDIST_MIN_VERSION, MSVC_REDIST_OUTDATED_SCORE and DISPLAY_DRIVER_OLD_SCORE, which only these checks use, and the tests that exercise them (the HIP SDK gate, the outdated-runtime comparison and the old display driver advice). The module has no platform-gated production code, so no cfg arms are split. The docs and the one feature-file comment that cite these unit tests by file now cite the new file. This is a pure move: no behaviour, text or score changes. Signed-off-by: Jussi Elo <jussi.elo@amd.com>

…L2-only checks Move check_wsl_1_gpu_not_exposed through check_wsl_7_wsl1 into diagnose/wsl.rs, with wsl_facts, WSL_DOCS_URL (only these checks use it) and the section comment that explains why the WSL2 catalog is a parallel catalog rather than a port of the bare-metal one, together with the tests for the WSL-only entries. The two comments in model_fit.rs that point at the fix-wsl-6 reasoning now name the new file. This is a pure move: no behaviour, text or score changes. Signed-off-by: Jussi Elo <jussi.elo@amd.com>

…st checks shared by several platforms Move check_1_arch_not_in_wheel, check_2_hsa_override_unneeded, check_6_path_missing, check_8_wheel_rocm_mismatch, check_9_igpu_dgpu_collision and check_20_gpu_unsupported_by_rocm into diagnose/host.rs, with notes_1_arch and major_version, and the tests that exercise them, including the PATH and override verify-line tests and the Windows-only fix-9 verify test. These checks apply on more than one platform, so they are grouped by what they inspect rather than by tag. The section comment above the first checker, which says where a new checker goes, is left in diagnose.rs next to CHECKERS; the last part rewrites it. The docs and comments that cite the moved tests and check_8_wheel_rocm_mismatch by file now cite the new file. This is a pure move: no behaviour, text or score changes. Signed-off-by: Jussi Elo <jussi.elo@amd.com>

…gine runtime checks Move check_16_vllm_oom, check_17_torch_dlpack_cuda_variant, check_18_comgr_conflict and check_19_shm_too_small into diagnose/engine.rs, with vllm_anchored_lines, scores_on_every_engine_host, VLLM_ANCHOR_PATTERN, SHM_MIN_BYTES and SHM_RECOMMENDED, the two vllm_*_symptom_is_diagnosable functions, the two canonical symptom constants, and the tests that exercise them (vLLM out-of-memory, the import failure, the code object manager conflict and shared memory). diagnose.rs re-exports VLLM_OOM_CANONICAL_SYMPTOM, VLLM_IMPORT_CANONICAL_SYMPTOM and both diagnosable functions, so the existing pub use of diagnose items in lib.rs is unchanged. The two comgr-conflict strings that advised_commands.rs excludes by file are re-keyed to the new file, and the comments in examine.rs, library_copies.rs and fix.rs that name check_18_comgr_conflict cite it there. This is a pure move: no behaviour, text or score changes. Signed-off-by: Jussi Elo <jussi.elo@amd.com>

…e catalog Move Checker, CHECKERS, WSL_ONLY, platform_family, run_all_checks, catalog_covers, uncovered_platform_message and take_applicability_from_the_catalog into diagnose/catalog.rs, with the catalog-level tests: the ASCII walk over CHECKERS, the shared checks under WSL, platform scoping, out-of-scope reports and the cross-checks against the fix catalog. CHECKERS keeps its entries, order, ids, platform tags and scores, so ties still break by catalog order and doctor-catalog.json is unchanged. sub_threshold_causes_leave_has_match_false_and_route_upstream stays in diagnose.rs, where tests/e2e-cucumber cites it by its diagnose::tests:: path. diagnose.rs is now the module root: the public types, the two thresholds, diagnose, the mod declarations and the re-exports. The comment above the checkers is rewritten for the new layout: a new checker goes in the child for its platform and its entry at the end of CHECKERS. The path pin for the out-of-scope prose in advised_commands.rs, the module doc in xtask/src/ascii_output_contract.rs, docs/architecture.md, AGENTS.md, the e2e README and the remaining comments in fix.rs and the feature and step files that cite diagnose.rs now cite the file that holds the code. Six intra-doc links in scoring.rs and windows.rs that stopped resolving when their targets moved are written as explicit paths. This is a pure move: no behaviour, text or score changes. Signed-off-by: Jussi Elo <jussi.elo@amd.com>

… moved code The fix.rs comments and assertion messages that describe what the fix-3, fix-7 and fix-12 checkers emit now name diagnose/linux.rs, and the needs_reboot message names the diagnose modules. The architecture notes for the examine probes name the files that now read comgr_matches_runtime and parse_dotted_version. Three test comments in host.rs and one in windows.rs that said "above" for a test that moved now name the file and the test. The citations of diagnose.rs in AGENTS.md and the e2e README are relative markdown links. Signed-off-by: Jussi Elo <jussi.elo@amd.com>

vllm_oom_symptom_is_diagnosable and vllm_import_symptom_is_diagnosable run the whole catalog, so while they sat in diagnose/engine.rs that module reached run_all_checks in catalog.rs through the root's private import, which hid a loop with catalog.rs, where the engine checkers are listed. They move into diagnose.rs, which already imports run_all_checks, and the engine module no longer uses anything outside the root types and scoring. The public paths are unchanged: rocm_core::diagnose::vllm_*_symptom_is_diagnosable and the lib.rs re-export resolve to the same functions. The helper they call, scores_on_every_engine_host, stays in engine.rs, widened to pub(super). The three tests that exercise the wrappers (the two canonical-symptom tests and the engine-host guard) follow them into the root test module, since their subject is the wrappers and the registry they run. No behaviour, text or score changes. Signed-off-by: Jussi Elo <jussi.elo@amd.com>

no_checker_hand_sets_auto_applicable listed the files that can hold a checker by hand, so a new child module had to be added to the list or its checkers would not be guarded. It now reads the diagnose/ directory, skips only the test-only test_support.rs, and fails if it scanned no child at all. Checked both ways: a hand-set auto_applicable in an existing child and in a new file dropped into diagnose/ each turn the test red; both were removed. Signed-off-by: Jussi Elo <jussi.elo@amd.com>

Copilot

🟢 Approval recommended

No active review issues were identified for the diagnosis-module refactor.

No open issues 🎉

What changed in this PR

Refactors rocm-core diagnosis into smaller platform and support modules, making the failure catalog easier to maintain while preserving public API paths and existing diagnostic behavior.

Changes:

Separates platform checks, scoring, reporting, and catalog dispatch into child modules.
Moves existing tests alongside their implementations and shares test fixtures.
Updates source-scan exclusions, documentation, and comments to reference the new locations.

File	Description
`xtask/src/ascii_output_contract.rs`	Updates the catalog-test location in a comment.
`tests/e2e-cucumber/tests/e2e/diagnose_steps.rs`	Repoints a comment to Linux checker tests.
`tests/e2e-cucumber/README.md`	Links to the reorganized diagnosis tests.
`tests/e2e-cucumber/features/examine.feature`	Updates a catalog-location comment; scenarios unchanged.
`tests/e2e-cucumber/features/diagnose.feature`	Updates test-location comments; scenarios unchanged.
`docs/testing.md`	Repoints references to relocated tests.
`docs/architecture.md`	Documents the diagnosis module split and responsibilities.
`crates/rocm-core/src/fix.rs`	Updates comments and assertion messages naming moved code.
`crates/rocm-core/src/examine/library_copies.rs`	Updates the conflict-checker location in documentation.
`crates/rocm-core/src/examine.rs`	Updates checker references in field documentation.
`crates/rocm-core/src/diagnose/wsl.rs`	Houses WSL checks and their existing tests.
`crates/rocm-core/src/diagnose/windows.rs`	Houses Windows checks and their existing tests.
`crates/rocm-core/src/diagnose/test_support.rs`	Shares examination fixtures across diagnosis tests.
`crates/rocm-core/src/diagnose/scoring.rs`	Houses keyword tables, scoring helpers, and tests.
`crates/rocm-core/src/diagnose/report.rs`	Houses report rendering, upstream routing, and tests.
`crates/rocm-core/src/diagnose/catalog.rs`	Houses checker registration, platform dispatch, and catalog tests.
`apps/rocm/src/model_fit.rs`	Updates WSL checker references in comments.
`apps/rocm/src/advised_commands.rs`	Re-keys source-scan exclusions to relocated strings.
`AGENTS.md`	Updates links to the authoritative diagnosis catalog.

_{Review effort: Balanced}

The guard read only the files directly in diagnose/, so a checker in a future nested module such as diagnose/linux/repos.rs could hand-set auto_applicable unnoticed. It now walks the directory recursively, still skipping only the test-only test_support.rs and still failing if it scanned no child. A hand-set flag in a scratch file at diagnose/linux/zz_scratch.rs turns it red; the file was removed. The module comment at the top of diagnose.rs now also lists the two vLLM symptom classifiers that moved into the root. Signed-off-by: Jussi Elo <jussi.elo@amd.com>

Copilot

🟢 Approval recommended

No active review issues have been identified for the diagnosis-module reorganization.

No open issues 🎉

_{Review effort: Balanced}

juhovainio

I reviewed the full diff. A mechanical comparison of the removed diagnose.rs lines against the added diagnose/*.rs lines shows a pure move: the only differences are module/re-export plumbing, comment-only edits, rustfmt reflow of three signatures, and the adapted guard test. I found no logic, keyword, score, message or CHECKERS change.

The guard test now walks diagnose/ recursively and asserts at least one child was scanned, so it can't pass vacuously. The five advised_commands.rs pins are re-keyed correctly, and the docs and AGENTS.md section 7 are repointed. CI is green.

Non-blocking notes:

The two vllm_*_symptom_is_diagnosable classifiers stay in the root rather than engine.rs as the ticket's seam table says. That avoids an engine/catalog cycle and is documented, but it's worth noting on the ticket.
The root diagnose.rs is ~235 production lines vs the ticket's ~200.
The PR body says 175 tests (174 on Linux), but the diff shows 171 #[test] removed and 171 added; worth reconciling the number in the body.
The ticket asks for one PR per cluster; this is one PR of 8 commits, which is fine given the body says to read commit by commit.

I did not build locally; CI is the build evidence.

Resolves the conflicts with the rocm-core fix/automation/net/openmpi split. fix.rs takes main's version, with this branch's comment re-pointers applied to fix/catalog.rs. diagnose.rs keeps this branch's slim module root, with main's fix.rs to fix/catalog.rs and fix::runners comment re-pointers applied to the diagnose/ children. AGENTS.md, docs/architecture.md and diagnose_steps.rs combine both sides. Signed-off-by: Jussi Elo <jussi.elo@amd.com>

Second sync with main; it merges cleanly, no conflicts. Signed-off-by: Jussi Elo <jussi.elo@amd.com>

Third sync with main; it merges cleanly, no conflicts. Signed-off-by: Jussi Elo <jussi.elo@amd.com>

jussielo-amd · 2026-11-05T14:30:57Z

Merged main into this branch (5fabf85, 0f95905, 0bebf52: plain merge commits, no rebase or force-push) because main moved under it, mainly the fix.rs split into fix/ (#886). The one real conflict was fix.rs: I took main's version and re-applied this PR's 21 comment-only re-pointers in fix/catalog.rs. In the other direction, main's six comment-only edits to the old diagnose.rs were re-applied in the diagnose/ children (linux, windows, catalog, host), and AGENTS.md, diagnose_steps.rs and docs/architecture.md combine both sides. Nothing in the moved code changed. cargo test -p rocm-core --lib passes, the leaf test names under diagnose are identical to main's (175 test functions, 174 on Linux), and cargo xtask catalog --check, manifest --check, check-crate-edges and the advised_commands tests pass.

On your non-blocking notes: the PR body now reconciles the test count, and the two vLLM symptom classifiers staying in the root instead of engine.rs (to avoid a catalog/engine cycle) is recorded on ROCMAI-813.

jussielo-amd added 11 commits November 4, 2026 18:20

jussielo-amd requested a balanced review from Copilot November 4, 2026 19:54

Copilot started reviewing on behalf of jussielo-amd November 4, 2026 19:55

View session

Copilot AI reviewed Nov 4, 2026

View reviewed changes

jussielo-amd requested a balanced review from Copilot November 4, 2026 20:12

Copilot started reviewing on behalf of jussielo-amd November 4, 2026 20:12

View session

Copilot AI reviewed Nov 4, 2026

View reviewed changes

jussielo-amd marked this pull request as ready for review November 4, 2026 20:16

jussielo-amd requested a review from a team as a code owner November 4, 2026 20:16

jussielo-amd requested a review from siloteemu November 4, 2026 20:16

juhovainio approved these changes Nov 5, 2026

View reviewed changes

jussielo-amd added this pull request to the merge queue Nov 5, 2026

github-merge-queue Bot removed this pull request from the merge queue due to a conflict with the base branch Nov 5, 2026

jussielo-amd added 3 commits November 5, 2026 14:17

Merge origin/main into refactor-rocm-core-split-diagnose

0f95905

Second sync with main; it merges cleanly, no conflicts. Signed-off-by: Jussi Elo <jussi.elo@amd.com>

Merge origin/main into refactor-rocm-core-split-diagnose

0bebf52

Third sync with main; it merges cleanly, no conflicts. Signed-off-by: Jussi Elo <jussi.elo@amd.com>

jussielo-amd added this pull request to the merge queue Nov 5, 2026

Merged via the queue into ROCm:main with commit 187fd64 Nov 5, 2026
30 checks passed

jussielo-amd deleted the refactor-rocm-core-split-diagnose branch November 5, 2026 17:51

jussielo-amd mentioned this pull request Nov 5, 2026

refactor(rocm): move the clap command tree into cli.rs (proposal) #889

Merged

Navigation Menu

Repository navigation

ROCMAI-813: split rocm-core diagnose.rs by platform family and extract the checker catalog - #890

ROCMAI-813: split rocm-core diagnose.rs by platform family and extract the checker catalog#890
jussielo-amd merged 15 commits into
ROCm:mainfrom
jussielo-amd:refactor-rocm-core-split-diagnose

jussielo-amd commented Nov 4, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

juhovainio left a comment

Uh oh!

jussielo-amd commented Nov 5, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

jussielo-amd commented Nov 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

What moved

Verification

User-visible behaviour

Notes for reviewers

Dependencies

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

juhovainio left a comment

Choose a reason for hiding this comment

Uh oh!

jussielo-amd commented Nov 5, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jussielo-amd commented Nov 4, 2026 •

edited

Loading