Skip to content

fix(vllm): serialize routed_experts as base64 with start offset - #10529

Merged
biswapanda merged 7 commits into
mainfrom
routed-experts-base64-start
Jun 10, 2026
Merged

fix(vllm): serialize routed_experts as base64 with start offset#10529
biswapanda merged 7 commits into
mainfrom
routed-experts-base64-start

Conversation

@biswapanda

@biswapanda biswapanda commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Overview:

Make MoE routed-experts capture portable for RL log-prob recomputation: base64-encoded with a start offset so consumers can align the routing to the completion.

Details:

_serialize_routed_experts now:

  • encodes the expert-id bytes as base64 (was base85), matching the encoding consumers already decode;
  • emits a start field (= SamplingParams.routed_experts_prompt_start, the leading prompt rows vLLM trims) so consumers can slice to the completion;
  • keeps shape and dtype so the consumer reads the right element width instead of assuming a fixed type.

The token-generation path forwards start from the request's sampling params (0 when unset).

Where should the reviewer start?

components/src/dynamo/vllm/handlers.py_serialize_routed_experts and its call site in the token-generation path.

Related Issues

🚫 This PR is NOT linked to an issue:

  • Confirmed — no related issue

Summary by CodeRabbit

Release Notes

  • Bug Fixes

    • Updated routed-experts serialization encoding for improved compatibility
    • Enhanced routed expert prompt window alignment with start position tracking
  • Tests

    • Added validation for routed expert prompt positioning

@biswapanda biswapanda self-assigned this Jun 10, 2026
@biswapanda
biswapanda requested review from a team as code owners June 10, 2026 09:37
@github-actions github-actions Bot added fix backend::vllm Relates to the vllm backend labels Jun 10, 2026
@coderabbitai

coderabbitai Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

Routed-experts serialization is updated to use base64 encoding instead of base85 and extended with an optional start offset parameter derived from sampling_params.routed_experts_prompt_start. The token generation path passes this offset, and tests validate both the encoding change and offset behavior.

Changes

Routed-experts serialization with start offset

Layer / File(s) Summary
Routed-experts serialization contract and implementation
components/src/dynamo/vllm/handlers.py
_serialize_routed_experts signature updated to accept optional start: int = 0 parameter. Serialization implementation switches from base85 to base64 encoding and emits start field in returned metadata dict alongside data, shape, and dtype.
Token generation integration
components/src/dynamo/vllm/handlers.py
Token generation call site updated to extract start offset from sampling_params.routed_experts_prompt_start (defaulting to 0 when absent) and pass it to _serialize_routed_experts.
Test updates and new coverage
components/src/dynamo/vllm/tests/test_vllm_worker_handler.py
Existing routed-experts test assertion updated to decode base64 instead of base85. New async test verifies routed_experts.start echoes SamplingParams.routed_experts_prompt_start with conditional skip for older vLLM versions lacking the field.

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 16.67% 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 summarizes the main changes: switching routed_experts serialization from base85 to base64 encoding and adding a start offset parameter.
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.
Description check ✅ Passed The pull request description is well-structured and complete, covering all required template sections with meaningful content.

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


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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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 `@components/src/dynamo/vllm/tests/test_vllm_worker_handler.py`:
- Around line 367-370: The test currently blanket-catches all exceptions when
assigning sampling_params.routed_experts_prompt_start which can hide unrelated
regressions; change the broad except Exception to only catch the specific
unsupported-assignment errors (e.g., except (AttributeError, TypeError):) so
only missing/unsupported attribute assignments trigger pytest.skip and any other
exceptions are allowed to surface and fail the test; locate the assignment to
sampling_params.routed_experts_prompt_start in the test_vllm_worker_handler.py
test and restrict the catch accordingly.
🪄 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: Enterprise

Run ID: f0747d31-d964-4aaa-a22b-dd232f2f8163

📥 Commits

Reviewing files that changed from the base of the PR and between a1ea8a0 and 47acc25.

📒 Files selected for processing (2)
  • components/src/dynamo/vllm/handlers.py
  • components/src/dynamo/vllm/tests/test_vllm_worker_handler.py

Comment thread components/src/dynamo/vllm/tests/test_vllm_worker_handler.py
Comment thread components/src/dynamo/vllm/tests/test_vllm_worker_handler.py Outdated
@biswapanda
biswapanda merged commit 1898840 into main Jun 10, 2026
84 of 85 checks passed
@biswapanda
biswapanda deleted the routed-experts-base64-start branch June 10, 2026 21:34
@biswapanda
biswapanda restored the routed-experts-base64-start branch June 11, 2026 00:56
@biswapanda
biswapanda deleted the routed-experts-base64-start branch June 11, 2026 00:57
Broduker pushed a commit to Broduker/dynamo that referenced this pull request Jun 12, 2026
yao531441 pushed a commit to yao531441/dynamo that referenced this pull request Jun 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend::vllm Relates to the vllm backend fix size/L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants