Skip to content

[cherrypick from #42349] [RL] Pass return_indexer_topk through chat completions and sessions - #42646

Merged
guapisolo merged 2 commits into
sglang-milesfrom
jiajun/sglang-miles-chat-session-return-indexer-topk
Oct 21, 2026
Merged

guapisolo merged 2 commits into
sglang-milesfrom
jiajun/sglang-miles-chat-session-return-indexer-topk

Conversation

@guapisolo

@guapisolo guapisolo commented Oct 5, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Port #42349 to sglang-miles: chat/session return_indexer_topk, plus its layer-count tests.

Symptom & Reproduction

  • Symptom: on sglang-miles, /v1/chat/completions with return_indexer_topk: true plus return_meta_info: true returns no meta_info.indexer_topk; a session turn drops the flag the same way, with no error. Miles session rollouts set this flag on chat requests (miles/rollout/session/request_args.py), so indexer replay through sessions silently gets no data.
  • Reproduction: python -m pytest test/registered/unit/entrypoints/openai/test_serving_chat.py -k "internal_request_single or indexer_topk_without_meta_info" expects the flag on GenerateReqInput plus a 400 without return_meta_info; on sglang-miles at 258bc1a8d9 both fail, and with this change both pass. The session drop is at Session.create_req (Root Cause step 3).

Root Cause

  1. ChatCompletionRequest declares no return_indexer_topk, so pydantic drops it.
  2. _convert_to_internal_request therefore never sets GenerateReqInput.return_indexer_topk.
  3. Session.create_req copies return_routed_experts onto the turn's Req, not return_indexer_topk.

Fix

The first commit cherry-picks #42349's two chat/session commits; the hunks are identical. Declare the field on ChatCompletionRequest, pass it into GenerateReqInput, and copy it in Session.create_req. Chat rejects return_indexer_topk without return_meta_info, matching return_sampling_mask, because meta_info is the only place a chat response carries it. /generate, /v1/completions, and requests without the flag are unchanged.

The second commit ports only the tests of #42349's layer-count commit (a4a57b3db1). sglang-miles already returns indexer_topk_num_layers through the scheduler payload, the multi-tokenizer split, and meta_info, and already has #42349's sequence-relative DSA capture, but nothing tested the layer count. The output-streamer test patches get_global_indexer_capturer at sglang.srt.state_capturer.indexer_topk, because sglang-miles imports it lazily in to_payload. The RL test reshapes by the returned meta_info["indexer_topk_num_layers"] instead of a hardcoded count. No production code changes in this commit.

Verification

  • The reproduction above: 2 failed on 258bc1a8d9, 2 passed with this change.
  • At 26336e6f98 (the first commit, on sglang-miles 9d86e9157a), on an H200 devbox whose image tree is 9d86e9157a with only that change applied: test_serving_chat.py and test_session_token_share_unit.py 161 passed, 113 subtests passed.
  • At this head (afd6d7e3eb), CPU on macOS with PYTHONPATH=python: test_serving_chat.py, test_session_token_share_unit.py, test_multi_tokenizer_mixin.py, test_output_streamer_customized_info.py, and test_tokenizer_manager_rid_cleanup.py 246 passed, 163 subtests passed, including the 3 new layer-count tests. With to_payload forced to send indexer_topk_num_layers=None, the output-streamer test fails in both IPC codecs.
  • Not run: the GPU e2e test/registered/rl/test_return_indexer_topk.py (DeepSeek-V3.2, 8 GPUs); only py_compile at this head.

Review Focus

  • OpenAIServingChat._validate_request: new 400 for return_indexer_topk without return_meta_info.
  • Session.create_req: its replay flags must stay in sync with the non-session Req built in scheduler.py.
  • TestOutputStreamerIndexerTopk: its capturer patch target must follow where output_streamer imports get_global_indexer_capturer from.

CI States

Latest PR Test (Base): ❌ Run #39675164268
Latest PR Test (Extra): ❌ Run #39675163815
Latest PR Test (AMD ROCm 10): ❌ Run #39675163936

@github-actions

This comment has been minimized.

…ompletions and sessions

ChatCompletionRequest had no return_indexer_topk field, so pydantic dropped
the flag and chat completions never returned indexer top-k. Session.create_req
also built the turn's Req without the flag, so requests that continue a
session never returned it either. Neither path reported an error.

Add return_indexer_topk to ChatCompletionRequest, reject it without
return_meta_info because meta_info is the only place a chat response carries
it, pass it into GenerateReqInput, and copy it onto the session turn's Req.
The shared fake session request in the session tests sets the field, since
Session.create_req now reads it.
@github-actions

This comment has been minimized.

@guapisolo
guapisolo force-pushed the jiajun/sglang-miles-chat-session-return-indexer-topk branch from 11fb5be to 26336e6 Compare October 17, 2026 20:23
@github-actions

This comment has been minimized.

@guapisolo
guapisolo added this pull request to stack #45664 October 21, 2026 20:42
sglang-miles already returns indexer_topk_num_layers with indexer_topk,
but nothing guards it: no unit test covers the scheduler payload, the
multi-tokenizer split, or meta_info, and the RL test reshapes the flat
int32 bytes with a hardcoded layer count instead of the returned one.

Port the tests from #42349 (a4a57b3). sglang-miles imports the
capturer lazily inside to_payload, so the output streamer test patches
get_global_indexer_capturer at its owning module instead of on
output_streamer.
@github-actions

Copy link
Copy Markdown
Contributor

Required checks failed on afd6d7e; this PR cannot be merged until they pass.

PR title (run)

PR title must use "[Type] scope: title" or "[Type] [Hardware] scope: title". Type must be one of: Feat, Fix, Refactor, Perf, Doc, Test, CI, Chore. Scope is required and may be any module or model name, with unrestricted letter case (letters, numbers, dots, underscores, slashes, or hyphens). Hardware is optional; use a single tag without spaces. Use uppercase hardware tags (AMD, NVIDIA, NPU, MI300X), except for exactly these four spellings: Intel, Apple, Huawei, Ascend. Reverts must use GitHub's default format: Revert "original title". Use one space between fields and after the colon, with a non-empty title and no leading or trailing whitespace. Examples: "[Fix] attention: Fix KV cache indexing", "[Feat] qwen3: Add model support", "[Perf] [AMD] flash-attn: Optimize decode performance".
Edit the title; the check reruns on its own.

@guapisolo
guapisolo merged commit 92fec69 into sglang-miles Oct 21, 2026
69 of 79 checks passed
@guapisolo
guapisolo deleted the jiajun/sglang-miles-chat-session-return-indexer-topk branch October 21, 2026 21:35
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.

1 participant