Repository navigation
Update DSv4 B300 vllm image tag #1588
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3200,3 +3200,8 @@ | |
| - "Bump image to lmsysorg/sglang-rocm:v0.5.12.post1-rocm720-mi35x-20260523, 1P1D TP8/EP1, dp-attn false, conc [8..512]" | ||
| - "MoRI conn.py overlay (48e459bd) via job.slurm; launcher qwen3.5_fp4_mi355x_sglang-disagg.sh" | ||
| pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/1579 | ||
|
|
||
| - config-keys: | ||
| - dsv4-fp4-b300-vllm | ||
| description: | ||
| - "Update DSv4 FP4 B300 vLLM image tag to v0.22.0" | ||
|
Check warning on line 3207 in perf-changelog.yaml
|
||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 The new
dsv4-fp4-b300-vllmentry at the bottom ofperf-changelog.yamlis missing thepr-linkfield that every other one of the 400 entries in the file carries, breaking changelog→PR traceability for this update. The file also no longer ends with a trailing newline (\ No newline at end of filein the diff). Addingpr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/1588plus a trailing newline fixes both at once.Extended reasoning...
What the bug is
perf-changelog.yamlfollows a strict, fully-uniform convention: each entry hasconfig-keys,description, andpr-link. The new entry added by this PR (lines 3204-3207) breaks that convention in two ways:pr-linkfield. A quick count confirms the asymmetry: the file has 401- config-keys:entries but only 400pr-link:lines after this PR — the newdsv4-fp4-b300-vllmblock is the sole exception. The five most recently added entries above it (PRs [NV] B300 (Agg): migrate model path #1539, [AMD] Add Qwen3.5 FP8 MI355X SGLang disaggregated benchmark #1570, [MoRI short term temp patch] GLM-5 FP8 MI355X SGLang disaggregated #1572, [MoRI short term patch] add qwen3.5-fp4 MI355X SGLang PD-disaggregation #1579, and the older ones) all end with apr-linkline such aspr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/1579.\ No newline at end of fileunder the new block. Every prior entry in the file ends with a newline.Why it matters / impact
The changelog is the canonical map from a config-key change to the PR that introduced it. Dropping
pr-linkfor this entry means a future reader looking atdsv4-fp4-b300-vllmin the changelog has no in-file pointer back to PR #1588 — they have togit log/git blameto recover the link that every other row gives them for free. For a file that has held this convention across 400 entries, the inconsistency is the bug, not the missing newline per se.Proof — step by step
grep -c '^- config-keys:' perf-changelog.yaml→ 401 after this PR.grep -c '^ pr-link:' perf-changelog.yaml→ 400 after this PR.main: pre-PR both counts are 400. The PR adds exactly oneconfig-keysblock (lines 3204-3207) and zeropr-linklines, producing the 401/400 mismatch.tail -c1 perf-changelog.yaml | xxd→ would show the last byte is"(closing the description string), not\n, matching the diff's\ No newline at end of filemarker. Pre-PR the file's last byte was\n(line 3203 was a blank separator).Why existing code/process doesn't prevent it
There is no schema or pre-commit check enforcing the presence of
pr-linkor a final newline on this file; the convention is maintained by author discipline and review. The PR is marked[WIP], which is consistent with the field simply not having been filled in yet.How to fix
Append the missing field plus a newline to the entry:
…and make sure the file ends with a trailing
\n. Note that addingpr-linkon its own line and saving with a final newline resolves the trailing-newline complaint at the same time, so this is a single one-line edit on the author's side.On the refutation of the trailing-newline issue
A reviewer argued the missing trailing newline is too trivial to surface. We agree it is purely a style/POSIX nit in isolation — that's why it is folded into this single
nit-severity report alongside thepr-linkfinding rather than filed separately. Fixingpr-linkwill incidentally fix the newline, so there is no extra ask on the author; the note is just so they don't accidentally re-introduce the same trailing-byte situation when adding the link.