Repository navigation
[Bugfix] Restore Fish Speech and CosyVoice3 audio quality at high concurrency - #8422
Conversation
…currency Track Fish Speech's absolute emitted-frame frontier across adaptive chunk size changes and flush the complete final tail without replaying frames. Preserve the uniform query bound of separate MRV2 decode graphs so FA3 capture matches the runtime GQA schedule, while retaining the mixed FULL prefill bound. Exclude graph padding rows from multimodal query boundaries. Add frame-integrity and real FA3 graph regressions and route them to the H100 ready pipeline. Signed-off-by: Sy03 <1370724210@qq.com>
|
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: @tzhouam @fake0fan @Gaohan123 Routing: @tzhouam via module of the changed files, semantic router, CODEOWNERS; @fake0fan via module of the changed files, semantic router; @Gaohan123 via module of the changed files, semantic router @Sy0307, 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. |
Omni ReviewBot routing recordAssigned Strict on zcode (GLM-5.3-Flash) under experiment |
vllm-omni-review-bot
left a comment
There was a problem hiding this comment.
Omni ReviewBot review
Scan:
| Category | Result |
|---|---|
| Tests / verification | 7 finding(s) below |
| Security | no finding reported |
| Docs / comments | no finding reported |
| Behavior / compatibility | 1 finding(s) below |
| Correctness | no finding reported |
Validated:
- [resolved] empty-terminal payload (fish_speech.py:198-201, 1-D empty even in tensor mode) is benign: _codes_from_runtime_info returns None on numel==0 (fish_speech_dac_decoder.py:274) and the flat path tolerates empty at :311-315/:349-358; residual checked — terminal meta lacks left_context_size, but msgspec omit_defaults (data_entry_keys.py:146-148) keeps the key ABSENT, so the meta merge (omni_connector_payload_transport.py:1446-1458) cannot clobber the accumulated ctx the final trim uses (fish_speech_dac_decoder.py:455)
- [claim-verified] New GPU lane '-m core_model and cuda and H100' (.buildkite/cuda/test-ready.yml:35) matches hardware_test-added marks for both GPU files: tests/helpers/mark.py:160 adds pytest.mark.cuda + cards mark + SKU mark; tests/helpers/tests/test_mark.py:41-47 shows H100/cuda/cards_1 in names.
- [claim-verified] new Buildkite lane markers: hardware_test(res={'cuda':'H100'}) attaches H100+cuda+cards_1 (tests/helpers/mark.py:152-168) and both GPU files have pytestmark core_model, so -m 'core_model and cuda and H100' runs — no collect-then-deselect; source_file_dependencies all exist; label/timeout/placement mirror the sibling 'Simple · CosyVoice3 Packed Flow Test' above it
- [claim-verified] PR body audio math: 25 frames * hop 2048 / 44100 Hz = 1.161 s ≈ the claimed 1.16 s
- [claim-verified] PR numbers: 48 param combinations = 2x3x2x4 (test lines 235-238); 25 frames ~ 1.16 s checks out (25*2048/44100 = 1.161); test_final_backlog_drop reproduces the described 40-frame loss example (frames 1-4 initial, tail 30-40 on main)
- [validated] request_payload reclamation claim (fish_speech.py:181-182) verified: chunk_transfer_adapter.py:1032 pops request_payload[external_req_id] via _clear_sender_state_locked reached from cancel/terminal paths (:1001-1012); omni_connector_payload_transport.py pops _send_side_request_payload at :399-401 and :1511-1513
Of 25 candidates, 6 survive as author-facing comments and 19 are dropped — the drops are almost entirely the adversary lens's no_issue verification notes (6-18, 21, 24) and resolved/excluded decision records (1, 5, 22, 23), none of which ask the author to change anything, and no consensus-2 candidate was dropped. The kept set engages both of the PR's primary fixes: for the prepare_attn gate flip, candidate 19 (minor) asks for a CPU-runnable pin of the new separate_routine arm whose only coverage is a single H100 FA3 lane shared across every worker_v2 backend, and candidate 20 (nit) corrects the new test's sm100-permissive skip guard; for the fish_speech frontier rewrite, candidate 2 (minor) flags the bespoke machinery duplicating the named chunk_size_utils.compute_adaptive_emit helper, candidate 3 (nit, demoted from minor) suggests moving _fish_speech_async_state out of the transported payload, and candidate 4 (nit) removes the now-dead put_req_chunk increments in the new tests; candidate 0 (minor) strengthens the new FA3 parity test by giving k and v distinct values so K/V-content swaps become detectable. Candidate 4's misattributed file path was fixed and every kept comment rewritten to lead with the diff hunk it grounds in; no duplicate pairs were found.
Checked, no defect found:
tests/worker_v2/test_omni_model_state.py:664— Second-round check of this uncovered hunk: the test drives the real OmniModelState.prepare_inputs_embeds (only collaborators are mocked), and the asserted [0, 2, 5] comes from the production slice query_start_loc_np[: num_reqs + 1] at omni_model_state.py:473 — with num_reqs=2 and query_start_loc_np=[0, 2, 5, 5, 5] it selects exactly [0, 2, 5], so the test fails under the removed full-array tolist() pass that included zero-length padding rows. Markers [core_model, cpu] at :22 are collected by the AMD CPU lane (.buildkite/amd/test-amd-merge.yml:90 runs pytest -sv tests/ -m 'core_model and cpu'; its --ignore list covers diffusion/model_executor/entrypoints/engine only, not tests/worker_v2). No change needed.vllm_omni/model_executor/stage_input_processors/fish_speech.py:128— Second-round check of the docstring hunk (:126-131) and SPDX hunk (:1-2): each docstring claim matches the implementation — absolute frontier (:187-192, advanced at :226), contiguous windows (:214, window starts at the stored frontier and ends where the frontier jumps), final-callback flush (finished bypasses the chunk-size gate at :210-211). The added header matches the style carried by 29 of the package's 36 .py files (7 pre-existing files, e.g. voxtral_tts.py, have no SPDX line); nothing in this hunk needs change.
Verdict: COMMENT
CI at
97f2de55fa6d(2026-10-02T10:13:46.370854+00:00): required check(s) blocking:buildkite/vllm-omni(missing).
Findings
- [P2] This test pins the FA3 decode GQA layout by asserting CUDA-graph replay of the… —
tests/worker_v2/test_decode_query_bound_gpu.py
This test pins the FA3 decode GQA layout by asserting CUDA-graph replay of the captured metadata matches the runtime AOT schedule, but k and v are the same tensor —k = v = torch.empty(8, 2, 64, device=device, dtype=torch.bfloat16)at :70 — and all three impl.forward calls (:84 warm-up, :88 capture, :94 eager reference) receive that aliased pair. A K/V role swap between the captured metadata md (:76) and the runtime metadata eager_md (:92) therefore reads identical bytes on both sides and torch.testing.assert_close(out, expected, atol=0.002, rtol=0.002) at :99 still passes. Use two separate randn calls for k and v so such a crossing changes the result.
Evidence: Both decisive lines are in this PR's diff (the whole file tests/worker_v2/test_decode_query_bound_gpu.py is new here): line 70 k = v = torch.empty(8, 2, 64, device=device, dtype=torch.bfloat16) binds k and v to one tensor object; line 99 torch.testing.assert_close(out, expected, atol=0.002, rtol=0.002) is the only output-equality check, and the compared sides are produced by line 88 impl.forward(layer, q, k, v, cache, md, out) (capture) vs line 94 impl.forward(layer, q, k, v, cache, eager_md, expected) (eager) — both with the same aliased k/v, so identical output holds under any K/V transposition. (Note: the draft cited :98 for assert_close, but :98 is assert md.max_query_len == 1; the assertion is at :99.)
Suggestion: k = torch.randn(8, 2, 64, device=device, dtype=torch.bfloat16)
v = torch.randn(8, 2, 64, device=device, dtype=torch.bfloat16)
- **[P2] The new conjunct at omni_model_state.py:433 (
and not self.vllm_config.compilat…** —vllm_omni/worker_v2/model_states/omni_model_state.pyThe new conjunct at omni_model_state.py:433 (and not self.vllm_config.compilation_config.cudagraph_mode.separate_routine()) changes capture-time attention binding for every omni-lifecycle stage — OmniModelState(vllm_config, ...) at worker_v2/model_states/__init__.py:45 is the sole production construction (unchanged by this diff; non-omni models fall through to the upstream factory at :46). Yet both arms of the gate are pinned only by H100+FA3 GPU tests: the new decode arm by tests/worker_v2/test_decode_query_bound_gpu.py and the preserved worst-case arm by the existing FULL-mode test_capture_query_bound_gpu.py (both@hardware_test(res={"cuda": "H100"}), both with Hopper skip guards). No CPU-runnable test pins the gate itself: repo-wide,for_capture` appears in tests only in those two GPU files. If the single H100 lane does not run, a regression of either arm lands silently on every backend. Please add a CPU test in tests/worker_v2/test_omni_model_state.py that patches DefaultModelState.prepare_attn and asserts the input_batch reaching super() keeps max_query_len None under a separate-routine cudagraph config and becomes num_tokens under FULL; and note in the PR body that non-FA3 backends inherit the uniform bound under separate-routine modes.
Evidence: In the diff: vllm_omni/worker_v2/model_states/omni_model_state.py:430-434 if ( for_capture and input_batch.max_query_len is None and not self.vllm_config.compilation_config.cudagraph_mode.separate_routine() ): — decisive conjunct is line 433. Unchanged by this diff, present in the PR-time tree: vllm_omni/worker_v2/model_states/init.py:45 return OmniModelState(vllm_config, model, encoder_cache, device) (sole in-repo production construction; lines 37-40 gate it on has_preprocess/has_postprocess/have_multimodal_outputs). Unchanged by this diff, present in the PR-time tree: tests/worker_v2/test_capture_query_bound_gpu.py:22 @hardware_test(res={"cuda": "H100"}, num_cards=1), :41 c.compilation_config.cudagraph_mode = CUDAGraphMode.FULL, :70 prepare(state, batch, CUDAGraphMode.NONE, (tables,), slots, [[group]], kvconfig, for_capture=True) — the only existing pin of the worst-case arm, GPU-only. Negative check (repo-wide grep, PR-time tree): for_capture in tests/ matches only test_capture_query_bound_gpu.py:70 and test_decode_query_bound_gpu.py:76; grep 'prepare_attn|for_capture|separate_routine|cudagraph|max_query_len' tests/worker_v2/test_omni_model_state.py returns no matches; live separate_routine code matches only omni_model_state.py:433 (other 4 hits are comments, e.g. vllm_omni/worker/gpu_model_runner.py:1006).
Suggestion: def test_capture_query_bound_tracks_config_graph_mode():
state = object.new(OmniModelState)
state.vllm_config = SimpleNamespace(
compilation_config=SimpleNamespace(cudagraph_mode=CUDAGraphMode.FULL_DECODE_ONLY)
)
batch = SimpleNamespace(max_query_len=None, num_tokens=8)
with patch.object(DefaultModelState, "prepare_attn", return_value={}) as parent:
state.prepare_attn(batch, CUDAGraphMode.FULL, (), torch.zeros((1, 0)), [], None, for_capture=True)
assert parent.call_args[0][0].max_query_len is None # FULL-mode twin asserts 8
- [P2] Non-blocking, evidence request: this rewrite is motivated by backlog-loss arith… —
fish_speech.py:205
Non-blocking, evidence request: this rewrite is motivated by backlog-loss arithmetic —_select_backlog_chunk_size(fish_speech.py:57) changes the chunk size mid-stream on load, and the old code re-derived emit boundaries fromlength % steady_chunk_size, so a flip could drop or repeat frames. The new absolute frontier (fish_speech.py:187-189, 192-193, 226) plus the in-tree tests test_backlog_changes_preserve_every_frame_once and test_final_backlog_drop_flushes_entire_pending_tail cover the mechanism. However, the PR body's motivating measurements — C64 counts (470/1088 requests, 7200 missing / 4975 repeated frames) and the claim that 53c7340 and b742f86 only touch unrelated diffusion tests — could not be checked during review (no git or benchmark artifacts available in the review sandbox), so they are recorded as assumptions, not objections. Since these numbers are the entire motivation for the frontier rewrite, please link the benchmark script/output or a repro command so they are independently checkable.
Evidence: fish_speech.py:205 (initial_chunk_size if single_initial_chunk else (chunk_size // initial_chunk_size) * initial_chunk_size) — the anchored arithmetic line inside the new absolute-frontier logic; fish_speech.py:187-188 state = request_payload.setdefault(request_id, {}).setdefault("_fish_speech_async_state", {"emitted_frames": 0, "terminal_sent": False}) and :226 state["emitted_frames"] = length implement the frontier; fish_speech.py:170 steady_chunk_size = _select_backlog_chunk_size(transfer_manager, cfg, chunk_size, length) (selector at :57-81 switches on fish_speech_backlog_load_threshold, default 0.75) shows the chunk size changes mid-stream, confirming the comment's motivation claim. In the PR-time tree, tests/model_executor/stage_input_processors/test_fish_speech_async_chunk.py:275 assert emitted == list(range(1, 141)) (test_backlog_changes_preserve_every_frame_once, :239) and :306 assert emitted == list(range(1, 41)) (test_final_backlog_drop_flushes_entire_pending_tail, :279) corroborate that backlog-change frame preservation is the rewrite's purpose. The C64 counts (470/1088 requests, 7200 missing / 4975 repeated frames) and commit SHAs 53c7340/b742f86 appear in NO file of the PR-time tree — they live only in the GitHub PR body/commit history, outside this read-only sandbox — which is exactly the limitation the draft comment itself discloses; those specific numbers therefore remain unverifiable, not confirmed or refuted.
- [P2] The new terminal_sent latch (init :187-189, check :190-191) does not cover the… —
fish_speech.py:172
The new terminal_sent latch (init :187-189, check :190-191) does not cover the pre-existing zero-frame finish at :172-178: that path returns an empty finished terminal before the latch state is created, so it never sets terminal_sent — its only writers are :197 and :227, both on latch-covered paths. Since finished = bool(is_finished or request.is_finished()) (:133), a repeated EOS with no frames buffered re-enters :172-177 and emits a second finished terminal; consumers are protected only by finished-flag idempotence, not by a structural at-most-one-terminal guarantee. Set terminal_sent in the :172-178 path (via the same request_payload state) so both terminal sites share the invariant, and add a double-EOS zero-frame regression.
Evidence: Unchanged by this diff (context block; the diff's first added line in the region is the comment at :180), present in the PR-time tree: fish_speech.py:172-177 if length <= 0: / if finished: / return OmniPayloadStruct( / codes=CodesStruct(audio=torch.empty(0, dtype=torch.long)), / meta=MetaStruct(finished=torch.tensor(True, dtype=torch.bool)), — no terminal_sent write, and this return precedes the latch: fish_speech.py:190 if state["terminal_sent"]:. Grep over the file shows terminal_sent is assigned only at fish_speech.py:197 state["terminal_sent"] = True and fish_speech.py:227 state["terminal_sent"] = finished. Reachability of the repeat: fish_speech.py:133 finished = bool(is_finished or request.is_finished()).
- [P2] The diff replaces the multi-branch boundary inference with hand-rolled frontier… —
fish_speech.py
The diff replaces the multi-branch boundary inference with hand-rolled frontier bookkeeping: the keptchunk_size // initial_chunk_size * initial_chunk_sizeinitial_coverage formula at :205, the target_chunk_size switch at :209-211, themin(emitted_frames, left_context_size_config)clamp at :213-214, and the terminal_sent latch with post-terminal drop at :190-191/:227.chunk_size_utils.compute_adaptive_emit(chunk_size_utils.py:307-340, already used by qwen3_tts.py:291) implements the same hold-until-target / flush-all-on-finish / empty-finished-sentinel contract overaccumulated_at_last_emit+target_size. Consider delegating the emit decision to it — keeping only the fish-specific target_chunk_size selection and left-context clamp — instead of maintaining a second copy of the frontier logic in this processor.
Evidence: fish_speech.py:193-196 pending = length - emitted_frames / if pending <= 0: / if not finished: / return None and :209-211 target_chunk_size = initial_chunk_size if emitted_frames < initial_coverage else steady_chunk_size / if not finished and pending < target_chunk_size: / return None — hand-rolled frontier logic added by this diff. fish_speech.py:205 (initial_chunk_size if single_initial_chunk else (chunk_size // initial_chunk_size) * initial_chunk_size) keeps the removed formula. Decisive helper is unchanged by this diff, present in the PR-time tree: chunk_size_utils.py:321 new_frames = length - accumulated_at_last_emit, :323 if finished and new_frames <= 0: return True, 0, :331-332 if not finished and new_frames < target_size: return False, 0 — same contract. Existing consumer unchanged by this diff: qwen3_tts.py:291-296 emit, context_length = compute_adaptive_emit( / length, / ctrl.accumulated_at_last_emit, / target_size, / finished, / ). Note the draft's ':206-210' for the latch is wrong: fish_speech.py:190-191 if state["terminal_sent"]: / return None (latch set at :197 and :227).
- [P3] This diff stores processor control state inside the transport accumulation buff… —
fish_speech.py
This diff stores processor control state inside the transport accumulation buffer:state = request_payload.setdefault(request_id, {}).setdefault("_fish_speech_async_state", ...)at fish_speech.py:187-189. On the payload-transport runtime therequest_payloadcompat property (omni_connector_payload_transport.py:469-475) aliases_send_side_request_payload, so the control key becomes part of the per-request entry that_accumulate_payloadcopies into every merged payload it returns (merged = dict(origin):1437,return ... dict(merged):1473, assigned into the send path at :1190); the entry is only reclaimed with the payload on completion/cancellation (:1419, :1511-1513; chunk_transfer_adapter.py:1032). Prefer a dedicated side-table keyed by request_id so control keys never ride with the accumulated media payload.
Evidence: In this diff, fish_speech.py:187-188: state = request_payload.setdefault(request_id, {}).setdefault( / "_fish_speech_async_state", {"emitted_frames": 0, "terminal_sent": False}. Unchanged by this diff, present in the PR-time tree: omni_connector_payload_transport.py:469-471 @property / def request_payload(self) -> dict[str, dict[str, Any]]: / return self._send_side_request_payload (compat alias into the accumulation buffer); :1436-1437 origin = self._send_side_request_payload[req_id] / merged = dict(origin) and :1472-1473 self._send_side_request_payload[req_id] = merged / return cast(OmniPayload, dict(merged)) — the buffer holding the control key is what the send path consumes (:1190 payload_data = self._accumulate_payload(external_req_id, payload_data)); reclaim unchanged in tree: :1419 self._send_side_request_payload.pop(cleanup_req_id, None), :1511 self._send_side_request_payload.pop(ext_req_id, None), chunk_transfer_adapter.py:1032 self.request_payload.pop(external_req_id, None).
- [P3] The rewritten slow_ar_to_dac_decoder_async_chunk tracks the output frontier sol… —
tests/worker_v2/test_fish_speech_async_chunk.py
The rewritten slow_ar_to_dac_decoder_async_chunk tracks the output frontier solely via the payload state (state["emitted_frames"], fish_speech.py:226); put_req_chunk has zero references left anywhere in fish_speech.py — the old already_sent = put_req_chunk[request_id] * initial_chunk_size branch was removed. Yet the new test test_backlog_changes_preserve_every_frame_once still executes tm.put_req_chunk["req"] += 1 at tests/model_executor/stage_input_processors/test_fish_speech_async_chunk.py:270 (inside 'if out is not None:' at :267), and nothing — code under test or test assertions — ever reads it. This bookkeeping is dead and can be deleted.
Evidence: In-diff, tests/model_executor/stage_input_processors/test_fish_speech_async_chunk.py:267 if out is not None: then :270 tm.put_req_chunk["req"] += 1 — sole remaining use of put_req_chunk in the file, and no assertion reads it. Outside the diff hunks but present in the PR-time tree: whole-file grep of vllm_omni/model_executor/stage_input_processors/fish_speech.py returns zero matches for put_req_chunk; the new frontier state is fish_speech.py:226 state["emitted_frames"] = length (init at :188 "_fish_speech_async_state", {"emitted_frames": 0, "terminal_sent": False}), replacing the removed diff line - already_sent = transfer_manager.put_req_chunk[request_id] * initial_chunk_size.
🤖 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!
| ubatch_idx: int = 0, | ||
| ) -> dict[str, Any]: | ||
| if for_capture and input_batch.max_query_len is None: | ||
| if ( |
There was a problem hiding this comment.
[P2] The new capture gate keys on CUDAGraphMode.separate_routine(), whose truth ta…
The new capture gate keys on CUDAGraphMode.separate_routine(), whose truth table lives in the installed vLLM and was not readable in this review environment (upstream-verify), so correctness here rests on the two GPU tests' opposing assertions plus the new H100 lane. The diff already explains the rationale at this gate ('Separate decode graphs must retain their uniform query bound...') but does not point to the in-repo precedent; extend that comment with one cross-reference so a future vllm-side semantic change is traceable from this call site: vllm_omni/worker/gpu_model_runner.py:1005-1018 applies the same predicate for the same purpose (max_query_len = self.uniform_decode_query_len if uniform_decode else num_tokens, i.e. uniform decode bound under separate_routine, num_tokens otherwise; same pattern at gpu_generation_model_runner.py:655-667, npu_model_runner.py:138-150, npu_generation_model_runner.py:645-657).
Evidence: This diff, vllm_omni/worker_v2/model_states/omni_model_state.py:430-433: if ( / for_capture / and input_batch.max_query_len is None / and not self.vllm_config.compilation_config.cudagraph_mode.separate_routine() — the gate the comment anchors on, with a rationale comment at 440-441 lacking the precedent citation. Unchanged by this diff, present in the PR-time tree: vllm_omni/worker/gpu_model_runner.py:1005-1006 # If cudagraph_mode.decode_mode() == FULL and / # cudagraph_mode.separate_routine(). This means that we are using and gpu_model_runner.py:1018 max_query_len = self.uniform_decode_query_len if uniform_decode else num_tokens — exactly the claimed precedent; sibling copies at gpu_generation_model_runner.py:655+667, npu_model_runner.py:138+150, npu_generation_model_runner.py:645+657, all pre-existing in-repo under vllm_omni/.
linyueqian
left a comment
There was a problem hiding this comment.
Looks good to me. Both fixes address real correctness bugs and the production changes are small.
Fish Speech: keeping an absolute emitted_frames frontier per request is the right model. The old code derived the emitted boundary from the current chunk size, so a 25/50 switch reinterpreted frames that were already sent. Now the chunk size only decides when to emit, each emission covers the contiguous pending span with bounded left context, and completion flushes the whole tail and sends the terminal marker exactly once. The parametrized test (steady, drop at finish, rise midstream, oscillate, with and without the initial chunk, list and tensor payloads) asserts every frame is emitted exactly once, which is the property that was broken. The state lives in request_payload, which the connector already clears on completion and cancellation.
MRv2 decode bound: same narrowing as in #8420, and I agree with it. Separate decode graphs keep their uniform Q=1 bound and only FULL graphs that also serve mixed batches get the worst-case bound. The new decode-bound test against the real FA3 backend covers FULL_AND_PIECEWISE and FULL_DECODE_ONLY.
Two practical notes, neither about the code itself:
- This PR and #8420 make the same change in
omni_model_state.py(with slightly different comments) and add a Buildkite step with the same label intest-ready.yml, so whichever merges second will need a small rebase. The step here also liststest_decode_query_bound_gpu.py, which is the fuller version to keep. - Buildkite has not run on this head yet (no
readylabel when I looked), so the new FA3 bound tests and the Fish tests have only run on your side so far.
| if request_payload is None: | ||
| request_payload = {} | ||
| transfer_manager.request_payload = request_payload | ||
| state = request_payload.setdefault(request_id, {}).setdefault( |
There was a problem hiding this comment.
Small thing: request_payload[request_id] is a shared per-request dict. Other processors keep their state at its top level, and on an AR receiver the connector merges incoming chunk payloads into the same dict. Nesting the frontier under the private _fish_speech_async_state key avoids a clash, the merge keeps unknown keys, and cleanup pops the whole entry, so this is fine. A one-line comment that the key must not collide with payload field names would help the next reader.
Purpose
Restore Fish Speech S2 Pro and CosyVoice3 audio quality under concurrent serving. These are two independent correctness bugs; the production changes preserve the deployment profiles, sampling settings, model precision, and codec/Flow settings.
What broke and why
FULL_AND_PIECEWISEandFULL_DECODE_ONLY.Fix
emitted_framesfrontier in the existing per-request payload. Chunk size controls when to emit, each emission covers the contiguous pending span with bounded left context, and completion flushes the entire tail. Existing connector completion/cancellation cleanup owns this state; no generic connector watermark is added.num_reqs + 1multimodal query boundaries. Zero-length graph padding rows otherwise violate CosyVoice3's strictly increasing boundary contract. This is a secondary boundary correction; the FA3 decode mismatch is the demonstrated quality root cause.Test Plan
vLLM Version: 0.30.0; PyTorch 2.13.0; Transformers 5.14.1; CUDA 13.2.1 container.
vLLM-Omni Commit: base
b742f86136d28423903a1213adde2a62a11067a2. Full generation used frozen maind93e1a8e09d2a0d386c4a58424470d685ee7743cplus the production fixes. The subsequent main changes,53c7340andb742f86, only modify unrelated diffusion tests; runtime, deploy profiles, and relevant test/evaluation files are unchanged. Regression tests ran on53c7340plus the fixes. Their production and test files are byte-identical to the submitted files, and the tests assert actual imported source paths.Runnable regression repro, from the checkout with vLLM 0.30 installed:
Fish coverage includes 48 combinations of initial-chunk policy, tensor/flat payload, and steady/falling/rising/oscillating backlog, separate and repeated EOS callbacks, and the 40-frame final-tail regression. GPU coverage uses real FlashAttention metadata and graph replay for both separate decode modes, plus the existing mixed-prefill test. The new decode tests use a local synthetic model configuration, not downloaded weights or a mocked attention backend. Main negative controls use the same tests with the two production files restored to main.
Buildkite: add a single-H100 ready job with source dependencies on both FA3 test files and
omni_model_state.py; existing CPU lanes collect the Fish and model-state tests.H200 quality/performance protocol:
GPU-62d67334-2671-f5f0-76f8-a9c784c8544c) per generation round; physical GPUs 4/5 were never used. Shared CPU affinity0-47,96-143, not an exclusive server.enforce_eager=trueprovide a healthy reference with unchanged codecs. These references run after the ABBA queues.fish_qwen3_omni_high_concurrency_single_gpu.yaml, stage capacities 64/64, 25/50-frame adaptive chunks, initial chunk 4, DAC fp16; temperature 0.8, top-p 0.9, top-k 30, repetition penalty 1, no request seed.cosyvoice3_full_response_high_concurrency_standard.yaml, Talker 64, two codec replicas of 32. Streaming:cosyvoice3_packed_streaming.yaml, Talker 64, codec 16, standard sampling override. Both: temperature 0.7, top-p 0.8, top-k 20, repetition penalty 1.21, request seed 42. All modes cap output at 2048 tokens.257826d526b718851a5323b8dab72a11522fb0de, with a local wrapper forfile://references,stream_format=audio, and unrounded wall-clock retention. WER uses Whisper large-v3 and main's official Seed-TTS normalization; speaker similarity usespopsoda2002/seedtts-wavlm-sim'swavlm_large_finetune.pthwith WavLM large base weights throughout, cosine ×100. All full WAVs, long outputs, and formal requests are retained and scored. The one-off bridge and raw experiment artifacts are outside this PR.Supplementary benchmark reproduction
Run each server from the vLLM-Omni checkout in its matching environment, on one available H200. Supply the local model snapshot and extracted Seed-TTS root using the variables below; the measured profiles colocate their stages on the visible GPU.
In the frozen SGLang-Omni benchmark environment, save the following compatibility shim outside either repository. It adapts HTTP media fields and retains the measured wall clock; generation and scoring stay in the existing harness.
Save it as
seedtts_vllm_bridge.py, make the frozen SGLang-Omni checkout importable, and run:Restart the server for each main/fixed arm, use distinct output directories, and score the saved full WAVs after generation. The healthy control overrides only stage 0
enforce_eager; all other stage settings inherit the same profile.Test Result
GPU-beba3720-a167-4372-2efa-61dba2bad69c).Fresh EN1088 C64 results, pooled across two independent starts per row:
Fish audio throughput is essentially unchanged (+0.24%), with request throughput -1.39% and slightly longer restored output. The two-start ranges overlap; this is not evidence of a reliable small speedup or regression.
Compared with the healthy Talker-eager alternative, fixed graphs retain higher audio throughput: 62.5% for full response and 20.6% for streaming in these runs. The streaming mean first-audio delay is 3.971s versus 4.320s. This compares graph execution with an eager fallback, not an optimization gain over an already-correct graph implementation.
Against corrupt main, Cosy request throughput decreases (full 39.42 to 33.70; stream 14.17 to 13.13), while audio duration grows (full 2.94 to 5.14s; stream 3.69 to 5.14s). Those main/fix percentages are not equal-quality performance gains. The performance protocol also has different reference-cache history from the earlier C32-then-C64 quality runs; their absolute throughput is not interchangeable.
Sampled peak GPU memory across starts (GiB; includes startup/capture, not allocator-exact):
Fixed Cosy rounds retain 6-9 outputs longer than 30s per 1088; the healthy eager rounds retain 4-11. Maxima are around 77s, and all are included in quality and timing. The measured quality restoration does not claim to remove these rare model/sampling long outputs.
The previously completed causal controls on the same EN128 set were main C1 WER 2.341% / SIM 69.66, main C64 90.225% / 33.39, only Talker eager C64 2.442% / 70.16, and fixed graph C64 1.597% / 70.67. Main/fixed C1 output WAVs were byte-identical for all 128 rows. Fish's traced fixed C32/C64 rounds had zero missing/repeated frames across both independent starts, including warmups.
These are English automated quality and numerical correctness results, not a human MOS study or proof of cross-framework equivalence. Fish SIM remains about one point below the historical SGLang comparison while matching its healthy main C32 range. Cosy retains occasional long outputs; they are included in timing and quality metrics. A corrupt Cosy baseline's shorter audio cannot establish an equal-quality speedup, and two-start/shared-host timing does not establish small percentage gains.
AI assistance: Codex investigated the failures, implemented the fixes and regression tests, ran H200 validation and the full precheck, and drafted this description.