[Router][Bindings] Settle the image embedding over-dimension contract on /api/v1/embeddings#2639
Open
WUKUNTAI-0211 wants to merge 8 commits into
Conversation
…ve dim over FFI encode_image_with_dim silently returned the native 384-dim vector for any target_dim >= native, unlike the text 2DMSE path which rejects target_dim > embedding_dim. Mirror that contract so an above-native image request errors instead of quietly ignoring the requested dimension. Add multimodal_get_embedding_dim() so the router can enforce the contract server-side against the model's real native dimension rather than a hardcoded constant (embedding_dim is overridable via the model config). Signed-off-by: WUKUNTAI-0211 <83541875+WUKUNTAI-0211@users.noreply.github.com>
Wire the multimodal native-dimension FFI getter into the Go binding (and the non-cgo mock), returning the loaded model's native embedding dimension or -1 when no multimodal model is loaded. Signed-off-by: WUKUNTAI-0211 <83541875+WUKUNTAI-0211@users.noreply.github.com>
…1/embeddings The image path defaulted dimension to 768 (the text default) while the multimodal model's native dimension is 384, so a default mixed text+image request returned some 768-dim and some 384-dim vectors in one 200 response, and 384 was the one value isValidDimension rejected. Give the image path an explicit contract: - accept 384 (the multimodal native dimension) in isValidDimension; - default image-bearing requests to the native dimension (via the FFI getter), so plain image and mixed requests return uniform vectors instead of failing or silently mixing dimensions; - reject an explicit above-native dimension for image inputs (4xx), matching the text 2DMSE contract; - report the loaded multimodal model id in model_used instead of the hardcoded "multi-modal-embed", derived from the configured path's base name; - reject target_layer and a text-only model selector on image-only requests rather than silently ignoring them. Signed-off-by: WUKUNTAI-0211 <83541875+WUKUNTAI-0211@users.noreply.github.com>
…e always uses native Mixed text+image /api/v1/embeddings previously defaulted req.Dimension to the multimodal native (384) so both sides returned uniform-width vectors. The rationale was a "response shape is easier to consume" convenience, but cross-encoder vectors live in different embedding spaces regardless of dimension: uniform width does not enable cross-modal cosine, so the consistency gain was illusory while the cost — silently downgrading text recall from 768 to 384 for every mixed request — was real and hidden. Revert mixed defaulting so req.Dimension follows the text default (768) and controls the text side only. The image side always encodes at the multimodal native dimension in mixed mode, independent of req.Dimension. Per-item response.dimension already disambiguates modality width for the caller. - applyEmbeddingDefaults: mixed and text-only default to defaultEmbeddingDimension; only pure image-only requests default to the multimodal native dimension. - validateEmbeddingRequest: the image-over-native ceiling now applies to image-only requests only. Mixed requests may pass a text-legal req.Dimension (e.g. 768) without being rejected as image-illegal. - buildEmbeddingResults: image encode uses a new imageTargetDimension helper — req.Dimension for image-only, native for mixed. Text-side req.Dimension does not leak into the image encoder. - config.go: EmbeddingRequest.Dimension doc updated to describe the split semantics and the mixed-mode image-native override. - Tests renamed/rewritten to cover the new invariants: mixed defaults to text default, image-only defaults to native, mixed accepts above-native req.Dimension, imageTargetDimension resolves per-request correctly, and image-only encode honors req.Dimension verbatim. Signed-off-by: WUKUNTAI-0211 <83541875+WUKUNTAI-0211@users.noreply.github.com>
…ests Mixed text+image requests resolved the image side to the multimodal native dimension unconditionally, so an explicit image-satisfiable dimension (e.g. 256 <= native 384) was ignored — the image came back at 384. That reintroduced the "requested dimension silently dropped" gap this PR set out to close, now scoped to mixed requests. Resolve the image side to min(req.Dimension, native): honor an explicitly requested dimension that fits the image ceiling (both modalities stay uniform), and only diverge — capping the image at native — when req.Dimension is a text-scale value above the ceiling (e.g. the 768 default). Text recall is still preserved in the divergent case. Signed-off-by: WUKUNTAI-0211 <83541875+WUKUNTAI-0211@users.noreply.github.com>
Use the multimodal native dimension for image-bearing requests, reject unsatisfiable mixed dimensions, and report the registry checkpoint id.\n\nCloses vllm-project#2407 Signed-off-by: WUKUNTAI-0211 <83541875+WUKUNTAI-0211@users.noreply.github.com>
✅ Deploy Preview for vllm-semantic-router ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
WUKUNTAI-0211
assigned
AayushSaini101,
drivebyer,
FAUST-BENCHOU,
ramkrishs,
shraderdm and
WUKUNTAI-0211
Contributor
👥 vLLM Semantic Team NotificationThe following members have been identified for the changed files in this PR and have been automatically assigned when their GitHub accounts are assignable in this repository: 📁
|
Contributor
🚨 Supply Chain Security Report — Issues Found
CRITICAL / HIGH findings in codebase7 finding(s) — click to expand
Scanned at |
The over-dimension contract calls candle_binding.MultiModalGetEmbeddingDim, but the onnx build swaps candle-binding for onnx-binding (go.onnx.mod replace), which lacked the function -> onnx-tag builds failed with an undefined symbol (extproc, vllm-sr-cuda/rocm images and every integration-test that builds them). The ONNX C ABI exposes no dimension getter, so discover the native dimension by encoding a minimal probe at the native dimension (targetDim=0) and reading the output length. The value is constant per loaded model, so cache it after the first successful probe; return -1 when no multimodal model is loaded, matching candle-binding semantics. Signed-off-by: WUKUNTAI-0211 <83541875+WUKUNTAI-0211@users.noreply.github.com>
Split the image-specific checks out of validateEmbeddingRequest into validateImageEmbeddingParams (cyclop 18 -> 11) and extract the dimension default selection from applyEmbeddingDefaults into defaultEmbeddingDimensionFor to flatten the nested conditionals (nestif). Behavior is unchanged; satisfies the repo lint gate (cyclop max 12, nestif). Signed-off-by: WUKUNTAI-0211 <83541875+WUKUNTAI-0211@users.noreply.github.com>
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
6 participants
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.
Closes #2407
What
Settle the over-dimension contract for image inputs on
/api/v1/embeddings.The image path silently ignored the requested
dimension:encode_image_with_dimtruncated only when the target was below the native 384 and otherwise returned the
native vector unchanged, unlike the text 2DMSE path which rejects
target_dim > embedding_dim. Because the handler defaulteddimensionto 768, a default mixedtext+image request returned some 768-dim (text) and some 384-dim (image) vectors in a
single
200response, andisValidDimensionexcluded 384 — the one value that is theimage model's native dimension.
Contract
Image inputs route to the multimodal model, whose native dimension is a ceiling
(MRL only truncates down). The native dimension (384 by default) is below the text
default (768), so:
isValidDimension.a new FFI getter, not a hardcoded constant), so a plain image or mixed text+image
request returns uniform vectors instead of failing or silently mixing dimensions.
the text 2DMSE reject contract.
model_used(base name of theconfigured path) instead of the hardcoded
"multi-modal-embed".target_layerand a text-onlymodelselector on image-only requestsrather than silently ignoring them. Mixed text+image requests still honor
modelfor the text side.
Changes
candle-binding(Rust): rejecttarget_dim > embedding_diminencode_image_with_dim; addmultimodal_get_embedding_dim()FFI export.candle-binding(Go):MultiModalGetEmbeddingDim()wrapper + non-cgo mock.pkg/apiserver: dimension defaulting/validation,model_used, andmodel/target_layerguards described above; dimension doc comment.Test Plan
multi-modal-embed-smallcheckpoint (CPU):test_image_invalid_dim_rejected— RED before the fix(
image dim=768 (> native) should be rejected— pre-fix returned the native vector),GREEN after.
pkg/apiserver): native-dimension default, mixed-requestuniform default, unloaded-model fallback, accept-384, reject-above-native,
reject
target_layer/text-model on image-only, allow multimodal selector, allow textmodel on mixed,
model_usedpropagation, and model-id base-naming. Full package green.cargo build --release(candle/ml/nlp),cargo fmt --check,gofmt -lclean.Model-gated multimodal tests only execute in CI once
MULTIMODAL_MODEL_PATHis wiredthere (tracked separately in #2319); the Rust assertion above is
#[ignore]-gated forthat reason and was run locally against the real checkpoint.