Skip to content

feat(android): use profile weight from Health Connect - #2493

Closed
paradix86 wants to merge 2 commits into
ryanbr:mainfrom
paradix86:feat/hc-profile-weight
Closed

paradix86 wants to merge 2 commits into
ryanbr:mainfrom
paradix86:feat/hc-profile-weight

Conversation

@paradix86

Copy link
Copy Markdown

What

An opt-in "Use weight from Health Connect" toggle in Settings → Profile (off by default), mirroring how the WHOOP app took body weight from the phone's health store.

  • On: after every Health Connect import (manual, periodic auto-sync, onboarding), the profile weight takes the newest Health Connect weight reading. Switching the toggle on also syncs once immediately from the already-imported rows.
  • While on, the weight +/− stepper is locked and a caption shows the source and the reading's day (From Health Connect · 26 Sep, locale-aware), or From Health Connect · no weight yet.
  • Off: the last synced weight stays and is editable by hand again.

Design notes

  • One resolver, two readers. The profile sync and the Today Weight tile both pick the reading through ProfileWeightSync.newest. With the toggle on, the tile reads Health Connect only, so the tile and the profile cannot show different weights (AGENTS.md, "two readouts of one fact").
  • The hook sits in HealthConnectImporter.import(), after persist, so every entry point is covered. It is best-effort: a profile write cannot fail a good import. CancellationException is rethrown.
  • Settings listens to the noop_profile prefs (the same mechanism as the existing noop_experiments listener), so a background import updates the open screen.
  • StepperField gains an enabled parameter (default true, uses Palette.disabledOpacity).
  • New strings are in all 9 locales.

Parity / scope

Android only. Apple platforms have no Health Connect. An Apple Health twin would be a separate change. The toggle and its sync day are not added to the .noopbak whitelist (documented in BackupSettings.kt). The synced weight still round-trips as the existing profile.weightKg. No schema change.

Verification

  • Unit tests (JVM): ProfileWeightSyncTest (4), HealthConnectWeightSyncTest (5), TodayMetricTilesTest (54, +2 new), plus ProfileStoreAgeMigrationTest and BackupSettingsCodecTest. All pass.
  • Tools/doc_comment_lint.py: no new detached doc comments. The i18n gate is clean for the new keys.
  • Emulator (Android 16, API 36.1, Full debug build), with weights 73.0 kg (yesterday) and 72.4 kg (today) written to Health Connect by a separate app:
    • Default off, weight is manual.
    • On, before any import: stepper locked, "no weight yet".
    • After Import from Health Connect: profile shows 72.4 kg, dated 26 Sep.
    • Today Weight tile shows 72.4 kg.
    • Off: stepper works again (72.4 → 72.9), caption gone.
    • Italian locale, switched back on: immediate sync, 72,4 and Da Health Connect · 26 set.
  • Not exercised on the emulator: a background auto-sync landing while Settings is open. That is covered by the prefs listener and code review only.
  • No BLE / strap code touched.

Screenshots below.


Translations reviewed by a native Italian speaker (the author).

@paradix86

Copy link
Copy Markdown
Author
06-italian-toggle-on 01-toggle-off-default 02-toggle-on-no-weight-yet 03-synced-72 4kg 04-today-weight-tile 05-toggle-off-manual-again

@ryanbr

ryanbr commented Sep 29, 2026

Copy link
Copy Markdown
Owner

@paradix86 Reviewed on a rebase onto current main. The feature is well built and the verification is unusually thorough for a UI change. One blocking thing, which your CI could not have told you about, and one small question.

What I want to single out first. The "one resolver, two readers" note is the best part of this PR. Routing both the profile sync and the Today tile through ProfileWeightSync.newest, and citing the rule you are honouring, means the tile and the profile cannot disagree by construction. I spent this morning removing a case where two functions computed the same value and were free to drift apart, so seeing it designed out rather than caught later is worth saying.

The locale coverage is complete, all eight plus values/, which matters more than it looks: the release lint treats ExtraTranslation as fatal and neither compile nor the i18n audit catches a missing one. And naming what you did NOT exercise, the background auto-sync landing while Settings is open, is the right way to write a verification section.

Blocking: the parity authority drifts

Rebased onto current main, the repository-acceptance suite fails:

