Skip to content

test(vllm): accept upstream validation of asymmetric features - #15373

Closed
bzsuni wants to merge 1 commit into
ai-dynamo:mainfrom
bzsuni:fix/vllm-asymmetric-feature-validation
Closed

bzsuni wants to merge 1 commit into
ai-dynamo:mainfrom
bzsuni:fix/vllm-asymmetric-feature-validation

Conversation

@bzsuni

@bzsuni bzsuni commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Overview:

Update the TITO asymmetric-feature validation tests for vLLM 0.30
Related PRs: #15179 #15182

vLLM 0.30 rejects mismatched multimodal keys during GenerateRequest
validation, before reaching Dynamo's image-feature validation. These inputs
therefore raise a Pydantic ValidationError instead of Dynamo's TypeError

Details:

Allow either validation path while still matching the specific rejection
messages. The same malformed inputs must still be rejected

No production behavior changes.

Observed in nightly CI

Where should the reviewer start?

test_tito_adapter_rejects_asymmetric_image_feature_objects() in
components/src/dynamo/vllm/tests/test_vllm_engine_generate.py

Validation

Black, isort, and git diff --check passed

Related Issues

  • Confirmed — no related issue

Summary by CodeRabbit

  • Tests
    • Updated validation coverage to accept either supported error type and the applicable validation message for asymmetric image-feature inputs.

Signed-off-by: bzsuni <bingzhe.sun@daocloud.io>
@bzsuni
bzsuni requested a review from a team as a code owner September 29, 2026 13:10
@copy-pr-bot

copy-pr-bot Bot commented Sep 29, 2026

Copy link
Copy Markdown

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.

@bzsuni
bzsuni deployed to external_collaborator September 29, 2026 13:11 — with GitHub Actions Active
@bzsuni
bzsuni deployed to external_collaborator September 29, 2026 13:11 — with GitHub Actions Active
@github-actions

Copy link
Copy Markdown
Contributor

👋 Hi bzsuni! Thank you for contributing to ai-dynamo/dynamo.

Just a reminder: The NVIDIA Test Github Validation CI runs an essential subset of the testing framework to quickly catch errors.Your PR reviewers may elect to test the changes comprehensively before approving your changes.

🚀

@github-actions github-actions Bot added test external-contribution Pull request is from an external contributor backend::vllm Relates to the vllm backend labels Sep 29, 2026
@coderabbitai

coderabbitai Bot commented Sep 29, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: ai-dynamo/dynamo/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 1ae3c772-8acf-474f-9178-92e6f3994a6b

📥 Commits

Reviewing files that changed from the base of the PR and between 8e41fd8 and 3b13f58.

📒 Files selected for processing (1)
  • components/src/dynamo/vllm/tests/test_vllm_engine_generate.py

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.


Walkthrough

The asymmetric image-feature validation test now accepts TypeError or Pydantic’s ValidationError and matches either of two validation messages.

Changes

Image-feature validation

Layer / File(s) Summary
Update validation test expectation
components/src/dynamo/vllm/tests/test_vllm_engine_generate.py
The test imports ValidationError and accepts it or TypeError. It matches either the existing list-validation error or the modality-mismatch error. A comment notes that vLLM 0.30 validates modality keys before Dynamo’s adapter.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~5 minutes

Merge Risk: ⚪ Minimal · up to 3b13f

This updates a single validation test to accept vLLM 0.30's earlier rejection of mismatched multimodal keys. It has no production behavior impact and is low risk to merge.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the test update to accept upstream validation of asymmetric features.
Description check ✅ Passed The description includes the required overview, details, reviewer starting point, and related-issues sections. It explains the vLLM 0.30 validation change, test impact, and validation performed.
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.
  • Fix all pre-merge checks with AI

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

@KrishnanPrash

Copy link
Copy Markdown
Contributor

/ok to test 3b13f58

@karen-sy

Copy link
Copy Markdown
Contributor

Thanks for the contribution! #15380 just merged for the same fix, so we are closing it as duplicate.

@karen-sy karen-sy closed this Sep 29, 2026

This branch was successfully deployed

1 active deployment
external_collaborator — 3b13f589 Deployed Sep 29, 2026 by bzsuni via ok-to-test #23628
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend::vllm Relates to the vllm backend external-contribution Pull request is from an external contributor size/S test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants