refactor: name the two CUDA paths instead of calling both of them `cuda` by justinchuby · Pull Request #1629 · justinchuby/onnx-genai

justinchuby · 2026-08-21T05:38:03Z

The word cuda meant two opposite things one crate boundary apart. The CLI's
cuda selected ONNX Runtime's CUDA EP; the server's cuda selected ours, and
the CLI's cuda was wired to the server's ort-cuda. Picking the one that
sounds right gave a binary with none of this repo's kernels in it, which reads
as a 22x regression in whatever you last edited rather than as a wrong build.
That is Trap 7 in the perf skill; it was written down because it cost real
measurement time.

Every crate that offers a choice now names the choice:

ort-cuda ONNX Runtime's CUDAExecutionProvider
native-cuda our onnx-runtime-ep-cuda, a strict superset of ort-cuda

Crates with only one CUDA path (onnx-genai, onnx-genai-ort,
onnx-runtime-session, onnx-runtime-ep-cuda, onnx-runtime-python) keep the
plain cuda, because there is nothing there to confuse it with. That rule, and
the feature matrix, are now written down in docs/build-features.md.

They are deliberately not merged. The CUDA wheels build these same crates with
--features ort-cuda precisely so no CUDA code is bundled and the wheel
builders need no CUDA toolkit; one merged cuda would force every wheel build
to compile our kernels.

native-cuda now also implies native-backend, which fixes a real trap rather
than just renaming one. onnx-genai-server --features cuda used to compile the
entire native CUDA EP and then leave out the native session that dispatches to
it, so the EP was present, unreachable, and the build behaved exactly like the
ORT one while looking like it should not. Same for onnx-genai-engine,
onnx-genai-bench and onnx-genai-capi. capi in particular has zero references
to onnx-runtime-ep-cuda, so it was imposing a CUDA toolkit on its build to
compile an EP nothing could call; it now forwards ort-cuda, which is what its
own comment already said it does.

Verification:

227 cfgs renamed. 77 of them were all(cuda, native-backend) pairs, now
redundant and collapsed. unexpected_cfgs is on by default and reports the
legal values, so a missed rename cannot silently drop code -- confirmed
empirically against a deliberately bogus feature name, and every build below
reports unexpected_cfg = 0.
All seven combinations compile: cli/server/bench x {native-cuda, ort-cuda},
plus capi ort-cuda.
The names now match the binaries. native-cuda gives 129 matmul_nbits_gemv
symbols and 52 MB; ort-cuda gives 0 and 39 MB.
Behaviourally equivalent to main: the native-cuda CLI built from this branch
and from main agree on symbol counts exactly (gemv 129/129, ep_cuda
3118/3118) and differ by 1952 bytes of embedded feature-name strings.
onnx-genai-engine 588 passed / 0 failed, onnx-genai-server 246 / 0. Engine
gains 3 tests over main because native-cuda now pulls the backend those
tests need, which previously had to be asked for separately.
End-to-end unchanged: p50 inter-token 23.4 ms. The ort-cuda binary starts.
Clippy warning set is byte-identical to main's; nothing new introduced.

Call sites updated: wheels.yml, publish.yml, ci.yml, the perf skill's Trap 7,
the H200 and Windows runbooks, the bench and parity READMEs, and the parity
script. Dated benchmark records and .squad archives are left alone; they
record what was actually run at the time.

Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com
Copilot-Session: 0190e2eb-abe4-451f-b36d-44a035a99b7e

