diff --git a/.agents/issue-index.md b/.agents/issue-index.md index e02db07a1..7c29bfb76 100644 --- a/.agents/issue-index.md +++ b/.agents/issue-index.md @@ -543,6 +543,8 @@ rather than merged. `scripts/check-agent-record.py` gates both. | [#1575](https://github.com/mudler/vllm.cpp/issues/1575) | `SPEC-DFLASH2` | **`build-newest-gcc` has been RED on `main` since `5702d8f83`, and it is a recurrence of the class `tests/support/process_id.h` was created to close, not a missing include.** `tests/vllm/models/test_qwen3_dflash2_gguf.cpp:547` calls `::getpid()` while including nothing that declares it; the lane builds in a `gcc:16` container where libstdc++ no longer supplies the transitive ``, so it fails to COMPILE (`error: '::getpid' has not been declared; did you mean 'getpt'?`) while every local toolchain at gcc 13 stays green. The seam's own header predicts this: it says the spelling "was fixed once in three files and came back in five more, because each new loader test copies the temp-directory helper from the last one", which is exactly what W5's GGUF test did. The convention is otherwise unanimous — every other file in the tree naming `getpid` either includes `` or uses the helper, and the sibling `tests/vllm/entrypoints/test_dspark_draft_routing.cpp` uses the helper AND carries a comment warning not to reintroduce this class. ATTRIBUTED, not inferred: `build-newest-gcc` is absent from the baseline's failed list at `92406c620` and present at `5702d8f83`, the commit that added the file. MEASURED red-first in the lane's own container at base `947e5f648`, file sha256 `83bba319…`: unmodified `RED_RC=1` with the error byte-identical to CI; with the seam applied `GREEN_RC=0`, `compile_err=0`, `git diff --stat` confirming the edit applied; local gcc 13 `GCC13_RC=0`, so no regression on the shipped toolchain. FIXED IN FLOW with the portable spelling (`#include "support/process_id.h"`, `vllm_test::ProcessId()`) rather than ``, which would work on POSIX but re-copies the idiom the helper centralises and does not compile on MSVC. Found while measuring [#1464](https://github.com/mudler/vllm.cpp/issues/1464) at `origin/main` | bug | | [#1454](https://github.com/mudler/vllm.cpp/issues/1454) | `SPEC-MTP-GGUF` | **`test_qwen3_5_gguf_mtp.cpp` reported `Status: SUCCESS!` with `assertions: 0` on every CI run, and its one arithmetic guarantee was a tautology.** Both cases opened `if (path == nullptr) return;` on `VLLM_MTP_GGUF_MODEL`, and a bare `return` from a doctest case is a PASS: re-derived on a clean Release build at `947e5f648`, unset, the file printed `test cases: 2 \| 2 passed \| 0 failed \| 0 skipped`, `assertions: 0`, `Status: SUCCESS!`, exit 0, and printed nothing else. The variable is set nowhere in `.github/workflows/`, so that was the state of every run. Second defect in the same file: the comment at `:52` stated `num_hidden_layers + depth == block_count` and the line under it asserted `CHECK(c.num_hidden_layers > 0)`, true of every valid model. MEASURED, not argued: mutating `src/vllm/model_executor/models/qwen3_5_gguf_weights.cpp:889` to `c.num_hidden_layers = block_count;` compiled clean and left the file at 2/2 cases, 0 assertions, `SUCCESS!`, exit 0. FIXED IN FLOW. The invariant is now pinned **HERMETICALLY** on KV-only synthetic GGUFs carrying no weight bytes, so CI checks it every run rather than never - 65/1 (the shipped Qwen3.8-27B pair), 25/1 (the Qwen3.5-2B reference this suite was developed against) and 28/3, the third arm separating `- nextn` from `- 1` - plus a head-less arm asserting the key is NOT published, which is the half `NumMtpLayers` cannot express because it answers 1 for an absent key. The two env-gated cases stay, now skipping with a `MESSAGE` naming the variable as `test_gguf_mmproj_reach.cpp` does, and the live one re-derives the invariant from the file's own `block_count` kv. Unset 4 cases / 18 assertions / `SUCCESS!` / rc 0; live on `Qwen3.8-27B-Q4_K_M.gguf` 4 / 38 / `SUCCESS!` / rc 0. Both mutants now red (9/18 and 5/18, exit 1), compiled clean, restored against a pre-taken sha256. **The production line is CORRECT and was not touched**: `block_count - nextn` landed `1a4db5c3c`, the `mtp_num_hidden_layers` republication `493327b4e`. Related but distinct: [#821](https://github.com/mudler/vllm.cpp/issues/821) W2 (`0adeb8b0e`) pins the same arithmetic for the 27B artifact on a committed manifest in `tests/vllm/models/test_qwen38_27b_gguf_manifest.cpp`, and that gate DOES catch both mutants - so the invariant was not globally unpinned, it was unpinned in this row's own file | bug | | [#1434](https://github.com/mudler/vllm.cpp/issues/1434) | `GATE-DOC-CHECKPOINT-STATES` | **`scripts/check-doc-checkpoint.py` could not see `PARTIAL`, so 118 state cells could move with no gate observing them.** `STATES` (`:56-66`) is the whole definition of what a lifecycle state IS for the gate that enforces AGENTS.md's `docs/STATUS.md` / `docs/BENCHMARKS.md` / spec `## Now` triple, and `row_states` drops any row it cannot match. `lifecycle_moves` and `moved_rows` then iterate the AFTER map, so leaving the matched set is silent by construction. Re-derived at `947e5f648` (the report measured `63d87805c`): `PARTIAL` **118** cells and `ANCHOR-BACKFILL` **73**, against `DONE` 77 and `BLOCKED` 9 — `PARTIAL` is the second most used state in the matrices and the gate was blind to it. Over the seven tables `ROW_TABLES` actually reads, the resolved population goes from **153 rows to 226**, a 47.7 % widening. Two of the transitions the report names behave differently from its description, measured with scratch commits at `947e5f648` on an unmodified checker: `READY -> PARTIAL` rc **0** and `PARTIAL -> READY` rc **0** are the real blind spots, while the report's suggested `PARTIAL -> ACTIVE` already reds — by accident, reporting **`added as ACTIVE`** for a row that has existed for months, because it is absent from the BEFORE map. FIXED IN FLOW for `PARTIAL` only. **`ANCHOR-BACKFILL` is deliberately excluded**: `.agents/feature-matrix.md:14-17` defines it as a property of the RECORD (*a legacy implemented row without exact code, test and real-spec anchors*), `docs/STATUS.md` carries no such term and would have nothing true to write on a `DONE <-> ANCHOR-BACKFILL` move, and `REQUIRED["lifecycle"]` cannot demand the spec's `## Now` alone — so admitting it would demand a public-document edit with nothing to say, which is the exact shape `check-doc-checkpoint.py:4-17` records as the reason the file was rewritten (16 of 20 red CI runs, six hardcoded escape hatches). One row's resolved state moves and the move is a REPAIR: `KV-BLOCK-POOL` says `` `PARTIAL` (not `DONE`) `` in its prose and the last-match heuristic believed the parenthesis, resolving `DONE`. No pinned counter moves — `check-gate-commands.py` has its own `GATED_STATES` and `RUNNABLE_BASELINE` is keyed on matrix rows, `UNOWNED_HIGH_WATER` is unmoved because this row names an owner, and no matrix row or public document changes — which was measured, not assumed, because this is the [#1376](https://github.com/mudler/vllm.cpp/issues/1376) ratchet shape. Remainder listed under `## Owed` in [doc-checkpoint-lifecycle-states.md](specs/doc-checkpoint-lifecycle-states.md): `ANCHOR-BACKFILL` moves, `.agents/sglang-matrix.md` never entering `ROW_TABLES`, a row that leaves the matched set entirely, and a new row added directly as `PARTIAL` | bug | +| [#1594](https://github.com/mudler/vllm.cpp/issues/1594) | `BACKEND-BENCH-CUDA-SGLANG-PREFLIGHT` | **`run_serve_low.py` could not name a third benchmark subject, and the flag that would name one is the key of the evidence tree.** `--model-key` was `choices=("27", "35")` at `:663`, and the value is a directory name at `:75` (`evidence/corpus//`), `:81` (`evidence/raw///`) and again at `:350`/`:352` for the in-container paths the pinned `sglang.bench_serving` reads and writes. So [#1574](https://github.com/mudler/vllm.cpp/issues/1574)'s subject, `r0b0tlab/Qwen3.8-27B-NVFP4-MTP-sm121` @ `36f717a2`, could only be measured by mislabelling its evidence under an existing key. **The failure to prevent is not a crash**: it is a run that completes, writes a plausible raw result, and files it under another subject's key, which a `choices` edit alone would not catch. FIXED IN FLOW. `MODEL_KEYS` is now one definition the parser's `choices` and a new fail-closed `build_bench_command` refusal both read, and the third key is `q38mtp` -- deliberately not digit-prefixed, because `27` already names a DIFFERENT 27B checkpoint (`unsloth/Qwen3.6-27B-NVFP4`) and `online_gate.py`'s `27`/`27n` pair records that two 27B checkpoints share no goldens and no comparable ratios. The test enters through `main()` on a real argv with a stubbed `subprocess.run` that resolves the container `--output-file` back through the command's OWN bind mount, so the host derivation and the container derivation must agree or the run cannot read its own result back; it asserts, for every admitted key, that all four derived paths carry that key and that no other admitted key's tree was named or created. Three negative mutations recorded red and restored: the corpus path hardcoded to `27` (`missing corpus partition`), the container raw path alone hardcoded to `27` (host read `FileNotFoundError` -- the insidious one the corpus precondition cannot see), and the key guard deleted. No workload shape, concurrency ladder or timed semantic changed, and no server was started. `docs/USAGE.md` is deliberately unedited: it documents no `tools/bench` harness, this flag reaches no shipped command, and adding the first `tools/bench` entry for one choice value would be a new obligation rather than a projection of a changed one | gap | +| [#1605](https://github.com/mudler/vllm.cpp/issues/1605) | `BACKEND-BENCH-CUDA-SGLANG-PREFLIGHT` | **`test_serve_low_client.py`'s concurrency-cap case reds at random, and the number it reports belongs to the fixture rather than to the client.** `tests/tools/test_serve_low_client.py:135` asserts `_CompletionHandler.peak <= 2` after `run_usage_batch(..., max_concurrency=2)`. Measured at `f7d517c09`: **1 failure in 30** standalone runs of that one case, `AssertionError: 3 not less than or equal to 2`; the fresh review of [#1596](https://github.com/mudler/vllm.cpp/pull/1596) measured 2 in 8 on the same head. `tests/CMakeLists.txt:12` registers all of `tests/tools` as the CTest target `test_serve_low_tools`, so it reds CI at random on whatever diff is in flight, and it has done so since `e58858a91` with no issue naming it. The cap itself is structural and is NOT what fails: `run_usage_batch` submits into a `ThreadPoolExecutor(max_workers=max_concurrency)`. `peak` counts handler threads inside the fixture's `do_POST`, between `active += 1` and the `finally: active -= 1`, and that window is strictly WIDER than the client's -- `post_json` returns once `response.read()` has the body and closes the connection in its own `finally`, while the handler thread has yet to leave its `try` and retake the lock. A worker that returns and submits its next request in that gap makes a third handler live with two client threads. So the reported 3 is the instrument's own overlap, and reporting it as a breached concurrency cap points the reader at `run_usage_batch`, where the 3 did not come from. **NOT FIXED IN FLOW, deliberately**: the repair changes what the case measures (count the client's in-flight window, or hold the handler open until the client has read), which changes the guarantee it asserts, so under `## Every change starts from an issue` it takes the row, spec and fresh-review path rather than riding inside a model-key change. The lower bound `assertGreaterEqual(peak, 2)` is the half that proves parallelism happened and must survive any repair: widening the bound to `<= max_concurrency + 1` would make the case green and measure nothing. Found by the fresh review of [#1596](https://github.com/mudler/vllm.cpp/pull/1596) (F4), reproduced independently before filing | bug | | [#1595](https://github.com/mudler/vllm.cpp/issues/1595) | `BACKEND-TENSTORRENT-HOST-FREE-FORWARD` | `tests/vllm/models/test_qwen3_dflash2_gguf.cpp:547` calls `::getpid()` with no `` in its include block, so the TU does not compile under clang-20 (`no member named 'getpid' in the global namespace`); landed on main in the SPEC-DFLASH2 GGUF arm and blocks every post-merge rebuild on a clang host, including the TT card gates; fixed in-flow with the one-line missing include, matching the sibling-test convention | bug | | [#1576](https://github.com/mudler/vllm.cpp/issues/1576) | `LTX25-DEVICE-RESIDENCY` | **`test_ltx2_video`'s `span_slack` bound is a 250 µs wall-clock budget with an allowance for sanitizers and NONE for CPU load, and the asserted quantity swung 171x between two runs of ONE binary.** NOT CURRENTLY RED, and recorded before it becomes an intermittent nobody can attribute: `build-test-cpu` is green on `main` in run [32464632413](https://github.com/mudler/vllm.cpp/actions/runs/32464632413) at `483cd3198`, `100% tests passed, 0 tests failed out of 584`, with `test_ltx2_video` Passed in 201.56 s. `6b48edb2c` (GATE-CI-RED-REPAIR, [#1499](https://github.com/mudler/vllm.cpp/issues/1499)/[#1494](https://github.com/mudler/vllm.cpp/issues/1494)) repaired the suite and INTRODUCED the bound in the same change: `span_bound = min(kSpanSlackPerRecord * leaf_records, 0.5 * leaf_seconds)` with `kSpanSlackPerRecord` 250 µs (`tests/vllm/multimodal/test_ltx2_video.cpp:3827`, `:3830`), raised to 3 ms under either sanitizer (`:3822`, `:3825`). Measured at `origin/main` `6b48edb2c` on a detached worktree, CPU-only, x86_64, `BUILD_RC=0`, SAME binary both samples: in the full 102-case `ctest -j 1` run at load 82-110 the `artifacts.frames` leaf reported **3.223 ms** of slack on a 4.935 ms leaf (65.3%) against the 250 µs bound and the CHECK failed 13x over; the same case alone under `-tc=` at load ~90 reported **18.8 µs** on a 2.98 ms leaf (0.63%) and passed 13x under. The suite read `102 cases, 101 passed, 1 failed`, `4194 assertions, 5 failed`, every failure inside *"the three carrying phases contain their work and the load keeps its order"* -- `span_slack <= span_bound` (`:3909`) and `covered >= c.min_coverage * leaf_seconds` (`:3945`). Same commit, same tree: `test_ltx2_video` runs 201.56 s on the CI runner and took 2863.35 s on that box, 14x slower. The bound is a FIXED absolute time charged per instrument boundary against a leaf that can be milliseconds long, so whether it holds is a property of SCHEDULER LATENCY, and a descheduled thread is off-CPU for far more than 250 µs; the sanitizer arm already concedes the quantity is environment-sensitive and contention moves it the same way with no allowance. Cost is attribution, not the red: a reviewer who sees this fail cannot separate a genuine phase-instrumentation regression from a loaded runner, which is [#433](https://github.com/mudler/vllm.cpp/issues/433)'s class. NOT FIXED IN FLOW, deliberately: a bigger constant only moves the threshold at which the same ambiguity appears, and the candidate repairs (assert the containment RELATION rather than a duration, derive the budget from measured per-boundary cost on the running host, or gate the case on a quiescence precondition and report "not measured" instead of "failed") are a numerics/instrument decision owned by the LTX-2.5 residency lane rather than by the row that found it. Found while measuring [#1464](https://github.com/mudler/vllm.cpp/issues/1464) at `origin/main` | bug | | [#1385](https://github.com/mudler/vllm.cpp/issues/1385) | `GATE-CI-AARCH64-COVERAGE` | **CI builds 4 of 552 test targets on aarch64, and every fleet GPU box and two release bundles are aarch64.** Re-derived at `e2a9e035d`: `tests/CMakeLists.txt` defines **552** `vllm_cpp_add_test` targets plus 32 direct `add_test(NAME ...)` registrations, and the x86-64 lane ran **584** CTest entries with 3 skipped on run 32465485947. `build-test-cpu-arm64` (`.github/workflows/ci.yml:1096`) builds four ISA and kernel-tier targets and runs no `ctest` at all, so 0.72 % of the suite executes on the architecture the project measures, gates and ships on. Its own flag block (`:1110`) additionally sets `VLLM_CPP_BUILD_EXAMPLES=OFF` and `VLLM_CPP_SERVER=OFF`, which puts `examples/tokenize` and the `/v1/completions` parse segment out of reach of any target-list change. Decided in [ci-aarch64-coverage.md](specs/ci-aarch64-coverage.md): a curated subset is REJECTED because a stated sensitivity principle (weak memory model, `char` signedness, floating-point contraction, hash order) selects **291 of 552** targets, so half a suite costs most of a full build; a self-hosted fleet runner is REJECTED because it takes a leased box outside `rc`. Recommended: one new `build-test-cpu-arm64-full` job that builds everything and runs `ctest` serially, landing `schedule`+`workflow_dispatch` only with `continue-on-error`, then promoted per-PR once measured. Measured cost: **$0** (`timing` reports `total_ms: 0`; the repository is public), **+45-55 job-minutes** against a median of 357 per scheduled run, and **zero** added wall-clock while it finishes inside `cuda-fat-build`'s measured 123.0-minute median finish. It makes the hermetic tokenizer parity goldens execute on aarch64 for the first time, which is the `## Owed` item [prompt-token-divergence.md](specs/prompt-token-divergence.md) names, but the committed corpus carries only **30** combining marks over 99 lines against the 74-150 per prompt that produced the anomaly, so it is a necessary and not a sufficient probe | bug | diff --git a/.agents/specs/cuda-sglang-low-concurrency.md b/.agents/specs/cuda-sglang-low-concurrency.md index 730ef4305..8d8874e1c 100644 --- a/.agents/specs/cuda-sglang-low-concurrency.md +++ b/.agents/specs/cuda-sglang-low-concurrency.md @@ -72,12 +72,16 @@ scheduler/HTTP test and requires at least five request waves |---|---|---|---|---| | 27B dense | `unsloth/Qwen3.6-27B-NVFP4` at `890bdef7a42feba6d83b6e17a03315c694112f2a` | `Qwen3_5ForConditionalGeneration`; compressed-tensors `nvfp4-pack-quantized`, dynamic W4A4, group 16 | Qwen3.5 entry class ([model lines 2035-2048](https://github.com/sgl-project/sglang/blob/28b095c01005d4a3a2a5b637b7d028b07fba31b2/python/sglang/srt/models/qwen3_5.py#L2035-L2048)); compressed-tensors W4A4 creates packed weights/scales and dispatches FP4 GEMM ([scheme lines 32-169](https://github.com/sgl-project/sglang/blob/28b095c01005d4a3a2a5b637b7d028b07fba31b2/python/sglang/srt/layers/quantization/compressed_tensors/schemes/compressed_tensors_w4a4_nvfp4.py#L32-L169)) | Native path exists. It becomes equivalent only after exact load, tensor/scale audit, and 16-prompt token check. | | 35B-A3B MoE | `nvidia/Qwen3.6-35B-A3B-NVFP4` at `491c2f1ea524c639598bf8fa787a93fed5a6fbce` | `Qwen3_5MoeForConditionalGeneration`; ModelOpt `MIXED_PRECISION`: selected FP8 linears/activations plus W4A16 NVFP4 MoE/shared-expert/lm-head layers; no FP8 KV-cache declaration | SGLang parses non-Nemotron `MIXED_PRECISION` as `w4afp8` ([model-config lines 1017-1033](https://github.com/sgl-project/sglang/blob/28b095c01005d4a3a2a5b637b7d028b07fba31b2/python/sglang/srt/configs/model_config.py#L1017-L1033)); that config uses FP8 linears and a legacy W4A8 MoE layout ([w4afp8 lines 35-105](https://github.com/sgl-project/sglang/blob/28b095c01005d4a3a2a5b637b7d028b07fba31b2/python/sglang/srt/layers/quantization/w4afp8.py#L35-L105)). SGLang also has a per-layer ModelOpt mixed dispatcher ([lines 605-779](https://github.com/sgl-project/sglang/blob/28b095c01005d4a3a2a5b637b7d028b07fba31b2/python/sglang/srt/layers/quantization/modelopt_quant.py#L605-L779)), but its parser selects that path only for Nemotron-H. | Compatibility is unresolved and must be probed. Source alone does not justify an equivalent claim. | +| Qwen3.8 27B dense, MTP | `r0b0tlab/Qwen3.8-27B-NVFP4-MTP-sm121` at `36f717a22990e82c54c1d48ee77c491b87825680` | W4A16 NVFP4 `group_size` 16 on 193 modules, weight-only, plus a static per-tensor FP8 tower on 208 modules; `hf_quant_config.json` declares `kv_cache_quant_algo: "FP8"` while publishing no `k_scale`/`v_scale` | The same Qwen3.5 entry class as the 27B dense row above; the checkpoint's own compressed-tensors scheme differs | Added 2026-08-21 for [#1574](https://github.com/mudler/vllm.cpp/issues/1574), whose harness key is `q38mtp` ([#1594](https://github.com/mudler/vllm.cpp/issues/1594)). NOT the `27` subject: that key is `unsloth/Qwen3.6-27B-NVFP4`, a different 27B checkpoint with different quantization, so the two share no goldens and no comparable ratios. Unclassified for equivalence and unrun here; [bench-qwen38-27b-four-way.md](bench-qwen38-27b-four-way.md) owns its campaign. | -Both architecture IDs are registered, and SGLang explicitly permits +Both Qwen3.6 architecture IDs are registered, and SGLang explicitly permits `--language-only` for both Qwen3.5 wrappers ([server validation lines 3972-4020](https://github.com/sgl-project/sglang/blob/28b095c01005d4a3a2a5b637b7d028b07fba31b2/python/sglang/srt/server_args.py#L3972-L4020)). That establishes model-family and text-only support; it does not establish that -these two particular NVFP4 snapshots load with the same quantization semantics. +those two particular NVFP4 snapshots load with the same quantization semantics. +The Qwen3.8 row is a NAMEABLE subject in the harness, not a classified one: what +[#1594](https://github.com/mudler/vllm.cpp/issues/1594) added is a key that can +own its own evidence tree, and nothing here has loaded that checkpoint. ### Exact-loader preflight @@ -427,9 +431,9 @@ capture/JIT-contaminated trace do not. | Source test/spec | Local test | Required behavior | |---|---|---| -| SGLang `test_bench_serving_functionality.py` ([file](https://github.com/sgl-project/sglang/blob/28b095c01005d4a3a2a5b637b7d028b07fba31b2/test/registered/bench_fn/test_bench_serving_functionality.py)) | `tests/tools/test_serve_low_client.py` | Mock OpenAI SSE server: request body, concurrency cap, error propagation, raw detail retention. | +| SGLang `test_bench_serving_functionality.py` ([file](https://github.com/sgl-project/sglang/blob/28b095c01005d4a3a2a5b637b7d028b07fba31b2/test/registered/bench_fn/test_bench_serving_functionality.py)) | `tests/tools/test_serve_low_client.py` | Mock OpenAI SSE server: request body, concurrency cap, error propagation, raw detail retention. Plus the subject-identity contract ([#1594](https://github.com/mudler/vllm.cpp/issues/1594)), which the SGLang source has no equivalent of because it never files evidence by key: every admitted key routes its OWN corpus and raw paths and names no other key's tree, the dry-run manifest names exactly the admitted keys with each one's repository and exact revision, and a key refuses another subject's `--model-revision` and a `--model-repo` that holds no snapshot for it. | | SGLang native generation response ([lines 2640-2660](https://github.com/sgl-project/sglang/blob/28b095c01005d4a3a2a5b637b7d028b07fba31b2/python/sglang/srt/managers/tokenizer_manager.py#L2640-L2660)) | `tests/tools/test_serve_low_token_ids.py` | Native `output_ids` are captured without text round-trip, exactly 128 IDs are required, and any 16-prompt mismatch prevents a binding result. | -| SGLang custom dataset loader ([lines 54-147](https://github.com/sgl-project/sglang/blob/28b095c01005d4a3a2a5b637b7d028b07fba31b2/python/sglang/benchmark/datasets/custom.py#L54-L147)) and vLLM custom-seed tests ([file](https://github.com/vllm-project/vllm/blob/e24d1b24fe96a56ba8b0d653efa076d03eb95d6c/tests/benchmarks/test_custom_dataset_seed.py)) | `tests/tools/test_serve_low_corpus.py` | Same seed/partition is byte-identical; different partitions are disjoint; every prompt has 1024 IDs; common-prefix bound and manifest hashes hold. | +| SGLang custom dataset loader ([lines 54-147](https://github.com/sgl-project/sglang/blob/28b095c01005d4a3a2a5b637b7d028b07fba31b2/python/sglang/benchmark/datasets/custom.py#L54-L147)) and vLLM custom-seed tests ([file](https://github.com/vllm-project/vllm/blob/e24d1b24fe96a56ba8b0d653efa076d03eb95d6c/tests/benchmarks/test_custom_dataset_seed.py)) | `tests/tools/test_serve_low_corpus.py` | Same seed/partition is byte-identical; different partitions are disjoint; every prompt has 1024 IDs; common-prefix bound and manifest hashes hold. A corpus is also filed under a subject key, so the generator and its command line both refuse a key no subject declares, and refuse a tokenizer revision that is not the one that key names ([#1594](https://github.com/mudler/vllm.cpp/issues/1594)). | | SGLang metric calculation ([lines 968-1140](https://github.com/sgl-project/sglang/blob/28b095c01005d4a3a2a5b637b7d028b07fba31b2/python/sglang/bench_serving.py#L968-L1140)) | `tests/tools/test_serve_low_summary.py` | Hand-computed req/s, tok/s, TTFT, TPOT, ITL percentiles, repetition spreads, axis-wise best-floor ratios, and void-result propagation. | | vLLM streaming completion tests ([lines 271-284](https://github.com/vllm-project/vllm/blob/e24d1b24fe96a56ba8b0d653efa076d03eb95d6c/tests/entrypoints/openai/completion/test_completion.py#L271-L284)) | Extend [API-server tests](../../tests/vllm/entrypoints/openai/test_api_server.cpp) under `SERVE-ASYNC-LLM` | Streaming begins before generation completion, chunks concatenate to non-stream output, one-token interval, concurrent requests interleave safely. Initially SKIP the timing assertion with reason `SERVE-ASYNC-LLM not implemented`; do not delete it. | | Linux sampler contract | `tests/tools/test_process_memory_sampler.py` | Synthetic parent/children and short-lived PID cases; valid JSONL; no `N/A -> 0`; peak is monotonic. | @@ -764,7 +768,15 @@ Three owned rows, three different states. `SERVE-ASYNC-LLM` blocker is discharged and 27B-NVFP4 c8 and c16 are measured over three repetitions with zero errors. Not supported yet: c1, c2 and c4, the vLLM arm inside the same series, the 35B arm, the `SGLANG-ORACLE-CORRECT` - token-ID cross-check, paired nsys traces, and any Qwen3.8-27B point. + token-ID cross-check, and paired nsys traces. **No Qwen3.8-27B point is + measured either.** Since [#1594](https://github.com/mudler/vllm.cpp/issues/1594) + the harness can NAME that subject -- key `q38mtp`, + `r0b0tlab/Qwen3.8-27B-NVFP4-MTP-sm121` @ `36f717a2`, which owns its own + corpus and raw tree and cannot be run against another key's checkpoint -- so + what remains open is the measurement and the equivalence classification, not + the instrument. Its campaign is + [bench-qwen38-27b-four-way.md](bench-qwen38-27b-four-way.md) + ([#1574](https://github.com/mudler/vllm.cpp/issues/1574)). - `BACKEND-GATE-CUDA-SGLANG-PREFIX` is `READY`, unchanged. Its binding run still needs `KV-MAMBA-ALIGN` and exact v0.5.15 equivalence. diff --git a/tests/tools/test_serve_low_client.py b/tests/tools/test_serve_low_client.py index 9bcaebf91..10220a896 100644 --- a/tests/tools/test_serve_low_client.py +++ b/tests/tools/test_serve_low_client.py @@ -6,17 +6,22 @@ from __future__ import annotations +import contextlib import http.server +import io import json import pathlib import subprocess +import sys import tempfile import threading import time import unittest +import unittest.mock from tools.bench import run_serve_low from tools.bench.run_serve_low import ( + MODEL_KEYS, BenchRun, build_bench_command, build_dry_run_manifest, @@ -172,14 +177,15 @@ def test_pinned_client_command_and_dry_run_refuse_floating_inputs(self) -> None: with tempfile.TemporaryDirectory() as temporary: root = pathlib.Path(temporary) evidence = root / "evidence" - (root / "models").mkdir() + revision = CAMPAIGN_SUBJECTS["27"]["revision"] + (root / "models" / "snapshots" / revision).mkdir(parents=True) corpus = evidence / "corpus" / "27" corpus.mkdir(parents=True) (corpus / "c1-r1.jsonl").write_text("{}\n") run = BenchRun( image=SGLANG_IMAGE, model_repo=root / "models", - model_revision="revision", + model_revision=revision, evidence_root=evidence, model_key="27", engine="ours", @@ -275,5 +281,343 @@ def test_campaign_shell_dry_run_creates_manifest_without_gpu_work(self) -> None: self.assertIn("--pull=never", manifest["planned_commands"]["client"]) +# The subjects this campaign benches, and the EXACT checkpoint each key means. +# Spelled out here rather than read back from the harness: a test that reads the +# value it is checking pins nothing, and what #1594 is about is precisely that a +# key is a claim about which checkpoint produced the evidence under it. +CAMPAIGN_SUBJECTS = { + "27": { + "repository": "unsloth/Qwen3.6-27B-NVFP4", + "revision": "890bdef7a42feba6d83b6e17a03315c694112f2a", + }, + "35": { + "repository": "nvidia/Qwen3.6-35B-A3B-NVFP4", + "revision": "491c2f1ea524c639598bf8fa787a93fed5a6fbce", + }, + "q38mtp": { + "repository": "r0b0tlab/Qwen3.8-27B-NVFP4-MTP-sm121", + "revision": "36f717a22990e82c54c1d48ee77c491b87825680", + }, +} + + +def _synthetic_raw_result( + *, + requests: int = 80, + prompt_len: int = 1024, + output_len: int = 128, + max_concurrency: int = 1, +) -> dict: + """The smallest record `validate_raw_result` accepts as a complete leg.""" + + return { + "completed": requests, + "errors": [""] * requests, + "generated_texts": ["x"] * requests, + "input_lens": [prompt_len] * requests, + "itls": [[0.001] * (output_len - 1) for _ in range(requests)], + "max_concurrent_requests": max_concurrency, + "num_prompts": requests, + "output_lens": [output_len] * requests, + "ttfts": [0.01] * requests, + } + + +def _container_to_host(container: str, mounts: dict[str, pathlib.Path]) -> pathlib.Path: + """Resolve an in-container path through the bind mounts of the same command. + + `BenchRun.corpus_path` / `BenchRun.output_path` and the `container_corpus` + / `container_output` strings in `build_bench_command` derive from + `model_key` INDEPENDENTLY. Translating one back through the mount the same + command declared is what makes a disagreement between the two derivations + observable instead of silent. + """ + + for destination, source in sorted(mounts.items(), key=lambda item: -len(item[0])): + if container == destination or container.startswith(destination + "/"): + return source / container[len(destination):].lstrip("/") + raise AssertionError(f"{container} is not under any declared bind mount") + + +class _FakeCompletedProcess: + returncode = 0 + + +class _BenchThroughMainMixin: + """Drive `run_serve_low.main()` on a real argv, with docker stubbed out. + + Shared by the routing and the checkpoint-binding cases so both enter the + wrapper the way the campaign does: a unit test that built `BenchRun` by + hand would prove the dataclass and not that `main()` reaches it. + """ + + def _run_bench_through_main( + self, + root: pathlib.Path, + model_key: str, + engine: str = "ours", + model_revision: str | None = None, + ) -> list[str]: + if model_revision is None: + model_revision = CAMPAIGN_SUBJECTS[model_key]["revision"] + evidence = root / "evidence" + models = root / "models" + # The host shape of an HF cache repository: the container is handed + # `/models/gate/snapshots/` as its tokenizer, so the run can + # only succeed when the repository actually holds that revision. + (models / "snapshots" / model_revision).mkdir(parents=True, exist_ok=True) + corpus = evidence / "corpus" / model_key + corpus.mkdir(parents=True, exist_ok=True) + (corpus / "c1-r1.jsonl").write_text("{}\n") + captured: list[list[str]] = [] + + def fake_run(command, check=False): # mirrors subprocess.run's call + captured.append(list(command)) + mounts: dict[str, pathlib.Path] = {} + for index, item in enumerate(command): + if item != "--mount": + continue + fields = dict( + field.split("=", 1) + for field in command[index + 1].split(",") + if "=" in field + ) + mounts[fields["dst"]] = pathlib.Path(fields["src"]) + dataset = command[command.index("--dataset-path") + 1] + self.assertTrue(_container_to_host(dataset, mounts).is_file()) + written = _container_to_host( + command[command.index("--output-file") + 1], mounts + ) + written.parent.mkdir(parents=True, exist_ok=True) + written.write_text(json.dumps(_synthetic_raw_result()) + "\n") + return _FakeCompletedProcess() + + argv = [ + "run_serve_low.py", + "bench", + "--model-repo", str(models), + "--model-revision", model_revision, + "--evidence", str(evidence), + "--model-key", model_key, + "--engine", engine, + "--base-url", "http://127.0.0.1:30000", + "--concurrency", "1", + "--repetition", "1", + ] + with unittest.mock.patch.object(run_serve_low.subprocess, "run", fake_run): + with unittest.mock.patch.object(sys, "argv", argv): + with contextlib.redirect_stdout(io.StringIO()): + self.assertEqual(run_serve_low.main(), 0) + self.assertEqual(len(captured), 1) + return captured[0] + + +class ModelKeyEvidenceRoutingTest(_BenchThroughMainMixin, unittest.TestCase): + """A named subject must own its evidence, and only its own (#1594). + + `--model-key` is a label that keys the evidence tree, so the failure this + pins is not a crash. It is a run that completes, writes a plausible raw + result, and files it under ANOTHER subject's key -- which is why asserting + the parser's `choices` would not be enough. The assertions below enter + through `main()` on a real argv and read the paths the wrapper actually + derived, so a key accepted at the parser but not threaded to the corpus and + raw paths fails here. + """ + + def test_each_admitted_key_owns_its_corpus_and_raw_tree(self) -> None: + self.assertIn("q38mtp", MODEL_KEYS) + for model_key in MODEL_KEYS: + with self.subTest(model_key=model_key): + with tempfile.TemporaryDirectory() as temporary: + root = pathlib.Path(temporary) + command = self._run_bench_through_main(root, model_key) + evidence = root / "evidence" + dataset = command[command.index("--dataset-path") + 1] + output = command[command.index("--output-file") + 1] + + # The in-container paths `build_bench_command` hands + # the pinned client. + self.assertEqual(dataset, f"/evidence/corpus/{model_key}/c1-r1.jsonl") + self.assertEqual( + output, f"/evidence/raw/{model_key}/ours/c1-r1.jsonl" + ) + + # The host paths `BenchRun` derives. The raw file + # exists only because the container path resolved back to + # it through the command's own bind mount. + self.assertTrue( + ( + evidence / "raw" / model_key / "ours" / "c1-r1.jsonl" + ).is_file() + ) + + # No OTHER admitted subject's tree was touched or named. + for other in MODEL_KEYS: + if other == model_key: + continue + self.assertNotIn(f"/{other}/", dataset) + self.assertNotIn(f"/{other}/", output) + self.assertFalse((evidence / "raw" / other).exists()) + self.assertFalse((evidence / "corpus" / other).exists()) + + def test_the_third_key_is_not_a_spelling_of_an_existing_one(self) -> None: + """`27` already names a DIFFERENT 27B checkpoint, so the keys must not + collide as substrings of each other's evidence paths.""" + + for model_key in MODEL_KEYS: + for other in MODEL_KEYS: + if other == model_key: + continue + with self.subTest(model_key=model_key, other=other): + self.assertNotIn(other, model_key) + + def test_an_unadmitted_key_is_refused_at_both_the_parser_and_the_command( + self, + ) -> None: + with tempfile.TemporaryDirectory() as temporary: + root = pathlib.Path(temporary) + evidence = root / "evidence" + (root / "models").mkdir() + corpus = evidence / "corpus" / "38" + corpus.mkdir(parents=True) + (corpus / "c1-r1.jsonl").write_text("{}\n") + with contextlib.redirect_stderr(io.StringIO()): + with self.assertRaises(SystemExit) as parser_refusal: + run_serve_low._parser().parse_args( + [ + "bench", + "--model-repo", str(root / "models"), + "--model-revision", "revision", + "--evidence", str(evidence), + "--model-key", "38", + "--engine", "ours", + "--base-url", "http://127.0.0.1:30000", + "--concurrency", "1", + "--repetition", "1", + ] + ) + self.assertEqual(parser_refusal.exception.code, 2) + + # The library entry refuses the same key, so a caller that bypasses + # the parser cannot open an evidence tree nobody declared. + run = BenchRun( + image=SGLANG_IMAGE, + model_repo=root / "models", + model_revision="revision", + evidence_root=evidence, + model_key="38", + engine="ours", + base_url="http://127.0.0.1:30000", + concurrency=1, + repetition=1, + ) + with self.assertRaises(HarnessError): + build_bench_command(run) + + +class SubjectCheckpointBindingTest(_BenchThroughMainMixin, unittest.TestCase): + """A key must NAME its checkpoint, everywhere the key is accepted (#1594). + + `--model-key` is the identity `summarize_serve_low.py` aggregates by, so + two different checkpoints accepted under one key produce one summary that + silently mixes two subjects. Nothing here starts a server or a container: + the refusals are host-side preconditions and the manifest is the dry run's. + """ + + def _dry_run_manifest(self, claim: pathlib.Path) -> dict: + """The manifest the campaign driver itself writes. + + The shell is the production entry point for `plan`; a manifest built by + calling `build_dry_run_manifest` directly would prove the function and + not that the driver reaches it. + """ + + repo = pathlib.Path(__file__).resolve().parents[2] + subprocess.run( + [ + str(repo / "scripts" / "dgx-sglang-low-concurrency.sh"), + "--dry-run", + "--claim-root", str(claim), + "--vllm-cpp-sha", "c" * 40, + ], + check=True, + capture_output=True, + text=True, + ) + return json.loads( + (claim / "evidence" / ("c" * 40) / "manifest.json").read_text() + ) + + def test_the_dry_run_manifest_names_every_admitted_subject(self) -> None: + """The manifest is the evidence tree's own record of what it measured. + + A key admitted by the parser but absent from this block writes a + `manifest.json` that names neither the subject nor its checkpoint, so + the run's own provenance record cannot say what it ran. + """ + + with tempfile.TemporaryDirectory() as temporary: + manifest = self._dry_run_manifest(pathlib.Path(temporary) / "claim") + self.assertEqual(manifest["models"], CAMPAIGN_SUBJECTS) + # Neither direction may drift: an admitted key with no manifest + # entry, and a manifest entry for a key nothing admits, are both + # a subject whose evidence tree nobody declared. + self.assertEqual(sorted(manifest["models"]), sorted(MODEL_KEYS)) + + def test_a_key_refuses_another_admitted_subjects_checkpoint(self) -> None: + """The reviewer's #1596 demonstration, inverted into a gate. + + On the unbound harness `27` accepted BOTH `890bdef7` and `36f717a2`, + each run exited 0, and the summary then aggregated two checkpoints as + one subject. Each pair below is that exact run. + """ + + for model_key, foreign in (("27", "q38mtp"), ("q38mtp", "27"), ("35", "27")): + with self.subTest(model_key=model_key, foreign=foreign): + with tempfile.TemporaryDirectory() as temporary: + with self.assertRaises(HarnessError) as refusal: + self._run_bench_through_main( + pathlib.Path(temporary), + model_key, + model_revision=CAMPAIGN_SUBJECTS[foreign]["revision"], + ) + self.assertIn( + CAMPAIGN_SUBJECTS[model_key]["revision"], + str(refusal.exception), + ) + + def test_a_key_refuses_a_repository_that_lacks_its_snapshot(self) -> None: + """`--model-repo` is bound to the key too, not only `--model-revision`. + + The command hands the container `/models/gate/snapshots/` as + its tokenizer, so a repository directory that does not hold the key's + revision is the wrong checkpoint under the right name. + """ + + with tempfile.TemporaryDirectory() as temporary: + root = pathlib.Path(temporary) + evidence = root / "evidence" + corpus = evidence / "corpus" / "27" + corpus.mkdir(parents=True) + (corpus / "c1-r1.jsonl").write_text("{}\n") + models = root / "models" + (models / "snapshots" / CAMPAIGN_SUBJECTS["35"]["revision"]).mkdir( + parents=True + ) + run = BenchRun( + image=SGLANG_IMAGE, + model_repo=models, + model_revision=CAMPAIGN_SUBJECTS["27"]["revision"], + evidence_root=evidence, + model_key="27", + engine="ours", + base_url="http://127.0.0.1:30000", + concurrency=1, + repetition=1, + ) + with self.assertRaises(HarnessError): + build_bench_command(run) + + if __name__ == "__main__": unittest.main() diff --git a/tests/tools/test_serve_low_corpus.py b/tests/tools/test_serve_low_corpus.py index 2c9b41560..43b40508b 100644 --- a/tests/tools/test_serve_low_corpus.py +++ b/tests/tools/test_serve_low_corpus.py @@ -7,13 +7,25 @@ from __future__ import annotations +import contextlib +import io import json import pathlib +import sys import tempfile import unittest +import unittest.mock +from tools.bench import make_serve_low_corpus from tools.bench.make_serve_low_corpus import CorpusConfig, generate_corpus -from tools.bench.serve_low_common import read_jsonl, sha256_file +from tools.bench.serve_low_common import HarnessError, read_jsonl, sha256_file + +# A corpus is consumed as `evidence/corpus//`, so the key is the claim that +# these prompts were tokenized by THAT subject's tokenizer. The expectation is +# written out here rather than read back from the tool under test. +_SUBJECT_KEY = "27" +_SUBJECT_REVISION = "890bdef7a42feba6d83b6e17a03315c694112f2a" +_FOREIGN_REVISION = "36f717a22990e82c54c1d48ee77c491b87825680" class CharacterTokenizer: @@ -35,8 +47,10 @@ def _files(root: pathlib.Path) -> dict[str, bytes]: class CorpusTests(unittest.TestCase): def _config(self, seed: int = 7) -> CorpusConfig: return CorpusConfig( - model_key="fixture", - tokenizer_revision="fixture-rev", + # A declared subject and ITS revision: a corpus is filed under the + # key, so the generator refuses a key that names no checkpoint. + model_key=_SUBJECT_KEY, + tokenizer_revision=_SUBJECT_REVISION, seed=seed, target_input_len=64, output_len=8, @@ -100,5 +114,85 @@ def test_different_seed_changes_the_selected_corpus(self) -> None: ) +class CorpusSubjectKeyTests(unittest.TestCase): + """The corpus is filed under a key, so the key must name a real subject. + + `make_serve_low_corpus.py` writes `manifest.json["model_key"]` verbatim and + takes its output directory from an independent `--out`, so an undeclared + key produced a corpus that claims a subject nothing declares -- the same + defect as the harness's `--model-key` in a second file (#1594). + """ + + def _config(self, **overrides) -> CorpusConfig: + fields = { + "model_key": _SUBJECT_KEY, + "tokenizer_revision": _SUBJECT_REVISION, + "seed": 7, + "target_input_len": 64, + "output_len": 8, + "requests_per_partition": 3, + "warmup_requests": 2, + "concurrencies": (1, 2), + "repetitions": 2, + "common_prefix_limit": 16, + } + fields.update(overrides) + return CorpusConfig(**fields) + + def test_the_generator_refuses_a_key_no_subject_declares(self) -> None: + with tempfile.TemporaryDirectory() as temporary: + with self.assertRaises(HarnessError): + generate_corpus( + CharacterTokenizer(), + pathlib.Path(temporary) / "out", + self._config(model_key="fixture"), + tokenizer_sha256="a" * 64, + ) + + def test_the_generator_refuses_another_subjects_tokenizer(self) -> None: + """A corpus is this subject's only if its tokenizer is.""" + + with tempfile.TemporaryDirectory() as temporary: + with self.assertRaises(HarnessError): + generate_corpus( + CharacterTokenizer(), + pathlib.Path(temporary) / "out", + self._config(tokenizer_revision=_FOREIGN_REVISION), + tokenizer_sha256="a" * 64, + ) + + def test_the_command_line_refuses_a_key_no_subject_declares(self) -> None: + """The parser refuses before the tokenizer is even opened. + + This is the entry point `online_gate.py`'s planned corpus command and + `scripts/mxfp4-online-serving-grid.sh` both invoke. + """ + + with tempfile.TemporaryDirectory() as temporary: + root = pathlib.Path(temporary) + argv = [ + "make_serve_low_corpus.py", + "--tokenizer-json", str(root / "tokenizer.json"), + "--tokenizer-revision", _SUBJECT_REVISION, + "--model-key", "38", + "--out", str(root / "out"), + ] + with unittest.mock.patch.object(sys, "argv", argv): + with contextlib.redirect_stderr(io.StringIO()) as stderr: + with self.assertRaises(SystemExit) as refusal: + make_serve_low_corpus.main() + self.assertEqual(refusal.exception.code, 2) + self.assertIn("--model-key", stderr.getvalue()) + + def test_an_admitted_key_still_generates_its_corpus(self) -> None: + with tempfile.TemporaryDirectory() as temporary: + out = pathlib.Path(temporary) / "out" + manifest = generate_corpus( + CharacterTokenizer(), out, self._config(), tokenizer_sha256="a" * 64 + ) + self.assertEqual(manifest["model_key"], _SUBJECT_KEY) + self.assertEqual(manifest["tokenizer_revision"], _SUBJECT_REVISION) + + if __name__ == "__main__": unittest.main() diff --git a/tools/bench/make_serve_low_corpus.py b/tools/bench/make_serve_low_corpus.py index 9cc2f6fb2..3f6cd951a 100755 --- a/tools/bench/make_serve_low_corpus.py +++ b/tools/bench/make_serve_low_corpus.py @@ -18,14 +18,22 @@ from tools.bench.serve_low_common import ( HarnessError, + MODEL_CHECKPOINTS, SGLANG_COMMIT, canonical_json, + require_model_checkpoint, sha256_bytes, sha256_file, write_json_atomic, write_jsonl_atomic, ) +# Every subject the registry names may have a corpus built for it. This is the +# whole registry rather than one harness's subset, because both the serve-low +# harness and `online_gate.py` build their corpora with this tool; what must not +# be possible is a corpus filed under a key NO subject declares (#1594). +MODEL_KEYS = tuple(MODEL_CHECKPOINTS) + class Tokenizer(Protocol): def encode(self, text: str) -> list[int]: ... @@ -147,6 +155,14 @@ def generate_corpus( *, tokenizer_sha256: str, ) -> dict: + # The corpus is consumed as `evidence/corpus//`, and the key is + # written verbatim into this manifest, so it is a claim about which + # tokenizer produced these prompt IDs. The parser is not the only way in, + # and a corpus filed under an undeclared key -- or under a real key it was + # not tokenized for -- is a subject nobody can reproduce (#1594). + require_model_checkpoint( + config.model_key, config.tokenizer_revision, field="--tokenizer-revision" + ) if config.target_input_len < 2 or config.output_len < 4: raise HarnessError("input length must be >=2 and output length must be >=4") if config.requests_per_partition <= 0 or config.warmup_requests <= 0: @@ -227,7 +243,7 @@ def _parse_args() -> argparse.Namespace: parser = argparse.ArgumentParser() parser.add_argument("--tokenizer-json", type=pathlib.Path, required=True) parser.add_argument("--tokenizer-revision", required=True) - parser.add_argument("--model-key", required=True) + parser.add_argument("--model-key", choices=MODEL_KEYS, required=True) parser.add_argument("--out", type=pathlib.Path, required=True) parser.add_argument("--seed", type=int, default=0) parser.add_argument("--target-input-len", type=int, default=1024) diff --git a/tools/bench/online_gate.py b/tools/bench/online_gate.py index 5fa081456..679a6532b 100755 --- a/tools/bench/online_gate.py +++ b/tools/bench/online_gate.py @@ -44,8 +44,10 @@ VLLM_COMMIT, VLLM_DISTRIBUTION_VERSION, VLLM_ORACLE_VERSION, + admitted_model_keys, assert_oracle_commit, canonical_json, + model_checkpoint, read_jsonl, require_complete_request_set, require_number, @@ -241,17 +243,19 @@ def server_binary(build_dir: pathlib.Path) -> pathlib.Path: # repo @890bdef7 ships a BF16 lm_head and a different mixed-precision layout; # the nvidia ModelOpt repo @0893e160 is NVFP4 MLP + FP8 W8A8 tower + NVFP4 head. # They do not share goldens and their ratios are not comparable. +# +# The subjects THIS gate runs, and the checkpoint each key means is read from +# `serve_low_common.MODEL_CHECKPOINTS` rather than copied here. That record is +# shared with the serve-low harness and the corpus generator, so admitting a key +# and pinning the weights it names is one edit and the two cannot drift (#1594). +# The admitted SET stays per harness: this gate has goldens, batched-token +# bounds and model-gate contracts keyed on exactly these four. +ONLINE_GATE_MODEL_KEYS = admitted_model_keys("27", "27n", "35", "q3mxfp4") MODEL_REVISIONS = { - "27": "890bdef7a42feba6d83b6e17a03315c694112f2a", - "27n": "0893e1606ff3d5f97a441f405d5fc541a6bdf404", - "35": "491c2f1ea524c639598bf8fa787a93fed5a6fbce", - "q3mxfp4": "b3e7ab32f7225ca779b3dbf6ef4ecefeb6de9b47", + key: model_checkpoint(key)["revision"] for key in ONLINE_GATE_MODEL_KEYS } MODEL_REPOSITORIES = { - "27": "unsloth/Qwen3.6-27B-NVFP4", - "27n": "nvidia/Qwen3.6-27B-NVFP4", - "35": "nvidia/Qwen3.6-35B-A3B-NVFP4", - "q3mxfp4": "Yi30/Qwen3-8B-MXFP4", + key: model_checkpoint(key)["repository"] for key in ONLINE_GATE_MODEL_KEYS } MAX_NUM_SEQS = 32 MAX_NUM_BATCHED_TOKENS = {"27": 2048, "27n": 2048, "35": 8192, "q3mxfp4": 2048} diff --git a/tools/bench/run_serve_low.py b/tools/bench/run_serve_low.py index 4b9d8b87f..03d4ac5ae 100755 --- a/tools/bench/run_serve_low.py +++ b/tools/bench/run_serve_low.py @@ -30,14 +30,28 @@ SGLANG_COMMIT, SGLANG_IMAGE, VLLM_COMMIT, + admitted_model_keys, canonical_json, + model_checkpoint, read_jsonl, require_complete_request_set, + require_model_checkpoint, require_number, write_json_atomic, ) +# The subjects this campaign benches. Each is admitted only once +# `serve_low_common.MODEL_CHECKPOINTS` says which weights it means, so the key +# that labels an evidence tree and the checkpoint that filled it are one edit +# and cannot drift apart. "27" is `unsloth/Qwen3.6-27B-NVFP4`, so the third +# subject -- also a 27B NVFP4 checkpoint -- is `q38mtp` rather than anything +# digit-prefixed: `online_gate.py`'s neighbouring "27"/"27n" pair records that +# two 27B checkpoints share no goldens and no comparable ratios, and a key that +# reads as a spelling of another one is that mistake waiting in a path. +MODEL_KEYS = admitted_model_keys("27", "35", "q38mtp") + + @dataclasses.dataclass(frozen=True) class HttpResult: status: int @@ -340,12 +354,31 @@ def build_bench_command(run: BenchRun) -> list[str]: raise HarnessError("benchmark image does not match the digest pin") if run.engine not in {"ours", "sglang", "vllm"}: raise HarnessError(f"unknown engine arm: {run.engine}") + # The parser is not the only way in, and an undeclared key would silently + # open an evidence tree nobody declared rather than fail (#1594). + if run.model_key not in MODEL_KEYS: + raise HarnessError(f"unknown model key: {run.model_key}") + # And an ADMITTED key beside a free-form revision is the same failure with + # the paperwork in order: the run completes, files a plausible result under + # this subject's key, and `summarize_serve_low.py` averages two checkpoints + # into one published subject. + require_model_checkpoint( + run.model_key, run.model_revision, field="--model-revision" + ) if run.concurrency not in {1, 2, 4, 8, 16} or run.repetition not in {1, 2, 3}: raise HarnessError("run is outside the canonical concurrency/repetition grid") if not run.corpus_path.is_file(): raise HarnessError(f"missing corpus partition: {run.corpus_path}") if not run.model_repo.is_dir(): raise HarnessError(f"missing model repository: {run.model_repo}") + # `--model-repo` is bound to the key through the same revision: the command + # below hands the container `/models/gate/snapshots/`, so a + # repository that does not hold this subject's snapshot is the wrong + # checkpoint under the right name. Refusing on the host also turns a + # confusing in-container tokenizer failure into one line here. + snapshot = run.model_repo / "snapshots" / run.model_revision + if not snapshot.is_dir(): + raise HarnessError(f"model repository has no snapshot {snapshot}") backend = "sglang-oai" if run.engine == "sglang" else "vllm" container_corpus = f"/evidence/corpus/{run.model_key}/{run.corpus_path.name}" container_output = ( @@ -476,16 +509,11 @@ def build_dry_run_manifest( "python": platform.python_version(), }, "image": image, - "models": { - "27": { - "repository": "unsloth/Qwen3.6-27B-NVFP4", - "revision": "890bdef7a42feba6d83b6e17a03315c694112f2a", - }, - "35": { - "repository": "nvidia/Qwen3.6-35B-A3B-NVFP4", - "revision": "491c2f1ea524c639598bf8fa787a93fed5a6fbce", - }, - }, + # Derived from the admitted keys, not listed again: this block is the + # evidence tree's own record of which checkpoint each subject is, so a + # key admitted here and absent there would write a manifest that names + # neither the subject nor its weights (#1594). + "models": {key: model_checkpoint(key) for key in MODEL_KEYS}, "pending_preconditions": [ "host_idle_proof", "image_platform_digest_and_revision", @@ -660,7 +688,7 @@ def _parser() -> argparse.ArgumentParser: bench.add_argument("--model-repo", type=pathlib.Path, required=True) bench.add_argument("--model-revision", required=True) bench.add_argument("--evidence", type=pathlib.Path, required=True) - bench.add_argument("--model-key", choices=("27", "35"), required=True) + bench.add_argument("--model-key", choices=MODEL_KEYS, required=True) bench.add_argument("--engine", choices=("ours", "vllm", "sglang"), required=True) bench.add_argument("--base-url", required=True) bench.add_argument("--concurrency", type=int, required=True) diff --git a/tools/bench/serve_low_common.py b/tools/bench/serve_low_common.py index 40b937b18..fe0caf0bf 100644 --- a/tools/bench/serve_low_common.py +++ b/tools/bench/serve_low_common.py @@ -148,6 +148,96 @@ def assert_oracle_commit(runtime_version: object) -> None: ) +# EVERY BENCHMARK SUBJECT, AND THE EXACT CHECKPOINT ITS KEY MEANS. +# +# A model key is a directory name -- `evidence/corpus//` and +# `evidence/raw///` -- and `summarize_serve_low.py` aggregates by +# it. So a key is a SUBJECT IDENTITY and never a size, and two checkpoints +# accepted under one key do not crash: they complete, write plausible raw +# results, and are averaged into one published number for a subject that never +# ran (#1594). A key that names no checkpoint has the same shape, because +# nothing then says which weights produced the evidence under it. +# +# ONE record, read by every tool that keys on a subject, so that admitting a key +# and pinning its checkpoint cannot be separate edits and cannot drift apart. +# "27" and "27n" are DIFFERENT MODELS rather than two spellings of one, and +# "q38mtp" is a THIRD 27B checkpoint that is neither -- which is why it is +# deliberately not digit-prefixed, so no evidence path can be read as another +# subject's. +MODEL_CHECKPOINTS: dict[str, dict[str, str]] = { + "27": { + "repository": "unsloth/Qwen3.6-27B-NVFP4", + "revision": "890bdef7a42feba6d83b6e17a03315c694112f2a", + }, + "27n": { + "repository": "nvidia/Qwen3.6-27B-NVFP4", + "revision": "0893e1606ff3d5f97a441f405d5fc541a6bdf404", + }, + "35": { + "repository": "nvidia/Qwen3.6-35B-A3B-NVFP4", + "revision": "491c2f1ea524c639598bf8fa787a93fed5a6fbce", + }, + "q3mxfp4": { + "repository": "Yi30/Qwen3-8B-MXFP4", + "revision": "b3e7ab32f7225ca779b3dbf6ef4ecefeb6de9b47", + }, + # #1574's subject: `r0b0tlab/Qwen3.8-27B-NVFP4-MTP-sm121`, a 27B NVFP4 + # checkpoint that is NOT the "27" subject. + "q38mtp": { + "repository": "r0b0tlab/Qwen3.8-27B-NVFP4-MTP-sm121", + "revision": "36f717a22990e82c54c1d48ee77c491b87825680", + }, +} + + +def model_checkpoint(model_key: str) -> dict[str, str]: + """Return the repository and revision *model_key* names, or refuse. + + Fails closed on a key the registry does not carry, and on a registry entry + that does not pin a full commit ID: a floating revision is a subject whose + weights can be re-quantized in place under an unchanged name. + """ + + entry = MODEL_CHECKPOINTS.get(model_key) + if entry is None: + raise HarnessError(f"unknown model key: {model_key}") + revision = entry["revision"] + if len(revision) != 40 or any(char not in "0123456789abcdef" for char in revision): + raise HarnessError( + f"model key {model_key} does not pin a full checkpoint revision" + ) + return dict(entry) + + +def admitted_model_keys(*model_keys: str) -> tuple[str, ...]: + """Return *model_keys* once each has been proven to name a checkpoint. + + A harness declares the subjects it benches by calling this, so a key cannot + enter an admitted set -- an argparse `choices`, an evidence tree, a manifest + -- without the registry saying which weights it means. + """ + + for model_key in model_keys: + model_checkpoint(model_key) + return model_keys + + +def require_model_checkpoint(model_key: str, revision: str, *, field: str) -> None: + """Require *revision* to be the revision *model_key* names. + + This is the binding #1594 exists for: without it `--model-key` is a free + label beside a free `--model-revision`, and the pair can disagree while the + run exits 0. + """ + + expected = model_checkpoint(model_key)["revision"] + if revision != expected: + raise HarnessError( + f"{field}={revision!r} is not the checkpoint model key " + f"{model_key!r} names ({expected})" + ) + + def canonical_json(value: Any) -> str: """Return deterministic UTF-8-safe JSON used by all hashed artifacts."""