Repository navigation
[Bugfix] Validate served model names in diffusion speech - #8882
linyueqian merged 2 commits into
Conversation
Signed-off-by: liuchenbing <chenliumail@163.com>
Signed-off-by: liuchenbing <chenliumail@163.com>
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
This PR appears to belong to: docs/design/module/error_contracts.md, docs/design/module/observability.md, docs/design/module/profiling.md, docs/design/module/diffusion/index.md. Module owners: @david6666666 @Isotr0py @princepride Routing: @david6666666 via module named in the PR description; @Isotr0py via module named in the PR description; @princepride via module named in the PR description @Liuchenbing-2026, 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. |
|
@NickCao could you review the speech API behavior and regression test when you have time? Fresh author-side technical self-check on 2026-10-14, at head
The executed checks use the unchanged production factory/admission/error methods and real request schema, with constructor infrastructure isolated and generation mocked. They are not a native full-module run, a live HTTP/model test, or an audio-quality/performance result. The current visible build, pre-commit, DCO and documentation statuses are successful; human approval remains pending. Integrated regression command for a supported development environment (provided for reproduction; not executed in this isolated run): python -m pytest tests/entrypoints/openai_api/test_serving_speech.py::test_diffusion_speech_checks_all_served_model_names -m "core_model and cpu" --run-level=core_model -qAI assistance: OpenAI Codex assisted with the code inspection, CPU rerun, and this follow-up. This is an automated technical self-check, not a claim of human review. |
Omni ReviewBot: no human activity for 7 days@Liuchenbing-2026 this pull request has had no human commit, comment or review since 2026-10-14. Please confirm the current plan and next step. The author or a maintainer decides whether to change the PR state. To keep it moving, any one of these is enough: push an update, reply to the open blocker, or post the current plan and timeline. |
linyueqian
left a comment
There was a problem hiding this comment.
Thanks, this looks right. Diffusion-only /v1/audio/speech was skipping _check_model, so passing the served names into for_diffusion and rejecting unknown names before _create_diffusion_speech closes the gap, while an omitted model or a --served-model-name alias still goes through.
Validated on an L20X host with vLLM 0.32.0 running k2-fsa/OmniVoice with --served-model-name k2-fsa/OmniVoice ov-alias, using a fresh server per side and 3 requests per case. On main f7d012b, model: "missing-model" returned 200 with audio all 3 times. On main merged with 8cf32ac (together with #8685, which does not touch the model check), it returned 404 NotFoundError all 3 times, while an omitted model, the model name, and the alias each returned 200. On CPU, test_serving_speech.py passes (351), and the new test fails against main's for_diffusion.
This touches the same create_speech lines as #8883 and #8884, so whichever of the three lands later will need a small rebase.
|
|
||
| @pytest.mark.asyncio | ||
| @pytest.mark.parametrize("model", [None, "test-model", "alias", "missing-model"]) | ||
| async def test_diffusion_speech_checks_all_served_model_names(mocker, monkeypatch, tmp_path, model): |
There was a problem hiding this comment.
[suggestion] This test sits between pytestmark and the module setup (logger, helpers), separate from the other diffusion speech tests. Moving it into TestSpeechAPI alongside the other for_diffusion cases would keep them together.
Contributor intent
1. What outcome do you want to achieve?
Reject unknown model names before diffusion speech generation while keeping omitted model names and served aliases valid.
2. What is the highest-leverage variable for that outcome?
Correct model-name validation at request admission, measured by HTTP404/NotFoundError and zero generation calls for an unknown model.
3. How will this work move that variable?
Carry served names into the diffusion factory and check the request model before dispatch.
4. Whose customer or user needs does this address, and what are those needs?
Developers using OpenAI-compatible speech clients need predictable routing and model-not-found errors.
Purpose
Diffusion-only /v1/audio/speech bypasses the ordinary model-name check. An unknown model currently reaches generation and returns audio. Validate the request against the served names before dispatch, preserve default/name/alias requests, and return HTTP404 with NotFoundError for an unknown model.
Reproduce with an existing diffusion-only speech service:
Test Plan
uv run pytest tests/entrypoints/openai_api/test_serving_speech.py::test_diffusion_speech_checks_all_served_model_names -v uv run pytest tests/entrypoints/openai_api/test_serving_speech.py::test_diffusion_speech_checks_all_served_model_names -m "core_model and cpu" --run-level core_model -v pre-commit run --files vllm_omni/entrypoints/openai/api_server.py vllm_omni/entrypoints/openai/serving_speech.py tests/entrypoints/openai_api/test_serving_speech.pyThe native pytest commands require a supported vLLM development installation. They are reproduction/CI commands; the entire native test module was not run locally.
vLLM Version: Targets current main's vLLM0.31.0 dependency. The local isolated checks do not import the native vLLM runtime.
vLLM-Omni Commit: main
b463516ff4cfe6d3c3e46aaad07f58e6ce63ac40; branch8cf32acc29fa45dfce9ced4cc42f3c0bb71f5987.Test Result
self._adapter.validate/self._adapter.build) reproduce on unmodified main with the same checker. No new mypy errors were introduced. Ruff, formatting, typos, SPDX, forbidden imports, test marks, CUDA-call and TTS-adapter checks passed.AI assistance: OpenAI Codex assisted with investigation, code, regression tests, and this description. The actual validation scope and remaining checks are stated above.