The word `cuda` meant two opposite things one crate boundary apart. The CLI's `cuda` selected ONNX Runtime's CUDA EP; the server's `cuda` selected ours, and the CLI's `cuda` was wired to the server's `ort-cuda`. Picking the one that sounds right gave a binary with none of this repo's kernels in it, which reads as a 22x regression in whatever you last edited rather than as a wrong build. That is Trap 7 in the perf skill; it was written down because it cost real measurement time. Every crate that offers a choice now names the choice: ort-cuda ONNX Runtime's CUDAExecutionProvider native-cuda our `onnx-runtime-ep-cuda`, a strict superset of ort-cuda Crates with only one CUDA path (`onnx-genai`, `onnx-genai-ort`, `onnx-runtime-session`, `onnx-runtime-ep-cuda`, `onnx-runtime-python`) keep the plain `cuda`, because there is nothing there to confuse it with. That rule, and the feature matrix, are now written down in `docs/build-features.md`. They are deliberately not merged. The CUDA wheels build these same crates with `--features ort-cuda` precisely so no CUDA code is bundled and the wheel builders need no CUDA toolkit; one merged `cuda` would force every wheel build to compile our kernels. `native-cuda` now also implies `native-backend`, which fixes a real trap rather than just renaming one. `onnx-genai-server --features cuda` used to compile the entire native CUDA EP and then leave out the native session that dispatches to it, so the EP was present, unreachable, and the build behaved exactly like the ORT one while looking like it should not. Same for `onnx-genai-engine`, `onnx-genai-bench` and `onnx-genai-capi`. capi in particular has zero references to `onnx-runtime-ep-cuda`, so it was imposing a CUDA toolkit on its build to compile an EP nothing could call; it now forwards `ort-cuda`, which is what its own comment already said it does. Verification: - 227 cfgs renamed. 77 of them were `all(cuda, native-backend)` pairs, now redundant and collapsed. `unexpected_cfgs` is on by default and reports the legal values, so a missed rename cannot silently drop code -- confirmed empirically against a deliberately bogus feature name, and every build below reports `unexpected_cfg = 0`. - All seven combinations compile: cli/server/bench x {native-cuda, ort-cuda}, plus capi ort-cuda. - The names now match the binaries. `native-cuda` gives 129 `matmul_nbits_gemv` symbols and 52 MB; `ort-cuda` gives 0 and 39 MB. - Behaviourally equivalent to main: the `native-cuda` CLI built from this branch and from main agree on symbol counts exactly (gemv 129/129, ep_cuda 3118/3118) and differ by 1952 bytes of embedded feature-name strings. - onnx-genai-engine 588 passed / 0 failed, onnx-genai-server 246 / 0. Engine gains 3 tests over main because `native-cuda` now pulls the backend those tests need, which previously had to be asked for separately. - End-to-end unchanged: p50 inter-token 23.4 ms. The ort-cuda binary starts. - Clippy warning set is byte-identical to main's; nothing new introduced. Call sites updated: wheels.yml, publish.yml, ci.yml, the perf skill's Trap 7, the H200 and Windows runbooks, the bench and parity READMEs, and the parity script. Dated benchmark records and `.squad` archives are left alone; they record what was actually run at the time. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0190e2eb-abe4-451f-b36d-44a035a99b7e

