B200/300 test#1233
Conversation
# Conflicts: # .github/configs/nvidia-master.yaml # perf-changelog.yaml
There was a problem hiding this comment.
Additional findings (outside current diff — PR may have been updated during review):
-
🟡
perf-changelog.yaml:2017— The new perf-changelog.yaml entry at line 2017 haspr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/XXX— a literal placeholder rather than the actual PR number. Every other entry in this file uses a real numeric PR id (e.g. the immediately preceding entry usespull/1221); this should be updated topull/1233before merge so the changelog correctly traces back to its PR.Extended reasoning...
What's broken. The newly added perf-changelog.yaml entry for
dsv4-fp4-b200-trtanddsv4-fp4-b300-trtends with:\n\nyaml\n pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/XXX\n\n\nXXXis a literal placeholder string left over from authoring — not the PR number. The current PR (per the metadata in this review) is #1233, so this should readhttps://github.com/SemiAnalysisAI/InferenceX/pull/1233.\n\nWhy this is wrong. Every other entry inperf-changelog.yamluses an actual numeric PR id. The three entries immediately preceding this one end withpull/1209,pull/1218, andpull/1221respectively, and a quick scan of the rest of the file confirms the pattern holds throughout. LeavingXXXin place breaks the changelog's only mechanism for tracing a config-change entry back to the PR that introduced it.\n\nWhy existing tooling didn't catch it.perf-changelog.yamlappears to be free-form YAML metadata; there is no schema validator that requirespr-linkto be a real GitHub URL. YAML parsers happily accept the placeholder as a string, and PR review is the last line of defense.\n\nImpact. Cosmetic / documentation only — no runtime behavior is affected. But once merged, anyone auditing the changelog (e.g., "which PR added the dsv4-fp4-b200-trt config?") will hit a 404 instead of landing on the correct PR. Fixing it post-merge requires another PR to touch the changelog, which is wasteful.\n\nHow to fix. ReplaceXXXwith1233on line 2017 ofperf-changelog.yamlbefore merging this PR.\n\nStep-by-step proof.\n1. Openperf-changelog.yamlat the bottom of the file (the new entry added by this PR).\n2. Observe the entry's last line:pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/XXX.\n3. Compare with the immediately preceding entry (the dsv4-fp4-b300-vllm "Change image to vllm/vllm-openai:v0.20.0-cu130" entry): itspr-linkishttps://github.com/SemiAnalysisAI/InferenceX/pull/1221— a real numeric id.\n4. Compare with the PR's own metadata: the current PR number is1233.\n5. Conclusion: the new entry should usepull/1233, matching the convention used by every other entry in the file.
No description provided.