[Bugfix] diffusion end points allow model mismatch#2805
[Bugfix] diffusion end points allow model mismatch#2805lishunyang12 merged 7 commits intovllm-project:mainfrom
Conversation
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
4c0aa85 to
d071f32
Compare
Signed-off-by: xiaohajiayou <923390377@qq.com>
d071f32 to
ca5dda2
Compare
lishunyang12
left a comment
There was a problem hiding this comment.
Review Summary
This PR correctly changes three diffusion endpoints from logging a warning on model mismatch to raising HTTPException(400). The change aligns these endpoints with the existing behavior in serving_chat.py and serving_speech.py, and with what the API docs already promise.
Correctness
The logic is straightforward and correct across all three sites:
/v1/images/generations- Replaceslogger.warning+ silent fallback withraise HTTPException(400). The stale comment# Validate model field (warn if mismatch, don't error)is properly removed./v1/images/edits- Same pattern, same fix._parse_video_form- The raise happens inside atryblock that already hasexcept HTTPException: raise, so the new exception propagates correctly without being swallowed by the genericexcept Exceptionhandler below it.
Error messages are clear and consistent across all three sites. Using HTTPStatus.BAD_REQUEST.value is consistent with the rest of the file.
Tests
Test coverage looks good: one test per endpoint (test_generate_images_rejects_model_mismatch, test_image_edit_rejects_model_mismatch, test_video_generation_rejects_model_mismatch), each asserting both the 400 status code and the presence of "model mismatch" in the error detail. The assertions use .lower() which makes them resilient to casing changes.
Minor Observations (non-blocking)
- The parentheses around the f-string in
detail=(f"Model mismatch: ...")are unnecessary (single expression, not a tuple), but this is purely cosmetic and not worth a revision.
LGTM. Clean, minimal, well-tested fix that resolves a real behavioral inconsistency.
There was a problem hiding this comment.
Pull request overview
Fixes #2804 by making diffusion-oriented OpenAI-compatible endpoints reject requests whose model does not match the server’s served model, aligning behavior with existing chat/speech paths and the image API docs (400 on invalid params) instead of silently falling back.
Changes:
/v1/images/generations: raise HTTP 400 onrequest.modelmismatch vs served model./v1/images/edits: raise HTTP 400 onmodelform field mismatch vs served model.- Video form parsing for
/v1/videos+/v1/videos/sync: raise HTTP 400 onrequest.modelmismatch. - Added regression tests asserting 400 + “model mismatch” detail for image + video endpoints.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
vllm_omni/entrypoints/openai/api_server.py |
Enforces model mismatch validation (HTTP 400) for image generation/edit and video request parsing. |
tests/entrypoints/openai_api/test_video_server.py |
Adds test ensuring video generation rejects model mismatch with HTTP 400. |
tests/entrypoints/openai_api/test_image_server.py |
Adds tests ensuring image generation and image edits reject model mismatch with HTTP 400. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Samit <285365963@qq.com>
Signed-off-by: xiaohajiayou <923390377@qq.com>
|
@lishunyang12 @hsliuustc0106 Could you please take a look if we can merge this? |
Signed-off-by: xiaohajiayou <923390377@qq.com> Signed-off-by: Samit <285365963@qq.com> Co-authored-by: Samit <285365963@qq.com> Co-authored-by: SYLAR <125541396+lishunyang12@users.noreply.github.com>
Signed-off-by: xiaohajiayou <923390377@qq.com> Signed-off-by: Samit <285365963@qq.com> Co-authored-by: Samit <285365963@qq.com> Co-authored-by: SYLAR <125541396+lishunyang12@users.noreply.github.com>
Purpose
Fix #2804
Return HTTP 400 when the request
modeldoes not match the served model for:/v1/images/generations/v1/images/editsPreviously, these paths only logged a warning and silently used the server model, which is inconsistent with the documented error behavior for invalid parameters such as model mismatch.
This was inconsistent with other OpenAI-serving paths in
vllm-omni, where chat and speech already go through_check_model()instead of silently falling back to the server model:vllm-omni/vllm_omni/entrypoints/openai/serving_chat.py
Lines 154 to 157 in d071f32
vllm-omni/vllm_omni/entrypoints/openai/serving_speech.py
Lines 1628 to 1631 in d071f32
vllm-omni/vllm_omni/entrypoints/openai/serving_speech.py
Lines 1712 to 1714 in d071f32
It was also inconsistent with the image API docs, which already classify model mismatch as an invalid-parameter
400 Bad Requestcase:vllm-omni/docs/serving/image_generation_api.md
Lines 176 to 179 in d071f32
vllm-omni/docs/serving/image_edit_api.md
Lines 162 to 165 in d071f32
Changes
/v1/images/generationsto raiseHTTPException(400)on model mismatch/v1/images/editsto raiseHTTPException(400)on model mismatchHTTPException(400)on model mismatchEssential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model. Please runmkdocs serveto sync the documentation editions to./docs.BEFORE SUBMITTING, PLEASE READ https://github.com/vllm-project/vllm-omni/blob/main/CONTRIBUTING.md (anything written below this line will be removed by GitHub Actions)