Update dsr1-fp4-b200-sglang SGLang image to v0.5.11-cu130#1320
Update dsr1-fp4-b200-sglang SGLang image to v0.5.11-cu130#1320Klaud-Cold wants to merge 1 commit into
Conversation
…\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. |
There was a problem hiding this comment.
LGTM — simple image version bump with a minor nit on the changelog PR link placeholder.
Extended reasoning...
Overview
This PR makes a single-line image tag bump for dsr1-fp4-b200-sglang from lmsysorg/sglang:v0.5.9-cu130 to lmsysorg/sglang:v0.5.11-cu130 in .github/configs/nvidia-master.yaml, and appends a corresponding entry to perf-changelog.yaml. Scope is minimal and self-contained.
Security risks
None. This is a config/version bump for a benchmark image; no auth, crypto, permissions, or input-handling code is touched.
Level of scrutiny
Low. Mechanical change following an established pattern (cf. preceding changelog entries at lines 2318, 2325, 2332, 2338, 2345). The substantive part (image tag) is correct.
Other factors
The one bug surfaced is a documentation nit: the pr-link in the new changelog entry still contains the XXX placeholder rather than the actual PR number (1320). This will cause a 404 in the changelog post-merge but has no runtime impact. The author can address it in a follow-up commit; it is not blocking.
| - dsr1-fp4-b200-sglang | ||
| description: | ||
| - "Update SGLang image from v0.5.9-cu130 to v0.5.11-cu130" | ||
| pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/XXX |
There was a problem hiding this comment.
🟡 The new perf-changelog entry's pr-link is the literal placeholder https://github.com/SemiAnalysisAI/InferenceX/pull/XXX instead of the actual PR number (1320). Once merged, this link will 404, breaking changelog traceability. Fix: replace XXX with 1320 on line 2351.
Extended reasoning...
What the bug is
The new entry added to perf-changelog.yaml (lines 2347–2351) sets pr-link to https://github.com/SemiAnalysisAI/InferenceX/pull/XXX. The token XXX is a documented template placeholder (per AGENTS.md and .github/workflows/claude.yml: "use XXX as placeholder until PR is created") that is expected to be replaced with the actual PR number before merge. The PR has been created — this is #1320 — but the placeholder was never updated.
Why existing code doesn't prevent it
This is a documentation/workflow defect, not a runtime defect. There is no CI validation that rejects pull/XXX links in perf-changelog.yaml. The workflow relies on PR review to catch the substitution, which is why the placeholder pattern exists in the first place.
Step-by-step proof
- Open
perf-changelog.yamlat line 2351. The value is literallyhttps://github.com/SemiAnalysisAI/InferenceX/pull/XXX. - Compare to the five most recent preceding entries (lines 2318, 2325, 2332, 2338, 2345) — all use real numbers:
/pull/1303,/pull/1304,/pull/1305,/pull/1308,/pull/1310. grep -n 'pull/XXX' perf-changelog.yamlreturns exactly one hit — this new entry. No other entry in the file has ever been merged with the placeholder intact, confirming the convention.- The PR metadata shows this is PR Update dsr1-fp4-b200-sglang SGLang image to v0.5.11-cu130 #1320, so the correct value is
https://github.com/SemiAnalysisAI/InferenceX/pull/1320. - After merge, clicking the link in the changelog will produce a GitHub 404, severing the traceability between the changelog entry and its originating PR.
Impact
Documentation-only. No effect on benchmarks, runtime, or the actual image update (which is the substantive content of the PR and looks correct). However, the changelog exists specifically to provide PR traceability for image/config changes, so a broken link defeats its purpose for this entry.
How to fix
Replace XXX with 1320 on line 2351 of perf-changelog.yaml:
pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/1320|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=25763401310 |
Summary
dsr1-fp4-b200-sglangimage fromlmsysorg/sglang:v0.5.9-cu130tolmsysorg/sglang:v0.5.11-cu130Ref #1154
Generated with Claude Code