Escalate strap battery alerts: a critical 12% crossing + a bedtime night-guard (Swift + Kotlin) - #864
Conversation
…guard The 15% low alert (#368) and the 24h predictive alert (#713) both fire at most ONCE per discharge cycle and then latch — lowAlerted until SoC recovers to 25%, runtimeAlerted until the estimate recovers to 36h. Neither re-arms on anything but a charge, so a strap that just keeps draining goes silent exactly as the news gets worse. Measured on a WHOOP 5.0 running R22 deep-data + high-rate IMU capture: the device's persisted flags show BOTH alerts had already fired, then nothing across the final descent, and a full night of biometrics was lost. At the run's measured 1.65 %/h (13.1% -> 10.9% over 80 min) that silence ran ~3 h. Two additions, each with its OWN persisted gate so a latched low/runtime alert can never suppress them: - Critical SoC alert at 12% (battery-critical). Derived from the hardware cutoff, not picked round: the strap goes dark near 10% — the last HR sample landed 27 min after the 10.9% reading and nothing below 10.9% was ever banked — so the whole usable band is (10%, 15%]. A conventional 5% critical alert is unreachable on this hardware and 10% is a coin flip. Fires once on the real curve, at 12.4%/11:57, 90 min before death. - Bedtime night-guard (battery-bedtime). Near the LEARNED habitual bedtime (habitualMidsleepSec minus half the median night — the #547 model, reused, not reinvented), fires when the strap won't clear the wait until bed plus the night plus a margin. Re-arms on leaving the window, so it is once per NIGHT rather than once per charge: it speaks even when everything else has latched. Cold-start (<14 nights) returns silent — no fabricated bedtime. Both rest on BatteryEstimator.usableRemainingHours, which models the ~10% cutoff. estimate() divides the FULL SoC by the drain rate, so remainingHours is time-to-0% and overstates usable life by cutoff/rate — ~6 h for this user, who had ~30 min left when it read 6.6 h. Added additively: remainingHours still feeds the Today badge and the Kotlin twin's fixtures, so re-anchoring it there would change a displayed number and break parity in the same commit. Policy is pure and lives in StrandAnalytics (the runtimeAlert precedent), so swift test reaches it: 1201 pass, 29 new, including the real drain curve, the cutoff scored against the observed time of death (~33 min predicted vs 27 min actual), cold-start, charging suppression and once-per-cycle. The cross-policy latch-independence assertion lives in StrandTests where BatteryAlertPolicy is. Kotlin twins (BatteryEstimator.kt, BatteryAlertNotifier.kt) are NOT updated here and remain at parity for the pre-existing surface only. (cherry picked from commit 0f73c79b068017a1eb316d14335d094f8ccc6ef6)
The Swift half of this branch added two alerts that fire below the ones NOOP already had, because the existing ones latch: the 15% SoC alert holds `lowAlerted` until the cell recovers to 25%, and the 24 h predictive alert holds `runtimeAlerted` until the estimate returns to 36 h. On the reference incident both had fired, both were latched, and the final ~3 h down to the strap's ~10% hardware cutoff passed in silence — a night of biometrics lost with the app showing ~6 h of runway that the hardware never spends. Android is an independent reimplementation, so the numbers have to be derived here too or the two platforms diverge on a policy that decides whether a night gets recorded. This adds the same pure functions to BatteryEstimator with the same names, constants and arithmetic — cutoffSocPct / usableRemainingHours, criticalSocPct 12 with a 25% re-arm, and the bedtime night-guard over the learned habitual midsleep (bedtimeLeadHours 3, bedtimeMarginHours 1, bedtimeMinNights 7, the same median rule and the same floored-modulo circular bedtime, so a 03:30 midsleep on an 8 h night is a 23:30 bedtime and a late sleeper's window spans midnight). BatteryAlertNotifier gains onCriticalBattery and onBedtimeRunway beside the existing pair, each over its OWN persisted NoopPrefs gate (noop.batteryCriticalAlerted / noop.batteryBedtimeAlerted) and its own notification id — a latched low or runtime alert must never be able to silence them, and the new notes must be able to stand beside an undismissed one. Charging suppression does not consume either gate, so unplugging inside the window can still fire. Android has no per-notification equivalent of the Swift .timeSensitive level; the shared battery channel is already IMPORTANCE_HIGH, and piercing Do Not Disturb would need user-granted policy access, so neither alert takes it. WhoopConnectionService wires both: the critical crossing sits beside the 15% alert (pure SoC policy, no read), while the night-guard rides the existing SoC-change gate so it never adds work to the live-HR path. The learned midsleep is a whole-history read, so it is cached and refreshed at most hourly, mirroring the AppModel throttle; the nightly durations come from the merged day rows already on the tick. Copy goes through strings.xml with de/es/fr/pt-PT translations, plus zh so the non-focus locale allowance does not grow. Verified: ./gradlew compileFullDebugKotlin builds; testFullDebugUnitTest runs 3089 tests with the 29 new BatteryCriticalAlertTest cases green (one-for-one with the Swift BatteryCriticalAlertTests, same measured WHOOP 5.0 discharge fixture), the only failure being the pre-existing, unrelated SyncChipStateTest.lastSyncedAt_takesPriorityOverHistorySyncExperimental. Tools/i18n_audit.py --ci and Tools/doc_comment_lint.py both pass.
ryanbr
left a comment
ryanbr
left a comment
There was a problem hiding this comment.
Strong PR. Verified the parts that carry the argument rather than taking them:
The usableRemainingHours algebra is exactly right. estimate() gives soc / rate; usable is (soc − cutoff) / rate, which rearranges to remainingHours × (soc − cutoff) / soc with the rate never recovered. Checked numerically across several (soc, rate) pairs — identical to computing it directly. And the #99-clamp reasoning holds: the clamp only ever lowers remainingHours, so anything derived from it can only be pessimistic.
Keeping it additive is the right call and the reason I would merge this as-is. Re-anchoring remainingHours in place would have moved the Today badge and broken documented Swift/Kotlin fixture parity in the same commit. Splitting "what we display" from "will it survive the next N hours" is the distinction that actually matters, and it is made without touching a shipped number.
Parity checked mechanically, not by reading the description — all six constants identical across platforms:
cutoffSocPct 10.0 · criticalSocPct 12 · criticalRearmAbovePct 25
bedtimeLeadHours 3.0 · bedtimeMarginHours 1.0 · bedtimeMinNights 7
One thing I went looking for and did not find. The description says cold start is "< 14 nights" while bedtimeMinNights = 7, which looked like a discrepancy. It is not: SleepStageTotals.habitualMinDays (14) already gates the whole night-guard — no learned midsleep, no bedtime, silent — and the 7 is a tighter local bound on the duration median so it is never taken off a night or two. Both are stated in the code comment. Worth noting only because the two numbers in the prose invite exactly the wrong inference.
The Android .timeSensitive gap is documented rather than silently diverged, which is the right handling. Refusing setBypassDnd() because it needs user-granted policy access, and saying so in the Kotlin, is the correct instinct for a battery alert.
The threshold derivation earns its number. 12 % from the observed (10 %, 15 %] band, with the reasoning for why a conventional 5 % is unreachable on this hardware, beats picking a round figure — and the reference run firing once at 12.4 % / 90 min before death is the useful evidence, not the anecdote.
Separate gates per alert so a latched low/runtime alert cannot suppress them is the fix for the actual reported failure, and re-arming the night-guard on window exit rather than on charge is what makes it once-per-night.
10/10 green. Policy is pure and in StrandAnalytics, so CI genuinely reaches it. Approving.
|
Checking merge safety I found one real gap — worth fixing, not a reason to hold the PR. The four new Apple notification strings are not in the String Catalog, while the Android twin is translated into all six locale files: So the escalation reads in German on Android and in English on iOS. Given the whole point is reaching someone in the hour before bed, the copy landing untranslated on one platform is the part most worth getting right. CI cannot see this, and that is the interesting bit. Two of the six literals in that file were already missing before this PR ( Everything else checks out, including the things I went looking to break:
Happy for this to merge and the strings to follow immediately — they are additive and touch nothing else. Want me to put up that follow-up? |
|
Follow-up is up: #866 adds the six missing catalog entries (the four from here plus the two pre-existing) in all eight Apple languages. Two notes on it: es and pt-PT are informal there rather than formal, to match the ~700 surrounding strings in each; and the reverse gap — Android hardcoding six battery strings as Kotlin literals instead of |
…c in the es bedtime body
Two things a read-through of the finished strings turned up.
The low-battery title put the adjective after the strap noun — "Batterie du bracelet faible",
"Batteria della fascia scarica", "Bateria da correia fraca" — where it attaches to the strap rather
than the battery. In fr and it it is worse than ambiguous: "faible" and "scarica" are also verb
forms, so the phrase can be read as a sentence about the strap discharging. Moved next to the noun
it qualifies, which also matches how each language already renders the existing "Low battery" key
(Batterie faible, Batteria scarica, Bateria fraca). es was already corrected this way.
The es bedtime body ended "Cárgala antes de acostarte" — a feminine clitic with no antecedent in
the sentence. Every other Spanish string in this feature treats the device as masculine ("tu
WHOOP", "Recárgalo", "Carga tu WHOOP ahora"); the only feminine noun, "la pulsera", appears in the
critical body and not here. Names the object instead: "Carga tu WHOOP antes de acostarte."
Inherited from the Android wording in #864, which has the same dangling pronoun.
Wording only. Argument types, positions and literal-percent counts re-checked against every key;
i18n_audit.py --ci origin/main green. Diff against main remains 6 keys added, 0 removed, 0
pre-existing entries modified.
…e catalog (#866) Follow-up to #864. Resource-only, no code change. #864 added four String(localized:) literals in Strand/System/BatteryNotifier.swift and translated its Android twin into all five shipped locale files, but the Apple catalog was never updated — so both escalations shipped English on iOS/macOS in every language. Two further literals in the same file (the #713 runtime alert) were already missing before that PR. Six entries added, all eight shipped Apple languages: Strap battery low %@ left on your WHOOP — recharge tonight. Charge your WHOOP now %lld%% left. The strap stops recording near 10%% — it won't capture tonight unless you charge it. Won't last the night %@ of recording left, but tonight needs about %@. Charge before bed. WHY CI WAS GREEN ON #864. The audit checks that keys already IN a catalog carry every language. A String(localized:) literal that was never extracted is not a missing translation, it is a missing key, and is invisible to it. That is #844 exactly; this is the manual patch, not the fix. KEYS. Derived mechanically from the source literals rather than retyped: String interpolation to %@, Int to %lld, literal % to %% inside an interpolated string (a non-interpolated one keeps a bare %, which is why the existing "Your WHOOP is at 100%." is stored unescaped). That mapping is not assumed — it is the observed encoding of XiaomiBandView.swift:268, whose literal produced the existing key "%@ in bed · %lld%% efficiency", exercising all three conversions in one string. Keys stay non-positional and translations use %1$/%2$ where word order moves, matching how the catalog already stores its two-argument entries. REGISTER. de, fr and zh-Hans reuse the Android wording. es and pt-PT do not: #864 translated those formally, while this catalog is informal by a wide margin (es tu/tus/tú 762 against su/sus/usted 66; pt-PT teu/tua 689 against seu/sua/você 20) and the battery strings it already ships next to these are informal. Each language also uses the word for "strap" the catalog already established for it (Strap, pulsera, bracelet, fascia, correia, браслет, 手环, 手環) rather than importing Android's. Register and vocabulary only; no numbers move and no Android file is touched. GRAMMATICAL AGREEMENT. The substituted %@ is BatteryEstimator.label output — "~14h" or "~1h" — so a body opening with a verb must commit to singular or plural against a value it cannot see, and because the argument is an opaque string, xcstrings plural variations cannot key on it either. The three languages that inflect here are recast as label-then-value ("Autonomía restante en tu WHOOP: %@"). de, fr, ru and both Chinese variants were already agreement-free. Same treatment in the critical body, where %lld can be anything from 1 to criticalSocPct = 12, so both "Queda 1%" and "Quedan 12%" occur: it states the level instead. Two further wording fixes from a final read-through: the fr/it/pt-PT low-battery title placed the adjective after the strap noun, where it attaches to the strap and — since faible and scarica are also verb forms — can be read as a sentence about the strap discharging; and the es bedtime body ended in a feminine clitic with no antecedent, where every other Spanish string in this feature treats the device as masculine. Both now match how each language already renders the existing "Low battery" key. it, ru and zh-Hant are new translations and have NOT been checked by a native speaker. VERIFICATION. Resource-only, so nothing to compile, and both shipping Apple targets pick it up: Strand and NOOPiOS each take "- path: Strand" wholesale, excluding only Info.plist, entitlements and Assets.xcassets, so the catalog and BatteryNotifier.swift land in both. All 10 String(localized:) literals in that file now resolve to catalog keys. Argument types, positions and literal-percent counts were checked against every key across 6 keys x 8 languages after each rewrite, so no format string can garble at substitution. Measured against origin/main: 6 keys added, 0 removed, 0 pre-existing entries modified, and the catalog moves 3259 to 3265 keys with every per-language missing count unchanged. i18n_audit.py --ci origin/main green; Tools/i18n_extra_locale_baseline.txt untouched and unaffected. NOT IN SCOPE. Android hardcodes six battery strings as raw Kotlin literals rather than R.string (BatteryAlertNotifier.kt:123,124,151,152,163,164), so the runtime, low and full alerts ship English there in every locale — the same bug in the opposite direction. Filed as #867.
2 participants
Both battery alerts NOOP has today fire at most once per discharge cycle and then latch: the 15% SoC alert holds
lowAlerteduntil the cell recovers to 25%, and the 24 h predictive alert (#713) holdsruntimeAlerteduntil the estimate returns to 36 h. Neither re-arms on anything but a charge — so a strap that just keeps draining goes silent exactly as the news gets worse.Measured on a WHOOP 5.0 running R22 deep-data + high-rate IMU capture: the device's persisted flags show both alerts had already fired, then nothing across the final descent, and a full night of biometrics was lost. At the run's measured 1.65 %/h (13.1% → 10.9% over 80 min) that silence ran ~3 h, with the app still showing ~6 h of runway.
Two additions, each with its own persisted gate so a latched low/runtime alert can never suppress them.
1. Critical SoC alert at 12% (
battery-critical)Derived from the hardware cutoff, not picked round. The strap goes dark near 10%: on the reference run the last HR sample landed 27 min after the 10.9% reading and nothing below 10.9% was ever banked, so the whole usable alert band is (10%, 15%]. A conventional 5% critical threshold is unreachable on this hardware and 10% is a coin flip. 12% sits mid-band, was reliably reported (12.4 / 12.1 / 11.9 / 11.6 / 11.4 / 11.1), and still clears the 15% line by 3 pp so it is a genuinely separate crossing rather than jitter around the same threshold.
Lead time scales with how hard the user drives the strap, which is the point: at 1.65 %/h that is ~1.2 h of warning; on a stock 5.0 sipping ~0.35 %/h it is ~5.7 h. On the real curve it fires once, at 12.4% / 11:57 — 90 min before death.
2. Bedtime night-guard (
battery-bedtime)Near the learned habitual bedtime (
habitualMidsleepSecminus half the median night — the #547 model, reused rather than reinvented), fires when the strap won't clear "wait until bed + the night + a margin".It re-arms on leaving the window, so it is once per night rather than once per charge: it speaks even when everything else has latched. Bedtime comes from the learned model rather than a fixed clock band, which would fire at the wrong hour for exactly the shift/late sleepers that learner exists to serve. Cold start (< 14 nights, or too little duration history) returns silent — no fabricated bedtime.
The estimator gap underneath both:
usableRemainingHoursestimate()divides the full SoC by the drain rate, soremainingHoursis time-to-0% and overstates usable life bycutoff / rate— ~6 h for this user, who had ~30 min left when the app read 6.6 h.usableRemainingHoursre-anchors that to the ~10% cutoff, algebraically (remainingHours × (soc − cutoff) / soc) so the rate is never recovered explicitly, which also carries the #99 clamp through conservatively: that clamp only ever lowersremainingHours, so the usable figure derived from it can only be pessimistic, never optimistic.Added additively, on purpose.
remainingHoursstill feeds the Today badge and the Kotlin twin's shared fixtures, so re-anchoring it there would change a displayed number and break documented Swift/Kotlin parity in the same commit. Anything asking "will the strap survive the next N hours" must callusableRemainingHours; anything displaying "~X left" keeps what it had.Relationship to #419 (and vishk23#1)
No dependency and no duplication. #419 adds a
NotificationAuthorizerbecause wrist alerts never requested OS permission. This PR touches nothing on that path:BatteryNotifieralready callsrequestAuthorization()when the toggle is enabled and already status-checks inpost(), and both new alerts go through that same existingpost().NotificationAuthorizerdoes not appear anywhere in this diff, and the branch applies tomainwith #419 unlanded (as it is now) or landed.The two do interact in one benign way worth stating: if #419 lands first, these alerts inherit its authorization improvements for free, because they share
post(). Merge order does not matter.The only overlap in
post()is a new defaulted parameter —interruptionLevel: UNNotificationInterruptionLevel = .active— so every existing call site is unchanged. The two escalations pass.timeSensitiveso they can break through a sleep Focus; the whole point is reaching the user in the hour before bed. Without the time-sensitive entitlement the OS silently treats this as.active, so requesting it is safe either way.Where the code lives
Policy is pure and lives in
StrandAnalytics(following theruntimeAlertprecedent from #713), soswift testreaches it with no app, no strap, no CoreBluetooth.BatteryNotifierholds only the gates and the copy;AppModelholds the two call sites plus an hourly-throttled cache of the learned midsleep (a whole-history read, so it is not done per battery packet).Cross-platform parity
Both platforms, in this PR — same names, same constants, same arithmetic:
com.noop.analytics.BatteryEstimatorgainscutoffSocPct,usableRemainingHours,criticalSocPct/criticalRearmAbovePct/criticalAlert,bedtimeLeadHours/bedtimeMarginHours/bedtimeMinNights,NightRunway,typicalSleepHours,bedtimeAlert, with the same median rule and the same floored-modulo circular bedtime (a 03:30 midsleep on an 8 h night is a 23:30 bedtime, and a late sleeper's window spans midnight).com.noop.notif.BatteryAlertNotifiergainsonCriticalBattery/onBedtimeRunway, each over its own persisted gate and its own notification id — a latched low/runtime alert must not be able to silence them, and the new notes must be able to stand beside an undismissed one. Charging suppression does not consume either gate, so unplugging inside the window can still fire.WhoopConnectionServicewires both: the critical crossing beside the 15% alert (pure SoC, no read), the night-guard on the existing SoC-change gate so it never adds work to the live-HR path.Android has no per-notification equivalent of
.timeSensitive. The shared battery channel is alreadyIMPORTANCE_HIGHso both escalations heads-up; genuinely piercing Do Not Disturb would require channelsetBypassDnd()plus user-granted notification-policy access, which a battery alert should not take for itself. That is a comment in the Kotlin, not a silent divergence.Verification
The single Android failure is the pre-existing, unrelated
com.noop.ui.SyncChipStateTest.lastSyncedAt_takesPriorityOverHistorySyncExperimental(IllegalStateExceptionatSyncChipStateTest.kt:59), which fails identically on cleanmain. Every battery test class is green:BatteryCriticalAlertTest29/0,BatteryRuntimeAlertTest7/0,BatteryEstimatorTest13/0,BatteryEstimatorTraceTest4/0,BatteryAlertPolicyTest8/0.The app-target half is compiled locally, since
app-build.ymlis disabled by default andswift-packages.ymldoes not compile app targets.What the tests actually pin, not just that they pass: the real measured drain curve; the cutoff scored against the observed time of death (~33 min predicted vs 27 min actual); cold start; charging suppression; once-per-cycle; and — in
StrandTests, whereBatteryAlertPolicylives — that a latched low/runtime gate cannot suppress either new alert. The Kotlin suite is one-for-one with the Swift suite over the same reconstructed discharge fixture.No hardware test claimed. Nothing here touches the BLE path: both alerts are pure functions over a SoC reading the app already receives. The evidence is the recorded discharge curve from the incident, replayed in tests, not a fresh strap session.
Localization
Six new user-facing strings on Android through
strings.xmlwith de/es/fr/pt-PT (the zero-tolerance focus locales) plus zh, without which the non-focus ratcheting allowance would have grown. The Apple side follows the existingBatteryNotifierconvention from #250 —String(localized:)at the call site, catalog extraction left to Xcode — andTools/i18n_audit.py --cipasses.