Validate official vLLM Router with srt-slurm

[cquil11]

Summary

• validate the new official vLLM Router frontend from SemiAnalysisAI/srt-slurm
• validate the refactored explicit sgl-router identity; SGL Router functionality already existed through the legacy sglang frontend type
• add B200 DeepSeek V4 MTP AgentX aggregate and 1P1D recipes, plus an H200 GLM-5.2 MTP 2P2D SGL Router recipe
• pin every config to an exact SemiAnalysisAI/srt-slurm commit and preserve every logical backend metrics endpoint
• propagate native-router Slurm terminal failures to GitHub instead of accepting a clean log-tail exit

Current test matrix

• vLLM Router aggregate: two independent DEP8 workers on two B200 nodes
• vLLM Router P/D: one DEP8 prefill plus one DEP8 decode on two B200 nodes, NIXL transfer
• SGL Router aggregate: two TP8/DEP8 workers on two B200 nodes
• SGL Router P/D: two PCP8 prefills plus two TP8/DEP8 decodes on four H200 nodes, Mooncake transfer
• every official arm uses MTP, AgentX, cache reporting, and backend server-metrics prefix validation

Validation

• srt-slurm PR CI and exact-head focused suite pass; full-suite failures are unchanged from pristine main
• InferenceX focused classifier/changelog/gating suites pass; latest targeted rerun after router-prefix ordering and drain hardening: 112 passed
• vLLM Router aggregate and 1P1D green: https://github.com/SemiAnalysisAI/InferenceX/actions/runs/31361949473
• aggregate result: 277 valid / 0 errors / 100% TTFT+ITL
• 1P1D result: 356 valid / 0 errors / 100% TTFT+ITL
• artifact audit proves all logical vLLM endpoints were passed once to AIPerf, scraped, and served successful traffic
• full-duration B200 SGL Router aggregate direct run is green: Slurm 29972, 612 valid / 0 errors, TTFT 98.7%, ITL 98.8%, required sglang: prefix validated
• current official H200 2P2D job: https://github.com/SemiAnalysisAI/InferenceX/actions/runs/31361949473/job/93372605876
• direct B300 SGL Router 1P1D MTP/Mooncake validation is green: Slurm 41708 completed 0:0 on two nodes; router registered exactly 1P+1D; fixed-sequence c1/c2/c8 completed 10/10, 20/20, and 80/80 requests
• explicit per-GPU RDMA mapping eliminated the prior auto-discovery failure: every TP rank discovered exactly one independently perftest-verified HCA, sustained decode batches proved real cross-node KV transfer, and transport-error scans remained zero

The terminal P/D failure isolated by earlier H200 runs was an upstream SGLang Model Gateway correctness bug, not an AgentX threshold or workload issue. The exact fix is under review in sgl-project/sglang#34216 and is pinned here at 9072e0dc5aaf1962b4e2c6f1a51094356b5d3324.

Related srt-slurm PR: SemiAnalysisAI/srt-slurm#2

This remains a draft until the current official H200 SGL Router 2P2D job and the final exact-head integration run complete.

[github-actions]

Thanks for the contribution! Please reach out to respective companies' CODEOWNER to fill in the latest PR_REVIEW_CHECKLIST.md before pinging core maintainer on Slack for review. In order for the signoff PR check bot to trigger, you must follow the PR_REVIEW_CHECKLIST.md template correctly, including the phrase As a PR reviewer and CODEOWNER, I have reviewed this and have.

For PR verification, add the full-sweep-fail-fast label (strongly recommended) to this PR — the benchmark sweep only runs on labeled PRs. Use full-sweep-enabled only if you need matrix jobs to keep running past a failure.

PR authors are responsible for ensuring that after merging, all GitHub Action jobs fully pass. A lot of the time, failures are just flakes and simply re-running the failed jobs will fix it. See GitHub's docs on re-running failed jobs

---

