[Power] feat: stamp whole-deployment semantics with a schema version / 为功耗指标加盖整机语义版本号

[edwingao28]

What

Stamp every power result — valid or invalid, single-node or multinode — with power_metric_schema_version = 2.

Version 2 defines each unprefixed joules_per_* field as whole-deployment GPU-board energy over the named denominator. Role-scoped energy keeps its explicit prefill_* / decode_* keys.

Why

The unprefixed joules_per_* fields silently switched from role-local to whole-deployment energy when multinode aggregation landed. The values alone cannot distinguish the two semantics, so any consumer comparing a pre-switch row against a post-switch row is off by the prefill/decode split ratio without any way to detect it.

Stamping the semantics lets consumers fail closed on unversioned rows instead of guessing.

Scope

Producer-side only. No numeric value changes — this adds one field and nothing else.

The consumer-side guard that acts on the field lands in a follow-up InferenceX-app PR. Merge order matters: this must land first, otherwise the app guard has nothing to read.

Verification

utils/ power suites: 126 passed.

test_aggregate_power.py            52 passed
test_aggregate_power_multinode.py  24 passed
test_process_result.py             50 passed

Docs: .github/AGENT_OPERATIONS.md power-telemetry section records the new field and its contract.

[claude]

🟡 Three modules independently reset the power fields on the aggregate JSON (pop stale keys, stamp power_metric_schema_version, set power_valid, pop power_invalid_reasons) — aggregate_power.py::_patch_power_result (762-767), aggregate_power_multinode.py::_patch_agg (~1167-1171), and process_result.py::record_power_internal_error (~77-83). This PR adds the same new schema-version line to all three copies instead of factoring the shared stamp logic into one helper in aggregate_power.py (which the other two already import from). Not a correctness issue, but the next contract change (e.g. bumping to version 3) has to be made correctly in three places with three different key-set variables.

Extended reasoning...

What: aggregate_power.py's _patch_power_result, aggregate_power_multinode.py's _patch_agg, and process_result.py's record_power_internal_error each contain an almost-identical four-step block: pop a set of stale power metric keys, set data['power_metric_schema_version'] = POWER_METRIC_SCHEMA_VERSION, set power_valid (as int(power_valid), int(audit.power_valid), and the literal 0 respectively), and pop power_invalid_reasons. This PR's diff makes the triplication visible in the most direct way possible: it adds the exact same new line — data["power_metric_schema_version"] = POWER_METRIC_SCHEMA_VERSION — to all three functions, each as a separate hunk in three different files.\n\nWhy it happens: the only real variation between the three sites is the set of keys being popped (_POWER_METRIC_KEYS vs _ALL_POWER_METRIC_KEYS vs _POWER_METRIC_KEYS + _MULTINODE_ROLE_METRIC_KEYS) and where power_valid comes from. process_result.py's copy exists for a legitimate reason — it's the fallback path used when aggregate_power/aggregate_power_multinode itself failed to import, so it deliberately keeps its own literal copy of the multinode role-metric keys rather than importing them. But that constraint only explains the key-set duplication, not the stamp/valid/pop sequence itself, which could still be centralized.\n\nWhy nothing catches this today: there's no test or lint rule enforcing that the three envelope-patching functions stay in sync; they're only kept consistent by whoever remembers to touch all three when the contract changes. This PR is itself proof of the pattern working correctly this time (all three got the new line), but that's diligence, not a structural guarantee.\n\nImpact: none today — the PR is correct as written and all 126 tests pass. The cost is future maintenance risk: the next schema-affecting change (e.g. bumping to version 3, adding a new required top-level field, or changing how power_invalid_reasons is handled) requires editing three call sites with three different key-set variables and remembering that process_result.py's copy is a fallback that must independently track aggregate_power_multinode.ROLE_METRIC_KEYS as a literal. Missing one site silently leaves stale keys or an unstamped schema version on some code path, which is exactly the kind of drift this PR itself was written to close (the unprefixed joules_per_* semantics silently changed once already without any version marker).\n\nSuggested fix: extract a small helper in aggregate_power.py, e.g. _stamp_power_envelope(data, *, metric_keys, power_valid) that performs the pop-loop, schema-version stamp, power_valid assignment, and power_invalid_reasons pop in one place. aggregate_power_multinode.py and process_result.py already import constants from aggregate_power.py, so they could import this helper too — process_result.py's fallback path would still pass its own literal _POWER_METRIC_KEYS + _MULTINODE_ROLE_METRIC_KEYS tuple as the metric_keys argument, preserving its independence from the multinode module's import.\n\nProof of the triplication — grep the three sites: aggregate_power.py:762-767 has for key in _POWER_METRIC_KEYS: data.pop(key, None) then the stamp/valid/pop trio; aggregate_power_multinode.py:_patch_agg (~1167-1171) has for key in _ALL_POWER_METRIC_KEYS: data.pop(key, None) then the identical stamp/valid/pop trio; process_result.py:record_power_internal_error (~77-83) has two pop loops (_POWER_METRIC_KEYS then _MULTINODE_ROLE_METRIC_KEYS) then the same trio again. Diff each function's tail four lines side by side and they are structurally identical modulo the power_valid source expression.

