fix(sbd): stop reporting GPU telemetry that was read but never parsed

[luochen-amd]

Problem

Every single-node session ships a gpu_monitor_aggregate of all zeros.

Not "no data" — the reports were found and opened. A production breakdown shows
samples: 27 next to avg_power_w: 0.0, while the same session's
benchmark_report.json carries power_watts avg 774.8. Twenty-seven reports
were read and every field came back empty.

The archive tells the same story: gpu_metrics.csv has 2 objects, newest
2026-06-14, against 11,620 server.logs over the same window.

Root cause

Three independent defects stacked in _aggregate_gpu_monitor:

	Defect
Key names	Magpie writes power_watts / gpu_clock_mhz; the aggregator looked for power_w / power and clock_mhz / sclk_mhz. Nothing matched.
Shape	temperature_c did match by name, but Magpie's value is a nested {min, max, avg} block and _to_float returns None for a dict. The one key that lined up was dropped silently anyway.
Fallback	_avg("power_w") or _avg("power") used or while _avg returned 0.0 for "absent". A metric nobody sampled and a card genuinely drawing 0.0 W were the same value, so the alias fired on a real reading and an all-absent metric shipped as a plausible-looking zero.

Any one of these alone would have surfaced as obviously-missing data. Together
they produce a well-formed section of zeros that nobody can distinguish from a
quiet GPU, which is why it lasted.

Fix

Both producer shapes are read through one _gpu_metric helper, the current
Magpie key names lead each alias tuple, and every metric is tri-state —
float | None, never coerced to 0.0. Read them with is None, not
truthiness: a real 0.0 is falsy too.

Two changes go slightly past a minimal repair

Trim them if you disagree:

• samples now counts underlying samples (the sum of each block's
  sample_count) rather than blocks, which is what its docstring always
  claimed. blocks is added alongside because the two answer different
  questions — blocks says reports were found, samples says how much
  measurement stands behind the numbers. That distinction is exactly what made
  this bug findable.
• Averages are weighted by sample_count. Unweighted, a 10-sample block pulls
  the session mean as hard as a 10,000-sample one.

Verification

Adds the function's first unit tests — it had none. Eight cases, seven of
which fail against the pre-fix code; the first is the verbatim production
Magpie block that used to aggregate to zeros.

Schema change is additive, so schema_version is unchanged, and the markdown
reporter does not render telemetry.

Notes

Independent of the KV-cache work in #1446 — no shared files, can merge in
either order. Worth merging first: single-node GPU data is being lost right now,
and the measurement-validity checks planned on top of this plan read
clock_mhz, which is one of the fields currently stuck at zero.

Refs: kvcache-metrics-plan.md §4.4, §8.2

[github-actions]

CI E2E report — ❌ Timeout

item	value
result	❌ Timeout
model	Qwen/Qwen3-0.6B (dense)
resources	1× GPU, TP=1
PR branch	fix/gpu-monitor-aggregate-keys
commit	8e0c941645d5dd4145d6242fa21af0d6a5933f71
session_id	c514abc8-34f5-4fa8-84c4-a4ffe1aaee9e
queue → dispatch	-38904s
run time	672m 24s
total	24m 0s
reason	Timed out — the run never reached a terminal state in time (task stuck, or the GPU stayed queued too long).
detail	not terminal after 13200s

details

[haishuok0525]

Reviewed for correctness only. I read the diff plus _common._to_float, the collect_telemetry → exporter call chain, and every consumer of gpu_monitor_aggregate / avg_power_w in the tree, then extracted the new _gpu_metric / _aggregate_gpu_monitor into a standalone harness and ran it. All eight test assertions hold, including the weighted 298.02.

The core fix looks right and I did not find a regression. The multi-node injector's flat power_w / temperature_c / clock_mhz are still covered by the alias tuples; nothing downstream actually reads the five metrics (the avg_power_w in benchmark_result.py belongs to its own gpu_monitor_by_role aggregate), so float → float | None breaks no consumer; schema.py has from __future__ import annotations, so float | None in the TypedDict is fine on 3.10; ruff only selects E/F/W here, so the bare zip is not a B905 hit.

Four things I'd call bugs.

1. The doc claims the key is absent, but it is always present as {}

The new doc paragraph says gpu_monitor_aggregate "is absent entirely when no benchmark report carried a gpu_monitor block". collect_telemetry writes the key unconditionally:

"gpu_monitor_aggregate": _aggregate_gpu_monitor(all_reports, warnings),

_aggregate_gpu_monitor returning {} still leaves the key in the section, and nothing on the exporter._pick / _safe_collect path prunes empty values. A consumer following the doc with if "gpu_monitor_aggregate" not in telemetry gets the wrong answer. Either say "empty object" in the doc, or actually omit the key in collect_telemetry.

2. samples can be large while every metric is None

samples is int(sum(weights)), summed over all blocks regardless of whether a block contributed any metric at all:

in:  {"sample_count": 27000, "duration_sec": 10.0}
out: {'samples': 27000, 'blocks': 1, 'avg_power_w': None, ...}