感谢你的贡献！请联系相应公司的 CODEOWNER 填写最新的 PR_REVIEW_CHECKLIST.md，然后再在 Slack 上联系核心维护者进行审阅。为了触发 signoff PR 检查机器人，你必须正确遵循 PR_REVIEW_CHECKLIST.md 模板，包括保留英文语句 As a PR reviewer and CODEOWNER, I have reviewed this and have。

如需进行 PR 验证，请为此 PR 添加 full-sweep-fail-fast 标签（强烈推荐）— 基准测试 sweep 仅在带有标签的 PR 上运行。仅当需要矩阵任务在失败后继续运行时才使用 full-sweep-enabled。

PR 作者有责任确保合并后所有 GitHub Action 任务完全通过。 很多时候失败只是偶发抖动（flake），重新运行失败的任务即可解决。参见 GitHub 关于重新运行失败任务的文档

[claude]

Claude finished @cquil11's task in 7m 29s —— View job

---

Review of PR #2543

• Read full diff and changed files
• Validate master config / perf-changelog rules (entry position, model prefixes, image accessibility)
• Review launch scripts, slurm utils, and new recipe YAMLs
• Post inline comments for any issues
• Post review summary

Summary

One 🟡 warning posted inline on runners/slurm_utils.sh:87: the new sacct success gate changes pass/fail semantics for every existing multinode sweep on gb200-nv and b200-nscale (clusters this PR's validation runs never touch — partial-result TIMEOUT sweeps that previously exported now hard-fail before collection, and accounting lag past the 60s default window will flake healthy runs), while the PR's own three B200 arms go through launch_b200-dgxc.sh's inline tail loop and don't get the guard at all.

Everything else checked out clean:

• Master config + perf-changelog: both updated, new changelog entry correctly appended at end of file; dsv4/glm5.2 prefixes and framework keys are consistent with existing entries; all images are public (Docker Hub vllm/vllm-openai, lmsysorg/sglang).
• Recipe staging: the H200 launcher's generic LOCAL_CONFIG_FILE copy (lines 214–217) stages the new glm5.2 sgl-router recipe, and the B200 branch explicitly copies both deepseek-v4 trees plus configs/, so all four CONFIG_FILE paths and setup_script deps resolve inside the pinned srt-slurm checkout. Exact-commit pinning with post-checkout rev-parse verification is solid.
• Deps scripts: version pins match the master config (vllm-router==0.1.15, sglang-router==0.3.2); the pd-fix wheel build is correctly serialized via flock on the shared /router_wheels mount, which is only wired on the H200 agentic path where it's used. The b200 launcher block guards every command with || exit 1 (it lacks set -e), and h200 runs under set -eo pipefail — both correct.
• Metrics backends: the sgl-router/vllm-router fallbacks in matches() keep the empty-metrics contract intact.

One note (not a code issue): the PR body says pass 3 uses the VLLM_USE_NCCL_SYMM_MEM=1 + NCCL_MNNVL_ENABLE=1 pairing, but the committed recipes ship VLLM_USE_NCCL_SYMM_MEM: "0" with a comment explaining symmetric-memory registration fails on this cluster — worth confirming the description vs. the recipes reflect the final intended configuration. I could not execute runners/test_slurm_utils.py in this environment (pytest invocation not permitted), so test results rely on the author's reported runs.

[claude]

Beyond the inline findings, I also checked whether native-sgl-router-deps.sh's pip install (no --break-system-packages) would fail on an externally-managed image — the sgl-router recipes set PIP_BREAK_SYSTEM_PACKAGES: "1" in both frontend.env and backend.aggregated_environment, which pip honors via env var, so this isn't a real issue.

Extended reasoning...

This PR is explicitly marked WIP/draft by the author, pending on-hardware validation of router traffic, P/D transfer, and metrics reachability. The bug hunting system already found a real terminal-status-verification gap in the B200 launcher plus several nits (CI priority ordering, an idle-drain metric gap, and duplicated launcher logic), so this needs human review regardless. I independently checked one additional candidate concern — a missing --break-system-packages pip flag in the new sgl-router deps script — and confirmed it's a non-issue because the recipe's env already sets PIP_BREAK_SYSTEM_PACKAGES=1 for the containers that run the setup script.

[claude]

🟡 The exact-commit clone/checkout/verify block for SRT_SLURM_REPOSITORY/SRT_SLURM_REF (both-vars guard, git clone, checkout --detach, rev-parse HEAD verification, identical error strings) is copy-pasted near-verbatim into both runners/launch_b200-dgxc.sh (~159-172) and runners/launch_h200-dgxc-slurm.sh (~67-76). This is pre-existing duplication that this PR extends to a second launcher rather than introducing; consider factoring it into a shared clone_srt_slurm_exact_commit() function in runners/slurm_utils.sh, which the H200 launcher already sources for other shared helpers like wait_for_slurm_job_success.

Extended reasoning...

This PR adds the exact-commit clone/checkout/verify block to both runners/launch_b200-dgxc.sh (lines ~159-172) and runners/launch_h200-dgxc-slurm.sh (lines ~67-76) as near-identical copies. Both blocks share: the [[ -n "${SRT_SLURM_REPOSITORY:-}" || -n "${SRT_SLURM_REF:-}" ]] outer guard, the must-be-set-together check with the identical error string "SRT_SLURM_REPOSITORY and SRT_SLURM_REF must be set together", git clone "$SRT_SLURM_REPOSITORY" "$SRT_REPO_DIR", cd "$SRT_REPO_DIR", git checkout --detach "$SRT_SLURM_REF", and the git rev-parse HEAD mismatch check with the identical error string "srt-slurm checkout does not match requested exact commit".

The two copies have already begun to drift: the B200 script runs under set -x without set -e, so its copy appends || exit 1 after every command, while the H200 script runs under set -eo pipefail and omits those suffixes. This is exactly the kind of divergence a shared helper would prevent going forward — today it's cosmetic, but the next edit to one copy (e.g. adding a retry, changing the clone depth, or tightening the ref-format validation) is easy to forget to port to the other.

runners/slurm_utils.sh is already sourced by the H200 launcher and is the established home for shared launcher logic — it already holds wait_for_slurm_job_success and stream_slurm_job_log. Extracting the clone+checkout+verify core into a clone_srt_slurm_exact_commit() function there (returning non-zero on failure, with callers handling || exit 1 vs relying on set -e as they already do for other sourced helpers) would let both launchers call one implementation. The B200 launcher would need one added source "$(dirname "${BASH_SOURCE[0]}")/slurm_utils.sh" line (it doesn't currently source that file), matching the pattern the H200 launcher already uses.

Concretely, a maintainer fixing a bug in this exact-commit-pin logic (e.g., handling a ref that's a branch name vs a full SHA, or adding a shallow-clone optimization) would today need to find and edit it in two separate files, and could easily miss one — as evidenced by the || exit 1 divergence already present between the two copies added in this very PR.

This is a code-quality/DRY observation, not a correctness bug: the duplicated logic itself is correct and functionally equivalent in both places, so it does not block merge. Recommend nit severity.

[github-actions]

see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=31357681380
see unofficial run visualizer at https://inferencex.semianalysis.com/evaluation?unofficialRun=31357681380

[claude]

Claude finished @cquil11's task in 3m 6s —— View job

---

Re-review of PR #2543 (marked ready for review)

• Identify changes since the previous review (commit 32671d2cb "Harden native router validation" + merge from main)
• Review the new diff for bugs / regressions
• Check whether previously flagged issues were addressed
• Post re-review summary

Re-review: no new blocking issues found. The only substantive changes since the last review are commit 32671d2cb and a merge of origin/main (unrelated CollectiveX changes, clean merge). The new commit directly resolves the two main findings from the previous review:

• 🔴 B200 launcher missing success verification — fixed. runners/launch_b200-dgxc.sh now sources slurm_utils.sh and calls wait_for_slurm_job_success "$JOB_ID" || exit 1 after wait $POLL_PID (lines 434–438), before result collection, so all three B200 router arms are now covered.
• 🟡 sacct gate changing semantics for unrelated clusters — fixed. The unconditional wait_for_slurm_job_success call was removed from stream_slurm_job_log in runners/slurm_utils.sh, restoring legacy behavior for gb200-nv/b200-nscale. Both launchers now apply the gate only when FRAMEWORK is sgl-router/vllm-router, and the accounting window default doubled from 60s to 120s. runners/test_slurm_utils.py was correctly refactored to test wait_for_slurm_job_success directly (positional arg indices $1/$3/$4 verified against the run_bash harness).

Other changes in the commit check out: the sglang pd-fix pin was moved to 9072e0dc5aaf consistently in both the deps script and the perf-changelog text; enable-prompt-tokens-details: true is a valid vLLM serve flag added to both vLLM Router recipes with a matching changelog line; and the changelog entry remains the last entry in the file.

Two previously flagged non-blocking items remain open and are still applicable (not repeating details): the unreachable vllm-router entry ordering in configs/ci-priority.yaml, and the latent idle-drain metric-name gap in agentic_srt.sh for the sgl-router recipes (harmless while conc-lists are single-element).

Note: this PR is still WIP-gated on its own exact-head validation run per the description — the sacct availability assumption on b200-dgxc will be proven or disproven by that run.

[claude]

Beyond the inline nit, I checked three other candidates and ruled them out: the vLLM disagg recipe reusing one YAML anchor for prefill and decode is fine since NixlConnector with kv_role: kv_both makes vLLM workers role-symmetric (unlike SGLang, which needs separate disaggregation-mode: prefill/decode); native-vllm-router-deps.sh lacking PIP_BREAK_SYSTEM_PACKAGES matches native-sgl-router-deps.sh, which also lacks it, so there is no inconsistency; and AGENTS.md's framework list already omits dynamo-vllm pre-existing this PR, so the missing sgl-router/vllm-router entries are pre-existing doc staleness, not a gap this PR introduces.

Extended reasoning...

This run found one new nit (the CI-priority classifier enum gap, posted inline) on top of the still-unresolved concerns from prior review passes (notably the missing sacct success-gate on the B200 launcher). Beyond the inline findings, I spent time verifying three additional candidates the bug-hunting system had flagged and refuted, confirmed above by reading the actual recipe YAML, deps scripts, and AGENTS.md — none of them are real gaps introduced by this PR. Given the PR is explicitly marked WIP, touches CI scheduling and multi-cluster Slurm launch scripts, and still has an open red-severity finding from a prior pass, this remains a case for human review rather than approval.

[claude]

Claude finished @cquil11's task in 5m 24s —— View job

---

Re-review of PR #2543 (ready_for_review)

• Identify changes since the last review (post-32671d2cb commits)
• Review new diff for bugs / regressions
• Check whether previously flagged items were addressed
• Post re-review summary

Re-review: one 🟡 warning posted inline; both previously open nits are now resolved. The changes since the last review are five commits (094b1aca0, a983e78d7, f58cd40a3, 20a1cd799, 95cc2cc8f) plus a clean merge of unrelated main content.

Previously flagged items — resolved

• 🟡 CI-priority vllm-router ordering — fixed. a983e78d7 moves sgl-router/vllm-router above the broader sglang/vllm prefixes in configs/ci-priority.yaml, so the dedicated router keys are now reachable in _first_prefix_adjustment()'s first-match walk.
• 🟡 Classifier enum gap — fixed. 094b1aca0 adds sgl-router/vllm-router to the JSON-schema enum and prompt bullet in .github/workflows/run-sweep.yml, so the priority classifier can actually emit the criteria the new config keys depend on.
• 🟡 Idle-drain metric gap — fixed. a983e78d7 makes the frontend gauge conditional on dynamo-* frameworks (FRAMEWORK is check_env_vars-guarded, so the case is safe) and adds sglang:num_requests_running/waiting to the worker drain sum. The metric names match the ones already scraped by the existing qwen3.5 single-node sglang scripts, so they're consistent with repo precedent.

New finding

• 🟡 Changelog no longer covers the sgl-router configs it ships (inline on perf-changelog.yaml:5714). The last commit (95cc2cc8f, "test official vLLM Router smoke points") narrowed the changelog entry to only the two vLLM Router keys, but dsv4-fp4-b200-sgl-router-agentic-agg (configs/nvidia-master.yaml:1030) and glm5.2-fp8-h200-sgl-router-agentic-mtp-2p2d (configs/nvidia-master.yaml:7774) are still added by this PR with framework: sgl-router and will run in labeled sweeps with no changelog documentation. Either restore those keys/bullets to the entry or descope the sgl-router configs.

Other changes checked — clean

• Frontend-type renames: the final state is frontend.type: vllm-router for both vLLM recipes and legacy frontend.type: sglang for both SGL Router recipes (20a1cd799 renamed all four, 95cc2cc8f restored the vLLM ones). The asymmetry matches the PR body's statement that SGL Router serves through the legacy sglang frontend type; recipe wiring (setup_script: native-sgl-router-deps.sh, AIPERF_REQUIRED_SERVER_METRIC_PREFIX: "sglang:") is intact, and launcher/metrics-backend routing keys off the unchanged framework: values, not the recipe frontend type. I could not clone srt-slurm in this environment to verify the pinned schema accepts these type names, so that rests on the author's linked green runs at the exact pin.
• srt-slurm pin bumps (f58cd40a3 → 0afc6c206, then 95cc2cc8f → ab98030f0): applied consistently to all four config entries.
• Changelog position: the PR's entry remains the last entry in the file, and the two new description bullets accurately describe the classifier/drain changes.

[github-actions]

see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=31361949473
see unofficial run visualizer at https://inferencex.semianalysis.com/evaluation?unofficialRun=31361949473

[claude]

🟡 WARNING: Commit 95cc2cc8f removed dsv4-fp4-b200-sgl-router-agentic-agg and glm5.2-fp8-h200-sgl-router-agentic-mtp-2p2d from this entry's config-keys (and deleted every SGL Router description bullet), but both config keys are still added by this PR in configs/nvidia-master.yaml (lines 1030 and 7774), still carry framework: sgl-router, and will run in labeled sweeps.

Why it matters: New master-config keys must be documented in perf-changelog.yaml. After this commit the two SGL Router configs this PR introduces have zero changelog coverage — including the sglang-router==0.3.2 pin, the sglang: metric-family requirement, and the H200 Mooncake topology rationale that earlier revisions of this entry documented. The entry's remaining text now describes only the vLLM Router arms while the diff ships four configs.

Fix: Either re-add the two sgl-router keys (with their description bullets) to this entry, or remove the sgl-router config entries and recipes from the PR if they're intentionally descoped from the "official vLLM Router" validation.

[claude]

This automated review ran again and found no new bugs. The two previously-flagged nits are now resolved: vllm-router/sgl-router are correctly ordered ahead of vllm/sglang in configs/ci-priority.yaml, and the priority-classifier's JSON-schema enum in run-sweep.yml now includes both router criteria. This PR is still a draft pending the final H200 SGL Router 2P2D validation run, and it touches multi-node Slurm launch/accounting logic and CI scheduling, so a human look is still worthwhile before merge.

Extended reasoning...

Verified directly against the current repo state that both previously-flagged nits are fixed: configs/ci-priority.yaml:22-23 now places sgl-router/vllm-router before the broader sglang/vllm prefixes, and .github/workflows/run-sweep.yml:252,267 now includes both router strings in the classifier's enum and prompt bullet list. No new bugs were found by the bug hunting system this run. This PR remains large in scope (multi-node Slurm launchers, sacct-based success verification, CI priority scheduling, new native-router recipes) and is still marked draft per its own description, so deferring rather than approving.

[github-actions]

see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=31418542762
see unofficial run visualizer at https://inferencex.semianalysis.com/evaluation?unofficialRun=31418542762