Skip to content

[Bugfix] Wire output buffers into duplex model test harnesses - #8489

Merged
andyluo7 merged 1 commit into
vllm-project:mainfrom
linyueqian:codex/fix-main16680-duplex-harness-20261004
Oct 4, 2026
Merged

andyluo7 merged 1 commit into
vllm-project:mainfrom
linyueqian:codex/fix-main16680-duplex-harness-20261004

Conversation

@linyueqian

Copy link
Copy Markdown
Collaborator

The main CUDA build 16680 failed after #7644 was merged, leading to eight PersonaPlex test failures and four Nemotron fixture errors. This happened because both model-specific harnesses construct an OpenDuplexSessionMessage without the newly required output_buffer argument, as these files landed after the PR base and were missing from the tested PR head.

To fix this, we now create a DuplexOutputBuffer using the same runtime limits that each manager uses and pass it directly to both the open message and the shared Harness. We also switched to using keyword arguments for the Nemotron harness to avoid issues with the changed dataclass field order. This keeps the production argument required and lets existing tests drain events through the shared buffer. Only the two test files change.

Validation shows that local pre-commit passes. On the h200 host (observed NVIDIA L20X, vLLM 0.30.0 and PyTorch 2.13.0+cu129), the three targeted test files produced eight failures, four fixture errors and 52 passes on main commit f781f7bb, and then 64 passes on the repair commit 2337af01, using isolated sources and the same GPU allocation. The exact validation command was pytest -sv tests/engine/duplex/test_session_runner_personaplex.py tests/engine/duplex/test_session_runner_nemotron_voicechat.py tests/engine/duplex/test_session_runner.py -m "core_model and cpu" --tb=short. The main CUDA CI for the repair remains pending.

Failure evidence: https://buildkite.com/vllm/vllm-omni/builds/16680#01a108c1-420b-4bb4-b3ae-a7b7790bb7fe

Signed-off-by: Yueqian Lin <linyueqian@outlook.com>
@linyueqian linyueqian added the ready label to trigger buildkite CI label Oct 4, 2026
@linyueqian

Copy link
Copy Markdown
Collaborator Author

Self-review: the changes are limited to the two model-specific test harnesses and output_buffer remains required in production. Each test fixture now passes a single buffer, configured from the manager's runtime limits, into both OpenDuplexSessionMessage and Harness.

Validation on the same h200 allocation with vLLM 0.30.0 shows main f781f7b reproduces 8 failures, 4 fixture errors, and 52 passes, while repair 2337af0 passes all 64 tests across the affected model suites and the generic runner suite. Local pre-commit passes and main CUDA build 16682 is pending.

@NolenLiang could you review this small follow-up to #7644 so we can restore main before resuming merges?

@vllm-omni-review-bot

Copy link
Copy Markdown

This PR appears to belong to: docs/design/module/engine_orchestration.md.

Module owners: @fake0fan @tzhouam @NickCao

Routing: @fake0fan via semantic router; @tzhouam via semantic router; @NickCao via CODEOWNERS

@linyueqian, 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 zcode (GLM-5.3-Flash) under experiment fleet-strict-cursor-grok46-zcode-glm53flash-5050-c5-z10-20261002.

@andyluo7 andyluo7 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 exact head 2337af0. The two test harnesses now create a bounded DuplexOutputBuffer from the same runtime limits and pass that exact buffer to both OpenDuplexSessionMessage and Harness, matching the generic runner contract introduced by #7644. The previously deterministic 8 failures and 4 fixture errors are resolved in both AMD #13285 and CUDA #16682: Simple · Engine&Entrypoints Test passed with 3709 passed on each platform. The NPU A5 failure is pre-start infrastructure (exit -1, no agent or log) and is unrelated to this test-only fix.

@andyluo7
andyluo7 enabled auto-merge (squash) October 4, 2026 23:46
@andyluo7
andyluo7 merged commit c1e84ce into vllm-project:main Oct 4, 2026
7 of 9 checks passed
chickeyton added a commit to chickeyton/vllm-omni that referenced this pull request Oct 5, 2026
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready label to trigger buildkite CI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants