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
2 changes: 1 addition & 1 deletion crates/multimodal/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ pub use media::{ImageFetchConfig, MediaConnector, MediaConnectorConfig, MediaSou
pub use registry::{ModelMetadata, ModelProcessorSpec, ModelRegistry};
pub use tracker::{AsyncMultiModalTracker, TrackerOutput};
pub use types::{
ChatContentPart, FieldLayout, ImageDetail, ImageFrame, ImageSize, ImageSource, Modality,
FieldLayout, ImageDetail, ImageFrame, ImageSize, ImageSource, MediaContentPart, Modality,
MultiModalData, MultiModalUUIDs, PlaceholderRange, PromptReplacement, TokenId, TrackedMedia,
Comment on lines 13 to 15

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Keep a compatibility export for the renamed public type.

Dropping ChatContentPart from the crate root will break downstream imports immediately. If llm-multimodal is consumed outside this workspace, please either keep a deprecated alias for one release or ship this behind an explicit breaking-version bump.

Possible compatibility shim
 pub use types::{
     MediaContentPart, FieldLayout, ImageDetail, ImageFrame, ImageSize, ImageSource, Modality,
     MultiModalData, MultiModalUUIDs, PlaceholderRange, PromptReplacement, TokenId, TrackedMedia,
 };
+#[deprecated(note = "renamed to MediaContentPart")]
+pub use types::MediaContentPart as ChatContentPart;
📝 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
pub use types::{
ChatContentPart, FieldLayout, ImageDetail, ImageFrame, ImageSize, ImageSource, Modality,
MediaContentPart, FieldLayout, ImageDetail, ImageFrame, ImageSize, ImageSource, Modality,
MultiModalData, MultiModalUUIDs, PlaceholderRange, PromptReplacement, TokenId, TrackedMedia,
pub use types::{
MediaContentPart, FieldLayout, ImageDetail, ImageFrame, ImageSize, ImageSource, Modality,
MultiModalData, MultiModalUUIDs, PlaceholderRange, PromptReplacement, TokenId, TrackedMedia,
};
#[deprecated(note = "renamed to MediaContentPart")]
pub use types::MediaContentPart as ChatContentPart;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@crates/multimodal/src/lib.rs` around lines 13 - 15, The crate root removed
the legacy export ChatContentPart causing downstream breakage; restore a
compatibility alias by re-exporting the new type (MediaContentPart) under the
old name (ChatContentPart) in lib.rs as a deprecated shim so consumers keep
working for one release—add a pub use or type alias for ChatContentPart that
points to MediaContentPart and mark it deprecated in the documentation/comments.

};
// Re-export vision processing components
Expand Down
12 changes: 6 additions & 6 deletions crates/multimodal/src/tracker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use super::{
error::{MultiModalError, MultiModalResult},
media::{ImageFetchConfig, MediaConnector, MediaSource},
types::{
ChatContentPart, ImageDetail, Modality, MultiModalData, MultiModalUUIDs, TrackedMedia,
ImageDetail, MediaContentPart, Modality, MultiModalData, MultiModalUUIDs, TrackedMedia,
},
};

Expand All @@ -33,17 +33,17 @@ impl AsyncMultiModalTracker {
}
}

pub fn push_part(&mut self, part: ChatContentPart) -> MultiModalResult<()> {
pub fn push_part(&mut self, part: MediaContentPart) -> MultiModalResult<()> {
match part {
ChatContentPart::Text { .. } => {}
ChatContentPart::ImageUrl { url, detail, uuid } => {
MediaContentPart::Text { .. } => {}
MediaContentPart::ImageUrl { url, detail, uuid } => {
let source = match url::Url::parse(&url) {
Ok(parsed) if parsed.scheme() == "data" => MediaSource::DataUrl(url),
_ => MediaSource::Url(url),
};
self.enqueue_image(source, detail.unwrap_or_default(), uuid);
}
ChatContentPart::ImageData {
MediaContentPart::ImageData {
data,
mime_type: _,
uuid,
Expand All @@ -55,7 +55,7 @@ impl AsyncMultiModalTracker {
uuid,
);
}
ChatContentPart::ImageEmbeds { .. } => {
MediaContentPart::ImageEmbeds { .. } => {
return Err(MultiModalError::UnsupportedContent("image_embeds"));
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/multimodal/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ pub enum ImageDetail {
/// A normalized content part understood by the tracker.
#[derive(Debug, Clone, Serialize, Deserialize)]
#[serde(tag = "type", rename_all = "snake_case")]
pub enum ChatContentPart {
pub enum MediaContentPart {
Text {
text: String,
},
Expand Down
10 changes: 5 additions & 5 deletions crates/multimodal/tests/multimodal_tracker_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ use std::{path::PathBuf, sync::Arc, time::Duration};

use base64::{engine::general_purpose::STANDARD as BASE64_STANDARD, Engine};
use llm_multimodal::{
AsyncMultiModalTracker, ChatContentPart, ImageFetchConfig, ImageSource, MediaConnector,
MediaConnectorConfig, MediaSource, Modality,
AsyncMultiModalTracker, ImageFetchConfig, ImageSource, MediaConnector, MediaConnectorConfig,
MediaContentPart, MediaSource, Modality,
};
use reqwest::Client;
use tempfile::tempdir;
Expand Down Expand Up @@ -104,20 +104,20 @@ async fn tracker_fetches_images_and_records_uuids() {
let mut tracker = AsyncMultiModalTracker::new(connector);

tracker
.push_part(ChatContentPart::Text {
.push_part(MediaContentPart::Text {
text: "before".into(),
})
.expect("text part");
tracker
.push_part(ChatContentPart::ImageData {
.push_part(MediaContentPart::ImageData {
data: tiny_png_bytes(),
mime_type: Some("image/png".into()),
uuid: Some("img-1".into()),
detail: None,
})
.expect("image part");
tracker
.push_part(ChatContentPart::Text {
.push_part(MediaContentPart::Text {
text: "after".into(),
})
.expect("text part");
Expand Down
140 changes: 129 additions & 11 deletions model_gateway/src/routers/grpc/multimodal.rs
Original file line number Diff line number Diff line change
@@ -1,23 +1,29 @@
//! Multimodal processing integration for gRPC chat pipeline.
//! Multimodal processing integration for gRPC pipeline (chat + messages).
//!
//! This module bridges the `llm-multimodal` crate with the gRPC router pipeline,
//! handling the full processing chain: extract content parts → fetch images →
//! preprocess pixels → expand placeholder tokens → build proto MultimodalInputs.
//!
//! Both the chat completion pipeline and the Messages API pipeline share the same
//! processing core (`process_multimodal_parts`). Only the detection and extraction
//! functions differ because they work with different input types (`ChatMessage` vs
//! `InputMessage`).

use std::{collections::HashMap, path::Path, sync::Arc};

use anyhow::{Context, Result};
use dashmap::DashMap;
use llm_multimodal::{
AsyncMultiModalTracker, ChatContentPart, FieldLayout, ImageDetail, ImageFrame,
ImageProcessorRegistry, MediaConnector, MediaConnectorConfig, Modality, ModelMetadata,
ModelRegistry, ModelSpecificValue, PlaceholderRange, PreProcessorConfig, PreprocessedImages,
AsyncMultiModalTracker, FieldLayout, ImageDetail, ImageFrame, ImageProcessorRegistry,
MediaConnector, MediaConnectorConfig, MediaContentPart, Modality, ModelMetadata, ModelRegistry,
ModelSpecificValue, PlaceholderRange, PreProcessorConfig, PreprocessedImages,
PromptReplacement, TrackedMedia, TrackerOutput,
};
use llm_tokenizer::TokenizerTrait;
use openai_protocol::{
chat::{ChatMessage, MessageContent},
common::ContentPart,
messages::{ImageSource, InputContent, InputContentBlock, InputMessage, Role},
};
use tracing::{debug, warn};

Expand Down Expand Up @@ -165,8 +171,8 @@ pub(crate) fn has_multimodal_content(messages: &[ChatMessage]) -> bool {
}

/// Extract multimodal content parts from OpenAI chat messages,
/// converting protocol `ContentPart` to multimodal crate `ChatContentPart`.
fn extract_content_parts(messages: &[ChatMessage]) -> Vec<ChatContentPart> {
/// converting protocol `ContentPart` to multimodal crate `MediaContentPart`.
fn extract_content_parts(messages: &[ChatMessage]) -> Vec<MediaContentPart> {
let mut parts = Vec::new();

for msg in messages {
Expand All @@ -182,14 +188,14 @@ fn extract_content_parts(messages: &[ChatMessage]) -> Vec<ChatContentPart> {
match part {
ContentPart::ImageUrl { image_url } => {
let detail = image_url.detail.as_deref().and_then(parse_detail);
parts.push(ChatContentPart::ImageUrl {
parts.push(MediaContentPart::ImageUrl {
url: image_url.url.clone(),
detail,
uuid: None,
});
}
ContentPart::Text { text } => {
parts.push(ChatContentPart::Text { text: text.clone() });
parts.push(MediaContentPart::Text { text: text.clone() });
}
ContentPart::VideoUrl { .. } => {} // Skip VideoUrl for now
}
Expand All @@ -210,6 +216,96 @@ fn parse_detail(detail: &str) -> Option<ImageDetail> {
}
}

// ---------------------------------------------------------------------------
// Messages API multimodal detection and extraction
// ---------------------------------------------------------------------------

/// Check if any messages in a Messages API request contain multimodal content.
pub(crate) fn has_multimodal_content_messages(messages: &[InputMessage]) -> bool {
messages.iter().any(|msg| {
if msg.role != Role::User {
return false;
}
match &msg.content {
InputContent::Blocks(blocks) => blocks
.iter()
.any(|block| matches!(block, InputContentBlock::Image(_))),
InputContent::String(_) => false,
}
})
Comment on lines +225 to +235

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The closure passed to any() can be simplified by combining the role check with the content check using a logical AND (&&). This makes the intent clearer and more concise.

    messages.iter().any(|msg| {
        msg.role == Role::User
            && match &msg.content {
                InputContent::Blocks(blocks) => blocks
                    .iter()
                    .any(|block| matches!(block, InputContentBlock::Image(_))),
                InputContent::String(_) => false,
            }
    })
References
  1. Prioritize code simplicity and clarity over micro-optimizations, especially when the performance gain is negligible for typical use cases (e.g., iterating over a small number of items).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This comment is valid

}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

/// Extract multimodal content parts from Messages API input messages,
/// converting `InputContentBlock::Image` to multimodal crate `MediaContentPart`.
fn extract_content_parts_messages(messages: &[InputMessage]) -> Vec<MediaContentPart> {
let mut parts = Vec::new();

for msg in messages {
if msg.role != Role::User {
continue;
}
let blocks = match &msg.content {
InputContent::Blocks(blocks) => blocks,
InputContent::String(_) => continue,
};

for block in blocks {
match block {
InputContentBlock::Image(image_block) => match &image_block.source {
ImageSource::Base64 { media_type, data } => {
// Convert base64 to data URL for the media connector
let data_url = format!("data:{media_type};base64,{data}");
parts.push(MediaContentPart::ImageUrl {
url: data_url,
detail: None,
uuid: None,
});
}
ImageSource::Url { url } => {
parts.push(MediaContentPart::ImageUrl {
url: url.clone(),
detail: None,
uuid: None,
});
}
},
InputContentBlock::Text(text_block) => {
parts.push(MediaContentPart::Text {
text: text_block.text.clone(),
});
}
_ => {}
}
}
}

parts
}
Comment on lines +238 to +283

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Add a brief comment explaining which block types are intentionally skipped.

The _ => {} catch-all at line 277 silently skips InputContentBlock::Document, ToolUse, ToolResult, etc. This is consistent with the existing pattern for VideoUrl in the chat pipeline, but a brief comment would improve clarity for future maintainers.

📝 Suggested comment
                 InputContentBlock::Text(text_block) => {
                     parts.push(MediaContentPart::Text {
                         text: text_block.text.clone(),
                     });
                 }
-                _ => {}
+                // Skip Document, ToolUse, ToolResult, etc. — only images and text are
+                // processed for multimodal; other block types pass through unchanged.
+                _ => {}
             }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@model_gateway/src/routers/grpc/multimodal.rs` around lines 238 - 283, In
extract_content_parts_messages, add a brief comment above the catch-all arm (the
`_ => {}` in the match over InputContentBlock inside
extract_content_parts_messages) explaining that non-image/text block types—e.g.,
InputContentBlock::Document, ToolUse, ToolResult, VideoUrl, etc.—are
intentionally ignored here because this pipeline only extracts multimodal Image
and Text parts for the media connector; reference the catch-all and the specific
handled arms (InputContentBlock::Image, InputContentBlock::Text) so future
maintainers understand the omission.


/// Process multimodal content from Messages API input messages.
///
/// Entry point for the messages preparation stage. Extracts image content parts
/// from `InputMessage`, then delegates to the shared processing core.
pub(crate) async fn process_multimodal_messages(
messages: &[InputMessage],
model_id: &str,
tokenizer: &dyn TokenizerTrait,
token_ids: Vec<u32>,
components: &MultimodalComponents,
tokenizer_source: &str,
) -> Result<MultimodalOutput> {
let content_parts = extract_content_parts_messages(messages);
process_multimodal_parts(
content_parts,
model_id,
tokenizer,
token_ids,
components,
tokenizer_source,
)
.await
}

/// Process multimodal content: fetch images, preprocess pixels, expand tokens, collect hashes.
///
/// Single entry point called from preparation.rs. Handles the full pipeline:
Expand All @@ -222,8 +318,30 @@ pub(crate) async fn process_multimodal(
components: &MultimodalComponents,
tokenizer_source: &str,
) -> Result<MultimodalOutput> {
// Step 1: Fetch images
let content_parts = extract_content_parts(messages);
process_multimodal_parts(
content_parts,
model_id,
tokenizer,
token_ids,
components,
tokenizer_source,
)
.await
}

/// Shared multimodal processing core.
///
/// Takes pre-extracted `MediaContentPart`s (from either chat or messages pipeline)
/// and runs the full processing chain: fetch → preprocess → expand → build intermediate.
async fn process_multimodal_parts(
content_parts: Vec<MediaContentPart>,
model_id: &str,
tokenizer: &dyn TokenizerTrait,
token_ids: Vec<u32>,
components: &MultimodalComponents,
tokenizer_source: &str,
) -> Result<MultimodalOutput> {
let mut tracker = AsyncMultiModalTracker::new(components.media_connector.clone());

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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


for part in content_parts {
Expand Down Expand Up @@ -674,12 +792,12 @@ mod tests {
assert_eq!(parts.len(), 2);

match &parts[0] {
ChatContentPart::Text { text } => assert_eq!(text, "Describe this:"),
MediaContentPart::Text { text } => assert_eq!(text, "Describe this:"),
_ => panic!("Expected Text part"),
}

match &parts[1] {
ChatContentPart::ImageUrl { url, detail, .. } => {
MediaContentPart::ImageUrl { url, detail, .. } => {
assert_eq!(url, "https://example.com/image.jpg");
assert_eq!(*detail, Some(ImageDetail::High));
}
Expand Down
Loading
Loading