Skip to content

[BugFix] Add field constraints to /v1/omni/sleep and /v1/omni/wakeup - #4740

Merged
alex-jw-brooks merged 4 commits into
vllm-project:mainfrom
redhat-et:fix/sleep-wakeup-validation
Sep 28, 2026
Merged

alex-jw-brooks merged 4 commits into
vllm-project:mainfrom
redhat-et:fix/sleep-wakeup-validation

Conversation

@Shaun-Walsh

Copy link
Copy Markdown
Contributor

Summary

  • Add min_length=1 to stage_ids on both OmniSleepRequest and OmniWakeupRequest to reject empty lists
  • Add ge=0 to OmniSleepRequest.level to reject negative sleep levels
  • Unskip sleep_empty_stage_ids, sleep_negative_level, and wakeup_empty_stage_ids tests

Partial fix for #3649.

Test plan

  • Verified validators locally with Pydantic v2 — invalid inputs rejected, valid inputs pass
  • CI passes on existing test suite
  • Live validation with Qwen/Qwen-Image (requires H100)

…requests

Reject empty stage_ids lists (min_length=1) and negative sleep levels
(ge=0) at the entrypoint level. Unskip sleep_empty_stage_ids,
sleep_negative_level, and wakeup_empty_stage_ids tests.

Partial fix for vllm-project#3649.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Shaun Walsh <shaunwalsh24@gmail.com>
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

@NickCao

NickCao commented Jun 29, 2026

Copy link
Copy Markdown
Collaborator

Claude:

The min_length=1 constraint on stage_ids will break two existing unit tests in tests/entrypoints/openai_api/test_omni_sleep_wakeup.py:

  • test_sleep_empty_stage_ids — expects stage_ids: [] to return 200 with {"status": "SUCCESS", "acks": []}
  • test_wakeup_empty_stage_ids — expects stage_ids: [] to return 200 with {"status": "SKIPPED"}

With min_length=1, Pydantic rejects empty lists with 422 before the handler runs, so both tests fail with assert 422 == 200.

These tests need to be updated to expect 422, or removed since the dfx tests in test_invalid_server_control.py now cover the same validation.

@Shaun-Walsh

Copy link
Copy Markdown
Contributor Author

Already fixed — updated both tests to expect 422 in aa309ae.

@hsliuustc0106 hsliuustc0106 added bug Something isn't working enhancement New feature or request omni code related to omni models labels Jul 17, 2026 — with ChatGPT Codex Connector
The min_length=1 constraint on stage_ids rejects empty lists at the
Pydantic layer (422) before the handler runs. Update tests to match.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Shaun Walsh <shaunwalsh24@gmail.com>
@Shaun-Walsh
Shaun-Walsh force-pushed the fix/sleep-wakeup-validation branch from aa309ae to ae717f3 Compare July 21, 2026 13:50
@Shaun-Walsh

Copy link
Copy Markdown
Contributor Author

Hi — just pushed a fix for the failing check. This PR (along with a few related ones from our team) is a dependency for the Red Hat Midstream build of vllm-omni. Could we get a review and merge when you have a chance? Happy to address any feedback quickly. Thanks!

@vllm-omni-review-bot

Copy link
Copy Markdown

Omni ReviewBot: no human activity for 48 days

@Shaun-Walsh this pull request has had no human commit, comment or review since 2026-07-21. Per repository policy it may be closed if it stays inactive.

To keep it moving, any one of these is enough: push an update, reply to the open blocker, or post the current plan and timeline.

@Shaun-Walsh

Copy link
Copy Markdown
Contributor Author

Hi @alex-jw-brooks & @yenuo26 just flagging this PR for review when you get a chance.

@yenuo26

yenuo26 commented Sep 17, 2026

Copy link
Copy Markdown
Collaborator

@Shaun-Walsh please fix conflicts

…lidation

Signed-off-by: Shaun Walsh <shaunwalsh24@gmail.com>

# Conflicts:
#	tests/dfx/reliability/invalid_param_test/test_invalid_server_control.py
#	vllm_omni/entrypoints/openai/api_server.py
@Shaun-Walsh

Copy link
Copy Markdown
Contributor Author

Hey @yenuo26 conflicts resolved :)

@vllm-omni-review-bot

Copy link
Copy Markdown

Omni ReviewBot: no human activity for 7 days

@Shaun-Walsh this pull request has had no human commit, comment or review since 2026-09-21. Please confirm the current plan and next step. The author or a maintainer decides whether to change the PR state.

To keep it moving, any one of these is enough: push an update, reply to the open blocker, or post the current plan and timeline.

@Shaun-Walsh

Copy link
Copy Markdown
Contributor Author

Hi @NickCao @yenuo26 @alex-jw-brooks @tzhouam @linyueqian looking to get this merged so I don't have as many pr's open per the new guidelines :)

@alex-jw-brooks alex-jw-brooks left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM, thanks

@alex-jw-brooks alex-jw-brooks added the ready label to trigger buildkite CI label Sep 28, 2026
@alex-jw-brooks
alex-jw-brooks merged commit e823f0d into vllm-project:main Sep 28, 2026
7 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working enhancement New feature or request omni code related to omni models ready label to trigger buildkite CI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Summary of Parameter Issues in Entrypoints Interface Exceptions

6 participants