From 7b8c6004e4b9507e38af0548e4d973cfa5766fa6 Mon Sep 17 00:00:00 2001 From: Ettore Di Giacinto Date: Sat, 15 Aug 2026 12:14:52 +0000 Subject: [PATCH 1/2] fix(FIX-REGISTRY-DOWNCAST-SWEEP): open every registry handle with a check, not a promise Closes #847. Follows PR #868 (#775), which fixed the one named site and built the seam this change spends. Thirty-four registered `prepare`/`forward` entry points opened their type-erased `LoadedModel&` with a bare downcast: auto& llama = static_cast(model); A `static_cast` down a hierarchy is a promise the compiler is entitled to act on. On an object whose dynamic type is not that model, every member call through the reference is type confusion. Production dispatch is self-consistent, which is why none of them ever fired: `ModelRegistry::Forward` routes through `model.registration().factory->forward`, so the type always matched. #775 surfaced only because a doctest stub reached one entry point while the sanitizer lane happened to be watching. The other 34 were latent by that same luck. All 34 now call `ModelAs(model, "")`, which establishes the dynamic type and refuses by name. The class residue under `src/vllm/model_executor/models/` is zero. THE DECISION #847 SAID BLOCKED THE SWEEP. Six registries share one forward across two registered architectures, so #847 called the refusal name a design question with three defensible answers. It is answerable: the aliases share ONE `ModelFactory` and therefore produce ONE `LoadedModel` subclass, so the string names the factory whose `load_weights` produces the type the entry point opens, and there is exactly one of those per site. That is also literally what the message says. Where the canonical name is already written down the sweep takes it from there (`RegistrationFor("Gemma4ForConditionalGeneration")` and two more) rather than choosing it. The refusal still reports the passed model's own registration separately, so an alias mismatch prints both names. THREE CORRECTIONS TO #847, all of them reasons it gave for the sweep being hard. Its multi-registration counts are wrong: the three registries it says carry three `REGISTER_VLLM_MODEL` lines carry two, and `mistral_registry.cpp` carries one, not two. It names four affected files and there are six; `olmo2_registry.cpp`, `muse_glimmer_registry.cpp` and `qwen3_5_moe.cpp` are missing from it. And the 14 `const auto&` sites do NOT select `ModelAs`'s const overload as both #847 and the dispatch assumed: `model` is a non-const `LoadedModel&` at all 34 sites, so overload resolution picks the non-const one and its result binds to the `const auto&`. Deleting the const overload and compiling all 30 swept TUs `-fsyntax-only` compiles all 30, so that overload has no caller in the tree at all. Filed as #897; the conclusion was right and the stated reason was not, which is worse than an open question. RED FIRST, on a class that is latent by construction so a naive test passes before and after. `test_registry_downcast_refusal.cpp` is one case per SHAPE, not per site: non-const forward, `const auto&` forward, an alias family on each, a `prepare` entry point, and the ONE site that was not `(const )?auto& x = cast(model);` but an inline member call on the cast expression (`qwen3_vl_registry.cpp:105`) -- the site a uniform search-and-replace over the other 33 steps past. Every case enters through the production seam (`reg.factory->forward` / `->prepare`) and hands it a foreign `LoadedModel` carrying the entry point's OWN registration, so the registration is right and only the object is wrong. That is the shape a cheaper architecture-string comparison cannot catch, which is why it is not one. Against the unmodified tree, Debug + address,undefined under `setarch -R`, each case run alone because `-fno-sanitize-recover=all` aborts on the first finding: all six reproduce #775's diagnostic verbatim at six distinct sites, process exit 1, no doctest summary. After the sweep: 6 cases, 33 assertions, 0 failed, no sanitizer finding. MUTATION. M2, replacing the refusal message with "model type mismatch", fails 6/6 cases and 19 of 33 assertions, so the tests assert a NAMED refusal rather than the absence of an abort. M3 is the one that matters here: a single WRONG architecture string (`olmo2_registry.cpp` refusing under `Olmo3ForCausalLM` instead of the family primary) compiles, runs, still refuses, and is caught by exactly one assertion. Thirty files of mechanical edits fail silently that way, and nothing but an assertion on message content sees it. COST. `ModelAs` is a `dynamic_cast` and a branch, entered once per forward step through `ModelRegistry::Forward` and never per layer, against a step that is milliseconds of GEMMs. No number is claimed and none is owed: what was checked is that the call count is unchanged and per-step -- each of the 34 is the first statement of its entry point, nothing moved into a loop, no site gained a second cast. BUILT, because 30 files of mechanical edits is exactly where a wrong architecture string or a missed `const` compiles and misbehaves. Full CPU sanitizer build of every target, 100%, zero errors. `agent-preflight.sh` and `pytest tests/scripts/` both show ZERO delta against `98f8e046d`: the same seven gates and the same nine pytest failures fail on the unmodified baseline worktree. NOT DONE HERE, deliberately. A checker refusing a new `static_cast<...LoadedModel&>` is warranted -- the class is exactly grep-able, unlike the unaligned-read class in #627, and it regrows the moment a new port copies its neighbour -- but a repository-wide refusal gate changes what every future PR must satisfy and does not belong in the same diff as the sweep it would guard. Decided yes and filed as #896. Making `forward` virtual on `LoadedModel` was weighed again and rejected: it retires the class outright and it is a seam redesign wearing a bug fix's clothes. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: Claude:claude-opus-5 [Claude Code] --- .agents/issue-index.md | 2 + .agents/specs/nemotron-h-model.md | 22 +- .agents/specs/registry-downcast-sweep.md | 318 ++++++++++++++++++ docs/FEATURES.md | 2 +- docs/USAGE.md | 13 +- .../models/commandr_registry.cpp | 2 +- .../models/deepseek_v2_registry.cpp | 2 +- .../models/deepseek_v4_registry.cpp | 2 +- .../model_executor/models/gemma2_registry.cpp | 2 +- .../model_executor/models/gemma3_registry.cpp | 2 +- .../model_executor/models/gemma4_registry.cpp | 4 +- .../model_executor/models/gemma_registry.cpp | 2 +- .../models/glm4_moe_lite_registry.cpp | 2 +- .../model_executor/models/glm4_registry.cpp | 2 +- .../models/granite_registry.cpp | 2 +- .../models/internlm2_registry.cpp | 2 +- .../models/kimi_k3_registry.cpp | 2 +- .../models/kimi_linear_registry.cpp | 2 +- .../model_executor/models/laguna_registry.cpp | 2 +- .../models/llama_embedding_registry.cpp | 2 +- .../model_executor/models/llama_registry.cpp | 2 +- .../models/minicpm3_registry.cpp | 2 +- .../models/minicpm_registry.cpp | 2 +- .../models/mistral_registry.cpp | 2 +- .../models/muse_glimmer_registry.cpp | 2 +- .../model_executor/models/olmo2_registry.cpp | 2 +- .../model_executor/models/opt_registry.cpp | 2 +- .../model_executor/models/phi3_registry.cpp | 2 +- .../model_executor/models/phi_registry.cpp | 2 +- .../model_executor/models/qwen3_5_dense.cpp | 4 +- .../model_executor/models/qwen3_5_moe.cpp | 4 +- .../model_executor/models/qwen3_dense.cpp | 2 +- .../models/qwen3_moe_registry.cpp | 2 +- .../models/qwen3_vl_registry.cpp | 8 +- .../models/stablelm_registry.cpp | 2 +- tests/CMakeLists.txt | 11 + .../models/test_registry_downcast_refusal.cpp | 223 ++++++++++++ 37 files changed, 616 insertions(+), 47 deletions(-) create mode 100644 .agents/specs/registry-downcast-sweep.md create mode 100644 tests/vllm/models/test_registry_downcast_refusal.cpp diff --git a/.agents/issue-index.md b/.agents/issue-index.md index 6a9221b8f..e7a5854bc 100644 --- a/.agents/issue-index.md +++ b/.agents/issue-index.md @@ -221,3 +221,5 @@ rather than merged. `scripts/check-agent-record.py` gates both. | [#513](https://github.com/mudler/vllm.cpp/issues/513) | `SPEC-DSPARK` | DFlash/DSpark draft loading hardcodes the VL weight prefix, so a text-only Qwen3.5 target cannot share its embedding | bug | | [#536](https://github.com/mudler/vllm.cpp/issues/536) | `ROAD-V1-C3` | C3's named tail records DSpark as unspiked three days after it landed, and files TLI under the wrong lane | bug | | [#886](https://github.com/mudler/vllm.cpp/issues/886) | `POLICY-NOTHING-LANDS-DEAD` | The seam checkers police where a capability routes and nothing polices whether anything reaches it, so a change can land wired downward and driven only by its own test, as tensor parallelism did (guide [`reachability.md`](reachability.md)) | feature | +| [#896](https://github.com/mudler/vllm.cpp/issues/896) | — | No gate refuses a new unchecked `static_cast<...LoadedModel&>` in a registry entry point. The #847 sweep decided the checker is warranted — the class is exactly grep-able, unlike the unaligned-read class in #627, and it regrows the moment a new model port copies its neighbour — and deliberately did not bundle a repository-wide refusal gate into a 30-file mechanical sweep. Listed under `## Owed` in [`registry-downcast-sweep.md`](specs/registry-downcast-sweep.md) §6 | feature | +| [#897](https://github.com/mudler/vllm.cpp/issues/897) | — | `ModelAs`'s `const LoadedModel&` overload has no caller anywhere in the tree: every registered `prepare`/`forward` takes a NON-const `LoadedModel&`, so all 35 call sites select the non-const overload and its `Model&` result merely binds to the `const auto&`. Proven by deleting the overload and compiling all 30 swept TUs `-fsyntax-only`. It is dead in the way that reads as covered — #847 cited its existence as the reason the 14 `const` sites needed no thought. Listed under `## Owed` in [`registry-downcast-sweep.md`](specs/registry-downcast-sweep.md) §3.3 | bug | diff --git a/.agents/specs/nemotron-h-model.md b/.agents/specs/nemotron-h-model.md index 17c5b5ad4..f35016dc0 100644 --- a/.agents/specs/nemotron-h-model.md +++ b/.agents/specs/nemotron-h-model.md @@ -2024,11 +2024,17 @@ forward is still the HOST reference and nothing runs on the paged runner (W6). ## Owed -- [#847](https://github.com/mudler/vllm.cpp/issues/847) — the registry - type-confusion class this row's §6d fix names but does not sweep: 34 - `prepare`/`forward` entry points across 32 model TUs still open a type-erased - `LoadedModel&` with an unchecked `static_cast`. Owed here, by the row that - found it, until a row claims the sweep. It is NOT NemotronH work — the sweep's - blocking question is what architecture name a shared forward refuses under when - one TU registers three architectures — and this entry exists so the class has a - named owner rather than sitting unowned. +- [#847](https://github.com/mudler/vllm.cpp/issues/847) — **SWEPT, no longer + owed here.** Row `FIX-REGISTRY-DOWNCAST-SWEEP` claimed the registry + type-confusion class this row's §6d fix named but did not sweep, and closed it: + all 34 remaining `prepare`/`forward` entry points now open their handle through + `ModelAs`, and the class residue is zero. Spec + [`registry-downcast-sweep.md`](registry-downcast-sweep.md), which also answers + the blocking question (a shared forward refuses under the FAMILY PRIMARY — the + architecture whose `load_weights` produces the type it opens) and corrects this + entry's arithmetic: 30 model TUs, not 32, and the affected registries carry two + architectures each, not three. + + The entry stays rather than being deleted because #847's row in the + append-only `.agents/issue-index.md` names no owning row, so a spec must keep + claiming it; GitHub holds the closed state. Do not read it as open work. diff --git a/.agents/specs/registry-downcast-sweep.md b/.agents/specs/registry-downcast-sweep.md new file mode 100644 index 000000000..c3b93fd63 --- /dev/null +++ b/.agents/specs/registry-downcast-sweep.md @@ -0,0 +1,318 @@ +# The registry's 34 remaining downcasts were promises, not checks + +Issue: [#847](https://github.com/mudler/vllm.cpp/issues/847) +Row: `FIX-REGISTRY-DOWNCAST-SWEEP` +Precedent: [#775](https://github.com/mudler/vllm.cpp/issues/775) / PR #868 +(`34962d96b`), which fixed one named site and created the seam this row spends. + +## 1. Scope + +Replace every remaining unchecked `static_cast(model)` in a +registered `prepare`/`forward` entry point with the checked +`ModelAs(model, "")` seam, and gate the class. + +**Out of scope, deliberately:** making `forward` a virtual on `LoadedModel` +(§5.2), and adding a checker that refuses a new `static_cast<...LoadedModel&>` +(§6). Both are recorded rather than done. + +## 2. What the defect is + +A `static_cast` down a hierarchy is a promise the compiler is entitled to act +on. On an object whose dynamic type is not that class, every member call through +the resulting reference is undefined behaviour. UBSan's vptr check reports +`member call on address ... which does not point to an object of type 'X'`, and +`-fno-sanitize-recover=all` aborts. + +Production dispatch is self-consistent — `ModelRegistry::Forward` routes through +`model.registration().factory->forward`, so the dynamic type always matches — and +that is exactly why the class is invisible. #775 surfaced only because a doctest +`StubModel` reached one of these entry points and the sanitizer lane was +watching. The other 34 were latent by the same luck. + +## 3. The measured shape + +`grep -rn 'static_cast<[A-Za-z0-9_:]*LoadedModel\s*&>' src/ include/` at +`98f8e046d`: **34 sites across 30 files**. Every site is inside a registered +`Prepare*`/`Forward*` whose first parameter is `LoadedModel& model`, and every +site casts that same `model`. There is no `const`-reference spelling and no +pointer spelling, so the grep is the whole class. 14 sites bind the result to a +`const auto&`, 20 to a plain `auto&`; 29 are `forward` entry points and 5 are +`prepare`. + +**None of the 34 turned out to be safe.** Each one is a real downcast of a +type-erased handle, reached through a live `ModelFactory` function pointer. + +### Three corrections to #847's description of the shape + +The issue's estimate was made before the sweep looked at the tree. It is wrong +in three ways that matter, because all three were reasons it gave for calling +the sweep non-mechanical. + +1. **The multi-registration count is wrong.** #847 says `llama_registry.cpp`, + `qwen3_5_dense.cpp` and `gemma4_registry.cpp` "each carry 3 + `REGISTER_VLLM_MODEL` lines" and `mistral_registry.cpp` two. Measured: each of + those three carries **two**, and `mistral_registry.cpp` carries **one**. +2. **It missed half the affected families.** Six files register more than one + architecture against one factory, not four: `gemma4_registry.cpp`, + `llama_registry.cpp`, `muse_glimmer_registry.cpp`, `olmo2_registry.cpp`, + `qwen3_5_dense.cpp`, `qwen3_5_moe.cpp`. The last three are not named in the + issue at all. +3. **The `const` overload is not what covers the `const` sites.** Both #847 and + the dispatch that opened this row assumed the 14 `const auto&` sites select + `ModelAs`'s `const LoadedModel&` overload. They do not: `model` is a non-const + `LoadedModel&` at all 34 sites, so overload resolution picks the non-const + `ModelAs` and its `Model&` result binds to the `const auto&`. Proven by + mutation — deleting the const overload from the header and running + `-fsyntax-only` over all 30 swept translation units compiles all 30 (§7, M4). + **The const overload therefore has no caller anywhere in the tree.** It is + left in place (it is #868's, it is harmless, and deleting another row's seam + half inside a sweep is not this row's business) and recorded under `## Owed`. + +## 4. The decision the sweep had to make + +`ModelAs` takes the caller's own architecture name so a refusal says *which* +entry point refused. Six entry points serve two registered architectures each, +so #847 called this a design question with three defensible answers and no +single correct string. + +**It is answerable, and the answer is #847's option 1: the family primary.** The +reason it is not arbitrary is that the aliases in every one of the six cases +share **one `ModelFactory`** and therefore produce **one `LoadedModel` subclass**. +The string does not identify a registration; it identifies the *factory whose +`load_weights` produces the type this entry point opens*, and there is exactly +one of those per site. That is also literally what the message says — "was not +produced by X's own `load_weights`" — so the primary is the true answer to the +sentence, not an approximation of it. + +Where a canonical name is already written down, it is taken from there rather +than chosen: `gemma4_registry.cpp`, `qwen3_5_dense.cpp` and `qwen3_5_moe.cpp` +each call `RegistrationFor("")` in their own loader, and the sweep uses +that exact string. For `llama_registry.cpp`, `olmo2_registry.cpp` and +`muse_glimmer_registry.cpp` the primary is the first `REGISTER_VLLM_MODEL` line, +which each file's own comment marks as the real model and the second as an alias +("Alias only; zero forward/loader delta", "Both arch strings map to the SAME +factory"). + +The refusal still reports the passed model's own registration separately, so an +alias mismatch prints both strings and neither is lost. The class gate asserts +exactly that on two alias families (§7). + +Options 2 and 3 from #847 are rejected: option 2 (report only +`model.registration().architecture`) drops the entry point's identity, which is +the only thing the parameter exists to add; option 3 (an overload taking +`ModelFactory*` and reporting every architecture registered against it) needs a +reverse index from factory to registrations that the registry does not keep, to +print a list where one name is correct. + +### The architecture string per site + +| File | Sites | Refuses under | Note | +|---|---:|---|---| +| `commandr_registry.cpp` | 69 | `CohereForCausalLM` | file name and arch differ | +| `deepseek_v2_registry.cpp` | 87 | `DeepseekV2ForCausalLM` | | +| `deepseek_v4_registry.cpp` | 101 | `DeepseekV4ForCausalLM` | | +| `gemma2_registry.cpp` | 69 | `Gemma2ForCausalLM` | | +| `gemma3_registry.cpp` | 70 | `Gemma3ForCausalLM` | | +| `gemma4_registry.cpp` | 104, 138 | `Gemma4ForConditionalGeneration` | alias family; `RegistrationFor` anchor | +| `gemma_registry.cpp` | 68 | `GemmaForCausalLM` | | +| `glm4_moe_lite_registry.cpp` | 114 | `Glm4MoeLiteForCausalLM` | | +| `glm4_registry.cpp` | 70 | `Glm4ForCausalLM` | | +| `granite_registry.cpp` | 67 | `GraniteForCausalLM` | | +| `internlm2_registry.cpp` | 94 | `InternLM2ForCausalLM` | | +| `kimi_k3_registry.cpp` | 81 | `KimiK3ForConditionalGeneration` | | +| `kimi_linear_registry.cpp` | 90 | `KimiLinearForCausalLM` | | +| `laguna_registry.cpp` | 84 | `LagunaForCausalLM` | | +| `llama_embedding_registry.cpp` | 114 | `LlamaModel` | the arch really is `LlamaModel` | +| `llama_registry.cpp` | 103 | `LlamaForCausalLM` | alias family (`InternLM3ForCausalLM`) | +| `minicpm3_registry.cpp` | 71 | `MiniCPM3ForCausalLM` | | +| `minicpm_registry.cpp` | 67 | `MiniCPMForCausalLM` | | +| `mistral_registry.cpp` | 91 | `MistralForCausalLM` | one registration, not two | +| `muse_glimmer_registry.cpp` | 89 | `MuseGlimmerForCausalLM` | alias family (`...ForConditionalGeneration`) | +| `olmo2_registry.cpp` | 74 | `Olmo2ForCausalLM` | alias family (`Olmo3ForCausalLM`) | +| `opt_registry.cpp` | 96 | `OPTForCausalLM` | | +| `phi3_registry.cpp` | 66 | `Phi3ForCausalLM` | | +| `phi_registry.cpp` | 71 | `PhiForCausalLM` | | +| `qwen3_5_dense.cpp` | 109, 120 | `Qwen3_5ForConditionalGeneration` | alias family; `RegistrationFor` anchor | +| `qwen3_5_moe.cpp` | 91, 97 | `Qwen3_5MoeForConditionalGeneration` | alias family; `RegistrationFor` anchor | +| `qwen3_dense.cpp` | 87 | `Qwen3ForCausalLM` | | +| `qwen3_moe_registry.cpp` | 87 | `Qwen3MoeForCausalLM` | | +| `qwen3_vl_registry.cpp` | 105, 110 | `Qwen3VLForConditionalGeneration` | 105 is the inline site, §5.1 | +| `stablelm_registry.cpp` | 69 | `StableLmForCausalLM` | arch spelling is `StableLm`, type is `Stablelm` | + +The sweep ran from a table keyed by file, asserting per file that the number of +`LoadedModel` casts *and* the number of casts of the expected type both equal the +planned count before writing anything, and that the plan totals 34. A plan that +matched a near-duplicate anchor, or a file whose count had drifted, refuses +rather than writing ([[assert-anchor-uniqueness-not-existence]]). + +## 5. The two judgement calls inside the sweep + +### 5.1 `qwen3_vl_registry.cpp:105`, the one site that is not the common shape + +Every other site is `(const )?auto& x = static_cast(model);`. This one makes +the member call on the cast expression itself: + +```cpp +static_cast(model).CosSinCache(queue, config); +``` + +It stays inline on the checked reference: + +```cpp +ModelAs(model, "Qwen3VLForConditionalGeneration") + .CosSinCache(queue, config); +``` + +Introducing a local was considered and rejected: `ModelAs` establishes the +dynamic type before the member call either way, so a binding would change the +site's shape without changing what it does, and a sweep that quietly restyles a +site it is only meant to make safe is harder to review, not easier. The wrap is +for the 100-column limit. This is the site a uniform search-and-replace over the +other 33 would step past, so the class gate covers it explicitly. + +### 5.2 No per-site comment + +#868 put a six-line comment at its single site. Repeating that 34 times would be +noise, and it would put 34 copies of one explanation where the seam already +carries it once (`model_registry.h:187-206`), which is the same reason #868 +authored the refusal itself out of line. The one exception is the inline site, +where the *shape* decision above is not self-evident from the diff. + +## 6. The checker question #847 raised + +#847 asks whether a checker should refuse a new `static_cast<...LoadedModel&>` +under `src/vllm/model_executor/models/`. **Answer: yes, and not in this change.** + +Yes, because the class is exactly grep-able — one regular expression, no false +positives in the tree today, and the residue after this sweep is zero — which is +what makes a checker cheap and exact here and does not make one cheap for the +unaligned-read class in #627. + +Not here, because a repository-wide refusal gate changes what every future pull +request must satisfy. AGENTS.md requires a spec, a red-before test and +green-after evidence for a change to a checker's semantics, and introducing one +is at least that. Bundling it into a 30-file mechanical sweep gives a reviewer +two unrelated things to judge at once and makes the argument for each weaker. +Recorded under `## Owed` as +[#896](https://github.com/mudler/vllm.cpp/issues/896). + +## 7. Tests and evidence + +`tests/vllm/models/test_registry_downcast_refusal.cpp` — one case per **shape**, +not per site, because the failure modes of a sweep are shape-shaped. Every case +enters through the production seam (`reg.factory->forward` / `->prepare`, which +is what `ModelRegistry::Forward` calls) and hands it a `ForeignLoadedModel`: a +complete, well-formed `LoadedModel` carrying the entry point's *own* +registration, so the registration is right and only the object is wrong. That is +deliberately the case a weaker check cannot catch (§8.1). + +| Case | Site | Shape | +|---|---|---| +| a NON-CONST forward | `qwen3_dense.cpp:87` | plain `auto&`, single registration | +| a CONST-bound forward | `stablelm_registry.cpp:69` | `const auto&` binding | +| an ALIAS arch refuses under the FAMILY PRIMARY | `olmo2_registry.cpp:74` | alias family + `const auto&` | +| a non-const ALIAS forward names both archs | `llama_registry.cpp:103` | alias family, unrelated spellings | +| a PREPARE entry point | `qwen3_5_moe.cpp:91` + `:97` | `prepare`, plus its `forward` sibling | +| the INLINE-call prepare site | `qwen3_vl_registry.cpp:105` | §5.1 | + +**RED, against the unmodified tree** at `98f8e046d`, Debug + `address,undefined`, +each case run alone because `-fno-sanitize-recover=all` aborts on the first +finding. All six reproduce #775's diagnostic verbatim at six distinct sites, +process exit 1, no doctest summary reached: + +| Case | UBSan report | +|---|---| +| non-const forward | `qwen3_dense.cpp:88:50: member call on address ... not ... 'Qwen3DenseLoadedModel'` | +| const-bound forward | `stablelm_registry.cpp:70:46: ... not ... 'StablelmLoadedModel'` | +| alias, const | `olmo2_registry.cpp:75:45: ... not ... 'Olmo2LoadedModel'` | +| alias, non-const | `llama_registry.cpp:104:46: ... not ... 'LlamaLoadedModel'` | +| prepare | `qwen3_5_moe.cpp:92:51: ... not ... 'Qwen3_5MoeLoadedModel'` | +| inline prepare | `qwen3_vl_registry.cpp:105:54: ... not ... 'Qwen3VLLoadedModel'` | + +Each carried `note: object is of type 'ForeignLoadedModel'` and a stack naming +the entry point. As in #775 the report lands on the member call, one line past +the cast, except at the inline site where cast and call share line 105. + +**GREEN after the sweep:** 6 cases, 33 assertions, 0 failed, no sanitizer +finding. + +**Mutation proof.** Each applied alone, rebuilt, run, reverted, tree verified +clean. + +| # | Mutation | Result | +|---|---|---| +| M1 | the sweep itself, absent | RED — the six reports above | +| M2 | `RaiseModelTypeMismatch` message replaced by `"model type mismatch"` | RED: 6/6 cases fail, 19 of 33 assertions — the tests assert a NAMED refusal, not merely the absence of an abort | +| M3 | one **wrong architecture string**: `olmo2_registry.cpp` refuses under `"Olmo3ForCausalLM"` instead of the family primary | RED: 1 case, 1 assertion — a wrong string compiles and runs, and only this gate sees it | +| M4 | the `const ModelAs` overload deleted from the header | GREEN, all 30 swept TUs still compile under `-fsyntax-only` — which is the *finding* of §3.3, not a passing mutation | + +M3 is the one that matters for this row specifically. Thirty files of mechanical +edits fail silently through a wrong architecture name: it compiles, it runs, the +refusal still fires, and only the message is wrong. Nothing but an assertion on +the message content catches it. + +## 8. Rejected alternatives + +### 8.1 Compare `model.registration().architecture` to the expected name + +Needs no RTTI and answers the wrong question: it establishes what the +registration *claims*, not what the object *is*. The realistic defect — a caller +that resolves a registration and hands `factory->forward` a model another path +produced — carries the right architecture string on the wrong object, so this +check passes and the UB proceeds. Every case in the class gate is that shape. +Already rejected by #775; restated because a sweep is where it would be +re-proposed as the cheaper option. + +### 8.2 Make `forward` a virtual on `LoadedModel` + +Removes the cast class outright and is arguably the better design. It is also a +change to the `ModelFactory` seam and 30 model translation units, against a +deliberate type-erasure contract (`model_registry.h:340`, "forward remains +type-erased over LoadedModel"). Weighed here because #775 explicitly deferred it +to the sweep, and still rejected: it is a seam redesign wearing a bug fix's +clothes, and it would land in the same diff as 34 edits it makes redundant. +Recorded under `## Owed`. + +### 8.3 One test per site + +Thirty-four near-identical cases assert the sweep rather than the guarantee, and +they would all pass against a sweep with 34 wrong architecture strings unless +each one also hard-coded its own expected string — at which point the test is a +transcription of the diff. One case per shape, plus M3, is what actually +distinguishes a correct sweep from a plausible one. + +## 9. Cost + +`ModelAs` is a `dynamic_cast` and a branch. The seam is entered once per +`ModelRegistry::Forward` — one forward *step* — and never per layer, against a +step that is milliseconds of GEMMs. `prepare` is once per load. + +No throughput measurement is claimed for this row and none is owed: this change +adds no work to any loop, and the honest statement is what was checked rather +than a number produced to look like evidence. What was checked is that the call +count is unchanged and per-step: the 34 sites are the *first* statement of their +entry point, each entry point is reached only through `factory->prepare` / +`factory->forward`, and `ModelRegistry::Forward` is that seam's single call site. +Nothing was moved into a loop, and no site gained a second cast. + +## 10. Now + +Landed as the sweep of #847. The class residue under +`src/vllm/model_executor/models/` is **zero**: `grep -rn +'static_cast<[A-Za-z0-9_:]*LoadedModel\s*&>' src/ include/` returns nothing, and +`ModelAs` has 35 call sites there — the 34 swept plus #868's NemotronH one. + +## Owed + +- [#896](https://github.com/mudler/vllm.cpp/issues/896) — a checker refusing a + new `static_cast<...LoadedModel&>` under `src/vllm/model_executor/models/` + (§6). Decided yes, scoped to its own row so the gate change is reviewable on + its own argument. +- [#897](https://github.com/mudler/vllm.cpp/issues/897) — `ModelAs`'s + `const LoadedModel&` overload has **no caller in the tree** (§3.3, M4). Either + a caller appears when an entry point takes a `const LoadedModel&`, or the + overload goes. Not decided here because it is #868's seam. +- Making `forward` virtual on `LoadedModel` and retiring the downcast class + outright (§8.2). Weighed and rejected for this row, not for the project. No + issue filed: it is a design option this spec records having considered, not a + defect, and #847 already names it. diff --git a/docs/FEATURES.md b/docs/FEATURES.md index 287c8a006..8075d46d8 100644 --- a/docs/FEATURES.md +++ b/docs/FEATURES.md @@ -279,7 +279,7 @@ Build with `-DVLLM_CPP_VULKAN=ON`; off by default. | Container images | ◐ `cuda`/`vulkan`/`cpu` lanes build and gate from one Dockerfile (amd64+arm64, `ENTRYPOINT vllm-server`, ffmpeg included); **nothing published to GHCR yet** | ✅ | ✅ | ✅ | | Graceful shutdown on `SIGTERM` | ✅ clean exit in 0.25 s, including as container PID 1 (#312) | ✅ | ✅ | ✅ | | Plugin / out-of-tree model registration | ✅ in-tree factory `DONE` + plugin seam | ✅ | ◐ | ☐ | -| A registered forward opens its OWN model type, not whatever it was handed | ◐ `NemotronHForCausalLM` establishes the concrete type and refuses a mismatch by name (#775); 34 other entry points still assume it ([#847](https://github.com/mudler/vllm.cpp/issues/847)) | n/a | n/a | n/a | +| A registered forward opens its OWN model type, not whatever it was handed | ✅ all 35 entry points establish the concrete type first and refuse a mismatch by name (#775, swept in [#847](https://github.com/mudler/vllm.cpp/issues/847)) | n/a | n/a | n/a | | Multiple engines in one process (build, destroy, rebuild) | ✅ resident device state is owned by the weights, so a new engine never inherits a freed one's pointers | ✅ | ✅ | ✅ | | LoRA adapters | ☐ CPU brick only | ✅ | ✅ | ✅ | | Embedding / pooling endpoints | ◐ `/v1/embeddings` live (task=embed; score/rerank/classify pending) | ✅ | ✅ | ✅ | diff --git a/docs/USAGE.md b/docs/USAGE.md index fbac7a728..cdbe3616d 100644 --- a/docs/USAGE.md +++ b/docs/USAGE.md @@ -447,10 +447,15 @@ tokens quietly. This is a deliberate state, not a bug: registering the architecture is what lets the config parse and weight-name mapping be tested before the forward exists. -A refusal here is always a thrown message you can read. `NemotronHForCausalLM` -also refuses when it is handed a model some other architecture loaded, naming -both itself and the architecture the passed model claims, instead of reading -that model as though it were its own (#775). +A refusal here is always a thrown message you can read. Every registered +architecture also refuses when it is handed a model some other architecture +loaded, naming both itself and the architecture the passed model claims, instead +of reading that model as though it were its own (#775, swept across the +remaining 34 entry points in #847). Where two architecture names share one +implementation — `Olmo2ForCausalLM` and `Olmo3ForCausalLM`, or +`LlamaForCausalLM` and `InternLM3ForCausalLM` — the refusal names the family's +primary architecture as the one that refused, and the alias you asked for as +what the passed model claimed. ### LTX-2.5: what runs, and what it cannot do diff --git a/src/vllm/model_executor/models/commandr_registry.cpp b/src/vllm/model_executor/models/commandr_registry.cpp index a56c50d1e..cbae8943c 100644 --- a/src/vllm/model_executor/models/commandr_registry.cpp +++ b/src/vllm/model_executor/models/commandr_registry.cpp @@ -66,7 +66,7 @@ void PrepareCohereForCausalLM(LoadedModel& model, const HfConfig& config, ForwardLogits ForwardCohereForCausalLM(LoadedModel& model, const ModelForwardInput& input) { - const auto& cm = static_cast(model); + const auto& cm = ModelAs(model, "CohereForCausalLM"); const CommandrWeights& weights = cm.weights(); if (input.gather_logits) { return CommandrModel::ForwardDevice(input.token_ids, input.positions, diff --git a/src/vllm/model_executor/models/deepseek_v2_registry.cpp b/src/vllm/model_executor/models/deepseek_v2_registry.cpp index dcc1f935e..3499aa8bf 100644 --- a/src/vllm/model_executor/models/deepseek_v2_registry.cpp +++ b/src/vllm/model_executor/models/deepseek_v2_registry.cpp @@ -84,7 +84,7 @@ void PrepareDeepseekV2ForCausalLM(LoadedModel& model, const HfConfig& config, ForwardLogits ForwardDeepseekV2ForCausalLM(LoadedModel& model, const ModelForwardInput& input) { - auto& ds = static_cast(model); + auto& ds = ModelAs(model, "DeepseekV2ForCausalLM"); const DeepseekV2Weights& weights = ds.weights(); // DECODE CUDA-GRAPH path (W9): route a PURE-DECODE CUDA step through the diff --git a/src/vllm/model_executor/models/deepseek_v4_registry.cpp b/src/vllm/model_executor/models/deepseek_v4_registry.cpp index bfd2f9ed9..bf528f825 100644 --- a/src/vllm/model_executor/models/deepseek_v4_registry.cpp +++ b/src/vllm/model_executor/models/deepseek_v4_registry.cpp @@ -98,7 +98,7 @@ void PrepareDeepseekV4ForCausalLM(LoadedModel& model, const HfConfig& config, ForwardLogits ForwardDeepseekV4ForCausalLM(LoadedModel& model, const ModelForwardInput& input) { - auto& ds = static_cast(model); + auto& ds = ModelAs(model, "DeepseekV4ForCausalLM"); const DeepseekV4Weights& weights = ds.weights(); if (input.gather_logits) { return DeepseekV4Model::ForwardDevice(input.token_ids, input.positions, diff --git a/src/vllm/model_executor/models/gemma2_registry.cpp b/src/vllm/model_executor/models/gemma2_registry.cpp index b615a30d5..99f8e327e 100644 --- a/src/vllm/model_executor/models/gemma2_registry.cpp +++ b/src/vllm/model_executor/models/gemma2_registry.cpp @@ -66,7 +66,7 @@ void PrepareGemma2ForCausalLM(LoadedModel& model, const HfConfig& config, ForwardLogits ForwardGemma2ForCausalLM(LoadedModel& model, const ModelForwardInput& input) { - const auto& gemma = static_cast(model); + const auto& gemma = ModelAs(model, "Gemma2ForCausalLM"); const Gemma2Weights& weights = gemma.weights(); if (input.gather_logits) { return Gemma2Model::ForwardDevice(input.token_ids, input.positions, diff --git a/src/vllm/model_executor/models/gemma3_registry.cpp b/src/vllm/model_executor/models/gemma3_registry.cpp index e9a55da87..38d758a2e 100644 --- a/src/vllm/model_executor/models/gemma3_registry.cpp +++ b/src/vllm/model_executor/models/gemma3_registry.cpp @@ -67,7 +67,7 @@ void PrepareGemma3ForCausalLM(LoadedModel& model, const HfConfig& config, ForwardLogits ForwardGemma3ForCausalLM(LoadedModel& model, const ModelForwardInput& input) { - const auto& gemma = static_cast(model); + const auto& gemma = ModelAs(model, "Gemma3ForCausalLM"); const Gemma3Weights& weights = gemma.weights(); if (input.gather_logits) { return Gemma3Model::ForwardDevice(input.token_ids, input.positions, diff --git a/src/vllm/model_executor/models/gemma4_registry.cpp b/src/vllm/model_executor/models/gemma4_registry.cpp index 214c9b08d..bece69a9e 100644 --- a/src/vllm/model_executor/models/gemma4_registry.cpp +++ b/src/vllm/model_executor/models/gemma4_registry.cpp @@ -101,7 +101,7 @@ void PrepareGemma4ForConditionalGeneration(LoadedModel& model, (void)queue; const char* env = std::getenv("VT_GEMMA4_RESIDENT_EXPERTS"); if (env == nullptr || env[0] != '1') return; - auto& gemma = static_cast(model); + auto& gemma = ModelAs(model, "Gemma4ForConditionalGeneration"); Gemma4Weights& w = const_cast(gemma.weights()); int ngpu = 2; // Upload clamps to hipGetDeviceCount if (const char* g = std::getenv("VT_GEMMA4_RESIDENT_GPUS")) @@ -135,7 +135,7 @@ void PrepareGemma4ForConditionalGeneration(LoadedModel& model, ForwardLogits ForwardGemma4ForConditionalGeneration( LoadedModel& model, const ModelForwardInput& input) { - const auto& gemma = static_cast(model); + const auto& gemma = ModelAs(model, "Gemma4ForConditionalGeneration"); const Gemma4Weights& weights = gemma.weights(); // CLAIM-GEMMA4-MM-E2E: the multimodal branch. When ModelForwardInput.mm is set // (the Gemma4GenerateGreedyViaRegistry driver / the runner mm-path) the hidden diff --git a/src/vllm/model_executor/models/gemma_registry.cpp b/src/vllm/model_executor/models/gemma_registry.cpp index 3a6858e1e..b800fd984 100644 --- a/src/vllm/model_executor/models/gemma_registry.cpp +++ b/src/vllm/model_executor/models/gemma_registry.cpp @@ -65,7 +65,7 @@ void PrepareGemmaForCausalLM(LoadedModel& model, const HfConfig& config, ForwardLogits ForwardGemmaForCausalLM(LoadedModel& model, const ModelForwardInput& input) { - const auto& gemma = static_cast(model); + const auto& gemma = ModelAs(model, "GemmaForCausalLM"); const GemmaWeights& weights = gemma.weights(); if (input.gather_logits) { return GemmaModel::ForwardDevice(input.token_ids, input.positions, diff --git a/src/vllm/model_executor/models/glm4_moe_lite_registry.cpp b/src/vllm/model_executor/models/glm4_moe_lite_registry.cpp index cf4eed7a9..40f029aee 100644 --- a/src/vllm/model_executor/models/glm4_moe_lite_registry.cpp +++ b/src/vllm/model_executor/models/glm4_moe_lite_registry.cpp @@ -111,7 +111,7 @@ void PrepareGlm4MoeLiteForCausalLM(LoadedModel& model, const HfConfig& config, ForwardLogits ForwardGlm4MoeLiteForCausalLM(LoadedModel& model, const ModelForwardInput& input) { - auto& glm = static_cast(model); + auto& glm = ModelAs(model, "Glm4MoeLiteForCausalLM"); const DeepseekV2Weights& weights = glm.weights(); // Identical dispatch to deepseek_v2_registry.cpp: a PURE-DECODE CUDA step goes diff --git a/src/vllm/model_executor/models/glm4_registry.cpp b/src/vllm/model_executor/models/glm4_registry.cpp index 7837604a3..7903eb70f 100644 --- a/src/vllm/model_executor/models/glm4_registry.cpp +++ b/src/vllm/model_executor/models/glm4_registry.cpp @@ -67,7 +67,7 @@ void PrepareGlm4ForCausalLM(LoadedModel& model, const HfConfig& config, ForwardLogits ForwardGlm4ForCausalLM(LoadedModel& model, const ModelForwardInput& input) { - const auto& glm = static_cast(model); + const auto& glm = ModelAs(model, "Glm4ForCausalLM"); const Glm4Weights& weights = glm.weights(); if (input.gather_logits) { return Glm4Model::ForwardDevice(input.token_ids, input.positions, diff --git a/src/vllm/model_executor/models/granite_registry.cpp b/src/vllm/model_executor/models/granite_registry.cpp index 34f97693c..bc65b5fe3 100644 --- a/src/vllm/model_executor/models/granite_registry.cpp +++ b/src/vllm/model_executor/models/granite_registry.cpp @@ -64,7 +64,7 @@ void PrepareGraniteForCausalLM(LoadedModel& model, const HfConfig& config, ForwardLogits ForwardGraniteForCausalLM(LoadedModel& model, const ModelForwardInput& input) { - const auto& granite = static_cast(model); + const auto& granite = ModelAs(model, "GraniteForCausalLM"); const GraniteWeights& weights = granite.weights(); if (input.gather_logits) { return GraniteModel::ForwardDevice(input.token_ids, input.positions, diff --git a/src/vllm/model_executor/models/internlm2_registry.cpp b/src/vllm/model_executor/models/internlm2_registry.cpp index 44f9cbe7f..d1fc0d8cb 100644 --- a/src/vllm/model_executor/models/internlm2_registry.cpp +++ b/src/vllm/model_executor/models/internlm2_registry.cpp @@ -91,7 +91,7 @@ ForwardLogits ForwardInternLM2ForCausalLM(LoadedModel& model, // non-async-CUDA path, RAII-scoped, byte-identical when the mirror is off. const detail::DeviceTokenIdsScope device_ids_scope( input.device_token_ids, static_cast(input.token_ids.size())); - auto& im2 = static_cast(model); + auto& im2 = ModelAs(model, "InternLM2ForCausalLM"); const InternLM2Weights& weights = im2.weights(); // Shared pure-dense decode CUDA-graph (opt-in via VLLM_CPP_QWEN3_DENSE_DECODE_ // GRAPH); std::nullopt falls through to the byte-identical eager path below. diff --git a/src/vllm/model_executor/models/kimi_k3_registry.cpp b/src/vllm/model_executor/models/kimi_k3_registry.cpp index a3e12f810..146182eed 100644 --- a/src/vllm/model_executor/models/kimi_k3_registry.cpp +++ b/src/vllm/model_executor/models/kimi_k3_registry.cpp @@ -78,7 +78,7 @@ void PrepareKimiK3ForConditionalGeneration(LoadedModel& model, ForwardLogits ForwardKimiK3ForConditionalGeneration( LoadedModel& model, const ModelForwardInput& input) { - auto& k3 = static_cast(model); + auto& k3 = ModelAs(model, "KimiK3ForConditionalGeneration"); const KimiK3Weights& weights = k3.weights(); if (input.gather_logits) { return KimiK3Model::ForwardDevice(input.token_ids, input.positions, diff --git a/src/vllm/model_executor/models/kimi_linear_registry.cpp b/src/vllm/model_executor/models/kimi_linear_registry.cpp index 3ad2f8c80..9b3390a22 100644 --- a/src/vllm/model_executor/models/kimi_linear_registry.cpp +++ b/src/vllm/model_executor/models/kimi_linear_registry.cpp @@ -87,7 +87,7 @@ void PrepareKimiLinearForCausalLM(LoadedModel& model, const HfConfig& config, ForwardLogits ForwardKimiLinearForCausalLM(LoadedModel& model, const ModelForwardInput& input) { - auto& kl = static_cast(model); + auto& kl = ModelAs(model, "KimiLinearForCausalLM"); const KimiLinearWeights& weights = kl.weights(); // ROW 7 (§20.3): the RUNNER path — real paged caches supplied (the runner // always hands its allocated attn_kv + gdn_state groups) with bf16-resident diff --git a/src/vllm/model_executor/models/laguna_registry.cpp b/src/vllm/model_executor/models/laguna_registry.cpp index 0d0af4f5b..01e3079a6 100644 --- a/src/vllm/model_executor/models/laguna_registry.cpp +++ b/src/vllm/model_executor/models/laguna_registry.cpp @@ -81,7 +81,7 @@ void PrepareLagunaForCausalLM(LoadedModel& model, const HfConfig& config, ForwardLogits ForwardLagunaForCausalLM(LoadedModel& model, const ModelForwardInput& input) { - auto& laguna = static_cast(model); + auto& laguna = ModelAs(model, "LagunaForCausalLM"); const LagunaWeights& weights = laguna.weights(); if (input.gather_logits) { return LagunaModel::ForwardDevice(input.token_ids, input.positions, diff --git a/src/vllm/model_executor/models/llama_embedding_registry.cpp b/src/vllm/model_executor/models/llama_embedding_registry.cpp index cf1e50b7c..447cd9f5b 100644 --- a/src/vllm/model_executor/models/llama_embedding_registry.cpp +++ b/src/vllm/model_executor/models/llama_embedding_registry.cpp @@ -111,7 +111,7 @@ ForwardLogits ForwardLlamaModelEmbedding(LoadedModel& model, // (pooling_runner.py:36). // (The runner passes empty logits_indices when the gather toggle is off; the // pooling branch then host-gathers, mirroring the text host path.) - auto& emb = static_cast(model); + auto& emb = ModelAs(model, "LlamaModel"); return LlamaModel::ForwardHidden(input.token_ids, input.positions, input.attn_meta, input.attn_kv, emb.weights(), input.config, input.queue, diff --git a/src/vllm/model_executor/models/llama_registry.cpp b/src/vllm/model_executor/models/llama_registry.cpp index 23ccde840..d85fcae63 100644 --- a/src/vllm/model_executor/models/llama_registry.cpp +++ b/src/vllm/model_executor/models/llama_registry.cpp @@ -100,7 +100,7 @@ ForwardLogits ForwardLlamaForCausalLM(LoadedModel& model, // non-async-CUDA path, RAII-scoped, byte-identical when the mirror is off. const detail::DeviceTokenIdsScope device_ids_scope( input.device_token_ids, static_cast(input.token_ids.size())); - auto& llama = static_cast(model); + auto& llama = ModelAs(model, "LlamaForCausalLM"); const LlamaWeights& weights = llama.weights(); // Shared pure-dense decode CUDA-graph (opt-in via VLLM_CPP_QWEN3_DENSE_DECODE_ // GRAPH); std::nullopt falls through to the byte-identical eager path below. diff --git a/src/vllm/model_executor/models/minicpm3_registry.cpp b/src/vllm/model_executor/models/minicpm3_registry.cpp index 3c0dc2c23..08cc57fe1 100644 --- a/src/vllm/model_executor/models/minicpm3_registry.cpp +++ b/src/vllm/model_executor/models/minicpm3_registry.cpp @@ -68,7 +68,7 @@ void PrepareMiniCPM3ForCausalLM(LoadedModel& model, const HfConfig& config, ForwardLogits ForwardMiniCPM3ForCausalLM(LoadedModel& model, const ModelForwardInput& input) { - const auto& m3 = static_cast(model); + const auto& m3 = ModelAs(model, "MiniCPM3ForCausalLM"); const MiniCPM3Weights& weights = m3.weights(); if (input.gather_logits) { return MiniCPM3Model::ForwardDevice(input.token_ids, input.positions, diff --git a/src/vllm/model_executor/models/minicpm_registry.cpp b/src/vllm/model_executor/models/minicpm_registry.cpp index 290197282..0f3845924 100644 --- a/src/vllm/model_executor/models/minicpm_registry.cpp +++ b/src/vllm/model_executor/models/minicpm_registry.cpp @@ -64,7 +64,7 @@ void PrepareMiniCPMForCausalLM(LoadedModel& model, const HfConfig& config, ForwardLogits ForwardMiniCPMForCausalLM(LoadedModel& model, const ModelForwardInput& input) { - const auto& minicpm = static_cast(model); + const auto& minicpm = ModelAs(model, "MiniCPMForCausalLM"); const MiniCPMWeights& weights = minicpm.weights(); if (input.gather_logits) { return MiniCPMModel::ForwardDevice(input.token_ids, input.positions, diff --git a/src/vllm/model_executor/models/mistral_registry.cpp b/src/vllm/model_executor/models/mistral_registry.cpp index 515796596..492034182 100644 --- a/src/vllm/model_executor/models/mistral_registry.cpp +++ b/src/vllm/model_executor/models/mistral_registry.cpp @@ -88,7 +88,7 @@ ForwardLogits ForwardMistralForCausalLM(LoadedModel& model, // non-async-CUDA path, RAII-scoped, byte-identical when the mirror is off. const detail::DeviceTokenIdsScope device_ids_scope( input.device_token_ids, static_cast(input.token_ids.size())); - auto& mistral = static_cast(model); + auto& mistral = ModelAs(model, "MistralForCausalLM"); const MistralWeights& weights = mistral.weights(); // Shared pure-dense decode CUDA-graph (opt-in via VLLM_CPP_QWEN3_DENSE_DECODE_ // GRAPH); std::nullopt falls through to the byte-identical eager path below. diff --git a/src/vllm/model_executor/models/muse_glimmer_registry.cpp b/src/vllm/model_executor/models/muse_glimmer_registry.cpp index 9c3bcc948..46ae71ab5 100644 --- a/src/vllm/model_executor/models/muse_glimmer_registry.cpp +++ b/src/vllm/model_executor/models/muse_glimmer_registry.cpp @@ -86,7 +86,7 @@ void PrepareMuseGlimmer(LoadedModel& model, const HfConfig& config, ForwardLogits ForwardMuseGlimmer(LoadedModel& model, const ModelForwardInput& input) { - auto& mg = static_cast(model); + auto& mg = ModelAs(model, "MuseGlimmerForCausalLM"); const MuseGlimmerWeights& weights = mg.weights(); // W4 WIRING: the MULTIMODAL branch. When ModelForwardInput.mm is set (the // MuseGlimmerGenerateGreedyViaRegistry driver / the runner mm-path) the hidden diff --git a/src/vllm/model_executor/models/olmo2_registry.cpp b/src/vllm/model_executor/models/olmo2_registry.cpp index 93a0ed2ab..9d1f73b5c 100644 --- a/src/vllm/model_executor/models/olmo2_registry.cpp +++ b/src/vllm/model_executor/models/olmo2_registry.cpp @@ -71,7 +71,7 @@ void PrepareOlmo2ForCausalLM(LoadedModel& model, const HfConfig& config, ForwardLogits ForwardOlmo2ForCausalLM(LoadedModel& model, const ModelForwardInput& input) { - const auto& olmo = static_cast(model); + const auto& olmo = ModelAs(model, "Olmo2ForCausalLM"); const Olmo2Weights& weights = olmo.weights(); if (input.gather_logits) { return Olmo2Model::ForwardDevice(input.token_ids, input.positions, diff --git a/src/vllm/model_executor/models/opt_registry.cpp b/src/vllm/model_executor/models/opt_registry.cpp index 0f16d09f4..ed6053ac0 100644 --- a/src/vllm/model_executor/models/opt_registry.cpp +++ b/src/vllm/model_executor/models/opt_registry.cpp @@ -93,7 +93,7 @@ void PrepareOPT(LoadedModel& model, const HfConfig& config, vt::Queue& queue) { } ForwardLogits ForwardOPT(LoadedModel& model, const ModelForwardInput& input) { - const auto& opt = static_cast(model); + const auto& opt = ModelAs(model, "OPTForCausalLM"); const OPTWeights& weights = opt.weights(); // DEVICE-resident logits (sampler-on-device) on the gather path; HOST logits // on the opt-out. OPT is pure full-attention (input.gdn_* unused). diff --git a/src/vllm/model_executor/models/phi3_registry.cpp b/src/vllm/model_executor/models/phi3_registry.cpp index 28447e808..55b484450 100644 --- a/src/vllm/model_executor/models/phi3_registry.cpp +++ b/src/vllm/model_executor/models/phi3_registry.cpp @@ -63,7 +63,7 @@ void PreparePhi3ForCausalLM(LoadedModel& model, const HfConfig& config, ForwardLogits ForwardPhi3ForCausalLM(LoadedModel& model, const ModelForwardInput& input) { - const auto& phi3 = static_cast(model); + const auto& phi3 = ModelAs(model, "Phi3ForCausalLM"); const Phi3Weights& weights = phi3.weights(); if (input.gather_logits) { return Phi3Model::ForwardDevice(input.token_ids, input.positions, diff --git a/src/vllm/model_executor/models/phi_registry.cpp b/src/vllm/model_executor/models/phi_registry.cpp index 2c6c2e29c..5d3b8cb81 100644 --- a/src/vllm/model_executor/models/phi_registry.cpp +++ b/src/vllm/model_executor/models/phi_registry.cpp @@ -68,7 +68,7 @@ void PreparePhiForCausalLM(LoadedModel& model, const HfConfig& config, ForwardLogits ForwardPhiForCausalLM(LoadedModel& model, const ModelForwardInput& input) { - const auto& pm = static_cast(model); + const auto& pm = ModelAs(model, "PhiForCausalLM"); const PhiWeights& weights = pm.weights(); if (input.gather_logits) { return PhiModel::ForwardDevice(input.token_ids, input.positions, input.attn_meta, diff --git a/src/vllm/model_executor/models/qwen3_5_dense.cpp b/src/vllm/model_executor/models/qwen3_5_dense.cpp index 4914e4733..07d9abcba 100644 --- a/src/vllm/model_executor/models/qwen3_5_dense.cpp +++ b/src/vllm/model_executor/models/qwen3_5_dense.cpp @@ -106,7 +106,7 @@ void PrepareQwen3_5Dense(LoadedModel& model, const HfConfig& config, // PERF-27B-LMHEAD-FP4 (issue #213): build the packed lm_head's resident HERE — // on CUDA before the runner captures a decode graph, elsewhere before the first // forward pays the dequant. Inert on every BF16/FP8/GGUF/tied checkpoint. - auto& qwen = static_cast(model); + auto& qwen = ModelAs(model, "Qwen3_5ForConditionalGeneration"); Qwen3_5DenseModel::PrepareLmHeadResident(qwen.weights(), queue); // PERF-27B-GDN-FP8-QKVZ: build the merged FP8 GDN [qkv;z] operand here, at // model prepare — before the first forward, so it can never allocate or copy @@ -117,7 +117,7 @@ void PrepareQwen3_5Dense(LoadedModel& model, const HfConfig& config, ForwardLogits ForwardQwen3_5Dense(LoadedModel& model, const ModelForwardInput& input) { - auto& qwen = static_cast(model); + auto& qwen = ModelAs(model, "Qwen3_5ForConditionalGeneration"); const Qwen3_5DenseWeights& weights = qwen.weights(); // ENG-ASYNC-SCHED W4: publish the async runner's device-resident input ids for diff --git a/src/vllm/model_executor/models/qwen3_5_moe.cpp b/src/vllm/model_executor/models/qwen3_5_moe.cpp index 5c87a464d..6ac92019a 100644 --- a/src/vllm/model_executor/models/qwen3_5_moe.cpp +++ b/src/vllm/model_executor/models/qwen3_5_moe.cpp @@ -88,13 +88,13 @@ std::unique_ptr LoadQwen3_5MoeModel( void PrepareQwen3_5Moe(LoadedModel& model, const HfConfig& config, vt::Queue& queue) { - auto& qwen = static_cast(model); + auto& qwen = ModelAs(model, "Qwen3_5MoeForConditionalGeneration"); Qwen3_5Model::PrepareMarlinResident(qwen.weights(), config, queue); } ForwardLogits ForwardQwen3_5Moe(LoadedModel& model, const ModelForwardInput& input) { - auto& qwen = static_cast(model); + auto& qwen = ModelAs(model, "Qwen3_5MoeForConditionalGeneration"); const Qwen3_5MoeWeights& weights = qwen.weights(); // ENG-ASYNC-SCHED W4 (see qwen3_5_dense.cpp): scope the async runner's diff --git a/src/vllm/model_executor/models/qwen3_dense.cpp b/src/vllm/model_executor/models/qwen3_dense.cpp index 74bbced94..2cce7e247 100644 --- a/src/vllm/model_executor/models/qwen3_dense.cpp +++ b/src/vllm/model_executor/models/qwen3_dense.cpp @@ -84,7 +84,7 @@ void PrepareQwen3ForCausalLM(LoadedModel& model, const HfConfig& config, ForwardLogits ForwardQwen3ForCausalLM(LoadedModel& model, const ModelForwardInput& input) { - auto& qwen = static_cast(model); + auto& qwen = ModelAs(model, "Qwen3ForCausalLM"); const Qwen3DenseWeights& weights = qwen.weights(); // ROW-SERVE-ASYNC-DENSE-MIRROR: publish the async runner's device-resident input // ids for the duration of THIS forward, so the shared EmbedInto (eager diff --git a/src/vllm/model_executor/models/qwen3_moe_registry.cpp b/src/vllm/model_executor/models/qwen3_moe_registry.cpp index 2e0731466..29f2435c2 100644 --- a/src/vllm/model_executor/models/qwen3_moe_registry.cpp +++ b/src/vllm/model_executor/models/qwen3_moe_registry.cpp @@ -84,7 +84,7 @@ void PrepareQwen3MoeForCausalLM(LoadedModel& model, const HfConfig& config, ForwardLogits ForwardQwen3MoeForCausalLM(LoadedModel& model, const ModelForwardInput& input) { - auto& qwen = static_cast(model); + auto& qwen = ModelAs(model, "Qwen3MoeForCausalLM"); const Qwen3MoeWeights& weights = qwen.weights(); // DECODE CUDA-GRAPH path (W7): route a PURE-DECODE CUDA step through the diff --git a/src/vllm/model_executor/models/qwen3_vl_registry.cpp b/src/vllm/model_executor/models/qwen3_vl_registry.cpp index 79e09c86f..4c419bcc6 100644 --- a/src/vllm/model_executor/models/qwen3_vl_registry.cpp +++ b/src/vllm/model_executor/models/qwen3_vl_registry.cpp @@ -102,12 +102,16 @@ void PrepareQwen3VLForConditionalGeneration(LoadedModel& model, const HfConfig& config, vt::Queue& queue) { // Warm the persistent cos|sin cache so the first forward step does not build it. - static_cast(model).CosSinCache(queue, config); + // The call stays INLINE on the checked reference rather than gaining a local: + // `ModelAs` establishes the dynamic type before the member call either way, so + // a binding would change this site's shape without changing what it does. + ModelAs(model, "Qwen3VLForConditionalGeneration") + .CosSinCache(queue, config); } ForwardLogits ForwardQwen3VLForConditionalGeneration( LoadedModel& model, const ModelForwardInput& input) { - auto& vl = static_cast(model); + auto& vl = ModelAs(model, "Qwen3VLForConditionalGeneration"); VT_CHECK(input.mm.has_value(), "Qwen3VLForConditionalGeneration registered forward requires " "multimodal inputs (ModelForwardInput.mm). Text-only Qwen3-VL through " diff --git a/src/vllm/model_executor/models/stablelm_registry.cpp b/src/vllm/model_executor/models/stablelm_registry.cpp index f26862d31..c04eaea8d 100644 --- a/src/vllm/model_executor/models/stablelm_registry.cpp +++ b/src/vllm/model_executor/models/stablelm_registry.cpp @@ -66,7 +66,7 @@ void PrepareStableLmForCausalLM(LoadedModel& model, const HfConfig& config, ForwardLogits ForwardStableLmForCausalLM(LoadedModel& model, const ModelForwardInput& input) { - const auto& sm = static_cast(model); + const auto& sm = ModelAs(model, "StableLmForCausalLM"); const StablelmWeights& weights = sm.weights(); if (input.gather_logits) { return StablelmModel::ForwardDevice(input.token_ids, input.positions, diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 5779c4dee..b498dc1c4 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -1994,3 +1994,14 @@ target_compile_definitions(test_qwen3_32b_nvfp4a16_paged_engine PRIVATE PARITY_GOLDENS_DIR="${CMAKE_SOURCE_DIR}/tests/parity/goldens") target_include_directories(test_qwen3_32b_nvfp4a16_paged_engine PRIVATE ${CMAKE_SOURCE_DIR}/tests/parity ${CMAKE_SOURCE_DIR}/src) + +# The registry type-confusion CLASS gate (#847, .agents/specs/registry-downcast-sweep.md). +# One case per SHAPE of the swept `static_cast(model)` site — const +# and non-const binding, `prepare` and `forward`, an alias family that shares one +# factory across two registered architectures, and the one inline-call site that a +# uniform search-and-replace over the other 33 would step past. Every case enters +# through the production seam (`reg.factory->forward` / `->prepare`, what +# `ModelRegistry::Forward` calls) and refuses before any weight is touched, so it is +# CPU-only and needs no checkpoint. +vllm_cpp_add_test(test_registry_downcast_refusal + vllm/models/test_registry_downcast_refusal.cpp) diff --git a/tests/vllm/models/test_registry_downcast_refusal.cpp b/tests/vllm/models/test_registry_downcast_refusal.cpp new file mode 100644 index 000000000..989929645 --- /dev/null +++ b/tests/vllm/models/test_registry_downcast_refusal.cpp @@ -0,0 +1,223 @@ +// The registry type-confusion CLASS gate — issue #847, spec +// `.agents/specs/registry-downcast-sweep.md`. +// +// Every registered `prepare`/`forward` is handed a type-erased `LoadedModel&` +// and has to open it as its own concrete model. Doing that with a bare +// `static_cast` down the hierarchy is a PROMISE the compiler is entitled to act +// on: on an object whose dynamic type is not that model, every member call +// through the resulting reference is undefined behaviour. UBSan's vptr check +// reports it as "member call on address ... which does not point to an object of +// type 'X'", and `-fno-sanitize-recover=all` aborts the process. #775 caught +// exactly one of these — the NemotronH forward — because a doctest stub happened +// to reach it. The other 34 were latent by that same luck. +// +// This suite is the class gate the sweep owes. It does NOT try to be one test +// per site; it is one test per SHAPE, because the sweep's failure modes are +// shape-shaped, not site-shaped: +// +// * a non-const `auto& x = static_cast(model)` forward (Qwen3 dense) +// * a `const auto& x = ...` forward, 14 of the 34 sites (StableLM) +// * an ALIAS FAMILY: two registered architectures sharing ONE factory, so +// the entry point has no single registered name of its own. The refusal +// must name the FAMILY PRIMARY (whose `load_weights` produces the model +// type) and, separately, the architecture the PASSED model's registration +// claims (Olmo3/Olmo2, InternLM3/Llama) +// * a `prepare` entry point rather than a forward (Qwen3.5 MoE) +// * the one site in the class that was NOT `(const )?auto& x = cast(model);` +// but an INLINE member call on the cast expression itself +// (`qwen3_vl_registry.cpp:105`), which is the site a mechanical +// search-and-replace over the 33 uniform ones would miss (Qwen3-VL) +// +// EVERY case here enters through the production seam — `reg.factory->forward` / +// `reg.factory->prepare`, which is exactly what `ModelRegistry::Forward` calls +// (`model_registry.cpp`). None of them constructs the concrete model by hand, +// because a test that does that proves the class works and never that anything +// reaches it. +// +// CPU-only, no checkpoint, no GPU: the refusal happens before any weight is +// touched, which is the entire point of moving the check to the top of the entry +// point. +#include + +#include +#include +#include + +#include "vllm/model_executor/models/model_registry.h" +// The type-erased seam only forward-declares the types a CALL through it needs. +// `nemotron_h_registry.cpp:28` and `test_nemotron_h_scaffold.cpp` reach for the +// same header for the same reason. +#include "vllm/model_executor/models/qwen3_5.h" // ForwardLogits, *KvCache +#include "vllm/transformers_utils/hf_config.h" +#include "vllm/v1/attention/backend.h" // CommonAttentionMetadata +#include "vllm/v1/attention/backends/gdn_attn.h" // GDNAttentionMetadata +#include "vllm/v1/kv_cache_interface.h" +#include "vt/device.h" + +using vllm::HfConfig; +using vllm::LoadedModel; +using vllm::ModelForwardInput; +using vllm::ModelRegistration; +using vllm::RegistrationFor; + +namespace { + +// A FOREIGN `LoadedModel`: complete, well-formed, and simply not any concrete +// model's type. This is the shape a caller produces by resolving one +// registration and handing `factory->forward` a model that some other path +// produced — which is what nothing in the type system prevents and what +// `ModelRegistry::Forward` cannot check on its callers' behalf. +// +// It carries a REAL registration (the entry point's own), so +// `model.registration().architecture` is right and only the OBJECT is wrong. +// That is deliberately the informative case rather than a strawman: a foreign +// object with a foreign registration would also be caught by a much weaker +// check that merely compared architecture strings, and such a check is +// explicitly rejected by the spec because it establishes what the registration +// CLAIMS rather than what the object IS. +class ForeignLoadedModel final : public LoadedModel { + public: + explicit ForeignLoadedModel(const ModelRegistration& registration) + : LoadedModel(registration) {} +}; + +// One reusable, entirely empty forward input. Every entry point under test +// refuses before it reads a single field of it, and proving that is half of +// what these cases assert: a refusal that needed a valid config or a live queue +// would not be a refusal at the top of the entry point. +struct EmptyForwardInput { + std::vector token_ids{0}; + std::vector positions{0}; + std::vector logits_indices{0}; + vllm::v1::CommonAttentionMetadata attn_meta{}; + vllm::v1::GDNAttentionMetadata gdn_meta{}; + std::vector attn_kv; + std::vector gdn_state; + HfConfig config{}; + vt::Queue queue{vt::Device{vt::DeviceType::kCPU, 0}, nullptr}; + + ModelForwardInput Get() { + return ModelForwardInput{.token_ids = token_ids, + .positions = positions, + .attn_meta = attn_meta, + .gdn_meta = gdn_meta, + .attn_kv = attn_kv, + .gdn_state = gdn_state, + .config = config, + .queue = queue, + .logits_indices = logits_indices, + .num_reqs = 1}; + } +}; + +// `registered` is the architecture whose registration the caller resolved. +// `refuser` is the architecture the entry point refuses UNDER — the family +// primary, which for a single-architecture registry is the same string and for +// an alias family is the architecture whose `load_weights` produces the model +// type the entry point opens. +void CheckForwardRefusesByName(const char* registered, const char* refuser) { + const ModelRegistration& reg = RegistrationFor(registered); + REQUIRE(reg.factory != nullptr); + REQUIRE(reg.factory->forward != nullptr); + ForeignLoadedModel foreign(reg); + EmptyForwardInput in; + const ModelForwardInput input = in.Get(); + + // Names the entry point that refused, so a mismatch reported from one of the + // 35 near-identical registry entry points can be told from another's. + CHECK_THROWS_WITH_AS(reg.factory->forward(foreign, input), + doctest::Contains(refuser), std::runtime_error); + // ...and names what actually went wrong, rather than blaming the weights the + // real object would have failed on next. + CHECK_THROWS_WITH_AS(reg.factory->forward(foreign, input), + doctest::Contains("was not produced by"), + std::runtime_error); + // ...and separately reports the architecture the PASSED model's registration + // claims. On an alias family these two strings DIFFER, which is the whole + // reason both are in the message. + CHECK_THROWS_WITH_AS(reg.factory->forward(foreign, input), + doctest::Contains(registered), std::runtime_error); +} + +void CheckPrepareRefusesByName(const char* registered, const char* refuser) { + const ModelRegistration& reg = RegistrationFor(registered); + REQUIRE(reg.factory != nullptr); + REQUIRE(reg.factory->prepare != nullptr); + ForeignLoadedModel foreign(reg); + const HfConfig config{}; + vt::Queue queue{vt::Device{vt::DeviceType::kCPU, 0}, nullptr}; + + CHECK_THROWS_WITH_AS(reg.factory->prepare(foreign, config, queue), + doctest::Contains(refuser), std::runtime_error); + CHECK_THROWS_WITH_AS(reg.factory->prepare(foreign, config, queue), + doctest::Contains("was not produced by"), + std::runtime_error); +} + +} // namespace + +// SHAPE 1 — the plain non-const forward, one registered architecture. +// `qwen3_dense.cpp:87`, `auto& qwen = static_cast(...)`. +TEST_CASE("registry downcast: a NON-CONST forward REFUSES a foreign model") { + CheckForwardRefusesByName("Qwen3ForCausalLM", "Qwen3ForCausalLM"); +} + +// SHAPE 2 — the `const auto&` forward. 14 of the 34 sites bind the cast to a +// `const auto&`. These are NOT the sites that select `ModelAs`'s const overload, +// which is worth stating because #847 assumed the opposite and called the sweep +// non-mechanical over it: `model` is a non-const `LoadedModel&` at every one of +// the 34, so overload resolution picks the NON-const `ModelAs` and its +// `Model&` result simply binds to the `const auto&`. The const overload has no +// caller in the tree at all (spec §4). What this case therefore proves is the +// thing that could actually break: that the returned reference is a real +// reference to the checked object rather than a copy or a dangling temporary, +// and that the site still refuses by name. `stablelm_registry.cpp:69`. +TEST_CASE("registry downcast: a CONST-bound forward REFUSES a foreign model") { + CheckForwardRefusesByName("StableLmForCausalLM", "StableLmForCausalLM"); +} + +// SHAPE 3 — an ALIAS FAMILY, const arm. `Olmo2ForCausalLM` and +// `Olmo3ForCausalLM` are two REGISTER_VLLM_MODEL lines against ONE +// `kOlmo2Factory`, so `ForwardOlmo2ForCausalLM` has no single registered name. +// The sweep's decision: refuse under the FAMILY PRIMARY. Asserting BOTH strings +// is what makes that decision executable — the message must name Olmo2 as the +// refuser and Olmo3 as what the passed model's registration claimed. +// `olmo2_registry.cpp:74`. +TEST_CASE("registry downcast: an ALIAS arch refuses under the FAMILY PRIMARY") { + CheckForwardRefusesByName("Olmo3ForCausalLM", "Olmo2ForCausalLM"); +} + +// SHAPE 3b — the same alias decision on a NON-const site, and on a family whose +// two architectures are not near-spellings of each other: `InternLM3ForCausalLM` +// is an alias-only registration against `kLlamaFactory`. +// `llama_registry.cpp:103`. +TEST_CASE("registry downcast: a non-const ALIAS forward names both archs") { + CheckForwardRefusesByName("InternLM3ForCausalLM", "LlamaForCausalLM"); +} + +// SHAPE 4 — a `prepare` entry point, and its `forward` sibling in the same TU. +// `prepare` runs once at load; the cast there is the same promise, and it is +// made BEFORE any weight is materialized, so nothing downstream would catch it. +// `qwen3_5_moe.cpp:91` (prepare) and `:97` (forward), reached through the +// text-only alias registration so the alias arm is covered on `prepare` too. +TEST_CASE("registry downcast: a PREPARE entry point REFUSES a foreign model") { + CheckPrepareRefusesByName("Qwen3_5MoeForCausalLM", + "Qwen3_5MoeForConditionalGeneration"); + CheckForwardRefusesByName("Qwen3_5MoeForCausalLM", + "Qwen3_5MoeForConditionalGeneration"); +} + +// SHAPE 5 — THE ODD SITE. Every other site in the class is +// `(const )?auto& x = static_cast(model);`. This one is an inline member +// call on the cast expression itself: +// +// static_cast(model).CosSinCache(queue, config); +// +// It is the site a uniform search-and-replace over the other 33 would step past, +// and it is also the site where the UB is most immediate: there is no +// intervening binding at all, the member call happens ON the cast expression. +// `qwen3_vl_registry.cpp:105`. +TEST_CASE("registry downcast: the INLINE-call prepare site REFUSES too") { + CheckPrepareRefusesByName("Qwen3VLForConditionalGeneration", + "Qwen3VLForConditionalGeneration"); +} From 90246150ab4fcc78d48fed2601c506f28edb27a7 Mon Sep 17 00:00:00 2001 From: Ettore Di Giacinto Date: Sat, 15 Aug 2026 12:25:59 +0000 Subject: [PATCH 2/2] docs(FIX-REGISTRY-DOWNCAST-SWEEP): correct the spec's own count of the swept sites MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two accuracy fixes to `.agents/specs/registry-downcast-sweep.md`, found re-deriving §3 from the survey rather than from the prose. The split was written as 29 `forward` and 5 `prepare`. Measured from the enclosing signatures it is 30 and 4, and the four are now named so the number is checkable instead of quotable: `gemma4_registry.cpp:104`, `qwen3_5_dense.cpp:109`, `qwen3_5_moe.cpp:91`, `qwen3_vl_registry.cpp:105`. A spec that miscounts its own class is the kind of record the next reader trusts and re-derives from, which is exactly what this one asks people not to do. And §3 now records that one swept site is reached only conditionally: `gemma4_registry.cpp:104` sits behind an early return unless `VT_GEMMA4_RESIDENT_EXPERTS=1`. It is swept anyway, because the cast is the defect and not the state it casts into, and the note explains why the class gate takes its `prepare` shape from `qwen3_5_moe.cpp:91` instead: that site runs with no environment set. A reviewer checking reachability would find the early return and deserve to see it already accounted for. No code change. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: Claude:claude-opus-5 [Claude Code] --- .agents/specs/registry-downcast-sweep.md | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/.agents/specs/registry-downcast-sweep.md b/.agents/specs/registry-downcast-sweep.md index c3b93fd63..f1c1ae888 100644 --- a/.agents/specs/registry-downcast-sweep.md +++ b/.agents/specs/registry-downcast-sweep.md @@ -36,12 +36,20 @@ watching. The other 34 were latent by the same luck. `Prepare*`/`Forward*` whose first parameter is `LoadedModel& model`, and every site casts that same `model`. There is no `const`-reference spelling and no pointer spelling, so the grep is the whole class. 14 sites bind the result to a -`const auto&`, 20 to a plain `auto&`; 29 are `forward` entry points and 5 are -`prepare`. +`const auto&` and 20 to a plain `auto&`; 30 are `forward` entry points and 4 are +`prepare` (`gemma4_registry.cpp:104`, `qwen3_5_dense.cpp:109`, +`qwen3_5_moe.cpp:91`, `qwen3_vl_registry.cpp:105`). **None of the 34 turned out to be safe.** Each one is a real downcast of a type-erased handle, reached through a live `ModelFactory` function pointer. +One is reached conditionally and is still swept: `gemma4_registry.cpp:104` sits +behind an early return unless `VT_GEMMA4_RESIDENT_EXPERTS=1`, so it is off on the +default path. That makes it harder to reach, not safe — the cast is the defect, +not the state it casts into — and it is why the class gate uses +`qwen3_5_moe.cpp:91` for the `prepare` shape rather than this one, which would +have needed the environment variable set to run at all. + ### Three corrections to #847's description of the shape The issue's estimate was made before the sweep looked at the tree. It is wrong