Skip to content

[WIP][NV] update glm5-fp4-b200 sglang image #1031

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
hshrivastava-droid wants to merge 2 commits into
base: main
Choose a base branch
from
+7 −1
Open

[WIP][NV] update glm5-fp4-b200 sglang image #1031

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d373d5f
update sglang image
hshrivastava-droid Apr 15, 2026
19f5cd3
update PR number
hshrivastava-droid Apr 15, 2026
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1826,7 +1826,7 @@ glm5-fp8-b200-sglang:
- { tp: 8, ep: 1, conc-start: 4, conc-end: 256 }

glm5-fp4-b200-sglang:
image: lmsysorg/sglang:nightly-dev-cu13-20260328-a27651d5
image: lmsysorg/sglang:v0.5.10.post1-cu130
model: nvidia/GLM-5-NVFP4
model-prefix: glm5
runner: b200
Expand Down
6 changes: 6 additions & 0 deletions perf-changelog.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1348,3 +1348,9 @@
description:
- "Enable SGLANG_ENABLE_SPEC_V2=1 for Qwen3.5 FP8 H200 SGLang MTP"
pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/1017

- config-keys:
- glm5-fp4-b200-sglang
description:
- "Update SGLang image from nightly-dev-cu13-20260328-a27651d5 to v0.5.10.post1-cu130"
pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/1031
Copy link
Copy Markdown
Contributor

@claude claude bot Apr 15, 2026

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 uses a placeholder pr-link of pull/XXX instead of the actual PR number 1031. Update line 1356 to pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/1031 before merging.

Extended reasoning...

What the bug is: The new changelog entry for glm5-fp4-b200-sglang added at the bottom of perf-changelog.yaml uses the placeholder string pull/XXX in its pr-link field rather than the actual PR number.

The specific code path: Line 1356 of perf-changelog.yaml reads:

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

The PR being reviewed is clearly PR #1031 (visible in the PR metadata), so the correct value is https://github.com/SemiAnalysisAI/InferenceX/pull/1031.

Why existing practices don't prevent it: The file already contains several other entries with pull/XXX (e.g., glm5-fp8-mi355x-sglang, minimaxm2.5-fp8-h200-vllm, qwen3.5-bf16-mi325x-sglang, qwen3.5-fp8-mi325x-sglang), indicating this placeholder pattern has slipped through in prior merges. However, those past failures to update the link do not make it acceptable to repeat the mistake in a new entry, especially when the correct PR number is definitively known.

Addressing the refutation: The refutation argues this is an accepted practice because other XXX entries exist and the PR is marked [WIP]. Neither argument holds: the presence of prior mistakes doesn't make them correct, and [WIP] status means the author should update the PR number before requesting final merge, not that the placeholder is intentionally permanent. The refutation also conflates this with bug_002—but this is an independently reportable, actionable issue on a new entry introduced by this PR.

Impact: Purely documentation/metadata. The pr-link field is informational only; no CI or runtime behavior is affected. However, a broken/placeholder link makes the changelog less useful for traceability.

How to fix: Replace line 1356 with:

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

Step-by-step proof:

  1. PR [WIP][NV] update glm5-fp4-b200 sglang image #1031 is titled "[NV] update sglang image" and modifies perf-changelog.yaml.
  2. The diff shows a new entry appended with pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/XXX.
  3. The PR number is 1031 by definition (it is the PR under review).
  4. Therefore the correct link is pull/1031, not pull/XXX.

Loading