Health Connect: Use sync run timestamp as clientRecordVersion for activity records

The per-minute steps, calories and distance records set clientRecordVersion
to the metric value itself. Health Connect keeps the record with the highest
clientRecordVersion on a clientRecordId collision (newVersion >= existing
overwrites), so each minute became pinned to the largest value ever sent for
it. When a device delivers activity in two phases and revises a minute's value
downward between them, the corrected record carries a lower version and is
silently ignored, leaving Health Connect above the device's own total.

Stamp clientRecordVersion with the sync run's wall-clock instead, shared by
every record in the slice. A later run always outranks the value it previously
wrote for a minute, so the freshest re-read wins regardless of whether the
value rose or fell. The clientRecordId remains the dedup key and is unchanged.

app/src/main/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/syncers/AbstractActivitySampleSyncer.kt

@@ -56,7 +56,8 @@ internal abstract class AbstractActivitySampleSyncer<TRecord : Record> : Activit
         sample: ActivitySample,
         offset: ZoneOffset,
         metadata: Metadata,
-        deviceName: String
+        deviceName: String,
+        version: Long
     ): TRecord?
 
     override suspend fun sync(
@@ -87,6 +88,13 @@ internal abstract class AbstractActivitySampleSyncer<TRecord : Record> : Activit
 
         logger.info("Processing ${relevantSamples.size} samples for steps for device '$deviceName' for slice $sliceStartBoundary to $sliceEndBoundary.")
 
+        // clientRecordVersion: every record in this slice shares the run's wall-clock so a later
+        // sync always outranks the value it previously wrote for a minute. HC keeps the highest
+        // version on a clientRecordId collision, so the freshest (most complete) re-read wins even
+        // when a value is revised downward. Must not be the metric value itself: a downward
+        // correction would then carry a lower version and be silently ignored.
+        val recordVersion = System.currentTimeMillis()
+
         val recordsToInsert = mutableListOf<Record>()
         var skippedCount = 0
         var latestSyncedTimestamp: Instant? = null
@@ -108,7 +116,7 @@ internal abstract class AbstractActivitySampleSyncer<TRecord : Record> : Activit
                 return@forEach
             }
 
-            val record = convertSample(sample = currentSample, offset.rules.getOffset(endTs), metadata, deviceName)
+            val record = convertSample(sample = currentSample, offset.rules.getOffset(endTs), metadata, deviceName, recordVersion)
             if (record == null) {
                 skippedCount++
                 return@forEach

app/src/main/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/syncers/ActiveCaloriesSyncer.kt

@@ -35,7 +35,8 @@ internal object ActiveCaloriesSyncer : AbstractActivitySampleSyncer<ActiveCalori
         sample: ActivitySample,
         offset: ZoneOffset,
         metadata: Metadata,
-        deviceName: String
+        deviceName: String,
+        version: Long
     ): ActiveCaloriesBurnedRecord? {
         val caloriesInMinute = sample.activeCalories
         if (caloriesInMinute <= 0) {
@@ -56,7 +57,7 @@ internal object ActiveCaloriesSyncer : AbstractActivitySampleSyncer<ActiveCalori
             endTs,
             offset,
             Energy.calories(caloriesInMinute.toDouble()),
-            clientRecordMetadata(metadata, "calories", endTs.epochSecond, caloriesInMinute.toLong())
+            clientRecordMetadata(metadata, "calories", endTs.epochSecond, version)
         )
     }
 }

app/src/main/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/syncers/DistanceSyncer.kt

@@ -35,7 +35,8 @@ internal object DistanceSyncer : AbstractActivitySampleSyncer<DistanceRecord>()
         sample: ActivitySample,
         offset: ZoneOffset,
         metadata: Metadata,
-        deviceName: String
+        deviceName: String,
+        version: Long
     ): DistanceRecord? {
         val distanceCm = sample.distanceCm
         if (distanceCm <= 0 || distanceCm == ActivitySample.NOT_MEASURED) {
@@ -56,7 +57,7 @@ internal object DistanceSyncer : AbstractActivitySampleSyncer<DistanceRecord>()
             endTime = endTs,
             endZoneOffset = offset,
             distance = Length.meters(distanceCm / 100.0),
-            metadata = clientRecordMetadata(metadata, "distance", endTs.epochSecond, distanceCm.toLong())
+            metadata = clientRecordMetadata(metadata, "distance", endTs.epochSecond, version)
         )
     }
 }

app/src/main/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/syncers/HealthConnectSyncer.kt

@@ -25,8 +25,10 @@ import java.time.Instant
 import java.time.ZoneId
 
 // Deterministic clientRecordId so re-emitting an overlapping window upserts instead of duplicating.
