fix: omit Apple Silicon GPU metrics instead of reporting 0 (#337)
## Summary
When `NativeMetricsManager::new()` fails, the singleton in `src/device/macos_native/manager.rs:51` stays empty for the life of the process. The Apple Silicon GPU reader fell back to `GpuMetrics::default()` and unwrapped every field to a literal zero, so a macOS host without IOReport published "GPU 0% / 0 W / 0 degrees" for a device that was measuring nothing. Zero is a legitimate reading for an idle GPU or a parked ANE, so no consumer could tell the two apart. This makes absence explicit instead, and aligns the four macOS readers on one written-down policy.
## The four readers as found
| Reader | Before | After |
|---|---|---|
| Memory (`src/device/memory_macos.rs:45`) | Unaffected. Pure `sysinfo`, no failure path. | Unchanged. |
| CPU (`src/device/cpu_macos.rs:160`) | Degraded honestly. `Option` fields go `None`, frequency falls back to `sysctl`. | Unchanged. This was already the correct behavior and became the model for the others. |
| Chassis (`src/device/readers/chassis/apple_silicon_native.rs:52`) | Absent. `get_chassis_info` returns `None`, no series at all. | Unchanged. Every field it reports comes from the manager, so there is nothing left to say. |
| GPU (`src/device/readers/apple_silicon_native.rs:165`) | **Fabricated.** `unwrap_or(0)` at `:216`, `:229`, `:236`, `:237`. | Emits the row with identity and memory intact; the five IOReport/SMC-sourced fields are marked absent. |
The unifying rule, now documented in the `manager` module docs and referenced from the `GpuReader` trait: **a reader emits a row whenever it can still say something true about the device, marks the fields it could not source as absent, and never substitutes 0.** It suppresses the row only when the device cannot be identified at all.
## Omission versus an explicit absence signal
**Both, at different layers, because they answer different questions.**
For the value series, omission. It is Prometheus' own convention for no-data and it is already this exporter's house style: `all_smi_gpu_performance_state` and the four thermal thresholds have been omitted-when-absent since #132, with a comment at `src/api/metrics/gpu.rs` saying exactly why. Introducing a second convention for five neighbouring families in the same exporter would be worse than either convention alone. The obvious objection, that a series vanishing mid-scrape resembles a dead target, is weaker here than it looks: the target is not silent. `all_smi_up` and `all_smi_build_info` from #333 are unconditional, the memory/CPU/disk families still render, and `all_smi_gpu_info` for this very device still renders. "Device present, not reporting" and "device gone" stay distinguishable, and `absent()` over a still-present `all_smi_gpu_info` expresses the first one precisely.
For the reason, an explicit signal. Omission says nothing about *why*, and "no IOReport on this host" is a permanent, actionable condition rather than a transient gap. The reader now sets `detail["native_metrics"] = "available" | "unavailable"`, which rides the already-emitted `all_smi_gpu_info` identity series as a label. This costs no new metric family, adds no new absence pattern to learn, and is queryable directly. `all_smi_gpu_info` already carries a churning label set on this platform (`thermal_pressure`, `combined_power_mw`), so one more conditional label is not a new problem class.
**Does the policy differ per metric?** No, deliberately. Temperature is the one worth arguing about: a vanishing temperature series could break a `max_over_time` thermal alert. But the alternative breaks it worse. `gpu_temp > 80` never fires either way, while a fabricated `0` makes any `gpu_temp < N` alert fire spuriously and drags every cluster temperature average down. More importantly all five fields come from a single subscription and fail together, so a split policy would force a dashboard to learn two rules for one failure mode. The codebase had already reached this conclusion internally: the TUI, the alert engine and the filter DSL all treated `temperature == 0` as unknown before this PR.
## Reader or exposition
**Both, with different jobs, because a fix in either one alone is incomplete.**
The reader must stop fabricating, because its `GpuInfo` feeds three consumers: the Prometheus exporter, the TUI, and `snapshot`. Fixing only the exporter would have left the TUI showing `0.0%`. The exposition must still translate, because the in-band absence encoding the reader uses would itself be fabricated data if it reached the wire.
There is a third boundary that neither of those covers, and missing it would have made the fix local-only. `src/network/metrics_parser.rs` builds a `GpuInfo` for remote monitoring with `utilization: 0.0, temperature: 0, power_consumption: 0.0` and only overwrites the fields whose series appear in the scrape. An omitted series therefore became a zero again on the viewing side. `all-smi view --hosts` scraping a degraded macOS node would still have shown "0.0%". The parser now starts every live field absent, so omission survives the round trip. Nodes that do report overwrite as before, so nothing changes for them.
## Why an in-band encoding rather than `Option<f64>`
`Option` is the better type and would let the compiler enforce every call site. It was rejected on blast radius: these four fields have roughly sixty consumers (gauges, sparklines, LED grid, sort comparators, energy accumulation, three CLI shims, the mock server, twelve readers), and converting them in a bug fix running alongside four sibling units on the same base would be a large regression surface for no behavioral gain over the alternative.
The encoding chosen is the one the codebase already had, rather than a new invention: negative for the `f64` fields, `0` for the `u32` fields. Both are outside the valid range of the quantities they carry. `src/ui/renderers/gpu_renderer.rs` and `src/ui/filter_dsl/eval.rs` were already reading `utilization < 0.0` and `power_consumption < 0.0` as N/A, and `temperature == 0` / `frequency == 0` as N/A, before this PR. Those branches simply had no producer. This PR gives them one, names the constant (`GPU_METRIC_UNAVAILABLE`), and puts five accessors in front of it so the magic numbers stop being scattered.
A practical consequence worth stating: the encoding is opt-in per reader, so **no other platform's exposition changes** except where noted below. `Option` would have forced a decision in all twelve readers.
## What the TUI shows
Verified by test (`absent_readings_render_as_na_not_zero`), which renders a degraded row through the real `print_gpu_info` and strips ANSI before asserting:
- `Util: N/A`, `Temp: N/A`, `Freq: N/A`, `Pwr: N/A`.
- The Util and ANE gauges draw empty with an `N/A` label. This is the "zero in a different costume" trap: a 0%-filled bar reads as *idle*, so a gauge cannot be left to render the absent value as 0. The test asserts `0.0%` and `0.0W` appear nowhere in the output.
- The paired test `zero_readings_render_as_zero` asserts the complement, so `N/A` unambiguously means no data on screen too.
Aggregations no longer fold absent values in. `src/metrics/gpu_readings.rs` (new) is the single place that skips them, used by the dashboard, the header, the sparkline panel, the LED grid, the snapshot writer and the cluster aggregator. Two of those mattered beyond cosmetics: the energy integrator would have accumulated negative joules, and the history graphs would have drawn a dip that never happened.
## Behavior changes beyond Apple Silicon
Two, both deliberate, both making the exposition agree with what the TUI already displayed:
- `all_smi_gpu_frequency_mhz` is now omitted for readers reporting a static `0` to mean "no clock probe" (Rebellions, Intel Gaudi, AMD via WMI). A flat 0 MHz line was misinformation.
- `all_smi_gpu_temperature_celsius` is now omitted when no sensor answered.
`all_smi_ane_utilization` is unchanged for non-Apple GPUs: they set a literal `0.0` meaning "not applicable" and keep publishing it, covered by a regression test.
## How the degraded path was exercised
IOReport cannot be removed from this host, so the reader was split at an honest seam instead of mocked. `build_gpu_info(static_info, apple_info, sample: Option<&NativeSample>)` holds the whole row-assembly path; `get_gpu_info` acquires the sample and calls it. Passing `sample: None` drives byte-for-byte the branch a macOS VM takes, with no `cfg(test)` switch and no forced-failure flag. Tests then push that row through the real Prometheus exporter (`degraded_row_renders_no_gpu_value_series`) and the real TUI renderer, so the seam between reader and consumer is covered, not just the reader.
The healthy half was confirmed against real hardware, an M1 Ultra, with `all-smi snapshot --format prometheus`: all six families present, `native_metrics="available"`, and `all_smi_ane_power_watts ... 0` demonstrating that a genuine zero from a live subscription is still published.
## What changed
- `src/device/types.rs`: `GPU_METRIC_UNAVAILABLE` plus five `*_reading()` accessors on `GpuInfo`.
- `src/device/readers/apple_silicon_native.rs`: extracted `build_gpu_info` + `NativeSample`; stopped unwrapping to zero; added the `native_metrics` label; 8 tests.
- `src/device/macos_native/manager.rs`, `src/device/traits.rs`: the written policy.
- `src/api/metrics/gpu.rs`: five families gated on availability; 3 tests.
- `src/network/metrics_parser.rs`: absent-by-default init, `native_metrics` allowlisted; 2 tests.
- `src/metrics/gpu_readings.rs` (new): absence-aware aggregations; 6 tests.
- `src/metrics/aggregator.rs`, `src/metrics/coordinator.rs`: cluster stats over reporting devices only; `Option` averages.
- `src/ui/{renderers/gpu_renderer,alerts,dashboard,led_grid,local_header,gpu_sparkline_panel,filter_dsl/eval}.rs`: N/A rendering and absence-aware aggregation; 2 renderer tests.
- `src/view/data_collection/aggregator.rs`, `src/api/collection_loop.rs`, `src/snapshot/collector.rs`: energy and power sums skip absent readings.
- `.github/workflows/ci.yml`: corrected the "degrade to zeros" comment and added an assertion that the runner publishes no fabricated `all_smi_gpu_utilization`.
`src/ui/renderers/gpu_renderer.rs` is one of the eight files PR #334 deliberately left alone to avoid colliding here. The diff in it is confined to the five value readouts, the two gauges, and the new tests; none of its dimension arithmetic is touched.
## Test plan
- [x] `cargo fmt --check`
- [x] `cargo clippy --lib --tests -j 9 -- -D warnings`
- [x] `cargo clippy --bin all-smi -j 9 -- -D warnings` (the crate compiles its module tree twice; #309/#310/#311 were bitten by a `pub` item live in the lib and dead in the bin)
- [x] `cargo test --lib -j 9 device::readers::apple_silicon_native` (8 passed)
- [x] `cargo test --lib -j 9 api::metrics` (71 passed), `metrics::gpu_readings` (6), `network::metrics_parser` (51), `ui::renderers::gpu_renderer` (37)
- [x] `cargo test --lib -j 9` by module group: `ui::` 543, `network::` 127, `metrics::` 113, `device::` 169, `snapshot::` 47, `api::` 116, `app_state` 16, `parsing::` 19, all passing
- [x] `cargo test --test {device_tests,library_api_test,snapshot_test,thermal_pstate_integration_test,hardware_details_integration_test}` (60 passed)
- [x] Real hardware: `all-smi snapshot --format prometheus` on an M1 Ultra
Not verified: the actual degraded path on a machine without IOReport, which needs a macOS VM. The launchd smoke test in `.github/workflows/ci.yml` runs on exactly such a host and now asserts the absence, so CI on this PR is the real check.
Closes #325 J
Jeongkyu Shin committed
11ccefa8ffe7b39e0b002e974c1f98e80c122cce
Parent: c4c17d8
Committed by GitHub <noreply@github.com>
on 8/6/2026, 1:16:05 PM