Skip to content

[DNM] [testing on behalf of inferact for v0.20.2 rc] dsv4-fp8-h200-vllm-mtp: switch to public.ecr.aws vllm-release-repo image #1284

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
functionstackx wants to merge 3 commits into
base: main
Choose a base branch
from
Open

[DNM] [testing on behalf of inferact for v0.20.2 rc] dsv4-fp8-h200-vllm-mtp: switch to public.ecr.aws vllm-release-repo image #1284

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e7db559
dsv4-fp8-h200-vllm-mtp: switch to public.ecr.aws release-repo image
functionstackx May 5, 2026
d12c52e
perf-changelog: fill in PR #1284 link
functionstackx May 5, 2026
9148532
dsv4-fp8-h200-vllm-mtp: use enroot-compat URL form (public.ecr.aws#...)
functionstackx May 5, 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 @@ -2634,7 +2634,7 @@ dsv4-fp8-h200-vllm:
# (the non-MTP entry above is still on the deepseekv4-cu129 tag) and adds
# --speculative-config '{"method":"mtp","num_speculative_tokens":2}'.
dsv4-fp8-h200-vllm-mtp:
image: vllm/vllm-openai:v0.20.1@sha256:9eff9734a30b6713a8566217d36f8277630fd2d31cec7f0a0292835901a23aa4
image: public.ecr.aws#q9t5s3a7/vllm-release-repo:fbd51e3dfc902364fddab316ef1337c4f261de1a-x86_64-cu129
model: deepseek-ai/DeepSeek-V4-Pro
model-prefix: dsv4
runner: h200
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 @@ -2214,3 +2214,9 @@
- "Bump --speculative-config num_speculative_tokens from 1 to 2 (`{\"method\":\"mtp\",\"num_speculative_tokens\":2}`)"
- "Re-test whether H200 MTP kernels accept 2 draft tokens — Blackwell MTP runs at 2 (per @wzhao18's vLLM Blackwell MTP submission); checking if H200 has parity now"
pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/1279

- config-keys:
- dsv4-fp8-h200-vllm-mtp
description:
- "Switch image from vllm/vllm-openai:v0.20.1 (cu130) to public.ecr.aws#q9t5s3a7/vllm-release-repo:fbd51e3dfc902364fddab316ef1337c4f261de1a-x86_64-cu129 (vLLM nightly release-repo build, cu129; enroot URL form with `#` separating registry from path)"
pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/1284
Comment on lines +2218 to +2222
Copy link
Copy Markdown
Contributor

@claude claude Bot May 5, 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 has pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/PLACEHOLDER instead of the real PR number — the placeholder substitution wasn't done before push. The link will 404, breaking traceability from this changelog row to the originating PR. Fix by replacing PLACEHOLDER with 1284.

Extended reasoning...

What the bug is

perf-changelog.yaml line 2222 (the pr-link for the new dsv4-fp8-h200-vllm-mtp image-switch entry added by this PR) reads:

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

The literal token PLACEHOLDER was committed instead of the real PR number 1284.

How it manifests / proof

Step-by-step verification against the tip of this branch (commit e7db559):

  1. git show e7db559 -- perf-changelog.yaml shows the diff hunk ending with pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/PLACEHOLDER — i.e. the committed bytes contain the literal string PLACEHOLDER, not 1284.
  2. Reading the working-tree file at line 2222 confirms the same: pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/PLACEHOLDER.
  3. All other pr-link entries in the same file (e.g. line 2189: pull/1256, line 2199: pull/1065, line 2209: pull/1222, line 2216: pull/1279) use real numeric PR ids, so the format and convention are unambiguous.
  4. Browsing https://github.com/SemiAnalysisAI/InferenceX/pull/PLACEHOLDER returns 404 (no such PR), so the link is dead on arrival.

Note: the rendered PR-diff block in some review tooling may post-process and display pull/1284, but the actual committed file content (the source of truth that downstream tooling and humans will read) contains PLACEHOLDER. Both this branch HEAD and the working tree confirm PLACEHOLDER.

Why existing code does not prevent it

There is no schema validation on pr-link values in this repo — the YAML is a plain document, so a bogus URL passes through silently. The author or a templating step (commonly a sed substitution that replaces PLACEHOLDER after the PR number is known) clearly intended to fill this in but missed it before pushing.

Impact

  • The changelog pr-link is the single field that ties a changelog row back to the PR that introduced the behavior change. With PLACEHOLDER, that traceability is broken for this entry only — humans following the link land on a 404; any tooling that resolves pr-link to fetch PR context (release notes, dashboards, automated changelog renderers) will fail or skip this row.
  • It is a documentation/metadata defect rather than a runtime defect, so it does not break the actual sweep. But it permanently corrupts the changelog history if merged as-is.

Fix

One-line edit on line 2222:

-  pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/PLACEHOLDER
+  pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/1284

Loading