Skip to content

fix(powermetrics): reject unsupported Macs and report 0 W when no samples - #1345

Open
davidberenstein1957 wants to merge 1 commit into
masterfrom
fix/powermetrics-nan-totals
Open

fix(powermetrics): reject unsupported Macs and report 0 W when no samples#1345
davidberenstein1957 wants to merge 1 commit into
masterfrom
fix/powermetrics-nan-totals

Conversation

@davidberenstein1957

Copy link
Copy Markdown
Collaborator

What

ApplePowermetrics.get_details() now guards against an empty match list before averaging: if powermetrics produced no CPU Power: / GPU Power: lines it logs a warning and reports 0.0 W instead of nan. The CPU and GPU branches were identical, so they are now one loop.

Why

np.mean([]) returns nan and only emits a RuntimeWarning, so the surrounding except Exception never fired and nothing was logged. The NaN flowed through AppleSiliconChip._get_power into the tracker accumulators, and since NaN is absorbing under addition, one bad sample made energy_consumed, emissions and emissions_rate NaN for the remainder of the run — written to emissions.csv and POSTed to the API as bare NaN.

This is reachable in the field whenever the powermetrics sudoers rule is missing (empty log), and on machines where the cpu_power sampler emits no GPU Power: lines, where it happened on every single call.

Reporting 0 W understates rather than invalidates, and the warning makes the condition visible.

Verification

Two new cases in tests/test_powermetrics.py (test_get_details_without_samples, test_get_details_without_gpu_samples) both fail on master and pass with this change. uv run pytest tests/test_powermetrics.py -q -> 18 passed. The existing test_get_details is unchanged, confirming the non-empty path still behaves identically.

Note: uv run task lint reports pre-existing violations across the repo and uv run task format reformats ~120 unrelated files, so neither was applied wholesale; black --check is clean on the two touched files.

Closes #1306

🤖 Generated with Claude Code

@codecov

codecov Bot commented Aug 12, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.45%. Comparing base (3ec31a0) to head (c583b1d).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1345      +/-   ##
==========================================
+ Coverage   91.43%   91.45%   +0.01%     
==========================================
  Files          49       49              
  Lines        5057     5055       -2     
==========================================
- Hits         4624     4623       -1     
+ Misses        433      432       -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@davidberenstein1957
davidberenstein1957 marked this pull request as ready for review August 12, 2026 17:37
@davidberenstein1957
davidberenstein1957 requested a review from a team as a code owner August 12, 2026 17:37
@davidberenstein1957
davidberenstein1957 force-pushed the fix/powermetrics-nan-totals branch 2 times, most recently from 5247f57 to f67379e Compare August 19, 2026 14:27
@davidberenstein1957 davidberenstein1957 changed the title fix(powermetrics): report 0 W when powermetrics returns no samples fix(powermetrics): reject unsupported Macs and report 0 W when no samples Aug 19, 2026
…ples

Two failure modes in the powermetrics backend produced silently wrong
totals.

`_setup_cli` returned without setting `_cli` and without raising on an Intel
Mac, so `ApplePowermetrics()` succeeded there and `ResourceTracker` could
register an `AppleSiliconChip` on hardware that has none. It also crashed
with `AttributeError` when `detect_cpu_model()` returned None. Both paths now
raise `SystemError`, the argv uses the validated `self._cli`, and a stray
empty argument is dropped.

Averaging an empty sample list returned NaN (numpy) or raised
(`statistics.fmean`), and the NaN poisoned every downstream total. Report
0 W instead. This also drops the numpy dependency from this module.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@davidberenstein1957
davidberenstein1957 force-pushed the fix/powermetrics-nan-totals branch from f67379e to c583b1d Compare August 20, 2026 06:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Empty powermetrics output yields NaN power, poisoning all downstream totals on Apple Silicon

1 participant