docs: copyedit README, CONTRIBUTING, and vLLM; add uninstall and verify guidance by pmoutsias-amd · Pull Request #616 · ROCm/rocm-cli

pmoutsias-amd · 2026-10-09T02:39:16Z

Summary

This PR has eighteen content commits plus five merges of main, described as eighteen numbered items below. Commits 2 to 4 are copyedits and add no new facts. The others add or correct facts, so they are described separately and can be reviewed apart from the wording changes. The merges of main resolve conflicts in README.md and docs/vllm.md; see the last section.

1. Call out supported ROCm versions (7.14, 10.0, 10.1)

States that ROCm CLI supports ROCm 7.14, 10.0, and 10.1, and that rocm install sdk currently installs ROCm 7.14 when --version is omitted. The statement appears in the ROCm installation intro and in Getting started (commits 13 and 14 reword it from "default 7.14").
Explains that ROCm 10.0 or newer needs a --version pin, and may need the exact GPU arch as a raw gfx code through --family, with an example (--version 10.1.0 --family gfx1200). Commit 16 softens this: --family is needed only when the CLI can't detect the GPU arch, and the error names the command to re-run.
Moves the "ROCm 10 and newer" section from the end of "ROCm installation" to just after "Install location". The new intro sentence points readers to that section, so it now sits next to the other install sdk subsections (--devel, approval prompt, install location) and ahead of the unrelated "Driver installation" and "Updates" sections. This is the only block move in the PR, and it belongs with the new pointer sentence: if the pointer goes, the move should go with it.
Updates the getting-started.md include anchor to match the reworded README sentence.

2. Copyedit README, CONTRIBUTING, and vLLM guide

Copyedit-only pass on README.md, CONTRIBUTING.md, and docs/vllm.md, which are single-sourced into the docs site.
Style and consistency changes: removed em dashes and spaced double hyphens, "instead of" and "through" in place of "rather than" and "via", replaced and/or and word slashes, spelled out TUI on first use in each standalone doc, American spellings, split long paragraphs.
Doc links in README.md and CONTRIBUTING.md now use absolute GitHub URLs so they resolve on the docs site.
"WSL" is now "WSL2" in the README heading and live-mode note, matching the platform table.
No new facts, no restructuring, no docs-site wiring changes.

3. Tighten README flag lists and add lead-ins

Keeps the rocm diagnose flag list tight (one paragraph per bullet) by moving the long --report and --send detail below the list.
Adds a colon lead-in to the diagnose, fix, dash, and bench load lists.

4. Rejoin prune lock sentence and complete acceptance-script list

Rejoins "The same lock runs in the other direction..." to the preceding services prune paragraph, so its reference to the wait is not left dangling.
Moves the two behaviors that sat outside the vLLM acceptance-script list ("rejects external vLLM command overrides", "does not allow CPU fallback") into the list.

5. Add install verification and `rocm uninstall` guidance (new content)

Adds a "Verify the installation" section after the install commands: run rocm version, which prints the CLI version (release tag or branch and commit hash), the ROCm SDK this machine would use, and the GPU driver version, or not detected for the last two when none is found.
Documents rocm uninstall under "Logs and cleanup": what --dry-run, --yes, and each --keep-* flag do (commit 13 adds the default-layout caveat for the --keep-* flags), and a new --force-dev-binaries entry in the usage block.
Notes what uninstall warns about before it removes anything: shared caches it leaves alone, managed service records (their processes are not stopped), and remote sessions (run rocm remote stop <session> first).
Statements were checked against the CLI source in apps/rocm/src/main.rs (version(), build_uninstall_plan(), and the Uninstall flag definitions), not copied from the earlier draft; commit 13 corrects one flag claim that this check missed, which said rocm version showed only the CLI version and that uninstall warned about services "still running".
Needs a content reviewer to confirm. No site wiring: the new verify section is picked up by the existing include on the installation page.

6. Clarify verify and uninstall guidance (new content)

Verify section: says what a successful rocm version run looks like, that not detected for the SDK and driver is expected before ROCm or a driver is installed, and what to check if rocm is not found.
Uninstall: names the four categories it removes (binaries, configuration, data, caches) and lists the managed ROCm SDK installs under --keep-data, since they live in the data directory.
Uninstall warnings: split into a list (managed service records, remote sessions) and a sentence naming the shared caches it leaves in place (the uv package cache and the Hugging Face model cache; commit 12 refines this). Checked against build_uninstall_plan() and shared_cache_candidates().

7. Link install pages to uninstall guidance

Adds one sentence in the README "ROCm installation" section and one on the site install page (docs/rocm-docs/install/installation.md) pointing to "Logs and cleanup", so readers can find how to remove ROCm CLI. The site-page sentence is first placed directly under Build from source; commit 8 moves it.

8. Give the install page uninstall pointer its own heading

On the site install page, moves the uninstall pointer under a new ## Uninstall ROCm CLI heading so it doesn't read as part of Build from source, and keeps the existing Contributing sentence in its original place. This and commit 7 are the only changes in this PR to a site wrapper; neither adds include anchors or alters existing ones.

9. Fix command synopses and document `rocm model`, `rocm chat --provider`, and automation modes (new content)

Synopses: adds --send (requires --report) to rocm diagnose and the positional [QUERY ...] to rocm logs.
Adds a short "Curated models" section for rocm model [--verbose] (alias rocm models), which the "Will a model run here?" text already referred to, and splits that text's undetermined conditions into a list.
Chat: replaces the anthropic|openai|... placeholder with local|openai|anthropic, and documents --provider setup (rocm config enable-provider and set-provider-key) and --tools.
Automations: describes observe, propose, and contained. In contained mode only server-recover acts on its own (restarts a failed managed service); the others stay review-gated or record only, per crates/rocm-core and apps/rocmd/src/watchers.rs. Per-watcher default modes are left out to avoid a table that can drift.
All checked against apps/rocm/src/main.rs.

10. Clarify that `rocm chat --provider` applies only to one-prompt use

In an interactive terminal, rocm chat without --prompt opens the dashboard chat, where you switch providers with /provider <name>. With --prompt or piped input, it sends one prompt and prints the reply.
Notes that --provider is ignored by the dashboard chat, and that local is the default.
Checked against the Command::Chat handler in apps/rocm/src/main.rs.

11. Correct contained-mode, chat flag, and model verbose descriptions

Chat: extends the note to --model and --tools, which the dashboard chat also ignores, and states it once for all three flags.
Automations: corrects the contained description, which overstated that every watcher applies changes on its own. propose now says it applies to watchers that can make a change.
rocm model --verbose: describes what it prints (every recipe, including hidden ones, with aliases, data type, minimum GPU memory, and engines) instead of repeating the flag's help text.
Checked against the Command::Chat handler in apps/rocm/src/main.rs, and against the watcher code in crates/rocm-core and apps/rocmd/src/watchers.rs.

12. Clarify uninstall cache and data scope, add APU case to diagnose

Uninstall: separates the managed uv cache (under the data directory, controlled by --keep-data) from the shared one (UV_CACHE_DIR or ~/.cache/uv, left in place and reported). Lists downloaded models under --keep-data, since they live in the data directory and uninstall removes them.
Diagnose: the undetermined conditions now include the APU case, where the CLI can't yet read the memory pool the engine uses. Checked against UndeterminedReason in model_readiness.rs.
Audit of the rest of the new text against source found no other discrepancies.

13. Address automated review: uninstall default layout, ROCm versions, synopses

Uninstall: on the default layout the config and data directories are both ~/.rocm and the cache is ~/.rocm/cache, so a single --keep-* flag doesn't protect its files. The intro now says each flag "skips the removal of one category", and a new paragraph says to pass --keep-config and --keep-data together and to run --dry-run first. docs/engine-plugins.md gets the same caveat. The earlier "checked against build_uninstall_plan()" claim had verified each flag in isolation, not the overlapping default layout. The behavior itself is tracked in uninstall: --keep-* flags are ignored when config, data, and cache directories overlap (default layout) #626.
ROCm versions: reworded as a support statement ("supports ROCm 7.14, 10.0, and 10.1") instead of a hardcoded default. Source has no allow-list or default version. Commit 14 replaces the "latest stable release, currently 7.14" wording, which was wrong because 10.1 is the latest stable release.
Disk space: the "never removed" claim is narrowed to rocm storage, because rocm uninstall does remove downloaded models along with the data directory (see the Storage section for the --keep-* caveat).
Synopses: rocm diagnose [--symptom TEXT] [--top N] [--json] [--distro [NAME] | --report [--send]] and rocm storage [report [--json]]. The undetermined list now points to rocm model --verbose.
getting-started.md: resyncs the hand-copied sentence with the README.

14. Avoid calling 7.14 the latest stable release

ROCm 10.1 is the latest stable release, so the README no longer describes the unpinned result as the "latest stable release". It now says rocm install sdk "currently installs ROCm 7.14" without --version, and that 10.0 or 10.1 need --version and --family.

15. Address review comments on getting started, uninstall, and vLLM

