Skip to content

[Core][Model] Unified Full-Duplex MRv2 Support and MiniCPM-o 4.5 Adaptation - #8536

Open
BeatSeat wants to merge 35 commits into
vllm-project:mainfrom
BeatSeat:feat/duplex-mrv2-minicpmo45
Open

BeatSeat wants to merge 35 commits into
vllm-project:mainfrom
BeatSeat:feat/duplex-mrv2-minicpmo45

Conversation

@BeatSeat

@BeatSeat BeatSeat commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Enable the opt-in CUDA MRv2 profile (vllm_omni/deploy/minicpmo_4_5_duplex_mrv2.yaml) for all three MiniCPM-o 4.5 duplex stages: Thinker, Talker and Code2Wav. The default duplex deployment stays on V1; non-CUDA platforms fall back to V1.

  • Thinker: wire duplex preprocessing, the duplex sampler (duplex/mrv2_sampling.py) and request lifecycle into MRv2, keeping the existing duplex policies and window behavior. The latent row ledger goes through the MRv2 output-channel contract (include_hidden_states=False, finalize_multimodal), so it is accumulated once. Special-token metadata stays on the host.
  • Talker: keep condition/audio state per request across overlapping sessions (MRv2 buffers carry req_id, not request_id); close a codec segment only on an EOS accepted in the current output event. The Talker window policy stays in the model-owned processor, reached through the existing update_streaming_prompt_for_condition hook.
  • Code2Wav: a native-duplex turn end closes the turn (last_chunk, turn_end, is_segment_finished) but not the resumable stream; meta.finished follows the request on the MRv2 native transport.
  • MRv2 host work that grows with sessions: the Thinker publishes the duplex prompt snapshot and special ids only on steps that complete an append prefill (the accumulator keeps a missing key's last value), exposes preprocess_batch_mrv2 so concurrent duplex appends batch across sessions as on V1, and uses the logits in place when every row follows the policy; the Talker declares mm_outputs_fresh_per_step because its MRv2 outputs are built per step.
  • CI: an independent real-weight MRv2 ready job (C1/C2/C4 two-turn sessions plus a staggered long response). Existing V1 ready, merge, nightly and performance coverage is unchanged.

Real-weight results

1×H200, vLLM 0.31 image, Seed-TTS duplex realtime benchmark (tests/dfx/perf/scripts/run_benchmark.py, 4 turns per session, num_prompts = 2×C), same node for both profiles; OMP_NUM_THREADS=1 and uclamp were set in the test environment only. All sessions completed on both profiles. In this harness the first output is the first audio chunk, so TTFT equals TTFP.

C TTFT/TTFP mean V1 → MRv2 (ms) TTFT/TTFP P99 V1 → MRv2 (ms) RTF median V1 → MRv2 RTF P99 V1 → MRv2
1 739 → 692 778 → 716 1.13 → 1.12 1.15 → 1.14
2 775 → 726 893 → 738 1.14 → 1.11 1.16 → 1.24
4 643 → 606 816 → 722 1.20 → 1.15 1.30 → 1.24
8 572 → 493 823 → 687 1.50 → 1.31 1.76 → 1.69

The reviewer-reported Talker crash with overlapping sessions is fixed: an earlier head failed every C2/C4 session, while this branch completed C1–C8 in two independent sweeps. Duplex TTFP is quantized by the Thinker unit cadence, so a 16-session mean moves by about ±100 ms between runs of the same code; P99 here is over session means.

Torch profiler, C=4, Stage 0: cudaStreamSynchronize 2108 → 49 per 10 s window after moving the special-token metadata to the host (V1: 51); blocking per step 3.6 → 0.05 ms.

Validation

  • CPU: MiniCPM-o models, stage input processors, worker/worker_v2, core, engine and config suites: 5573 passed, 26 skipped. The 21 failures also fail on main in the same environment: test_arg_utils (SOCKS proxy), test_async_omni_engine_abort (import), test_gpu_generation_async_output.
  • CPU on the current head: 5578 passed with the same 21 environment-only failures.
  • Pending on a GPU: a real-weight rerun of the MRv2 ready job on this head (the Code2Wav turn-end fix is unit-tested only) and a C4/C8 A/B of the session-scaling host-work change (64+ sessions per point, since 16-session TTFP means vary ±100 ms).

Out of scope / follow-ups

@vllm-omni-review-bot

Copy link
Copy Markdown

This PR appears to belong to: docs/design/module/model_integration.md, docs/design/module/ar_runtime.md.

Module owners: @tzhouam @fake0fan @Gaohan123

Routing: @tzhouam via module of the changed files, CODEOWNERS; @fake0fan via module of the changed files; @Gaohan123 via module of the changed files

@BeatSeat, 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.

@vllm-omni-review-bot

Copy link
Copy Markdown
Omni ReviewBot routing record

Assigned Strict on cursor (cursor-grok-4.6-high) under experiment fleet-strict-cursor-grok46-zcode-glm53flash-5050-c5-z10-20261002.

@BeatSeat
BeatSeat requested a review from congw729 as a code owner October 6, 2026 05:18
@vllm-omni-review-bot

vllm-omni-review-bot commented Oct 6, 2026 •

Copy link
Copy Markdown

Omni ReviewBot: superseded

The CI failure noted on 39bd8b1a1d72 refers to an earlier head; the pull request now points at 050ef698b053.

@BeatSeat
BeatSeat force-pushed the feat/duplex-mrv2-minicpmo45 branch 3 times, most recently from 8e345fb to 6f58580 Compare October 6, 2026 15:12
@hsliuustc0106 hsliuustc0106 added high priority high priority issue, needs to be done asap omni code related to omni models core related to core module: cache, scheduler, engine, worker, modelrunner tts code related to tts models labels Oct 6, 2026

@amy-why-3459 amy-why-3459 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.

Reviewed commit 6f58580ba477e3d62e4df38cbf04d66c54e17c3e. I recommend changes before merging because the duplex Thinker preprocessing and sampler wiring are incomplete.

[P1] Enable duplex Thinker preprocessing on MRv2

In minicpmo_4_5_omni.py:228, has_preprocess is still computed as self.model_stage == "tts" or not self._use_v2_model_runner. Consequently, a duplex Thinker using MRv2 gets False, and OmniModelState.run_preprocess() returns without running the duplex embedding construction and input-token replacement. Removing the unsupported-mode guard does not enable that path. Please enable custom preprocessing for duplex Thinker while retaining the native multimodal path for turn mode. The newly added test_mrv2_thinker_duplex_output_and_prompt_rows reproduces this: it fails at assert model.has_preprocess is True.

[P1] Wire the duplex sampler into the production MRv2 lifecycle

duplex/mrv2_sampling.py defines MiniCPMO45DuplexSampler, but there is no production import/instantiation of this class. OmniModelState.custom_sampler() uses the model's mrv2_custom_sampler hook; the wrapper exposes that hook for Talker, but not for Thinker. Also, _mrv2_sampling_infos, which the new sampler reads, has no production writer. Registering the sampler alone would therefore still leave it falling back to the default sampler with no duplex rows, bypassing the listen/speak and turn-end policy. Please add Thinker sampler registration, current request metadata/sampling-parameter updates, and request cleanup. Add a test exercising the actual OmniModelState.custom_sampler() entry point and request lifecycle; manually constructing the sampler and injecting _mrv2_sampling_infos does not cover this gap.

Reuse the existing preprocessing metadata instead of adding duplex aliases to MRv2

In worker_v2/model_states/omni_model_state.py:794-800, the newly assigned duplex_prompt_len and duplex_token_offset duplicate _omni_prompt_len and _omni_num_computed_tokens, respectively. Here the offset is request-relative, not batch-relative or audio-chunk-relative, so the generic fields already carry the required semantics. Please keep the runner generic and update MiniCPM's preprocess() to read the existing _omni_* fields consistently for decode detection, prompt padding and embedding slicing. If compatibility is needed, fall back to the legacy duplex fields in the model when the generic value is None (not with or, since zero is valid). Do not remove V1's legacy fields without migrating its other consumers: Personaplex also reads them. This is an interface-maintenance suggestion, not a claim of meaningful speedup from removing dictionary assignments.

Validation

On this exact head, the following targeted command produced 395 passed, 1 failed, 1 skipped in the local Python 3.12 / vLLM 0.31.0 environment:

python -m pytest -q \
  tests/model_executor/models/minicpmo_4_5/test_duplex_mrv2_sampling.py \
  tests/model_executor/models/minicpmo_4_5/duplex/test_window_wiring.py \
  tests/model_executor/models/minicpmo_4_5/test_thinker_mrv2.py \
  tests/model_executor/models/minicpmo_4_5/test_talker_mrv2.py \
  tests/worker_v2/test_omni_ar_model_runner.py \
  tests/config/test_omni_config.py

The failure is the preprocessing assertion described above. The environment also emitted a vLLM/vLLM-Omni version-mismatch warning. This was targeted verification, not a complete unit-test run or model end-to-end/performance validation.

Please provide complete repository unit-test evidence on the updated head after fixes, using the repository's supported CI unit-test shards and applicable backend-specific jobs, including those defined in .buildkite/cuda/test-ready.yml. Attach exact commands, dependency/environment versions, passed/failed/skipped/deselected/collection-error counts where available, and CI or complete-log links. Explain skips and unavailable jobs; if infrastructure is unavailable, obtain an explicit maintainer disposition for the remaining gap. For failures believed to be pre-existing, include a same-environment base/head comparison. The currently visible build/pre-commit/DCO successes and the targeted results above do not establish full-suite coverage.

@amy-why-3459

Copy link
Copy Markdown
Collaborator

Follow-up on deployment profiles and V1 vs V2 performance, reviewed at 6f58580ba477e3d62e4df38cbf04d66c54e17c3e:

Please justify/minimize the additional deployment YAMLs.

  • minicpmo_4_5_duplex_mrv2.yaml is reasonable as an explicit opt-in V2 overlay while the default remains V1, but a separate file is a deployment convenience rather than a runtime requirement. Please document which overrides are required for correctness versus optional tuning. session_mode: duplex is already inherited from minicpmo_4_5.yaml; explain the need for stage 0 async_chunk: false, stage 2 async_scheduling: false, and the audio graph bucket override. Keep the non-CUDA V1 fallback explicit if retaining this profile.
  • minicpmo_4_5_duplex_mrv2_h200.yaml appears redundant as written: its only settings are stage 1 max_num_seqs: 16 and CUDA kv_cache_memory_bytes: 4294967296, both already inherited from minicpmo_4_5.yaml through the generic duplex MRv2 profile. Please remove/consolidate it unless there is a demonstrated difference in the resolved configuration. Its description also advertises H200 throughput optimizations/TF32, but it does not inherit minicpmo_4_5_duplex_h200.yaml or explicitly enable that profile's extra optimizations.
  • The BF16-cache H200 profile has an actual configuration distinction from the FP32 fused-body profile, so it is not the same redundancy. However, please justify including this independent cache-precision/profile change in the MRv2 PR, and support any throughput/concurrency claims with measurements (or separate that tuning into its own PR).

A configuration-resolution test comparing the effective profiles would help prevent no-op aliases and documentation drift.

Please add reproducible model runner V1 vs V2 performance validation before merging.

This PR changes the full-duplex execution path, so turn-mode measurements or standalone kernel timings are insufficient. After fixing the functional issues, please benchmark V1 and V2 on the same updated commit, model/tokenizer revision, GPU, dependency stack, input trace and seeds. Keep concurrency, stage capacities, KV budgets, cache precision, TF32 policy, graph settings, codec chunk size and sampling settings matched. Include the fully resolved configs and exact launch/client commands. If a setting must differ for V2 correctness, explicitly identify it; distinguish that deployment-level comparison from a controlled runner comparison. Do not use FP32/fused V1 versus BF16/non-fused V2 as evidence of runner-only speedup.

Please include:

  1. Concurrency sweeps (e.g. 1, 2, 4, 8, 16 sessions where supported), fixed-duration audio traces and an audio/video case, with warmup, measured duration, repeated runs and sample counts documented.
  2. End-to-end first-audio latency and inter-audio-chunk latency at p50/p95/p99, real-time factor with its definition, throughput/sustainable concurrency under a stated latency target, and peak GPU memory. Report errors/OOMs, preemptions and audio underruns or missed playback deadlines rather than excluding unsuccessful runs.
  3. A stage-level breakdown (Thinker, Talker, Code2Wav), ideally a profiler trace showing where V2 saves time and the effect of the adapter's host history reads/synchronization. Separate startup/graph-capture costs from steady-state latency.
  4. Duplex correctness under the same load: listen/speak boundaries, turn-end drain, interruption/cancellation and stale-audio suppression, plus long sessions that trigger KV-window reanchor and Talker rollover. Report generated text/audio duration or token/frame counts so shorter or incorrect output cannot appear as a speedup; include an audio-quality check when numerical policies differ.

Please attach raw benchmark results/logs and a V1/V2 comparison table tied to the tested SHA, explaining regressions and the workloads on which V2 is recommended. This performance evidence is in addition to the full unit-test evidence requested in the earlier review.

@BeatSeat
BeatSeat force-pushed the feat/duplex-mrv2-minicpmo45 branch 2 times, most recently from 1db15d3 to e431e32 Compare October 7, 2026 15:24
The turn MRv2 profile and the MRv2 design note still said duplex needs the
V1 path; minicpmo_4_5_duplex_mrv2.yaml now runs all three duplex stages on
MRv2 as an opt-in CUDA overlay.

Signed-off-by: BeatSeat <wendavid552@gmail.com>
…m this PR

The Talker crash at overlapping sessions came from all MRv2 Talker requests
sharing one condition state; per-request keying fixes it. Containing a
failed preprocess to its request is a generic runner/scheduler contract that
V1 does not have, and it overlaps the request_errors field vllm-project#8635 adds. Leave
it for a separate change: remove RequestPreprocessingError, the runner and
scheduler request_errors plumbing and the Talker wrapper.

Signed-off-by: BeatSeat <wendavid552@gmail.com>
- Thinker: publish the duplex prompt snapshot and special ids only on
  steps that complete an append prefill; the accumulator keeps a missing
  key's last value, so decode steps stop copying every row's whole prompt
  through the snapshot, output thread and IPC (cost grows with sessions x
  prompt length).
- Thinker: expose preprocess_batch_mrv2 so MRv2 batches concurrent duplex
  appends across sessions like V1's preprocess_batch.
- Sampler: when every row follows the policy, use the logits in place
  instead of an index upload and a full-vocabulary gather.
- Talker: declare mm_outputs_fresh_per_step under MRv2; its outputs are
  built per step, so the async snapshot ring only added a device copy.

Signed-off-by: BeatSeat <wendavid552@gmail.com>
Signed-off-by: BeatSeat <wendavid552@gmail.com>
Signed-off-by: BeatSeat <wendavid552@gmail.com>
@BeatSeat

Copy link
Copy Markdown
Contributor Author

Unit-test evidence at 0eade7d47 (this head, which includes #8717), compared with its main base e4af781dc. Both trees ran the four CUDA-ready Simple CPU shards with the commands in .buildkite/cuda/test-ready.yml (-m 'core_model and cpu') and the repository addopts.

Environment: CPU-only pod (no GPU), image vllm/vllm-openai:v0.31.0 (vLLM 0.31.0, torch 2.13.0+cu130, Python 3.12.3, pytest-xdist 3.8.0), editable .[dev] install.

Differences from CI: to finish in minutes, each tree ran with -n 4 (the --dist=loadgroup from addopts) and OMP_NUM_THREADS=4. Both trees ran at the same time in one pod, with identical settings.

Shard PR head: passed / failed / errors / skipped main base: passed / failed / errors / skipped
Model Executor (tests/model_executor) 4308 / 23 / 7 / 32 4246 / 23 / 7 / 26
Diffusion (tests/diffusion) 7990 / 132 / 191 / 86 7987 / 131 / 195 / 86
Engine & Entrypoints (tests/entrypoints tests/engine) 3925 / 2 / 0 / 7 (1 xfailed) 3925 / 2 / 0 / 7 (1 xfailed)
Other (tests/ minus the above) 4967 / 18 / 76 / 74 4967 / 17 / 61 / 74

Comparison by test id (junit XML):

  • Model Executor, Engine & Entrypoints: the failing set is identical on both trees. The failures are environment-only: NotImplementedError / Found no NVIDIA driver / Failed to infer device type on a GPU-less host, and a VllmConfig validation error in test_arg_utils.
  • Diffusion, Other: the PR-only failures are port collisions. Each tree also has its own "main-only" failures of the same kind.
    • tests/distributed/omni_connectors/test_nixl_connector.py: NixlConnector failed to bind handshake socket on 127.0.0.1:47431. The same error causes 58 failures common to both trees. The two trees and their xdist workers shared one network namespace.
    • tests/diffusion/distributed/test_comm.py spawn tests: DistNetworkError.
    • One teardown error in test_layerwise_backend.py.
    • A collection error in test_bagel_expansion.py, which collects cleanly when run serially. Main had the equivalent collection error on test_qwen2_5_omni_autoround_w4a16_expansion.
  • Serial rerun: every PR-only failure and the whole nixl file, rerun on the PR tree without xdist, gave 113 passed.
  • Most common failures on both trees: in the Diffusion shard, ProcessGroupNCCL is only supported with GPUs, socket setup errors, and NotImplementedError (no CUDA). In the Other shard, libcuda.so.1 missing.

Not covered here: the two GPU jobs in the same group, Simple · CosyVoice3 Packed Flow Test and Simple · MRV2 FA3 Graph Bounds Test (both H100), plus the real-weight MRv2 ready job. They need GPU CI. The PR has the ready label; the required buildkite/vllm-omni check still has to run.

@Sy0307 Sy0307 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.

Requesting changes for the first-prefill cancellation leak identified inline, reviewed and tested on 0eade7d47c2059e1806c3d4d91cc55a59f5fc72e.

Validation on this head:

  • Existing targeted tests: 243 passed, 2 deselected.
  • New CPU cancellation probe: MRv2 incomplete first prefill fails cleanup; completed MRv2 prefill and both V1 controls pass (1 failed, 3 passed). The probe uses the real preprocessing, sampler and cleanup methods with model construction/encoding stubbed.
  • Real-weight H200 E2E: all 4 cases passed (C1/C2/C4, two turns each, plus a staggered long response/new session), 0 skips/errors, 322.94 s including startup; physical GPU 1, Python 3.12.3, vLLM 0.31.0, Torch 2.13.0. These cases do not exercise cancellation during prefill, and I have not measured this leak with real weights.

Non-blocking cleanup: tests/config/test_minicpmo_4_5_duplex_mrv2_deploy.py:28,43 and tests/model_executor/models/minicpmo_4_5/test_thinker_mrv2.py:18 repeat the same duplex-profile runner, async-chunk, capacity and KV-budget expectations. Consolidate them in the config suite while retaining the distinct platform/async-scheduling checks.

if not input_batch.is_prefilling_np[row.row_idx]
or int(input_batch.num_computed_prefill_tokens_np[row.row_idx])
+ int(input_batch.num_scheduled_tokens[row.row_idx])
>= int(input_batch.prefill_len_np[row.row_idx])

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.

Please bind request-to-session ownership before filtering partial-prefill rows. Stage-0 preprocessing has already created helper.sessions[session_id], but _minicpmo45_duplex_request_sessions is populated only by prepare_duplex_sampling; cancelling/closing during the first incomplete prefill therefore calls on_requests_finished without a mapping and leaves the session (including embeddings/audio-cache state) alive. A CPU probe using the real preprocessing, sampler and cleanup methods failed only for MRv2 incomplete prefill; completed MRv2 prefill and both V1 controls passed. Register ownership when preprocessing creates the session, retain the partial-prefill sampling/RNG exclusion, and add a cancel-before-prefill-completion regression.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Confirmed and fixed in e9f7816. preprocess now records request → session ownership as soon as it creates or fetches the Stage-0 session, before any later failure path, so on_requests_finished frees the session even when the row never reaches the sampler. prepare_duplex_sampling uses the same map, and the partial-prefill sampling/RNG exclusion here is unchanged.

Regression: test_mrv2_cancel_before_first_prefill_completes_frees_session (test_thinker_mrv2.py) runs the real preprocess and on_requests_finished and stubs only the session construction and audio encoding. It fails on 0eade7d (the session is left in helper.sessions) and passes with this change.

I also took the non-blocking cleanup: the duplex-profile runner/async-chunk/capacity/KV expectations now live in a single CUDA test in tests/config/test_minicpmo_4_5_duplex_mrv2_deploy.py, the non-CUDA platform check stays a separate test, and the duplex row is removed from test_thinker_mrv2.py.

@BeatSeat

Copy link
Copy Markdown
Contributor Author

@amy-why-3459 the repository unit-test evidence you asked for is in #8536 (comment).

It covers all four CUDA-ready Simple CPU shards at 0eade7d47, compared against the main base e4af781dc, with exact commands, environment, counts and a per-test diff. No test fails on this head that does not also fail on main; the PR-only cases were port collisions and pass when rerun serially (113 passed). The two H100 Simple jobs and the real-weight MRv2 ready job still need GPU CI.

@linyueqian the real-weight C1–C8 results for your blocking finding are in the PR description.

…ll completes

MRv2 samples no partial-prefill row, so a request cancelled during its
first prefill never reached prepare_duplex_sampling, the only place that
recorded its session; on_requests_finished then left the Stage-0 session
alive. Record ownership when preprocessing creates the session, and fold
the repeated duplex-profile expectations into the config suite.

Signed-off-by: BeatSeat <wendavid552@gmail.com>
@amy-why-3459 amy-why-3459 added ready label to trigger buildkite CI and removed ready label to trigger buildkite CI labels Oct 10, 2026
On MRv2 the block table lives on the device, so resolving it inside the
layer loop issued one blocking device-to-host read per attention layer for
the same request and group.

Signed-off-by: BeatSeat <wendavid552@gmail.com>

@Sy0307 Sy0307 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.

Two maintenance comments on 8973921:

  1. Please consolidate the final-segment tests in tests/model_executor/stage_input_processors/test_minicpmo_4_5_async_chunk.py:197-246. Both tests maintain the same setup and assertions for preserving codec frames, avoiding early closure, and suppressing a duplicate terminal flush. Please share that test logic, for example by parameterizing delta size, while retaining both single-frame and batched-delta inputs and the chunk-boundary cases.

  2. For the Talker metadata construction in vllm_omni/model_executor/models/minicpmo_4_5/minicpmo_4_5_omni_tts.py:1008-1045, please keep the V1 and MRv2 contracts aligned as they evolve. V1 rejects invalid native-duplex epoch/turn_id values at lines 1102-1109, whereas MRv2 allows negative integers/bools and substitutes -1 for non-integers; downstream condition fencing requires non-negative identities. Separate implementations are fine if needed, but their validation and shared field semantics should agree; a common model-local helper is optional. I have not reproduced a live failure from this difference, so this is a non-blocking consistency suggestion.

Validation on this head: H200 physical GPU 2, real-weight three-stage MRv2 serving E2E, 4/4 passed (C1/C2/C4 two-turn sessions and a staggered long-response case).

…MRv2

MRv2 accepted negative or bool epoch/turn_id values and replaced other
non-integers with -1, while V1 rejects them; condition fencing downstream
needs non-negative identities. Both outputs now build each row through one
helper with V1's validation.

Signed-off-by: BeatSeat <wendavid552@gmail.com>
…size

Keeps the single-frame chunk-boundary cases and the batched-delta case in
one test with shared setup and assertions.

Signed-off-by: BeatSeat <wendavid552@gmail.com>
On MRv2 an append that builds no unit (empty audio or a rejected frame)
leaves its row without special token ids. When another row completed its
prefill in the same step, the shared row builder published None for that
request, and the output processor kept the None over every later value,
so each following llm2tts failed on the missing tts_bos id. These ids are
tokenizer constants; fill such rows from the batch.

Signed-off-by: BeatSeat <wendavid552@gmail.com>
…orted

The MRv2 native transport hands the processor a request snapshot without
a scheduler status, so _is_aborted never fired: a barge-in or session close
flushed up to a chunk of codec frames, plus the reference audio before the
first chunk, as a last chunk to an already-aborted Code2Wav, on the Talker
thread. The data plane now marks an aborted request's terminal snapshot,
and the processor treats it like V1's FINISHED_ABORTED.

Signed-off-by: BeatSeat <wendavid552@gmail.com>
The duplex server also serves /v1/chat/completions on the same Stage-0
engine. A duplex Thinker on MRv2 builds its audio appends through
preprocess, and the runner disabled the multimodal encoder for every
preprocess model, so the image, audio and video of a chat request were
answered from placeholder tokens without an error. V1 encodes before
preprocess and was unaffected.

The Thinker now declares preprocess_keeps_mm_inputs. The runner keeps
the encoder for such a model, and its model state uses the encoder
runner's embeddings buffer as the static FULL-graph buffer: the encoder
rewrites every active row each step, so preprocess no longer refills it
from token ids and only duplex append rows are replaced. Add a real-weight
check that an image chat on the duplex MRv2 server is answered from the
image.

Signed-off-by: BeatSeat <wendavid552@gmail.com>
@BeatSeat

Copy link
Copy Markdown
Contributor Author

@Sy0307 thanks, both are addressed at 56f9f2461.

  1. Final-segment tests (2731d8b9b). The two tests are now one, parameterized by delta size: single-frame deltas at 1/24/25/26/60 frames (the chunk-boundary cases), plus 52 frames sent as 25-frame batched deltas. Setup and assertions are shared, and each case also asserts its expected number of body chunks.
  2. Talker metadata (1e9aee16e). V1 make_omni_output and MRv2 make_omni_output_mrv2 now build every row through one model-local helper, _native_duplex_row_meta, which applies V1's validation. MRv2 therefore rejects a negative, bool or missing epoch/turn_id the same way V1 does. The new MRv2 test fails on the previous head.

Other MRv2 fixes from a review of the V1-only paths that the MRv2 duplex path skips

  • 56f9f2461: multimodal chat on the duplex MRv2 profile.
    • The duplex server also serves /v1/chat/completions. The MRv2 runner disabled the multimodal encoder for every model with a preprocess hook. As a result, a chat request's image, audio or video was answered from placeholder tokens, with no error. V1 runs the encoder before preprocess, so it was not affected.
    • The duplex Thinker now declares preprocess_keeps_mm_inputs, and for such a model the runner keeps the encoder. The model state uses the encoder runner's buffer as its static FULL-graph buffer. The encoder rewrites every active row each step, and preprocess only replaces the rows of duplex appends.
    • The flag defaults to False, so other models are unchanged.
    • New real-weight test in the MRv2 duplex job: test_mrv2_duplex_server_answers_image_chat_from_the_image.
  • 12580352e: aborts on MRv2.
    • The native transport's request snapshot has no scheduler status, so _is_aborted never fired. A barge-in or session close then flushed the pending codec frames (and, before the first chunk, the reference audio) to the already-aborted Code2Wav on the Talker thread.
    • The data plane now marks the terminal snapshot of an aborted request.
    • This was not audible: the client already drops outputs by request epoch.
  • db14d6f2f: Stage-0 special token ids.
    • On MRv2, an append that builds no unit leaves its row without special token ids. When it was batched with another session's completing prefill, None was published for it. That None then stayed in the request's accumulated meta, so every later llm2tts failed on the missing tts_bos.
    • These ids are tokenizer constants, so they are now filled from the batch.
  • f775f789d: reanchor block table reads. Reanchor now reads each KV group's block table once, instead of one device read per layer on MRv2.

Validation: the CPU suites these changes touch (minicpmo_4_5, stage_input_processors, worker_v2, engine duplex, config, examples) pass: 2617 passed, 5 skipped. Each fix's new test fails without the fix. The GPU jobs run on this head in CI.

Found but not in this PR (shared code or pre-existing on main; I will file them separately):

  • On any MRv2 native data-plane model, the transport still sends a finish marker for an aborted request, and it never unlinks shared-memory segments the receiver did not read (there is no chunk adapter). This leaks a small segment in /dev/shm per abort.
  • In duplex, a Stage-0 overflow FINISHED_ERROR is dropped before it reaches the session (both runners).
  • The in-place reanchor sink is offset by the context suffix ([Performance][MiniCPM-o] Zero-copy block-aligned KV sliding window with in-place Re-RoPE for duplex Stage-0 #7821, both runners).
  • Preemption replay pads the earlier appends of a resumable duplex request (both runners).

@vllm-omni-review-bot

Copy link
Copy Markdown
Omni ReviewBot attempt record

Review attempt ended as failed (failed; retrying strict/cursor/cursor-grok-4.6-high in 120s (try 2 of 3)).

@vllm-omni-review-bot

Copy link
Copy Markdown
Omni ReviewBot attempt record

Review attempt ended as failed (failed; retrying strict/cursor/cursor-grok-4.6-high in 600s (try 3 of 3)).

@vllm-omni-review-bot

Copy link
Copy Markdown
Omni ReviewBot attempt record

Review attempt ended as failed (failed; falling back to direct/cursor/auto).

@vllm-omni-review-bot vllm-omni-review-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Omni ReviewBot review

Changes since the previous review

  • 2 new inline finding(s); 0 finding(s) below.
  • Still needs attention: test_mrv2_talker_reuses_confirmed_prompt_window only admits streaming_condition… — tests/core/sched/test_omni_ar_scheduler_streaming.py
  • Fixed: This PR appends ## Full-duplex MRv2 stating minicpmo_4_5_duplex_mrv2.yaml o… — docs/design/minicpm_o45_mrv2_performance.md
  • Fixed: minicpmo_4_5_turn_mrv2.yaml:5 still says duplex needs the V1 chunk adapter — vllm_omni/deploy/minicpmo_4_5_turn_mrv2.yaml

CI at 56f9f2461f15 (2026-10-10T17:18:03.376065+00:00): required check(s) blocking: buildkite/vllm-omni (missing).

Note: The assigned review arm strict/cursor/cursor-grok-4.6-high could not complete this review, so it was produced by the fallback arm direct/cursor/auto. It is excluded from the routing experiment.

Full review analysis

PR description

This change adds an opt-in CUDA profile, minicpmo_4_5_duplex_mrv2.yaml, that runs MiniCPM-o 4.5 Thinker, Talker, and Code2Wav duplex stages on Model Runner V2. The default duplex deploy stays on V1, and non-CUDA platforms in the new profile force V1. The Thinker publishes duplex prompt and special-token metadata on append-prefill completion and samples through a new MRv2 duplex sampler; the Talker keeps condition and codec state per req_id; Code2Wav closes a turn on a stop token from the current output while meta.finished follows the request.

Change flow

flowchart LR
  A["[EXISTING] Default duplex deploy stays V1"]:::existing
  B["[NEW] minicpmo_4_5_duplex_mrv2.yaml"]:::new
  C["[NEW] MiniCPMO45DuplexSampler"]:::new
  D["[CHANGED] Thinker preprocess and prompt snapshot"]:::changed
  E["[CHANGED] Talker req_id state and Code2Wav turn close"]:::changed
  F["[CHANGED] Native Talker window hook"]:::changed
  A --> B
  B --> D
  B --> E
  C --> D
  D --> F
  E --> F
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
Loading

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!

# tensor delta. Keep it as row-local metadata so output
# accumulation replaces the previous value instead of
# attempting to concatenate variable-length prompts.
prompt_rows.append(list(prompt_token_ids) if isinstance(prompt_token_ids, list) else None)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[P1] Mixed prefill/decode step replaces a live duplex prompt with None

Evidence and suggested fix

When any row finishes an append prefill, make_omni_output_mrv2 publishes _duplex_row_outputs for the whole batch. A decode row has no duplex_prompt_token_ids, so the positional list contains None (test_mrv2_thinker_duplex_output_and_prompt_rows locks [[1, 2, 3], None]). The same helper says this snapshot is row-local metadata whose accumulation replaces the previous value, and that a None entry stays in the request's accumulated metadata (special-token ids are filled from the batch constant for that reason; the prompt is not). MRv2 async output already indexes a request-aligned list per request (codes.ref in test_omni_ar_model_runner). A session that already stored this unit's prompt, then decodes in the same step as another session's completing prefill, gets that key replaced by None. Later pure-decode steps omit the key, so the None remains until the segment is handed to llm2tts. _has_native_duplex_prompt_metadata is false for None, and the handoff falls back to scheduler prompt ids, so the Talker slice no longer matches this unit. Omit the key for rows without a snapshot (skip None in the per-request split) instead of writing None into the positional list.

if chunk_transfer_adapter is not None:
chunk_transfer_adapter.record_receive_failure(req_id, str(exc))
else:
self._streaming_context_overflow[req_id] = (session.client_index, str(exc))

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[P2] MRv2 native prompt seq-skip never runs the overflow finish path

Evidence and suggested fix

test_mrv2_talker_reuses_confirmed_prompt_window is the native-plane Talker update test. It sets _native_data_plane=True with the stub's chunk_transfer_adapter=None and only advances streaming_condition_seq 0 to 1, then asserts finish_requests was not called. A skipped seq (0 to 3) raises in _update_native_talker_prompt and takes the new adapter-less branch: _streaming_context_overflow[req_id] = ... and finish_requests(..., FINISHED_ERROR). _make_scheduler uses OmniARScheduler.__new__ and does not allocate that dict; production __init__ does, so this is an unrun branch rather than a live KeyError. V1 still covers record_receive_failure. Add a native skip-seq test that sets _streaming_context_overflow = {} and asserts finish_requests plus the overflow record.

@vllm-omni-review-bot

Copy link
Copy Markdown

Omni ReviewBot: finding feedback

[p1] Mixed prefill/decode step replaces a live duplex prompt with None — vllm_omni/model_executor/models/minicpmo_4_5/minicpmo_4_5_omni.py:70

See the review for details. If you are the PR author and disagree, react 👎 here; the maintainer will see your disagreement.

@linyueqian linyueqian 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.

Thanks for the fast turnaround. I reran the same comparison as last time on 56f9f2461: real weights, Seed-TTS realtime duplex benchmark, 4 turns per session, one H200-class GPU, the default profile against minicpmo_4_5_duplex_mrv2.yaml on the same tree. Each profile had two fresh server launches with two reps per point (one rep at 8 sessions).

Profile Sessions at once Sessions ok / failed First audio, mean (ms) RTF, median
default (V1) 1 16 / 0 244 to 307 1.01
default (V1) 2 32 / 0 290 to 304 1.03 to 1.04
default (V1) 4 64 / 0 352 to 373 1.05 to 1.06
default (V1) 8 32 / 0 470 to 560 1.09 to 1.13
MRv2 1 16 / 0 213 to 287 1.05
MRv2 2 32 / 0 280 to 306 1.03 to 1.05
MRv2 4 64 / 0 323 to 332 1.01 to 1.03
MRv2 8 32 / 0 540 to 568 1.03 to 1.08

Ranges are across launches and reps. The MRv2 profile completed every session at 2, 4 and 8 on both launches, with no traceback in either server log, so the concurrency crash I reported is fixed. Performance is on par with V1: slightly lower first audio and RTF at 4 sessions, slightly higher RTF at 1 session, and the 8-session first-audio difference is inside the run-to-run spread.

The other points from my first review are resolved in the code as described in the replies: a segment now closes only on a stop token sampled in the current output event, V1 CI coverage is back to what main has with MRv2 as an additional job, the Talker window logic sits behind the processor hook, the shared runner change is reverted, and the two per-step host costs are gone from the steady-state path.

One note for whoever merges: Buildkite has not reported on 56f9f2461 yet, so the new real-weight MRv2 ready job has not run on this head.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core related to core module: cache, scheduler, engine, worker, modelrunner high priority high priority issue, needs to be done asap omni code related to omni models ready label to trigger buildkite CI tts code related to tts models

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants