Skip to content

Fix carried Charge source label - #412

Merged
ryanbr merged 1 commit into
ryanbr:mainfrom
kavemang:agent/fix-android-charge-source
Jul 15, 2026
Merged

ryanbr merged 1 commit into
ryanbr:mainfrom
kavemang:agent/fix-android-charge-source

Conversation

@kavemang

Copy link
Copy Markdown

Summary

  • Resolve the Today hero source label for carried Charge from the prior scored recovery day.
  • Reuse the existing per-metric provenance label path so *-noop recovery sources display as On-device.
  • Add focused Today explainability tests for carried Charge source labels.

Why

Fixes #390.

Today can show a carried Charge while the current day has no recovery score yet. In that state, the current-day recovery provenance is also absent, so the hero source label could omit or misrepresent the source behind the visible Charge value.

This PR looks up the carried night's resolved recovery source and uses it only for that carried Charge case. Current-day recovery provenance still wins when it exists, so imported WHOOP data continues to display as Whoop; a NOOP-computed carried Charge displays as On-device.

Test

  • cd android && ./gradlew :app:testFullDebugUnitTest --tests 'com.noop.ui.TodayExplainabilityTest'

@kavemang
kavemang marked this pull request as ready for review July 14, 2026 02:58
@ryanbr
ryanbr merged commit 39d9511 into ryanbr:main Jul 15, 2026
@ryanbr

ryanbr commented Jul 15, 2026

Copy link
Copy Markdown
Owner

Parity note (no iOS twin needed): checked whether the same mislabeled-carried-Charge symptom exists on iOS/macOS. It doesn't, on either Today surface:

  • LiquidTodayView — the Charge hero is score: displayDay?.recovery and resolveDisplayDay() returns repo.today with no carry, so Liquid never shows a carried Charge. The combined hero pill has no carried source to mislabel.
  • Classic TodayView — it does carry the Charge (dimmed ring + "Last night · date" caption, Device disconnected whoop 5.0 MG #802), but its source badge is gated by ringHasValue("recovery") = displayDay?.recovery != nil, which is false in the carried state. So a carried Charge shows the caption and no source badge — it never prints a wrong source.

So the #390 defect (carried Charge labeled with the wrong source) has no iOS equivalent. The only residual difference after this PR is cosmetic — Android now shows the correct source badge on a carried Charge, whereas classic iOS shows none (caption only). That's a missing nicety, not a bug, so no parity fix is required for #390. Surfacing the carried source badge on classic TodayView remains available as an optional future enhancement.

ryanbr added a commit that referenced this pull request Jul 15, 2026
Rolling testing build off main: #452 i18n gate, #449 calibration count,
#412 carried-Charge source label, #451 Trends l10n, #476 IMU decode-log.
Build numbers bumped (iOS 185→186, Android 274→275); no app-version change.
ryanbr added a commit that referenced this pull request Jul 15, 2026
Rolling testing build off main: #452 i18n gate, #449 calibration count,
#412 carried-Charge source label, #451 Trends l10n, #476 IMU decode-log.
Staging 8.8.0; build numbers 195/284 -> 196/285.
tanarchytan added a commit to tanarchytan/noop that referenced this pull request Jul 15, 2026
ryanbr#409 turned the per-ring 'beneath' source labels into one card-level badge anchored to the Rest
vessel and offset up to straddle the card's top border, so it floated over the liquid vessels (the
later nudge into the top-right corner still overlapped the top edge).

Put it back where it sat before ryanbr#409: BENEATH the circles, in normal flow. The hero column now owns
the vertical padding and lays out [rings row] then the single consolidated SourceBadge as a centred
child, so the label hugs the ring cluster and never overlaps a vessel (label and no-label cases).
Also centre the SourceBadge text within the pill (Box + contentAlignment/textAlign Center) — the
heightIn(min) height floor could otherwise let the single line ride the pill's top edge.

Data path unchanged (real per-metric merge winners incl. a carried Charge's source, ryanbr#412). Dropped
the now-unused overlay measurement (wrapContentWidth/offset/onSizeChanged) and the fixed-width Rest
wrapper. Verified on device (com.noop.whoop 9.0.0).
@kavemang
kavemang deleted the agent/fix-android-charge-source branch July 16, 2026 07:16
DX23876 pushed a commit to DX23876/noop that referenced this pull request Jul 22, 2026
Co-authored-by: kavemang <kavemang@users.noreply.github.com>
simoncad7 pushed a commit to simoncad7/noop that referenced this pull request Aug 17, 2026
Co-authored-by: kavemang <kavemang@users.noreply.github.com>
J-Gur added a commit to J-Gur/noopjg that referenced this pull request Sep 28, 2026
…DISTANCE)

Selecting the Activity import category (needed to read Steps) batches
DistanceRecord's read permission into the same runtime request. That
permission was never declared in the manifest, and Health Connect
silently drops the whole batched request when one permission in it is
undeclared — the same trap already hit once for WRITE_EXERCISE/
WRITE_DISTANCE (ryanbr#412) and again for READ_HYDRATION. Tapping "Import
from Health Connect" showed no permission dialog and no Toast at all.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
J-Gur added a commit to J-Gur/noopjg that referenced this pull request Sep 28, 2026
hcWritePermissionLauncher batches VO2MAX_PERMISSIONS into the same
runtime request as the vitals + exercise write permissions
(startWriteback()), but WRITE_VO2_MAX was never declared in the
manifest. Health Connect silently drops the whole batched request when
one permission in it is undeclared — the same trap already hit for
WRITE_EXERCISE/WRITE_DISTANCE (ryanbr#412) and READ_HYDRATION, and again for
READ_DISTANCE — so writeback kept failing PERMISSION_DENIED regardless
of what was granted elsewhere.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
DX23876 added a commit to DX23876/noop that referenced this pull request Nov 1, 2026
Health Connect silently drops a whole batched permission request when
one permission in it is not declared in the manifest (the trap already
hit for WRITE_EXERCISE/WRITE_DISTANCE in ryanbr#412 and for READ_HYDRATION).

- The Activity import category bundles DistanceRecord with Steps
  (HealthConnectImporter.ImportCategory.ACTIVITY), but READ_DISTANCE was
  undeclared, so "Import from Health Connect" for Activity showed no
  dialog and imported nothing.
- A fresh install's first writeback request batches VO2MAX_PERMISSIONS
  with the vitals and exercise writes, but WRITE_VO2_MAX was undeclared,
  so the whole writeback request was dropped.

Not verified on an Android device.

Adopted from J-Gur/noopjg (79a5256, 8ff2a7d) by @J-Gur.

Analysis migration required: no

Co-authored-by: J-Gur <162961589+J-Gur@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Whoop mention for charge whereas it should be on-device

2 participants