Skip to content
Open
Changes from 2 commits
Commits
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
11 changes: 11 additions & 0 deletions perf-changelog.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3474,3 +3474,14 @@
- "Use scheduler-recv-interval values 2/60/30/1200/600/1920 for conc 1-4/8/16/32/64/128-256"
- "Set max-running-requests=256, chunked-prefill-size=16384, mem-fraction-static=0.8, cuda-graph-max-bs=CONC, and enable symm-mem"
pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/1544

- config-keys:
- minimaxm2.5-fp8-h100-vllm
- minimaxm2.5-fp8-h200-vllm
- minimaxm2.5-fp4-b200-vllm
- minimaxm2.5-fp4-b300-vllm
- minimaxm2.5-fp4-mi355x-vllm
description:
- "Re-run MiniMax-M2.5 single-node vLLM sweeps (H100/H200 FP8, B200/B300/MI355X FP4) with no recipe change, to capture per-GPU power telemetry (avg_power_w) added in #1558 for the power/energy canvas"
- "Source rows for the canvas predate the 2026-05-27 power-capture merge, so they carry throughput/latency but no measured power; this re-run replaces the modeled power layer with measured power"
pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/1666
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 changelog entry's pr-link is set to https://github.com/SemiAnalysisAI/InferenceX/pull/XXX — a literal XXX placeholder rather than the actual PR number. The PR description references pull/1666 and every other entry in this file resolves to a real PR number; please replace XXX with 1666 before merge so the canvas re-run rows remain traceable.

Extended reasoning...

What the bug is

perf-changelog.yaml:3487 (the only line added by this PR's pr-link: field) literally reads:

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

The XXX is a leftover template placeholder that was never substituted with the real PR number (1666).

How it manifests

Every other entry in perf-changelog.yaml resolves to a numeric PR — e.g. /pull/1544 at line 3476, and /pull/1648, /pull/1663, /pull/1647 in nearby blocks. This entry is the only one whose link does not resolve. As shipped, anyone clicking the link from a canvas row that originated in this sweep would get a 404, and any tooling that joins changelog rows back to their originating PR (for traceability or audit) will see an unparseable PR id.

Code path that triggers it

This is a pure data/config bug — the row is appended verbatim to perf-changelog.yaml, which is the authoritative changelog for sweep triggers. The placeholder is in the field that downstream tooling (and humans) use to map a sweep back to the PR that armed it. Because the sweep itself is armed by config-keys/description, the bad pr-link will not block execution, so it will silently land on main.

Why existing code doesn't prevent it

There is no schema validator on pr-link requiring a numeric PR id, and utils/process_changelog.py (mentioned in the PR description as the local validator) keys on config-keys, not the link. The author validated processing but not link well-formedness, so the placeholder slipped through.

Impact

Traceability is broken for the five canvas re-run rows generated by this entry. A follow-up cleanup PR will be required to replace XXX with 1666 (or any future correct number) — exactly the kind of trivial follow-up that wastes a review cycle when it could be caught here.

Fix

Change line 3487 from:

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

to:

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

Step-by-step proof

  1. Read perf-changelog.yaml at lines 3474–3487 on the PR's HEAD (commit c772387).
  2. Line 3476 (prior entry) ends in /pull/1544 — a valid PR id.
  3. Lines 3478–3487 are the new entry added by this PR.
  4. Line 3487 ends in /pull/XXX — a literal three-character placeholder, not a number.
  5. The PR description explicitly states the canvas should point to pull/1666 ("the canvas re-points to the new dump…"), and this PR is itself #1666, confirming the intended value is 1666.
  6. Conclusion: the placeholder was never substituted before commit, and will be merged as-is unless fixed.

Loading