Treat GPU activity as optional and hand the worktree back clean

.claude/skills/validate-kernel-pr/SKILL.md

@@ -69,6 +69,7 @@ For a local candidate with no remote head, omit `--head-sha`. The report then re
 | `--expected-route` | exact `module:function` route the validator-owned profiler must observe |
 | `--shape-vars` | comma-separated local names captured from each route call, in grid order |
 | `--shape-env` `--grid` | env var and shape list for the S1-owned grid |
+| `--shape-arg` | the target's own CLI flag that accepts shapes, for script targets that read no env var |
 | `--tol-table` | reference tolerances, e.g. `f32=1e-5,f16=2e-3,bf16=1e-2` |
 | `--label` `--out` | run name and report path (default `./validation_report.json`) |
 
@@ -96,6 +97,11 @@ The supplied worktree must be clean. The report records the base commit, patch S
 caller-supplied head OID. A direct head checkout without a patch can run diagnostics, but cannot
 prove mergeability or base attribution and therefore cannot produce `PASS`.
 
+The patch is reverted when the process exits, including on interrupt and on every degraded path,
+so the worktree is handed back in the state it was supplied. Consecutive runs in the same worktree
+are therefore supported; a run that left the patch applied would make the next one report
+`not isolated-clean` and blame the caller.
+
 ### 2 — `gpu_claim`
 
 Claim a GPU over a **sampling window**, not one instantaneous reading, and acquire a non-blocking
@@ -111,9 +117,18 @@ The report records host, HIP index, matching AMD SMI index, BDF, market name, ar
 GFX activity before the run. `pick-idle-gpu.py` emits the **translated HIP index**; the validator
 maps it back through AMD SMI enumeration instead of incorrectly using it as an AMD SMI index.
 
+`amdsmi_get_gpu_activity` is not available everywhere — some driver and amd-smi combinations fail
+it outright or report `N/A` while enumeration, BDF, ASIC and VRAM queries all work. Activity is
+therefore treated as optional, and `gpu_claim.idleness_basis` names the evidence the claim rests
+on: `activity+vram` when busy percentages were measured, `vram-only` when only resident VRAM
+separated the devices. In the `vram-only` case `gfx_activity_before_pct` is `null`, which means
+unknown, not zero — an unavailable metric is never reported as an observed idle GPU.
+
 If no GPU stays idle, `gpu_claim` is `skip`, `degraded_mode` is `NO_GPU`, both correctness stages
 are `skip`, and the verdict is `INCONCLUSIVE`. The script performs no architecture-specific
-compile in this branch, so it does not call the result `compile-only`.
+compile in this branch, so it does not call the result `compile-only`. That skip distinguishes two
+different facts: GPUs present but none idle is an environment fact, whereas AMD SMI being
+unqueryable is a portability gap in the validator and says nothing about the GPUs.
 
 ### 3 — `runtime_compat`
 
@@ -157,9 +172,10 @@ Runner selection is structural, not assumed:
 - otherwise a file with an `if __name__ == "__main__"` guard runs as `python <file>`;
 - a file with neither is `skip`, never a test failure.
 
-The report records `test_selection.runner` and `runner_reason`. Script targets can establish that
-their real repository entry point succeeds or fails, but cannot currently use the pytest route
-profiler, so even a successful script run tops out at `INCONCLUSIVE`.
+The report records `test_selection.runner` and `runner_reason`. A script target is profiled the
+same way a pytest target is: the probe is installed by a validator-owned runner that then executes
+the file under `runpy` with `run_name="__main__"`, so `execution_receipt` is reachable for both.
+Nothing about `sys.setprofile` needed pytest; pytest was only where the hook was installed.
 
 Both, and they are reported separately, because the interesting case is when they disagree.
 Pytest runs emit JUnit XML and a zero-executed/all-skipped target is `skip`, never `pass`.
@@ -181,9 +197,23 @@ The S1-owned grid must cover three classes the PR's own tests routinely miss:
 | boundary / odd | odd N, N not a multiple of the tile — where tail masks fail |
 | long-context / large M | where 32-bit index arithmetic wraps |
 
