feat(BACKEND-BENCH-CUDA-SGLANG-PREFLIGHT): a third benchmark subject can be named, and the key that names it NAMES its checkpoint (#1594) - #1596
Merged
Conversation
…can be named, and the key it is named by reaches every path that keys on it (#1594) `tools/bench/run_serve_low.py` declared `--model-key` as `choices=("27", "35")`, and that value is the key of the evidence tree: it is a directory name in `BenchRun.corpus_path`, in `BenchRun.output_path`, and again in the two in-container paths `build_bench_command` hands the pinned `sglang.bench_serving`. #1574's subject, `r0b0tlab/Qwen3.8-27B-NVFP4-MTP-sm121` @ `36f717a2`, could therefore only be measured by filing its evidence under an existing subject's key. `MODEL_KEYS` is now the one definition the parser's `choices` reads, and `build_bench_command` refuses a key that is not in it, because the parser is not the only way in and an undeclared key would open an evidence tree nobody declared rather than fail. The third key is `q38mtp`, deliberately not digit-prefixed: `27` already names a DIFFERENT 27B checkpoint, `unsloth/Qwen3.6-27B-NVFP4`, and `online_gate.py`'s neighbouring `27`/`27n` pair records that two 27B checkpoints share no goldens and no comparable ratios. The failure this guards 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 a `choices` edit alone would not be enough and why the test does not read `choices`. It enters through `main()` on a real argv with a stubbed `subprocess.run` that resolves the container `--output-file` back through the bind mount the same command declared, so the host derivation and the container derivation must agree or the run cannot read its own result back. For every admitted key it asserts that all four derived paths carry that key and that no other admitted key's tree was named or created. Three negative mutations, each red then restored to the pre-taken sha256 `2531f899`: the corpus path hardcoded to `27`, refused as `missing corpus partition`; the container raw path alone hardcoded to `27`, which the corpus precondition cannot see and which surfaces as a host `FileNotFoundError` on the run's own result; and the key guard deleted. Focused suite 9/9, `tests/tools` 333/333. No workload shape, concurrency ladder, or timed semantic changed, and nothing starts a server. `docs/USAGE.md` is deliberately unedited: it documents no `tools/bench` harness at all, this flag reaches no shipped command, and adding the first `tools/bench` entry for one choice value would create an obligation rather than project a changed one. Closes #1594. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [Claude Code]
…point, so two subjects cannot share one evidence tree (#1594) Repairs the three code findings a fresh review raised against this branch, and its record finding; the fourth is filed as #1605 and named below. **A key that names no checkpoint is the same defect as a key nothing admits.** On the previous head `--model-key 27` accepted BOTH `890bdef7` (the `27` subject) and `36f717a2` (the `q38mtp` subject): each run exited 0, wrote a plausible raw result, and `summarize_serve_low.py` then aggregated two checkpoints as one subject. That is verbatim the failure #1594 exists to prevent, and this branch widened it, because `27` and `q38mtp` are both 27B NVFP4 checkpoints where `27` and `35` at least differed in size. `serve_low_common.MODEL_CHECKPOINTS` is now the one record of which weights a key means, mirroring `online_gate.py:244-255`, which already derived its `choices` from key->repository and key->revision maps. `admitted_model_keys` refuses to admit a key the registry does not carry, `require_model_checkpoint` binds `--model-revision` to the key, and the host must hold `<model-repo>/snapshots/<revision>` -- the tokenizer path the command hands the container, so a repository without it is the wrong checkpoint under the right name. `online_gate.py` now reads the same record instead of keeping a second copy of the same four pins; its admitted SET stays its own, because its goldens, batched-token bounds and model-gate contracts are keyed on exactly those four. Two further paths keyed on the subject and were missed. `build_dry_run_manifest` hardcoded a two-entry `"models"` block, so the driver `scripts/dgx-sglang-low-concurrency.sh --dry-run` wrote a `manifest.json` naming neither `q38mtp` nor its checkpoint; it is now derived from the admitted keys, which is also why the registry had to come first. `make_serve_low_corpus.py` declared `--model-key` with no `choices` and wrote it verbatim into the corpus manifest, so a corpus could claim a subject nothing declares; both its parser and `generate_corpus` now refuse, and the corpus's `--tokenizer-revision` is bound to its key too, because a corpus is only that subject's if its tokenizer is. Its admitted set is the whole registry rather than this campaign's three, since `online_gate.py` and `scripts/mxfp4-online-serving-grid.sh` build their corpora with the same tool. Seven cases added, each red first. Eight negative mutations, each printed as an applied diff and restored byte-identically against a pre-taken sha256: a fourth admitted key with no checkpoint (the tool now refuses to import at all, `unknown model key: 38`); `35` deleted from the admitted set and, separately, from the registry; the revision binding, the repository-snapshot binding, the corpus binding and the corpus `choices` each deleted; and the manifest re-listed as the two-entry literal. `tests/tools` 340/340. The spec was never touched by the previous head and is now reconciled: the `Shared checkpoint candidates` table carries the third subject with its repository and exact revision, its `## Now` no longer says any Qwen3.8-27B point is unsupported for want of an instrument, and the two `## Tests to port and add` rows describe what those files now assert. #1605 is filed and indexed, and deliberately not fixed here. `tests/tools/test_serve_low_client.py:135` asserts `_CompletionHandler.peak <= 2` and is flaky: 1 failure in 30 standalone runs here, `AssertionError: 3 not less than or equal to 2`, and `tests/CMakeLists.txt:12` registers all of `tests/tools` as one CTest target, so it reds CI at random. The number is the fixture's rather than the client's: `peak` counts handler threads inside `do_POST`, a window strictly wider than the client's in-flight window. Repairing it changes what the case measures, so it takes its own row, spec and fresh review. No public document is owed. `tools/bench`, `run_serve_low` and `--model-key` appear zero times in `docs/USAGE.md`, `docs/FEATURES.md` and `README.md`: this harness is documented nowhere public, so there is no projection to update. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [Claude Code]
localai-bot
marked this pull request as ready for review
August 21, 2026 20:35
…e the merge commit GitHub refused the squash with "the merge commit cannot be cleanly created". The conflict is `.agents/issue-index.md`: the file carries `merge=union` in `.gitattributes`, two branches each appended a row, and the forge does not run the union driver. Locally the driver applies and the merge is clean, so landing the merged head is the repair rather than a rewrite of either append. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [Claude Code]
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
tools/bench/run_serve_low.pydeclared--model-keyaschoices=("27", "35"), and that value is the key of the evidence tree: it is a directory name inBenchRun.corpus_path, inBenchRun.output_path, and again in the two in-container pathsbuild_bench_commandhands the pinnedsglang.bench_serving. #1574's subject,r0b0tlab/Qwen3.8-27B-NVFP4-MTP-sm121@36f717a2, could therefore only be measured by filing its evidence under an existing subject's key.The third key is
q38mtp, deliberately not digit-prefixed:27already names a DIFFERENT 27B checkpoint,unsloth/Qwen3.6-27B-NVFP4, andonline_gate.py's neighbouring27/27npair records that two 27B checkpoints share no goldens and no comparable ratios.A key that names no checkpoint is the same defect as a key nothing admits
The failure this guards is not a crash. It is a run that completes, writes a plausible raw result, and files it under another subject's key. A
choicesedit alone does not stop that, and neither does achoicesedit plus a fail-closed guard on the same list, which is what the first head of this branch shipped. Measured on that head:--model-key 27accepted BOTH890bdef7(the27subject) and36f717a2(theq38mtpsubject), each run exited 0, andsummarize_serve_low.pyaggregated them as one subject. This branch materially widened that hole rather than opening it, because27andq38mtpare both 27B NVFP4 checkpoints, where27and35at least differed in size.The mitigation already existed one file away.
tools/bench/online_gate.py:244-255derives itschoicesfrom key-to-repository and key-to-revision maps, so a key cannot be named there without naming its checkpoint. That record is nowserve_low_common.MODEL_CHECKPOINTS, read by every tool that keys on a subject:admitted_model_keysrefuses to admit a key the registry does not carry, so admitting a key and pinning the weights it means are one edit and cannot drift apart.require_model_checkpointbinds--model-revisionto the key.build_bench_commandadditionally requires the host to hold<model-repo>/snapshots/<revision>. That is the tokenizer path the command hands the container, so a repository directory without it is the wrong checkpoint under the right name, and refusing on the host turns a confusing in-container failure into one line.online_gate.pyreads the same record instead of keeping a second copy of the same four pins. Its admitted SET stays its own: its goldens,MAX_NUM_BATCHED_TOKENS,MAX_MODEL_LENand model-gate contracts are keyed on exactly27,27n,35andq3mxfp4.Two more paths keyed on the subject
build_dry_run_manifesthardcoded a two-entry"models"block, so aq38mtprun throughscripts/dgx-sglang-low-concurrency.sh --dry-runwrote amanifest.jsonnaming neither the subject nor its checkpoint. The manifest is now derived from the admitted keys, which is the second reason the registry had to come first.make_serve_low_corpus.pydeclared--model-keywith nochoicesat all and wrote the value verbatim into the corpus manifest, while the output directory came from an independent--out. Both its parser andgenerate_corpusnow refuse, and--tokenizer-revisionis bound to the key as well, because a corpus is that subject's corpus only if its tokenizer is. Its admitted set is the whole registry rather than this campaign's three, sinceonline_gate.py's planned corpus command andscripts/mxfp4-online-serving-grid.shbuild their corpora with the same tool and legitimately passq3mxfp4.Evidence
The tests enter through
main()on a real argv with a stubbedsubprocess.runthat resolves the container--output-fileback through the bind mount the same command declared, so the host derivation and the container derivation must agree or the run cannot read its own result back. For every admitted key they assert that all four derived paths carry that key and that no other admitted key's tree was named or created. The manifest case runs the campaign shell itself, because a manifest built by callingbuild_dry_run_manifestdirectly would prove the function and not that the driver reaches it. Each expectation is written out in the test rather than read back from the harness.Eleven negative mutations across the branch, each printed as an applied diff and restored byte-identically against a pre-taken sha256. Three from the first head: the corpus path hardcoded to
27, refused asmissing corpus partition; the container raw path alone hardcoded to27, which the corpus precondition cannot see and which surfaces as a hostFileNotFoundErroron the run's own result; and the key guard deleted. Eight from the repair: a fourth admitted key with no checkpoint, which now makes the tool refuse to import at all withunknown model key: 38;35deleted from the admitted set, and separately from the registry; the revision binding, the repository-snapshot binding, the corpus key binding and the corpuschoiceseach deleted; and the manifest re-listed as the two-entry literal. Every one is red.tests/tools340/340.Records
The spec
.agents/specs/cuda-sglang-low-concurrency.mdis reconciled with the tree, which the first head left untouched. ItsShared checkpoint candidatestable carries the third subject with its repository and exact revision and says plainly that it is not the27subject; its## Nowno longer reports any Qwen3.8-27B point as unsupported for want of an instrument, and separates what the harness can now name from the measurement and the equivalence classification that stay open; and the two## Tests to port and addrows describe what those files now assert.#1605 is filed and indexed, not fixed here.
tests/tools/test_serve_low_client.py:135asserts_CompletionHandler.peak <= 2and is genuinely flaky: 1 failure in 30 standalone runs here,AssertionError: 3 not less than or equal to 2, andtests/CMakeLists.txt:12registers all oftests/toolsas one CTest target, so it reds CI at random. The number is the fixture's, not the client's:peakcounts handler threads insidedo_POST, a window strictly wider than the client's in-flight window. Repairing it changes what the case measures, so under## Every change starts from an issueit takes its own row, spec and fresh review rather than riding inside a change about model keys.No public document is owed.
tools/bench,run_serve_lowand--model-keyappear zero times indocs/USAGE.md,docs/FEATURES.mdandREADME.md: this harness is documented nowhere public, so there is no projection to update, and adding the first entry for one flag would create an obligation rather than project a changed one. No workload shape, concurrency ladder or timed semantic changed, and nothing starts a server or a container.Closes #1594.
FOLLOWING_AGENTS_PROTOCOL
Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: AGENT:claude-opus-5 [Claude Code]