You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
residency config: absent-means-unchanged is implemented as both 'a change' and 'a clear', so a legal second load throws and a partial install drops the first engine's fields #1133
A fresh scoped review of PR #1119 at c7fa7084b (repairing #1122) returned FAIL with two behaviour defects reachable from a production entry point, plus a false ABI guarantee. Row ENG-RESIDENCY-CONFIG, spec .agents/specs/weight-residency-config.md, issues #1110, #1122.
The root cause, common to H1 and H2
The schema says an absent field means unchanged (include/vllm/config/weight_residency.h:28). The code implements absent as two different things, neither of which is "unchanged":
FrozenFields treats absent as a change — it compares in.expert_stream != g.config.expert_stream, and nullopt != engaged is true.
The install treats absent as a clear — g.config = config at weight_residency.cpp:396 replaces wholesale.
Both are the same misuse of optional, and each produces a distinct user-visible failure.
H1 — the narrowed refusal still fires on a legal second load, and the message denies it
weight_residency.cpp:343-395. Driven through LoadedEngine::FromModelDir, the entry point the server flag and the C ABI both fill in:
A: {"vllm_cpp":{"mmap":{...},"expert_stream":{"enabled":true,"slots":8000}}} -> installed
A takes the decision -> streaming=1
B: {"vllm_cpp":{"mmap":{"enabled":true}}} -> std::logic_error: expert_stream cannot be changed
B: {"vllm_cpp":{"prefault":...}} -> std::logic_error: (same)
B: A's document again -> installed (control)
B: no document -> installed (control)
So #1122's M1 was narrowed, not removed — the same hard failure on a legal two-model process, now reached by a document that merely omits the latched field. Worse, a document asking for exactly what the process already resolved is refused, while the message says "accepting this would record a configuration the engine is not running" — the engine is running it.
Falsified by measurement, all new prose in that delta: weight_residency.h:58-61, :63-66, :181-184, the commit body, the PR body, and spec :64. Both new tests dodge the shape they claim to pin: test_weight_residency_config.cpp:537 builds mmap_too = cfg and :585 builds mmap_only = sizes — each copies the previous config instead of constructing a genuinely partial document, so the failing case cannot arise.
H2 — a partial second install silently drops the first engine's fields
Engine A's expert_stream=on and its 8000-slot reservation vanish with no diagnostic. The slot store is built lazily "on the first slice taken, which can be long after a second engine loaded" — the code's own words at :369-374 — and then reads 64 and OFF. This is precisely the invisible-fallback shape this row's prose says the tree refuses.
The wholesale replace predates the delta, but the delta widened its reach: before the narrowing, any differing config after a latch threw, so the drop could not happen post-latch.
H3 — include/vllm.h promises a refusal the engine does not perform
include/vllm.h:455-462 claims refusal of "an UNKNOWN key anywhere in the document". Measured:
The enumeration closes the top level and the inside of vllm_cpp, not the inside of uva/prefetch. So --offload-config '{"uva":{"cpu_offload_GB":10}}' starts a server with a budget the operator believes is set — H1's failure mode one level over, with a public ABI comment saying it cannot happen. Same over-claim at docs/USAGE.md:3441, :3447, spec :75, :556; the test refused[] list asserts {"uvaa":...} but never {"uva":{"cpu_offload_gbb":...}}, so coverage looks complete.
The repair's own rationale argues against the gap: upstream UVAOffloadConfig/PrefetchOffloadConfig carry @config -> extra="forbid", so a nested typo is refused upstream too. Enumerating them is mirror-faithful.
Medium and low
M1 spec :350 still says the install line "prints the RESOLVED values" — the one M3 sentence that survived, three lines above a bullet the same commit rewrote.
M2qwen3_5.cpp:5161-5164 states the pre-narrowing contract ("REFUSES a config that arrives after a decision was taken"); an equal re-install is now accepted. The 46-sentence audit never reached this file.
L1weight_residency.h:50, :195 attribute the function-local static to Qwen35ExpertStreamRequested; it is now a pure delegation and the static lives in ResolveExpertStreamRequested (weight_residency.cpp:513). Repeated in the commit body, PR body and spec.
L2weight_residency.h:314 says "refuses a late install"; it refuses a late change.
L3 the commit body states ctest 505/505; measured 506/506 (the PR body is right).
L4test_weight_residency_reach.cpp:176 says its weight load reads the GGUF policy "exactly as every GGUF load in the tree does"; load A throws on the missing checkpoint first, which is why the case calls FromEnv() itself two lines later. The case is sensitive; the framing overstates it.
L5model_loader.cpp:1288-1293's "every knob resolves lazily" is over-general — mmap/prefault/slots could resolve at install now that shared resolvers latch nothing.
L6include/vllm.h:451 says "one line"; it prints one plus a second conditional line.
L7weight_residency.h:28-29 claims an absent vllm_cpp key is "byte-identical to the engine before this row existed"; --offload-config '{"typo":1}' used to start and now aborts.
The mirror argument holds: no --offload-config string anywhere in vLLM at the pin (git grep rc 1), upstream builds OffloadConfig from individual flags, and @config sets extra="forbid" at utils.py:68-69. parse_offload_config_json is byte-faithful and its 4-key list is exact. The relaxed atomic is honest — two monotonic flags, nothing published through them, and the recorded window is real and unclosable by ordering. The refusal to relocate the misplaced index rows is correct: a relocation is delete+add, which the append-only rule forbids and which merges into a duplicate under merge=union. No force-push; merge-tree of the parents equals the merge's tree exactly, so nothing was evil-merged.
Gate on the reviewed head reproduces exactly: 1485/1485 build, 0 warnings, 506/506 rc 0, all four focused counts matching.
The test shape that would have caught both
No case exercises two different partial documents in one process — every existing case either re-installs a copy of the first or installs the empty config. That is the hole both defects hid in, and it is worth a case shape rather than another sentence.
Claims audit by the reviewer: 55 examined, 21 wrong, collapsing to 9 distinct defects, 2 of them behaviour.
A fresh scoped review of PR #1119 at
c7fa7084b(repairing #1122) returned FAIL with two behaviour defects reachable from a production entry point, plus a false ABI guarantee. RowENG-RESIDENCY-CONFIG, spec.agents/specs/weight-residency-config.md, issues #1110, #1122.The root cause, common to H1 and H2
The schema says an absent field means unchanged (
include/vllm/config/weight_residency.h:28). The code implements absent as two different things, neither of which is "unchanged":FrozenFieldstreats absent as a change — it comparesin.expert_stream != g.config.expert_stream, andnullopt != engagedis true.g.config = configatweight_residency.cpp:396replaces wholesale.Both are the same misuse of
optional, and each produces a distinct user-visible failure.H1 — the narrowed refusal still fires on a legal second load, and the message denies it
weight_residency.cpp:343-395. Driven throughLoadedEngine::FromModelDir, the entry point the server flag and the C ABI both fill in:So #1122's M1 was narrowed, not removed — the same hard failure on a legal two-model process, now reached by a document that merely omits the latched field. Worse, a document asking for exactly what the process already resolved is refused, while the message says "accepting this would record a configuration the engine is not running" — the engine is running it.
Falsified by measurement, all new prose in that delta:
weight_residency.h:58-61,:63-66,:181-184, the commit body, the PR body, and spec:64. Both new tests dodge the shape they claim to pin:test_weight_residency_config.cpp:537buildsmmap_too = cfgand:585buildsmmap_only = sizes— each copies the previous config instead of constructing a genuinely partial document, so the failing case cannot arise.H2 — a partial second install silently drops the first engine's fields
weight_residency.cpp:396:Engine A's
expert_stream=onand its 8000-slot reservation vanish with no diagnostic. The slot store is built lazily "on the first slice taken, which can be long after a second engine loaded" — the code's own words at:369-374— and then reads 64 and OFF. This is precisely the invisible-fallback shape this row's prose says the tree refuses.The wholesale replace predates the delta, but the delta widened its reach: before the narrowing, any differing config after a latch threw, so the drop could not happen post-latch.
H3 —
include/vllm.hpromises a refusal the engine does not performinclude/vllm.h:455-462claims refusal of "an UNKNOWN key anywhere in the document". Measured:The enumeration closes the top level and the inside of
vllm_cpp, not the inside ofuva/prefetch. So--offload-config '{"uva":{"cpu_offload_GB":10}}'starts a server with a budget the operator believes is set — H1's failure mode one level over, with a public ABI comment saying it cannot happen. Same over-claim atdocs/USAGE.md:3441,:3447, spec:75,:556; the testrefused[]list asserts{"uvaa":...}but never{"uva":{"cpu_offload_gbb":...}}, so coverage looks complete.The repair's own rationale argues against the gap: upstream
UVAOffloadConfig/PrefetchOffloadConfigcarry@config->extra="forbid", so a nested typo is refused upstream too. Enumerating them is mirror-faithful.Medium and low
:350still says the install line "prints the RESOLVED values" — the one M3 sentence that survived, three lines above a bullet the same commit rewrote.qwen3_5.cpp:5161-5164states the pre-narrowing contract ("REFUSES a config that arrives after a decision was taken"); an equal re-install is now accepted. The 46-sentence audit never reached this file.weight_residency.h:50,:195attribute the function-local static toQwen35ExpertStreamRequested; it is now a pure delegation and the static lives inResolveExpertStreamRequested(weight_residency.cpp:513). Repeated in the commit body, PR body and spec.weight_residency.h:314says "refuses a late install"; it refuses a late change.ctest505/505; measured 506/506 (the PR body is right).test_weight_residency_reach.cpp:176says its weight load reads the GGUF policy "exactly as every GGUF load in the tree does"; load A throws on the missing checkpoint first, which is why the case callsFromEnv()itself two lines later. The case is sensitive; the framing overstates it.model_loader.cpp:1288-1293's "every knob resolves lazily" is over-general —mmap/prefault/slotscould resolve at install now that shared resolvers latch nothing.include/vllm.h:451says "one line"; it prints one plus a second conditional line.weight_residency.h:28-29claims an absentvllm_cppkey is "byte-identical to the engine before this row existed";--offload-config '{"typo":1}'used to start and now aborts.## Owedentry for the unreached entry points names residency config #1119: a hyphen typo silently disables the tier, and nothing gates the expert-stream knob reaching its decision #1122 as owner — the very issue this PR closes, so on landing the gap has no open issue.Verified sound, for the record
The mirror argument holds: no
--offload-configstring anywhere in vLLM at the pin (git greprc 1), upstream buildsOffloadConfigfrom individual flags, and@configsetsextra="forbid"atutils.py:68-69.parse_offload_config_jsonis byte-faithful and its 4-key list is exact. The relaxed atomic is honest — two monotonic flags, nothing published through them, and the recorded window is real and unclosable by ordering. The refusal to relocate the misplaced index rows is correct: a relocation is delete+add, which the append-only rule forbids and which merges into a duplicate undermerge=union. No force-push;merge-treeof the parents equals the merge's tree exactly, so nothing was evil-merged.Gate on the reviewed head reproduces exactly: 1485/1485 build, 0 warnings, 506/506 rc 0, all four focused counts matching.
The test shape that would have caught both
No case exercises two different partial documents in one process — every existing case either re-installs a copy of the first or installs the empty config. That is the hole both defects hid in, and it is worth a case shape rather than another sentence.
Claims audit by the reviewer: 55 examined, 21 wrong, collapsing to 9 distinct defects, 2 of them behaviour.
Owning row:
ENG-RESIDENCY-CONFIG.