ci: grade the rocm-doctor skill with skillscope by volen-silo · Pull Request #356 · ROCm/rocm-cli

volen-silo · 2026-09-08T10:42:39Z

Stacked on #149 — based on test/rocm-doctor-skill-contract, so this diff
is just the one commit. Draft until #149 lands; it will be rebased onto main
after that.

What

#149 makes the rocm-doctor skill's prose testable against the binary it drives.
This adds the other half: the parts an agent runtime reads before it gets that
far.

A name that disagrees with its folder, a description that never got written,
frontmatter that is not valid YAML — each one stops the skill loading or stops it
being found, and every one of them fails silently. What you see is an agent that
simply never uses the skill. A relative link into a renamed file fails the same
way: the agent follows it, finds nothing, and improvises.

amd/skillscope is AMD's harness for this.

Changes

skill-evals job — runs only skillscope's structural command: no agent, no
API key, and no network beyond the install, so it can gate a merge without ever
being wrong for a reason of its own. It checks the frontmatter, the
skill-card.md sections, every internal markdown reference, and the dataset's
coverage bar. Gated on a new skills paths filter, and modelled on the existing
license-headers job.

evals/evals.json — four prompts the skill must answer and two near misses it
must decline, covering the scope gate, the consent rule and the Phase 0 probe.
Seeded from the dataset amd/skills already grades this skill with, plus one
case that pins the correction #149 makes: a vague report where nothing clears the
threshold must be routed upstream on has_match, not diagnosed from the
sub-threshold entries still sitting in matched.

Federation does not carry a skill's evals/ folder in either direction, so this
dataset is ours and the catalog keeps its own.

Scope

routing and behavioral are not run. Both need an authenticated claude
CLI and an ANTHROPIC_API_KEY, and this repo has no such secret. They are the
half of the question that asks whether the skill actually fires — the failure
most skills have — so this job is not a substitute for them. The dataset is
written and structurally checked so switching them on is a workflow change and
nothing more.

skills/rocm-cli-assistant/ is deliberately out of scope. Despite the path it is
not a published skill: apps/rocm/src/main.rs embeds it verbatim into the chat
assistant's system prompt with include_str!, so giving it the YAML frontmatter
this check requires would inject that frontmatter into a live prompt. It would
also need a dataset of its own, for something no agent runtime ever routes to.

Verification

Run against the exact SHA the workflow pins (6dde8e8a, v0.1.0), with the
exact CI arguments:

skillscope structural --skills-dir 'skills/rocm-doctor' --skill-files skill-card.md --skill-sections Description,Owner,License — green, exit 0
--external — 9 outbound URLs answered. CI omits it on purpose: a rate-limited
host is a fact about the run, not a broken link.

Checked that it is not a vacuous green — each of these exits 1 with a specific
message, and the restored state exits 0:

broken deliberately	reported
`name: rocm-doctorr`	`name` disagrees with the folder
all negative cases removed	0 of a required 2 `skill_should_trigger: false`
`[reference.md](refrence.md)`	points at a file that does not exist

Also confirmed the action's launcher accepts a commit-SHA pin
(github.action_ref → resolve_version.py, whose ref pattern takes a 40-hex
SHA), and that /.skillscope/ is ignored.

rominf

Reviewed against test/rocm-doctor-skill-contract rather than main, so #149's changes aren't counted against this PR. Noting it's still a draft — reviewing because you asked, not to push it forward before its dependency lands.

The CI shape is safe and I checked the parts that usually go wrong: pull_request rather than pull_request_target, inherited contents: read, no secrets reaching the job, a timeout, fail-closed on grader error. The evals file validates against skillscope's schema and clears the real Tier 0 minimum with 4 positive and 2 negative cases, and both logs_contain literals do appear in the skill, so it isn't vacuous in the ways I could check. .gitignore adds only /.skillscope/. Stacked conduct is right: draft, and the body links #149 with the rebase plan.

Two things need fixing before this is ready, both in the same category — the PR is about making a check real, and in two places it asserts an enforcement that doesn't currently exist.

I'd also fold this into a conversation with #149 rather than treating them separately: that PR deleted its drift-check on the strength of a federation that doesn't cover this repo, and this one documents a gate that isn't in branch protection. Same shape twice.

rominf · 2026-09-10T10:11:18Z

