From 05203e753b8a7c86572e923ec577420bc3e94d97 Mon Sep 17 00:00:00 2001 From: Fanboynz Date: Wed, 2 Sep 2026 22:27:31 +1200 Subject: [PATCH 1/2] sleep: stop the HR-only fallback inheriting an exclusion written to keep WHOOP out The spine has never once run. The gate line added in #1812 said so on the first build that carried it: [sleep] hr-only gate attempted=false reason=imported-owner grav=0 stored=0 on a day holding 165,980 HR rows. Not a band too tight, not a duration gate - the branch was skipped before any heart rate was looked at. #1806 took its condition verbatim from #804 Fix A, whose comment says it exists "so WHOOP straps and the my-whoop import namespace are untouched". #804 is about Oura RINGS and deliberately excludes WHOOP. The HR-only fallback is FOR a WHOOP strap, so it inherited a condition whose entire purpose was to exclude the case it was built for. resolveDayOwner returns importedDeviceId whenever the owner source is absent or the candidates collapse to it, which on this install is every day - so it was never going to fire, and the p10 anchor I spent a pass tuning was never reached. Split by question rather than by gate. #804's stored-hypnogram branch keeps the owner exclusion, a ring's own night still not coming from the import namespace. The heart-rate fallback drops it. Between them, a new branch: a day that HAS stored sessions is left alone whoever owns it, because inferring a night from heart rate when the device recorded a real one replaces better evidence with worse. The stored lookup now runs for EVERY no-motion day, so "nothing else knows about this night" is checked rather than assumed. That is a new query per no-motion day for the import namespace, which previously skipped it - one indexed LIMIT read, and it is the read that keeps this from inventing nights for import users who already have real ones. Not free, and worth the correctness. Pinned by asserting the shape: the HR-only branch must not mention importedDeviceId, and the stored branch must. A behavioural test cannot reach a gate that needs a repository, and the failure mode is someone reinstating the exclusion for symmetry. Both platforms. 26 HR-only tests, 0 failures. Refs #1801. --- Strand/Data/IntelligenceEngine.swift | 51 ++++++++-------- .../com/noop/analytics/IntelligenceEngine.kt | 59 +++++++++++-------- .../SleepStagerHrOnlySessionsTest.kt | 11 ++++ 3 files changed, 70 insertions(+), 51 deletions(-) diff --git a/Strand/Data/IntelligenceEngine.swift b/Strand/Data/IntelligenceEngine.swift index 9de476ebcf..ae1e81a07d 100644 --- a/Strand/Data/IntelligenceEngine.swift +++ b/Strand/Data/IntelligenceEngine.swift @@ -1167,45 +1167,44 @@ final class IntelligenceEngine: ObservableObject { // untouched; analyzeDay still lets a DETECTED session win where the two overlap. Reconstruct the // pure SleepSession from each stored CachedSleepSession (a minute-dict import row decodes to // nothing and is skipped, so only real stage timelines are injected). + // #804 Fix A + #1801. Two different questions share the "this day has no motion" gate. + // + // #804 hands over a device's OWN persisted hypnogram (an Oura ring's SleepNet night) and is + // deliberately not applied to the import namespace. #1801 stages from heart rate when there + // is no hypnogram at all — and must NOT inherit #804's owner exclusion, which is the bug + // this replaces: `resolveDayOwner` returns the imported id whenever the owner source is + // absent or the candidates collapse to it, so on a live 5/MG install the whole branch was + // skipped before any heart rate was looked at. + // + // The stored lookup now runs for every no-motion day, so "nothing else knows about this + // night" is checked rather than assumed. A day that HAS stored sessions is left alone + // whoever owns it. let providedSleep: [SleepSession] - if owner != Repository.whoopSource, grav.count < 2 { + if grav.count < 2 { let persisted = (try? await store.sleepSessions(deviceId: owner, from: from, to: to, limit: 4000)) ?? [] let stored = persisted.compactMap { AnalyticsEngine.sleepSession(fromProvided: $0) } - // #1801: the comment above assumes a WHOOP always streams a gravity vector. A 5/MG that - // cannot bond does not — it is never clocked, so it banks nothing and persists no - // hypnogram either, leaving neither a spine nor a stored night however much HR it holds. - // Fall back to the HR-only spine ONLY when the device supplied nothing of its own: a - // hypnogram the device actually recorded is always better evidence than one inferred - // from heart rate. - // Route the spine's funnel through the SAME `traceSink` the gate lines use, so one - // report explains both halves rather than one going quiet — which is exactly what - // shipping it silent cost on Android (#1801). That sink is nil unless Sleep test mode - // is active, so this collects nothing on a normal pass; passing a closure of my own - // here would have appended on every scored day regardless. - if stored.isEmpty { - traceSink?(SleepStager.GateTrace.hrOnlyGateLine( - attempted: true, reason: "no-motion-no-hypnogram", - gravRows: grav.count, storedNights: 0)) - providedSleep = SleepStager.hrOnlySessions(hr: hr, rr: rr, resp: resp, - traceSink: traceSink) - } else { + if owner != Repository.whoopSource, !stored.isEmpty { traceSink?(SleepStager.GateTrace.hrOnlyGateLine( attempted: false, reason: "stored-hypnogram", gravRows: grav.count, storedNights: stored.count)) providedSleep = stored - } - } else { - // Only worth saying on a day with NO motion: there the spine was the remaining option - // and something declined it, which a silent absence could not distinguish. A day WITH - // motion skips this — the motion spine is the answer there. - if grav.count < 2 { + } else if !stored.isEmpty { traceSink?(SleepStager.GateTrace.hrOnlyGateLine( - attempted: false, reason: "imported-owner", + attempted: false, reason: "stored-sessions-exist", + gravRows: grav.count, storedNights: stored.count)) + providedSleep = [] + } else { + traceSink?(SleepStager.GateTrace.hrOnlyGateLine( + attempted: true, reason: "no-motion-no-hypnogram", gravRows: grav.count, storedNights: 0)) + providedSleep = SleepStager.hrOnlySessions(hr: hr, rr: rr, resp: resp, + traceSink: traceSink) } + } else { providedSleep = [] } + let tScore0 = Date() dayPrepSeconds += tScore0.timeIntervalSince(tPrep0) // #1770 follow-up: the Effort ring's funnel. Collected here rather than sent straight 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 876ffec253..92096b906d 100644 --- a/android/app/src/main/java/com/noop/analytics/IntelligenceEngine.kt +++ b/android/app/src/main/java/com/noop/analytics/IntelligenceEngine.kt @@ -956,44 +956,53 @@ object IntelligenceEngine { // scores. Gated on absent gravity (`grav.size < 2` — a ring streams zero; a WHOOP always streams a // gravity vector) plus a non-canonical-WHOOP-import owner, so WHOOP straps and the "my-whoop" // import namespace are untouched; analyzeDay still lets a DETECTED session win where they overlap. - val providedSleep: List = - if (owner != importedDeviceId && grav.size < 2) { - val stored = repo.sleepSessionsForDevice(owner, from, to, 4000) - .mapNotNull { AnalyticsEngine.sleepSessionFromProvided(it) } - // #1801: a ring persists its OWN hypnogram, so `stored` covers it. A 5/MG that cannot - // bond persists nothing (it is never clocked, so it banks nothing) and streams no motion - // either — neither a spine nor a stored night, which is why this day scored blank with - // `reason=no-motion` however much HR it held. Fall back to the HR-only spine ONLY when - // the device supplied nothing of its own: a hypnogram the device actually recorded is - // always better evidence than one inferred from heart rate. - if (stored.isNotEmpty()) { + // #804 Fix A + #1801. Two different questions share the "this day has no motion" gate. + // + // #804 hands over a device's OWN persisted hypnogram (an Oura ring's SleepNet night), and is + // deliberately not applied to the import namespace. #1801 stages from heart rate when there is + // no hypnogram at all — and that one must NOT inherit #804's owner exclusion, which is the bug + // this replaces: `resolveDayOwner` returns [importedDeviceId] whenever the owner source is + // absent or the candidates collapse to it, so on a live 5/MG install the whole branch was + // skipped before any heart rate was looked at. The field log said so outright once the gate + // line existed: `attempted=false reason=imported-owner grav=0`, on a day holding 165,980 HR + // rows. A condition written to exclude WHOOP straps was guarding a fallback FOR one. + // + // The stored lookup now runs for every no-motion day, so "nothing else knows about this + // night" is checked rather than assumed. A day that HAS stored sessions is left alone whoever + // owns it: inferring a night from heart rate when the device recorded a real one would be + // strictly worse evidence replacing better. + val providedSleep: List = if (grav.size < 2) { + val stored = repo.sleepSessionsForDevice(owner, from, to, 4000) + .mapNotNull { AnalyticsEngine.sleepSessionFromProvided(it) } + when { + owner != importedDeviceId && stored.isNotEmpty() -> { dayDiag(SleepStagerTrace.hrOnlyGateLine( attempted = false, reason = "stored-hypnogram", gravRows = grav.size, storedNights = stored.size, )) stored - } else { + } + stored.isNotEmpty() -> { + // The import namespace keeps #804's exclusion — its rows are not handed to + // analyzeDay as "provided" — but they still mean this night is already known, + // so the heart-rate fallback stays out of it. dayDiag(SleepStagerTrace.hrOnlyGateLine( - attempted = true, reason = "no-motion-no-hypnogram", - gravRows = grav.size, storedNights = 0, + attempted = false, reason = "stored-sessions-exist", + gravRows = grav.size, storedNights = stored.size, )) - // Route the spine's funnel through the SAME per-day recorder as `sleep-detect`, - // so one report explains both halves rather than one of them going quiet. - SleepStager.hrOnlySessions(hr, rr, resp, traceSink = ::dayDiag) + emptyList() } - } else { - // Only worth saying on a day with NO motion: there the spine was the remaining - // option and something declined it, which is precisely what a silent absence could - // not distinguish. A day WITH motion skips this — the motion spine is the answer - // there and nothing was passed over. - if (grav.size < 2) { + else -> { dayDiag(SleepStagerTrace.hrOnlyGateLine( - attempted = false, reason = "imported-owner", + attempted = true, reason = "no-motion-no-hypnogram", gravRows = grav.size, storedNights = 0, )) + SleepStager.hrOnlySessions(hr, rr, resp, traceSink = ::dayDiag) } - emptyList() } + } else { + emptyList() + } val tScore0 = System.nanoTime() dayPrepNanos += tScore0 - tPrep0 diff --git a/android/app/src/test/java/com/noop/analytics/SleepStagerHrOnlySessionsTest.kt b/android/app/src/test/java/com/noop/analytics/SleepStagerHrOnlySessionsTest.kt index 06a7d981da..e8f317771d 100644 --- a/android/app/src/test/java/com/noop/analytics/SleepStagerHrOnlySessionsTest.kt +++ b/android/app/src/test/java/com/noop/analytics/SleepStagerHrOnlySessionsTest.kt @@ -109,6 +109,17 @@ class SleepStagerHrOnlySessionsTest { expr.contains("SleepStager.hrOnlySessions(")) assertTrue("and must only run when the device supplied no hypnogram of its own", expr.contains("stored.isNotEmpty()")) + // The bug this replaced: the fallback inherited #804's `owner != importedDeviceId` exclusion, + // written to keep WHOOP straps OUT of a ring's hypnogram path — and so it never fired on the + // WHOOP strap it exists for. `resolveDayOwner` returns importedDeviceId whenever the owner + // source is absent, which on a live 5/MG install is every day. Pinned by asserting the owner + // check does not stand between the gravity gate and the call: it may still scope the STORED + // branch, but not the heart-rate one. + val hrOnlyBranch = expr.substring(expr.indexOf("else ->")) + assertTrue("the HR-only branch must not be gated on the day's owner", + !hrOnlyBranch.contains("importedDeviceId")) + assertTrue("but the stored-hypnogram branch keeps #804's exclusion", + expr.substring(0, expr.indexOf("else ->")).contains("owner != importedDeviceId")) } /** No HR at all cannot produce a night, and must not throw. */ From c8c2ce0cdef4587344171997b43df938593c7574 Mon Sep 17 00:00:00 2001 From: Fanboynz Date: Wed, 2 Sep 2026 22:30:44 +1200 Subject: [PATCH 2/2] sleep: record that dropping the owner check widens who can reach the fallback Re-review. Removing the owner exclusion does not only unblock the 5/MG it was written for - it makes the branch reachable for ANY owner, including a WHOOP 4.0 day that banked nothing at all. I had told the maintainer a 4.0 was untouched "because grav.size < 2 is false", which is true of a normal 4.0 day and not true of an empty one. Low risk, and not zero: such a day has too little of anything to clear minSleepMin, but "too little" is not "none", and the night it could produce is display-only and marked hrOnly like every other. The comment now says so at the branch rather than leaving the next reader to work out that the gate widened. Also verified this pass, and sound: the guard test splits the expression on "else ->", and there is exactly one of those, one importedDeviceId, none after the split. The middle branch returning emptyList() for an import-namespace day with stored sessions preserves what both #804 and pre-#1806 did - that path has always yielded nothing there, so nothing moved. And a normal WHOOP day never reaches the stored lookup at all, since grav.size >= 2 short-circuits it, so the added query lands only on days that were already scoring blank. --- Strand/Data/IntelligenceEngine.swift | 4 ++++ .../src/main/java/com/noop/analytics/IntelligenceEngine.kt | 7 +++++++ 2 files changed, 11 insertions(+) diff --git a/Strand/Data/IntelligenceEngine.swift b/Strand/Data/IntelligenceEngine.swift index ae1e81a07d..495e6e43ca 100644 --- a/Strand/Data/IntelligenceEngine.swift +++ b/Strand/Data/IntelligenceEngine.swift @@ -1195,6 +1195,10 @@ final class IntelligenceEngine: ObservableObject { gravRows: grav.count, storedNights: stored.count)) providedSleep = [] } else { + // Reachable for ANY owner now, which widens this past the 5/MG it was built for: + // a WHOOP 4.0 day that banked nothing at all also lands here, where the owner + // check previously blocked it. A normal 4.0 day is untouched — it streams + // gravity, so it never reaches this gate. traceSink?(SleepStager.GateTrace.hrOnlyGateLine( attempted: true, reason: "no-motion-no-hypnogram", gravRows: grav.count, storedNights: 0)) 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 92096b906d..24bf3e974a 100644 --- a/android/app/src/main/java/com/noop/analytics/IntelligenceEngine.kt +++ b/android/app/src/main/java/com/noop/analytics/IntelligenceEngine.kt @@ -993,6 +993,13 @@ object IntelligenceEngine { emptyList() } else -> { + // Reachable for ANY owner now, which widens this past the 5/MG it was built for: + // a WHOOP 4.0 day that banked nothing at all (`grav.size < 2`, no stored night) + // also lands here, where the owner check previously blocked it. A normal 4.0 day + // is untouched — it streams gravity, so it never reaches this gate — and a day + // with no motion has too little of anything to clear `minSleepMin`, but "too + // little" is not "none", so the night it could produce is display-only and + // marked [DetectedSleep.hrOnly] like every other. dayDiag(SleepStagerTrace.hrOnlyGateLine( attempted = true, reason = "no-motion-no-hypnogram", gravRows = grav.size, storedNights = 0,