diff --git a/Strand/Data/IntelligenceEngine.swift b/Strand/Data/IntelligenceEngine.swift index 9de476ebcf..495e6e43ca 100644 --- a/Strand/Data/IntelligenceEngine.swift +++ b/Strand/Data/IntelligenceEngine.swift @@ -1167,45 +1167,48 @@ 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 { + // 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)) + 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..24bf3e974a 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,60 @@ 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 -> { + // 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 = 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. */