test(ep-cpu): report the leader-cpuset premise instead of inferring it from a check its failure satisfies

[justinchuby]

Rescoped. This opened as a fix for main's two red Windows lanes; #2078 landed the same platform gate ~20 minutes earlier and I merged it in rather than compete with it. Its skip message and idiom are kept verbatim. What remains is the part #2078 does not cover — and one sentence in it that I can show is false.

The claim I am correcting

#2078's comment on the skip reads:

The allowed == cores guard below would then fail for the platform rather than for a defect.

That guard does not fail when the restriction is absent. It passes. It is satisfied by the exact failure it exists to catch.

restrict_self_to_leader_cpus gives up silently when the topology is unreadable, and the child's cores then falls back to allowed:

let cores = crate::core_topology::require_host_for_placement()
    .map_or(allowed, |topology| /* ... */);   // <- fallback is `allowed`

So on an entirely unrestricted process the two are trivially equal. Measured, with the narrowing suppressed and the child's topology read forced to its fallback:

RealizedWidth { allowed: 8, cores: 8, restricted: false, workers: 4, ... }

allowed == cores passes there. What follows is worse than a vacuous pass — it is a misattribution: the run fails at workers == cores and blames #1780, a resolver defect, on a host where the resolver was correct and the premise was never established. On a cpuset that already holds one CPU per core, it passes outright.

green run  -> tells you nothing
red run    -> points at the wrong file

The fix

The premise is reported by the process that established it, not inferred from a consequence. The child prints restricted=, and the parent requires it:

• NXRT_REQUIRE_PLACEMENT_TESTS=1 → fail, naming the three causes (unreadable allowed set, unreadable topology, leaderless cpuset);
• otherwise → skip with a stated cause, in fix(ep-cpu): skip the leader-CPU width test where the cpuset cannot be built #2078's SKIP <test>: idiom.

That is the same fail-closed/skip split require_host_for_placement and #1916's DETECTION_SUPPORTED already draw, and it is why required is a parameter rather than a global read.

The allowed == cores check stays, with its comment corrected: it still catches a leader set that is not one CPU per core on a host that answered, which is a different fault.

Second change: one spelling of the platform fact

#2078 spells it cfg!(target_os = "linux") inline, in two places. This replaces both with decode_affinity::PROCESS_AFFINITY_MASKING_SUPPORTED, next to the two cfg arms it describes, for the reason DETECTION_SUPPORTED is a constant: a caller must be able to tell "this platform never had the capability" from "the call failed here" without making the call — and when Windows process-wide masking does land, there is one place to change instead of a grep.

A constant that lies about a capability is worse than no constant, so the_masking_capability_constant_agrees_with_what_this_platform_does asserts it against the implementation that actually compiled, on every lane. It runs on a spawned thread and re-applies the process's own mask, so it cannot leak an affinity into whatever test the runner schedules on that thread next.

Evidence

Mutation results, not readings. All runs taskset -c 16-23, CARGO_INCREMENTAL=0, under scripts/hostlock.sh with a stated reason. The host was shared throughout — no claim of a quiet machine is made or needed, since none of these are timings.

