Skip to content

fix(server): update shrink-test assertion to match post-#740 error message - #821

Merged
justinchuby merged 1 commit into
mainfrom
squad/fix-server-registry-shrink-test
Aug 12, 2026
Merged

justinchuby merged 1 commit into
mainfrom
squad/fix-server-registry-shrink-test

Conversation

@justinchuby

Copy link
Copy Markdown
Owner

Root Cause

Case (b): the test was over-fitted to an exact error string that legitimately changed.

Commit c7633eec4 ("Pool CUDA VMM physical handles across reservations", PR #740) refactored the server memory-shrink rejection path in crates/onnx-genai-server/src/state.rs. The old message was:

cannot satisfy lowered resource limit of {resolved} bytes: {domain} currently has {used} committed bytes

The new message is:

cannot satisfy lowered resource limit of {resolved} bytes: {domain} has {used} leased bytes but only {releasable} pooled-unmapped bytes are releasable

The test failed_runtime_shrink_preserves_policy_and_ledger_limit (added one commit earlier in 7fa131833, PR #744) asserted error.contains("committed bytes"), which broke when #740 landed.

Introducing commit: c7633eec4 (PR #740)

Fix

Update the assertion to check for "cannot satisfy lowered resource limit" — the stable prefix shared by all rejection paths in both state.rs and memory_authority.rs.

Why this is correct, not merely green: The test verifies two properties:

  1. Shrinking below committed memory fails with a limit-rejection error (not silently succeeds or panics).
  2. The policy and ledger limits are rolled back to their original values.

Property (1) is now checked by asserting the error contains the stable semantic prefix. Property (2) is checked by the subsequent assert_eq! lines on the snapshot and policy, which remain unchanged.

Validation

$ cargo test -p onnx-genai-server --lib --no-fail-fast
test result: ok. 183 passed; 0 failed; 2 ignored; 0 measured; 0 filtered out; finished in 10.42s

$ cargo clippy -p onnx-genai-server -- -D warnings
Finished (clean)

Integration tests (http, vlm_image_bundle) have 10 pre-existing failures unrelated to this change (they fail identically on origin/main).

…ssage

Commit c7633ee ("Pool CUDA VMM physical handles across reservations", #740)
changed the rejection message from "committed bytes" to "leased bytes" but
did not update the test assertion in
failed_runtime_shrink_preserves_policy_and_ledger_limit.

Update the assertion to check for the stable prefix
"cannot satisfy lowered resource limit" which appears in all rejection paths
and still detects the regression: a silent success or wrong error category.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@codecov

codecov Bot commented Aug 12, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 77.47%. Comparing base (e9c0ab6) to head (736d048).
⚠️ Report is 13 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #821      +/-   ##
==========================================
- Coverage   78.20%   77.47%   -0.74%     
==========================================
  Files         350      350              
  Lines      141311   141312       +1     
  Branches   141311   141312       +1     
==========================================
- Hits       110519   109476    -1043     
- Misses      26389    27436    +1047     
+ Partials     4403     4400       -3     
Flag Coverage Δ
cli-ort-linux 83.50% <ø> (ø)
cli-ort-windows 82.99% <ø> (-0.10%) ⬇️
mlas 80.96% <ø> (-0.37%) ⬇️
offline 77.17% <ø> (-0.78%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.
see 10 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
⚠️ tokenization/decode_tokens_per_second 6.36 ms 8.02 ms +26.1%
⚠️ sampling_latency/top_k_per_token 63.95 µs 79.28 µs +24.0%
⚠️ block_quantized_moe_cached_dense/mxfp4_uncached_expert_dequant_each_call/rows=1,H=256,I=256,E=4,top_k=1 418.36 µs 517.53 µs +23.7%
⚠️ matmul/large_generic_f32_threads=8/32x1024x1024 4.11 ms 5.07 ms +23.3%
⚠️ sampling_latency/greedy_per_token 3.19 µs 3.91 µs +22.5%
⚠️ matmul/small_generic_bf16_threads=8/1x256x256 42.49 µs 51.26 µs +20.6%
✅ matmul/medium_generic_bf16_threads=1/32x512x512 583.88 µs 663.58 µs +13.7%
✅ gather/medium_f32_threads=1-internal/32768 4.62 µs 5.23 µs +13.2%
✅ gather/small_f16_threads=1-internal/4096 541.3 ns 595.7 ns +10.1%
✅ matmul/small_generic_f16_threads=8/1x256x256 39.20 µs 41.62 µs +6.2%
✅ matmul/small_generic_bf16_threads=1/1x256x256 39.14 µs 41.55 µs +6.2%
✅ matmul/large_generic_f32_threads=1/32x1024x1024 10.02 ms 10.61 ms +5.9%
✅ gather/small_f32_threads=1-internal/4096 737.2 ns 777.1 ns +5.4%
✅ tokenization/encode_tokens_per_second 384.50 µs 403.05 µs +4.8%
✅ qwen3_sampling_processors/top_p_full_sort_after_top_k_baseline 3.35 ms 3.44 ms +2.7%
✅ add/large_f16_threads=1-internal/4194304 41.39 ms 42.29 ms +2.2%
✅ gather/medium_bf16_threads=1-internal/32768 2.84 µs 2.89 µs +1.9%
✅ gather/small_bf16_threads=1-internal/4096 515.9 ns 524.7 ns +1.7%
✅ block_quantized_matmul_cached_dense/mxfp4_uncached_dequant_each_call/1x1024x1024 701.82 µs 706.18 µs +0.6%
✅ qwen3_sampling_processors/top_k_partial_selection 146.97 µs 147.57 µs +0.4%
✅ block_quantized_moe_cached_dense/mxfp4_cached_dense_expert_repeated_call/rows=1,H=256,I=256,E=4,top_k=1 81.06 µs 80.55 µs -0.6%
✅ block_quantized_matmul_cached_dense/mxfp4_preexpanded_dense_oncelock_like_proxy/1x1024x1024 53.98 µs 53.45 µs -1.0%
✅ matmul/medium_generic_f16_threads=1/32x512x512 38.12 µs 37.67 µs -1.2%
✅ matmul/medium_generic_f16_threads=8/32x512x512 36.63 µs 35.93 µs -1.9%
✅ logit_processing/seven_processor_chain_per_step 329.50 µs 322.82 µs -2.0%
✅ sampling_latency/top_p_per_token 412.44 µs 401.14 µs -2.7%
✅ matmul/small_generic_f32_threads=1/1x256x256 43.44 µs 42.21 µs -2.8%
✅ matmul/large_generic_f16_threads=1/32x1024x1024 86.66 µs 83.46 µs -3.7%
✅ qwen3_sampling_processors/top_p_fast_after_top_k 558.35 µs 537.15 µs -3.8%
✅ qwen3_sampling_processors/top_k_full_sort_baseline 2.25 ms 2.15 ms -4.4%
✅ reduce_mean/large_f32_threads=1-internal/262144 1.02 ms 947.24 µs -6.9%
✅ matmul/small_generic_f32_threads=8/1x256x256 57.67 µs 53.30 µs -7.6%
✅ gather/medium_f16_threads=1-internal/32768 2.87 µs 2.65 µs -7.7%
✅ sampling_latency/min_p_per_token 243.76 µs 221.00 µs -9.3%
✅ qwen3_sampling_processors/top_k_top_p_full_sort_baseline 6.62 ms 5.84 ms -11.8%
✅ kv_cache/alloc_dealloc_pages 43.44 µs 38.01 µs -12.5%
✅ reduce_mean/medium_f32_threads=1-internal/65536 260.81 µs 226.44 µs -13.2%
✅ grammar_masking/llguidance_compute_mask/32 79.88 µs 68.99 µs -13.6%
✅ add/large_f32_threads=1-internal/4194304 46.81 ms 40.13 ms -14.3%
✅ gather/large_f16_threads=1-internal/131072 17.07 µs 14.60 µs -14.5%
✅ add/medium_bf16_threads=1-internal/262144 3.18 ms 2.72 ms -14.6%
✅ reduce_mean/small_f32_threads=1-internal/4096 16.51 µs 14.07 µs -14.8%
✅ add/small_bf16_threads=1-internal/1024 14.53 µs 12.36 µs -15.0%
🟢 add/medium_f16_threads=1-internal/262144 3.16 ms 2.66 ms -15.8%
🟢 qwen3_sampling_processors/top_k_top_p_fast 791.11 µs 665.45 µs -15.9%
🟢 matmul/medium_generic_f32_threads=1/32x512x512 3.13 ms 2.62 ms -16.5%
🟢 matmul/medium_generic_bf16_threads=8/32x512x512 686.09 µs 564.01 µs -17.8%
🟢 matmul/small_generic_f16_threads=1/1x256x256 39.32 µs 32.24 µs -18.0%
🟢 matmul/large_generic_f16_threads=8/32x1024x1024 117.31 µs 95.28 µs -18.8%
🟢 add/medium_f32_threads=1-internal/262144 3.27 ms 2.64 ms -19.2%
🟢 add/small_f32_threads=1-internal/1024 241.8 ns 192.8 ns -20.3%
🟢 add/large_bf16_threads=1-internal/4194304 49.86 ms 39.27 ms -21.2%
🟢 matmul/large_generic_bf16_threads=1/32x1024x1024 2.82 ms 2.19 ms -22.3%
🟢 add/small_f16_threads=1-internal/1024 17.68 µs 13.64 µs -22.8%
🟢 block_quantized_matmul_cached_dense/mxfp4_cached_dense_repeated_call/1x1024x1024 67.30 µs 49.58 µs -26.3%
🟢 gather/large_bf16_threads=1-internal/131072 18.54 µs 13.47 µs -27.4%
🟢 matmul/medium_generic_f32_threads=8/32x512x512 1.73 ms 1.18 ms -32.0%
🟢 gather/large_f32_threads=1-internal/131072 53.19 µs 33.03 µs -37.9%
🟢 matmul/large_generic_bf16_threads=8/32x1024x1024 2.70 ms 1.50 ms -44.4%

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: { 2.93 3.12 5.72 }
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)

@justinchuby
justinchuby marked this pull request as ready for review August 12, 2026 18:31
@justinchuby
justinchuby merged commit bfd66c7 into main Aug 12, 2026
10 of 15 checks passed
@justinchuby
justinchuby deleted the squad/fix-server-registry-shrink-test branch August 12, 2026 18:31
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