feat(vllm): support multimodal sidecar requests

[connorcarpenter15]

Summary

Part 4 of a 4-PR stack.

• Convert Dynamo multimodal inputs into vLLM MediaItem messages.
• Map canonical mm_hashes to MediaItem.uuid so repeated images use the same keys as vLLM KV events.
• Carry multimodal prompt token IDs through the private preprocessor-to-sidecar handoff.
• Support aggregate and disaggregated multimodal generation without duplicating media processing.
• Keep this layer free of mocker changes.

Stack

1. #12734 — split gRPC services and Control discovery
2. #12736 — preserve decode handoff on cancellation
3. #12735 — deterministic DP and KV routing
4. #12214 — multimodal sidecar requests

Base: #12735

Validation

• cargo test -p dynamo-sidecar-common -p dynamo-vllm-sidecar -p dynamo-vllm-mocker — 38 tests passed
• cargo clippy -p dynamo-sidecar-common -p dynamo-vllm-sidecar -p dynamo-vllm-mocker --all-targets -- -D warnings
• cargo fmt --all -- --check
• The cumulative implementation was previously exercised across the planned 2×H100 aggregate, cancellation, DP2/KV-routing, disaggregated, KVBM, multimodal, and recovery matrix. This stack split preserves that implementation; the rebase added only an unrelated upstream documentation commit.

[github-actions]

🌿 Fern Docs Preview: https://nvidia-preview-019fd481-4fb6-73eb-bff7-02297dfa6aca.docs.buildwithfern.com/dynamo/dev

[devin-ai-integration]

Devin Review found 5 potential issues.

[coderabbitai]

Walkthrough

The vLLM sidecar now uses separate Inference, Control, and health gRPC services. It discovers model and server metadata at startup, supports multimodal requests and KV event sources, removes local model-path configuration, and updates mock services, launches, tests, and documentation.

Changes

vLLM split API integration

