Skip to content

refactor: centralize results in infx and speed up tests / 集中结果处理并加速测试 - #2940

Merged
adibarra merged 1 commit into
mainfrom
refactor/infx-results
Sep 9, 2026
Merged

refactor: centralize results in infx and speed up tests / 集中结果处理并加速测试#2940
adibarra merged 1 commit into
mainfrom
refactor/infx-results

Conversation

@adibarra

@adibarra adibarra commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

Description / 说明

Result processors duplicated component metadata validation and power-metric replacement, while fixed-sequence conversion was only available through a script. This change exposes composable functions in infx.results and routes existing processors through the shared rules.

结果处理器重复实现了组件元数据校验和功耗指标替换,固定序列长度的结果转换也只能通过脚本调用。本次将可组合的函数集中到 infx.results,让现有处理器复用相同规则。

  • infx.results.fixed_sequence.build_result accepts explicit, read-only inputs; CLI adapters own environment and artifact I/O. Shared metadata parsing preserves each caller's validation policy, and power transformations accept the metric family and schema version.

  • Existing commands, environment variables, output JSON, failure diagnostics, exit codes, and best-effort power fallback remain compatible on the tested cases. Recipe maintainers need no changes or new runtime dependencies.

  • Test fixtures control clocks and wait for observable readiness while retaining real process termination, HTTP requests, and artifact writes. The changelog CI job uses four pytest-xdist workers; parameter collection is deterministic.

  • infx.results.fixed_sequence.build_result 接收显式只读输入,CLI 适配层负责环境变量和产物读写。共享元数据解析保留各调用方的校验策略,功耗转换由调用方传入指标集合和模式版本。

  • 在已验证用例中,现有命令、环境变量、输出 JSON、失败诊断、退出码和尽力完成的功耗处理回退行为保持兼容。配方维护者无需调整,也没有新增运行时依赖。

  • 测试 fixture 控制时钟,并根据可观察到的就绪状态结束等待,同时保留真实的进程终止、HTTP 请求和产物写入。changelog CI 任务使用四个 pytest-xdist worker,参数化用例收集顺序保持确定。

Related Issue / 关联事项

Continues the internal package cleanup from #2911. / 延续 #2911 的内部包结构整理。

Type of Change / 变更类型

  • Other: internal refactor / 其他:内部重构
  • Configuration change: test execution only / 配置变更:仅测试执行
  • Documentation update / 文档更新

Validation / 验证

  • Rebased onto 8260f95de; final commit f6f726a7107eae3cd5482ad240ae40b308847b20: python -m pytest utils/ runners/ experimental/CollectiveX/tests/ -n 41,223 passed, 207 subtests passed, zero skips, 13.93s locally.

  • Compatibility baseline b51c65807: the same 254 behavior tests passed before and after extraction; two additional tests cover the new importable APIs. The relevant pre-refactor production files are unchanged between that baseline and the rebased main.

  • 531 CLI comparisons: 81 captured test invocations, 446 boundary cases, and four direct power CLI checks. Exit status, stdout, JSON artifacts, and diagnostics matched after normalizing temporary paths and traceback frames; exception types, messages, and chaining were retained.

  • Independent expected values cover arithmetic, rounding, malformed input rejection, metadata policies, stale power removal, and internal-error fallback. Negative controls confirm malformed aggregate coercion and missing process termination are detected; six concurrent repeat runs passed the 12 lifecycle checks.

  • Before the rebase added upstream tests, the same 1,192-test suite went from 43.91s to 10.39s locally through controlled waits and four workers. This is a local execution comparison, not hosted CI duration.

  • actionlint passes for both changed workflows; git diff --check passes.

  • 已变基到 8260f95de;最终提交 f6f726a7107eae3cd5482ad240ae40b308847b20 运行上述完整命令,本地 1,223 个测试及 207 个子测试通过,无跳过,耗时 13.93s

  • 兼容性基线为 b51c65807:提取前后相同的 254 个行为测试均通过,另有两个测试覆盖新的可导入 API。该基线与变基后的 main 之间,相关的重构前生产代码没有变化。

  • 531 次 CLI 对照包括 81 次测试调用、446 个边界用例和四项直接调用功耗 CLI 的检查。规范化临时路径和 traceback 帧后,退出状态、stdout、JSON 产物和诊断一致;保留了异常类型、消息及异常链。

  • 使用独立预期值验证计算、舍入、无效输入拒绝、元数据策略、旧功耗字段清理和内部错误回退。负向对照确认测试能发现错误的聚合数据强制转换及缺失的进程终止;六次并发重复运行中的 12 项生命周期检查全部通过。

  • 在变基引入上游新测试前,同一组 1,192 个测试通过可控等待和四个 worker,本地耗时从 43.91s 降至 10.39s。这是本地执行对比,并非托管 CI 的总耗时。

  • 两个修改后的工作流均通过 actionlintgit diff --check 通过。

