From 0894ec095d4a986d1d22b213c0a26521d45f4d67 Mon Sep 17 00:00:00 2001 From: Fanboynz Date: Thu, 3 Sep 2026 23:11:37 +1200 Subject: [PATCH 1/3] =?UTF-8?q?health:=20stop=20predicting=20which=20devic?= =?UTF-8?q?e=20a=20night=20belongs=20to=20=E2=80=94=20read=20it=20off=20th?= =?UTF-8?q?e=20row?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Third field report of "still five days" on the third build. Two releases of this backfill found nothing, and both failures were the same mistake with different values: they PREDICTED the device id a scored row carries. - #1852 used the strap id, for rows the engine writes under `-noop`. - #1854 fixed that by deriving `-noop` — still a prediction, and the active device is not reliably the one a row belongs to: #1303's serial adoption moves the strap id, a second strap owns its own rows, an imported history carries whatever id it arrived under. Both find nothing rather than something wrong, which is the worst failure available here: zero outstanding is indistinguishable from "already done", so the feature reports success and does nothing. Three builds shipped that way. The query no longer filters by device. It returns `(deviceId, day)` and every id is taken FROM THE ROW: the row's own id for the fill and the session read, and `sampleIdFor` — its id with the computed suffix removed — for the raw sample reads. There is no id left to get wrong. Same fix one level up, where the same assumption was hiding: family, anchor and worn tolerance were resolved ONCE from the active device and applied to every row. On a two-strap install that is not a missing number, it is a wrong one — one strap's raw register converted on another strap's scale. `run` now takes a `deviceContext` resolver, called per strap and cached, and a strap it declines is declined alone rather than stalling the sweep. Full suite 5146/0. Gates clean. --- .../com/noop/analytics/SkinTempBackfill.kt | 72 +++++++++++++------ .../src/main/java/com/noop/data/Entities.kt | 13 ++++ .../src/main/java/com/noop/data/WhoopDao.kt | 16 +++-- .../java/com/noop/data/WhoopRepository.kt | 10 +-- .../src/main/java/com/noop/ui/AppViewModel.kt | 46 ++++++------ .../noop/analytics/SkinTempBackfillTest.kt | 17 +++++ 6 files changed, 121 insertions(+), 53 deletions(-) diff --git a/android/app/src/main/java/com/noop/analytics/SkinTempBackfill.kt b/android/app/src/main/java/com/noop/analytics/SkinTempBackfill.kt index 1facb7f3e2..8dc1185d72 100644 --- a/android/app/src/main/java/com/noop/analytics/SkinTempBackfill.kt +++ b/android/app/src/main/java/com/noop/analytics/SkinTempBackfill.kt @@ -177,19 +177,37 @@ object SkinTempBackfill { fun computedIdFor(deviceId: String): String = if (deviceId.endsWith("-noop")) deviceId else "$deviceId-noop" + /** + * The STRAP id a scored row's raw samples are banked under — its own id with the computed suffix + * removed. Derived from the row rather than from whatever device happens to be active, because the + * two are not reliably the same (#1303 serial adoption, a second strap, an imported history). + */ + fun sampleIdFor(rowDeviceId: String): String = rowDeviceId.removeSuffix("-noop") + /** One PAGE of candidate day keys, strictly after [afterDay]. See the DAO's note on why this pages. */ suspend fun candidates( repo: WhoopRepository, - deviceId: String, afterDay: String, max: Int = DEFAULT_MAX_NIGHTS, - ): List = repo.daysMissingSkinTempAbsolute(deviceId, afterDay, max) + ): List = repo.daysMissingSkinTempAbsolute(afterDay, max) + + /** + * Everything a strap's nights need to convert raw to °C. Null from the resolver DECLINES that strap — + * used when a WHOOP 4.0 has no anchor, where writing on the global default would bank temperatures + * outside human range. + */ + data class DeviceContext( + val family: DeviceFamily, + val anchorRaw: Double?, + val wornToleranceSec: Long, + ) /** * Walk the candidate nights and fill what can be re-derived. * - * [currentAnchorRaw] must be the anchor the engine is using NOW, not one learned from the window being - * filled — see the anchor note on this object. A 4.0 without one is declined outright. + * Every id comes off the ROW, and every conversion context off [deviceContext] for that row's strap — + * nothing here predicts which device a night belongs to. A 4.0 whose anchor will not resolve is + * declined for that strap alone, so one unusable device cannot stall the rest. * * Reads one night at a time rather than one big span: the deep-history case is exactly where a single * unbounded read would be worst, and a per-night read is also what lets a cancelled run leave every @@ -197,23 +215,23 @@ object SkinTempBackfill { */ suspend fun run( repo: WhoopRepository, - /** The STRAP id. Row reads/writes use its computed twin; raw sample reads use it directly. */ - deviceId: String, - family: DeviceFamily, - currentAnchorRaw: Double?, - wornToleranceSec: Long, + /** + * Resolves a STRAP id to its conversion context, or null to decline it. + * + * A lambda rather than three fixed values because rows can belong to ANY device — a second strap, + * an imported history, an id #1303's serial adoption has moved. Resolving once from whatever + * device is "active" and applying it to every row is the same predicted-id mistake that made two + * earlier releases of this find nothing, just one level up: here it would not find nothing, it + * would convert one strap's raw register on another strap's scale. + */ + deviceContext: suspend (String) -> DeviceContext?, zone: java.time.ZoneId = java.time.ZoneId.systemDefault(), nowSeconds: Long = System.currentTimeMillis() / 1000, afterDay: String = "", max: Int = DEFAULT_MAX_NIGHTS, limit: Int = STREAM_LIMIT, ): Report { - if (requiresAnchor(family) && currentAnchorRaw == null) { - return Report(declinedNoAnchor = true) - } - val anchor = anchorFor(family, currentAnchorRaw) - val rowId = computedIdFor(deviceId) - val days = candidates(repo, rowId, afterDay, max) + val days = candidates(repo, afterDay, max) if (days.isEmpty()) return Report(sweepComplete = true) val nowLocalMidnight = java.time.Instant.ofEpochSecond(nowSeconds) @@ -222,7 +240,19 @@ object SkinTempBackfill { var noMean = 0 var noSamples = 0 var noSessions = 0 - for (day in days) { + var declined = 0 + // One resolve per strap, not per night. + val contexts = HashMap() + for (row in days) { + val day = row.day + // Both namespaces come FROM THE ROW: the scored row's own id, and the strap id its raw + // samples sit under. Nothing here predicts an id. + val rowId = row.deviceId + val sampleId = sampleIdFor(rowId) + val ctx = contexts.getOrPut(sampleId) { deviceContext(sampleId) } + if (ctx == null) { declined++; continue } + if (requiresAnchor(ctx.family) && ctx.anchorRaw == null) { declined++; continue } + val anchor = anchorFor(ctx.family, ctx.anchorRaw) val dayStart = runCatching { java.time.LocalDate.parse(day).atStartOfDay(zone).toEpochSecond() }.getOrNull() ?: continue @@ -239,18 +269,18 @@ object SkinTempBackfill { if (sessions.isEmpty()) { noSessions++; continue } val readFrom = sessions.minOf { it.start } val readTo = sessions.maxOf { it.end } - val skin = repo.skinTempSamples(deviceId, readFrom, readTo, limit) + val skin = repo.skinTempSamples(sampleId, readFrom, readTo, limit) if (skin.isEmpty()) { noSamples++; continue } - val hr = repo.hrSamplesForDevice(deviceId, readFrom, readTo, limit) - val mean = nightlyAbsolute(sessions, hr, skin, family, anchor, wornToleranceSec) + val hr = repo.hrSamplesForDevice(sampleId, readFrom, readTo, limit) + val mean = nightlyAbsolute(sessions, hr, skin, ctx.family, anchor, ctx.wornToleranceSec) if (mean == null) { noMean++; continue } // Fill-only by construction; a row that gained an absolute meanwhile reports 0 and is left be. if (repo.fillSkinTempAbsolute(rowId, day, mean) > 0) filled++ else noMean++ } return Report( candidates = days.size, filled = filled, noMean = noMean, - noSamples = noSamples, noSessions = noSessions, - lastDay = days.last(), + noSamples = noSamples, noSessions = noSessions, declinedNoAnchor = declined > 0, + lastDay = days.last().day, // A short page means this sweep has seen every outstanding night. sweepComplete = days.size < max, ) diff --git a/android/app/src/main/java/com/noop/data/Entities.kt b/android/app/src/main/java/com/noop/data/Entities.kt index 9e4bcfad05..0a8249c631 100644 --- a/android/app/src/main/java/com/noop/data/Entities.kt +++ b/android/app/src/main/java/com/noop/data/Entities.kt @@ -745,3 +745,16 @@ data class V18AuxSampleEntity( return result } } + +/** + * #1855: one candidate row for the skin-temp absolutes backfill — the day AND the device namespace it + * actually lives in. + * + * The id is READ, never assumed. Scored rows sit under the engine's `-noop` namespace while raw + * samples sit under the strap id, and #1303's serial adoption can move the strap id underneath both. Two + * separate releases of this backfill found nothing because they predicted the id instead of reading it. + */ +data class SkinTempBackfillRow( + val deviceId: String, + val day: String, +) diff --git a/android/app/src/main/java/com/noop/data/WhoopDao.kt b/android/app/src/main/java/com/noop/data/WhoopDao.kt index c08ceb2b4d..3e0a8c8a8b 100644 --- a/android/app/src/main/java/com/noop/data/WhoopDao.kt +++ b/android/app/src/main/java/com/noop/data/WhoopDao.kt @@ -167,25 +167,29 @@ interface WhoopDao : DeviceRegistryDao { * #1851: the day keys that have a skin-temp DEVIATION but no ABSOLUTE beside it — the nights the * 21-night `analyzeRecent` window never revisited after `skinTempC` shipped (2026-08-27). * + * NOT filtered by device, deliberately. The id a scored row carries is not something a caller can + * safely predict: the engine writes under `-noop`, and #1303's serial adoption can move the + * strap id itself. Guessing it wrong finds nothing and is indistinguishable from "already done" — + * which is exactly how this shipped twice. The row carries its own id; take it from there. + * * PAGED on [afterDay], not just capped. A plain "oldest N" returns the SAME nights every pass, so a * run whose first page cannot fill — and the oldest nights are exactly the ones most likely to have * lost their raw samples — would latch on that page and never reach the newer nights that CAN fill. * The caller advances a cursor, so one sweep visits every candidate exactly once. */ @Query( - "SELECT day FROM dailyMetric WHERE deviceId = :deviceId " + - "AND skinTempC IS NULL AND skinTempDevC IS NOT NULL AND day > :afterDay " + + "SELECT deviceId, day FROM dailyMetric " + + "WHERE skinTempC IS NULL AND skinTempDevC IS NOT NULL AND day > :afterDay " + "ORDER BY day ASC LIMIT :limit" ) - suspend fun daysMissingSkinTempAbsolute(deviceId: String, afterDay: String, limit: Int): List + suspend fun daysMissingSkinTempAbsolute(afterDay: String, limit: Int): List /** #1851: how many nights are outstanding in total — the uncapped count the "nothing left to try" * watermark keys on, so a newly imported night re-arms the sweep. */ @Query( - "SELECT COUNT(*) FROM dailyMetric WHERE deviceId = :deviceId " + - "AND skinTempC IS NULL AND skinTempDevC IS NOT NULL" + "SELECT COUNT(*) FROM dailyMetric WHERE skinTempC IS NULL AND skinTempDevC IS NOT NULL" ) - suspend fun countDaysMissingSkinTempAbsolute(deviceId: String): Int + suspend fun countDaysMissingSkinTempAbsolute(): Int /** * #1851: fill ONE night's absolute, and only when it is absent. 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 a9a5b53d8b..e9277b3a0f 100644 --- a/android/app/src/main/java/com/noop/data/WhoopRepository.kt +++ b/android/app/src/main/java/com/noop/data/WhoopRepository.kt @@ -1176,12 +1176,12 @@ class WhoopRepository( dao.spo2Samples(deviceId, from, to, limit) /** #1851: nights holding a skin-temp deviation but no absolute, oldest first. See [SkinTempBackfill]. */ - suspend fun daysMissingSkinTempAbsolute(deviceId: String, afterDay: String, limit: Int): List = - dao.daysMissingSkinTempAbsolute(deviceId, afterDay, limit) + suspend fun daysMissingSkinTempAbsolute(afterDay: String, limit: Int): List = + dao.daysMissingSkinTempAbsolute(afterDay, limit) - /** #1851: total outstanding nights, uncapped. See [daysMissingSkinTempAbsolute]. */ - suspend fun countDaysMissingSkinTempAbsolute(deviceId: String): Int = - dao.countDaysMissingSkinTempAbsolute(deviceId) + /** #1851: total outstanding nights, uncapped and across every device. See the DAO's note on why this + * is not filtered by id. */ + suspend fun countDaysMissingSkinTempAbsolute(): Int = dao.countDaysMissingSkinTempAbsolute() /** * #1851: fill ONE night's absolute, only where none exists. Returns rows changed (0 = already had one). diff --git a/android/app/src/main/java/com/noop/ui/AppViewModel.kt b/android/app/src/main/java/com/noop/ui/AppViewModel.kt index c4cc6ebf67..b7c20b048a 100644 --- a/android/app/src/main/java/com/noop/ui/AppViewModel.kt +++ b/android/app/src/main/java/com/noop/ui/AppViewModel.kt @@ -738,35 +738,39 @@ class AppViewModel(app: Application) : AndroidViewModel(app) { */ private suspend fun backfillSkinTempAbsolutes(deviceId: String) { val prefs = NoopPrefs.of(appContext) - // Scored rows live in the COMPUTED namespace; raw samples under the strap id. Counting the strap - // id here found nothing at all, so the sweep looked finished before it began. - val outstanding = repository.countDaysMissingSkinTempAbsolute( - com.noop.analytics.SkinTempBackfill.computedIdFor(deviceId), - ) + // #1855: not filtered by device. Two releases of this found nothing because they PREDICTED the id + // a scored row carries — the engine's "-noop" namespace, and a strap id that #1303's serial + // adoption can move underneath it. Each row now carries the id it actually lives under. + val outstanding = repository.countDaysMissingSkinTempAbsolute() if (outstanding == 0) return if (prefs.getInt(skinTempBackfillStuckKey, -1) == outstanding) return val ownerSource = RegistryDayOwnerSource(noopApp.deviceRegistry) - val family = ownerSource.skinTempFamily(deviceId) - val tolerance = ownerSource.skinTempWornToleranceSec(deviceId) val now = System.currentTimeMillis() / 1000 - val anchor = if (family == com.noop.protocol.DeviceFamily.WHOOP4) { - // Same span the engine learns its anchor over, NOT the window being filled. - val scanFrom = now - 21L * 24 * 3_600 - val windowSkin = repository.skinTempSamples( - deviceId, scanFrom, now, com.noop.analytics.SkinTempBackfill.STREAM_LIMIT, - ) - com.noop.protocol.Whoop4SkinTemp.deviceAnchorRaw(windowSkin.map { it.raw }) - } else { - null - } val cursor = prefs.getString(skinTempBackfillCursorKey, "") ?: "" val report = com.noop.analytics.SkinTempBackfill.run( repo = repository, - deviceId = deviceId, - family = family, - currentAnchorRaw = anchor, - wornToleranceSec = tolerance, + // Resolved per STRAP the rows actually belong to, not once from whatever device is active — + // converting one strap's raw register on another strap's scale is a wrong number, not a + // missing one. + deviceContext = { strapId -> + val family = ownerSource.skinTempFamily(strapId) + val anchor = if (family == com.noop.protocol.DeviceFamily.WHOOP4) { + // The span the engine learns its anchor over, NOT the window being filled. + val windowSkin = repository.skinTempSamples( + strapId, now - 21L * 24 * 3_600, now, + com.noop.analytics.SkinTempBackfill.STREAM_LIMIT, + ) + com.noop.protocol.Whoop4SkinTemp.deviceAnchorRaw(windowSkin.map { it.raw }) + } else { + null + } + com.noop.analytics.SkinTempBackfill.DeviceContext( + family = family, + anchorRaw = anchor, + wornToleranceSec = ownerSource.skinTempWornToleranceSec(strapId), + ) + }, nowSeconds = now, afterDay = cursor, ) diff --git a/android/app/src/test/java/com/noop/analytics/SkinTempBackfillTest.kt b/android/app/src/test/java/com/noop/analytics/SkinTempBackfillTest.kt index b97ff028c0..62577c33e5 100644 --- a/android/app/src/test/java/com/noop/analytics/SkinTempBackfillTest.kt +++ b/android/app/src/test/java/com/noop/analytics/SkinTempBackfillTest.kt @@ -233,6 +233,23 @@ class SkinTempBackfillTest { assertEquals("my-whoop-noop", SkinTempBackfill.computedIdFor("my-whoop")) } + /** + * #1855: the STRAP id a row's raw samples sit under is DERIVED FROM THE ROW, never predicted. + * + * Two releases of this backfill found nothing because they guessed the id — first the strap id for + * rows the engine writes under "-noop", then the active device's id for rows that may belong to + * another strap entirely (#1303 serial adoption, a second strap, an imported history). Both failures + * were silent and read as "already done". + */ + @Test + fun theSampleIdComesOffTheRowsOwnId() { + assertEquals("my-whoop", SkinTempBackfill.sampleIdFor("my-whoop-noop")) + // An id that never carried the suffix is already a strap id. + assertEquals("whoop-4A2B", SkinTempBackfill.sampleIdFor("whoop-4A2B")) + // Round-trips with computedIdFor, so the pair cannot drift apart. + assertEquals("whoop-4A2B", SkinTempBackfill.sampleIdFor(SkinTempBackfill.computedIdFor("whoop-4A2B"))) + } + /** Idempotent on an id that is already computed, mirroring the repository's own ownerComputed idiom — * otherwise a re-entry would look for "…-noop-noop" and, again, find nothing. */ @Test From 1c8896c2099844ffcc107450a954ae9b3dcf1673 Mon Sep 17 00:00:00 2001 From: Fanboynz Date: Thu, 3 Sep 2026 23:19:14 +1200 Subject: [PATCH 2/3] =?UTF-8?q?health:=20re-review=20=E2=80=94=20dropping?= =?UTF-8?q?=20the=20device=20filter=20broke=20the=20cursor=20and=20the=20d?= =?UTF-8?q?ecline?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two defects, both introduced by THIS change. Removing the device filter was right; I did not follow it through the two places that assumed one. 1. A DECLINED STRAP FROZE THE SWEEP. `declinedNoAnchor` used to mean "the whole run declined, nothing examined" — the run resolved one device and bailed. Now it means "SOME strap was declined", but the caller still read it as "record nothing", so it skipped the cursor advance too. One un-anchored WHOOP 4.0 anywhere in the history would therefore pin the sweep on page one and re-read it on EVERY tick, forever: no progress, and a standing battery cost. The cursor now always advances; a decline withholds only the STUCK watermark, so the sweep retries once that strap's anchor resolves. 2. THE CURSOR WAS A DAY, AND DAYS ARE NO LONGER UNIQUE. Without a device filter one day can hold rows for several devices. A page boundary falling between two same-day rows skipped the far side for the whole sweep — a night silently never attempted, which is precisely the failure this feature has now produced three times. The cursor is composite (`day|deviceId`) and the query orders by both, so every row is visited exactly once. Neither is a new idea; both are the old assumption surviving in a corner I did not re-read after changing what the query returns. Full suite 5147/0 (20 in this file). Gates clean. --- .../com/noop/analytics/SkinTempBackfill.kt | 18 +++++---- .../src/main/java/com/noop/data/WhoopDao.kt | 13 +++++-- .../java/com/noop/data/WhoopRepository.kt | 4 +- .../src/main/java/com/noop/ui/AppViewModel.kt | 18 ++++++--- .../noop/analytics/SkinTempBackfillTest.kt | 38 ++++++++++++++++--- 5 files changed, 66 insertions(+), 25 deletions(-) diff --git a/android/app/src/main/java/com/noop/analytics/SkinTempBackfill.kt b/android/app/src/main/java/com/noop/analytics/SkinTempBackfill.kt index 8dc1185d72..359088cd19 100644 --- a/android/app/src/main/java/com/noop/analytics/SkinTempBackfill.kt +++ b/android/app/src/main/java/com/noop/analytics/SkinTempBackfill.kt @@ -72,8 +72,9 @@ object SkinTempBackfill { val noSamples: Int = 0, /** Nights with no stored sleep session ending on them — nothing to compute a nightly mean over. */ val noSessions: Int = 0, - /** The newest day this page examined; the caller's cursor for the next page. Empty when none. */ - val lastDay: String = "", + /** COMPOSITE cursor (`day|deviceId`) for the next page — a day alone would skip a same-day row on + * the far side of a page boundary. Empty when the page was empty. */ + val lastCursor: String = "", /** True when the page came back short — every outstanding night has now been visited this sweep. */ val sweepComplete: Boolean = false, val declinedNoAnchor: Boolean = false, @@ -187,9 +188,9 @@ object SkinTempBackfill { /** One PAGE of candidate day keys, strictly after [afterDay]. See the DAO's note on why this pages. */ suspend fun candidates( repo: WhoopRepository, - afterDay: String, + afterCursor: String, max: Int = DEFAULT_MAX_NIGHTS, - ): List = repo.daysMissingSkinTempAbsolute(afterDay, max) + ): List = repo.daysMissingSkinTempAbsolute(afterCursor, max) /** * Everything a strap's nights need to convert raw to °C. Null from the resolver DECLINES that strap — @@ -227,11 +228,11 @@ object SkinTempBackfill { deviceContext: suspend (String) -> DeviceContext?, zone: java.time.ZoneId = java.time.ZoneId.systemDefault(), nowSeconds: Long = System.currentTimeMillis() / 1000, - afterDay: String = "", + afterCursor: String = "", max: Int = DEFAULT_MAX_NIGHTS, limit: Int = STREAM_LIMIT, ): Report { - val days = candidates(repo, afterDay, max) + val days = candidates(repo, afterCursor, max) if (days.isEmpty()) return Report(sweepComplete = true) val nowLocalMidnight = java.time.Instant.ofEpochSecond(nowSeconds) @@ -280,7 +281,7 @@ object SkinTempBackfill { return Report( candidates = days.size, filled = filled, noMean = noMean, noSamples = noSamples, noSessions = noSessions, declinedNoAnchor = declined > 0, - lastDay = days.last().day, + lastCursor = cursorOf(days.last()), // A short page means this sweep has seen every outstanding night. sweepComplete = days.size < max, ) @@ -289,6 +290,9 @@ object SkinTempBackfill { /** Per-night read cap, matching the engine's stream limit. */ const val STREAM_LIMIT = 200_000 + /** The composite paging cursor for a row — see [Report.lastCursor]. */ + fun cursorOf(row: com.noop.data.SkinTempBackfillRow): String = "${row.day}|${row.deviceId}" + /** Seconds in a local day, for the end-of-day bound the engine attributes sessions by. */ const val SECONDS_PER_DAY: Long = 86_400L } diff --git a/android/app/src/main/java/com/noop/data/WhoopDao.kt b/android/app/src/main/java/com/noop/data/WhoopDao.kt index 3e0a8c8a8b..ffbe725e09 100644 --- a/android/app/src/main/java/com/noop/data/WhoopDao.kt +++ b/android/app/src/main/java/com/noop/data/WhoopDao.kt @@ -172,17 +172,22 @@ interface WhoopDao : DeviceRegistryDao { * strap id itself. Guessing it wrong finds nothing and is indistinguishable from "already done" — * which is exactly how this shipped twice. The row carries its own id; take it from there. * - * PAGED on [afterDay], not just capped. A plain "oldest N" returns the SAME nights every pass, so a + * The cursor is COMPOSITE (`day|deviceId`), not the day alone. Without a device filter a single day + * can hold rows for several devices, and a page boundary falling between them would skip the row on + * the far side for the whole sweep — a night silently never attempted. + * + * PAGED, not just capped. A plain "oldest N" returns the SAME nights every pass, so a * run whose first page cannot fill — and the oldest nights are exactly the ones most likely to have * lost their raw samples — would latch on that page and never reach the newer nights that CAN fill. * The caller advances a cursor, so one sweep visits every candidate exactly once. */ @Query( "SELECT deviceId, day FROM dailyMetric " + - "WHERE skinTempC IS NULL AND skinTempDevC IS NOT NULL AND day > :afterDay " + - "ORDER BY day ASC LIMIT :limit" + "WHERE skinTempC IS NULL AND skinTempDevC IS NOT NULL " + + "AND (day || '|' || deviceId) > :afterCursor " + + "ORDER BY day ASC, deviceId ASC LIMIT :limit" ) - suspend fun daysMissingSkinTempAbsolute(afterDay: String, limit: Int): List + suspend fun daysMissingSkinTempAbsolute(afterCursor: String, limit: Int): List /** #1851: how many nights are outstanding in total — the uncapped count the "nothing left to try" * watermark keys on, so a newly imported night re-arms the sweep. */ 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 e9277b3a0f..b0574a85c0 100644 --- a/android/app/src/main/java/com/noop/data/WhoopRepository.kt +++ b/android/app/src/main/java/com/noop/data/WhoopRepository.kt @@ -1176,8 +1176,8 @@ class WhoopRepository( dao.spo2Samples(deviceId, from, to, limit) /** #1851: nights holding a skin-temp deviation but no absolute, oldest first. See [SkinTempBackfill]. */ - suspend fun daysMissingSkinTempAbsolute(afterDay: String, limit: Int): List = - dao.daysMissingSkinTempAbsolute(afterDay, limit) + suspend fun daysMissingSkinTempAbsolute(afterCursor: String, limit: Int): List = + dao.daysMissingSkinTempAbsolute(afterCursor, limit) /** #1851: total outstanding nights, uncapped and across every device. See the DAO's note on why this * is not filtered by id. */ diff --git a/android/app/src/main/java/com/noop/ui/AppViewModel.kt b/android/app/src/main/java/com/noop/ui/AppViewModel.kt index b7c20b048a..01cdeaafb5 100644 --- a/android/app/src/main/java/com/noop/ui/AppViewModel.kt +++ b/android/app/src/main/java/com/noop/ui/AppViewModel.kt @@ -772,19 +772,25 @@ class AppViewModel(app: Application) : AndroidViewModel(app) { ) }, nowSeconds = now, - afterDay = cursor, + afterCursor = cursor, ) - // A decline records nothing: a 4.0 whose anchor resolves later must still get its backfill. - if (!report.declinedNoAnchor) { + // The cursor ALWAYS advances. `declinedNoAnchor` now means "some strap was declined", not "the run + // did nothing" — withholding the bookkeeping on it would freeze the cursor and re-process page one + // on every tick, forever, for any install holding one un-anchored 4.0. A decline only withholds + // the STUCK watermark, so the sweep retries once that strap's anchor resolves. + run { val sweepFills = prefs.getInt(skinTempBackfillSweepFillsKey, 0) + report.filled val edit = prefs.edit() if (report.sweepComplete) { // Every outstanding night has been visited. Start the next sweep from the top, and only - // stop paying for reads when the WHOLE sweep — not just its last page — found nothing. + // stop paying for reads when the WHOLE sweep — not just its last page — found nothing AND + // nothing was declined. edit.remove(skinTempBackfillCursorKey).remove(skinTempBackfillSweepFillsKey) - if (sweepFills == 0) edit.putInt(skinTempBackfillStuckKey, outstanding) + if (sweepFills == 0 && !report.declinedNoAnchor) { + edit.putInt(skinTempBackfillStuckKey, outstanding) + } } else { - edit.putString(skinTempBackfillCursorKey, report.lastDay) + edit.putString(skinTempBackfillCursorKey, report.lastCursor) .putInt(skinTempBackfillSweepFillsKey, sweepFills) } if (report.filled > 0) edit.remove(skinTempBackfillStuckKey) diff --git a/android/app/src/test/java/com/noop/analytics/SkinTempBackfillTest.kt b/android/app/src/test/java/com/noop/analytics/SkinTempBackfillTest.kt index 62577c33e5..b879020269 100644 --- a/android/app/src/test/java/com/noop/analytics/SkinTempBackfillTest.kt +++ b/android/app/src/test/java/com/noop/analytics/SkinTempBackfillTest.kt @@ -206,16 +206,42 @@ class SkinTempBackfillTest { fun anEmptyPageEndsTheSweep() { val empty = SkinTempBackfill.Report(sweepComplete = true) assertTrue(empty.sweepComplete) - assertEquals("", empty.lastDay) + assertEquals("", empty.lastCursor) assertEquals(0, empty.examined) } - /** The cursor is the newest day the page examined, so the next page starts strictly after it and no - * night is visited twice within one sweep. */ + /** + * The cursor is COMPOSITE (`day|deviceId`), not the day alone. + * + * With no device filter a single day can hold rows for several devices. A day-only cursor would skip + * whichever same-day row fell on the far side of a page boundary — a night silently never attempted, + * which is the failure mode this whole feature keeps producing. + */ + @Test + fun theCursorCarriesTheDeviceToo() { + val row = com.noop.data.SkinTempBackfillRow(deviceId = "my-whoop-noop", day = "2026-08-11") + assertEquals("2026-08-11|my-whoop-noop", SkinTempBackfill.cursorOf(row)) + // Two rows sharing a day produce DIFFERENT cursors, which is the whole point. + val other = com.noop.data.SkinTempBackfillRow(deviceId = "my-whoop", day = "2026-08-11") + assertTrue(SkinTempBackfill.cursorOf(row) != SkinTempBackfill.cursorOf(other)) + } + + /** + * A decline must not look like "the run did nothing". + * + * `declinedNoAnchor` now means SOME strap was declined, not that the run examined nothing. The caller + * must still advance its cursor on it — withholding the bookkeeping would freeze the sweep on page one + * and re-read it every tick, forever, on any install holding one un-anchored 4.0. + */ @Test - fun theCursorIsThePagesNewestDay() { - val r = SkinTempBackfill.Report(candidates = 2, filled = 1, lastDay = "2026-08-11") - assertEquals("2026-08-11", r.lastDay) + fun aDeclinedStrapStillLeavesUsableProgress() { + val r = SkinTempBackfill.Report( + candidates = 60, filled = 3, noSamples = 40, declinedNoAnchor = true, + lastCursor = "2026-08-11|my-whoop-noop", + ) + assertTrue(r.declinedNoAnchor) + assertTrue("a declining run can still fill other straps' nights", r.filled > 0) + assertTrue("and must still hand back a cursor", r.lastCursor.isNotEmpty()) } // MARK: re-review 4 — the backfill straddles TWO device namespaces From 99e9873acba2e77394892ea10b8e02f2b6e72a13 Mon Sep 17 00:00:00 2001 From: Fanboynz Date: Thu, 3 Sep 2026 23:30:13 +1200 Subject: [PATCH 3/3] =?UTF-8?q?health:=20re-review=20=E2=80=94=20make=20th?= =?UTF-8?q?e=20backfill's=20own=20diagnostic=20reconcile?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The log line this run emits is how "why did nothing fill?" gets answered from an export, so a gap in it costs a round-trip with a real person. It had one. Declined nights were counted locally and never reported. A page of un-anchored 4.0 nights logged `candidates=60 examined=0` with no category covering the other sixty — the reader's next move is to hunt for a missing bug, when the answer was already known inside the run. `declined` is now a Report field, included in `examined`, and a test asserts candidates == examined so every night is accounted for by exactly one outcome. `declinedNoAnchor` became a derived property over that count rather than a separate boolean it could disagree with. That dual source of truth is what produced last pass's freeze, where the flag meant one thing to the runner and another to the caller. Documented an assumption the composite cursor rests on rather than leaving it implied: comparing `day|deviceId` as a plain string only equals ordering by (day, deviceId) because the day key is FIXED-WIDTH ISO. A variable-width key would silently reorder the sweep — silent reordering being this feature's recurring failure, it is worth a sentence. Also dropped the `run { }` wrapper left over from removing the decline gate; it was holding a block that no longer needed one. Full suite 5149/0 (2 new). Gates clean. --- .../com/noop/analytics/SkinTempBackfill.kt | 23 +++++++++--- .../src/main/java/com/noop/ui/AppViewModel.kt | 33 +++++++++-------- .../noop/analytics/SkinTempBackfillTest.kt | 35 +++++++++++++++---- 3 files changed, 64 insertions(+), 27 deletions(-) diff --git a/android/app/src/main/java/com/noop/analytics/SkinTempBackfill.kt b/android/app/src/main/java/com/noop/analytics/SkinTempBackfill.kt index 359088cd19..cbdf8a273b 100644 --- a/android/app/src/main/java/com/noop/analytics/SkinTempBackfill.kt +++ b/android/app/src/main/java/com/noop/analytics/SkinTempBackfill.kt @@ -77,9 +77,17 @@ object SkinTempBackfill { val lastCursor: String = "", /** True when the page came back short — every outstanding night has now been visited this sweep. */ val sweepComplete: Boolean = false, - val declinedNoAnchor: Boolean = false, + /** + * Nights skipped because their strap could not be converted — a WHOOP 4.0 with no resolvable + * anchor. Counted, not silent: this log line exists to answer "why did nothing fill?", and a + * candidates/examined pair that does not reconcile sends the reader looking in the wrong place. + */ + val declined: Int = 0, ) { - val examined: Int get() = filled + noMean + noSamples + noSessions + val examined: Int get() = filled + noMean + noSamples + noSessions + declined + + /** Whether ANY strap was declined this page. Derived, so it cannot drift from [declined]. */ + val declinedNoAnchor: Boolean get() = declined > 0 } /** @@ -280,7 +288,7 @@ object SkinTempBackfill { } return Report( candidates = days.size, filled = filled, noMean = noMean, - noSamples = noSamples, noSessions = noSessions, declinedNoAnchor = declined > 0, + noSamples = noSamples, noSessions = noSessions, declined = declined, lastCursor = cursorOf(days.last()), // A short page means this sweep has seen every outstanding night. sweepComplete = days.size < max, @@ -290,7 +298,14 @@ object SkinTempBackfill { /** Per-night read cap, matching the engine's stream limit. */ const val STREAM_LIMIT = 200_000 - /** The composite paging cursor for a row — see [Report.lastCursor]. */ + /** + * The composite paging cursor for a row — see [Report.lastCursor]. + * + * Safe as a plain string comparison because [com.noop.data.DailyMetric.day] is FIXED-WIDTH ISO + * (`yyyy-MM-dd`): the separator always lands at the same offset, so comparing the concatenation is + * exactly comparing `(day, deviceId)`, which is the order the query pages in. A variable-width key + * here would silently reorder the sweep. + */ fun cursorOf(row: com.noop.data.SkinTempBackfillRow): String = "${row.day}|${row.deviceId}" /** Seconds in a local day, for the end-of-day bound the engine attributes sessions by. */ diff --git a/android/app/src/main/java/com/noop/ui/AppViewModel.kt b/android/app/src/main/java/com/noop/ui/AppViewModel.kt index 01cdeaafb5..fdcedabcac 100644 --- a/android/app/src/main/java/com/noop/ui/AppViewModel.kt +++ b/android/app/src/main/java/com/noop/ui/AppViewModel.kt @@ -778,29 +778,28 @@ class AppViewModel(app: Application) : AndroidViewModel(app) { // did nothing" — withholding the bookkeeping on it would freeze the cursor and re-process page one // on every tick, forever, for any install holding one un-anchored 4.0. A decline only withholds // the STUCK watermark, so the sweep retries once that strap's anchor resolves. - run { - val sweepFills = prefs.getInt(skinTempBackfillSweepFillsKey, 0) + report.filled - val edit = prefs.edit() - if (report.sweepComplete) { - // Every outstanding night has been visited. Start the next sweep from the top, and only - // stop paying for reads when the WHOLE sweep — not just its last page — found nothing AND - // nothing was declined. - edit.remove(skinTempBackfillCursorKey).remove(skinTempBackfillSweepFillsKey) - if (sweepFills == 0 && !report.declinedNoAnchor) { - edit.putInt(skinTempBackfillStuckKey, outstanding) - } - } else { - edit.putString(skinTempBackfillCursorKey, report.lastCursor) - .putInt(skinTempBackfillSweepFillsKey, sweepFills) + val sweepFills = prefs.getInt(skinTempBackfillSweepFillsKey, 0) + report.filled + val edit = prefs.edit() + if (report.sweepComplete) { + // Every outstanding night has been visited. Start the next sweep from the top, and only stop + // paying for reads when the WHOLE sweep — not just its last page — found nothing AND nothing + // was declined. + edit.remove(skinTempBackfillCursorKey).remove(skinTempBackfillSweepFillsKey) + if (sweepFills == 0 && !report.declinedNoAnchor) { + edit.putInt(skinTempBackfillStuckKey, outstanding) } - if (report.filled > 0) edit.remove(skinTempBackfillStuckKey) - edit.apply() + } else { + edit.putString(skinTempBackfillCursorKey, report.lastCursor) + .putInt(skinTempBackfillSweepFillsKey, sweepFills) } + if (report.filled > 0) edit.remove(skinTempBackfillStuckKey) + edit.apply() ble.externalLog( "skinTempBackfill: page=${report.candidates} filled=${report.filled} noMean=${report.noMean} " + "noSamples=${report.noSamples} noSessions=${report.noSessions} " + + "declined=${report.declined} " + "outstanding=$outstanding sweepComplete=${report.sweepComplete} " + - "declined=${report.declinedNoAnchor}", + "sweepFills=$sweepFills", com.noop.testcentre.TestDomain.UNIVERSAL, ) } diff --git a/android/app/src/test/java/com/noop/analytics/SkinTempBackfillTest.kt b/android/app/src/test/java/com/noop/analytics/SkinTempBackfillTest.kt index b879020269..7973db003e 100644 --- a/android/app/src/test/java/com/noop/analytics/SkinTempBackfillTest.kt +++ b/android/app/src/test/java/com/noop/analytics/SkinTempBackfillTest.kt @@ -90,8 +90,8 @@ class SkinTempBackfillTest { fun theReportDistinguishesWhyANightDidNotFill() { val r = SkinTempBackfill.Report(candidates = 10, filled = 4, noMean = 2, noSamples = 4) assertEquals(10, r.examined) - val declined = SkinTempBackfill.Report(declinedNoAnchor = true) - assertEquals(0, declined.examined) + val declined = SkinTempBackfill.Report(candidates = 2, declined = 2) + assertEquals(2, declined.examined) assertTrue(declined.declinedNoAnchor) } @@ -115,14 +115,14 @@ class SkinTempBackfillTest { */ @Test fun aDeclineIsDistinguishableFromAFruitlessRun() { - val declined = SkinTempBackfill.Report(declinedNoAnchor = true) + val declined = SkinTempBackfill.Report(candidates = 3, declined = 3) val fruitless = SkinTempBackfill.Report(candidates = 5, noSamples = 5) assertTrue(declined.declinedNoAnchor) assertFalse(fruitless.declinedNoAnchor) assertEquals(0, declined.filled) assertEquals(0, fruitless.filled) - // The two differ in whether anything was looked at, which is what the caller keys on. - assertEquals(0, declined.examined) + // Both examined their nights; they differ in WHY nothing filled, which is what the log must say. + assertEquals(3, declined.examined) assertEquals(5, fruitless.examined) } @@ -210,6 +210,29 @@ class SkinTempBackfillTest { assertEquals(0, empty.examined) } + /** + * Every candidate on a page must be accounted for by exactly one outcome. + * + * The run's log line exists to answer "why did nothing fill?". A candidates/examined pair that does + * not reconcile sends the reader hunting for a missing category — and declined nights were counted + * locally but never surfaced, so a page of un-anchored 4.0 nights reported candidates=60, examined=0 + * and no reason at all. + */ + @Test + fun everyCandidateIsAccountedFor() { + val r = SkinTempBackfill.Report( + candidates = 10, filled = 2, noMean = 1, noSamples = 3, noSessions = 1, declined = 3, + ) + assertEquals("candidates must reconcile with the outcomes", r.candidates, r.examined) + } + + /** The flag is DERIVED from the count, so the two cannot drift apart. */ + @Test + fun theDeclineFlagFollowsTheCount() { + assertFalse(SkinTempBackfill.Report(declined = 0).declinedNoAnchor) + assertTrue(SkinTempBackfill.Report(declined = 1).declinedNoAnchor) + } + /** * The cursor is COMPOSITE (`day|deviceId`), not the day alone. * @@ -236,7 +259,7 @@ class SkinTempBackfillTest { @Test fun aDeclinedStrapStillLeavesUsableProgress() { val r = SkinTempBackfill.Report( - candidates = 60, filled = 3, noSamples = 40, declinedNoAnchor = true, + candidates = 60, filled = 3, noSamples = 40, declined = 17, lastCursor = "2026-08-11|my-whoop-noop", ) assertTrue(r.declinedNoAnchor)