Skip to content

test(vllm): fix omni 0.23 contract guards (regressed by #11374) #11516

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
dagil-nvidia merged 1 commit into from
Jul 10, 2026
+18 −3
Merged

test(vllm): fix omni 0.23 contract guards (regressed by #11374) #11516

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 3 additions & 0 deletions components/src/dynamo/vllm/tests/omni/test_omni_base_handler.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@
"sequence_parallel_size",
"enable_expert_parallel",
"ulysses_mode",
# Diffusion sequence-parallel mask padding; internal SP detail, defaults False
# and is not a benchmark-facing knob (same rationale as sequence_parallel_size).
"mask_sp_padding",
}


Expand Down
12 changes: 11 additions & 1 deletion components/src/dynamo/vllm/tests/test_vllm_api_contract.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,17 @@ def test_request_exposes_all_token_ids():
private attribute so a rename is caught here, not at runtime."""
from vllm.v1.request import Request

assert "_all_token_ids" in inspect.getsource(Request), (
# vllm-omni monkeypatches vllm.v1.request.Request with its OmniRequest subclass,
# whose body does not redeclare `_all_token_ids` (it inherits it). Walk the MRO so
# the inherited attribute is still detected, while a real rename in vLLM still fails.
sources = []
for klass in Request.__mro__:
try:
sources.append(inspect.getsource(klass))
except (OSError, TypeError):
continue

assert any("_all_token_ids" in src for src in sources), (
"vllm.v1.request.Request no longer exposes `_all_token_ids` — "
"InstrumentedScheduler relies on it for NewRequestData.prefill_token_ids."
)
6 changes: 4 additions & 2 deletions components/src/dynamo/vllm/tests/test_vllm_renderer_api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -431,9 +431,11 @@ def test_engine_core_struct_contract(self):
"routed_experts",
"num_nans_in_logits",
)
# vllm-omni extends EngineCoreOutput with streaming segment metadata
# (only installed on amd64, not arm64).
# vllm-omni's bundled vLLM adds `multimodal_output`, and vllm-omni extends
# EngineCoreOutput with streaming segment metadata (only installed on amd64,
# not arm64).
omni_output_extra_fields = (
"multimodal_output",
"is_segment_finished",
"new_prompt_len_snapshot",
)
Comment on lines 437 to 441

@devin-ai-integration devin-ai-integration Bot Jul 10, 2026

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.

🔍 vllm_processor.py does not forward multimodal_output to EngineCoreOutput

The new multimodal_output field is added to the valid omni output field variants in this test, but components/src/dynamo/frontend/vllm_processor.py:666-674 only dynamically forwards is_segment_finished and new_prompt_len_snapshot when constructing EngineCoreOutput. It does not check for or forward multimodal_output. If the downstream OutputProcessor or omni formatters (like AudioFormatter at components/src/dynamo/vllm/omni/output_formatter.py:280) expect multimodal_output to be populated on EngineCoreOutput, it would be None/default when constructed by vllm_processor.py. This may be intentional if multimodal_output is only populated by the omni engine path (not the disaggregated router path through vllm_processor.py), but worth confirming.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Expand Down
Loading