Skip to content

ci: lint the cli/server native-CUDA paths, the last 12 cfg sites nothing compiles - #1645

Merged
justinchuby merged 1 commit into
mainfrom
justinchuby-cli-native-cuda-lane
Aug 21, 2026
Merged

justinchuby merged 1 commit into
mainfrom
justinchuby-cli-native-cuda-lane

Conversation

@justinchuby

Copy link
Copy Markdown
Owner

#1632 closed the native-CUDA compile-coverage gap for onnx-genai-engine (167
cfg sites) by adding a strict clippy lane to the CUDA job. It could not close
the same gap for onnx-genai-cli (2 sites) and onnx-genai-server (10 sites):
both crates link the downloaded ONNX Runtime, and the CUDA job deliberately
excludes ORT-linked crates.

This adds their lane to CLI ORT, which already has ORT staged and already
runs clippy on the CLI -- but only with default features, under which every
#[cfg(feature = "native-cuda")] block evaluates to false and is never parsed
as code. Code that is never compiled cannot be linted, so a defect there is
invisible rather than red.

Kept as a separate step: the default configuration is what ships, so it must
keep being checked on its own rather than replaced by a union of both.

Verified (pinned 1.98.0, macOS aarch64, no CUDA toolkit -- CUDA is dynamically
loaded, so the crate graph builds without one):

cargo clippy --locked -p onnx-genai-cli -p onnx-genai-server
--features onnx-genai-cli/native-cuda,onnx-genai-server/native-cuda
--all-targets -- -D warnings -> exit 0

Paired negative control, one probe per crate, each run against BOTH lanes on
the same tree so the comparison is not confounded:

