fix(ep-cuda): give coarse_residency_plan_gpu a fault seam instead of a target-level cfg by justinchuby · Pull Request #1927 · justinchuby/onnx-genai

justinchuby · 2026-08-24T03:00:50Z

`coarse_residency_plan_gpu` contains no tests in the configuration CI builds

crates/onnx-runtime-ep-cuda/tests/coarse_residency_plan_gpu.rs:90 carries #![cfg(feature = "gpu-tests")], so the whole target vanishes without the feature:

$ cargo test -p onnx-runtime-ep-cuda --features cuda --test coarse_residency_plan_gpu -- --list
0 tests, 0 benchmarks

$ cargo test -p onnx-runtime-ep-cuda --features cuda,gpu-tests --test coarse_residency_plan_gpu -- --list
13 tests, 0 benchmarks

That is the inventory drift verify_cuda_test_honesty.py exists to catch, and it is currently red on CUDA compile (Linux x86_64).

The gate was load-bearing, not gratuitous

It was the only thing making the file compile. apply_residency_plan_at_boundary_with_phase8_faults is gated #[cfg(any(test, feature = "gpu-tests"))], and the test arm does not reach an integration test — tests/*.rs are separate crates linking the library built without cfg(test), so with gpu-tests off the item does not exist and the top-level use naming it is E0432. Removing line 90 alone reproduces exactly that:

error[E0432]: unresolved import
  --> tests/coarse_residency_plan_gpu.rs:112
   |
112|     apply_residency_plan_at_boundary_with_phase8_faults,
   |     ^^^ no `apply_residency_plan_at_boundary_with_phase8_faults` in `coarse_residency`
note: found an item that was configured out
  --> src/coarse_residency.rs:411:8
409| #[cfg(any(test, feature = "gpu-tests"))]

Gating the target makes that compile, at the cost of the target silently containing nothing. This is the sixth instance of the #1875 class in four days, and the second sub-class of it — not a missing #[ignore], but a test that is not there at all. Both sub-classes have the same cause: the lane that reports them has been red for unrelated reasons, so the signal cost nothing to ignore.

The fix

The shape this repo already uses for exactly this problem: gate a helper, never the tests, so every test stays in both inventories. Precedent, same repo, same shape — crates/onnx-runtime-cuda-memory/tests/vmm_release_quarantine_gpu.rs's install_faults.

A two-arm local shim keeps the name identical, so the three call sites are unchanged, and the production gate on the underlying entry point is untouched: fault injection is still compiled only under gpu-tests, and the other arm is unreachable!.

Ungating the library item was the alternative and is rejected. It would put a fault-injection seam into production builds, and its documented mirror transition_granule_range_with_phase8_faults (granule_transition.rs:246) carries the same gate — ungating one of a documented pair would be inconsistent as well as wrong.

Validation

check	result
inventory without `gpu-tests`	13 tests (was 0)
inventory with `gpu-tests`	13 tests
`verify_cuda_test_honesty.py`	every `coarse_residency_plan_gpu` error gone
`clippy --features cuda --test coarse_residency_plan_gpu -- -D warnings`	clean
`clippy --features cuda,gpu-tests ...`	clean
`cargo fmt --all -- --check`	clean

On local red, stated plainly: the honesty script is not fully green on this branch. The only failures left are the two inherited activations_gpu / Celu errors from #1909 — this branch does not touch that file, and #1920 fixes them. Verified by running the script here and confirming coarse_residency_plan_gpu no longer appears in the output at all. I will rebase and confirm a fully green run once #1920 lands, and will not merge before that.

No test bodies, assertions or GPU semantics changed — the thirteen tests still carry their #[ignore] and still require a device.

Follow-up, deliberately not in this PR

The fast source scan added by #1920 is blind to this sub-class: it checks that every test carries an ignore, and a target with no tests trivially satisfies that. Detection for target-level #![cfg(...)] in a policed CUDA target belongs in a follow-up on top of #1920, so the guard and the green tree land together rather than a guard landing red.

Refs #1875, #1854. Related: #1920, #1881 (e42fa9470), #1911 (3f8103478).

…a target-level cfg `coarse_residency_plan_gpu.rs` carried `#![cfg(feature = "gpu-tests")]`, so the target reported no tests at all in the default configuration and thirteen with the feature on: $ cargo test -p onnx-runtime-ep-cuda --features cuda \ --test coarse_residency_plan_gpu -- --list 0 tests, 0 benchmarks $ cargo test -p onnx-runtime-ep-cuda --features cuda,gpu-tests \ --test coarse_residency_plan_gpu -- --list 13 tests, 0 benchmarks That is the inventory drift `verify_cuda_test_honesty.py` exists to catch, and it is why `CUDA compile (Linux x86_64)` is red on main. The target-level gate was not gratuitous -- it was the only thing making the file compile. `apply_residency_plan_at_boundary_with_phase8_faults` is gated `#[cfg(any(test, feature = "gpu-tests"))]`, and the `test` arm does not reach an integration test: `tests/*.rs` are separate crates linking the library built *without* `cfg(test)`, so with `gpu-tests` off the item does not exist and the top-level `use` naming it is E0432. Gating the whole target makes that compile, at the cost of the target silently containing nothing in the configuration CI actually builds. This is the sixth instance of the #1875 class in four days and the second sub-class of it: not a missing ignore, but a test that is not there at all. The fix is the shape already used in this repo for exactly this problem -- gate a *helper*, never the tests, so every test stays in both inventories. `crates/onnx-runtime-cuda-memory/tests/vmm_release_quarantine_gpu.rs` does this with `install_faults`. A two-arm local shim keeps the name identical, so the three call sites are unchanged, and the production gate on the underlying entry point is untouched: fault injection is still compiled only under `gpu-tests`, and the other arm is `unreachable!`. Ungating the library item was the alternative and is rejected: it would put a fault-injection seam in production builds, and its documented mirror `transition_granule_range_with_phase8_faults` carries the same gate, so ungating one of the pair would be inconsistent as well as wrong. Validation: - inventory parity restored, the point of the change: 13 tests without `gpu-tests`, 13 with. Was 0/13. - `verify_cuda_test_honesty.py`: every `coarse_residency_plan_gpu` error is gone. The only failures left on this branch are the two inherited `activations_gpu`/Celu ones from #1909, which #1920 fixes; this branch does not touch that file. - `cargo clippy -p onnx-runtime-ep-cuda --features cuda --test coarse_residency_plan_gpu -- -D warnings` clean, and clean again with `cuda,gpu-tests`. - `cargo fmt --all -- --check` clean. No test bodies, assertions or GPU semantics were changed -- the thirteen tests still carry their `#[ignore]` and still require a device to run. Refs #1875, #1854. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

