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/nvidia-master.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2335,7 +2335,7 @@ qwen3.5-fp8-b300-sglang-mtp:
- { tp: 4, ep: 1, conc-start: 4, conc-end: 256, spec-decoding: mtp }

qwen3.5-fp8-b300-sglang:
image: lmsysorg/sglang:v0.5.10.post1-cu130
image: lmsysorg/sglang:v0.5.11-cu130
model: Qwen/Qwen3.5-397B-A17B-FP8
model-prefix: qwen3.5
runner: b300
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-b300-sglang
description:
- "Update SGLang image from v0.5.10.post1-cu130 to v0.5.11-cu130"
pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/XXX

Check failure on line 2351 in perf-changelog.yaml

View check run for this annotation

Claude / Claude Code Review

Placeholder XXX in perf-changelog pr-link

The new perf-changelog entry's `pr-link` still contains the placeholder `XXX` (`pull/XXX`) instead of this PR's actual number 1345. Please replace `XXX` with `1345` at perf-changelog.yaml:2351 so the changelog link resolves correctly — every preceding entry in this file uses a real PR number.
Comment on lines +2347 to +2351
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's pr-link still contains the placeholder XXX (pull/XXX) instead of this PR's actual number 1345. Please replace XXX with 1345 at perf-changelog.yaml:2351 so the changelog link resolves correctly — every preceding entry in this file uses a real PR number.

Extended reasoning...

What the bug is

The new entry appended to perf-changelog.yaml (lines 2347-2351 of the diff) ends with:

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

XXX is a literal placeholder — clearly left over from a template — rather than the real PR number. The current PR (per the metadata at the top of this review) is #1345, so the link should be https://github.com/SemiAnalysisAI/InferenceX/pull/1345.

How it manifests

After merge, the changelog will contain a URL pointing to /pull/XXX. GitHub does not resolve non-numeric pull request paths, so anyone clicking that link from rendered docs, the raw YAML, or any downstream tooling that consumes perf-changelog.yaml will hit a 404.

Why existing code doesn't prevent it

perf-changelog.yaml is plain data — there is no schema validator in the repo that checks pr-link resolves to a numeric ID. The surrounding entries (lines 2318, 2325, 2332, 2338, 2345) all use real PR numbers (1303, 1304, 1305, 1308, 1310), which is the established convention, but nothing enforces it automatically, so the placeholder slipped through.

Step-by-step proof

  1. Open this PR's metadata: the PR number is 1345.
  2. Open the diff for perf-changelog.yaml in this PR — the appended entry's last line is pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/XXX.
  3. Compare to the entry immediately above it (lines 2338-2345 of the diff context), which ends pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/1310 — a real, resolvable URL.
  4. Visit https://github.com/SemiAnalysisAI/InferenceX/pull/XXX after merge: GitHub will respond with a 404 because XXX is not a valid PR ID.
  5. Visit https://github.com/SemiAnalysisAI/InferenceX/pull/1345: this PR resolves correctly, confirming the intended target.

Impact

Low — this is a documentation/cosmetic issue with no runtime effect. But it is user-visible (anyone consulting the changelog to find the PR that introduced an image bump will hit a dead link) and trivial to fix.

Fix

Replace XXX with 1345 on perf-changelog.yaml:2351:

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

Loading