[Misc] Fix skipped max-model-len validation when deriving max model length from tokenizer config #19660

yeqcharlotte · 2025-06-15T08:33:55Z

Purpose

_get_and_verify_max_len attempts to derive max_model_len from a few HF configs then applies a bunch of validation.
Overriding max_model_len through tokenizer after the validation is already done causes some validation to be skipped.

Hence, moving that logic to happen before we execute the logic around none of the max-model-len key is found inside _get_and_verify_max_len.

Introduced in #19201.

Test Plan

Added unit tests

E2E edge case:

vllm serve BAAI/bge-reranker-base --max-model-len 513

Test Result

unit tests passed

max-model-len would throw now while previously it got silently overriden.

File "/home/yeq/uv_env/dsvllm/lib/python3.12/site-packages/pydantic/_internal/_dataclasses.py", line 123, in __init__
    s.__pydantic_validator__.validate_python(ArgsKwargs(args, kwargs), self_instance=s)
pydantic_core._pydantic_core.ValidationError: 1 validation error for ModelConfig
  Value error, User-specified max_model_len (513) is greater than the derived max_model_len (max_position_embeddings=512 or model_max_length=None in model's config.json). This may lead to incorrect model outputs or CUDA errors. To allow overriding this maximum, set the env var VLLM_ALLOW_LONG_MAX_MODEL_LEN=1 [type=value_error, input_value=ArgsKwargs((), {'model': ...attention_dtype': None}), input_type=ArgsKwargs]

Signed-off-by: Ye (Charlotte) Qi <yeq@meta.com>

github-actions · 2025-06-15T08:34:04Z

👋 Hi! Thank you for contributing to the vLLM project.

💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in #pr-reviews, coordinate on features in #feat- channels, or join special interest groups in #sig- channels.

Just a reminder: PRs would not trigger full CI run by default. Instead, it would only run fastcheck CI which starts running only a small and essential subset of CI tests to quickly catch errors. You can run other CI tests on top of those by going to your fastcheck build on Buildkite UI (linked in the PR checks section) and unblock them. If you do not have permission to unblock, ping simon-mo or khluu to add you in our Buildkite org.

Once the PR is approved and ready to go, your PR reviewer(s) can run CI to test the changes comprehensively before merging.

To run CI, PR reviewers can either: Add ready label to the PR or enable auto-merge.

🚀

gemini-code-assist

Summary of Changes

Hello @yeqcharlotte, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses a bug where the validation of the max_model_len parameter could be skipped if the final value was derived from the tokenizer configuration after the initial validation steps. By moving the tokenizer configuration lookup and consideration earlier in the validation process, I ensure that all relevant constraints, including those from the tokenizer, are applied correctly, preventing potential issues like exceeding the model's true maximum sequence length.

Highlights

Configuration Validation: I've refactored the logic for determining and validating the maximum model length (max_model_len) to ensure that the value derived from the tokenizer configuration is considered before the main validation checks are performed. This fixes a bug where tokenizer-derived lengths could bypass validation.
Test Coverage: I've added a new unit test (test_get_and_verify_max_len) specifically to verify the correct behavior of the get_and_verify_max_len function under different scenarios, including cases where the user-provided max_model_len exceeds the derived length from the model or tokenizer config.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configureGemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

The pull request effectively addresses the issue of skipped max_model_len validation. The changes are well-structured, and the addition of unit tests confirms the fix. The reordering of logic ensures tokenizer_config is considered earlier, improving the robustness of the model configuration.

gemini-code-assist · 2025-06-15T08:35:20Z

tests/test_config.py

+    ("model_id", "max_model_len", "expected_max_len", "should_raise"), [
+        ("BAAI/bge-reranker-base", None, 512, False),
+        ("BAAI/bge-reranker-base", 256, 256, False),
+        ("BAAI/bge-reranker-base", 513, 512, True),


Consider adding a test case where max_model_len is a string (e.g., '1k') to ensure the parsing logic handles human-readable formats correctly.

("BAAI/bge-reranker-base", '1k', 512, True),

gemini-code-assist · 2025-06-15T08:35:21Z

vllm/config.py

+            self.tokenizer,
+            trust_remote_code=self.trust_remote_code,
+            revision=self.tokenizer_revision)


Consider adding a comment explaining why tokenizer_config is fetched here, as opposed to being passed down from the caller.

# Fetch tokenizer config to determine model max length. tokenizer_config = try_get_tokenizer_config( self.tokenizer, trust_remote_code=self.trust_remote_code, revision=self.tokenizer_revision)

gemini-code-assist · 2025-06-15T08:35:21Z

vllm/config.py

+            "model_max_length", derived_max_model_len)
+        derived_max_model_len = min(derived_max_model_len,
+                                    tokenizer_model_max_length)


Consider adding a log message here to indicate when model_max_length from the tokenizer config is being used to further limit derived_max_model_len.

tokenizer_model_max_length = tokenizer_config.get( "model_max_length", derived_max_model_len) if tokenizer_model_max_length < derived_max_model_len: logger.info(f"Limiting max model length to {tokenizer_model_max_length} based on tokenizer config.") derived_max_model_len = min(derived_max_model_len, tokenizer_model_max_length)

houseroad

Looks good to me.

DarkLight1337

Thanks for fixing!

noooop · 2025-06-16T04:20:43Z

Thanks for the fix

…ength from tokenizer config (vllm-project#19660) Signed-off-by: Ye (Charlotte) Qi <yeq@meta.com>

…ength from tokenizer config (vllm-project#19660) Signed-off-by: Ye (Charlotte) Qi <yeq@meta.com> Signed-off-by: minpeter <kali2005611@gmail.com>

…ength from tokenizer config (vllm-project#19660) Signed-off-by: Ye (Charlotte) Qi <yeq@meta.com> Signed-off-by: Yang Wang <elainewy@meta.com>

…ength from tokenizer config (vllm-project#19660) Signed-off-by: Ye (Charlotte) Qi <yeq@meta.com>

…ength from tokenizer config (vllm-project#19660) Signed-off-by: Ye (Charlotte) Qi <yeq@meta.com> Signed-off-by: avigny <47987522+avigny@users.noreply.github.com>

…ength from tokenizer config (vllm-project#19660) Signed-off-by: Ye (Charlotte) Qi <yeq@meta.com>

clean up max_model_len

59212c6

Signed-off-by: Ye (Charlotte) Qi <yeq@meta.com>

gemini-code-assist bot reviewed Jun 15, 2025

View reviewed changes

yeqcharlotte requested a review from DarkLight1337 June 15, 2025 08:35

gemini-code-assist bot reviewed Jun 15, 2025

View reviewed changes

houseroad approved these changes Jun 15, 2025

View reviewed changes

houseroad added ready ONLY add when PR is ready to merge/full CI is needed llama Related to Llama models labels Jun 15, 2025

DarkLight1337 approved these changes Jun 15, 2025

View reviewed changes

DarkLight1337 enabled auto-merge (squash) June 15, 2025 14:16

yeqcharlotte added ready ONLY add when PR is ready to merge/full CI is needed and removed llama Related to Llama models ready ONLY add when PR is ready to merge/full CI is needed labels Jun 15, 2025

DarkLight1337 merged commit b692e9c into vllm-project:main Jun 16, 2025
77 checks passed

yeqcharlotte added a commit to yeqcharlotte/vllm that referenced this pull request Jun 22, 2025

[Misc] Fix skipped max-model-len validation when deriving max model l…

ec471c4

…ength from tokenizer config (vllm-project#19660) Signed-off-by: Ye (Charlotte) Qi <yeq@meta.com>

xjpang pushed a commit to xjpang/vllm that referenced this pull request Jun 30, 2025

[Misc] Fix skipped max-model-len validation when deriving max model l…

dab5ff9

…ength from tokenizer config (vllm-project#19660) Signed-off-by: Ye (Charlotte) Qi <yeq@meta.com>

wseaton pushed a commit to wseaton/vllm that referenced this pull request Jun 30, 2025

[Misc] Fix skipped max-model-len validation when deriving max model l…

987870c

…ength from tokenizer config (vllm-project#19660) Signed-off-by: Ye (Charlotte) Qi <yeq@meta.com>

googlercolin pushed a commit to googlercolin/vllm that referenced this pull request Aug 29, 2025

[Misc] Fix skipped max-model-len validation when deriving max model l…

f6739ad

…ength from tokenizer config (vllm-project#19660) Signed-off-by: Ye (Charlotte) Qi <yeq@meta.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

[Misc] Fix skipped max-model-len validation when deriving max model length from tokenizer config #19660

[Misc] Fix skipped max-model-len validation when deriving max model length from tokenizer config #19660

Uh oh!

yeqcharlotte commented Jun 15, 2025 •

edited by github-actions bot

Loading

Uh oh!

github-actions bot commented Jun 15, 2025

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

gemini-code-assist bot Jun 15, 2025

Uh oh!

gemini-code-assist bot Jun 15, 2025

Uh oh!

gemini-code-assist bot Jun 15, 2025

Uh oh!

houseroad left a comment

Uh oh!

DarkLight1337 left a comment

Uh oh!

noooop commented Jun 16, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

[Misc] Fix skipped max-model-len validation when deriving max model length from tokenizer config #19660

[Misc] Fix skipped max-model-len validation when deriving max model length from tokenizer config #19660

Uh oh!

Conversation

yeqcharlotte commented Jun 15, 2025 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Purpose

Test Plan

Test Result

Uh oh!

github-actions bot commented Jun 15, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Jun 15, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jun 15, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jun 15, 2025

Choose a reason for hiding this comment

Uh oh!

houseroad left a comment

Choose a reason for hiding this comment

Uh oh!

DarkLight1337 left a comment

Choose a reason for hiding this comment

Uh oh!

noooop commented Jun 16, 2025

Uh oh!

Uh oh!

Uh oh!

yeqcharlotte commented Jun 15, 2025 •

edited by github-actions bot

Loading