fix(fitness): stop the partial Health Connect pass from downgrading what is already on screen

Audit of the progressive-read commit. All three findings share one root cause:
a DetectedWorkout with no metrics is now visible to the UI, which was
impossible before — previously nothing left HealthConnectManager until every
per-session aggregate had come back.

1. Sharing during the pending window published a lossy event.
   `toNewWorkoutRoute` collapses the null metrics to `0` (the composer route
   carries primitives), and `NewWorkoutViewModel.applyPrefill` reads `0` as
   absent and clears the field. So a tap on Share before stage 2 landed opened
   the composer with duration only, and publishing it marked the workout as
   shared — the real numbers never got their turn. The Share button now waits
   for the metrics. Everything still offering one in that window is a Health
   Connect workout, since a published one is already published, so the gate is
   blanket rather than per-workout.

2. Every resume stripped an already-complete dashboard.
   `refresh` runs on `LifecycleResumeEffect`, and the collect body overwrote
   the contribution with stage 1's skeletons — removing the distance, calories,
   heart-rate, steps and elevation cells and most of Best Efforts for as long
   as stage 2 took. The comment claiming a resume "re-reads in place" was only
   true for the few ms before stage 1 arrived. A partial stage now reaches the
   screen only when there is nothing better on it already; otherwise the
   previous numbers stay up, unannotated, and stage 2 swaps them atomically.
   The rule is a pure fold on the new top-level `HealthConnectContribution`,
   so it is testable without a device.

3. Stage 1 evicted published workouts that still had their metrics.
   `TrainingLog.merge` prefers the Health Connect copy because it "carries the
   metrics the published event may have dropped" — a rationale that inverts
   when that copy has not read its metrics yet. A user with published workouts
   watched their distance and calorie totals fall to zero and climb back. A
   copy carrying no metric at all now yields to a matching one that does. This
   also fixes the non-progressive case where aggregation legitimately returns
   nothing and the published event holds the only numbers.

The dedupe rewrite preserves the existing shape exactly: the rule changes
which copy survives a tie, never how many. One session matching several
published events still collapses to one entry.

