From 38dfe56fdfd907ee95d249ba9e5547720e51a7af Mon Sep 17 00:00:00 2001 From: Reid <61492567+reidliu41@users.noreply.github.com> Date: Sun, 12 Apr 2026 12:14:16 +0800 Subject: [PATCH] [Bugfix] Validate speaker in chat endpoint and fix case-insensitive lookup (#2407) Signed-off-by: reidliu41 Co-authored-by: Hongsheng Liu --- .../openai_api/test_serving_chat_speaker.py | 111 ++++++++++++++++++ vllm_omni/entrypoints/openai/serving_chat.py | 29 ++++- vllm_omni/entrypoints/openai/utils.py | 30 +++++ .../models/qwen3_omni/qwen3_omni.py | 5 +- 4 files changed, 169 insertions(+), 6 deletions(-) create mode 100644 tests/entrypoints/openai_api/test_serving_chat_speaker.py diff --git a/tests/entrypoints/openai_api/test_serving_chat_speaker.py b/tests/entrypoints/openai_api/test_serving_chat_speaker.py new file mode 100644 index 000000000..3b9151120 --- /dev/null +++ b/tests/entrypoints/openai_api/test_serving_chat_speaker.py @@ -0,0 +1,111 @@ +# SPDX-License-Identifier: Apache-2.0 +# SPDX-FileCopyrightText: Copyright contributors to the vLLM project +"""Tests for chat endpoint speaker validation.""" + +import asyncio +from types import SimpleNamespace +from unittest.mock import AsyncMock, MagicMock + +import pytest + +from vllm_omni.entrypoints.openai.utils import ( + get_supported_speakers_from_hf_config, + validate_requested_speaker, +) + +pytestmark = [pytest.mark.core_model, pytest.mark.cpu] + + +@pytest.fixture +def serving_chat(): + from vllm_omni.entrypoints.openai.serving_chat import OmniOpenAIServingChat + + instance = object.__new__(OmniOpenAIServingChat) + instance._supported_speakers = None + return instance + + +def _make_hf_config(*, speaker_id: dict | None = None, spk_id: dict | None = None): + hf_config = MagicMock() + talker_config = MagicMock() + talker_config.speaker_id = speaker_id + talker_config.spk_id = spk_id + hf_config.talker_config = talker_config + return hf_config + + +def test_validate_requested_speaker_accepts_case_insensitive_value(): + supported = {"vivian", "ethan"} + assert validate_requested_speaker("Vivian", supported) == "vivian" + assert validate_requested_speaker(" vivian ", supported) == "vivian" + + +def test_validate_requested_speaker_rejects_invalid_value_with_supported_list(): + supported = {"vivian", "ethan"} + with pytest.raises(ValueError, match="Invalid speaker 'uncle_fu'. Supported: ethan, vivian"): + validate_requested_speaker("uncle_fu", supported) + + +def test_validate_requested_speaker_skips_validation_when_supported_empty(): + assert validate_requested_speaker("anything", set()) == "anything" + assert validate_requested_speaker(" ", {"vivian"}) is None + + +def test_get_supported_speakers_from_hf_config_uses_spk_id_fallback(): + hf_config = _make_hf_config(speaker_id=None, spk_id={"Serena": 0}) + assert get_supported_speakers_from_hf_config(hf_config) == {"serena"} + + +def test_get_supported_speakers_caches_normalized_keys(serving_chat): + serving_chat.model_config = MagicMock() + serving_chat.model_config.hf_config = _make_hf_config(speaker_id={"Vivian": 0, "Ethan": 1}) + + assert serving_chat._get_supported_speakers() == {"vivian", "ethan"} + + # Cached value should be reused even if the config changes afterwards. + serving_chat.model_config.hf_config.talker_config.speaker_id = {"Serena": 2} + assert serving_chat._get_supported_speakers() == {"vivian", "ethan"} + + +def test_create_chat_completion_converts_value_error_to_error_response(serving_chat): + serving_chat._diffusion_mode = False + serving_chat._check_model = AsyncMock(return_value=None) + serving_chat.engine_client = MagicMock(errored=False) + serving_chat._maybe_get_adapters = MagicMock(return_value=None) + serving_chat.models = MagicMock() + serving_chat.models.model_name.return_value = "test-model" + serving_chat.renderer = MagicMock() + serving_chat.renderer.get_tokenizer.return_value = MagicMock() + serving_chat.reasoning_parser_cls = None + serving_chat.tool_parser = None + serving_chat.use_harmony = False + serving_chat.enable_auto_tools = False + serving_chat.exclude_tools_when_tool_choice_none = False + serving_chat.trust_request_chat_template = False + serving_chat.chat_template = None + serving_chat.chat_template_content_format = "string" + serving_chat.default_chat_template_kwargs = {} + serving_chat._validate_chat_template = MagicMock(return_value=None) + serving_chat._prepare_extra_chat_template_kwargs = MagicMock(return_value={}) + serving_chat._preprocess_chat = AsyncMock( + side_effect=ValueError("Invalid speaker 'uncle_fu'. Supported: ethan, vivian") + ) + serving_chat.create_error_response = MagicMock(return_value="error-response") + + request = SimpleNamespace( + tool_choice=None, + tools=None, + chat_template=None, + chat_template_kwargs=None, + reasoning_effort=None, + messages=[], + add_generation_prompt=False, + continue_final_message=False, + add_special_tokens=False, + request_id="speaker-test", + ) + + result = asyncio.run(serving_chat.create_chat_completion(request)) + + assert result == "error-response" + serving_chat.create_error_response.assert_called_once_with("Invalid speaker 'uncle_fu'. Supported: ethan, vivian") diff --git a/vllm_omni/entrypoints/openai/serving_chat.py b/vllm_omni/entrypoints/openai/serving_chat.py index e84a49aac..39fcbc9a0 100644 --- a/vllm_omni/entrypoints/openai/serving_chat.py +++ b/vllm_omni/entrypoints/openai/serving_chat.py @@ -85,7 +85,11 @@ from vllm_omni.entrypoints.openai.audio_utils_mixin import AudioMixin from vllm_omni.entrypoints.openai.image_api_utils import validate_layered_layers from vllm_omni.entrypoints.openai.protocol import OmniChatCompletionStreamResponse from vllm_omni.entrypoints.openai.protocol.audio import AudioResponse, CreateAudio -from vllm_omni.entrypoints.openai.utils import parse_lora_request +from vllm_omni.entrypoints.openai.utils import ( + get_supported_speakers_from_hf_config, + parse_lora_request, + validate_requested_speaker, +) from vllm_omni.lora.request import LoRARequest from vllm_omni.outputs import OmniRequestOutput @@ -106,6 +110,7 @@ class OmniOpenAIServingChat(OpenAIServingChat, AudioMixin): _diffusion_mode: bool = False _diffusion_engine: AsyncOmni | None = None _diffusion_model_name: str = "" + _supported_speakers: set[str] | None = None @classmethod def for_diffusion( @@ -132,6 +137,18 @@ class OmniOpenAIServingChat(OpenAIServingChat, AudioMixin): instance._diffusion_model_name = model_name return instance + def _get_supported_speakers(self) -> set[str]: + """Load supported speakers from model config (cached).""" + if self._supported_speakers is not None: + return self._supported_speakers + try: + self._supported_speakers = get_supported_speakers_from_hf_config(self.model_config.hf_config) + return self._supported_speakers + except Exception as e: + logger.warning("Could not load speakers from model config: %s", e) + self._supported_speakers = set() + return self._supported_speakers + async def create_chat_completion( self, request: ChatCompletionRequest, @@ -260,7 +277,10 @@ class OmniOpenAIServingChat(OpenAIServingChat, AudioMixin): except (ValueError, TypeError, RuntimeError, jinja2.TemplateError) as e: logger.exception("Error in preprocessing prompt inputs") - return self.create_error_response(f"{e} {e.__cause__}") + message = str(e) + if e.__cause__ is not None: + message = f"{message} {e.__cause__}" + return self.create_error_response(message) request_id = f"chatcmpl-{self._base_request_id(raw_request, request.request_id)}" @@ -540,10 +560,11 @@ class OmniOpenAIServingChat(OpenAIServingChat, AudioMixin): engine_prompt["cache_salt"] = request.cache_salt speaker = getattr(request, "speaker", None) - if speaker is not None and isinstance(speaker, str) and speaker.strip(): + normalized = validate_requested_speaker(speaker, self._get_supported_speakers()) + if normalized is not None: if "additional_information" not in engine_prompt or engine_prompt["additional_information"] is None: engine_prompt["additional_information"] = {} - engine_prompt["additional_information"]["speaker"] = [speaker.lower().strip()] + engine_prompt["additional_information"]["speaker"] = [normalized] language = getattr(request, "language", None) if language is not None and isinstance(language, str) and language.strip(): diff --git a/vllm_omni/entrypoints/openai/utils.py b/vllm_omni/entrypoints/openai/utils.py index 84b28ef5b..f411526fd 100644 --- a/vllm_omni/entrypoints/openai/utils.py +++ b/vllm_omni/entrypoints/openai/utils.py @@ -53,3 +53,33 @@ def parse_lora_request(lora_body: Any) -> tuple[LoRARequest | None, float | None scale = float(lora_scale) if lora_scale is not None else None return LoRARequest(str(lora_name), int(lora_int_id), str(lora_path)), scale + + +def get_supported_speakers_from_hf_config(hf_config: Any) -> set[str]: + """Extract supported speaker names from a model hf_config.""" + config = ( + hf_config.get("talker_config") if isinstance(hf_config, dict) else getattr(hf_config, "talker_config", None) + ) + if config is None: + return set() + + for spk_attr in ("speaker_id", "spk_id"): + speakers_dict = config.get(spk_attr) if isinstance(config, dict) else getattr(config, spk_attr, None) + if speakers_dict and isinstance(speakers_dict, dict): + return {speaker.lower() for speaker in speakers_dict} + return set() + + +def validate_requested_speaker(speaker: str | None, supported_speakers: set[str]) -> str | None: + """Normalize and validate an optional speaker value. + + Returns the normalized speaker string when provided, otherwise ``None``. + Raises ``ValueError`` when the speaker is not in the supported list. + """ + if not isinstance(speaker, str) or not speaker.strip(): + return None + + normalized = speaker.lower().strip() + if supported_speakers and normalized not in supported_speakers: + raise ValueError(f"Invalid speaker '{speaker}'. Supported: {', '.join(sorted(supported_speakers))}") + return normalized diff --git a/vllm_omni/model_executor/models/qwen3_omni/qwen3_omni.py b/vllm_omni/model_executor/models/qwen3_omni/qwen3_omni.py index ed6df6af3..7df694797 100644 --- a/vllm_omni/model_executor/models/qwen3_omni/qwen3_omni.py +++ b/vllm_omni/model_executor/models/qwen3_omni/qwen3_omni.py @@ -610,13 +610,14 @@ class Qwen3OmniMoeForConditionalGeneration( # Speaker token IDs (for voice selection) # In Qwen3, speaker_id mapping is in talker_config.speaker_id + # Keys are lowercased for case-insensitive matching with serving layer. if hasattr(talker_hf_config, "speaker_id") and talker_hf_config.speaker_id: - self.tts_text_spk_token_ids = talker_hf_config.speaker_id + self.tts_text_spk_token_ids = {k.lower(): v for k, v in talker_hf_config.speaker_id.items()} else: # Default to audio_start_token_id if no speaker mapping self.tts_text_spk_token_ids = { "default": talker_hf_config.audio_start_token_id, - "Ethan": talker_hf_config.audio_start_token_id, + "ethan": talker_hf_config.audio_start_token_id, "prefix_caching": talker_hf_config.audio_start_token_id, }