feat(vllm): OpenAI embeddings dimensions truncation (DIS-2093 vLLM side) - #9751
Conversation
dc4f5d2 to
d5a2053
Compare
f1a64d9 to
f532e6e
Compare
c12c8be to
295335e
Compare
f532e6e to
a0fa5a4
Compare
a0fa5a4 to
71fbe88
Compare
71fbe88 to
06f181e
Compare
1618ad7 to
ac3242b
Compare
8ad9834 to
3d974aa
Compare
ac3242b to
fb74245
Compare
3d974aa to
5be8483
Compare
fb74245 to
f4c8f4f
Compare
5be8483 to
894e159
Compare
f4c8f4f to
0edca9a
Compare
932f5e4 to
2a231ed
Compare
Adds OpenAI `/v1/embeddings` `dimensions` (Matryoshka truncation) support to the vLLM embedding worker. Mirrors PR #9722 which did the same for SGLang. In `EmbeddingWorkerHandler.generate`, the handler now reads `request.get("dimensions")` once before the per-input loop, validates it (must be a positive int), then per embedding asserts `dimensions <= len(embedding)` and slices to `embedding[:dimensions]`. Out-of-range values raise `ValueError` which the Rust frontend surfaces as HTTP 400. `encoding_format=base64` is not yet supported end-to-end because the Rust frontend's response type is `Vec<f32>` and rejects base64 strings; that requires owning the embedding response type in `lib/protocols/` and is tracked as a separate follow-up. Test: `embedding_agg` config in `tests/serve/test_vllm.py` gains a fourth payload — `dimensions=128` against Qwen3-Embedding-0.6B (hidden dim 1024). Built inline using `EmbeddingPayload(...)` directly because the `embedding_payload(..., extra_body=...)` helper added in PR #9722 isn't on this branch's base. Signed-off-by: Tzu-Ling <tzulingk@nvidia.com>
2a231ed to
79234a7
Compare
WalkthroughThis PR adds support for dedicated embedding worker mode in the Dynamo vLLM wrapper, enabling OpenAI-compatible embedding serving in aggregated disaggregation mode. It introduces CLI configuration with strict validation, a specialized handler for embedding requests, factory-level routing, and end-to-end launch and integration testing. ChangesEmbedding Worker Feature
🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches⚔️ Resolve merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
components/src/dynamo/vllm/handlers.py (1)
2887-2889: ⚡ Quick winMove
PoolingParamsto module scope.This function-local import violates the repo Python rule and adds import work on every embeddings request. If the lazy-load concern is real, the embedding handler should live in its own module instead of importing inside
generate().As per coding guidelines, "Keep all imports at module scope (flag any import inside functions/classes)."♻️ Proposed fix
+from vllm import PoolingParams from vllm.config import ModelConfig, VllmConfig from vllm.inputs import EmbedsPrompt, TextPrompt, TokensPrompt @@ - # Lazy import to avoid pulling PoolingParams into handlers.py at module - # load time for non-embedding workers. - from vllm import PoolingParams - model_name = request.get("model") or self.config.served_model_name or ""🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@components/src/dynamo/vllm/handlers.py` around lines 2887 - 2889, The local import of PoolingParams inside generate() should be moved to module scope to comply with repo rules and avoid per-request import overhead: remove the in-function "from vllm import PoolingParams" and add "from vllm import PoolingParams" at the top of this handlers.py module; if the original lazy-load rationale is still required, extract the embedding handler into a dedicated module (e.g., vllm_embedding_handler) and place the PoolingParams import at that module's top so generate() no longer contains inline imports.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@components/src/dynamo/vllm/handlers.py`:
- Around line 2887-2889: The local import of PoolingParams inside generate()
should be moved to module scope to comply with repo rules and avoid per-request
import overhead: remove the in-function "from vllm import PoolingParams" and add
"from vllm import PoolingParams" at the top of this handlers.py module; if the
original lazy-load rationale is still required, extract the embedding handler
into a dedicated module (e.g., vllm_embedding_handler) and place the
PoolingParams import at that module's top so generate() no longer contains
inline imports.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3e14961f-ffff-475d-bc9e-db07488f7023
📒 Files selected for processing (9)
components/src/dynamo/vllm/backend_args.pycomponents/src/dynamo/vllm/handlers.pycomponents/src/dynamo/vllm/tests/test_backend_args.pycomponents/src/dynamo/vllm/tests/test_vllm_unit.pycomponents/src/dynamo/vllm/tests/test_vllm_worker_factory.pycomponents/src/dynamo/vllm/tests/test_vllm_worker_handler.pycomponents/src/dynamo/vllm/worker_factory.pyexamples/backends/vllm/launch/agg_embed.shtests/serve/test_vllm.py
Signed-off-by: Tzu-Ling <tzulingk@nvidia.com>
…de) (ai-dynamo#9751) Signed-off-by: Tzu-Ling <tzulingk@nvidia.com>
Overview:
Adds OpenAI
/v1/embeddingsdimensions(Matryoshka truncation) support to the vLLM embedding worker. Mirror of PR #9722 which did the same for SGLang.Scope: Only changes the vLLM
EmbeddingWorkerHandlerfrom #9713. Once #9713 and #9722 both land, this PR closes out the vLLM half of DIS-2093 (thedimensionshalf).Details:
In
EmbeddingWorkerHandler.generate, the handler now readsrequest.get("dimensions")once before the per-input loop, validates it (must be a positive int), then per embedding assertsdimensions <= len(embedding)and slices toembedding[:dimensions]. Out-of-range values raiseValueErrorwhich the Rust frontend surfaces as HTTP 400.Test:
embedding_aggconfig intests/serve/test_vllm.pygains a fourth payload —dimensions=128against Qwen3-Embedding-0.6B (hidden dim 1024). Built inline usingEmbeddingPayload(...)directly because theembedding_payload(..., extra_body=...)helper from PR #9722 isn't on this branch's base.Not included —
encoding_format=base64Same as the SGLang half of DIS-2093: the Rust frontend's response type is
Vec<f32>(inherited from the upstreamasync_openai::types::embeddings::*re-export) and rejects base64 strings. End-to-end support requires owning the embedding response type inlib/protocols/. Tracked in DIS-2099.Stacking
Based on
feat/vllm-embedding-worker-mvp(PR #9713). Will rebase tomainonce #9713 lands.Reviewer focus
components/src/dynamo/vllm/handlers.py::EmbeddingWorkerHandler.generate— the new validation + slicing.tests/serve/test_vllm.py— one newdimensions=128payload.Refs DIS-2093.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
--embedding-workerconfiguration flag/v1/embeddingsendpoint supporting text, token, and batched input formatsTests