Repository navigation
[Core] Make protocol/duplex the single duplex vocabulary; drop the engine copies (follow-up to #7640) - #7861
chickeyton wants to merge 12 commits into
Conversation
4e2129b to
6082cbe
Compare
|
This PR appears to belong to: docs/design/module/observability.md, docs/design/module/entrypoints.md. Module owners: @NickCao @alex-jw-brooks @linyueqian Routing: @NickCao via module named in the PR description, CODEOWNERS; @alex-jw-brooks via module named in the PR description; @linyueqian via module named in the PR description @chickeyton, 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. |
…gine copies Follow-up to vllm-project#7640. Three overlapping sets of command/event definitions (protocol/realtime, protocol/duplex, and commands.py / events.py / realtime_commands.py / realtime_events.py / audio.py in engine/duplex) collapse into two layers: * protocol/realtime: the OpenAI v1/realtime command and event classes and their helpers, unchanged. * protocol/duplex: DuplexCommand / DuplexEvent as aliases of RealtimeCommand / RealtimeEvent, the Tier 2 extensions, the Tier 3 duplex-only classes, and the wire decoder (decode_duplex_command, build_append_audio, DUPLEX_COMMAND_TYPES) moved in from the engine. Consumer capabilities are injected instead of imported, and the decoder raises RealtimeProtocolError; DuplexCommandError is removed. The engine keeps only its own representation under non-protocol names: engine/duplex/mailbox.py (DUPLEX_REALTIME_CAPABILITIES, command_from_realtime, mailbox_payload / mailbox_channel replacing the payload() methods and mailbox type on the removed parallel command classes) and engine/duplex/projection.py (renamed realtime_events.py). The epoch-filter sets move into session/emitter.py and duplex_response_format into config.py, their only consumers. The duplex engine, entrypoints and clients now import protocol.duplex only; tests and docs are repointed and the facade / single-source / import-boundary tests assert the new layout. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: chickeyton <ngton2014@gmail.com>
The method renders the socket payload, and the class already names that concept "wire" (wire_type, optional_wire_fields, _wire_fields). to_wire() completes the set and reads the same from RealtimeEvent and from its DuplexEvent alias, whose Tier 3 events travel on the same Realtime wire. Mechanical rename across code, tests and docs; no behaviour change. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: chickeyton <ngton2014@gmail.com>
The check-forbidden-imports pre-commit hook rejects the stdlib base64 module. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: chickeyton <ngton2014@gmail.com>
6082cbe to
2f887b9
Compare
precheck-pr report (self-check, full mode)Run with the repo's precheck-pr skill against Updated 2026-10-05 after merging main through #8489 (177 commits, 15 conflicts, all import repoints of the removed engine modules); previously updated 2026-09-23 after the vLLM 0.30.0 rebase (#7820). The first version of this report ran at Test evidence at this head
Notes
🤖 Generated with Claude Code |
mailbox_channel and mailbox_payload walked a command's MRO separately; one _for_class helper now serves both tables. DUPLEX_COMMAND_TYPES had no production caller: the one test that read it now derives the canonical wire types from the command classes themselves, so the vocabulary is not listed twice. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: chickeyton <ngton2014@gmail.com>
|
Follow-up to the precheck report above: both advisory warnings are now addressed in
Re-verified at this head: full duplex sweep on the L20X host 1161 passed, 0 failed, 4 skipped (same four 2-GPU skips); DCO, pre-commit and build 3.11/3.12 green. Updated verdict: 0 blocking | 0 warnings. 🤖 Generated with Claude Code |
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>
Resolves the modify/delete on engine/duplex/commands.py from vllm-project#7633 (AURA): the empty-audio (frames-only) append rendering moves into engine/duplex/mailbox.py, and the Tier 1 AppendAudio default main added already lives in protocol/realtime. runner.py keeps main's audio+video admission release with mailbox_payload(); helpers.py keeps main's new contracts import. main's new tests (test_input_transcribed.py, the engine-session additions) are repointed at protocol.duplex / engine.duplex.projection and to_wire(). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: chickeyton <ngton2014@gmail.com>
Resolves the modify/delete on engine/duplex/events.py (removed on this branch; main only touched its docstring), keeps main's TranscriptDelta Tier 2 tests and the response-metrics projection tests with the to_wire() spelling, and combines the metrics imports with the protocol TurnEvent import in session/engine_session.py. Signed-off-by: chickeyton <ngton2014@gmail.com> Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Omni ReviewBot: supersededThe CI failure noted on |
Omni ReviewBot: no human activity for 11 days@chickeyton this pull request has had no human commit, comment or review since 2026-09-20. 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. |
|
@chickeyton this PR is labeled Could you please take a look and push an update to get CI green? Once the checks pass we can proceed with review/merge. Thanks! |
Brings in 177 upstream commits (through vllm-project#8489). Conflicts were all the same shape: upstream imports from engine.duplex.events / .commands / .realtime_events, which this branch removed, so the merged files import the same names from protocol.duplex.events / protocol.duplex.commands / engine.duplex.projection, and to_realtime() reads to_wire(). - engine/duplex/commands.py stays deleted. Upstream's vllm-project#7985 change there (an explicit empty __slots__ on the engine DuplexCommand mixin for Python 3.10) has no counterpart here because DuplexCommand is the wire base itself; test_duplex_commands.py pins the single-slotted-base layout instead of the removed mixin test. - The new delivery.py (vllm-project#7644) and its tests are repointed the same way. - test_realtime_codec_single_source.py takes upstream's parametrized error-type test and drops the engine-commands subclass test, whose subject no longer exists. - docs/design/fullduplex.md keeps this branch's module tree plus the new delivery.py entry. Signed-off-by: chickeyton <ngton2014@gmail.com> Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Omni ReviewBot routing recordAssigned Strict on zcode (GLM-5.3-Flash) under experiment |
Omni ReviewBot attempt recordReview attempt ended as failed (step 'review' (agent.review_diff): unhandled error: RuntimeError: zcode exited 1 without a result event: statusCode: undefined } Error: Turn execution failed (traceId: 662d4d9c-9676-4c00-852c-936bda8cbb24) — check |
Omni ReviewBot attempt recordReview attempt ended as failed (step 'review' (agent.review_diff): unhandled error: RuntimeError: zcode exited 1 without a result event: statusCode: undefined } Error: Turn execution failed (traceId: e3c922a8-175e-419b-b2e1-f336849204f8) — check |
Omni ReviewBot attempt recordReview attempt ended as failed (step 'review' (agent.review_diff): unhandled error: RuntimeError: zcode exited 1 without a result event: statusCode: undefined } Error: Turn execution failed (traceId: 431c3f25-9a3b-4ce6-835c-a8edd0637588) — check |
vllm-omni-review-bot
left a comment
There was a problem hiding this comment.
Omni ReviewBot review
1 actionable finding(s).
CI at
455c077ce04e(2026-10-10T11:31:23.233250+00:00): required check(s) blocking:buildkite/vllm-omni(missing).
Note: The assigned review arm
strict/zcode/GLM-5.3-Flashcould not complete this review, so it was produced by the fallback armdirect/cursor/auto. It is excluded from the routing experiment.
Full review analysis
PR description
This change makes protocol/duplex the only duplex command and event vocabulary. DuplexCommand and DuplexEvent are aliases of the OpenAI realtime bases, the wire decoder lives there as decode_duplex_command, and to_realtime() is renamed to to_wire().
The engine no longer keeps a parallel command hierarchy. engine/duplex/mailbox.py binds that decoder to the engine's turn-detection capabilities and renders the runner mailbox dictionary; projection state moves to engine/duplex/projection.py.
Websocket and inline clients still enter through command_from_realtime, and the mailbox dictionaries keep the same channels, including turn.signal for session and item updates. No wire-format change is intended.
Change flow
flowchart LR
A["[EXISTING] OpenAI realtime classes"]:::existing --> B["[CHANGED] protocol.duplex decoder"]:::changed
B --> C["[NEW] engine mailbox binding"]:::new
D["[REMOVED] engine command copies"]:::removed --> C
C --> E["[CHANGED] session runner and clients"]:::changed
classDef existing fill:#e5e7eb,stroke:#6b7280,color:#111827
classDef changed fill:#fef3c7,stroke:#d97706,color:#451a03,stroke-width:2px
classDef new fill:#dcfce7,stroke:#16a34a,color:#052e16,stroke-width:2px
classDef removed fill:#fee2e2,stroke:#dc2626,color:#450a0a,stroke-width:2px
See inline comments below.
🤖 This review was generated by InferMatrix Copilot, an open-source repo-maintenance agent for PR review, CI debugging and issue triage. Try it on your own repo, and ⭐ star it if it helped!
| ) | ||
|
|
||
|
|
||
| def command_from_realtime( |
There was a problem hiding this comment.
[P1] Reported tests skip the duplex e2e guard this diff selects
Evidence and suggested fix
command_from_realtime is the websocket and inline-client intake for every duplex command, including audio append and session update. Changing vllm_omni/protocol/ and vllm_omni/engine/duplex/ selects the ready-pipeline step "Omni · MiniCPM-o 4.5 Duplex Test" (source_file_dependencies: omni_minicpmo_4_5_duplex_function in .buildkite/cuda/test-ready.yml). That step runs pytest -s -v tests/e2e/online_serving/test_minicpmo_4_5_duplex.py tests/e2e/online_serving/test_duplex_client_live.py -m 'core_model and cuda' --run-level 'core_model'. The merge step "Omni · MiniCPM-o 4.5 Duplex Test" is the same dependency key with pytest -s -v tests/e2e/online_serving/test_minicpmo_4_5_duplex.py tests/e2e/online_serving/test_minicpmo_4_5_window.py -m 'advanced_model and cuda' --run-level 'advanced_model'.
The Test Plan reports a CPU sweep of tests/protocol/, tests/engine/duplex/, entrypoints, and tests/e2e/features/fullduplex/, plus a 2-GPU run of tests/entrypoints/openai_api/test_qwen3_omni_realtime_websocket.py. Those selectors do not include test_minicpmo_4_5_duplex.py or test_duplex_client_live.py, and they do not use the ready step's core_model and cuda / --run-level core_model pair. The body does not say that GPU duplex e2e was left to CI. Run that ready selector, or state the omitted target and marker in the Test Plan.
Omni ReviewBot: finding feedback[p1] Reported tests skip the duplex e2e guard this diff selects — See the review for details. If you are the PR author and disagree, react 👎 here; the maintainer will see your disagreement. |
Omni ReviewBot: no human activity for 13 days@chickeyton this pull request has had no human commit, comment or review since 2026-10-04. 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. |
Follow-up to #7640 (RFC #6592 P0a).
Purpose
After #7640 there were three overlapping sets of Realtime/duplex command and event definitions:
protocol/realtime,protocol/duplex, andcommands.py/events.py/realtime_commands.py/realtime_events.py/audio.pyinengine/duplex. This PR collapses them into two layers with one direction of dependency:protocol/realtime— the OpenAIv1/realtimecommand and event classes (RealtimeCommand/RealtimeEventsubclasses) and their helpers. Unchanged apart from docstrings.protocol/duplex— the wholev1/realtime?duplex=1vocabulary.DuplexCommandandDuplexEventare plain aliases ofRealtimeCommand/RealtimeEvent; the Tier 2 extensions and Tier 3 duplex-only classes live here, together with the wire decoder moved in from the engine (decode_duplex_command,build_append_audio). Consumer capabilities are injected throughRealtimeProtocolCapabilitiesinstead of imported from the engine, and the decoder raisesRealtimeProtocolErrordirectly (DuplexCommandErroris removed).The engine keeps only its own representation, under names that no longer look like protocol modules:
engine/duplex/mailbox.py(new):DUPLEX_REALTIME_CAPABILITIES,command_from_realtime(the decoder bound to those capabilities), andmailbox_payload/mailbox_channel, which replace thepayload()methods and mailboxtypeof the removed parallel command hierarchy.engine/duplex/projection.py: renamed fromrealtime_events.py(the per-session projection state andresolve_*helpers).DOMAIN_TERMINAL_EVENTS/MODEL_OUTPUT_EVENTSmove intosession/emitter.pyandduplex_response_formatintoconfig.py, their only consumers.Dependency direction after this PR (one chain, no mesh):
protocol.duplex; nothing below it importsprotocol.realtimedirectly (tests/protocol/duplex/test_duplex_protocol_facade.py).protocol/**imports no runtime package (tests/protocol/realtime/test_protocol_import_boundary.py).protocol.duplexis the same object as inprotocol.realtime, or a subclass that only adds fields (tests/protocol/realtime/test_realtime_codec_single_source.py).The duplex engine, entrypoints and clients import
vllm_omni.protocol.duplexonly; nothing outsidevllm_omni/protocolimportsprotocol.realtime. Tests and docs are repointed; the facade, single-source and import-boundary tests assert the new layout.Also in this PR:
RealtimeEvent.to_realtime()is renamed toto_wire(), completing thewire_type/optional_wire_fields/_wire_fieldsvocabulary of the class and reading the same fromRealtimeEventand itsDuplexEventalias. Mechanical rename, no behaviour change.No wire-format or behaviour change is intended anywhere in this PR.
Test Plan
vLLM Version: 0.30.0
vLLM-Omni Commit: 455c077 (branch
duplex_protocolwith main c1e84ce merged in; the sweep and pre-commit below were run at this head; the 2-GPU websocket comparison at the pre-rebase head 4e2129b vs its base 9fbc36a)Run on a 4x L20X host, one pytest session:
plus
tests/entrypoints/openai_api/test_qwen3_omni_realtime_websocket.pyon 2 GPUs, on this branch and on the base commit 9fbc36a for comparison, andpre-commit run --files <branch diff>.Test Result
canhazgpu run --gpus 1reservation; the host's GPU guard kills unreserved GPU processes.speech_to_textrealtime serving). That route does not go through the duplex handler or the protocol packages touched here.mainc1e84ce for the same files (two of them moved with the code fromcommands.pytomailbox.py; theno-redefinprotocol/realtime/audio_input.pycomes from [Model] Integrate AURA into Full-Duplex Runtime #7633, which this PR only touches in a docstring; the seven test-file errors come with main's [Frontend][Benchmark] Add duplex performance metrics to refactored full-duplex framework #7714 tests). The protocol package also imports cleanly on Python 3.10, with every command and event instance__dict__-free, which is what [Bugfix] Fix the remaining Python 3.10 breakages #7985's__slots__fix guarded on the removed engine mixin.🤖 Generated with Claude Code