perf(agentx): refresh dsv4-fp4-gb300-dynamo-vllm-agentic-mtp harness - #2518
perf(agentx): refresh dsv4-fp4-gb300-dynamo-vllm-agentic-mtp harness#2518cquil11 wants to merge 12 commits into
Conversation
0574c2d to
1035a2c
Compare
|
Thanks for the contribution! Please reach out to respective companies' CODEOWNER to fill in the latest PR_REVIEW_CHECKLIST.md before pinging core maintainer on Slack for review. In order for the signoff PR check bot to trigger, you must follow the PR_REVIEW_CHECKLIST.md template correctly, including the phrase For PR verification, add the PR authors are responsible for ensuring that after merging, all GitHub Action jobs fully pass. A lot of the time, failures are just flakes and simply re-running the failed jobs will fix it. See GitHub's docs on re-running failed jobs 感谢你的贡献！请联系相应公司的 CODEOWNER 填写最新的 PR_REVIEW_CHECKLIST.md，然后再在 Slack 上联系核心维护者进行审阅。为了触发 signoff PR 检查机器人，你必须正确遵循 PR_REVIEW_CHECKLIST.md 模板，包括保留英文语句 如需进行 PR 验证，请为此 PR 添加 PR 作者有责任确保合并后所有 GitHub Action 任务完全通过。 很多时候失败只是偶发抖动（flake），重新运行失败的任务即可解决。参见 GitHub 关于重新运行失败任务的文档 |
|
Claude finished @cquil11's task in 1m 11s —— View job LGTM - no blocking issues found
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — straightforward perf-changelog entry addition.
Extended reasoning...
Overview
The PR adds a single new entry to perf-changelog.yaml documenting a harness refresh for the dsv4-fp4-gb300-dynamo-vllm-agentic-mtp-agg config. No code, scripts, or configs are modified — just an append to the changelog record.
Security risks
None. This is a static YAML data file used for changelog tracking; no executable logic, credentials, or user input is involved.
Level of scrutiny
Minimal scrutiny is appropriate. The change is additive, mechanical, and follows the exact structure of preceding entries in the file (config-keys, scenario-type, description, pr-link). I confirmed the referenced config key (dsv4-fp4-gb300-dynamo-vllm-agentic-mtp-agg) exists in configs/nvidia-master.yaml, so the entry isn't dangling.
Other factors
No outstanding reviewer comments beyond the automated CODEOWNER reminder bot message, which is boilerplate and not a blocking concern. No prior review from this bot exists on this PR.
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=31150505585 |
…300-dynamo-vllm-agentic-mtp-agg
…300-dynamo-vllm-agentic-mtp-disagg
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=31152067173 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=31153314597 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=31153940335 |
3 similar comments
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=31153940335 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=31153940335 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=31153940335 |
cquil11
changed the title
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=31206346802 |
…300-dynamo-vllm-agentic-mtp-agg # Conflicts: # perf-changelog.yaml
|
|
||
| - config-keys: | ||
| - dsv4-fp4-b300-vllm-agentic-mtp | ||
| - dsv4-fp4-gb300-dynamo-vllm-agentic-mtp-agg | ||
| - dsv4-fp4-gb300-dynamo-vllm-agentic-mtp-disagg | ||
| scenario-type: | ||
| - agentic-coding | ||
| description: | ||
| - "Refresh submission with up to date AgentX harness." | ||
| - "Capture the vLLM backend Prometheus endpoint for every topology and require vllm: metrics before publishing AgentX results." | ||
| pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/2517 | ||
| - "Require vLLM backend metrics in every AgentX artifact." | ||
| pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/2518 |
There was a problem hiding this comment.
🟡 This PR's title and description are English-only, missing the bilingual ' / <中文标题>' format and mirroring Chinese section (e.g. ## 中文说明) required by AGENTS.md. Please add a Chinese title suffix and translated summary/validation section.
Extended reasoning...
AGENTS.md (the single guidance file CLAUDE.md points to) states explicitly and emphatically near the top of the document:
PR and GitHub-issue titles & descriptions must be bilingual — include a Simplified Chinese version in addition to English. Title format:
<English title> / <中文标题>. In the PR/issue body, follow the English content with its Chinese translation (e.g. a## 中文说明section mirroring the summary...) ... This applies to every PR and every issue.
PR #2518's title is perf(agentx): refresh dsv4-fp4-gb300-dynamo-vllm-agentic-mtp harness — English only, with no / <中文标题> suffix. The PR body's ## Summary and ## Validation sections likewise have no mirroring Chinese translation (no ## 中文说明 or equivalent). This is the PR's own metadata, not a pre-existing file in the repo, so the convention is fully applicable and was skipped entirely rather than partially satisfied.
Why this isn't a false positive: the rule is not aspirational boilerplate — it's actively followed elsewhere in this same repo's history. Commit ed5e88b, for example, uses a fully bilingual title: "Qwen3.5 H100 AgentX MTP recipe with golden-AL simulated acceptance / Qwen3.5 H100 AgentX MTP 配方（黄金 AL 模拟接受）", and its corresponding PR #2427 carries a mirrored Chinese body section. That shows the convention is live and enforced by convention/maintainer expectation, not dead text.
Why it doesn't block CI: the automated "signoff PR check" bot referenced in the recipe-reminder comment on this PR keys only on the CODEOWNER checklist phrase ("As a PR reviewer and CODEOWNER, I have reviewed this and have"), which AGENTS.md explicitly carves out as an English-verbatim exception. There is no automated gate for the bilingual title/body rule itself, so this finding is a documentation/metadata gap rather than something that will fail a check or block merge mechanically.
Step-by-step proof:
- Open AGENTS.md and note the bolded bilingual mandate near the top of the file (quoted above), which explicitly says "This applies to every PR and every issue."
- Look at PR perf(agentx): refresh dsv4-fp4-gb300-dynamo-vllm-agentic-mtp harness #2518's title:
perf(agentx): refresh dsv4-fp4-gb300-dynamo-vllm-agentic-mtp harness. Search for a/followed by CJK characters — none exists. - Look at the PR body's
## Summaryand## Validationsections — both are English bullet lists with no accompanying## 中文说明(or similarly named) section translating them. - Compare against commit ed5e88b / PR [Klaud Cold] qwen3.5-fp8-h100-sglang-agentic-mtp: Qwen3.5 H100 AgentX MTP recipe with golden-AL simulated acceptance / Qwen3.5 H100 AgentX MTP 配方（黄金 AL 模拟接受） #2427, which does include the bilingual title format, confirming the convention is actively used and not merely theoretical.
- Conclude: the PR's own title and description fail an explicit, currently-enforced repo convention.
Fix: append a Chinese translation to the PR title (perf(agentx): refresh dsv4-fp4-gb300-dynamo-vllm-agentic-mtp harness / 刷新 dsv4-fp4-gb300-dynamo-vllm-agentic-mtp 测试框架, or similar idiomatic phrasing) and add a ## 中文说明 section under the body mirroring the Summary/Validation content, using the AGENTS.md-preferred term mappings (基准测试, 配置, etc.) rather than literal machine translation.
|
|
||
| - config-keys: | ||
| - dsv4-fp4-b300-vllm-agentic-mtp | ||
| - dsv4-fp4-gb300-dynamo-vllm-agentic-mtp-agg | ||
| - dsv4-fp4-gb300-dynamo-vllm-agentic-mtp-disagg | ||
| scenario-type: | ||
| - agentic-coding | ||
| description: | ||
| - "Refresh submission with up to date AgentX harness." | ||
| - "Capture the vLLM backend Prometheus endpoint for every topology and require vllm: metrics before publishing AgentX results." | ||
| pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/2517 | ||
| - "Require vLLM backend metrics in every AgentX artifact." | ||
| pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/2518 |
There was a problem hiding this comment.
🟡 The new perf-changelog.yaml entry for PR #2518 (lines 5530-5539) was appended right after the #2477 entry, but AGENTS.md requires entries to be appended to the actual END of the file — this entry now sits mid-file, ahead of five already-merged entries (#2353, #2404, #2514, #2515, #2427) that landed after it via a merge from main. This happened because the branch was rebased/merged with origin/main after the original append; per AGENTS.md's documented conflict-recipe (checkout origin/main's copy of perf-changelog.yaml, then re-append this PR's entry at the tail), the #2518 entry should be moved to follow the #2427 entry at the true end of the file (currently line 5592).
Extended reasoning...
What the bug is: AGENTS.md line 78 states explicitly: perf-changelog.yaml 'is read chronologically (oldest at top, newest at bottom) - new entries MUST be appended to the END, never inserted in the middle or prepended.' The entry this PR adds for dsv4-fp4-gb300-dynamo-vllm-agentic-mtp-agg/-disagg (PR #2518) is currently at lines 5530-5539, but the file continues for over 60 more lines afterward with entries for PR #2353 (kimik3-fp4-h200-vllm-agentic), #2404, #2514, #2515, and #2427 — the actual last entry in the file, at line 5592.
How it happened: The PR diff hunk (@@ -5526,6 +5526,16 @@) shows the #2518 entry inserted immediately after the pre-existing #2477 entry, with the kimik3-fp4-h200-vllm-agentic (#2353) entry appearing as unchanged context directly below it — meaning #2353 already existed below the insertion point before this PR touched the file. So at the time the branch's own commits were authored, #2477 genuinely was the tail (this part of the append was correct). But the later 'Merge origin/main into GB300 vLLM combined refresh' commit (d60c5e1) brought in the newer #2353/#2404/#2514/#2515/#2427 entries from main, and a straightforward 3-way merge landed them after the #2518 entry the branch had already added — leaving #2518 stranded in the middle instead of at the tail.
Why nothing already prevents this: process_changelog.py (referenced in AGENTS.md's CI notes) only forbids line deletions via git diff to guard against tampering with historical entries — it does not check ordering or position. So this mid-file insertion passes CI and the benchmark-trigger sweep (which diffs the whole file) unaffected. The only guard against this class of bug is the documented human convention, which wasn't followed here.
The fix AGENTS.md prescribes: AGENTS.md line 100 gives the exact recipe for precisely this merge scenario: git checkout origin/main -- perf-changelog.yaml, then re-append the PR's own changelog entry at the tail. That step was skipped — a normal git merge was used instead, which interleaves entries by diff-hunk order rather than re-applying the PR's entry after main's newer entries.
Proof (verified against current file state):
wc -l perf-changelog.yaml→ 5592 lines total.grep -n 'pr-link' perf-changelog.yaml | tail -8shows, in file order: [Klaud Cold] glm5.2-fp4-b300-sglang-agentic-mtp: GLM-5.2 NVFP4 B300 AgentX with EAGLE MTP and golden-AL synthetic acceptance / 新增 GLM-5.2 NVFP4 B300 SGLang EAGLE MTP AgentX 配置（黄金 AL 合成接受率） #2447 (5517), perf(agentx): refresh Qwen3.5 GB300 recipes with correct metrics #2477 (5528), perf(agentx): refresh dsv4-fp4-gb300-dynamo-vllm-agentic-mtp harness #2518 (5538), [KimiK3][AgentX]: H200 KimiK3 Day 0 support #2353 (5544), [KimiK3][AgentX]: GB200 DSpark and Simple CPU KV offload #2404 (5558), Add Qwen3.5 FP8 B200 AgentX MTP #2514 (5569), Add Qwen3.5 FP8 B300 AgentX MTP #2515 (5580), [Klaud Cold] qwen3.5-fp8-h100-sglang-agentic-mtp: Qwen3.5 H100 AgentX MTP recipe with golden-AL simulated acceptance / Qwen3.5 H100 AgentX MTP 配方（黄金 AL 模拟接受） #2427 (5592).- Git log confirms [KimiK3][AgentX]: H200 KimiK3 Day 0 support #2353, [KimiK3][AgentX]: GB200 DSpark and Simple CPU KV offload #2404, Add Qwen3.5 FP8 B200 AgentX MTP #2514, Add Qwen3.5 FP8 B300 AgentX MTP #2515, and [Klaud Cold] qwen3.5-fp8-h100-sglang-agentic-mtp: Qwen3.5 H100 AgentX MTP recipe with golden-AL simulated acceptance / Qwen3.5 H100 AgentX MTP 配方（黄金 AL 模拟接受） #2427 are all already-merged commits on main (e.g.
9cc21aa Add Qwen3.5 FP8 B300 AgentX MTP (#2515),ed5e88b ... (#2427)), i.e. older changes than this open PR. - Since perf(agentx): refresh dsv4-fp4-gb300-dynamo-vllm-agentic-mtp harness #2518 is still an open/unmerged PR, it should be the newest (bottommost) entry once merged, per the 'newest at bottom' rule — instead it sits 5 entries above the true tail.
Impact: Because CI doesn't check position, this doesn't break the sweep-trigger mechanism or fail any check — it's purely a violation of the documented chronological-append convention that will confuse future readers scanning the file top-to-bottom for history, and make future merges more error-prone if not corrected now.
Suggested fix: Follow the AGENTS.md merge-conflict recipe: git checkout origin/main -- perf-changelog.yaml to restore the canonical (pre-PR) file, then re-append the #2518 entry (config-keys dsv4-fp4-gb300-dynamo-vllm-agentic-mtp-agg/-disagg) as the new last entry, after the current #2427 entry at line 5592.
|
Closing and stopping this sweep because it has been superseded by another submission. |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=31242897025 |
1 participant
Summary
dsv4-fp4-gb300-dynamo-vllm-agentic-mtp-agganddsv4-fp4-gb300-dynamo-vllm-agentic-mtp-disaggwith the up-to-date AgentX harness, matching the combined submission structure of original PR Add DeepSeek-V4 GB300 Dynamo-vLLM AgentX MTP recipes / 新增 DeepSeek-V4 GB300 Dynamo-vLLM AgentX MTP 配置 #2341.vllm:metrics before publishing results.Validation
perf-changelog.yamlsuccessfully.git diff --checkpasses.