Skip to content

fix(engine): restore rustfmt on main (#1200 left a trailing blank line) - #1204

Closed
justinchuby wants to merge 1 commit into
mainfrom
squad/resch-fmt-main-1200
Closed

justinchuby wants to merge 1 commit into
mainfrom
squad/resch-fmt-main-1200

Conversation

@justinchuby

Copy link
Copy Markdown
Owner

cargo fmt --all --check fails on main as of #1200 (2f8ba9b88), which appended a trailing blank
line to the end of crates/onnx-genai-engine/src/pipeline/mod.rs:

Diff in crates/onnx-genai-engine/src/pipeline/mod.rs:3681:
         }
     }
 }
-

Rust quality is one of the two required checks on main, so until this lands every open PR in the
repository is blocked on a stray newline — including three of mine sitting on auto-merge.

The fix is the one-line deletion cargo fmt --all produces. Nothing else in the tree moved.

After: cargo fmt --all --check clean.

#1200 left a trailing blank line at the end of `pipeline/mod.rs`, so
`cargo fmt --all --check` fails on `main`. `Rust quality` is a required
check, which means every open PR is currently blocked on a stray newline.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@justinchuby
justinchuby enabled auto-merge (squash) August 18, 2026 04:43
justinchuby added a commit that referenced this pull request Aug 18, 2026
This branch merged `main` after #1200 landed a trailing blank line in
`pipeline/mod.rs`, which fails the required `Rust quality` check. #1204
fixes it on `main`; the identical one-line deletion here lets this PR go
green without waiting, and merges as a no-op once #1204 lands.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
justinchuby added a commit that referenced this pull request Aug 18, 2026
`cargo fmt --all -- --check` was red on `main`: #1214 inserted
`bind_response_tokens` into two `use super::output::{...}` lists without
reflowing them. Pure formatting, no behaviour change; `--check` is clean
afterwards.