-The grid stage runs only when the selected target source references the configured
-`--shape-env`; otherwise it is `skip` and the verdict is `INCONCLUSIVE`. This is a positive
-control against reporting the same default test run twice under different stage names.
+The grid reaches the target through whichever channel that target actually reads, and the channel
+must be proven structurally before it is used:
+
+| channel | flag | proof the hook exists |
+|---|---|---|
+| environment variable | `--shape-env` | the source references `os.getenv(VAR)` / `os.environ[VAR]` |
+| the target's own CLI flag | `--shape-arg` | the source passes that flag literal to `add_argument` |
+
+Injecting through an env var only would have made this stage permanently inert for repositories
+whose tests take shapes on the command line — a limit of the injector, not of the target. The flag
+is named by the caller rather than guessed, because a wrong guess appends argv the target silently
+ignores. Neither channel is trusted on the strength of the AST scan alone: the stage re-runs the
+target with a deliberately invalid grid value and requires it to fail. A target that passes with
+garbage shapes is not consuming the grid, so the stage is `skip`, never credited.
+
+With no channel configured the stage is `skip` and the verdict is `INCONCLUSIVE`. This is a
+positive control against reporting the same default test run twice under different stage names.
 
 When the kernel exposes no shape override, the report says `repo-default-only` rather than
 claiming coverage it does not have.
@@ -205,7 +235,13 @@ records actual calls and writes:
 
 `PASS` requires the observed route to equal `--expected-route`, at least one observed route
 symbol, and every shape named by `--grid`. The tested PR cannot obtain credit merely by writing
-its own receipt; `validate-kernel-pr.validation_probe` owns the receipt producer.
+its own receipt; `validate-kernel-pr.validation_probe` owns the receipt producer, and the script
+runner calls that producer's own hooks rather than re-implementing them.
+
+A receipt is validated whenever a route was named, including when no grid was configured or the
+grid channel could not be established. With no grid it asserts route execution and nothing about
+shapes, which is all it is then entitled to claim. Abandoning the receipt along with the grid
+would discard evidence that was already collected.
 
 ### 7 — `index_width_scan` (informational)
 
@@ -280,8 +316,9 @@ a seeded defect, and these have not been:
   Choosing the right `--target` from a diff is the unsolved part; an irrelevant target can
   still produce `PASS`. The report names the target so a reviewer can reject that evidence, but
   the executor cannot decide relevance itself.
-- **External grid adapters.** A script-only PR target may lack a shape hook. The validator does
-  not yet accept an independently hashed `--extra-target`, because that harness must be bound
+- **External grid adapters.** A target that exposes no shape channel at all — neither an env var
+  for `--shape-env` nor a CLI flag for `--shape-arg` — still cannot be given a grid. The validator
+  does not accept an independently hashed `--extra-target`, because that harness must be bound
   without changing the PR diff hash or live-base identity. Such runs remain `INCONCLUSIVE`.
 - **Cross-architecture compilation.** `arch_coverage: compile-only` is reserved for a future
   stage that actually invokes an architecture-specific compiler. No-GPU mode does not claim it.

.claude/skills/validate-kernel-pr/pick-idle-gpu.py

@@ -1,12 +1,17 @@
+#!/usr/bin/env python3
 """Select an AMD GPU that stays idle across a sampling window."""
 
 from __future__ import annotations
 
 import argparse
+import os
 import sys
 import time
 from pathlib import Path
 
+ACTIVITY_BASIS = "activity+vram"
+VRAM_ONLY_BASIS = "vram-only"
+
 
 def parse_args() -> argparse.Namespace:
     parser = argparse.ArgumentParser(description=__doc__)
@@ -24,26 +29,60 @@ def parse_args() -> argparse.Namespace:
     return args
 
 
+def amdsmi_search_paths() -> list[Path]:
+    """Directories that have shipped the amdsmi bindings across ROCm releases."""
+    roots = [Path(os.environ["ROCM_PATH"])] if os.environ.get("ROCM_PATH") else []
+    roots.append(Path("/opt/rocm"))
+    roots.extend(sorted(Path("/opt").glob("rocm-*"), reverse=True))
+
+    candidates = [
+        Path("/usr/lib/python3/dist-packages"),
+        Path(
+            f"/usr/lib/python{sys.version_info.major}.{sys.version_info.minor}/dist-packages"
+        ),
+    ]
+    for root in roots:
+        # ROCm >= 7.1 ships the bindings under share/amd_smi; older builds put
+        # them next to the CLI. Probe both, newest ROCm first.
+        candidates.append(root / "share" / "amd_smi")
+        candidates.append(root / "libexec" / "amdsmi_cli")
+    return candidates
+
+
 def import_amdsmi():
     try:
         import amdsmi
 
         return amdsmi
     except ImportError:
