Repository navigation
[Frontend] Consolidate startup utils. - #62390
Conversation
mergify
Bot
added
ci/build
cohere
|
@claude review |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Findings marked 🟡 are optional suggestions and need no follow-up push.
Additional findings (outside the current diff — GitHub can't attach inline comments there):
-
🔴
vllm/entrypoints/common/setup.py— Everyvllm servestart and every Rust-frontend launch will crash with AttributeError while logging non-default args.get_non_default_argsimportsFlexibleArgumentParserfromvllm.platforms.interface. That module defines the real class only underTYPE_CHECKING, and at runtime it bindsFlexibleArgumentParser = object. Somake_arg_parser(FlexibleArgumentParser())is passed a bareobject()and fails onparser.add_argument. Fix: import it fromvllm.utils.argparse_utilsas the base did. This restores both callers that pass a Namespace:log_non_default_argsandjsonify_non_default_args.Why this was flagged
vllm/platforms/interface.py:27 imports FlexibleArgumentParser only inside
if TYPE_CHECKING:, and line 33 setsFlexibleArgumentParser = objectat runtime. The new setup.py:297 imports that runtime name, and setup.py:303 callsmake_arg_parser(FlexibleArgumentParser())for any Namespace argument. make_arg_parser (common/cli_args.py:428) immediately callsparser.add_argument(...), which raises AttributeError on a plain object. The Namespace path is the one used by launcher.setup_server via log_non_default_args(args), on everyvllm serve/API server start. It is also used by RustFrontendProcessManager in vllm/v1/utils.py:334 via jsonify_non_default_args(args). On the base branch api_utils imported FlexibleArgumentParser from vllm.utils.argparse_utils, so both paths worked. Only the EngineArgs path, used by LLM in llm.py, still works.Verification: In vllm/entrypoints/common/setup.py, get_non_default_args imports FlexibleArgumentParser from vllm.platforms.interface (line 65) and calls
make_arg_parser(FlexibleArgumentParser())(line 71) for a Namespace. vllm/platforms/interface.py bindsFlexibleArgumentParser = objectat runtime, soparser.add_argument((common/cli_args.py:428) raises AttributeError. The base api_utils.py imported it from vllm.utils.argparse_utils (line 26), so this is a regression. -
🔴
vllm/entrypoints/common/cli_args.py— After this merge the docs build makescli/serve.mdfrom a MagicMock instead of the realvllm serveparser, so the serve CLI reference breaks or comes out empty. docs/mkdocs/gen_files/generate_argparse.py:186 still callsauto_mock("vllm.entrypoints.launchers", "cli_args"). That module no longer exists, soauto_mocktreats theModuleNotFoundErroras a missing dependency and swaps in aPydanticMagicMock. The build raises no error. Fix: point line 186 atauto_mock("vllm.entrypoints.common", "cli_args").Why this was flagged
The mkdocs build runs docs/mkdocs/gen_files/generate_argparse.py. At line 186 it calls
auto_mock("vllm.entrypoints.launchers", "cli_args"). This PR renames vllm/entrypoints/launchers/cli_args.py to vllm/entrypoints/common/cli_args.py, soimportlib.import_module("vllm.entrypoints.launchers.cli_args")raises ModuleNotFoundError with e.name set to that missing module.auto_mock(lines 135-138) handles that by putting aPydanticMagicMockinto sys.modules and returning it. At line 334,create_parser(openai_cli_args.make_arg_parser)then calls a mock. The mock returns a truthy MagicMock, and_parser or parserreturns that mock instead of a real FlexibleArgumentParser. The generatedcli/serve.mdreference therefore has no realvllm serveflags, or formatting the mock fails. On the base branch the module exists and the page lists every serve argument. The repo-wide grep forlaunchers.cli_argsmisses this because the path is split into two string arguments.Verification: Trigger: any mkdocs docs build after this merge. docs/mkdocs/gen_files/generate_argparse.py:186 still has
openai_cli_args = auto_mock("vllm.entrypoints.launchers", "cli_args"). Line 138 then stores aPydanticMagicMockin sys.modules under that name. No error comes out of auto_mock at this point. The realvllm serveparser is never built.
|
Documentation preview: https://vllm--62390.org.readthedocs.build/en/62390/ |
|
@claude review |
There was a problem hiding this comment.
Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.
Additional findings (outside the current diff — GitHub can't attach inline comments there):
-
🟡
tools/pre_commit/mypy.py— nit: The moved offline and setup tests lose the stricter mypy check they had before. The newtests/entrypoints/commondir is not inSILENT_GROUPS, so it falls back to the lenienttestsgroup. Both files used to sit undertests/entrypoints/unit_testsandtests/entrypoints/serve, which are listed. Fix: add"tests/entrypoints/common"toSILENT_GROUPSin tools/pre_commit/mypy.py next totests/entrypoints/unit_tests.Why this was flagged
The PR renames tests/entrypoints/unit_tests/test_offline_utils.py to tests/entrypoints/common/test_offline.py. It also moves TestRedactSensitiveArgs out of tests/entrypoints/serve/utils/test_api_utils.py into tests/entrypoints/common/test_setup.py. tools/pre_commit/mypy.py:30-77 lists
tests/entrypoints/unit_tests(line 44) andtests/entrypoints/serve(line 42) in SILENT_GROUPS. Those paths run in the default group, which uses the stricter follow_imports setting from pyproject.toml.tests/entrypoints/commonis not listed, so in group_files (lines 128-138) it matches thetestsentry in SEPARATE_GROUPS (line 82). That group runs at the lenient setting. On the base branch these tests were type-checked strictly; after the move, type errors in them stop being reported by the pre-commit mypy hook.Verification: In tools/pre_commit/mypy.py, SILENT_GROUPS lists "tests/entrypoints/serve" (line 42) and "tests/entrypoints/unit_tests" (line 44). It has no "tests/entrypoints/common". Every other file under tests/ matches the "tests" entry in SEPARATE_GROUPS (line 82), run with
follow_imports = "skip"(line 183). So the moved tests are now type-checked with imports skipped, a weaker check than they had before.
github-actions
Bot
added
the
ready
|
✅ Triggered Buildkite CI #95845 for commit |
|
/ci run |
|
✅ Triggered Buildkite CI #95863 for commit |
|
/ci run |
|
✅ Triggered Buildkite CI #95864 for commit |
… cli_args and cli_env_setup from vllm.entrypoints.common Root cause: vllm#62390 moved launchers/cli_args.py to common/cli_args.py and cli_env_setup to common/startup.py Upstream: vllm-project/vllm#62390 Fix: point the multi-model server imports at common.cli_args and common.startup, where they live at the pin Signed-off-by: Paweł Olejniczak <pawelx.olejniczak@intel.com>
… cli_args and cli_env_setup from vllm.entrypoints.common (#1910) This PR consolidates 1 hourly-CI fix against vllm@`876d5da8316a81180b8c7f10dea5ffab5b5a849f`. ## Bug 1: Import cli_args and cli_env_setup from vllm.entrypoints.common - **State machine id**: missing_module_vllm_entrypoints_launchers_cli_args - **Commit**: fa53aeb ### Root cause vllm#62390 moved launchers/cli_args.py to common/cli_args.py and cli_env_setup to common/startup.py ### Culprit Probable culprit: [PR #62390](vllm-project/vllm#62390) - pinned by symbol archaeology, bisect not run. Candidate range: [compare](vllm-project/vllm@852989f...876d5da). ### Fix point the multi-model server imports at common.cli_args and common.startup, where they live at the pin
Signed-off-by: wang.yuqi <yuqi.wang@daocloud.io>
#1249 moved vLLM to 0.33.0 but left the `omni` extra on vLLM-Omni `c4ac0642bd`. vLLM 0.33.0 moved `vllm.entrypoints.launchers.cli_args` to `vllm.entrypoints.common.cli_args` (vllm-project/vllm#62390), and that Omni commit still imports the old path, so on current main any Omni entry point fails at import: ``` $ vllm serve --omni --help File ".../vllm_omni/entrypoints/cli/serve.py", line 24, in <module> from vllm.entrypoints.launchers.cli_args import make_arg_parser, validate_parsed_serve_args ModuleNotFoundError: No module named 'vllm.entrypoints.launchers.cli_args' ``` vLLM-Omni v0.33.0rc1 (`4b63581cf6`, vllm-project/vllm-omni#9648) is the Omni commit aligned with vLLM 0.33.0. This PR pins the extra to it. Tested on M1 Pro / 16 GB, macOS 26.6.2, with `./install.sh` and `uv pip install -e '.[dev,omni]'` from this branch (vLLM 0.33.0+cpu, Torch 2.16.0.dev20261013, vLLM-Omni 0.33.0rc1): - The import error above reproduces with the old pin and is gone with the new one. - `VLLM_METAL_BACKEND=mps` resolves the Omni platform to `MPSOmniPlatform`. - Qwen3-TTS-12Hz-0.6B-CustomVoice served with the command in `docs/qwen3-tts-mps.md`; the doc's curl request returns a valid 24 kHz mono WAV. - `pytest -m "not slow" tests/`: 6400 passed, 46 skipped, 352 deselected. #1235 also edits this line (to a fork commit); whichever lands second will need a rebase. CI does not install the `omni` extra, which is why this went unnoticed. I left that out of this PR to keep it to the pin. Signed-off-by: Diao Shengjia <104132148+GoDiao@users.noreply.github.com>
…is a package, ChatTemplateResolutionError in vllm/exceptions.py Classification: version-drift (moved paths and symbols) Source: vllm-project/vllm#60972 (commit 669f85e686, first in v0.33.0) Source: vllm-project/vllm#62390 (commit 5caabdbb65, first in v0.33.0) Before: SKILL.md cited vllm/renderers/chat_utils.py and _get_full_multimodal_text_prompt(); debugging.md labelled five validation errors ValueError. After: vllm/renderers/chat_utils/{chat_template,chat_message,content_parser}.py, get_full_multimodal_text_prompt(), vllm/exceptions.py; the five errors raise VLLMValidationError (read at the v0.33.0 tag; the path-like one already did at v0.32.0). sources.md carries the v0.33.0 line anchors. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
…ypoints/common/cli_args.py at v0.33.0 Classification: deprecation (path removed, no shim) Source: vllm-project/vllm#62390 (merged 2026-10-19), v0.33.0 tree Before: SKILL.md 'Where things live' and custom-parser-plugin.md step 5 point at vllm/entrypoints/launchers/cli_args.py and at import_tool_parser in entry.py/launcher.py After: vllm/entrypoints/common/cli_args.py for the flags and init_parser_plugin, with the v0.29.0-v0.32.0 location kept for older trees Every other path the skill cites was checked against git ls-tree -r v0.33.0 and exists. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
Overview
Consolidate setup utils.
Claims
Validation
Details
Pull Request Checklist
I used vLLM's
/pr-checklistskill. (Mandatory for agents, optional for humans).AI assistance was used during the creation of this PR.
Design Fit: Minimizes impact on core components, reuses existing functionality, and justifies added complexity.
Testing and Validation: Validates the change and ensures any added tests are meaningful and reliable, with CI coverage or documented CI resource constraints and validation performed outside CI.
Code Quality and Style: Keeps code and comments clear and concise, and updates relevant documentation and examples.
Pull Request Contents: Includes a brief summary and relevant links, supports claims with evidence, explains root causes and implementation trade-offs, and follows the contributing guide.