validate-kernel-pr

name	validate-kernel-pr
description	Reproducible validation executor for kernel PRs. Applies an explicit base-to-head patch in an isolated worktree, runs it on a verified-idle GPU, compares the same targets against base, policy-checks the test diff, and emits a head-bound validation_report.json. Missing environment evidence is INCONCLUSIVE, never PASS.
argument-hint	--repo <worktree> --target <script file or pytest target>

validate-kernel-pr

review-pr reads the diff; it does not build and it does not run. It is a static reviewer, and a good one — but three failure modes are invisible to it, and this skill exists for exactly those three:

1. The PR's own tests pass while the kernel is wrong. A suite whose non-aligned shapes are commented out reports green on an out-of-bounds tail store.
2. A green suite that cannot fail. Loosening a comparison tolerance leaves every test passing and the kernel unguarded.
3. Defects that only exist at runtime. LDS over-allocation on one arch, an accuracy gate failing against the reference, a JIT path that no-ops on cache miss.

Output is validation_report.json: deterministic execution evidence kept separate from review-pr's advisory judgement. A review may consume it only when repo.head matches the exact PR head; a review written without one must mark validation NOT RUN.

The two skills stay split at judgement, not at invocation. review-pr triages whether a PR has runtime surface at all and, when it does and the PR ships a single target, runs this script itself rather than asking a human to. Everything below is still produced here and merely consumed there: the executor never writes an advisory verdict, and review-pr never manufactures evidence it did not get from a report.

---

Invocation

The caller supplies a clean base checkout, the base-to-head patch, the exact head OID, and the test target; this script does not fetch PRs itself (see Not implemented yet).

# Example for a FlyDSL softmax PR.
REPO=ROCm/FlyDSL
PR="${PR:?set PR to the open softmax PR number}"

# 1. pin the PR identity and put its base in an isolated worktree
BASE_REF=$(gh pr view "$PR" --repo "$REPO" --json baseRefName --jq .baseRefName)
BASE_REF_PATH=$(python3 -c \
  'import sys,urllib.parse; print(urllib.parse.quote(sys.argv[1], safe=""))' \
  "$BASE_REF")
BASE=$(gh api "repos/$REPO/branches/$BASE_REF_PATH" --jq .commit.sha)
HEAD=$(gh pr view "$PR" --repo "$REPO" --json headRefOid --jq .headRefOid)
git worktree add --detach "/tmp/pr-$PR" "$BASE"
gh pr diff "$PR" --repo "$REPO" > "/tmp/pr-$PR.patch"

# 2. validate base and head under the same runner and GPU lock
.claude/skills/validate-kernel-pr/validate_pr.sh \
    --repo "/tmp/pr-$PR" \
    --patch "/tmp/pr-$PR.patch" \
    --head-sha "$HEAD" \
    --target tests/kernels/test_softmax.py \
    --expected-route kernels.softmax_kernel:build_softmax_module \
    --shape-vars M,N,dtype_str \
    --shape-env ROCDSL_SOFTMAX_SHAPES \
    --grid "64,2048,f32;64,2000,f32" \
    --tol-table "f32=1e-5,f16=2e-3,bf16=1e-2" \
    --out validation_report.json

For a local candidate with no remote head, omit --head-sha. The report then records repo.head: null; it remains useful locally but review-pr will reject it as PR evidence.

flag	meaning
--repo	worktree to validate (required)
--target	script file or pytest node/file the PR ships (--tests remains an alias)
--patch	patch to apply first; conflict is a blocker
--head-sha	exact remote PR head represented by the patch
--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
--shape-argnames	the pytest parameter names the grid should replace, for targets whose shapes are literals inside @pytest.mark.parametrize
--axis	repeatable NAME=FLAG:v1;v2;… — an extra independent test axis on its own CLI flag (see axes)
--runner	force pytest or script when the structural classifier gets it wrong; the report records both the forced choice and what selection had said
--perf-control-column	a timing column the patch does not touch; required before a transplanted baseline is believed (see perf)
--tol-table	reference tolerances recorded alongside the head-vs-base comparison (see test_policy)
--perf-args	benchmark entry point for the timing stage; also forces perf on when detection would decline
--no-perf	skip the timing stage entirely
--label --out	run name and report path (default ./validation_report.json)

Several settings are environment variables rather than flags, because they describe the host rather than the PR under test, and a caller validating many PRs on one machine sets them once:

