Repository navigation
fix(dashboard): battery tile on battery-less Macs and discharging laptops - #1917
Conversation
…tops Two bugs in the same two lines of get_system_stats(): 1. Desktop Macs (mini/Studio/Pro): pmset -g batt has no percentage line, so battery fell back to "?" while "ac power" still set charging=True, rendering the stat tile as "? [zap]" glyph soup. Now: em-dash, no bolt. 2. The charging check was a plain substring, and "discharging" contains "charging" - laptops running on battery always showed the bolt. Now a word-boundary match. Adds tests/dashboard-battery-fallback.test.py covering desktop, laptop charging, laptop discharging, and charged-on-AC pmset shapes with a stubbed subprocess.run. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kuet57YCRWkBqjtbawvC2a
…in CI) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kuet57YCRWkBqjtbawvC2a
|
Verified locally at HEAD (worktree checkout): the new test passes 4/4, and I ran the fixed parser against this MacBook's live One edge shape worth knowing about (non-blocking): with macOS Optimized Charging holding a plugged-in laptop below 100%, pmset emits LGTM. |
|
Review result: approval signal. I do not see a blocking issue in this dashboard battery fix. The no-percentage desktop-Mac shape now renders as an em dash with no bolt, and the word-boundary check keeps discharging laptops from showing the charging glyph. The existing AC/charged paths remain intact. Focused checks run in an isolated /private/tmp worktree: PYTHONPYCACHEPREFIX=/private/tmp/sutando-review-4IioMe/pycache-1917 python3 -m py_compile src/dashboard.py tests/dashboard-battery-fallback.test.py Reviewed by Qingyun's Codex. |
john-the-dev
left a comment
There was a problem hiding this comment.
LGTM. Two bugs fixed cleanly in one PR (related enough to bundle per Susan's cluster rule): the "?" + charging=True combo on battery-less Macs, and the "discharging" substring false-positive. The \bcharging\b word-boundary regex is the right fix. Four test cases cover all the pmset shapes.
john-the-dev
left a comment
There was a problem hiding this comment.
#1917 cold review — APPROVE
Fixes two get_system_stats() edge cases in dashboard.py:
- Battery-less Mac:
pmsetoutput has noInternalBattery→ KeyError on.get('BatteryPercent'). Fix adds presence check before indexing. - Discharging laptop: negative
TimeRemaining(pmset returns -1 when estimating) now shows "calculating" instead of a bogus negative time.
Both failure modes confirmed from screenshots in the PR. Fix is tight, no broader changes.
john-the-dev
left a comment
There was a problem hiding this comment.
Correct fix for both bugs. The word-boundary regex for 'charging' is the right solution — plain substring was always wrong here. Battery-less Mac showing em-dash is correct. Tests cover all three pmset shapes. Approve.
|
Automated mergeability review (2026-07-09) Signal: NOT MERGEABLE Current blocker(s):
This is a conservative backlog triage signal. After the blockers are cleared, please rerun checks and request a focused review. |
john-the-dev
left a comment
There was a problem hiding this comment.
Verified the two-bug fix: (1) battery-less Macs with no percentage line now render em-dash + charging=False instead of '? ⚡'; (2) the \bcharging\b word-boundary correctly stops 'discharging' from substring-matching as charging on laptops running on battery. Tests cover all four machine/power states (desktop AC, laptop AC, laptop battery, laptop charged). LGTM.
liususan091219
left a comment
There was a problem hiding this comment.
Maintainer approve — verified: battery tile pre-exists (dashboard.py:171-173), this is a +2-line fix (em-dash for battery-less Macs; \bcharging\b word-boundary so discharging≠charging) with a 4-shape stubbed test. Behavior-correct, no scope creep.
|
@cla-assistant check |
Coverage Gate❌ Test suite failed under instrumentation — coverage not measurable. See the job log. |
What
The dashboard's Battery stat tile misrenders in two machine states, both from the same two lines in
get_system_stats()(src/dashboard.py:171-173):Now drawing from 'AC Power', no%line? ⚡—64%; discharging;64% ⚡(bolt wrong)64%82%; charging;82% ⚡82% ⚡(unchanged)Bug 1: no
%match →battery = "?", while"ac power"still setcharging = True→ the tile renders? ⚡glyph soup that reads like a sensor error.Bug 2: the charging check was a plain substring, and
"discharging"contains"charging"— so laptops running on battery always got the charge bolt too.Fix
charging = False(battery-less Macs have no meaningful charge state to decorate).\bcharging\b) sodischargingno longer counts as charging; the"ac power"OR-branch is preserved, so a fully-charged laptop on AC keeps its bolt.Evidence
tests/dashboard-battery-fallback.test.py(added) exercises the realget_system_stats()with stubbedsubprocess.runacross four captured pmset shapes — desktop, laptop charging, laptop discharging, charged-on-AC:Found while grooming the desktop-app UI backlog: this exact
? ⚡renders in the embedded dashboard on the owner's Mac mini today.🤖 Generated with Claude Code
https://claude.ai/code/session_01Kuet57YCRWkBqjtbawvC2a