perf(agentx): refresh MI355X MiniMax M3 - #2458
Conversation
|
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 关于重新运行失败任务的文档 |
e49338f to
94074fb
Compare
cquil11
changed the title
94074fb to
100b98b
Compare
|
Claude finished @cquil11's task in 1m 24s —— View job Review
LGTM - no blocking issues found Verified: the new entry is appended at the end of |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM aside from the inline nit — this is a minor, low-risk config-only change.
Extended reasoning...
Overview
The PR adds a single new changelog entry to perf-changelog.yaml (lines 5359-5363), registering the minimaxm3-fp4-mi355x-vllm-agentic config for its initial MI355X MiniMax M3 submission via the new AgentX harness. No other files are touched. I confirmed this is genuinely the first changelog entry for this config key (only other reference is the config definition itself in configs/amd-master.yaml), so the 'Initial' description is accurate and the ruled-out mislabeling concern does not apply.
Security risks
None. This is a declarative YAML changelog entry with no code execution, credentials, or user input involved.
Level of scrutiny
Low. This follows the repo's well-established append-only changelog pattern (list of config-keys/description/pr-link entries) with no schema changes or logic modifications. The change is purely additive and mechanical.
Other factors
The one inline finding is a nit about the PR title/description missing the bilingual (English/Chinese) format mandated by AGENTS.md — a real, actionable process gap, but purely metadata and not something that affects correctness or functionality. Recent merged PRs (#2457, #2455) show this convention is not currently CI-enforced, so it's not a blocker here either. No outstanding unaddressed reviewer comments exist on this PR.
| - config-keys: | ||
| - minimaxm3-fp4-mi355x-vllm-agentic | ||
| description: | ||
| - "Initial MI355X MiniMax M3 with new AgentX harness" | ||
| pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/2458 |
There was a problem hiding this comment.
🟡 This PR's title and description are English-only, but AGENTS.md mandates bilingual PR titles (<English title> / <中文标题>) and a ## 中文说明 section in the body for every PR / 本 PR 的标题和描述仅为英文，但 AGENTS.md 要求所有 PR 标题必须为双语格式（<英文标题> / <中文标题>），并在正文中包含 ## 中文说明 部分。请补充中文标题和说明以符合仓库规范。
Extended reasoning...
AGENTS.md line 7 (a checked-in, mandatory repository convention, not subjective style) states verbatim: "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."
This PR's title is perf(agentx): refresh MI355X MiniMax M3 — there is no / <中文标题> suffix. Its body has only ## Summary and ## Validation sections, both in English, with no ## 中文说明 section translating the summary. The only documented exceptions in AGENTS.md are the CODEOWNER sign-off template (which must stay English-verbatim so the sign-off verifier bot can match it) and bot-generated comments following their own workflow templates — neither applies here, since this is a human-authored PR title and description.
Step-by-step proof:
- AGENTS.md:7 requires: title format
<English> / <中文>, and body with an English section followed by a## 中文说明mirror. - PR perf(agentx): refresh MI355X MiniMax M3 #2458 title:
perf(agentx): refresh MI355X MiniMax M3→ no/separator, no Chinese text at all. - PR perf(agentx): refresh MI355X MiniMax M3 #2458 body:
## Summary(3 bullets, English) +## Validation(2 bullets, English) → no## 中文说明section exists anywhere in the body. - Checking the stated exceptions: CODEOWNER sign-off template (not applicable — no sign-off here) and bot comment templates (not applicable — this is the PR author's own title/description). Neither exempts this PR.
- Conclusion: the PR violates the checked-in bilingual convention as written, with no applicable exception.
Why existing tooling doesn't catch this: there's no CI gate that lints PR titles/descriptions for a Chinese section — this convention is enforced only by human/agent review, not automation. That's corroborated by recent history: PR #2457 (fix(agentx): pin AIPerf v1 timing watchdog) and PR #2455 also merged with English-only titles, showing the rule is currently unenforced by any hard gate.
Impact of merging as-is: purely PR-metadata hygiene — the underlying YAML change (adding the MiniMax M3 config entry) is unaffected and nothing breaks functionally. Fix is trivial: the author (or a maintainer/bot) can edit the PR title to perf(agentx): refresh MI355X MiniMax M3 / perf(agentx)：更新 MI355X MiniMax M3 and append a ## 中文说明 section mirroring the Summary/Validation bullets.
Severity: since this causes no runtime failure, crash, or data loss, and AGENTS.md does not declare the bilingual rule a hard merge-blocker (nor is it CI-enforced), this is a nit — a real, actionable convention violation worth flagging, but not one that should hold up the merge.
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=30777851549 |
1 participant
Summary
Validation
perf-changelog.yamlsuccessfully.