Getting started: "Configure ROCm and serve a model" now starts with rocm examine and the active_runtime_status: ready check, and runs rocm install sdk only to install a runtime or get a different version or configuration.
Installation page: the uninstall pointer now shows a rocm uninstall example (with --dry-run) and points to "Logs and cleanup".
vLLM guide: reworded the torch-release sentence and split a long sentence.
Re-edits the getting-started.md include anchor to match the README.
Not in this PR: the WSL page wiring (tracked for a future PR) and the --keep-* help-text mismatch (tracked in uninstall: --keep-* flags are ignored when config, data, and cache directories overlap (default layout) #626).

16. Correct stale install, serve, and cleanup guidance

README: the rocm: command not found advice now covers Windows as well as Linux (~/.local/bin, %USERPROFILE%\.local\bin, or ROCM_CLI_INSTALL_DIR), checked against install.sh and install.ps1.
README synopses: rocm fix gains --json (valid only without a fix id), rocm engines install gains --yes, and rocm serve gains --require-api-key, --api-key, --gpu-memory-utilization, and --tool-call-parser, with short explanations. Prefer ROCM_SERVE_API_KEY over --api-key.
docs/vllm.md: the pin-table sentence is superseded in commit 17.
--help text in apps/rocm/src/main.rs: rocm chat and rocm diagnose --send now describe current behavior.
The only Rust change is --help doc-comment text. It was not compiled or run (no Rust toolchain on the authoring machine), so CI is the first check.

17. Align vLLM, uninstall, and help wording with the code

docs/vllm.md: the pin-table sentence now reads "For every ROCm SDK version outside a discovery major (today, everything but 10.x)". Any version without a matching row falls back to the table's default row, so "7.x" was too narrow and contradicted the later "Every other ROCm SDK version" line.
MANIFEST.md: the uv-cache note no longer says --keep-data alone keeps it. On the default layout, keeping it takes --keep-data and --keep-config together.
--send help in apps/rocm/src/main.rs: "read a form" is now "read and send a mail".
Reverts the apps/rocm/src/storage.rs wording change and its loosened test, so this PR stays docs-only. The rocm storage output wording ("never removed by ROCm CLI" is not accurate, because rocm uninstall removes downloaded models with the data directory) is not changed here and needs a separate change with a full-string test.

18. Tie the serve step to the install it follows

README "Configure ROCm and serve a model" (single-sourced into Getting started): "Then serve a model:" followed a paragraph about the activation gate, so "Then" had no clear referent. It now reads "After the install finishes, serve a model:".

Merges of `main`

The merge of main conflicted in the README "Logs and cleanup" section. main had added two paragraphs after the rocm uninstall synopsis (symlinked config, data, and cache directories on Linux; the error when uninstall cannot check whether a directory exists). Both are now carried into the four-category uninstall section.
The latest merge of main (after fix(therock): pin torchaudio to the requested ROCm 10 build #630, fix(uninstall): undo the installer's PATH change and remove the running exe #636, and ROCMAI-88: partition cluster tests out of main.rs, update docs (3/3) #668) conflicted in README.md and docs/vllm.md. Every conflict is resolved by keeping what main now says and applying this PR's copyedit rules to it:
- README Ctrl-C and quit-confirmation paragraph, the DIAGNOSE-ONLY marker text, and the rocm chat ChatGPT OAuth fallback note, each carried over from main in this PR's wording.
- README rocm uninstall: the four-category intro stays, and main's rocm.exe removal and PATH undo paragraph (fix(uninstall): undo the installer's PATH change and remove the running exe #636) follows it.
- README "ROCm 10 and newer" (moved earlier in this PR): the torchaudio paragraph is main's text from fix(therock): pin torchaudio to the requested ROCm 10 build #630, copyedited, in place of a version this PR wrote before fix(therock): pin torchaudio to the requested ROCm 10 build #630 merged. The --family sentence now says the error names the command to re-run only when a GPU family is detected without an exact arch, and that you pass the raw gfx code yourself when no GPU is detected.
- docs/vllm.md: the "One value flows the other way" paragraph is main's text from fix(therock): pin torchaudio to the requested ROCm 10 build #630, copyedited and split.
All 18 {include} directives in docs/rocm-docs/ were re-checked against the merged files.
Every commit now carries a Signed-off-by trailer, and none carries a Co-Authored-By trailer.
An earlier merge also corrects two claims the reviews flagged: the Storage section now says keeping downloaded models on a default installation takes both --keep-data and --keep-config, and --force-dev-binaries is described as removing the ROCm CLI binaries in that build directory.

Dependencies on code changes

Uninstall removing the running rocm.exe on Windows and undoing the installer's PATH change is a CLI fix in fix(uninstall): undo the installer's PATH change and remove the running exe #636, which has merged, so this PR doesn't document a manual workaround.
The ROCm 10.0 --version resolution (torchaudio pinned to the requested build) is fixed in fix(therock): pin torchaudio to the requested ROCm 10 build #630, which has merged. The README and vLLM guide text about the torchaudio pin is fix(therock): pin torchaudio to the requested ROCm 10 build #630's own wording, copyedited.

Test plan

All 18 {include} directives in docs/rocm-docs/ still find their :start-after: and :end-before: strings, in order, against the edited files.
Code fences and tables are balanced, intra-doc anchors resolve, and no flags, numbers, or versions were added or removed by the copyedit commits (commits 2 to 4).
All commits are signed and carry a DCO Signed-off-by trailer.
New technical claims in commits 5, 6, 9, 10, 11, and 16 were checked by reading the CLI source. The binary was not built or run.
The docs example rocm install sdk --version 10.1.0 --family gfx1200 --dry-run resolves on WSL2 (run 2026-10-09): ROCm 10.1.0, torch 2.14.0, torchvision 0.28.0, and torchaudio 2.11.0.3, all +rocm10.1.0, with no error. A review dry-run the same day showed an unpinned install picking 7.14.1. The dry-run on WSL2 did not use a gfx1200 GPU. A review dry-run of --version 10.0.0 --family gfx1200 failed on a torchaudio mismatch (it selected the 10.1.0 build); fix(therock): pin torchaudio to the requested ROCm 10 build #630 fixes that resolution.
Content reviewer confirms three claims: rocm uninstall warns about remote sessions, contained mode acts on its own only for server-recover, and the dashboard chat ignores --model and --tools.
docs-build passes in CI.
Spot-check the rendered getting-started, installation, and vLLM pages.

State the default ROCm version and the supported versions in the ROCm installation intro and Getting started, and move the "ROCm 10 and newer" section ahead of driver installation. Explain that ROCm 10 or newer needs a `--version` pin and the exact GPU arch as a raw gfx code via `--family`. Signed-off-by: pmoutsias-amd <peter.moutsias@amd.com>

Editorial pass for style and consistency: remove em dashes and spaced double hyphens, use "instead of" and "through", replace and/or and word slashes, spell out TUI on first use, use American spellings, split long paragraphs, and use absolute GitHub URLs for doc links in CONTRIBUTING.md and README.md so they resolve on the docs site. No changes to technical content or to the anchor strings the docs-site include directives depend on. Signed-off-by: pmoutsias-amd <peter.moutsias@amd.com>

Keep the rocm diagnose flag list tight by moving the long --report and --send detail below it, add colon lead-ins to the flag lists that had none, and clarify the --family sentence. Signed-off-by: pmoutsias-amd <peter.moutsias@amd.com>

Signed-off-by: pmoutsias-amd <peter.moutsias@amd.com>

Add expected output and a PATH fallback to the verify section. Name the four uninstall categories, list managed ROCm SDK installs under --keep-data, and split the uninstall warnings into a list that also names the shared caches left in place. Signed-off-by: pmoutsias-amd <peter.moutsias@amd.com>

Point the ROCm installation section and the site install page at Logs and cleanup, so readers who finish installing can find how to remove ROCm CLI. Signed-off-by: pmoutsias-amd <peter.moutsias@amd.com>

Signed-off-by: pmoutsias-amd <peter.moutsias@amd.com>

…omation modes Signed-off-by: pmoutsias-amd <peter.moutsias@amd.com>

Signed-off-by: pmoutsias-amd <peter.moutsias@amd.com>

Distinguish the managed uv cache from the shared one, list downloaded models under --keep-data, and include the APU case in the undetermined verdict conditions. Signed-off-by: pmoutsias-amd <peter.moutsias@amd.com>

rominf

🔴 Automated review · pr-review-watcher · cf49378

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.
No ticket named. The description lists twelve commits and covers what the diff does. One of its claims is wrong: it says the uninstall text was "checked against build_uninstall_plan()", and the first blocking finding below shows it was not.

This is the first automated round on this PR, so every finding below is new.

Blocking

(new) On a default install, the new rocm uninstall text promises that each --keep-* flag keeps its category. It does not. — README.md:924-934; same claim at docs/engine-plugins.md:98; root cause apps/rocm/src/main.rs:21012 (build_uninstall_plan) with crates/rocm-core/src/runtime.rs:215-227
- By default the config dir and the data dir are both ~/.rocm, and the cache dir is ~/.rocm/cache. The plan deletes each category's directory unless that category's flag is passed.
- I ran rocm uninstall --dry-run --keep-binaries with a throwaway HOME:
  - --keep-data still plans config: ~/.rocm, which wipes the models, managed SDK installs and uv-cache the README says it keeps.
  - --keep-config still plans data: ~/.rocm, which contains the config.
  - --keep-cache still plans config: ~/.rocm, which contains cache/.
- "Each --keep-* flag leaves one category in place" is therefore false on the default layout. A user who trusts --keep-data loses every managed runtime and downloaded model.
- The code decides what actually happens, so the text is the side that is wrong today. The bug underneath is in the code.
- Confidence 100 · logic · Fix: either make the plan honour a kept category when directories overlap (skip deleting a directory that contains or equals a kept one), or document that on the default layout these flags only work in combination. The code fix is the better one.
(new) The new ROCm version claims state a moving value as fixed and give a list of installable versions that is wrong today. — README.md:228, README.md:395 (README.md:228 is also included into docs/rocm-docs/getting-started.md)
- The source has no allow-list of versions. Without --version, therock.rs installs whatever the release index currently lists as latest.
- I ran dry-runs against the live public index:
  - install sdk --family gfx120X-all --version 7.13.0 --dry-run resolves 7.13.0, which "It can install ROCm 7.14, 10.0, or 10.1" excludes.
  - --version 10.0.0 --family gfx1200 --dry-run fails ("selected torchaudio 2.11.0.3+rocm10.1.0, which does not share ROCm build 10.0.0").
  - The default today picks 7.14.1.
- "By default … installs ROCm 7.14" becomes wrong at the next release, and install sdk --help says nothing about a default version.
- Confidence 85 (run-confirmed, but it depends on what the index serves today) · logic · Fix: say "the latest stable release (currently 7.14)". If 7.14, 10.0 and 10.1 are a support statement, call them "supported" rather than "can install", and recheck that 10.0 actually works before listing it.

Non-blocking

(new) The README now contradicts itself about downloaded models. — README.md:557-562 against README.md:931-932
- The new uninstall text says --keep-data keeps downloaded models, which is correct: they sit under <data>/models, and uninstall deletes data_dir.
- The reworded Disk space paragraph, and rocm storage's "Shared with other tools (never removed by ROCm CLI)" heading (apps/rocm/src/storage.rs:508-514), still say they are never removed.
- The code decides this, so the Disk space sentence and the storage label are the ones to fix. That claim was there before this PR.
- Confidence 88 · mechanical · Fix: limit "never removed" to the uv and Hugging Face caches, or to rocm storage's commands.
(new) Two --help texts now contradict the README this PR corrected (AGENTS.md §5: check the same claim on every surface). — apps/rocm/src/main.rs:299-302, apps/rocm/src/main.rs:185-195
- rocm chat --help still says it "Reads the prompt from --prompt or, if omitted, from standard input". The README now correctly says an interactive run without --prompt opens the dashboard chat (main.rs:2226).
- rocm diagnose --help describes --send as a "prefilled issue form" that "reaches the tracker". The README correctly describes a prefilled mail (report_delivery.rs).
- Both were stale before this PR.
- Confidence 85 · mechanical · Fix: update both doc comments to match the README.
(new) The pointer from diagnose --model's undetermined list to rocm model is incomplete. — README.md:594-595
- The new Curated models section (README.md:607-610) says the default list hides some recipes, and only --verbose shows them.
- assess_model_readiness matches against every recipe, hidden ones included.
- Confidence 75 · mechanical · Fix: point at rocm model --verbose.
(new) Synopsis errors that predate this PR survive a commit titled "fix command synopses". All confirmed against the built binary's --help or by a run.
- README.md:533, rocm storage [report] [--json]: rocm storage --json fails with "unexpected argument '--json'". It should read rocm storage [report [--json]].
- README.md:298: the edited [--distro [NAME]] [--report [--send]] implies the two can be combined, but the CLI rejects --report with --distro.
- README.md:617-626: the rocm serve synopsis omits --gpu-memory-utilization, --tool-call-parser, --api-key and --require-api-key.
- README.md:568: the rocm engines install synopsis omits --yes.
- Confidence 100 · mechanical · Fix: correct the synopses, or narrow what the commit claims to fix.
(new) The hand-copied sentence on the docs site was not updated with its README original. — docs/rocm-docs/getting-started.md:35-37
- The README now reads "GGUF versus safetensors rule, because which…", but the copy still says "GGUF-vs-safetensors rule, since which…".
- The include anchor is unaffected, so this is wording drift only.
- Confidence 85 · mechanical · Fix: copy the new wording over.

Decisions for the author

Absolute blob/main links instead of relative ones — tradeoff
- In favour: CONTRIBUTING.md and the README "More docs" list are pulled into the Sphinx site, where relative docs/… links would break. The files already use this pattern.
- Against: those links are no longer checked by the offline docs-links (lychee) job. On a versioned docs site they always point at main, not the release being read.

Positive signals

Every {include} slice in docs/rocm-docs/ still resolves to a unique, intended span. The only changes against the base are the intended content. A local sphinx-build -W passed, and CI's Sphinx and lychee jobs are green.
The new undetermined bullets, including the APU case, match UndeterminedReason::UnifiedMemoryUnreadable in model_readiness.rs exactly.
The rocm chat text now describes the code's real split between the interactive dashboard chat and the one-prompt form (main.rs:2226).
The docs/vllm.md restructure keeps every qualifier and condition. Each technical claim in it was confirmed against engines/vllm/src/install.rs, and the (0, 1] range check was confirmed by a run.

Deployment notes

None

What this covered

Read: every changed file in full, against 2c37f5bc…cf49378a (base merge-base 7f72556d → head), using the PR's three-dot diff. That is README.md, CONTRIBUTING.md, docs/vllm.md, docs/rocm-docs/getting-started.md and docs/rocm-docs/install/installation.md.
Also read: every docs/rocm-docs include wrapper, and the CLI source behind each new claim (main.rs, therock.rs, storage.rs, model_readiness.rs, runtime.rs, apps/rocmd/src/watchers.rs, engines/vllm/src/install.rs).
Ran: the CLI built from this head, for --help and dry-runs, with throwaway HOME dirs. CI status was read for head cf49378: all checks green, GPU lanes skipped as docs-only. The --keep-data dry-run in the first blocking finding was re-run independently before filing.
Fan-out: five independent workers. Three covered disjoint README ranges plus include integrity, one covered CONTRIBUTING.md and docs/vllm.md, and one ran agent-instruction adherence, history, code comments, prior-PR review comments (#473, #396, #311, #579) and commit quality.
Did not run: the version findings depend on the live package index on 2026-10-09.
Not reconciled against any discussion on this PR.

Note that --keep-* flags don't protect their files by themselves on the default layout, where config and data share ~/.rocm. State the supported ROCm versions and describe the default as the latest stable release. Correct the storage and diagnose synopses, point the undetermined list at rocm model --verbose, narrow the "never removed" claim to rocm storage, and resync the copied getting-started sentence. Signed-off-by: pmoutsias-amd <peter.moutsias@amd.com>

ROCm 10.1 is the latest stable release, so describe the unpinned `rocm install sdk` result as currently ROCm 7.14 and point to the `--version` and `--family` steps for 10.0 and 10.1. Signed-off-by: pmoutsias-amd <peter.moutsias@amd.com>

Resolve CONTRIBUTING.md conflicts with #475 and #602 by keeping main's hawkeye, lychee, module-organization and link-checking text, with mechanical copyedits only (em dashes, Markdown capitalization, repository, code font for tool names). Signed-off-by: pmoutsias-amd <peter.moutsias@amd.com>

randyh62 · 2026-10-09T14:57:24Z

+To remove ROCm CLI and what it manages, see
+[Logs and cleanup](../commands.md#logs-and-cleanup).


Maybe this could simply have a rocm uninstall command example, and then point to Logs and Cleanup?

randyh62 · 2026-10-09T15:53:42Z

This starts with installing a rocm sdk, but shouldn't it start by checking if the needed sdk is available using rocm examine? Then if you need a different version of configuration you would run rocm install sdk?

randyh62 · 2026-10-09T15:54:46Z

+     reused as the `:start-after:` anchor for the next include, which also
+     matches the original sentence in README.md; edit both together. -->
 Running the command when a managed runtime is already the active default asks
 first, because the new install takes over as the active default; see


Suggested change

first, because the new install takes over as the active default; see

Running _rocm install sdk_ when a managed runtime is already active prompts for confirmation first, because the new install takes over as the active default; see

randyh62 · 2026-10-09T16:42:33Z

Suggested change

moved to the SDK's *build* of the torch *release* that the engine pins: the release

randyh62 · 2026-10-09T17:11:06Z


 ## ROCm 10.x wheel discovery

 For most ROCm SDK versions, `rocm engines install vllm` pins a fixed vLLM wheel


Is it most SDK versions or 7.14?

randyh62 · 2026-10-09T17:19:00Z

 host from 10.0's production index. Both rows take their torch stack from the
 same `whl-next` index, which is where `rocm install sdk` resolves either SDK's
 own `+rocmX.Y` torch from, but vLLM's discovery is independent of that: it
-resolves its own torch/torchvision/torchaudio pins fresh from the row's own
-static version prefixes, rather than reusing whatever the SDK install
-resolved, which may be a different torch version than this row pins. Patch
-and any dev/pre-release suffix are still ignored within a row, since AMD
+resolves its own torch, torchvision, and torchaudio pins fresh from the row's
+own static version prefixes, instead of reusing whatever the SDK install
+resolved, which might be a different torch version than this row pins. Patch


Maybe break this sentence into two or three sentences?

rominf

🔴 Automated review · pr-review-watcher · 5ed97f6

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.
No ticket named. The description refers to #626, which tracks the --keep-* default-layout behaviour; it is not a closing claim.

This is the second automated round. Findings are marked new (first raised this round) or standing (carried from the round at cf49378).

Blocking

(standing, part still holds) The README still tells users ROCm 10.0 installs with --version and --family, and that path fails today — README.md:237-240 (included into docs/rocm-docs/getting-started.md), README.md:405-407
- This round's blind review did not re-find it. It was settled separately by putting the objection to the reviewer after its report was returned, so the verdict below is anchored, not blind.
- Answered since cf49378: the list is now worded as a support statement ("supports ROCm 7.14, 10.0, and 10.1"), and the default reads "currently installs ROCm 7.14".
- Still holds: both paragraphs say 10.0 is installed by adding --version and --family. The PR's own test plan records that --version 10.0.0 --family gfx1200 --dry-run failed on a torchaudio mismatch (it selected the +rocm10.1.0 build). The code refuses mixed builds at that point (apps/rocm/src/therock.rs:3496), and nothing in this diff changes resolution. A user who follows the 10.0 instructions gets an error.
- Not re-run this round: no binary built at this head was run against the live index, so whether the index has since gained a matching 10.0 torchaudio is unconfirmed.
- Fix: drop 10.0 from the "install with --version and --family" sentences until the 10.0 path resolves, or confirm on a current dry run that it now does and say so in the test plan.
(new) The new uninstall docs say rocm uninstall removes ROCm CLI's binaries, but on Windows it leaves the running rocm.exe in place. On every platform it also leaves the PATH change the installer made — docs/rocm-docs/install/installation.md:34-43, README.md:937-940
On Windows, build_uninstall_plan skips the running executable: apps/rocm/src/main.rs:17853-17858 ("skipping running executable on Windows: …"). Nothing in the uninstall path reverses the PATH edits made by install.sh (shell profile line) or install.ps1 (user PATH). The installation page says "To remove ROCm CLI and what it manages: rocm uninstall", and the README lists "its binaries" among the four categories removed. Windows is a supported platform, so a Windows user who follows the page still has rocm.exe and a dangling PATH entry, and nothing in the docs tells them. The code is what governs here; the docs describe it incompletely.
Confidence 85 · logic · Fix: add a sentence covering both. On Windows, delete rocm.exe yourself after uninstall finishes. On any platform, remove the PATH line or entry that the installer added.
(new) The PR description misstates the change's scope — PR #616 description
It says "This PR has fourteen commits" and describes items 1–14. The branch has 16 commits. The final one (5ed97f6) adds content the description never mentions:
- "Configure ROCm" now starts with rocm examine and the active_runtime_status: ready check.
- installation.md gains the rocm uninstall / --dry-run example.
- docs/vllm.md:66 gains a new factual claim, "For ROCm SDK 7.x versions, including 7.14".
Readers rely on the description's split between "adds facts" and "copyedit only" to decide what to check, and this content falls outside both lists. With squash merges the description does not land in history, but it is the map used for this review round.
Confidence 100 · mechanical · Fix: update the description to cover the last commit and the merge (narrowing is not an option, because the work is already in the diff).

On the merge gate: this change request is held on the standing 10.0 item above. The two new items are listed as the review rated them, but neither alone would have been filed as a change request: rocm uninstall itself prints the skipped Windows executable under "Left alone", and the description does not land in the squash commit.

Non-blocking

(new) --family is presented as required for ROCm 10.x, but the code only needs an exact arch, and host detection supplies one — README.md:238-240, README.md:405-407
resolve_family returns raw_arch: Some(..) from detect_host_gfx_target() when no --family is passed (apps/rocm/src/therock.rs:3390-3399). select_source_layout bails only when raw_arch.is_none() (therock.rs:314). The advice works but overstates the requirement. This repeats the existing "two things together" wording at README.md:467 in the more prominent getting-started path.
Confidence 85 · logic · Fix: "add --version; pass --family <gfx code> only when rocm examine shows no detected_gfx_target, or when you target a different GPU."
(new) docs/vllm.md:66 "including 7.14" suggests a 7.14-specific pin. There is none: 7.14 falls through to the 7.2.3 default row — docs/vllm.md:66-67
VLLM_ROCM_BUILD_TABLE has a single row, 7.2.3 → vllm 0.26.0 / rocm723 (engines/vllm/src/install.rs:36-40). Every version with no row and no discovery match gets that row (install.rs:1184-1207). The old wording, "most ROCm SDK versions", matched the code better.
Confidence 85 · logic · Fix: restore "most", or state that 7.14 uses the default 0.26.0+rocm723 pin.
(new) The storage section now says rocm uninstall removes "downloaded models". The CLI's own printed note for that row still says "ROCm CLI never removes them", and the README calls the row "shared with other tools" — README.md:570-575, README.md:944-945, apps/rocm/src/storage.rs:508-515
uninstall removes data_dir wholesale (main.rs:17867-17880), so the README's uninstall statement is correct. The printed string is false, and AGENTS.md §5 asks that the same claim agree across README and printed output. Also, the only thing rocm-cli writes under <data>/models is artifact marker JSON (crates/rocm-core/src/lib.rs:3198-3207), while model weights live in the Hugging Face cache, which uninstall leaves alone. So "--keep-data keeps downloaded models" promises more than that directory holds. The printed string predates this PR; the PR documents around it.
Confidence 70 · logic · Fix: describe the row as what it actually holds, and correct the storage.rs note in this or a follow-up change.
(new) The rocm chat --help text still describes only a one-shot, stdin-reading command. The README now correctly documents the interactive dash routing — apps/rocm/src/main.rs:312-316, apps/rocm/src/main.rs:328, against README.md:839-854
The handler opens the dash chat when the terminal is interactive and --prompt is absent (main.rs:2240-2260). AGENTS.md §5 asks that README and --help claims be kept in sync. The help text drift predates this PR.
Confidence 85 · mechanical · Fix: update the Chat doc comment and the --prompt help.
(new) Commit messages that will land in the squash body contain review-process notes — commits 5ed97f6, 633629d, 59b3d9f
5ed97f6 has the subject "address Randy's review comments…", and its body ends with a first-person note to a reviewer: "Review comment #1 (the WSL page) is not in this commit. I removed the --keep-* caveat and the inlined qwen copy on purpose, so the message doesn't mention them." That note refers to changes that are not in the diff. The merge commit's body describes conflict resolution. Squash merges build the commit body from these messages.
Confidence 100 · mechanical · Fix: reword 5ed97f6, or write the squash message by hand at merge time.

Decisions for the author

The hardcoded "supports ROCm 7.14, 10.0, and 10.1" and "currently installs ROCm 7.14" (README.md:237-238, README.md:405-406) — tradeoff
Readers get a concrete answer. Against that: the code has no allow-list or default version. Unpinned selection is "highest stable with a matching torch stack" (therock.rs select_rocm_version / channel_rocm_candidates), so the sentence goes stale silently. It is stated twice, and it does not cover --channel nightly or --format tarball. By the description's own account, the 7.14 figure comes from a dry run, not from source. Confirm it with the CLI owners, or qualify it ("on the default release channel, as of this release").
Absolute github.com/ROCm/rocm-cli/blob/main/... links in README and CONTRIBUTING — tradeoff
AGENTS.md sanctions them for files the docs site includes, and they resolve on the site. However, they always point at main rather than the revision being read, and the offline lychee check no longer validates them.
One PR combines a copyedit pass with substantive new content across 16 commits — non-blocking-improvement
AGENTS.md §11 asks for one logical change per PR. The description tries to separate the two kinds of change, but the content commits would be easier to review and revert on their own.

Positive signals

The new "Verify the installation" section matches version() exactly, including the not detected lines for the SDK and driver.
The --keep-* paragraph warns that on the default layout a single flag protects nothing, which matches build_uninstall_plan. docs/engine-plugins.md carries the same caveat.
The comment in getting-started.md says that the hand-copied sentence and the reused :start-after: anchor must be edited together. The new anchors (Running \rocm install sdk` when a, newer requirements.`) are unique in README.

Deployment notes

None

What this covered

I read every changed file completely: README.md, CONTRIBUTING.md, docs/vllm.md, docs/engine-plugins.md, docs/rocm-docs/getting-started.md and docs/rocm-docs/install/installation.md. I also checked the behavioural claims against the implementing code (clap definitions, therock.rs, uninstall plan, storage.rs, watchers.rs, the vLLM install table) and against the docs-site include directives. The diff reviewed was prw-base...5ed97f69.

Ran: lychee --offline on the changed Markdown (0 errors; the absolute links are excluded from that check). The CI rollup for this head is all green or skipped, including the Sphinx docs build, lychee, the commit-signature and DCO check, and the e2e tests.

Not run, or not run independently:

The prior-changes pass did not run, because review comments on earlier PRs touching these files were not read.
The design-review step (Step 7) ran in the driver context, not as independent workers.
No CLI binary was built or run. Claims were verified by reading the code.
sphinx-build was not run locally; I relied on the CI result.
The 7.14 default could not be confirmed from source, because it depends on the package index.

The four code and source-pass workers were independent, run on a cheaper model tier. This report was not reconciled against the PR discussion.

Standing objections from cf49378, settled after the review by putting each to the reviewer (anchored evidence, not blind findings):

--keep-* on the default layout: cleared. README and docs/engine-plugins.md now say a single flag does not protect its files on the default layout and give the combination that does; that advice matches build_uninstall_plan (traced, not run). The behaviour itself is unchanged and tracked in #626. The --keep-* --help texts (apps/rocm/src/main.rs:619-628), which this PR does not touch, still describe each flag as self-sufficient.
ROCm versions: partly answered; the 10.0 part still holds (above).

rominf · 2026-10-12T09:48:35Z

On the uninstall finding in my review above (rocm.exe left on Windows, PATH change left everywhere): this is a CLI bug, not a docs gap, so please don't add the manual workaround the review suggested. #636 fixes it in code:

rocm uninstall now removes the running rocm.exe on Windows.
It undoes the PATH change the installer recorded making, that is, the marked shell-profile block or the Windows user PATH entry.

Once #636 lands, the "removes its binaries" wording here is accurate. Two cases where the PATH change is intentionally kept, in case you want to mention them:

when other programs share the install folder (often ~/.local/bin), and the review names them;
with --keep-binaries.

siloteemu

🔴 Automated review · pr-review-watcher · 5ed97f6

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 (prw-base...5ed97f69, 15 non-merge commits, 6 documentation files).
The description says the PR is "fourteen commits" and covers ROCm version support, install verification, uninstall guidance, command synopses and chat/automation text. No ticket is named.

Blocking

[new] The branch conflicts with the base in README.md's "Logs and cleanup" section. README.md:930-960
A merge-tree of prw-base and HEAD reports CONFLICT (content): Merge conflict in README.md, and GitHub reports mergeable: CONFLICTING, mergeStateStatus: DIRTY. A change that landed on the base after this branch last merged it added two paragraphs right after the rocm uninstall synopsis. One covers symlinked config, data and cache directories on Linux. The other covers the error when uninstall cannot check whether a directory exists. This PR rewrites exactly that spot. Whoever resolves the conflict must carry those paragraphs into the new four-category structure, or that fix's documentation is lost.
Confidence 100 · logic · Fix: merge the base again and fold the two paragraphs into the new uninstall section.
[new] The storage section says --keep-data alone keeps downloaded models, which the PR's own uninstall caveat says is false. README.md:573-575
New text: "rocm uninstall removes downloaded models along with the data directory unless you pass --keep-data". The PR's uninstall section (README.md:949-953) and its docs/engine-plugins.md:98-101 edit say that on the default layout config and data are both ~/.rocm, so one flag does not protect its files and both flags are needed.
The code governs here, and it agrees with the caveat. build_uninstall_plan (apps/rocm/src/main.rs:17868-17882) queues the config, data and cache directories independently. --keep-data alone still queues config_dir, which is the same directory as data_dir on the default layout, and models live under it. I traced this and did not run it.
Confidence 85 · logic · Fix: add "and --keep-config on the default layout", or link to the caveat instead of restating a shorter rule.
[new] The description does not account for the last commit, and its commit count is wrong. PR description, Summary; docs/rocm-docs/install/installation.md:34-43, README.md:215-256, docs/rocm-docs/getting-started.md, docs/vllm.md:66-67
The description says "fourteen commits" and describes commits 1 to 14. The branch has 15 non-merge commits. The last, 5ed97f6 ("address Randy's review comments on getting started, uninstall, and vLLM"), is not in the description. It changes things the description does not mention:
- the opening of "Configure ROCm and serve a model", which now starts with rocm examine keyed on active_runtime_status: ready;
- a new rocm uninstall example and a --dry-run note on the install page;
- the "7.x, including 7.14" and "torch release" wording in docs/vllm.md;
- a re-edit of the getting-started.md include anchor.
  Unmentioned changes are unreviewed changes, so this mismatch blocks.
  Confidence 85 · mechanical · Fix: add a section for that commit and correct the count. If the PR is squash-merged, the squash message needs the same content.

Non-blocking

[new] The ROCm version statement is hardcoded and not derived from code. README.md:237-238 and README.md:405-407, repeated in the rendered getting-started page.
"ROCm CLI supports ROCm 7.14, 10.0, and 10.1. Without --version, rocm install sdk currently installs ROCm 7.14."
- With no --version, the code picks the newest build on the selected channel's index (apps/rocm/src/therock.rs:3421-3427). Nothing enforces "supports 7.14, 10.0 and 10.1": the version check rejects only empty input (therock.rs:138-151).
- "currently installs 7.14" ignores --channel nightly, which is in the synopsis directly above it.
- The PR description itself says the 7.14 default "comes from an automated review's dry run, not from source" and needs confirmation. The doc rules list version numbers that will change as a maintenance liability, and this one is stated in two places.
  Confidence 75 · logic · Fix: say once that the default is the newest build on the selected channel and point to rocm install sdk --dry-run, which prints selected_rocm_version:. If the three versions are a policy statement, label them as the validated set.
[new] --family is presented as required for ROCm 10, but the code detects the arch from the host. README.md:238-240
resolve_family fills raw_arch from detect_host_gfx_target() when --family is omitted (apps/rocm/src/therock.rs:3394-3401). The refusal at therock.rs:314 fires only when raw_arch is None. The steps given work, but they overstate the requirement.
Confidence 75 · logic · Fix: "add --version; if the host GPU is not detected, also pass --family <exact gfx arch>".
[new] --force-dev-binaries is described as removing "the debug and release binaries beside it". README.md:955-957
is_dev_binary_layout matches a parent directory named debug or release under target (apps/rocm/src/main.rs:18103-18118). The plan then removes only the binaries in the directory the running rocm sits in (collect_installed_binary_candidates, main.rs:18068-18090). target/debug is untouched when you run from target/release.
Confidence 75 · logic · Fix: "remove the ROCm CLI binaries in that build directory".
[new] "removes … its binaries" is not fully true on Windows. README.md:937-942
On Windows the running executable is skipped (apps/rocm/src/main.rs:17853-17859) and listed under "Left alone". The new section never says so.
Confidence 75 · logic · Fix: add a one-line Windows note.
[new] The "Verify the installation" failure advice is POSIX-only. README.md:110-125, included in docs/rocm-docs/install/installation.md
"If the shell reports rocm: command not found, confirm that ~/.local/bin is on your PATH" is in a bash block. On Windows the installer uses %USERPROFILE%\.local\bin (install.ps1:568-574) and PowerShell words the error differently. Both installers also honour ROCM_CLI_INSTALL_DIR, which the note ignores.
Confidence 75 · mechanical · Fix: add the Windows form and mention ROCM_CLI_INSTALL_DIR.
[new] The new "rocm fix takes these flags:" lead-in introduces an incomplete list. README.md:306, README.md:382-388
rocm fix --json exists (apps/rocm/src/main.rs:237-239) but appears in neither the synopsis nor the list.
Confidence 70 · mechanical · Fix: add [--json] and a bullet.
[new] Commit 5ed97f6's message carries a meta note and a reviewer's first name. git log body of 5ed97f6
The body says "Review comment #1 (the WSL page) is not in this commit. I removed the --keep-* caveat and the inlined qwen copy on purpose…". That describes edits the commit does not contain and reads as a note about the review exchange. It would reach main's history in a squash. The subject also names a reviewer by first name. Neither is an internal system, so this is noise rather than a rule violation.
Confidence 70 · mechanical · Fix: drop the sentence and the name when writing the squash message.

Decisions for the author

Hardcoded version numbers in user docs — tradeoff
Naming 7.14, 10.0 and 10.1 helps a reader who wants to know what works today. It also needs a manual edit whenever the index moves, and it sits in two places. A pointer to rocm install sdk --dry-run stays correct on its own but tells the reader less. Confirm which you want.
Absolute github.com/.../blob/main/... links in README.md and CONTRIBUTING.md — tradeoff
AGENTS.md section 5 exempts both files from the relative-link rule because they are included into a site built with -W, so this complies, and every target exists at HEAD. The cost is that on a fork or a release branch the links point at upstream main, and the offline link checkers do not verify them. This matches the pre-existing style of the other absolute links in CONTRIBUTING.md.

Positive signals

The {include} contract held. All 14 :start-after:/:end-before: anchors in docs/rocm-docs/ still resolve once, in order, and the hand-copied sentence in getting-started.md is resynced with its HTML comment updated to describe the new "end of the sentence" mechanism.
The docs/engine-plugins.md uninstall guidance now says --keep-data --keep-config and explains why. Under the old --keep-data-only advice ~/.rocm was deleted anyway. It is consistent with the README caveat and matches build_uninstall_plan.
The Chat section now documents the real interactive versus one-prompt split and which flags each form ignores, matching the handler at apps/rocm/src/main.rs:2229-2290.
README.md:607-608 points readers to rocm model --verbose. The default view hides recipes that diagnose --model still matches.
The new rocm diagnose synopsis [--distro [NAME] | --report [--send]] matches the clap conflicts_with/requires constraints.

Deployment notes

None. The PR is documentation-only and alters no behaviour. AGENTS.md section 3's regression-test and Gherkin requirements therefore do not apply, but the PR text should say that.

What this covered

Every changed file was read completely at HEAD (CONTRIBUTING.md, README.md, docs/engine-plugins.md, docs/rocm-docs/getting-started.md, docs/rocm-docs/install/installation.md, docs/vllm.md), and the diff against prw-base. Claims were checked against apps/rocm, apps/rocmd, crates/rocm-core, engines/vllm, install.sh, install.ps1 and the local prw-base history. Five read-only workers covered the file groups plus the source-reading passes. The conflict, the --keep-data/models contradiction, the --force-dev-binaries scope and the PR description's commit count were re-verified directly. The other code-backed claims in the findings above are the workers' reads, each cited to a line.

Checks at this head: the build, test, clippy, docs-build (-W), link check, commit-signature and prek jobs passed. The E2E lanes (MI300X, Strix Halo Ubuntu, Windows, WSL2), both consolidated reports, skill checks, the Linux PR binaries job and the containerised remote-control job were SKIPPED rather than passed. For a documentation-only change that is expected, but a skipped check is not a pass.

Not run: no build or test run, because the change is Markdown-only and no finding needed one. The mutation check on tests does not apply because the PR adds none. The history and prior-changes passes ran. The conflict finding came from the history pass.

Not reconciled against any discussion here. The PR's comments and earlier reviews were deliberately not read.

Revert the rocm storage output wording and its loosened test so this PR stays docs-only; the wording moves to a separate change. Fix the vLLM pin-table scope sentence, the stale --keep-data claim in MANIFEST.md, and the --send help text. Signed-off-by: pmoutsias-amd <peter.moutsias@amd.com>

rominf

🔴 Automated review · pr-review-watcher · 9915a90

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.
The description says the PR is docs-only plus --help wording, and the diff matches that. It names #626 (open) as the place the --keep-* help mismatch is tracked, and #630 and #636 as code fixes some wording depends on. No ticket is closed by this PR.

This is the fourth automated round. The previous review point (aeed0c8) is no longer in the branch, so this round reviewed the whole change again. Findings are marked new (first raised this round) or standing (raised in an earlier round and still holding).

Blocking

(new) The PR no longer merges cleanly, and resolving the conflict in favour of this branch would revert #604 — README.md, rocm serve sampling paragraph (README.md:679-680 on head)
- git merge-tree --write-tree prw-base HEAD reports CONFLICT (content): Merge conflict in README.md. GitHub reports mergeable_state: dirty.
- This is the only conflicting hunk. The base side, from e4b4155 (#604, "print copy-pasteable --yes on the remaining service stop hints"), says (`rocm services stop <service-id> --yes`). This branch rewords the same sentence and keeps the old (`rocm services stop`).
- #604 added --yes because the stop command fails without it. Taking the PR side reintroduces advice that doesn't work.
- The description's "Merges of main" section, which says one conflict was resolved, is also out of date now.
- Confidence 100 · logic · Fix: merge or rebase onto the current base, keep the copyedited wording but with rocm services stop <service-id> --yes, and update the merge note in the description.
(standing since the round at cf49378; not re-found blind; settled by putting it to the reviewer) The README states as current behaviour two things that are only true once the open PRs #630 and #636 merge, and this PR is not a draft — README.md:239-240, README.md:410-411, README.md:961-962
- README says "ROCm CLI supports ROCm 7.14, 10.0, and 10.1" with the 10.0 --version/--family steps, and that uninstall removes "its binaries", both without any "once #630/#636 lands" qualifier.
- #630 and #636 are both still open and unmerged (checked live this round), and #616 is draft: false.
- On the base code, a 10.0 install asks for plain torchaudio with no ROCm-build constraint (published_pip_requirements, apps/rocm/src/therock.rs:2192-2207), so the resolver can pick a 10.1.0 build, which the later check rejects (therock.rs:3488-3497). That matches the 10.0.0 dry-run failure recorded in the description's test plan. Whether it fires on a given day depends on what the index publishes.
- On Windows, build_uninstall_plan (apps/rocm/src/main.rs:17867-17874) skips the running rocm.exe, so "removes its binaries" is not true there today.
- The reviewer noted that AGENTS.md §11 is written for stacked PRs and applies here only in spirit. The direct defect is a README describing behaviour that has not shipped while those PRs are open.
- Confidence 85 · architectural · Fix: put the PR in draft until #630 and #636 merge, or scope the 10.0 and Windows-binary wording to what works today.
- Anchored evidence: this verdict came from asking the reviewer about the objection after its report was returned. It is not a blind finding.

Non-blocking

(new) The new README text on contained mode contradicts what rocm automations list prints — README.md:910-912; apps/rocm/src/main.rs:16372, 16420, 1210
- The new README says that in contained mode server-recover acts on its own and restarts a failed managed service. The README is the side that's right: rocmd maps every (_, Contained) to RunContained (apps/rocmd/src/watchers.rs:1344), and the RunContained arm restarts with no proposal (watchers.rs:1245).
- The CLI still prints ask before changes; keep actions limited as the contained-mode label, and asks before restarting a managed server for server-recover. A user reading rocm automations list is told the opposite of the README.
- AGENTS.md §5 says to grep a changed claim's twins in "printed CLI output" and check each against the code. The CLI strings are pre-existing, but this change adds the README claim without fixing them or recording the gap. That differs from how the PR handled the storage wording, which it did record.
- Confidence 75 · mechanical · Fix: correct the two labels, or record the mismatch as a tracked follow-up the way the storage wording was.
(standing, non-gating) rocm uninstall leaves the installer's PATH change on every platform. The reviewer confirmed no uninstall code undoes it. The README makes no false claim about it, so there is no sentence to fix here. #636 covers it.

Decisions for the author

(new) rocm storage still prints that ROCm CLI never removes downloaded models, while the README now says rocm uninstall does — answered-by-intent
- Locations: apps/rocm/src/storage.rs:509-512 and the "Shared with other tools (never removed by ROCm CLI):" heading in the same file.
- The README is the side that's right: build_uninstall_plan removes paths.data_dir, and data_dir/models is inside it.
- The description (commit 17) says: "The rocm storage output wording ("never removed by ROCm CLI" is not accurate …) is not changed here and needs a separate change with a full-string test."
- Without that passage this would be blocking, under the AGENTS.md §5 twin-sync rule.
- No tracker item is named for it, unlike the --keep-* help. Confirm it has somewhere to live.
(standing) The --keep-data and --keep-cache help ("use --keep-data for that") is wrong on the default layout, which the README, MANIFEST.md and engine-plugins.md now warn about — answered-by-intent
- Location: apps/rocm/src/main.rs:633-638.
- The description (commit 15) says: "Not in this PR: … the --keep-* help-text mismatch (tracked in #626)." #626 is open and describes exactly this.
- Without that passage this would be blocking, under AGENTS.md §5 ("update … its --help … in the same change").
(new) "rocm install sdk currently installs ROCm 7.14" (README.md:315-317, 521-523) — tradeoff
- The default is computed at run time as the newest version below the ROCm 10 boundary (therock.rs, resolve_latest_for_manifest), so the hardcoded number will go stale on the next 7.x release.
- The other side: a concrete version is more useful to a reader than "latest compatible", and commit 558fd45 chose "currently" on purpose. Confirm you're willing to maintain it.

Positive signals

The diagnose --send help used to describe a GitHub issue form. It now matches the real delivery: a mailto: to report_delivery::DESTINATION, and the link is printed instead of opened when there is no desktop. The old help was the stale part.
Links in README.md and CONTRIBUTING.md now point to absolute GitHub URLs. Both files are {include}d into the Sphinx site, so relative links would fail -W. The rewritten getting-started.md include anchors each match exactly once and in order, and the hand-copied sentence matches README word for word.

Deployment notes

None

What this covered

What was read: all 8 changed files, completely, in the diff prw-base...9915a907 (merge-base 213c2cb).
Claims checked: every changed behavioural claim was traced into its code path, including:
- rocm chat dispatch
- perform_delivery / report_delivery
- build_uninstall_plan and the default ~/.rocm layout
- the therock.rs ROCm 10 resolution
- the vLLM install routes
- rocmd's watcher policy
- the storage.rs report
Tests run: cargo test -p rocm --bin rocm with the filters help (10 tests), preview (3), delivery (1) and chat (57). All passed.
Merge check: a merge-tree against the current base.
CI on the head: all checks pass except two that have not reported, and neither is a failure of this PR. E2E tests (MI300X) is still queued, waiting on a self-hosted runner. Skill checks (skillscope) was skipped because this change does not touch skills/.

What did not run:

Prior-changes pass: not run, because it would need earlier review comments, which this run is not permitted to read.
Step 7 design questions: answered by the driver directly, not by separate workers. This is a docs-only change with little design surface.
Leak scan: clean. The only external address, ROCmCLI@amd.com, was already public in code and docs.
Prompt injection: none found.

The discussion on the PR was not reconciled here.

Standing objections from earlier rounds, settled after the review by putting each to the reviewer (anchored evidence, not blind findings):

The README's 10.0 and Windows-binary claims while #630 and #636 are open: still holds (second blocking item).
The loosened storage.rs test for a changed printed promise: cleared. At this head the PR no longer touches apps/rocm/src/storage.rs (empty diff against the base), because commit 17 reverted the wording change and its test.

On the merge gate: this change request is held on the two blocking items. The earlier change requests stay in place until neither holds.

Resolve the README.md conflict in the rocm serve sampling paragraph: keep the copyedited wording and the copy-pasteable 'rocm services stop <service-id> --yes' from #604. Signed-off-by: pmoutsia_amdeng <peter.moutsias@amd.com>

Resolve the README.md conflict in the rocm diagnose --distro paragraph: keep the copyedited wording and add the library and shared-memory checks (fix-18, fix-19) that #627 documents as skipped on a remote WSL diagnosis. Signed-off-by: pmoutsia_amdeng <peter.moutsias@amd.com>

rominf

🔴 Automated review · pr-review-watcher · 3dbac26

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.
The description names #626, #630 and #636 as related work. All three are open. The PR records the --keep-* help text as tracked in #626, and it says two of its README claims become true only once #630 and #636 land.

This is the fifth automated round. Findings are marked new (first raised this round) or standing (raised in an earlier round and still holding).

Blocking

(standing since the round at cf49378; re-found blind this round) README documents ROCm 10.0 install and Windows uninstall behaviour that is only true once two open PRs merge, and this PR is not held behind them — README.md:241-245, README.md:414-416, README.md:1027-1028
- The README says "ROCm CLI supports ROCm 7.14, 10.0, and 10.1" and gives the --version/--family route for 10.0. It also says rocm uninstall removes "its binaries".
- The description's own test plan reports that --version 10.0.0 --family gfx1200 "failed on a torchaudio mismatch", which "#630 fixes". It also says "The 'removes its binaries' wording is accurate once #636 lands".
- On the base, build_uninstall_plan still skips the running executable on Windows (apps/rocm/src/main.rs, the "skipping running executable on Windows" branch).
- #630 and #636 are both OPEN, and #616 is not a draft. If this merges first, the published docs promise a 10.0 install that fails and a Windows uninstall that leaves rocm.exe behind.
- AGENTS.md §11: "Keep stacked PRs in draft until dependencies merge upstream", and link them with "Depends on #…".
- Confidence 85 · logic · Fix: either convert to draft with "Depends on #630, #636" until both merge, or narrow those sentences to what is true on the current base.

Non-blocking

(new) The description's commit count is wrong — PR description, Summary
- It says "sixteen content commits plus two merges of main".
- git log prw-base..HEAD shows 17 non-merge commits (the description's own sections run 1 to 17) and 4 merges (30b5022, 985f773, b80ab1c, 3dbac26).
- Confidence 85 · mechanical · Fix: correct the counts.
(new) The new README sentence documents a Linux-only default for the shared uv cache as if it applied everywhere, and the code behind it diverges from its twin — README.md "Logs and cleanup" (the UV_CACHE_DIR or default ~/.cache/uv sentence), apps/rocm/src/main.rs:13628-13629, apps/rocm/src/storage.rs:399-408
- shared_cache_candidates() uses ~/.cache/uv on every OS.
- storage.rs::shared_uv_cache_dir() uses AppData/Local/uv/cache on Windows.
- So on Windows, uninstall warns about a path where uv's cache does not live, and the README repeats that. The code bug is pre-existing; this PR newly documents it.
- storage.rs governs, because its path matches uv's documented Windows default.
- Confidence 85 · logic · Fix: say the default is the Linux path in the README, and file the main.rs twin for a code fix.
(new) CONTRIBUTING's reworded sentence keeps an incomplete list of which files docs/rocm-docs/ includes — CONTRIBUTING.md:92
- It says the included files "are this file and README.md".
- docs/rocm-docs/engines/vllm.md:7 also includes docs/vllm.md, and docs/rocm-docs/install/wsl.md includes docs/wsl.md. Those {include} directives are what govern.
- The incomplete list is pre-existing, but this PR rewrote the sentence, and it edits docs/vllm.md, which that sentence leaves out.
- Confidence 85 · mechanical · Fix: name all included files, or say "the files it {include}s".
(standing from the round at 9915a90, non-gating; not re-found blind this round) The new README text on contained mode contradicts what rocm automations list prints — README.md:978; apps/rocm/src/main.rs:11952, apps/rocm/src/main.rs:12000
- The README says contained lets a watcher act on its own. The CLI still labels the mode ask before changes; keep actions limited and server-recover as asks before restarting a managed server. Both strings are unchanged at this head (checked directly by the poster this round, not by the blind review).
- Fix: correct the two labels, or record the mismatch as a tracked follow-up the way the storage wording was.
(standing, non-gating) rocm uninstall leaves the installer's PATH change on every platform. The README makes no false claim about it; #636 covers it.

Decisions for the author

(standing) rocm uninstall --keep-data / --keep-cache --help still says --keep-data keeps the uv package cache — answered-by-intent
- The PR corrects this claim in README.md, MANIFEST.md and docs/engine-plugins.md: on the default layout, config and data are both ~/.rocm.
- build_uninstall_plan removes config_dir unless --keep-config is passed, so --keep-data alone keeps nothing.
- But apps/rocm/src/main.rs:663-669 still says "Keep app data such as … the uv package cache", and --keep-cache says "use --keep-data for that".
- This would otherwise be blocking under the AGENTS.md §5 claim-sync rule, because the PR itself creates the disagreement and edits other --help text in the same file.
- Description: "Not in this PR: … the --keep-* help-text mismatch (tracked in #626)."
- Confirm that is still the plan. Until #626 lands, --help is the one surface a user consults when picking the flag, and it is the one still wrong.
(standing) rocm storage output still says downloaded models are "never removed by ROCm CLI" — answered-by-intent
- The new README text says rocm uninstall removes them. apps/rocm/src/storage.rs:511-516 still prints "ROCm CLI never removes them" under "Shared with other tools (never removed by ROCm CLI)".
- The README's own "Those last three are shared with other tools" still counts <data_dir>/models as shared, which the next sentence contradicts.
- Would otherwise be blocking (AGENTS.md §5).
- Description, §17: "The rocm storage output wording … is not changed here and needs a separate change with a full-string test." No tracking issue is named for it. Consider filing one, as was done for #626.
(new) A single PR mixing a copyedit pass with new behavioural documentation — tradeoff
- Pro: the description separates the fact-bearing commits from the wording ones.
- Con: the 557-line README diff interleaves both, so a content reviewer has to re-verify every edited claim. The unchecked "content reviewer confirms three claims" item shows that verification is still open.

Positive signals

The README-only include anchors in docs/rocm-docs/getting-started.md were updated together with the README sentence. Every :start-after:/:end-before: string still occurs exactly once and in order, and the Sphinx -W build is green.
The diagnose --send and chat --help rewrites now match the code: the mailto: delivery in crates/rocm-core/src/report_delivery.rs, and the interactive-terminal check in the Command::Chat handler.

Deployment notes

None

What this covered

What was read: all 8 changed files in full (README.md, CONTRIBUTING.md, docs/vllm.md, MANIFEST.md, docs/engine-plugins.md, docs/rocm-docs/getting-started.md, docs/rocm-docs/install/installation.md, and the main.rs doc-comment hunks). Every behavioural claim was checked against the clap definitions and implementing code: uninstall plan, storage report, chat dispatch, report delivery, therock install routing, vLLM install routing, rocmd watchers.
Commits: prw-base…3dbac266ea4a08527e2ca4ee2537314a32b290a7.
Passes: coupled code groups, history (git log/blame against prw-base), and agent-instruction adherence each ran as an independent worker.
CI: read from statusCheckRollup. All checks passed (skillscope was skipped), including Sphinx -W and lychee.
Did not run:
- The prior-changes pass, because earlier PR reviews are deliberately not read here.
- The Step 7 design questions ran in the driver context, not as independent workers.
- No Sphinx build locally, because it is not installed; anchors were verified by grep, and CI covers the build.
- The 10.0 install failure was not reproduced here. It rests on the description's recorded run and #630's open state.
Not reconciled: this report was not reconciled against the PR's discussion.

Standing objections from the previous round, settled by the poster:

The README conflict with main that would have dropped #604's --yes: cleared. A merge-tree against the current base is clean, and README.md:747 and :799 carry rocm services stop <service-id> --yes.
The 10.0 / Windows-binary claims while #630 and #636 are open: still holds. Re-found blind this round; #630 and #636 were checked live and are both open.

On the merge gate: this change request is held on the one blocking item. The earlier change requests stay in place until it no longer holds.

….2.0 docs Docs-only. Audited against the v0.2.0 code: omits post-v0.2.0 behavior text and marks ROCm 10 as a preview with a known issue (torchaudio resolution). Signed-off-by: pmoutsia_amdeng <peter.moutsias@amd.com>

…all intro Keep the #616 framing: ROCm CLI supports 7.14 and currently installs it without --version. ROCm 10 and newer are a preview: 10.1.0 works today and may change as newer builds publish, and 10.0 currently fails because of the known issue. Restore the --family sentence and the gfx1200 example. Signed-off-by: pmoutsia_amdeng <peter.moutsias@amd.com>

….2.0 docs Docs-only. Audited against the v0.2.0 code: omits post-v0.2.0 behavior text and marks ROCm 10 as a preview with a known issue (torchaudio resolution). Signed-off-by: pmoutsia_amdeng <peter.moutsias@amd.com>

rominf

🔴 Automated review · pr-review-watcher · 89815e4

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.
The description names no closing issue. It cites #630 and #636 as code dependencies (both merged) and #626 as a deferral (open). Two commits are not described at all; see Blocking.

This is the sixth automated round. Findings are marked new (first raised this round) or standing (raised in an earlier round and still holding).

Why this round files no change request, and lifts the earlier ones. The three blocking findings below are each already held by GitHub itself, or do not land on main: the PR cannot merge while it conflicts with main (mergeable_state: dirty), Commit signatures + sign-off is a required check, and the PR description is not what a squash merge writes into main. A change request would add a second gate on top of gates that already hold, so none is filed. The earlier automated change requests on this PR are lifted, because every objection they made no longer holds (see the end of this report). Fixing the findings below is still needed before this can merge.

Blocking

(new) The PR conflicts with main, and its new torchaudio text describes code this head does not have — README.md:528-546, docs/vllm.md:86-96
- Running git merge-tree --write-tree prw-base HEAD exits 1 with CONFLICT (content) in both README.md and docs/vllm.md. The GitHub API reports mergeable: false, mergeable_state: dirty.
- Cause: the PR's last merge from main is older than #630 (3067a10). #630 rewrote these same paragraphs.
- Commit 11eee18 adds its own version of #630's text: "pins torchaudio itself … the torchaudio_selection: line …" in the README, and "One value flows the other way …" in docs/vllm.md. In this head, published_pip_requirements (apps/rocm/src/therock.rs:2192-2207) still passes a bare, unpinned "torchaudio". torchaudio_selection and verified_torchaudio_release exist nowhere in the tree.
- So at 89815e4 the docs describe behaviour the code at that commit lacks. They only become accurate once this branch is rebased onto main.
- Confidence 100 · architectural · Fix: rebase onto current main. Keep one version of each torchaudio paragraph, checked against #630's code.
(new) Required check Commit signatures + sign-off fails because of this PR — commits 11eee18 and 4905d64
- CI log: "verify-commits: 2 check(s) failed: -> missing Signed-off-by trailer" (twice).
- Locally, git log confirms that 11eee18 and the merge commit 4905d64 are the only commits in prw-base..HEAD without the trailer.
- This job checks the PR's own commits. The red CI runs on main (c7a0d65, CI failure) are a different workflow, so they don't explain this failure.
- AGENTS.md §2 requires DCO sign-off.
- Confidence 100 · mechanical · Fix: sign off both commits. The rebase above can do it with git rebase --signoff.
(new) The description misstates the change — PR body
- The test plan ticks "All commits are signed and carry a DCO Signed-off-by trailer", which is false (see above).
- The summary says "sixteen content commits plus two merges" and describes 17 numbered items. It never mentions 11eee18 ("describe the ROCm 10 torchaudio pin …"), which adds the new torchaudio paragraphs to README.md and docs/vllm.md, or 89815e4.
- Unmentioned work goes unreviewed. These are exactly the paragraphs that describe code missing from this head.
- Confidence 100 · mechanical · Fix: correct the description. Uncheck the sign-off box until it is true, and add the torchaudio and serve-step commits.

Non-blocking

(new) "If --family is needed and missing, the error names the command to re-run" is only half true — README.md:525-526
- It holds when a family is detected but no exact arch: select_source_layout, therock.rs:314-320, suggests --version {version} --family gfx1200.
- The paragraph above also covers the case where no GPU is detected ("or none"). On that path resolve_family fails first with the generic error at therock.rs:3445-3451. That error suggests rocm install sdk --family <FAMILY>, which drops --version, and lists family labels that this same section says ROCm 10 rejects.
- Confidence 75 · logic · Fix: limit the sentence to the detected-family case, or say that with no GPU detected you pass the raw gfx code yourself.
(new) Two commits carry Co-Authored-By: Claude <noreply@anthropic.com> trailers — 11eee18 and 89815e4
- AGENTS.md §11 says to avoid AI-generated boilerplate footers. A squash merge can carry these trailers into main's history.
- Confidence 70 · mechanical · Fix: drop the trailers when re-signing these commits.

Decisions for the author

(standing) The --keep-data / --keep-config --help text still says nothing about the default layout — answered-by-intent
- The PR now says in three docs (README.md, MANIFEST.md, docs/engine-plugins.md) that on the default layout both flags are needed.
- The clap doc comment at apps/rocm/src/main.rs:674-677 still reads "Keep app data such as logs, services, engines, and the uv package cache".
- default_config_dir() and default_data_dir() both return ~/.rocm (crates/rocm-core/src/runtime.rs:224-230), so the docs are right and the help is incomplete.
- The description says: "Not in this PR: … the --keep-* help-text mismatch (tracked in #626)."
- Without that deferral this would be blocking: AGENTS.md §5 says to update --help in the same change. Confirm the deferral is still intended. #626 is open.
(standing) The rocm storage printout still says ROCm CLI "never removes" model files, while the README now says rocm uninstall does — answered-by-intent
- The printout is at apps/rocm/src/storage.rs:511-517.
- The description says: "The rocm storage output wording … is not changed here and needs a separate change with a full-string test."
- The README is the side that matches build_uninstall_plan. Confirm a follow-up exists.
(standing) "currently installs ROCm 7.14" when --version is omitted — tradeoff
- The code (select_rocm_version) picks the newest stable release on the index. The number goes stale on the next release, and nothing checks it.
- On the other side, a concrete version is more useful to readers than "the newest".

Positive signals

The diagnose --send help now matches the code. perform_delivery and report_delivery::DESTINATION really do send a mailto: to ROCmCLI@amd.com, and the old text described an issue form that no longer exists.
The new rocm chat help matches the handler: the interactive branch is taken only with a terminal and no --prompt, and --provider gets only a note there.
All {include} anchors in docs/rocm-docs/ that point into README.md still match exactly once, in order. One worker checked them with a script, and I re-checked the start/end strings independently.

Deployment notes

None

What this covered

What was read: every changed file in full, against prw-base (c7a0d65) and head 89815e4; the merge-base is 0acf698.
- README.md and docs/vllm.md / MANIFEST.md / docs/engine-plugins.md / install/installation.md / CONTRIBUTING.md were split across two independent read-only workers.
- I reviewed apps/rocm/src/main.rs and docs/rocm-docs/getting-started.md myself.
- Claims were checked against therock.rs, main.rs (uninstall plan, Chat handler, perform_delivery), rocm-core (runtime.rs, report_delivery.rs, uv.rs) and apps/rocmd/src/watchers.rs.
- CI was read from statusCheckRollup and the failing job's log.
Not run:
- The prior-changes pass: earlier review comments are off-limits to this run.
- Builds and tests: the only Rust change is doc-comment text, and CI's build, clippy, test and e2e lanes are green on this head.
- No tests were added. None are needed, because no behaviour changed.
Not reconciled against the PR's discussion here.

Earlier objections, checked by the poster against this head (not by the blind review):

README claims ROCm 10.0 support and that uninstall removes "its binaries", while #630 and #636 were open. Both have now merged (#636 is in this head; #630 is on main, which this PR must be rebased onto before it can merge). This no longer holds.
--keep-* flags promised to keep their category on a default install. The docs now say both flags are needed on the default layout. This no longer holds.
The README's version list was wrong while --version 10.0.0 failed. Later rounds narrowed this to that 10.0 failure, which #630 fixes. The remaining point, that "currently installs ROCm 7.14" will go stale, is carried above as a tradeoff, not a blocker.
A loosened storage.rs test. storage.rs is no longer part of this PR. This no longer holds.
Resolving the conflict in favour of this branch would revert #604's sampling paragraph. That paragraph is present on this head. The current conflict is only in the torchaudio text (see Blocking). This no longer holds.
Uninstall leaves the installer's PATH change. #636 fixes this and is in this head. This no longer holds.

Every objection in the automated change requests on this PR no longer holds at 89815e4: #630 and #636 have merged, and the other points were fixed or dropped. The remaining blockers (conflict with main, missing sign-off, description) are already held by GitHub's own merge gates. See the latest automated review on this PR.

"Then serve a model:" followed a paragraph about the activation gate, so "Then" had no clear referent. Signed-off-by: pmoutsia_amdeng <peter.moutsias@amd.com>

Signed-off-by: pmoutsia_amdeng <peter.moutsias@amd.com>

pmoutsias-amd · 2026-10-21T21:45:40Z

Addressed the sixth automated review at 0e08f3f. The merge conflict with main is resolved, every commit now carries a DCO sign-off, and no commit carries an AI co-author trailer. The description is updated, and the --family sentence is corrected. The torchaudio text is now #630's own wording, copyedited. Three items are deliberately left out of this PR. The --keep-* help text is tracked in #626. The rocm storage wording needs its own change with a full-string test. The "installs ROCm 7.14" statement was verified. The old CHANGES_REQUESTED review from 5ed97f6 was an automated report whose three blockers are all fixed. Re-review requested.

rominf

🔴 Automated review · pr-review-watcher · 0e08f3f

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 — no blocking findings

Full review of the whole change.
Docs-only PR, apart from --help doc-comment text in apps/rocm/src/main.rs. The description matches the diff and closes no issue. #626 is cited as an open follow-up and #630/#636 as merged dependencies; all three were checked and are in the states the description gives.

This is the seventh automated round. Findings are marked new (first raised this round) or standing (raised in an earlier round and still holding). The commit the last round reviewed (89815e4) is no longer in the branch's history, so this round reviewed the whole change again rather than only what moved.

Blocking

None

Non-blocking

(new) "Without --version, rocm install sdk currently installs ROCm 7.14" does not say it only holds on the release channel — README.md:254-255, README.md:435-437
therock.rs::select_rocm_version has no pinned default. It takes the newest candidate the channel's index publishes, and channel_rocm_candidates drops prereleases only for TheRockChannel::Release. On --channel nightly, which the synopsis at README.md:424 offers, it picks whatever is newest. Tests auto_select_landing_on_next_major_needs_next_python and tarball_selection_never_crosses_channels show a fake nightly index with a 10.1 prerelease being selected. I did not query the live nightly index, so this is argued from the code, not reproduced.
Confidence 70 · logic · Fix: scope the sentence to the default release channel, e.g. "the newest release on the default channel (7.14 today)".
(new) The README now calls ~/.cache/uv the default uv cache, which is not uv's default on Windows (pre-existing code inconsistency that this PR now documents) — README.md:1182-1184, apps/rocm/src/main.rs::shared_cache_candidates, apps/rocm/src/storage.rs::shared_uv_cache_dir
The two implementations disagree. shared_cache_candidates (used by rocm uninstall) uses ~/.cache/uv on every platform. Its twin storage.rs::shared_uv_cache_dir branches on runtime_is_windows() for the Windows location. So the sentence matches what uninstall reports, but on Windows uninstall never reports the cache uv actually uses there. The code governs, and the uninstall copy is the one that is wrong.
Confidence 70 · logic · Fix: a follow-up issue to make shared_cache_candidates use the same Windows default as storage.rs, then reword the sentence. Not a change for this docs PR.

Decisions for the author

None

Positive signals

The diagnose --send help text on main still promised a "prefilled issue form", but the code (crates/rocm-core/src/report_delivery.rs, DESTINATION/mailto:) sends a prefilled mail. This PR brings the help text into line with the code, and README line 361 and docs/testing.md agree with it.
The new rocm chat help text matches the Command::Chat handler exactly. In an interactive terminal with no --prompt, dash::run_chat ignores provider, model and tools and still applies the sampling parameters.
The new default-layout caveat for --keep-* was checked against rocm-core default_config_dir/default_data_dir/default_cache_dir (all under ~/.rocm) and the nested-removal loop in build_uninstall_plan, and it is accurate.

Deployment notes

None

What this covered

I read every file in the change completely: README.md, CONTRIBUTING.md, docs/vllm.md, MANIFEST.md, docs/engine-plugins.md, docs/rocm-docs/getting-started.md, docs/rocm-docs/install/installation.md and apps/rocm/src/main.rs. Each added or changed behaviour claim was cross-checked against the code it describes: the uninstall planner, path defaults, chat dispatch, diagnose send/report delivery, therock version selection, and the vLLM discover table. Range: 32834b2…0e08f3f1.

How the work was split:

One worker reviewed README.md.
One worker reviewed docs/vllm.md and CONTRIBUTING.md.
I did the rest myself: main.rs, MANIFEST.md, docs/engine-plugins.md and the docs-site wrappers, plus the agent-instruction pass, the history pass, the leak scan and the description check.

Other checks:

A script confirmed that every {include} :start-after:/:end-before: anchor in docs/rocm-docs/ still resolves against the new files.
The leak scan against the AGENTS.md §2 pattern turned up only a pre-existing "no internal hostnames" line in CONTRIBUTING.
All 23 commits carry a Signed-off-by trailer and none carries an AI footer.

Not done, and why:

Prior-changes pass: not run, because this run forbids reading PR comments or earlier reviews.
Builds and tests: none run locally. The only Rust change is doc-comment text, and CI's non-E2E checks passed. The three failing E2E jobs fail identically on base 32834b2, so they are not attributed to this PR.
Rendered docs site: not checked; I did not run sphinx-build.
Live nightly index: not queried, so the first finding is argued from the code only.

One ordering deviation: I read the PR description before consolidating, not after. Both workers stayed blind to it.

Not reconciled against any PR discussion.

The previous round's blockers, settled by the poster against this head (not by the blind review):

The PR conflicts with main, and its torchaudio text describes code the head lacks. git merge-tree --write-tree against current main (32834b2) exits 0, and GitHub reports mergeable: true. For the torchaudio paragraphs, the blind review raised nothing; the objection was then put to the reviewer, which answered that it no longer holds: published_pip_requirements now emits a pinned torchaudio==…, the plan prints a torchaudio_selection: line, and verified_torchaudio_release is defined in engines/vllm/src/install.rs with a test. That answer is anchored (the reviewer was pointed at the objection). It did not check every failure-path clause of README.md:529-545. Dropped.
Required check Commit signatures + sign-off fails. It passes on 0e08f3f, and every commit in the range carries Signed-off-by (checked locally). Dropped.
The description misstates the change. It now describes eighteen content commits and five merges, which matches the range, and its sign-off claim is now true. The blind review found the description matches the diff. Dropped.

The two earlier non-blocking items are also gone: no commit carries a Co-Authored-By trailer, and the --family sentence is now limited to the detected-family case. The "currently installs ROCm 7.14" tradeoff from earlier rounds is narrowed by the first new non-blocking finding above.

CI. E2E tests, E2E tests (Strix Halo, Windows, current) and E2E consolidated report (self-hosted) fail on this head. The same three jobs fail on main at 32834b2, and E2E tests fails on the same 4 steps (229 scenarios, 225 passed) in both, so they are not caused by this PR.

Other change requests. This automation holds no change request on this PR. The CHANGES_REQUESTED review at 5ed97f6 belongs to another reviewer's account, so this automation cannot withdraw it; only that reviewer can.

Clearing stale review.

pmoutsias-amd added 2 commits October 8, 2026 22:41

pmoutsias-amd force-pushed the docs/editorial-copyedit-readme-contributing-vllm branch from 969ee1e to 9bf2e37 Compare October 9, 2026 02:42

pmoutsias-amd added 9 commits October 8, 2026 22:50

docs: tighten README flag lists and add lead-ins

7d2986b

Keep the rocm diagnose flag list tight by moving the long --report and --send detail below it, add colon lead-ins to the flag lists that had none, and clarify the --family sentence. Signed-off-by: pmoutsias-amd <peter.moutsias@amd.com>

docs: rejoin prune lock sentence and complete acceptance-script list

638807b

Signed-off-by: pmoutsias-amd <peter.moutsias@amd.com>

docs: add install verification and rocm uninstall guidance

3d8b1f9

Signed-off-by: pmoutsias-amd <peter.moutsias@amd.com>

docs: link install pages to uninstall guidance

e764eba

Point the ROCm installation section and the site install page at Logs and cleanup, so readers who finish installing can find how to remove ROCm CLI. Signed-off-by: pmoutsias-amd <peter.moutsias@amd.com>

docs: give the install page uninstall pointer its own heading

a87de84

Signed-off-by: pmoutsias-amd <peter.moutsias@amd.com>

docs: fix command synopses and document model, chat provider, and aut…

74d08fb

…omation modes Signed-off-by: pmoutsias-amd <peter.moutsias@amd.com>

docs: clarify that rocm chat --provider applies only to one-prompt use

2153b6e

Signed-off-by: pmoutsias-amd <peter.moutsias@amd.com>

docs: correct contained-mode, chat flag, and model verbose descriptions

f5fee37

Signed-off-by: pmoutsias-amd <peter.moutsias@amd.com>

pmoutsias-amd changed the title ~~docs: copyedit README, CONTRIBUTING, and vLLM guide~~ docs: copyedit README, CONTRIBUTING, and vLLM; add uninstall and verify guidance Oct 9, 2026

docs: clarify uninstall cache and data scope, add APU case to diagnose

c92e88a

Distinguish the managed uv cache from the shared one, list downloaded models under --keep-data, and include the APU case in the undetermined verdict conditions. Signed-off-by: pmoutsias-amd <peter.moutsias@amd.com>

pmoutsias-amd marked this pull request as ready for review October 9, 2026 04:04

pmoutsias-amd requested a review from a team as a code owner October 9, 2026 04:04

pmoutsias-amd requested a review from volen-silo October 9, 2026 04:04

rominf previously requested changes Oct 9, 2026

View reviewed changes

pmoutsias-amd mentioned this pull request Oct 9, 2026

uninstall: --keep-* flags are ignored when config, data, and cache directories overlap (default layout) #626

Open

pmoutsias-amd requested a review from rominf October 9, 2026 13:56

randyh62 approved these changes Oct 9, 2026

View reviewed changes

pmoutsias-amd mentioned this pull request Oct 9, 2026

docs: add the WSL2 guide on the docs site #627

Merged

2 of 8 tasks

rominf previously requested changes Oct 12, 2026

View reviewed changes

This was referenced Oct 12, 2026

fix(therock): pin torchaudio to the requested ROCm 10 build #630

Merged

fix(uninstall): undo the installer's PATH change and remove the running exe #636

Merged

siloteemu previously requested changes Oct 12, 2026

View reviewed changes

pmoutsias-amd requested review from randyh62 and rominf October 13, 2026 02:02

pmoutsias-amd force-pushed the docs/editorial-copyedit-readme-contributing-vllm branch from 91c4448 to 9915a90 Compare October 13, 2026 02:18

rominf previously requested changes Oct 13, 2026

View reviewed changes

pmoutsias-amd requested a review from rominf October 13, 2026 12:08

rominf previously requested changes Oct 13, 2026

View reviewed changes

pmoutsias-amd mentioned this pull request Oct 19, 2026

docs: port editorial copyedits and WSL2 guide to the v0.2.0 docs #711

Closed

3 tasks

rominf reviewed Oct 21, 2026

View reviewed changes

pmoutsias-amd added 2 commits October 21, 2026 17:17

docs: tie the serve step to the install it follows

b308ca3

"Then serve a model:" followed a paragraph about the activation gate, so "Then" had no clear referent. Signed-off-by: pmoutsia_amdeng <peter.moutsias@amd.com>

Merge main into docs/editorial-copyedit-readme-contributing-vllm

0e08f3f

Signed-off-by: pmoutsia_amdeng <peter.moutsias@amd.com>

pmoutsias-amd force-pushed the docs/editorial-copyedit-readme-contributing-vllm branch from 89815e4 to 0e08f3f Compare October 21, 2026 21:37

pmoutsias-amd requested a review from rominf October 21, 2026 21:43

rominf reviewed Oct 22, 2026

View reviewed changes

rominf approved these changes Oct 22, 2026

View reviewed changes

rominf enabled auto-merge October 22, 2026 08:16

rominf added this pull request to the merge queue Oct 22, 2026

github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 22, 2026

		To remove ROCm CLI and what it manages, see
		[Logs and cleanup](../commands.md#logs-and-cleanup).

	first, because the new install takes over as the active default; see
	Running _rocm install sdk_ when a managed runtime is already active prompts for confirmation first, because the new install takes over as the active default; see


	moved to the SDK's build of the torch release that the engine pins: the release


		## ROCm 10.x wheel discovery

		For most ROCm SDK versions, `rocm engines install vllm` pins a fixed vLLM wheel

Conversation

pmoutsias-amd commented Oct 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

1. Call out supported ROCm versions (7.14, 10.0, 10.1)

2. Copyedit README, CONTRIBUTING, and vLLM guide

3. Tighten README flag lists and add lead-ins

4. Rejoin prune lock sentence and complete acceptance-script list

5. Add install verification and rocm uninstall guidance (new content)

6. Clarify verify and uninstall guidance (new content)

7. Link install pages to uninstall guidance

8. Give the install page uninstall pointer its own heading

9. Fix command synopses and document rocm model, rocm chat --provider, and automation modes (new content)

10. Clarify that rocm chat --provider applies only to one-prompt use

11. Correct contained-mode, chat flag, and model verbose descriptions

12. Clarify uninstall cache and data scope, add APU case to diagnose

13. Address automated review: uninstall default layout, ROCm versions, synopses

14. Avoid calling 7.14 the latest stable release

15. Address review comments on getting started, uninstall, and vLLM

16. Correct stale install, serve, and cleanup guidance

17. Align vLLM, uninstall, and help wording with the code

18. Tie the serve step to the install it follows

Merges of main

Dependencies on code changes

Test plan

Uh oh!

rominf left a comment

Choose a reason for hiding this comment

Review — needs work

Blocking

Non-blocking

Decisions for the author

Positive signals

Deployment notes

What this covered

Uh oh!

Uh oh!

randyh62 Oct 9, 2026

Choose a reason for hiding this comment

Uh oh!

randyh62 Oct 9, 2026

Choose a reason for hiding this comment

Uh oh!

randyh62 Oct 9, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

randyh62 Oct 9, 2026

Choose a reason for hiding this comment

Uh oh!

randyh62 Oct 9, 2026

Choose a reason for hiding this comment

Uh oh!

randyh62 Oct 9, 2026

Choose a reason for hiding this comment

Uh oh!

rominf left a comment

Choose a reason for hiding this comment

Review — needs work

Blocking

Non-blocking

Decisions for the author

Positive signals

Deployment notes

What this covered

Uh oh!

rominf commented Oct 12, 2026

Uh oh!

siloteemu left a comment

Choose a reason for hiding this comment

Review — needs work

Blocking

Non-blocking

Decisions for the author

Positive signals

Deployment notes

What this covered

Uh oh!

rominf left a comment

Choose a reason for hiding this comment

pmoutsias-amd commented Oct 9, 2026 •

edited

Loading

5. Add install verification and `rocm uninstall` guidance (new content)

9. Fix command synopses and document `rocm model`, `rocm chat --provider`, and automation modes (new content)

10. Clarify that `rocm chat --provider` applies only to one-prompt use

Merges of `main`