codecov · 2026-08-24T03:24:54Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.34%. Comparing base (b69ea05) to head (3985763).
⚠️ Report is 12 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1927      +/-   ##
==========================================
+ Coverage   80.19%   80.34%   +0.14%     
==========================================
  Files         399      415      +16     
  Lines      185939   204833   +18894     
  Branches   185939   204833   +18894     
==========================================
+ Hits       149115   164570   +15455     
- Misses      31471    34685    +3214     
- Partials     5353     5578     +225

Flag	Coverage Δ
cli-ort-linux	`72.51% <ø> (?)`
cli-ort-windows	`72.10% <ø> (?)`
mlas	`85.20% <ø> (?)`
offline	`80.47% <ø> (+0.27%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.
see 63 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.

…ours (#1941) ## What broke `celu_matches_cpu_including_nan_and_alpha_variants` (added in #1909) has a bare `#[test]`. The other four tests in `activations_gpu.rs` all carry: ```rust #[cfg_attr( not(feature = "gpu-tests"), ignore = "requires CUDA device; enable the gpu-tests feature on a CUDA runner" )] ``` So without the feature, this one test alone tried to construct a CUDA EP and **failed** on a CPU-only runner instead of reporting ignored. `verify_cuda_test_honesty.py` flagged it precisely: ``` - activations_gpu: 1 tests failed while checking ignored status (celu_matches_cpu_including_nan_and_alpha_variants) - activations_gpu: Cargo inventory has 5 tests but libtest reported 4 ignored ``` ## Why it reached main #1909 was merged with `gh pr merge --squash --admin`, which bypasses the check that reports this. The check was working; nothing was listening. Worth noting plainly rather than filing under "flake". ## Verification On a CUDA host, in a registry-rebuilt child process: | Configuration | Result | |---|---| | No feature (what a CPU-only runner does) | `0 passed; 0 failed; 5 ignored` — was 4 ignored plus one live test | | `--features gpu-tests --test-threads=1` | `5 passed; 0 failed; 0 ignored` | The second row matters as much as the first: the point is to make the test *ignored where it cannot run*, not ignored everywhere. A one-line `#[ignore]` would have satisfied the honesty check while silently deleting the coverage. ## Note on a second failure in the same CI run The same job also reported 13 `coarse_residency_plan_gpu` inventory failures. Those are **not** addressed here and need no action: they are the pre-existing issue #1927 fixed, and the branch that surfaced them predates that merge. Co-authored-by: justinchuby <223556219+Copilot@users.noreply.github.com> Copilot-Session: d60eb808-7cc6-4abc-b48d-2a6dd3841624