twin-map-authority-drift|unpaired_files cannot enter the compact baseline
authority files: 505 -> 506

ProfileWeightSync.kt lives in com/noop/analytics/, which is inside the governed Kotlin roots, so the new file and its two functions enter the inventory one-sided while Tools/parity_twin_map.json still describes the old tree. Merged as-is this turns main red.

Exactly three identities need a typed disposition:

kotlin | android/app/src/main/java/com/noop/analytics/ProfileWeightSync.kt
kotlin | android/app/src/main/java/com/noop/analytics/ProfileWeightSync.kt::newest/1#1
kotlin | android/app/src/main/java/com/noop/analytics/ProfileWeightSync.kt::captionDate/2#1

then python3 Tools/parity_ledger.py --refresh-derived.

Here platform_specific is the right type and your own words are the rationale: Apple has no Health Connect, so there is no counterpart to pair with and never will be. That is the genuine meaning of the type, unlike several entries in that file that use it for a twin which merely lives outside the governed roots.

Your green CI is not wrong, it is older than the gate. A change can move the declaration graph without parity-governance running, because that workflow is filtered on Tools/parity_* paths. That cost main a red ratchet earlier today, and #2582 has since moved the repository-acceptance suite onto the unfiltered leg. You are the fourth PR this has caught, and in every case the PR's own checks described a base that no longer existed.

One question, not blocking

newest(readings) = readings.maxByOrNull { it.day } resolves by day string. For YYYY-MM-DD the lexicographic order is the chronological one, so that is sound. But two readings on the SAME day are separated only by input order, since maxByOrNull returns the first maximal element.

If Health Connect can return a morning and an evening weigh-in for one day, "newest" is choosing between them by whatever order the query produced, which is not necessarily the later one. For body weight that is probably a fraction of a kilogram and nobody will notice. It is worth a sentence saying the resolution is day-granular, so the next reader does not assume it is timestamp-granular and build something on it that needs to be.

Sort the authority and I will take it.

Adds an opt-in "Use weight from Health Connect" toggle to the Profile card
(off by default). While on, every Health Connect import writes the newest
Health Connect weight into the profile, the weight stepper is locked, and a
caption names the source and the reading's day. Switching it on syncs once
immediately from the already-imported rows.

The profile sync and the Today Weight tile resolve the reading through one
helper (ProfileWeightSync.newest), so with the toggle on they cannot show
different weights.

Android only: Apple platforms have no Health Connect. The toggle and its sync
day stay out of the .noopbak whitelist; the synced weight still round-trips
as profile.weightKg.
ProfileWeightSync.kt sits under the governed com/noop/analytics root, so
the new file and its newest/captionDate functions entered the parity
inventory one-sided. Record all three as platform_specific (Apple
platforms have no Health Connect, so there is no Swift counterpart) and
refresh the derived twin-map authority.

Also document that ProfileWeightSync.newest resolves by day: two
readings on the same day are separated only by input order.
@paradix86
paradix86 force-pushed the feat/hc-profile-weight branch from 4c90425 to 0aa0439 Compare September 29, 2026 15:37
@paradix86

Copy link
Copy Markdown
Author

@ryanbr Thanks for the thorough review, and for rebasing it yourself to find the drift.

Parity authority: I rebased the branch onto current main and added the three identities to Tools/parity_dispositions.json as platform_specific, with the rationale you suggested: Apple has no Health Connect, so there's no counterpart to pair with. Then I ran --refresh-derived. The authority diff only moves files (507 → 508), functions (+2), unpaired_files (+1) and unpaired_functions (+2), while the pair sets are unchanged. parity_ledger.py --base upstream/main reports no new findings, and tests.test_parity_governance_acceptance passes locally (50 tests).

Same-day readings: good catch. I added a paragraph to the newest KDoc saying resolution is day-granular. Two weigh-ins on the same day are separated only by input order, since maxByOrNull keeps the first maximal element. Anything that needs the latest reading within a day has to carry the time and resolve on it. The behaviour is unchanged. As you say, the difference is a fraction of a kilogram for the profile.

@paradix86

Copy link
Copy Markdown
Author

#2632 already rebased into main.

@paradix86 paradix86 closed this Oct 2, 2026
@ryanbr

ryanbr commented Oct 2, 2026

Copy link
Copy Markdown
Owner

Merged, with your authorship intact on both commits. Thanks @paradix86.

It needed a rebase onto current main first. The only conflict was the generated Tools/parity_twin_map.json, which I regenerated from source rather than hand-merging, so it went in via #2632 as a rebase-merge rather than straight from this branch. On main now as:

  • 46414e136 feat(android): use profile weight from Health Connect
  • 1509a72b0 parity: dispose ProfileWeightSync as Android-only; note day granularity

Both still authored by you, and deliberately not squashed.

Verification on the rebased tree: 81 Android unit tests green across ProfileWeightSyncTest, HealthConnectWeightSyncTest, TodayMetricTilesTest, BackupSettingsCodecTest and ProfileStoreAgeMigrationTest, plus all five CI checks including parity-governance. The locale work held up as well: all four keys present in the default and in all eight translations, and no locale carrying a key the default lacks, which is the one class that slips past the i18n gate and only trips release lint.

Two notes from the review, neither blocking and neither altered before merge:

  • ProfileWeightSync.newest warns in its doc comment that two readings on the same day are separated only by input order. That cannot happen through the real path: HealthConnectImporter already takes the day's latest by timestamp (if (r.time.epochSecond >= b.weightTs), the same guard its BodyFatRecord and LeanBodyMassRecord neighbours use), and appleDaily holds one row per (deviceId, day), so newest only ever sees one already-latest reading per day. I will correct the comment in a follow-up so it does not invite anyone to thread timestamps through for a problem that is not there.
  • Turning the toggle on while Health Connect holds no weight yet leaves the stepper locked with nothing to sync. It is disclosed by the caption and the detail string and is reversible, so it stays as you built it.

Superseded by #2632. Genuinely well-tested work, and the emulator write-up with the Italian locale pass made this much quicker to review.

ryanbr added a commit that referenced this pull request Oct 2, 2026
`ProfileWeightSync.newest` warned that two same-day readings resolve only by
input order, citing a morning and an evening weigh-in. That example cannot
occur: `HealthConnectImporter` already keeps the day's latest by timestamp
(`r.time.epochSecond >= b.weightTs`) before the row is stored, so each
deviceId contributes one already-latest reading per day.

The warning itself is correct, but for a reason the comment never gave.
`appleDaily`'s natural key is (deviceId, day), and the Weight tile's default
path passes `apple + healthConnect`: two deviceIds that can each carry a row
for the same day. `maxByOrNull` keeping the first maximal element is what
makes Apple Health win that tie, and that is the behaviour the tile had
before this resolver existed. Reversing the concatenation would silently
change which source the tile names.

Comment only, no behaviour change. Local doc_comment_lint.py and
compileFullDebugKotlin both clean.

Refs #2493
@ryanbr

ryanbr commented Oct 2, 2026

Copy link
Copy Markdown
Owner

Correction to my earlier comment, @paradix86: I had this backwards. The note I left about ProfileWeightSync.newest was wrong.

I said the doc comment's same-day warning could not happen through the real path, and that I would correct the comment on that basis. The warning is in fact right, and the ordering it describes is load-bearing. appleDaily's natural key is (deviceId, day), and the Weight tile's default path (toggle off, which is every existing install) passes apple + healthConnect: two different deviceIds that can each hold a row for the same day. So newest really does see same-day ties, and maxByOrNull keeping the first maximal element is precisely what makes Apple Health win one. That is the behaviour the tile had before your resolver existed, so reversing the concatenation would silently change which source the tile names.

What is genuinely unreachable is only the example the comment gave, a morning and an evening weigh-in from a single source, because HealthConnectImporter already keeps the day's latest by timestamp (r.time.epochSecond >= b.weightTs) before the row is ever stored.

So instead of removing the warning I rewrote it to give the actual reason, in 3ecf1414e on main. Comment only, no behaviour change. Your code was correct as written; only the explanation of it needed work.

One gap that is mine rather than yours: nothing pins the Apple-Health-wins-a-tie ordering in a test, so a later tidy-up of apple + healthConnect could change tile behaviour without anything going red. The rewritten comment warns a reader, but a test would hold it properly.

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.

2 participants