Repository navigation
Preserve legacy WHOOP 5 HRV and Recovery across re-score - #2115
Conversation
|
Thanks @bhelm, and thanks for the trace on #2101: you landed on the same commit and the same mechanism I did coming at it from a separate report, which is worth stating because two independent paths to Three wearers are on this now. @rafaelphh here, @ItsMeDrath in the thread above, and a third via Discord whose report I wrote up as #2117 before finding this one: HRV and respiratory rate blank, heart-rate-derived values fine, restored by downgrading to 11.5. Same signature in all three. #2117 is the duplicate and I will point it here. The approach is rightPreserving the previously computed pair rather than reinterpreting ambiguous rows is the correct instinct. Those units genuinely cannot be trusted, and scoring them would replace missing data with WRONG data, which is worse than a gap. The boundary is well chosen too: a labelled transport ends the protection even when it is too sparse to yield HRV, because that is a real answer rather than an absence. Blocking: the Swift tests do not compile
Both are Worth knowing that it means the new assertions in that file had never executed anywhere, on any machine. I applied the two arguments: [self.id] // was [id], lines 145 and 151If it helps for future passes: The parity-governance failure is not yoursThat one is a stale checked-in baseline, and it predates this branch. It drifted from #2105 and from merges earlier today, and your PR only triggers it because it adds a file under It is a fair illustration of the refresh problem I raised on #1534: the baseline has now gone stale three times in a day from ordinary product work, and the guarded refresh cannot repair it, since Two things before this mergesThe PR body opens with a closing keyword against the issue number. This repo deliberately avoids that pattern, so it wants rewording, otherwise the merge auto-closes the report rather than leaving the maintainer to judge when it is actually settled. Worth knowing the same applies in comments: my first draft of this one quoted your line verbatim and the pre-post check stopped it, because GitHub would have acted on the quote too. Eleven of the twenty-one files are unrelated. The fix is six source files and four test files. The rest is On the untested boundaryYour "not yet tested" section is the right way to write one, and the honesty is appreciated. The part that still needs a human is a real One thing to flag for him explicitly: values already cleared by an 11.6.0 re-score cannot be reconstructed by this change. Anyone who has already upgraded and lost them needs an older backup or a fresh labelled offload. That is worth saying plainly in the release notes when this ships, because "update and it comes back" is the reasonable assumption and it is not what happens. |
`test (WhoopStore)` was red on a compile error rather than a logic failure: `arguments: [id]` inside
`dbWriter.write { db in ... }` references the class's `private let id`, which an escaping closure has
to spell as `self.id`.
The Linux environment this branch was developed in cannot compile the Swift tests, so the file had
never been through a compiler anywhere. With the two additions the whole package runs: 535 tests, 0
failures, including the new legacy-withheld coverage in this file.
docs/BUILD.md has a snapshot-enabled SQLite recipe that makes WhoopStore, StrandAnalytics,
StrandImport and NoopLocalAccess build AND test on Linux, which is what this was verified with.
|
I pushed the two-line compile fix to this branch ( The one ask before this mergesCould you split the Eleven of the twenty-one files here are that scaffolding, including a repo-level What is left after the split is the actual change, and it is proportionate: six source files and four test files for a cross-platform fix with real coverage. On the approach, since it came upThe maintainer asked whether, this being a regression, the simpler answer is to re-implement rather than layer preservation on top. Having looked, I do not think so, and it is worth writing down so nobody re-opens it later:
So preserving the previously computed pair, inventing nothing, and stopping the moment a labelled transport arrives is the conservative option rather than a workaround. That is worth keeping in the PR description, because the next reader will have the same instinct. One thing for the release notesValues already cleared by an 11.6.0 re-score cannot be reconstructed by this, as your body says. That deserves to be stated plainly when it ships, because "update and it comes back" is the natural assumption and it is not what happens. Anyone already affected needs an older backup or a fresh labelled offload. |
Maintainer commit, not the PR author's work: these are generated files and the branch's environment cannot produce them, so they are regenerated here rather than asked for. The gate was red for two reasons, neither of them this branch's doing and only one of them fixable on main. The checked-in snapshots were stale before this branch existed, which main has since corrected. And the authority pins an exact count and hash of the whole inventory, so the three functions this branch adds (legacyWhoop5RRWithheld and its two Kotlin twins) invalidate it again no matter how correct they are. Derived from a pristine clone of this branch merged with main, because the scanner walks the filesystem rather than the index and local .build directories are counted. 486 files, 4308 functions, 151 twin pairs, 295 findings. Findings are UNCHANGED from main: the new declarations are reached from production, so none of them lands as test-only debt. Verified in that clone: 105 of 105.
|
@ryanbr I can validate this using the same setup as #2101. Current state:
Please let me know the exact build and validation steps you want me to use. I can provide before/after screenshots and diagnostic logs if needed. |
…2141) The exact-window legacy WHOOP 5 snapshot now preserves the remaining stored nightly values that are exclusively derived from R-R: respiratory rate and SDNN. They use the same guard as HRV and Recovery and are released by the same labelled-transport boundary, including when that transport is too sparse to score. No R-R rows are converted or relabelled. Respiratory rate was the specific gap #2115 left open and #2123 tracked, named independently by two reporters. Also removes the repository scaffolding that #2115 carried in by accident: three okf/ files and eight openspec/ files, including a German archived change and a published spec. Verified referenced by no workflow, tool, doc or CONTRIBUTING entry. Reviewed on a checkout rather than from the description. The load-bearing claim holds: walking DailyMetric, avgHrv and recovery were already covered, respRateBpm and avgSdnn are the two added here, and every remaining field traces to HR, optical, thermistor, accelerometer or the hypnogram. Swift and Kotlin move together under one guard, so the four cells cannot drift apart. One nuance recorded on the PR rather than blocking it: respRateBpm is not always R-R-derived, since AnalyticsEngine prefers a device-measured rate where one exists. It is safe inside this gate because a WHOOP 5 carries no raw respiration ADC, so RSA is the only producer, but that is a device fact stated elsewhere holding up the claim. The parity-governance failure on this PR was inherited, not caused: the same authority drift failed on main and at 0b0d206 before any of it. Re-derived separately, and this is green on the updated base.
…rtialBaseline The 260921-0740 log, read through the ledgers 18.16 shipped, showed the Charge flip's real mechanism. The 260906 abort gate stops a full pass at night N, and everything after it kept running AS a full pass. Two of those steps are window-wide: the stale eviction deletes every computed row in [oldestDay, newestDay] the pass did not reproduce, and persistComputedScores wide-deletes metricSeries + provenance over the same span. oldestDay is the whole 21-day window regardless of where the scan stopped, so a pass abandoned after night 1 deleted the other twenty computed days — four times in one morning (`ABANDONED after 1/21` x4). Today was then written nil (sub-seed baseline), and with Sep 12-20 gone the Today/widget anchor fell back to the oldest scored row OUTSIDE the window, immune to eviction: 18 August, Charge 36. The next foreground pass restored the window and today read 62. Every abandoned background pass repeated it. The 260902 light-pass guard describes this exact failure and keyed it on `lightPass`; the abort gate opened a second route the flag cannot see. Every post-scan gate now reads `partialBaseline` (light OR abandoned): persist window bounded to the scanned days, rest points and weekly stages skipped, eviction not run. A partial pass rewrites what it produced and nothing else. Nothing changes for a completed pass or a light pass. A row ryanbr#2115 preserves — a night whose R-R the WHOOP 5 unit policy (ryanbr#2046) withholds — has no raw to re-derive from; evicted once, it is gone. That is how Aug 19-Sep 11 lost their HRV/Charge: the fork ran v11.6.0 (ryanbr#2046, no ryanbr#2115) from Sep 11 to 19 and each full pass overwrote them with nil; the eviction then removed any survivors. Not relabelled here, by instruction; the window heals as it slides. Two ledgers, always-on, counts only: - `evict: removed N stale computed day(s)` — only when N > 0. A delete of computed history is never silent. - `legacyScores preserved=X hrvMissingWithSleep=Y of N` — per completed pass. ryanbr#2115 had no log line; this is the number that must only ever fall. - `chargePublish` gains `battery=`, for the widget report. Tests: AbandonedPassBaselineTests grows to 9 — the eviction predicate and the persist window, both directions. Full 2488-test macOS suite: only the two pre-existing failures. Verified: Strand (macOS) + NOOPiOS build. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
3 participants
Summary
Addresses the regression reported in #2101.
NOOP 11.6.0 correctly stops scoring ambiguous, unlabelled WHOOP 5 R-R rows, but the forced re-score then replaced previously computed HRV and Recovery with null. This change preserves an existing HRV/Recovery pair as an opaque legacy snapshot only when the exact scoring window contains valid unlabelled legacy R-R rows and no scorable labelled transport.
All other daily fields remain freshly computed. A labelled scoring transport ends the protection, including when the labelled data is too sparse to produce HRV. WHOOP 4, other brands, ordinary no-RR nights, quarantined rows, and the general upsert semantics are unchanged. No old R-R rows are converted or relabelled.
Verification
Not yet tested
This has not been tested with a real user-created v11.5
.noopbakfile or on physical WHOOP 5 hardware. The current Linux environment also cannot compile or run the Swift app/tests; CI/macOS must provide that evidence. The Android behavior and cross-platform source contract are locally green.Users who have not yet upgraded from 11.5 should retain their existing computed HRV/Recovery when moving directly to a build containing this fix. Values already cleared by a 11.6.0 re-score cannot be reconstructed without an older backup or a fresh labelled strap offload.