…with a source scan (#1920) ## What Two things, one cause. 1. **The fix:** `celu_matches_cpu_including_nan_and_alpha_variants` (landed in #1909, `d6e542509`) has no `#[cfg_attr(not(feature = "gpu-tests"), ignore = "...")]`, so it *runs* on the CUDA lane's no-GPU builder and reds it. 2. **The guard:** a `--source-scan` mode in `verify_cuda_test_honesty.py`, wired into the fast `rust-quality` lane, that catches this class **in ~75 ms** instead of after a 20-minute CUDA build. ## Why a guard and not just the fix This is the **sixth** instance of the class in four days, and the fifth of this exact sub-class: | # | test | landed | fixed by | |---|---|---|---| | 1-3 | `content_preserving_transition_gpu` (+2 more) | | #1881 `e42fa9470` | | 4 | `causal_conv_with_state_gpu` | | #1881 | | 5 | `activations_gpu::mish` | | #1911 `3f8103478` | | 6 | `activations_gpu::celu` | #1909 | **this PR** | The shared cause isn't carelessness. The authoritative check only runs on `CUDA compile (Linux x86_64)`, which takes ~20 minutes and has itself been red for unrelated reasons for much of this week — so an author gets no usable signal at the time they can act on it. Another one-off fix leaves instance seven to the same trap. The scan runs on a lane that is fast, always-on for any `.rs` change, and green. ## Scope of the guard, stated honestly The scan detects **exactly one** sub-class: a `#[test]` in a policed CUDA target with no effective ignore. It is **blind** to the target-level `#![cfg(feature = "gpu-tests")]` inventory-drift sub-class — which I found while validating this PR, and which is fixed separately in **#1927**. Extending the scan to that sub-class is deliberately deferred to a follow-up so a guard and a green tree land together. It reuses `is_cuda_test_target()` rather than re-deriving "policed target", so there remains one definition. ## Validation **Positive controls** (would a real defect be caught?) — reverting the ignore flags **exactly one** test, by correct name and line, in each case: - Celu, on this branch - `mish` at `3f8103478^` — a genuine historical instance - `causal_conv_with_state_gpu` at `e42fa9470^` — another **Negative controls** (does correct code pass?) — clean on all 79 CUDA targets in 75 ms, including `qmoe_gpu` (10 macro-generated tests) and `coarse_residency_plan_gpu` (2536 lines / 13 tests, written by someone else, landed mid-branch — a negative control I did not construct). **Combined with #1927**, the authoritative script is green: `554 tests / 81 targets` in both feature configurations, `exit 0`. Each branch alone is red only on what the other fixes. `--self-test`: **30 fixtures**. `cargo fmt --all -- --check` clean. ## Two rounds of independent Opus review, and what they found I reproduced every reported defect myself before fixing it, and mutation-tested every defense afterwards. **Round 1 — three real defects, one of them serious:** - **Fail-open (major):** bracket counting did not ignore string literals, so the upward attribute walk could escape an item and adopt the *previous* test's ignore — silently clearing a genuinely un-ignored test. - That same escape **resurrected the exact vacuity this PR exists to fix**: v1 of the scan searched the item head for the substring `"ignore"`, and Celu's doc comment reads "a kernel that **ignored** the attribute entirely". The guard passed on English prose. It failed its own positive control, which is the only reason I know. - `#[cfg_attr(not(feature = "something-else"), ignore)]` was accepted as a valid ignore. Fixed by rewriting the internals: length-preserving, column-exact masking of literals and comments; hard item boundaries; predicate validation by `fullmatch`. While fixing, I **found a fourth defect myself** — a false positive on `vmm_kv_layout_residency_gpu.rs:355`, caused by `.strip()`ing lines when building the masked and raw blobs, which broke the column alignment the span mapping depends on. My own fail-closed length guard caught it. **Round 2 — four residual holes:** - A blank line inside an attribute list broke the walk → **false positive on correct code**, which would red a green lane. Worse than a miss, in a guard whose whole value is being trusted. - The `cfg_attr` predicate was matched as a substring, so a nested `cfg_attr` could smuggle a different predicate through. - Non-canonical `#[test]` spellings (e.g. `#[ test ]`) were never inspected at all. - `pub(crate) fn` was not matched by the `pub\s` item boundary. All four fixed, each with a fixture. ## Mutation matrix Every defense is falsifiable — I broke each one and confirmed a **distinct** failure: | mutation | caught by | |---|---| | ungate the blank-line break | blank-line-in-attribute fixture | | match the predicate by substring | nested-`cfg_attr` fixture | | permit a nested `cfg_attr` | predicate fixture | | match only canonical `#[test]` | spacing fixture | | always walk downward | one-line `#[test] fn a() {}` fixture | | remove the `fn` item boundary | `pub(crate)` fixture | | relax `IGNORE_ATTR` to a substring | doc-prose fixture | | disable masking | literal-bracket fixture | One finding worth passing on: `FN_DECLARATION` and an enumerated `pub[\s(]` boundary turned out to be **redundant**, and the symptom was that *neither could be mutation-caught* — each was covered by the other. An unfalsifiable guard is dead weight regardless of whether it is correct. I removed the enumeration, kept the token anchor, and confirmed the fixture then fires. ## Risk The scan is additive and read-only. Its failure mode if wrong is a false positive on the fast lane — visible immediately, fixable in one line, and specifically fixture-guarded above. The Celu change is a single attribute. Closes part of #1875. --- ## Maintainer follow-up: rebased, fix half already landed, and one number corrected **The fix half is gone from this PR.** I hit the same Celu failure independently and — not having checked the open PR list first, which was my mistake — landed the one-line ignore separately as #1941. Rebasing this branch onto `main` dropped that hunk cleanly. What remains is **only the guard**, which was always the valuable half: ``` .github/scripts/verify_cuda_test_honesty.py | 456 +++++++++++++++++++++++++++- .github/workflows/ci.yml | 17 ++ ``` **Independently re-verified the guard on a Windows CUDA host**, since a guard I have not personally seen fire is not a guard I should merge: | Check | Result | |---|---| | Scan on current `main` | `CUDA test source scan passed: every test in a CUDA target carries an ignore`, exit 0 | | **Falsified** — Celu's `cfg_attr` removed again | exit 1, `activations_gpu.rs:336: celu_matches_cpu_including_nan_and_alpha_variants has no ignore attribute` — correct file, correct line, correct test, with the fix spelled out | **One number is wrong and I am correcting it rather than repeating it.** The body claims the scan is clean *"in 75 ms"*. On this Windows host the median of three steady-state runs is **~3.4 s wall** (3383 / 3546 / 4438 ms), of which ~0.6 s is interpreter startup alone. I could not reproduce 75 ms here; it may hold on a Linux runner with a warm cache, but as written it reads as a cross-platform property and is not one. This changes nothing about the argument. The comparison that matters is 3.4 s on an always-on fast lane against ~20 minutes on a CUDA lane that has been red for unrelated reasons much of this week — still three orders of magnitude, and still the difference between signal an author can act on and signal they cannot. But a number that is off by 160x reads as evidence, so it should not stand. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

`CUDA compile (Linux x86_64)` is red on main at `d3bf2ebaa` in `capture_sync_contract`, which is a pin over the set of kernel functions that synchronize unconditionally. #1913 implemented `com.microsoft::MultiHeadAttention`, whose `execute` drains the trailing transpose before returning per-call scratch to the pool, and did not add it to the pin. I checked that the delta is accounted for rather than making the numbers match. The set differs by exactly one entry, added and none removed, and it comes from exactly one commit touching `src/kernels/` -- #1913. The pin's own comment states the admission rule: an entry must be a path whose `capture_support` is explicitly `Unsupported`. `MultiHeadAttentionKernel::capture_support` returns `CaptureSupport::unsupported("... allocates scratch and synchronizes")`, so the kernel is already excluded from graph capture and the sync is sound. It is the same shape as `packed_varlen_attention.rs::execute`, which is in the list for the same reason. So this is a pin that fell behind a deliberate change, not a regression in capture behaviour, and the fix is to admit the entry with the justification next to it. Worth noting how it went unseen: the CUDA lane has been red more or less continuously this week -- #1881, #1911, #1920, #1927 -- so a lane that was already failing absorbed a new failure without anyone learning anything from the colour. That is the argument for the pre-merge source scan in #1920 rather than for more diligence. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…ded (#1939) ## What `CUDA compile (Linux x86_64)` is red on `main` (reproduced at `d3bf2ebaa`, and on `origin/main` locally with no changes applied). The failing step is **Audit CUDA kernel capture-sync contract**, not the honesty script: ``` ---- unconditional_syncs_are_limited_to_capture_unsupported_paths ---- assertion `left == right` failed left has "multi_head_attention.rs::execute" right does not ``` #1913 implemented `com.microsoft::MultiHeadAttention`. Its `execute` drains the trailing transpose before returning per-call scratch to the allocator pool, which is an unconditional `synchronize()`. The pin that exists to notice exactly that was not updated. ## Why bumping the pin is the right fix here I checked that the delta is **accounted for** rather than just making the assertion pass — a pin you bump reflexively is worse than no pin. 1. **The delta is exactly one entry, added, with none removed.** Both sets are otherwise identical. 2. **It comes from exactly one commit.** `git log -- src/kernels/multi_head_attention.rs` is a single commit, #1913 (`027e0cfb8`). 3. **It satisfies the pin's own stated admission rule.** The comment above `expected` reads: *"Every entry is a path whose `capture_support` is explicitly `Unsupported`, or a dynamically-admitted kernel's fallback path that `capture_support` rejects."* `MultiHeadAttentionKernel::capture_support` returns: ```rust CaptureSupport::unsupported( "cuda_ep MultiHeadAttention uses the per-call Phase-2a workspace path (allocates scratch and synchronizes)", ) ``` So the kernel is already excluded from graph capture; the sync cannot be captured because the path cannot be. 4. **It is structurally the same as an entry already in the list.** `packed_varlen_attention.rs::execute` is admitted for the identical reason — trailing stream synchronize on a path declared `unsupported`. This is not a new category. So: a pin that fell behind a deliberate change, not a regression in capture behaviour. The entry carries its justification inline so the next reader doesn't have to re-derive it. ## Verification - **Positive control, already observed:** the pin *did* fire on a real behavioural change. That is the test working, not failing. - After this change: `test result: ok. 1 passed; 0 failed`. - Reproduced the failure on `origin/main` with zero local modifications first, to establish it was not mine. - `cargo fmt --all -- --check` clean. ## How it went unseen, which is the part worth fixing The CUDA lane has been red more or less continuously this week — #1881, #1911, #1920, #1927 — so a lane that was **already failing** absorbed a new failure without anyone learning anything from the colour. Resch hit the identical shape with the `op_rules` count pin (#1873), and Gaff with a grep-based falsifier that couldn't distinguish the case it claimed to detect. That is an argument for the pre-merge source scan in #1920 rather than for more diligence: a check nobody can read the output of is not a check. Unblocks #1920. Part of #1875. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…ead of running The source scan added in #1920 catches a CUDA test with no ignore, which runs where there is no device. #1854 shipped the other shape: a target-level `#![cfg(feature = "gpu-tests")]` that took a 13-test file to 0 tests in the configuration CI builds, fixed in #1927. The lane was green because the tests were not there. Both spellings break the same rule -- a policed target must offer the same tests with `gpu-tests` off as on, ignored but present -- and the scan could only see one of them. It now sees four: a target-level gate, a `#[cfg(feature = "gpu-tests")]` on a test, the same on a `mod` that contains tests, and `required-features` on a policed target. Positive control: against `coarse_residency_plan_gpu.rs` as it stood before #1927, the scan reports exactly one error, at line 90, and nothing else across the 81 policed targets. An independent review found two defects worth more than the feature: **It flagged a `cfg` on any feature, not just `gpu-tests`.** Only `gpu-tests` separates the two configurations this script builds, and it is a leaf -- ep-cuda forwards it to cuda-memory, where it is `[]`. A `cfg` on `cuda` or `cuda-13000` resolves identically in both builds, causes no drift, and is passed by the authoritative check -- so flagging it would have failed the fast lane on correct code, and the error would have advised an `ignore` that does not apply. This was latent, not firing, but `#![cfg(feature = "cuda")]` is already in this tree on an unpoliced target. A false positive on a lane that gates every PR is worse than a miss. **`required-features` was parsed with regexes, and was wrong in both directions** -- a `[[test]]` span absorbed a following `[[bench]]` table, and a header with a trailing comment was not recognised. It also had no fixture and no `[[test]]` section exists today, so it was a shipped defense that had never executed. Now parsed with `tomllib`, with both failure shapes pinned. Also from that review: a `mod` gated on `gpu-tests` deletes every test inside it, and the head walk stops at the `mod` boundary so it was invisible; the drift scan skipped a misaligned item where `un_ignored_tests` reports one, which is the less safe of the two dispositions; and the target count is 81, not the 79 I had written. Eight mutations, eight distinct fixture failures. Two guards survived the first matrix and were therefore unfalsifiable. Rather than delete them I looked for the case that distinguishes each, and both were real: reading the feature name from raw text without first establishing the key on masked text lets a feature named inside a string literal read as a gate, and starting the module brace walk anywhere but the module's own body counts an earlier module's tests as the gated one's. Runtime measured, not asserted: 0.99s before, 2.90s masking each file twice, 1.10s with `mask_source` memoised. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Independent review of the section found no blocking defect but three statements stated more strongly, or more loosely, than the evidence supports. Since the page's whole subject is guards that claim more than they check, leaving them would have been self-refuting. 1. The policed set was given as "files ending in _gpu". That is wrong in both directions against is_cuda_test_target(): it omits the allowlist (matmul_nbits_marlin_numerics is policed with no _gpu suffix) and it includes suite_canary_gpu, which is in ALWAYS_RUN. The canary has two #[test] and zero cfg_attr *by design* -- it exists to execute on deviceless runs, and rule 1 applied to it would remove it from exactly the runs it polices. A reader following the rule as written would have broken the canary, so this was an actionably wrong instruction, not a simplification. 2. "The disabled arm is never executed" is stronger than the mechanism gives. It is not executed under a normal run; `cargo test -- --ignored` with gpu-tests off reaches it. That is what unreachable!() is for -- fail loud rather than return a fake result -- so the code is right and only the sentence was over-claimed. 3. The trap note read as though #1927 deleted thirteen tests. Backwards: #1854 (26974e5) introduced the target-level #![cfg] that deleted them, #1927 (be788b9) replaced it with the shim and put them back. Confirmed with git log -S on the exact gate string. Also dropping the ci.yml line numbers cited in the previous commit message. They were correct for this branch's base, but the honesty-script step has already moved 1011 -> 1021 on main since, which is the argument against citing line numbers at all rather than for correcting them. The rendered page cites none. Re-verified after the edits: shim example still an exact substring of vmm_release_quarantine_gpu.rs, fences balanced, ALWAYS_RUN and CUDA_TARGETS_WITHOUT_SUFFIX quoted from the script rather than recalled. Refs #1875 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…ead of running The source scan added in #1920 catches a CUDA test with no ignore, which runs where there is no device. #1854 shipped the other shape: a target-level `#![cfg(feature = "gpu-tests")]` that took a 13-test file to 0 tests in the configuration CI builds, fixed in #1927. The lane was green because the tests were not there. Both spellings break the same rule -- a policed target must offer the same tests with `gpu-tests` off as on, ignored but present -- and the scan could only see one of them. It now sees four: a target-level gate, a `#[cfg(feature = "gpu-tests")]` on a test, the same on a `mod` that contains tests, and `required-features` on a policed target. Positive control: against `coarse_residency_plan_gpu.rs` as it stood before #1927, the scan reports exactly one error, at line 90, and nothing else across the 81 policed targets. An independent review found two defects worth more than the feature: **It flagged a `cfg` on any feature, not just `gpu-tests`.** Only `gpu-tests` separates the two configurations this script builds, and it is a leaf -- ep-cuda forwards it to cuda-memory, where it is `[]`. A `cfg` on `cuda` or `cuda-13000` resolves identically in both builds, causes no drift, and is passed by the authoritative check -- so flagging it would have failed the fast lane on correct code, and the error would have advised an `ignore` that does not apply. This was latent, not firing, but `#![cfg(feature = "cuda")]` is already in this tree on an unpoliced target. A false positive on a lane that gates every PR is worse than a miss. **`required-features` was parsed with regexes, and was wrong in both directions** -- a `[[test]]` span absorbed a following `[[bench]]` table, and a header with a trailing comment was not recognised. It also had no fixture and no `[[test]]` section exists today, so it was a shipped defense that had never executed. Now parsed with `tomllib`, with both failure shapes pinned. Also from that review: a `mod` gated on `gpu-tests` deletes every test inside it, and the head walk stops at the `mod` boundary so it was invisible; the drift scan skipped a misaligned item where `un_ignored_tests` reports one, which is the less safe of the two dispositions; and the target count is 81, not the 79 I had written. Eight mutations, eight distinct fixture failures. Two guards survived the first matrix and were therefore unfalsifiable. Rather than delete them I looked for the case that distinguishes each, and both were real: reading the feature name from raw text without first establishing the key on masked text lets a feature named inside a string literal read as a gate, and starting the module brace walk anywhere but the module's own body counts an earlier module's tests as the gated one's. Runtime measured, not asserted: 0.99s before, 2.90s masking each file twice, 1.10s with `mask_source` memoised. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

… the worse half The testing page said the CUDA honesty check runs on "CUDA CI". That has been stale since #1920 put --self-test/--source-scan on the rust-quality lane, and the staleness is not incidental: believing the check only runs on a 20-minute lane is exactly what let this class recur across four successive fixes (#1881, #1911, #1920, #1927) under #1875. An author who thinks the signal arrives after merge does not look for it before. Records both lanes and their division of labour -- the CUDA lane is authoritative because it builds the test targets twice and compares inventories, the fast lane is the same rule read off the source in about a second and cannot see macro-expanded tests or inventory parity -- and the two hard rules for CUDA targets: 1. every #[test] needs cfg_attr(not(feature = "gpu-tests"), ignore = ...) 2. never cfg a test, its mod, or the whole target on gpu-tests Rule 2 is the dangerous one and was undocumented. It does not red a lane; it deletes the tests from the inventory, so the lane is green because the tests are not there (#1927 removed thirteen that way). The fix is a two-arm shim around the helper, quoted verbatim from crates/onnx-runtime-cuda-memory/tests/vmm_release_quarantine_gpu.rs rather than invented, so the example compiles by construction. Also records the trap that produces rule-2 violations in the first place: #[cfg(any(test, feature = "gpu-tests"))] does not cover integration tests, because tests/*.rs are separate crates linking the library built without cfg(test). Reaching such an item from a top-level use is E0432, and gating the whole target is the plausible-looking "fix" that hides the tests instead. Docs-only, so this PR sets docs_only=true and rust-quality is skipped: CI green here is not evidence for any claim above. Verified locally instead -- the lane assertions against ci.yml (L367, L370, L1011), and the shim example by exact string match against the source file. Refs #1875 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Independent review of the section found no blocking defect but three statements stated more strongly, or more loosely, than the evidence supports. Since the page's whole subject is guards that claim more than they check, leaving them would have been self-refuting. 1. The policed set was given as "files ending in _gpu". That is wrong in both directions against is_cuda_test_target(): it omits the allowlist (matmul_nbits_marlin_numerics is policed with no _gpu suffix) and it includes suite_canary_gpu, which is in ALWAYS_RUN. The canary has two #[test] and zero cfg_attr *by design* -- it exists to execute on deviceless runs, and rule 1 applied to it would remove it from exactly the runs it polices. A reader following the rule as written would have broken the canary, so this was an actionably wrong instruction, not a simplification. 2. "The disabled arm is never executed" is stronger than the mechanism gives. It is not executed under a normal run; `cargo test -- --ignored` with gpu-tests off reaches it. That is what unreachable!() is for -- fail loud rather than return a fake result -- so the code is right and only the sentence was over-claimed. 3. The trap note read as though #1927 deleted thirteen tests. Backwards: #1854 (26974e5) introduced the target-level #![cfg] that deleted them, #1927 (be788b9) replaced it with the shim and put them back. Confirmed with git log -S on the exact gate string. Also dropping the ci.yml line numbers cited in the previous commit message. They were correct for this branch's base, but the honesty-script step has already moved 1011 -> 1021 on main since, which is the argument against citing line numbers at all rather than for correcting them. The rendered page cites none. Re-verified after the edits: shim example still an exact substring of vmm_release_quarantine_gpu.rs, fences balanced, ALWAYS_RUN and CUDA_TARGETS_WITHOUT_SUFFIX quoted from the script rather than recalled. Refs #1875 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

… the worse half (#1959) The testing wiki said the CUDA test-honesty check runs on "CUDA CI". That has been stale since #1920 put `--self-test` / `--source-scan` on the `rust-quality` lane — and the staleness is the interesting part, because *believing the check only runs on the slow lane is the belief that let this class recur*. Under #1875 the same defect class needed four successive fixes: #1881, #1911, #1920, #1927. The authoritative check is real and correct, but it lives on a ~20-minute lane that was red continuously for other reasons, so an author got no signal at a point where they could still act on it. A check nobody reads in time is not a check. ## What the section now says **Both lanes, and the division of labour.** The CUDA lane is authoritative — it builds the CUDA test targets twice (with and without `gpu-tests`) and requires identical target sets *and* identical per-target test-name sets. The fast lane is the same rule read off the source in ~1 s with no CUDA toolchain. It **supplements and does not replace**: it cannot see macro-expanded tests, nor inventory parity between feature configs. **Which targets are policed**, quoted from `is_cuda_test_target()` rather than paraphrased: stem ends `_gpu` **or** is in `CUDA_TARGETS_WITHOUT_SUFFIX`, **and** is not in `ALWAYS_RUN`. **The two hard rules**, which were not written down anywhere: 1. Every `#[test]` in a policed target needs `#[cfg_attr(not(feature = "gpu-tests"), ignore = "...")]`, or it really runs on a machine with no GPU — *except* `ALWAYS_RUN` (`suite_canary_gpu`), which is deliberately un-ignored because it exists to run on deviceless machines. 2. Never `cfg` a test, its `mod`, or the whole target on `gpu-tests` (nor use manifest `required-features`). Rule 2 is the one worth documenting, and it is the more dangerous of the two. Rule 1 reds a lane loudly. Rule 2 **deletes the tests from the inventory** — the lane is green *because the tests are not there*. #1854 lost thirteen tests exactly that way, until #1927 put them back. **The trap that produces rule-2 violations.** `#[cfg(any(test, feature = "gpu-tests"))]` does **not** cover integration tests: `tests/*.rs` are separate crates linking the library built *without* `cfg(test)`, so the `test` arm never applies. Naming such an item from a top-level `use` is `E0432` — and gating the entire target is the plausible-looking "fix" that silently hides the tests instead. Documenting the trap next to the rule matters more than documenting the rule, since the trap is what makes the wrong fix look right. **The correct shape** is a two-arm shim around the *helper* — same name, same signature in both arms — leaving the test and its call sites byte-identical across configs. The example is quoted **verbatim** from `crates/onnx-runtime-cuda-memory/tests/vmm_release_quarantine_gpu.rs` rather than invented, so it compiles by construction and cannot drift into being wrong on its own. ## Independent review No blocking defects, but it falsified three claims, all in the direction of stating more than the evidence supports — which is the exact failure mode this page is about, so they are fixed in `248cceba2`: 1. **The policed set was actionably wrong, not merely simplified.** I wrote "files ending in `_gpu`". That both omits `matmul_nbits_marlin_numerics` (policed, no suffix) and wrongly includes `suite_canary_gpu`, which has two `#[test]` and zero `cfg_attr` *by design*. A reader applying rule 1 to the canary would have removed it from precisely the deviceless runs it exists to police. The worst kind of doc bug: confidently instructing a change that breaks a guard. 2. **"The disabled arm is never executed" was over-claimed.** `cargo test -- --ignored` with `gpu-tests` off reaches it. That is what `unreachable!()` is for — the code was right, the sentence was not. 3. **The `#1927` attribution was backwards.** #1854 (`26974e5fd`) introduced the target gate that deleted the thirteen tests; #1927 (`be788b924`) removed it and restored them. Confirmed with `git log -S` on the exact gate string. A fourth finding — that my commit message's `ci.yml` line numbers were off by seven — I checked and did **not** accept as stated: the reviewer read `ci.yml` from a different branch of mine whose comment edit shifted those lines. `L367`/`L370` were correct for this branch's base. The point survives in a better form, though: the honesty-script step has independently moved `1011` → `1021` on main since I wrote it, so I dropped the line numbers rather than correcting them. The rendered page cites none. ## Verification, and its limits **This is a docs-only change, so `changes` sets `docs_only=true` and `rust-quality` is skipped** — along with every other Rust job, each of which carries `if: needs.changes.outputs.docs_only != 'true'`. Green CI on this PR is therefore *not* evidence for anything asserted above. Saying so explicitly because the alternative — letting a green tick imply coverage it did not get — is the same failure mode this page documents. Verified locally instead: - Both lane claims checked against `.github/workflows/ci.yml`: `--self-test` and `--source-scan` in `rust-quality`, the bare script on the CUDA lane. - `wiki/*` confirmed a docs path in `is_docs_path()`, hence the skip above. - `ALWAYS_RUN` and `CUDA_TARGETS_WITHOUT_SUFFIX` read out of the script, not recalled. - `suite_canary_gpu.rs` confirmed at 2 `#[test]` / 0 `cfg_attr`, which is what makes finding 1 load-bearing. - The shim example checked by **exact string containment** against the source file, not by eye — before and after the edits. My first attempt at this check compared the wrong line span and reported a spurious `1d0`/`9a9`; the framing was off, not the code. - Markdown fences balanced (20, even); no docs/markdown lint gate exists in `ci.yml` to satisfy. Refs #1875 --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…ead of running (#1947) ## What #1920 taught the fast `rust-quality` lane to catch a CUDA test with **no ignore** — one that *runs* where there is no device. This adds the other half of the same rule: a test that **isn't there at all**. The rule both halves enforce is the one `compare_inventories` already enforces authoritatively: *a policed CUDA target must offer the same tests with `gpu-tests` off as on — ignored, but present.* ## Why the second half matters more than it sounds A missing ignore reds the lane loudly. The drift half is the quiet one: **the target compiles, the suite is green, and it is green because the tests are not there.** #1854 shipped `coarse_residency_plan_gpu.rs` with a target-level `#![cfg(feature = "gpu-tests")]`. That file is 2536 lines and 13 tests. In the configuration CI builds, it contributed **zero**. Fixed in #1927; the scan added in #1920 was structurally blind to it. Four spellings cause this, and the scan now sees all four: | spelling | effect with `gpu-tests` off | |---|---| | `#![cfg(feature = "gpu-tests")]` on the target | the whole file holds 0 tests (#1854) | | `#[cfg(feature = "gpu-tests")]` on a test | that test is absent from the inventory | | the same on a `mod` that contains tests | every test in it is absent | | `required-features` on a policed `[[test]]` | the target isn't built at all | All four are already forbidden by `compare_inventories`. This says so in ~1 second instead of after a 20-minute lane. ## Independent review found two defects worth more than the feature Both were things I could not have found by re-reading my own code, and I fixed both before this description was written. ### MAJOR — it flagged a `cfg` on *any* feature, not just `gpu-tests` Only `gpu-tests` separates the two configurations this script builds (`cuda` vs `cuda,gpu-tests`), and it is a leaf feature — ep-cuda forwards it to cuda-memory, where it is `[]`. So a `cfg` on `cuda`, `cuda-13000` or `tracing` resolves **identically in both builds**, causes no drift, and is passed by the authoritative check. Flagging those would have **failed the fast lane on code the CUDA lane accepts**, and the error message would have advised an `ignore` that doesn't apply. It was latent — nothing in the tree triggers it — but `#![cfg(feature = "cuda")]` already exists at `qwen35_0_8b_placement_lock.rs:46`, escaping only because that stem isn't policed. On a lane that gates every PR in the repo, a false positive is worse than a miss. Now restricted to `gpu-tests`, with `cuda` / `cuda-13000` / `tracing` as negative-control fixtures. ### MAJOR — `required-features` was regex-parsed, wrong in both directions, and had never executed - **False positive:** a `[[test]]` span absorbed a following `[[bench]]` table, so a target with *no* `required-features` was flagged for its neighbour's. - **Miss:** `[[test]] # comment` was not recognised as a table header at all. - And no `[[test]]` section exists in either crate today, and there was **no fixture** — so this was a shipped defense that had never run in either direction. Now parsed with `tomllib`, with both failure shapes pinned as fixtures. ### Also from that review - A **`mod` gated on `gpu-tests`** deletes every test inside it. The head walk deliberately stops at the `mod` boundary, so it was invisible from inside. Now detected — but only when the module actually contains a test, because gating a module of *helpers* is the sanctioned two-arm shim this check recommends. - The drift scan **skipped** a misaligned item where `un_ignored_tests` **reports** one. Same condition, less safe disposition. Now fails closed in both. - The policed target count is **81**, not the 79 I had written. ## Validation **Positive control against the real historical defect** — `coarse_residency_plan_gpu.rs` exactly as it stood before #1927: ``` crates/onnx-runtime-ep-cuda/tests/coarse_residency_plan_gpu.rs:90: the whole target is gated on feature "gpu-tests". With the feature off the target holds no tests at all, so it cannot disagree with the device suite and cannot go red. ``` One error, correct line, nothing else flagged across 81 targets. **Negative controls** — all correct code, none flagged: `#![allow(..)]`, `#![cfg(target_os = "linux")]`, `#![cfg(feature = "cuda")]`, `#![cfg(feature = "cuda-13000")]`, `#[cfg(target_os = "linux")]` on a test, `#[cfg(feature = "tracing")]` on a test, `#[cfg_attr(not(feature = "gpu-tests"), ignore)]`, a gated helper module, and a gated module preceded by an unrelated module that does have tests. **Still catches real drift:** `#[cfg(feature = "gpu-tests")]`, `#[cfg(not(feature = "gpu-tests"))]` (drift in the other direction), and `#[cfg(all(feature = "gpu-tests", unix))]`. **Authoritative script:** `554 tests / 81 targets` in both configurations, `exit 0`. `cargo fmt --all -- --check` clean. ## Mutation matrix — 8 mutations, 8 distinct failures | mutation | caught by | |---|---| | flag any feature, not just `gpu-tests` | `#![cfg(feature = "cuda")]` causes no drift | | match `cfg` by prefix (so `cfg_attr` matches) | `cfg_attr keeps the test in both inventories` | | drop the feature-mention guard | `a feature named inside a string literal is not a gate` | | find gate spans in raw instead of masked text | `an unbalanced bracket … must not hide the gate below it` | | flag gated modules with no tests | `a gated helper module holds no tests` | | break the module brace walk | `an earlier module's tests must not be counted as the gated module's` | | revert `required-features` to section-splitting | `a following bench table must not be read as part of the test target` | | revert it to `^\[\[test\]\]$` | `a trailing comment on the table header must not hide required-features` | ### Two guards were unfalsifiable, and chasing them found two real holes `FEATURE_MENTION` and the module brace-walk start survived the first matrix — mutating either left every fixture green. Rather than delete them as dead weight (the disposition I applied to a redundant boundary check in #1920), I looked for the case that distinguishes each. **Both were real:** 1. Establishing the feature *key* on masked text is load-bearing, because the *name* is read from raw text. Without it, `#[cfg(all(unix, target_env = r#"feature = "gpu-tests""#))]` reads a feature named inside a string literal as a real gate. 2. Starting the brace walk anywhere but the module's own body counts an **earlier** module's tests as the gated one's — which flags a correct gated helper and reds a green lane. I would not have found either by reading the code. The matrix found them by telling me a line I believed in didn't matter. ## Runtime — measured, not asserted | | wall | |---|---:| | before this change | 0.99 s | | naive (masking each file twice) | 2.90 s | | after memoising `mask_source` | **1.10 s** | I timed the revisions against each other rather than trusting a note — an earlier revision of #1920's description claimed "~75 ms", which was simply wrong, and only a measurement caught it. ## Scope Deliberately narrow. This does not replace `compare_inventories`, which stays authoritative: the scan cannot see macro-expanded tests and cannot verify parity across two real builds. It catches the specific spellings that have actually reached `main`. Part of #1875. Follows #1920, #1927, #1939. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

justinchuby mentioned this pull request Aug 24, 2026

fix(ep-cuda): ignore the Celu GPU test, and stop the class recurring with a source scan #1920

Merged

justinchuby marked this pull request as ready for review August 24, 2026 04:22

justinchuby merged commit be788b9 into main Aug 24, 2026
14 of 17 checks passed

justinchuby deleted the squad/1854-coarse-residency-target-cfg branch August 24, 2026 04:23

This was referenced Aug 24, 2026

test(cuda-ep): pin the MultiHeadAttention trailing sync that #1913 added #1939

Merged

fix(cuda-ep): gate the Celu GPU test behind gpu-tests like its neighbours #1941

Merged

justinchuby mentioned this pull request Aug 24, 2026

ci(cuda): catch the other half of the class -- tests that vanish instead of running #1947

Merged

This was referenced Aug 24, 2026

main: CUDA compile lane red — a #[cfg(test)] item is imported by an integration test, which is a separate crate #1875

Closed

docs(wiki): the honesty check runs on the fast lane too, and drift is the worse half #1959

Merged

Navigation Menu

Repository navigation

fix(ep-cuda): give coarse_residency_plan_gpu a fault seam instead of a target-level cfg - #1927

fix(ep-cuda): give coarse_residency_plan_gpu a fault seam instead of a target-level cfg#1927
justinchuby merged 1 commit into
mainfrom
squad/1854-coarse-residency-target-cfg

justinchuby commented Aug 24, 2026

Uh oh!

codecov Bot commented Aug 24, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

justinchuby commented Aug 24, 2026

coarse_residency_plan_gpu contains no tests in the configuration CI builds

The gate was load-bearing, not gratuitous

The fix

Validation

Follow-up, deliberately not in this PR

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

`coarse_residency_plan_gpu` contains no tests in the configuration CI builds

codecov Bot commented Aug 24, 2026 •

edited

Loading