Repository navigation
sleep: bridge nights fragmented under minSleepMin (#1937) - #1984
Merged
Merged
Conversation
ryanbr
force-pushed
the
sleep-fragmented-night-rescue
branch
2 times, most recently
from
September 7, 2026 23:59
5e9cf18 to
5b7d018
Compare
A night whose sleep runs are all shorter than minSleepMin yields no session at all, however much sleep they add up to. From the report in #1937: sleep-detect summary: sleepRuns=5 droppedMinSleep=5 kept=0 detectedSpanMin=298 survivingSpanMin=0 sparse=false The detector found 298 minutes of sleep across five runs and dropped every one, because each run individually fell about a minute under the 60-minute floor. The bridge that exists to re-stitch fragments was switched off, because it is gated on the night being sparse (#308, so a dense night kept its original path byte-for-byte) and the night was dense: gravity coverage on the reporting device was 99.9%. The rescue was declined precisely because the data was good. So the bridge now also runs when the night is fragmented to nothing. The gate is deliberately "today's answer is zero": if any single run already clears the floor the predicate is false, the sparse rule decides exactly as before, and a night that currently scores cannot change. It can only turn nothing into something. Requiring at least two runs keeps one short fragment from qualifying, and requiring their sum to clear the floor keeps a handful of brief stirs from becoming a night. The bridge's own rules still apply underneath: a gap over sparseBridgeGapMin, an intervening active run that is too long, or HR above the sleep band all still refuse. This enables the attempt, it does not weaken what the attempt checks. The predicate is deliberately not passed to buildRuns, which takes its own sparse flag for the HR-vouched gap rule. That decides how runs are formed, so widening it would change the input to everything downstream including nights that currently score. The E10 trace line now carries both gates as their own k=v keys, sparse= and fragmentedToNothing=, rather than one flag naming the winner. It used to hardcode sparse=true, which since this change could contradict the sparse=false on the summary line beside it. Emitting both keeps the key set identical on every night, which is what a reader diffing two nights, and any k=v parser of the detail field, needs. Swift and Kotlin twins with matching tests on both sides, including an end-to-end one that runs the reported shape (five 59-minute runs, dense gravity, four stirs the bridge must chain through) all the way through detectSleep and asserts both that a session appears and that the rescue is what produced it.
ryanbr
force-pushed
the
sleep-fragmented-night-rescue
branch
from
September 8, 2026 00:11
5b7d018 to
8b7bfe7
Compare
ryanbr
added a commit
that referenced
this pull request
Sep 8, 2026
The end-to-end fixture added in #1984 builds GravitySample and HRSample values, which live in WhoopProtocol, not StrandAnalytics. The file imported only StrandAnalytics, so test (StrandAnalytics) failed to compile with "cannot find type 'GravitySample' in scope". SleepStagerActiveBridgeTests, the sibling fixture file this was modelled on, carries the same import for the same reason. Swift cannot be compiled in the WSL dev environment, so this was invisible locally until CI ran the package leg.
DX23876
added a commit
to DX23876/noop
that referenced
this pull request
Sep 16, 2026
Thirty real conflicts. The Android tree is dropped again per FORK_GUIDE; every other hunk was resolved by asking which mechanism this fork actually runs, not by preferring a side. Taken from upstream: • Protocol: ryanbr#2017/ryanbr#2018/ryanbr#2020 — the 5/MG historical optical and raw-IMU layouts, and the v26 window's ABSOLUTE base code beside its deltas. Its migration lands as `v57-ppg-waveform-base-code` (upstream's v44), since this fork's sequence is nine ahead of theirs. • BLE diagnostics ryanbr#1998-ryanbr#2003: a bond end that says WHY, named BLE integers, a session-budgeted hex dump, and "we hold the link" only when the OS says so. • Battery: ryanbr#1951 stop asking a 5/MG for a reading it refuses; ryanbr#1959 poll 0x2A19 for an unbonded one. • Scoring: ryanbr#1978/ryanbr#1982/ryanbr#1990 the RHR/HRV baseline gates, ryanbr#1977 the strain denominator's domain, RMSSD withheld on an over-counted night, ryanbr#1943 the resting-HR bin gate, the closed final HR/HRV window, HR-recovery requiring one CONTINUOUS effort, ryanbr#1984 nights bridged under minSleepMin, ryanbr#2015 motion re-folded only when that day's gravity moved, ryanbr#2016 a provisional Rest. • Charts ryanbr#2004/ryanbr#2005/ryanbr#2007/ryanbr#2009/ryanbr#2011 and ryanbr#1995's hero-ring detail route, ryanbr#1164's "Pending sync" caption under Rest. • Widgets: the heart-rate widget with its trace (ryanbr#1957/ryanbr#1964) and the widget cost counters. • Health: ryanbr#1503 — the Apple Health external UUID drops the strap id, so a re-pair no longer strands every prior record, plus the one-off sweep that reclaims the stranded ones; ryanbr#1205 GPS routes read back from Apple Health. • Storage: rolling retention for ppgWaveformSample, the skin-temp absolute backfill outside the 21-night window, per-table byte estimates. • Units split (body vs exercise distance), per-day wake overrides, the wind-down nudge pinned to its own day, and the German Sie→du conversion (29 entries, merged per LOCALE so the fork's own de edits stayed). Kept on the fork's side, with the reason: • ryanbr#1005's in-memory `dayScanCache` stays out, again: the persisted `DayScanFingerprint` survives relaunch and its key folds the per-day weight and max HR upstream's does not. • The steps-motion cache stays PERSISTED for the same reason. Upstream's failed-read-vs-empty-day distinction IS adopted into it — on a persisted cache a stale zero from a read error would outlive the process that made it. • Every stream read in the day loop keeps `limit: Int.max`. All five are uncapped together; taking StreamReadCap for three of them would leave the loop half-capped, which is worse than either choice. • The coach engine. Upstream's K1-K15 layer is not adopted: its single 40-row `coachMessage` table, its tool-less pre-baked context and its own brief are behind what this fork already ships. Its ADDITIVE half is kept — the on-device voice input, the Siri "Ask Coach" intent, the brief scheduler and brief widget, and streaming for the OpenAI-compatible providers this fork's Anthropic-only StreamingToolClient does not cover. They are wired up in the next commit; `CoachBriefWidget` is deliberately not registered until then. • Liquid's `DomainTheme` labels, value-sampled Charge tint, `LiquidScoreGauge` and `TodayCardSurface`; classic Today's ring order and `LiveEffort.today`. • RootTabView's four-tab shell and `MoreCatalog`: Coach hangs on content here, not on a tab. Two duplicate mechanisms collapsed rather than carried: • `ChargeBreakdownWiring` (upstream) and `ChargeBreakdownFormat` (fork) are the same extraction done twice. The fork's survives, Coupled now reads it, and upstream's TEST is kept — it is coverage this path never had. • Upstream's `CoachMessageStore` is dropped; `CoachConversationStore` already autosaves the whole multi-conversation history. One defect found while resolving, fixed here: `LiveEffort.today` read HR with no explicit limit, so `hrSamples`' 8000-row default truncated it — at ~18k rows a day that covered about ten hours and the live Effort silently stopped climbing. It failed safe (`effectiveEffort` takes the max), which is why it went unseen. `Live5RestFrozenTests` is removed by upstream, which replaces it with `Live5NoGravityRestChainTests`. `local_day_windows_oracle.json` moves out of upstream's android path into `Tests/StrandAnalyticsTests/Oracles/` — NOT `Fixtures/`, which this repo's .gitignore excludes everywhere. Verified: StrandAnalytics 2233, WhoopStore 627, WhoopProtocol 700, StrandImport 293, StrandDesign 120, NoopLocalAccess 14, SemanticMemory 19, OuraProtocol 209 and PolarProtocol 25 all green; macOS and iOS both build; doc_comment_lint clean; `i18n_audit --ci` byte-identical to its pre-merge output (it is red on main for reasons that predate this, and the seven new widget literals were given all nine locales so the gate did not get worse). Six StrandTests were ALREADY failing on main before this merge and still are — `CoachMyLogsAndZonesTests` (the tool census pin was not bumped when the Hevy tools landed) and five in `RawPhysiologyUnionTests`. Every input to them — CoachTools.swift, ToolConsent.swift, Repository.swift and both test files — is byte-identical to main, so this merge neither caused nor fixed them. They went unnoticed because app-build.yml last ran on 2026-08-25. Analysis migration required: YES. The scoring changes above make stored derived values stale; the recipe bump and its resumable migration are the next commit.
DX23876
added a commit
to DX23876/noop
that referenced
this pull request
Sep 16, 2026
The 2026-09-09 sync moves scored VALUES, not just how they are presented, so a `dailyMetric` row computed under v2 no longer means what the current code would produce from the same inputs. Nine changes do it, each on some subset of nights: the RHR and HRV baseline gates (ryanbr#1978/ryanbr#1982/ryanbr#1990), the strain denominator's domain check (ryanbr#1977), RMSSD withheld on a night whose R-R over-counts, the ryanbr#1943 resting-HR bin gate, the closed final HR/HRV window, heart-rate recovery requiring one CONTINUOUS effort, ryanbr#1984's bridged fragmented nights, and ryanbr#2015's gravity-conditional motion re-fold. So: `Analysis migration required: yes`. `currentAnalysisRecipeVersion` 2 → 3. No new migration machinery — the existing one already has the properties CLAUDE.md asks for. `prepareAnalysisRecipe` reads the cursor, sees `.migrate(2, 3)` and runs `runAnalysisMaintenance`, which is a forced `analyzeRecent(maxDays: 21, allowDayReuse: false)` that marks the recipe cursor ONLY after the pass returns, so an interrupted or failed run re-runs rather than silently claiming the rescore. It re-derives; it deletes no raw samples and no user sleep or workout corrections. 21 nights is the narrowest interval provable rather than a round number: every one of the nine is a per-night derivation, and the engine's own scoring window is 21 nights. Older nights keep their v2 values until re-scored for another reason — the same contract v1 → v2 shipped with. Deliberately NOT in this bump: the skin-temp absolute backfill for nights OUTSIDE that window. It is its own resumable walker, fill-only on a NULL column, reached on purpose from the Test Centre rather than on launch, so folding it in would widen the migration to the whole library for a value the rescore does not compute anyway. The decision tests are extended where they were weakest: the three existing cases are written against `current`, so they stay green THROUGH a bump without ever witnessing one. Two literal pins are added — that a v2 install migrates as `2 → 3`, and that an install already at 3 (or rolled back from a 4) never rescores on relaunch, which is the failure mode using a marketing or build number here would cause and which is invisible until someone's phone spends twenty minutes re-scoring after a cosmetic update. The first pin is MEANT to fail on the next bump: that failure is the prompt to answer the migration question for whatever the bump carries. Verified: StrandTests 2950, 0 failures; all nine packages green; macOS and iOS both build; doc_comment_lint clean. Recorded in docs/fork/decisions.md. Analysis migration required: yes (this commit is it).
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.
Reported in #1937.
What is happening
Staging is not failing. From the report's own diagnostics:
The detector found 298 minutes of sleep across five runs, and dropped every one, because each run individually fell about a minute under the 60-minute
minSleepMinfloor. Five runs averaging 59.6 minutes, so a whole night of real sleep scored nothing.Why the existing rescue declined
bridgeSparseSleepexists precisely to re-stitch runs fragmented by gravity dropouts. It was switched off, because it is gated on the night being sparse (#308 kept a dense night's original path byte-for-byte) and this night was dense: gravity coverage on the reporting device was 99.9%, and 48% on mine.The rescue was declined because the data was good.
The change
The bridge also runs when the night is fragmented to nothing:
The safety property. The gate is deliberately "today's answer is zero". If any single run already clears the floor the predicate is false, the sparse rule decides exactly as before, and a night that currently scores cannot change. It can only turn nothing into something.
Two runs at minimum, because one fragment is not a fragmented night. The sum must clear the floor, so a handful of brief stirs does not become a night. And the bridge's own rules still apply underneath: a gap over
sparseBridgeGapMin, an intervening active run that is too long, or HR above the sleep band all still refuse. This enables the attempt; it does not weaken what the attempt checks. One of the tests pins exactly that, by giving a fragmented night a 10-hour gap and asserting the bridge still declines.The predicate is deliberately not passed to
buildRuns, which takes its ownsparseflag for the HR-vouched gap rule. That decides how runs are formed, so widening it would change the input to everything downstream, including nights that currently score. This only re-stitches runs that are already built, and only when every one of them was about to be discarded.Diagnostics
The E10 trace line now carries both gates as their own
k=vkeys (sparse=false fragmentedToNothing=true), rather than one flag naming the winner. It used to hardcodesparse=true, which since this change could print next to a summary line sayingsparse=false: the kind of quiet disagreement that costs a reader an hour. Emitting both keeps the key set identical on every night, which is what a reader diffing two nights, and anyk=vparser of the detail field, needs. Both platforms pin the rendered fragmentsparse=false fragmentedToNothing=true gapMin=so the two languages cannot drift apart in silence.Tests
Swift and Kotlin twins covering the reported five-fragment night, the safety property (a night with one qualifying run is untouched), brief stirs declining, a single short run declining, and the bridge's own gap rule still refusing.
Plus an end-to-end test on both platforms, which is the only one that proves the change does anything: it runs the reported shape (five 59-minute sleep runs, continuous gravity so the night is dense, four stirs the bridge has to chain through) all the way through
detectSleep, and asserts both that a session now appears and, via the trace, that the rescue is what produced it.That last assertion exists because the first version of this test passed for the wrong reason. With 3-minute stirs,
mergePeriods(which absorbs any run undermergeMin= 15 and rejoins the same-stage neighbours) swallowed them, so the night arrived as one 306-minute run that always scored:sleepRuns=1 kept=1. The fixture was never fragmented and the code under test never ran. The stir is now 20 minutes, which is abovemergeMinand belowsparseBridgeActiveMaxInBandMin, and the anti-vacuity assertion makes that failure mode impossible to reintroduce silently.Android suite green locally: 666 classes, 5625 tests, 0 failures. Swift is CI-verified.
The two existing "dense night has no sparse-bridge line" trace tests still pass, but their names and comments asserted density as the reason. Since a dense night can now open the bridge, they are repinned to the real invariant: this night is dense and its single run clears the floor, so neither gate opens.