Repository navigation
perf(agentx): refresh dsv4-fp4-gb300-dynamo-vllm-agentic-mtp harness #2518
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 11 commits
1035a2c
5a00917
2900e6f
f7161a3
3c4bedb
d72626a
8755c2f
1befac5
00eb507
d60c5e1
8f88208
146534c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5526,6 +5526,16 @@ | |
| - "Enable SGLang backend metrics on every aggregate, prefill, and decode engine, and fail before publishing a partial trace artifact if required sglang: metrics are absent." | ||
| - "Record the recipes' active HiCache host-DRAM tier and, for disaggregated points, Dynamo router commit 5a638087 in nvidia-master metadata so generated artifacts no longer report kv_offloading=none, allocated_cpu_dram_gb=0, or a null router." | ||
| pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/2477 | ||
|
|
||
| - config-keys: | ||
| - 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." | ||
| - "Require vLLM backend metrics in every AgentX artifact." | ||
| pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/2518 | ||
|
Check warning on line 5538 in perf-changelog.yaml
|
||
|
Comment on lines
5529
to
+5538
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 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: How it happened: The PR diff hunk ( Why nothing already prevents this: The fix AGENTS.md prescribes: AGENTS.md line 100 gives the exact recipe for precisely this merge scenario: Proof (verified against current file state):
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: |
||
|
|
||
| - config-keys: | ||
| - kimik3-fp4-h200-vllm-agentic | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 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 #2518's title is
perf(agentx): refresh dsv4-fp4-gb300-dynamo-vllm-agentic-mtp harness— English only, with no/ <中文标题>suffix. The PR body's## Summaryand## Validationsections 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:
perf(agentx): refresh dsv4-fp4-gb300-dynamo-vllm-agentic-mtp harness. Search for a/followed by CJK characters — none exists.## Summaryand## Validationsections — both are English bullet lists with no accompanying## 中文说明(or similarly named) section translating them.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.