#	mutation	expected	observed
baseline	none (merged with origin/main)	green	1796 passed / 0 failed
F1	narrowing suppressed + topology unreadable, with this PR	fails under required mode	FAILED: NXRT_REQUIRE_PLACEMENT_TESTS=1 but the child could not narrow itself to a leader-only cpuset
F1b	same, against the pre-#2078 test	shows the blind spot	allowed == cores passed; the run then failed blaming #1780
F2	PROCESS_AFFINITY_MASKING_SUPPORTED = false	agreement test fails; width test skips	agreement test FAILED (says false but re-applying this process's own mask returned ok=true); width test skipped, stated cause
F3	const false and the Linux impl forced to the non-Linux Err — a faithful emulation of a Windows lane	stated skip, green	2 passed, SKIP line printed
F3b	same emulation against the pre-fix test	reproduces the CI failure	FAILED: restrict the child to leader CPUs: "process-wide CPU affinity masking is only implemented on Linux (no-op)" — byte-identical to the Windows ARM64 log

F3b was how I confirmed the Windows diagnosis before #2078 was visible to me; it is kept because it is also the falsifier for the constant, which is new here.

Also: cargo fmt --all -- --check clean, cargo clippy --locked -p onnx-runtime-ep-cpu --all-targets -- -D warnings clean, scripts/check_cross_compile.sh green at full scope (full offline set (aarch64 cross toolchain present)) — not the FFI-free subset it silently falls back to without the cross toolchain.

Species sweep

Three call sites reach set_current_thread_affinity outside its own module. The other two already do the right thing: production code at matmul_nbits.rs:4379 logs the Err and carries on, and the budget-lane child at decode_spmd.rs:8772 prints a skip marker with the reason — "Only Linux implements a process-wide mask, so on other hosts there is no way to manufacture the reduction." That is this shape, one file over, written before it. No other caller .expect()s the capability.

Limits

• No Windows target compiles locally: ep-cpu → ep-api → ort-sys, whose build script bindgens the ORT headers, so --target x86_64-pc-windows-msvc dies in the build script before rustc sees this crate; scripts/check_cross_compile.sh documents the same Windows exclusion. F3/F3b emulate the platform's behaviour on Linux; only this PR's Windows lanes can confirm the compile.
• main's third red lane, CLI ORT (Linux x86_64), is unrelated and untouched here: plugin_ort_e2e::initializer_chain_still_fuses_into_one_claim fails with Only one instance of LoggingManager created with InstanceType::Default can exist at any point in time — already tracked as flaky: ORT LoggingManager singleton fails CreateEnv in plugin_ort_e2e as the binary's Env count grows #2065 (and cpu-plugin: ARM64 plugin_ort_e2e failures are one LoggingManager race, not per-test regressions #1123 on ARM64). Checked before saying so: every #[test] in that file that creates an OrtEnv holds ORT_EP_LOCK, none via a let _ = that would drop the guard immediately, and the binary spawns no threads; tests after the failure created their own Env and passed, which rules out a leaked one.

🤖 Working as Gaff (Code Reviewer / Quality).

[codecov]

Codecov Report

❌ Patch coverage is 75.00000% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.73%. Comparing base (d30118a) to head (7a58856).
⚠️ Report is 41 commits behind head on main.

Files with missing lines	Patch %	Lines
...es/onnx-runtime-ep-cpu/src/kernels/matmul_nbits.rs	65.21%	8 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2084      +/-   ##
==========================================
+ Coverage   80.54%   80.73%   +0.18%     
==========================================
  Files         414      429      +15     
  Lines      199542   214775   +15233     
  Branches   199542   214775   +15233     
==========================================
+ Hits       160715   173388   +12673     
- Misses      33339    35620    +2281     
- Partials     5488     5767     +279     

Flag	Coverage Δ
cli-ort-linux	72.51% <ø> (?)
cli-ort-windows	72.01% <ø> (?)
mlas	85.90% <ø> (?)
offline	80.85% <75.00%> (+0.31%)	⬆️

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/decode_affinity.rs	94.13% <100.00%> (-0.41%)	⬇️
...es/onnx-runtime-ep-cpu/src/kernels/matmul_nbits.rs	80.60% <65.21%> (+0.33%)	⬆️

... and 75 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]

Status: the only two red lanes here are inherited from main, and both now have fixes open.

lane	cause	fix
CUDA compile (Linux x86_64)	#2080 added an unconditional stream().synchronize() to dft.rs::run without listing it in the capture-sync contract. Set difference is exactly one entry.	#2099
Rust coverage (macOS arm64)	#2083's STFT test asserts on the radix-2 counter, which Apple targets never reach for n >= 4 — false by construction there.	#2097

Neither is caused by anything in this branch: this PR touches neither the CUDA crate nor the DFT/STFT kernels. I verified the CUDA one directly — the contract test is a pure source scan, so it reproduces locally without --features cuda and without a GPU, byte-identically to CI's left/right sets.

No action needed here beyond a re-run once those land.

— Gaff

[justinchuby]

Merged latest origin/main (c00aeb9c1) and revalidated. One conflict, in matmul_nbits.rs, where main's a_default_width_pool_on_leader_cpus_uses_every_core_it_was_given grew a #[cfg_attr(not(target_os = "linux"), ignore = …)] and a cfg!(target_os = "linux") runtime arm.

Resolved by keeping main's prose and this PR's constant. Main's comment is better than mine and explains the belt-and-braces structure — ignore is the visible signal in a default log, the runtime arm is the belt for --ignored runs — so I took it verbatim. The predicate is the one line I changed back, because it is the entire point of this PR:

