Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ qwen3.5-bf16-mi355x-sglang-mtp:
- { tp: 8, ep: 1, conc-start: 4, conc-end: 256, spec-decoding: mtp }

qwen3.5-bf16-mi300x-sglang:
image: lmsysorg/sglang:v0.5.10-rocm720-mi30x
image: lmsysorg/sglang:v0.5.11-rocm720-mi30x
model: Qwen/Qwen3.5-397B-A17B
model-prefix: qwen3.5
runner: mi300x
Expand Down
6 changes: 6 additions & 0 deletions perf-changelog.yaml
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:
- qwen3.5-bf16-mi300x-sglang
description:
- "Update SGLang image from v0.5.10-rocm720-mi30x to v0.5.11-rocm720-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

perf-changelog entry uses placeholder PR link XXX instead of #1352

The new perf-changelog entry for qwen3.5-bf16-mi300x-sglang uses the placeholder `pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/XXX` instead of this PR's number. Please change `XXX` to `1352` so the link resolves and matches the convention used by every other entry in `perf-changelog.yaml`.
Copy link
Copy Markdown
Contributor

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 for qwen3.5-bf16-mi300x-sglang uses the placeholder pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/XXX instead of this PR's number. Please change XXX to 1352 so the link resolves and matches the convention used by every other entry in perf-changelog.yaml.

Extended reasoning...

What's wrong

The diff adds a new entry at the bottom of perf-changelog.yaml:

- config-keys:
    - qwen3.5-bf16-mi300x-sglang
  description:
    - "Update SGLang image from v0.5.10-rocm720-mi30x to v0.5.11-rocm720-mi30x"
  pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/XXX

The pr-link value still contains the literal XXX placeholder rather than the actual PR number. Per the PR metadata, this is PR #1352, so the link should be https://github.com/SemiAnalysisAI/InferenceX/pull/1352.

Why this is a bug, not stylistic

AGENTS.md (lines 300 / 323) documents pull/XXX as the template placeholder that authors must fill in when adding a changelog entry. Every other entry in the file uses a concrete PR number — the four entries immediately preceding this one reference /pull/1304, /1305, /1308, and /1310 (around lines 2325, 2332, 2338, 2345). The invariant the file maintains is: each changelog entry's pr-link points at the PR that introduced the corresponding config change, so the changelog is traceable back to its source review.

Step-by-step proof

  1. Open the PR in GitHub: this PR is numbered 1352 (see PR metadata title "Update qwen3.5-bf16-mi300x-sglang SGLang image to v0.5.11-rocm720-mi30x", PR Update qwen3.5-bf16-mi300x-sglang SGLang image to v0.5.11-rocm720-mi30x #1352).
  2. Open perf-changelog.yaml at line 2351 — the new entry's pr-link reads https://github.com/SemiAnalysisAI/InferenceX/pull/XXX.
  3. Visit that URL in a browser — GitHub returns a 404, since /pull/XXX is not a valid PR identifier.
  4. Compare with the immediately preceding entries: e.g. line 2345 reads pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/1310, which resolves to the actual PR that introduced that entry. The XXX entry breaks this invariant.

Impact

Purely a metadata/docs issue — no runtime, benchmark, or config behavior is affected. The only consequence is that the changelog entry for this image bump is not traceable to its originating PR (404 link).

Fix

Replace XXX with 1352 on line 2351:

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

Loading