Skip to content

fix(agentx): bump AIPerf to v1.0.1 - #2484

Merged
cquil11 merged 1 commit into
mainfrom
agent/bump-aiperf-agentx-v1.0.1
Aug 3, 2026
Merged

fix(agentx): bump AIPerf to v1.0.1#2484
cquil11 merged 1 commit into
mainfrom
agent/bump-aiperf-agentx-v1.0.1

Conversation

@cquil11

@cquil11 cquil11 commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator

Pins utils/aiperf to b7b16cf8 (agentx-v1.0.1).

This adds the 98% profiling metric coverage check so incomplete AgentX runs fail instead of passing.

Pin the shared AIPerf submodule to agentx-v1.0.1 so AgentX runs reject incomplete profiling metric coverage while retaining diagnostic artifacts.\n\n中文:将共享 AIPerf 子模块固定到 agentx-v1.0.1,使 AgentX 运行能够拒绝分析指标覆盖不完整的结果,同时保留诊断产物。

Signed-off-by: Cam Quilici <cjquilici@gmail.com>
@cquil11
cquil11 marked this pull request as ready for review August 3, 2026 23:04
@cquil11
cquil11 requested a review from a team August 3, 2026 23:04
@cquil11 cquil11 changed the title fix(agentx): bump AIPerf to v1.0.1 / 将 AIPerf 升级至 v1.0.1 fix(agentx): bump AIPerf to v1.0.1 Aug 3, 2026
@claude

claude Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Claude finished @cquil11's task in 2m 38s —— View job


Review

  • Gather context (PR diff)
  • Verify submodule commit matches the claimed SHA
  • Check repo references to aiperf for anything else needing an update
  • Post review

LGTM - no blocking issues found.

Verified:

  • The diff is exactly one submodule pointer bump: utils/aiperf 818c3a5a2b7b16cf851885567988a643282266bce74e34437, matching the SHA claimed in the PR description. The PR's CI run completed successfully, confirming the commit resolves from the public SemiAnalysisAI/aiperf repo.
  • No other file in the repo pins an aiperf version or tag (benchmarks/benchmark_lib.sh installs aiperf editable from the submodule path and already uses the --scenario inferencex-agentx-mvp flag), so no wrapper change is needed — consistent with the PR description.
  • The harness at benchmarks/benchmark_lib.sh:1944-1947 propagates a non-zero aiperf exit code after write_agentic_result_json and validation run, so the new coverage-failure path retains diagnostic artifacts as described.

One note (not blocking): I could not independently verify the agentx-v1.0.1 tag → commit mapping from this sandbox (network access to the aiperf repo was not permitted), so that association rests on the PR description and the passing CI checkout. Also, this is an intentional behavioral tightening — any existing recipe that produces <98% TTFT/ITL metric coverage will start failing sweeps after this merges.


@cquil11
cquil11 merged commit 65e0b1c into main Aug 3, 2026
6 checks passed
@cquil11
cquil11 deleted the agent/bump-aiperf-agentx-v1.0.1 branch August 3, 2026 23:06

@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.

Looks good — the actual change is a simple one-line submodule pointer bump for utils/aiperf to agentx-v1.0.1, adding the 98% profiling metric coverage check. The two flagged issues are nits (commit message escaping, missing bilingual PR title/description) and don't affect the correctness of the submodule pin itself.

Extended reasoning...

Overview

This PR touches a single file: the utils/aiperf submodule pointer, bumping it from 818c3a5a2 to b7b16cf85 (tagged agentx-v1.0.1). No InferenceX source code changes — it's purely a dependency pin update that pulls in a new check (98% profiling metric coverage) in the upstream aiperf tool so incomplete AgentX runs fail loudly instead of silently passing.

Security risks

None. There's no new code path introduced in this repo; the change only updates which commit of an external submodule is checked out. The behavioral change (stricter coverage validation failing incomplete runs) is a tightening of validation, not a loosening, so no new attack surface or bypass is introduced.

Level of scrutiny

Low. This is a mechanical, single-line dependency bump with an unambiguous intent (pin to a known upstream tag) and no logic in this repo to review. The two findings surfaced are both tagged [Nit] and concern process/documentation compliance (commit message escaping and missing bilingual PR title/description per AGENTS.md), not functional or security correctness of the change itself.

Other factors

No outstanding unaddressed reviewer comments in the timeline — this is a fresh PR with no prior review activity. Given the change is a trivial, correct submodule pointer update and both findings are minor process nits rather than substantive bugs, this doesn't warrant blocking on human review.

Comment thread utils/aiperf
@@ -1 +1 @@
Subproject commit 818c3a5a2922c535af6271ff296ed374e292b8e4
Subproject commit b7b16cf851885567988a643282266bce74e34437

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.

🟡 The squash commit for this PR (2bbe372) has a literal two-character \n\n escape sequence jammed between the English summary sentence and the required 中文: translation paragraph, instead of a real newline/paragraph break. This means git log, GitHub's commit view, and blame all render the backslash-n text verbatim rather than showing the translation as its own paragraph, per AGENTS.md's bilingual commit format requirement.

Extended reasoning...

The Bug

AGENTS.md requires every commit body to end with the Chinese translation as its own trailing 中文:<translation> paragraph — i.e. separated from the English body by a real blank line / paragraph break. The squash commit for this PR, 2bbe372, does not do this. Instead the English sentence and the Chinese translation are concatenated onto a single physical line, separated only by the two literal characters \ and n (twice), not an actual newline.

Proof

