Repository navigation
fix(oura): heal banked-sleep duplicates across every device, not just computedId (#1248) - #1249
Merged
Merged
Conversation
… computedId (#1248) The #899 overlap heal read/deduped/deleted sleep sessions only under computedId. A live source (an Oura ring) banks its OWN hypnogram under its OWN device id, so those rows were never collapsed — and, worse, re-read as providedSleep and re-detected every pass, so one night ballooned to 14 stored rows / 5 distinct hypnograms and rendered 9 overlapping 'naps' (NAP(S) 43h52m, matched=9). Sweep the heal over the de-duplicated union of computedId + every registered device id, deduping each device's rows AMONG THEMSELVES and deleting stale copies under that SAME id (never across ids, so a survivor is never orphaned under an id the day-owner read skips; Kotlin deletes under the row's own deviceId, Swift under the explicit heal id). Safe by the dedup's own invariant (two real sleeps never overlap; userEdited rows are exempt) and window-bounded to the days this pass rescored. Pure healDeviceIds formatter for the scope, byte-identical Swift + Kotlin, with a matched test each side pinning that the ring id is now in scope. Blast radius: the heal now also collapses overlapping duplicates under raw/import device ids (previously only computed) — an improvement under the same overlap rule. Minor perf: N small sleepSession reads instead of one (N = device count, ~1-3). Independent of #1246 (the 0xFF-padding decode), per the reporter. Co-Authored-By: ryanbr <mp3geek@gmail.com>
5 tasks done
ryanbr
added a commit
that referenced
this pull request
Aug 12, 2026
…ual 3) (#1297) * sleep(#1210): shadow-instrument the dormant band wake-veto (output-neutral) Follow-up to #1210 / #738. The band sleep_state wake-veto is landed but default-off, so we have no measurement of what it WOULD do on real banded nights - the exact recovered-vs-reverse ratio #1210 says is still too thin (12 nights). This adds an OUTPUT-NEUTRAL shadow: when a band stream is present, the flag is off, and a diagnostic traceSink is collecting, scoring computes what the veto would recover and emits one namespaced trace line per banded night: bandVeto(shadow): startTs=<unix> recoveredMin=<n> eff <raw>%-> <shadow>% Nothing changes in the persisted hypnogram or efficiency - the shadow stages are computed for the log only; the flag-gated (unchanged) stages are still what is stored and displayed. Guarded on traceSink, so zero cost in normal operation; it fires only while a diagnostic is being generated, which re-scores the recent window and thus reports the delta for every recent banded night. Honest scope: the line reports MAGNITUDE and FREQUENCY (how often the band disputes our interior wake, and by how much), which is what #1210 item 1 asks to widen. It does NOT settle the SIGN question - whether that move is toward truth. The PSG harness currently measures the recipe UNDER-calling wake overall (bias -4.92 pp), so the flip decision still needs the truth comparison, not just the band. This gathers the band side of that evidence across wearers without any visible change. Retire on the flip. Byte-identical Swift (StrandAnalytics) + Kotlin analytics; the shadow line carries no sleep day= / gate run= / day= token so CaptureAccumulator never counts it as a captured day (pinned by a test both platforms). Plus an end-to-end test that a banded night with interior false-wake emits the line while the returned hypnogram stays byte-identical to the untraced call. No app-target Swift; swift-packages + android unit tests cover it. * sleep(#1210 item 2): add the new-nights-only cutoff to the band wake-veto Complements the shadow instrumentation with the SAFE half of option B: a dormant bandStateWakeVetoCutoffTs (0 = every night). When the veto is eventually flipped on, a non-zero cutoff (a unix second) restricts the correction to sessions whose start is at or after it, so nights already in history keep their raw efficiency - flipping the default can never silently re-score months of banked nights upward. Inclusive boundary. This resolves #1210 item 2 (the retroactive-rescore decision) by building the gate-to-new-nights path, without flipping anything: the flag stays false, so the cutoff is inert. The eventual flip is then a dated one-liner (set enabled = true AND cutoffTs = the flip date) with history provably untouched. The shadow (previous commit) deliberately passes cutoffTs = 0 so it still measures the FULL veto potential across every banded night, history included, for the validation distribution - independent of whatever cutoff the flip later uses. Byte-identical Swift + Kotlin; pinned by a test both platforms (a night before the cutoff is unchanged, one at/after it gets the veto). No app-target Swift. * sleep: fix stale bandStateWakeVetoEnabled doc (Kotlin) - it is default-OFF The Kotlin flag doc read 'Default-ON gate ... Flip to false', contradicting both the actual value (false) and the Swift twin it claims to mirror (default-off, flip to true only with PSG evidence, -4.92 pp under-call). A maintainer reading the Kotlin side would misjudge the shipped state. Align it to reality + the Swift source of truth; note the new-nights cutoff to set alongside the flip. Doc-only, no behaviour change. * oura(sleep): clamp the trailing-0xFF floor to >= 2 so a lone byte can't be eaten (#1284) Hardening surfaced by @pipiche38's six-night hardware validation of #1293. The trailing-run unwritten rule keys off minTrailingUnwritten (shipped 6). Nothing stopped that constant going below 2, and at 1 a LONE trailing 0xFF byte - four genuine awake epochs, exactly the case #1246 was careful to protect - would be flagged unwritten and dropped. The existing FloorIsExclusive test asserts minTrailingUnwritten*4, so it scaled with the constant and would have stayed green. Clamp the effective floor to max(2, minTrailingUnwritten) in both decoders. The whole-record rule already fires only at codeCount >= 2, so this just holds the trailing rule to the same minimum: the constant stays freely tunable upward (the validation confirms 6 is right), but can never drop low enough to eat a single real-wake byte. Shipped behaviour is unchanged (max(2,6) = 6). Byte-identical Swift (OuraProtocol) + Kotlin (com.noop.oura), each with a new test pinning 'a written record ending in a single trailing 0xFF stays awake' plus a floor >= 2 tripwire. Swift 37/37 + Android 39/39 green locally. No app-target Swift. Addresses the hardening item in #1284; residuals 2 (banked-night heal) and 3 (0x49 session keying) remain open. * oura(sleep): log the duplicate-session GENERATION moment (#1284 residual 3) @pipiche38's #1284 shows a night still mints a second row - a short session banked by an early partial drain, nested inside the real one banked by the later full burst - which the #899 heal only cleans up afterward. From the strap log alone the GENERATION is invisible; #1249 fixed the heal, not the cause. Instrument-first (the cause is a session-identity change in hardware-gated BLE persist code that needs a strap to validate): track each sleep session's [start,end] banked this connection, and when a new persist lands wholly inside - or overlapping - an already-banked one, log it with the 0x49 anchor state: Oura: duplicate-gen(#1284) new session [S -> E] NESTED IN already-banked [s -> e] this connection - NO 0x49 anchor (write-envelope end); PK=(deviceId,startTs) mints a 2nd row A persist with NO 0x49 anchor is the prime suspect for the short row, so the line names it. Diagnostic ONLY - nothing is skipped, no BLE/connection/write path changes; the fix is the 0x49-window session keying pipiche recommends, which this pins on the next capture. Byte-parity Swift (Strand/BLE, app-target) + Kotlin (com.noop.ble); bounded in-memory list, reset per connection with the 0x49 windows. Android compiles; Swift app-target verified via the app-build gate. Residual 1 done+hardened (#1293/#1296); residual 2 (banked heal) open. * oura(sleep): stamp the 0x49 anchor state on every persist line (#1284 residual 3) The duplicate-gen line fires on the SECOND (real) persist, but the key evidence is that the FIRST (short) row was banked WITHOUT a 0x49 anchor - and that first persist has nothing prior to overlap, so it never trips the duplicate-gen line. Stamp [0x49-anchored] / [NO-0x49-anchor] on every 'sleep session persisted' line so the unanchored early-drain row is self-evident on its own, making the whole generation sequence legible from the strap log alone. Byte-parity Swift + Kotlin; diagnostic only. * oura(sleep): use sleepStart (not w) for the Kotlin duplicate-gen anchor label (#1284) Re-review parity fix. The Kotlin duplicate-gen line labelled the anchor off the 0x49 window match (w != null) while the Swift twin and both persist-tag lines use the 0x49 ONSET (sleepStart). Three problems: the two platforms emitted different text for the same scenario; the window-match is the wrong signal for a startTs-PK duplicate (a persist can match the window yet leave the START unanchored when the onset fails its plausibility clamp, exactly the duplicate-prone case); and the same persist could read one anchor label on the duplicate-gen line and the opposite on the persist tag. Use sleepStart on both, byte-identical to Swift and consistent with the persist tag. Kotlin-only (Swift was already correct); Android compiles. * oura(sleep): label the persist anchor by 0x49 ONSET, not generic anchor (#1284) Re-review clarity fix. The tag said 0x49-anchored / NO-0x49-anchor, keyed on sleepStart (the 0x49 ONSET). But a burst can have its END 0x49-refined while the onset fails its plausibility clamp, so the old NO-0x49-anchor tag could sit right under the existing 'hypnogram burst end refined by 0x49' line and read as a contradiction. The tag measures the ONSET (the start anchor that governs the startTs PK), so name it that: 0x49-onset / no-0x49-onset, and the duplicate-gen line likewise. Removes the ambiguity for the reader. Byte-parity Swift + Kotlin; diagnostic only; Android compiles.
simoncad7
pushed a commit
to simoncad7/noop
that referenced
this pull request
Aug 17, 2026
…residual 3) (ryanbr#1297) * sleep(ryanbr#1210): shadow-instrument the dormant band wake-veto (output-neutral) Follow-up to ryanbr#1210 / ryanbr#738. The band sleep_state wake-veto is landed but default-off, so we have no measurement of what it WOULD do on real banded nights - the exact recovered-vs-reverse ratio ryanbr#1210 says is still too thin (12 nights). This adds an OUTPUT-NEUTRAL shadow: when a band stream is present, the flag is off, and a diagnostic traceSink is collecting, scoring computes what the veto would recover and emits one namespaced trace line per banded night: bandVeto(shadow): startTs=<unix> recoveredMin=<n> eff <raw>%-> <shadow>% Nothing changes in the persisted hypnogram or efficiency - the shadow stages are computed for the log only; the flag-gated (unchanged) stages are still what is stored and displayed. Guarded on traceSink, so zero cost in normal operation; it fires only while a diagnostic is being generated, which re-scores the recent window and thus reports the delta for every recent banded night. Honest scope: the line reports MAGNITUDE and FREQUENCY (how often the band disputes our interior wake, and by how much), which is what ryanbr#1210 item 1 asks to widen. It does NOT settle the SIGN question - whether that move is toward truth. The PSG harness currently measures the recipe UNDER-calling wake overall (bias -4.92 pp), so the flip decision still needs the truth comparison, not just the band. This gathers the band side of that evidence across wearers without any visible change. Retire on the flip. Byte-identical Swift (StrandAnalytics) + Kotlin analytics; the shadow line carries no sleep day= / gate run= / day= token so CaptureAccumulator never counts it as a captured day (pinned by a test both platforms). Plus an end-to-end test that a banded night with interior false-wake emits the line while the returned hypnogram stays byte-identical to the untraced call. No app-target Swift; swift-packages + android unit tests cover it. * sleep(ryanbr#1210 item 2): add the new-nights-only cutoff to the band wake-veto Complements the shadow instrumentation with the SAFE half of option B: a dormant bandStateWakeVetoCutoffTs (0 = every night). When the veto is eventually flipped on, a non-zero cutoff (a unix second) restricts the correction to sessions whose start is at or after it, so nights already in history keep their raw efficiency - flipping the default can never silently re-score months of banked nights upward. Inclusive boundary. This resolves ryanbr#1210 item 2 (the retroactive-rescore decision) by building the gate-to-new-nights path, without flipping anything: the flag stays false, so the cutoff is inert. The eventual flip is then a dated one-liner (set enabled = true AND cutoffTs = the flip date) with history provably untouched. The shadow (previous commit) deliberately passes cutoffTs = 0 so it still measures the FULL veto potential across every banded night, history included, for the validation distribution - independent of whatever cutoff the flip later uses. Byte-identical Swift + Kotlin; pinned by a test both platforms (a night before the cutoff is unchanged, one at/after it gets the veto). No app-target Swift. * sleep: fix stale bandStateWakeVetoEnabled doc (Kotlin) - it is default-OFF The Kotlin flag doc read 'Default-ON gate ... Flip to false', contradicting both the actual value (false) and the Swift twin it claims to mirror (default-off, flip to true only with PSG evidence, -4.92 pp under-call). A maintainer reading the Kotlin side would misjudge the shipped state. Align it to reality + the Swift source of truth; note the new-nights cutoff to set alongside the flip. Doc-only, no behaviour change. * oura(sleep): clamp the trailing-0xFF floor to >= 2 so a lone byte can't be eaten (ryanbr#1284) Hardening surfaced by @pipiche38's six-night hardware validation of ryanbr#1293. The trailing-run unwritten rule keys off minTrailingUnwritten (shipped 6). Nothing stopped that constant going below 2, and at 1 a LONE trailing 0xFF byte - four genuine awake epochs, exactly the case ryanbr#1246 was careful to protect - would be flagged unwritten and dropped. The existing FloorIsExclusive test asserts minTrailingUnwritten*4, so it scaled with the constant and would have stayed green. Clamp the effective floor to max(2, minTrailingUnwritten) in both decoders. The whole-record rule already fires only at codeCount >= 2, so this just holds the trailing rule to the same minimum: the constant stays freely tunable upward (the validation confirms 6 is right), but can never drop low enough to eat a single real-wake byte. Shipped behaviour is unchanged (max(2,6) = 6). Byte-identical Swift (OuraProtocol) + Kotlin (com.noop.oura), each with a new test pinning 'a written record ending in a single trailing 0xFF stays awake' plus a floor >= 2 tripwire. Swift 37/37 + Android 39/39 green locally. No app-target Swift. Addresses the hardening item in ryanbr#1284; residuals 2 (banked-night heal) and 3 (0x49 session keying) remain open. * oura(sleep): log the duplicate-session GENERATION moment (ryanbr#1284 residual 3) @pipiche38's ryanbr#1284 shows a night still mints a second row - a short session banked by an early partial drain, nested inside the real one banked by the later full burst - which the ryanbr#899 heal only cleans up afterward. From the strap log alone the GENERATION is invisible; ryanbr#1249 fixed the heal, not the cause. Instrument-first (the cause is a session-identity change in hardware-gated BLE persist code that needs a strap to validate): track each sleep session's [start,end] banked this connection, and when a new persist lands wholly inside - or overlapping - an already-banked one, log it with the 0x49 anchor state: Oura: duplicate-gen(ryanbr#1284) new session [S -> E] NESTED IN already-banked [s -> e] this connection - NO 0x49 anchor (write-envelope end); PK=(deviceId,startTs) mints a 2nd row A persist with NO 0x49 anchor is the prime suspect for the short row, so the line names it. Diagnostic ONLY - nothing is skipped, no BLE/connection/write path changes; the fix is the 0x49-window session keying pipiche recommends, which this pins on the next capture. Byte-parity Swift (Strand/BLE, app-target) + Kotlin (com.noop.ble); bounded in-memory list, reset per connection with the 0x49 windows. Android compiles; Swift app-target verified via the app-build gate. Residual 1 done+hardened (ryanbr#1293/ryanbr#1296); residual 2 (banked heal) open. * oura(sleep): stamp the 0x49 anchor state on every persist line (ryanbr#1284 residual 3) The duplicate-gen line fires on the SECOND (real) persist, but the key evidence is that the FIRST (short) row was banked WITHOUT a 0x49 anchor - and that first persist has nothing prior to overlap, so it never trips the duplicate-gen line. Stamp [0x49-anchored] / [NO-0x49-anchor] on every 'sleep session persisted' line so the unanchored early-drain row is self-evident on its own, making the whole generation sequence legible from the strap log alone. Byte-parity Swift + Kotlin; diagnostic only. * oura(sleep): use sleepStart (not w) for the Kotlin duplicate-gen anchor label (ryanbr#1284) Re-review parity fix. The Kotlin duplicate-gen line labelled the anchor off the 0x49 window match (w != null) while the Swift twin and both persist-tag lines use the 0x49 ONSET (sleepStart). Three problems: the two platforms emitted different text for the same scenario; the window-match is the wrong signal for a startTs-PK duplicate (a persist can match the window yet leave the START unanchored when the onset fails its plausibility clamp, exactly the duplicate-prone case); and the same persist could read one anchor label on the duplicate-gen line and the opposite on the persist tag. Use sleepStart on both, byte-identical to Swift and consistent with the persist tag. Kotlin-only (Swift was already correct); Android compiles. * oura(sleep): label the persist anchor by 0x49 ONSET, not generic anchor (ryanbr#1284) Re-review clarity fix. The tag said 0x49-anchored / NO-0x49-anchor, keyed on sleepStart (the 0x49 ONSET). But a burst can have its END 0x49-refined while the onset fails its plausibility clamp, so the old NO-0x49-anchor tag could sit right under the existing 'hypnogram burst end refined by 0x49' line and read as a contradiction. The tag measures the ONSET (the start anchor that governs the startTs PK), so name it that: 0x49-onset / no-0x49-onset, and the duplicate-gen line likewise. Removes the ambiguity for the reader. Byte-parity Swift + Kotlin; diagnostic only; Android compiles.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug (#1248)
The
#899overlap heal inanalyzeRecentreads/dedupes/deletes banked sleep sessions only undercomputedId(IntelligenceEngine.swift/.kt). But a live source — an Oura ring — banks its own hypnogram under its own device id. So those rows were never collapsed, and because the ring's hypnogram is re-read asprovidedSleepand re-detected on every pass, one night ballooned to 14 stored rows / 5 distinct hypnograms, rendering 9 overlapping "naps" (NAP(S) 43h52m,matched=9). Verified againstmain.The fix
Sweep the heal over the de-duplicated union of
computedId+ every registered device id, deduping each device's rows among themselves and deleting stale copies under that same id:deviceId, Swift under the explicit heal id);SleepSessionDedupcollapse rule verbatim;providedSleep, detects one), and the existing#899-Are-arm reconciles in one forced re-pass.Safety
userEditedrows are exempt, and it's window-bounded to the days this pass rescored. Safe by the dedup's own documented invariant — two real sleeps never overlap.sleepSessionreads instead of one (N = device count, typically 1–3).Tests
Pure
healDeviceIdsscope formatter, byte-identical Swift + Kotlin, with a matched test each side pinning that the ring id is now in scope (the exact regression) + dedup/sort determinism. Android test run locally: green; Swift validated via the app-build gate. The collapse itself is already covered bySleepSessionDedupTest. (FullanalyzeRecentcan't be constructed in a unit context on either platform — the existing heal tests model the logic — so the scope helper is the testable crux.)Scope
Independent of #1246 (the
0xFF-padding decode) per the reporter — the wake block is byte-identical across all copies, so decode and dedup don't interact. Oura is experimental-gated, but the defect lives inmain's shared engine.