diff --git a/Strand/Data/Repository.swift b/Strand/Data/Repository.swift index 6124cbef0a..9d5c4cc837 100644 --- a/Strand/Data/Repository.swift +++ b/Strand/Data/Repository.swift @@ -1187,7 +1187,10 @@ final class Repository: ObservableObject { /// Cost is one COUNT plus one MAX per id, straight over the `(deviceId, ts)` index with no rows /// materialized, against the per-day row fetches a caller would otherwise repeat. Compared only to /// itself in memory, so the format is free to change. Kotlin twin: - /// `WhoopRepository.hrFingerprintWindowUnion`. + /// `WhoopRepository.hrUnionFingerprint`, which is a twin in ROLE only: each side compares its own + /// value against its own previous value, neither is persisted or sent anywhere, and the two encode + /// the same facts differently. There is no byte-identity contract here and no oracle asserting one, + /// so do not "align" the encodings on the assumption that there is. func hrFingerprintUnion(from: Int, to: Int) async -> String { guard let store = await ensureStore() else { return "" } var parts: [String] = [] diff --git a/Tools/parity_twin_map.json b/Tools/parity_twin_map.json index 641d9fc4af..3a4795b275 100644 --- a/Tools/parity_twin_map.json +++ b/Tools/parity_twin_map.json @@ -19,7 +19,7 @@ }, "authority": { "files": {"count": 505, "sha256": "581ec0dcab0d8486ce113646cb5cbad71769353eb3aef75f45486e03591733a0"}, - "functions": {"count": 4508, "sha256": "e0669951cb737a12aa4e67b8a52c89f9ed407799902e8dfc97c63790c0e9d665"}, + "functions": {"count": 4507, "sha256": "ce9326a53287c00561b1c642cfea7bca0132f8f89bbb9d45711f4c294518d532"}, "properties": {"count": 465, "sha256": "64fcd18c85bf33a4738d154d45ec86967fdc974e78a448d16e9504d9b13609eb"}, "constants": {"count": 1965, "sha256": "bc257e17e354a48ebbade1e5035aaa41caf21784a119a83488e17270573724da"}, "file_pairs": {"count": 75, "sha256": "d581f60241ec523c595a7e5ff123f7987caee501d8be4ebf6df5ba105e26fe1f"}, @@ -27,7 +27,7 @@ "property_pairs": {"count": 150, "sha256": "be7168d309c8a9cf5f3f25438c028cc60ab64202bb6e9c09a6c553e3699ad0e2"}, "constant_pairs": {"count": 683, "sha256": "c3452cb90df3bda98100154f7f2d109b4b473f6358d6ec1d8476ea9fc183eaa3"}, "unpaired_files": {"count": 377, "sha256": "14e70f77d7e11b4eae3b8412cc493c1aa1bf24dfe8fbc5eca71f81b5daf5253f"}, - "unpaired_functions": {"count": 4109, "sha256": "1fd1fd39d23fcf7624e0dd3de1cb0c5f7245bf8dc3c5f385a4f2091bd1fb7726"}, + "unpaired_functions": {"count": 4108, "sha256": "0aad0d7db52de523a86a5e7438ed4e48bc7a4b54aa1300eb3b56ce907c4616ac"}, "unpaired_properties": {"count": 165, "sha256": "fe609dd25384cda6de1616d36753fed68431b1a1f9a73b6fd664c7ff35089955"}, "unpaired_constants": {"count": 599, "sha256": "b7467c65623ecc296604558a639d62eee2e00874c56d21acc7ec6d3b6264fdc7"} } diff --git a/android/app/src/main/java/com/noop/data/WhoopRepository.kt b/android/app/src/main/java/com/noop/data/WhoopRepository.kt index ede27fa1d3..60e4da481a 100644 --- a/android/app/src/main/java/com/noop/data/WhoopRepository.kt +++ b/android/app/src/main/java/com/noop/data/WhoopRepository.kt @@ -714,29 +714,6 @@ class WhoopRepository( suspend fun hrFingerprintWindow(deviceId: String, from: Long, to: Long): Pair = Pair(dao.countHrInWindow(deviceId, from, to), dao.maxHrTsInWindow(deviceId, from, to)) - /** The same HR fingerprint over the UNION [hrSamplesUnion] reads, as one opaque string. - * - * A fingerprint narrower than the read it guards is worse than none: it would serve a cached result - * after a backfill landed rows under an alias id, which is exactly the id set [hrSamplesUnion] exists - * to cover (#908, a re-added strap banking under its own fresh id). So this walks the same ids rather - * than the bare [activeDeviceId]. - * - * Cost is one COUNT plus one MAX per id, index range walks that materialise no rows, against the full - * per-day row fetches a caller would otherwise repeat. Compared only to itself in memory, so the - * format is free to change. - */ - suspend fun hrFingerprintWindowUnion(activeDeviceId: String, from: Long, to: Long): String { - // An explicit loop rather than joinToString: the per-id read suspends and that builder's lambda - // is not a suspend function. - val parts = StringBuilder() - for (id in rawWhoopSourceIds(activeDeviceId)) { - val (count, maxTs) = hrFingerprintWindow(id, from, to) - if (parts.isNotEmpty()) parts.append('|') - parts.append(id).append(':').append(count).append(':').append(maxTs) - } - return parts.toString() - } - /** Whether [deviceId] has ANY heart-rate row in the window, as a scalar EXISTS rather than a fetched * row. The day-owner resolver's per-candidate-per-day probe; see [WhoopDao.hasHrInWindow]. */ suspend fun hasHrInWindow(deviceId: String, from: Long, to: Long): Boolean { @@ -1180,12 +1157,32 @@ class WhoopRepository( if (deviceIds.isEmpty()) emptyList() else mergeHrByTs(deviceIds.map { dao.hrSamples(it, from, to, limit) }) - /** Count and newest timestamp of measured HR per source [hrSamplesUnion] reads, as one string: an - * index-only witness of whether a window's heart rate changed, without fetching a row. */ + /** Count and newest timestamp of measured HR per source [hrSamplesUnion] reads, as one opaque string: + * an index-only witness of whether a window's heart rate changed, without fetching a row. + * + * A fingerprint narrower than the read it guards is worse than none: it would serve a cached result + * after a backfill landed rows under an alias id, which is exactly the id set [hrSamplesUnion] exists + * to cover (#908, a re-added strap banking under its own fresh id). So this walks the same ids rather + * than the bare [activeDeviceId]. + * + * Cost is one COUNT plus one MAX per id via [hrFingerprintWindow], index range walks that materialise + * no rows, against the full per-day row fetches a caller would otherwise repeat. Compared only to + * itself in memory, so the format is free to change, and no caller persists it. The Swift + * `Repository.hrFingerprintUnion` is a twin in ROLE only, encoding the same facts differently; there + * is no byte-identity contract between them and no oracle asserting one. + * + * The single union witness for both callers (#2566): the cycle load cache in + * [com.noop.analytics.PhysiologicalStepCycleEngine] and the daytime stress lens memo in + * [com.noop.ui.selectedDaytimeStressMode]. Two of these that were free to disagree is what #2566 + * removed, so route a new caller here rather than adding a third. + */ suspend fun hrUnionFingerprint(activeDeviceId: String, from: Long, to: Long): String { + // An explicit loop rather than joinToString: the per-id read suspends and that builder's lambda + // is not a suspend function. val parts = ArrayList() for (id in rawWhoopSourceIds(activeDeviceId)) { - parts += "$id=${dao.countHrInWindow(id, from, to)}:${dao.maxHrTsInWindow(id, from, to)}" + val (count, maxTs) = hrFingerprintWindow(id, from, to) + parts += "$id=$count:$maxTs" } return parts.joinToString(",") } diff --git a/android/app/src/main/java/com/noop/ui/StressDaytimeMode.kt b/android/app/src/main/java/com/noop/ui/StressDaytimeMode.kt index 383dc59df8..36f7697001 100644 --- a/android/app/src/main/java/com/noop/ui/StressDaytimeMode.kt +++ b/android/app/src/main/java/com/noop/ui/StressDaytimeMode.kt @@ -103,7 +103,7 @@ internal suspend fun selectedDaytimeStressMode( val spanFrom = stressLocalDayWindow(todayLocalDay.minusDays(baselineHistoryDays.toLong()), zone).fromEpochSecond val spanTo = stressLocalDayWindow(todayLocalDay.minusDays(1L), zone).toEpochSecondInclusive val cacheKey = "$deviceId|$todayLocalDay|$zone|" + - repo.hrFingerprintWindowUnion(deviceId, spanFrom, spanTo) + repo.hrUnionFingerprint(deviceId, spanFrom, spanTo) return StressLensCache.resolve(cacheKey) { val aggregates = ArrayList(baselineHistoryDays) // Oldest -> newest so the EWMA fold replays history in order. Reduce each day immediately: the diff --git a/android/app/src/test/java/com/noop/data/HrUnionFingerprintTest.kt b/android/app/src/test/java/com/noop/data/HrUnionFingerprintTest.kt new file mode 100644 index 0000000000..6255b8f1d8 --- /dev/null +++ b/android/app/src/test/java/com/noop/data/HrUnionFingerprintTest.kt @@ -0,0 +1,154 @@ +package com.noop.data + +import kotlinx.coroutines.runBlocking +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNotEquals +import org.junit.Test +import java.lang.reflect.Proxy + +/** + * #2566 — [WhoopRepository.hrUnionFingerprint] is the SINGLE index-only witness of whether a window's + * heart rate changed, shared by the cycle load cache and the daytime stress lens memo. Until #2566 there + * were two of these computing the same facts in different encodings, and nothing stopped them drifting. + * + * The property that matters is #908: a strap re-added through the device manager banks its live raw under + * a FRESH id, so a fingerprint keyed on the active id alone would serve a stale cached result after a + * backfill landed rows under the alias. These pin that the witness walks the same id set + * [WhoopRepository.hrSamplesUnion] reads, and that it does so with index aggregates only. + * + * Stubbed through a Proxy [WhoopDao] (no Room), answering per-id so a change under ONE id is visible. + */ +class HrUnionFingerprintTest { + + private fun row(id: String) = PairedDeviceRow( + id = id, brand = "WHOOP", model = "4.0", nickname = null, + sourceKind = "strap", capabilities = "hr", status = "active", + addedAt = 0L, lastSeenAt = 0L, + ) + + /** [counts] and [maxTs] are keyed by deviceId, so each union id answers independently. */ + private fun repo( + registered: List, + counts: Map, + maxTs: Map, + seen: MutableList? = null, + ): WhoopRepository { + val dao = Proxy.newProxyInstance( + WhoopDao::class.java.classLoader, + arrayOf(WhoopDao::class.java), + ) { _, method, args -> + when (method.name) { + "pairedDevices" -> registered.map(::row) + "countHrInWindow" -> { + val id = args[0] as String + seen?.add("count:$id") + counts[id] ?: 0 + } + "maxHrTsInWindow" -> { + val id = args[0] as String + seen?.add("maxTs:$id") + maxTs[id] ?: 0L + } + // Any row fetch here would defeat the point: this is the cheap witness that AVOIDS one. + else -> throw UnsupportedOperationException("hrUnionFingerprint must not call ${method.name}") + } + } as WhoopDao + return WhoopRepository(dao) + } + + // The union is (active, other registered WHOOPs, canonical "my-whoop"), so a single-strap install + // still reads two ids and a re-added strap reads three. + @Test fun walksEveryUnionIdNotJustTheActive() = runBlocking { + val seen = mutableListOf() + val fp = repo( + registered = listOf("whoop-alias"), + counts = mapOf("whoop-active" to 3, "whoop-alias" to 7, "my-whoop" to 11), + maxTs = mapOf("whoop-active" to 300L, "whoop-alias" to 700L, "my-whoop" to 1100L), + seen = seen, + ).hrUnionFingerprint("whoop-active", 0L, 9_999L) + + assertEquals("whoop-active=3:300,whoop-alias=7:700,my-whoop=11:1100", fp) + // Exactly one COUNT and one MAX per id, never a repeat and never a fourth id. + assertEquals( + listOf( + "count:whoop-active", "maxTs:whoop-active", + "count:whoop-alias", "maxTs:whoop-alias", + "count:my-whoop", "maxTs:my-whoop", + ), + seen, + ) + } + + // #908, the reason this is a union at all: rows landing under the ALIAS id must move the witness, + // although the active id is untouched. A fingerprint keyed on the active id alone stays equal here + // and serves a stale cached score. + @Test fun movesWhenARowLandsUnderAnAliasIdOnly() = runBlocking { + val before = repo( + registered = listOf("whoop-alias"), + counts = mapOf("whoop-active" to 5, "whoop-alias" to 0, "my-whoop" to 0), + maxTs = mapOf("whoop-active" to 500L, "whoop-alias" to 0L, "my-whoop" to 0L), + ).hrUnionFingerprint("whoop-active", 0L, 9_999L) + + val after = repo( + registered = listOf("whoop-alias"), + counts = mapOf("whoop-active" to 5, "whoop-alias" to 1, "my-whoop" to 0), + maxTs = mapOf("whoop-active" to 500L, "whoop-alias" to 900L, "my-whoop" to 0L), + ).hrUnionFingerprint("whoop-active", 0L, 9_999L) + + assertNotEquals(before, after) + } + + // The canonical import id is in the union too, so a backfill under "my-whoop" moves it as well. + @Test fun movesWhenARowLandsUnderTheCanonicalIdOnly() = runBlocking { + val before = repo( + registered = emptyList(), + counts = mapOf("whoop-active" to 5, "my-whoop" to 0), + maxTs = mapOf("whoop-active" to 500L, "my-whoop" to 0L), + ).hrUnionFingerprint("whoop-active", 0L, 9_999L) + + val after = repo( + registered = emptyList(), + counts = mapOf("whoop-active" to 5, "my-whoop" to 2), + maxTs = mapOf("whoop-active" to 500L, "my-whoop" to 800L), + ).hrUnionFingerprint("whoop-active", 0L, 9_999L) + + assertEquals("whoop-active=5:500,my-whoop=0:0", before) + assertNotEquals(before, after) + } + + // Unchanged inputs must produce the SAME string, or every caller re-folds on every pass and the + // memo is worse than no memo at all. + @Test fun stableWhenNothingChanged() = runBlocking { + fun fp() = runBlocking { + repo( + registered = listOf("whoop-alias"), + counts = mapOf("whoop-active" to 4, "whoop-alias" to 6, "my-whoop" to 8), + maxTs = mapOf("whoop-active" to 400L, "whoop-alias" to 600L, "my-whoop" to 800L), + ).hrUnionFingerprint("whoop-active", 0L, 9_999L) + } + assertEquals(fp(), fp()) + } + + // An empty window is a stable non-null value per id, so a first run still differs from an unset + // watermark while two empty reads match and nothing churns. + @Test fun emptyWindowIsStableZeroes() = runBlocking { + assertEquals( + "whoop-active=0:0,my-whoop=0:0", + repo(registered = emptyList(), counts = emptyMap(), maxTs = emptyMap()) + .hrUnionFingerprint("whoop-active", 0L, 9_999L), + ) + } + + // The active id is never duplicated when it is also the canonical id (single-strap install that + // never re-paired), so the witness stays one entry rather than comparing "my-whoop" against itself. + @Test fun canonicalActiveIsNotDuplicated() = runBlocking { + assertEquals( + "my-whoop=9:900", + repo( + registered = emptyList(), + counts = mapOf("my-whoop" to 9), + maxTs = mapOf("my-whoop" to 900L), + ).hrUnionFingerprint("my-whoop", 0L, 9_999L), + ) + } +}