Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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 @@ -2354,7 +2354,7 @@ qwen3.5-fp8-b300-sglang:
- { tp: 4, ep: 1, conc-start: 4, conc-end: 256 }

qwen3.5-fp4-b300-sglang:
image: lmsysorg/sglang:v0.5.10.post1-cu130
image: lmsysorg/sglang:v0.5.11-cu130
model: nvidia/Qwen3.5-397B-A17B-NVFP4
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-fp4-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

perf-changelog pr-link placeholder not replaced

The new perf-changelog entry has `pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/XXX` — the literal placeholder `XXX` was never replaced with the actual PR number (1343). All other entries in the file use real numeric PR numbers (e.g., /pull/1304, /pull/1305, /pull/1308, /pull/1310). Fix by replacing `XXX` with `1343`.

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 has pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/XXX — the literal placeholder XXX was never replaced with the actual PR number (1343). All other entries in the file use real numeric PR numbers (e.g., /pull/1304, /pull/1305, /pull/1308, /pull/1310). Fix by replacing XXX with 1343.

Extended reasoning...

What the bug is

The PR appends a new entry to perf-changelog.yaml documenting the SGLang image bump for qwen3.5-fp4-b300-sglang. The added entry ends with:

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

The XXX is a literal placeholder string that should have been substituted with the real PR number — 1343 — before opening the PR.

Why this matters / how it manifests

The pr-link field is the changelog's authoritative reference back to the PR that introduced a given change. Every other entry in perf-changelog.yaml follows the convention of including a real numeric PR ID (visible in the surrounding context: /pull/1304, /pull/1305, /pull/1308, /pull/1310). With XXX in place, the URL https://github.com/SemiAnalysisAI/InferenceX/pull/XXX resolves to a 404 — the changelog reference is broken and useless for anyone trying to follow the link to context, discussion, or review history.

Why existing safeguards don't prevent it

There is no schema validation or CI check that enforces a numeric value in the pr-link field, so a placeholder slips through unnoticed. The PR is also marked full-sweep-enabled, suggesting it is intended to be merged once benchmarks pass — there is no human guardrail catching this string before merge.

Step-by-step proof

  1. Open the PR diff. The final hunk in perf-changelog.yaml adds (literally, with the XXX):
    - config-keys:
        - qwen3.5-fp4-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
  2. Compare against the immediately preceding entry (also in the diff context), which uses the real value https://github.com/SemiAnalysisAI/InferenceX/pull/1310.
  3. Navigate to https://github.com/SemiAnalysisAI/InferenceX/pull/XXX — GitHub returns a 404 because XXX is not a valid PR number.
  4. The actual PR is Update qwen3.5-fp4-b300-sglang SGLang image to v0.5.11-cu130 #1343 (per the PR metadata), so the intended URL is https://github.com/SemiAnalysisAI/InferenceX/pull/1343.

How to fix

Replace XXX with 1343 on line 2351 of perf-changelog.yaml:

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

This is a trivial documentation-only fix but should be done before merge so the changelog entry is actually useful to future readers.