…dent ways (#1630) Found while sweeping for call sites the CUDA feature rename (#1629) might have invalidated. This script was already broken before that change, and would have been mistaken for fallout from it. Both failures are reproducible and neither is subtle: $ cargo build --release -p onnx-genai --bin onnx-genai --features cuda error: no bin target named `onnx-genai` in `onnx-genai` package The `onnx-genai` package ships only the diffusion bins (`run_comfyui`, `render_sd`). The CLI binary lives in `onnx-genai-cli`. The feature is now spelled `ort-cuda`, which is also what the script's own comment describes ("propagates to onnx-genai-ort/cuda"). $ onnx-genai generate --model DIR --max-new-tokens 4 "hi" error: unexpected argument '--model' found `generate` takes the model directory positionally; there is no `--model` flag. Verified: the corrected build line produces `target/release/onnx-genai`, and the corrected invocation gets past argument parsing into model loading (it fails on a deliberately absent directory with the package-inspection error, which is the next stage), while the old form is still rejected by clap. `bash -n` clean. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0190e2eb-abe4-451f-b36d-44a035a99b7e Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0190e2eb-abe4-451f-b36d-44a035a99b7e

codecov · 2026-08-21T05:50:24Z

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 81.05%. Comparing base (9cd2a7d) to head (753760e).
⚠️ Report is 3 commits behind head on main.

Files with missing lines	Patch %	Lines
crates/onnx-genai-cli/src/model_inspection.rs	0.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1629      +/-   ##
==========================================
+ Coverage   80.85%   81.05%   +0.20%     
==========================================
  Files         383      384       +1     
  Lines      175785   180225    +4440     
  Branches   175785   180225    +4440     
==========================================
+ Hits       142137   146089    +3952     
- Misses      28743    29193     +450     
- Partials     4905     4943      +38

Flag	Coverage Δ
cli-ort-linux	`82.60% <0.00%> (?)`
cli-ort-windows	`82.20% <0.00%> (+0.09%)`	⬆️
mlas	`85.05% <ø> (ø)`
offline	`80.93% <ø> (+0.19%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
crates/onnx-genai-cli/src/model_inspection.rs	`55.42% <0.00%> (ø)`

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

…p rotting (#1635) Answering "is everything broken fixed?" properly meant checking rather than recalling, so I validated every `cargo` command written down in the repo against `cargo metadata`. 262 commands; six named something that does not exist. Two were my own fallout from the feature rename (#1629), which I missed because I updated `.github/skills/` and not `.agents/skills/`: - `.agents/skills/profiling/SKILL.md` still asked for `bench-native,cuda`. - `crates/onnx-genai-capi/README.md` still asked for `--features cuda`. One is a shell script with exactly the two bugs I fixed in #1630, which I should have found then by looking at the class of defect instead of the instance: - `scripts/build_real_model.sh` built `-p onnx-genai --bin onnx-genai` (that package ships only the diffusion bins) and called `generate --model DIR` (the model directory is positional). Three predate all of this and are worse than stale -- they were never true: - `docs/ep-plugin/EP_PLUGIN_EXPORT_TEST_PLAN.md` built a `plugin-export` feature on `onnx-runtime-ep-cpu` and grepped for `CreateEpApiFactories`. What shipped is a separate `onnx-runtime-ep-cpu-plugin` cdylib exporting `CreateEpFactories` and `ReleaseEpFactory`. Verified by building it and reading `nm -D`. - `docs/performance/CPU_MATMUL_ASSIGNMENT.md` gave a repro recipe using a `bench_prec` binary with `--native-threads` / `--ort-intra-threads`. `git log` finds no commit that ever added or removed it. The recipe is removed rather than annotated, because a command that cannot run is not worth keeping; the section now says plainly that those numbers cannot be reproduced as written. - A fixture generator credited its canonical output to `cargo run -p onnx-std --example convert_fixture`. That example has never existed either. The reason all six survived is that nothing ever checks a command that nothing ever runs; the failure then looks like the reader's environment rather than the line. `scripts/check_documented_commands.py` now validates the parts that can be checked statically -- package, `--bin`, `--example`, `--test`, `--bench`, `--features` -- and runs in the `rust-quality` lane. Dated docs and `.squad` archives are skipped: they record what was run at the time. Verification: - The check reports 262 commands, all resolving, on this branch. - It has teeth, by the same `--self-test` convention the dispatch-manifest lint uses: six cases, five that must be detected (bad feature, bad bin, unknown package, bad bench, bad `dep/feature`) and one valid command that must stay silent. Also confirmed by hand against the real tree, by breaking `docs/build-features.md` and the profiling skill and watching each get caught. - `bash -n` clean on the repaired script, and its new CLI form reaches model loading instead of being rejected by clap. - `ci.yml` parses as YAML. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0190e2eb-abe4-451f-b36d-44a035a99b7e Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0190e2eb-abe4-451f-b36d-44a035a99b7e

Main's #1629 renamed this crate's `cuda` feature to `native-cuda`. A stale `#[cfg(feature = "cuda")]` is not a compile error: an unknown feature simply evaluates false, so the rename would have silently switched the workflow island runner's device-memory sampling off on every CUDA build while the tree still compiled and every test still passed. Gate on `ort-cuda` instead. That is the feature that actually brings in `onnx-genai-ort/cuda`, where `cuda_rt::device_memory_info` lives, and `native-cuda` enables it transitively -- so the single condition covers both CUDA paths exactly, with no narrowing relative to the old disjunction. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Justin Chuby <justinchuby@users.noreply.github.com>

Main's #1635 added a lint that resolves every documented `cargo` command against the actual workspace, and it caught two real inconsistencies this branch had left behind. `crates/onnx-genai/src/bin/run_diffusion.rs` was deleted here in "Remove legacy composite pipeline execution", because it drove the strategy/phase composite runtime that `pipeline.workflow` replaces. Ten `scripts/*.py` helpers still shelled out to `target/release/run_diffusion` and told the reader to build it with `cargo build -p onnx-genai --bin run_diffusion`. None of them could run: the binary they exec cannot be built. Delete them with the runner they drive rather than leaving instructions that cannot be followed -- exactly the rot the new lint exists to prevent. Nothing outside the group references them; the only mentions are among themselves and in a dated decisions archive. Also point the workflow performance doc at `native-cuda`, since main's #1629 renamed the feature its example passed to `cargo test`. `scripts/check_documented_commands.py` now exits clean, as do the other quality-gate lints. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Justin Chuby <justinchuby@users.noreply.github.com>

… hiding (#1632) Closes #1631. ## The gap No job in any workflow enabled `onnx-genai-engine`'s `native-cuda` feature. Three independent checks on `origin/main` (`f8eb8a3e2`): 1. **`native-cuda` appears in `ci.yml` exactly once — inside a comment** (line 838). There is no `--features native-cuda` anywhere. 2. **Nothing enables it transitively.** No `default` reaches it (`engine`, `cli`, `server`, `bench` all default to `["cuda-13000", …]`). Every other mention across `crates/*/Cargo.toml` is `required-features = ["native-cuda"]`, which *restricts* a target rather than enabling a feature, plus one forwarding entry in `onnx-genai-server`. 3. **The CUDA jobs do not build the engine.** They run `cargo check --locked -p onnx-runtime-ep-cuda -p onnx-runtime-python --features onnx-runtime-python/cuda` — no `-p onnx-genai-engine`. So roughly **167 `#[cfg(feature = "native-cuda")]` sites** — `native_decode/{mod,load,cuda,tensor,tests}.rs`, `memory_authority.rs`, `pipeline/*` — were never type-checked, linted, or tested. A change could break or orphan any of them with every check on the PR green. ## Why the usual safety net does not cover this `unexpected_cfgs` is warn-by-default and does fire, and `RUSTFLAGS: "-D warnings"` escalates it. But that protection is conditional on the code being **compiled**, so it inverts exactly where it is needed: | stale `feature = "cuda"` written… | compiled? | result | |---|---|---| | in ordinary engine code | yes | warns → **CI red** | | inside a `#[cfg(feature = "native-cuda")]` block | **no** | never linted → **CI green** | The CUDA code is by definition inside those blocks. Worth noting separately that `-D warnings` is set on only four jobs (`Fast (Linux x86_64)`, `Rust coverage (*)`, `Rust (Windows ARM64)`, `EP conformance`) and on **neither** `CUDA compile` job, so this step carries `-- -D warnings` on the invocation itself rather than relying on job env. ## It was already broken Not a hypothetical. `main` fails the command today: ``` $ cargo clippy --locked -p onnx-genai-engine --features native-cuda --all-targets -- -D warnings error: method `kv_commits_on_demand` is never used --> crates/onnx-genai-engine/src/native_decode/cuda.rs:3005:19 = note: `-D dead-code` implied by `-D warnings` exit=101 ``` `kv_commits_on_demand` exists only so a regression test can assert the VMM path is genuinely being exercised, and its **only caller is inside a `#[cfg(test)]` module** — so in the plain `lib` target it is really dead. It is `pub(crate)`, so no integration test outside the crate could reach it either. I gated it on `test` as well as the feature, which states that intent, rather than reaching for `#[allow(dead_code)]` which would only silence the report. This also shows why `--all-targets` is load-bearing: `--lib` alone does not surface this class of defect, and `--lib` is exactly what someone verifying by hand would reach for. ## Verification All under the pinned 1.98.0. **Negative control on a clean worktree of `origin/main`:** | tree | exit | |---|---| | `origin/main` (`f8eb8a3e2`) | **101** — `-D dead-code` | | this branch | **0** | **The lane can actually fail** — the acceptance criterion from #1631 and #1621, since a new check that is green on day one proves nothing about whether it is wired correctly. I injected the exact mistake this is meant to catch, a stale feature name inside a `native-cuda` module: ``` +#[cfg(feature = "cuda")] +fn injected_stale_feature_probe() -> bool { true } ``` ``` error: unexpected `cfg` condition value: `cuda` error: could not compile `onnx-genai-engine` (lib) due to 1 previous error exit=101 ``` Injection reverted and the tree re-verified at exit 0. So a botched `cuda` → `native-cuda` rename is now caught, where before it would have compiled to `false` silently. **No collateral damage:** `cargo fmt --all -- --check` clean; `cargo test --locked -p onnx-genai-engine --features native-backend --lib` → **568 passed, 0 failed**, identical to `main`. **Cost:** check-only. CUDA is dynamically loaded, so no GPU and no CUDA toolkit — the same premise the neighbouring `Clippy CUDA execution provider` step already relies on. It runs in the existing `CUDA compile (Linux x86_64)` job rather than adding one, so no new runner is provisioned. ## What I could not verify - **The step is Linux-only.** Any Windows-specific `cfg` inside the `native-cuda` blocks stays uncovered. I chose not to double the cost on a first pass; if that surface is real it should be a follow-up rather than assumed covered. - **This is compile coverage, not test coverage.** The CUDA integration tests remain behind `required-features = ["native-cuda"]` and are still not run — they need a GPU runner. This closes "the code is never compiled", not "the code is never exercised". - **I did not audit the other three renamed crates** (`cli`, `server`, `bench`) for the same gap; they also define `native-cuda` and may have their own uncompiled surface. ## Note on #1629 To be fair to it: its `cuda` → `native-cuda` rename is **clean** — zero lingering `feature = "cuda"` references in any of the four renamed crates. This PR is about the gap that rename exposed, not a defect in it. The rename is also **partial by design**: four crates use `native-cuda` while nine still define `cuda` (`onnx-runtime-ep-cuda`, `onnx-genai-ort`, `onnx-runtime-session`, …), so "which name does this crate use" cannot be answered from memory — and until this PR, guessing wrong inside CUDA code was invisible. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 08760f2f-160f-41e5-828d-9d9b6045c00d

Main's #1629 renamed this crate's `cuda` feature to `native-cuda`. A stale `#[cfg(feature = "cuda")]` is not a compile error: an unknown feature simply evaluates false, so the rename would have silently switched the workflow island runner's device-memory sampling off on every CUDA build while the tree still compiled and every test still passed. Gate on `ort-cuda` instead. That is the feature that actually brings in `onnx-genai-ort/cuda`, where `cuda_rt::device_memory_info` lives, and `native-cuda` enables it transitively -- so the single condition covers both CUDA paths exactly, with no narrowing relative to the old disjunction. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Justin Chuby <justinchuby@users.noreply.github.com>

Main's #1635 added a lint that resolves every documented `cargo` command against the actual workspace, and it caught two real inconsistencies this branch had left behind. `crates/onnx-genai/src/bin/run_diffusion.rs` was deleted here in "Remove legacy composite pipeline execution", because it drove the strategy/phase composite runtime that `pipeline.workflow` replaces. Ten `scripts/*.py` helpers still shelled out to `target/release/run_diffusion` and told the reader to build it with `cargo build -p onnx-genai --bin run_diffusion`. None of them could run: the binary they exec cannot be built. Delete them with the runner they drive rather than leaving instructions that cannot be followed -- exactly the rot the new lint exists to prevent. Nothing outside the group references them; the only mentions are among themselves and in a dated decisions archive. Also point the workflow performance doc at `native-cuda`, since main's #1629 renamed the feature its example passed to `cargo test`. `scripts/check_documented_commands.py` now exits clean, as do the other quality-gate lints. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Justin Chuby <justinchuby@users.noreply.github.com>

justinchuby merged commit 836c04d into main Aug 21, 2026
3 checks passed

justinchuby deleted the clarify-cuda-features branch August 21, 2026 05:38

justinchuby mentioned this pull request Aug 21, 2026

fix(scripts): repair run_target_greedy_cuda.sh, broken in two independent ways #1630

Merged

Navigation Menu

Repository navigation

refactor: name the two CUDA paths instead of calling both of them `cuda` - #1629

refactor: name the two CUDA paths instead of calling both of them `cuda`#1629
justinchuby merged 1 commit into
mainfrom
clarify-cuda-features

justinchuby commented Aug 21, 2026

Uh oh!

Uh oh!

codecov Bot commented Aug 21, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

justinchuby commented Aug 21, 2026

Uh oh!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

codecov Bot commented Aug 21, 2026 •

edited

Loading