Layer / File(s)	Summary
Split vLLM protocol contracts lib/sidecar/vllm/proto/*, lib/sidecar/vllm/build.rs, .pre-commit-config.yaml, .github/workflows/copyright-check.ps1	The protocol now defines separate Inference and Control services. It adds model, server, KV-event, multimodal, session, rank, and encoder-cache fields.
Mock Control, Inference, and health services lib/mocker/servers/vllm/*	The mock server registers split services and health reporting. It publishes metadata, supports cancellation and KV discovery, and updates RPC tests.
Health-gated startup and model discovery lib/sidecar/vllm/src/client.rs, lib/sidecar/vllm/src/model.rs, lib/sidecar/vllm/src/engine.rs	Startup waits for service health, discovers model and server metadata, validates identity and capabilities, applies shared deadlines, and configures the engine from observed values.
Multimodal inference and cancellation flow lib/sidecar/vllm/src/convert.rs, lib/sidecar/vllm/src/engine.rs	Request conversion forwards media, ranks, cache salts, and expanded prompt tokens. Generation validates model capabilities and coordinates transfer-safe cancellation.
Launch integration and validation coverage lib/sidecar/vllm/launch/*, lib/sidecar/vllm/README.md, docs/fern/.../sidecar.md, lib/sidecar/vllm/src/tests.rs, lib/sidecar/vllm/tests/executable.rs	Launch commands no longer pass --model-path. Documentation describes split services and deferred Kubernetes deployment. Tests cover discovery, health, routing, multimodal handoff, deadlines, and cancellation.

Estimated code review effort: 4 (Complex) | ~60 minutes

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 27.17% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check	⚠️ Warning	The description explains the implementation and validation, but it omits the required Related Issues section and checkbox or issue reference.	Add the required Related Issues section and select either the applicable issue-closing option or the confirmation that no related issue exists.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly identifies the vLLM sidecar multimodal request support added by the pull request.

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 6

🧹 Nitpick comments (4)

lib/sidecar/vllm/src/model.rs (1)

107-118: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win

Return the trimmed value from required and nonempty.

Both helpers test value.trim().is_empty() but return the original string. A served_model_name of " llama " therefore passes validation and is used verbatim. engine.rs copies that value into proto_request.model at line 206 and into WorkerConfig.served_model_name at line 121, so the surrounding whitespace becomes part of the routing key and of the model name sent back to vLLM.

♻️ Proposed change

 fn required(field: &str, value: String) -> Result<String, DynamoError> {
-    if value.trim().is_empty() {
+    let value = value.trim();
+    if value.is_empty() {
         return Err(client::protocol_error(format!(
             "Control returned an empty {field}"
         )));
     }
-    Ok(value)
+    Ok(value.to_string())
 }
 
 fn nonempty(value: String) -> Option<String> {
-    (!value.trim().is_empty()).then_some(value)
+    let value = value.trim();
+    (!value.is_empty()).then(|| value.to_string())
 }

🤖 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 `@lib/sidecar/vllm/src/model.rs` around lines 107 - 118, Update the `required`
and `nonempty` helpers to return the trimmed input value after validating it,
while preserving their existing error and `Option` behavior for blank strings.
Ensure callers such as `proto_request.model` and
`WorkerConfig.served_model_name` receive the normalized value without
surrounding whitespace.

lib/sidecar/vllm/src/convert.rs (1)

16-17: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy lift

Centralize the cache-salt prefix in a shared protocol crate.

The two crates define separate copies of this protocol value. A prefix change can produce incompatible cache salts while both crates compile. Move the value to a common dependency and import it from there.

🤖 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 `@lib/sidecar/vllm/src/convert.rs` around lines 16 - 17, Remove the local
DYNAMO_CACHE_SALT_PREFIX definition in convert.rs and source the prefix from a
shared protocol crate instead. Add or use the appropriate common-crate
dependency and import its canonical cache-salt prefix, ensuring the existing
cache-salt construction continues using that shared symbol.

lib/sidecar/vllm/src/tests.rs (2)

1087-1096: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider asserting that Control Abort is never called.

lib/sidecar/vllm/README.md line 76 states that the sidecar does not call the Control Abort RPC and relies on stream drop instead. The fake implements abort at lines 216-221, but no test observes it. A counter on FakeVllm would pin that documented contract and catch a future change that starts sending Abort.

♻️ Proposed test addition

 struct FakeVllm {
     requests: Arc<Mutex<Vec<pb::GenerateRequest>>>,
+    aborts: Arc<AtomicUsize>,

     async fn abort(
         &self,
         _request: Request<pb::AbortRequest>,
     ) -> Result<Response<pb::AbortResponse>, Status> {
+        self.aborts.fetch_add(1, Ordering::SeqCst);
         Ok(Response::new(pb::AbortResponse {}))
     }

Then assert server.service.aborts.load(Ordering::SeqCst) == 0 after the cancellation completes.

🤖 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 `@lib/sidecar/vllm/src/tests.rs` around lines 1087 - 1096, Update the FakeVllm
test fixture and decode_cancellation_waits_for_submission_and_first_token to
track Control Abort invocations with an atomic counter, increment it in the fake
abort implementation, and assert the counter remains zero after cancellation
completes, preserving the sidecar’s stream-drop-only cancellation contract.

---

94-98: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Name the expanded multimodal prompt token count.

The literal 601 encodes "expanded multimodal prompt length" and appears again at lines 728, 765, 780, and 798. A named constant states the intent and keeps the fake and its assertions aligned when the value changes.

♻️ Proposed refactor

+/// Prompt length the fake reports once media expands the prompt.
+const EXPANDED_MULTIMODAL_PROMPT_TOKENS: u32 = 601;
+
 let prompt_tokens = if request.media.is_empty() {
     prompt_tokens
 } else {
-    601
+    EXPANDED_MULTIMODAL_PROMPT_TOKENS
 };

🤖 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 `@lib/sidecar/vllm/src/tests.rs` around lines 94 - 98, Replace the repeated
multimodal prompt token literal 601 with a clearly named shared constant
representing the expanded multimodal prompt length, including the assignment in
the shown test and the references near lines 728, 765, 780, and 798. Reuse that
constant in the fake and its assertions so all related expectations remain
aligned.

🤖 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.

Inline comments:
In `@lib/mocker/servers/vllm/src/server.rs`:
- Around line 279-288: Ensure cancellable requests retain an addressable
caller-supplied ID by validating that GenerateRequest.request_id is non-empty
before scheduling, rather than allowing PreparedRequest::new to replace it with
a random UUID. Update the Generate request handling path and return an
appropriate invalid-argument error for empty IDs, preserving normal scheduling
and Abort’s stable_uuid lookup for valid IDs.
- Line 134: Update the mock server’s explicit data-parallel rank handling:
either validate GenerateRequest.data_parallel_rank against DP_RANK and reject
mismatches, or set supports_explicit_data_parallel_rank to false. Preserve
rank-zero scheduling only for matching requests, since convert.rs forwards the
routed rank into this field.

In `@lib/sidecar/vllm/proto/control.proto`:
- Around line 14-16: The vendored Control protocol must match the pinned source
byte-for-byte: replace lib/sidecar/vllm/proto/control.proto lines 14-16 with the
verbatim pinned content, removing the extra blank line between
GetServerInfoRequest and ServerInfo. In lib/sidecar/vllm/proto/README.md lines
8-13, retain the unmodified-copy statement and checksum only after verifying
they match the corrected checked-in bytes.

In `@lib/sidecar/vllm/src/engine.rs`:
- Around line 100-107: Update VllmSidecarEngine construction and start flow to
retain the configured startup Duration rather than the absolute deadline
computed during argument parsing. Keep bootstrap_discover bounded by its
separately derived deadline, then have VllmSidecarEngine::start derive a fresh
deadline via client::startup_deadline(self.transport.startup_deadline) and use
it for connect, wait_for_services, and discover.
- Around line 238-243: Update the request_cancelled branch in the engine’s
stream-message handling to wrap stream.message() in a bounded timeout while
draining deferred cancellation. If the timeout expires, drop the gRPC stream and
continue by emitting the cancelled result; preserve the existing
shutdown-cancellation race behavior.

In `@lib/sidecar/vllm/src/model.rs`:
- Around line 59-65: Normalize model aliases before constructing ModelIdentity
so equivalent alias sets compare consistently across RPC responses. In the
identity construction block, sort model.served_model_aliases and assign the
normalized order to aliases while preserving the existing ModelIdentity fields
and ensure_same_identity behavior.

---

Nitpick comments:
In `@lib/sidecar/vllm/src/convert.rs`:
- Around line 16-17: Remove the local DYNAMO_CACHE_SALT_PREFIX definition in
convert.rs and source the prefix from a shared protocol crate instead. Add or
use the appropriate common-crate dependency and import its canonical cache-salt
prefix, ensuring the existing cache-salt construction continues using that
shared symbol.

In `@lib/sidecar/vllm/src/model.rs`:
- Around line 107-118: Update the `required` and `nonempty` helpers to return
the trimmed input value after validating it, while preserving their existing
error and `Option` behavior for blank strings. Ensure callers such as
`proto_request.model` and `WorkerConfig.served_model_name` receive the
normalized value without surrounding whitespace.

In `@lib/sidecar/vllm/src/tests.rs`:
- Around line 1087-1096: Update the FakeVllm test fixture and
decode_cancellation_waits_for_submission_and_first_token to track Control Abort
invocations with an atomic counter, increment it in the fake abort
implementation, and assert the counter remains zero after cancellation
completes, preserving the sidecar’s stream-drop-only cancellation contract.
- Around line 94-98: Replace the repeated multimodal prompt token literal 601
with a clearly named shared constant representing the expanded multimodal prompt
length, including the assignment in the shown test and the references near lines
728, 765, 780, and 798. Reuse that constant in the fake and its assertions so
all related expectations remain aligned.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 4ceb21e6-8a5e-4a78-a8df-105f48e668f2

📥 Commits

Reviewing files that changed from the base of the PR and between a738e7c and 33dad93.

⛔ Files ignored due to path filters (1)

• Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (29)

• .github/workflows/copyright-check.ps1
• .pre-commit-config.yaml
• docs/fern/pages/developer-guide/knowledge-base/modular-components/backends/vllm/sidecar.md
• lib/backend-common/src/lib.rs
• lib/mocker/servers/vllm/Cargo.toml
• lib/mocker/servers/vllm/README.md
• lib/mocker/servers/vllm/src/main.rs
• lib/mocker/servers/vllm/src/server.rs
• lib/mocker/servers/vllm/src/server_request.rs
• lib/mocker/servers/vllm/src/server_tests.rs
• lib/mocker/servers/vllm/tests/sidecar.rs
• lib/sidecar/vllm/Cargo.toml
• lib/sidecar/vllm/Dockerfile
• lib/sidecar/vllm/README.md
• lib/sidecar/vllm/build.rs
• lib/sidecar/vllm/deploy/agg.yaml
• lib/sidecar/vllm/deploy/disagg.yaml
• lib/sidecar/vllm/launch/agg.sh
• lib/sidecar/vllm/launch/disagg.sh
• lib/sidecar/vllm/proto/README.md
• lib/sidecar/vllm/proto/control.proto
• lib/sidecar/vllm/proto/inference.proto
• lib/sidecar/vllm/src/args.rs
• lib/sidecar/vllm/src/client.rs
• lib/sidecar/vllm/src/convert.rs
• lib/sidecar/vllm/src/engine.rs
• lib/sidecar/vllm/src/model.rs
• lib/sidecar/vllm/src/tests.rs
• lib/sidecar/vllm/tests/executable.rs

💤 Files with no reviewable changes (4)

• lib/sidecar/vllm/deploy/disagg.yaml
• lib/sidecar/vllm/deploy/agg.yaml
• lib/sidecar/vllm/src/args.rs
• lib/sidecar/vllm/launch/disagg.sh

[tanmayv25]

Automated multi-agent code-review pass over this PR's diff (framing + expert lenses, each finding adversarially verified). Findings below are all minor / non-blocking.

[furionw]

Do we have e2e test for sidecar agg, P/D in general?

can we also have ones for multimodal agg and multimodal P/D ?

[connorcarpenter15]

We don’t have any tests for sidecar yet (including E2E). We have some launch scripts and deploy examples, but only for regular agg/disagg. CI tests and more extensive launch examples will be added prior to 1.5.0 once we work it out with ops. We will add multimodal support at that time.

[furionw]

Thank you

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[connorcarpenter15]

/ok to test e5f0c8e

[connorcarpenter15]

/ok to test 9b0962b