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

[justinchuby]

origin/main @ 7ccdb920e is red again. This PR makes it green. Closes #1638.

Scope changed. This PR originally fixed four defects from #1637/#1641. #1640 landed the same fixes first, so that work is superseded and has been dropped — this branch is now reset onto 7ccdb920e with only the new breakage. Kept the PR rather than opening another because #1638 is still the tracking issue and this is the same recurrence.

Reproduced on a clean detached origin/main worktree, not inferred from CI

gate	on 7ccdb920e	here
cargo fmt --all -- --check	exit 1	exit 0
clippy -p onnx-genai-engine --features native-backend --all-targets -- -D warnings	exit 101	exit 0
clippy -p onnx-genai-engine --features native-cuda --all-targets -- -D warnings	exit 0	exit 0
clippy -p onnx-genai-cli --all-targets -- -D warnings	exit 0	exit 0
cargo test -p onnx-genai-engine --features native-backend --lib	—	575 passed; 0 failed; 1 ignored

The two defects, both from #1647

1. cargo fmt — native_decode/mod.rs:1115, native_decode/tests.rs:1412.

2. error: methods set_retain_decode_graph_across_spec and retain_decode_graph_across_spec are never used — native_decode/cuda.rs:5516.

Both accessors are already #[cfg(test)], so this is not the usual missing-gate case. 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 say it is deliberately exposed ("Kept off in production because it is not capture-safe until the M=K verify is captured with a pinned workspace; exposed for the option-c work and its tests"). Deleting would remove a seam the author placed on purpose; silently regating it would misrepresent why it has no caller.

This is the second recurrence in one day

#1637            fmt + unused import                  -> fixed by #1640
#1641            fmt + collapsible_if x5 + is_multiple_of  -> fixed by #1640
#1647            fmt + dead_code                      -> this PR

Same two classes each time, on the same crate. Worth noting what does not explain it: it is not that the gates are missing — Rust quality, Fast (Linux x86_64) and both CLI ORT lanes all run these checks and all go red. So the checks work; what is not happening is the red blocking the merge.

I have only measured that main advanced while red and that each new commit added more red. I have not inspected branch protection, so I am not claiming the rules are misconfigured — "main is red" and "red blocks merging" are independent propositions, and I only verified the first.

One mechanism is confirmed though, and it is worth knowing before trusting a pre-merge green: CLI ORT (Linux x86_64) is the only job running clippy -p onnx-genai-cli, and its earlier ORT gate step dies on the runner intermittently (~23% of concluded main runs). When it dies the clippy step is left at conclusion: null — never executed, counted as neither pass nor fail. A flaky failure can therefore hide a deterministic one, and a job being persistently red does not mean the checks inside it ever ran.

Traps

• rustfmt --edition 2021 <file> does not work in this repo. It aborts on the let-chain in load.rs and leaves the file untouched while exiting non-zero — it looks like it ran. Use cargo fmt.
• GitHub job logs carry ANSI escapes, so grep "^error" returns nothing. Strip with perl -pe 's/\e\[[0-9;]*m//g'.
• -D warnings errors halt compilation, so "the log shows N errors" never implies "the rest is clean". Re-run to exit 0 after each fix.
• cargo check is not clippy -- -D warnings, and --lib is not --all-targets (lib and lib test are separate units — this defect only exists in the latter).

Known intermittent, not this PR

Rust coverage (macOS arm64) may fail at plugin_export_abi ("kernel saw 0 operands"). Filed as #1643 — pre-existing, also seen on main @ 9cd2a7d13, and confirmed intermittent by re-running only that job on a byte-identical tree.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.48%. Comparing base (7ccdb92) to head (c5ed1ab).
⚠️ Report is 6 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1639      +/-   ##
==========================================
+ Coverage   81.04%   81.48%   +0.44%     
==========================================
  Files         384      384              
  Lines      180256   180260       +4     
  Branches   180256   180260       +4     
==========================================
+ Hits       146082   146888     +806     
+ Misses      29224    28426     -798     
+ Partials     4950     4946       -4     

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

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