env	meaning
PYLIB	runtime modules living outside the checkout
PYTHON_BIN	the one interpreter used for both pytest and script targets
PICKER	override the shipped pick-idle-gpu.py; unset, the shipped picker is used, and only then one found on PATH
TIMEOUT	per-target budget, default 1800s
PERF_TIMEOUT	the timing stage's own budget, defaulting to TIMEOUT, because a bench sweep is legitimately longer than a correctness run
PERF_REPEAT	runs per side, default 3
PERF_THRESHOLD	head/base ratio that counts as a regression, default 0.95
PERF_MIN_ROWS	matched rows required before any ratio is reported, default 3

Everything that describes the PR is a flag. The executor also overrides AITER_JIT_DIR with separate fresh base/head directories and sets PYTHONDONTWRITEBYTECODE=1, so repository JIT output cannot cross phases or dirty the worktree.

Which shape channel to name

The three shape flags are alternatives, not a sequence; supply the one the target actually has. The report says which channel was established in test_selection.grid_channel, and when none was, test_selection.grid_channel_reason names each channel tried and what was found in the target — so a failed guess costs one run, not a reading of this file.

the target takes its shapes from	flag	what is checked before the channel is credited
an environment variable it reads	--shape-env	the source reads that name via os.getenv / os.environ
its own CLI flag	--shape-arg	the source passes that flag literal to add_argument
@pytest.mark.parametrize literals	--shape-argnames	the source binds all those names as test parameters

All three are then held to the same proof: a deliberately unusable grid must make the target fail. A target that ignores the grid produces a skip, never a pass.

---

Stages

Each stage writes its own status into the report. A stage that cannot run says skip with a reason — it never reports pass for work it did not do.

1 — merge_sim

Apply the PR head on top of the current base. A conflict is a blocker and short-circuits: no number produced downstream would describe the merged code. Known collision surfaces worth a second look because they are edited by many PRs at once: tuning CSVs (duplicate shape rows), csrc/include/rocm_ops.hpp, aiter/jit/optCompilerConfig.json.

The supplied worktree must be clean. The report records the base commit, patch SHA-256, and the 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 lock immediately after selection. Hold that file descriptor for the whole run:

PICK=$(python3 .claude/skills/validate-kernel-pr/pick-idle-gpu.py \
    --samples 10 --interval 1 --quiet)
flock /tmp/gpu-$PICK.lock <command>

The report records host, HIP index, matching AMD SMI index, BDF, market name, architecture, and 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, and the script performs no architecture-specific compile in this branch, so it does not call the result compile-only. That skip names which fact it rests on: no GPUs on this host (picker exit 3) and GPUs present but none idle (exit 1) are environment facts, while AMD SMI being unqueryable (exit 2) says nothing about the GPUs at all.

When no GPU was claimed, the target is then asked whether it needs one: it is run once with no visible device, and test_selection.gpu_requirement becomes not-required only if it passes and executes at least one test. The executed count is what makes this evidence — a suite guarded by skipif(not torch.cuda.is_available()) also exits 0 while proving nothing. This is deliberately an observation rather than a judgement about the diff: a Python-level dispatch change reroutes kernels without touching kernel source, and ROCm/aiter#5089 decides whether 34 gfx950 kernels compile from a seven-line helper, so no static rule over changed paths could settle it.

A not-required target runs its correctness stages instead of skipping them, which is the evidence a CPU-only fix is able to supply. It still claims nothing further: arch_coverage stays empty because only a passing gpu_claim credits an architecture, and PASS is unchanged — it continues to require gpu_claim: pass, so this path cannot produce a clearance that was previously unreachable.

3 — runtime_compat

Does the repository's own package import from the supplied checkout against the runtime that is actually installed? The probe is repository-aware: Aiter resolves aiter from the checkout; FlyDSL resolves the pinned package from PYLIB (when supplied) and compares its version with the checkout's python/flydsl. This keeps compiled _mlir bindings available without pretending an unrelated FlyDSL install validates an Aiter checkout. A pinned prebuilt runtime can drift behind the tree, and the resulting ImportError looks exactly like a defect in the PR. A mismatch is an environment fact: runtime_compat and correctness are skipped, the verdict is INCONCLUSIVE, and nothing is attributed to the author.

The report records the Python executable/version, resolved package path/version, and SHA-256 identities for native libraries loaded by the runtime probe. If a FlyDSL PR changes Python, C++/MLIR bindings, headers, CMake, or packaging inputs, a prebuilt PYLIB is not accepted: trusted build-system provenance is not implemented, and caller-authored metadata cannot prove which source produced a binary. Such runs return INCONCLUSIVE instead of testing a stale package.

This matters most for FlyDSL kernels: the Python kernels import symbols from a compiled runtime, so "one fresh container per PR" would mean rebuilding MLIR/LLVM per PR. The workable shape is a pinned prebuilt image plus this compatibility gate.

