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 @@ -4333,7 +4333,7 @@ gptoss-fp4-h200-vllm:
- { tp: 8, conc-start: 4, conc-end: 32 }

minimaxm2.5-fp8-h200-vllm:
image: vllm/vllm-openai:v0.20.2
image: vllm/vllm-openai:v0.21.0
model: MiniMaxAI/MiniMax-M2.5
model-prefix: minimaxm2.5
runner: h200
Expand Down
6 changes: 6 additions & 0 deletions perf-changelog.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2493,3 +2493,9 @@
- "Update image tag to vllm/vllm-openai:v0.20.2"
- "Add DEP configs for B300 vLLM MTP"
pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/1271

- config-keys:
- minimaxm2.5-fp8-h200-vllm
description:
- "Update vLLM image from v0.20.2 to v0.21.0"
pr-link: XXX

Check warning on line 2501 in perf-changelog.yaml

View check run for this annotation

Claude / Claude Code Review

perf-changelog.yaml: placeholder pr-link: XXX not filled in

The new perf-changelog.yaml entry has `pr-link: XXX` instead of an actual PR URL. All ~250 other entries use a fully qualified GitHub PR URL — this should be replaced with `https://github.com/SemiAnalysisAI/InferenceX/pull/1402` to preserve traceability.

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.yaml entry has pr-link: XXX instead of an actual PR URL. All ~250 other entries use a fully qualified GitHub PR URL — this should be replaced with https://github.com/SemiAnalysisAI/InferenceX/pull/1402 to preserve traceability.

Extended reasoning...

What the bug is

In perf-changelog.yaml at line 2501, the newly added changelog entry for minimaxm2.5-fp8-h200-vllm has pr-link: XXX as its value. XXX is clearly an unfilled placeholder that was left in by accident — it is not a real GitHub URL.

Why this matters

Every other entry in perf-changelog.yaml (around 250 entries) uses a fully qualified GitHub PR URL of the form https://github.com/SemiAnalysisAI/InferenceX/pull/<N>. For example, the immediately preceding entry on line 2495 uses https://github.com/SemiAnalysisAI/InferenceX/pull/1271. The whole point of the pr-link field is to provide traceability from a performance change back to the PR that introduced it. With XXX in place, this entry has no traceability whatsoever.

Code path

The diff adds the following entry at the end of perf-changelog.yaml:

- config-keys:
    - minimaxm2.5-fp8-h200-vllm
  description:
    - "Update vLLM image from v0.20.2 to v0.21.0"
  pr-link: XXX

The XXX is a literal string left from the author's template, not a valid URL.

Step-by-step proof

  1. Open perf-changelog.yaml at line 2501.
  2. Observe the value: pr-link: XXX.
  3. Compare against line 2495: pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/1271.
  4. Compare against any other entry in the file — all use the https://github.com/SemiAnalysisAI/InferenceX/pull/<N> format.
  5. Confirm via PR metadata: this PR is Update minimaxm2.5-fp8-h200-vllm vLLM image to v0.21.0 #1402.
  6. Conclusion: XXX is an unfilled placeholder and the correct value is https://github.com/SemiAnalysisAI/InferenceX/pull/1402.

How to fix

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

Impact

This is metadata only, with no runtime impact. It is a documentation/traceability defect — hence nit severity — but it is trivially fixable and should be corrected before merge to keep the changelog consistent with every other entry.

Loading