fix(BUILD-MAIN-TIP-TESTFIX): main tip builds again — DSA test follows W9, glm5_next_weights stops shadowing (#2372) - #2399
Closed
lu-zero wants to merge 4 commits into
Closed
Conversation
lu-zero
force-pushed
the
row/BUILD-MAIN-TIP-TESTFIX
branch
from
August 31, 2026 12:57
9215260 to
baf0975
Compare
Scope: the two build breaks mudler#2372 attributes to main's tip - the DSA schedule test call left at 11 arguments when W9 (11f34ef) inserted attn_pre_o_proj before shared, and the glm5_next_weights KdaHeadCount else-if chain whose second and third `const GgufValue* v` shadow the first under MSVC C4456 (escalated by C2220; gcc/clang stay silent, so Linux CI never saw it). Between them they red six CI job names on every open PR. Design: the test call mirrors the production call exactly (glm_moe_dsa_forward.cpp:455-460 passes /*attn_pre_o_proj=*/nullptr, shared); the loader fix renames only the two shadowing declarations and their immediate uses. No behavior change in either facet. Evidence plan: red-first build of test_glm_moe_dsa_schedule on this worktree before the fix; the MSVC arm cannot red locally (no MSVC on this host) and its red is the CI log, with CI on this PR's head as the green authority - both named in the spec as the unavoidable adaptation. Mutation plan: the 11-arg revert re-reds the build, and gcc -Wshadow flags a reintroduced shadow, giving the MSVC-only warning a local detection mechanism. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:zai-glm-5.3-flash [maki]
…-Werror (mudler#2372) KdaHeadCount's else-if chain declares `const GgufValue* v` in each condition. An if-condition declaration is visible in the else branch, so the declarations at lines 223 and 226 hide the one at 220. MSVC /W4 reports C4456 twice and the Windows -Werror regime escalates to C2220, reding both windows-msvc jobs. gcc and clang do not warn by default, which is why every Linux build stayed green while Windows broke. The fix renames only the two shadowing declarations and their immediate uses (v_group, v_inner); the first branch's v and every other line of the function are untouched, and both KvInt calls keep their exact arguments, so no behavior changes. Detection proof: the project builds with -Werror but without -Wshadow, which is exactly how this stayed invisible on Linux. Appending -Wshadow to the TU's compile command flags 223:31 and 226:31 on the pre-fix form and nothing there on the fixed form, so the MSVC-only warning now has a local detector. The MSVC arm's green evidence remains CI on this PR's head, per the spec's named adaptation. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:zai-glm-5.3-flash [maki]
…udler#2372) The fresh reviewer's mutation pass placed the two renamed uses at 225 and 228, not the 224 and 227 the design section claimed, and the shadowed declaration at 220 rather than 221. Numbers only; the design and the fixes are unchanged. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:zai-glm-5.3-flash [maki]
…record the split (mudler#2372) The schedule-test repair landed on main from an independent find while this row was in review: 08fa2f5 (mudler#2395) applies the same twelve-argument call, found by building main. The rebased branch drops its duplicate as already applied and now carries only the KdaHeadCount rename; it still closes mudler#2372, which names both breaks. The spec gains the same note, so the record and the branch agree. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:zai-glm-5.3-flash [maki]
lu-zero
force-pushed
the
row/BUILD-MAIN-TIP-TESTFIX
branch
from
August 31, 2026 15:54
baf0975 to
ff25006
Compare
mudler
added a commit
that referenced
this pull request
Sep 1, 2026
…o the model (#2398) Merges `row/ENG-MM-INPUT-PIPELINE` (43ed5cb, PR #2398). The runner now builds device-resident multimodal buffers and hands them to the registered forward through `ModelForwardInput.mm`, so a vision tower is reachable from the server's default configuration rather than only from a test that constructs it by hand. CORRECTION TO A COMMIT BODY THIS MERGE CARRIES. Commit 9775230's body explains the new preemption test by saying it sizes the hog's decode chunk to exhaust the token budget "so the waiting loop is never entered". A fresh review refuted that. The waiting loop is not gated by the token budget at all: `Scheduler::schedule` wraps the waiting section in `if (preempted_reqs.empty())` (src/vllm/v1/core/sched/scheduler.cpp:849), which sits before `while (!waiting->empty() && token_budget > 0)`, so a step that preempted anything can never reach that loop whatever the budget. Upstream mirrors this at the pin 5559679229, vllm/v1/core/sched/scheduler.py:668. The reviewer proved it by rerunning the mutation with half the budget unspent; the case still went red. The test is therefore MORE robust than its own commit body claimed, not less. The in-tree comment was corrected by 43ed5cb; this paragraph corrects the history that 9775230 already wrote, because a merge cannot rewrite it. Landing with five red CI lanes, every one of them attributed to a defect that predates this branch and reproduces on `main` without it: `build-test-cpu` fails only `test_gguf_device_fit_reach` (#2450, confirmed by building the same binary at origin/main with this branch absent); both `sanitize-cpu` arms fail on #2430 (an out-of-bounds write in `dense_attn::MakeTensor`, which indexes a rank-4 array by an unchecked incoming rank) and #2431 (a 384-byte leak); both `windows-msvc` arms fail on the C4456 shadowing in glm5_next_weights.cpp that #2399 owns. `build-newest-gcc` passed, and `build-test-cpu` compiled clean before reaching ctest, which is what independently confirms the plugin compile break this branch introduced and then fixed is gone. Verified before merging: whole test tree built against this exact merge, nine suites green (test_scheduler 47/513, test_plugin_system 1/29, test_openai_api_server_mm_forward 4/30, test_runner 36/1851, test_model_registry 24/993, test_chat_mm 11/126, test_openai_serving 48/1365, test_openai_api_server 79/1212, test_glm_moe_dsa_schedule 12/533). FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [claude-code]
mudler
added a commit
that referenced
this pull request
Sep 1, 2026
…n architecture (#2481) Merges `row/ENG-MM-INPUT-PIPELINE-mm-processor-registry` (8f4f19e, PR #2481). Closes #2475. The server used to install its multimodal seam by the FILE EXISTENCE of `preprocessor_config.json` and then hardcode Qwen3-VL's processor and chat function, with a swallowed catch falling back silently to the text path. That is not merely absent for a second multimodal model: `dots3-note` ships a `preprocessor_config.json` too, with a different `vision_config` shape, so it would have been handed Qwen3-VL's processor, or answered image requests as text-only with no diagnostic. The silent arm is the worse one, because a multimodal server that answers as text-only looks like a working server. It now dispatches on the model architecture through a self-registering registry, mirroring vLLM: `register_processor` keys on the model class (vllm/multimodal/registry.py:142-172 at pin 5559679229), `_get_model_cls` raises by name for an unregistered architecture (:176-186), and Qwen3-VL registers in its own translation unit (qwen3_vl.py:1673-1678) rather than editing a shared table. The registry lives in the entrypoints layer because the registered artefact needs `ChatPromptRenderFn`, `ImageCodecFn` and `ChatMessage`; putting it on `ModelFactory` would make `model_executor` depend on `entrypoints`, and upstream splits it the same way. An unregistered architecture now refuses by name, and a factory that produces no callable refuses too rather than reporting success -- that second arm was found by the fresh review, not by the author, and without it a registered-but-empty seam logged "seam wired" while the image request went to the text path. WHAT IS STILL OWED, stated because a green suite hides it. Half of #2408 item 5 is closed and half is not. The closed half: the test harness used to RE-IMPLEMENT the install, which is why deleting the production line left the suite green; it now calls the production `InstallMultiModalChatSeam`, and both mutations go red through it. The open half: the production call site in `main` is still ungated, and the residual is wider than one line -- it is the whole context assembly plus both new `LoadedEngine` accessors. Two mutations prove it, each verified to have reached the binary by sha256: `mm_ctx.architecture = ""` and `InstallMultiModalChatSeam(chat, false, ...)` both leave the suite green, and the second reintroduces exactly the defect this change removes. Closing it was attempted and measured, not skipped: a loadable checkpoint does carry the server past the install and out through a controlled exit, so neither reach nor exit is the blocker. The blocker is that the only loadable fixture is text-only, where the mutated and real calls are observationally identical. What is missing is a multimodal-declaring checkpoint with a tokenizer and weights the loader accepts -- a fixture with its own generator, not a test. Verified before merging: whole test tree built against this exact merge with zero FAILED targets, and eight suites green -- test_openai_api_server_mm_forward 9/73 (the ninth case is the empty-seam refusal), test_chat_mm 11/126, test_openai_serving 48/1365, test_serve_mm_limits 11/109, test_openai_api_server 79/1216, test_tower_skip 7/94, test_capi 66/668, test_model_registry 24/993. Landing with inherited red CI lanes, none caused by this branch and each reproducing on `main` without it: `windows-msvc` twice on the glm5_next_weights C4456 shadowing that #2399 owns, `build-test-cpu` on `test_gguf_device_fit_reach` (#2450), and both `sanitize-cpu` arms on #2430 and #2431. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [claude-code]
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fix(BUILD-MAIN-TIP-TESTFIX): glm5_next_weights stops shadowing v under MSVC -Werror (#2372)
Closes #2372
What changed: the loader facet of main-tip's build break. The other facet the
issue named, the schedule test's 11-argument call, landed on main from an
independent find while this row was in review —
08fa2f5aa(#2395) appliesthe same twelve-argument repair — so the rebased branch drops its duplicate
as already applied and carries one fix.
src/vllm/model_executor/models/glm5_next_weights.cpp—KdaHeadCount'selse-if chain declared
const GgufValue* vthree times. An if-conditiondeclaration is visible in the else branch, so the declarations at 223 and
226 shadow the one at 220; MSVC /W4 reports C4456 twice and the Windows
-Werror regime escalates to C2220, reding both
windows-msvc-*jobs.gcc/clang do not warn by default, which is why every Linux build stayed
green. The fix renames only the two shadowing declarations and their
immediate uses (
v_group,v_inner); no behavior change.Evidence: red captured locally before the fix — the loader compile under
the shadow form is what MSVC reded on; after the fix,
gcc -Wshadowon theTU flags exactly 223:31/226:31 on the pre-fix form and nothing there after —
the project builds with -Werror but without -Wshadow, which is how this
stayed invisible on Linux. CI on this PR's previous head already proved the
gcc facet of the issue green:
build-test-cpu,build-newest-gcc, and bothsanitize-cpujobs, including the schedule test that the landing of08fa2f5aanow keeps green on main. On the Windows arm, this PR's CI roundshows the compile error gone — no C4456/C2220 anywhere in either
windows-msvc log; the residual red there is a new, distinct defect:
test_openai_api_server.exeaborts at runtime (0xC0000409) in 3 of 79cases, filed as #2403 with the same first-execution exposure as this break
(those binaries could not link until the compile error cleared). Fresh
reviewer verdict: PASS on all six guarantees with each mutation restored
byte-for-byte; its two prose findings (line numbers in the spec and in the
loader commit body) are repaired in-tree.
Remaining red after this lands, out of scope:
windows-msvc-*(#2403,owned by the api_server row; not caused by this change) and
documentation-checkpoint(#2375, main-only, owned by the gate).FOLLOWING_AGENTS_PROTOCOL
Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: AGENT:zai-glm-5.3-flash [maki]