Skip to content

[Bugfix][MiniCPM-o] Restore the duplex tests removed in #7413 - #7872

Open
chickeyton wants to merge 4 commits into
vllm-project:mainfrom
chickeyton:duplex_restore_tests
Open

chickeyton wants to merge 4 commits into
vllm-project:mainfrom
chickeyton:duplex_restore_tests

Conversation

@chickeyton

@chickeyton chickeyton commented Sep 20, 2026 •

Copy link
Copy Markdown
Contributor

What broke

Since #7413 a duplex session never reports the server-side per-response TTFT/TTFP that #7242 added for the MiniCPM-o Omni-DuplexEval / OmniInteract benchmarks. response.created and the audio deltas carry no response_request_metrics, so vllm_omni.clients.duplex.EventCollector and benchmarks/duplex_session_metrics.py silently fall back to the client's receive clock ("source": "client_monotonic_receive"), while docs/cli/bench/serve.md still promises "Per-response TTFT and TTFP start when the server begins executing the native model-turn request". The same commit's TPOT weighting fix was lost too: a one-token segment is weighted as one interval again and skews the response mean.

Repro

# on main: 4 of the 5 new tests fail (the methods and the wire fields do not exist)
pytest -q tests/engine/duplex/test_engine_session.py tests/engine/duplex/test_session_runner.py \
  -k "response_timing or response_tpot or response_request_metrics"

The strengthened e2e gate fails on main the same way: every response's request_metrics.source is client_monotonic_receive instead of server_request_start_and_client_receive.

CUDA_VISIBLE_DEVICES=0 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

Root cause

#7413 replaced entrypoints/duplex/{protocol,runtime_bridge,session_runner}.py with the engine-resident DuplexEngineSession / ModelChannel, and the port did not carry over mark_model_turn_request_started, mark_response_first_outputs, the response_request_metrics attachment, or the max(tokens - 1, 0) TPOT weight. The client side of #7242 survived, so the feature degraded silently instead of failing.

Fix

  • engine_session.py: per-turn request-start ledger on ResponseState, mark_model_turn_request_started / mark_response_first_outputs, binding at begin_response, pruning at complete_model_turn, resets on end / barge-in / close, and the interval-weighted TPOT mean.
  • model_channel.py: anchors the clock when the append is submitted and attaches the metrics to response.created (lands under response.metadata.duplex_event) and to every speak / audio delta (metadata.vllm_omni), the two places the client already reads.
  • Tests: the three [Frontend][Benchmark] Add duplex performance metrics for OmniInteract and Omni-DuplexEval benchmark #7242 session tests plus a lifecycle test, a runner test over the real MiniCPM-o plugin, the MiniCPM-o plugin policy tests re-homed from the deleted tests/engine/duplex/test_duplex_runtime.py, and the duplex e2e gate now asserts the server-side anchor. The 11 pre-existing mypy-3.10 errors in the two touched test files are fixed so pre-commit passes.
  • Docs: three passages still described the session-per-request DuplexChatCompletionsAdapter that was removed before [Core][Frontend] Unified Full-duplex Framework #7413 merged; they now match the chat route that exists.

Tests restored

Thirteen tests across four files: ten restore tests #7413 deleted, three are new.

From tests/entrypoints/openai_api/test_duplex_handler.py (deleted by #7413), now in tests/engine/duplex/test_engine_session.py

Original Restored as
test_response_timing_binds_latest_request_start_for_model_turn same name, on DuplexEngineSession
test_response_tpot_fallback_ignores_single_token_segment same name
test_response_tpot_keeps_token_weighted_value_when_itls_exist same name

From the same file, now in tests/engine/duplex/test_session_runner.py

Original Restored as
test_continuous_response_metrics_accumulate_only_owned_model_units (the response_request_metrics half) test_response_request_metrics_are_measured_from_the_model_turn_request_start, driven through the runner harness with the real MiniCPM-o plugin

From tests/engine/duplex/test_duplex_runtime.py (deleted by #7413), now in the new tests/model_executor/models/minicpmo_4_5/duplex/test_plugin_policy.py

Original Restored as
test_minicpmo_extension_owns_stage_sampling_overrides test_plugin_owns_stage_sampling_overrides_without_mutating_defaults
test_minicpmo_output_decision_uses_raw_streaming_token_snapshot test_listen_decision_uses_the_raw_streaming_token_snapshot
test_minicpmo_output_decision_ignores_output_level_token_history (x2 params) test_listen_decision_ignores_output_level_token_history
test_minicpmo_output_decision_uses_completion_token_ids (x2 params) test_listen_decision_uses_completion_token_ids
test_minicpmo_output_decision_uses_completion_stop_reason test_listen_decision_uses_completion_stop_reason
test_duplex_scheduler_token_budget_estimates_pcm_slots test_scheduler_token_budget_estimates_pcm_slots
test_duplex_scheduler_token_budget_ignores_client_budget_fields test_scheduler_token_budget_ignores_client_budget_fields

New, no deleted counterpart

  • test_response_timing_is_empty_without_a_request_start_and_is_scoped_to_one_response (test_engine_session.py): lifecycle of the timing state across end, turn completion, and barge-in.
  • test_a_speak_segment_is_not_a_listen_decision (test_plugin_policy.py): negative case for the listen decision.
  • tests/e2e/online_serving/test_minicpmo_4_5_duplex.py: _assert_request_metrics now requires source == "server_request_start_and_client_receive" and the native request-start measurement origin, so the CI e2e gate catches this regression.

Deliberately not restored

Validation (1x NVIDIA L20X, vllm 0.29.0, torch 2.13.0+cu129)

Suite Result
pre-commit run --files <changed> (all hooks incl. mypy-3.10) passed
Duplex CPU sweep: tests/engine/duplex tests/clients tests/model_executor/models/minicpmo_4_5 tests/benchmarks/{test_omniinteract.py,patch,metrics} tests/entrypoints/duplex tests/engine/test_duplex_orchestrator.py tests/engine/test_duplex_omni_engine.py tests/entrypoints/test_duplex_omni.py tests/entrypoints/openai_api/test_duplex_api_server.py tests/entrypoints/openai/test_duplex_session_attachment.py tests/worker/test_native_duplex_hooks.py tests/protocol tests/engine/test_duplex_import_boundary.py tests/engine/test_output_processor.py 1477 passed, 5 skipped in 7m14s
MiniCPM-o duplex e2e, -m "core_model and cuda" --run-level core_model 2 passed, 11 deselected in 15m00s
New and restored tests (test_engine_session.py, test_session_runner.py, test_plugin_policy.py) 89 passed

Not restored on purpose: the frame-size-aware vision slot budget from #7271 was also dropped by #7413 but has since been re-fixed by #7654; the duplex Seed-TTS perf baselines removed in #7413 are left out because the native per-unit path changed.

🤖 Generated with Claude Code

…st in vllm-project#7413

vllm-project#7413 rebuilt the duplex serving stack around engine-resident sessions and
dropped the per-response request timing that vllm-project#7242 had added for the
MiniCPM-o Omni-DuplexEval / OmniInteract benchmarks: the engine no longer
emitted ``response_request_metrics``, so ``DuplexClient`` and the benchmark
summaries silently fell back to the client's receive clock, and the TPOT
weighting regressed to counting a one-token segment as one interval.

Port the timing into the engine session: the model-turn request start is
recorded when the model channel submits the append, the first text / audio
of the response fix TTFT / TTFP, and the numbers ride response.created
(under response.metadata.duplex_event) and every speak / audio delta
(under metadata.vllm_omni), exactly where the client already reads them.
Restore the interval-weighted TPOT mean.

Re-home the MiniCPM-o plugin policy unit tests deleted with
tests/engine/duplex/test_duplex_runtime.py, make the duplex e2e gate
assert the server-side anchor, fix the pre-existing mypy errors in the
two touched test files, and drop three doc passages that still describe
the session-per-request chat adapter removed before vllm-project#7413 merged.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: chickeyton <ngton2014@gmail.com>
@chickeyton

Copy link
Copy Markdown
Contributor Author

Pre-check report (precheck-pr, full mode, bug-fix checklist)

Run after fixing the blocking issue found on the first pass: pre-commit's mypy-3.10 hook reported 11 pre-existing type errors in the two touched test files (tests/engine/duplex/test_session_runner.py: object typed harness output / event; tests/e2e/online_serving/test_minicpmo_4_5_duplex.py: dict[str, object] result indexed as str / int / list). Fixed by typing the harness fakes as SimpleNamespace / DuplexEvent and giving the seeded-turn helper a TypedDict result.

Pre-check report for duplex_restore_tests

  Mode: full
  Type: bug-fix

  Dimension            Result
  ───────────────────  ──────
  PR title format      ✓ [Bugfix][MiniCPM-o] prefix, no WIP
  Code quality         ⚠ 11 new SimpleNamespace fakes in tests (duck-typed stage
                         outputs and a clock stub, same convention as the existing
                         runner harness); no new **kwargs lookup, broad except,
                         Any hint, hot-path copy, or event-loop blocking
  Examples policy      ✓ no new Python example paths
  Simplification       ✓ no candidates (two new session methods, both called)
  PR body 4 sections   ✓ what broke / repro / root cause / fix
  Fix is minimal       ⚠ three doc passages about the removed session-per-request
                         chat adapter are corrected alongside the fix
  Repro is runnable    ✓ pytest -k "response_timing or response_tpot or
                         response_request_metrics": 4 failed on main, 5 pass here
  Branch is rebased    ✓ merge-base == origin/main (573ec4cd)
  Regression test      ✓ session, runner (real MiniCPM-o plugin) and e2e assertion
  Root cause match     ✓ restores exactly the #7242 semantics the port dropped
  No silent failure    ✓ no new except / fallback
  Upstream pattern     ✓ same methods and wire placement the client already reads
  Environment          — not version-dependent
  Pre-commit gates     ✓ all hooks pass on Linux after the mypy fix (SPDX,
                         forbidden imports, torch.cuda, test marks, markdownlint,
                         ruff, typos, mypy-3.10)
  Dead code            ✓
  Test coverage        ✓ engine/ change covered by unit, runner and e2e tests
  Import hygiene       ✓ one new import (TypedDict), used
  Unrelated changes    ⚠ see "Fix is minimal"

  Verdict: 0 blocking | 2 warnings | ready for review

Evidence: pre-commit PRECOMMIT_EXIT=0; duplex CPU sweep 1477 passed / 5 skipped; MiniCPM-o duplex e2e (core_model, 1x L20X) 2 passed.

@vllm-omni-review-bot

Copy link
Copy Markdown

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

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

@vllm-omni-review-bot

vllm-omni-review-bot commented Sep 20, 2026 •

Copy link
Copy Markdown

Omni ReviewBot triage note

Automated triage of commit e3f0acf4776d produced:

  • Priority: high. Prompt maintainer attention is suggested.

These are automated triage suggestions only — the final decision belongs to the maintainers.

@chickeyton chickeyton changed the title [Bugfix][MiniCPM-o] Restore the duplex server-side response timing lost in #7413 [Bugfix][MiniCPM-o] Restore the duplex tests removed in #7413 Sep 20, 2026
@amy-why-3459

Copy link
Copy Markdown
Collaborator

@yenuo26 PTAL

@hsliuustc0106 hsliuustc0106 added high priority high priority issue, needs to be done asap bug Something isn't working labels Sep 20, 2026
@vllm-omni-review-bot

Copy link
Copy Markdown

Omni ReviewBot: no human activity for 7 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.

@vllm-omni-review-bot

Copy link
Copy Markdown

Omni ReviewBot: no human activity for 14 days

@chickeyton this pull request has had no human commit, comment or review since 2026-09-20. Please consider marking this PR as draft until work can resume. 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.

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

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

Labels

bug Something isn't working high priority high priority issue, needs to be done asap

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants