Skip to content

ci: restore a red main — refresh the conformance negative control and formatting - #1578

Merged
justinchuby merged 4 commits into
mainfrom
fix/ci-restore
Aug 20, 2026
Merged

justinchuby merged 4 commits into
mainfrom
fix/ci-restore

Conversation

@justinchuby

@justinchuby justinchuby commented Aug 20, 2026 •

Copy link
Copy Markdown
Owner

main's two required checks are both red, and neither failure is a real finding. This restores them.

1. Rust quality + Fast: formatting drift

cargo fmt --all --check failed on onnx-runtime-ep-cuda/src/optimizer.rs, then on kernels/normalization.rs and matmul_nbits.rs. Ran cargo fmt --all; no logic touched.

2. EP conformance: a negative control that went stale

The harness keeps one real ONNX op the EP declines, to prove the decline path still works. That op was Celu — and the CPU EP has since grown a Celu kernel, so the control started reporting expected UNSUPPORTED, got PASS.

The control was working, not failing. It detected exactly what it exists to detect; nothing is wrong with the EP. Moved it to Det (verified: reference evaluates to -2.0, runner returns rc=2 UNSUPPORTED_OP … no handler for ai.onnx::Det), and added a hint so the next person to hit this reads "your EP grew a kernel" instead of "conformance is broken".

3. Fast: the MLAS-free gate could not run its probe

Unmasked once formatting was fixed — it had never been reached.

The gate asks cargo metadata --offline for the whole workspace resolve, and by design fails loud rather than skipping when it cannot read one (a probe that cannot run must not report a pass — correct design). But the Linux lanes build a subset of the workspace, so only that subset's packages reach the registry cache. base64 v0.13.1 is reachable only through tokenizers, which no subset crate builds.

Result: an --offline download failure for a crate with no connection to MLAS turned the gate red.

Verified causally, not by inference — evicted exactly base64-0.13.1 from the local registry:

probe result
cache complete rc=0, 7/7 pass
base64-0.13.1 evicted rc=101, byte-identical to the CI failure
after cargo fetch --locked rc=0, downloads exactly that crate

Added cargo fetch --locked to both Linux lanes. This keeps the probe hermetic and the gate strict — the alternative (letting an unrunnable probe skip) would convert a required merge gate into no gate at all.

The gate's logic and strength are unchanged. I only added the remedy to its failure message, and confirmed both directions: it still fails loud when it cannot run, and it now names the cause.

Scope

#1173's MLAS research work is not otherwise touched — no gate logic, thresholds or coverage changed.

Verification

  • default_artifacts_are_mlas_free 7/7 · hint fires only on the offline-download failure
  • python -m unittest conformance.test_run_onnx_tests 5/5 · new control end-to-end status: UNSUPPORTED, failures: []
  • cargo fmt --all --check clean · clippy clean · ci.yml parses

justinchuby and others added 3 commits August 20, 2026 14:59
… formatting

Two independent failures were making `Rust quality`, `Fast` and
`EP conformance` red on main, so every PR inherited them.

EP conformance reported `PASS=29 UNSUPPORTED=0 MISMATCH=0 ERROR=0` and then
exited 1 on `SyntheticUnsupportedCelu: expected UNSUPPORTED, got PASS`. The
harness keeps one live negative control — a real op this EP declines — so that
a broken decline path cannot pass silently. The CPU EP has since grown a
`Celu` kernel, so the control was asserting something no longer true. That is
the control working, not failing. Moved to `Det`, verified to exit 2 with
`UNSUPPORTED_OP` against the real runner while `onnx.reference` still
evaluates it, and chosen because a linear-algebra op sits well outside this
EP's dispatch and activation roadmap. A stale control now says so in the
failure text instead of reading like a regression.

Formatting drifted in `onnx-runtime-ep-cuda` and `onnx-runtime-ep-cpu`;
`cargo fmt --all` restores it.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@codecov

codecov Bot commented Aug 20, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.77%. Comparing base (70db811) to head (9fae9d2).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##             main    #1578       +/-   ##
===========================================
- Coverage   82.64%   80.77%    -1.88%     
===========================================
  Files          12      381      +369     
  Lines        5475   173645   +168170     
  Branches     5475   173645   +168170     
===========================================
+ Hits         4525   140265   +135740     
- Misses        757    28490    +27733     
- Partials      193     4890     +4697     
Flag Coverage Δ
cli-ort-linux 82.60% <ø> (ø)
cli-ort-windows 82.19% <ø> (ø)
mlas 85.09% <ø> (?)
offline 80.63% <ø> (?)

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

The MLAS-free merge gate asks `cargo metadata --offline` for the whole
workspace resolve, and by design fails loud rather than skipping when it
cannot read one — a probe that cannot run must not report a pass.

The Linux lanes build a subset of the workspace, so only that subset's
packages reach the registry cache. `base64 v0.13.1` is reachable only
through tokenizers, which no subset crate builds, so the probe hit an
`--offline` download failure and turned the gate red for a reason with no
connection to MLAS. Verified by evicting exactly that crate locally: the
probe reproduces the CI failure, and `cargo fetch --locked` repairs it.

Fetching the locked graph first keeps the probe hermetic and the gate
strict. The gate's own logic is unchanged; it only names the remedy now,
since an unrelated crate failing to download reads as a mystery.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@justinchuby
justinchuby merged commit b77e3d3 into main Aug 20, 2026
10 of 16 checks passed
@justinchuby
justinchuby deleted the fix/ci-restore branch August 20, 2026 15:39
justinchuby added a commit that referenced this pull request Aug 20, 2026
## Problem

`cargo fmt --all --check` is **red on `origin/main`** (`b77e3d3fc`), so
every open PR fails the fmt gate on code it did not touch.

#1578 repaired
`crates/onnx-runtime-ep-cuda/src/kernels/normalization.rs` but left two
rustfmt diffs in the *same crate's* `matmul_nbits.rs`:

- `:19596` — a `.map().collect()` chain that fits on one line
- `:19753` — a stray blank line before a closing brace

## Fix

`cargo fmt --all`. One file, +1/-4, formatting only, no functional
change.

## Verification

```
$ git log --oneline -1
b77e3d3 ci: restore a red main ... (#1578)
$ cargo fmt --all --check    # before
Diff in .../ep-cuda/src/kernels/matmul_nbits.rs:19596
Diff in .../ep-cuda/src/kernels/matmul_nbits.rs:19753
exit 1
$ cargo fmt --all && cargo fmt --all --check    # after
exit 0
```

This is the second time this session that a red-on-main fmt gate has
blocked unrelated PRs (see #1567). Both were introduced by changes that
landed without the fmt gate passing.

Co-authored-by: Roy <roy@squad.local>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
justinchuby added a commit that referenced this pull request Aug 20, 2026
main advanced past this branch's base (#1578, #1580, #1583..#1585, #1588,
#1491, #1571, #1592). 23 files changed on main, 3 of them also touched here.

Conflict resolution, and the audit behind it:

  kernels/matmul_nbits.rs -- taken from main wholesale. This branch's blob
  is byte-identical to main's blob at 3542ae7 (#1580): the only "change"
  on our side was restoring main's probe after an earlier merge dropped it
  (the Tier 2 audit). main has since carried that same file forward through
  #1584/#1585, so our content is a strict ancestor of main's and taking
  main loses nothing. Verified by blob hash, not by reading the diff.

  .github/workflows/ci.yml -- auto-merged. Result equals main plus the four
  `-p onnx-runtime-memory-api` package selections this stack adds. main's
  two new `cargo fetch --locked` steps are both present (counted).

  pipeline/decoder_component.rs -- auto-merged. Result equals main plus the
  two `ProcessMemoryManager` lines this stack adds. main's #1592 additions
  (supports_argmax, step_argmax, decode_argmax_with_step_inputs,
  captured_step_input_greedy_supported) all survive at main's reference
  counts.

The other 20 files main changed are byte-identical to origin/main in the
merge result, checked by blob hash for every one rather than by eye.

Verified: cargo check --workspace --all-targets and
cargo check -p onnx-genai-engine --features cuda,native-backend clean;
cargo fmt --all --check clean; onnx-genai-engine native-backend suite
559 passed / 2 failed / 1 ignored, the two failures being the same macOS
statvfs pair that fails on main and is fixed by #1586.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: c80f8522-983c-47f7-8241-2155a823aabe
justinchuby added a commit that referenced this pull request Aug 20, 2026
## Summary

The seven failed CI jobs reduced to four independent baseline defects:

1. Nested offline Cargo metadata resolved the full graph and required
registry packages absent from CI caches. The MLAS exclusion gate now
uses dependency-free metadata and resolves workspace feature activation
deterministically in memory.
2. The ARM SIMD log sweep treated zero numerical error as proof that no
samples ran. Exercise accounting is now independent of error magnitude,
with direct AVX2 and pure ARM/NEON sample-generation coverage.
3. The CELU unsupported sentinel became stale after CELU support landed.
Latest `main` now supplies the replacement `Det` negative control via
#1578, so the redundant PR commit was dropped during rebase.
4. A source-structure test assumed LF line endings. It now strips
test-only items structurally, normalizes CRLF/LF input, and covers CRLF
explicitly.

The PR also retains the original CUDA CI repairs:

- classify only genuinely CUDA-bound integration targets in the honesty
checker
- make GPU-enabled Marlin numerics fail loudly when CUDA initialization
is unavailable
- resolve Rust 1.97 CUDA Clippy/rustfmt failures and remove the
duplicate split-K test attribute

## Validation

Validated after rebasing onto `main` at `0faa6354`:

- `cargo fmt --all -- --check`
- CPU EP, CPU plugin, and EP plugin test suites
- relevant CPU/plugin/session Clippy with `-D warnings`
- CUDA test suite: 451 passed, 25 ignored
- CUDA Clippy with `-D warnings`
- CUDA honesty with CUDA hidden: 433 tests / 68 targets; 425 fail-loud,
8 ignored, 0 false passes
- conformance harness self-tests: 5 passed
- full EP conformance: 28 PASS, 1 expected UNSUPPORTED, 0 mismatch/error

All commits are signed off.

---------

Signed-off-by: Justin Chu <justinchuby@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 39ff6824-d35f-4d3b-8f5f-043a7119a100
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant