Repository navigation
Conversation
|
This PR appears to belong to: docs/design/module/input_output_modality_contracts.md, docs/design/module/model_integration.md, docs/design/module/ar_runtime.md. Module owners: @alex-jw-brooks @amy-why-3459 @Gaohan123 Routing: @alex-jw-brooks via module of the changed files, semantic router; @amy-why-3459 via module of the changed files, semantic router; @Gaohan123 via module of the changed files, semantic router @andyluo7, 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. |
6970591 to
bee3f0d
Compare
Omni ReviewBot routing recordAssigned Direct under experiment |
vllm-omni-review-bot
left a comment
There was a problem hiding this comment.
Omni ReviewBot review
PR description
The CosyVoice3 non-async full-payload producer previously required a plain dict, so accumulator-emitted MultimodalPayload values (Mapping but not dict) were rejected and returned None, starving the downstream full-payload wait gate of prompt conditioning. This change accepts any Mapping for the top-level pooling output and for nested embed conditioning, matching the Mapping checks already used elsewhere in the same processor. A focused regression test builds a MultimodalPayload with flat embed.* tensors and asserts embeds are forwarded while codec IDs stay out of the connector payload.
Change flow
flowchart LR
A["[EXISTING] Accumulator emits MultimodalPayload"]:::existing
B["[CHANGED] text2flow_full_payload accepts Mapping"]:::changed
C["[NEW] structured MultimodalPayload regression test"]:::new
D["[EXISTING] Connector payload embed + full-payload wait gate"]:::existing
A --> B
B --> D
C --> B
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
CI at
bee3f0dfb5f5(2026-09-30T21:43:12.492102+00:00): required check(s) blocking:buildkite/vllm-omni(missing).
No actionable findings.
🤖 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!
|
Self-review at source head Checked the full-payload Mapping contract for MultimodalPayload and nested embed mappings. Its regression preserves prompt speech tokens while excluding codec IDs. Exact source diff, Python compilation and applicable hooks were checked. Full AMD CosyVoice with #8343 passes all three cases in c30 #13061/#13064, including reference voice cloning. Those combined observations do not certify this source head's standalone unit/full-suite gates. Those gates and human approval remain. |
Signed-off-by: andyluo7 <andy.luo@amd.com>
bee3f0d to
e66f33a
Compare
vllm-omni-review-bot
left a comment
There was a problem hiding this comment.
Omni ReviewBot review
PR description
This PR widens CosyVoice3’s non-async full-payload producer so text2flow_full_payload accepts any collections.abc.Mapping, not only plain dict. That matches structured MultimodalPayload objects from the output accumulator, which implement Mapping but fail a dict type gate and previously returned None. Nested embed conditioning uses the same Mapping check, and a regression test asserts prompt embeddings are kept while codec IDs stay out of the connector payload, so the downstream full-payload wait gate can receive prompt conditioning again.
Change flow
flowchart LR
A["[EXISTING] Accumulator emits MultimodalPayload"]:::existing --> B["[CHANGED] text2flow_full_payload accepts Mapping"]:::changed
B --> C["[EXISTING] Downstream full-payload wait gate"]:::existing
D["[NEW] Structured MultimodalPayload regression test"]:::new --> B
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
CI at
e66f33a47c09(2026-10-03T17:55:16.788449+00:00): required check(s) blocking:pre-commit(pending),buildkite/vllm-omni(missing),docs/readthedocs.org:vllm-omni(pending).
No actionable findings.
🤖 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!
|
Superseded by #8343, which consolidated this CosyVoice full-payload handling and has now merged. Closing this duplicate. |
Current combined AMD evidence - September 30, 2026, 22:53 UTC
The integration at
c30e63d04dc9a54c5c4b43235eb2ce535fe68df3now has two complete AMD passes at the identical commit. Each build has 65/65 jobs finished, every exit 0 and non-soft, with no retries. Both include all five NIGHTLY leaves.Phase time runs from the earliest phase job start to the latest finish; queue and image-build time are excluded. Both READY and MERGE are below 40 minutes in both observations.
Repeat Accuracy evaluates all 1,088 Seed-TTS samples, mean WER 0.3088705089, with zero request, PCM or ASR failures. Daily-Omni evaluates all 1,197 samples, accuracy 0.7218045113, with zero HTTP or parse failures. Both result JSON files are downloaded and SHA-256 verified against Buildkite metadata. Full CosyVoice passes all 3 cases in 2,317.40s, including both reference voice-clone cases.
These results exercise the combined c30 tree. They establish combined implementation and interaction coverage; they do not establish a second standalone run at a different source PR SHA. Earlier 024 results remain separate evidence.
CUDA #16446 at c30 still has a Documentation Whisper-small transcript failure against the unchanged 0.8 gate; WAV/JSON retention is qualified, and the exact-model replay remains pending. Intel #9379 failed during a dependency download before runtime testing. Source PR checks and maintainer review remain separate merge gates.
Source head:
bee3f0dfb5f5c6b2504109c899dea8897d8f3787. 2 shared changed file(s) also contain other repairs in c30. The standalone source checks are classified in the retained validation history.Retained validation history
Summary
Mappingin the CosyVoice3 full-payload producer, including the structuredMultimodalPayloademitted by the output accumulatorembedconditioning as well as plain dictionariesRoot cause
The non-async CosyVoice3 connector path receives a
MultimodalPayload, which implementsMappingbut is not adict.text2flow_full_payloadrejected that valid payload and returnedNone, leaving the downstream full-payload wait gate without the prompt conditioning it needs.Validation
pre-commit run ruff-check --files vllm_omni/model_executor/stage_input_processors/cosyvoice3.py tests/model_executor/stage_input_processors/test_cosyvoice3_stage_input_processors.pypython3 -m py_compile vllm_omni/model_executor/stage_input_processors/cosyvoice3.py tests/model_executor/stage_input_processors/test_cosyvoice3_stage_input_processors.py