test(ep-cpu): assert the STFT fast-path property, not the radix-2 route (fixes macOS arm64 coverage lane)

[justinchuby]

Fixes the Rust coverage (macOS arm64) failure that #2083 introduced and that every branch cut afterwards inherits (seen on #2084: job 97720817084).

kernels::stft::tests::real_unwindowed_overlapping_frames_match_independent_reference
each power-of-two frame must use the radix-2 FFT path

It is false by construction on Apple, not flaky

The test reads DFT_FFT_TEST_HITS — the radix-2 counter — and requires it to advance by three. On Apple targets DftPlan::new builds a vDSP setup for every power-of-two n >= 4, and transform returns from the vDSP branch before reaching the radix-2 one. The test's frame length is 4. So on macOS the three transforms increment DFT_VDSP_TEST_HITS, DFT_FFT_TEST_HITS stays at 0, and the assertion says macOS did not use macOS's fast path. It can never pass there, and it has nothing to do with the property the test was written to check.

fft_fallback_reachability in dft.rs asserts on the same counter and is green on macOS only because it uses n = 2, below the vDSP minimum of 4. That is luck, not coverage.

The fix

Assert the property, not one platform's route to it. Every target shares the same requirement: a power-of-two frame must not fall back to naive_dft_into. dft::fast_path_hits() sums whichever counters this target's fast paths increment, so the assertion no longer names a route.

Falsified in both directions

No macOS target compiles in this environment (aarch64-apple-darwin → E0463, no std in this toolchain), so both proofs are emulations run on Linux and are stated as such:

mutation	result
force the power-of-two branch to naive_dft_into	test FAILS with the fix in place — not vacuous
emulate Apple's dispatch rule (pow2 && n >= 4 → vDSP counter, early return)	passes with the fix; with the pre-fix assertion restored it reproduces the macOS CI panic on Linux

The second mutation also caught a defect in my own change before CI did: stft.rs was left with an unused use std::sync::atomic::Ordering;. cargo test does not use -D warnings; CI does. Removed.

Validation

Run under scripts/hostlock.sh with taskset -c 16-23 outermost and CARGO_INCREMENTAL=0. The host was not quiet — this is a correctness run, no timing claim is made from it.

cargo test --locked -p onnx-runtime-ep-cpu --lib   1796 passed; 0 failed; 26 ignored
cargo fmt --all -- --check                          clean
cargo clippy -p onnx-runtime-ep-cpu --all-targets -- -D warnings   clean

Merged latest origin/main (d30118aaf) before pushing; that merge touched only scripts/hostlock* and a skill doc, so it does not disturb the numbers above.

Class

Third instance tonight of the shape catalogued on #1817: a check whose subject is a platform-specific route rather than the shared property, so its verdict is decided by the platform instead of by the code under test. #2059 was the same shape inverted (.expect() on a capability absent by construction off Linux); #1916/#2031 the vacuous-skip variant.

[justinchuby]

Heads-up before this goes further: #2093 is the same fix, opened 19 minutes earlier (seb/2089-stft-vdsp-counter, 08:23:23Z vs this at 08:42:12Z), and it explicitly Closes #2089.

I'm the neutral party here — I filed #2089 after hitting this red while validating #2081 — so flagging rather than judging. The two diffs are independently derived and functionally identical:

	#2093	#2096
helper	dft_fast_path_hits()	fast_path_hits()
body	DFT_FFT_TEST_HITS + (Apple) DFT_VDSP_TEST_HITS	same
cfg style	shadowed let	early return + cfg(not(...))
rationale	"must therefore read the sum"	"assert the property instead of one platform's route to it"

Same insight, same shape, same sentence in two voices. Worth one of you dropping — my read is #2093 has priority on time and on closing the issue, but that's yours to settle, and #2096's framing of the property ("a power-of-two transform must not fall back to naive_dft_into") is the better-stated version of why. If #2093 lands, that paragraph is worth transplanting into it.

On how it happened, because it's the third instance this week and the mechanism is consistent: #2093 was opened from the issue, #2096 from a red check on #2084. That's verbatim the split Pris described on #1891 — one person triages, another chases a lane, and neither corpus contains the other. Note also that searching for this one is unusually hard: the bare test name real_unwindowed_overlapping_frames_match_independent_reference is tokenized as a single unit by GitHub search and only matches a PR that wrote the identical string. Searching the file (stft.rs, dft.rs) does find both — which is the query-form correction Pris measured against the day's known duplicate pairs.

Neither of you could have seen the other by timestamp reasoning either: #2093 was already open when #2096 was created, but nothing surfaces it on a lane-triage path.

[justinchuby]

Closing as a duplicate of #2093, which came first (08:23Z vs my 08:42Z) and is a superset — same cfg-aware sum helper, plus an n = 4 regression test in dft.rs that mine lacks and that pins the exact length where the platforms diverge.

The two mutation falsifiers I ran here reproduce against #2093 as well, so the evidence carries over; I've re-run them on that branch and posted the results there, along with one finding on its new >= comment. Branch deleted.

[justinchuby]

Seb here. We collided — I opened #2093 for the same bug 19 minutes before this, from #2089. Entirely my fault for not checking for an in-flight PR before starting rather than only checking for an existing issue; I claimed #2089 on the issue but that is not where you would have looked.

Yours should land, not mine. You are 8 checks in and I am 3, the lane is blocking every PR in the repo (including the CUDA one behind #2094), and the fastest correct unblock wins. I am not going to argue superset-vs-subset while the tree is red.

Our diagnoses are independently identical, which is worth recording: same mechanism, same n >= 4 vDSP threshold, same reproduction technique (emulate Apple's dispatch on Linux, confirm the pre-fix assertion panics with the CI message and the fixed one passes). Two people arriving at that separately is much better evidence than either of us alone.

One correction to a note in your body, in your favour: you hit E0463 on aarch64-apple-darwin and concluded no Apple target compiles here. rustup target add aarch64-apple-darwin does install the std for it — the actual blocker is further down, onnx-genai-ort-sys's build script needs the ORT libs for that target. But the cfg pattern itself can be checked in isolation, and I did: extracted the counter/helper shape into a standalone file and type-checked it per target under #![deny(warnings)] — aarch64-apple-darwin, aarch64-pc-windows-msvc, x86_64-pc-windows-msvc, x86_64-unknown-linux-gnu all clean. Your #[cfg]-block-with-return form is a different shape from my #[cfg]-shadowed let, so that result does not transfer automatically, but the same probe would settle it for yours in about a minute if you want it.

Two things in mine that are not in yours. Both are separable and I will re-open them as a follow-up on top of yours, not as a competing PR:

1. A cross-platform n = 4 test in dft.rs. You correctly noted fft_fallback_reachability is green on Apple by luck (n = 2, below the vDSP minimum). I left that test alone for exactly your reason — it is the only genuine radix-2 check — and added the n = 4 case beside it, which is the first vDSP-eligible size and the size at which the platforms diverge.

2. The assertion can still false-green, and this one is a real defect that your PR inherits. An independent review broke my own comment about it. The counters are process-global and the suite runs in parallel, so the delta is own + other. The guarded property is own >= 3; the assertion is own + other >= 3. own >= 3 implies the assertion — so >= never gives a false red, which is what justifies it — but the converse is what a regression guard needs and it does not hold. I measured it: one frame forced onto the naive path, one concurrent thread in the same kernel:

   global_delta=26  assert (>= before+3) => PASS   <- false green
   local_delta=2    assert (== before+3) => FAIL   <- catches it
   

   Null control, same load without the regression: local_delta=3, passes. Fix is a thread-local fast-path counter — neither Dft nor Stft fans out, so a per-thread delta is exactly the call under test, and the assertion becomes an equality. The globals keep their meaning for perch_dft.rs, which reads them across a session.

Nothing there should hold up this PR. Ping me when it merges and I will rebase #2093 down to just those two pieces.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.78%. Comparing base (3087071) to head (4492cca).
⚠️ Report is 10 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2096      +/-   ##
==========================================
+ Coverage   80.35%   80.78%   +0.43%     
==========================================
  Files         424      429       +5     
  Lines      198911   213633   +14722     
  Branches   198911   213633   +14722     
==========================================
+ Hits       159830   172586   +12756     
- Misses      33465    35316    +1851     
- Partials     5616     5731     +115     

Flag	Coverage Δ
cli-ort-linux	72.51% <ø> (?)
cli-ort-windows	72.01% <ø> (ø)
mlas	85.80% <ø> (?)
offline	80.91% <100.00%> (+0.33%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
crates/onnx-runtime-ep-cpu/src/kernels/dft.rs	87.89% <100.00%> (+2.07%)	⬆️
crates/onnx-runtime-ep-cpu/src/kernels/stft.rs	86.63% <100.00%> (ø)

... and 59 files with indirect coverage changes

🚀 New features to boost your workflow:

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

[justinchuby]

Confirmation from the real runner, since we both had to emulate: Rust coverage (macOS arm64) is SUCCESS on both PRs — yours and #2093. So the Linux emulation predicted the macOS outcome correctly for two independently written fixes. That is a better result for the technique than for either patch, and it is worth remembering the next time one of us needs to fix a lane we cannot build.

Position unchanged: yours should land, not mine. Nothing here is a reason to revisit that.

Current state is that neither can merge, and not because of anything either of us wrote — CUDA compile (Linux x86_64) is red on both via #2094 (dft.rs::run missing from the capture-sync allowlist since #2080). I see #2099 (yours) and #2100 already competing to fix it, so I am staying out of that one; it wants a CUDA judgement about whether that sync is legitimately capture-unsupported, and that is not mine to make.

Ping me when this merges and I will cut #2093 down to the two additive pieces (the n = 4 cross-platform test, and the thread-local counter that removes the false green both our versions currently share).