-// version is the clientRecordVersion; HC keeps the highest on conflict, so a later, more complete
-// value wins.
+// version is the clientRecordVersion; HC keeps the highest on conflict (newVersion >= existing
+// overwrites). Callers pass the sync run's wall-clock so a later run always outranks the value it
+// previously wrote for a minute. It must never be the metric value: a downward correction would
+// then carry a lower version and be silently ignored, freezing the minute at its stale maximum.
 internal fun clientRecordMetadata(
     base: Metadata,
     type: String,

app/src/main/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/syncers/StepsSyncer.kt

@@ -34,7 +34,8 @@ internal object StepsSyncer : AbstractActivitySampleSyncer<StepsRecord>() {
         sample: ActivitySample,
         offset: ZoneOffset,
         metadata: Metadata,
-        deviceName: String
+        deviceName: String,
+        version: Long
     ): StepsRecord? {
         val stepsInMinute = sample.steps.toLong()
         // <= 0 means "no steps in that minute" - common, drop silently.
@@ -55,7 +56,7 @@ internal object StepsSyncer : AbstractActivitySampleSyncer<StepsRecord>() {
             endTs,
             offset,
             stepsInMinute,
-            clientRecordMetadata(metadata, "steps", endTs.epochSecond, stepsInMinute)
+            clientRecordMetadata(metadata, "steps", endTs.epochSecond, version)
         )
     }
 }

app/src/test/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/syncers/ActivitySampleSliceBoundaryTest.kt

@@ -1,6 +1,7 @@
 package nodomain.freeyourgadget.gadgetbridge.util.healthconnect.syncers
 
 import androidx.health.connect.client.records.StepsRecord
+import androidx.health.connect.client.records.metadata.Device
 import androidx.health.connect.client.records.metadata.Metadata
 import nodomain.freeyourgadget.gadgetbridge.model.ActivitySample
 import org.junit.Assert.*
@@ -20,7 +21,7 @@ class ActivitySampleSliceBoundaryTest {
     private object IdempotentSyncer : AbstractActivitySampleSyncer<StepsRecord>() {
         override val logger: Logger = LoggerFactory.getLogger("test")
         override val recordClass: KClass<StepsRecord> = StepsRecord::class
-        override fun convertSample(sample: ActivitySample, offset: ZoneOffset, metadata: Metadata, deviceName: String): StepsRecord? = null
+        override fun convertSample(sample: ActivitySample, offset: ZoneOffset, metadata: Metadata, deviceName: String, version: Long): StepsRecord? = null
     }
 
     // Syncer that opts out of the lookback (mirrors a non-idempotent record type).
@@ -28,7 +29,7 @@ class ActivitySampleSliceBoundaryTest {
         override val logger: Logger = LoggerFactory.getLogger("test")
         override val recordClass: KClass<StepsRecord> = StepsRecord::class
         override val lateSampleLookback = java.time.Duration.ZERO
-        override fun convertSample(sample: ActivitySample, offset: ZoneOffset, metadata: Metadata, deviceName: String): StepsRecord? = null
+        override fun convertSample(sample: ActivitySample, offset: ZoneOffset, metadata: Metadata, deviceName: String, version: Long): StepsRecord? = null
     }
 
     private fun within(syncer: AbstractActivitySampleSyncer<*>, endTs: Instant): Boolean =
@@ -72,4 +73,27 @@ class ActivitySampleSliceBoundaryTest {
         val straddle = sliceEnd.plusSeconds(30)
         assertTrue(within(IdempotentSyncer, straddle))
     }
+
+    // The clientRecordVersion must carry the supplied version verbatim (the sync run's wall-clock),
+    // never the metric value: HC keeps the highest version on a clientRecordId collision, so a value
+    // revised downward would be silently ignored if the value doubled as the version.
+    @Test
+    fun clientRecordVersion_carriesSuppliedVersion_notMetricValue() {
+        val base = Metadata.autoRecorded(Device(Device.TYPE_WATCH, "Acme", "Band"))
+        val version = 1_700_000_500_000L
+        val meta = clientRecordMetadata(base, "steps", endEpoch, version)
+        assertEquals(version, meta.clientRecordVersion)
+    }
+
+    // The dedup key (clientRecordId) is fixed by type + device + timestamp and must not vary with the
+    // version, so a re-emit of the same minute collides and upserts instead of duplicating.
+    @Test
+    fun clientRecordId_independentOfVersion() {
+        val base = Metadata.autoRecorded(Device(Device.TYPE_WATCH, "Acme", "Band"))
+        val low = clientRecordMetadata(base, "steps", endEpoch, 1L)
+        val high = clientRecordMetadata(base, "steps", endEpoch, 999L)
+        assertEquals(low.clientRecordId, high.clientRecordId)
+    }
+
+    private val endEpoch = sliceStart.epochSecond
 }