Repository navigation
fix(ci): account for the deleted ONNX_GENAI_GEMV_FOLDSCALE knob - #1536
Conversation
verify_documented_env_vars is failing on main. #1527 deleted the fold-scale GEMV and gemv_foldscale_enabled() after the +2.7% failed to reproduce, but dense-decode-megakernel-feasibility.md still names the env var that selected it, so the guard sees a documented knob no crate reads -- which is exactly what it exists to catch. The prose is already honest: 9.2 records the re-measurement (a 2.9% cost against a 28.62/28.62 baseline) and the repro note says the entries were removed. Two of the three mentions carry that caveat; the Phase C lever paragraph did not, and now does. So this is an allowlist entry, not a rewrite. The script requires each KNOWN_UNIMPLEMENTED name to stay accompanied by the statement in the prose, and after this commit all three mentions carry it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The caveat sits inside 9.2, so 'see 9.2' left the reader in the section they were already in -- and the next numbers there are the retracted +2.7% table. The re-measurement that retracts it is in 9.3. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Opus review: APPROVE_WITH_COMMENTS, one non-blocking catch, fixed in The caveat I added sits inside §9.2, so The review independently confirmed: the guard fails on clean It also checked the thing I most wanted checked — that the entry cannot mask a future gap. The script's reverse One correction to my own framing that the review caught: FOLDSCALE is not in the same category as its two allowlist siblings. |
`cargo fmt --all --check` is **failing on unmodified `main`**
(`31f1b02b6`), in four files across two crates.
The one that looks like a paste accident:
```
Diff in crates/onnx-genai-bench/src/bin/profile_native.rs:2978:
-fn run_pipeline(args: &Args, model_dir: &Path) -> Result<()> { if args.synthetic {
+fn run_pipeline(args: &Args, model_dir: &Path) -> Result<()> {
+ if args.synthetic {
```
and three more that are ordinary width/wrapping drift:
| file | what |
|---|---|
| `onnx-genai-bench/src/bin/profile_native.rs` | brace and first
statement on one line; one `generate_with_callback` call over width |
| `onnx-runtime-ep-cuda/src/kernels/group_query_attention.rs` | two
`assert!` conditions wrapped that now fit on one line |
| `onnx-runtime-ep-cuda/src/kernels/matmul_nbits.rs` | two `set_var`
calls over width |
| `onnx-runtime-ep-cuda/src/runtime.rs` | a method chain and an
`assert!` over width |
This commit is the output of `cargo fmt --all` and nothing but
formatting changed — no condition, no argument, no control flow.
```
$ cargo fmt --all --check && echo CLEAN
CLEAN
```
Second broken-`main` gate I have hit in this validation pass, after
#1536 (`verify_documented_env_vars`). Both were found by running the
gates locally against a clean `origin/main` rather than trusting a PR's
own green tick, and both are cheap to keep green.
`strict_required_status_checks_policy` is `false` on this repo, so a
check that passed against an older base stays green against a newer one
— which is exactly how a formatting break survives on `main`.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
`cargo fmt --all --check` — one of this repo's two required checks — has been failing on unmodified `main` since #1550 merged. A test's `eprintln!` exceeds the line width: ``` Diff in crates/onnx-runtime-ep-cuda/src/kernels/matmul_nbits.rs:18919: - eprintln!("skipping MatMulNBits bf16 small-batch direct-store test: CUDA runtime unavailable"); + eprintln!( + "skipping MatMulNBits bf16 small-batch direct-store test: CUDA runtime unavailable" + ); ``` Reproduced on a clean checkout of `origin/main` with no local edits, so it is not an artifact of any open branch. This PR is **pure `cargo fmt --all` output** over that one file — 3 insertions, 1 deletion, no other change. This is the third gate found red on `main` in as many days (see #1536, #1546). The mechanism is the same each time: with only two required checks and `strict_required_status_checks_policy=false`, a PR can merge on a green run that predates the commit that breaks the gate. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1536 +/- ##
==========================================
+ Coverage 82.19% 82.64% +0.45%
==========================================
Files 12 12
Lines 5471 5475 +4
Branches 5471 5475 +4
==========================================
+ Hits 4497 4525 +28
+ Misses 775 757 -18
+ Partials 199 193 -6
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
1 participant
verify_documented_env_varsis red onmainright now (f08e3a451):#1527 deleted the fold-scale GEMV and
gemv_foldscale_enabled()after the +2.7% failed to reproduce.docs/research/dense-decode-megakernel-feasibility.mdstill names the env var that used to select it, so the guard sees a documented knob nothing reads — precisely the failure it was written to catch.The prose is already honest. §9.2 records the re-measurement (enabling it cost 2.9%, 28.62 → 29.46/29.54 ms/token against a 28.62/28.62 baseline) and the repro note says the entries were removed on 2026-08-20. Two of the three mentions carried that caveat; the Phase C lever paragraph did not, and now does.
So the fix is the allowlist entry the script asks for, plus the missing caveat — not a rewrite of someone else's research doc. The script's own contract is that every
KNOWN_UNIMPLEMENTEDname must stay accompanied by the not-implemented statement in the prose; after this commit all three mentions carry it.Found while validating the MatMul stack — this guard was the only non-pre-existing failure in my local matrix, and it turned out not to be mine.