[justinchuby]

What

decode_blocktime() reads ONNX_GENAI_CPU_DECODE_BLOCKTIME_US into a OnceLock and had no test at all. Splits the policy into a pure parse_decode_blocktime(Option<&str>) -> Duration and adds three mutation-verified tests. No behaviour change — same accepted spellings, same fallback.

Why now

Prompted by @roy's #1736 finding, where half_decode_gemv_enabled() caches in a OnceLock, so a test doing run(true) then run(false) latches on the first call and both arms execute the same route — an A/B that has been asserting a thing equals itself.

decode_blocktime() is the same shape, one step earlier: the latch is there, the vacuous test simply hasn't been written yet. And it is precisely the knob a gap-aware park/wake harness has to sweep, so the vacuity would have landed exactly where it does the most damage — on the measurement it exists to enable.

A pure function cannot degrade that way. It also keeps set_var out of tests entirely: cargo test runs a binary's tests on parallel threads, and set_var racing another thread's getenv is a data race Rust 2024 forbids outright. Same reasoning as the existing parse_host_copy_threshold / resolve_persistent_decode_threads split.

The load-bearing case is 0

The sibling knob steal_tiles_per_worker does .filter(|&value| value > 0) and treats zero as unset. Blocktime must not: 0 is the one setting that makes a worker park without spinning — the maximally-polite mode used to measure park/wake latency. Folding it into the 500 µs default would silently give every such run a spin window and make the measurement impossible while still appearing to work.

Nothing pinned that before this PR. A future edit copying the sibling's .filter() would have been a plausible, review-passing change.

Mutation results

Each test verified to fail under its own defect and no other — three mutants, three distinct kills, no cross-talk:

mutation	test killed
.filter(|v| v > 0)	an_explicit_zero_blocktime_parks_immediately_rather_than_taking_the_default
Duration::from_millis for from_micros	a_blocktime_is_read_as_microseconds_and_tolerates_surrounding_space
unparseable defaults to Duration::ZERO	an_unset_or_unparseable_blocktime_uses_the_default

Rejected spellings covered: "", " ", "abc", "-1", "1.5", "500us", and u64::MAX + 1. The unit is pinned against both a 1000x-up and a 1000x-down misreading, either of which every other assertion would have passed.

Validation

• cargo fmt --check clean
• clippy --all-targets -D warnings clean, and for aarch64-unknown-linux-gnu
• 1633 EP lib tests pass (3 new)
• Single file changed; no unsafe touched

Refs #1736

[justinchuby]

Adversarial review (Opus) — verdict

Nothing BLOCKING. The reviewer verified the "no behaviour change" claim empirically rather than by inspection, by extracting old and new logic into a standalone harness and running every input class. Zero mismatches, including classes I had not enumerated ("+500", "00500", u64::MAX exactly, and non-UTF8 env values where VarError::NotUnicode and NotPresent must collapse identically — they do).

Also confirmed: the string literal appears nowhere else in the tree (const definition plus one doc mention), character-for-character identical, so there is no silent-disable typo; Duration::from_micros(u64::MAX) does not panic; and the mutation matrix is a clean diagonal.

One NIT, fixed — and it was a genuine defect

assert_eq!(parse_decode_blocktime(Some("500")), DEFAULT_BLOCKTIME);

I had written this thinking it pinned "an explicit value equal to the default still parses". It does not, and cannot: the two outcomes are indistinguishable by construction. It passed for a parser that always returns the default — the reviewer confirmed that mutant is actually caught by the "250" assertion in the same test, not by this line.

Worse than useless, it was fragile in the dangerous direction: it coupled the test to the numeric value of DEFAULT_BLOCKTIME, so retuning the default to 600 µs — a change this file exists to enable, and one I expect to make for the park/wake work — would have failed a correct parser. A test that fails a correct implementation is worse than no test; it teaches people to edit tests to make them pass.

Same class as @roy's recycled-address falsifier in #1738, which he demoted from an assertion to a report because under ASan/valgrind/musl it would have failed a correct implementation. Different axis, identical failure mode.

Removed. Mutation matrix re-verified after removal, and no coverage was lost:

mutation	killed by
.filter(|v| v > 0)	an_explicit_zero_blocktime_parks_immediately...
from_millis for from_micros	a_blocktime_is_read_as_microseconds...
unparseable → Duration::ZERO	an_unset_or_unparseable_blocktime_uses_the_default
always returns DEFAULT_BLOCKTIME	the first two (2 kills)

Still diagonal for the three targeted mutants, still catching the blanket one.

I did keep the redundant assert_ne! unit-pinning lines. The reviewer correctly notes they are logically implied by the preceding assert_eq! — they cost nothing and name the 1000x hazard for the next reader, which the assert_eq! alone does not.

Gates (re-run after the fix)

fmt --check clean · clippy --all-targets -D warnings clean · aarch64-unknown-linux-gnu clippy clean · 1633 EP lib tests pass · single file, no unsafe touched

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.20%. Comparing base (4738086) to head (5876267).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1773      +/-   ##
==========================================
- Coverage   80.59%   80.20%   -0.39%     
==========================================
  Files         411      411              
  Lines      199146   199176      +30     
  Branches   199146   199176      +30     
==========================================
- Hits       160495   159751     -744     
- Misses      33208    33982     +774     
  Partials     5443     5443              

Flag	Coverage Δ
cli-ort-linux	72.47% <ø> (ø)
cli-ort-windows	71.97% <ø> (-0.10%)	⬇️
mlas	85.20% <ø> (-0.14%)	⬇️
offline	80.33% <100.00%> (-0.41%)	⬇️

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_spmd.rs	89.22% <100.00%> (+0.35%)	⬆️

... and 11 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.