4 — test_policy — run before the suite

A suite that cannot fail is worse than no suite, because it produces a green report. Two checks:

• Tolerance, compared head-vs-base. Repos legitimately differ per kernel; the question is whether this change loosened what was there. A test-only widening is a deterministic blocker. If kernel code changed too, the widening is NEEDS_WORK pending numerical justification rather than a false deterministic block.
• Commented-out shape rows, compared head-vs-base. Existing rows are recorded as coverage context; only rows newly disabled by the change produce NEEDS_WORK. The independent grid remains visible either way.

5 — correctness — the repo's tests, then a grid the repo does not run

Runner selection is structural, not assumed:

• an explicit path::node, or a file defining test*/Test*, uses pytest;
• otherwise a file with an if __name__ == "__main__" guard runs as python <file>;
• a file with neither is skip, never a test failure.

Selection can still pick a runner the target cannot survive. A file that defines test* nodes and parses argv in its module body is collected by pytest, which imports it with pytest's own argv — and argparse exits the process, while the same file is green run as a script. test_selection.runner_risk names that structurally, and a run that executes nothing under the selected runner says so: "red on both sides" is an attribution, not an explanation, and a reader who is not told otherwise concludes the code is broken when the runner choice is.

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.

A script target publishes no per-case count, so its executed is a liveness signal and nothing more — it says the process ran. What it must not do is stand in for work. A target that returns silently with exit 0 and a log line (aiter#4538's does, when the arch is unsupported or an optional package is missing) produced exactly the same executed: 1 as the run that graded 56 cases, and earned the same arch_coverage: runtime on basis script-exit-zero-with-output — a statement about the process, not the kernel. So:

• stats.observed_work carries the only number backed by evidence: route calls counted in that run's own execution receipt. It is null when no route was named, because nothing was watched;
• arch_coverage credits nothing when observed_work is 0 — a route was watched for and never reached the device — and arch_coverage_basis prints the count and its provenance rather than implying a measurement.

For a patch run, the validator reverses the exact patch to create the baseline, verifies that the worktree is clean, runs both targets under base-only caches, and reapplies the patch before a head run with separate caches. This removes new files too; a PR-added failing test is therefore target-not-present on base, not falsely classified as a pre-existing failure. Any worktree artifact, reverse/reapply failure, or cache-isolation failure aborts the head run and produces INCONCLUSIVE.

The S1-owned grid must cover three classes the PR's own tests routinely miss:

class	why
non-toy	M=1 / M=16 only is the standard agent-generated test
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 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.

A proven channel is not the same as added coverage. A target that consumes the grid can still be handed cells it already runs by default. On ROCm/aiter#4538 all three requested shapes were in the target's own --shapes default list, so the "independent" grid re-ran a strict subset of the repository run and the stage reported pass — the exact duplication this stage exists to prevent, invisible in the report. The grid cells are therefore compared against the target's own declared default for the same flag, and test_selection.grid_independence is one of:

value	meaning
adds-coverage	at least one cell is outside the target's own default
duplicates-target-defaults	every cell is already a default; a passing run is downgraded to skip and the verdict to INCONCLUSIVE, because a passing duplicate proves only what correctness_repo_tests already said
unknown	the channel exposes no literal default to compare against (every env-var channel, and a flag whose default is computed)

A duplicate grid that fails keeps its fail. The finding is real; what a duplicate cannot do is earn a pass.

Axes: when the failing configuration is not a shape

--grid is one ordered tuple on one channel, which is all a target's shape flag accepts. A target whose remaining knobs are separate flags — head counts, dtypes, window modes — could not be gridded over them at all, so entire configurations were unreachable however the grid was spelled. That is not a missing shape; it is a missing axis.

aiter#4538 is the case: --shapes carries (seq_len, seq_len_kv) while --num-heads is its own flag defaulting to 64 128, and the public API asserts at num_heads=16 — a real blocker the validator had no way to request.

--axis 'num_heads=--num-heads:16;32'

Axes obey the same burden of proof as --shape-arg, in two steps, and test_selection.axis_state names which one it reached:

state	meaning
none / unusable	none requested, or the target is not a script (argv reaches script targets only)
hook-not-found	the target's source declares no add_argument for that flag
hook-not-consumed	the flag was declared but accepted __VALIDATOR_INVALID_AXIS__; the axis is dropped and named, never dropped quietly
proven	every axis flag refused an invalid value, and its values rode the grid run's argv

Each axis records its own independence against the flag's declared default, on the same terms as the shape cells. A proven axis that asks for values outside the default makes the run independent even when the shape cells duplicate.