expert-stream #1100: the teardown comment asserts a hook that does not exist, and the nesting refusal is pinned nowhere

[localai-bot]

A fresh review of #1100 (3da7b4ca2), which repairs #1091. The six functional repairs are correct and all 13 of the mutation claims reproduce independently. Four defects sit around them, and three are the same class the pull request was fixing.

1. The comment asserts a mechanism that does not exist. src/vllm/model_executor/models/qwen3_5_internal.h:421-424 says the final statistics line is reached "at process teardown: a static registered the first time streaming is requested, plus the store's own destructor, whichever runs first". There is no such static: grep -rn "atexit\|quick_exit" src/vllm/model_executor/models/qwen3_5.cpp returns nothing, and #1100 says in its own body that the hook was deliberately NOT built. The comment also drops the two qualifiers docs/USAGE.md carries correctly, claiming "exactly one line per process, even on a run with zero steps" without "a store must have been built" and "static destructors must run". This is verbatim the defect #1091 finding 5 reports about TowerUid — a comment promising more than the code — reintroduced one file away in the change that fixes it.

2. "Nothing lands dead" is claimed for four step guards and holds for one. Only Qwen3_5MTPModel::ForwardPaged has a production caller (src/vllm/v1/worker/gpu/runner.cpp:2183 -> src/vllm/v1/worker/gpu/spec_decode/mtp/speculator.cpp:107,262). The other three reach nothing outside tests/: Qwen3_5MTPModel::Forward (only test_mtp_speculator.cpp, through the ForwardLogitsHost parity convenience, which is itself test-only); Qwen3_5Model::ForwardDense, which qwen3_5.h:234 calls "the parity reference"; and Qwen3_5ReplayLayer, which qwen3_5.h:322 calls "per-layer parity replay". Per .agents/reachability.md a call site inside a test is not reach. The guards are correct where they sit — the defect is the claim, not the code.

3. The nesting refusal is asserted everywhere and pinned nowhere. qwen3_5.cpp:5517 states VT_CHECK(!Open(), ...) as a designed guarantee, and so do the spec and the pull request body. Deleting it leaves both focused binaries fully green (test_expert_stream_steps 6/6 rc 0, test_expert_stream_wiring 4/4 rc 0); it appears in none of the 13 mutations. It is unreachable through production code by construction, because every forward that takes expert slices is a complete forward that no other one contains. The constructor also carries no Qwen35ExpertStreamRequested() gate, so the refusal is armed on the default-on path and a forced nest reds every Qwen3.5 forward, not only the streamed ones.

4. The MSVC repair is incomplete. tests/vllm/model_executor/test_expert_stream_steps.cpp:82-86 calls ::setenv at namespace scope with no _WIN32 guard. setenv(3) is POSIX and MSVC's CRT has only _putenv_s. tests/CMakeLists.txt:1087 adds the target unconditionally and scripts/build-windows-release.ps1 configures VLLM_CPP_BUILD_TESTS=ON, so the file comment claiming the step-clock cases are "built everywhere" and 3da7b4ca2's claim that only the statistics questions need POSIX are both false. tests/vllm/model_executor/test_expert_stream_wiring.cpp:76-90 has the same shape. CI cannot see it because the Windows lanes fail earlier, in the product library, on the pre-existing #1068 — so it would go red the moment that lands. tests/support/test_env.h has held the portable shim since #603 and is what a new env-flipping test is supposed to use.

Row ENG-EXPERT-STREAM, spec expert-streaming.md.

[localai-bot]

Fixed and landed in 268da6b27 (PR #1100).

The qwen3_5_internal.h comment no longer describes an atexit static that was never built; it names ~Qwen35ExpertStream as the only production path to the line, states that no hook is registered, and carries both qualifiers the public docs carry. The claims audit that followed examined 51 assertions and found 12 wrong, seven of which this issue did not cite — including the same false reachability claim sitting in .agents/issue-index.md, which was only repairable because its PR was still open.

The remaining findings were repaired as recorded in the PR body: the docs/FEATURES.md cell traded (#1106) for "on a clean exit" to fit the 220-char limit, docs/USAGE.md's fourth shape is now keyed on its cause rather than on an observable that cannot discriminate it from a crash, and the reachability nuance about ForwardPaged needing a configured speculator was recorded on #1108 rather than expanding scope.