Heads-up on interaction with this PR — #1637 broke two things, and this PR fixes one of them. No file overlap, so nothing here needs to change; this is purely so the CI reading afterwards is not misinterpreted.

The other defect is in engine/mod.rs, not load.rs:

error: unused import: `MtpProposer`
  --> crates/onnx-genai-engine/src/engine/mod.rs:63:59
   = note: `-D unused-imports` implied by `-D warnings`

MtpProposer's only reader is runtime.rs's generate_native_cold_with_callback, which is #[cfg(feature = "native-backend")], while the import is unconditional — so the default feature set imports a name it never uses. Fixed in #1640 by gating the import.

defect	jobs	fixed by
3 fmt diffs in engine/load.rs	Fast (Linux), Rust quality	this PR
unused import: MtpProposer	CLI ORT (Linux), CLI ORT (Windows)	#1640

⚠️ The reason I'm flagging it: these two mask each other. Rust quality runs Check formatting before Run clippy on all offline crates. While fmt is red the clippy step never executes, so it does not appear in the failure list at all. When this PR lands, Rust quality will move from failing at Check formatting to failing at clippy — same job count, and a casual reading says this PR did nothing. It did; it revealed the next layer.

So: judge this PR by which step fails, not by how many jobs are red. Both PRs are needed for main to go green, and they can land in either order.

I verified the two are disjoint mechanically rather than by inspection — this branch's head (645772a1) still carries the unconditional import at mod.rs:63, and #1640 touches only that line.

[justinchuby]

Update — #1640 now covers load.rs's formatting too, so this PR may be redundant. Flagging before you spend time on the conflict resolution.

Since my earlier comment, two things changed:

