Skip to content

[Bugfix][Qwen3-TTS] Reject task mismatches before EngineCore dispatch - #6113

Merged
linyueqian merged 11 commits into
vllm-project:mainfrom
gxxx-hum:checkout-tts-params
Sep 2, 2026
Merged

linyueqian merged 11 commits into
vllm-project:mainfrom
gxxx-hum:checkout-tts-params

Conversation

@gxxx-hum

@gxxx-hum gxxx-hum commented Aug 12, 2026 •

Copy link
Copy Markdown
Contributor

Purpose

Qwen3-TTS lacks robust validation between the requested task_type and the loaded checkpoint variant, including requests using uploaded voices that are later handled as Base.

For example, a Base request sent to a 1.7B CustomVoice checkpoint reaches incompatible preprocessing. The resulting 1024/2048 embedding mismatch triggers a fatal aclnnCat error, crashes EngineCore, and takes down the online service.

This PR:

  • Determines the loaded Qwen3-TTS variant from
    hf_config.tts_model_type first.
  • Falls back to strictly delimited model-path markers when the metadata is
    absent or unrecognized, including checkpoints stored under dated snapshot
    directories.
  • Avoids inferring variants from unrelated path components such as
    base_models or database.
  • Treats uploaded and precomputed stored voices as Base before checkpoint
    compatibility validation, matching the task ultimately dispatched by
    _build_tts_params.
  • Returns HTTP 400 for a task/checkpoint mismatch when the checkpoint variant
    can be identified.

Built-in CustomVoice preset speakers are not treated as stored voices and
continue to use the CustomVoice task.

If neither checkpoint metadata nor the model path identifies a known variant,
validation fails open rather than rejecting a potentially valid deployment.

Test Plan

vLLM Version: 0.27.0

vLLM-Omni Commit: #877beeaa4957e8c

Test Result

nohup vllm serve /model/ModelScope/Qwen/Qwen3-TTS-12Hz-1.7B-CustomVoice \
    --port 8000 --omni > vllm.log 2>&1 &
curl -X POST http://localhost:8000/v1/audio/speech \
  -H "Content-Type: application/json" \
  -d '{
    "model": "qwen3-tts-12hz-1p7b-customvoice",
    "input": "测试Base任务模式,音频文件格式base64",
    "task_type": "Base",
    "ref_audio": "data:audio/wav;base64,<BASE64_AUDIO_DATA>",
    "x_vector_only_mode": true,
    "response_format": "wav",
    "stream": false
  }'

Before PR :

StageEngineCoreProc encountered a fatal error.

File "qwen3_tts_talker.py", line 701, in preprocess
    self._prompt_builder.build_prompt_embeds(task_type=task_type, info_dict=info_dict)

File "prompt_embeds_builder.py", line 1233, in build_prompt_embeds
    codec_input = torch.cat([codec_input_0, speaker_embed, codec_input_1], dim=1)

RuntimeError: aclnnCat failed

AclNN_Parameter_Error(EZ1001):
dim 2 of tensor 1 is [1024], should be equal to tensor 0 [2048].

EngineDeadError: EngineCore encountered an issue.
[Orchestrator] Stage-0 replica-0 is dead
POST /v1/audio/speech HTTP/1.1" 500 Internal Server Error
Shutting down

After PR :

{
  "error": {
    "message": "Qwen3-TTS CustomVoice checkpoint does not support task_type='Base'. Use task_type='CustomVoice' or load the matching Base checkpoint.",
    "type": "BadRequestError",
    "param": null,
    "code": 400
  }
}

Signed-off-by: XIN GAO <1037396230@qq.com>
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

@vllm-omni-review-bot

Copy link
Copy Markdown

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

Module owners: @alex-jw-brooks @linyueqian @NickCao @tzhouam @gcanlin

Please take a look when you have a chance. If you would like an automated review, mention @vllm-omni-review-bot in a comment.

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

