Repository navigation
feat(multimodal): let vLLM workers expand Kimi-K3 and GLM-5.3-Flash image anchors - #2623
Conversation
…mage anchors Both specs left worker_expandable at its false default, so a router in auto mode kept preprocessing images itself and counted every request as model_not_opted_in even when the engine advertised mm_processor. Kimi-K3's <|media_pad|> and GLM-5.3-Flash's <|image|> are the single tokens vLLM's prompt updates replace, so a worker can expand them. GLM video stays router-side: vLLM targets a three-token block for it and the spec's video anchor id is the image id. Signed-off-by: Alex McC <319643551+hello-alexmcc@users.noreply.github.com>
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (3)
Included review availability: 9 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 10 reviews per hour. 📝 SummarySummary by CodeRabbit
WalkthroughChangesWorker Anchor Parity
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Feature Merge Risk: ⚪ Minimal · up to The PR adds image-anchor support for both models while preserving the intended router-side video behavior; no merge-blocking risk remains. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Reviewed the full diff. No issues found — 0 🔴 Important, 0 🟡 Nit, 0 🟣 Pre-existing.
- Both
worker_expandableoverrides follow the established opt-in pattern (qwen3_vl, llama4, minimax_m3 in #2609) and are covered by unit tests asserting the exact modality set. - The parity-script anchor rules mirror the Rust specs' derivation (
media_placeholder_token_idfor kimi_k3 viaplaceholder_token_id,image_token_idfor glm53_flash viaplaceholder_token_id_for(Image)). - Checked the mixed-modality path:
ensure_worker_expandablerequires every modality in the plan to be expandable, so a GLM image+video request falls back to router-side processing as a whole — leaving video opted out is safe, and is the right call givenplaceholder_token_id_for(Video)returns the image id. - No e2e lane hardcodes
worker_expandableexpectations for these two models, so nothing flips.
The one open risk — whether vLLM really expands the lone anchors — is clearly disclosed in the Test Plan with a decision rule (drop the opt-in if the parity script fails), so nothing to flag there beyond a reminder to run check_worker_anchor_parity.py for both models before merging.
Description
Problem
Kimi-K3 and GLM-5.3-Flash engines that advertise worker-side media processing (
mm_processor=inprocess/mm_processor=redis) never receive media from the router. It keeps preprocessing on its own side and counts every image request asIn strict mode (
SMG_MM_PROCESSING=worker) both models fail every image request withmultimodal_worker_processing_unsupported_model.ModelProcessorSpec::worker_expandabledefaults tofalseand neitherKimiK3VisionSpecnorGlm53FlashSpecoverrides it, soensure_worker_expandablein the gRPC router's media planning classifies both as not opted in and falls back to router-side processing.Solution
Opt both specs in for images, the same way MiniMax-M3 was opted in (#2609):
<|media_pad|>(configmedia_placeholder_token_id). vLLM'skimi_k3prompt updates build the whole<|media_begin|>image {w}x{h}<|media_content|>…<|media_end|>block from that single token, so a worker can expand it itself.<|image|>(configimage_token_id), the single token vLLM's GLM processors replace with the image's placeholder run.GLM-5.3-Flash video is deliberately not opted in, for two independent reasons: vLLM's GLM family targets the three-token
<|begin_of_video|><|video|><|end_of_video|>block for video, not a lone token, so a single anchor would never be replaced; andplaceholder_token_id_for(Video)in this spec returns the image id (the router builds the<|begin_of_image|>…frame blocks itself), so even the anchor id would be wrong for worker expansion. Enabling video here needs a change to how the router emits video anchors and is out of scope. Kimi-K3 declares images only.Changes
crates/multimodal/src/registry/kimi_k3.rs:worker_expandablereturns true forModality::Image, plus a unit test.crates/multimodal/src/registry/glm53_flash.rs: same, image only, plus a unit test.crates/multimodal/scripts/check_worker_anchor_parity.py:kimi_k3andglm53_flashentries (config_idanchors onmedia_placeholder_token_id/image_token_id; no video entry for GLM).Test Plan
cargo test -p llm-multimodal kimi_k3(21 passed) andcargo test -p llm-multimodal glm53_flash(14 passed): the new testsimage_anchor_can_be_expanded_by_the_workerandonly_the_image_anchor_can_be_expanded_by_the_workerfail onmainand pass here.cargo test -p llm-multimodal: 465 passed, 0 failed.cargo test --workspace: 136 suites, 6000 passed, 0 failed, 43 ignored.cargo clippy --workspace --all-targets -- -D warningsclean (run without--all-features: this machine has no OpenCV for theopencv-videofeature);cargo +nightly fmt --allclean; pre-commit hooks (codespell, whitespace, ruff, ruff-format) pass.python crates/multimodal/scripts/check_worker_anchor_parity.py --model kimi_k3 --trust-remote-codeand--model glm53_flash --trust-remote-code: for each model the lone anchor becomes exactly one placeholder range and the surrounding tokens are unchanged. If it reports that vLLM does not expand a lone anchor for either model, that model's opt-in should be dropped rather than forced.image_urlrequest per model reportingsmg_mm_processing_total{model=…,mode="worker",reason="auto_uniform"}with the sameusage.prompt_tokensas router mode.Checklist
cargo +nightly fmtpassescargo clippy --all-targets -- -D warningspasses (without--all-features, see above)