Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions crates/grpc_client/proto/common.proto
Original file line number Diff line number Diff line change
Expand Up @@ -115,3 +115,12 @@ message RemoteTensorHandle {
bytes descriptor = 2;
uint64 nbytes = 3;
}

// Multimodal input modality, shared across engine protos so the servicers can
// route each request to the matching encoder (image vs video vs audio).
enum Modality {
MODALITY_UNSPECIFIED = 0;
IMAGE = 1;
AUDIO = 2;
VIDEO = 3;
}
11 changes: 2 additions & 9 deletions crates/grpc_client/proto/tokenspeed_scheduler.proto
Original file line number Diff line number Diff line change
Expand Up @@ -191,15 +191,8 @@ message PlaceholderRange {
uint32 length = 2;
}

enum Modality {
MODALITY_UNSPECIFIED = 0;
IMAGE = 1;
AUDIO = 2;
VIDEO = 3;
}

message MultimodalItem {
Modality modality = 1;
smg.grpc.common.Modality modality = 1;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Update encode servicer to use common modality constants

When the TokenSpeed encode-stage servicer handles any EncodeRequest, this field now comes from smg.grpc.common.Modality, so the generated tokenspeed_scheduler_pb2 module no longer defines IMAGE, VIDEO, or MODALITY_UNSPECIFIED; those constants live in common_pb2. The unchanged grpc_servicer/smg_grpc_servicer/tokenspeed/encoder_servicer.py still reads tokenspeed_scheduler_pb2.IMAGE/VIDEO in _items_from_proto, so the first encode RPC raises AttributeError before it can enqueue the item. Please update that servicer to use the common enum constants (or numeric-compatible comparisons) together with this proto move.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Expose common_pb2 after moving modality enum

This moves the public modality field to smg.grpc.common.Modality, but the Python package's lazy public surface still omits common_pb2 (crates/grpc_client/python/smg_grpc_proto/__init__.py:8-23). For Python clients that previously used tokenspeed_scheduler_pb2.IMAGE/VIDEO, the generated scheduler module no longer defines those names, and from smg_grpc_proto import common_pb2 raises AttributeError, so they have no documented named constants to populate the field. Please add common_pb2 to _GENERATED_MODULES/__all__.

Useful? React with 👍 / 👎.

bytes content_hash = 2;
// Primary input to the multimodal encoder for this item. For vision models
// this is the preprocessed image/video tensor; for audio models this can be
Expand Down Expand Up @@ -325,7 +318,7 @@ message GetModelInfoResponse {
// non-vision workers before tokenization).
bool supports_vision = 14;
bool supports_multimodal = 15;
repeated Modality supported_modalities = 16;
repeated smg.grpc.common.Modality supported_modalities = 16;
string model_dtype = 17;
string multimodal_encoder_dtype = 18;
}
Expand Down
5 changes: 5 additions & 0 deletions crates/grpc_client/proto/vllm_engine.proto
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,11 @@ message MultimodalInputs {
// Tensor keys that should remain on CPU (not transferred to GPU).
// Maps to vLLM's MultiModalFieldConfig keep_on_cpu flag.
repeated string keep_on_cpu_keys = 9;

// Input modality (image/video). The servicer routes video to vLLM's video
// modality (`pixel_values_videos` / `video_grid_thw`) and expands the video
// placeholder token instead of the image one. Unset (0) is treated as image.
smg.grpc.common.Modality modality = 10;
}

// =====================
Expand Down
2 changes: 1 addition & 1 deletion crates/grpc_client/python/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ build-backend = "setuptools.build_meta"

[project]
name = "smg-grpc-proto"
version = "0.4.12"
version = "0.4.13"
description = "SMG gRPC proto definitions for vLLM, TRT-LLM, MLX, TokenSpeed, and SGLang"
requires-python = ">=3.10"
dependencies = [
Expand Down
7 changes: 6 additions & 1 deletion crates/grpc_client/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,12 @@
//! and SGLang scheduler backends.

pub mod common_proto {
#![allow(clippy::all, clippy::absolute_paths, unused_qualifications)]
#![allow(
clippy::all,
clippy::absolute_paths,
clippy::trivially_copy_pass_by_ref,
unused_qualifications
)]
tonic::include_proto!("smg.grpc.common");
}
pub mod abort_on_drop;
Expand Down
100 changes: 77 additions & 23 deletions crates/multimodal/src/registry/qwen3_vl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,14 +55,6 @@ impl Qwen3VLVisionSpec {
})
}

fn is_qwen3_5(metadata: &ModelMetadata) -> bool {
let id = metadata.model_id.to_ascii_lowercase();
let model_type = metadata.config_model_type();
id.contains("qwen3.5")
|| id.contains("qwen3.6")
|| model_type.is_some_and(|mt| mt == "qwen3_5" || mt == "qwen3_5_moe")
}

fn video_grid_t(preprocessed: &PreprocessedEncoderInputs) -> Option<usize> {
match preprocessed.model_specific.get("video_grid_thw") {
Some(ModelSpecificValue::IntTensor { data, shape })
Expand Down Expand Up @@ -92,7 +84,16 @@ impl Qwen3VLVisionSpec {
.unwrap_or_default()
}

fn qwen3_5_video_replacement_tokens(
/// Build the per-frame video placeholder body for the Qwen3-VL family.
///
/// Qwen3-VL lays out video as one `<|vision_start|> .. <|vision_end|>` block
/// per temporal frame with a `<seconds>` timestamp between frames. The chat
/// template already supplies the outer `<|vision_start|>`/`<|vision_end|>`, so
/// this emits only the inner per-frame structure (hence the `grid_idx > 0`
/// guards that reuse the template's opener/closer for the first/last frame).
Comment on lines +91 to +93

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Emit Qwen3-VL's full per-frame video wrappers

For Qwen3-VL video requests with grid_t > 1, reusing the chat template's outer <|vision_start|>/<|vision_end|> drops the inner start marker for the first frame and the inner end marker for the last frame. The Qwen3VL processor expands the bare video pad into timestamp + vision_start + video pads + vision_end for every temporal frame while preserving the outer wrapper, so this produces different prompt tokens/MRoPE positions and can corrupt all vLLM video results for this model family.

Useful? React with 👍 / 👎.

/// Returns `None` when the layout can't apply (single-frame or ragged token
/// counts), leaving the caller to fall back to a flat pad block.
fn per_frame_video_tokens(
metadata: &ModelMetadata,
pad_token_id: TokenId,
num_tokens: usize,
Expand Down Expand Up @@ -246,21 +247,29 @@ impl ModelProcessorSpec for Qwen3VLVisionSpec {
.feature_token_counts
.iter()
.map(|&num_tokens| {
let tokens = if Self::is_qwen3_5(metadata) {
video_grid_t
.and_then(|grid_t| {
Self::qwen3_5_video_replacement_tokens(
metadata,
pad_token_id,
num_tokens,
grid_t,
)
})
.unwrap_or_else(|| vec![pad_token_id; num_tokens])
} else {
vec![pad_token_id; num_tokens]
};
// Every Qwen3-VL model routed to this spec (base VL and the
// 3.5/3.6 family) needs the per-frame video layout: vLLM's
// mrope pass scans for one <|vision_start|> per temporal
// frame, so a single flat block crashes any multi-frame
// video. Fall back to a flat block only when the per-frame
// layout can't be built (single-frame or unknown grid_t).
let tokens = video_grid_t
.and_then(|grid_t| {
Self::per_frame_video_tokens(
metadata,
pad_token_id,
num_tokens,
grid_t,
)
})
.unwrap_or_else(|| vec![pad_token_id; num_tokens]);
// The chat template wraps the placeholder as
// <|vision_start|><|video_pad|><|vision_end|>; the leading
// <|vision_start|> belongs to the placeholder range so vLLM's
// per-frame video mrope finds one marker per frame starting at
// the range offset (it scans even for a single frame).
PromptReplacement::sequence(Modality::Video, &placeholder_token, tokens)
.with_structural_prefix(1)
})
.collect())
}
Expand Down Expand Up @@ -399,6 +408,51 @@ mod tests {
assert!(tokens[82..].iter().all(|&token| token == 151656));
}

#[test]
fn qwen3_vl_video_splits_temporal_grid() {
// Base Qwen3-VL (not the 3.5/3.6 family) must ALSO emit one vision block
// per temporal frame. vLLM's mrope pass scans for a <|vision_start|> per
// frame, so a flat single block crashes any multi-frame video. Regression
// guard for the is_qwen3_5-only gate that previously left base VL flat.
let tokenizer = TestTokenizer::new(&[
("<|video_pad|>", 151656),
("<|vision_start|>", 151652),
("<|vision_end|>", 151653),
]);
let config = json!({
"model_type": "qwen3_vl",
"image_token_id": 151655,
"video_token_id": 151656,
"vision_start_token_id": 151652,
"vision_end_token_id": 151653,
});
let metadata = ModelMetadata {
model_id: "Qwen/Qwen3-VL-8B-Instruct",
tokenizer: &tokenizer,
config: &config,
};
let registry = ModelRegistry::new();
let spec = registry.lookup(&metadata).expect("qwen3_vl spec");
assert_eq!(spec.name(), "qwen3_vl");
let preprocessed = test_preprocessed_with_tokens(&[ImageSize::new(320, 256)], &[160])
.with_extra(
"video_grid_thw",
ModelSpecificValue::int_2d(vec![2, 16, 20], 1, 3),
);
let replacements = spec
.prompt_replacements_for(&metadata, &preprocessed, crate::types::Modality::Video)
.unwrap();

// Two temporal frames -> a vision_end/vision_start seam splits the 160
// pads into two 80-token halves (mirrors the 3.5 case above).
let tokens = &replacements[0].tokens;
assert_eq!(tokens.len(), 162);
assert!(tokens[..80].iter().all(|&token| token == 151656));
assert_eq!(tokens[80], 151653);
assert_eq!(tokens[81], 151652);
assert!(tokens[82..].iter().all(|&token| token == 151656));
}

#[test]
fn qwen2_vl_does_not_match_qwen3() {
let tokenizer = TestTokenizer::new(&[("<image>", 999)]);
Expand Down
20 changes: 20 additions & 0 deletions crates/multimodal/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,15 @@ pub struct PromptReplacement {
pub modality: Modality,
pub placeholder_token: String,
pub tokens: Vec<TokenId>,
/// Number of structural tokens the chat template emits *immediately before*
/// this placeholder (e.g. Qwen's leading `<|vision_start|>`) that belong to
/// the placeholder's range. `expand_tokens` folds them into the reported
/// [`PlaceholderRange`] without re-emitting them, so backends that scan the
/// range for structural markers see the leading marker. vLLM's video mrope
/// walks each frame from `<|vision_start|>` starting at the range offset, so
/// the offset must sit on (or before) the first marker. 0 for the common
/// case where the range is exactly the replacement.
pub structural_prefix: usize,
}

impl PromptReplacement {
Expand All @@ -358,6 +367,7 @@ impl PromptReplacement {
modality,
placeholder_token: placeholder_token.to_string(),
tokens: vec![token_id; count],
structural_prefix: 0,
}
}

Expand All @@ -366,8 +376,18 @@ impl PromptReplacement {
modality,
placeholder_token: placeholder_token.to_string(),
tokens: sequence,
structural_prefix: 0,
}
}

/// Declare that `n` template-emitted structural tokens precede this
/// placeholder and should be included in its reported range. See
/// [`Self::structural_prefix`].
#[must_use]
pub fn with_structural_prefix(mut self, n: usize) -> Self {
self.structural_prefix = n;
self
}
}

#[cfg(test)]
Expand Down
2 changes: 1 addition & 1 deletion grpc_servicer/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ version = "0.6.0"
description = "SMG gRPC servicer implementations for LLM inference engines (vLLM, MLX, TokenSpeed, SGLang)"
requires-python = ">=3.10"
dependencies = [
"smg-grpc-proto>=0.4.11",
"smg-grpc-proto>=0.4.13",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Bump servicer version before publishing this dependency change

At this dependency floor change, the project version above remains 0.6.0. The checked release workflow runs on grpc_servicer/pyproject.toml changes (.github/workflows/release-grpc.yml:7-9,38-39) and uploads with twine upload ... --skip-existing (.github/workflows/release-grpc.yml:163-165); if 0.6.0 is already published, the new wheel is skipped, so pip install -U smg-grpc-servicer will not get the proto floor or servicer fixes. Please bump the servicer package version with this change.

Useful? React with 👍 / 👎.

"grpcio>=1.81.1",
"grpcio-reflection>=1.81.1",
"grpcio-health-checking>=1.81.1",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@

import grpc
from smg_grpc_proto.generated import (
common_pb2,
tokenspeed_encoder_pb2,
tokenspeed_encoder_pb2_grpc,
tokenspeed_scheduler_pb2,
)

from smg_grpc_servicer.tokenspeed.rdma_pixel import RdmaPixelPuller
Expand Down Expand Up @@ -145,12 +145,12 @@ def _items_from_proto(self, mm_inputs, bootstrap_room: int = 0):
}

if item_proto.modality in (
tokenspeed_scheduler_pb2.IMAGE,
tokenspeed_scheduler_pb2.MODALITY_UNSPECIFIED,
common_pb2.IMAGE,
common_pb2.MODALITY_UNSPECIFIED,
):
item_modality = Modality.IMAGE
grid_key = "image_grid_thw"
elif item_proto.modality == tokenspeed_scheduler_pb2.VIDEO:
elif item_proto.modality == common_pb2.VIDEO:
item_modality = Modality.VIDEO
grid_key = "video_grid_thw"
else:
Expand Down
10 changes: 5 additions & 5 deletions grpc_servicer/smg_grpc_servicer/tokenspeed/servicer.py
Original file line number Diff line number Diff line change
Expand Up @@ -432,8 +432,8 @@ async def GetModelInfo(
self.server_args, "tokenizer_path", ""
)
supports_vision = bool(getattr(model_config, "is_multimodal", False))
image_modality = getattr(tokenspeed_scheduler_pb2, "IMAGE", 1)
video_modality = getattr(tokenspeed_scheduler_pb2, "VIDEO", 3)
image_modality = common_pb2.IMAGE
video_modality = common_pb2.VIDEO
supported_modalities = []
if supports_vision:
supported_modalities.append(image_modality)
Expand Down Expand Up @@ -1147,11 +1147,11 @@ def _mm_inputs_from_itemized_proto(

@staticmethod
def _modality_from_proto(modality: int) -> Modality:
if modality == getattr(tokenspeed_scheduler_pb2, "IMAGE", 1):
if modality == common_pb2.IMAGE:
return Modality.IMAGE
if modality == getattr(tokenspeed_scheduler_pb2, "VIDEO", 3):
if modality == common_pb2.VIDEO:
return Modality.VIDEO
if modality == getattr(tokenspeed_scheduler_pb2, "AUDIO", 2):
if modality == common_pb2.AUDIO:
raise ValueError("TokenSpeed audio multimodal inputs are not supported yet")
raise ValueError(f"Unsupported multimodal item modality: {modality}")

Expand Down
34 changes: 23 additions & 11 deletions grpc_servicer/smg_grpc_servicer/vllm/servicer.py
Original file line number Diff line number Diff line change
Expand Up @@ -614,14 +614,26 @@ def _build_preprocessed_mm_inputs(
``batched_keys`` and ``flat_keys`` proto fields.
"""
prompt_token_ids = list(tokenized.input_ids)
num_images = len(mm_proto.mm_placeholders)
num_items = len(mm_proto.mm_placeholders)

# Image vs video: vLLM routes each modality to a different encoder and
# expects the pixel tensor under a modality-specific key. The router sends
# the generic ``pixel_values`` field; rename it to ``pixel_values_videos``
# for the video path (grid/size tensors already carry video-specific keys).
is_video = mm_proto.modality == common_pb2.VIDEO
mm_modality = "video" if is_video else "image"
Comment on lines +617 to +624

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Silently treats unknown/unsupported modality as image instead of failing fast.

is_video only checks for VIDEO; any other value (e.g. AUDIO, or a future enum member) falls through to mm_modality = "image" without validation. TokenSpeed's _modality_from_proto and encoder_servicer.py's dispatch both explicitly raise for unsupported modalities — this path should follow the same convention rather than silently mis-routing.

🛡️ Proposed fix
-        is_video = mm_proto.modality == common_pb2.VIDEO
+        if mm_proto.modality not in (
+            common_pb2.MODALITY_UNSPECIFIED,
+            common_pb2.IMAGE,
+            common_pb2.VIDEO,
+        ):
+            raise ValueError(
+                f"vLLM multimodal path modality={mm_proto.modality} is not supported"
+            )
+        is_video = mm_proto.modality == common_pb2.VIDEO
         mm_modality = "video" if is_video else "image"
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
num_items = len(mm_proto.mm_placeholders)
# Image vs video: vLLM routes each modality to a different encoder and
# expects the pixel tensor under a modality-specific key. The router sends
# the generic ``pixel_values`` field; rename it to ``pixel_values_videos``
# for the video path (grid/size tensors already carry video-specific keys).
is_video = mm_proto.modality == common_pb2.VIDEO
mm_modality = "video" if is_video else "image"
num_items = len(mm_proto.mm_placeholders)
# Image vs video: vLLM routes each modality to a different encoder and
# expects the pixel tensor under a modality-specific key. The router sends
# the generic ``pixel_values`` field; rename it to ``pixel_values_videos``
# for the video path (grid/size tensors already carry video-specific keys).
if mm_proto.modality not in (
common_pb2.MODALITY_UNSPECIFIED,
common_pb2.IMAGE,
common_pb2.VIDEO,
):
raise ValueError(
f"vLLM multimodal path modality={mm_proto.modality} is not supported"
)
is_video = mm_proto.modality == common_pb2.VIDEO
mm_modality = "video" if is_video else "image"
🤖 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 `@grpc_servicer/smg_grpc_servicer/vllm/servicer.py` around lines 617 - 624, The
modality routing in `servicer.py` currently treats any non-VIDEO value as image,
so add explicit validation in the `mm_modality`/`is_video` branch and fail fast
for unsupported `common_pb2` modalities. Mirror the behavior used by
`_modality_from_proto` and `encoder_servicer.py` by checking only the supported
image/video enum values before assigning the modality-specific key, and raise an
error for anything else instead of defaulting to image.


def mm_key(key: str) -> str:
if is_video and key == "pixel_values":
return "pixel_values_videos"
return key

# Deserialize all tensors from proto
hf_dict: dict[str, torch.Tensor] = {
"pixel_values": _tensor_from_proto(mm_proto.pixel_values),
mm_key("pixel_values"): _tensor_from_proto(mm_proto.pixel_values),
}
for key, td in mm_proto.model_specific_tensors.items():
hf_dict[key] = _tensor_from_proto(td)
hf_dict[mm_key(key)] = _tensor_from_proto(td)

# Cast floating-point tensors to model dtype (e.g. bfloat16).
# This mirrors _postprocess_output in multimodal/processing/context.py
Expand All @@ -631,34 +643,34 @@ def _build_preprocessed_mm_inputs(
if hf_dict[key].is_floating_point():
hf_dict[key] = hf_dict[key].to(dtype=model_dtype)

cpu_keys = set(mm_proto.keep_on_cpu_keys)
cpu_keys = {mm_key(k) for k in mm_proto.keep_on_cpu_keys}

# Field configs are fully determined by the Rust router.
batched = set(mm_proto.batched_keys)
flat = dict(mm_proto.flat_keys)
batched = {mm_key(k) for k in mm_proto.batched_keys}
flat = {mm_key(k): mm_key(v) for k, v in mm_proto.flat_keys.items()}
fields_config: dict[str, MultiModalFieldConfig] = {}
flat_sizes_cache: dict[str, torch.Tensor] = {}
for key in hf_dict:
on_cpu = key in cpu_keys
if key in batched:
fields_config[key] = MultiModalFieldConfig.batched("image", keep_on_cpu=on_cpu)
fields_config[key] = MultiModalFieldConfig.batched(mm_modality, keep_on_cpu=on_cpu)
elif key in flat:
sizes_key = flat[key]
if sizes_key not in flat_sizes_cache:
flat_sizes_cache[sizes_key] = hf_dict[sizes_key].flatten().to(torch.int64)
fields_config[key] = MultiModalFieldConfig.flat_from_sizes(
"image", flat_sizes_cache[sizes_key], keep_on_cpu=on_cpu
mm_modality, flat_sizes_cache[sizes_key], keep_on_cpu=on_cpu
)
else:
fields_config[key] = MultiModalFieldConfig.shared("image", num_images)
fields_config[key] = MultiModalFieldConfig.shared(mm_modality, num_items)

batch_feature = BatchFeature(hf_dict, tensor_type="pt")
mm_kwargs = MultiModalKwargsItems.from_hf_inputs(batch_feature, fields_config)

# Build mm_hashes: dict[str, list[str]]
mm_hashes: dict[str, list[str]] = {}
if mm_proto.mm_hashes:
mm_hashes["image"] = list(mm_proto.mm_hashes)
mm_hashes[mm_modality] = list(mm_proto.mm_hashes)

# Build mm_placeholders: dict[str, list[PlaceholderRange]]
# When structural tokens (e.g. <|image_start|>, separators) are present
Expand Down Expand Up @@ -686,7 +698,7 @@ def _build_preprocessed_mm_inputs(
placeholders.append(
PlaceholderRange(offset=p.offset, length=p.length, is_embed=is_embed)
)
mm_placeholders["image"] = placeholders
mm_placeholders[mm_modality] = placeholders

return mm_input(
prompt_token_ids=prompt_token_ids,
Expand Down
Loading
Loading