[Bugfix] Validate speaker in chat endpoint and fix case-insensitive lookup (#2407)
Signed-off-by: reidliu41 <reid201711@gmail.com> Co-authored-by: Hongsheng Liu <liuhongsheng4@huawei.com>
This commit is contained in:
1 parent
c20cac86ce
commit
38dfe56fdf
4 files changed
+169
-6
No files matched your search
@@ -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")
|
||||
@@ -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():
|
||||
|
||||
@@ -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
|
||||
@@ -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,
|
||||
}
|
||||
|
||||
|
||||
Reference in new issue
Block a user