Update dsr1-fp8-h200-sglang SGLang image to v0.5.12-cu130#1423
Conversation
Ref #1154 Co-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. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — straightforward SGLang image version bump.
Extended reasoning...
Overview
This PR is a simple two-line version bump: lmsysorg/sglang:v0.5.11-cu130 → v0.5.12-cu130 for the dsr1-fp8-h200-sglang config, plus a corresponding entry appended to perf-changelog.yaml. No runtime code is touched — only declarative YAML config.
Security risks
None. This is a third-party container image tag bump in a config file; there is no auth, crypto, or permission surface affected. The image registry and namespace are unchanged.
Level of scrutiny
Low. Mechanical config-only change following the exact same pattern as the surrounding recent commits (e.g. 40f8c47, 539ea1b, c09c3e2 — all identical image-bump PRs). The full-sweep-enabled label indicates the benchmark sweep will validate the new image works as expected.
Other factors
The bug hunting system flagged a single nit — the new changelog entry uses pr-link: XXX instead of the actual PR URL. That inline comment is already attached; this is changelog metadata only and does not affect runtime behavior, so it is not worth blocking on. The author can fix it in place before merge.
| - config-keys: | ||
| - dsr1-fp8-h200-sglang | ||
| description: | ||
| - "Update SGLang image from v0.5.11-cu130 to v0.5.12-cu130" | ||
| pr-link: XXX |
There was a problem hiding this comment.
🟡 The new changelog entry for dsr1-fp8-h200-sglang has pr-link: XXX instead of an actual PR URL. Every other entry in this file uses the format https://github.com/SemiAnalysisAI/InferenceX/pull/<number> (e.g. line 2550 uses /pull/1402). It should be https://github.com/SemiAnalysisAI/InferenceX/pull/1423 to match this PR.
Extended reasoning...
What the bug is
The diff appends a new entry to perf-changelog.yaml (lines 2552-2556) recording the SGLang image bump for dsr1-fp8-h200-sglang. The final field is:
pr-link: XXXXXX is a placeholder from the PR-generation template (AGENTS.md uses pull/XXX as the placeholder pattern to be filled in). It was never replaced with the real URL before the PR was opened.
Why the convention says this should be a URL
The file contains over 100 entries, and the standard form is a fully qualified GitHub PR URL — for example, the entry immediately above this one (line 2550) reads:
pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/1402Since this PR is #1423, the correct value is https://github.com/SemiAnalysisAI/InferenceX/pull/1423.
Step-by-step proof
- Open
perf-changelog.yamland jump to the new entry at line 2552. - Observe line 2556:
pr-link: XXX. - Compare to the preceding entry at line 2550:
pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/1402— a proper URL. grep -c "pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/" perf-changelog.yamlshows the URL form is the convention used throughout the file.- The PR number for this change is 1423 (per the PR metadata), so the placeholder resolves to
https://github.com/SemiAnalysisAI/InferenceX/pull/1423.
Addressing the refutation
One refutation noted that line 2502 (the minimaxm2.5-fp8-mi355x-vllm entry) also contains pr-link: XXX, and concluded that XXX is therefore an "accepted convention." I disagree: a single prior occurrence among 100+ entries — versus the overwhelmingly consistent URL format used everywhere else — is more naturally explained as another unfilled placeholder that slipped past review than as an accepted convention. The AGENTS.md template uses pull/XXX as the explicit fill-in marker, which is exactly what is left here. The right fix is to replace XXX with the real URL for this PR (and, separately, ideally backfill line 2502 too, though that is out of scope here).
How to fix
Replace line 2556:
- pr-link: XXX
+ pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/1423Impact
Low — this is changelog metadata, not runtime behavior. But the link will not resolve as written, which defeats the purpose of including it, so it is worth fixing before merge.
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=25980019087 |
1 similar comment
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=25980019087 |
|
/reuse-sweep-run |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=25984919473 |
2 participants
Updates SGLang image for
dsr1-fp8-h200-sglangfrom v0.5.11-cu130 to v0.5.12-cu130.\nRef #1154
Generated with Claude Code