test: validate upstream-native vLLM Router topologies - #2731
Conversation
|
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 For PR verification, add the 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 模板，包括保留英文语句 如需进行 PR 验证，请为此 PR 添加 PR 作者有责任确保合并后所有 GitHub Action 任务完全通过。 很多时候失败只是偶发抖动（flake），重新运行失败的任务即可解决。参见 GitHub 关于重新运行失败任务的文档 |
2 similar comments
|
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 For PR verification, add the 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 模板，包括保留英文语句 如需进行 PR 验证，请为此 PR 添加 PR 作者有责任确保合并后所有 GitHub Action 任务完全通过。 很多时候失败只是偶发抖动（flake），重新运行失败的任务即可解决。参见 GitHub 关于重新运行失败任务的文档 |
|
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 For PR verification, add the 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 模板，包括保留英文语句 如需进行 PR 验证，请为此 PR 添加 PR 作者有责任确保合并后所有 GitHub Action 任务完全通过。 很多时候失败只是偶发抖动（flake），重新运行失败的任务即可解决。参见 GitHub 关于重新运行失败任务的文档 |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Beyond the inline findings, I also checked the ci-priority.yaml framework-prefix change for a vllm/vllm-router collision — _first_prefix_adjustment in utils/ci_priority.py resolves by first match in dict insertion order, and vllm-router is listed ahead of vllm with an identical 0.5 weight, so there's no scoring conflict.
Extended reasoning...
Findings were already reported as CONFIRMED inline comments (missing perf-changelog.yaml entry for the four new recipes, and a stale comment in launch_gb200-nv.sh). I independently verified one of the ruled-out candidate issues from the diff: the new vllm-router: 0.5 entry added to configs/ci-priority.yaml's framework-prefix map, ahead of the existing vllm: 0.5 entry. Reading utils/ci_priority.py, _first_prefix_adjustment iterates the adjustments dict in insertion order and returns on first match (value == prefix or value.startswith(f"{prefix}-")), so placing vllm-router before vllm correctly ensures a vllm-router framework value matches its own entry first — and since both entries carry the same 0.5 adjustment, there is no scoring divergence today regardless of order. This confirms the added inline comment ("Match the explicit Router runtime before the broader vLLM prefix") is accurate and the ordering is intentional, not a latent bug.
Additional findings (outside the current diff — GitHub can't attach inline comments there):
-
🔴
perf-changelog.yaml— PR adds four new multi-node srt-slurm recipes (vllm-router agg/agg-dep8/disagg/disagg-2p2d in configs/nvidia-master.yaml) but does not append a perf-changelog.yaml entry, violating AGENTS.md's non-negotiable rule that every recipe addition requires a new perf-changelog.yaml entry. [also at: configs/nvidia-master.yaml:2617 - AGENTS.md requires every recipe addition to append a perf-changelog.yaml entry ("Every change that can affect…]Extended reasoning...
Downstream consumers of perf-changelog.yaml (e.g. dashboards or release notes generated from it) will have no record of these four new GB200 DeepSeek-R1 vLLM-Router benchmarks, and CI/reviewers relying on this changelog for coverage tracking will silently miss the addition; a correct fix appends a changelog entry at the tail for this recipe/master-config addition.
Verification: normal (per the repo's non-negotiable invariant). AGENTS.md line 21 states an absolute rule: "Every change that can affect benchmark performance and every recipe addition or modification requires a new
perf-changelog.yamlentry. The file is append-only and byte-sensitive... append only at the tail." The diff adds four new recipes — configs/nvidia-master.yaml gains `dsr1-fp4-gb200-vllm-router-a
|
|
||
| # GLM-5.2 and MiniMax-M3 AgentX use v1.0.50 for complete logical-worker | ||
| # metrics discovery across aggregate, DP-attention, and disaggregated topologies. |
There was a problem hiding this comment.
🟡 The pre-existing comment "GLM-5.2 and MiniMax-M3 AgentX use v1.0.50 for complete logical-worker metrics discovery..." was left in place while a new if [[ ... FRAMEWORK == "vllm-router" ]] branch was inserted above the elif it used to document, so the comment now sits over the wrong block (nit).
Extended reasoning...
A maintainer reading the vllm-router branch sees a comment claiming it's about GLM-5.2/MiniMax-M3 v1.0.50 pinning, which is misleading since that block actually pins the SemiAnalysisAI/srt-slurm commit for vllm-router; future edits risk being made in the wrong place based on the stale comment context.
Verification: nit. In runners/launch_gb200-nv.sh the pre-existing comment at lines 404-405 ("GLM-5.2 and MiniMax-M3 AgentX use v1.0.50 for complete logical-worker metrics discovery across aggregate, DP-attention, and disaggregated topologies.") was left in place while the diff inserted a new if [[ "$IS_AGENTIC" == "1" && "$FRAMEWORK" == "vllm-router" ]] branch immediately below it (cloning SemiAnaly
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit d643611. Configure here.
| dp-attn: true | ||
| additional-settings: | ||
| - "SRT_SLURM_REPOSITORY=https://github.com/SemiAnalysisAI/srt-slurm.git" | ||
| - "SRT_SLURM_REF=f7b5fe5f08740667d756785086d5a3e9800ce060" |
There was a problem hiding this comment.
Validated srt-slurm pin replaced
High Severity
This commit replaces the PR-documented and hardware-validated SRT_SLURM_REF (20e4762a…) with f7b5fe5f… across all four router configs and the GB200 launcher default. The PR’s purpose is to validate srt-slurm PR #6 at that exact head, so jobs now exercise a different checkout than the cited evidence.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit d643611. Configure here.
1 participant
Summary
Validate SemiAnalysisAI/srt-slurm #6 through InferenceX AgentX on GB200 using only topologies where a request Router is materially required.
The integration pins srt-slurm
f7b5fe5f08740667d756785086d5a3e9800ce060andvllm-router==0.1.15. It also restores the missing single-node eval-matrix assignment ine2e-tests.yml, preventing no-eval dispatches from evaluatingfromJson('')and failing the workflow wrapper after otherwise successful jobs.Validation matrix
dsr1-fp4-gb200-vllm-router-agentic-aggdsr1-fp4-gb200-vllm-router-agentic-agg-dep8dsr1-fp4-gb200-vllm-router-agentic-disaggdsr1-fp4-gb200-vllm-router-agentic-disagg-2p2dSingle-worker DP1 cases are excluded because they do not demonstrate Router selection or KV-aware/session-affine routing.
All cases use DeepSeek R1 NVFP4 with vLLM 0.25.1, the 256k AgentX corpus, a 163,840-token model/AIPerf context limit,
consistent_hash, and AgentX correlation IDs forwarded asX-Session-ID. AIPerf receives every logical backend metrics URL and requires thevllm:metric family.Hardware validation
Exact-head workflow: 32923452280 (
success)fec5884c03b9bfab36025411e6ea24e7e9c57c2af7b5fe5f08740667d756785086d5a3e9800ce060COMPLETED 0:0vllm:passedCOMPLETED 0:0vllm:passedCOMPLETED 0:0vllm:passedCOMPLETED 0:0vllm:passedThe disaggregated jobs completed 555 two-stage requests each. All four jobs passed TTFT/ITL coverage, uploaded nonempty profile and server-metrics exports plus Router/backend logs, and had empty fatal-log scans. The vLLM Router does not expose the optional auto-discovered metrics endpoint in P/D mode; every explicitly configured backend endpoint was reachable and exported.