Checklist / 检查清单

  • Tested locally, including the rebased commit / 已完成本地测试,包括变基后的提交
  • Updated English documentation and synchronized Simplified Chinese pages / 已更新英文文档及对应简体中文页面
  • Performance changelog: not applicable; changes cover postprocessing, tests, and test CI, with no recipe or benchmark-performance changes. / 性能 changelog:不适用;本次仅涉及后处理、测试及测试 CI,不修改配方或基准性能行为。
  • Sweep reuse and CODEOWNER sign-off remain pending review; this draft does not claim GPU sweep/eval evidence. / 扫描复用及 CODEOWNER 签署留待评审;本草稿不声明已取得 GPU 扫描或评测证据。

Note

Medium Risk
Touches benchmark aggregate JSON and power-metric shaping on fixed-sequence and AgentX paths; behavior is intended to stay CLI-compatible but regressions would affect CI ingest contracts.

Overview
Introduces infx.results so fixed-sequence aggregation, component metadata parsing, and power-metric replacement are importable library functions instead of duplicated script logic. utils/process_result.py becomes a thin CLI shim to infx.results.fixed_sequence, while aggregate_power, aggregate_power_multinode, and AgentX aggregation route through with_power_metrics and parse_component_metadata.

Tests and CI add pytest-xdist (-n 4 in the changelog-gate job), mock clocks / readiness-based waits to avoid hung workers, and expand coverage for builders, power scrubbing, and multinode power fallback. test-process-result.yml now triggers on infx/results/**. Architecture and testing docs (EN/ZH) document the new extension points and parallel local runs.

Reviewed by Cursor Bugbot for commit f6f726a. Bugbot is set up for automated code reviews on this repo. Configure here.

Extract composable result builders, component metadata parsing, and power
metric transformations while preserving legacy CLI and artifact behavior.
Expand independent behavior checks and shorten tests with controlled waits
and bounded parallel execution.

将可组合的结果构建函数、组件元数据解析和功耗指标转换集中到 infx,
保留现有 CLI 和产物行为。补充独立的行为测试,并通过可控等待和
有限并行执行缩短测试时间。
@adibarra
adibarra marked this pull request as ready for review September 9, 2026 20:42
@adibarra
adibarra requested a review from a team September 9, 2026 20:42
@claude

claude Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Claude finished @adibarra's task in 5m 29s —— View job


Review of PR #2940

  • Gather context (diff vs origin/main, repo guidelines)
  • Review new infx/results/ package (fixed_sequence, metadata, power)
  • Review refactored callers (utils/process_result.py, aggregate_power*, agentic aggregation)
  • Review test changes and workflow changes
  • Post findings and summary

LGTM - no blocking issues found.

I traced the old utils/process_result.py logic line-by-line against infx/results/fixed_sequence.py: field ordering, validation messages, exit codes, the REQUIRE_POWER policy, and both internal-error fallback paths are preserved. parse_component_metadata keeps each caller's exact error type (ValueError vs SystemExit) and messages, and with_power_metrics matches both _patch_power_result and _patch_agg semantics (rounding precision, non-finite rejection before write, non-mapping passthrough). Import compatibility holds across all invocation modes I could find: the compat shim in utils/process_result.py, the if not __package__ shim in utils/aggregate_power.py (which also makes the direct-execution path of aggregate_power_multinode.py work, since its infx import runs after the aggregate_power import), the re-exported WHOLE_METRIC_KEYS/ROLE_METRIC_KEYS used by utils/test_process_result.py:11, and pythonpath = .. in utils/pytest.ini for the CI invocation from utils/. Docs are synchronized between English and Chinese pages, and test-process-result.yml correctly gains the infx/results/** trigger path.

Note: I could not execute the test suite in this environment (Bash approval restrictions on python/pip), so this review is static; the Test Process Result and changelog-gate CI runs on this PR will confirm the claimed results.

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

I reviewed this PR and didn't find any bugs. Because it's a large refactor of core result/power aggregation code plus a CI concurrency change, a human look would still be worthwhile.

What was reviewed:

  • The new infx.results package (fixed_sequence, metadata, power) and how aggregate_power.py, aggregate_power_multinode.py, process_result.py, and process_agentic_result.py now route through its shared metadata/power-metric logic.
  • The switch to 4 pytest-xdist workers in test-changelog-gate.yml — checked utils/evals/test_run_eval_dispatch.py's HOME/env-mutation tests for cross-worker interference; each test scopes its own temp state, so no shared-state race was evident.
  • aggregate_power_multinode.py's new top-level from infx.results.power import (...) lacks the sys.path guard aggregate_power.py has for direct execution; it works today only because utils/pytest.ini sets pythonpath = .. and no current caller invokes the script directly outside pytest.
Extended reasoning...

Overview

This PR is a substantial internal refactor: it extracts result-processing logic (fixed-sequence result building, component metadata validation, power-metric replacement) that was duplicated across utils/process_result.py, utils/agentic/aggregation/process_agentic_result.py, and the power aggregation scripts into a new infx/results package (fixed_sequence.py, metadata.py, power.py), then routes the existing CLI entry points through the shared implementation. It also changes CI test execution to use pytest-xdist (4 workers) for the changelog-gate workflow, and updates docs and several test files to match. Twenty files changed across production code, tests, workflows, and docs — a wide but conceptually cohesive change.

Security risks

No injection, auth, or data-exposure surface is touched — this is internal benchmark result/telemetry post-processing and test infrastructure. The main risk class here is correctness/behavioral regression (e.g., silently changing exit codes, JSON schema, or power validity semantics that downstream tooling depends on) and CI-concurrency-induced test flakiness, not classic security vulnerabilities.

Level of scrutiny

This warrants a moderate-to-high level of scrutiny: it is a large, multi-file refactor of code that governs benchmark power/energy metric integrity (REQUIRE_POWER, power_valid, energy accounting) and it changes how CI parallelizes tests, both of which have already produced one plausible latent issue found during review — aggregate_power_multinode.py gained a top-level infx.results.power import with no fallback sys.path guard, unlike its sibling aggregate_power.py, which does have one for direct-script execution. Under the test suite this is masked by utils/pytest.ini's pythonpath = .., and no current caller runs the script directly outside pytest, so it isn't exercised as a live bug today — but it is an inconsistency worth a maintainer's eyes given the stated goal of preserving direct-execution CLI compatibility.

Other factors

The PR description cites extensive local validation (1,223 tests, 531 CLI comparisons, actionlint) but this is unverified self-reported testing from the author, not independent evidence. Given the scope of the refactor, the power/metadata semantics involved, and the one inconsistency noted above, I'm deferring rather than approving even though the automated bug hunt reported no confirmed bugs.

@adibarra
adibarra merged commit 533c1b2 into main Sep 9, 2026
12 checks passed
@adibarra
adibarra deleted the refactor/infx-results branch September 9, 2026 21:04
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