+    },
+    {
+      "id": "rocm-under-wsl2",
+      "skill_should_trigger": false,


I think this case is the wrong shape, and it will read as a skill bug rather than an eval bug when it fires.

Per skillscope's evals.schema.json, skill_should_trigger: false means no skill loads at all, so there's no behavioural phase to grade. But SKILL.md devotes a scope gate and an "Out of scope" section to WSL2 — rocm examine/diagnose detect it and route out, relay that guidance, point at AMD's ROCm-on-WSL guide — and that's behaviour that only happens once the skill has loaded. This case's own note says the skill "declines rather than troubleshoots", which is a description of it firing.

The prompt also matches the frontmatter description almost word for word (torch.cuda.is_available() false, AMD GPU, Linux or Windows), so routing will very likely fire it. Nothing catches the mismatch today because structural never runs the prompts — it'll surface the moment routing is enabled, and the tempting fix at that point is to weaken the skill description, which would be the wrong repair.

Suggest making it a triggering case that grades the decline: skill_should_trigger: true, expected_behavior = state WSL2 is out of scope and point at the ROCm-on-WSL guide, unexpected_behavior = run examine/diagnose/fix or offer any WSL2 troubleshooting. You'd then want a genuine second near-miss to keep the two-negative floor — a pure Windows-driver or NVIDIA-container prompt would do it. cuda-broken-on-nvidia is a clean negative as written; it's only this one.

Agreed, this was the wrong shape. Flipped rocm-under-wsl2 to skill_should_trigger: true with expected_behavior (state WSL2 is out of scope, point at AMD's ROCm-on-WSL guide) and unexpected_behavior (run examine/diagnose/fix, or offer any WSL2 troubleshooting), matching the skill's own Out-of-scope section almost verbatim. Added a fresh negative, nvidia-container-toolkit-unrelated (a pure NVIDIA container-runtime error with no ROCm/AMD vocabulary), to keep the two-negative floor. Re-ran the pinned skillscope structural command locally against the new file: 12 cases, exit 0.

rominf · 2026-09-10T10:11:18Z

+        uses: amd/skillscope@6dde8e8a34ad5456d3c2ff3418bc81c2cd9d5d69 # v0.1.0
+        with:
+          command: structural
+          skills: skills/rocm-doctor


This names one directory while the comment above and the paths filter both describe the whole of skills/, so a second skill added later is silently never graded.

skillscope's own safety net can't help here: structure.errors() only reports "no skill found" when the configured set is empty, and this still resolves rocm-doctor, so a new skills/foo/ with broken frontmatter passes green. That's the same silent-non-grading failure this PR is built to prevent, just deferred by one skill.

Either pass a glob once rocm-cli-assistant is excluded or moved, or add a one-line guard that fails when ls -d skills/*/ turns up a directory that isn't in the graded list — so adding a skill forces a deliberate CI edit instead of quietly widening the gap.

Added the cheap guard rather than a glob: a step that lists skills/*/ and fails closed if it finds a directory that's neither graded by the skillscope step nor excluded by design (rocm-cli-assistant, per AGENTS.md #7). A third skill folder now forces a deliberate CI edit instead of passing green ungraded. Left the skillscope 'skills:' input pointed at skills/rocm-doctor specifically, since skillscope grades one directory at a time and rocm-cli-assistant still needs to stay excluded either way.

Addressed

The skill's prose is now tested against the binary, but nothing checked the parts an agent runtime reads before it ever gets that far: the frontmatter that decides whether the skill loads at all, and the links it follows into reference.md. A `name` that disagrees with its folder, or a link into a renamed file, fails silently -- the agent simply never uses the skill, or follows the link, finds nothing, and improvises. skillscope is AMD's harness for exactly this. Only its `structural` command runs here: no agent, no API key, and no network beyond the install, so it can gate a merge without ever being wrong for a reason of its own. It checks the frontmatter, the `skill-card.md` sections, every internal reference, and the dataset's coverage bar. `evals/evals.json` is that dataset -- four prompts the skill must answer and two near misses it must decline, over the scope gate, the consent rule and the Phase 0 probe. One case pins the correction this series just made: a vague report where nothing clears the threshold must be routed upstream on `has_match`, not diagnosed from the sub-threshold entries still sitting in `matched`. Federation does not carry `evals/`, so this dataset is ours and the catalog keeps its own. `routing` and `behavioral` stay off: both need an authenticated `claude` CLI and an ANTHROPIC_API_KEY this repo does not have. They are the half of the question that asks whether the skill actually fires, so the dataset is written and checked to make switching them on a workflow change and nothing more. skills/rocm-cli-assistant/ is deliberately out of scope. Despite living under skills/ it is not a published skill: main.rs embeds it verbatim into the chat system prompt with include_str!, so giving it the frontmatter this check requires would inject that frontmatter into a live prompt. Signed-off-by: Eugene Volen <Eugene.Volen@amd.com>

Address review feedback: the AGENTS.md/ci.yml text claimed skill-evals already blocks merges, when it isn't a required status check yet, and claimed the pinned skillscope action fully covers supply-chain drift, when the composite's own steps and the uvx-resolved harness aren't pinned. Both are now stated accurately instead of asserted. Also: the changes-job `skills` filter now matches its siblings by covering all of `.github/workflows/**` instead of just `ci.yml`; a new guard step fails the job if a skills/ directory shows up that isn't graded or excluded by design, so a second skill can't go ungraded silently; and the WSL2 eval case is reshaped to `skill_should_trigger: true` (the skill loads to decline it, per its own scope gate) with a fresh negative case restoring the two-negative floor. Signed-off-by: Eugene Volen <Eugene.Volen@amd.com>

jussielo-amd · 2026-09-25T06:54:42Z

Review: clean diff, approve-in-principle — a couple of process items to sort before merge.

Code quality: No dead code — every addition is exercised (the skills filter output feeds the new job's if, the guard step actually runs, evals.json is consumed by skillscope's coverage-bar check, .gitignore matches a real artifact dir). No user-facing/runtime impact — this is CI + docs only, no changes to the rocm binary.

Two minor non-blocking nits:

The SHA-pin comment in the new skill-evals job slightly overstates its own coverage — it reads as if the pin secures the whole supply chain, but a few lines later it correctly notes the uvx-resolved harness and the composite's own moving-tag steps aren't covered by it. Worth tightening the wording so it doesn't contradict itself.
The guard step's for dir in skills/*/ would fail loudly on a spurious unglobbed literal if skills/ ever had zero subdirectories (nullglob isn't set). Dormant today since that can't happen yet, but cheap to harden with shopt -s nullglob.

Worth confirming before anyone merges anything: the PR description says "Draft until #149 lands" and "this diff is just the one commit," but the PR itself isn't marked as a GitHub draft, and it has 2 commits. Since GitHub's merge button doesn't read PR body text, could you either flip this back to draft or update the description so the stacked-dependency intent is unambiguous? Not a concern about the code — just want to make sure this doesn't get merged into test/rocm-doctor-skill-contract (or worse, main) before #149 is ready.

Also noting for context: independently confirmed this doesn't conflict with #413's already-merged fix-9 auto_applicable fix, so no rebase risk there.

jussielo-amd

Approving the diff — clean, no blocking issues found (see comment above for the two minor nits and the process note).

Contingent on #149 landing: this targets test/rocm-doctor-skill-contract, not main, and #149 still has open review feedback. Please also square the description's "Draft until #149 lands" / "one commit" wording against the actual (non-draft, 2-commit) state before this merges anywhere.

* ci: grade the rocm-doctor skill with skillscope The skill's prose is now tested against the binary, but nothing checked the parts an agent runtime reads before it ever gets that far: the frontmatter that decides whether the skill loads at all, and the links it follows into reference.md. A `name` that disagrees with its folder, or a link into a renamed file, fails silently -- the agent simply never uses the skill, or follows the link, finds nothing, and improvises. skillscope is AMD's harness for exactly this. Only its `structural` command runs here: no agent, no API key, and no network beyond the install, so it can gate a merge without ever being wrong for a reason of its own. It checks the frontmatter, the `skill-card.md` sections, every internal reference, and the dataset's coverage bar. `evals/evals.json` is that dataset -- four prompts the skill must answer and two near misses it must decline, over the scope gate, the consent rule and the Phase 0 probe. One case pins the correction this series just made: a vague report where nothing clears the threshold must be routed upstream on `has_match`, not diagnosed from the sub-threshold entries still sitting in `matched`. Federation does not carry `evals/`, so this dataset is ours and the catalog keeps its own. `routing` and `behavioral` stay off: both need an authenticated `claude` CLI and an ANTHROPIC_API_KEY this repo does not have. They are the half of the question that asks whether the skill actually fires, so the dataset is written and checked to make switching them on a workflow change and nothing more. skills/rocm-cli-assistant/ is deliberately out of scope. Despite living under skills/ it is not a published skill: main.rs embeds it verbatim into the chat system prompt with include_str!, so giving it the frontmatter this check requires would inject that frontmatter into a live prompt. Signed-off-by: Eugene Volen <Eugene.Volen@amd.com> * ci: correct skill-evals claims and close its coverage gaps Address review feedback: the AGENTS.md/ci.yml text claimed skill-evals already blocks merges, when it isn't a required status check yet, and claimed the pinned skillscope action fully covers supply-chain drift, when the composite's own steps and the uvx-resolved harness aren't pinned. Both are now stated accurately instead of asserted. Also: the changes-job `skills` filter now matches its siblings by covering all of `.github/workflows/**` instead of just `ci.yml`; a new guard step fails the job if a skills/ directory shows up that isn't graded or excluded by design, so a second skill can't go ungraded silently; and the WSL2 eval case is reshaped to `skill_should_trigger: true` (the skill loads to decline it, per its own scope gate) with a fresh negative case restoring the two-negative floor. Signed-off-by: Eugene Volen <Eugene.Volen@amd.com> --------- Signed-off-by: Eugene Volen <Eugene.Volen@amd.com>

…tract (ROCm#149) * test(rocm-core): cover the fix consent and exit-code contract Exit codes 1, 4 and 5 were asserted nowhere, and no test reached a runner that writes to disk. The two that looked like coverage did not: the dry-run test picks fix-2, whose Linux runner takes no FixOptions and never prompts, and the e2e preview scenario picks print-only fix-1, which returns before any runner runs. Split consent_without_prompt out of confirm, and let pin_device_in_rc_file take the rc path plus an injected consent verdict, so the mutation path is reachable from a test without touching $HOME or stdin. Behaviour is unchanged. Ten tests then cover the consent gate, both mutation outcomes, and the dry-run short-circuit. Signed-off-by: Eugene Volen <Eugene.Volen@amd.com> * fix(diagnose): correct fix-9's auto flag and drop unreachable routing check_9_igpu_dgpu_collision reported auto_applicable: false on Linux while fix::RECIPES -- what `rocm fix` dispatches on -- says true. It was the only OS-split checker whose branches diverged, and an agent branches on that field, so it was told fix-9 was print-only while `rocm fix` would happily apply it. route_when_no_match also matched on lemonade / ollama / lm-studio, and upstream_tracker carried those plus amdgpu-install, but Examination::probe only ever reports skipped, pytorch, llama-cpp or unknown. Those arms were unreachable, so the CLI advertised routing it cannot do. Routing an app that merely appears in the symptom text belongs to the caller, not the probe. Both are pinned: one test runs diagnose for each OS family and asserts every emitted fix agrees with the catalog, another pins the reachable target set. Signed-off-by: Eugene Volen <Eugene.Volen@amd.com> * test(e2e): publish the rocm-doctor skill and test its CLI contract The rocm-doctor skill is a thin driver: the probe, the closed 15-mode catalog and the fixes all ship inside the `rocm` binary. What the skill owns is a contract -- which fix-ids exist, which the CLI applies itself, which machines each is for, and what a diagnosis carries. Because it describes this binary, this repo is its source of truth; the catalog it is published through vendors the folder from here. Nothing tested that seam. diagnose.feature deliberately asserts only the shape of a diagnosis so it stays host-independent, so a renamed fix-id or a 16th failure mode would merge green and silently break the published skill. rocm_doctor_skill.feature parses reference.md's catalog table, its auto-applicable prose, the fields and thresholds it names, the verdicts it enumerates and the trackers it lists, then diffs each against what the CLI really reports. Every scenario is a query -- no GPU, no serve, no download, no mutation -- so they run on the blocking mock lane and need no capability tags. Reading a document as test data is a deliberate, narrow exception to the suite's black-box rule: nothing is imported from the codebase, and the document is the thing under test. The skill folder is excluded from the licence-header check: SKILL.md must open with YAML frontmatter for the skill loader, which pushes any header out of hawkeye's detection window. Verified in both directions -- the folder is exempt, and stripping a header elsewhere still fails the check. skills/** joins the `heavy` paths filter, since reference.md is now a fixture and a skill-only edit has to run the e2e job. Signed-off-by: Eugene Volen <Eugene.Volen@amd.com> * fix(doctor-skill): gate on has_match, and check the contract both ways The skill told an agent to route upstream "when `matched` is empty", which `DiagnoseReport::has_match` exists precisely to prevent: several checkers open with a nonzero base score for a merely potentially relevant situation, so a healthy host returns a non-empty `matched` of sub-threshold entries. An agent following that instruction proposes a fix for a machine with nothing wrong and never routes upstream. Both skill files now name `has_match` and gate on it. That gap was invisible to the contract feature because the field check ran one way only -- documented ⊆ emitted -- so a field the CLI emits and the document never mentions could not fail anything. It is now an equality, and `rocm diagnose` emits exactly the six keys the reference names. The route check had the same shape. It compared the CLI's target against every tracker in the Framework routing section, including the three the skill hands over itself, so a documented-but-unreachable CLI target passed silently. The section now separates the two lists and the CLI is held to its own. Scenario 6's Given was inert: `route_when_no_match` is populated whether or not anything matched, so the scenario passed identically for a symptom that did match. It now asserts `has_match` is false first. Where the catalog is out of scope the check still holds, since the catalog is not run at all; a matching symptom can only be produced on a host the catalog covers, so the no-GPU Linux lane is where it bites. Finally, `route_when_no_match`'s comment claimed a test would flag a newly added framework. That test iterates a hand-maintained list and reads nothing from `examine.rs`, so it would not. Reworded to name the three places that must be edited together. Signed-off-by: Eugene Volen <Eugene.Volen@amd.com> * test(e2e): index the doctor feature, and pin has_match where the host holds still Two CI failures, both mine. `rocm_doctor_skill.feature` had no `FEATURE_KEYS` entry, so `feature_files_and_declared_keys_agree` panicked and took `Test (affected crates)`, `windows-build-and-test` and `E2E tests` down with it. Adds the key `skill` and renames the six scenarios to `skill-01`..`skill-06`, which the sibling naming tests require once a file is indexed. The `@id:` tags already carried the prefix. I missed this locally by running the workspace suite with `--exclude e2e-cucumber` -- the crate that holds the test. Scenario 6 then asserted `has_match == false` on the grounds that its symptom carried no catalog keyword. That premise does not survive a real host: `diagnose` scores several checkers from host state alone, and the GitHub-hosted Linux runner ships /etc/modprobe.d/blacklist-radeon-instinct.conf with amdgpu unloaded, which is `fix-5-amdgpu-load` at score 90 no matter what symptom is passed. No symptom can hold that still, so the assertion only encoded a runner's state. It passed on WSL2 because the catalog is skipped there entirely, which is why it looked host-independent. The claim moves to `rocm-core`, where the Examination is constructed instead of probed: a host whose only causes are sub-threshold has a NON-empty `matched` and `has_match` false, and the route still names somewhere to go -- the exact shape that makes gating on emptiness wrong. A companion test asserts a matching symptom flips the flag, so the assertion is not free. What is left in the feature is the half only it can check: whatever target the CLI hands back is one the published document names. The scenario is renamed to say that, rather than to claim a premise it does not establish. Signed-off-by: Eugene Volen <Eugene.Volen@amd.com> * docs(doctor-skill): stop promising a dry run fix-2 never performs on Linux `reference.md` told an agent that auto fixes "print the exact command, honor --dry-run, refuse on a non-interactive shell without --yes, and confirm before mutating". That is false for fix-2 on Linux. `run_unset_override` splits by platform: the Windows arm takes `FixOptions` and does exactly that, while `run_unset_override_linux` takes no options at all -- it reports where the override is set and which rc files carry it, then stops, because editing a user's dotfiles is not its business. So there is no prompt to answer, nothing for --dry-run to preview, and --yes is never read. An agent following the doc would tell a Linux user a change was previewed that was never going to happen. `auto_applicable` is a flat bool on a const recipe, and making it OS-aware would ripple through the listing, the diagnosis JSON and the catalog row, so the flag keeps its meaning and the docs stop overpromising: the Auto-fix column is now defined as what `rocm fix` actually reports -- the CLI has a runner and will carry it out itself -- and the mutation contract moves to prose that names fix-2-on-Linux as the exception. Pinned so it cannot drift back. `auto_applicable_recipes_have_a_runner` cannot catch this, because fix-2 does have a runner. The rc-file scan is split into `report_persistent_override`, taking its candidate paths the way this PR already made `pin_device_in_rc_file` take its rc path -- deriving them from $HOME would mean mutating a process-global in a parallel test suite. The test asserts the files are byte-identical afterwards rather than asserting an exit code, since a runner that started stripping the line would still return 0; confirmed it fails when the runner is made to mutate. Also drops a stale duplicate doc line left on `upstream_tracker` when the framework-keyed arms were removed. Signed-off-by: Eugene Volen <Eugene.Volen@amd.com> * fix(rocm-doctor): correct fix-9 scope, federation, and test-claim wording fix-9-igpu-dgpu mutates on both Linux and Windows, gated only by --device-index, not by platform like fix-2's documented exception. Note both in reference.md and SKILL.md, and surface the same caveat in the CLI's own diagnosis notes so an agent reading a diagnosis (not just the docs) sees it too. AGENTS.md's federation description no longer matches amd/skills: there is no automated nightly vendoring job for rocm-doctor, and the skill sits under staging/ which no federation job covers yet. Rewrite the section to say what actually happens today (hand-sync during Phase-1 incubation) instead of describing a job that doesn't exist, and attribute the failure catalog's checkers/OS-scoping to diagnose.rs alongside fix.rs. Narrow the e2e suite's and README's claims to what skill_steps.rs actually parses: the catalog table and the auto-applicable prose line are checked; the OS-scope summary line and the failure-mode/print-only counts are not, and can drift without failing anything here. Also reword the dead None arm in assert_route_is_documented into a loud panic, since documented_cli_routes only ever returns Some in practice. Add skills/** to e2e-selfhosted.yml's heavy path filter so it matches ci.yml: a skill-only edit should trigger the e2e job that uses it as a fixture. Signed-off-by: Eugene Volen <Eugene.Volen@amd.com> * docs(doctor-skill): document fix-17 so the catalog matches the CLI `fix-17-torch-dlpack` landed in `crates/rocm-core` but never reached `reference.md`, so the doc-vs-CLI scenarios this branch introduces (skill-01..03) failed on every E2E lane: `rocm fix` offered an id the published catalog did not name. Add the table row, correct the mode count and the Linux-only recap, and bump SKILL.md's print-only count. Call out the `fix-16` gap explicitly -- the id is a reserved handle, so a reader counting rows against the heading would otherwise read the jump as a missing entry. The counts and recap line are free-standing prose that no step parses; only the table row is what turned the lanes green. Signed-off-by: Eugene Volen <Eugene.Volen@amd.com> * docs(doctor-skill): bring the published catalog onto the WSL2 platform family WSL2 stopped being off-catalog: it is now a platform family of its own with seven entries, and four cross-platform fixes widened to reach it. The skill still described the old world, so `reference.md` named 16 of the CLI's 23 remediations and scoped four of them wrongly -- the doc-vs-CLI scenarios caught both. Add the seven `fix-wsl-*` rows, replace the `both` OS cell with the explicit family list, and retire the WSL2 out-of-scope entry in favour of a note on why it still gets its own entries rather than inheriting the bare-metal ones. SKILL.md needed more than a count. Its Scope gate told an agent to decline a WSL2 user outright and offer no troubleshooting whatsoever, which is now the opposite of what the CLI does; the gate narrows to the GPU vendor, and WSL2 is called out as explicitly in scope in the four places that excluded it. The `out_of_scope` field is documented as what it now means -- no entries for this platform family at all, so nothing was checked. Signal cells are written from what each checker actually keys on rather than from its rationale: fix-wsl-5 fires on the distro release, not on a glibc error, and fix-wsl-6 on the host reporting no AMD adapter. Signed-off-by: Eugene Volen <Eugene.Volen@amd.com> * docs(doctor-skill): document fix-19 so the catalog matches the CLI `fix-19-shm-too-small` landed in the CLI catalog while this branch was in review, so rebasing onto main put the published skill one entry behind what `rocm fix` offers. The contract scenarios caught it: skill-01, skill-02 and skill-03 all failed on the same missing row. Add it with the scope and flag the CLI actually reports -- linux/wsl, print-only -- and carry the three free-standing prose restatements that nothing parses: the heading count, the OS-scope summary line, and SKILL.md's print-only count. Signed-off-by: Eugene Volen <Eugene.Volen@amd.com> * ci: grade the rocm-doctor skill with skillscope (ROCm#356) * ci: grade the rocm-doctor skill with skillscope The skill's prose is now tested against the binary, but nothing checked the parts an agent runtime reads before it ever gets that far: the frontmatter that decides whether the skill loads at all, and the links it follows into reference.md. A `name` that disagrees with its folder, or a link into a renamed file, fails silently -- the agent simply never uses the skill, or follows the link, finds nothing, and improvises. skillscope is AMD's harness for exactly this. Only its `structural` command runs here: no agent, no API key, and no network beyond the install, so it can gate a merge without ever being wrong for a reason of its own. It checks the frontmatter, the `skill-card.md` sections, every internal reference, and the dataset's coverage bar. `evals/evals.json` is that dataset -- four prompts the skill must answer and two near misses it must decline, over the scope gate, the consent rule and the Phase 0 probe. One case pins the correction this series just made: a vague report where nothing clears the threshold must be routed upstream on `has_match`, not diagnosed from the sub-threshold entries still sitting in `matched`. Federation does not carry `evals/`, so this dataset is ours and the catalog keeps its own. `routing` and `behavioral` stay off: both need an authenticated `claude` CLI and an ANTHROPIC_API_KEY this repo does not have. They are the half of the question that asks whether the skill actually fires, so the dataset is written and checked to make switching them on a workflow change and nothing more. skills/rocm-cli-assistant/ is deliberately out of scope. Despite living under skills/ it is not a published skill: main.rs embeds it verbatim into the chat system prompt with include_str!, so giving it the frontmatter this check requires would inject that frontmatter into a live prompt. Signed-off-by: Eugene Volen <Eugene.Volen@amd.com> * ci: correct skill-evals claims and close its coverage gaps Address review feedback: the AGENTS.md/ci.yml text claimed skill-evals already blocks merges, when it isn't a required status check yet, and claimed the pinned skillscope action fully covers supply-chain drift, when the composite's own steps and the uvx-resolved harness aren't pinned. Both are now stated accurately instead of asserted. Also: the changes-job `skills` filter now matches its siblings by covering all of `.github/workflows/**` instead of just `ci.yml`; a new guard step fails the job if a skills/ directory shows up that isn't graded or excluded by design, so a second skill can't go ungraded silently; and the WSL2 eval case is reshaped to `skill_should_trigger: true` (the skill loads to decline it, per its own scope gate) with a fresh negative case restoring the two-negative floor. Signed-off-by: Eugene Volen <Eugene.Volen@amd.com> --------- Signed-off-by: Eugene Volen <Eugene.Volen@amd.com> * fix(doctor-skill): carry WSL2's in-scope move to the surfaces it missed Bringing WSL2 into the catalog updated the skill's body but not the two places that decide whether the body is ever read, nor the dataset that grades it. The frontmatter description and the skill card both said Linux and Windows. That is the surface a runtime routes on, so a WSL2 user could fail to load the skill at all and never reach the five places the body insists WSL2 is in scope. The `rocm-under-wsl2` eval asserted the opposite of what the skill now requires: decline the platform, run nothing, offer no troubleshooting. Its `skill_should_trigger` flag had been flipped to true without the body following, so a skill obeying SKILL.md failed the case and one passing it violated SKILL.md. Rewrite it to grade the in-scope workflow, and to check the CLI's own scoping holds -- a bare-metal Linux remediation reaching a WSL2 host means the agent went around it. Separately, `routing_targets_cover_every_framework_the_probe_reports` did not pin what its commit said it did. The loop carried only the four frameworks the probe reports, so restoring an unreachable `lemonade` arm to `route_when_no_match` left the observed target set unchanged and the test green. Add the three removed names: absent arms route them to `rocm-core` and nothing changes, a restored arm grows the set and fails. Signed-off-by: Eugene Volen <Eugene.Volen@amd.com> * fix(doctor-skill): stop the WSL2 eval and the drift test over-claiming The WSL2 eval's expectation said the CLI returns "the `fix-wsl-*` family" on that platform. It does not: twelve catalog entries reach the `wsl` family, because `fix-1`, `fix-2`, `fix-6` and `fix-8` are scoped to all three and `fix-19` to Linux and WSL. The entry's own prompt -- `torch.cuda.is_available()` false -- is exactly what `fix-1` and `fix-8` fire on, so an agent surfacing them would have been graded as violating an expectation while doing what the skill asks. Drop the enumeration and let the instruction stand on what it actually means: stay on the entries the CLI returned, whatever they are. `every_diagnosis_agrees_with_the_fix_catalog_on_auto_applicability` carried a doc comment describing fix-9's flag as a self-contradiction this branch repaired. `main` landed that correction independently while this branch was in review, so the rebase left the claim without a diff behind it. Reword it to say what the test guards going forward, which is what a reader needs from it anyway. Signed-off-by: Eugene Volen <Eugene.Volen@amd.com> * fix(doctor-skill): drop a repeated note and two silent traps fix-9 said the same thing twice. The detected-targets note carried an appended "--device-index is required" sentence and the notes list carried a second, longer one saying exactly that; render_report_text prints every note on its own line, so `rocm diagnose` showed two bullets repeating each other on Linux and on Windows alike. Keep the standalone note, which also explains what the AUTO flag does and does not promise, and drop the appended copy. A test now pins the caveat to exactly one rendered note line, and pins the notes to be distinct, on both OS branches. The skill contract's catalog reader rewrote an os-scope cell of "both" to "linux/windows". No row spells it that way, so it was dead, but it was dead code that could only ever do harm: reintroduced for a fix scoped to linux, windows and wsl, it would have dropped "wsl" and made the document and the CLI agree when they did not -- the one mismatch the contract exists to catch. Take the cell verbatim instead, so an invented spelling surfaces as the os-scope diff it is. A new guard test rejects any cell that is not a "/"-joined list of the platforms `rocm fix` prints, and it runs in the ordinary `cargo test` set rather than behind the E2E suite. The ungraded-skills CI guard iterated `skills/*/` under `set -euo pipefail` with no nullglob. Verified against an empty tree: it reported "skills/* is neither graded ..." and exited 1, naming a skill that does not exist. With `shopt -s nullglob` the loop does not run, which is the right answer for nothing to grade, and an ungraded directory is still caught. Signed-off-by: Eugene Volen <Eugene.Volen@amd.com> * fix(diagnose): reattach a test doc comment to the test it describes Two doc comments had merged. The block explaining what `routing_targets_cover_every_framework_the_probe_reports` pins ended up on `sub_threshold_causes_leave_has_match_false_and_route_upstream`, because a later commit inserted the has_match tests between the comment and its function and the two blocks ran together with no separator. It reads as false prose on one test, which has no framework list below it and pins no reachable set, and as a missing warning on the other: that paragraph is the only record that the list is hand-maintained and will not notice a fifth framework added to the probe, which is exactly what a maintainer needs when they touch it. Also make the paragraph honest about the list it now describes. Three entries were added to catch a re-added unreachable arm, so the list is no longer only the probe's own values. Signed-off-by: Eugene Volen <Eugene.Volen@amd.com> * fix(rocm-core): fix-2's Windows arm never returns 5 on decline An `else if confirm(...)` with no matching `else` let a declined (or, without --yes, a non-interactive) User-scope clear fall through silently. Nothing printed, nothing ran, and the function still returned 0 at the bottom -- indistinguishable from "nothing to clear" or "cleared successfully". skills/rocm-doctor/reference.md documents 5 as "user declined"; an agent driving this fix through the CLI had no way to tell a no from a yes. A decline still can't return 5 on the spot, though: if the Machine scope is also set, its elevated-shell guidance has to print unconditionally regardless of what happened with the User scope, and an early return would skip it. So the decline is recorded in a flag and checked once after both scopes have had their say. Pulled the scope-dependent logic into report_and_clear_override_windows, taking the two scope values and consent as parameters instead of reading them from ps_env_scope/ confirm directly -- the same shape pin_device_in_rc_file already uses, and for the same reason: it lets the decline path be exercised by a test without a real Windows host. Signed-off-by: Eugene Volen <Eugene.Volen@amd.com> --------- Signed-off-by: Eugene Volen <Eugene.Volen@amd.com>

volen-silo force-pushed the feat/skillscope-skill-evals branch 2 times, most recently from ea5f1d3 to a2d814d Compare September 8, 2026 11:42

rominf previously requested changes Sep 10, 2026

View reviewed changes

volen-silo force-pushed the test/rocm-doctor-skill-contract branch 2 times, most recently from c7a236c to 88b35c6 Compare September 11, 2026 11:07

volen-silo force-pushed the test/rocm-doctor-skill-contract branch from 88b35c6 to d6096b9 Compare September 25, 2026 05:55

volen-silo force-pushed the feat/skillscope-skill-evals branch from 4b9f663 to 52ed882 Compare September 25, 2026 05:58

volen-silo marked this pull request as ready for review September 25, 2026 05:58

volen-silo requested a review from a team as a code owner September 25, 2026 05:58

volen-silo requested review from juhovainio and jussielo-amd and removed request for a team and jussielo-amd September 25, 2026 05:58

volen-silo added 2 commits September 25, 2026 06:18

volen-silo force-pushed the feat/skillscope-skill-evals branch from 52ed882 to c7a7d99 Compare September 25, 2026 06:18

jussielo-amd approved these changes Sep 25, 2026

View reviewed changes

volen-silo merged commit 9ef3777 into test/rocm-doctor-skill-contract Sep 25, 2026
23 of 25 checks passed

volen-silo deleted the feat/skillscope-skill-evals branch September 25, 2026 06:55

Navigation Menu

ci: grade the rocm-doctor skill with skillscope - #356

ci: grade the rocm-doctor skill with skillscope#356
volen-silo merged 2 commits into
test/rocm-doctor-skill-contractfrom
feat/skillscope-skill-evals

volen-silo commented Sep 8, 2026

Uh oh!

rominf left a comment

Uh oh!

Uh oh!

Uh oh!

rominf Sep 10, 2026

Uh oh!

volen-silo Sep 11, 2026

Uh oh!

rominf Sep 10, 2026

Uh oh!

volen-silo Sep 11, 2026

Uh oh!

Uh oh!

jussielo-amd commented Sep 25, 2026

Uh oh!

jussielo-amd left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

volen-silo commented Sep 8, 2026

What

Changes

Scope

Verification

Uh oh!

rominf left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

rominf Sep 10, 2026

Choose a reason for hiding this comment

Uh oh!

volen-silo Sep 11, 2026

Choose a reason for hiding this comment

Uh oh!

rominf Sep 10, 2026

Choose a reason for hiding this comment

Uh oh!

volen-silo Sep 11, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jussielo-amd commented Sep 25, 2026

Uh oh!

jussielo-amd left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants