Skip to content

perf(agentx): refresh dsv4-fp4-gb300-dynamo-sglang-agentic-agg harness - #2520

Open
cquil11 wants to merge 5 commits into
mainfrom
agent/refresh-dsv4-gb300-dynamo-sglang-agentic-agg
Open

perf(agentx): refresh dsv4-fp4-gb300-dynamo-sglang-agentic-agg harness#2520
cquil11 wants to merge 5 commits into
mainfrom
agent/refresh-dsv4-gb300-dynamo-sglang-agentic-agg

Conversation

@cquil11

@cquil11 cquil11 commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Add a perf changelog entry for dsv4-fp4-gb300-dynamo-sglang-agentic-agg.
  • Refresh the submission with the up-to-date AgentX harness.

Validation

  • Parsed perf-changelog.yaml successfully.
  • git diff --check passes.

@cquil11
cquil11 force-pushed the agent/refresh-dsv4-gb300-dynamo-sglang-agentic-agg branch from db0bd06 to dbaa77b Compare August 7, 2026 05:12
@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

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 As a PR reviewer and CODEOWNER, I have reviewed this and have.

For PR verification, add the full-sweep-fail-fast label (strongly recommended) to this PR — the benchmark sweep only runs on labeled PRs. Use full-sweep-enabled only if you need matrix jobs to keep running past a failure.

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 模板,包括保留英文语句 As a PR reviewer and CODEOWNER, I have reviewed this and have

如需进行 PR 验证,请为此 PR 添加 full-sweep-fail-fast 标签(强烈推荐)— 基准测试 sweep 仅在带有标签的 PR 上运行。仅当需要矩阵任务在失败后继续运行时才使用 full-sweep-enabled

PR 作者有责任确保合并后所有 GitHub Action 任务完全通过。 很多时候失败只是偶发抖动(flake),重新运行失败的任务即可解决。参见 GitHub 关于重新运行失败任务的文档

@cquil11
cquil11 marked this pull request as ready for review August 7, 2026 05:24
@cquil11
cquil11 requested a review from a team August 7, 2026 05:24
@claude

claude Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Claude finished @cquil11's task in 1m 44s —— View job


Review

  • Gather context and read PR diff
  • Validate perf-changelog.yaml entry (position, format, parse)
  • Post review feedback

LGTM - no blocking issues found

Verified for the single changed file (perf-changelog.yaml, +8):

Note: I couldn't run a PyYAML parse directly (Python execution isn't in my allowed tools for this run), but the entry is structurally identical to the existing 5k+ lines of entries and introduces no unusual YAML constructs.

@claude claude Bot left a comment

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.

LGTM — approving despite the inline nit. The changelog entry itself is a single, well-formed addition consistent with existing schema and other entries in the file.

Extended reasoning...

Overview

This PR adds a single changelog entry to perf-changelog.yaml documenting a refresh of the dsv4-fp4-gb300-dynamo-sglang-agentic-agg AgentX harness. No other files are touched. The new entry follows the existing schema (config-keys, scenario-type, description, pr-link) and is consistent in structure with neighboring entries in the file.

Security risks

None — this is a metadata-only YAML changelog file with no executable code, credentials, or config that affects runtime behavior of the inference stack.

Level of scrutiny

Low. This is a mechanical, single-entry, single-file changelog addition with no logic changes. The one finding from this review round is a process/documentation nit (missing bilingual PR title/description per AGENTS.md), not a functional or correctness issue with the changelog content itself.

Other factors

A candidate concern that the changelog claims an 'AgentX harness refresh' with no corresponding code change was raised and ruled out by the verifier pass — changelog-only entries documenting external/out-of-repo harness refreshes appear elsewhere in this file's history, so this is not anomalous. Given the trivial scope and the nit being editorial rather than substantive, I'm comfortable approving.

Comment thread perf-changelog.yaml
Comment on lines 5528 to +5535
pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/2477

- config-keys:
- dsv4-fp4-gb300-dynamo-sglang-agentic-agg
scenario-type:
- agentic-coding
description:
- "Refresh submission with up to date AgentX harness."

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.

🟡 PR title and description are English-only, but AGENTS.md line 7 requires bilingual (English + Simplified Chinese) titles and descriptions for every PR. Please retitle to perf(agentx): refresh dsv4-fp4-gb300-dynamo-sglang-agentic-agg harness / <中文标题> and add a ## 中文说明 section mirroring the Summary/Validation bullets.

Extended reasoning...

AGENTS.md (the repo's mandatory-reading doc, referenced from CLAUDE.md) states in its second paragraph, in bold, an explicit and unconditional requirement:

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; ...)

PR #2520's title is perf(agentx): refresh dsv4-fp4-gb300-dynamo-sglang-agentic-agg harness — there is no / <中文标题> suffix at all. The body only contains the English ## Summary and ## Validation sections; there is no accompanying ## 中文说明 (or any Chinese-language section) mirroring them.

This isn't a stale or ambiguous doc — the convention is actively followed elsewhere in this repo. For example, commit bb8e26d in this same repo's history uses the exact bilingual format the rule describes: [Klaud Cold] glm5.2-fp4-b300-sglang-agentic-mtp: ... / 新增 GLM-5.2 NVFP4 B300 SGLang EAGLE MTP AgentX 配置(黄金 AL 合成接受率). So this is a live, enforced convention that PR #2520 simply didn't follow, not a dead rule.

Step-by-step proof:

  1. Open AGENTS.md at the repo root, line 7 (second bolded paragraph) — it mandates bilingual title format <English title> / <中文标题> and a ## 中文说明 body section.
  2. Open PR perf(agentx): refresh dsv4-fp4-gb300-dynamo-sglang-agentic-agg harness #2520's title: perf(agentx): refresh dsv4-fp4-gb300-dynamo-sglang-agentic-agg harness. There is no / followed by Chinese characters anywhere in it.
  3. Open PR perf(agentx): refresh dsv4-fp4-gb300-dynamo-sglang-agentic-agg harness #2520's description: it contains ## Summary (2 bullets) and ## Validation (2 bullets), both entirely in English, with no ## 中文说明 or any Chinese text.
  4. Compare against a compliant example already in this repo's git history (commit bb8e26d), which does include the / <中文标题> suffix — confirming the rule is actively enforced, not aspirational.

Impact: this is a documentation/process compliance gap only. The change itself (a one-entry addition to perf-changelog.yaml) is functionally fine and parses correctly, so this does not block the benchmark sweep or cause any runtime issue. The fix is purely editorial: append a Chinese title suffix and add a ## 中文说明 section translating the Summary/Validation bullets, following the terminology and style guidance also given in AGENTS.md (e.g. using the provided English/Chinese term mapping table).

@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

1 participant