That is the same "well-formed section with heavy-looking sample counts behind nothing measured" shape this PR sets out to remove. _avg already computes a per-metric weight_sum; either surface that, or have samples count only blocks that contributed at least one reading.

3. weights uses the falsy fallback the PR argues against

weights = [max(1.0, _to_float(b.get("sample_count")) or 1.0) for b in blocks]

sample_count: 0 — a monitor that started and sampled nothing — takes the or 1.0 branch and is reported as one sample. This is the same pattern as the _avg("power_w") or _avg("power") form the description calls out. An explicit is None check before deciding whether to clamp would be consistent with the rest of the change.

4. avg and max can resolve to different aliases within one block

_gpu_metric does continue when the key is present but field is not, so it falls through to the next alias, and the two statistics can come from different keys:

in:  {"power_watts": {"avg": 300.0}, "power_w": 12.0}
out: avg_power_w = 300.0 (from power_watts), max_power_w = 12.0 (from power_w)

The result is a max below the avg. A related variant, {"power_watts": {"avg": None, "max": 310.0}}, yields avg=None, max=310.0. Magpie writes all three statistics today so this cannot fire yet, but the failure shape — a producer omitting one field and the aggregator silently emitting a self-contradictory number — is the same class of defect this PR fixes. Safer to select the alias once, on "key present and at least one statistic parses", and not fall through afterwards.

Minor and ignorable: int(sum(weights)) truncates rather than rounds, which only matters if sample_count is ever non-integral.

I'd fix #4 before merge since it is cheap and squarely on-topic. #1–#3 are fine to fold into a follow-up.

[haishuok0525]

One remaining telemetry gap: this fixes parsing for power, temperature, and GPU clock, but it still does not provide GPU utilization or VRAM usage.

The multi-node injector already emits flat gpu_util_pct and vram_pct samples, but _aggregate_gpu_monitor ignores both fields. The single-node Magpie block used by the regression test contains only power_watts, temperature_c, and gpu_clock_mhz, so this change also does not add utilization or VRAM collection on that path.

As a result, gpu_monitor_aggregate remains unable to report whether the GPU was compute-idle or memory-constrained even when the multi-node source carried those readings, and single-node sessions still have no such readings at all. Please extend the producer/normalization path as needed and add aggregate fields for average/peak GPU utilization and VRAM usage, with coverage for both the Magpie nested shape and the multi-node flat shape.

[luochen-amd]

Added, with one finding that changes what "coverage for the Magpie nested shape" can mean.

Multi-node: done. _aggregate_gpu_monitor now reads gpu_util_pct and vram_pct and reports avg_gpu_util_pct / max_gpu_util_pct / avg_vram_pct / max_vram_pct, through the same alias resolution, sample-count weighting and tri-state path as power and temperature. A block that yields only occupancy now counts as contributing, so a util-only source is no longer scored as an empty block.

Single-node: the readings do not exist to collect. I checked 13 production benchmark_report.json files rather than reasoning from the fixture. Every Magpie gpu_monitor block carries exactly:

{"sample_count": 59, "duration_sec": 124.02,
 "temperature_c": {"min": 46.0, "max": 68.0, "avg": 61.3},
 "gpu_clock_mhz":  {"min": 131, "max": 2096, "avg": 1411.6},
 "mem_clock_mhz":  {"min": 900, "max": 1300, "avg": 1171.2},
 "power_watts":    {"min": 149.0, "max": 751.0, "avg": 524.3}}

No utilization, no VRAM, in any of them. The regression fixture was thin because the producer is thin. And Magpie is external (github.com/AMD-AGI/Magpie, installed by install.sh) -- its GPUMonitor is not in this tree, so the producer side of single-node cannot be extended from this PR. The only in-repo lever is _magpie_patcher.py, which patches Magpie's benchmarker.py at runtime; extending that to rewrite its telemetry collection is a materially riskier change than this one and I would rather not smuggle it in here. Happy to open it as a follow-up against Magpie (or as a patcher change) if you want that path -- say which.

So single-node reports None for both, and the docs now state that plainly with a per-topology table. None is load-bearing here: 0% utilization asserts the GPU sat idle through the round, which is the opposite of "nobody sampled it", and that conflation is what this branch started from.

Coverage. Both shapes are tested: the flat multi-node samples, and the nested {min,max,avg} form for util/VRAM (which pins the parsing so it works the day a producer emits them). Plus: a real Magpie block reports None rather than 0.0; a genuinely idle GPU at 0.0 is kept as a reading; a mixed-topology session keeps the multi-node occupancy; and vram_used_mb / memory_used_bytes are not accepted into a _pct field -- an absolute MiB reading is a different quantity, and letting it fall through would put 81920 where a percentage belongs, which is the same class of bug as the original one.

End to end over those 13 real reports:

{"samples": 1984, "blocks": 13,
 "avg_power_w": 347.8, "max_power_w": 753.0,
 "avg_temp_c": 57.52, "max_temp_c": 78.0, "avg_clock_mhz": 1732.54,
 "avg_gpu_util_pct": null, "max_gpu_util_pct": null,
 "avg_vram_pct": null, "max_vram_pct": null}