Repository navigation
[CI/Build] Re-enable turn-based server VAD realtime test - #9300
Conversation
e840112 to
3b4e104
Compare
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
This PR was classified as CI work. Routing: @yenuo26 via CI owner, CODEOWNERS; @congw729 via CODEOWNERS @NickCao, please review your own changes and leave a short self-review comment describing what you checked. PRs without author self-review may not be assigned a reviewer. Please take a look when you have a chance. If you would like an automated review, mention @vllm-omni-review-bot in a comment. |
Omni ReviewBot triage noteAutomated triage of commit
These are automated triage suggestions only — the final decision belongs to the maintainers. |
hsliuustc0106
left a comment
There was a problem hiding this comment.
@NickCao — evidence TODO for 3b4e104a5dae7dcc73a6f5350af51a3b6e61352f (base cbe4cc4b7c1126a903553f8e79e44ddfab782cf3).
- Please attach the verbose result for
TestQwen3OmniRealtimeWebSocket::test_server_vad_multi_turn_without_client_commitat this head, with GPU/model/runtime details and the effective session settings plus terminal-event summary for both turns. The supplied25 passedaggregate does not identify whether this newly enabled test executed. - Acceptance: the 24 kHz PCM session completes two distinct automatically triggered responses without a client commit, retaining the existing nonempty-audio, unique-ID and terminal-event assertions. A focused negative control (for example, disabling automatic response) would also establish that the test catches the behavior it is intended to guard.
I inspected the two-file change and related audio/protocol handling, but did not execute the test. This is an evidence request, not a code-defect finding; current-head CUDA readiness is still unverified.
The full log is at https://gist.github.com/NickCao/8f0f431ec1e45bf0841ab7eeaf3ee6d1: |
3b4e104 to
0ba262c
Compare
0ba262c to
0be3109
Compare
alex-jw-brooks
left a comment
There was a problem hiding this comment.
Small nit, but otherwise lgtm
0be3109 to
dfad251
Compare
dfad251 to
958fed7
Compare
Turn-based server VAD is implemented, so stop deselecting the Qwen3-Omni multi-turn E2E. Update the test to the Realtime 24 kHz PCM contract and assert explicit interruptible server VAD settings. Co-authored-by: Codex <noreply@openai.com> Signed-off-by: Nick Cao <ncao@redhat.com>
958fed7 to
a9c9424
Compare
|
Re-tested |
Turn-based server VAD is implemented, so stop deselecting the Qwen3-Omni multi-turn E2E. Update the test to the Realtime 24 kHz PCM contract and assert explicit interruptible server VAD settings.
Contributor intent (required human answers)
All four questions must be answered by the human contributor before submission.
An agent may reuse explicit human answers with a public-safe source reference;
it must not invent, infer, or prefill missing answers. If an answer is missing
or materially ambiguous, ask the human and wait. Do not publish private
conversation content without authorization.
1. What outcome do you want to achieve?
Re-enable the turn-based server VAD realtime test.
2. What is the highest-leverage variable for that outcome?
Drop the pytest deselect and set proper turn_detection parameters.
3. How will this work move that variable?
Drop the pytest deselect and set proper turn_detection parameters, as well as fix the incompatible sample rate.
4. Whose customer or user needs does this address, and what are those needs?
Developer can now easily test the turn-based server VAD realtime behavior end to end.
Purpose
Previous the turn-based server VAD realtime test was disabled as server vad support was removed during refactoring, re-enable that since this has been reimplemented.
Test Plan
vLLM Version: 0.32.0
vLLM-Omni Commit: 3b4e104
Test Result
25 passed, 2893 deselected, 36 warnings in 1291.99s (0:21:31)
BEFORE SUBMITTING: read CONTRIBUTING.md and run the precheck-pr skill with the code agent for a self-check against project conventions.
Note
To avoid agent spam, vLLM-Omni limits contributors without write access to 5 open PRs. If this blocks well-intentioned critical work, please contact a committer to request bypass list access.
(anything written below this line will be removed by GitHub Actions)