fix(ep-cuda): ignore the Celu GPU test, and stop the class recurring with a source scan by justinchuby · Pull Request #1920 · justinchuby/onnx-genai

justinchuby · 2026-08-24T01:56:01Z

What

Two things, one cause.

The fix: celu_matches_cpu_including_nan_and_alpha_variants (landed in cuda_ep: implement Celu activation (opset 12) #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.
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.

codecov · 2026-08-24T02:36:48Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.80%. Comparing base (ab91f07) to head (9f4e504).
⚠️ Report is 14 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1920      +/-   ##
==========================================
+ Coverage   80.20%   80.80%   +0.60%     
==========================================
  Files         400      422      +22     
  Lines      186458   206747   +20289     
  Branches   186458   206747   +20289     
==========================================
+ Hits       149540   167054   +17514     
- Misses      31546    34065    +2519     
- Partials     5372     5628     +256

Flag	Coverage Δ
cli-ort-linux	`72.51% <ø> (?)`
cli-ort-windows	`72.01% <ø> (?)`
mlas	`85.20% <ø> (?)`
offline	`80.95% <ø> (+0.74%)`	⬆️

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

…a target-level cfg (#1927) ## `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`). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…with a source scan The CUDA lane is red again on main. `celu_matches_cpu_including_nan_and_alpha_variants` (#1909) carries no ignore, so on a machine with no device it does not skip -- it runs. That is the fifth instance of the same defect in three days, after `content_preserving_transition_gpu`, `causal_conv_with_state_gpu`, `expert_route_telemetry_probe_gpu` and `activations_gpu::mish`. Five instances is a property of the signal, not of the authors. The authoritative check, verify_cuda_test_honesty.py, needs two full CUDA test builds and so runs only on the CUDA lane. An author therefore learns about the omission after merge, and only if that lane is green enough to be believed -- which it has not been. Every one of the five was reported by a lane already red for an unrelated reason, where one more red line is free. So this fixes the instance and adds `--source-scan`: the same rule read straight off the source in 75ms with no CUDA toolchain, wired into the `Rust quality` lane so it runs on every pull request. It supplements the inventory check rather than replacing it -- it cannot see macro-expanded tests the compiler sees, nor inventory parity between the two feature configurations. The first version of the scan was vacuous, and the fixtures record why. It searched the item's head for the substring "ignore", and Celu's doc comment says "a kernel that ignored the attribute entirely" -- so the scan passed on the exact defect it was written to catch. The fix is to read only attribute lines, never doc prose, and to anchor on the punctuation an attribute must have around it. Two of the fourteen fixtures exist solely to keep both defences falsifiable: relaxing the regex to a bare substring, or reading doc comments as attributes, each fails a distinct fixture. Evidence it detects what it claims: - positive control, per instance: restoring the pre-fix tree for Celu, for `mish` (3f81034^) and for `causal_conv_with_state_gpu` (e42fa94^) flags exactly one test each, by name and line, and nothing else. - negative control: passes on the fixed tree, all 79 CUDA targets. In particular it does not false-positive on `qmoe_gpu`, whose 10 macro-generated tests bake the attribute into the macro body, nor on `coarse_residency_plan_gpu` -- 2536 lines and 13 tests landed by #1854 while this branch was open, correctly clean. - self-test: 14 fixtures, mutation-checked in both directions. verify_cuda_test_honesty.py full run: 532 tests/79 targets without gpu-tests (532 ignored), 532 tests/79 targets with gpu-tests (476 fail-loud, 0 passed on this no-CUDA host). Refs #1875, #1909. Follows #1881 (e42fa94) and #1911 (3f81034). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Independent review falsified three claims the scan made about itself, and verifying the fix surfaced a fourth. All four are now fixtures. 1. Fail-open on any bracket inside a string literal (the serious one). The upward walk counted `(`/`)`/`[`/`]` as structure wherever they appeared, so a single stray bracket in an attribute string -- `#[should_panic(expected = "boom )")]` -- left the walk believing it was inside an unterminated attribute. It then climbed out of the item, through the `fn` above it, and adopted the *previous* test's ignore. The un-ignored test was reported clean. A guard that must fail closed was failing open, which is the exact class of defect this scan exists to catch. 2. The same escape resurrected the vacuity the scan was written to fix: with the walk running past the item head, a doc comment reading "we do not ignore, ever" was read as an attribute again. 3. `#[cfg_attr(not(feature = "something-else"), ignore = "x")]` was accepted. The predicate was never read, so an ignore conditioned on the wrong feature counted as an ignore, while the test still runs with gpu-tests off. 4. Found while fixing the above: a false positive on `vmm_kv_layout_residency_gpu.rs:355`, which is correctly ignored. Its `#[allow(...)]` sits below `#[test]` with a trailing `// comment`; masking the comment leaves trailing spaces, and joining the *stripped* lines made the masked and raw blobs disagree in length, so the span mapping drifted. The length guard caught it and reported rather than trusting the mapping -- fail-closed behaving as intended -- but the join was simply wrong. The fix is to stop pattern-matching lines and mask the source first: string literals, char literals, raw strings and comments are blanked length-preservingly, so brackets inside them are not structure and `ignore` inside them is not an attribute. Bracket accounting then runs on real code only. The `cfg_attr` predicate is read from the original text at the same offsets, because masking blanks the feature name it needs. Two backstops, both now falsifiable rather than merely present: - the upward walk stops at an item boundary regardless of bracket depth, so it degrades closed if accounting is ever wrong again. Exercised through `head_line_indices` with deliberately unbalanced input, since no valid Rust reaches that state once masking is correct. - the masked and raw blobs must agree in length before any span is mapped. 22 fixtures, and every defence is mutation-checked with a distinct failure: relaxing the ignore regex to a substring, dropping the cfg_attr predicate check, removing the item boundary, disabling masking, and letting comments truncate the head each fail a different one. Controls re-run against the real tree after the rewrite: passes on all 79 CUDA targets, and reverting the ignore on Celu, on mish (3f81034^) and on causal_conv_with_state_gpu (e42fa94^) flags exactly one test each, by name and line. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…e scan Three were fail-open and one would have reddened a green lane on correct code. All four are now fixtures, and every defence is mutation-checked. 1. A blank line inside a multi-line attribute truncated the head, so a correctly ignored test was reported. The upward walk broke on a blank line unconditionally while the downward walk gated the same break on bracket depth. Because the scan also runs inside the authoritative check, this would have turned a green CUDA lane red on valid source -- worse than useless for a pre-merge gate. 2. The `cfg_attr` predicate was matched as a substring, so an ignore that does not actually depend on `gpu-tests` being off was accepted: `all(not(feature = "gpu-tests"), windows)` contains the predicate and still runs the test on the Linux lane, and `cfg_attr(not(feature = "gpu-tests"), cfg_attr(feature = "foo", ignore))` gates the ignore on something else entirely. The predicate must now be the whole predicate, and the ignore must be the outer attribute's own. This is the same defect the previous round fixed, one level deeper -- reading a condition by substring rather than reading the condition. 3. `#[test]` was matched by exact line equality, so two spellings that compile and run were never inspected at all: `#[test] fn a() {}` on one line, and `#[ test ]` with internal whitespace. Detection is now an attribute-token match. The same-line form also has to suppress the downward walk, or the item inherits the *next* item's attributes -- including its ignore. 4. `pub(crate) fn` is an item opener that the boundary's `pub\s` did not match, letting the walk climb into the neighbour above and adopt its ignore. The boundary now anchors on the `fn` token itself, which covers every visibility spelling at once instead of enumerating them and being one spelling short again later. The enumerated `pub[\s(]` alternative was tried and removed: it made the `fn` anchor unfalsifiable, and a guard no fixture can break is a guard nobody should trust. 30 fixtures. Each defence fails a distinct one when mutated: ungating the blank-line break, matching the predicate by substring, permitting a nested `cfg_attr`, matching only the canonical `#[test]`, always walking downward, and removing the `fn` boundary. Controls re-run: clean on all 79 CUDA targets; reverting the ignore on Celu, on mish (3f81034^) and on causal_conv_with_state_gpu (e42fa94^) still flags exactly one test each, by name and line. 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>

… 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>

… 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 force-pushed the squad/1909-celu-ignore-and-fast-source-scan branch from 611a09d to f8bb670 Compare August 24, 2026 02:47

justinchuby mentioned this pull request Aug 24, 2026

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

Merged

justinchuby force-pushed the squad/1909-celu-ignore-and-fast-source-scan branch from 8bfe0dc to 002209b Compare August 24, 2026 04:28

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

justinchuby mentioned this pull request Aug 24, 2026

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

Merged

justinchuby and others added 3 commits August 23, 2026 22:43

justinchuby force-pushed the squad/1909-celu-ignore-and-fast-source-scan branch from 002209b to 9f4e504 Compare August 24, 2026 05:47

justinchuby merged commit 683861f into main Aug 24, 2026
4 checks passed

justinchuby deleted the squad/1909-celu-ignore-and-fast-source-scan branch August 24, 2026 05:47

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

justinchuby mentioned this pull request Aug 24, 2026

Flaky: async_pagein_fence_orders_weight_page_in_consumer fails 2 of 4 full-suite runs, passes in isolation #1896

Closed

Navigation Menu

Repository navigation

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

fix(ep-cuda): ignore the Celu GPU test, and stop the class recurring with a source scan#1920
justinchuby merged 3 commits into
mainfrom
squad/1909-celu-ignore-and-fast-source-scan

justinchuby commented Aug 24, 2026 •

edited

Loading

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 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Why a guard and not just the fix

Scope of the guard, stated honestly

Validation

Two rounds of independent Opus review, and what they found

Mutation matrix

Risk

Maintainer follow-up: rebased, fix half already landed, and one number corrected

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

justinchuby commented Aug 24, 2026 •

edited

Loading

codecov Bot commented Aug 24, 2026 •

edited

Loading