Reviewed as a module owner (the bot routed this to entrypoints/model_integration). The direction is right and the diagnosis is clean: one malformed request should never be able to take the service down, and a 400 is the correct answer for a task/checkpoint mismatch. Keying the worker-side check on talker_config.hidden_size is also right — codec_embed is the talker's embed_input_ids, so codec embeddings are hidden_size-dim and speaker_embed is concatenated with them at prompt_embeds_builder.py:1242 with no intermediate projection. That agrees with _get_qwen_tts_expected_speaker_embedding_dim by construction rather than by coincidence, and uploads are already validated against the same number.

I traced the uploaded-voice path expecting to flag the stored_voice widening as a regression, and it is not one — details inline, but the short version is that _build_tts_params already rewrote uploaded voices to task_type="Base" regardless of what validate decided, so the pre-PR request crashed rather than working. That makes the widening an improvement. It is worth one line in the Purpose section anyway, since a reader would not expect it from the current description.

Three things I would like resolved. The path heuristic is wrong in both directions, not just the one I first assumed; the worker-side guard does not do what the description implies; and the tests do not pin the branch the whole design leans on.

Two smaller notes.

The Test Plan lists vLLM 0.26.0 at a4ea67a21, which is [Model] Add soundfile fallback for MiniMax H3 audio loading (#5699) and predates 2ce15ab7a ([Rebase] Rebase to vllm 0.27.0, #5976, merged 11 Aug). main now requires vLLM 0.27.0. The validation logic here is pure Python in the entrypoint layer so I do not expect the verdict to change, but the crash you captured came from a different stack than the one this merges into. I got caught by exactly this on #6003 two days ago: on vLLM 0.26.0 current main does not even reach startup, it dies in the profiling run with an unrelated TypeError that is easy to misread as the bug under test. Worth re-running the after-PR case on main's pinned vLLM if that is cheap for you.

The required buildkite/vllm-omni lane has not run on this head — the five green checks are all GitHub Actions. It needs the ready label, and that lane rather than this review is what should gate the merge.

for candidate in candidates:
if not isinstance(candidate, str):
continue
normalized = re.sub(r"[^a-z]", "", candidate.lower())

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.

[Med] re.sub(r"[^a-z]", "", candidate.lower()) strips path separators along with everything else, so when the candidate is model_config.model the whole path collapses into a single letter blob and directory names upstream of the checkpoint can decide the variant. The heuristic is wrong in both directions.

False positive. A CustomVoice or VoiceDesign checkpoint served from a path that contains base but not its own variant token is classified Base, and then every correctly-formed request for that deployment is rejected at line 65. I transcribed the function and ran it:

detected actual path
CustomVoice CustomVoice Qwen/Qwen3-TTS-12Hz-1.7B-CustomVoice
CustomVoice CustomVoice /models/Qwen3-TTS-12Hz-1.7B-CustomVoice/20260623_01
CustomVoice CustomVoice /data/database/qwen3-tts-customvoice
Base CustomVoice /mnt/base_models/qwen3-tts-1.7b-ckpt
Base CustomVoice /opt/basedir/qwen3-tts-12hz
Base VoiceDesign s3://my-database/ckpt-1000
CustomVoice Base /models/customvoice-exports/Qwen3-TTS-Base

The marker order does limit this: a path containing the real variant token wins, so the trigger needs a path that lacks the variant token and contains another marker. Canonical HF cache paths and repo ids are safe. To be precise about blast radius, it is not literally every request — a request that happens to use the falsely inferred task passes this check and fails later — but every request a correctly configured client sends is rejected, which is a dark deployment either way.

False negative, which I think is the more interesting half. A Base checkpoint under a renamed or generic directory (no base token, no tts_model_type) returns None, the model_variant is not None guard at line 65 skips the rejection entirely, and the original EngineCore-killing crash still happens for that deployment. So the heuristic is simultaneously too broad and too narrow, and the too-narrow case silently reopens the bug this PR exists to close.

On the mitigation: I checked the three official checkpoints out-of-band and all of them do populate the field —

Qwen3-TTS-12Hz-1.7B-CustomVoice   tts_model_type: custom_voice
Qwen3-TTS-12Hz-1.7B-Base          tts_model_type: base
Qwen3-TTS-12Hz-1.7B-VoiceDesign   tts_model_type: voice_design

and your [^a-z] normalisation handles those snake_case values correctly, so official deployments never reach the path branch. But configuration_qwen3_tts.py:476 defaults tts_model_type=None with no enum validation, and test_task_type_variant_falls_back_to_model_path makes the path branch a supported scenario — so fine-tunes, re-exports and older local snapshots land on the fragile heuristic, and those are exactly the checkpoints that live under directories named base_models.

Cheapest fix that keeps the fallback: match on the final path component only and require the marker to be a delimited token rather than a substring — split Path(candidate).name on [-_.] and compare whole segments. Anchoring on the Qwen3-TTS-<rate>-<size>-<Variant> suffix would be tighter still.

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.

Thanks for pointing this out. I updated the logic to use hf_config.tts_model_type first, and only fall back to the final component of the model path when that field is absent.

wav_np, sr = _get_ref_audio()
speaker_embed = self.extract_speaker_embedding(wav_np, sr).view(1, 1, -1)

expected_speaker_dim = int(self._talker_config.hidden_size)

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.

[Med] This guard is worth keeping, but it does not do what the Purpose section implies, and I would not want it landing on the belief that it protects the service.

An exception escaping here is still fatal. stage_engine_core_proc.py:189-194:

except Exception:
    logger.exception("StageEngineCoreProc encountered a fatal error.")
    engine_core._send_engine_dead()
    raise

So for a caller that bypasses the Speech API, this converts RuntimeError: aclnnCat failed / dim 2 of tensor 1 is [1024], should be equal to tensor 0 [2048] into a legible ValueError naming both numbers — a real improvement to the diagnostic — but EngineCore still dies and the service still goes down. The API-layer check at qwen3_tts.py:65 is the only thing that actually keeps the server up.

That is fine as a design; defence in depth at the tensor boundary is the right instinct and I would keep it. I am asking for the description to say so, because "one invalid request can therefore interrupt the entire TTS service instead of returning a client-side validation error" reads as though both layers now prevent the interruption, and only one of them does. It also matters for finding 1: when _get_model_variant() returns None the API check is skipped, and this guard is all that is left — which means a better log line, not a surviving server.

def test_task_type_must_match_loaded_qwen3_tts_variant(self, speech_server, configured_variant, requested_task):
speech_server._tts_model_type = "qwen3_tts"
speech_server.engine_client.model_config = SimpleNamespace(
model=f"Qwen/Qwen3-TTS-12Hz-1.7B-{configured_variant}",

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.

[Low] These tests set both model=f"Qwen/Qwen3-TTS-12Hz-1.7B-{configured_variant}" and hf_config.tts_model_type=configured_variant, so the path encodes the same answer as the field. The test therefore passes even if the field branch were ignored or broken, because the path supplies the variant on its own.

That is the one branch worth pinning, since the field branch is the entire reason the path heuristic in finding 1 is tolerable. A case with a conflicting path and field — say tts_model_type="custom_voice" with model="/mnt/base_models/ckpt" — asserting that the field wins, would pin the priority and would also serve as the regression test for the false positive above. A None-result case (neither field nor a recognisable path) asserting that validation is skipped rather than guessed would cover the false negative.

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.

Thanks,I'll update the tests. 🫶🏼

if request.voice is not None:
request.voice = request.voice.lower()
if request.task_type is None and request.voice in server.precomputed_speakers:
stored_voice = request.voice in server.uploaded_speakers or request.voice in server.precomputed_speakers

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.

[Nit] No change requested — recording the trace so the next reader does not have to redo it, since this looked like a regression to me at first.

Adding uploaded_speakers to the Base-inference condition does change what happens for {"voice": "alice"} with no task_type. On main that request keeps task_type=None through validate (this adapter never calls _apply_uploaded_speaker, it only stats the file), falls to "CustomVoice", and passes because uploads are added to supported_speakers at serving_speech.py:393. With this PR it becomes "Base" and is rejected on a CustomVoice checkpoint.

What makes that an improvement rather than a regression is _build_tts_params at serving_speech.py:2563 and :2572, which already forced params["task_type"] = ["Base"] for uploaded voices and injected the stored embedding or audio, whatever validate had concluded. So on a CustomVoice checkpoint the pre-PR request reached the engine as Base with a mismatched embedding and hit the same torch.cat failure this PR is fixing. A crashing request becoming a clean 400 is strictly better, and on a Base checkpoint the widening is a no-op.

Worth one line in the Purpose section regardless — it changes the observable behaviour of uploaded voices, and a reader of the current description would not expect it.

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.

Okay, I’ll update the PR Purpose accordingly.

@hsliuustc0106 hsliuustc0106 added bug Something isn't working tts code related to tts models labels Aug 13, 2026
Signed-off-by: XIN GAO <1037396230@qq.com>
@gxxx-hum

Copy link
Copy Markdown
Contributor Author

@linyueqian I’ve updated all the code and the PR description, and verified the changes against the latest main branch as of the 0.27.0.

@gxxx-hum
gxxx-hum requested a review from linyueqian August 17, 2026 02:14
stage_keys = frozenset({"qwen3_tts"})
name = "qwen3_tts"

def _get_model_variant(self) -> str | None:

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.

do we need to change other models as well or we can make it as public function for all related models

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.

I’ll check the other TTS adapters. If model-variant resolution is shared, I’ll move it to a common helper/base adapter and reuse it for the related models;models without variants can remain unchanged.

@linyueqian linyueqian added the ready label to trigger buildkite CI label Aug 26, 2026

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

Re-reviewed the delta 0e6f8ff..80fb47f. All three round-1 findings are resolved and I verified each in code rather than taking the replies at face value. But the delta introduces a new blocking regression on the ordinary request path, so this needs one more push before it can land.

The blocker is server.precomputed_speakers in the new validate code. That attribute does not exist on the serving class: every other access in the tree goes through the adapter capabilities, including qwen3_tts.py:149 in this same file 76 lines below, and the line this PR replaced used self.capabilities.precomputed_speakers correctly. Because Python evaluates or left to right, an uploaded voice short-circuits on the first operand and survives, while any built-in precomputed voice reaches the second operand and raises AttributeError inside validate. The new uploaded-voice test uses "alice", an uploaded speaker, so it short-circuits and never evaluates the broken operand, but four pre-existing tests in the same file do hit it. Buildkite build 14179, which started once the ready label went on, confirms it in three separate jobs: Simple - Engine&Entrypoints Test reports 4 failed with AttributeError: 'OmniOpenAIServingSpeech' object has no attribute 'precomputed_speakers', and both TTS - Qwen3-TTS CustomVoice Test and TTS - Qwen3-TTS Base Test fail end to end against a real server with a 400 carrying the same message. Details and the fix inline.

There is also a control-flow gap that predates this PR but matters more now that validate is being presented as the admission gate: _build_tts_params rewrites task_type to Base unconditionally for stored voices, after validation, so an explicit task_type="CustomVoice" with a stored voice is validated as CustomVoice and dispatched as Base. Inline.

On the parts that are right: _get_model_variant is now field-first and terminal, with the path fallback reading only Path(model_path).name and matching the marker as a delimited token at the end. I transcribed the new function and re-ran the table from round 1, and every false positive is gone, each now resolving to None (validation skipped) rather than a wrong rejection, which is the right direction to fail in.

detected actual path
None (was Base) CustomVoice /mnt/base_models/qwen3-tts-1.7b-ckpt
None (was Base) CustomVoice /opt/basedir/qwen3-tts-12hz
None (was Base) VoiceDesign s3://my-database/ckpt-1000
Base (was CustomVoice) Base /models/customvoice-exports/Qwen3-TTS-Base

The Purpose section now states that the worker-side check "is retained only for clearer diagnostics when the Speech API is bypassed; it does not prevent EngineCore termination", which is the distinction I asked for. On tests, test_task_type_variant_config_wins_over_conflicting_path pins the field-over-path priority and test_task_type_variant_path_does_not_use_parent_directories is a regression test for the false positive, and the parametrized variant test now uses an uninformative path so the field branch is genuinely exercised instead of being shadowed by the path. The Test Plan is on vLLM 0.27.0, which clears the stack-mismatch note from round 1. The worker-side dimension guard and its stubs are correct as written. The import re to import regex as re switch is required by tools/pre_commit/check_forbidden_imports.py.

Validation: static reads only, no PR code executed. I transcribed _get_model_variant out of the diff and ran the round-1 path table against it in a scratch interpreter rather than importing anything from the branch, grepped every precomputed_speakers access at this head to confirm the attribute does not exist on the serving class, and read _build_tts_params to confirm the post-validation rewrite. This round also went through an external review panel (Codex, Gemini, DeepSeek); the blocking finding came from that panel and I confirmed it in code before repeating it.

buildkite/vllm-omni is now running on this head as build 14179 after the ready label. That lane rather than this review is what should gate the merge.

@hsliuustc0106 on sharing this across adapters: worth doing, but as a follow-up rather than growing this PR. The field-first-then-path shape generalises; the marker vocabulary does not, so a shared helper wants the variant table passed in per adapter.

if request.voice is not None:
request.voice = request.voice.lower()
if request.task_type is None and request.voice in self.capabilities.precomputed_speakers:
stored_voice = request.voice in server.uploaded_speakers or request.voice in server.precomputed_speakers

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.

[blocking] server.precomputed_speakers does not exist, so this raises AttributeError for every request naming a built-in voice. server.uploaded_speakers is real (serving_speech.py:291), but precomputed_speakers is not an attribute of OmniOpenAIServingSpeech. Every other access in the tree reaches it through the adapter capabilities: serving_speech.py:518, :824, :2449, voxcpm2.py:73, and qwen3_tts.py:149-150 in this same file 76 lines below. The line this PR replaced also had it right, as self.capabilities.precomputed_speakers. There is no __getattr__ on the class, so this is a hard failure rather than a fallback.

Python evaluates or left to right: an uploaded voice satisfies the first operand and short-circuits, so the broken operand is never touched. Any voice that is not uploaded, meaning every built-in precomputed voice, evaluates the second operand and raises inside validate. That is the ordinary request path for this model, and the new uploaded-voice test added here uses "alice", an uploaded speaker, so it short-circuits and does not exercise it.

Buildkite build 14179 confirms this in three jobs. Simple - Engine&Entrypoints Test: 4 failed, 2054 passed, all four AttributeError: 'OmniOpenAIServingSpeech' object has no attribute 'precomputed_speakers', in test_validate_tts_request_basic, test_precomputed_qwen3_voice_infers_base_without_ref_audio, test_precomputed_qwen3_missing_safetensors_is_not_registered and test_precomputed_qwen3_icl_without_ref_code_is_not_registered. TTS - Qwen3-TTS CustomVoice Test and TTS - Qwen3-TTS Base Test both fail against a live server, for example test_qwen3_tts_customvoice.py::test_text_to_audio_001[async_chunk] returning 400 ... Speech generation failed: 'OmniOpenAIServingSpeech' object has no attribute 'precomputed_speakers'. So the regression is confirmed on real hardware, not just by reading.

Fix is one receiver:

Suggested change
stored_voice = request.voice in server.uploaded_speakers or request.voice in server.precomputed_speakers
stored_voice = request.voice in server.uploaded_speakers or request.voice in self.capabilities.precomputed_speakers

Worth adding a case with a precomputed, non-uploaded voice as well, since nothing currently exercises that operand.

if request.task_type is None and request.voice in self.capabilities.precomputed_speakers:
stored_voice = request.voice in server.uploaded_speakers or request.voice in server.precomputed_speakers
if stored_voice and request.task_type is None:
request.task_type = "Base"

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.

[important] Inferring Base only when task_type is None leaves the stored-voice path able to bypass the check this PR exists to add. _build_tts_params at serving_speech.py:2455-2492 sets params["task_type"] = ["Base"] unconditionally in both the uploaded-voice branch and the precomputed-voice branch, and it runs after validate. So a request that names a stored voice and passes task_type="CustomVoice" explicitly is validated as CustomVoice, passes the comparison below, and is then dispatched as Base regardless.

I traced this same rewrite in round 1, where it was what made the uploaded-voice widening an improvement rather than a regression. What changed is that this PR now presents validate as the admission gate, and for stored voices the gate's conclusion is discarded downstream. Whether that ends in the EngineCore crash depends on whether the stored embedding already matches talker_config.hidden_size, and uploads are dimension-checked at upload time, so I am flagging the control-flow gap rather than claiming a guaranteed crash.

The cheap fix is to treat a stored voice as Base regardless of what the client asked for, since that is what actually gets dispatched: drop the and request.task_type is None and let the comparison below run against the effective task. That also makes the mismatch visible as a 400 instead of a silent rewrite.

if not isinstance(model_path, str):
return None
final_component = Path(model_path.rstrip("/\\")).name.lower()
match = re.search(r"(?:^|[-_.])(custom[-_.]?voice|voice[-_.]?design|base)$", final_component)

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.

[suggestion] Anchoring on the final component fixed the false positives, but it also dropped a layout the previous version handled, and the test that covered it was changed rather than kept. Round 1's table had /models/Qwen3-TTS-12Hz-1.7B-CustomVoice/20260623_01 detecting correctly as CustomVoice; under the new regex the final component is 20260623_01, nothing matches, and it returns None. That was the exact path in the old test_task_type_variant_falls_back_to_model_path, and the new parametrization replaced it with /models/base_models/Qwen3-TTS-12Hz-1.7B-CustomVoice, so nothing pins the dated-snapshot case any more.

This is narrower than I first rated it: the official Base, CustomVoice and VoiceDesign configs all carry tts_model_type, which wins before the path is consulted, so it only bites metadata-less checkpoints, meaning fine-tunes and re-exports where configuration_qwen3_tts.py defaults the field to None with no enum validation. Those are also exactly the checkpoints that live under dated export directories. Walking the path components from the last one and taking the first that matches would cover it without reintroducing the parent-directory false positive. If you would rather keep the strict single-component rule, that is defensible, but then the dated-snapshot case deserves an explicit test asserting None so the narrowing is a recorded decision.


if isinstance(configured_variant, str):
normalized = re.sub(r"[^a-z]", "", configured_variant.lower())
return variants.get(normalized)

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.

[suggestion] return variants.get(normalized) makes the field branch terminal even when the value is not one of the three known variants, so a checkpoint carrying a typo or a future variant name returns None and never reaches the path fallback that would have resolved it. Falling through to the path when the field is present but unrecognised would be strictly more informative and costs one line. Fail-open either way, so this is only about not discarding a signal you already have.

…k_type

Signed-off-by: XIN GAO <1037396230@qq.com>
Signed-off-by: XIN GAO <1037396230@qq.com>

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

Re-approving at 7a83f023. My previous approval was drafted against 755b69e6 and the new commit superseded it, so I re-checked rather than letting a stale approval ride.

The delta is documentation only: docs/serving/speech_api.md, the two text_to_speech guides and recipes/Qwen/Qwen3-TTS.md, 235 lines added and no source change. So the technical basis of the previous approval is untouched, and what is worth reviewing is whether the prose matches the behaviour.

It does, including on the subtle point. The docs say an uploaded or precomputed voice "always selects Base", and explicitly that this holds "even when the client supplies a different task_type". That is exactly the unconditional if stored_voice: request.task_type = "Base" normalization, and it is the behaviour I raised as an issue two rounds ago, so having it stated plainly for users is the right outcome rather than an overclaim.

I did chase one apparent contradiction and it turned out to be correct. The docs say precomputed voices follow the Base rule while built-in presets such as vivian and ryan "remain CustomVoice speakers", which reads like a conflict if you assume built-ins are precomputed. They are not. There are three separate sets: uploaded_speakers, capabilities.supported_speakers (the built-in presets, defined in protocol/audio.py) and capabilities.precomputed_speakers (loaded from custom_voice_dir). stored_voice tests only the first and third, so built-in presets genuinely stay on the CustomVoice path. The distinction the docs draw is the one the code makes.

I also checked for the failure mode I would most expect in docs written alongside a validation change, which is promising more than the code delivers. There are no claims of automatic checkpoint or variant detection, and nothing asserting that mismatches are always rejected, which matters because variant resolution deliberately fails open when metadata is absent and the path is uninformative. The docs stay on the right side of that.

Everything from the previous round stands: the precomputed_speakers receiver fix, the unconditional stored-voice normalization, the leaf-upward path walk, and the five tests covering them.

@gxxx-hum

Copy link
Copy Markdown
Contributor Author

@hsliuustc0106 All code and documentation updates are now complete. I also reviewed the other TTS adapters and confirmed that MOSS-TTS is currently the only other model family with multiple variants, which it already handles through its own model-specific private logic. Any shared abstraction can be considered separately in a follow-up PR. This PR is ready to merge.

Signed-off-by: XIN GAO <1037396230@qq.com>
@vllm-omni-review-bot

vllm-omni-review-bot commented Aug 31, 2026 •

Copy link
Copy Markdown

Omni ReviewBot triage note

Automated triage of commit c69bb840bdb2 produced:

  • Priority: high. Prompt maintainer attention is suggested.

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

@hsliuustc0106 hsliuustc0106 added ready label to trigger buildkite CI high priority high priority issue, needs to be done asap and removed ready label to trigger buildkite CI labels Sep 1, 2026
@hsliuustc0106 hsliuustc0106 added ready label to trigger buildkite CI and removed ready label to trigger buildkite CI labels Sep 1, 2026

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

Reviewed again at c69bb840. This head only brings the already approved patch onto current main; git range-diff confirms all four author commits are unchanged, and the merge resolutions preserve both the reviewed Qwen3-TTS validation and the intervening Speech API changes. The branch is mergeable and all four per-PR Buildkite lanes are green. This remains good to land.

Validation: static range-diff, combined-diff, and current CI review. No fork code was executed locally.

@linyueqian
linyueqian merged commit 3ab582c into vllm-project:main Sep 2, 2026
9 checks passed
JoseCarlosGarcia95 pushed a commit to valendra-tech/vllm-omni that referenced this pull request Sep 5, 2026
…vllm-project#6113)

Signed-off-by: XIN GAO <1037396230@qq.com>
Co-authored-by: Hongsheng Liu <liuhongsheng4@huawei.com>
ZhengWG pushed a commit to ZhengWG/vllm-omni that referenced this pull request Sep 8, 2026
…vllm-project#6113)

Signed-off-by: XIN GAO <1037396230@qq.com>
Co-authored-by: Hongsheng Liu <liuhongsheng4@huawei.com>
Signed-off-by: ZhengWG <zwg0606@gmail.com>
khairulkabir1661 pushed a commit to khairulkabir1661/vllm-omni that referenced this pull request Sep 25, 2026
…vllm-project#6113)

Signed-off-by: XIN GAO <1037396230@qq.com>
Co-authored-by: Hongsheng Liu <liuhongsheng4@huawei.com>
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 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.

4 participants