[claude]

🟡 In _patch_power_result, non-finite metrics (e.g. an absurd-but-finite power sample like 9e307 W overflowing the trapezoidal integration to inf) are only detected inside the per-key write loop, which raises ValueError before _write_json_atomic runs. Since run()'s except handler doesn't retry the write, the on-disk agg keeps whatever process_result.py originally wrote -- no power_metric_schema_version and no power_valid -- breaking this PR's own contract that every power result carries the schema version. The multinode sibling (aggregate_power_multinode.py) already guards this exact overflow case up front so _patch_agg still stamps power_valid=0 plus the schema version; aggregate_power.py should get the analogous guard.

Extended reasoning...

What happens: In utils/aggregate_power.py, run() calls _derived_metrics() unguarded and passes the result straight to _patch_power_result(). That function only checks for non-finite values inside its per-key write loop (if value is None or not math.isfinite(value): raise ValueError(...)), and this check happens after it has already mutated the in-memory data dict (stamping power_metric_schema_version and power_valid) but before _write_json_atomic(agg_path, data) is called. Since the raise happens before the write, the on-disk agg file is left completely untouched by this invocation.

Trigger path: integrate_power() only rejects a power sample if it's non-finite or negative (not math.isfinite(power) or power < 0). An absurd-but-finite value like 9e307 W passes that check. When _integrate_device() sums such values across the trapezoidal integration, the result overflows to inf. power_valid stays True (no reasons were ever appended), so _derived_metrics() runs and returns a metrics dict containing inf/nan values (it only guards against None, not non-finite floats). _patch_power_result() then raises ValueError on the first such key, mid-loop, before the atomic write.

Why the except handler doesn't save it: run() wraps the _patch_power_result() call in except (OSError, json.JSONDecodeError, ValueError), which does catch the raised error -- so there's no crash and the run still exits 0 (or 1 under REQUIRE_POWER). But the handler only resets the local power_valid/metrics variables that feed the sidecar/validation JSON; it never re-invokes _patch_power_result or otherwise re-writes the agg JSON. The agg file on disk ends up exactly what process_result.py wrote before power aggregation ran -- meaning it has neither power_metric_schema_version nor power_valid.

Why this matters for this PR specifically: the PR's entire premise (per AGENT_OPERATIONS.md) is that every power result, valid or invalid, carries power_metric_schema_version, and that rows missing the field predate the whole-deployment semantics switch. This bug produces a post-PR row that is missing the field for an unrelated reason (an aggregation-time crash), which a downstream consumer would misinterpret as "this row predates the schema" rather than "this row's power integration overflowed."

The sibling module already fixed this exact case: aggregate_power_multinode.py's validate_and_integrate() computes metrics, then explicitly checks non_finite = sorted(key for key, value in metrics.items() if not math.isfinite(value)) before calling _patch_agg, and if any are found it adds reason non_finite_power_metric and returns an invalid audit -- so _patch_agg still runs and stamps power_valid=0 plus the schema version. There's even a dedicated test, test_overflowing_power_metric_is_invalid_not_stale, proving the maintainers consider this scenario test-worthy. aggregate_power.py never received the analogous up-front guard.

Proof walkthrough:

1. Telemetry CSV contains a power reading of 9e307 W for a GPU device across the benchmark window.
2. integrate_power(): the per-sample check not math.isfinite(power) or power < 0 is False (9e307 is finite and positive) so the sample is accepted with no reasons appended.
3. _integrate_device(): the trapezoidal sum (right_time - left_time) * (left_power + right_power) / 2.0 overflows float64 range and returns inf.
4. Back in integrate_power(), per_gpu_energy_j contains inf; total_gpu_energy_j = sum(per_gpu_energy_j.values()) is inf; power_valid = not reasons is still True.
5. run() calls derived_metrics(integration, benchmark) since power_valid and benchmark is not None, returning a dict where total_gpu_energy_j, joules_per*, etc. are all inf.
6. _patch_power_result(agg_result, power_valid=True, metrics={...inf...}) is invoked: it pops old keys, sets data['power_metric_schema_version'] = 2 and data['power_valid'] = 1 in memory only, then in the per-key loop hits the first inf value and raises ValueError('non-finite power metric: ...') before ever reaching _write_json_atomic.
7. run()'s except (OSError, json.JSONDecodeError, ValueError) catches it, appends aggregate_result_unwritable to the local reasons list, and resets local power_valid = False, metrics = {} -- but these locals only feed the validation sidecar, not the agg file.
8. Final state: agg_.json on disk is exactly what process_result.py originally wrote -- no power_metric_schema_version, no power_valid key at all -- even though the sidecar (power_validation_.json) correctly records power_valid: false and the reason.

Suggested fix: mirror the multinode guard -- compute the non-finite metric keys up front in run() (or inside _derived_metrics / before calling _patch_power_result) and fall back to power_valid=False, metrics={} so _patch_power_result is always called with values that can be written, guaranteeing the schema version and power_valid=0 land on disk even in this overflow case.