Tests: 5 more in PartialMetricsReportTest (published totals held through the
partial pass, the loaded copy still winning, both-metric-less and
several-matches edges), and HealthConnectContributionTest covering the fold —
first load shows stage 1, a resume does not, an empty completed read still
clears the gate.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VgfgpDWnu635p2qc2K2yMX
This commit is contained in:
Claude
2026-09-19 01:03:41 +00:00
parent 7a528d7127
commit 1e4c076cf0
6 changed files with 346 additions and 42 deletions
@@ -309,7 +309,7 @@ private fun Dashboard(
WindowTotalsCard(report, miles)
ActivityBreakdown(report, miles)
BestEfforts(report, miles)
RecentWorkouts(report, miles, onShare)
RecentWorkouts(report, miles, metricsPending, onShare)
Text(
text = stringRes(Res.string.my_fitness_window_note),
@@ -528,6 +528,7 @@ private fun bestValue(
private fun RecentWorkouts(
report: WorkoutStats.Report,
miles: Boolean,
metricsPending: Boolean,
onShare: (DetectedWorkout, String) -> Unit,
) {
SectionCard(stringRes(Res.string.my_fitness_recent)) {
@@ -555,7 +556,16 @@ private fun RecentWorkouts(
// workout that is already posted would publish a second kind 1301 for
// the same effort — whether it came back from a relay, or is the
// Health Connect copy of one the user shared earlier.
if (!workout.alreadyPublished) {
//
// Nor while the metrics are still loading. The composer route carries
// them as primitives where 0 means absent, so sharing a workout whose
// aggregations have not come back yet posts a kind 1301 with its
// duration and nothing else — and then the workout counts as shared, so
// the full numbers never get their turn. Everything still offering a
// Share button in that window is a Health Connect workout, since a
// published one is by definition already published, so the wait is
// blanket rather than per-workout.
if (!workout.alreadyPublished && !metricsPending) {
TextButton(onClick = { onShare(workout, label) }) {
Text(stringRes(Res.string.my_fitness_share), style = MaterialTheme.typography.labelMedium)
}
@@ -98,26 +98,11 @@ class MyFitnessViewModel : ViewModel() {
private val pubkeyHex = MutableStateFlow<String?>(null)
/**
* Health Connect's contribution, and how far along reading it is.
*
* [sessionsPending] and [metricsPending] are the two stages of
* [HealthConnectManager.readWorkoutsProgressively]: the session list costs one IPC, the
* metrics cost one per session. They are tracked separately because they mean different
* things to the screen — an empty dashboard must not say "nothing logged yet" while the
* sessions are still coming, but it can show real counts and times while the metrics are.
*/
private data class Contribution(
val workouts: List<DetectedWorkout> = emptyList(),
val sessionsPending: Boolean = false,
val metricsPending: Boolean = false,
)
/**
* Health Connect's contribution. A push source: the platform has no change feed we can
* observe, so [refresh] re-reads it when the screen resumes or a permission is granted.
*/
private val fromHealthConnect = MutableStateFlow(Contribution())
private val fromHealthConnect = MutableStateFlow(HealthConnectContribution())
/** The in-flight [refresh], cancelled by the next one so two reads never interleave. */
private var refreshJob: Job? = null
@@ -187,7 +172,7 @@ class MyFitnessViewModel : ViewModel() {
// Read through the local rather than the field: nothing may set sessionsPending
// without a reader that will clear it again, or an empty dashboard waits forever.
if (status != HealthConnectStatus.CONNECTED || hc == null) {
fromHealthConnect.value = Contribution()
fromHealthConnect.value = HealthConnectContribution()
healthConnectStatus.value = status
return@launch
}
@@ -202,14 +187,7 @@ class MyFitnessViewModel : ViewModel() {
val now = Instant.now()
hc
.readWorkoutsProgressively(now.minus(Duration.ofDays(WorkoutStats.WINDOW_DAYS)), now)
.collect { read ->
fromHealthConnect.value =
Contribution(
workouts = read.workouts,
sessionsPending = false,
metricsPending = read.metricsPending,
)
}
.collect { read -> fromHealthConnect.value = fromHealthConnect.value.after(read) }
}
}
@@ -227,3 +205,41 @@ class MyFitnessViewModel : ViewModel() {
if (hc.hasAllPermissions()) HealthConnectStatus.CONNECTED else HealthConnectStatus.AVAILABLE
}
}
/**
* Health Connect's contribution to the dashboard, and how far along reading it is.
*
* [sessionsPending] and [metricsPending] are the two stages of
* [HealthConnectManager.readWorkoutsProgressively]: the session list costs one IPC, the metrics
* cost one per session. They are tracked separately because they mean different things to the
* screen — an empty dashboard must not say "nothing logged yet" while the sessions are still
* coming, but it can show real counts and times while the metrics are.
*/
data class HealthConnectContribution(
val workouts: List<DetectedWorkout> = emptyList(),
val sessionsPending: Boolean = false,
val metricsPending: Boolean = false,
) {
/**
* Folds one stage of a read in — and declines the ones that would take information off a
* screen that already has it.
*
* A stage-1 result carries no metrics. On a first load that is exactly what makes the
* dashboard appear in one IPC instead of dozens. On a re-read of an already-populated
* dashboard — [MyFitnessViewModel.refresh] runs on every resume — publishing it would strip
* the distance, calories, heart-rate, steps and elevation cells and most of Best Efforts for
* as long as stage 2 takes, then put them back. So a partial stage only reaches the screen
* when there is nothing better on it already; otherwise the previous read's numbers stay up,
* correct and unannotated, and stage 2 swaps them atomically.
*/
fun after(read: HealthConnectManager.WorkoutRead): HealthConnectContribution =
if (read.metricsPending && workouts.isNotEmpty()) {
copy(sessionsPending = false)
} else {
HealthConnectContribution(
workouts = read.workouts,
sessionsPending = false,
metricsPending = read.metricsPending,
)
}
}
@@ -0,0 +1,144 @@
/*
* Copyright (c) 2025 Vitor Pamplona
*
* Permission is hereby granted, free of charge, to any person obtaining a copy of
* this software and associated documentation files (the "Software"), to deal in
* the Software without restriction, including without limitation the rights to use,
* copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the
* Software, and to permit persons to whom the Software is furnished to do so,
* subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in all
* copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS
* FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
* COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN
* AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
* WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
*/
package com.vitorpamplona.amethyst.ui.screen.loggedIn.workouts.fitness
import com.vitorpamplona.amethyst.commons.fitness.DetectedWorkout
import com.vitorpamplona.amethyst.service.workouts.health.HealthConnectManager
import com.vitorpamplona.quartz.experimental.fitness.workout.tags.ExerciseType
import org.junit.Assert.assertEquals
import org.junit.Assert.assertFalse
import org.junit.Assert.assertSame
import org.junit.Assert.assertTrue
import org.junit.Test
/**
* The rule that decides which stages of a progressive Health Connect read are allowed to reach
* the My Fitness dashboard.
*
* The screen re-reads on every resume, and a read's first stage carries no metrics. Applying it
* unconditionally would be a downgrade for anyone coming back to a dashboard that was already
* complete.
*/
class HealthConnectContributionTest {
private fun workout(
id: String,
distanceMeters: Double? = null,
) = DetectedWorkout(
id = id,
exercise = ExerciseType.RUNNING,
title = null,
startTimeEpochSeconds = 1_700_000_000,
durationSeconds = 1800,
distanceMeters = distanceMeters,
calories = null,
avgHeartRate = null,
maxHeartRate = null,
steps = null,
elevationGainMeters = null,
source = "Samsung Health",
)
private fun stageOne(vararg workouts: DetectedWorkout) = HealthConnectManager.WorkoutRead(workouts.toList(), metricsPending = true)
private fun stageTwo(vararg workouts: DetectedWorkout) = HealthConnectManager.WorkoutRead(workouts.toList(), metricsPending = false)
@Test
fun `the first load shows stage one, which is the whole point of reading in stages`() {
val skeleton = workout("a")
val after = HealthConnectContribution().after(stageOne(skeleton))
assertEquals(listOf(skeleton), after.workouts)
assertTrue(after.metricsPending)
assertFalse(after.sessionsPending)
}
@Test
fun `stage two replaces stage one and clears the pending flag`() {
val loaded = workout("a", distanceMeters = 10_000.0)
val after = HealthConnectContribution().after(stageOne(workout("a"))).after(stageTwo(loaded))
assertEquals(listOf(loaded), after.workouts)
assertFalse(after.metricsPending)
}
@Test
fun `a resume does not strip the metrics off an already-complete dashboard`() {
val loaded = workout("a", distanceMeters = 10_000.0)
val complete = HealthConnectContribution(workouts = listOf(loaded))
// What refresh() does on resume, then the new read's stage 1 arriving.
val rereading = complete.copy(sessionsPending = true)
val after = rereading.after(stageOne(workout("a")))
assertEquals(listOf(loaded), after.workouts)
// Nothing on screen has changed, so nothing should be annotated as loading either.
assertFalse(after.metricsPending)
assertFalse(after.sessionsPending)
}
@Test
fun `the resumed read still lands, atomically, when its metrics arrive`() {
val old = workout("a", distanceMeters = 10_000.0)
val fresh = workout("b", distanceMeters = 12_000.0)
val after =
HealthConnectContribution(workouts = listOf(old))
.copy(sessionsPending = true)
.after(stageOne(workout("a"), workout("b")))
.after(stageTwo(old, fresh))
assertEquals(listOf(old, fresh), after.workouts)
assertFalse(after.metricsPending)
}
@Test
fun `an empty stage two is applied even when workouts are on screen, so revoked data clears`() {
val complete = HealthConnectContribution(workouts = listOf(workout("a", distanceMeters = 10_000.0)))
// Not a downgrade to decline — a completed read that found nothing is the truth.
val after = complete.after(stageTwo())
assertTrue(after.workouts.isEmpty())
assertFalse(after.metricsPending)
}
@Test
fun `a read that finds nothing clears the sessions-pending gate`() {
// Health Connect unavailable, the read failing, and a device with no mappable sessions
// all emit one empty non-pending result. Any of them leaving sessionsPending set would
// strand an empty dashboard on its spinner forever.
val after = HealthConnectContribution(sessionsPending = true).after(stageTwo())
assertTrue(after.workouts.isEmpty())
assertFalse(after.sessionsPending)
assertFalse(after.metricsPending)
}
@Test
fun `declining a stage keeps the same workout list instance rather than rebuilding it`() {
val workouts = listOf(workout("a", distanceMeters = 10_000.0))
val complete = HealthConnectContribution(workouts = workouts, sessionsPending = true)
assertSame(workouts, complete.after(stageOne(workout("a"))).workouts)
}
}
@@ -80,4 +80,22 @@ data class DetectedWorkout(
* sessions (e.g. a long run split around breaks) into a single suggestion.
*/
val sessionCount: Int = 1,
)
) {
/**
* Whether this carries any metric beyond its duration.
*
* False means one of two things, and the difference matters to whoever is holding two copies
* of the same effort: the activity genuinely has none to record (a gym session has no
* distance and often no calories), or they are simply not known yet — a Health Connect
* session read before its per-session aggregations have come back. Either way a copy that
* has some is the better one to keep, which is what [TrainingLog.merge] uses this for.
*/
val hasAnyMetric: Boolean
get() =
distanceMeters != null ||
calories != null ||
avgHeartRate != null ||
maxHeartRate != null ||
steps != null ||
elevationGainMeters != null
}
@@ -21,6 +21,8 @@
package com.vitorpamplona.amethyst.commons.fitness
import com.vitorpamplona.quartz.experimental.fitness.workout.WorkoutRecordEvent
import java.util.Collections
import java.util.IdentityHashMap
import kotlin.math.abs
/**
@@ -47,9 +49,16 @@ object TrainingLog {
* Combines both sources into one log, newest first.
*
* Where the same workout appears in both — the usual case once a user shares one that came
* from their watch — the Health Connect copy wins: it carries the metrics the published
* event may have dropped (heart rate, steps, climb), and its start time is the recorded one
* rather than a publish timestamp.
* from their watch — the Health Connect copy normally wins: it carries the metrics the
* published event may have dropped (heart rate, steps, climb), and its start time is the
* recorded one rather than a publish timestamp.
*
* That preference is only justified while the Health Connect copy actually has those metrics.
* It can arrive without them — its per-session aggregations are read separately and may not
* have come back yet, or may have failed — and then it is strictly worse than the published
* event it would displace. So a copy carrying no metric at all yields to a matching one that
* does, rather than evicting it and taking numbers off the screen. See
* [DetectedWorkout.hasAnyMetric].
*
* The winner keeps the loser's one piece of information: that a kind 1301 for this workout
* exists. Dropping the published copy would otherwise lose that fact, and the survivor —
@@ -60,21 +69,33 @@ object TrainingLog {
healthConnect: List<DetectedWorkout>,
published: List<DetectedWorkout>,
): List<DetectedWorkout> {
val flagged =
healthConnect.map { recorded ->
if (published.any { recorded.isProbablySameWorkoutAs(it) }) {
recorded.copy(alreadyPublished = true)
} else {
recorded
val merged = ArrayList<DetectedWorkout>(healthConnect.size + published.size)
// The published copies that won their tie and are therefore already in [merged].
// Identity, not equality: two published workouts can be equal in every field, and one
// winning must not silently exclude the other from the pass below.
val kept = Collections.newSetFromMap(IdentityHashMap<DetectedWorkout, Boolean>())
healthConnect.forEach { recorded ->
val match = published.firstOrNull { candidate -> recorded.isProbablySameWorkoutAs(candidate) }
when {
match == null -> merged.add(recorded)
recorded.hasAnyMetric || !match.hasAnyMetric -> merged.add(recorded.copy(alreadyPublished = true))
else -> {
// The published event is flagged as published by construction, so no copy.
merged.add(match)
kept.add(match)
}
}
}
val deduped =
published.filterNot { candidate ->
healthConnect.any { it.isProbablySameWorkoutAs(candidate) }
}
// A published workout that any Health Connect copy matched is already represented by
// whichever of the two won — including the ones just added above.
published.forEach { candidate ->
if (candidate !in kept && healthConnect.none { it.isProbablySameWorkoutAs(candidate) }) merged.add(candidate)
}
return (flagged + deduped).sortedByDescending { it.startTimeEpochSeconds }
return merged.sortedByDescending { it.startTimeEpochSeconds }
}
private fun DetectedWorkout.isProbablySameWorkoutAs(other: DetectedWorkout): Boolean =
@@ -183,4 +183,99 @@ class PartialMetricsReportTest {
assertEquals(partial.size, merged.size)
assertEquals(1, merged.count { it.alreadyPublished })
}
@Test
fun `a metric-less health connect copy yields to the published one that still has its numbers`() {
val published =
fullyLoaded.take(1).map {
it.copy(id = "published", origin = WorkoutOrigin.PUBLISHED, alreadyPublished = true)
}
// Health Connect normally wins the tie because it carries more. During stage 1 it carries
// less, and evicting the published copy then would take 10 km off a dashboard that was
// already showing it — down to zero, and back up a second later.
val merged = TrainingLog.merge(partial, published)
val survivor = merged.first { it.startTimeEpochSeconds == published.single().startTimeEpochSeconds }
assertEquals(10_000.0, survivor.distanceMeters!!, 0.001)
assertEquals(700, survivor.calories)
assertTrue(survivor.alreadyPublished)
}
@Test
fun `the whole window keeps its published totals through the partial pass`() {
val published =
fullyLoaded.map {
it.copy(id = "published-${'$'}{it.id}", origin = WorkoutOrigin.PUBLISHED, alreadyPublished = true)
}
// The user has published every one of these. Stage 1 must not make their distance and
// calorie totals dip to zero on the way to showing the same numbers again.
val beforeAnyHealthConnect = WorkoutStats.report(TrainingLog.merge(emptyList(), published), now, zone)
val duringStageOne = WorkoutStats.report(TrainingLog.merge(partial, published), now, zone)
assertEquals(beforeAnyHealthConnect.windowTotals.distanceMeters, duringStageOne.windowTotals.distanceMeters, 0.001)
assertEquals(beforeAnyHealthConnect.windowTotals.calories, duringStageOne.windowTotals.calories)
assertEquals(beforeAnyHealthConnect.windowTotals.steps, duringStageOne.windowTotals.steps)
assertEquals(beforeAnyHealthConnect.windowTotals.maxHeartRate, duringStageOne.windowTotals.maxHeartRate)
assertEquals(beforeAnyHealthConnect.bests.map { it.kind }.toSet(), duringStageOne.bests.map { it.kind }.toSet())
}
@Test
fun `a loaded health connect copy still wins over the published one, as before`() {
// The yielding rule is narrow: it must not invert the normal preference, or a shared
// workout would lose the device detail the published event dropped.
val published =
fullyLoaded.take(1).map {
it.copy(
id = "published",
origin = WorkoutOrigin.PUBLISHED,
alreadyPublished = true,
steps = null,
elevationGainMeters = null,
)
}
val merged = TrainingLog.merge(fullyLoaded, published)
val survivor = merged.first { it.startTimeEpochSeconds == published.single().startTimeEpochSeconds }
assertEquals(WorkoutOrigin.HEALTH_CONNECT, survivor.origin)
assertEquals(9_000, survivor.steps)
assertTrue(survivor.alreadyPublished)
}
@Test
fun `one health connect session still absorbs every published copy it matches`() {
// Pre-existing dedupe behaviour, kept: the yielding rule changes which copy survives a
// tie, never how many survive. Two published events inside the 15-minute tolerance and
// one recorded session is still one workout, not two.
val recorded = listOf(workout(daysAgo = 1, durationSeconds = 1800, distanceMeters = 5_000.0))
val start = recorded.single().startTimeEpochSeconds
val published =
listOf(
recorded.single().copy(id = "p1", startTimeEpochSeconds = start, origin = WorkoutOrigin.PUBLISHED, alreadyPublished = true),
recorded.single().copy(id = "p2", startTimeEpochSeconds = start + 300, origin = WorkoutOrigin.PUBLISHED, alreadyPublished = true),
)
assertEquals(1, TrainingLog.merge(recorded, published).size)
// And the same when the recorded copy is the metric-less one that yields.
assertEquals(1, TrainingLog.merge(recorded.map { it.withoutMetrics() }, published).size)
}
@Test
fun `two copies that both lack metrics keep the health connect one`() {
// A gym session genuinely has no distance or steps on either side. Nothing to prefer, so
// the normal rule stands and the pair still collapses to one entry.
val strengthOnly = listOf(workout(daysAgo = 3, exercise = ExerciseType.STRENGTH, durationSeconds = 2700))
val published =
strengthOnly.map {
it.copy(id = "published", origin = WorkoutOrigin.PUBLISHED, alreadyPublished = true)
}
val merged = TrainingLog.merge(strengthOnly, published)
assertEquals(1, merged.size)
assertEquals(WorkoutOrigin.HEALTH_CONNECT, merged.single().origin)
assertTrue(merged.single().alreadyPublished)
}
}