feat(diagnose): add the vLLM out-of-memory failure mode to the catalog (EAI-8060) by r0x0r · Pull Request #290 · ROCm/rocm-cli

r0x0r · 2026-08-20T10:18:18Z

Summary

Adds the vLLM startup out-of-memory failure mode to the rocm-core diagnosis
catalog, with a matching print-only remediation recipe.

The error string covers two distinct faults, and the remediation splits on which
one it is:

a tenancy collision — vLLM's fixed ~90% VRAM reservation runs into memory
already in use on a shared or busy GPU; lowering the reservation is the fix, or
steer onto a less-busy device with --gpu <index>.
a model that genuinely does not fit — lowering the reservation only trades
an earlier OOM for a later one; the fix is a smaller/quantized model or
sharding across GPUs with --tensor-parallel-size <n>.

So the wording stays conditional: it never presents --gpu-memory-utilization
as the unconditional answer, and the verify step avoids the tenancy knob so it
does not unconditionally prescribe the tenancy workaround.

What changed

crates/rocm-core/src/diagnose.rs — KEYWORDS_VLLM_OOM keyword table and
check_16_vllm_oom. The match is keyword-only: an Examination carries no
per-GPU VRAM or tenancy fields, so nothing structural can corroborate it — the
user passes the error via --symptom, or arrives from the serve failure hint.
The checker is gated to Linux and WSL2 (vLLM is Linux/WSL-only). On
WSL2 this means a qualifying keyword match now produces a normal
diagnosis instead of the platform's previous unconditional "out of
scope" routing -- an exit-code/output contract change worth calling out
explicitly here, not just in code comments. A required vLLM anchor (the
word vllm, or one of its distinctive flags/logs) gates the whole
keyword table, so a bare framework OOM string (e.g. torch.OutOfMemoryError: CUDA out of memory from an unrelated PyTorch job) is not misattributed
to vLLM.
crates/rocm-core/src/fix.rs — print-only fix-16-vllm-oom recipe
(auto_applicable: false, no runner). Catalog count assertion 15 → 16 and the
AUTO-set assertion strengthened to pin fix-16 as print-only.
engines/vllm/src/lib.rs — the serve OOM hint points users at
rocm diagnose --symptom '…'.
tests/e2e-cucumber — a conditional-remediation scenario
(@id:diagnose-vllm-oom-is-conditional, @requires-os:linux), its step
definitions, and the catalog fix-id contract updated.

Dependencies / stacking

Stacked on feat(vllm): Tackle out of memory errors (EAI-8058) #251 (gpu-out-of-memory, EAI-8058) — this PR targets that
branch, not main, and stays in draft until feat(vllm): Tackle out of memory errors (EAI-8058) #251 merges upstream.
Sibling sub-task: feat(vllm): enhance OOM diagnostics and guidance in serve summary (eai-8059) #284 (EAI-8059) adds the serve-summary OOM guidance that
routes users here.

Test plan

cargo test -p rocm-core --lib — passes (includes the checker tests: a
high-confidence anchored OOM match, a bare "out of memory" staying below
threshold, a bare framework OOM without a vLLM anchor not matching at all,
Linux/WSL gating, and a WSL sub-threshold hit staying in matched instead
of being routed out of scope).
cargo test -p rocm-engine-vllm --lib — passes.
cargo clippy --workspace --all-targets -- -D warnings — clean.
e2e-cucumber crate compiles; the new @requires-os:linux scenario runs on the
self-hosted Linux/GPU lane rather than locally.

volen-silo · 2026-08-20T11:20:17Z

A few observations from a read-through:

KEYWORDS_VLLM_OOM needs nothing vLLM-specific to hit high confidence: torch.OutOfMemoryError: CUDA out of memory scores 45+45=90 under the top-2 rule, so any ROCm PyTorch job's OOM is reported as "vLLM ran the GPU out of memory at startup" with rocm serve-only remediation. The one vLLM-specific token, gpu_memory_utilization, is weighted lowest and never required — and all three new tests use vLLM-shaped text.
The description says Linux-gated, but the checker registers for ["linux", "wsl"] and rewrites the diagnose() WSL branch so WSL2 is no longer unconditionally out-of-scope. That's an exit-code contract change worth stating in the body.
In that branch, sub-threshold hits are dropped from matched, which the DiagnoseReport::matched doc says should never happen.
feat(vllm): enhance OOM diagnostics and guidance in serve summary (eai-8059) #284 deletes the local log_tail_shows_oom this PR still calls, and edits the same oom_utilization_hint — conflict for whichever lands second.
Body says this stays in draft until feat(vllm): Tackle out of memory errors (EAI-8058) #251 merges; it's currently open.

