Repository navigation
Conversation
|
@paradix86 Reviewed on a rebase onto current 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 The locale coverage is complete, all eight plus Blocking: the parity authority driftsRebased onto current
Exactly three identities need a typed disposition: then Here Your green CI is not wrong, it is older than the gate. A change can move the declaration graph without One question, not blocking
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.
4c90425 to
0aa0439
Compare
|
@ryanbr Thanks for the thorough review, and for rebasing it yourself to find the drift. Parity authority: I rebased the branch onto current Same-day readings: good catch. I added a paragraph to the |
0aa0439 to
d271fa9
Compare
|
#2632 already rebased into main. |
|
Merged, with your authorship intact on both commits. Thanks @paradix86. It needed a rebase onto current main first. The only conflict was the generated
Both still authored by you, and deliberately not squashed. Verification on the rebased tree: 81 Android unit tests green across Two notes from the review, neither blocking and neither altered before merge:
Superseded by #2632. Genuinely well-tested work, and the emulator write-up with the Italian locale pass made this much quicker to review. |
`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
|
Correction to my earlier comment, @paradix86: I had this backwards. The note I left about 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. What is genuinely unreachable is only the example the comment gave, a morning and an evening weigh-in from a single source, because So instead of removing the warning I rewrote it to give the actual reason, in 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 |






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.
From Health Connect · 26 Sep, locale-aware), orFrom Health Connect · no weight yet.Design notes
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").HealthConnectImporter.import(), after persist, so every entry point is covered. It is best-effort: a profile write cannot fail a good import.CancellationExceptionis rethrown.noop_profileprefs (the same mechanism as the existingnoop_experimentslistener), so a background import updates the open screen.StepperFieldgains anenabledparameter (defaulttrue, usesPalette.disabledOpacity).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
.noopbakwhitelist (documented inBackupSettings.kt). The synced weight still round-trips as the existingprofile.weightKg. No schema change.Verification
ProfileWeightSyncTest(4),HealthConnectWeightSyncTest(5),TodayMetricTilesTest(54, +2 new), plusProfileStoreAgeMigrationTestandBackupSettingsCodecTest. All pass.Tools/doc_comment_lint.py: no new detached doc comments. The i18n gate is clean for the new keys.72,4andDa Health Connect · 26 set.Screenshots below.
Translations reviewed by a native Italian speaker (the author).