Update qwen3.5-fp8-b300-sglang-mtp SGLang image to v0.5.11-cu130#1346
Conversation
…Ref #1154\n\nCo-authored-by: Klaud Cold <Klaud-Cold@users.noreply.github.com>
|
Thanks for the contribution! For vLLM & SGLang, please ensure that your recipes is similar to the official vLLM recipes and/or the SGLang cookbook If it is not, please create a PR first before we can merge your single node PR into the master branch. Let's ensure that the documentation is first class such that the entire ML community can benefit from your hard work! Thank you 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. If re-running failed jobs is attempted, PR authors are responsible for ensuring it passes. See GitHub's docs on re-running failed jobs: https://docs.github.com/en/actions/how-tos/manage-workflow-runs/re-run-workflows-and-jobs#re-running-failed-jobs-in-a-workflow As a rule of thumb, generally, PR authors should request a review & get a PR approval from the respective companies' CODEOWNERS before requesting a review from core maintainers. If additional help is needed, PR authors can reach out to core maintainers over Slack. |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=25763464221 |
There was a problem hiding this comment.
LGTM — straightforward SGLang image version bump; the XXX placeholder nit in the changelog is flagged inline.
Extended reasoning...
Overview
This PR updates the SGLang Docker image for the qwen3.5-fp8-b300-sglang-mtp config from v0.5.10.post1-cu130 to v0.5.11-cu130 in .github/configs/nvidia-master.yaml, and adds a corresponding entry to perf-changelog.yaml. The diff is two trivial config edits totaling ~7 lines.
Security risks
None. The change is a version bump for an upstream third-party container image used in benchmarking configs; no auth, crypto, permissions, or secret-handling code is involved.
Level of scrutiny
Low. This is a mechanical configuration change matching an established pattern used many times before in the same file. It does not affect production-critical code paths, only benchmark runner configs.
Other factors
A single nit was flagged: the new changelog entry uses pull/XXX instead of the actual PR number (1346) for pr-link. This is documentation-only and produces a broken link, not a functional defect. The inline comment already calls this out so the author can fix it before merge; it does not warrant blocking approval.
| - qwen3.5-fp8-b300-sglang-mtp | ||
| description: | ||
| - "Update SGLang image from v0.5.10.post1-cu130 to v0.5.11-cu130" | ||
| pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/XXX |
There was a problem hiding this comment.
🟡 The new entry in perf-changelog.yaml uses pull/XXX as the pr-link instead of the actual PR number (1346). This will produce a 404 link once merged; replace XXX with 1346 to match the convention used by all other entries in the file.
Extended reasoning...
What the bug is
The new perf-changelog.yaml entry added at line 2351 has pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/XXX. The literal string XXX is an unfilled placeholder — it should be the actual PR number, 1346.
How it manifests
Once this PR is merged, the link https://github.com/SemiAnalysisAI/InferenceX/pull/XXX will resolve to a GitHub 404 page (there is no PR numbered XXX). Anyone clicking the changelog entry to find the source PR will hit a dead link.
Why existing code doesn't prevent it
There appears to be no validation (CI lint, schema check, etc.) that enforces pr-link to be a numeric PR URL — XXX is accepted as a valid string. The convention is enforced socially via the template in AGENTS.md, where XXX is the literal placeholder authors are expected to replace before submitting.
Step-by-step proof
- Look at the new diff hunk in
perf-changelog.yaml:- config-keys: - qwen3.5-fp8-b300-sglang-mtp description: - "Update SGLang image from v0.5.10.post1-cu130 to v0.5.11-cu130" pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/XXX
- Compare to the preceding entries in the same file, which use real PR numbers (e.g.
/pull/1304,/pull/1305,/pull/1308,/pull/1310). - The PR being reviewed is
#1346(from the PR metadata title/number). - Therefore the
XXXshould be1346. - Visiting
https://github.com/SemiAnalysisAI/InferenceX/pull/XXXreturns a 404.
Impact
Low — the changelog is documentation, and the broken link doesn't affect benchmark functionality. However, it breaks the integrity of the changelog and any downstream tooling that parses pr-link references (e.g., automated release notes, changelog browsers).
How to fix
Replace XXX with 1346 on the last line of the new entry:
pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/1346|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=25763464785 |
|
/reuse-sweep-run |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=25783677786 |
Summary
qwen3.5-fp8-b300-sglang-mtpimage fromlmsysorg/sglang:v0.5.10.post1-cu130tolmsysorg/sglang:v0.5.11-cu130Ref #1154
Generated with Claude Code