if !crate::decode_affinity::PROCESS_AFFINITY_MASKING_SUPPORTED {

A bare cfg!(target_os = "linux") re-spelled at each site is checked by nothing; the constant is checked against what the platform actually does by the_masking_capability_constant_agrees_with_what_this_platform_does. Two sites now read it (restrict_self_to_leader_cpus and this skip arm), and neither can drift from set_current_thread_affinity's real implementation without that agreement test failing. I added a comment saying exactly that, so the next merge doesn't quietly reintroduce the cfg!.

cargo test --locked -p onnx-runtime-ep-cpu --lib   1797 passed; 0 failed; 26 ignored
   (NXRT_REQUIRE_PLACEMENT_TESTS=1)
cargo clippy --locked -p onnx-runtime-ep-cpu --all-targets -- -D warnings   clean
cargo fmt --all -- --check                                                  clean

CUDA compile (Linux x86_64), one of the two inherited reds, is fixed on main as of #2099 (c00aeb9c1) and this branch now carries the fix. The macOS lane is #2097, which is already green on Rust coverage (macOS arm64).

— Gaff

Run under scripts/hostlock.sh with taskset -c 16-23 outermost and CARGO_INCREMENTAL=0; host not quiet, and every result above is pass/fail.

[justinchuby]

Self-correction: the Root file allowlist failure on this PR was mine, and it is the dumbest possible version of the thing I spend my time flagging.

Resolving the origin/main merge, I staged the resolution with git add -A — which swept my untracked .review/ scratch directory into the merge commit. The repo has a gate for exactly that, the gate did its job, and I'd have shipped review notes into the tree without it. Removed in the follow-up commit; git ls-files | grep '^\.review/' is now 0.

Two things worth saying rather than quietly fixing.

The gate caught what my local validation could not. I ran the full lib suite, clippy -D warnings, and fmt --check on the merged tree and all three were green — none of them looks at what is tracked. I had verified the code and not the commit. That is a scope error on my part, not a tooling gap: git status after git add -A would have shown it in one line, and I didn't look because I was reading the conflict, not the index.

I am not adding an exclude rule, deliberately. The obvious fix is .git/info/exclude, but in a worktree that resolves to /workspace/dev/onnx-genai/.git/info/exclude — the shared git directory, which every agent's worktree and the user's primary checkout read. Silently changing what everyone else's git status hides, to paper over my own mistake, would be a worse trade than the mistake. The correct fix is to stop using git add -A for conflict resolution and name the files, which is what I'll do.

No change to the substance of this PR; the merge resolution and its validation stand as posted above.

— Gaff

[justinchuby]

Closing this as superseded by #2034, which landed the same fix independently and better-argued. Verified by merging latest main into this branch and diffing, not by reading the description.

#2034 has, in matmul_nbits.rs:

• narrowed_to_leaders: bool on the child report (:20076), parsed from the marker line as field("narrowed") == 1 (:20686) and asserted (:20906) — this PR's restricted: bool, same mechanism.
• AFFINITY_MASKING_SUPPORTED in decode_affinity.rs:645 — this PR's PROCESS_AFFINITY_MASKING_SUPPORTED, derived the same way (cfg!, not a runtime call), and scoped better: #[cfg(test)] pub(crate) rather than widening the crate's public API for a test-only fact.

Its doc comment also states the core finding more sharply than mine did:

A separate field rather than an inference from allowed == cores, because that equality is also what the arm asserts: deriving the precondition from the conclusion is how a check comes to confirm itself.

On the one assertion where we differed, #2034 is right and I defer. Mine asserted bidirectional agreement between the flag and the call; #2034 asserts only the fail-open direction (!succeeded || AFFINITY_MASKING_SUPPORTED) and says why the converse is excluded — it would false-fail on a CPU hot-unplugged between reading the mask and re-applying it, and that direction is already loud because callers panic rather than skip. My version would have been flaky for a fault that is not the one being guarded.

One thing survives, and it is small: #2034's version of that test skips silently on allowed_cpus() == None. Salvaged to #2119 — eight lines, log-only.

Third duplicate I've hit in ~24h (#2096/#2093, #2097/#2093, now this). The common factor is that all three pairs were opened within an hour of each other against a red or newly-changed area, so the cost is real but the cause is contention, not carelessness. Not proposing process here; noting it on #1817.