[jussielo-amd]

Summary

Phase 6f-8 of the main.rs modularization (epic ROCMAI-27) — the last sub-phase before the file's shared mod tests block. Mechanical relocation, no behavior change: moves engine/runtime selection and HTTP readiness-polling logic out of apps/rocm/src/main.rs into a new apps/rocm/src/engine_runtime_select.rs.

• resolve_engine_selection/validate_engine_selection_runtime (plus struct EngineSelection, their return type, not named in the ticket but part of the cluster), single_ready_runtime_key, resolve_runtime_selector_to_exact_key, optional_arg, engine_recipe_json_arg/parse_engine_recipe_json_arg, app_path_env_vars/app_path_env_var_values/app_path_env_var_refs, daemon_run_argv, managed_service_launcher_path, apply_app_path_env, wait_for_service_http_ready(_with_progress), service_http_readiness_paths, http_get_local_service/http_post_local_service_json, service_http_readiness_response_ready, lemonade_health_ready_for_model, model_list_ready_for_model, service_model_gpu_ready/service_model_reports_rocm_backend/service_model_names_match moved.
• const fn status_for_readiness sits physically inside the span but has external callers (service_launch.rs, main.rs) and stayed behind, same treatment as earlier PRs' device_policy_name/ROLLBACK_RECOVERY_HINT.
• treat_as_natural_language, immediately following this cluster, is confirmed out of scope and untouched — dispatch-adjacent routing logic from ROCMAI-87, staying in main.rs permanently.
• Real external callers fixed across chat.rs, engine_ipc.rs, engines_cmd.rs, serve_cmd.rs, service_launch.rs, and runtimes_cmd.rs's own test module.
• dispatch() confirmed byte-identical (independently re-verified by SHA-256).
• 11 tests partitioned into the new module's own test block; total workspace test count unchanged.
• docs/architecture.md updated with this module's entry.

Test plan

• cargo build --workspace
• cargo clippy --workspace --all-targets -- -D warnings
• cargo fmt --all -- --check
• cargo xtask manifest --check
• cargo xtask check-crate-edges
• python scripts/smoke_local.py --profile debug --skip-build
• cargo test --workspace --no-fail-fast — 4322 passed, 0 failed, 17 ignored (independently re-run, matches baseline exactly)
• dispatch() call sites confirmed byte-identical (SHA-256 of the function body)

Depends on #752 (ROCMAI-210) — sixth PR in the ROCMAI-86 (Phase 6f) stack.
Part of Jira ROCMAI-215 / epic ROCMAI-27.

🤖 Generated with Claude Code

[Copilot]

🔵 Needs a closer look

Integration with prerequisite #752 remains unverified, and build/test results were not independently validated in this review environment.

No open issues 🎉

What changed in this PR

Continues the main.rs modularization by moving engine/runtime selection and HTTP readiness polling into a dedicated module, with no intended behavior change.

Changes:

• Relocates selection, managed-child environment, argument-building, and readiness helpers into engine_runtime_select.rs.
• Updates caller imports and moves 11 existing tests alongside the extracted code.
• Documents the module’s responsibilities and shared crate-root helpers.

File	Description
docs/​architecture.md	Documents the extracted module and its callers.
apps/​rocm/​src/​service_launch.rs	Updates launch-helper imports, preserving platform gates.
apps/​rocm/​src/​serve_cmd.rs	Imports runtime selection from the new module.
apps/​rocm/​src/​runtimes_cmd.rs	Updates the runtime-selector test import.
apps/​rocm/​src/​main.rs	Registers the module and removes relocated helpers and tests.
apps/​rocm/​src/​engines_cmd.rs	Updates selection and environment-helper imports.
apps/​rocm/​src/​engine_runtime_select.rs	Houses extracted helpers and 11 existing tests.
apps/​rocm/​src/​engine_ipc.rs	Updates helper imports and module documentation.
apps/​rocm/​src/​chat.rs	Updates environment and model-name helper imports.

_{Review effort: Balanced}

[juhovainio]

Looks good. This is a clean extraction. I compared the code removed from main.rs against what was added in engine_runtime_select.rs: the function bodies are identical apart from the pub(crate) visibility, rewrapped imports, and fs:: becoming std::fs:: in two moved tests. No logic was added or dropped, the 11 tests moved over one-for-one, the Clap enums and dispatch() are untouched, docs/architecture.md has the new module entry, and the scope matches ROCMAI-215. This is based on reading the diff; I did not build it locally and am relying on CI for compile and Windows cfg coverage.

Two optional nits, neither blocking:

• The module doc comment and the architecture.md bullet repeat the same long lists of pub(crate) callers. These will drift as callers change. It matches what sibling modules like engine_ipc.rs do, so fine to leave, or trim one of the two.
• The comment in main.rs says the moved code reaches select_runtime_manifest via its own use crate::runtimes_cmd::select_runtime_manifest;, but the new file uses a grouped use crate::runtimes_cmd::{...}. Wording only.

CI note: the two Strix Halo Ubuntu E2E jobs show as cancelled at the run step, which looks like runner contention rather than a regression; the MI300X and W7900 E2E jobs were still pending when I checked.