Note on #1204, which reports a trailing blank line at
`pipeline/mod.rs:3681` left by #1200: that no longer reproduces.
`main`'s copy is 3702 lines and ends on `}`, and `cargo fmt --all --
--check` does not flag it. That PR looks stale rather than wrong.

Co-authored-by: justinchuby <223556219+Copilot@users.noreply.github.com>
Copilot-Session: d60eb808-7cc6-4abc-b48d-2a6dd3841624
@justinchuby

Copy link
Copy Markdown
Owner Author

Checked this against current main and it no longer reproduces. crates/onnx-genai-engine/src/pipeline/mod.rs is 3702 lines and its last line is } -- there is no trailing blank line at 3681 -- and cargo fmt --all -- --check does not flag the file. The diff here is against an older main.

--check was red on main, but for a different reason: #1214 inserted an import without reflowing two use lists in the CLI. Fixed in #1217. Closing this as stale; reopen if you can still reproduce the blank line.

auto-merge was automatically disabled August 18, 2026 05:44

Pull request was closed

@codecov

codecov Bot commented Aug 18, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.90%. Comparing base (af9ee4e) to head (6bc3c62).
⚠️ Report is 20 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##           main    #1204       +/-   ##
=========================================
+ Coverage      0   79.90%   +79.90%     
=========================================
  Files         0      359      +359     
  Lines         0   157892   +157892     
  Branches      0   157892   +157892     
=========================================
+ Hits          0   126162   +126162     
- Misses        0    27117    +27117     
- Partials      0     4613     +4613     
Flag Coverage Δ
mlas 85.39% <ø> (?)
offline 79.79% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.
see 359 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions

Copy link
Copy Markdown

⚠️ Benchmark Change Detected

Comparison of criterion micro-benchmarks: PR head vs merge-base, measured on the same runner in the same job (base first → PR second).

ℹ️ Absolute times are informational only — they vary with runner load. The % change column is the reliable signal because both sides ran under identical conditions.

Status Scenario Base PR Change
⚠️ gather/large_f16_threads=1-internal/131072 16.52 µs 19.47 µs +17.9%
⚠️ gather/medium_bf16_threads=1-internal/32768 2.45 µs 2.89 µs +17.6%
✅ gather/large_bf16_threads=1-internal/131072 14.87 µs 17.03 µs +14.5%
✅ gather/small_f32_threads=1-internal/4096 671.9 ns 730.9 ns +8.8%
✅ add/medium_f16_threads=1-internal/262144 106.63 µs 115.16 µs +8.0%
✅ reduce_mean/large_f32_threads=1-internal/262144 1.00 ms 1.07 ms +7.1%
✅ reduce_mean/small_f32_threads=1-internal/4096 16.49 µs 17.46 µs +5.9%
✅ matmul/medium_generic_f32_threads=1/32x512x512 2.50 ms 2.63 ms +5.0%
✅ matmul/medium_generic_bf16_threads=1/32x512x512 559.34 µs 577.75 µs +3.3%
✅ add/large_f16_threads=1-internal/4194304 1.77 ms 1.83 ms +3.1%
✅ gather/medium_f16_threads=1-internal/32768 2.55 µs 2.61 µs +2.5%
✅ matmul/large_generic_bf16_threads=1/32x1024x1024 2.15 ms 2.20 ms +2.5%
✅ add/medium_f32_threads=1-internal/262144 24.96 µs 25.43 µs +1.9%
✅ add/large_bf16_threads=1-internal/4194304 1.70 ms 1.72 ms +1.7%
✅ gather/large_f32_threads=1-internal/131072 38.54 µs 38.93 µs +1.0%
✅ block_quantized_moe_cached_dense/mxfp4_cached_dense_expert_repeated_call/rows=1,H=256,I=256,E=4,top_k=1 175.18 µs 176.90 µs +1.0%
✅ add/small_bf16_threads=1-internal/1024 453.1 ns 454.9 ns +0.4%
✅ matmul/large_generic_f32_threads=1/32x1024x1024 10.02 ms 9.95 ms -0.7%
✅ gather/medium_f32_threads=1-internal/32768 4.15 µs 4.12 µs -0.8%
✅ reduce_mean/medium_f32_threads=1-internal/65536 255.93 µs 253.37 µs -1.0%
✅ gather/small_f16_threads=1-internal/4096 514.1 ns 499.7 ns -2.8%
✅ add/medium_bf16_threads=1-internal/262144 107.61 µs 103.21 µs -4.1%
✅ gather/small_bf16_threads=1-internal/4096 492.6 ns 471.9 ns -4.2%
✅ tokenization/decode_tokens_per_second 7.85 ms 7.45 ms -5.1%
✅ matmul/medium_generic_f16_threads=1/32x512x512 38.37 µs 35.95 µs -6.3%
✅ tokenization/encode_tokens_per_second 435.28 µs 407.12 µs -6.5%
✅ sampling_latency/greedy_per_token 3.76 µs 3.47 µs -7.7%
✅ qwen3_sampling_processors/top_k_top_p_full_sort_baseline 6.18 ms 5.67 ms -8.3%
✅ add/small_f32_threads=1-internal/1024 236.8 ns 215.5 ns -9.0%
✅ sampling_latency/top_p_per_token 441.79 µs 395.56 µs -10.5%
✅ add/small_f16_threads=1-internal/1024 558.9 ns 500.1 ns -10.5%
✅ sampling_latency/top_k_per_token 60.71 µs 54.31 µs -10.5%
✅ matmul/large_generic_f16_threads=1/32x1024x1024 88.14 µs 78.81 µs -10.6%
✅ logit_processing/seven_processor_chain_per_step 377.51 µs 336.74 µs -10.8%
✅ qwen3_sampling_processors/top_k_top_p_fast 698.71 µs 612.32 µs -12.4%
✅ matmul/small_generic_bf16_threads=1/1x256x256 38.12 µs 33.34 µs -12.6%
✅ add/large_f32_threads=1-internal/4194304 763.78 µs 660.09 µs -13.6%
✅ matmul/small_generic_f16_threads=8/1x256x256 38.61 µs 33.33 µs -13.7%
✅ qwen3_sampling_processors/top_p_fast_after_top_k 562.20 µs 483.46 µs -14.0%
✅ matmul/small_generic_f32_threads=1/1x256x256 46.34 µs 39.75 µs -14.2%
✅ matmul/large_generic_f16_threads=8/32x1024x1024 108.60 µs 92.79 µs -14.6%
✅ matmul/small_generic_f16_threads=1/1x256x256 38.60 µs 32.94 µs -14.7%
✅ kv_cache/alloc_dealloc_pages 43.82 µs 37.25 µs -15.0%
🟢 matmul/medium_generic_f16_threads=8/32x512x512 45.67 µs 37.87 µs -17.1%
🟢 qwen3_sampling_processors/top_p_full_sort_after_top_k_baseline 4.02 ms 3.30 ms -18.0%
🟢 qwen3_sampling_processors/top_k_partial_selection 160.47 µs 131.27 µs -18.2%
🟢 matmul/small_generic_f32_threads=8/1x256x256 50.62 µs 41.07 µs -18.9%
🟢 matmul/small_generic_bf16_threads=8/1x256x256 40.03 µs 32.47 µs -18.9%
🟢 matmul/medium_generic_f32_threads=8/32x512x512 1.84 ms 1.39 ms -24.6%
🟢 block_quantized_moe_cached_dense/mxfp4_uncached_expert_dequant_each_call/rows=1,H=256,I=256,E=4,top_k=1 638.93 µs 461.54 µs -27.8%
🟢 matmul/large_generic_f32_threads=8/32x1024x1024 6.97 ms 4.82 ms -30.8%
🟢 qwen3_sampling_processors/top_k_full_sort_baseline 3.05 ms 1.99 ms -34.6%
🟢 matmul/medium_generic_bf16_threads=8/32x512x512 704.91 µs 437.06 µs -38.0%
🟢 matmul/large_generic_bf16_threads=8/32x1024x1024 2.33 ms 1.33 ms -42.9%
🟢 grammar_masking/llguidance_compute_mask/32 130.36 µs 70.87 µs -45.6%
🟢 block_quantized_matmul_cached_dense/mxfp4_uncached_dequant_each_call/1x1024x1024 1.21 ms 655.43 µs -45.7%
🟢 block_quantized_matmul_cached_dense/mxfp4_preexpanded_dense_oncelock_like_proxy/1x1024x1024 98.44 µs 51.55 µs -47.6%
🟢 block_quantized_matmul_cached_dense/mxfp4_cached_dense_repeated_call/1x1024x1024 128.31 µs 66.38 µs -48.3%
🟢 sampling_latency/min_p_per_token 455.85 µs 213.81 µs -53.1%

Visual flags: ⚠️ ≥ 15% slower, 🔴 ≥ 30% slower — calibrated against measured runner noise (~27% worst-case on multi-threaded matmul)

Host info
CPU: Apple M1 (Virtual)
Cores: 3
OS: Darwin 25.5.0 arm64
Rust: rustc 1.97.1 (8bab26f4f 2026-07-14)
Load avg: { 3.63 4.45 5.56 }
What this cannot catch
  • Regressions in code paths not covered by these benchmarks (e.g., end-to-end decode with a real model)
  • Sub-threshold regressions that compound over multiple PRs
  • Performance changes that only manifest under GPU execution
  • Latency changes in the ORT integration path (these benchmarks exercise the native Rust kernels)

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