Update DSV4 GB300 Dynamo vLLM Recipes by hjjq · Pull Request #2010 · SemiAnalysisAI/InferenceX · GitHub
Skip to content

Update DSV4 GB300 Dynamo vLLM Recipes#2010

Open
hjjq wants to merge 2 commits into
mainfrom
hjjq/dsv4-gb300-vllm
Open

Update DSV4 GB300 Dynamo vLLM Recipes#2010
hjjq wants to merge 2 commits into
mainfrom
hjjq/dsv4-gb300-vllm

Conversation

@hjjq

@hjjq hjjq commented Jul 3, 2026

Copy link
Copy Markdown
Collaborator

No description provided.

@github-actions

github-actions Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Comment thread perf-changelog.yaml
Comment on lines +4437 to +4441

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 dsv4-fp4-gb300-dynamo-vllm entry in perf-changelog.yaml (line 4441) has pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/XXX — a template placeholder that wasn't filled in. This should be .../pull/2010 to match the actual PR number, consistent with every other entry in the file.

Extended reasoning...

What the bug is

The new entry added at the bottom of perf-changelog.yaml for dsv4-fp4-gb300-dynamo-vllm uses a literal placeholder for its pr-link:

- config-keys:
    - dsv4-fp4-gb300-dynamo-vllm
  description:
    - "Refresh DSV4 8k/1k vLLM GB300 recipes with new w4a4 container and updated configs"
  pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/XXX

The XXX is clearly a fill-in-the-blank the author forgot to substitute. Every other entry in this file uses a concrete numeric PR (e.g. pull/2001, pull/1990, pull/1978, pull/1975). This PR is #2010, so it should read https://github.com/SemiAnalysisAI/InferenceX/pull/2010.

Why existing tooling doesn't catch this on the PR

utils/validate_perf_changelog.py explicitly allows the placeholder on PR runs (via a PR_LINK_PLACEHOLDERS set) so that authors can open the PR before knowing the assigned number. That is why CI is green here.

Impact after merge

On main-branch runs, validate_added_pr_link() enforces CANONICAL_PR_LINK = r'https://github\.com/SemiAnalysisAI/InferenceX/pull/\d+' with fullmatch. XXX does not match \d+, so if this merges as-is, the next main-branch changelog validation will fail on this entry. Beyond that, the entry becomes useless as a tracking record — clicking the link goes to a 404, and the change loses its anchor to the PR that introduced it.

Step-by-step proof

  1. Line 4441 of the file after this PR: pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/XXX.
  2. On PR CI, validate_added_pr_link sees the link is in PR_LINK_PLACEHOLDERS → passes.
  3. After merge to main, validate_added_pr_link runs and executes CANONICAL_PR_LINK.fullmatch(link) with the pattern ^https://github\.com/SemiAnalysisAI/InferenceX/pull/\d+$.
  4. The value .../pull/XXX fails \d+fullmatch returns None → the check errors out.

Fix

Replace pull/XXX with pull/2010 on line 4441 before merging.

Severity

Nit — it does not affect runtime/benchmark behavior and is a one-character fix, but it should be corrected before merge so the changelog stays consistent and main-branch validation stays green.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

1 participant