Skip to content

feat(cuda-ep): claim CausalConvWithState under the standard ai.onnx opset-27 spelling - #1895

Merged
justinchuby merged 1 commit into
mainfrom
feat/cuda-causal-conv
Aug 23, 2026
Merged

justinchuby merged 1 commit into
mainfrom
feat/cuda-causal-conv

Conversation

@justinchuby

Copy link
Copy Markdown
Owner

What

Registers CausalConvWithState under the standard ai.onnx opset-27 spelling on the CUDA EP, completing the op across both EPs (#1857 did the CPU side).

No new kernel. The CUDA EP already implements the identical operation as a com.microsoft contrib kernel, so the standard spelling registers against the same factory — matching what the CPU EP does for this op and what both EPs already do for LinearAttention.

Checked, not assumed

  • Contract is identical — rank-3 [B, C, L], depthwise [C, 1, k] weight, k-1 carry state, none/silu/swish activation. The opset-27 schema defines only activation and has no ndim, so the contrib factory's ndim default of 1 cannot silently disagree.
  • Dtype advertisement is honest — it resolves to CUDA_FLOAT_DTYPES (f32/f16/bf16), which is exactly the set the kernel dispatches. That is the trap that bit the TensorScatter work in feat(ep): TensorScatter (opset 24) and standard-domain CausalConvWithState (opset 27) #1798: a missing arm fail-closes to f32-only and the node is silently handed to ORT.
  • No optional-input bug here. The CUDA kernel already detects optional inputs with is_absent() rather than by arity, so it does not have the defect feat(cpu-ep): claim CausalConvWithState under the standard ai.onnx opset-27 spelling #1857 had to fix on the CPU side, where an omitted bias before a present past_state was read as the bias. I checked specifically because that one was found by review, not by tests.

Test

the_standard_domain_spelling_reaches_the_same_kernel — added to the existing GPU parity suite, which already covers fp32/fp16/bf16 × decode/prefill × bias × state × activation against the CPU EP oracle.

It checks the claim two ways: the standard-domain CUDA result matches the standard-domain CPU oracle, and it is bit-identical to the contrib-domain CUDA result. If the registration were missing, the node would find no kernel and never reach this EP at all.

Also: a pre-existing compile break

CI's CUDA compile job was already failing on main, and this fixes it.

content_preserving_transition_gpu.rs imports fault-injection helpers that the library gates on #[cfg(any(test, feature = "gpu-tests"))]. An integration test is a separate compilation unit, so the library's cfg(test) does not apply to it — only the feature does. Without --features gpu-tests, the whole test target failed with an unresolved import, which is not a useful signal about a suite that cannot run on a machine with no GPU anyway. The file now declares the feature it needs.

Results

cargo build -p onnx-runtime-ep-cuda --tests → clean (was: unresolved import).
cargo test -p onnx-runtime-ep-cuda --lib → 533 passed, 0 failed.

One flake, reported separately

Across four full runs of the suite I saw provider::tests::async_pagein_fence_orders_weight_page_in_consumer fail 2 of 4 times, and pass every time in isolation (--test-threads=1). It is a GPU-timing flake in the async page-in fencing, unrelated to this change — I only add a registration line and a test.

I have not attempted a fix: making the suite green by adjusting a fence could just as easily mask a real ordering bug in weight paging. Filed separately with the reproduction rate.

…pset-27 spelling

Completes the op across both EPs. The CUDA EP already implements the identical
operation as a `com.microsoft` contrib kernel, so the standard opset-27 spelling
registers against the SAME factory rather than adding a second implementation to
keep in step -- matching what the CPU EP does for this op (#1857) and what both
EPs already do for `LinearAttention`.

Checked rather than assumed:

* The contract is identical -- rank-3 `[B, C, L]`, depthwise `[C, 1, k]` weight,
  `k-1` carry state, `none`/`silu`/`swish` activation -- and the opset-27 schema
  defines only `activation`, so the contrib factory's `ndim` default of 1 cannot
  silently disagree.
* The dtype advertisement resolves to CUDA_FLOAT_DTYPES (f32/f16/bf16), which is
  exactly what the kernel dispatches, so the claim is honest in both directions.
* The kernel already detects optional inputs with `is_absent()` rather than by
  arity, so it does not have the bug #1857 had to fix on the CPU side, where an
  omitted bias before a present past_state was read as the bias.

Also fixes a pre-existing compile break that CI's "CUDA compile" job was
reporting: `content_preserving_transition_gpu.rs` imports fault-injection
helpers the library gates on `#[cfg(any(test, feature = "gpu-tests"))]`. An
integration test is a separate compilation unit, so the library's `cfg(test)`
does not apply to it -- only the feature does, and without it the whole test
target failed to compile with an unresolved import. The file now declares the
feature it needs.
@justinchuby
justinchuby merged commit 1be9f2c into main Aug 23, 2026
4 checks passed
@justinchuby
justinchuby deleted the feat/cuda-causal-conv branch August 23, 2026 21:35
@codecov

codecov Bot commented Aug 23, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.33%. Comparing base (9aa4f94) to head (7eaf324).
⚠️ Report is 10 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##             main    #1895       +/-   ##
===========================================
+ Coverage   72.56%   80.33%    +7.77%     
===========================================
  Files          12      415      +403     
  Lines        5231   204440   +199209     
  Branches     5231   204440   +199209     
===========================================
+ Hits         3796   164246   +160450     
- Misses       1307    34620    +33313     
- Partials      128     5574     +5446     
Flag Coverage Δ
cli-ort-linux 72.51% <ø> (ø)
cli-ort-windows 72.01% <ø> (ø)
mlas 85.20% <ø> (?)
offline 80.47% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.
see 403 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

justinchuby added a commit that referenced this pull request Aug 23, 2026
…f which was reported

The `CUDA compile (Linux x86_64)` lane has been red since #1836 (`6e4b0ebb3`);
last green was `cb81745b0`. The reported compile error was masking two further
failures, so fixing it alone would have moved the lane from "red at build" to
"red at the honesty script". Two more arrived on `main` while this PR was open,
in #1884 and #1895. All five are fixed here; the lane cannot go green on any
proper subset.

1. Unresolved import (the reported error).

`content_preserving_transition_gpu.rs` imported
`transition_granule_range_with_phase8_faults` unconditionally, but that item is
gated `#[cfg(any(test, feature = "gpu-tests"))]`. The `test` arm does not cover
an integration test: `tests/*.rs` are separate crates linking the library built
*without* `cfg(test)`, so the item is genuinely absent in the
`without-gpu-tests` configuration. Invisible to any local run that passes
`--features gpu-tests`.

Fixed with a two-arm local shim, mirroring `with_faults` in
`crates/onnx-runtime-cuda-memory/tests/virtual_memory_gpu.rs`, which already
solves this exact problem for `CudaVirtualBacking::with_driver_faults`. The
gate itself is left alone: widening it would ship a driver fault injector that
can force `cuMemUnmap`/`cuMemMap`/`cuMemSetAccess` to fail into production, and
`required-features` is precisely what the honesty script exists to forbid
("exists only with gpu-tests enabled; CUDA tests must not hide from CPU
inventory").

#1895 (`1be9f2cc2`) reached this file first, with `#![cfg(feature =
"gpu-tests")]` on the whole target -- the option rejected above. It fixed the
compile and traded it for an inventory failure on the same lane: `main` at
`1be9f2cc2` reports "content_preserving_transition_gpu: Cargo reported no
integration tests" plus 19 x "test exists only with gpu-tests enabled" (job
`97266415620`). The target was not repaired, it was hidden. That line is
removed here and the module doc records why, so the option is not re-tried a
third time.

2. Five non-ignored tests in a `_gpu` target (4 honesty violations).

`verify_cuda_test_honesty.py` requires every test in a `_gpu` target to be
ignored, not passed, on a CPU-only runner -- that is how the suite is stopped
from reporting green for a GPU it never touched.

Three were genuinely CPU-only predicate tests over `verify_safe_point`. Moved
to a new non-`_gpu` target rather than `#[ignore]`d: silencing them would have
greened the lane by deleting coverage, and target naming is the escape hatch
the script's own comment names for CPU-only tests in a CUDA crate.

Moving them alone would *also* have stopped them running. A non-`_gpu` target
is skipped by the honesty script, and `workspace_test_packages.py` deny-lists
this crate from every offline lane, so the target would have been compiled and
never executed. The CUDA lane therefore gains an explicit `--test
content_preserving_transition` step, alongside the two CPU-only targets in this
crate that already have one for the same reason.

The other two asserted nothing -- `fault_injection_safe_point_recheck_rejects`
is comments plus a `println!` and self-describes as "a documentation test";
`zero_len_is_committed_noop` `println!`s that the behaviour is "verified by
implementation". Both passed unconditionally. Removed. The first has real
sibling coverage in `fault_injection_recheck_safe_point_rejected_gpu`; the
second does not -- the `len == 0` early return has no test anywhere. Deleting a
test that asserts nothing loses no coverage, but the gap is pre-existing and
real, and a genuine test needs a device (the early return still takes
`&CudaRuntime`/`&mut CudaReservation`), so it is left for a GPU-capable change
rather than papered over.

Added `safe_point_accepts_a_clean_state`, because the three moved tests only
ever assert `is_err()` and so all three survive a mutant that makes
`verify_safe_point` reject unconditionally. Verified: under that mutant the
inherited three pass and only the new test fails.

3. The manifest guard checked a proxy, not the property it claimed.

The check was the literal substring `"gpu-tests = []"`, so #1860 turned it red
by changing the value to `["onnx-runtime-cuda-memory/gpu-tests"]` -- a
legitimate and necessary forwarding. Now matches the feature *key* whatever its
value, extracted into `declares_gpu_tests_feature` so it is covered by the
script's own `--self-test` fixtures, which it previously was not.

4. A GPU test in a `_gpu` target with no `#[ignore]` (#1895).

`causal_conv_with_state_gpu::the_standard_domain_spelling_reaches_the_same_kernel`
calls `require_cuda()` exactly like its two siblings but is missing their
`#[cfg_attr(not(feature = "gpu-tests"), ignore = ...)]`, so it ran and failed on
a CPU runner. Fixed by adding the sibling attribute verbatim; no design choice
involved.

5. A CPU-only test inside a `_gpu` target (#1884).

`expert_route_telemetry_probe_gpu::cpu_oracle_and_validator_self_consistent`
passes in both configurations, which the script reports as "executed without
gpu-tests". Its doc comment states the intent plainly: it "runs without a GPU so
the reference cannot silently rot".

`#[ignore]` would clear the checker by destroying exactly that property -- an
ignored test runs nowhere -- so this takes the same route as defect 2. The
pure-CPU oracle (`cpu_bitmap`, `cpu_dedup`, `consume_and_validate`,
`synth_routes` and the header indices) moves verbatim to a shared
`tests/expert_route_oracle/mod.rs`, which is a module and not a target: Cargo
auto-discovers `tests/*.rs` and `tests/*/main.rs` only. Both the `_gpu` target
and a new `expert_route_telemetry_probe` target declare it, so there is one
copy of the oracle and the seven GPU tests keep diffing against the same code
the CPU test checks. The new target gets its own `ci.yml` step, for the reason
in defect 2.

Verified live in its new home by mutation rather than by its own green: perturb
the shift in `cpu_bitmap` -> FAILED, revert -> ok. "It compiles in the new file"
is not evidence that it executes there, which is the defect an earlier review
caught in defect 2's target.

Each fix independently falsified by re-introducing it: import -> E0432;
un-ignored test -> "must be ignored, not pass" (+3 inventory errors); manifest
value -> "must define a gpu-tests feature"; missing `cfg_attr` -> "1 tests
failed while checking ignored status"; relocated CPU test -> "executed without
gpu-tests; CUDA tests must be ignored, not pass".

Honesty script exits 0: 530 tests/79 targets identical in both configurations,
530 ignored without gpu-tests, 0 passed on this no-CUDA host.
`cargo clippy -p onnx-runtime-ep-cuda --features cuda -- -D warnings` clean (the
lane's exact command, both invocations of it); both explicit `ci.yml` test steps
pass 4/4 and 1/1; fmt clean.

Not fixed here: `cargo clippy -p onnx-runtime-ep-cuda --features cuda
--all-targets -- -D warnings` fails on ~8 pre-existing lints in `_gpu` targets
this PR does not touch. No lane runs that command -- both ep-cuda clippy steps
omit `--all-targets` and `workspace_test_packages.py` deny-lists the crate --
so it is a real gap but a separate one, and folding it in would put unrelated
files in a lane-restoration PR.

Closes #1875

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
justinchuby added a commit that referenced this pull request Aug 23, 2026
…f which was reported

The `CUDA compile (Linux x86_64)` lane has been red since #1836 (`6e4b0ebb3`);
last green was `cb81745b0`. The reported compile error was masking two further
failures, so fixing it alone would have moved the lane from "red at build" to
"red at the honesty script". Two more arrived on `main` while this PR was open,
in #1884 and #1895. All five are fixed here; the lane cannot go green on any
proper subset.

1. Unresolved import (the reported error).

`content_preserving_transition_gpu.rs` imported
`transition_granule_range_with_phase8_faults` unconditionally, but that item is
gated `#[cfg(any(test, feature = "gpu-tests"))]`. The `test` arm does not cover
an integration test: `tests/*.rs` are separate crates linking the library built
*without* `cfg(test)`, so the item is genuinely absent in the
`without-gpu-tests` configuration. Invisible to any local run that passes
`--features gpu-tests`.

Fixed with a two-arm local shim, mirroring `with_faults` in
`crates/onnx-runtime-cuda-memory/tests/virtual_memory_gpu.rs`, which already
solves this exact problem for `CudaVirtualBacking::with_driver_faults`. The
gate itself is left alone: widening it would ship a driver fault injector that
can force `cuMemUnmap`/`cuMemMap`/`cuMemSetAccess` to fail into production, and
`required-features` is precisely what the honesty script exists to forbid
("exists only with gpu-tests enabled; CUDA tests must not hide from CPU
inventory").

#1895 (`1be9f2cc2`) reached this file first, with `#![cfg(feature =
"gpu-tests")]` on the whole target -- the option rejected above. It fixed the
compile and traded it for an inventory failure on the same lane: `main` at
`1be9f2cc2` reports "content_preserving_transition_gpu: Cargo reported no
integration tests" plus 19 x "test exists only with gpu-tests enabled" (job
`97266415620`). The target was not repaired, it was hidden. That line is
removed here and the module doc records why, so the option is not re-tried a
third time.

2. Five non-ignored tests in a `_gpu` target (4 honesty violations).

`verify_cuda_test_honesty.py` requires every test in a `_gpu` target to be
ignored, not passed, on a CPU-only runner -- that is how the suite is stopped
from reporting green for a GPU it never touched.

Three were genuinely CPU-only predicate tests over `verify_safe_point`. Moved
to a new non-`_gpu` target rather than `#[ignore]`d: silencing them would have
greened the lane by deleting coverage, and target naming is the escape hatch
the script's own comment names for CPU-only tests in a CUDA crate.

Moving them alone would *also* have stopped them running. A non-`_gpu` target
is skipped by the honesty script, and `workspace_test_packages.py` deny-lists
this crate from every offline lane, so the target would have been compiled and
never executed. The CUDA lane therefore gains an explicit `--test
content_preserving_transition` step, alongside the two CPU-only targets in this
crate that already have one for the same reason.

The other two asserted nothing -- `fault_injection_safe_point_recheck_rejects`
is comments plus a `println!` and self-describes as "a documentation test";
`zero_len_is_committed_noop` `println!`s that the behaviour is "verified by
implementation". Both passed unconditionally. Removed. The first has real
sibling coverage in `fault_injection_recheck_safe_point_rejected_gpu`; the
second does not -- the `len == 0` early return has no test anywhere. Deleting a
test that asserts nothing loses no coverage, but the gap is pre-existing and
real, and a genuine test needs a device (the early return still takes
`&CudaRuntime`/`&mut CudaReservation`), so it is left for a GPU-capable change
rather than papered over.

Added `safe_point_accepts_a_clean_state`, because the three moved tests only
ever assert `is_err()` and so all three survive a mutant that makes
`verify_safe_point` reject unconditionally. Verified: under that mutant the
inherited three pass and only the new test fails.

3. The manifest guard checked a proxy, not the property it claimed.

The check was the literal substring `"gpu-tests = []"`, so #1860 turned it red
by changing the value to `["onnx-runtime-cuda-memory/gpu-tests"]` -- a
legitimate and necessary forwarding. Now matches the feature *key* whatever its
value, extracted into `declares_gpu_tests_feature` so it is covered by the
script's own `--self-test` fixtures, which it previously was not.

The key is looked for only inside the `[features]` table. Review falsified the
first version of this fix: a file-wide regex also accepts a *dependency* named
`gpu-tests`, or one under `[target.'cfg(...)'.dependencies]` -- the same proxy
mistake in a new costume, inside the fix for that mistake. Twelve fixtures now,
including both false-positive shapes.

4. A GPU test in a `_gpu` target with no `#[ignore]` (#1895).

`causal_conv_with_state_gpu::the_standard_domain_spelling_reaches_the_same_kernel`
calls `require_cuda()` exactly like its two siblings but is missing their
`#[cfg_attr(not(feature = "gpu-tests"), ignore = ...)]`, so it ran and failed on
a CPU runner. Fixed by adding the sibling attribute verbatim; no design choice
involved.

5. A CPU-only test inside a `_gpu` target (#1884).

`expert_route_telemetry_probe_gpu::cpu_oracle_and_validator_self_consistent`
passes in both configurations, which the script reports as "executed without
gpu-tests". Its doc comment states the intent plainly: it "runs without a GPU so
the reference cannot silently rot".

`#[ignore]` would clear the checker by destroying exactly that property -- an
ignored test runs nowhere -- so this takes the same route as defect 2. The
pure-CPU oracle (`cpu_bitmap`, `cpu_dedup`, `consume_and_validate`,
`synth_routes` and the header indices) moves verbatim to a shared
`tests/expert_route_oracle/mod.rs`, which is a module and not a target: Cargo
auto-discovers `tests/*.rs` and `tests/*/main.rs` only. Both the `_gpu` target
and a new `expert_route_telemetry_probe` target declare it, so there is one
copy of the oracle and the seven GPU tests keep diffing against the same code
the CPU test checks. The new target gets its own `ci.yml` step, for the reason
in defect 2.

Verified live in its new home by mutation rather than by its own green: perturb
the shift in `cpu_bitmap` -> FAILED, revert -> ok. "It compiles in the new file"
is not evidence that it executes there, which is the defect an earlier review
caught in defect 2's target.

Each fix independently falsified by re-introducing it: import -> E0432;
un-ignored test -> "must be ignored, not pass" (+3 inventory errors); manifest
value -> "must define a gpu-tests feature"; missing `cfg_attr` -> "1 tests
failed while checking ignored status"; relocated CPU test -> "executed without
gpu-tests; CUDA tests must be ignored, not pass".

Honesty script exits 0: 530 tests/79 targets identical in both configurations,
530 ignored without gpu-tests, 0 passed on this no-CUDA host.
`cargo clippy -p onnx-runtime-ep-cuda --features cuda -- -D warnings` clean (the
lane's exact command, both invocations of it); both explicit `ci.yml` test steps
pass 4/4 and 1/1; fmt clean.

Not fixed here: `cargo clippy -p onnx-runtime-ep-cuda --features cuda
--all-targets -- -D warnings` reports 23 pre-existing errors and dies on
`index_share_gpu` and `qmoe_zero_copy_cold_expert_spike_gpu` before reaching the
rest, so 23 is a floor, not a total. None are in a file this PR touches -- no
diagnostic location matches any of them. No lane runs that command: both
ep-cuda clippy steps omit `--all-targets` and `workspace_test_packages.py`
deny-lists the crate. A real gap, but a separate one; folding it in would put
unrelated files in a lane-restoration PR.

Closes #1875

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
justinchuby added a commit that referenced this pull request Aug 24, 2026
…f which was reported (#1881)

Closes #1875.

`CUDA compile (Linux x86_64)` has been red since **#1836
(`6e4b0ebb3`)**; last green was `cb81745b0`. Resch handed this off
explicitly rather than guessing at another domain's intended API
visibility — that was the right call, because **the reported compile
error was masking two further failures on the same lane.** Fixing only
the error would have moved it from "red at build" to "red at the honesty
script".

**Two more arrived on `main` while this PR was open** — #1884 and #1895,
both today. Five independent defects from five different PRs, reviewable
separately. The lane cannot go green on any proper subset of them.

---

### 1. Unresolved import — the reported error

`content_preserving_transition_gpu.rs` imports
`transition_granule_range_with_phase8_faults`, gated `#[cfg(any(test,
feature = "gpu-tests"))]`. The `test` arm does not cover an integration
test: `tests/*.rs` are separate crates linking the library built
*without* `cfg(test)`, so in the `without-gpu-tests` configuration the
item is genuinely absent. Invisible to any local run passing `--features
gpu-tests`.

**I rejected both options in the issue, with evidence:**

- *Widen the gate* contradicts the function's own doc — "Not reachable
from production" — and would ship an API that forces `cuMemUnmap` /
`cuMemMap` / `cuMemSetAccess` to fail to every consumer.
- *`required-features`* is exactly what the honesty lane exists to
forbid: `compare_inventories` emits *"exists only with gpu-tests
enabled; CUDA tests must not hide from CPU inventory"*. It trades a
compile error for a lane failure and deletes the CPU-side inventory of
14 tests.

**Fix:** a two-arm local shim, mirroring `with_faults` in
`crates/onnx-runtime-cuda-memory/tests/virtual_memory_gpu.rs`, which
already solves this identical problem for
`CudaVirtualBacking::with_driver_faults`. The gate stays intact; the
target compiles and lists its tests in both configurations. The
`unreachable!` arm is genuinely unreachable — after change 2 every test
in that file is `#[ignore]`d.

> **#1895 reached this file first, and took the option rejected above.**
> `1be9f2cc2` applied `#![cfg(feature = "gpu-tests")]` to the whole
target. It fixed the compile and **traded it for an inventory failure on
the same lane.** From the job log on `main@1be9f2c` (job
`97266415620`), not inferred:
> ```
> - content_preserving_transition_gpu: Cargo reported no integration
tests
> + 19 × "test exists only with gpu-tests enabled"
> ```
> The target was not repaired, it was **hidden** — the symptom stopped
being reported without the property becoming true. Its module doc argued
that a compile failure "is not a useful signal about a suite that cannot
run on a machine with no GPU anyway", which is the exact proposition
`verify_cuda_test_honesty.py` exists to reject. This PR removes that
line and records the reasoning in the module doc so the option isn't
tried a third time.

### 2. Five non-ignored tests in a `_gpu` target — 4 honesty violations

The script requires every test in a `_gpu` target to be **ignored, not
passed** on a CPU-only runner. That rule is how the suite is stopped
from reporting green for a GPU it never touched.

**Three were genuinely CPU-only** predicate tests over
`verify_safe_point` — pure logic over `ResizeSafePoint`, no driver.
**Moved** to a new non-`_gpu` target, not `#[ignore]`d: silencing them
would green the lane *by deleting coverage*, and target naming is the
escape hatch the script's own comment names, so that "a genuinely
CPU-only target is not policed as a device test merely because it lives
in a CUDA crate".

> **Correction after review — moving them was not sufficient, and my
first commit shipped the same defect this PR is about.**
> A non-`_gpu` target is skipped by the honesty script, and
`.github/scripts/workspace_test_packages.py:26` deny-lists this crate
from every offline lane. So the new target was **compiled and never
executed** — the moved tests, and the mutation guard I added
specifically to catch a bad `verify_safe_point`, would have run on
**zero** CI runs. My claim that moving "keeps them executing on every
run" was exactly backwards: before the move the honesty script *did*
execute them (that was the violation); after it, nothing did.
> This is the same failure mode as the defect being fixed — *a test that
isn't on the leg you're citing* — and it would have produced a green
check proving nothing. Fixed by adding an explicit `--test
content_preserving_transition` step to the CUDA lane, mirroring the two
CPU-only targets in this crate that already have one for precisely this
reason (`ci.yml:989-998`). Verified by running the added command
verbatim: 4 passed.

**Two asserted nothing** and are removed:
- `fault_injection_safe_point_recheck_rejects` — comments plus a
`println!`; self-describes as "this test documents the contract".
- `zero_len_is_committed_noop` — `println!`s that the behaviour is
"verified by implementation".

Both passed unconditionally. This touches another author's tests, so I
flag it explicitly rather than burying it in the diff. Deleting a test
that asserts nothing loses no coverage — but to be precise about what is
and isn't covered: the first *does* have real sibling coverage in
`fault_injection_recheck_safe_point_rejected_gpu`, and **the second does
not** — the `len == 0` early return is untested anywhere. That gap is
pre-existing, and a genuine test needs a device (the early return still
takes `&CudaRuntime`/`&mut CudaReservation`), so I have left it for a
GPU-capable change rather than claim coverage that doesn't exist.

**Added `safe_point_accepts_a_clean_state`.** The three moved tests only
ever assert `is_err()`, so they cannot distinguish "rejects the unsafe
field" from "rejects everything". Empirically confirmed — with
`verify_safe_point` mutated to reject unconditionally:

```
test result: FAILED. 3 passed; 1 failed
failures:
    safe_point_accepts_a_clean_state
```

All three inherited tests survive the mutant. Only the new one kills it.

### 3. The manifest guard checked a proxy, not the property it claimed

```python
if "gpu-tests = []" not in manifest:
    errors.append(f"crates/{crate.name}/Cargo.toml must define a gpu-tests feature")
```

A literal substring match. **#1860 turned this red** by changing the
value to `["onnx-runtime-cuda-memory/gpu-tests"]` — a legitimate and
necessary forwarding. The guard's message claims to check that the
feature *is defined*; it actually checked that it was defined *with one
particular empty value*.

Now matches the feature **key** whatever its value, extracted into
`declares_gpu_tests_feature()` so it is covered by the script's own
`--self-test` fixtures — which it previously was not. Fixtures include
the forwarding form, a multi-line list, a commented-out line, and a
`features = ["gpu-tests"]` dependency mention that must *not* count.

> Note for @resch: #1860 broke this lane as well as the shape-inference
pin you fixed in #1870 — same commit, two unrelated pins that both
encoded a value rather than the property.

### 4. A GPU test in a `_gpu` target with no `#[ignore]` — #1895


`causal_conv_with_state_gpu::the_standard_domain_spelling_reaches_the_same_kernel`
calls `require_cuda()` exactly like its two siblings in the same file,
but is missing their `#[cfg_attr(not(feature = "gpu-tests"), ignore =
…)]`. So it ran on a CPU runner and failed:

```
- causal_conv_with_state_gpu: 1 tests failed while checking ignored status
- causal_conv_with_state_gpu: Cargo inventory has 3 tests but libtest reported 2 ignored
```

**Fix:** add the sibling attribute verbatim. No design choice involved —
a four-line omission.

### 5. A CPU-only test inside a `_gpu` target — #1884


`expert_route_telemetry_probe_gpu::cpu_oracle_and_validator_self_consistent`
passes in **both** configurations, which the script reports as `executed
without gpu-tests`. Its doc comment states the intent plainly:

> *CPU-only: the oracle and the boundary validator are self-consistent.
Runs without a GPU so the reference cannot silently rot.*

That intent is correct and worth keeping. **`#[ignore]` would clear the
checker by destroying exactly the property the test was written for** —
an ignored test runs nowhere, so the reference could then rot precisely
as its author feared, with a green check over it. Same reasoning as
defect 2, so the same route.

**Fix:** the pure-CPU oracle — `cpu_bitmap`, `cpu_dedup`,
`consume_and_validate`, `synth_routes`, the `Decision` enum and the
header indices — moves **verbatim** to a shared
`tests/expert_route_oracle/mod.rs`, and the test moves to a new
`expert_route_telemetry_probe` target that declares it.
`tests/expert_route_oracle/mod.rs` is a module, not a target: Cargo
auto-discovers `tests/*.rs` and `tests/*/main.rs` only. Both targets
declare it, so there is **one** copy of the oracle and the seven GPU
tests keep diffing against the same code the CPU test checks — the
alternative, duplicating it, would let the two copies drift and quietly
defeat the point.

The new target gets its own `ci.yml` step, for the reason in defect 2's
correction.

**Verified live in its new home by mutation, not by its own green.**
Perturbing the shift in `cpu_bitmap`:

```
test result: FAILED. 0 passed; 1 failed
failures:
    cpu_oracle_and_validator_self_consistent
```

and `ok` again on revert. "It compiles in the new file" is not evidence
that it *executes* there — which is the defect the review caught in
defect 2, so this time it is checked rather than asserted.

> Defects 4 and 5 are in other people's very recent files. No open PR
touches either (checked across all open PRs), so there is no
concurrent-edit hazard, and neither fix requires domain knowledge of the
CUDA kernels or the telemetry design — only of where a test must live to
be run honestly.

---

## Validation

Every fix independently falsified by re-introducing it — each re-reds
with a **distinct** error:

| reverted | resulting failure |
|---|---|
| the shim | `error[E0432]: unresolved import ...with_phase8_faults` |
| one un-ignored test | `1 tests executed without gpu-tests; CUDA tests
must be ignored, not pass` + 3 inventory errors |
| the manifest check | `must define a gpu-tests feature` |
| the `cfg_attr` (4) | `1 tests failed while checking ignored status` |
| the relocation (5) | `1 tests executed without gpu-tests; CUDA tests
must be ignored, not pass` |

```
CUDA test honesty check passed: 530 tests/79 targets without gpu-tests (530 ignored),
530 tests/79 targets with gpu-tests (474 fail-loud, 56 ignored, 0 passed on this no-CUDA host)
```

- `cargo clippy -p onnx-runtime-ep-cuda --features cuda -- -D warnings`
— clean (the lane's exact command, and both of the two places `ci.yml`
invokes it)
- `cargo test -p onnx-runtime-ep-cuda --features cuda --lib` — **538
passed, 0 failed**
- both explicit CI test steps, run verbatim —
`content_preserving_transition` 4 passed, `expert_route_telemetry_probe`
1 passed
- `cargo fmt --all --check` — clean
- Rebased onto `496597764`; honesty script re-run green on the exact
tree being shipped (not on an earlier one — main moved four times during
this work).
- Independent Opus review — found the never-executed-target defect
above; all other categories (shim signature/arg-order vs the real
function across all 11 params and 7 call sites, error-string assertion,
regex, target classification, and every honesty-script rule) checked and
ruled out.

**One observation I am not fixing here.** The lane's clippy step omits
`--all-targets`, so this crate's test code is linted by **no** lane —
`workspace_test_packages.py` also deny-lists it. Running it now yields
23 errors and dies on `index_share_gpu` and
`qmoe_zero_copy_cold_expert_spike_gpu` before reaching the rest, all
pre-existing and none in a file this PR touches (verified: no diagnostic
location matches any of them). Unrelated and out of scope — folding it
in would put a pile of unrelated files into a lane-restoration PR — but
it is the same absent-coverage shape that let defects 2, 4 and 5 sit
un-noticed.

### Credit

@Gaff established that this is the **only** instance of the class in the
workspace — 24 `cfg(any(test, feature = …))` sites, the other 23 either
unreferenced from `tests/` or referenced only from gated positions — and
stated the root cause more crisply than I had: an item so gated is
broken **only when a `tests/` file names it from a position not itself
gated**, in practice a top-level `use`, because a `use` resolves
unconditionally regardless of how the functions below it are gated. A
gated call site is safe. He also retracted his own grep-based falsifier
for the class on #1817 after running the negative control on it.

One correction to his note, since it changes what the precedent
demonstrates: `onnx-runtime-cuda-memory` does **not** gate the
consumers. `vmm_release_quarantine_gpu.rs:59-67` is a **two-arm shim** —
a `#[cfg(feature = "gpu-tests")]` helper *and* a `#[cfg(not(feature =
"gpu-tests"))]` `unreachable!` twin — with its callers ungated. That
distinction is load-bearing here: genuinely gating the consumers would
delete the tests from the base inventory and red this lane under
`compare_inventories`. So the crate is still the right worked example;
it is an example of the shim, which is what this PR adopts (making it
the third in the repo, after `virtual_memory_gpu.rs::with_faults` and
`vmm_release_quarantine_gpu.rs::install_faults`).

🤖 Generated with [Copilot
CLI](https://githubnext.com/projects/copilot-cli)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
justinchuby added a commit that referenced this pull request Aug 24, 2026
… test it means (#1911)

`CUDA compile (Linux x86_64)` is **red on `main` again**, one commit
after #1881 restored it.

```
CUDA test honesty check failed:
  - activations_gpu: 1 tests failed while checking ignored status
  - activations_gpu: Cargo inventory has 4 tests but libtest reported 3 ignored
```

#1905 added
`activations_gpu::mish_matches_cpu_including_the_saturating_tail`
without the `#[cfg_attr(not(feature = "gpu-tests"), ignore = …)]` that
its **three siblings in the same file** carry, so it runs and fails on a
CPU runner. Fix is the sibling attribute, verbatim.

No criticism of @-the-author intended, and I want to be explicit about
why. **This is the fourth instance of the same omission in three days**
— #1884, #1895, and now #1905. That is a property of the *signal*, not
of the authors: while the lane is red for an unrelated reason, adding a
`_gpu` test without an ignore is free, because the only check that would
object is a job nobody can distinguish from already-broken. #1881
restored the lane; this keeps it restored.

---

## The second half: the checker didn't say which test

The message above names a **count**, in a four-test target, and the run
that produced it was a CI job on a merge commit. Finding out which test
needed a rebase and a grep.

A check whose entire job is to notice that a `_gpu` test was added
without an ignore should **say which one**. `run_libtest` now parses
libtest's per-test outcome lines, so the same failure reads:

```
activations_gpu: 1 tests failed while checking ignored status
  (mish_matches_cpu_including_the_saturating_tail)
```

**Verified on the real path, not only in fixtures.** With the `cfg_attr`
reverted, the full script emits exactly that line; restored, it exits 0.
Fixtures alone would only have proved the formatter works.

Naming also applies to `executed without gpu-tests` and `passed with
gpu-tests on a no-CUDA host`, which have the same problem — those are
the two messages that fired for #1884 and #1895. `IgnoredResult` and
`ActiveResult` now share a `LibtestResult` base for it.

It **degrades to the bare count** if the per-test lines can't be parsed,
rather than rendering an empty `()`. The count is still true, so a
future change in libtest's output format must not turn a real failure
into a confusing one.

Four new `--self-test` fixtures cover the naming, the inventory-mismatch
message, the empty-name degradation, and the outcome regex itself —
because a guard whose own correctness is unverified is precisely the
defect this checker exists to catch, and I'd rather not add one to it
while fixing it.

## Validation

```
CUDA test honesty check passed: 531 tests/79 targets without gpu-tests (531 ignored),
531 tests/79 targets with gpu-tests (475 fail-loud, 56 ignored, 0 passed on this no-CUDA host)
```

- run on this branch, based on current `main` (`7e274a4e2`) — not on an
older tree
- `--self-test` passes
- `cargo fmt --all --check` clean
- mutation-effective both ways: revert the `cfg_attr` → the named
failure above; revert the naming → the bare count

Follow-up to #1881. Refs #1875.

🤖 Generated with [Copilot
CLI](https://githubnext.com/projects/copilot-cli)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants