Preserve rope_scaling when building Eagle3 transformer config

[SuperMarioYL]

Summary

Eagle3Converter._create_transformer_config_from_eagle builds the drafter's LlamaConfig through build_llama_config_rope_kwargs(...), but only forwards rope_theta — it never passes rope_scaling. As a result, any Eagle3 checkpoint converted for a rope-scaled target silently loses its RoPE scaling and falls back to default RoPE.

This is inconsistent with the Eagle v1 converter, which already forwards it:

# src/speculators/convert/eagle/eagle_converter.py
**build_llama_config_rope_kwargs(
    rope_theta=eagle_config.get("rope_theta", 10000.0),
    rope_scaling=eagle_config.get("rope_scaling"),   # <-- forwarded
),

# src/speculators/convert/eagle/eagle3_converter.py (before this PR)
**build_llama_config_rope_kwargs(
    rope_theta=eagle_config.get("rope_theta", 10000.0),
    # rope_scaling dropped
),

build_llama_config_rope_kwargs already fully supports rope_scaling (both the transformers v4 rope_scaling and v5 rope_parameters layouts), so the value was simply not being threaded through.

Impact

Drafters converted for common long-context targets — Llama‑3.1 / Llama‑3.3 (rope_type: "llama3"), linear/dynamic NTK, YaRN — get a transformer config with default RoPE instead of the scaled RoPE, i.e. wrong positional encoding at long context. The conversion succeeds with no error, so the regression is silent.

Fix

Forward rope_scaling from the Eagle config, mirroring the v1 converter (one line).

Tests

Added to tests/unit/convert/test_eagle3_converter.py:

• test_config_preserves_rope_scaling — a llama3 scaling config survives conversion (asserts on rope_parameters for transformers v5 and rope_scaling for v4).
• test_config_no_rope_scaling_when_absent — regression guard: when the Eagle config has no rope_scaling, none is introduced.

The first test fails on main (rope_type stays "default" instead of "llama3") and passes with this change. Full tests/unit/convert/test_eagle3_converter.py passes, and ruff check, ruff format --check, and mypy --check-untyped-defs are clean.

[coderabbitai]

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: fa13e500-aaca-42e4-b44f-cd82f3eb8676

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

📝 Walkthrough

Walkthrough

The PR extends Eagle-3 checkpoint conversion to preserve RoPE scaling configuration. The converter now extracts rope_scaling from the Eagle3 config and passes it to the RoPE builder, enabling the generated LlamaConfig to reflect scaling settings from the source checkpoint. Two tests validate the behavior for both Transformers versions and handle the regression case where scaling is absent.

Changes

RoPE scaling parameter propagation

Layer / File(s)	Summary
RoPE scaling parameter passing and tests src/speculators/convert/eagle/eagle3_converter.py, tests/unit/convert/test_eagle3_converter.py	_create_transformer_config_from_eagle now passes eagle_config.get("rope_scaling") into build_llama_config_rope_kwargs. Tests verify that rope_scaling is correctly mapped to rope_parameters (Transformers v5) or rope_scaling (Transformers v4) when present, and confirm no scaling fields are introduced when the Eagle3 config omits rope_scaling.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

• vllm-project/speculators#462: Introduced the initial RoPE-argument wiring refactor for newer Transformers versions that this PR extends to rope_scaling propagation.

Suggested labels

bug, eagle3

Suggested reviewers

• shanjiaz
• dsikka

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately and clearly summarizes the main change: forwarding rope_scaling in Eagle3 converter configuration to preserve RoPE scaling settings during conversion.
Description check	✅ Passed	The description is comprehensive and directly related to the changeset, explaining the bug, its impact on rope-scaled targets, and the fix with supporting test details.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

Warning

Review ran into problems

🔥 Problems

Linked repositories: Your configuration references 1 linked repositories, but your current plan allows 0. Analyzed ``, skipped vllm-project/vllm.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/unit/convert/test_eagle3_converter.py`:
- Around line 157-158: The test currently assumes llama_config.rope_parameters
is a dict-like and calls .get("factor") which can raise AttributeError when
rope_parameters is None; update the negative assertion to first verify that
llama_config.rope_parameters is not None before calling .get (or assert that it
is None), i.e., guard the access to llama_config.rope_parameters.get("factor")
in the test_eagle3_converter.py negative case so it mirrors the defensive check
used in the positive test and will not call .get on a None value.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 5de00bae-00d1-45f2-b868-6a8ec2a5add1

📥 Commits

Reviewing files that changed from the base of the PR and between 4c7240f and fcb796f.

📒 Files selected for processing (2)

• src/speculators/convert/eagle/eagle3_converter.py
• tests/unit/convert/test_eagle3_converter.py

[fynnsu]

Thanks for the pr!