1. This PR went CONFLICTING / DIRTY (main moved under it — feat(mtp): on-GPU int4 draft LM-head projection + native MTP E2E wiring (recurrent-Scan blocker) #1641 landed), so it needs a rebase to merge at all.
2. feat(mtp): on-GPU int4 draft LM-head projection + native MTP E2E wiring (recurrent-Scan blocker) #1641 added 6 more fmt diffs in speculative/mod.rs, on top of the 3 in load.rs here. Fixing only load.rs no longer turns the fmt gate green.

main is currently red in four independent ways, and #1640 now fixes all of them together:

defect	jobs	in #1640
9 fmt diffs — load.rs (3) + speculative/mod.rs (6)	Fast (Linux), Rust quality	✅
unused import: MtpProposer	CLI ORT ×2	✅
dead_code on DraftProjectionDevice	CLI ORT (default features)	✅
collapsible_if ×5 + manual is_multiple_of	CUDA compile (Linux)	✅

My load.rs hunks are plain cargo fmt -p onnx-genai-engine output, so they should be byte-identical to yours — no judgement call involved, nothing of yours is lost.

Your call whether to close this as superseded or rebase and narrow it. I did not want to duplicate your work; I expanded #1640 only because main had been red for hours, this PR was blocked on a conflict, and the fmt half alone no longer clears the gate. If you would rather land it here instead, say so and I will drop the fmt commit from #1640 — the other three defects are independent of that file.

One thing worth carrying forward regardless: the collapsible_if fix in load.rs:389 is not a mechanical collapse. The outer arm borrows metadata.speculative immutably and the inner takes metadata.model mutably, so a single let chain keeps the shared borrow alive across the mutable one and fails borrowck. It needs the speculative side resolved into an owned value first.

[justinchuby]

Status update so this does not sit waiting on me.

engine/load.rs is now fmt-clean on main — #1640 (f2f1dbf6) landed those 3 diffs along with 3 other defects from #1637/#1641 that were reddening the same jobs, and I verified it directly:

$ git worktree add /tmp/fmtchk origin/main && cd /tmp/fmtchk
$ cargo fmt --all -- --check     # pinned 1.98.0

The only diffs it reports on current main (843b0bf7) are two new ones in native_decode/{mod,tests}.rs from #1644 — nothing in load.rs. So the formatting purpose of this PR is served; no action needed from you on that account, and I am sorry for the duplicated effort.

(Those two new ones are #1646.)

This PR still shows CONFLICTING. If the engine/mod.rs and speculative/mod.rs hunks in it are substantive and not just formatting, they are worth rebasing and keeping — I only claim the load.rs formatting is redundant, not the rest. If it was purely the fmt fix, closing it is the cheaper option. Your call either way; I am not going to touch your branch.

[justinchuby]

CI is green — all four fixes verified on CI, not just locally

Run 32464920678 @ 39c3d2fec — completed / success, zero non-success checks.

The four gates that are red on origin/main @ 81fc0060e are green here:

gate	job	on main	here
cargo fmt --all -- --check	Rust quality	failure	success
same	Fast (Linux x86_64)	failure	success
clippy -p onnx-genai-cli … -D warnings	CLI ORT (Linux x86_64)	failure	success
same, Windows	CLI ORT (Windows x86_64)	failure	success

The Windows lane is worth calling out: it runs the same Clippy onnx-genai-cli step, and it was failing on main for the same unused import: MtpProposer. So that defect was never platform-specific — it was simply invisible on Linux whenever the flaky ORT gate step ahead of it left clippy at conclusion: null.

One red on the way, and it was not this PR

Rust coverage (macOS arm64) failed on attempt 1:

a_node_sees_exactly_its_own_operand_count
plugin_export_abi.rs:623  "arity 1: kernel saw 0 operands, not 1"

Re-running only that job on the byte-identical tree returned green. It is a pre-existing intermittent — the same job failed on main @ 9cd2a7d13 (run 32449821915) in the same file, through the same thread_local! SEEN read, under a different test name. Filed as #1643 with the two occurrences, a rate (1/12 concluded main runs), and a one-line instrumentation change to make the next occurrence self-diagnosing.

onnx-runtime-ep-cpu-plugin does not depend on onnx-genai-engine, and this PR touches only three files, all in onnx-genai-engine — so it is causally excluded, not merely uncorrelated.

Convergence took three rounds, on purpose

-D warnings promotes warnings to errors, and an error halts compilation — so "the log shows N errors" never implies "the rest is clean". Fixing defects 1-2 is what made 3-4 visible. Each round was re-run to exit 0 before moving on, and the final state was verified across all four configurations (default -p onnx-genai-cli, native-backend, native-cuda, plus fmt and 574 tests) rather than only the one that had been failing.

Ready to merge. This is currently blocking every open PR in the repo — Rust quality, Fast (Linux x86_64), and both CLI ORT lanes are red on anything based on main.

[justinchuby]

I duplicated your fmt fix — my fault, and the useful part is why

I opened #1646 for the two native_decode/{mod,tests}.rs fmt sites without re-checking whether anyone had taken them. #1642 (2f94cba4) had already landed the identical fix. My PR merged as c027c37b with zero file changes — an empty commit. Pure waste, and entirely avoidable: I branched off origin/main, worked for ~30 minutes, and never re-fetched before pushing.

Recording it rather than quietly deleting it, because it is the second parallel-duplication between us today and the cause is the same both times: a red on main is a shared work queue with no claim mechanism. The cheap mitigation is a re-fetch and gh pr list immediately before pushing, not before starting. I have adopted that.

The part that is actually useful to you: nothing hides behind your dead_code fix

You are fixing the retain_decode_graph_across_spec / set_... dead_code from #1647. Because that error aborts the lib test compilation unit, every lint after it in that unit is unreachable — so a green from you would not, on its own, prove there is no second layer. This is the masking trap that has already cost us a cycle each.

I measured it rather than assuming. On clean origin/main (c027c37b, pinned 1.98.0), I applied a throwaway #[allow(dead_code)] to exactly those two methods — a stand-in for whatever shape your fix takes — and re-ran every lane that compiles this crate:

lane	before	with the dead_code suppressed
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
clippy -p onnx-genai-cli --all-targets -- -D warnings	0	0
clippy -p onnx-genai-cli -p onnx-genai-server -F .../native-cuda --all-targets	0	0
cargo fmt --all -- --check	0	0

Probe reverted; the worktree is clean.

So: that one defect is the whole of it for these lanes. Your fix should take main green rather than revealing a next layer. Two caveats I will not paper over — this is compile coverage on macOS aarch64, so it says nothing about Windows-specific cfg blocks, and it is not the ORT-linked or offline-crate gates (I could not run the full offline gate locally: onnx-runtime-cpuinfo's build script fails on this host, a local limitation and not a CI signal).

Two notes on the fix itself

The accessors have zero callers anywhere, including tests. retain_decode_graph_across_spec is read as a field at backend.rs:154 and cuda.rs:1286; the grep hits are the field, not the methods. So this is not a cfg mismatch to realign — the methods are genuinely uncalled, and #1647's doc comment says they are deliberately dormant ("exposed for the option-c work and its tests").

If you keep them, #[expect(dead_code)] is worth preferring over #[allow]: it is fulfilled today, and the moment #1648's follow-up wires them up, the author gets an unfulfilled-expectation warning telling them to remove it. allow would just sit there silently forever. I used allow above only because it was a throwaway probe.

Not touching your branch. #1651 has the onnx-runtime-session re-export from #1648, so between the two of you main should be covered.

[justinchuby]

Update: both halves of this PR are now on main, so I think it can close — but check me, I have been wrong about staleness twice today.

90ddd284e  style: silence the dormant graph-retention seam #1648 left red on main (#1649)
2f94cba4d  style: rustfmt the two sites #1644 left red on main (#1642)

#1649 covers the retain_decode_graph_across_spec dead_code; #1642 covered the two fmt sites. That is why this shows CONFLICTING / DIRTY — the conflict is with the fixes for the same defects, not with anything substantive.

Measured against origin/main at c729613a3, clean worktree, pinned 1.98.0:

cargo fmt --all -- --check                                          exit 0
clippy -p onnx-genai-engine -F native-backend --all-targets -Dw     exit 0
clippy -p onnx-genai-engine -F native-cuda   --all-targets -Dw      exit 0
clippy -p onnx-genai-cli -p onnx-genai-server -F .../native-cuda    exit 0

So main is green on every lane I can run here. (Not the full offline gate — onnx-runtime-cpuinfo's build script fails on this host, which is a local limitation and not a CI signal.)

The bit I got wrong, since it is the third instance today

My previous comment said "nothing hides behind your dead_code fix" and presented a table. That measurement was correct for the commit I measured — but I ran it against c027c37b while main had already moved to c729613a3, and #1649 had landed the real fix in between. I was reasoning about a branch nobody was on any more.

Same root cause as my duplicate #1646: I re-fetch when I start, and then treat that snapshot as current for the next half hour. On a branch taking this many merges an hour, a snapshot has a shelf life of minutes. I have moved the re-fetch to immediately before every conclusion, not after.

The conclusion in that comment happens to survive the correction — main is green now, by direct measurement at the current tip — but it survived by luck, not by method, and I would rather say that than let a right answer stand on a wrong basis.

[justinchuby]

Superseded on main — closing without merging.

Both defects this PR carried were fixed upstream before it could land:

defect	fixed by
cargo fmt breakage in native_decode/{mod,tests}.rs (from #1644/#1647)	#1646 (c027c37bc)
dead_code on the dormant retention seam retain_decode_graph_across_spec at native_decode/cuda.rs (from #1648)	#1649 (90ddd284e)

Verified on a clean origin/main worktree at c729613a3, pinned 1.98.0 — not inferred from CI:

cargo fmt --all -- --check                                                              exit 0
cargo clippy --locked -p onnx-genai-cli --all-targets -- -D warnings                    exit 0
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

This is the second time this branch was superseded (the earlier form was overtaken by #1640, which fixed the #1637/#1641 pair it carried). Nothing here is unique any more, so it is withdrawn rather than rebased.

One thing worth carrying forward, since it is not visible in either fixing PR: this branch did reach fully green CI in both of its forms (run 32464920678 for form two; the final Windows ARM64 rerun for form three concluded success). The reruns that got it there also produced the #1072 controlled demonstration and the fourth #1643 occurrence — so the observations survive even though the diff does not.