Repository navigation
[Bugfix] Fix Qwen3-Omni realtime playback interruption and conversation history - #7791
Conversation
…on history Signed-off-by: psv666 <2693925048@qq.com>
|
This PR appears to belong to: docs/design/module/engine_orchestration.md, docs/design/module/observability.md. Module owners: @tzhouam @fake0fan @lishunyang12 Routing: @tzhouam via module of the changed files, CODEOWNERS; @fake0fan via module of the changed files; @lishunyang12 via module of the changed files @psv666, 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. |
amy-why-3459
left a comment
There was a problem hiding this comment.
I did not find a confirmed blocking correctness issue in this revision. The interruption path, late-response isolation, preservation of queued newer-response events, and identity-based history removal look reasonable. Two non-blocking suggestions are attached inline.
Validation performed: browser/playback-worklet regressions (31 passed), and targeted event projection, EngineSession, and Qwen duplex plugin regressions (140 passed). I did not rerun the real-checkpoint GPU tests; the hardware results in the PR description are author-provided evidence.
| let echoTimer = null; | ||
| let sessionGeneration = 0; | ||
| let audioChain = Promise.resolve(); | ||
| const interruptedResponses = new Set(); |
There was a problem hiding this comment.
Non-blocking: could we document the lifetime of interruptedResponses, or consider a safe bounded-retention policy?
The set retains every interrupted response ID throughout a running session and is cleared when a new session starts. For a long-running call with repeated interruptions, this grows with the number of interruptions. This is a low-priority lifecycle concern, not a blocker for the playback fix.
Any cleanup must preserve the protection against queued late events and outstanding asynchronous decodes; removing an ID immediately on response.done would not necessarily be safe. A short lifecycle comment or a follow-up with a long-session regression would be useful.
There was a problem hiding this comment.
Documented in 11a1fd7. Interrupted response IDs are retained until startSession() clears the set, since queued events and pending audio decodes can outlive response.done. The comment also calls out growth during a call and how sessionGeneration rejects old-session decodes after the set is cleared.
| selected = [m for m in history[:-16] if has_image(m)] + history[-16:] | ||
| dropped_user = False | ||
| for message in selected: | ||
| if message.get("role") == "user": |
There was a problem hiding this comment.
Non-blocking test suggestion: please cover, or document why ordering rules exclude, a camera image inserted between an evicted audio input and its assistant reply:
evicted user audio -> user camera image -> assistant reply -> retained user audio
Every user message resets dropped_user, including camera image messages. A test would clarify which input owns the assistant reply in this ordering and whether it should survive audio eviction.
I have not established that this ordering causes a reachable correctness bug, so this is a request to make the ordering/ownership invariant explicit rather than a request to change the algorithm without evidence.
There was a problem hiding this comment.
Added a session regression that commits audio with create_response=False, inserts a camera image, then creates the response. It checks that the image and audio share a turn and that, after the audio-history limit is reached, that turn's image and assistant reply are evicted together. A separate byte-budget test covers the same ordering with an image-only item.
The adapter now groups and prunes complete turns before format_history() merges user items. Empty assistant boundaries and individually addressable source items are preserved. The ownership rule is documented in realtime_duplex_api.md (11a1fd7).
|
Please resolve conflicts |
Signed-off-by: psv666 <2693925048@qq.com>
Signed-off-by: psv666 <2693925048@qq.com>
|
@Gaohan123 Resolved in a9c96d9 by merging Validation of the merged tree: 420 backend duplex/Qwen tests and 31 browser/playback-worklet tests passed. Pre-commit passed, including Python 3.10 mypy. |
Brings in vllm-project#7791 (Qwen3-Omni realtime playback interruption fix), whose new tests use the pre-refactor module paths and to_realtime(); resolved by repointing them at vllm_omni.protocol.duplex / engine.duplex.projection and to_wire(). Also fixes the stale DuplexCommand.payload() mention in docs/design/module/entrypoints.md. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: chickeyton <ngton2014@gmail.com>
…on history (vllm-project#7791) Signed-off-by: psv666 <2693925048@qq.com> Signed-off-by: Matthieu Laneuville <matthieu.laneuville@surf.nl>
Integrate upstream main after vllm-project#7791 superseded the original completed-clear resolver. Keep that resolver unchanged, cap discarded history against late ACKs, and enqueue already-requested retained input only after its cancellation fence succeeds. The complete source tree matches measured candidate 25763aef99d2eea2fb1bfa8384cad080e2646d72 (859 passed, 1 skipped; all five real-Qwen clear scenarios passed). Preserve the existing published branch history. Signed-off-by: Nolen Liang <nliang@nvidia.com>
…on history (vllm-project#7791) Signed-off-by: psv666 <2693925048@qq.com>
Purpose
Qwen3-Omni realtime audio could keep playing after the user started speaking, including when generation had already completed. Cancellation could also wait behind an outstanding audio decode. Stop interrupted playback immediately, discard late output from the old response, and preserve queued events for the next response.
Forward clears for the latest completed response to the engine so its playback cursor is released before the next input; late clears for older responses cannot interrupt a newer turn. Preserve the assistant turn position while unaligned audio awaits playback acknowledgement, remove history items by identity, and evict assistant replies together with discarded input audio.
Follow-up to #7585. The default voice prompt and protocol migration are outside this change.
Test Plan
tests/engine/duplex/andtests/model_executor/models/qwen3_omni/test_duplex_plugin.py.node --test tests/examples/test_realtime_web_profiles.cjs, including actual playback-worklet clearing and asynchronous decode ordering.qwen3_omni_duplex.yaml, Silero VAD, checked-in speech and synthetic camera image: reuse one deployment for 20 interruptions during generation and 20 after generation completes with audio still queued.The original six-turn fixture withheld every playback ACK and required nonempty generation from repeated audio plus consecutive empty assistant turns. A raw Thinker trace showed only EOS (token 151645) on turn three, before event projection. The corrected fixture acknowledges completed playback and checks paired eviction of real answers; output assertions remain enabled.
vLLM Version: 0.29.0; PyTorch 2.13.0+cu129; Transformers 5.14.1.
vLLM-Omni Commit:
2b987db3b7cb216c3eb5ac81cf29e21478d439eb(base6df62d51573ca5be7826321ba2de1cd071d997f8).Test Result
GPU results use two NVIDIA L20X GPUs, a local substitute for CI hardware. They do not establish H100/L4 equivalence. No performance claim is made.
Targeted GPU invocation (report paths omitted):
The interruption run selected the whole
TestQwen3OmniDuplexPlaybackclass with the default 20 repetitions, on one deployment. Its original history case failed as described above; the corrected history case was then rerun independently. Test weights and VAD were supplied from local caches through the existing test environment variables.Additional local ready/merge CI attempts and remaining gaps
The selected matrix contains 24 deduplicated tasks. It was not completed; nightly/weekly were not run. Results below are the latest attempts, not an all-CI-pass claim.
SystemExit: 15); no completed testMissing-dependency skips (including FlashAttention, torchao, boogu and cosmos), incomplete tasks and hardware-specific paths are not counted as verified. Local prerequisite overrides used isolated temporary directories; no dependency or CI configuration changes are included in this PR. Full upstream CI remains necessary.
AI assistance: Used Codex to review changes, add regression coverage, diagnose the history fixture, run validation, and prepare this description.