…AI-8060) Add a keyword-scored OOM signature to the rocm-core diagnosis catalog and a matching print-only remediation recipe. The error covers two distinct faults (a tenancy collision with vLLM's fixed ~90% VRAM reservation, versus a model that genuinely does not fit), so the wording stays conditional: it never prescribes lowering --gpu-memory-utilization as the unconditional answer, and the verify step avoids the tenancy knob. Because an Examination carries no per-GPU VRAM or tenancy fields, the match is keyword-only and the checker is gated to Linux (vLLM is Linux/WSL-only). - diagnose.rs: KEYWORDS_VLLM_OOM table + check_16_vllm_oom (linux-only) - fix.rs: print-only fix-16-vllm-oom recipe; catalog count 15 -> 16 and AUTO-set assertion strengthened - engines/vllm: serve OOM hint points users at 'rocm diagnose --symptom' - e2e-cucumber: conditional-remediation scenario + step defs and catalog contract updated Signed-off-by: Roman Sirokov <roman.sirokov@amd.com>

Review feedback on #290 flagged that KEYWORDS_VLLM_OOM never required anything vLLM-specific: 'torch.OutOfMemoryError: CUDA out of memory' scores 45+45=90 (high confidence) under the keyword table alone, so any ROCm PyTorch job's OOM would be misreported as a vLLM startup OOM with rocm-serve-only remediation. gpu_memory_utilization -- the one vLLM-specific token -- was weighted lowest and never required. check_16_vllm_oom now requires an explicit vLLM anchor (the word 'vllm', or one of its distinctive flags: gpu[-_]memory[-_]utilization, tensor[-_]parallel) before the keyword table is scored at all. Update the serve OOM hint's suggested --symptom text and the matching e2e-cucumber step to carry that anchor so the self-referential 'rocm serve' -> 'rocm diagnose' flow keeps working, and add a regression test for the reported false positive. Also fix a second issue from the same review: the diagnose() WSL branch dropped sub-threshold hits from matched entirely when nothing cleared MIN_SCORE_FOR_MATCH, which the DiagnoseReport::matched doc says should never happen. It now keeps whatever run_all_checks returns (empty only when no wsl-applicable checker fired at all) and adds a regression test. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com>

r0x0r · 2026-08-21T11:20:52Z

Thanks for the review — addressed both issues in bac9ce2 / rebased and pushed.

Generic PyTorch OOM misattributed to vLLM: check_16_vllm_oom now requires an explicit vLLM anchor (vllm, gpu_memory_utilization/gpu-memory-utilization, or tensor_parallel/tensor-parallel) before KEYWORDS_VLLM_OOM is even scored. torch.OutOfMemoryError: CUDA out of memory on its own no longer matches at all — added generic_pytorch_oom_without_a_vllm_signal_does_not_match as a regression test for exactly that case. The vLLM engine's serve-failure hint and the matching e2e-cucumber step were updated to include the anchor ('vllm: torch.OutOfMemoryError: ...') so the self-referential rocm serve → rocm diagnose --symptom flow keeps working.
Sub-threshold hits dropped in the WSL branch: fixed — diagnose() now keeps whatever run_all_checks returns on WSL (empty only when no wsl-applicable checker fired at all), instead of discarding nonzero sub-threshold matches in favor of the out-of-scope message. This restores the DiagnoseReport::matched contract ("every checker that fired at all lands here"). Added wsl_sub_threshold_vllm_signal_is_preserved_not_routed_out_of_scope to cover it.
Linux/WSL exit-code contract change: confirmed and worth calling out explicitly — yes, check_16_vllm_oom being registered for ["linux", "wsl"] means WSL2 is no longer unconditionally out of scope; a keyword match on WSL now returns a normal diagnosis rather than always routing to the WSL "out of scope" message. This is intentional (vLLM does run under WSL2), but the PR body undersold it by saying "gated to Linux" — I'll tighten that wording.
Conflict with feat(vllm): enhance OOM diagnostics and guidance in serve summary (eai-8059) #284 (log_tail_shows_oom / oom_utilization_hint): noted. feat(vllm): enhance OOM diagnostics and guidance in serve summary (eai-8059) #284 is still open/unmerged, so nothing to resolve yet, but since this PR just touched oom_utilization_hint's suggested --symptom text again, whichever of feat(vllm): enhance OOM diagnostics and guidance in serve summary (eai-8059) #284/feat(diagnose): add the vLLM out-of-memory failure mode to the catalog (EAI-8060) #290 lands second will need to rebase past the other's edit to that function.
Draft status: this PR is out of draft already (not something I changed); base gpu-out-of-memory (feat(vllm): Tackle out of memory errors (EAI-8058) #251) has since merged its own follow-up fix (fix(vllm): validate --gpu against sysfs GPU count and harden DRM fallback), and this branch is now rebased on top of that tip.

All of cargo test -p rocm-core --lib, cargo test -p rocm-engine-vllm --lib, cargo clippy -p rocm-core -p rocm-engine-vllm --all-targets -- -D warnings, and cargo check -p e2e-cucumber --tests pass locally after the rebase.

Review feedback on #290 flagged that KEYWORDS_VLLM_OOM never required anything vLLM-specific: 'torch.OutOfMemoryError: CUDA out of memory' scores 45+45=90 (high confidence) under the keyword table alone, so any ROCm PyTorch job's OOM would be misreported as a vLLM startup OOM with rocm-serve-only remediation. gpu_memory_utilization -- the one vLLM-specific token -- was weighted lowest and never required. check_16_vllm_oom now requires an explicit vLLM anchor (the word 'vllm', or one of its distinctive flags: gpu[-_]memory[-_]utilization, tensor[-_]parallel) before the keyword table is scored at all. Update the serve OOM hint's suggested --symptom text and the matching e2e-cucumber step to carry that anchor so the self-referential 'rocm serve' -> 'rocm diagnose' flow keeps working, and add a regression test for the reported false positive. Also fix a second issue from the same review: the diagnose() WSL branch dropped sub-threshold hits from matched entirely when nothing cleared MIN_SCORE_FOR_MATCH, which the DiagnoseReport::matched doc says should never happen. It now keeps whatever run_all_checks returns (empty only when no wsl-applicable checker fired at all) and adds a regression test. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com>

volen-silo · 2026-08-25T08:45:34Z

Round 2. The sub-threshold/matched fix is clean and the regression test targets the contract rather than the implementation — good. The #284 conflict I flagged is refuted: git merge-tree --write-tree --merge-base=a97a39f0 HEAD origin/eai-8059-oom-memory-knobs-note exits 0, the two branches edit the if condition and the format! body respectively. No rebase needed.

Two blocking:

rocm serve --tensor-parallel-size doesn't exist. fix.rs:379 and diagnose.rs:1343/:1355 all tell the user to run it. The Serve clap variant has no such field and there's no trailing-var-arg or engine-arg passthrough — the only injection point into vLLM's argv is EngineRecipeHint.required_flags, which nothing populates with it. Clap rejects the command. It also contradicts docs/vllm.md:90 ("Serving one model across multiple GPUs is not supported", enforced at main.rs:16687 and rocm-engine-protocol/src/lib.rs:840). So "Case 2 — shard it across more GPUs" isn't actionable through this CLI in any form. diagnose_steps.rs:379 only asserts the literal string --tensor-parallel-size appears in the rendered fix, so it can never catch this. --gpu-memory-utilization and --gpu <index> are both genuine — only Case 2 is fabricated.
The tensor[-_]parallel anchor alternative reopens the round-1 false-positive class at full confidence. vllm and gpu[-_]memory[-_]utilization are genuinely vLLM-specific; tensor_parallel is standard vocabulary in Megatron-LM, DeepSpeed, torch.distributed.tensor.parallel, HF Accelerate, SGLang and TensorRT-LLM. Hand-simulating the anchor plus the top-2 rule against the actual table:
```
Initializing tensor_parallel_size=8 ... torch.OutOfMemoryError: CUDA out of memory. Tried to allocate 2.00 GiB.
  anchor hit, score 90 (torch.outofmemoryerror 45 + cuda out of memory 45) -> HIGH_CONFIDENCE

megatron: tensor-parallel rank 3: HIP out of memory. Tried to allocate 2.00 GiB
  anchor hit, score 75 (hip out of memory 45 + tried to allocate...gib 30) -> HIGH_CONFIDENCE
```
A Megatron/DeepSpeed training crash gets "vLLM ran the GPU out of memory at startup" with rocm serve-only remediation. Dropping that alternative closes it — I couldn't construct a realistic non-vLLM symptom that clears threshold via the bare vllm substring alone.

Smaller things:

a_bare_out_of_memory_stays_below_the_match_threshold guards with if let Some(d) — if the checker ever stops firing on that input, the body never runs and the test passes asserting nothing. Its two siblings correctly .expect(...) first.
The gpu_memory_utilization keyword row is underscores-only while the anchor accepts gpu[-_]memory[-_]utilization. A symptom using the hyphenated CLI spelling — the spelling this PR's own recipe prints — passes the anchor and scores 0 on that row.
diagnose_steps.rs:350 hardcodes score >= 75; the report publishes high_confidence_threshold, and assert_verdict_follows_scores in the same file reads min_score_for_match off the report with a comment explaining why hardcoding is wrong.
The two new Then steps repeat the same find-by-id block, and :374 does fix.to_string() + four .contains() over the whole serialized object while the next assertion correctly scopes to fix.get("verify").
The suggested --symptom in engines/vllm/src/lib.rs:1833 is a canned literal engineered to score 90 against the new table, while the function already holds the real log_tail. As written the round-trip demonstrates the matcher against its own fixture rather than routing the user's actual error.
Body says "exit-code contract change" — main.rs:2198 states and implements that diagnose exits 0 whether it matched, found nothing, or is out of scope. The output contract change on WSL is real; the exit code isn't.
Commit 6097efbc's message still says "gated to Linux" / "linux-only"; the body was corrected but the message wasn't.
AGENTS.md §11 wants stacked PRs in draft until dependencies merge; feat(vllm): Tackle out of memory errors (EAI-8058) #251 is still open and this is out of draft, while the body claims it "stays in draft until feat(vllm): Tackle out of memory errors (EAI-8058) #251 merges".
docs/vllm.md:135 describes the serve-failure message this PR extends, without the new --symptom pointer or the catalog entry.
tests/e2e-cucumber/src/expectation.rs:65 justifies @requires-bare-metal with "a scenario needing a catalog match has no premise" on WSL2 — now stale.
"torch.outofmemoryerror" leaves . unescaped; every other literal-dot pattern in the file is a raw string with \..

One worth confirming as deliberate: on WSL, any nonzero fix-16 score (a bare "vllm ... out of memory" scores 25) now sets out_of_scope to None, so the renderer prints a WEAK entry with apply with: rocm fix fix-16-vllm-oom and the /dev/dxg+WSL-docs routing message disappears. Keeping weak hits in matched is right; coupling out_of_scope to matched.is_empty() rather than to "nothing cleared MIN_SCORE_FOR_MATCH" is the part I'd double-check.

@requires-os:linux is the right tag here, not @requires-bare-metal — WSL2 reports os_family: "linux", so the scenario covers Linux and WSL and skips only native Windows.

… the vLLM OOM anchor Round-2 review (EAI-8060): - Remove the `--tensor-parallel-size` / multi-GPU sharding remediation from both the fix catalog (fix-16-vllm-oom) and the diagnose summary: the flag does not exist and rocm-cli serves one model on a single GPU (docs/vllm.md). The 'model does not fit' branch now points only at a smaller/quantized model. - Drop `tensor[-_]parallel` from VLLM_ANCHOR_PATTERN: it is a Megatron/DeepSpeed term, so anchoring on it would misattribute those frameworks' OOMs to vLLM. The anchor is now just `vllm|gpu[-_]memory[-_]utilization`. - Route the user's *actual* failing log line into the `rocm diagnose --symptom` hint (vllm-anchored) instead of a canned literal. - Escape the dot in the `torch\.outofmemoryerror` keyword regex and let the `gpu[-_]memory[-_]utilization` keyword accept a hyphen like the anchor. - Tests: e2e reads high_confidence_threshold from the report instead of a hardcoded 75, locks out `--tensor-parallel-size`, and shares a find_vllm_oom helper; the sub-threshold unit test uses `.expect()` instead of a vacuous `if let`. - docs/vllm.md and the @requires-bare-metal doc note the `rocm diagnose --symptom` pointer and the WSL keyword-only exception. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com>

r0x0r · 2026-08-25T10:22:20Z

Thanks for the Round-2 review — pushed d233af7 addressing it.

rocm serve --tensor-parallel-size doesn't exist / contradicts single-GPU serving. Removed the sharding remediation from both surfaces (fix-16-vllm-oom in fix.rs and the check_16_vllm_oom summary/commands in diagnose.rs). The flag doesn't exist and docs/vllm.md states one model is served on a single GPU. The "model genuinely does not fit" branch now points only at a smaller/quantized model.

tensor[-_]parallel anchor reopened false positives. Dropped that alternative from VLLM_ANCHOR_PATTERN; it's a Megatron/DeepSpeed term, so anchoring on it would misattribute those frameworks' OOMs to vLLM. The anchor is now vllm|gpu[-_]memory[-_]utilization (comment updated to explain the omission).

Smaller items:

a_bare_out_of_memory_stays_below_the_match_threshold now uses .expect() instead of a vacuous if let Some (the weak out of memory hit is always present in matched).
The gpu_memory_utilization keyword now accepts a hyphen (r"gpu[-_]memory[-_]utilization") to match the anchor.
diagnose_steps.rs reads high_confidence_threshold from the report instead of a hardcoded 75, and the two new Then steps share a find_vllm_oom helper.
The --symptom hint in engines/vllm now routes the user's actual failing log line (vllm-anchored) instead of a canned literal.
Escaped the dot in the torch\.outofmemoryerror keyword regex.
docs/vllm.md now points at rocm diagnose --symptom '<the error>'; the @requires-bare-metal doc-comment notes the WSL keyword-only exception (the vLLM OOM entry does run on WSL, so it belongs under @requires-os:linux).

Two notes:

The change is to the output contract, not an exit-code one — rocm diagnose still always exits 0. I've corrected the body wording accordingly.
One item I deliberately left as-is (want your call): the WSL branch in diagnose() sets out_of_scope from wsl_matches.is_empty() rather than keying off MIN_SCORE_FOR_MATCH. The current code keeps sub-threshold hits in matched (per the DiagnoseReport::matched contract) with out_of_scope = None; keying off the threshold would make a sub-threshold-only WSL result both populate matched and carry the out-of-scope message. Happy to flip it if you'd rather.

Signed-off-by: Roman Sirokov <roman.sirokov@amd.com>

volen-silo

LGTM.

Round 3. Both round-2 blockers are genuinely fixed, not just claimed: --tensor-parallel-size is gone from the recipe, the diagnose summary and the commands, with a negative e2e assertion locking it out; the anchor is down to vllm|gpu[-_]memory[-_]utilization, and re-simulating the Megatron/DeepSpeed cases confirms they now fail the anchor before the table is scored at all. I re-checked the surviving prescriptions against the clap definitions (--gpu-memory-utilization, --gpu <index> both real), re-derived scores by hand for the anchored / unanchored / sub-threshold cases, confirmed applies_on: LINUX_ONLY still lets rocm fix fix-16-vllm-oom run under WSL2 (current_os() has no wsl concept), and ran fmt, clippy, rocm-core, rocm-engine-vllm and cargo check -p e2e-cucumber --tests — all clean.

Your open question — out_of_scope keyed off wsl_matches.is_empty() rather than MIN_SCORE_FOR_MATCH: keep it as-is. On WSL only fix-16 can fire, and it cannot fire without the vLLM anchor, so the only symptom that suppresses the /dev/dxg routing is one that literally names vLLM or its reservation flag. A topical WEAK entry beats printing both that entry and "out of scope for this platform", which would contradict itself.

Nits (non-blocking)

PR body still reads "exit-code/output contract change". rocm diagnose always exits 0 (main.rs), so only the output contract changed — and since this repo squash-merges, the body becomes the permanent commit message. Commit 6097efbc's body still says "gated to Linux" / "(linux-only)" against its own &["linux", "wsl"] registration, though that one won't survive the squash.
oom_utilization_hint interpolates the raw log line into --symptom '<line>' with no quote escaping; an apostrophe breaks the copy-paste. Display-only, nothing is executed, and lines are uncapped in length.
A line matching only the bare out of memory phrase (e.g. RuntimeError: HIP error: out of memory) scores 25, so the command the serve hint just handed the user comes back WEAK. render_report_text still prints the whole remediation for sub-threshold entries, so it is a mislabeled tier rather than a lost answer — but the round trip the hint advertises is not guaranteed by the keyword table.
torch\.outofmemoryerror does not match the older fully-qualified torch.cuda.OutOfMemoryError; harmless today because hip out of memory + tried to allocate ... GiB still reaches 75.
.unwrap_or("out of memory") in oom_utilization_hint is unreachable: a literal containing no newline cannot span a lines() boundary, so the guard above it already guarantees a hit.
Recipe examples say --gpu-memory-utilization 0.5 while the shared hint constant and docs/vllm.md use 0.1.
Still out of draft while its base (#251) is open, which AGENTS.md §11 asks against.

…g (EAI-8060) (#290) * feat(diagnose): add vLLM out-of-memory failure mode to the catalog (EAI-8060) Add a keyword-scored OOM signature to the rocm-core diagnosis catalog and a matching print-only remediation recipe. The error covers two distinct faults (a tenancy collision with vLLM's fixed ~90% VRAM reservation, versus a model that genuinely does not fit), so the wording stays conditional: it never prescribes lowering --gpu-memory-utilization as the unconditional answer, and the verify step avoids the tenancy knob. Because an Examination carries no per-GPU VRAM or tenancy fields, the match is keyword-only and the checker is gated to Linux (vLLM is Linux/WSL-only). - diagnose.rs: KEYWORDS_VLLM_OOM table + check_16_vllm_oom (linux-only) - fix.rs: print-only fix-16-vllm-oom recipe; catalog count 15 -> 16 and AUTO-set assertion strengthened - engines/vllm: serve OOM hint points users at 'rocm diagnose --symptom' - e2e-cucumber: conditional-remediation scenario + step defs and catalog contract updated Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * fix(diagnose): require a vLLM anchor before matching the OOM checker Review feedback on #290 flagged that KEYWORDS_VLLM_OOM never required anything vLLM-specific: 'torch.OutOfMemoryError: CUDA out of memory' scores 45+45=90 (high confidence) under the keyword table alone, so any ROCm PyTorch job's OOM would be misreported as a vLLM startup OOM with rocm-serve-only remediation. gpu_memory_utilization -- the one vLLM-specific token -- was weighted lowest and never required. check_16_vllm_oom now requires an explicit vLLM anchor (the word 'vllm', or one of its distinctive flags: gpu[-_]memory[-_]utilization, tensor[-_]parallel) before the keyword table is scored at all. Update the serve OOM hint's suggested --symptom text and the matching e2e-cucumber step to carry that anchor so the self-referential 'rocm serve' -> 'rocm diagnose' flow keeps working, and add a regression test for the reported false positive. Also fix a second issue from the same review: the diagnose() WSL branch dropped sub-threshold hits from matched entirely when nothing cleared MIN_SCORE_FOR_MATCH, which the DiagnoseReport::matched doc says should never happen. It now keeps whatever run_all_checks returns (empty only when no wsl-applicable checker fired at all) and adds a regression test. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * diagnose: drop the non-existent multi-GPU sharding remedy and tighten the vLLM OOM anchor Round-2 review (EAI-8060): - Remove the `--tensor-parallel-size` / multi-GPU sharding remediation from both the fix catalog (fix-16-vllm-oom) and the diagnose summary: the flag does not exist and rocm-cli serves one model on a single GPU (docs/vllm.md). The 'model does not fit' branch now points only at a smaller/quantized model. - Drop `tensor[-_]parallel` from VLLM_ANCHOR_PATTERN: it is a Megatron/DeepSpeed term, so anchoring on it would misattribute those frameworks' OOMs to vLLM. The anchor is now just `vllm|gpu[-_]memory[-_]utilization`. - Route the user's *actual* failing log line into the `rocm diagnose --symptom` hint (vllm-anchored) instead of a canned literal. - Escape the dot in the `torch\.outofmemoryerror` keyword regex and let the `gpu[-_]memory[-_]utilization` keyword accept a hyphen like the anchor. - Tests: e2e reads high_confidence_threshold from the report instead of a hardcoded 75, locks out `--tensor-parallel-size`, and shares a find_vllm_oom helper; the sub-threshold unit test uses `.expect()` instead of a vacuous `if let`. - docs/vllm.md and the @requires-bare-metal doc note the `rocm diagnose --symptom` pointer and the WSL keyword-only exception. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * style: rustfmt the new OOM --symptom routing test assertion Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> --------- Signed-off-by: Roman Sirokov <roman.sirokov@amd.com>

* feat(vllm): add GPU memory utilization hint for OOM errors and improve documentation Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * fix(vllm): validate --gpu against sysfs GPU count and harden DRM fallback Address review feedback on PR #251: - resolve_gpu_indices now validates an explicit --gpu <index> against the DRM sysfs fallback's device count when amd-smi is unavailable, instead of only the amd-smi count. Previously an out-of-range index was silently accepted on a host with no amd-smi (sysfs works) and only failed later inside the engine. - read_drm_vram_usage no longer treats an unreadable mem_info_vram_used counter as 0 bytes used (which made the card look 100% free -- exactly what --gpu auto prefers first); it now skips that card instead. - read_drm_vram_usage withholds telemetry entirely when more than one AMD DRM card is present, since ascending card<N> order is only guaranteed to match HIP's compute-topology ordinal on a single-GPU host (an APU passes the same vendor + mem_info_vram_total filter as a discrete GPU, so an APU+dGPU host could previously feed a diverged ordinal into HIP_VISIBLE_DEVICES). - docs/vllm.md: corrected the fallback description, which only ever probes amd-smi (never rocm-smi) before falling back to DRM sysfs. Extracted the count-fallback logic into a new effective_gpu_count helper shared by --gpu auto ranking and --gpu <index> validation, and added unit tests for all of the above. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * fix(vllm): validate --gpu against amd-smi-independent count; harden DRM guard Round-2 review follow-ups on the DRM sysfs GPU-selection fallback. - Validate an explicit --gpu <index> against the amd-smi list count or, when amd-smi is absent, the KFD/DRM authority behind usable_amd_gpu_indices() (pinned_index_bound) instead of the DRM VRAM-fallback row count. The row count withholds telemetry on multi-GPU hosts and shrinks on a transient counter-read failure, so borrowing it as a validation bound hard-rejected legitimate indices. VRAM rows stay scoped to --gpu auto ranking. - The multi-card ordinal-ambiguity guard now counts AMD cards *found* (before the telemetry filters) rather than surviving rows, so a second AMD card with an unreadable counter still trips the guard instead of mislabelling the survivor ordinal 0 (the APU+dGPU misattribution the guard exists to prevent). - drm_device_is_amd now matches on vendor id OR an amdgpu uevent DRIVER line, the same two-signal test as rocm_core::is_amdgpu_device, so the fallback probe and the count authority agree on what an AMD card is. - Drop the now-dead sort/enumerate tail in read_drm_vram_usage (at most one card survives the guard) and fix the stale doc comment. - Tests: exercise the real Index-arm delegate (resolve_pinned_gpu_index) and add a two-AMD-card-with-unreadable-counter regression; add pinned_index_bound coverage. - Docs: README and the assistant SKILL note the DRM sysfs VRAM fallback rather than presenting amd-smi as the only source. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * feat(diagnose): add the vLLM out-of-memory failure mode to the catalog (EAI-8060) (#290) * feat(diagnose): add vLLM out-of-memory failure mode to the catalog (EAI-8060) Add a keyword-scored OOM signature to the rocm-core diagnosis catalog and a matching print-only remediation recipe. The error covers two distinct faults (a tenancy collision with vLLM's fixed ~90% VRAM reservation, versus a model that genuinely does not fit), so the wording stays conditional: it never prescribes lowering --gpu-memory-utilization as the unconditional answer, and the verify step avoids the tenancy knob. Because an Examination carries no per-GPU VRAM or tenancy fields, the match is keyword-only and the checker is gated to Linux (vLLM is Linux/WSL-only). - diagnose.rs: KEYWORDS_VLLM_OOM table + check_16_vllm_oom (linux-only) - fix.rs: print-only fix-16-vllm-oom recipe; catalog count 15 -> 16 and AUTO-set assertion strengthened - engines/vllm: serve OOM hint points users at 'rocm diagnose --symptom' - e2e-cucumber: conditional-remediation scenario + step defs and catalog contract updated Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * fix(diagnose): require a vLLM anchor before matching the OOM checker Review feedback on #290 flagged that KEYWORDS_VLLM_OOM never required anything vLLM-specific: 'torch.OutOfMemoryError: CUDA out of memory' scores 45+45=90 (high confidence) under the keyword table alone, so any ROCm PyTorch job's OOM would be misreported as a vLLM startup OOM with rocm-serve-only remediation. gpu_memory_utilization -- the one vLLM-specific token -- was weighted lowest and never required. check_16_vllm_oom now requires an explicit vLLM anchor (the word 'vllm', or one of its distinctive flags: gpu[-_]memory[-_]utilization, tensor[-_]parallel) before the keyword table is scored at all. Update the serve OOM hint's suggested --symptom text and the matching e2e-cucumber step to carry that anchor so the self-referential 'rocm serve' -> 'rocm diagnose' flow keeps working, and add a regression test for the reported false positive. Also fix a second issue from the same review: the diagnose() WSL branch dropped sub-threshold hits from matched entirely when nothing cleared MIN_SCORE_FOR_MATCH, which the DiagnoseReport::matched doc says should never happen. It now keeps whatever run_all_checks returns (empty only when no wsl-applicable checker fired at all) and adds a regression test. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * diagnose: drop the non-existent multi-GPU sharding remedy and tighten the vLLM OOM anchor Round-2 review (EAI-8060): - Remove the `--tensor-parallel-size` / multi-GPU sharding remediation from both the fix catalog (fix-16-vllm-oom) and the diagnose summary: the flag does not exist and rocm-cli serves one model on a single GPU (docs/vllm.md). The 'model does not fit' branch now points only at a smaller/quantized model. - Drop `tensor[-_]parallel` from VLLM_ANCHOR_PATTERN: it is a Megatron/DeepSpeed term, so anchoring on it would misattribute those frameworks' OOMs to vLLM. The anchor is now just `vllm|gpu[-_]memory[-_]utilization`. - Route the user's *actual* failing log line into the `rocm diagnose --symptom` hint (vllm-anchored) instead of a canned literal. - Escape the dot in the `torch\.outofmemoryerror` keyword regex and let the `gpu[-_]memory[-_]utilization` keyword accept a hyphen like the anchor. - Tests: e2e reads high_confidence_threshold from the report instead of a hardcoded 75, locks out `--tensor-parallel-size`, and shares a find_vllm_oom helper; the sub-threshold unit test uses `.expect()` instead of a vacuous `if let`. - docs/vllm.md and the @requires-bare-metal doc note the `rocm diagnose --symptom` pointer and the WSL keyword-only exception. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * style: rustfmt the new OOM --symptom routing test assertion Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> --------- Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * fix(serve): validate --gpu against the usable-device set, not its length (EAI-8058) When amd-smi is unavailable, `--gpu <index>` validation fell back to the *length* of usable_amd_gpu_indices(). That set holds absolute, unrenumbered ordinals after the visibility mask, so on a masked multi-GPU host (e.g. HIP_VISIBLE_DEVICES=2 on a 4-GPU box -> usable [2]) the length-1 bound rejected --gpu 2 (the sole usable device) and accepted a hidden --gpu 0. Replace the count bound with an injectable validate_pinned_gpu_index_against that prefers the amd-smi count and otherwise checks index membership in the usable set -- the same authority the engine device gate uses. The detected/usable seam makes both branches unit-testable without hardware. Also correct two docs/vllm.md inaccuracies (note timing; missing --engine vllm in the workaround examples). Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * docs(vllm): remove duplicate code-fence opener in OOM workaround example A merge-conflict resolution left two consecutive ```bash fence openers before the shared-card OOM workaround example, so the block opened and immediately reopened on the literal text and the commands rendered outside a code block. Drop the duplicate opener so the example renders as a single fenced block. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * serve: address OOM diagnosis review — decouple engine hint from scorer, dedup keyword spans, anchor per-line Reviewer follow-ups on the vLLM OOM diagnosis path: - Engine no longer hard-codes `vllm: <line>` into the printed `rocm diagnose --symptom`. It now consults rocm_core::vllm_oom_symptom_is_diagnosable and falls back to VLLM_OOM_CANONICAL_SYMPTOM when the user's actual line would not score, so the command it prints always reports a cause. The user's real line stays visible in the human-readable message. Adds a loop-invariant test that every emitted --symptom value is diagnosable. - keyword_score now de-duplicates overlapping regex spans before taking the top two, so a phrase nested inside a stronger one is not double-counted. The VLLM_OOM table splits allocator *messages* (score 50, clear alone) from the exception *class* name (45, sub-threshold). - vLLM OOM anchor must appear on the same line as an OOM token, not merely somewhere in a pasted log, closing a misattribution gap. - WSL out-of-scope routing is gated on has_match rather than a non-empty match list, so a sub-threshold signal is preserved and still routed out. - serve --gpu <index> membership rejection now says "not available" and lists the usable set, matching the engine wording and the e2e contract. - --gpu auto ranks the actual reported VRAM row indices (which can be non-contiguous under a visibility mask) instead of a synthetic 0..count. - diagnosis Fix for shared fix ids (fix-15, fix-16) is cross-checked against the fix catalog so verify/notes/commands cannot silently diverge. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * test(vllm): cover serve-plan low-VRAM OOM guidance on the GPU lane (EAI-8058) The serve plan's pre-launch low-VRAM warning and its vLLM `--gpu-memory-utilization` note had unit coverage but no end-to-end scenario, so the user-observable behavior on a real serve went unverified. The plan note is reached only when a *selected* GPU is below the free-VRAM bar, which the GPU lane's genuinely-free cards never trip. Add an e2e-test-hooks-gated seam in gpu_vram_usage() (ROCM_E2E_FORCE_LOW_VRAM=<ordinal>) that reports a single near-full synthetic device, making the warning deterministic on the non-APU vLLM lane without touching the device the engine actually launches on. Add serve-19 (@requires-gpu @requires-engine:vllm @requires-os:linux) asserting the plan warns about low VRAM and names the memory-utilization knob, with its step definitions and a unit test for the seam. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * test(e2e): give the low-VRAM serve scenario its runtime precondition (EAI-8058) serve-19 (@id:serve-vllm-low-vram-oom-guidance) is the only real-serve scenario on the GPU lane that does not open with `a managed runtime is active`. It runs `rocm serve ... --managed` in a fresh isolated root, so the GPU-required device policy refuses before a plan is ever built: Error: device_policy: gpu_required; no active ROCm runtime is configured; run `rocm runtimes list` and `rocm runtimes activate <runtime_key>`, or pass --runtime-id/--env-id That is the single unexpected failure in the E2E tests (GPU) lane on this branch (the other five are declared xfail), so the scenario added to cover the pre-launch OOM note never actually exercised it. Add the same precondition serve-05/06/07 use; the `ROCM_E2E_FORCE_LOW_VRAM` Given stays second so the runtime setup's own `rocm` calls do not drain command_env. Also correct auto_selection_uses_vram_row_count_when_amd_smi_count_is_unknown, which attributed a three-row fixture to the DRM sysfs fallback. That fallback withholds telemetry once a second AMD card is found and so emits at most one row; a multi-row `detected == None` state is amd-smi `metric` answering while `list` does not. Relabel it and add the single-row case the sysfs fallback can actually reach. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * fix(diagnose): scope keyword overlap de-duplication to the vLLM OOM table The overlapping-span de-duplication added for `KEYWORDS_VLLM_OOM` was applied by `keyword_score` to all 16 keyword tables. Several older tables pair a greedy `.*` pattern with a second, genuinely independent keyword, and the greedy match span swallows it: `api-ms-win-crt-.*\.dll` runs to the last `.dll` on the line and covers an independent `msvcp140.dll`, `dkms .*failed` covers an independent `dpkg: error`, `amdhip64.*not found` covers `could not find hip`, and `404.*repo\.radeon\.com` covers `unable to locate package rocm`. Collapsing can only lower a score, so those pairs lost their second signal and dropped below MIN_SCORE_FOR_MATCH or HIGH_CONFIDENCE. On a Windows host with no PATH probe result, the symptom "api-ms-win-crt-runtime-l1-1-0.dll missing and msvcp140.dll not found" scored 35 instead of 65 and fix-15-msvc-redist disappeared from the report entirely. Keep the default "every matching entry is an independent signal" scoring for the catalog and route only `KEYWORDS_VLLM_OOM` — whose overlaps are true nestings of literal phrases rather than wildcard artifacts — through the collapsing variant. Pin the four pre-change scores plus the user-visible fix-15 outcome so the dedup cannot be re-generalized. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * fix(diagnose): score only the vLLM-anchored lines, not the whole symptom The same-line anchor was a boolean gate: `vllm_oom_line_is_anchored` returned true if any one line carried the anchor together with an OOM token, and `check_16_vllm_oom` then scored the *entire* symptom. Regex matching ignores line boundaries, so keyword hits on unanchored lines still counted at full weight, which is exactly the misattribution the anchor exists to prevent. A paste of vllm config: gpu_memory_utilization=0.9 llama.cpp: torch.OutOfMemoryError: CUDA out of memory passed the gate on the benign config echo, then scored `cuda out of memory` (50) plus `torch.outofmemoryerror` (45) off the llama.cpp line — 95, above HIGH_CONFIDENCE — and reported another framework's failure as a confident "vLLM ran the GPU out of memory". Replace the gate with `vllm_anchored_lines`, which returns the anchored lines joined by newlines, and score that. The joins are by `\n` and the table's patterns are literals or `.*` (which does not cross a newline), so dropping unanchored lines cannot fabricate a match across the seam. The same-line requirement now falls out of the scoring itself: an anchored line with no OOM token contributes nothing. `gpu[-_]memory[-_]utilization` stays an anchor despite also being a 20-point scoring entry: with only anchored lines scored, a bare flag echo (a config dump, or rocm-cli's own low-VRAM hint) is worth 20 — far below MIN_SCORE_FOR_MATCH — so it can surface as a weak signal but can never carry a verdict, nor lend its anchor to an OOM elsewhere in the paste. Scoring is otherwise unchanged: only `check_16_vllm_oom` is touched, so no other catalog table moves. Refs: EAI-8058 Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * docs(vllm): align the OOM reservation example with the fix recipe The catalog recipe and the --gpu-memory-utilization parse error both use 0.5; the docs said 0.1 for the same shared-GPU case. A 0.1 budget is below the weights of most models people serve, so it trades one startup failure for another. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * test(e2e): make the low-VRAM serve scenario assert the new guidance (EAI-8058) The scenario's warning step matched "serving may fail on VRAM", which predates this change, and its sibling matched only the bare flag name --- which a recipe echo of a user-supplied --gpu-memory-utilization would also satisfy. Neither fragment is specific to the pre-launch hint this scenario exists to prove. Require the warning to name the pinned GPU and the note to carry vLLM's total-VRAM reservation rationale, and pin both literals in the always-run unit test so a rewording fails on every lane rather than only on the GPU lane that can observe the scenario. Also record on serve-16 why the masked, non-contiguous usable-set case behind the --gpu membership rule has no scenario of its own: it is reachable only where amd-smi is absent, so a scenario would assert against an injected seam rather than the product. It stays unit-covered. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * test(diagnose): retire the WSL out-of-scope routing assertions The WSL2 catalog (#340) makes WSL a covered platform, so `diagnose` no longer emits an out-of-scope routing note there. Two tests written against the old behaviour were asserting that note, and one of them also assumed a default WSL examination produces no match -- it now legitimately does, since a host with no /dev/dxg really is a `fix-wsl-1-gpu-not-exposed` finding. Both now build on `wsl_base()`, the healthy-WSL fixture, so the vLLM OOM signal is the only thing under test. The load-bearing half is unchanged and still asserted: a score-25 `out of memory` mention stays visible in `matched` as a WEAK row and does not promote itself into a match. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * fix(fix): let fix-16-vllm-oom run on WSL2, where its checker answers `check_16_vllm_oom` is registered for `["linux", "wsl"]` -- deliberately, per the comment at diagnose.rs -- but the matching recipe was `LINUX_ONLY`. `rocm fix` re-gates a fix-id on the *recipe's* platform list against the running OS, where WSL2 is its own family, so on WSL2 `rocm diagnose` printed `apply with: rocm fix fix-16-vllm-oom` and that command then printed the plan and exited 3 with "This fix only applies on: linux. Running OS is: wsl." Nothing about the remediation is bare-metal: it is vLLM CLI flags. WSL2 is the supported way to serve from a Windows host and no `fix-wsl-*` entry covers the failure mode, so the platform had a diagnosis and no working fix path. The class, not just the instance: the two platform lists are written in separate modules and each looks right alone. Add `every_checker_platform_is_covered_by_its_recipe`, which walks `CHECKERS` and requires every family a checker answers on to be one the recipe will act on. Containment rather than equality -- a recipe may legitimately apply more widely than its checker fires, since `rocm fix <id>` is reachable without a diagnosis. It found no other mismatch across the remaining 23 entries. The loop is user-observable, so diagnose-20 gains a step that runs the fix-id the report named and requires the success rc (the wrong-OS refusal prints the recipe first, so output alone cannot tell the outcomes apart). It is `@requires-os:linux`, which WSL2 matches, so the `e2e-wsl` self-hosted lane is the one that can observe the difference; every Linux lane runs the step. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * fix(core): the shared OOM hint recommended 0.1 while every command said 0.5 `VLLM_GPU_MEMORY_UTILIZATION_HINT` still carried "(e.g. 0.1 for a small model)". It is printed verbatim by `rocm serve`'s low-VRAM note and the vLLM engine's post-failure OOM hint, and interpolated into the `fix-16-vllm-oom` diagnosis summary -- so one `rocm diagnose` printed "e.g. 0.1 for a small model" directly above `rocm serve <model> --gpu-memory-utilization 0.5`. 0.5 is the settled value; every command surface already used it (the recipe, the diagnosis Fix, docs/vllm.md). 2c3a84e moved the docs and left the const behind, and that commit's own message says why 0.1 is wrong: a 0.1 budget is below the weights of most models people serve, so it trades one startup failure for another. The harmful advice was what the product actually printed. Pin the two together with `the_utilization_hint_example_matches_the_recipe_command`, which compares the hint's worked value against the value the recipe commands. Neither existing guard covers this: the diagnose/fix cross-check exempts the summary as prose framing, and a line-based grep misses the const because the flag and the value sit on different continuation lines -- which is exactly how it was missed the first time. Also correct the const's own doc claim, which named two sharing surfaces when there are three, and record that the catalog rationale is deliberately separate prose whose worked value is nonetheless pinned to this one. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * fix(diagnose): drop the duplicate LINUX_AND_WSL const from the merge Both branches added an identical const — this branch for the fix-16 WSL gate, main for #386 — at different offsets, so git merged both copies without a conflict marker and the lib test failed to compile (E0428). Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * fix(vllm): keep untrusted log text out of the suggested diagnose command On a failed vLLM serve the post-failure hint printed the user's own failing log line inside a single-quoted `rocm diagnose --symptom '...'` command it invites them to paste into a shell. The line is the vLLM subprocess's own startup-log tail and nothing escaped it, so a routine apostrophe in Python error text ("can't allocate", "model 'foo'") closed the quote: at best the command the feature exists to provide was syntactically broken, at worst text the user does not control landed outside the quotes. The neighbouring human-readable echo passed the same bytes through verbatim, ANSI and all. A line that cannot be rendered as one intact single-quoted word is now treated as unusable and takes the fallback to VLLM_OOM_CANONICAL_SYMPTOM that already existed for exactly that case, rather than being escaped: escaping preserves the bytes but yields a command a reader cannot check by eye, and a wrong escape is runnable and misleading instead of obviously broken. The echoed line is stripped of ANSI sequences and control bytes. Also restores the `out_of_scope` doc comment to what `diagnose` does. It had been changed to claim `matched` may hold sub-threshold entries alongside it and that the field is set when nothing clears MIN_SCORE_FOR_MATCH; neither holds — `matched` is cleared whenever the field is set, and the field is decided purely by platform coverage. The WSL2 example can no longer occur now that WSL2 is a covered platform. That is the documented JSON contract, so a consumer trusting it waits for rows that can never arrive. The relationship is now asserted. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * test(vllm): pin the exact rendering instead of fragment absence The control-byte test carried the message "the ANSI sequence must be removed whole, not just its escape byte" but only asserted that "[31m" and "[0m" were absent. A stripper that drops the escape byte and the "[" it introduces and nothing further leaves "31m"/"0m" behind, which satisfies both absence checks and carries no control byte, so the test passed under exactly the defect it named. Verified by running that mutant: green before, failing now. Assert the stripper's output and the full rendered hint by equality instead. The neighbouring quote-rejection test had the same shape -- "no apostrophe in the symptom" also holds if the apostrophes were silently deleted, the escaping-style rewrite the canonical fallback exists to avoid -- so pin the fallback symptom by equality too. That mutant was also green before and fails now. Tests only; no behaviour change. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * test(serve): pin the all-busy auto-GPU fallback to a reported ordinal `select_auto_gpu_index` now sources its candidates from the actual VRAM rows, because a visibility mask can make reported ordinals non-contiguous. That removed the invariant the terminal all-candidates-busy fallback quietly relied on: with candidates drawn from a dense `0..count`, ordinal 0 was always a real device, so returning a literal `0` was safe by construction. With rows reported at [2, 3] it is not — the busy filter empties the candidate list, every pass falls through, and a fabricated 0 goes straight into `HIP_VISIBLE_DEVICES`, pinning a device nothing ever reported. Under the strict GPU-required / no-silent-fallback policy that is a silently wrong pin, not a fail-fast. The merge resolution already routes the fallback through the reported, mask-filtered set, so the behaviour is correct. What was missing is coverage: `auto_selection_ranks_the_actual_reported_row_indices` claims pass 3 "never returns a fabricated index 0", but it only exercises the *partially* busy case (busy = [3]), which never reaches the fallback at all. This adds the all-busy case that does. Falsified: with the fallback reverted to `vec![0]` this test fails with left: [0], right: [2] (and takes main's `auto_selection_all_visible_busy_falls_back_to_lowest_visible_not_zero` down with it); both pass with the fallback restored. Equality assertion rather than fragment absence, so a future regression cannot pass it vacuously. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * docs(e2e): drop a serve-16 note the merge left dangling The comment on `serve-16` described `--gpu` validation as falling back to membership in the KFD/DRM usable set and pointed at `validate_pinned_gpu_index_falls_back_to_usable_set_membership` as the unit coverage for the masked sub-case. Merging #267 replaced that validator with one that consults the visibility-resolved set directly, so the named test no longer exists, and the masked sub-case it called uncovered now has two scenarios of its own (serve-19, serve-20). Nothing here changes behaviour or which lane runs what; the paragraph is rewritten to say what serve-16 still covers and to point at the scenarios that own the rest. The conflict was textual-merge-clean — this block came wholly from one side — so only reading it caught the dangling reference. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * fix(diagnose): treat a bare CR as a line boundary in the vLLM anchor filter `vllm_anchored_lines` split with `str::lines()`, which splits only on `\n` and `\r\n` -- a lone `\r` is ordinary text to it. Every progress bar in this ecosystem repaints with a bare CR (tqdm, pip, huggingface), and pasting a captured terminal scrollback is the expected way to use `rocm diagnose --symptom '...'`, so a real capture collapsed into one giant "line". The anchor then matched anywhere in it and every keyword in the paste scored at full weight: a llama.cpp OOM in a capture that also mentions vLLM came back at 95, well above HIGH_CONFIDENCE, as "vLLM ran the GPU out of memory at startup". That is the whole-paste scoring `only_the_anchored_lines_are_scored_not_the_ whole_paste` exists to prevent, reinstated by one byte. Neither existing guard saw it: both use `\n`-only fixtures. Split on `\r` as well, so the line boundary is the one the terminal actually renders, and add a regression case built from a bare-CR capture. It also pins the `\n` form of the same content and an anchored-across-CRs control, so the case stays a boundary question rather than a scoring one. EAI-8058 Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * fix(vllm): parse the escape grammar and reject format characters The OOM hint quotes a line of untrusted subprocess output into a `rocm diagnose --symptom '...'` command and echoes it in a `bail!`-ed message, so the sanitizer in front of it is load-bearing. Three holes: The escape scan ran to the next byte in `0x40..=0x7E` with no notion of the CSI grammar's parameter, intermediate and final classes. On well-formed colour codes that is indistinguishable from correct; on the malformed and truncated sequences a *killed* process emits -- which is the only case this code path exists for -- every one of them corrupted the message. A truncated CSI swallowed the next sequence's introducer (`ESC [ 1 ; 2 ESC [ 0 m Killed` rendered as `0mKilled`); a multi-byte scalar can never be in that ASCII range, so the scan ran past it and ate to the next byte that happened to land in it (`ESC [ 1 2 é out of memory` rendered as `ut of memory`); an OSC title leaked its body as text; a stray `ESC ESC` left `[0m` printed literally. Follow the grammar, and on an invalid byte stop *without consuming it*, so the damage is bounded to the malformed sequence and a following well-formed one is still recognised. `char::is_control` is Unicode category `Cc` only, so `Cf` format characters survived both the stripper and `quotable_in_single_quotes`. U+202E in a log line cannot escape the single quotes, but it does reverse how the printed command renders -- the same "untrusted output must not control what the terminal shows" concern, so it gets the same answer. The stripper's doc comment promised control bytes could not repaint the terminal "from inside rocm-cli's own error message", while the log tail a few lines above it in the same message went out raw. Sanitize the tail too, per line so it keeps its shape. Also states why `.rev()` is deliberate (the last OOM line is the proximate failure) and what it costs, since nothing said so; behaviour is unchanged. Tests: the existing control-byte test only exercised an `'m'`-terminated CSI, so narrowing the final-byte range to `'m'` left it green. The new case table pins the range, the byte classes and every introducer. The diagnosability test could not fail for the feature it is named for either -- a mutant that always returns VLLM_OOM_CANONICAL_SYMPTOM satisfies it, because that symptom is diagnosable by construction -- so it now pins the exact symptom per line, over a table that deliberately straddles the routing and fallback branches. EAI-8058 Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * test(diagnose): pin the fix-16 plan to its catalog copy The command block is maintained twice -- once on the `Fix` that `diagnose` attaches, once on the `rocm fix` catalog recipe -- and the two had already drifted. The cross-check that runs over every matched finding could not see it: `recipe_verify_notes_and_run_commands` filters `#` lines by construction, and only the comments differed. That prose is the point of this entry. Two different faults produce the same OOM and need opposite responses, so the comments are what tell the user which of the two each step addresses -- a user who meets the `diagnose` copy and a user who meets the `rocm fix` copy must be told the same thing. Align the `diagnose` copy to the catalog's Case 1/Case 2 wording and wire fix-16 into `assert_plan_matches_the_catalog_copy`, the byte-identical guard that already existed for fix-17-torch-dlpack. Also restores the general rule to `LINUX_AND_WSL`'s doc. Deduplicating the const in 1e23717d kept the fix-16-specific comment and dropped main's general one, leaving a shared const documented as if fix-16 were its only user; fix-19-shm-too-small uses it too. EAI-8058 Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * refactor(serve): drop a vacuous guard and the duplicated AMD-device test Three leftovers from the auto-selection rewrite and the merge around it: `select_auto_gpu_index`'s pass-2 `max_by` still carried `&& usage_for(index).is_some()`. Inside `if vram.is_some()` every candidate came from a reported VRAM row, so that is vacuously true -- it implies a failure mode (a selected ordinal telemetry never reported) that only existed when candidates were a synthetic `0..count` range. Replace it with a comment saying why no guard is needed. `drm_device_is_amd` re-implemented the private `rocm_core::is_amdgpu_device`, with a doc comment asserting the two were "the same two-signal test" and that divergence would let the multi-card ordinal guard slip -- a promise nothing enforced. They must agree because both count AMD cards and this one feeds that guard, so make the rocm-core function public and delegate. One copy is better than two copies plus a cross-check. The second assertion in `auto_selection_all_reported_busy_falls_back_to_a_ reported_ordinal` varied the order of the `busy` slice, which is only ever read through `.contains()` -- so it was a duplicate of the first while its comment claimed an order-independence property nobody was testing. Vary the order of the VRAM *rows* instead: `amd-smi` orders by enumeration, not by index, and that is what `reported.sort_unstable()` is there for. Dropping the sort now turns it red with `left: [3], right: [2]`; it could not before. EAI-8058 Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * fix(diagnose): split the vLLM anchor filter on rendered lines, not two bytes The previous round fixed the whole-paste collapse for a bare `\r` and left it open for every other line advance. `.split(['\n', '\r'])` is an allowlist of two characters under a doc comment claiming the result was "the line boundary the one the terminal actually renders", and it was not: with the `vllm` anchor and a llama.cpp OOM on genuinely separate rendered lines, `ESC E` (NEL), `ESC D` (IND), `CSI 1 B` (CUD), `\x0b`, `\x0c`, `\x85`, U+2028/U+2029 and any stray control byte each scored 95 against a `high_confidence_threshold` of 75 -- citing the llama.cpp line's tokens as the evidence for a vLLM verdict. `ESC E`, `ESC D` and `CSI n B` are terminfo's `nel`/`ind`/`cud1`, i.e. ordinary output from curses- and `rich`-style progress UIs, and `rocm serve`'s own failure path invites exactly this paste. The one-line `char::is_control()` split that suggests itself does close every one of those, but it trades the false positive for a false negative on the most ordinary input there is: a colourised logger emits SGR *inside* a line, so splitting at `ESC` cuts `ESC[31m vllm: ESC[1m torch.OutOfMemoryError` between its anchor and its error token. Measured on that candidate, that line went from 95 to 0. Getting both halves right needs the escape grammar, and this crate's dependents already have one -- `strip_terminal_control_sequences`, which took three rounds to get right in the vLLM engine. Rather than write a second parser, move the ECMA-48 walk down into `rocm-core::terminal` as a classifier over three tokens (`Text`, `LineBreak`, `Ignorable`) and give it two consumers: the engine's stripper, whose behaviour is unchanged, and a new `rendered_lines` the anchor filter splits on. The classification is deliberately lopsided rather than terminal-accurate, and the doc says so instead of claiming fidelity it does not deliver. Only SGR and the string-argument sequences (OSC/DCS/SOS/PM/APC) are `Ignorable`, because neither can move the cursor; `\t` stays intra-line text; everything else is a boundary. That is a one-sided guarantee -- two things drawn on different rows are never scored as one line, while one row may come back split -- and it is the right asymmetry here: merging rows produces a confident wrong cause, splitting one loses a diagnosis the canonical-symptom fallback already covers. A real terminal emulator is not decidable on a byte stream with no screen model (`CSI H` addresses a row that depends on everything before it), and over-splitting needs none of that. After: every separator above scores 0, `\n` and `\r` stay 0, and both the plain and the colourised genuine vLLM OOM stay at 95. Tests: the CR regression becomes a table over the separators rather than one test per byte, plus the SGR case in the other direction, plus a property test over the whole C0/C1 range pinned to `char::is_control` rather than to this module's own classifier. Reverting the boundary set turns three of them red (`nel esc-e`, `U+0000`, `a ESC E (NEL)-separated llama.cpp OOM ...`); making SGR a boundary instead turns the colour test red on `SGR inside a line must not change the verdict`. A confidently wrong cause is user-observable, so it is also pinned as `@id:diagnose-vllm-oom-not-attributed-across-rendered-lines` (diagnose-21), which fails on the pre-fix boundary set and passes after. EAI-8058 Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * chore(core): add the licence header the new terminal module was missing `fcf99156` created `crates/rocm-core/src/terminal.rs` without the SPDX header every other source file carries, so `License header check (hawkeye)` failed — its `licenserc.toml` includes `**/*.rs`. Header only; no code change. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * docs(core): scope the consumption rule and name the one stripper change Two prose defects in the module the previous commit added, both of which a reviewer would rely on. `next_token`'s doc asserted that "the contract here is 'render what the terminal would have rendered', which is the only rule that stays decidable on a byte stream with no framing". That was true of the stripper the paragraph came from, where the only question was how much of a well-formed sequence to consume. It is false of `next_token`, which also decides line boundaries and deliberately over-splits -- twenty-eight lines further down the same comment says a terminal emulator "is not decidable on a byte stream with no screen model", and the section between explains the classification is lopsided rather than terminal-accurate. Left standing, that sentence is a documented licence to "fix" the over-splitting toward terminal accuracy, which reopens the row-merging misattribution the previous commit closed. It is now scoped to the consumption decision it is actually about: a well-formed sequence is consumed whole because a real terminal consumes it too, and which token it is then labelled with is the separate, deliberately non-terminal-like question below. The same commit said the engine stripper's "behaviour is unchanged" by the move. It is unchanged except for U+2028/U+2029. The engine-local stripper classified non-escape characters with `is_control_or_format` alone, which is false for both (neither is `Cc`, neither is in the enumerated `Cf` set), so both survived into the stripped message; `classify_char` puts them in the boundary set, so they are now dropped. Measured on this head: the old code returns "a\u{2028}b" for input "a\u{2028}b", the new one returns "ab". The widening is the better behaviour -- a mandatory Unicode line break is exactly what this stripper exists to keep untrusted output from injecting -- but nothing stated it and no test pinned either scalar, so a silent drift back would have gone unnoticed. `classify_char` now says so, and the vLLM stripper table gains both cases: reverting the pair out of the boundary set turns `the_stripper_follows_the_escape_grammar_not_just_ the_colour_case` red on "a line separator must not survive into the message". Also from the same review, three prose corrections with no behaviour change: `rendered_lines`'s lead sentence now states the over-approximation up front instead of stating the strong property and retracting it a paragraph later; the string-argument arm documents the one crafted-input hole in the never-merge guarantee (a properly terminated OSC body containing a line feed is swallowed whole, so `"vllm ESC ] 0 ; ti \n tle BEL llama.cpp OOM"` comes back as one line, where common terminals abort the control string and draw two rows); and the `fix-16` catalog test's comment cites the cross-check that actually exists, `diagnosis_remediation_matches_the_fix_catalog_for_shared_fix_ids`, rather than a name that is in neither the tree nor its history. Last, `effective_gpu_count`'s doc claimed the VRAM-row fallback applies when the amd-smi count is absent, while the code also falls through when it is present and zero -- a host amd-smi cannot enumerate but sysfs can. The doc now names both triggers and the second one gains the test case it never had; dropping the `count > 0` filter on `detected` turns it red. EAI-8058 Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * docs(core): qualify the never-merge guarantee where a consumer reads it `rendered_lines` promised, unqualified, that "two pieces of text the terminal drew on different rows never end up in the same returned segment". That is the invariant the module exists for -- it is what stops a `llama.cpp` OOM being scored against a `vllm` anchor -- and it is not what the code holds. The counterexample was already written down in the same commit, 140 lines above, but in a body comment inside the private `next_token`, so it reached no reader of the generated documentation, and the public doc's pointer to it (`See [next_token]`) was a link to a private item that rustdoc will not resolve. `cargo doc -p rocm-core` warned on exactly that line. This is the third finding of the same class on this module, so the fix is structural rather than a reworded sentence. The exception now has one authoritative statement, in the module documentation, which is public and therefore rendered: a line break inside a well-formed string-argument body (`OSC`, `DCS`, `SOS`, `PM`, `APC`) is consumed with that body, with the worked example spelled out. `rendered_lines`'s own doc carries the qualifier inline -- so the only absolute a consumer can read is true -- and links to the module docs rather than to a private item. The `next_token` arm that creates the exception no longer restates it; it points at the module paragraph and names what has to change with it. The behaviour is unchanged. Following the grammar rather than guessing where an unterminated body ends is the right call: the alternative reintroduces the leak of OSC bodies as text. `a_line_break_inside_a_well_formed_string_body_is_consumed_with_it` pins the worked example so prose and behaviour move together; flipping its expectation to the two-segment reading fails with `left: ["vllmllama.cpp OOM"]`. It also pins that the same break outside a string body still splits, and that an unterminated body still stops at the following `ESC`, so the hole cannot widen unnoticed. Also, both non-blocking wording items and one more instance of the same dangling-link defect the PR introduced: - `the_split_never_merges_two_rendered_lines` overstated what the module guarantees; its body already scoped itself to single non-drawing scalars, so it is renamed to `a_lone_non_drawing_scalar_never_merges_two_rendered_lines`. - the citation in `the_oom_plan_matches_the_catalog_copy` called `diagnosis_remediation_matches_the_fix_catalog_for_shared_fix_ids` "the cross-check that runs over every matched finding"; it iterates a fixed two-element array of hand-built diagnoses, so it is now "the cross-check over the fixes shared with the catalog". - `DiagnoseReport::out_of_scope`'s doc linked `[catalog_covers]`, also a private item, also warned on by `cargo doc`, and also added by this branch. It now names the rule in prose instead. `cargo doc -p rocm-core --no-deps` goes from 6 warnings to 3; the remaining three are pre-existing on `main` and are not touched here. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * docs(core): drop the terminator from the merge exception; it is not required `rendered_lines`'s doc said two rendered rows share a segment "only when the break between them sits inside a well-formed string-argument sequence ... whose body ECMA-48 defines as running to its terminator", and the module section said the shape "needs a well-formed introducer and terminator around the line break". Both make a terminator necessary for the merge. It is not. `skip_string_sequence_body` ends the body scan on `BEL`, on `ST`, on any other bare `ESC` (left in place), or on end of input, and `next_token` returns `Token::Ignorable` for all of them. The break is swallowed either way. The test added in this PR is itself the counterexample and passes: its input carries no `BEL` and no `ST` anywhere, and the two rows still merge. The private helper's own doc comment already said this, so the public doc was contradicting the private one. State what the code does instead, and add the unterminated input as a second worked example so the prose has to move with it. That also invalidates the reassurance built on top of the terminator being required. An unterminated body followed by more output is reachable from ordinary output by two routes: a process killed part-way through writing an `OSC` title, which is the case this walk exists for, and the interleaved capture itself, since the serve log hands the subprocess's stdout and stderr the same file handle and either stream can cut the other's title. Say so, and say why following the grammar is still right -- guessing where an unterminated body ends reintroduces the leak of `OSC` bodies as text. `vllm_anchored_lines` still carried the original unqualified "two things drawn on different rows are never scored as one line", which is the sentence the module section claims to be the single authoritative statement of. Replace the restatement with a pointer rather than a second qualified copy; a second copy is a stale copy waiting to happen, and this is the call site where a stale one does the damage. Docs, comments and one test rename only; no behaviour change. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * docs(core): delete the restated reachability claim, and name the third behaviour `rendered_lines`'s own doc claimed a process killed mid-title was enough to reach the merge shape. It is not. The merge needs drawable text on both sides of the swallowed break landing in one segment, and a kill leaves the body running to the end of the input, where there is no far side to join to: rendered_lines("vllm\u{1b}]0;ti\ntle") == ["vllm"] The `tle` a terminal drew on its second row comes back in no segment at all. That is a third behaviour -- neither the split nor the merge the one-sided framing describes -- and it was undocumented, so the prose reached for it as an example of the merge instead. This is the fifth round on one sentence, and the sentence is always in the same place: `rendered_lines` restating the exception rather than pointing at the module section that owns it. So the restatement goes rather than gets reworded. The qualifier that binds callers stays; the terminator detail and the reachability claim are deleted and the `[the module documentation](self)` pointer now carries the routes, which makes the "single authoritative statement" claim true rather than aspirational. The drop is documented rather than filed as a defect: it is the direct, already-argued consequence of not guessing where an unterminated body ends, and it errs in the losing direction the contract leaves unpromised -- it costs a diagnosis, it cannot invent one. Both one-sided framings are widened to admit it, so neither now claims two directions while a third exists. Also corrected: the interleaved capture reaches `rendered_lines` through a user paste into `rocm diagnose --symptom`, not through the engine's own path, which pre-splits its log tail with `str::lines`; and the contract is not "stated once" in `vllm_anchored_lines`'s doc, which names two places in the same breath. The end-of-input case is now pinned by a test. Nothing covered it, which is why the prose could cite it wrongly twice. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * docs(core): the merge trigger is text following the sequence, not end of input The module section named end of input as the reason a swallowed break loses a row rather than merging. It is not the reason: a properly BEL-terminated body with nothing after it loses the row just the same, and so does one with text after it that sits past a break. What decides is whether drawable text follows the sequence in the same segment, however the body ended. Rather than restate that correctly in each of the three places the section said it, keep one statement where the third behaviour is introduced and delete the other two -- the divergence between copies is what produced this round and the five before it. Pin the two shapes that were not covered, so no future wording can name a narrower trigger than the code has without a test failing. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * docs(core): qualify the closing summary's loss condition to match the rule The section states the trigger precisely as drawable text following the sequence *in that same segment*, then the closing summary restated it without the qualifier. rendered_lines("vllm\u{1b}]0;ti\ntle\u{7}\nx") is the case that splits them: text does follow the sequence, the row is lost anyway, because the text is past a break and so in another segment. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * docs(core): retitle the residual-behaviour section and stop restating its rule The section documents two residual behaviours, not one exception, and its own claim about the file's structure was false: the code arm and the test do restate the rule, accurately and usefully, as local commentary. Say instead that this section is the normative statement. Three further corrections, all the same recurring defect of the same rule living in more than one place and drifting: - a break inside a string body can lose more than one rendered row (`rendered_lines("vllm\u{1b}]0;ti\ntle\nmore") == ["vllm"]`), so "one or more"; - the "what decides" clause named only the far side, but a row is lost with no near side too (`rendered_lines("\u{1b}]0;ti\ntle\u{7}x") == ["x"]`) — dropped in favour of the both-sides sentence before it; - the closing summary repeated the same far-side-only qualifier, so it is dropped there as well. Net two lines shorter. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * fix(serve): keep a detected GPU with no VRAM row as an auto-select candidate `select_auto_gpu_index` builds its candidate ordinals from the VRAM rows alone whenever telemetry is present. That was introduced so a visibility mask reporting non-contiguous ordinals is scanned by its real indices instead of a synthetic `0..count` range, which is right — but a short row set is not always a mask. `parse_gpu_vram_usage` drops any device entry missing `/mem_usage/used_vram/value` or `/mem_usage/total_vram/value`, so a device the lighter `list` enumeration counts can simply have no row. With device 1 absent from the rows and device 0 pinned by a managed service, `reported` was `[0]`, the busy filter emptied the candidate list, all three passes iterated nothing and the terminal fallback returned device 0 — `serve` pinned to an already-occupied GPU, then failing on VRAM, which is the fault this selection exists to reduce. Union the detected `0..count` range back in only when `visible` is known. The existing retain then validates every ordinal against the mask, so no unconfirmed index is ever synthesised: under a `[2, 3]` mask with `count == 2` the synthetic `[0, 1]` is dropped wholesale and the rows still stand alone. Without `visible` there is no second source to confirm an ordinal against, so the rows remain authoritative. Pins the case with a test: a two-GPU count, one row, an all-busy reported set, which the suite never exercised (every all-busy case passed `detected: None`). Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * fix(serve): discriminate a masked row set by its indices, not by `visible` The previous commit unioned the detected `0..count` range back into the auto-select candidates only `if visible.is_some()`, so the fix reached only hosts whose visibility set could be probed. `probe_usable_amd_gpu_indices` returns `None` unconditionally for `#[cfg(not(target_os = "linux"))]`, and `serve` threads that straight into `visible` through `select_gpu_indices_under_launch_lock`. So `visible` is always `None` on Windows — a supported platform — and on any Linux host whose KFD topology and DRM cards are both unreadable. With `count == 2`, one VRAM row for GPU 0 and GPU 0 pinned by a managed service, those hosts still returned the busy GPU 0 where the base branch returned the idle GPU 1: the exact fault this selection exists to reduce. `visible` was never the right discriminator. The rows are a re-indexed or sparse set precisely when some row index falls at or above `count` (`[2, 3]` against `count == 2`), which is the shape the rows-only construction was introduced for; below the count they are a subset of a dense range the count already asserts exists, and unioning `0..count` there is no stronger an assumption than the telemetry-less arm already makes. Require `detected` to be `Some(n > 0)` as well, so the count comes from the `list` enumeration rather than from `rows.len()` restating the rows' own size — `parse_gpu_vram_usage` only falls back to the array position when an entry has no `gpu` field, so a payload naming `"gpu": 0` twice yields duplicate ordinals that would otherwise conjure a GPU 1. The visible retain still runs on top whenever a mask is known, so nothing synthesised survives where the mask contradicts it. Pins both halves of the condition: forcing either to `true` turns an assertion of the new test red, and the masked assertion now holds with `visible: None` so it pins the row-index rule itself rather than the retain. Also replaces the Pass 2 rationale, which the union falsified. `usage_for` is no longer total over `candidate_indices`, but no row guard is needed for a different reason: the comparator keys are `Option<u64>` and `None` sorts below every `Some`, so a rowless ordinal can win only when no candidate has a row, and then the index tie-break returns the lowest — exactly what Pass 3 would return. Re-adding the guard leaves all 771 unit tests green, so the guard is behaviourally inert and stays out. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * fix(vllm): reject a mandatory line break from the quoted symptom too `quotable_in_single_quotes` gates what is embedded in the `rocm diagnose --symptom '...'` command the OOM hint prints for the user to paste, and it tested each character with `is_control_or_format` alone. That predicate is `char::is_control` plus the enumerated `Cf` set, and `U+2028 LINE SEPARATOR` / `U+2029 PARAGRAPH SEPARATOR` are `Zl`/`Zp`, so both passed the gate. The value reaching it is raw: `candidate` is built from `raw_line`, not from the stripped `symptom_line` beside it, and `str::lines` does not split on `U+2028`, so the scalar survives inside a selected line. A tail line like `RuntimeError: HIP out of memory<U+2028>...` cleared the OOM scan, cleared this gate, scored above threshold and was printed verbatim inside the quotes — the bytes stay inside them, but the rest of the command renders on the next row. That is the same class of display control the guard's own documentation says it closes. `classify_char` already widened past `is_control_or_format` for exactly this pair, so the module stated one contract and the gate implemented a narrower one. Rather than repeat the `matches!` at the gate, export `is_control_or_line_separator` from `rocm_core::terminal` and use it at both sites: `is_control_or_format`'s own documentation advertises itself as the answer to this concern, so the next call site would reach for the narrower test again. One predicate removes the trap, and the rationale moves onto it instead of being restated at each user. The stripper table test pinned both scalars against `strip_terminal_control_sequences` only; it now pins them against the quoting guard as well, which is where they got through. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * docs(skill): document fix-16-vllm-oom in the rocm-doctor reference `fix-16-vllm-oom` is added to the CLI's closed catalog by this branch, but `skills/rocm-doctor/reference.md` still described `fix-16` as "a reserved handle, not a missing row". The skill doc is asserted against the CLI catalog by three e2e scenarios, and all three failed on the merge: `rocm fix` offers fix-16-vllm-oom, which reference.md does not document. Add the catalog row (linux/wsl, not auto-applicable, matching the recipe's LINUX_AND_WSL and auto_applicable: false), raise the catalog count from 24 to 25, and move the reserved-handle note to `fix-18`, which is still the only gap in the numeric series. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * fix(diagnose): let the catalog fill fix-16's auto_applicable `check_16_vllm_oom` built its `Fix` with an inline `auto_applicable: false`. `no_checker_hand_sets_auto_applicable` forbids that: only `take_applicability_from_the_catalog` may set the field, because the catalog pass overwrites whatever a checker puts there. The output was right either way -- which is the point. A checker that disagreed with the catalog would be silently masked rather than caught, the way `fix-9-igpu-dgpu` once drifted. The field is now left at `Fix::default()` and filled from the recipe, whose `applies_on: PRINT_ON_LINUX_AND_WSL` already carries `FixClass::PrintOnly`. This was latent rather than new: the guard never ran on the previous head because the suite aborted at an unrelated flaky test first, so the offending line reached `gpu-out-of-memory` unnoticed. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> --------- Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>

* feat(vllm): add GPU memory utilization hint for OOM errors and improve documentation Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * fix(vllm): validate --gpu against sysfs GPU count and harden DRM fallback Address review feedback on PR #251: - resolve_gpu_indices now validates an explicit --gpu <index> against the DRM sysfs fallback's device count when amd-smi is unavailable, instead of only the amd-smi count. Previously an out-of-range index was silently accepted on a host with no amd-smi (sysfs works) and only failed later inside the engine. - read_drm_vram_usage no longer treats an unreadable mem_info_vram_used counter as 0 bytes used (which made the card look 100% free -- exactly what --gpu auto prefers first); it now skips that card instead. - read_drm_vram_usage withholds telemetry entirely when more than one AMD DRM card is present, since ascending card<N> order is only guaranteed to match HIP's compute-topology ordinal on a single-GPU host (an APU passes the same vendor + mem_info_vram_total filter as a discrete GPU, so an APU+dGPU host could previously feed a diverged ordinal into HIP_VISIBLE_DEVICES). - docs/vllm.md: corrected the fallback description, which only ever probes amd-smi (never rocm-smi) before falling back to DRM sysfs. Extracted the count-fallback logic into a new effective_gpu_count helper shared by --gpu auto ranking and --gpu <index> validation, and added unit tests for all of the above. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * fix(vllm): validate --gpu against amd-smi-independent count; harden DRM guard Round-2 review follow-ups on the DRM sysfs GPU-selection fallback. - Validate an explicit --gpu <index> against the amd-smi list count or, when amd-smi is absent, the KFD/DRM authority behind usable_amd_gpu_indices() (pinned_index_bound) instead of the DRM VRAM-fallback row count. The row count withholds telemetry on multi-GPU hosts and shrinks on a transient counter-read failure, so borrowing it as a validation bound hard-rejected legitimate indices. VRAM rows stay scoped to --gpu auto ranking. - The multi-card ordinal-ambiguity guard now counts AMD cards *found* (before the telemetry filters) rather than surviving rows, so a second AMD card with an unreadable counter still trips the guard instead of mislabelling the survivor ordinal 0 (the APU+dGPU misattribution the guard exists to prevent). - drm_device_is_amd now matches on vendor id OR an amdgpu uevent DRIVER line, the same two-signal test as rocm_core::is_amdgpu_device, so the fallback probe and the count authority agree on what an AMD card is. - Drop the now-dead sort/enumerate tail in read_drm_vram_usage (at most one card survives the guard) and fix the stale doc comment. - Tests: exercise the real Index-arm delegate (resolve_pinned_gpu_index) and add a two-AMD-card-with-unreadable-counter regression; add pinned_index_bound coverage. - Docs: README and the assistant SKILL note the DRM sysfs VRAM fallback rather than presenting amd-smi as the only source. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * feat(diagnose): add the vLLM out-of-memory failure mode to the catalog (EAI-8060) (#290) * feat(diagnose): add vLLM out-of-memory failure mode to the catalog (EAI-8060) Add a keyword-scored OOM signature to the rocm-core diagnosis catalog and a matching print-only remediation recipe. The error covers two distinct faults (a tenancy collision with vLLM's fixed ~90% VRAM reservation, versus a model that genuinely does not fit), so the wording stays conditional: it never prescribes lowering --gpu-memory-utilization as the unconditional answer, and the verify step avoids the tenancy knob. Because an Examination carries no per-GPU VRAM or tenancy fields, the match is keyword-only and the checker is gated to Linux (vLLM is Linux/WSL-only). - diagnose.rs: KEYWORDS_VLLM_OOM table + check_16_vllm_oom (linux-only) - fix.rs: print-only fix-16-vllm-oom recipe; catalog count 15 -> 16 and AUTO-set assertion strengthened - engines/vllm: serve OOM hint points users at 'rocm diagnose --symptom' - e2e-cucumber: conditional-remediation scenario + step defs and catalog contract updated Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * fix(diagnose): require a vLLM anchor before matching the OOM checker Review feedback on #290 flagged that KEYWORDS_VLLM_OOM never required anything vLLM-specific: 'torch.OutOfMemoryError: CUDA out of memory' scores 45+45=90 (high confidence) under the keyword table alone, so any ROCm PyTorch job's OOM would be misreported as a vLLM startup OOM with rocm-serve-only remediation. gpu_memory_utilization -- the one vLLM-specific token -- was weighted lowest and never required. check_16_vllm_oom now requires an explicit vLLM anchor (the word 'vllm', or one of its distinctive flags: gpu[-_]memory[-_]utilization, tensor[-_]parallel) before the keyword table is scored at all. Update the serve OOM hint's suggested --symptom text and the matching e2e-cucumber step to carry that anchor so the self-referential 'rocm serve' -> 'rocm diagnose' flow keeps working, and add a regression test for the reported false positive. Also fix a second issue from the same review: the diagnose() WSL branch dropped sub-threshold hits from matched entirely when nothing cleared MIN_SCORE_FOR_MATCH, which the DiagnoseReport::matched doc says should never happen. It now keeps whatever run_all_checks returns (empty only when no wsl-applicable checker fired at all) and adds a regression test. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * diagnose: drop the non-existent multi-GPU sharding remedy and tighten the vLLM OOM anchor Round-2 review (EAI-8060): - Remove the `--tensor-parallel-size` / multi-GPU sharding remediation from both the fix catalog (fix-16-vllm-oom) and the diagnose summary: the flag does not exist and rocm-cli serves one model on a single GPU (docs/vllm.md). The 'model does not fit' branch now points only at a smaller/quantized model. - Drop `tensor[-_]parallel` from VLLM_ANCHOR_PATTERN: it is a Megatron/DeepSpeed term, so anchoring on it would misattribute those frameworks' OOMs to vLLM. The anchor is now just `vllm|gpu[-_]memory[-_]utilization`. - Route the user's *actual* failing log line into the `rocm diagnose --symptom` hint (vllm-anchored) instead of a canned literal. - Escape the dot in the `torch\.outofmemoryerror` keyword regex and let the `gpu[-_]memory[-_]utilization` keyword accept a hyphen like the anchor. - Tests: e2e reads high_confidence_threshold from the report instead of a hardcoded 75, locks out `--tensor-parallel-size`, and shares a find_vllm_oom helper; the sub-threshold unit test uses `.expect()` instead of a vacuous `if let`. - docs/vllm.md and the @requires-bare-metal doc note the `rocm diagnose --symptom` pointer and the WSL keyword-only exception. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * style: rustfmt the new OOM --symptom routing test assertion Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> --------- Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * fix(serve): validate --gpu against the usable-device set, not its length (EAI-8058) When amd-smi is unavailable, `--gpu <index>` validation fell back to the *length* of usable_amd_gpu_indices(). That set holds absolute, unrenumbered ordinals after the visibility mask, so on a masked multi-GPU host (e.g. HIP_VISIBLE_DEVICES=2 on a 4-GPU box -> usable [2]) the length-1 bound rejected --gpu 2 (the sole usable device) and accepted a hidden --gpu 0. Replace the count bound with an injectable validate_pinned_gpu_index_against that prefers the amd-smi count and otherwise checks index membership in the usable set -- the same authority the engine device gate uses. The detected/usable seam makes both branches unit-testable without hardware. Also correct two docs/vllm.md inaccuracies (note timing; missing --engine vllm in the workaround examples). Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * docs(vllm): remove duplicate code-fence opener in OOM workaround example A merge-conflict resolution left two consecutive ```bash fence openers before the shared-card OOM workaround example, so the block opened and immediately reopened on the literal text and the commands rendered outside a code block. Drop the duplicate opener so the example renders as a single fenced block. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * serve: address OOM diagnosis review — decouple engine hint from scorer, dedup keyword spans, anchor per-line Reviewer follow-ups on the vLLM OOM diagnosis path: - Engine no longer hard-codes `vllm: <line>` into the printed `rocm diagnose --symptom`. It now consults rocm_core::vllm_oom_symptom_is_diagnosable and falls back to VLLM_OOM_CANONICAL_SYMPTOM when the user's actual line would not score, so the command it prints always reports a cause. The user's real line stays visible in the human-readable message. Adds a loop-invariant test that every emitted --symptom value is diagnosable. - keyword_score now de-duplicates overlapping regex spans before taking the top two, so a phrase nested inside a stronger one is not double-counted. The VLLM_OOM table splits allocator *messages* (score 50, clear alone) from the exception *class* name (45, sub-threshold). - vLLM OOM anchor must appear on the same line as an OOM token, not merely somewhere in a pasted log, closing a misattribution gap. - WSL out-of-scope routing is gated on has_match rather than a non-empty match list, so a sub-threshold signal is preserved and still routed out. - serve --gpu <index> membership rejection now says "not available" and lists the usable set, matching the engine wording and the e2e contract. - --gpu auto ranks the actual reported VRAM row indices (which can be non-contiguous under a visibility mask) instead of a synthetic 0..count. - diagnosis Fix for shared fix ids (fix-15, fix-16) is cross-checked against the fix catalog so verify/notes/commands cannot silently diverge. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * test(vllm): cover serve-plan low-VRAM OOM guidance on the GPU lane (EAI-8058) The serve plan's pre-launch low-VRAM warning and its vLLM `--gpu-memory-utilization` note had unit coverage but no end-to-end scenario, so the user-observable behavior on a real serve went unverified. The plan note is reached only when a *selected* GPU is below the free-VRAM bar, which the GPU lane's genuinely-free cards never trip. Add an e2e-test-hooks-gated seam in gpu_vram_usage() (ROCM_E2E_FORCE_LOW_VRAM=<ordinal>) that reports a single near-full synthetic device, making the warning deterministic on the non-APU vLLM lane without touching the device the engine actually launches on. Add serve-19 (@requires-gpu @requires-engine:vllm @requires-os:linux) asserting the plan warns about low VRAM and names the memory-utilization knob, with its step definitions and a unit test for the seam. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * test(e2e): give the low-VRAM serve scenario its runtime precondition (EAI-8058) serve-19 (@id:serve-vllm-low-vram-oom-guidance) is the only real-serve scenario on the GPU lane that does not open with `a managed runtime is active`. It runs `rocm serve ... --managed` in a fresh isolated root, so the GPU-required device policy refuses before a plan is ever built: Error: device_policy: gpu_required; no active ROCm runtime is configured; run `rocm runtimes list` and `rocm runtimes activate <runtime_key>`, or pass --runtime-id/--env-id That is the single unexpected failure in the E2E tests (GPU) lane on this branch (the other five are declared xfail), so the scenario added to cover the pre-launch OOM note never actually exercised it. Add the same precondition serve-05/06/07 use; the `ROCM_E2E_FORCE_LOW_VRAM` Given stays second so the runtime setup's own `rocm` calls do not drain command_env. Also correct auto_selection_uses_vram_row_count_when_amd_smi_count_is_unknown, which attributed a three-row fixture to the DRM sysfs fallback. That fallback withholds telemetry once a second AMD card is found and so emits at most one row; a multi-row `detected == None` state is amd-smi `metric` answering while `list` does not. Relabel it and add the single-row case the sysfs fallback can actually reach. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * fix(diagnose): scope keyword overlap de-duplication to the vLLM OOM table The overlapping-span de-duplication added for `KEYWORDS_VLLM_OOM` was applied by `keyword_score` to all 16 keyword tables. Several older tables pair a greedy `.*` pattern with a second, genuinely independent keyword, and the greedy match span swallows it: `api-ms-win-crt-.*\.dll` runs to the last `.dll` on the line and covers an independent `msvcp140.dll`, `dkms .*failed` covers an independent `dpkg: error`, `amdhip64.*not found` covers `could not find hip`, and `404.*repo\.radeon\.com` covers `unable to locate package rocm`. Collapsing can only lower a score, so those pairs lost their second signal and dropped below MIN_SCORE_FOR_MATCH or HIGH_CONFIDENCE. On a Windows host with no PATH probe result, the symptom "api-ms-win-crt-runtime-l1-1-0.dll missing and msvcp140.dll not found" scored 35 instead of 65 and fix-15-msvc-redist disappeared from the report entirely. Keep the default "every matching entry is an independent signal" scoring for the catalog and route only `KEYWORDS_VLLM_OOM` — whose overlaps are true nestings of literal phrases rather than wildcard artifacts — through the collapsing variant. Pin the four pre-change scores plus the user-visible fix-15 outcome so the dedup cannot be re-generalized. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * fix(diagnose): score only the vLLM-anchored lines, not the whole symptom The same-line anchor was a boolean gate: `vllm_oom_line_is_anchored` returned true if any one line carried the anchor together with an OOM token, and `check_16_vllm_oom` then scored the *entire* symptom. Regex matching ignores line boundaries, so keyword hits on unanchored lines still counted at full weight, which is exactly the misattribution the anchor exists to prevent. A paste of vllm config: gpu_memory_utilization=0.9 llama.cpp: torch.OutOfMemoryError: CUDA out of memory passed the gate on the benign config echo, then scored `cuda out of memory` (50) plus `torch.outofmemoryerror` (45) off the llama.cpp line — 95, above HIGH_CONFIDENCE — and reported another framework's failure as a confident "vLLM ran the GPU out of memory". Replace the gate with `vllm_anchored_lines`, which returns the anchored lines joined by newlines, and score that. The joins are by `\n` and the table's patterns are literals or `.*` (which does not cross a newline), so dropping unanchored lines cannot fabricate a match across the seam. The same-line requirement now falls out of the scoring itself: an anchored line with no OOM token contributes nothing. `gpu[-_]memory[-_]utilization` stays an anchor despite also being a 20-point scoring entry: with only anchored lines scored, a bare flag echo (a config dump, or rocm-cli's own low-VRAM hint) is worth 20 — far below MIN_SCORE_FOR_MATCH — so it can surface as a weak signal but can never carry a verdict, nor lend its anchor to an OOM elsewhere in the paste. Scoring is otherwise unchanged: only `check_16_vllm_oom` is touched, so no other catalog table moves. Refs: EAI-8058 Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * docs(vllm): align the OOM reservation example with the fix recipe The catalog recipe and the --gpu-memory-utilization parse error both use 0.5; the docs said 0.1 for the same shared-GPU case. A 0.1 budget is below the weights of most models people serve, so it trades one startup failure for another. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * test(e2e): make the low-VRAM serve scenario assert the new guidance (EAI-8058) The scenario's warning step matched "serving may fail on VRAM", which predates this change, and its sibling matched only the bare flag name --- which a recipe echo of a user-supplied --gpu-memory-utilization would also satisfy. Neither fragment is specific to the pre-launch hint this scenario exists to prove. Require the warning to name the pinned GPU and the note to carry vLLM's total-VRAM reservation rationale, and pin both literals in the always-run unit test so a rewording fails on every lane rather than only on the GPU lane that can observe the scenario. Also record on serve-16 why the masked, non-contiguous usable-set case behind the --gpu membership rule has no scenario of its own: it is reachable only where amd-smi is absent, so a scenario would assert against an injected seam rather than the product. It stays unit-covered. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * test(diagnose): retire the WSL out-of-scope routing assertions The WSL2 catalog (#340) makes WSL a covered platform, so `diagnose` no longer emits an out-of-scope routing note there. Two tests written against the old behaviour were asserting that note, and one of them also assumed a default WSL examination produces no match -- it now legitimately does, since a host with no /dev/dxg really is a `fix-wsl-1-gpu-not-exposed` finding. Both now build on `wsl_base()`, the healthy-WSL fixture, so the vLLM OOM signal is the only thing under test. The load-bearing half is unchanged and still asserted: a score-25 `out of memory` mention stays visible in `matched` as a WEAK row and does not promote itself into a match. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * fix(fix): let fix-16-vllm-oom run on WSL2, where its checker answers `check_16_vllm_oom` is registered for `["linux", "wsl"]` -- deliberately, per the comment at diagnose.rs -- but the matching recipe was `LINUX_ONLY`. `rocm fix` re-gates a fix-id on the *recipe's* platform list against the running OS, where WSL2 is its own family, so on WSL2 `rocm diagnose` printed `apply with: rocm fix fix-16-vllm-oom` and that command then printed the plan and exited 3 with "This fix only applies on: linux. Running OS is: wsl." Nothing about the remediation is bare-metal: it is vLLM CLI flags. WSL2 is the supported way to serve from a Windows host and no `fix-wsl-*` entry covers the failure mode, so the platform had a diagnosis and no working fix path. The class, not just the instance: the two platform lists are written in separate modules and each looks right alone. Add `every_checker_platform_is_covered_by_its_recipe`, which walks `CHECKERS` and requires every family a checker answers on to be one the recipe will act on. Containment rather than equality -- a recipe may legitimately apply more widely than its checker fires, since `rocm fix <id>` is reachable without a diagnosis. It found no other mismatch across the remaining 23 entries. The loop is user-observable, so diagnose-20 gains a step that runs the fix-id the report named and requires the success rc (the wrong-OS refusal prints the recipe first, so output alone cannot tell the outcomes apart). It is `@requires-os:linux`, which WSL2 matches, so the `e2e-wsl` self-hosted lane is the one that can observe the difference; every Linux lane runs the step. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * fix(core): the shared OOM hint recommended 0.1 while every command said 0.5 `VLLM_GPU_MEMORY_UTILIZATION_HINT` still carried "(e.g. 0.1 for a small model)". It is printed verbatim by `rocm serve`'s low-VRAM note and the vLLM engine's post-failure OOM hint, and interpolated into the `fix-16-vllm-oom` diagnosis summary -- so one `rocm diagnose` printed "e.g. 0.1 for a small model" directly above `rocm serve <model> --gpu-memory-utilization 0.5`. 0.5 is the settled value; every command surface already used it (the recipe, the diagnosis Fix, docs/vllm.md). 2c3a84e moved the docs and left the const behind, and that commit's own message says why 0.1 is wrong: a 0.1 budget is below the weights of most models people serve, so it trades one startup failure for another. The harmful advice was what the product actually printed. Pin the two together with `the_utilization_hint_example_matches_the_recipe_command`, which compares the hint's worked value against the value the recipe commands. Neither existing guard covers this: the diagnose/fix cross-check exempts the summary as prose framing, and a line-based grep misses the const because the flag and the value sit on different continuation lines -- which is exactly how it was missed the first time. Also correct the const's own doc claim, which named two sharing surfaces when there are three, and record that the catalog rationale is deliberately separate prose whose worked value is nonetheless pinned to this one. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * fix(diagnose): drop the duplicate LINUX_AND_WSL const from the merge Both branches added an identical const — this branch for the fix-16 WSL gate, main for #386 — at different offsets, so git merged both copies without a conflict marker and the lib test failed to compile (E0428). Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * fix(vllm): keep untrusted log text out of the suggested diagnose command On a failed vLLM serve the post-failure hint printed the user's own failing log line inside a single-quoted `rocm diagnose --symptom '...'` command it invites them to paste into a shell. The line is the vLLM subprocess's own startup-log tail and nothing escaped it, so a routine apostrophe in Python error text ("can't allocate", "model 'foo'") closed the quote: at best the command the feature exists to provide was syntactically broken, at worst text the user does not control landed outside the quotes. The neighbouring human-readable echo passed the same bytes through verbatim, ANSI and all. A line that cannot be rendered as one intact single-quoted word is now treated as unusable and takes the fallback to VLLM_OOM_CANONICAL_SYMPTOM that already existed for exactly that case, rather than being escaped: escaping preserves the bytes but yields a command a reader cannot check by eye, and a wrong escape is runnable and misleading instead of obviously broken. The echoed line is stripped of ANSI sequences and control bytes. Also restores the `out_of_scope` doc comment to what `diagnose` does. It had been changed to claim `matched` may hold sub-threshold entries alongside it and that the field is set when nothing clears MIN_SCORE_FOR_MATCH; neither holds — `matched` is cleared whenever the field is set, and the field is decided purely by platform coverage. The WSL2 example can no longer occur now that WSL2 is a covered platform. That is the documented JSON contract, so a consumer trusting it waits for rows that can never arrive. The relationship is now asserted. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * test(vllm): pin the exact rendering instead of fragment absence The control-byte test carried the message "the ANSI sequence must be removed whole, not just its escape byte" but only asserted that "[31m" and "[0m" were absent. A stripper that drops the escape byte and the "[" it introduces and nothing further leaves "31m"/"0m" behind, which satisfies both absence checks and carries no control byte, so the test passed under exactly the defect it named. Verified by running that mutant: green before, failing now. Assert the stripper's output and the full rendered hint by equality instead. The neighbouring quote-rejection test had the same shape -- "no apostrophe in the symptom" also holds if the apostrophes were silently deleted, the escaping-style rewrite the canonical fallback exists to avoid -- so pin the fallback symptom by equality too. That mutant was also green before and fails now. Tests only; no behaviour change. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * test(serve): pin the all-busy auto-GPU fallback to a reported ordinal `select_auto_gpu_index` now sources its candidates from the actual VRAM rows, because a visibility mask can make reported ordinals non-contiguous. That removed the invariant the terminal all-candidates-busy fallback quietly relied on: with candidates drawn from a dense `0..count`, ordinal 0 was always a real device, so returning a literal `0` was safe by construction. With rows reported at [2, 3] it is not — the busy filter empties the candidate list, every pass falls through, and a fabricated 0 goes straight into `HIP_VISIBLE_DEVICES`, pinning a device nothing ever reported. Under the strict GPU-required / no-silent-fallback policy that is a silently wrong pin, not a fail-fast. The merge resolution already routes the fallback through the reported, mask-filtered set, so the behaviour is correct. What was missing is coverage: `auto_selection_ranks_the_actual_reported_row_indices` claims pass 3 "never returns a fabricated index 0", but it only exercises the *partially* busy case (busy = [3]), which never reaches the fallback at all. This adds the all-busy case that does. Falsified: with the fallback reverted to `vec![0]` this test fails with left: [0], right: [2] (and takes main's `auto_selection_all_visible_busy_falls_back_to_lowest_visible_not_zero` down with it); both pass with the fallback restored. Equality assertion rather than fragment absence, so a future regression cannot pass it vacuously. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * docs(e2e): drop a serve-16 note the merge left dangling The comment on `serve-16` described `--gpu` validation as falling back to membership in the KFD/DRM usable set and pointed at `validate_pinned_gpu_index_falls_back_to_usable_set_membership` as the unit coverage for the masked sub-case. Merging #267 replaced that validator with one that consults the visibility-resolved set directly, so the named test no longer exists, and the masked sub-case it called uncovered now has two scenarios of its own (serve-19, serve-20). Nothing here changes behaviour or which lane runs what; the paragraph is rewritten to say what serve-16 still covers and to point at the scenarios that own the rest. The conflict was textual-merge-clean — this block came wholly from one side — so only reading it caught the dangling reference. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * fix(diagnose): treat a bare CR as a line boundary in the vLLM anchor filter `vllm_anchored_lines` split with `str::lines()`, which splits only on `\n` and `\r\n` -- a lone `\r` is ordinary text to it. Every progress bar in this ecosystem repaints with a bare CR (tqdm, pip, huggingface), and pasting a captured terminal scrollback is the expected way to use `rocm diagnose --symptom '...'`, so a real capture collapsed into one giant "line". The anchor then matched anywhere in it and every keyword in the paste scored at full weight: a llama.cpp OOM in a capture that also mentions vLLM came back at 95, well above HIGH_CONFIDENCE, as "vLLM ran the GPU out of memory at startup". That is the whole-paste scoring `only_the_anchored_lines_are_scored_not_the_ whole_paste` exists to prevent, reinstated by one byte. Neither existing guard saw it: both use `\n`-only fixtures. Split on `\r` as well, so the line boundary is the one the terminal actually renders, and add a regression case built from a bare-CR capture. It also pins the `\n` form of the same content and an anchored-across-CRs control, so the case stays a boundary question rather than a scoring one. EAI-8058 Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * fix(vllm): parse the escape grammar and reject format characters The OOM hint quotes a line of untrusted subprocess output into a `rocm diagnose --symptom '...'` command and echoes it in a `bail!`-ed message, so the sanitizer in front of it is load-bearing. Three holes: The escape scan ran to the next byte in `0x40..=0x7E` with no notion of the CSI grammar's parameter, intermediate and final classes. On well-formed colour codes that is indistinguishable from correct; on the malformed and truncated sequences a *killed* process emits -- which is the only case this code path exists for -- every one of them corrupted the message. A truncated CSI swallowed the next sequence's introducer (`ESC [ 1 ; 2 ESC [ 0 m Killed` rendered as `0mKilled`); a multi-byte scalar can never be in that ASCII range, so the scan ran past it and ate to the next byte that happened to land in it (`ESC [ 1 2 é out of memory` rendered as `ut of memory`); an OSC title leaked its body as text; a stray `ESC ESC` left `[0m` printed literally. Follow the grammar, and on an invalid byte stop *without consuming it*, so the damage is bounded to the malformed sequence and a following well-formed one is still recognised. `char::is_control` is Unicode category `Cc` only, so `Cf` format characters survived both the stripper and `quotable_in_single_quotes`. U+202E in a log line cannot escape the single quotes, but it does reverse how the printed command renders -- the same "untrusted output must not control what the terminal shows" concern, so it gets the same answer. The stripper's doc comment promised control bytes could not repaint the terminal "from inside rocm-cli's own error message", while the log tail a few lines above it in the same message went out raw. Sanitize the tail too, per line so it keeps its shape. Also states why `.rev()` is deliberate (the last OOM line is the proximate failure) and what it costs, since nothing said so; behaviour is unchanged. Tests: the existing control-byte test only exercised an `'m'`-terminated CSI, so narrowing the final-byte range to `'m'` left it green. The new case table pins the range, the byte classes and every introducer. The diagnosability test could not fail for the feature it is named for either -- a mutant that always returns VLLM_OOM_CANONICAL_SYMPTOM satisfies it, because that symptom is diagnosable by construction -- so it now pins the exact symptom per line, over a table that deliberately straddles the routing and fallback branches. EAI-8058 Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * test(diagnose): pin the fix-16 plan to its catalog copy The command block is maintained twice -- once on the `Fix` that `diagnose` attaches, once on the `rocm fix` catalog recipe -- and the two had already drifted. The cross-check that runs over every matched finding could not see it: `recipe_verify_notes_and_run_commands` filters `#` lines by construction, and only the comments differed. That prose is the point of this entry. Two different faults produce the same OOM and need opposite responses, so the comments are what tell the user which of the two each step addresses -- a user who meets the `diagnose` copy and a user who meets the `rocm fix` copy must be told the same thing. Align the `diagnose` copy to the catalog's Case 1/Case 2 wording and wire fix-16 into `assert_plan_matches_the_catalog_copy`, the byte-identical guard that already existed for fix-17-torch-dlpack. Also restores the general rule to `LINUX_AND_WSL`'s doc. Deduplicating the const in 1e23717d kept the fix-16-specific comment and dropped main's general one, leaving a shared const documented as if fix-16 were its only user; fix-19-shm-too-small uses it too. EAI-8058 Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * refactor(serve): drop a vacuous guard and the duplicated AMD-device test Three leftovers from the auto-selection rewrite and the merge around it: `select_auto_gpu_index`'s pass-2 `max_by` still carried `&& usage_for(index).is_some()`. Inside `if vram.is_some()` every candidate came from a reported VRAM row, so that is vacuously true -- it implies a failure mode (a selected ordinal telemetry never reported) that only existed when candidates were a synthetic `0..count` range. Replace it with a comment saying why no guard is needed. `drm_device_is_amd` re-implemented the private `rocm_core::is_amdgpu_device`, with a doc comment asserting the two were "the same two-signal test" and that divergence would let the multi-card ordinal guard slip -- a promise nothing enforced. They must agree because both count AMD cards and this one feeds that guard, so make the rocm-core function public and delegate. One copy is better than two copies plus a cross-check. The second assertion in `auto_selection_all_reported_busy_falls_back_to_a_ reported_ordinal` varied the order of the `busy` slice, which is only ever read through `.contains()` -- so it was a duplicate of the first while its comment claimed an order-independence property nobody was testing. Vary the order of the VRAM *rows* instead: `amd-smi` orders by enumeration, not by index, and that is what `reported.sort_unstable()` is there for. Dropping the sort now turns it red with `left: [3], right: [2]`; it could not before. EAI-8058 Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * fix(diagnose): split the vLLM anchor filter on rendered lines, not two bytes The previous round fixed the whole-paste collapse for a bare `\r` and left it open for every other line advance. `.split(['\n', '\r'])` is an allowlist of two characters under a doc comment claiming the result was "the line boundary the one the terminal actually renders", and it was not: with the `vllm` anchor and a llama.cpp OOM on genuinely separate rendered lines, `ESC E` (NEL), `ESC D` (IND), `CSI 1 B` (CUD), `\x0b`, `\x0c`, `\x85`, U+2028/U+2029 and any stray control byte each scored 95 against a `high_confidence_threshold` of 75 -- citing the llama.cpp line's tokens as the evidence for a vLLM verdict. `ESC E`, `ESC D` and `CSI n B` are terminfo's `nel`/`ind`/`cud1`, i.e. ordinary output from curses- and `rich`-style progress UIs, and `rocm serve`'s own failure path invites exactly this paste. The one-line `char::is_control()` split that suggests itself does close every one of those, but it trades the false positive for a false negative on the most ordinary input there is: a colourised logger emits SGR *inside* a line, so splitting at `ESC` cuts `ESC[31m vllm: ESC[1m torch.OutOfMemoryError` between its anchor and its error token. Measured on that candidate, that line went from 95 to 0. Getting both halves right needs the escape grammar, and this crate's dependents already have one -- `strip_terminal_control_sequences`, which took three rounds to get right in the vLLM engine. Rather than write a second parser, move the ECMA-48 walk down into `rocm-core::terminal` as a classifier over three tokens (`Text`, `LineBreak`, `Ignorable`) and give it two consumers: the engine's stripper, whose behaviour is unchanged, and a new `rendered_lines` the anchor filter splits on. The classification is deliberately lopsided rather than terminal-accurate, and the doc says so instead of claiming fidelity it does not deliver. Only SGR and the string-argument sequences (OSC/DCS/SOS/PM/APC) are `Ignorable`, because neither can move the cursor; `\t` stays intra-line text; everything else is a boundary. That is a one-sided guarantee -- two things drawn on different rows are never scored as one line, while one row may come back split -- and it is the right asymmetry here: merging rows produces a confident wrong cause, splitting one loses a diagnosis the canonical-symptom fallback already covers. A real terminal emulator is not decidable on a byte stream with no screen model (`CSI H` addresses a row that depends on everything before it), and over-splitting needs none of that. After: every separator above scores 0, `\n` and `\r` stay 0, and both the plain and the colourised genuine vLLM OOM stay at 95. Tests: the CR regression becomes a table over the separators rather than one test per byte, plus the SGR case in the other direction, plus a property test over the whole C0/C1 range pinned to `char::is_control` rather than to this module's own classifier. Reverting the boundary set turns three of them red (`nel esc-e`, `U+0000`, `a ESC E (NEL)-separated llama.cpp OOM ...`); making SGR a boundary instead turns the colour test red on `SGR inside a line must not change the verdict`. A confidently wrong cause is user-observable, so it is also pinned as `@id:diagnose-vllm-oom-not-attributed-across-rendered-lines` (diagnose-21), which fails on the pre-fix boundary set and passes after. EAI-8058 Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * chore(core): add the licence header the new terminal module was missing `fcf99156` created `crates/rocm-core/src/terminal.rs` without the SPDX header every other source file carries, so `License header check (hawkeye)` failed — its `licenserc.toml` includes `**/*.rs`. Header only; no code change. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * docs(core): scope the consumption rule and name the one stripper change Two prose defects in the module the previous commit added, both of which a reviewer would rely on. `next_token`'s doc asserted that "the contract here is 'render what the terminal would have rendered', which is the only rule that stays decidable on a byte stream with no framing". That was true of the stripper the paragraph came from, where the only question was how much of a well-formed sequence to consume. It is false of `next_token`, which also decides line boundaries and deliberately over-splits -- twenty-eight lines further down the same comment says a terminal emulator "is not decidable on a byte stream with no screen model", and the section between explains the classification is lopsided rather than terminal-accurate. Left standing, that sentence is a documented licence to "fix" the over-splitting toward terminal accuracy, which reopens the row-merging misattribution the previous commit closed. It is now scoped to the consumption decision it is actually about: a well-formed sequence is consumed whole because a real terminal consumes it too, and which token it is then labelled with is the separate, deliberately non-terminal-like question below. The same commit said the engine stripper's "behaviour is unchanged" by the move. It is unchanged except for U+2028/U+2029. The engine-local stripper classified non-escape characters with `is_control_or_format` alone, which is false for both (neither is `Cc`, neither is in the enumerated `Cf` set), so both survived into the stripped message; `classify_char` puts them in the boundary set, so they are now dropped. Measured on this head: the old code returns "a\u{2028}b" for input "a\u{2028}b", the new one returns "ab". The widening is the better behaviour -- a mandatory Unicode line break is exactly what this stripper exists to keep untrusted output from injecting -- but nothing stated it and no test pinned either scalar, so a silent drift back would have gone unnoticed. `classify_char` now says so, and the vLLM stripper table gains both cases: reverting the pair out of the boundary set turns `the_stripper_follows_the_escape_grammar_not_just_ the_colour_case` red on "a line separator must not survive into the message". Also from the same review, three prose corrections with no behaviour change: `rendered_lines`'s lead sentence now states the over-approximation up front instead of stating the strong property and retracting it a paragraph later; the string-argument arm documents the one crafted-input hole in the never-merge guarantee (a properly terminated OSC body containing a line feed is swallowed whole, so `"vllm ESC ] 0 ; ti \n tle BEL llama.cpp OOM"` comes back as one line, where common terminals abort the control string and draw two rows); and the `fix-16` catalog test's comment cites the cross-check that actually exists, `diagnosis_remediation_matches_the_fix_catalog_for_shared_fix_ids`, rather than a name that is in neither the tree nor its history. Last, `effective_gpu_count`'s doc claimed the VRAM-row fallback applies when the amd-smi count is absent, while the code also falls through when it is present and zero -- a host amd-smi cannot enumerate but sysfs can. The doc now names both triggers and the second one gains the test case it never had; dropping the `count > 0` filter on `detected` turns it red. EAI-8058 Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * docs(core): qualify the never-merge guarantee where a consumer reads it `rendered_lines` promised, unqualified, that "two pieces of text the terminal drew on different rows never end up in the same returned segment". That is the invariant the module exists for -- it is what stops a `llama.cpp` OOM being scored against a `vllm` anchor -- and it is not what the code holds. The counterexample was already written down in the same commit, 140 lines above, but in a body comment inside the private `next_token`, so it reached no reader of the generated documentation, and the public doc's pointer to it (`See [next_token]`) was a link to a private item that rustdoc will not resolve. `cargo doc -p rocm-core` warned on exactly that line. This is the third finding of the same class on this module, so the fix is structural rather than a reworded sentence. The exception now has one authoritative statement, in the module documentation, which is public and therefore rendered: a line break inside a well-formed string-argument body (`OSC`, `DCS`, `SOS`, `PM`, `APC`) is consumed with that body, with the worked example spelled out. `rendered_lines`'s own doc carries the qualifier inline -- so the only absolute a consumer can read is true -- and links to the module docs rather than to a private item. The `next_token` arm that creates the exception no longer restates it; it points at the module paragraph and names what has to change with it. The behaviour is unchanged. Following the grammar rather than guessing where an unterminated body ends is the right call: the alternative reintroduces the leak of OSC bodies as text. `a_line_break_inside_a_well_formed_string_body_is_consumed_with_it` pins the worked example so prose and behaviour move together; flipping its expectation to the two-segment reading fails with `left: ["vllmllama.cpp OOM"]`. It also pins that the same break outside a string body still splits, and that an unterminated body still stops at the following `ESC`, so the hole cannot widen unnoticed. Also, both non-blocking wording items and one more instance of the same dangling-link defect the PR introduced: - `the_split_never_merges_two_rendered_lines` overstated what the module guarantees; its body already scoped itself to single non-drawing scalars, so it is renamed to `a_lone_non_drawing_scalar_never_merges_two_rendered_lines`. - the citation in `the_oom_plan_matches_the_catalog_copy` called `diagnosis_remediation_matches_the_fix_catalog_for_shared_fix_ids` "the cross-check that runs over every matched finding"; it iterates a fixed two-element array of hand-built diagnoses, so it is now "the cross-check over the fixes shared with the catalog". - `DiagnoseReport::out_of_scope`'s doc linked `[catalog_covers]`, also a private item, also warned on by `cargo doc`, and also added by this branch. It now names the rule in prose instead. `cargo doc -p rocm-core --no-deps` goes from 6 warnings to 3; the remaining three are pre-existing on `main` and are not touched here. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * docs(core): drop the terminator from the merge exception; it is not required `rendered_lines`'s doc said two rendered rows share a segment "only when the break between them sits inside a well-formed string-argument sequence ... whose body ECMA-48 defines as running to its terminator", and the module section said the shape "needs a well-formed introducer and terminator around the line break". Both make a terminator necessary for the merge. It is not. `skip_string_sequence_body` ends the body scan on `BEL`, on `ST`, on any other bare `ESC` (left in place), or on end of input, and `next_token` returns `Token::Ignorable` for all of them. The break is swallowed either way. The test added in this PR is itself the counterexample and passes: its input carries no `BEL` and no `ST` anywhere, and the two rows still merge. The private helper's own doc comment already said this, so the public doc was contradicting the private one. State what the code does instead, and add the unterminated input as a second worked example so the prose has to move with it. That also invalidates the reassurance built on top of the terminator being required. An unterminated body followed by more output is reachable from ordinary output by two routes: a process killed part-way through writing an `OSC` title, which is the case this walk exists for, and the interleaved capture itself, since the serve log hands the subprocess's stdout and stderr the same file handle and either stream can cut the other's title. Say so, and say why following the grammar is still right -- guessing where an unterminated body ends reintroduces the leak of `OSC` bodies as text. `vllm_anchored_lines` still carried the original unqualified "two things drawn on different rows are never scored as one line", which is the sentence the module section claims to be the single authoritative statement of. Replace the restatement with a pointer rather than a second qualified copy; a second copy is a stale copy waiting to happen, and this is the call site where a stale one does the damage. Docs, comments and one test rename only; no behaviour change. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * docs(core): delete the restated reachability claim, and name the third behaviour `rendered_lines`'s own doc claimed a process killed mid-title was enough to reach the merge shape. It is not. The merge needs drawable text on both sides of the swallowed break landing in one segment, and a kill leaves the body running to the end of the input, where there is no far side to join to: rendered_lines("vllm\u{1b}]0;ti\ntle") == ["vllm"] The `tle` a terminal drew on its second row comes back in no segment at all. That is a third behaviour -- neither the split nor the merge the one-sided framing describes -- and it was undocumented, so the prose reached for it as an example of the merge instead. This is the fifth round on one sentence, and the sentence is always in the same place: `rendered_lines` restating the exception rather than pointing at the module section that owns it. So the restatement goes rather than gets reworded. The qualifier that binds callers stays; the terminator detail and the reachability claim are deleted and the `[the module documentation](self)` pointer now carries the routes, which makes the "single authoritative statement" claim true rather than aspirational. The drop is documented rather than filed as a defect: it is the direct, already-argued consequence of not guessing where an unterminated body ends, and it errs in the losing direction the contract leaves unpromised -- it costs a diagnosis, it cannot invent one. Both one-sided framings are widened to admit it, so neither now claims two directions while a third exists. Also corrected: the interleaved capture reaches `rendered_lines` through a user paste into `rocm diagnose --symptom`, not through the engine's own path, which pre-splits its log tail with `str::lines`; and the contract is not "stated once" in `vllm_anchored_lines`'s doc, which names two places in the same breath. The end-of-input case is now pinned by a test. Nothing covered it, which is why the prose could cite it wrongly twice. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * docs(core): the merge trigger is text following the sequence, not end of input The module section named end of input as the reason a swallowed break loses a row rather than merging. It is not the reason: a properly BEL-terminated body with nothing after it loses the row just the same, and so does one with text after it that sits past a break. What decides is whether drawable text follows the sequence in the same segment, however the body ended. Rather than restate that correctly in each of the three places the section said it, keep one statement where the third behaviour is introduced and delete the other two -- the divergence between copies is what produced this round and the five before it. Pin the two shapes that were not covered, so no future wording can name a narrower trigger than the code has without a test failing. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * docs(core): qualify the closing summary's loss condition to match the rule The section states the trigger precisely as drawable text following the sequence *in that same segment*, then the closing summary restated it without the qualifier. rendered_lines("vllm\u{1b}]0;ti\ntle\u{7}\nx") is the case that splits them: text does follow the sequence, the row is lost anyway, because the text is past a break and so in another segment. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * docs(core): retitle the residual-behaviour section and stop restating its rule The section documents two residual behaviours, not one exception, and its own claim about the file's structure was false: the code arm and the test do restate the rule, accurately and usefully, as local commentary. Say instead that this section is the normative statement. Three further corrections, all the same recurring defect of the same rule living in more than one place and drifting: - a break inside a string body can lose more than one rendered row (`rendered_lines("vllm\u{1b}]0;ti\ntle\nmore") == ["vllm"]`), so "one or more"; - the "what decides" clause named only the far side, but a row is lost with no near side too (`rendered_lines("\u{1b}]0;ti\ntle\u{7}x") == ["x"]`) — dropped in favour of the both-sides sentence before it; - the closing summary repeated the same far-side-only qualifier, so it is dropped there as well. Net two lines shorter. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * fix(serve): keep a detected GPU with no VRAM row as an auto-select candidate `select_auto_gpu_index` builds its candidate ordinals from the VRAM rows alone whenever telemetry is present. That was introduced so a visibility mask reporting non-contiguous ordinals is scanned by its real indices instead of a synthetic `0..count` range, which is right — but a short row set is not always a mask. `parse_gpu_vram_usage` drops any device entry missing `/mem_usage/used_vram/value` or `/mem_usage/total_vram/value`, so a device the lighter `list` enumeration counts can simply have no row. With device 1 absent from the rows and device 0 pinned by a managed service, `reported` was `[0]`, the busy filter emptied the candidate list, all three passes iterated nothing and the terminal fallback returned device 0 — `serve` pinned to an already-occupied GPU, then failing on VRAM, which is the fault this selection exists to reduce. Union the detected `0..count` range back in only when `visible` is known. The existing retain then validates every ordinal against the mask, so no unconfirmed index is ever synthesised: under a `[2, 3]` mask with `count == 2` the synthetic `[0, 1]` is dropped wholesale and the rows still stand alone. Without `visible` there is no second source to confirm an ordinal against, so the rows remain authoritative. Pins the case with a test: a two-GPU count, one row, an all-busy reported set, which the suite never exercised (every all-busy case passed `detected: None`). Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * fix(serve): discriminate a masked row set by its indices, not by `visible` The previous commit unioned the detected `0..count` range back into the auto-select candidates only `if visible.is_some()`, so the fix reached only hosts whose visibility set could be probed. `probe_usable_amd_gpu_indices` returns `None` unconditionally for `#[cfg(not(target_os = "linux"))]`, and `serve` threads that straight into `visible` through `select_gpu_indices_under_launch_lock`. So `visible` is always `None` on Windows — a supported platform — and on any Linux host whose KFD topology and DRM cards are both unreadable. With `count == 2`, one VRAM row for GPU 0 and GPU 0 pinned by a managed service, those hosts still returned the busy GPU 0 where the base branch returned the idle GPU 1: the exact fault this selection exists to reduce. `visible` was never the right discriminator. The rows are a re-indexed or sparse set precisely when some row index falls at or above `count` (`[2, 3]` against `count == 2`), which is the shape the rows-only construction was introduced for; below the count they are a subset of a dense range the count already asserts exists, and unioning `0..count` there is no stronger an assumption than the telemetry-less arm already makes. Require `detected` to be `Some(n > 0)` as well, so the count comes from the `list` enumeration rather than from `rows.len()` restating the rows' own size — `parse_gpu_vram_usage` only falls back to the array position when an entry has no `gpu` field, so a payload naming `"gpu": 0` twice yields duplicate ordinals that would otherwise conjure a GPU 1. The visible retain still runs on top whenever a mask is known, so nothing synthesised survives where the mask contradicts it. Pins both halves of the condition: forcing either to `true` turns an assertion of the new test red, and the masked assertion now holds with `visible: None` so it pins the row-index rule itself rather than the retain. Also replaces the Pass 2 rationale, which the union falsified. `usage_for` is no longer total over `candidate_indices`, but no row guard is needed for a different reason: the comparator keys are `Option<u64>` and `None` sorts below every `Some`, so a rowless ordinal can win only when no candidate has a row, and then the index tie-break returns the lowest — exactly what Pass 3 would return. Re-adding the guard leaves all 771 unit tests green, so the guard is behaviourally inert and stays out. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * fix(vllm): reject a mandatory line break from the quoted symptom too `quotable_in_single_quotes` gates what is embedded in the `rocm diagnose --symptom '...'` command the OOM hint prints for the user to paste, and it tested each character with `is_control_or_format` alone. That predicate is `char::is_control` plus the enumerated `Cf` set, and `U+2028 LINE SEPARATOR` / `U+2029 PARAGRAPH SEPARATOR` are `Zl`/`Zp`, so both passed the gate. The value reaching it is raw: `candidate` is built from `raw_line`, not from the stripped `symptom_line` beside it, and `str::lines` does not split on `U+2028`, so the scalar survives inside a selected line. A tail line like `RuntimeError: HIP out of memory<U+2028>...` cleared the OOM scan, cleared this gate, scored above threshold and was printed verbatim inside the quotes — the bytes stay inside them, but the rest of the command renders on the next row. That is the same class of display control the guard's own documentation says it closes. `classify_char` already widened past `is_control_or_format` for exactly this pair, so the module stated one contract and the gate implemented a narrower one. Rather than repeat the `matches!` at the gate, export `is_control_or_line_separator` from `rocm_core::terminal` and use it at both sites: `is_control_or_format`'s own documentation advertises itself as the answer to this concern, so the next call site would reach for the narrower test again. One predicate removes the trap, and the rationale moves onto it instead of being restated at each user. The stripper table test pinned both scalars against `strip_terminal_control_sequences` only; it now pins them against the quoting guard as well, which is where they got through. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * docs(skill): document fix-16-vllm-oom in the rocm-doctor reference `fix-16-vllm-oom` is added to the CLI's closed catalog by this branch, but `skills/rocm-doctor/reference.md` still described `fix-16` as "a reserved handle, not a missing row". The skill doc is asserted against the CLI catalog by three e2e scenarios, and all three failed on the merge: `rocm fix` offers fix-16-vllm-oom, which reference.md does not document. Add the catalog row (linux/wsl, not auto-applicable, matching the recipe's LINUX_AND_WSL and auto_applicable: false), raise the catalog count from 24 to 25, and move the reserved-handle note to `fix-18`, which is still the only gap in the numeric series. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * fix(diagnose): let the catalog fill fix-16's auto_applicable `check_16_vllm_oom` built its `Fix` with an inline `auto_applicable: false`. `no_checker_hand_sets_auto_applicable` forbids that: only `take_applicability_from_the_catalog` may set the field, because the catalog pass overwrites whatever a checker puts there. The output was right either way -- which is the point. A checker that disagreed with the catalog would be silently masked rather than caught, the way `fix-9-igpu-dgpu` once drifted. The field is now left at `Fix::default()` and filled from the recipe, whose `applies_on: PRINT_ON_LINUX_AND_WSL` already carries `FixClass::PrintOnly`. This was latent rather than new: the guard never ran on the previous head because the suite aborted at an unrelated flaky test first, so the offending line reached `gpu-out-of-memory` unnoticed. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> --------- Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com> Signed-off-by: Juho Vainio <juho.vainio@amd.com>

r0x0r marked this pull request as ready for review August 20, 2026 10:20

r0x0r requested a review from a team as a code owner August 20, 2026 10:20

r0x0r requested a review from juhovainio August 20, 2026 10:20

volen-silo mentioned this pull request Aug 20, 2026

feat(vllm): enhance OOM diagnostics and guidance in serve summary (eai-8059) #284

Open

r0x0r force-pushed the gpu-out-of-memory branch from ee202f0 to a97a39f Compare August 21, 2026 10:14

r0x0r force-pushed the oom-diagnostics-catalog branch from d2cd3ac to 8d9f22d Compare August 21, 2026 11:20

r0x0r marked this pull request as draft August 21, 2026 11:21

r0x0r marked this pull request as ready for review August 21, 2026 12:36

r0x0r force-pushed the oom-diagnostics-catalog branch from 8d9f22d to a3e7db8 Compare August 25, 2026 08:00

r0x0r force-pushed the oom-diagnostics-catalog branch from a3e7db8 to 2b23692 Compare August 25, 2026 08:13

r0x0r force-pushed the oom-diagnostics-catalog branch from 2b23692 to 7bd33b8 Compare August 25, 2026 08:16

style: rustfmt the new OOM --symptom routing test assertion

df2d04b

Signed-off-by: Roman Sirokov <roman.sirokov@amd.com>

volen-silo approved these changes Aug 28, 2026

View reviewed changes

r0x0r merged commit 697d6e3 into gpu-out-of-memory Aug 28, 2026
24 of 26 checks passed

r0x0r deleted the oom-diagnostics-catalog branch August 28, 2026 08:12

r0x0r mentioned this pull request Aug 28, 2026

feat(vllm): Tackle out of memory errors (EAI-8058) #251

Merged

Navigation Menu

Repository navigation

feat(diagnose): add the vLLM out-of-memory failure mode to the catalog (EAI-8060) - #290

feat(diagnose): add the vLLM out-of-memory failure mode to the catalog (EAI-8060)#290
r0x0r merged 4 commits into
gpu-out-of-memoryfrom
oom-diagnostics-catalog

r0x0r commented Aug 20, 2026 •

edited

Loading

Uh oh!

volen-silo commented Aug 20, 2026

Uh oh!

r0x0r commented Aug 21, 2026

Uh oh!

volen-silo commented Aug 25, 2026

Uh oh!

r0x0r commented Aug 25, 2026

Uh oh!

volen-silo left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

r0x0r commented Aug 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

What changed

Dependencies / stacking

Test plan

Uh oh!

volen-silo commented Aug 20, 2026

Uh oh!

r0x0r commented Aug 21, 2026

Uh oh!

volen-silo commented Aug 25, 2026

Uh oh!

r0x0r commented Aug 25, 2026

Uh oh!

volen-silo left a comment

Choose a reason for hiding this comment

Nits (non-blocking)

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

r0x0r commented Aug 20, 2026 •

edited

Loading