-        for candidate in (
-            Path("/usr/lib/python3/dist-packages"),
-            Path(
-                f"/usr/lib/python{sys.version_info.major}.{sys.version_info.minor}/dist-packages"
-            ),
-            Path("/opt/rocm/libexec/amdsmi_cli"),
-        ):
+        for candidate in amdsmi_search_paths():
             if candidate.is_dir() and str(candidate) not in sys.path:
                 sys.path.append(str(candidate))
         import amdsmi
 
         return amdsmi
 
 
+def read_activity(amdsmi, handle) -> tuple[int | None, int | None]:
+    """Return (gfx, umc) busy percentages, or None for whichever is unknown.
+
+    Some driver and amd-smi combinations fail this query outright (MI308X on
+    ROCm 7.0 raises AMDSMI_STATUS_UNEXPECTED_DATA) or report "N/A". Both mean
+    unknown, which must stay distinct from a measured 0 -- reporting unknown as
+    idle would claim an idleness that was never observed.
+    """
+    try:
+        activity = amdsmi.amdsmi_get_gpu_activity(handle)
+    except (OSError, amdsmi.AmdSmiException):
+        return None, None
+    gfx = activity.get("gfx_activity")
+    umc = activity.get("umc_activity")
+    return (
+        gfx if isinstance(gfx, int) else None,
+        umc if isinstance(umc, int) else None,
+    )
+
+
 def sample(amdsmi, count: int, interval: float) -> tuple[list[dict], int]:
     gpus = []
     for smi_index, handle in enumerate(amdsmi.amdsmi_get_processor_handles()):
@@ -64,14 +103,12 @@ def sample(amdsmi, count: int, interval: float) -> tuple[list[dict], int]:
             time.sleep(interval)
         busy = 0
         for gpu in gpus:
-            activity = amdsmi.amdsmi_get_gpu_activity(gpu["handle"])
-            gfx = activity.get("gfx_activity")
-            umc = activity.get("umc_activity")
-            gfx = gfx if isinstance(gfx, int) else 0
-            umc = umc if isinstance(umc, int) else 0
-            gpu["gfx"].append(gfx)
-            gpu["umc"].append(umc)
-            busy += int(gfx > 5)
+            gfx, umc = read_activity(amdsmi, gpu["handle"])
+            if gfx is not None:
+                gpu["gfx"].append(gfx)
+                busy += int(gfx > 5)
+            if umc is not None:
+                gpu["umc"].append(umc)
         peak_concurrent = max(peak_concurrent, busy)
     for gpu in gpus:
         memory = amdsmi.amdsmi_get_gpu_vram_usage(gpu["handle"])
@@ -81,9 +118,11 @@ def sample(amdsmi, count: int, interval: float) -> tuple[list[dict], int]:
             {
                 "used_gib": used,
                 "free_gib": total - used,
-                "peak_gfx": max(gpu["gfx"]),
-                "mean_gfx": sum(gpu["gfx"]) / len(gpu["gfx"]),
-                "peak_umc": max(gpu["umc"]),
+                "peak_gfx": max(gpu["gfx"]) if gpu["gfx"] else None,
+                "mean_gfx": (
+                    sum(gpu["gfx"]) / len(gpu["gfx"]) if gpu["gfx"] else None
+                ),
+                "peak_umc": max(gpu["umc"]) if gpu["umc"] else None,
             }
         )
         del gpu["handle"]
@@ -111,14 +150,17 @@ def main() -> int:
         gpu
         for gpu in gpus
         if gpu["hip_index"] is not None
-        and gpu["peak_gfx"] <= args.max_busy
+        and (gpu["peak_gfx"] is None or gpu["peak_gfx"] <= args.max_busy)
         and gpu["used_gib"] <= args.max_used_gib
         and gpu["free_gib"] >= args.min_free_gib
     ]
