From 1e4c076cf048ad0d00de77149de45e19e9cf831f Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 19 Sep 2026 01:03:41 +0000 Subject: [PATCH] fix(fitness): stop the partial Health Connect pass from downgrading what is already on screen MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Claude-Session: https://claude.ai/code/session_01VgfgpDWnu635p2qc2K2yMX --- .../workouts/fitness/MyFitnessScreen.kt | 14 +- .../workouts/fitness/MyFitnessViewModel.kt | 66 +++++--- .../fitness/HealthConnectContributionTest.kt | 144 ++++++++++++++++++ .../commons/fitness/DetectedWorkout.kt | 20 ++- .../amethyst/commons/fitness/TrainingLog.kt | 49 ++++-- .../fitness/PartialMetricsReportTest.kt | 95 ++++++++++++ 6 files changed, 346 insertions(+), 42 deletions(-) create mode 100644 amethyst/src/test/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/workouts/fitness/HealthConnectContributionTest.kt diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/workouts/fitness/MyFitnessScreen.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/workouts/fitness/MyFitnessScreen.kt index fa77b92f63..ba50cd377b 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/workouts/fitness/MyFitnessScreen.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/workouts/fitness/MyFitnessScreen.kt @@ -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) } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/workouts/fitness/MyFitnessViewModel.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/workouts/fitness/MyFitnessViewModel.kt index 33a92f9810..5889789b81 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/workouts/fitness/MyFitnessViewModel.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/workouts/fitness/MyFitnessViewModel.kt @@ -98,26 +98,11 @@ class MyFitnessViewModel : ViewModel() { private val pubkeyHex = MutableStateFlow(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 = 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 = 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, + ) + } +} diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/workouts/fitness/HealthConnectContributionTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/workouts/fitness/HealthConnectContributionTest.kt new file mode 100644 index 0000000000..636ac5c3ad --- /dev/null +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/workouts/fitness/HealthConnectContributionTest.kt @@ -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) + } +} diff --git a/commons/src/jvmAndroid/kotlin/com/vitorpamplona/amethyst/commons/fitness/DetectedWorkout.kt b/commons/src/jvmAndroid/kotlin/com/vitorpamplona/amethyst/commons/fitness/DetectedWorkout.kt index a99e9bf58d..f06a970de3 100644 --- a/commons/src/jvmAndroid/kotlin/com/vitorpamplona/amethyst/commons/fitness/DetectedWorkout.kt +++ b/commons/src/jvmAndroid/kotlin/com/vitorpamplona/amethyst/commons/fitness/DetectedWorkout.kt @@ -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 +} diff --git a/commons/src/jvmAndroid/kotlin/com/vitorpamplona/amethyst/commons/fitness/TrainingLog.kt b/commons/src/jvmAndroid/kotlin/com/vitorpamplona/amethyst/commons/fitness/TrainingLog.kt index d6cb6c6d08..ee4061bb85 100644 --- a/commons/src/jvmAndroid/kotlin/com/vitorpamplona/amethyst/commons/fitness/TrainingLog.kt +++ b/commons/src/jvmAndroid/kotlin/com/vitorpamplona/amethyst/commons/fitness/TrainingLog.kt @@ -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, published: List, ): List { - val flagged = - healthConnect.map { recorded -> - if (published.any { recorded.isProbablySameWorkoutAs(it) }) { - recorded.copy(alreadyPublished = true) - } else { - recorded + val merged = ArrayList(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()) + + 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 = diff --git a/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/fitness/PartialMetricsReportTest.kt b/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/fitness/PartialMetricsReportTest.kt index d01f2ddfff..8cbedd1529 100644 --- a/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/fitness/PartialMetricsReportTest.kt +++ b/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/fitness/PartialMetricsReportTest.kt @@ -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) + } }