From e6bc54df310b9e714e80f58e4e6e958cbeaa594d Mon Sep 17 00:00:00 2001 From: Pipiche Date: Mon, 21 Sep 2026 12:12:15 +0200 Subject: [PATCH] fix(sleep): hand the #899 heal's bank-recency witness to the computed id only, never to a ring's own rows The post-upsert heal ran `dedupe(healable, freshStarts: keptStarts)` for every id in `healDeviceIds` (#1248), including an Oura ring's own. `keptStarts` is the computed pass's bank witness, and the comment claimed it "only matches the computedId rows" - false on an Oura day, where the pass's sessions ARE the ring's `providedSleep` rows with `startTs` copied verbatim. The ring row the pass had READ was therefore ranked "fresh" in the ring's own sweep and outranked every fuller re-serve the ring banked while the pass was in flight (hours on a phone, where the OS suspends the app between the read and the heal and wakes it precisely for the drain that persists the fuller row). On 2026-09-19/20 the heal deleted the 598-min full night (22:23 -> 08:21) one second after it landed and kept the 337-min row read at 04:14; the day scored 315 min ending 04:48. `SleepSessionDedup.healWitness` is now the one shared rule: the witness goes to the computedId sweep only; every other id gets none and falls back to longest-wins, the read-side default. The heal's existing re-arm re-pass then scores the kept full night. Kotlin twin case-for-case; no schema change and no behaviour change for a WHOOP strap, whose rows live only under computedId. Tests: SleepSessionDedupTests (Swift 33/33) and SleepSessionDedupTest (Kotlin 33/33) gain the witness rule and the 09-19/20 scenario, with the leaked-witness outcome pinned so it cannot creep back. macOS `Strand` target compiled locally. Refs #899, #1248, #1284. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01TcXLwCpyu7skJQcfYAoDzi --- .../WhoopStore/SleepSessionDedup.swift | 13 ++++++++ .../SleepSessionDedupTests.swift | 31 +++++++++++++++++++ Strand/Data/IntelligenceEngine.swift | 15 +++++++-- .../com/noop/analytics/IntelligenceEngine.kt | 15 +++++++-- .../com/noop/analytics/SleepSessionDedup.kt | 14 +++++++++ .../noop/analytics/SleepSessionDedupTest.kt | 29 +++++++++++++++++ 6 files changed, 111 insertions(+), 6 deletions(-) diff --git a/Packages/WhoopStore/Sources/WhoopStore/SleepSessionDedup.swift b/Packages/WhoopStore/Sources/WhoopStore/SleepSessionDedup.swift index a44284eb16..9123ed83d8 100644 --- a/Packages/WhoopStore/Sources/WhoopStore/SleepSessionDedup.swift +++ b/Packages/WhoopStore/Sources/WhoopStore/SleepSessionDedup.swift @@ -120,6 +120,19 @@ public enum SleepSessionDedup { dropped.sorted { $0.startTs < $1.startTs }) } + /// The bank-recency witness the post-upsert heal may hand `dedupe` for ONE device id's rows. + /// + /// `keptStarts` are the `startTs` of the sessions the analyze pass just banked under `computedId`. They + /// witness recency ONLY there: on a day scored from a device-provided hypnogram (an Oura ring) the pass's + /// sessions are that device's own stored rows with `startTs` copied verbatim, so the same keys name the + /// row the pass READ in the device's own table — the stalest row of the night by the time the heal runs, + /// not the freshest. Handing them to that sweep ranked the read row above every fuller re-serve banked + /// while the pass was in flight, and the heal deleted the full night. Every id but `computedId` therefore + /// gets no witness and falls back to longest-wins, the read-side default. Twin of Kotlin's `healWitness`. + public static func healWitness(for healId: String, computedId: String, keptStarts: Set) -> Set { + healId == computedId ? keptStarts : [] + } + // MARK: - #1284 residual 3: generation-side 0x49-onset keying (at-persist, no schema migration) /// The grid (seconds) the 0x49 onset is rounded to before it becomes a session's `startTs`. The ring diff --git a/Packages/WhoopStore/Tests/WhoopStoreTests/SleepSessionDedupTests.swift b/Packages/WhoopStore/Tests/WhoopStoreTests/SleepSessionDedupTests.swift index ff4091b3b0..eeb5859f1b 100644 --- a/Packages/WhoopStore/Tests/WhoopStoreTests/SleepSessionDedupTests.swift +++ b/Packages/WhoopStore/Tests/WhoopStoreTests/SleepSessionDedupTests.swift @@ -96,6 +96,37 @@ final class SleepSessionDedupTests: XCTestCase { XCTAssertEqual(result.kept.map(\.startTs), [long.startTs]) } + // MARK: - Heal witness: the computed bank-recency witness must not reach a provided device's rows + + func testHealWitnessIsHandedOnlyToTheComputedId() { + let kept: Set = [midnight - 2 * 3600, midnight + 15 * 3600] + XCTAssertEqual(SleepSessionDedup.healWitness(for: "my-whoop-noop", computedId: "my-whoop-noop", + keptStarts: kept), kept) + XCTAssertEqual(SleepSessionDedup.healWitness(for: "oura-Y12", computedId: "my-whoop-noop", + keptStarts: kept), []) + XCTAssertEqual(SleepSessionDedup.healWitness(for: "oura-Y12", computedId: "my-whoop-noop", + keptStarts: []), []) + } + + func testRingSweepKeepsTheFullerReserveBankedWhileThePassWasInFlight() { + // 09-19/20 (iOS 11.8.0): the pass READ the ring's 22:20 → 03:57 row, iOS suspended it, and by the + // time its heal ran the ring had re-served the night out to 08:21. The read row's startTs is in + // `keptStarts` (the pass banked it verbatim under computedId). Handed to the ring's own sweep as the + // witness, it outranked the full night and the heal deleted 598 min in favour of 337 — the wake + // time the user saw was 04:48. With `healWitness` the ring id gets no witness: longest wins. + let read = session(start: midnight - 2 * 3600 + 53, end: midnight + 3 * 3600 + 57 * 60) // 337 min + let full = session(start: midnight - 2 * 3600 + 231, end: midnight + 8 * 3600 + 21 * 60) // 598 min + let keptStarts: Set = [read.startTs] + let ringWitness = SleepSessionDedup.healWitness(for: "oura-Y12", computedId: "my-whoop-noop", + keptStarts: keptStarts) + let healed = SleepSessionDedup.dedupe([read, full], freshStarts: ringWitness) + XCTAssertEqual(healed.kept.map(\.startTs), [full.startTs], "the fuller re-serve survives the heal") + XCTAssertEqual(healed.dropped.map(\.startTs), [read.startTs]) + // The regression, pinned so it cannot creep back: the leaked witness keeps the stale read row. + let leaked = SleepSessionDedup.dedupe([read, full], freshStarts: keptStarts) + XCTAssertEqual(leaked.kept.map(\.startTs), [read.startTs]) + } + func testUserEditedSessionIsNeverDropped() { // A hand-corrected night outranks everything, including a fresh re-detection. let edited = session(start: midnight - 8 * 3600, end: midnight, edited: true) diff --git a/Strand/Data/IntelligenceEngine.swift b/Strand/Data/IntelligenceEngine.swift index ed27af9faf..d657e2a979 100644 --- a/Strand/Data/IntelligenceEngine.swift +++ b/Strand/Data/IntelligenceEngine.swift @@ -2795,8 +2795,16 @@ final class IntelligenceEngine: ObservableObject { // re-read as `providedSleep` and re-detected every pass, so one night ballooned to 14 rows / 9 // "naps". Dedup each device's rows AMONG THEMSELVES and delete stale copies under that SAME id // (never across ids, so a survivor is never orphaned under an id the day-owner read skips). - // `freshStarts` (this pass's computed bank witness) only matches the computedId rows; the others - // fall back to longest-wins, the read-side dedup's own default. Sorted for a deterministic order. + // `freshStarts` (this pass's computed bank witness) is handed ONLY to the computedId sweep; every + // other id falls back to longest-wins, the read-side dedup's own default. It used to be passed to + // every id on the claim that it "only matches the computedId rows" — false on an Oura day, where the + // pass's sessions ARE the ring's `providedSleep` rows with `startTs` copied verbatim. The ring row + // the pass had READ was then ranked "fresh" in the ring's own sweep and outranked every fuller + // re-serve the ring banked while the pass was in flight (hours, when iOS suspends the app between + // the read and this heal): on 09-19/20 the heal deleted the 598-min full night one second after it + // landed and kept the 337-min row read at 04:14, so the day ended at 04:48 instead of 08:21. + // `SleepSessionDedup.healWitness` is the one shared rule (twin of Kotlin's). Sorted for a + // deterministic order. let healDeviceIds = Self.healDeviceIds(computedId: computedId, registeredIds: regDevices.map { $0.id }) // Compact shape of a row for the #1284 heal log — the two measures that adjudicate WHICH copy is // fuller (stage-segment count + decoded JSON length), in the SAME format as the dup-gen diagnostic @@ -2814,7 +2822,8 @@ final class IntelligenceEngine: ObservableObject { let healable = storedSessions.filter { (oldestDay...newestDay).contains(AnalyticsEngine.dayString($0.endTs, offsetSec: tzOffset)) } - let sweep = SleepSessionDedup.dedupe(healable, freshStarts: keptStarts) + let witness = SleepSessionDedup.healWitness(for: healId, computedId: computedId, keptStarts: keptStarts) + let sweep = SleepSessionDedup.dedupe(healable, freshStarts: witness) for stale in sweep.dropped { _ = try? await store.deleteSleepSession(deviceId: healId, startTs: stale.startTs) // #1284: log which copy was dropped and which survived, so the corpus can confirm the heal diff --git a/android/app/src/main/java/com/noop/analytics/IntelligenceEngine.kt b/android/app/src/main/java/com/noop/analytics/IntelligenceEngine.kt index ba6c4bf978..0831a654ef 100644 --- a/android/app/src/main/java/com/noop/analytics/IntelligenceEngine.kt +++ b/android/app/src/main/java/com/noop/analytics/IntelligenceEngine.kt @@ -1986,8 +1986,16 @@ object IntelligenceEngine { // "naps". Dedup each device's rows AMONG THEMSELVES and delete stale copies under that SAME id // (deleteSleepSessionRowOnly deletes under the row's own deviceId), never across ids, so a survivor // is never orphaned under an id the day-owner read skips. `freshStarts` (this pass's computed bank - // witness) only matches the computedId rows; the others fall back to longest-wins, the read-side - // dedup's own default. Sorted for a deterministic order. Mirrors the Swift analyzeRecent heal. + // witness) is handed ONLY to the computedId sweep; every other id falls back to longest-wins, the + // read-side dedup's own default. It used to be passed to every id on the claim that it "only + // matches the computedId rows" — false on an Oura day, where the pass's sessions ARE the ring's + // `providedSleep` rows with `startTs` copied verbatim. The ring row the pass had READ was then + // ranked "fresh" in the ring's own sweep and outranked every fuller re-serve the ring banked while + // the pass was in flight (hours, when the OS suspends the app between the read and this heal): on + // 09-19/20 the heal deleted the 598-min full night one second after it landed and kept the 337-min + // row read at 04:14, so the day ended at 04:48 instead of 08:21. `SleepSessionDedup.healWitness` is + // the one shared rule (twin of Swift's). Sorted for a deterministic order. Mirrors the Swift + // analyzeRecent heal. val healDeviceIds = healDeviceIds(computedId, candidatePriorities.map { it.first }) // Compact shape of a row for the #1284 heal log — the two measures that adjudicate WHICH copy is // fuller (stage-segment count + decoded JSON length), in the SAME format as the dup-gen diagnostic @@ -2004,7 +2012,8 @@ object IntelligenceEngine { val healable = storedSessions.filter { AnalyticsEngine.dayString(it.endTs, tzOffsetSeconds) in oldestDay..newestDay } - val sweep = SleepSessionDedup.dedupe(healable, freshStarts = keptStarts) + val witness = SleepSessionDedup.healWitness(healId, computedId, keptStarts) + val sweep = SleepSessionDedup.dedupe(healable, freshStarts = witness) // Row-only delete: the user-facing deleteSleepSession writes a #33 dismissal tombstone, which // would overlap the SURVIVING night's window and permanently suppress its re-detection. for (stale in sweep.dropped) { diff --git a/android/app/src/main/java/com/noop/analytics/SleepSessionDedup.kt b/android/app/src/main/java/com/noop/analytics/SleepSessionDedup.kt index 137cc3272b..f8422f5222 100644 --- a/android/app/src/main/java/com/noop/analytics/SleepSessionDedup.kt +++ b/android/app/src/main/java/com/noop/analytics/SleepSessionDedup.kt @@ -133,6 +133,20 @@ object SleepSessionDedup { return Result(kept.sortedBy { it.startTs }, dropped.sortedBy { it.startTs }) } + /** + * The bank-recency witness the post-upsert heal may hand [dedupe] for ONE device id's rows. + * + * [keptStarts] are the startTs of the sessions the analyze pass just banked under [computedId]. They + * witness recency ONLY there: on a day scored from a device-provided hypnogram (an Oura ring) the pass's + * sessions are that device's own stored rows with startTs copied verbatim, so the same keys name the row + * the pass READ in the device's own table — the stalest row of the night by the time the heal runs, not + * the freshest. Handing them to that sweep ranked the read row above every fuller re-serve banked while + * the pass was in flight, and the heal deleted the full night. Every id but [computedId] therefore gets + * no witness and falls back to longest-wins, the read-side default. Twin of Swift's `healWitness`. + */ + fun healWitness(healId: String, computedId: String, keptStarts: Set): Set = + if (healId == computedId) keptStarts else emptySet() + // ── #1284 residual 3: generation-side 0x49-onset keying (at-persist, no schema migration) ───── /** diff --git a/android/app/src/test/java/com/noop/analytics/SleepSessionDedupTest.kt b/android/app/src/test/java/com/noop/analytics/SleepSessionDedupTest.kt index 56259622de..dd330247ab 100644 --- a/android/app/src/test/java/com/noop/analytics/SleepSessionDedupTest.kt +++ b/android/app/src/test/java/com/noop/analytics/SleepSessionDedupTest.kt @@ -104,6 +104,35 @@ class SleepSessionDedupTest { assertEquals(listOf(long.startTs), result.kept.map { it.startTs }) } + // ── Heal witness: the computed bank-recency witness must not reach a provided device's rows ── + + @Test + fun healWitness_isHandedOnlyToTheComputedId() { + val kept = setOf(midnight - 2 * 3600L, midnight + 15 * 3600L) + assertEquals(kept, SleepSessionDedup.healWitness("my-whoop-noop", "my-whoop-noop", kept)) + assertEquals(emptySet(), SleepSessionDedup.healWitness("oura-Y12", "my-whoop-noop", kept)) + assertEquals(emptySet(), SleepSessionDedup.healWitness("oura-Y12", "my-whoop-noop", emptySet())) + } + + @Test + fun ringSweep_keepsTheFullerReserveBankedWhileThePassWasInFlight() { + // 09-19/20 (iOS 11.8.0): the pass READ the ring's 22:20 → 03:57 row, the OS suspended it, and by + // the time its heal ran the ring had re-served the night out to 08:21. The read row's startTs is in + // keptStarts (the pass banked it verbatim under computedId). Handed to the ring's own sweep as the + // witness, it outranked the full night and the heal deleted 598 min in favour of 337 — the wake + // time the user saw was 04:48. With healWitness the ring id gets no witness: longest wins. + val read = session(midnight - 2 * 3600L + 53, midnight + 3 * 3600L + 57 * 60) // 337 min + val full = session(midnight - 2 * 3600L + 231, midnight + 8 * 3600L + 21 * 60) // 598 min + val keptStarts = setOf(read.startTs) + val ringWitness = SleepSessionDedup.healWitness("oura-Y12", "my-whoop-noop", keptStarts) + val healed = SleepSessionDedup.dedupe(listOf(read, full), freshStarts = ringWitness) + assertEquals("the fuller re-serve survives the heal", listOf(full.startTs), healed.kept.map { it.startTs }) + assertEquals(listOf(read.startTs), healed.dropped.map { it.startTs }) + // The regression, pinned so it cannot creep back: the leaked witness keeps the stale read row. + val leaked = SleepSessionDedup.dedupe(listOf(read, full), freshStarts = keptStarts) + assertEquals(listOf(read.startTs), leaked.kept.map { it.startTs }) + } + @Test fun userEditedSession_isNeverDropped() { // A hand-corrected night outranks everything, including a fresh re-detection.