Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
8 changes: 4 additions & 4 deletions .github/configs/nvidia-master.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2660,13 +2660,13 @@ kimik2.5-fp4-b200-vllm:
- isl: 1024
osl: 1024
search-space:
- { tp: 8, ep: 1, conc-start: 4, conc-end: 4 }
- { tp: 4, ep: 1, conc-start: 4, conc-end: 64 }
- { tp: 8, ep: 1, conc-start: 1, conc-end: 4 }
- { tp: 4, ep: 1, conc-start: 1, conc-end: 128 }
- isl: 8192
osl: 1024
search-space:
- { tp: 8, ep: 1, conc-start: 4, conc-end: 4 }
- { tp: 4, ep: 1, conc-start: 4, conc-end: 64 }
- { tp: 8, ep: 1, conc-start: 1, conc-end: 4 }
- { tp: 4, ep: 1, conc-start: 1, conc-end: 128 }

# NOTE: At the time of submission, https://docs.vllm.ai/projects/recipes/en/latest/moonshotai/Kimi-K2.5.html
# does not have a B300-specific recipe, so this config reuses the existing
Expand Down
6 changes: 6 additions & 0 deletions perf-changelog.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3508,3 +3508,9 @@
- "Update vLLM image from v0.21.0 to v0.22.0"
- "Expand concurrency sweep for the 1k/1k and 8k/1k cells: TP4/EP1 conc 4-64 -> 1-128, TP8/EP1 conc-start 4 -> 1 (conc-end 4 unchanged)."
pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/1674

- config-keys:
- kimik2.5-fp4-b200-vllm
description:
- "Expand concurrency sweep for the 1k/1k and 8k/1k cells: TP4/EP1 conc 4-64 -> 1-128, TP8/EP1 conc-start 4 -> 1 (conc-end 4 unchanged)."
pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/1677

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 perf-changelog.yaml entry has pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/<TODO> instead of /pull/1677. The literal <TODO> produces a broken URL (encoded as %3CTODO%3E, 404s) and breaks any downstream tooling that parses pr-link as a numeric PR id. Replace <TODO> with 1677 before merging — the PR's own Cursor Bugbot summary already flags this placeholder.

Extended reasoning...

What the bug is

At perf-changelog.yaml:3509 the newly appended entry contains:

  pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/<TODO>

The string <TODO> is a literal placeholder that was never replaced with the real PR number (1677). I verified this by running git show HEAD -- perf-changelog.yaml on commit bfa691a (the PR's tip) — the added line in the commit itself reads pull/<TODO>, not pull/1677. The PR description's own embedded Cursor Bugbot summary calls this out explicitly: "PR link still has a <TODO> placeholder".

Addressing the refutation

One verifier argued the PR diff shows 1677 and is authoritative over the local checkout. I re-checked directly: git show bfa691a -- perf-changelog.yaml (the PR's tip commit) shows the added line as pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/<TODO>. The file in the working tree at perf-changelog.yaml:3509 also contains <TODO>. Whatever diff rendering showed 1677 does not match the actual committed contents on this branch. If the PR is merged as-is from bfa691a, <TODO> lands in main.

Impact

  1. Broken URL. GitHub URL-encodes < and > as %3C/%3E, so the rendered link becomes https://github.com/SemiAnalysisAI/InferenceX/pull/%3CTODO%3E, which 404s.
  2. Downstream tooling. Every other entry in perf-changelog.yaml uses a real integer PR id (e.g. 1668, 1669, 1670 just above, 1544, etc.). Any consumer that parses the trailing path component as an int (int(url.rsplit('/', 1)[-1])) will throw ValueError on this row.
  3. Data integrity for the changelog itself — the file is the canonical record of perf-config changes and should be self-consistent.

Step-by-step proof

$ git rev-parse HEAD
bfa691ad40dbc3074e00073d9afdaf1f68b37c18

$ sed -n '3505,3510p' perf-changelog.yaml
- config-keys:
    - kimik2.5-fp4-b200-vllm
  description:
    - "Expand concurrency sweep for the 1k/1k and 8k/1k cells: ..."
  pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/<TODO>

$ git show HEAD -- perf-changelog.yaml | grep pr-link
+  pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/<TODO>

Fix

One-character cleanup: replace <TODO> with 1677 on line 3509. The PR number is already known (this PR is #1677, as referenced throughout the description).

Loading