diff --git a/.agents/issue-index.md b/.agents/issue-index.md index eabf4a3e0..fb36c663a 100644 --- a/.agents/issue-index.md +++ b/.agents/issue-index.md @@ -347,6 +347,7 @@ rather than merged. `scripts/check-agent-record.py` gates both. | [#1150](https://github.com/mudler/vllm.cpp/issues/1150) | `LTX25-TI2VID-RECIPE` | The sigma SHIFT is derived from the target latent on every arm, where six of upstream's seven `LTX2Scheduler.execute` call sites pass NO latent and take `default_number_of_tokens` = `MAX_SHIFT_ANCHOR` = 4096 (`schedulers.py:11,:29,:31`). `grep -rn '\.execute(' packages/ltx-pipelines/src/ltx_pipelines/` at `fd4ded7f` returns seven and that grep is the whole population: only `ti2vid_two_stages_hq.py:267` passes `latent=empty_latent`. This engine passes `target_tokens` at `src/vllm/multimodal/ltx2_video.cpp:3442-3443`, so it mirrors the exception and diverges from the rule. Correct today: `t2a_one_stage` (passes 0 at `src/vllm/model_executor/models/ltx2_t2a.cpp:178`) and `res2s_two_stage`. DIVERGENT: `one_stage` at four version keys (`ti2vid_one_stage.py:207`), `a2vid_two_stage` stage 1 (`a2vid_two_stage.py:226`) and `retake`'s non-distilled arm (`retake.py:287`). Recipes carrying explicit `sigmas` never reach the derivation and are unaffected. Not a rounding difference: at the recipe default geometry the target latent is 6144 tokens, giving `sigma_shift` 2.78 against upstream's 2.05, so every sigma moves. Invisible because the trajectory changes while the frame count, shapes, sample rate and errors do not, and our goldens were captured from this engine so they PIN it rather than detect it. `.agents/specs/ltx25-res2s-loop.md:80-88` saw the HQ/plain split and concluded the divergence was on the plain two-stage arm alone; that is right about HQ and wrong about the blast radius. Found by row `LTX25-TI2VID-RECIPE`, which added the seam — `Ltx2PhaseRecipe::schedule_tokens`, defaulted to today's behaviour so nothing moves — and set it on the one phase it ships. Not fixed in flow because flipping the other three re-samples five shipped, gated arms and rewrites their goldens, which needs its own spec and fresh review. Listed under `## Owed` in [`ltx25-ti2vid-recipe.md`](specs/ltx25-ti2vid-recipe.md) | bug | | [#1151](https://github.com/mudler/vllm.cpp/issues/1151) | `LTX25-TI2VID-RECIPE` | The `requires_distilled_lora` refusal still advertised [#1118](https://github.com/mudler/vllm.cpp/issues/1118) as OPEN, and cited `a2vid_two_stage.py`'s line numbers to every other pipeline. #1118 closed at `4ae0f54ab` (row `LTX25-PHASE-LORA`, PR [#1140](https://github.com/mudler/vllm.cpp/pull/1140)), which added `Ltx2PhaseRecipe::loras` and `Ltx2RebindDitLoras`; the message at `src/vllm/multimodal/ltx2_video.cpp:1039-1041` still ended "upstream fuses that adapter into stage 2 ALONE and this engine fuses once at load, so stage 1 sees it too", every clause of which had become false, and the comment above the refusal said the same. `ltx25-phase-lora.md` repaired the REFERENCE-CONDITIONING refusal, which carried the identical claim ~1100 lines away, and named only that one in its port map, so this site and the `ltx2-gen --help` text (`examples/ltx2_gen/main.cpp:210-212`) were both missed. Second defect at the same site: the refusal is deliberately keyed on the FLAG rather than on the kind string so the next recipe inherits it (the comment names #1093 and #1096 as waiting), yet its body interpolated `im.pipeline_kind` into the first sentence and hard-coded a2vid's `:164`, `:114`, `:107` into the rest — so the first arm to inherit it would be told its own name and then a different pipeline's source lines. `--distilled-lora required=True` lives on `default_2_stage_arg_parser` (`utils/args.py:1123`, `:1140-1155`), which all of these pipelines select, and that shared anchor is what the message now cites. Found and fixed IN FLOW by row `LTX25-TI2VID-RECIPE`, the second user of the flag; `tests/vllm/multimodal/test_ltx2_video.cpp` asserted the string `1118` was PRESENT and now asserts it is absent | bug | | [#1152](https://github.com/mudler/vllm.cpp/issues/1152) | `LTX25-TI2VID-RECIPE` | `Ltx2PipelineRecipe::allow_request_latents` (`include/vllm/model_executor/models/ltx2_pipeline.h:705`) is WRITTEN by every recipe and READ by nothing — the "a parameter no caller passes" shape AGENTS.md `## Nothing lands dead` names. Measured at `c83b96934`: `grep -rn allow_request_latents src include examples` minus the declaration returns FIVE lines and all five are assignments (`ltx2_pipeline.cpp:1264` false, `:1345` false, `:1473` true, `:1604` false). Positive control, the field declared one line above and set in the same blocks: `allow_request_sigmas` returns its assignments PLUS a real reader at `src/vllm/multimodal/ltx2_video.cpp:3476`, so the grep is well-formed and the absence is the finding. Consequence: `Res2sTwoStageRecipe` carries `true` where every other recipe carries `false` and nothing can tell the difference — no upstream `__call__` among these pipelines takes an initial-latent parameter, so `false` is what the signatures support and the `true` looks like an oversight, but it is unfalsifiable while nothing reads the field. The tests assert the VALUES, so they gate the record against itself and cannot see that nothing consumes it, which is the tautology shape [#911](https://github.com/mudler/vllm.cpp/issues/911) recorded on the anchor checker. Two closes: give it a reader (a refusal on a request supplying a latent to a recipe whose upstream signature has none) or delete it and its assertions — deleting is defensible, since no request surface carries a latent at all so the refusal could never fire either. Found by row `LTX25-TI2VID-RECIPE` while deriving the same field for a sixth recipe; not fixed in flow because both closes touch five landed recipes and one deletes gated assertions. Listed under `## Owed` in [`ltx25-ti2vid-recipe.md`](specs/ltx25-ti2vid-recipe.md) | bug | +| [#1160](https://github.com/mudler/vllm.cpp/issues/1160) | `SPEC-MTP` | `--speculative-config` read five keys (`src/vllm/config/speculative.cpp:14-131` @ `affc2a7fd`) and DROPPED every other one, so `{"method":"dspark","model":"...","num_speculative_tokens":7,"draft_sample_method":"probabilistic"}` started a server that drafted GREEDY, logged nothing and exited 0, and a misspelled `"num_speculatve_tokens"` silently took the resolved default. Upstream gets this refusal free: `SpeculativeConfig` carries `extra="forbid"` through its `@config` decorator (`vllm/config/speculative.py:81-83` @ `555967922`), and the hand-written C++ parser reproduced the reads without the guard. The method value WAS validated (`speculative.cpp:43,48-54`), so the strictness existed for one field and was absent for the object. Not merely an inert flag: draft sampling and verify are greedy here (`include/vllm/v1/worker/gpu/spec_decode/dspark/speculator.h:36-38`, `include/vllm/v1/spec_decode/rejection_sampler.h:53-57`), so a dropped `probabilistic` yields a DETERMINISTIC run when a sampled draft was requested, and a deterministic run is adjudicable by the token-exact greedy gate while the requested configuration is not, which lets a parity or benchmark number be taken under a configuration nobody chose. Fixed by admitting the object key by key against the `SpeculativeConfig` field set at the pin (`speculative.py:85-283`), in three classes. The five honoured keys pass. `draft_sample_method` and `rejection_sample_method` pass at their upstream defaults `greedy` and `standard`, which are what this engine implements, and any other value names row `SPEC-ACCEPT-VARIANTS`. Every other name is refused, worded differently for a declared vLLM field than for a typo. Scope item 2 of the issue, probabilistic draft sampling itself, stays owned by `SPEC-ACCEPT-VARIANTS` (`.agents/engine-matrix.md:191`, `INVENTORIED`) and is NOT in this change. Gated by `tests/vllm/config/test_speculative_unknown_keys.cpp` (9/9, 63 assertions, RED-first at 6/9 failing) plus three black-box server-CLI cases in `examples/CMakeLists.txt`, which are the reach proof: the unit file calls the parser directly and stays GREEN with the server call site deleted, while the CLI cases go RED | bug | | [#1165](https://github.com/mudler/vllm.cpp/issues/1165) | `FIX-GPU-MEM-UTIL-INERT` | `--gpu-memory-utilization` is parsed (`src/vllm/entrypoints/openai/server_main.cpp:440-441`), threaded to both engines (`:952`, `:1039`), carried on the C ABI (`include/vllm.h:486`, `src/capi/vllm_c.cpp:577-580`), spelled by `examples/cli/main.cpp:118-119`, defaulted to 0.92 (`include/vllm/entrypoints/model_loader.h:90`) and then read by NOTHING: `LoadedEngine::ResolveNumBlocks` falls through knob 1 (`num_blocks`) and knob 2 (`kv_cache_memory_bytes`) to a bare `return 256` under a `TODO(ROAD-V1-MEM M3)` (`src/vllm/entrypoints/model_loader.cpp:954-959`), so a user who passes `--gpu-memory-utilization 0.85` believes they sized the KV pool and sized nothing. DISTINCT from [#83](https://github.com/mudler/vllm.cpp/issues/83), which owns IMPLEMENTING the utilization path (`ROAD-V1-MEM` M3, dgx-gated on a profile run and an oracle-matched pool). This row owns not lying about it: accept the flag, keeping vLLM's exact name and fraction semantics per `.agents/roadmap_v1.md:71`, and emit one notice per engine load when the caller set it explicitly AND the utilization path is the one that resolved the pool. Fixed in flow. Spec [`gpu-mem-util-inert.md`](specs/gpu-mem-util-inert.md) | bug | | [#1166](https://github.com/mudler/vllm.cpp/issues/1166) | `FIX-FP8-BLOCKWISE-REFUSAL` | `Qwen/Qwen3.8-27B-FP8` is block-wise (fine-grained 128x128) FP8 and this tree implements per-tensor FP8 only, so the load stops on a message that names the wrong thing. Measured live at revision `017b9c7af6b5689d5dd426a76e0bc077eb5ca20a` on 2026-08-17: the config declares `quant_method` `fp8`, `weight_block_size` `[128, 128]` and `activation_scheme` `dynamic`, and the safetensors header of `layers-3.safetensors`, read by RANGE REQUEST rather than downloaded, gives `self_attn.q_proj.weight` `F8_E4M3` `[12288, 5120]` beside `self_attn.q_proj.weight_scale_inv` `BF16` `[96, 40]`, which is exactly `[12288/128, 5120/128]`, with ZERO `input_scale` tensors in the shard. `LoadAttnDense` branches on the weight dtype alone (`qwen3_5_dense_weights.cpp:479`) so the block-wise projection enters the per-tensor arm at `:480`, and `LoadFp8Raw` (`qwen3_5_weights.cpp:449`) asks for `.weight_scale` at `:458`, which this checkpoint spells `weight_scale_inv`, so the resolver at `qwen3_5_dense_weights.cpp:682` raises `tensor not found: ...q_proj.weight_scale`. Nothing is missing from the checkpoint. The reader is sent after a tensor upstream never writes in this mode instead of being told the fine-grained arm is absent. NOT the silently-wrong-numerics case, and the check that rules it out is recorded rather than assumed: `ReadF32Scalar` (`qwen3_5_weights.cpp:312`) bounds its input with `t.nbytes >= sizeof(float)`, a LOWER bound, so a `[96, 40]` scale would pass and read as block `(0,0)`, but the NAME miss stops the load before that scalar read, and upstream makes the spelling strictly conditional on block quant (`weight_scale_inv if self.block_quant else weight_scale`, `fp8.py:511` at pin `555967922`), so no upstream block-wise checkpoint reaches it. FIXED IN FLOW as a named refusal at `ModelRegistry::Load`; the block-wise arm itself stays owed | bug | | [#1161](https://github.com/mudler/vllm.cpp/issues/1161) | `ENG-CUDAGRAPH` | SGLang landed **Breakable CUDA Graph (BCG)** and the whole surface was absent from `.agents/sglang-matrix.md`, while `ENG-CUDAGRAPH` carried no verdict on it. Grounded at pin `v0.5.15`/`f63458b5be`: BCG captures one forward as a SEQUENCE of `torch.cuda.CUDAGraph` segments split at eager break points on the attention and recurrent calls, sharing one mempool with weak-ref'd intermediates, with **no `torch.compile`** (`breakable_cuda_graph.py:204-241,244-260,14-23,156-169`; `breakable_cuda_graph_backend.py:14-17`; break points `radix_attention.py:256`, `forward_mla.py:1092`, `radix_linear_attention.py:159`, `nemotron_h.py:1240`, `dsa_indexer.py:2414`). **The public priority claim is narrower than it reads:** vLLM's v1 default is ALREADY `FULL_AND_PIECEWISE`, splitting at `splitting_ops` (`vllm/config/compilation.py:60-63,517,615,630` @ `555967922`), so the capability has been in our benchmark denominator all along — what SGLang landed first is the CONSTRUCTION (piecewise coverage from runtime stream capture plus a decorator, no Dynamo/Inductor/FX), which matters to us only because we have no compiler either. **The load-bearing outcome is a REFUTATION:** prefill graph capture is not a lever on our CUDA gate models, because GB10 2026-07-09 measured steady-state prefill GPU-idle-between-launches at **3.8%** with GPU-busy >96% on both arms, and attributed the 27B prefill gap to **92.5% non-GEMM glue GPU work** with the dominant GEMM the same symbol at the same call count (+0.17%) and attention AHEAD. There are no launch bubbles in our prefill for a graph to collapse, and vLLM's piecewise prefill graph is already inside the denominator we lose to, so it does not explain the gap either. Decode is a separate and already-banked case (conc-64 launch overhead was ~24%, already one `cudaGraphLaunch`/step). FIXED IN FLOW: `SGLANG-BCG` row added under a new "Graph capture and runtime execution" section of `sglang-matrix.md`, the refutation recorded on `ENG-CUDAGRAPH` (which stays `PARTIAL`, because nothing shipped), roadmap track `C12` added, and spec [`sglang-breakable-cuda-graph.md`](specs/sglang-breakable-cuda-graph.md) written to carry the analysis. NOT fixed here and owned by the three issues below: no engine code, no measurement taken, no oracle run, and no advance of the SGLang pin | record | diff --git a/docs/SPECULATIVE-DECODING.md b/docs/SPECULATIVE-DECODING.md index 15c1454f7..656b3c09e 100644 --- a/docs/SPECULATIVE-DECODING.md +++ b/docs/SPECULATIVE-DECODING.md @@ -24,6 +24,39 @@ the list of accepted ones (`src/vllm/config/speculative.cpp`). MTP and DFlash are the two with binding numbers behind them; the per-method detail below and in [BENCHMARKS.md](BENCHMARKS.md) says which is which. +## Which keys the JSON accepts + +The object is checked key by key and **nothing is dropped**. A name this engine +cannot honour is refused at startup, by name, before a byte of weights is read. +Until [#1160](https://github.com/mudler/vllm.cpp/issues/1160) an unrecognised key +was silently discarded, so `"draft_sample_method":"probabilistic"` started a +server that drafted greedily and reported nothing, and a typo such as +`"num_speculatve_tokens"` quietly took the resolved default instead of the value +that was typed. + +| Key | Accepted | +|---|---| +| `method` | `mtp`, `dflash`, `dspark`, `ngram`, `draft_model` | +| `num_speculative_tokens` | a positive integer, or absent for the method's own default | +| `model` | the draft checkpoint path or HF repo id | +| `prompt_lookup_min`, `prompt_lookup_max` | an integer of at least 1, `ngram` only | +| `draft_sample_method` | `greedy` only, which is upstream's default and what this engine does | +| `rejection_sample_method` | `standard` only, which is upstream's default and what this engine does | + +There are two kinds of refusal, worded differently on purpose. A name vLLM's own +`SpeculativeConfig` declares, such as `quantization` or `max_model_len`, is +reported as a real vLLM field this engine does not implement. Any other name is +reported as unknown, together with the list above, because that one is usually a +typo. `draft_sample_method: probabilistic` and the `synthetic` and `block` +acceptance variants name row `SPEC-ACCEPT-VARIANTS`, which owes them. + +The distinction matters beyond ergonomics. Draft sampling and verify are greedy +here, so a dropped `probabilistic` produced a **deterministic** run when a sampled +draft was asked for, and a deterministic run is adjudicable by the token-exact +greedy gate while the configuration the user actually requested is not. A +silently downgraded flag therefore lets a parity or benchmark number be taken +under a configuration nobody chose. + ## MTP - **Models:** the Qwen3.5 / 3.6 gate checkpoints that ship an `mtp.*` draft head diff --git a/docs/USAGE.md b/docs/USAGE.md index 35164e786..8a567464b 100644 --- a/docs/USAGE.md +++ b/docs/USAGE.md @@ -248,7 +248,7 @@ build/examples/vllm-cli \ | `--top-k K` | `0` | Top-k (`0` means all) | | `--seed S` | (unset) | RNG seed (enables seeded sampling) | | `--stream` | off | Stream token deltas to stdout | -| `--speculative-config ''` | (unset) | Speculative decoding, same JSON as vLLM's flag. See [docs/SPECULATIVE-DECODING.md](SPECULATIVE-DECODING.md) | +| `--speculative-config ''` | (unset) | Speculative decoding, same JSON as vLLM's flag. Every key is checked and none is dropped: an unknown or misspelled name is refused at startup by name, and a real vLLM key this engine does not implement is refused as such ([#1160](https://github.com/mudler/vllm.cpp/issues/1160)). See [docs/SPECULATIVE-DECODING.md](SPECULATIVE-DECODING.md) | | `--max-num-seqs N` | engine default (32) | Max concurrent sequences. Under speculative decoding on a GDN model the recurrent state is `max-num-seqs x (k+1)` per slot, so this is the knob to lower when a run is refused for state budget | | `--repeat N` | `1` | Load once, then run N blocking completions. Use it to read a warm decode tok/s without paying model load each time. Not supported with `--stream`, which falls back to 1 | | `-h`, `--help` | | Print usage and exit | @@ -2199,7 +2199,7 @@ a stop token early. | `--reasoning-parser ` | `none` | Reasoning parser (`think_auto`, `deepseek_r1`, `deepseek_v3`, `holo2`, `mistral`, `minimax_m2`, `minimax_m2_append_think`, `step3`, `olmo3`, `muse_glimmer`, `qwen3`, `mimo`). `auto` detects, `none` disables. `qwen3` and its `mimo` alias are the engine-backed adapter (one upstream class, two registry names): thinking is ON, so a marker-less stream is reasoning and a `` ends reasoning with no ``. `auto` never selects it — a generic `` template resolves to `think_auto`, which is the right default for hybrid-thinking models that may answer with no think block at all | | `--kv-transfer-config ''` | (unset) | External KV connector, same JSON as vLLM's flag. See [docs/KV-OFFLOAD.md](KV-OFFLOAD.md) | | `--offload-config ''` | (unset) | Weight offload, the same JSON vLLM's `OffloadConfig` takes (distinct from `--kv-transfer-config`, which offloads KV blocks). Parsed and validated at startup, so a malformed document, an unknown backend or a validator violation is refused before any model I/O; a backend/field mismatch is a warning, as upstream. **Enabling it fails startup on every model today**: no loader consults the offloader, so the engine refuses the configuration by architecture name rather than accept a budget that frees nothing. A config that leaves offloading disabled still parses and reports normally. On unified memory such as GB10 offload cannot help at all, because host and device share one pool. See [docs/WEIGHT-OFFLOAD.md](WEIGHT-OFFLOAD.md) | -| `--speculative-config ''` | (unset) | Speculative decoding (`mtp`, `dflash`, `ngram`), same JSON as vLLM's flag. For `mtp`, `num_speculative_tokens` sets the draft DEPTH and defaults to the checkpoint's `mtp_num_hidden_layers`, which is 1 on both gate checkpoints, so the default is unchanged. A value above it must be a multiple of it, mirroring vLLM. Depth cannot move the emitted tokens under greedy decoding, and no speed number is claimed above k=1 yet ([#81](https://github.com/mudler/vllm.cpp/issues/81)). What is gated on CPU at k=1..4 is that the propose runs `k-1` draft decode forwards per propose call, that k drafts reach the verify path, and that the drafts DELIVERED to the verify path vary with depth rather than repeating the first one. That last one is counted over a RUN and never per call, because a correct drafter may resample the same token and this fixture does. Two things are NOT gated there. A draft is never accepted at depth, because acceptance is zero at every depth on the synthetic gate model. And nothing here proves the draft at depth j came from the j-th forward. Both are owed to the GPU gate, which must close the second by comparing the per-depth acceptance RATE against a PADDED control rather than by asserting a non-zero acceptance count, because a padded drafter earns acceptance at depth whenever the target's own greedy continuation repeats a token. `dspark` speculates on the Qwen3.6 gate models (native + Speculators drafts), token-identically to speculative-off, but is not gated on speed: the cross-engine ratio is UNSETTLED, with a matched-and-warm paired measurement of 0.834x against the pinned oracle and the earlier 0.957x-0.989x figures taken against a single COLD oracle invocation on a machine that has since been reimaged. A GGUF target, or a target with no aux multi-tap, is refused by name (`SPEC-DSPARK`). Its sequential Markov sampling runs on device by default; `VT_DSPARK_DEVICE_SAMPLE=0` restores the host loop (token-identical, cost only). The speculative verify runs from a captured CUDA graph, worth +12.2%/+3.5% on the 35B cells; `VT_SPEC_DECODE_GRAPH=0` restores the eager verify (also token-identical). See [docs/SPECULATIVE-DECODING.md](SPECULATIVE-DECODING.md) | +| `--speculative-config ''` | (unset) | Speculative decoding (`mtp`, `dflash`, `ngram`), same JSON as vLLM's flag. For `mtp`, `num_speculative_tokens` sets the draft DEPTH and defaults to the checkpoint's `mtp_num_hidden_layers`, which is 1 on both gate checkpoints, so the default is unchanged. A value above it must be a multiple of it, mirroring vLLM. Depth cannot move the emitted tokens under greedy decoding, and no speed number is claimed above k=1 yet ([#81](https://github.com/mudler/vllm.cpp/issues/81)). What is gated on CPU at k=1..4 is that the propose runs `k-1` draft decode forwards per propose call, that k drafts reach the verify path, and that the drafts DELIVERED to the verify path vary with depth rather than repeating the first one. That last one is counted over a RUN and never per call, because a correct drafter may resample the same token and this fixture does. Two things are NOT gated there. A draft is never accepted at depth, because acceptance is zero at every depth on the synthetic gate model. And nothing here proves the draft at depth j came from the j-th forward. Both are owed to the GPU gate, which must close the second by comparing the per-depth acceptance RATE against a PADDED control rather than by asserting a non-zero acceptance count, because a padded drafter earns acceptance at depth whenever the target's own greedy continuation repeats a token. `dspark` speculates on the Qwen3.6 gate models (native + Speculators drafts), token-identically to speculative-off, but is not gated on speed: the cross-engine ratio is UNSETTLED, with a matched-and-warm paired measurement of 0.834x against the pinned oracle and the earlier 0.957x-0.989x figures taken against a single COLD oracle invocation on a machine that has since been reimaged. A GGUF target, or a target with no aux multi-tap, is refused by name (`SPEC-DSPARK`). Its sequential Markov sampling runs on device by default; `VT_DSPARK_DEVICE_SAMPLE=0` restores the host loop (token-identical, cost only). The speculative verify runs from a captured CUDA graph, worth +12.2%/+3.5% on the 35B cells; `VT_SPEC_DECODE_GRAPH=0` restores the eager verify (also token-identical). The object is admitted key by key and NOTHING is dropped ([#1160](https://github.com/mudler/vllm.cpp/issues/1160)): the honoured keys are `method`, `num_speculative_tokens`, `model`, `prompt_lookup_min` and `prompt_lookup_max`, plus `draft_sample_method` and `rejection_sample_method` at their upstream defaults `greedy` and `standard`, which are what this engine implements. Any other value of those two names row `SPEC-ACCEPT-VARIANTS` and is refused. A name vLLM's `SpeculativeConfig` declares but this engine does not implement, such as `quantization`, is refused as exactly that, and any other name is refused as unknown with the accepted list. Before this the extra key was discarded, so `draft_sample_method=probabilistic` ran GREEDY and a misspelled `num_speculatve_tokens` took the default, both silently and both at exit 0. See [docs/SPECULATIVE-DECODING.md](SPECULATIVE-DECODING.md) | | `--language-model-only` / `--no-language-model-only` | off | Disable all multimodal input by setting **every** modality limit to 0, mirroring vLLM's flag of the same name. It is not a "skip the encoder" switch: the server then **refuses** a multimodal request with ``400 At most 0 image(s) may be provided in one prompt. Set `--limit-mm-per-prompt` to increase this limit.`` It does **not** free VRAM yet — nothing gates tower construction on it ([#607](https://github.com/mudler/vllm.cpp/issues/607) wave L3) | | `--limit-mm-per-prompt ''` | (unset ⇒ 999 per modality) | Maximum multimodal input items per prompt, per modality, as the same JSON object vLLM's flag takes: `'{"image": 2, "video": 0}'`, or with profiling options `'{"video": {"count": 1, "num_frames": 32}}'` (the options are validated and ignored — they size dummy inputs for memory profiling, which this engine does not do). A limit can only **lower** what the model/seam supports, never raise it. Malformed JSON, a negative count, or an unknown option on `image` / `video` / `audio` is refused at startup rather than defaulted. An unknown option on any other modality name is dropped rather than refused, mirroring upstream, whose fallback `BaseDummyOptions` is the one such dataclass without `extra="forbid"`. Upstream's dotted spelling (`--limit-mm-per-prompt.image 2`) is not accepted here, as for `--kv-transfer-config` and `--speculative-config` | | `--enable-log-requests` / `--disable-log-requests` | on | Log each incoming request. Mirrors vLLM's flag of the same name | diff --git a/examples/CMakeLists.txt b/examples/CMakeLists.txt index 6ac37b929..1e57bf5e4 100644 --- a/examples/CMakeLists.txt +++ b/examples/CMakeLists.txt @@ -277,6 +277,46 @@ if(VLLM_CPP_SERVER) --speech-family minimax-music3) set_tests_properties(test_server_speech_family_still_needs_a_checkpoint PROPERTIES PASS_REGULAR_EXPRESSION "--speech-family names a family but --speech-model names no checkpoint") + + # --speculative-config key ADMISSION, from the production entry point (#1160). + # `tests/vllm/config/test_speculative_unknown_keys.cpp` calls the parser + # directly, so it localizes a failure and proves NOTHING about reach: it stays + # green with the server's call site deleted. These three enter through the + # server binary on its default configuration, which is where a user meets this + # flag, and go RED when `server_main.cpp` stops calling + # `ParseSpeculativeConfigJson`. All three exit before a byte of weights is + # read, because the parse runs before `LoadedEngine::FromModelDir`. + # + # 1. The issue's reproduction. Before the fix this key was DROPPED, so the + # server started and drafted greedy, and the run reached the model-load + # error below instead of this refusal. + add_test(NAME test_server_speculative_rejects_probabilistic_draft + COMMAND $ --model /nonexistent-model-dir + --speculative-config + "{\"method\":\"dspark\",\"model\":\"d\",\"num_speculative_tokens\":7,\"draft_sample_method\":\"probabilistic\"}") + set_tests_properties(test_server_speculative_rejects_probabilistic_draft PROPERTIES + PASS_REGULAR_EXPRESSION "speculative-config: draft_sample_method \"probabilistic\" is not implemented.*SPEC-ACCEPT-VARIANTS" + FAIL_REGULAR_EXPRESSION "model path is not a directory") + + # 2. A MISSPELLED key. `num_speculatve_tokens` used to fall through to the + # resolved default, so the user silently got a k they did not ask for. + add_test(NAME test_server_speculative_rejects_a_misspelled_key + COMMAND $ --model /nonexistent-model-dir + --speculative-config "{\"method\":\"mtp\",\"num_speculatve_tokens\":7}") + set_tests_properties(test_server_speculative_rejects_a_misspelled_key PROPERTIES + PASS_REGULAR_EXPRESSION "speculative-config: unknown key \"num_speculatve_tokens\"" + FAIL_REGULAR_EXPRESSION "model path is not a directory") + + # 3. The CONTROL, and the case that makes the two above mean something. A + # valid config gets PAST the admission check and on to the model load, so + # an admission check that refused everything would go red here rather than + # passing the two refusal cases for the wrong reason. + add_test(NAME test_server_speculative_accepts_a_valid_config + COMMAND $ --model /nonexistent-model-dir + --speculative-config "{\"method\":\"mtp\",\"num_speculative_tokens\":1}") + set_tests_properties(test_server_speculative_accepts_a_valid_config PROPERTIES + PASS_REGULAR_EXPRESSION "model path is not a directory: /nonexistent-model-dir" + FAIL_REGULAR_EXPRESSION "speculative-config") endif() endif() diff --git a/src/vllm/config/speculative.cpp b/src/vllm/config/speculative.cpp index 9996dc701..7d7112e97 100644 --- a/src/vllm/config/speculative.cpp +++ b/src/vllm/config/speculative.cpp @@ -4,12 +4,114 @@ // loader (see SpeculativeConfig::ResolveMtp). #include "vllm/config/speculative.h" +#include #include #include +#include #include namespace vllm { +namespace { + +// #1160 — key ADMISSION for `--speculative-config`. +// +// Upstream deserializes this JSON into the `SpeculativeConfig` dataclass, which +// carries `extra="forbid"` through its `@config` decorator +// (vllm/config/speculative.py:81-83 @ the pinned oracle `555967922`), so vLLM +// refuses a name it does not declare. This hand-written parser read five keys +// and dropped everything else, which turned `"draft_sample_method": +// "probabilistic"` into a silent GREEDY run and a misspelled +// `"num_speculatve_tokens"` into the resolved default. A dropped key does not +// merely do nothing: it changes what configuration a measurement was taken +// under, which is the one thing a gate cannot recover afterwards. +// +// The three classes below restore that refusal. The split matters because a key +// vLLM genuinely declares deserves a different message from a typo: telling a +// user that `quantization` is "unknown" sends them hunting for a spelling +// mistake that is not there. + +// Class 1: parsed and used below. +constexpr std::array kHonouredKeys = { + "method", "num_speculative_tokens", "model", "prompt_lookup_min", + "prompt_lookup_max", +}; + +// Class 3: declared by `SpeculativeConfig` at the pin +// (vllm/config/speculative.py:85-283 @ `555967922`) and NOT implemented here. +// Mirrored field-for-field so a pin bump that adds a field is reconciled here +// rather than silently widening what this parser drops. The internal +// `SkipValidation` fields (`target_model_config`, `draft_model_config`, +// `target_parallel_config`, `draft_parallel_config`, `draft_load_config`) are +// listed too: upstream builds them rather than reading them from this JSON, so a +// user who names one is asking for something no spelling of this flag delivers. +constexpr std::array kUpstreamUnimplementedKeys = { + "enforce_eager", // :85 + "draft_tensor_parallel_size", // :102 + "tensor_parallel_size", // :105 + "quantization", // :110 + "moe_backend", // :114 + "attention_backend", // :119 + "kv_cache_dtype", // :123 + "max_model_len", // :126 + "revision", // :129 + "code_revision", // :133 + "disable_padded_drafter_batch", // :139 + "use_local_argmax_reduction", // :144 + "use_heterogeneous_vocab", // :150 + "parallel_drafting", // :165 + "target_model_config", // :172 + "target_parallel_config", // :174 + "num_speculative_tokens_per_batch_size", // :178 + "draft_model_config", // :186 + "draft_parallel_config", // :188 + "suffix_decoding_max_tree_depth", // :192 + "suffix_decoding_max_cached_requests", // :196 + "suffix_decoding_max_spec_factor", // :202 + "suffix_decoding_min_token_prob", // :207 + "draft_load_config", // :212 + "synthetic_acceptance_rates", // :224 + "synthetic_acceptance_length", // :232 +}; + +// The tail every refusal carries, so the message closes the user's search +// instead of only ending it. Class 2 is spelled with its one accepted value +// because "supported" would otherwise overstate what those keys accept. +constexpr std::string_view kSupportedKeys = + "supported keys: method, num_speculative_tokens, model, prompt_lookup_min, " + "prompt_lookup_max, draft_sample_method (only \"greedy\"), " + "rejection_sample_method (only \"standard\")"; + +bool Contains(const std::array& set, const std::string& key) { + for (std::string_view k : set) { + if (k == key) return true; + } + return false; +} + +bool Contains(const std::array& set, const std::string& key) { + for (std::string_view k : set) { + if (k == key) return true; + } + return false; +} + +// Class 2: an upstream key whose value space we implement exactly one point of. +// The accepted value is upstream's own DEFAULT in both cases, so a vLLM config +// that spells the default explicitly keeps running, and only a request for +// behavior we do not have is refused. `owed_by` names the missing part, which is +// what AGENTS.md requires of a refused arm. +void CheckValueGatedKey(const nlohmann::json& doc, const char* key, + const char* accepted, const std::string& reason) { + if (!doc.contains(key)) return; + const nlohmann::json& v = doc.at(key); + if (v.is_string() && v.get() == accepted) return; + const std::string got = v.is_string() ? ("\"" + v.get() + "\"") : v.dump(); + throw std::invalid_argument(std::string("speculative-config: ") + key + " " + got + + " is not implemented. " + reason); +} + +} // namespace SpeculativeConfig ParseSpeculativeConfigJson(const std::string& json_text) { nlohmann::json doc; @@ -53,6 +155,53 @@ SpeculativeConfig ParseSpeculativeConfigJson(const std::string& json_text) { cfg.method + "\")"); } + // #1160: key admission, AFTER the method check so an unsupported method is + // still the error a user sees first (it is the field that selects everything + // else), and BEFORE every other read so no key can be dropped on the way past. + // Presence is what is judged for classes 1 and 3, including an explicit + // `null`: a null on a key we do not implement still names a capability we do + // not have, and answering it with silence is the defect this restores. + for (auto it = doc.begin(); it != doc.end(); ++it) { + const std::string& key = it.key(); + if (Contains(kHonouredKeys, key)) continue; + if (key == "draft_sample_method" || key == "rejection_sample_method") continue; + if (Contains(kUpstreamUnimplementedKeys, key)) { + throw std::invalid_argument( + "speculative-config: \"" + key + + "\" is a vLLM SpeculativeConfig field (vllm/config/speculative.py:85-283 " + "@ 555967922) that this engine does not implement at this pin, so it is " + "refused rather than dropped (" + std::string(kSupportedKeys) + ")"); + } + throw std::invalid_argument( + "speculative-config: unknown key \"" + key + + "\" (vLLM's SpeculativeConfig declares no such field at the pinned oracle " + "555967922; " + std::string(kSupportedKeys) + ")"); + } + // draft_sample_method (speculative.py:77,283). The draft is GREEDY by + // construction here — `speculator.h` takes the argmax and the verify accepts + // iff equal — so "probabilistic" would change the acceptance rule, and with it + // whether the run is adjudicable by the token-exact greedy gate at all. + CheckValueGatedKey( + doc, "draft_sample_method", "greedy", + "This engine drafts greedy only (include/vllm/v1/worker/gpu/spec_decode/" + "dspark/speculator.h) and verifies accept-iff-equal (include/vllm/v1/" + "spec_decode/rejection_sampler.h), so \"greedy\", which is upstream's own " + "default, is the only accepted value at this pin. Probabilistic draft " + "sampling is owed by row SPEC-ACCEPT-VARIANTS (.agents/engine-matrix.md), " + "vllm/config/speculative.py:77,283 @ 555967922."); + // rejection_sample_method (speculative.py:78,216). "standard" is upstream's + // default and the semantics the landed verify implements. "synthetic" and + // "block" are the two branches at + // vllm/v1/worker/gpu/spec_decode/rejection_sampler.py:82-91, both listed as + // deferred in rejection_sampler.h. + CheckValueGatedKey( + doc, "rejection_sample_method", "standard", + "This engine implements upstream's default \"standard\" acceptance only, " + "as accept-iff-equal under greedy decode (include/vllm/v1/spec_decode/" + "rejection_sampler.h). The other two are owed by row SPEC-ACCEPT-VARIANTS " + "(.agents/engine-matrix.md), vllm/config/speculative.py:78,216 and " + "vllm/v1/worker/gpu/spec_decode/rejection_sampler.py:82-91 @ 555967922."); + // num_speculative_tokens (k). Optional; the loader defaults it to n_predict // (mtp_num_hidden_layers) via ResolveMtp when absent (speculative.py:865-875). if (doc.contains("num_speculative_tokens") && diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index c9f2b15c0..8610d5c22 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -1104,6 +1104,11 @@ vllm_cpp_add_test(test_speculative_draft_max_position_embeddings # SPEC-DSPARK W1: the DSpark config slice (method acceptance, block-size floor, # lookahead slots, draft identity). vllm_cpp_add_test(test_speculative_dspark vllm/config/test_speculative_dspark.cpp) +# #1160: `--speculative-config` key ADMISSION — an unknown or misspelled key is +# refused by name, and an upstream key this engine does not implement is refused +# with the missing part named, instead of being silently dropped. +vllm_cpp_add_test(test_speculative_unknown_keys + vllm/config/test_speculative_unknown_keys.cpp) # SPEC-MTP-K-GT-1 (#81): the MTP speculation-DEPTH config contract — the k=1 # default, the upstream divisibility rule, and that a configured k above 1 is # carried rather than silently degraded. diff --git a/tests/vllm/config/test_speculative_unknown_keys.cpp b/tests/vllm/config/test_speculative_unknown_keys.cpp new file mode 100644 index 000000000..28a87871e --- /dev/null +++ b/tests/vllm/config/test_speculative_unknown_keys.cpp @@ -0,0 +1,241 @@ +// #1160 — `--speculative-config` accepted unknown JSON keys and dropped them. +// +// The defect IS the happy path. Before the fix, every case below that expects a +// refusal PARSED CLEANLY and returned a config with the key discarded, so a run +// carrying `"draft_sample_method":"probabilistic"` proceeded GREEDY with no +// warning and exit 0, and a misspelled `"num_speculatve_tokens"` fell back to +// the resolved default. A happy-path-only test cannot see either, which is why +// these cases assert on refusals and on the refusal MESSAGE. +// +// The key set mirrored here is the `SpeculativeConfig` field set at the pinned +// oracle `555967922`, `vllm/config/speculative.py:85-283`. Upstream deserializes +// `--speculative-config` into that dataclass, so a name declared there is a name +// vLLM accepts. Three classes, and the tests below pin all three: +// +// 1. HONOURED — `method`, `num_speculative_tokens`, `model`, +// `prompt_lookup_min`, `prompt_lookup_max`. Parsed and used. +// 2. ACCEPTED AT THE IMPLEMENTED VALUE — `draft_sample_method` (:283, only +// "greedy") and `rejection_sample_method` (:216, only "standard"). Those +// values ARE what this engine does, so a vLLM config that spells the +// upstream default explicitly still runs. Any other value names +// `SPEC-ACCEPT-VARIANTS`, per AGENTS.md "Refuse an unimplemented arm with a +// message that names the missing part". +// 3. REFUSED — every other upstream field, and every name upstream does not +// declare at all. Both are named in the message; the two messages differ so +// a real vLLM key does not read as a typo. +// +// Upstream has no unit test for `--speculative-config` key admission: its +// dataclass gets the rejection for free from pydantic's `extra="forbid"` +// (`@config` on `SpeculativeConfig`, speculative.py:81-83), which is exactly the +// guarantee this file re-establishes for the hand-written C++ parser. +#include + +#include +#include +#include + +#include "vllm/config/speculative.h" + +using vllm::ParseSpeculativeConfigJson; +using vllm::SpeculativeConfig; + +namespace { + +// The refusal message, or "" when the parse SUCCEEDED. Returning the message +// rather than a bool is what lets a case assert the offending key is NAMED: a +// refusal that says only "invalid config" leaves the user with the same search +// the silent drop did. +std::string RefusalMessage(const std::string& json_text) { + try { + ParseSpeculativeConfigJson(json_text); + } catch (const std::invalid_argument& e) { + return e.what(); + } + return ""; +} + +bool Mentions(const std::string& haystack, const std::string& needle) { + return haystack.find(needle) != std::string::npos; +} + +// A valid DSpark config with one extra key spliced in — the exact reproduction +// shape from the issue. +std::string DsparkWith(const std::string& extra_key_and_value) { + return R"({"method":"dspark","model":"deepseek-ai/dspark_qwen3_4b_block7",)" + R"("num_speculative_tokens":7,)" + + extra_key_and_value + "}"; +} + +} // namespace + +TEST_CASE("draft_sample_method=probabilistic is refused, not silently dropped") { + // The issue's reproduction. speculative.py:283 declares the key and + // speculative.py:77 declares its two values; we implement only the greedy one + // (`include/vllm/v1/worker/gpu/spec_decode/dspark/speculator.h:36-38` and the + // deferred stochastic branch at + // `include/vllm/v1/spec_decode/rejection_sampler.h:53-57`). + const std::string msg = + RefusalMessage(DsparkWith(R"("draft_sample_method":"probabilistic")")); + REQUIRE(msg != ""); + CHECK(Mentions(msg, "draft_sample_method")); + CHECK(Mentions(msg, "probabilistic")); + // Names the missing part and where it is owed, so the refusal is actionable. + CHECK(Mentions(msg, "SPEC-ACCEPT-VARIANTS")); +} + +TEST_CASE("draft_sample_method=greedy is ACCEPTED") { + // "greedy" IS what this engine does, and it is upstream's default + // (speculative.py:283), so a vLLM config that spells the default explicitly + // must keep running. Refusing this would be over-strict, not safe. + const SpeculativeConfig cfg = + ParseSpeculativeConfigJson(DsparkWith(R"("draft_sample_method":"greedy")")); + CHECK(cfg.method == "dspark"); + CHECK(cfg.num_speculative_tokens.has_value()); + CHECK(*cfg.num_speculative_tokens == 7); + CHECK(cfg.draft_model_path.has_value()); +} + +TEST_CASE("rejection_sample_method: standard accepted, synthetic and block refused") { + // speculative.py:216 — "standard" is upstream's default and the semantics the + // landed verify implements (accept-iff-equal under greedy decode, + // `rejection_sampler.h`). "synthetic" (:224-232) and "block" are the branches + // at `vllm/v1/worker/gpu/spec_decode/rejection_sampler.py:82-91`, and both are + // listed as DEFERRED in `rejection_sampler.h`. + const SpeculativeConfig cfg = ParseSpeculativeConfigJson( + DsparkWith(R"("rejection_sample_method":"standard")")); + CHECK(cfg.method == "dspark"); + + for (const std::string variant : {"synthetic", "block"}) { + const std::string msg = RefusalMessage( + DsparkWith(R"("rejection_sample_method":")" + variant + "\"")); + REQUIRE_MESSAGE(msg != "", variant); + // The OFFENDING value, quoted in place. Asserting the bare word would pass + // even if the message named the other variant, because the refusal text + // mentions both. + CHECK(Mentions(msg, "rejection_sample_method \"" + variant + "\"")); + CHECK(Mentions(msg, "SPEC-ACCEPT-VARIANTS")); + } +} + +TEST_CASE("a misspelled honoured key is refused by name") { + // The issue's second case: `num_speculatve_tokens` used to fall through to the + // resolved default, so the user got a k they did not ask for. + const std::string msg = + RefusalMessage(R"({"method":"ngram","num_speculatve_tokens":7})"); + REQUIRE(msg != ""); + CHECK(Mentions(msg, "num_speculatve_tokens")); + // The correct spelling is offered, so the message closes the search. + CHECK(Mentions(msg, "num_speculative_tokens")); +} + +TEST_CASE("an unknown key upstream does not declare is refused by name") { + const std::string msg = RefusalMessage(DsparkWith(R"("not_a_vllm_key":1)")); + REQUIRE(msg != ""); + CHECK(Mentions(msg, "not_a_vllm_key")); + CHECK(Mentions(msg, "unknown key")); +} + +TEST_CASE("an upstream key this engine does not implement is refused by name") { + // These ARE vLLM keys (speculative.py:110, :126, :139, :178), so the message + // must not call them unknown — that would send the user hunting for a typo + // that is not there. It names the key and says the engine does not implement + // it at this pin. + // `const std::string`, not `const char*`: doctest stringifies a `char*` INFO + // payload as a bool, so a failure would log "1" instead of the key that failed. + for (const std::string key : {"quantization", "max_model_len", + "disable_padded_drafter_batch", + "num_speculative_tokens_per_batch_size"}) { + const std::string msg = RefusalMessage(DsparkWith("\"" + key + "\":null")); + REQUIRE_MESSAGE(msg != "", key); + CHECK(Mentions(msg, key)); + CHECK_FALSE(Mentions(msg, "unknown key")); + } +} + +TEST_CASE("a key is judged even when its value is null") { + // `null` is not "absent". The honoured optional keys treat null as absent + // (that is the landed contract, exercised below), but an UNIMPLEMENTED key + // spelled with a null value still names a capability we do not have, and + // dropping it silently is the defect this issue is about. + CHECK(RefusalMessage(DsparkWith(R"("draft_sample_method":null)")) != ""); + CHECK(RefusalMessage(DsparkWith(R"("not_a_vllm_key":null)")) != ""); +} + +TEST_CASE("REGRESSION: every currently valid config still parses") { + // The five honoured keys, across all five accepted methods. If the admission + // check is too strict, this is what goes red. + SUBCASE("mtp, method only") { + const SpeculativeConfig cfg = ParseSpeculativeConfigJson(R"({"method":"mtp"})"); + CHECK(cfg.method == "mtp"); + CHECK_FALSE(cfg.num_speculative_tokens.has_value()); + } + SUBCASE("mtp with an explicit k") { + const SpeculativeConfig cfg = + ParseSpeculativeConfigJson(R"({"method":"mtp","num_speculative_tokens":4})"); + CHECK(*cfg.num_speculative_tokens == 4); + } + SUBCASE("dflash with a draft checkpoint") { + const SpeculativeConfig cfg = ParseSpeculativeConfigJson( + R"({"method":"dflash","model":"z-lab/Qwen3.6-27B-DFlash",)" + R"("num_speculative_tokens":16})"); + CHECK(cfg.method == "dflash"); + CHECK(*cfg.draft_model_path == "z-lab/Qwen3.6-27B-DFlash"); + CHECK(*cfg.num_speculative_tokens == 16); + } + SUBCASE("ngram with the full lookup window") { + const SpeculativeConfig cfg = ParseSpeculativeConfigJson( + R"({"method":"ngram","num_speculative_tokens":4,)" + R"("prompt_lookup_min":3,"prompt_lookup_max":5})"); + CHECK(cfg.method == "ngram"); + CHECK(*cfg.prompt_lookup_min == 3); + CHECK(*cfg.prompt_lookup_max == 5); + } + SUBCASE("dspark with a draft checkpoint") { + const SpeculativeConfig cfg = ParseSpeculativeConfigJson( + R"({"method":"dspark","model":"deepseek-ai/dspark_qwen3_4b_block7",)" + R"("num_speculative_tokens":7})"); + CHECK(cfg.method == "dspark"); + CHECK(*cfg.num_speculative_tokens == 7); + } + SUBCASE("draft_model with a separate checkpoint") { + const SpeculativeConfig cfg = ParseSpeculativeConfigJson( + R"({"method":"draft_model","model":"Qwen/Qwen3.6-0.6B",)" + R"("num_speculative_tokens":3})"); + CHECK(cfg.method == "draft_model"); + CHECK(*cfg.draft_model_path == "Qwen/Qwen3.6-0.6B"); + } + SUBCASE("an explicit null on an honoured optional key still means absent") { + const SpeculativeConfig cfg = ParseSpeculativeConfigJson( + R"({"method":"mtp","num_speculative_tokens":null,)" + R"("prompt_lookup_min":null,"prompt_lookup_max":null})"); + CHECK_FALSE(cfg.num_speculative_tokens.has_value()); + CHECK_FALSE(cfg.prompt_lookup_min.has_value()); + CHECK_FALSE(cfg.prompt_lookup_max.has_value()); + } +} + +TEST_CASE("REGRESSION: the landed refusals are unchanged") { + // The admission check runs BEFORE these, so a key error must not mask a + // method error, and the method/required-key refusals must still fire. + CHECK_THROWS_AS(ParseSpeculativeConfigJson(R"({"method":"eagle3"})"), + std::invalid_argument); + CHECK_THROWS_AS(ParseSpeculativeConfigJson(R"({"num_speculative_tokens":4})"), + std::invalid_argument); + CHECK_THROWS_AS(ParseSpeculativeConfigJson(R"({"method":"dflash"})"), + std::invalid_argument); + CHECK_THROWS_AS(ParseSpeculativeConfigJson(R"({"method":"ngram"})"), + std::invalid_argument); + CHECK_THROWS_AS(ParseSpeculativeConfigJson("not json"), std::invalid_argument); + CHECK_THROWS_AS(ParseSpeculativeConfigJson("[1,2]"), std::invalid_argument); + // The method check runs FIRST. `method` selects everything else, so its error + // is the actionable one, and a key error must not displace it. Both spellings + // are pinned: a key that PASSES admission, and one that does not. + for (const std::string extra : {R"("draft_sample_method":"greedy")", + R"("quantization":"fp8")"}) { + const std::string msg = + RefusalMessage(R"({"method":"suffix",)" + extra + "}"); + REQUIRE_MESSAGE(msg != "", extra); + CHECK(Mentions(msg, "suffix")); + CHECK(Mentions(msg, "only methods")); + } +}