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 @@ -349,7 +349,7 @@ qwen3.5-fp4-mi355x-atom:
- { tp: 4, conc-start: 4, conc-end: 16 }

qwen3.5-fp8-mi300x-sglang:
image: lmsysorg/sglang:v0.5.10-rocm720-mi30x
image: lmsysorg/sglang:v0.5.11-rocm720-mi30x
model: Qwen/Qwen3.5-397B-A17B-FP8
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-fp8-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 pr-link contains placeholder XXX instead of PR number

The newly added perf-changelog entry uses an unfilled placeholder `pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/XXX` instead of the actual PR number (1353). Replace `XXX` with `1353` so the changelog navigation link resolves rather than 404s.
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 newly added perf-changelog entry uses an unfilled placeholder pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/XXX instead of the actual PR number (1353). Replace XXX with 1353 so the changelog navigation link resolves rather than 404s.

Extended reasoning...

What the bug is

At perf-changelog.yaml:2351, the new entry added by this PR contains a literal placeholder:

- config-keys:
    - qwen3.5-fp8-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 XXX is a placeholder that was never substituted with the real PR number. Every surrounding entry in this file (e.g. lines around 2318/2325/2332/2338/2345 referencing /pull/1304, /pull/1305, /pull/1308, /pull/1310) uses a real numeric PR ID, so this entry is clearly malformed relative to the rest of the file.

Why this matters

perf-changelog.yaml is the project's authoritative provenance log mapping config-key changes back to the PR that introduced them. Once this PR merges as #1353, https://github.com/SemiAnalysisAI/InferenceX/pull/XXX will resolve to a non-existent PR (GitHub returns 404 for non-numeric pull paths), so anyone using the changelog to trace why qwen3.5-fp8-mi300x-sglang jumped from v0.5.10 to v0.5.11 will hit a dead end on this row specifically.

Step-by-step proof

  1. Open perf-changelog.yaml at the bottom — the diff appends exactly one entry.
  2. Read line 2351: pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/XXX.
  3. Inspect the immediately preceding entry (the FP8 MI355X ATOM block): pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/1310 — a real PR number.
  4. PR metadata for this change identifies it as PR Update qwen3.5-fp8-mi300x-sglang SGLang image to v0.5.11-rocm720-mi30x #1353.
  5. Therefore the correct value on line 2351 is https://github.com/SemiAnalysisAI/InferenceX/pull/1353; XXX is an unfilled template marker.

Fix

Change line 2351 to:

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

Addressing the refutation

A refuting verifier argued this should be dropped because it duplicates bug_002/bug_003 (which were refuted) and is purely cosmetic. The duplication concern is resolved — synthesis merged the three duplicate reports into this single canonical entry, so this is the surviving (not duplicate) record of the issue. On severity: the refuter is correct that there is no runtime, build, or behavioral impact — only a broken provenance link in a documentation file — so this is filed as nit rather than normal. It's still worth flagging because it's a single one-character fix the author can make before merge, and the surrounding entries set the convention that pr-links resolve.

Loading