Running git log -1 --format=%B 2bbe372 | cat -A shows:

Pin the shared AIPerf submodule to agentx-v1.0.1 so AgentX runs reject incomplete profiling metric coverage while retaining diagnostic artifacts.\n\n中文:将共享 AIPerf 子模块...$

The cat -A output uses $ to mark real end-of-line. There is only one $ at the very end of this whole line — meaning everything before it, including the \n\n and the entire Chinese translation, is on the same physical line as the English sentence. If real newlines had been used, cat -A would show two separate lines each terminated by their own $. Instead, \n\n appears as four literal ASCII characters (backslash, n, backslash, n) sitting in the byte stream, immediately followed by the UTF-8 bytes of "中文:...".

Why this happened / why existing tooling did not catch it

This is the same escaping mistake present in the upstream aiperf commits (2e146d30, f77c31a7) that this PR pins to — almost certainly the commit message was built by a script/template that interpolated \n\n into a shell string without allowing the shell/language to interpret the escape as an actual newline (e.g. using single quotes around a string containing \n instead of $'...\n...' or printf). That same broken string then got reused verbatim when authoring the InferenceX squash commit body, so the defect propagated into this repo's history.

Impact

Nothing functional breaks — the Chinese translation content itself is present and correct, and the submodule pin is functionally fine. The only consequence is cosmetic: anyone viewing the raw commit message via git log, GitHub's commit page, git blame, or any tool that does not itself post-process \n sequences will see the literal escape text \n\n glued between the English and Chinese text rather than a clean paragraph break, which does not match the AGENTS.md requirement that the translation form its own trailing paragraph.

Fix

Reword the commit message (e.g. via git commit --amend if this commit has not been pushed/merged yet, or accept as a known-cosmetic issue if it has) so the body is built with a real newline character (e.g. $'...\n\n中文:...' in bash, or a multi-line string) between the English sentence and the 中文: paragraph, rather than a literal backslash-n sequence.

Comment thread utils/aiperf
@@ -1 +1 @@
Subproject commit 818c3a5a2922c535af6271ff296ed374e292b8e4
Subproject commit b7b16cf851885567988a643282266bce74e34437

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.

🟡 This PR's title and description are English-only, violating the repo's bilingual policy in AGENTS.md line 7, which requires PR titles to follow <English title> / <中文标题> and descriptions to include a ## 中文说明 section. Please add a Chinese translation to both the title and description.

Extended reasoning...

What's wrong: AGENTS.md line 7 states an explicit, repo-wide policy: > **PR and GitHub-issue titles & descriptions must be bilingual — include a Simplified Chinese version in addition to English.** Title format: / <中文标题>. In the PR/issue body, follow the English content with its Chinese translation (e.g. a ## 中文说明 section mirroring the summary...). It explicitly states "This applies to every PR and every issue."

Where this PR falls short: The title is fix(agentx): bump AIPerf to v1.0.1 — English only, missing the required / <中文标题> suffix. The description is a two-paragraph English summary of the submodule bump with no ## 中文说明 (or equivalent) section translating it into Simplified Chinese.

Why nothing else catches this: This is a metadata/process check on the PR's title and body text, not something enforced by CI, tests, or the diff itself (the diff is just a one-line submodule pointer bump, so there's no code path that would surface this). It's purely a policy compliance gap that a human or reviewing agent has to check against AGENTS.md directly.

Step-by-step verification:

  1. Read AGENTS.md line 7: confirms the bilingual title/description mandate and its exact required formats.
  2. Read the PR title: fix(agentx): bump AIPerf to v1.0.1 — no / <中文标题> component present.
  3. Read the PR description: two English paragraphs (submodule pin + rationale about the 98% profiling metric coverage check) — no Chinese section anywhere.
  4. Conclusion: the policy is violated in both required places (title and body).

Impact and fix: Nothing functionally breaks — the actual change (bumping the utils/aiperf submodule pointer) is correct and self-contained. This is purely a documentation/process compliance issue. The author can fix it by renaming the title to something like fix(agentx): bump AIPerf to v1.0.1 / 修复(agentx):将 AIPerf 升级至 v1.0.1 and appending a ## 中文说明 section to the description summarizing the change in idiomatic Simplified Chinese, per the translation quality bar also defined in AGENTS.md.

charxwu pushed a commit that referenced this pull request Aug 6, 2026
Pin the shared AIPerf submodule to agentx-v1.0.1 so AgentX runs reject incomplete profiling metric coverage while retaining diagnostic artifacts.\n\n中文:将共享 AIPerf 子模块固定到 agentx-v1.0.1,使 AgentX 运行能够拒绝分析指标覆盖不完整的结果,同时保留诊断产物。

Signed-off-by: Cam Quilici <cjquilici@gmail.com>
jiacao-amd added a commit that referenced this pull request Aug 6, 2026
Brings in fix(agentx): bump AIPerf to v1.0.1 (#2484) [65e0b1c].

# Conflicts:
#	perf-changelog.yaml
jiacao-amd pushed a commit that referenced this pull request Aug 7, 2026
Resolves the perf-changelog.yaml conflict by keeping both sides: the
branch's dsv4-fp4-mi355x-vllm-agentic-mtp DEP8 entry and main's
dsr1/dsv4-b200/gb300/glm5.2/qwen3.5 entries are independent appends at
the same list position.

Brings in the AIPerf submodule bump past agentx-v1.0.1 (b7b16cf, #2484)
to main's current 743fb82 (#2504, full-response ITL + interactivity
aggregation).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

1 participant