Skip to content

Update dsr1-fp8-mi300x-sglang SGLang image to v0.5.11-rocm700-mi30x #1348

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Klaud-Cold wants to merge 1 commit into
base: main
Choose a base branch
from
+7 −1
Open

Update dsr1-fp8-mi300x-sglang SGLang image to v0.5.11-rocm700-mi30x #1348

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/configs/amd-master.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ dsr1-fp4-mi355x-atom-mtp:
- { tp: 8, conc-start: 4, conc-end: 256, spec-decoding: mtp }

dsr1-fp8-mi300x-sglang:
image: lmsysorg/sglang:v0.5.9-rocm700-mi30x
image: lmsysorg/sglang:v0.5.11-rocm700-mi30x
model: deepseek-ai/DeepSeek-R1-0528
model-prefix: dsr1
runner: mi300x
Expand Down
6 changes: 6 additions & 0 deletions perf-changelog.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2343,3 +2343,9 @@
description:
- "Add Qwen3.5-397B-A17B FP8 MI355X ATOM benchmark configs with and without MTP"
pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/1310

- config-keys:
- dsr1-fp8-mi300x-sglang
description:
- "Update SGLang image from v0.5.9-rocm700-mi30x to v0.5.11-rocm700-mi30x"
pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/XXX

Check warning on line 2351 in perf-changelog.yaml

View check run for this annotation

Claude / Claude Code Review

Placeholder PR link pull/XXX in perf-changelog entry 

The new perf-changelog entry uses a placeholder PR link `https://github.com/SemiAnalysisAI/InferenceX/pull/XXX` instead of the actual PR number (#1348). Every other entry in this file uses a real numeric PR ID, so this leaves a permanently broken (404) cross-reference in changelog history once merged. Please replace `XXX` with `1348` before merging.
Comment on lines +2347 to +2351
Copy link
Copy Markdown
Contributor

@claude claude Bot May 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 The new perf-changelog entry uses a placeholder PR link https://github.com/SemiAnalysisAI/InferenceX/pull/XXX instead of the actual PR number (#1348). Every other entry in this file uses a real numeric PR ID, so this leaves a permanently broken (404) cross-reference in changelog history once merged. Please replace XXX with 1348 before merging.

Extended reasoning...

What the bug is

The diff appends a new entry to perf-changelog.yaml for dsr1-fp8-mi300x-sglang:

- config-keys:
    - dsr1-fp8-mi300x-sglang
  description:
    - "Update SGLang image from v0.5.9-rocm700-mi30x to v0.5.11-rocm700-mi30x"
  pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/XXX

The pr-link field contains the literal placeholder XXX rather than a numeric PR id. This appears to be a template that the author (or a generator) intended to fill in but forgot to substitute.

Why this is wrong

Every other entry in perf-changelog.yaml uses a real numeric GitHub PR id. The five entries immediately preceding this one use /pull/1310, /pull/1308, /pull/1305, /pull/1304, and /pull/1303 respectively, establishing a strong and unambiguous convention. This PR is #1348 (per the PR metadata), so the link should be https://github.com/SemiAnalysisAI/InferenceX/pull/1348.

Impact

Functionally this is benign — the YAML still parses and any consumer that treats pr-link as an opaque string will not fail. The harm is purely to the changelog as documentation: clicking the link will resolve to GitHub's /pull/XXX path, which is not a valid PR number and returns a 404. Because the changelog is append-only history, once this merges the broken link is permanent unless a follow-up commit corrects it.

How to fix

Replace XXX with 1348 on line 2351:

  pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/1348

Step-by-step proof

  1. Look at perf-changelog.yaml line 2351 in the diff: pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/XXX.
  2. Look at the PR metadata: this PR is number 1348.
  3. Look at the existing convention in the file — the preceding entry (line 2345 in the new file) reads pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/1310, a real numeric id pointing at PR [DEBUG] Add Qwen3.5-397B-A17B FP8 MI355X ATOM perf-changelog entries #1310.
  4. Construct the URL the changelog will produce after merge: https://github.com/SemiAnalysisAI/InferenceX/pull/XXX. GitHub's pull-request route requires an integer; XXX is not an integer, so the link returns 404.
  5. Therefore the changelog as written contains a permanently broken cross-reference to this PR.

Loading