Skip to content

Update minimaxm2.5-fp8-mi355x-vllm vLLM ROCm image to v0.21.0 #1410

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

Merged
functionstackx merged 1 commit into from
May 16, 2026
Merged

Update minimaxm2.5-fp8-mi355x-vllm vLLM ROCm image to v0.21.0 #1410

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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/amd-master.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -568,7 +568,7 @@ kimik2.5-fp4-mi355x-atom:
- { tp: 4, conc-start: 4, conc-end: 128 }

minimaxm2.5-fp8-mi355x-vllm:
image: vllm/vllm-openai-rocm:v0.19.0
image: vllm/vllm-openai-rocm:v0.21.0
model: MiniMaxAI/MiniMax-M2.5
model-prefix: minimaxm2.5
runner: mi355x
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 @@ -2493,3 +2493,9 @@
- "Update image tag to vllm/vllm-openai:v0.20.2"
- "Add DEP configs for B300 vLLM MTP"
pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/1271

- config-keys:
- minimaxm2.5-fp8-mi355x-vllm
description:
- "Update vLLM ROCm image from v0.19.0 to v0.21.0"
pr-link: XXX

Check failure on line 2501 in perf-changelog.yaml

View check run for this annotation

Claude / Claude Code Review

perf-changelog.yaml entry uses placeholder pr-link: XXX 

The new `perf-changelog.yaml` entry sets `pr-link: XXX` as a literal placeholder instead of a real URL. Every other entry uses the form `https://github.com/SemiAnalysisAI/InferenceX/pull/<number>`, and `utils/merge_with_reuse.sh` relies on the documented `pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/XXX` template to auto-resolve changelog merge conflicts — its `block.replace('/pull/XXX', ...)` becomes a no-op and the subsequent `endswith('/<pr>')` assertion fails. Please replace `X
Copy link
Copy Markdown
Contributor

@claude claude Bot May 16, 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 sets pr-link: XXX as a literal placeholder instead of a real URL. Every other entry uses the form https://github.com/SemiAnalysisAI/InferenceX/pull/<number>, and utils/merge_with_reuse.sh relies on the documented pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/XXX template to auto-resolve changelog merge conflicts — its block.replace('/pull/XXX', ...) becomes a no-op and the subsequent endswith('/<pr>') assertion fails. Please replace XXX with https://github.com/SemiAnalysisAI/InferenceX/pull/1410 before merge.

Extended reasoning...

What the bug is

The new entry added at perf-changelog.yaml:2496-2501 ends with:

  pr-link: XXX

Every other entry in the file (~250+) follows the convention pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/<number>, and the template documented in AGENTS.md (line 123) is pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/XXX — i.e. a full URL whose trailing XXX is intended to be substituted with the actual PR number. The PR here committed the bare string XXX instead.

Why this breaks tooling, not just style

utils/merge_with_reuse.sh automates resolution of perf-changelog.yaml merge conflicts by finding the last entry tagged with XXX and rewriting it to point at the real PR number. The relevant logic (around lines 134–160) does:

link = str(entry.get("pr-link") or "")
if "XXX" not in link and not link.endswith(f"/pull/{pr}"):
    continue
...
contribs.append(block.replace("/pull/XXX", f"/pull/{pr}"))
...
assert last["pr-link"].endswith(f"/{pr}"), f"last entry not for PR #: {last}"

With pr-link: XXX:

  1. The "XXX" in link check still matches, so the entry is selected.
  2. block.replace("/pull/XXX", f"/pull/{pr}") is a no-op — the block contains XXX but not /pull/XXX. So pr-link remains the literal string XXX.
  3. The final assertion last["pr-link"].endswith("/1410") then fires because "XXX".endswith("/1410") is False, aborting auto-resolution.

Step-by-step proof (PR #1410 with a changelog conflict)

  1. Maintainer runs utils/merge_with_reuse.sh 1410 to resolve a perf-changelog.yaml conflict on this PR.
  2. The script reads the entry whose pr-link is XXX, passes the "XXX" in link guard.
  3. It tries block.replace("/pull/XXX", "/pull/1410") — no match, block unchanged.
  4. The resulting last entry still has pr-link: XXX.
  5. assert last["pr-link"].endswith("/1410") → AssertionError: last entry not for PR #: {...pr-link: XXX...}.
  6. Auto-merge fails; the maintainer has to fix the placeholder by hand anyway.

If the changelog never conflicts during merge, the literal string XXX is committed to main as the permanent pr-link, leaving a traceless entry in a file that downstream tooling/dashboards treat as URL-bearing.

Why existing validation does not catch it

utils/matrix_logic/validation.py only requires pr_link to be a str, so Pydantic accepts "XXX". No CI check enforces the URL shape, so this slips through.

Fix

Replace line 2501 with:

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

Or, if the intent is to defer substitution to merge_with_reuse.sh, use the documented template form https://github.com/SemiAnalysisAI/InferenceX/pull/XXX so the script can rewrite it. Either form would be consistent with the rest of the file; the current bare XXX is neither.