chore(agentx): sync support with production results - #2561
Conversation
Remove unsupported Kimi K2.5 AgentX support and the Kimi K3 non-DSpark and DeepSeek V4 GB300 Dynamo-vLLM recipes whose results were purged from production.
|
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 3m 50s —— View job Review of PR #2561
SummaryOne blocking issue: The removal itself checks out cleanly:
Minor (non-blocking): Note: I could not run |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| search-space: | ||
| - { tp: 8, ep: 1, conc-start: 1, conc-end: 512, spec-decoding: mtp } | ||
|
|
||
| kimik3-fp4-b300-vllm-agentic: |
There was a problem hiding this comment.
🔴 BLOCKING: Master config files were modified but perf-changelog.yaml was not updated. When changing configs/amd-master.yaml or configs/nvidia-master.yaml, you must add a corresponding entry to perf-changelog.yaml documenting the changes.
Why it matters: The perf changelog is the chronological record used to correlate benchmark result changes with config changes. Removing three config keys (kimik3-fp4-b300-vllm-agentic, dsv4-fp4-gb300-dynamo-vllm-agentic, dsv4-fp4-gb300-dynamo-vllm-agentic-mtp-agg, dsv4-fp4-gb300-dynamo-vllm-agentic-mtp-disagg) without a changelog entry breaks that traceability.
Fix: Append an entry to the end of perf-changelog.yaml (the file is read chronologically, oldest at top) listing the removed config keys and linking this PR and the production purge PRs (SemiAnalysisAI/InferenceX-app#649, #722, #723).
There was a problem hiding this comment.
LGTM — the nits found (missing bilingual PR title/description, stale comments referencing files this PR deletes) are minor and do not affect correctness of the cleanup.
Extended reasoning...
Overview
This PR deletes purged/deprecated AgentX benchmark recipes and configs: Kimi K2.5 agentic scripts+configs, the Kimi K3 B300 non-DSpark script+config, and several DeepSeek V4 GB300 Dynamo-vLLM AgentX multi-node YAMLs, while updating MODELS.md/MODELS_zh.md changelog text to match. It also makes one small in-diff cleanup (a stale filename reference in kimik3_fp4_b300_vllm_mtp.sh line 184) that the bug hunter flagged as evidence the author is aware of this issue class but missed two adjacent sites.
Security risks
None. This is a pure deletion/cleanup of benchmark scripts and YAML configs (no production code paths, no auth/crypto/permission logic, no user input handling).
Level of scrutiny
Low-to-moderate is appropriate: the change is almost entirely file deletions and corresponding config-key removals for benchmarks that are confirmed already purged in production (per the linked InferenceX-app PRs). The one in-diff functional file (kimik3_fp4_b300_vllm_mtp.sh) only has a comment-text edit, no logic change. Both bug findings are nits — a bilingual PR-metadata process gap and two dangling comments referencing deleted files — with zero runtime impact.
Other factors
The PR includes validation output (YAML parse success, matrix logic 231 passed), and the deletions align with the MODELS.md changelog updates included in the same diff, so the change is internally consistent. No outstanding reviewer comments in the timeline to address.
|
|
||
| **Thursday, August 6, 2026** is the last day for the **Single-turn 8k1k** scenario on **Kimi-K2.5/2.6/2.7-Code** (`kimik2.5`). The scenario is deprecated for these models after that date. Rationale: Kimi-K3 launched on July 27, 2026, so GPU cluster time shifts to the newer frontier model. Combined with the Agentic coding deprecation above, this leaves `kimik2.5` with no active scenario. The model is **fully retired after August 6, 2026**. | ||
|
|
||
| **Enacted on 2026-08-07** in [#2527](https://github.com/SemiAnalysisAI/InferenceX/pull/2527): 17 `kimik2.5` config keys were removed from the active master configs and archived under [`configs/deprecated/`](configs/deprecated/) as `nvidia-kimik2.5-8k1k-master.yaml` (10) and `amd-kimik2.5-8k1k-master.yaml` (7), and their 12 benchmark scripts were moved to the sibling `deprecated/` directories. `kimik2.5` now has **no active configuration in any master config** and is fully retired. The same PR archived `kimik2.5-int4-h100-vllm`, an agentic-coding key that #2493 left behind in `nvidia-master.yaml` after moving its script to `benchmarks/single_node/agentic/deprecated/`. It is now in `nvidia-kimik2.5-agentic-master.yaml` with its siblings. The SPEED-Bench acceptance-length script `benchmarks/single_node/speedbench/kimik2.5_fp4_b300_vllm.sh` is intentionally kept. Speedbench is driven by `speedbench-al.yml`, not the master configs, matching how #2493 treated MiniMax-M3. | ||
| **Enacted on 2026-08-07** in [#2527](https://github.com/SemiAnalysisAI/InferenceX/pull/2527): 17 `kimik2.5` config keys were removed from the active master configs and archived under [`configs/deprecated/`](configs/deprecated/) as `nvidia-kimik2.5-8k1k-master.yaml` (10) and `amd-kimik2.5-8k1k-master.yaml` (7), and their 12 benchmark scripts were moved to the sibling `deprecated/` directories. `kimik2.5` now has **no active configuration in any master config** and is fully retired. The same PR archived `kimik2.5-int4-h100-vllm`, an agentic-coding key that #2493 left behind in `nvidia-master.yaml` after moving its script to `benchmarks/single_node/agentic/deprecated/`. A later cleanup permanently removed the archived AgentX configs and scripts after their production results were purged. The SPEED-Bench acceptance-length script `benchmarks/single_node/speedbench/kimik2.5_fp4_b300_vllm.sh` is intentionally kept. Speedbench is driven by `speedbench-al.yml`, not the master configs, matching how #2493 treated MiniMax-M3. | ||
|
|
||
| ## Scenarios | ||
|
|
There was a problem hiding this comment.
🟡 This PR's title ("chore(agentx): clean up purged and unsupported recipes") and description are English-only, but AGENTS.md line 15 requires PR titles/descriptions to include both English and natural Simplified Chinese (with a "/" separator, as sibling PRs #2536 and #2553 do). Pre-existing convention, not a code issue — please add the Chinese translation to the title/description.
Extended reasoning...
What the rule is and how it's violated: AGENTS.md line 15 states verbatim: "PR and issue titles, descriptions, and human-authored PR comments must include English and natural Simplified Chinese." This PR's title, chore(agentx): clean up purged and unsupported recipes, and its full body (Summary / Related production purges / Validation sections) are English-only — there is no Simplified Chinese anywhere in the PR metadata.
Why this is a real, actionable gap and not a stale-rule false positive: the convention is demonstrably live in this repository right now, not a vestigial rule nobody follows. The git log shown in this PR's own commit history includes sibling PRs merged in the same time window that follow it exactly: #2536 Retune DSV4 B300 AgentX MTP sweep / 调优 DSV4 B300 AgentX MTP 扫描 and #2553 Emit per-role average power on multinode disaggregated runs / 多节点分离式运行输出每角色平均功率, both pairing an English title with a Chinese translation via a / separator. This PR itself keeps MODELS.md and MODELS_zh.md in sync (both are in the diff, in parallel), showing the author is aware of and follows the repo's bilingual-documentation convention everywhere except the PR title/description itself.
Why nothing in the existing tooling catches this: the bilingual requirement lives only in AGENTS.md as a written instruction for contributors (human or agent) — there's no CI lint or GitHub Action that validates PR title/description language, so nothing blocks a PR from merging without the translation. It relies entirely on the author (or their reviewing agent) reading and following AGENTS.md.
Impact: this is documentation/process hygiene, not a functional or correctness issue — none of the benchmark configs, scripts, or MODELS.md/MODELS_zh.md content are affected, and the change itself (removing purged AgentX recipes) is correct and complete. The only consequence of merging as-is is a PR record that doesn't match the repo's own documented bilingual convention, which a maintainer could flag but which has no effect on runtime behavior, benchmark results, or the docs delivered by this diff.
Concrete proof / how to verify: compare this PR's title (chore(agentx): clean up purged and unsupported recipes, English-only) against git log --oneline -5, where #2536 and #2553 both show the English title / 中文标题 pattern mandated by AGENTS.md:15. Grepping the PR body for CJK characters ([一-鿿]) returns zero matches, whereas a compliant PR would have Chinese text in the Summary/description alongside the English text.
How to fix: add a Simplified Chinese translation to the PR title (e.g. chore(agentx): clean up purged and unsupported recipes / 清理已淘汰和不再支持的配方) and add a Chinese-language paragraph or section mirroring the English Summary/Related production purges/Validation content in the description, consistent with how #2527, #2536, and #2553 structured their bilingual bodies.
| # real target verification instead. Synthetic acceptance commits drafted tokens | ||
| # regardless of the target's logits, so the generated text is wrong and the | ||
| # SWE-bench eval scores 0.0000 -- the same split dsv4_fp4_b300_vllm_mtp.sh makes | ||
| # (and which kimik2.5_fp4_b300_mtp.sh omits; follow dsv4, not kimik2.5). | ||
| # (follow the DSV4 MTP precedent for this split). | ||
| # rejection_sample_method=block does real verification, so it is what EVAL_ONLY | ||
| # uses. vLLM rejects synthetic_acceptance_length unless the method is 'synthetic'. | ||
| if [ "${EVAL_ONLY:-false}" = "true" ]; then |
There was a problem hiding this comment.
🟡 This PR deletes benchmarks/single_node/agentic/kimik3_fp4_b300_vllm.sh and the kimik3-fp4-b300-vllm-agentic key in configs/nvidia-master.yaml, but leaves two now-dangling comments pointing at them: the header of kimik3_fp4_b300_vllm_mtp.sh (line 20) still says "MTP sibling of kimik3_fp4_b300_vllm.sh... the target server is identical", and the surviving kimik3-fp4-b300-vllm-agentic-dspark entry in configs/nvidia-master.yaml (line 1505) still says "Mirrors the non-MTP entry's KV arms". Both are comment-only staleness with no functional impact, but the PR author already fixed an identical stale reference elsewhere in the same file (line 184), so these should be cleaned up alongside it.
Extended reasoning...
What's stale. This PR removes two things: the non-DSpark agentic script benchmarks/single_node/agentic/kimik3_fp4_b300_vllm.sh (full file deletion, confirmed via deleted file mode in the diff) and the corresponding kimik3-fp4-b300-vllm-agentic key from configs/nvidia-master.yaml (the ~31-line block removed just above the surviving kimik3-fp4-b300-vllm-agentic-dspark entry). Neither deletion is itself a problem — the PR's stated goal is exactly to purge the non-DSpark arm. The problem is that two comments elsewhere in the same PR's diff-adjacent code still reference what was just deleted.
Site 1 — script header. benchmarks/single_node/agentic/kimik3_fp4_b300_vllm_mtp.sh line 20 (outside this PR's diff hunks, so untouched by it) reads: MTP sibling of kimik3_fp4_b300_vllm.sh. Everything about the target server is identical; the deltas are: .... After this PR merges, kimik3_fp4_b300_vllm.sh no longer exists anywhere in the repo (there is a same-named file under speedbench/, but the header is explicitly describing the deleted agentic non-MTP sibling with an identical target server, not the speedbench AL collector). A reader trying to verify "everything about the target server is identical" has no baseline left to diff against.
Site 2 — YAML comment. configs/nvidia-master.yaml around line 1505, in the untouched kimik3-fp4-b300-vllm-agentic-dspark entry, still reads: # Mirrors the non-MTP entry's KV arms so the spec-decoding delta is readable at equal concurrency, but stops at conc 16 rather than 24. After the kimik3-fp4-b300-vllm-agentic key is deleted a few lines above, there is no non-MTP entry left in the file to mirror or diff against.
Why existing review didn't catch it. Both lines sit outside this PR's diff hunks — line 20 of the .sh file and line 1505 of the .yaml are unmodified context, not additions, so a line-by-line diff review naturally skips over them. They only become stale as a side effect of what this same PR deletes elsewhere in the file.
Evidence the author was aware of this class of issue. This very PR's diff shows the author actively fixing the identical pattern at line 184 of kimik3_fp4_b300_vllm_mtp.sh, changing a reference from the concrete kimik2.5_fp4_b300_mtp.sh to generic wording (follow the DSV4 MTP precedent for this split) — precisely because a prior cleanup PR (#2527, per MODELS.md) had already retired that file. The two sites here are the same category of dangling cross-reference, just missed.
Step-by-step proof for Site 1:
- Before this PR:
benchmarks/single_node/agentic/kimik3_fp4_b300_vllm.shexists; line 20 of the MTP sibling correctly names it and invites a target-server diff. - This PR's diff contains
deleted file mode 100755for that exact path. - After merge,
git show HEAD:benchmarks/single_node/agentic/kimik3_fp4_b300_vllm.shfails — the file is gone. - Line 20 of
kimik3_fp4_b300_vllm_mtp.shis unchanged by the diff (confirmed by grep against the current working tree), so it still names the deleted file and asserts a baseline that no longer exists on disk.
Fix. Update line 20 to either drop the specific dead filename (similar to the generic wording already used at line 184) or note that the non-MTP arm was removed and point to the DSpark arm instead. Update line 1505 (and optionally the historical line 1507 CI-run reference, which is more defensible since it cites a past measurement rather than a live file) similarly — e.g. "Mirrors the KV arms of the now-removed non-MTP entry" or simply drop the comparison framing.
cquil11
changed the title
1 participant
Summary
Synchronize upstream AgentX support with the effective production database inventory.
Inventory after this PR
Related production purges
Validation