+    # Prefer GPUs whose idleness was actually measured over ones where the
+    # activity query failed and only VRAM could be checked.
     eligible.sort(
         key=lambda gpu: (
-            gpu["peak_gfx"],
-            gpu["mean_gfx"],
+            gpu["peak_gfx"] is None,
+            gpu["peak_gfx"] or 0,
+            gpu["mean_gfx"] or 0.0,
             gpu["used_gib"],
             -gpu["free_gib"],
         )
@@ -137,19 +179,24 @@ def main() -> int:
         for gpu in sorted(gpus, key=lambda item: item["smi_index"]):
             if gpu["hip_index"] is None:
                 verdict = "SKIP no hip_id"
-            elif gpu["peak_gfx"] > args.max_busy:
+            elif gpu["peak_gfx"] is not None and gpu["peak_gfx"] > args.max_busy:
                 verdict = f"BUSY peaked {gpu['peak_gfx']}%"
             elif gpu["used_gib"] > args.max_used_gib:
                 verdict = f"HELD {gpu['used_gib']:.1f} GiB used"
             elif gpu["free_gib"] < args.min_free_gib:
                 verdict = f"FULL {gpu['free_gib']:.1f} GiB free"
+            elif gpu["peak_gfx"] is None:
+                verdict = "idle by VRAM only (activity unavailable)"
             else:
                 verdict = "idle"
             hip_index = "-" if gpu["hip_index"] is None else gpu["hip_index"]
+            peak_gfx = "n/a" if gpu["peak_gfx"] is None else str(gpu["peak_gfx"])
+            mean_gfx = "n/a" if gpu["mean_gfx"] is None else f"{gpu['mean_gfx']:.1f}"
+            peak_umc = "n/a" if gpu["peak_umc"] is None else str(gpu["peak_umc"])
             print(
                 f"{gpu['smi_index']:>4} {hip_index:>4} {gpu['bdf']:<14} "
-                f"{gpu['peak_gfx']:>6} {gpu['mean_gfx']:>6.1f} "
-                f"{gpu['peak_umc']:>5} {gpu['used_gib']:>6.1f} GiB "
+                f"{peak_gfx:>6} {mean_gfx:>6} "
+                f"{peak_umc:>5} {gpu['used_gib']:>6.1f} GiB "
                 f"{gpu['free_gib']:>6.1f} GiB  {verdict}",
                 file=sys.stderr,
             )
@@ -161,12 +208,23 @@ def main() -> int:
             )
 
     if not eligible:
-        print(
-            "No GPU stayed below the activity and resident-memory thresholds.",
-            file=sys.stderr,
-        )
+        if all(gpu["peak_gfx"] is None for gpu in gpus):
+            print(
+                "No GPU stayed below the resident-memory thresholds; GPU activity "
+                "is unavailable on this host, so only VRAM was considered.",
+                file=sys.stderr,
+            )
+        else:
+            print(
+                "No GPU stayed below the activity and resident-memory thresholds.",
+                file=sys.stderr,
+            )
         return 1
     selected = eligible[0]
+    basis = ACTIVITY_BASIS if selected["peak_gfx"] is not None else VRAM_ONLY_BASIS
+    # Machine-readable and deliberately outside the --quiet guard: callers record
+    # this so a report never presents a VRAM-only claim as a measured-idle one.
+    print(f"idleness-basis: {basis}", file=sys.stderr)
     if not args.quiet:
         print(
             f"Chose HIP index {selected['hip_index']} "

.claude/skills/validate-kernel-pr/report_schema.json

@@ -152,6 +152,10 @@
           "type": "string",
           "description": "Environment variable used to inject the independent grid, or an empty string."
         },
+        "shape_arg": {
+          "type": "string",
+          "description": "The target's own CLI flag used to inject the independent grid, or an empty string when the environment-variable channel or no channel was used."
+        },
         "grid": {
           "type": "string",
           "description": "Independent shape grid supplied by the caller, or an empty string."
@@ -206,6 +210,11 @@
             "model": { "type": "string" },
             "arch": { "type": "string" },
             "bdf": { "type": "string" },
+            "idleness_basis": {
+              "type": "string",
+              "enum": ["activity+vram", "vram-only", "unknown"],
+              "description": "Evidence the idleness claim rests on. 'vram-only' means the activity API was unavailable, so a null gfx_activity_before_pct is unknown rather than a measured zero."
+            },
             "gfx_activity_before_pct": { "type": ["integer", "null"] },
             "gfx_activity_after_pct": { "type": ["integer", "null"] },
             "host": { "type": "string" }
@@ -329,7 +338,7 @@
             "properties": {
               "expected_route": { "minLength": 1 },
               "shape_vars": { "minLength": 1 },
-              "runner": { "const": "pytest" }
+              "runner": { "enum": ["pytest", "script"] }
             }
           },
           "stages": {