fix(engine): make main green — fmt, dead code, and three 1.98.0 lints from #1637/#1641 by justinchuby · Pull Request #1640 · justinchuby/onnx-genai

justinchuby · 2026-08-21T08:03:08Z

What

main is red. This makes it green. Four independent defects from #1637 (6e359012) and #1641 (81fc0060), across five jobs.

#	defect	where	jobs it reddens
1	`unused import: MtpProposer`	`engine/mod.rs:63`	`CLI ORT (Linux)`, `CLI ORT (Windows)`
2	9 fmt diffs	`engine/load.rs` (3, #1637), `speculative/mod.rs` (6, #1641)	`Fast (Linux)`, `Rust quality`
3	`dead_code` on `DraftProjectionDevice`	`speculative/mod.rs:342`	`CLI ORT` (default features)
4	`collapsible_if` ×5 + `manual is_multiple_of`	`load.rs:389`, `speculative/mod.rs:629`	`CUDA compile (Linux)`

The two that were not mechanical

collapsible_if ×5 — the obvious single let chain does not compile. The outer arm borrows metadata.speculative immutably; the inner arm takes metadata.model mutably. In the nested form NLL ends the shared borrow at its last use; in a let chain every binding stays live to the end, so the two borrows overlap and borrowck rejects it. I resolved the speculative side into an owned Option<String> first — which ends that borrow — then chained the mutable half. Same control flow, same result, and a comment records why it cannot be folded further.

manual is_multiple_of — k % k_blocks != 0 → !k.is_multiple_of(k_blocks). is_multiple_of(0) is self == 0 rather than a panic, so the semantics differ at zero; the existing k_blocks == 0 || short-circuit is deliberately kept ahead of it, so the zero case still bails and never reaches the call. Behaviour identical.

dead_code — DraftProjectionDevice's only non-test construction is in load.rs's load_native_mtp_proposer, which is #[cfg(feature = "native-backend")]. Under default features the lib target sees both variants as never constructed. Gated with cfg_attr, matching the treatment #1641 already gave the index field one level down — this is the same problem one level up.

⭐ #1632's lane earned itself here

Defect 4 fired on main as CUDA compile (Linux x86_64) / Clippy engine native-CUDA integration — run 32462970281. That step is the lane added in #1632 two hours earlier, and no other job compiles that code. Without it, those six errors would have sat in main unreported until someone built with native-cuda by hand.

That is a better argument for the lane than the injected-failure test I used to justify it: it caught a real regression in production, unprompted, within hours.

Verification

All under the pinned 1.98.0. Every failure reproduced on a clean origin/main first.

command	before	after
`clippy -p onnx-genai-cli --all-targets -- -D warnings`	101	0
`clippy -p onnx-genai-engine -F native-backend --all-targets -- -D warnings`	101	0
`clippy -p onnx-genai-engine -F native-cuda --all-targets -- -D warnings`	101	0
`cargo fmt --all -- --check`	101	0
`cargo test -p onnx-genai-engine -F native-backend --lib`	—	574 passed; 0 failed

Relationship to #1639

#1639 fixes defect 2's load.rs half only, and is currently CONFLICTING/DIRTY so it cannot merge as-is; its own CLI ORT (Linux) is red on defect 1, which it does not fix. This PR covers that file and the other three defects, so #1639 can be closed as superseded — that is its author's call, not mine, and I have said so on the PR.

⚠️ Note for whoever reads CI here: these defects mask each other. Rust quality runs Check formatting before clippy, so while fmt was red the clippy step never ran and never appeared in the failure list. Fixing fmt alone moves that job to failing at clippy — same job count. Judge by which step fails.

What I did not verify

I did not run the Windows legs. None of these lints is target-dependent, but I have not observed them clear there — CI is the oracle.
The native-cuda checks are compile coverage, not execution: no GPU is involved, so this says the code type-checks and lints, not that it runs correctly.
I did not audit the rest of Native MTP self-spec: land Gap 2 (hidden-seed shape) + Gap 1 wiring; block on int4 shared LM-head #1637/feat(mtp): on-GPU int4 draft LM-head projection + native MTP E2E wiring (recurrent-Scan blocker) #1641 for defects beyond what the five commands above reach.

codecov · 2026-08-21T08:14:33Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.05%. Comparing base (81fc006) to head (d26cdaa).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1640      +/-   ##
==========================================
- Coverage   81.05%   81.05%   -0.01%     
==========================================
  Files         384      384              
  Lines      180260   180260              
  Branches   180260   180260              
==========================================
- Hits       146115   146114       -1     
  Misses      29201    29201              
- Partials     4944     4945       +1

Flag	Coverage Δ
cli-ort-linux	`82.60% <ø> (ø)`
cli-ort-windows	`82.11% <ø> (-0.10%)`	⬇️
mlas	`85.09% <ø> (-0.10%)`	⬇️
offline	`80.93% <ø> (+<0.01%)`	⬆️

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

`Clippy onnx-genai-cli` fails on main (both the Linux and Windows legs of CLI ORT) with: error: unused import: `MtpProposer` --> crates/onnx-genai-engine/src/engine/mod.rs:63:59 = note: `-D unused-imports` implied by `-D warnings` The import arrived with #1637. Its only reader in this module tree is `runtime.rs`'s `generate_native_cold_with_callback`, which is `#[cfg(feature = "native-backend")]`; the import itself was unconditional. `native-backend` is opt-in and is not in onnx-genai-cli's default feature set, so the default build imports a name it never uses. Gate the import to match its use. Its siblings in that `use` stay ungated because each has uses that are not feature-dependent (load.rs, model.rs, runtime.rs) -- `MtpProposer` was the only one that did not. Verified under the pinned 1.98.0, both directions: # the failing CI step, on clean origin/main -> reproduces cargo clippy --locked -p onnx-genai-cli --all-targets -- -D warnings error: unused import: `MtpProposer` exit=101 # with this change exit=0 # and the feature that does use it still compiles cargo clippy --locked -p onnx-genai-engine --features native-backend \ --all-targets -- -D warnings exit=0 cargo clippy --locked -p onnx-genai-engine --features native-cuda \ --all-targets -- -D warnings exit=0 cargo test --locked -p onnx-genai-engine --features native-backend --lib 573 passed; 0 failed Does not touch the `Check formatting` failure also on main from #1637 (three diffs in engine/load.rs); #1639 covers that file. The two are independent, and both are needed -- Rust quality runs fmt before clippy, so fixing fmt alone moves that job to failing here instead. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 08760f2f-160f-41e5-828d-9d9b6045c00d

main is red in four ways from #1637 and #1641. The MtpProposer import (previous commit) was one; these are the rest. fmt (Fast Linux, Rust quality): 9 diffs -- 3 in engine/load.rs from #1637, 6 in speculative/mod.rs from #1641. `cargo fmt -p onnx-genai-engine`. dead_code (default features): `DraftProjectionDevice`'s variants are only constructed in `#[cfg(test)]` and in load.rs's `load_native_mtp_proposer`, which is `#[cfg(feature = "native-backend")]`, so the default lib target sees them as never constructed. Gated with cfg_attr, matching the treatment the `index` field already had one level down. collapsible_if x5 (load.rs:389): 1.98.0 policies `if let` chains. Not a mechanical collapse -- the outer arm borrows `metadata.speculative` immutably and the inner one takes `metadata.model` mutably, so a single let chain would keep the shared borrow alive across the mutable one and fail borrowck. Resolved the speculative side into an owned Option first, which ends that borrow, then chained the mutable half. manual is_multiple_of (speculative/mod.rs:629): `k % k_blocks != 0` -> `!k.is_multiple_of(k_blocks)`. The `k_blocks == 0` short-circuit is kept ahead of it, so the zero case still bails before any division. Verified under the pinned 1.98.0: clippy -p onnx-genai-cli --all-targets -- -D warnings exit=0 clippy -p onnx-genai-engine -F native-backend --all-targets exit=0 clippy -p onnx-genai-engine -F native-cuda --all-targets exit=0 cargo fmt --all -- --check exit=0 cargo test -p onnx-genai-engine -F native-backend --lib 574 passed; 0 failed The native-cuda line is the lane added in #1632, and it is what caught the collapsible_if and is_multiple_of errors: they fired on main as `CUDA compile (Linux x86_64) / Clippy engine native-CUDA integration` in run 32462970281. No other job compiles that code. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 08760f2f-160f-41e5-828d-9d9b6045c00d

`cargo fmt --all -- --check` fails on an unmodified `origin/main` (843b0bf): crates/onnx-genai-engine/src/native_decode/mod.rs:1115 crates/onnx-genai-engine/src/native_decode/tests.rs:1412 Formatting is a required check, so this blocks every open PR regardless of its contents. This is the second repair in a day. #1640 fixed the fmt and clippy debt from #1637/#1641; #1644 merged a few hours later and reintroduced fmt violations. The clippy gates (`native-backend`, `native-cuda`, and `onnx-genai-cli` at default features) are all clean on this commit, so formatting is the only outstanding gate and this change is deliberately scoped to it -- no overlap with #1640. Pure `cargo fmt --all` output: one function signature that now fits on one line, and one `.expect()` chain that no longer does. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…d_code #1640 cleared the four defects from #1637/#1641, but #1647 reintroduced the same two classes on `7ccdb920e`. Verified on a clean detached `origin/main` worktree, not inferred from CI: cargo fmt --all -- --check exit 1 native_decode/mod.rs:1115, native_decode/tests.rs:1412 clippy -p onnx-genai-engine --features native-backend ... exit 101 error: methods `set_retain_decode_graph_across_spec` and `retain_decode_graph_across_spec` are never used (cuda.rs:5516) The two accessors are already `#[cfg(test)]`, but the seam was landed ahead of the option-c tests that will drive it, so it has no caller in the `lib test` target either. Kept and marked `#[allow(dead_code)]` rather than deleted: the field docs state it is deliberately exposed for the option-c work. Verified: fmt exit 0; clippy exit 0 for default `-p onnx-genai-cli`, `--features native-backend`, and `--features native-cuda`; engine lib tests pass. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c80f8522-983c-47f7-8241-2155a823aabe

`cargo fmt --all -- --check` fails on an **unmodified `origin/main`** (`843b0bf7d`): ``` crates/onnx-genai-engine/src/native_decode/mod.rs:1115 crates/onnx-genai-engine/src/native_decode/tests.rs:1412 ``` Formatting is a required check, so this blocks *every* open PR regardless of its own contents. It surfaced on an unrelated CPU-kernel PR (#1628) whose own tree is clean. Pure `cargo fmt --all` output — one function signature that now fits on one line, one `.expect()` chain that no longer does. No hand edits. ### Scope, and why this does not overlap #1640 This PR originally carried a larger repair for the fmt + clippy debt from #1637/#1641. While I was validating it, **#1640 landed and fixed exactly that set**, so I reset this branch onto current main and reduced it to only what is still red. I verified the rest of the matrix is genuinely green on `843b0bf7d` rather than assuming #1640 covered it: | gate on current main | result | |---|---| | `cargo fmt --all -- --check` | **RED** — this PR | | `clippy -p onnx-genai-engine --features native-backend --all-targets -D warnings` | clean | | `clippy -p onnx-genai-engine --features native-cuda --all-targets -D warnings` | clean | | `clippy -p onnx-genai-cli --all-targets -D warnings` | clean | So formatting is the only outstanding gate, and this change is scoped to it. > Process note: this is the **second** fmt repair against main today. #1640 cleaned up #1637/#1641, and #1644 merged a few hours later and reintroduced violations in new files. Four required gates were red on main simultaneously this morning, and the clippy ones are only visible after formatting is fixed — so they surface one round-trip at a time on whichever unrelated PR happens to be open. A merge queue, or running the quality job on `main` post-merge, would catch this at the source. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

#1649) `origin/main` (`73e6fe15a`) fails a required check on an unmodified checkout: ``` error: methods `set_retain_decode_graph_across_spec` and `retain_decode_graph_across_spec` are never used --> crates/onnx-genai-engine/src/native_decode/cuda.rs:5516 ``` That is the **Check the native backend compiles** step of `Rust quality`, so it blocks every open PR regardless of contents. It surfaced on an unrelated CPU-kernel PR (#1628). Both methods are `#[cfg(test)]` accessors for the option-c graph-retention seam that #1648 landed as an *enabling primitive* — deliberately ahead of the WP4 tests that will drive them. Their `#[cfg(test)]` siblings either side (`set_retain_graph_on_rewind`, `padded_query_capacity`) are already called, which is why only these two trip. Fix is `#[allow(dead_code)]` on the pair with the reason recorded at the site — rather than deleting a seam that is about to be used, or widening the allow to the whole `impl` block. ### Local verification | gate | result | |---|---| | `cargo fmt --all -- --check` | clean | | `clippy -p onnx-genai-engine --features native-backend --all-targets -D warnings` | clean | | `clippy -p onnx-genai-engine --features native-cuda --all-targets -D warnings` | clean | | `clippy -p onnx-genai-cli --all-targets -D warnings` | clean | | `cargo test -p onnx-genai-engine --features native-backend` | 0 failed | > **Process note — this is the third main-is-red repair today**, and I am only finding them because they land on an unrelated PR: > - this morning: fmt + clippy debt from #1637/#1641 (four required gates red at once) → fixed by #1640 > - midday: #1644 reintroduced fmt violations → fixed by #1642 > - now: #1648 introduces this dead-code lint > > Each costs a full CI round-trip to discover, because the gates are sequential — the clippy steps only run once formatting passes. Running the `Rust quality` job on `main` post-merge, or a merge queue, would catch these at the source instead of on whoever's PR is open next. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

`cargo fmt --all -- --check` fails on an **unmodified `origin/main`** (`90ddd284e`): ``` crates/onnx-runtime-session/src/lib.rs:38 ``` `pub use onnx_runtime_ep_api::DeviceGraphSlot;` was added above the existing `WorkspaceRequirement` re-export rather than in sorted order. One-line swap, pure `cargo fmt --all` output. Formatting is a required check, so this blocks every open PR regardless of contents. It surfaced on an unrelated CPU-kernel PR (#1628). ### This is the fourth main-is-red repair today | # | PR | what was red on main | source | |---|---|---|---| | 1 | #1640 (not mine) | fmt + 3 clippy lints, four required gates at once | #1637 / #1641 | | 2 | #1642 | fmt, two sites | #1644 | | 3 | #1649 | clippy `dead_code`, `native_decode/cuda.rs` | #1648 | | 4 | **this** | fmt, one re-export | #1647 / #1648 | The pattern is consistent and worth fixing at the source: quality gates run on PR branches *before* merge but not on the merge result, so any merge can land violations that then fail whoever opens the next PR. Because the gates are sequential — clippy steps only run once formatting passes — each breakage costs a full CI round-trip to even *discover*, and they arrive one at a time. Two concrete options: enable a merge queue (gates run on the merge result), or run the `Rust quality` job on `main` post-merge so the break is attributed to the PR that caused it instead of the next unrelated one. Also still red on main and **not** fixed here, because I cannot reproduce it locally and it is not mine: `Rust (Windows ARM64)` → *Test cross-platform offline crates* has been failing on main since at least `73e6fe15a` (it is non-required, so it does not block merges). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

@justinchuby

`Rust quality → Check formatting` is failing on `main` at `6923a016b` (#1652). That job runs its steps sequentially, so while formatting is red **every other check in it is skipped** — clippy, publish order, the dispatch-manifest lints, feature-gate coverage, all of it. Every open PR is blocked and none of them are getting linted. Three sites, all in `crates/onnx-genai-engine/src/native_decode/cuda.rs`: two `verify_graph_phase` assignments (`:1815`, `:1823`) and one `assert_eq!` in `verify_capture_helper_tests` (`:7042`). Straight `cargo fmt --all` output, no hand edits. **Verified inert.** The before/after texts are identical after stripping whitespace *and* trailing commas — the only non-whitespace delta is commas rustfmt adds before a closing delimiter when it breaks a call across lines, which are semantically meaningless in Rust. Reproduced on a pristine `origin/main` worktree first, so this is main's breakage and not an artifact of my branch. ### This is the fifth time today `main` has been red on formatting or clippy five separate times in one day: #1637/#1641 (fixed by #1640), #1644 (#1642), #1648 (#1649), #1647/#1648 (#1651), and now #1652. The cause is structural, not carelessness. Required checks run on a PR's **merge ref**, but nothing re-runs them on `main` **after** the merge, so two PRs that are each green against an older base can land in sequence and leave the result red. Because the quality job is sequential, the breakage also masks every later step in it. The cost lands on whoever opens the next PR, who then has to distinguish "my change broke this" from "main was already broken" — a full CI round-trip each time. Two things would fix it, either one sufficient: - a **merge queue**, which tests the actual post-merge result; or - running **`Rust quality` on `main` post-merge**, which at least detects it immediately and attributes it correctly. I have now spent four PRs on this. I would rather not spend a fifth. cc @justinchuby Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

justinchuby mentioned this pull request Aug 21, 2026

fix(engine): make main green again — fmt + dormant retention seam dead_code (#1647) #1639

Closed

justinchuby and others added 2 commits August 21, 2026 01:32

justinchuby force-pushed the justinchuby-fix-mtp-proposer-import branch from 2e2ecba to d26cdaa Compare August 21, 2026 08:36

justinchuby changed the title ~~fix(engine): gate the MtpProposer import to the feature that uses it~~ fix(engine): make main green — fmt, dead code, and three 1.98.0 lints from #1637/#1641 Aug 21, 2026

justinchuby merged commit f2f1dbf into main Aug 21, 2026
16 checks passed

justinchuby deleted the justinchuby-fix-mtp-proposer-import branch August 21, 2026 09:28

justinchuby mentioned this pull request Aug 21, 2026

CI on main is red across 8 jobs, and has been since 08-03 — attribution table + reproductions #1600

Closed

justinchuby mentioned this pull request Aug 21, 2026

style: rustfmt the two sites #1644 left red on main #1642

Merged

justinchuby mentioned this pull request Aug 21, 2026

style: silence the dormant graph-retention seam #1648 left red on main #1649

Merged

justinchuby mentioned this pull request Aug 21, 2026

style: reorder one re-export #1648 left unsorted on main #1651

Merged

This was referenced Aug 21, 2026

Merge the #1186 memory stack (phases 1-7) onto main #1579

Merged

fmt: reformat three sites in native_decode/cuda.rs (main is red) #1653

Merged

Navigation Menu

Repository navigation

fix(engine): make main green — fmt, dead code, and three 1.98.0 lints from #1637/#1641 - #1640

fix(engine): make main green — fmt, dead code, and three 1.98.0 lints from #1637/#1641#1640
justinchuby merged 2 commits into
mainfrom
justinchuby-fix-mtp-proposer-import

justinchuby commented Aug 21, 2026 •

edited

Loading

Uh oh!

codecov Bot commented Aug 21, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

justinchuby commented Aug 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

The two that were not mechanical

⭐ #1632's lane earned itself here

Verification

Relationship to #1639

What I did not verify

Uh oh!

codecov Bot commented Aug 21, 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 21, 2026 •

edited

Loading

codecov Bot commented Aug 21, 2026 •

edited

Loading