probe in onnx-genai-server/src/state.rs:316 (inside #[cfg(native-cuda)])
existing default lane -> exit 0, probe not mentioned
new lane -> exit 101, unused variable: probe_negative_control

probe in onnx-genai-cli/src/generate.rs:200 (inside #[cfg(native-cuda)])
existing default lane -> exit 0, probe not mentioned
new lane -> exit 101

Both probes reverted; the tree is clean.

That the existing lane stays green with a hard error sitting in the file is the
point: it is not that the check was lenient, it is that those lines were not
code in that configuration.

Note on the first attempt, because it nearly produced a false pass: my initial
probe was let _probe... = 1u32;. A leading underscore suppresses
unused_variables, so the new lane returned exit 0 and looked blind. The probe
was broken, not the lane. A negative control that fails to fail proves nothing
until you have shown the control itself works.

What I did not verify: the Windows leg. cli-ort is a ubuntu-latest +
windows-latest matrix, so this step runs on both, but I have no Windows host
and did not measure it. The crate graph builds with no CUDA toolkit on this
host, which is evidence it does not need one, not proof for Windows. CI is the
oracle for that leg.

Co-authored-by: Copilot App 223556219+Copilot@users.noreply.github.com
Copilot-Session: 08760f2f-160f-41e5-828d-9d9b6045c00d

@codecov

codecov Bot commented Aug 21, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.03%. Comparing base (c729613) to head (d717546).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1645      +/-   ##
==========================================
- Coverage   81.46%   81.03%   -0.44%     
==========================================
  Files         384      384              
  Lines      180322   180322              
  Branches   180322   180322              
==========================================
- Hits       146900   146118     -782     
- Misses      28477    29259     +782     
  Partials     4945     4945              
Flag Coverage Δ
cli-ort-linux 82.60% <ø> (ø)
cli-ort-windows 82.11% <ø> (-0.10%) ⬇️
mlas 85.09% <ø> (-0.23%) ⬇️
offline 80.90% <ø> (-0.46%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.
see 9 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
justinchuby force-pushed the justinchuby-cli-native-cuda-lane branch from 0c7935d to 29ed6de Compare August 21, 2026 10:43
@justinchuby

Copy link
Copy Markdown
Owner Author

CI status: the two reds on this PR are inherited from main, and the new lane itself is green on both legs

Run 32473920068 (rebased onto c027c37b): 10 success, 2 failure.

Both failures are one defect, and it is not this PR's:

CUDA compile (Linux x86_64)  ->  step "Clippy engine native-CUDA integration"
Rust quality                 ->  step "Check the native backend compiles"

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
  = note: `-D dead-code` implied by `-D warnings`

Attribution, measured rather than inferred — git log -S puts it in #1647 (7ccdb920), and it reproduces on a clean origin/main worktree with no changes of mine present:

$ git worktree add /tmp/fmtchk origin/main && cd /tmp/fmtchk
$ cargo clippy --locked --all-targets -p onnx-genai-engine --features native-backend -- -D warnings
exit=101   # same two methods

This PR touches .github/workflows/ci.yml only, so it cannot be the cause. It is already being fixed in #1639. This will go green on a rebase once that lands; no action needed here.

The lane this PR adds executed and passed — on both legs, in both runs

That is the claim worth checking, since a step that silently does not run is the failure mode this whole line of work exists to close:

run leg step result duration
32468236480 Linux x86_64 Clippy cli+server with native CUDA success 19s
32468236480 Windows x86_64 Clippy cli+server with native CUDA success 41s
32473920068 Linux x86_64 Clippy cli+server with native CUDA success 22s
32473920068 Windows x86_64 Clippy cli+server with native CUDA success 40s

This retires a caveat I flagged as unverified in #1632. There I said the native-CUDA lint coverage was Linux-only and I had no way to check Windows-specific cfg blocks. cli-ort is a ubuntu-latest + windows-latest matrix, so this step runs on both — and the Windows leg is now measured, not assumed. It is still compile coverage, not execution: no GPU is involved anywhere in this.

Correction to my own PR body

I wrote that the Windows leg was unverified and that CI would have to be the oracle. CI has now been the oracle, twice. Updating that here rather than silently leaving the stale caveat in the description.

…ing compiles

#1632 closed the native-CUDA compile-coverage gap for `onnx-genai-engine` (167
cfg sites) by adding a strict clippy lane to the CUDA job. It could not close
the same gap for `onnx-genai-cli` (2 sites) and `onnx-genai-server` (10 sites):
both crates link the downloaded ONNX Runtime, and the CUDA job deliberately
excludes ORT-linked crates.

This adds their lane to `CLI ORT`, which already has ORT staged and already
runs clippy on the CLI -- but only with default features, under which every
`#[cfg(feature = "native-cuda")]` block evaluates to false and is never parsed
as code. Code that is never compiled cannot be linted, so a defect there is
invisible rather than red.

Kept as a separate step: the default configuration is what ships, so it must
keep being checked on its own rather than replaced by a union of both.

Verified (pinned 1.98.0, macOS aarch64, no CUDA toolkit -- CUDA is dynamically
loaded, so the crate graph builds without one):

  cargo clippy --locked -p onnx-genai-cli -p onnx-genai-server \
    --features onnx-genai-cli/native-cuda,onnx-genai-server/native-cuda \
    --all-targets -- -D warnings          -> exit 0

Paired negative control, one probe per crate, each run against BOTH lanes on
the same tree so the comparison is not confounded:

  probe in onnx-genai-server/src/state.rs:316 (inside `#[cfg(native-cuda)]`)
    existing default lane -> exit 0, probe not mentioned
    new lane              -> exit 101, `unused variable: probe_negative_control`

  probe in onnx-genai-cli/src/generate.rs:200 (inside `#[cfg(native-cuda)]`)
    existing default lane -> exit 0, probe not mentioned
    new lane              -> exit 101

Both probes reverted; the tree is clean.

That the existing lane stays green with a hard error sitting in the file is the
point: it is not that the check was lenient, it is that those lines were not
code in that configuration.

Note on the first attempt, because it nearly produced a false pass: my initial
probe was `let _probe... = 1u32;`. A leading underscore suppresses
`unused_variables`, so the new lane returned exit 0 and looked blind. The probe
was broken, not the lane. A negative control that fails to fail proves nothing
until you have shown the control itself works.

What I did not verify: the Windows leg. `cli-ort` is a ubuntu-latest +
windows-latest matrix, so this step runs on both, but I have no Windows host
and did not measure it. The crate graph builds with no CUDA toolkit on this
host, which is evidence it does not need one, not proof for Windows. CI is the
oracle for that leg.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 08760f2f-160f-41e5-828d-9d9b6045c00d
@justinchuby
justinchuby force-pushed the justinchuby-cli-native-cuda-lane branch from 29ed6de to d717546 Compare August 21, 2026 11:41
@justinchuby
justinchuby merged commit 9fd0526 into main Aug 21, 2026
16 checks passed
@justinchuby
justinchuby deleted the justinchuby-cli-native-cuda-lane branch August 21, 2026 12:27
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