docs: add a validation protocol for scoring changes - #988
Open
vishk23 wants to merge 192 commits into
Open
Conversation
Off-by-default OAuth (BYO Oura app, authorization-code) one-time pull of the full Oura API v2 surface. Lossless raw archive (new ouraRaw table, migration v19) plus normalized projection into existing WhoopStore tables, incl. the sleep_phase_5_min hypnogram the file-import lane can't carry. Networking lives in the app target beside AICoach; StrandImport gains only pure API parsers and stays offline-pure. New DataSourceKind.ouraApi; honest-data doctrine preserved.
… store) TDD task breakdown for the network-free core: ouraRaw archive (migration v19) + OuraRawStore, the OuraHypnogram decoder, and the pure OuraApiParser (sleep/daily/events) reusing OuraExportParser semantics. Additive only.
Rebased target is now ryanbr/noop v8.5.2 (NoopApp/noop upstream went private). Study of v8.5.2 vs the v7.2.3-era design, verified against code: - ouraRaw migration is v24 (migrator is at v23, not v18); v19 already taken. - Provenance uses existing PairedDevice.sourceKind .cloudImport (non-day-owning via IntelligenceEngine:1369) — no new SourceKind; deviceId stays 'oura-api'. - motionJSON goes through persistSessionMotion(), not the sleep upsert. - Add oura-api to Repository.wearableImportSources; disconnect via store actor. - Citation shifts (parseISOWithOffset -> CSVParsing:509). Foundation design (models, 5 upsert APIs, tables, parsers) confirmed intact.
…ement, in-sleep HR
…m (Task 3 review)
…pt reference-only
Keychain token store, OuraCredentials (xcconfig->Info.plist), OAuth2 authorization-code (BYO Oura app) behind an AuthProvider seam, and the URLSession client (next_token paging, 429 backoff, 401 refresh, sandbox/prod). Networking confined to Strand/Oura/; packages stay network-free. Tested via xcodebuild StrandTests with a URLProtocol stub + pure builders/parsers.
… test was broken) The app PRODUCT_NAME is 'NOOP Staging' but the StrandTests TEST_HOST/BUNDLE_LOADER still pointed at NOOP.app/NOOP, so xcodebuild test failed with 'Could not find test host' for the whole macOS scheme. CI only runs build (never test) so it went unnoticed.
getWithRetry's 401 branch called validAccessToken(), which only refreshes when clock-expired -- a 401 for a revoked/invalidated-but-unexpired token re-served the same rejected token and the retry failed silently. Add refreshedAccessToken() to AuthProvider for an unconditional refresh; validAccessToken() now delegates to it on expiry. Cover both the 401 force-refresh path and 429 exhaustion with tests.
Wires the OURA_CLIENT_ID/SECRET/REDIRECT_URI Info.plist keys (read by OuraCredentials.fromBundle) and the noop:// CFBundleURLTypes scheme into both the Strand (macOS) and NOOPiOS targets via a new untracked Strand/Oura/OuraSecrets.xcconfig (gitignored; committed template at OuraSecrets.example.xcconfig). Also regenerates Strand/Resources/Info.plist and StrandiOS/Resources/Info.plist via xcodegen generate — both targets set GENERATE_INFOPLIST_FILE: NO, so xcodegen pre-materializes these tracked plists from project.yml's info.properties (same convention as the prior "regenerate Info.plist to match project.yml" commits). Escapes the redirect URI's "//" with the empty-macro $() trick in both xcconfigs: .xcconfig treats // as a start-of-comment anywhere on the line (not just at line-start), so a literal noop://oura/callback silently truncates to noop: at build time and would break the redirect_uri match against the app registered at the Oura developer portal. Verified: xcodegen generate + macOS build-for-testing succeeds, and the built app's Info.plist resolves all four keys correctly (not left as literal $(OURA_*)), including the full noop://oura/callback string. NOOPiOS gets the same wiring but isn't build-verified in this environment.
…ing base64 secrets)
OuraSyncWriter (persist backfill to WhoopStore as .cloudImport, honest-data + coalesce + persistSessionMotion + hrSample + raw archive), OuraSyncCoordinator (one-time backfill across all endpoints, readiness-RHR precedence), deleteOuraRaw for clean disconnect, the Connect Oura card + presentation-anchor helper, scoring source + docs. Verified against v8.5.2 signatures.
…nvariant) + harden test OuraSyncWriter.persist registered the Oura PairedDevice with status: .active, creating a second active row alongside the live WHOOP. DeviceRegistryStore.add does not enforce the at-most-one-active invariant (only setActive() does), and activeDeviceId() feeds day-owner priority-0 plus BLEManager's live-sample deviceId routing, so this was a real data-integrity violation. Every other PairedDevice construction in the app already uses .paired; an import source must never be the active device. Also hardens OuraSyncWriterTests: exact hrSamples count (2, not just >0), asserts the registered source is .paired, and reads the day back via dailyMetrics() to prove recovery/strain stay nil (honest data).
…nts, coalesced days
…se (Task 2 review)
…isconnect (Task 4 review)
… on disconnect; ouraRaw doc accuracy (final review)
Two liters repos exist and only one is pinned. `vishk23/liters-mobile` is what Cargo.toml points at; `vishk23/liters` is the working clone, and it is the only place `fix(replica): bound and cancel the replica-file lock instead of blocking forever` lives. That commit has not been merged to liters-mobile main, so the F_SETLKW fix is NOT in the archive this directory builds, contrary to what a reader would reasonably assume from the repo description. Also records the SQLITE_CANTOPEN gotcha found while verifying the linkage: liters passes 80/80 of its own tests with bundled SQLite and 31/80 with the system SQLite NOOP actually links, every failure the same error, all on the Replica restore path, all from the crate's single read-only open (replica.rs:648). Reduced to a four-case C program against Apple's libsqlite3: a WAL database whose -shm is absent cannot run a statement when opened SQLITE_OPEN_READONLY. Both are Replica-side and so cannot affect the phone, which is a Writer and never opens read-only. Written down because "the linkage is verified at the symbol level" is not the same claim as "the behaviour is verified", and the difference was invisible until someone ran the suite in the shipping configuration.
uniffi's Swift output ends `// swiftlint:enable all` with no trailing newline, so `cat file; echo "#endif"` appended the directive to that comment line, where the comment ate it. The wrapper's `#if LITERS` was therefore never closed and every build with liters enabled died at EOF with "expected #else or #endif at end of conditional compilation block" — an error that points at the last line of a 4,459-line generated file and says nothing about the comment that caused it. `printf '\n#endif\n'` instead. Caught by the iOS simulator build, which is the first thing that actually compiled the file.
… draws Running the round trip for real answered a question the symbol-level check could not. 5 tests, all green: testWriterPushesCommittedContent passed testASecondPushIsIncrementalAndNotASnapshot passed testWriterReopenResumesAgainstTheSameBucket passed testReplicaRestoreIsBrokenUnderSystemSQLite passed testOpeningAMissingDatabaseThrows passed The first version of this file asserted a writer->replica->rows round trip in one test and 4 of 5 failed, every one of them on `replica.sync()` and every one with `sqlite: unable to open database file`. Not one `writer.push()` failed. That is not a flaky suite, it is the boundary: the Writer half works in the linkage NOOP ships and the Replica half does not, for the reason documented in Rust/README.md (replica.rs:648 opens READ_ONLY, and Apple's libsqlite3 cannot run a statement on a WAL database with no -shm). The phone is a Writer. The server, which is the Replica, has no GRDB and so builds liters with bundled SQLite. So the tests are split along that line rather than papered over. The writer tests assert real behaviour. The replica test asserts the *bug*, deliberately, and says in its doc comment that it must be inverted into the real round trip the moment it starts failing — which is the moment liters is fixed. A skipped test would have recorded nothing; this one is a tripwire. The reopen test also produced the measurement SYNC_BUILD_VS_BUY.md §"The one thing that must be tested" asks for, and it is not the comfortable answer: [liters] reopen push snapshotted=true reason=wal truncated by another process A writer that closes, has a foreign connection write and close, then reopens, pays a FULL SNAPSHOT. That is the unmitigated case — this test does not install the `.external` policy that sets wal_autocheckpoint=0 — so it does not predict the device rate. It does show the mechanism is real and reachable, which is exactly why StoreReplication's `.external` policy is load-bearing rather than tidy. project.yml: StrandTests needed SWIFT_INCLUDE_PATHS and nothing else. Without it `@testable import Strand` fails to resolve `liters_ffiFFI` transitively even though the app compiled; with the link flags it would pull a second copy of the Rust staticlib into the test bundle, and UniFFI's handle map is static storage.
The previous commit swept up a stale copy of the generated file. Editing Rust/build-ios.sh while an instance of it was still running meant bash kept reading the OLD script text from its byte offset, so that run re-installed the bindings with the pre-fix `echo "#endif"` and re-wrote the xcconfig with the SWIFT_ACTIVE_COMPILATION_CONDITIONS line that was already known not to survive xcodegen's Debug defaults. Both are outputs, so both silently reverted. Re-running the committed script from a clean shell produces the right thing and exercises it end to end: 3 xcframework slices, `sqlite linkage OK: 0 defined, 22 undefined`, 139/139 balanced conditionals, and OTHER_SWIFT_FLAGS in the xcconfig.
…est when it is not Nothing had ever pushed an LTX frame from the phone. The bindings round-tripped, the trial flag existed, and every sync still uploaded the whole database. This wires LitersWriter into the real push path behind the existing flag. Three conditions gate it, all cheap: the trial flag (UserDefaults, default false), `.external` actually being in force rather than merely requested, and the client having a liters destination (it has none when LITERS is not compiled in, and no test double has one). Any failure — unreachable sink, full volume, lease conflict, rotated token — falls through to /ingest, which is untouched and remains both the fallback and the recovery path. The branch sits BEFORE the exporter, deliberately. `defaultExporter`'s first act is `wal_checkpoint(TRUNCATE)`, which is precisely what a page replicator must be the sole caller of; exporting and then pushing would restart the WAL underneath the writer and force a full snapshot on every sync. LitersURLSessionClient exists because liters' built-in socket transport rejects https:// outright, so without it the path cannot reach noop-cloud at all. It streams a PUT body from liters' pull-based HttpRequestBody through bound streams, so a snapshot push does not buffer. It also rewrites content-length from the buffered body: URLSession does not reliably surface the header, and liters fails the whole push with "list level 0: missing content-length" without it — found by running it, not by reading. LitersReplicator holds ONE writer for the process lifetime, which is the load- bearing decision here. Measured against a real noop-cloud sink on the real 782 MB / 3.18M-row database: a writer opened and dropped around each push snapshotted every time (~312 MB each), because closing the last connection checkpoints and truncates the WAL and `wal_autocheckpoint = 0` does not stop that. One writer held across pushes snapshots once and then runs incremental — 143-176 KB per 10-minute delta, ~2.0 MB per 4-hour delta, against 164.7 MB for the /ingest upload of the same database. maintain() is deliberately not called: one run took 177 s and uploaded a fresh 312 MB snapshot, which is neither a BGTask-sized operation nor compatible with the point of the exercise. The cost is bucket growth (~12 MB/day), swept server-side.
…e that never ran The trial reported healthy on both ends while every push failed. The server said applies=0 with an empty bucket; the phone said the flag was on and /ingest was fine. Nothing on either side named the failure, because litersPushIfEnabled's catch was write-only: it NSLogged the error and returned nil, and an NSLog on a phone that is not attached to Xcode is not readable. A push that fails and a push that never happens leave byte-identical evidence — no telemetry record (record() only runs on success), an empty bucket, and a healthy /ingest. Persist the outcome of every branch to UserDefaults, error verbatim, and show it in the Test Centre card next to the three facts that were already there. Those three describe intent, reality and result-so-far, and all three read healthy in exactly this failure. Also stop treating a push that shipped no files as a completed sync. push() does not throw for uploaded == 0, and neither synced nor uploaded was read: such a push would have returned success, skipped /ingest, and stamped lastUploadToken so the NEXT sync skipped its upload too. It now falls through to /ingest like every other way of not pushing. /ingest is untouched and remains the fallback.
…#909)
inside `Strand/Liquid` and so could only ever reach the liquid layer. Two things it
did not reach:
**1. The tilt sensor kept running.** `LiquidMotion` starts `CMMotionManager` device
motion at **60 Hz** — accelerometer + gyro sensor fusion — to drive the decorative
slosh, the "the bubbles shift when you tilt the phone" effect. #909 poses the Canvas
still, and because `acquire()` sits on the animated branch the sensor did stop when a
gauge posed. But `onDisappear` is NOT called when the app is backgrounded, and NOOP
declares `bluetooth-central` + `location` background modes, so the app keeps RUNNING
behind the lock screen while a strap is connected. The 60 Hz fusion kept being
delivered all day, for a picture nobody could see. `LiquidMotion` now stops at the
app boundary and re-arms on foreground, and `startIfWanted()` consults the gate
itself rather than trusting every call site to route around it.
**2. Everything outside the liquid layer.** A census (`QuietMotionCoverageTests`,
the Apple twin of #911's `PoseStillCoverageTest`) found five more never-settling
loops, none of which #909 covered:
- `TimeOfDayBackground` atmosphere drift — the Android twin has consulted battery
saver since #911; this side read Reduce Motion alone.
- `LiveSessionView`'s guardian breath (`repeatForever`).
- `ConnectionDot` (StatePill) and `PulseDot` (Components) halos — on screen for
long stretches: a connected strap in Settings, a backfill on every scaffold.
- `RecordingStatusLight`'s sync ring, which had **no motion gate at all** — it
pulsed under system Reduce Motion too, which was already a bug, and it ran
precisely while the strap was offloading history. (Exactly the bug #911 found in
the Android ConnectionDot.) The steady accent dot still says "syncing"; only the
loop stops.
- The onboarding glows and the radar sweep.
The gate moves out of `Strand/Liquid` and into `StrandDesign` as `NoopMotionState`,
so the design system's own surfaces can reach it — the Android twin has been in
`com.noop.ui.NoopMotion` since #911, read through `rememberPoseStill()`. Same shape:
one process-wide monitor, one observer, never per-view (the primitives alone have
dozens of call sites, and registering an observer as gauges scroll in and out would
be churn introduced by the change that exists to remove per-frame work).
**Three signals, OR-ed:** system Reduce Motion, Low Power Mode, and a new in-app
"Reduce motion in NOOP" toggle (Settings → Appearance, default OFF). The third is
the point of this change for the user who asked for it: he wants the app quiet
without putting the whole phone in battery saver. Key `noop.quietMotion`, mirroring
Kotlin `NoopPrefs.quietMotion`; live via `UserDefaults.didChangeNotification`, so
flipping it stops the sensor and poses the surfaces without leaving the screen.
iPhone 17 Pro simulator seeded with a real 746 MB store, Today idle, no touch input.
Three 15 s CPU windows after the first-run analysis had fully plateaued (that takes
~340 s at ~110% of a core on a freshly seeded DB — measuring before it settles is
what makes an idle number meaningless). Debug build.
| Today idle | CPU (% of one core) | RSS | load1 |
|---|---|---|---|
| animating (nothing asks for quiet) | 18.9 / 19.8 / 18.9 | 192 MB | 3.8 |
| **"Reduce motion in NOOP" ON** | **0.0 / 0.0 / 0.0** | 97-122 MB | 4.6 |
| system Reduce Motion ON (the #909 path) | 0.0 / 0.0 / 0.0 | 157-182 MB | 4.2 |
| animating, re-checked | 16.8 / 16.1 / 15.2 | 99-115 MB | 9.2-13.4 |
The last row is the not-in-quiet behaviour check: still animating, unchanged. It
reads lower than the first only because the machine was under 2-3x the load — per
process CPU-seconds are depressed by contention, so the two animating rows bracket
the real figure rather than either being it.
Two honest limits on those numbers:
- I could not toggle real Low Power Mode in the simulator. The Low Power row is
the Reduce Motion route, which is the same `staticGauge` / `LiquidSkyStatic` /
`paused:` branch low-power users take. What is inferred, not exercised, is only
that iOS reports the flag — which `IOSDiagnostics` already relies on.
- **The sensor saving is not in that table.** `isDeviceMotionAvailable` is false in
the simulator, so the 60 Hz CoreMotion feed never ran in any of these runs. The
interval it requests (`deviceMotionUpdateInterval = 1/60`) and the missing
background stop are read off the code, not measured. On device it is additional
to the figures above, not included in them.
Scroll was not re-measured: the earlier investigation could not reproduce the user's
jank in the simulator at all (zero `body` rebuilds during driven scrolls, 60 fps),
and the simulator is 60 Hz where his iPhone 16 Pro is 120 Hz with an 8.33 ms budget.
Any claim that this fixes scrolling would be simulator-only and unsupported.
- One-shot animations (`CountUpText`, `staggeredAppear`, `LiquidPressStyle`).
They settle and stop, so they are not the cost battery saver asks an app to
avoid — same line #911 drew.
- `TimelineView(.periodic…)` clocks. A 1 s elapsed-time label is not per-frame
drawing.
- The classic Today top-bar "Swipe / Tap" hint cycler. Two runloop wakeups per
11.5 s, and it carries a discoverability affordance rather than decoration —
removing it would cost information, not motion.
- `BreathingView` / `WatchBreatheView` run a 20 Hz `Timer.publish` even when not
breathing. Real, but bounded by that screen being on-screen, and fixing it means
restructuring the publisher rather than adding a gate. Left for its own change.
`QuietMotionCoverageTests` passes here, and **fails against the ungated tree** —
checked in both directions: removing the gate from `RecordingStatusLight` fails the
loop census by file and line, and aliasing `poseStill` back to bare `reduceMotion` in
`ConnectionDot` fails the breathe-call-site census (the call-site check requires the
file to reach `NoopMotionState`, not merely to contain the token). It fails rather
than skips when the repo root is unlocatable, and it pins that the gate still reads
all three signals and stays live — losing one half is invisible, since the screen
looks right in whichever mode still works.
`swift build` clean for StrandDesign; `xcodebuild` clean for NOOPiOS (simulator) and
the macOS Strand target; `Tools/i18n_audit.py --ci` green with the new Settings copy
translated into all eight catalog locales.
The Settings copy has said since #174 that the R22 unlock is "reversible". That was true about the hardware and false about the app: NOOP shipped an enable button and nothing that writes the flags back, so the only ways out were the official WHOOP app or a factory reset. Turning the switch off only gated future sends — it left all sixteen flags set on the strap. This adds the undo, and verifies it by reading the strap rather than by believing an ack. What it writes `Whoop5Config.disableR22Sequence` is the same sixteen keys in the same order as the enable sequence, each carrying `featureFlagOffValue` = ASCII '0' (0x30). Master flag first, so a run interrupted by a disconnect leaves the strap nearer off than it started. On the off value, precisely: 0x30 is CONFIRMED as this firmware's off value in the device-config namespace — `setBroadcastHr(false)` has written it to `whoop_live_hr_in_adv_ind_pkt` through SET_DEVICE_CONFIG_VALUE(119) since #181, hardware-validated against a Garmin Edge 840, and `enable_raw_data_w_ecg` independently reads '0' on an MG whose ECG is idle. It is INFERRED for SET_FF_VALUE(120) by shared convention: identical body layout, identical ASCII-digit value convention, same `[0x01] +` prefix, differing only in opcode and body length. Writes through 120 demonstrably change stored state — the enable sequence moved `enable_sig12` from '2' to '1', confirmed by a GET_FF_VALUE(128) read either side. What is NOT claimed: no feature flag has ever been observed holding '0'; the only values ever read back in that namespace are '1' and '2', and those are a round-trip of NOOP's own writes. `disable_pip_r26_packets` is written '2' despite being named `disable_*`, and `enable_sig12` was corrected '2'→'1' from a real capture, so the official app picks between '1' and '2' per flag rather than using one canonical true. Tri-state semantics stay unestablished. So the run tests the byte instead of asserting it `R22DisableReport` is staged. It writes '0' to ONE flag, reads it back, and declines to touch the other fifteen unless the strap stopped reporting the old value. The probe key is `enable_sig12` — the only key with a hardware demonstration that a write to it moves stored state, so a failure there is attributable to the VALUE rather than to the key or the verb. Read-back separates three outcomes that a boolean "did it work" would blur: value '0' (cleared), FAILURE (no stored value at all — nothing predicts it), or the old value (write refused). Enumeration cannot substitute: '0' is a stored value, so a cleared key stays listed, and the 117/118 walk carries no value field. Every write ack is recorded and not believed. SELECT_WRIST returns SUCCESS for a no-op and FAILURE for a real mutation on this firmware, so only the value a 128 read returns is reported as state (#907/#891 discipline). Also in here - `FeatureFlagWriteGate` tightens the send allowlist for opcode 120. It was opcode-only — any feature-flag key with any value travelled while the deep-data opt-in happened to be on. It is now key- and value-aware: the sixteen R22 keys, each only for its own enable value or the off value. Same weakness #907 closed on opcode 119. - The toggle gets an `.onChange` that offers the disable instead of silently writing nothing. Broadcast HR has had one since #181. - The card said "15" while the sequence carried 16, so it declared success a flag early. Threshold and number now come from the sequence. - A disable run's acks no longer tick the enable counter upward. - An interrupted run is rendered, not dropped: it has already written, so the user is told how far it got and which keys are still set. Verification Swift `swift test` in Packages/WhoopProtocol: 454 tests, 0 failures (24 new). Android `testFullDebugUnitTest`: 3226 tests, 0 failures across 392 classes, counted from the JUnit XML rather than the wrapper exit code (25 new). `assembleFullDebug` clean. macOS `Strand` and iOS `NOOPiOS` both build. `Tools/i18n_audit.py --ci` passes; new copy is translated in every focus locale and in it/ru/zh-Hans/zh-Hant so the extra-locale ratchet does not move. NOT verified on hardware: whether the strap accepts '0' for a feature flag, and whether clearing the flags makes the deep records stop. The first is what the probe stage measures on first run; the second needs a wear-and-sync afterwards and is called out in the UI and the report rather than claimed.
…ll snapshot
Page replication was measured at a 100% snapshot rate on VK's device: every recorded
push shipped the whole database, which makes liters a full upload with extra steps. The
cause was the `/ingest` fallback's export, which checkpoints the live store before
archiving it — and that fallback runs on exactly the syncs where the liters push did
not, so it lands between every pair of pushes.
Device evidence (push-telemetry.json, 2026-07-28): `/ingest` at 06:03, then a push at
14:23 with `snapshotReason: "wal truncated by another process"` and `bytesUploaded:
640736289`, matching the server's `bucketBytes` exactly.
The obvious fix — a gentler checkpoint mode — does NOT work, and that was measured
rather than assumed. Against a real liters writer and Apple's libsqlite3, with no
replicator read lock held:
foreign TRUNCATE -> -wal 0 bytes -> next push SNAPSHOT ("wal truncated ...")
foreign FULL -> -wal unchanged -> next push SNAPSHOT (same reason)
`FULL` looks harmless because the `-wal` file keeps its size. It is not: once a
checkpoint has fully backfilled the WAL and no reader still needs those frames, SQLite
restarts the WAL on the next write transaction — new salt, frame 1. The property that
matters is "did the WAL end up fully backfilled with nothing pinning it", and every
checkpoint mode reaches it.
So under `WalCheckpointing.external` the fallback takes no checkpoint at all. It stages
a consistent full copy through SQLite's Online Backup API — which reads *through* the
WAL and never checkpoints — and archives that. `/ingest` still ships a complete, fresh,
`quick_check`-verified database through the identical `.noopbak` container; the
replicator's resume point is untouched. Staging failure degrades to the old
checkpointing path rather than failing the sync.
Under `.automatic` — every build that has not opted into the trial, upstream included —
nothing changes: same checkpoint, same live file, byte for byte.
`Repository.checkpointForBackup()` is deliberately left truncating; its callers are the
user-initiated Export button and the at-most-daily folder backup, which want the disk
reclaimed. The hazard is named in a comment there.
No Android twin: this is fork-local cloud-sync plumbing with no stored-data, analytics,
migration or `.noopbak` container change.
Verified:
LitersRoundTripTests, against the real Liters.xcframework and a real bucket —
foreign TRUNCATE -> snapshotted=true
foreign FULL -> snapshotted=true
staged copy -> snapshotted=FALSE, 37,997 B vs the 159,077 B snapshot it follows
WhoopStore package: 391 tests, 0 failures (5 new in ConsistentCopyTests, incl. the
live `-wal` being byte-identical after a copy and the main file not backfilled).
StrandTests (macOS): 1143 tests, 1 failure — `testReplicaRestoreIsBrokenUnderSystem
SQLite`, which asserts a liters bug on purpose and now fails because the bug is
fixed. Confirmed pre-existing: it fails identically with the pristine test file from
d69ffbb against the same xcframework.
`bind(to:)`'s only call site is `DataSourcesView.onAppear`, and `onAppear`
fires on EVERY appearance — every tab switch back to Data Sources, every
push-and-pop — while the broadcaster is a `@StateObject` that outlives all of
them. Each call appended another sink to `cancellables` and nothing ever
removed one, so a user who had visited the screen N times sent N duplicate
0x2A37 Heart Rate Measurement notifications per heartbeat to every subscribed
central. The call site's own comment says "bind ... once", which is what it
intended and not what `onAppear` does.
Fixed in `bind(to:)` rather than at the call site so it holds for any future
caller: the previous subscription is cancelled before the new one is stored,
making the method idempotent. Re-binding to a DIFFERENT LiveState also
replaces rather than adds, so an old screen's state stops feeding the
broadcaster instead of racing the new one.
`cancellables` becomes `private(set)` (internal get) so the regression test
can pin the count — the leak is invisible from the outside, so the number of
sinks is the only observable that can carry it.
Verification: `xcodebuild ... test -only-testing:StrandTests/HrBroadcasterEncodeTests`
— 9 tests, 0 failures (2 new). Confirmed the new test FAILS without the fix
("11" is not equal to "1"), so it genuinely covers the bug rather than
passing vacuously. macOS `Strand` builds.
Android has no twin: its `HrBroadcaster` has no `bind` — the Compose screen
pushes samples in directly.
`ppgWaveformSample`'s migration (v27) carries an explicit CONSUMER STATUS note ending "Do NOT 'clean up' the reader as dead code: the rows are the point, and the reader is how they are reachable". `v18AuxSample` (v31 / Room MIGRATION_24_25) is in exactly the same position — every `v18AuxSamples` call site on both platforms is a test — and carried no such note. A future tidy-up pass reading it cold would delete a reader that is deliberately unused, and with it the only way the banked rows are reachable. Adds the equivalent note on both platforms, and records the part that is easy to lose: before this migration those fields were not merely unread, they were DESTROYED. The strap trims its history the moment an offload is acked, so each one was unrecoverable and could never be censused. The migration converts permanent loss into retained-but-unread, which is the whole fix and is complete. Fifteen of the slots are unpinned bytes whose names deliberately assert nothing, so wiring them to a consumer before a census would be the overclaiming this project has already had to retract. Also names the four sibling columns the same migration added (`gravitySample.dynAccel`, `sleepStateSample.rawByte`, `skinTempSample.aux1Raw/aux2Raw`), which are SELECTed into their structs with no consumer touching the properties, on purpose. Comments only — no schema, no behaviour, no stored value changes. Verification: `swift build` (WhoopStore) clean; macOS `Strand` builds; Android `assembleFullDebug` + `testFullDebugUnitTest` — 3209 tests, 0 failures, counted from the JUnit XML.
# Conflicts: # Strand/Resources/Localizable.xcstrings
…tas, not snapshots Brings the liters page-replication integration and the fix that makes it worth anything onto fork main: * the xcframework link, the generated bindings, the trial switch and the per-push telemetry; * observable push outcomes, so a failing push and a push that never ran are distinguishable (build 218); * and the /ingest fallback no longer restarting the WAL underneath the replicator, which was holding the measured snapshot rate at 100%. Default is unchanged for anyone who has not switched the trial on: the trial flag is UserDefaults-absent (false), StoreReplication is never configured to .external, and every code path here reduces to what shipped before.
…on of the session (#930)
`SleepStagerV2.cyclePrior` suppressed REM while `c < 0.12` — 12 % of THAT session's
length. First-REM latency is an absolute physiological interval, so the guard's width
scaled with how long the wearer stayed in bed: 7.4–84.5 min, an 11.5x spread, across
one WHOOP 5 user's own 36 recorded nights.
Replaces the step with a graded penalty in minutes since a MEASURED sleep onset:
rem = 1.0 * c - K * clamp01(1 - m / M0) K = 3.0, M0 = 60 min
K is the incumbent step's own magnitude (e^-3 ~ 0.05 on the REM emission, strong
suppression but never a veto). The sleep-accel grid — {cliff, graded} x {fraction,
minutes} x K in 1..8 x threshold, 210 cells — ties on accuracy in every cell, so
nothing discriminates K, and the smallest correct change fixes the units and the shape
without also retuning a magnitude no measurement can justify. M0 = 60 sits inside the
measured-defensible band: against PSG truth a guard reaching 45 min costs 0.36 % of all
real REM (1 of 31 subjects) and one reaching 90 min costs 6.85 % (15 of 31).
The `1.0 * c` ramp and the deep term are untouched and stay fractions of the session —
both describe where in the night you are, which is inherently proportional. Removing
the ramp collapses REM to 1.4 % of night and kappa to 0.143; it is load-bearing.
THIS IS A ROBUSTNESS FIX, NOT AN ACCURACY WIN. On sleep-accel the step is measurably
inert (kappa 0.349 -> 0.349, median first-REM latency 142.0 -> 141.0 min when removed)
because that cohort spans only 2.4x in session length. Replayed over the 36-night
device database (15 stage-locked human-authored references, 9163 epochs): 4-class kappa
0.691 -> 0.689, sleep/wake 0.583 -> 0.582, accuracy 78.2 % both.
kappa is not the guardrail that matters here — #437 reverted #348 for exactly that
reason ("kappa doesn't guard stage-fraction calibration"), so stage fractions are
reported too. Per-night bias vs the human reference (predicted - truth, pp) improves on
three stages and is unchanged on deep: wake -7.26 -> -7.13, light -7.54 -> -6.10, deep
+6.65 -> +6.65, rem +8.15 -> +6.57. On the healthy stratum #348 broke (in-bed >= 5 h,
n = 20) nothing moves more than 0.42 pp and wake moves +0.03 pp; deep is byte-identical
on all 36 nights. 12 of 36 nights change at all, and the change is monotone in session
length — mean REM delta -20.9 pp under 2 h, -2.4 pp at 2-5 h, -0.4 pp at >= 5 h — i.e.
it lands on precisely the short sessions the fractional guard was mis-scaling.
Plumbing: `Epoch` gains `minutesSinceOnset`; `stageEpochs` stages once with the guard
disabled, takes the first sustained non-wake run as onset (5 min = 10 epochs; measured
against PSG onset at bias -3.8 min, MAE 7.4 min, n = 31), then re-applies the guard and
re-runs Viterbi. Only the guard differs between passes, so the cost is one extra
Viterbi, not a second featurisation. The guard is clamped to [0, K] so a pre-onset epoch
cannot be penalised harder than onset itself — #271 can place a window start hours early.
Android twin updated in lockstep per the parity contract, including the constants and
the frozen golden. Tests: Swift 1247 passed / 0 failed; Android 3238 passed / 0 failed
/ 5 skipped (393 suites, counts read from the JUnit XML). Both frozen goldens are
unchanged, so the recipe's pinned end-to-end shape is preserved.
Known and separately tracked, NOT addressed here: median first-REM latency runs ~54 min
late against PSG (142 vs 88.5 min truth; removing the step gives 141, so this term is
not the cause), and deep is under-called 2-3x (4.7-6.2 % predicted vs 14.8 % truth).
Refs #930.
… not just kappa
PR #348 fitted the stager to DREAMT. It raised kappa on all three benchmarks
with a held-out gap of -0.027 — clean by every ML criterion — and PR #437
reverted it 48 hours later because it re-scored a healthy night from 6% to 23%
awake. The revert's own words: "kappa doesn't guard stage-fraction
calibration."
SleepBench as it landed in #925 reproduces exactly that blind spot. Sections B
and C score agreement (accuracy, kappa, per-stage sensitivity/specificity) and
wake minutes, none of which constrain how much of the night a recipe spends at
each stage. A recipe can raise kappa while systematically reallocating stages,
because the epochs it newly gets right can outnumber the epochs it newly
mislabels.
Adds two sections. Nothing existing is replaced; the old section E is renamed G.
E. PER-STAGE FRACTION CALIBRATION. Predicted % of night vs reference % of
night for wake/light/deep/REM, as a signed bias in percentage points,
per night and aggregated, plus the unsigned MAE so a recipe that over- and
under-calls in equal measure cannot pass as well calibrated.
- E.0 prints accuracy and kappa on the SAME nights, so the number that is
not sufficient sits directly above the numbers that guard it.
- E.2 reports a HEALTHY stratum (in-bed >= 5 h) separately. The aggregate
hides the failure: #348's healthy-night blowout was ~17 pp of wake, which
pooling with short fragmented nights dilutes.
- E.3 reports the shipped population's stage fractions over every replayed
night with no reference at all, so a distribution shift is catchable on a
database with no human labels.
F. FIRST-REM LATENCY. Minutes from staged sleep onset to the first REM epoch,
per night and as a median/p10/p90/min/max, for each recipe and for the human
reference. Calibration pins how MUCH REM a recipe emits, not WHEN; REM in
the first minutes after onset is physiologically implausible in a healthy
adult. Nights that never reach REM are counted separately rather than folded
in as a zero.
Reference set. The calibration sections score the stage-locked rows, not
section B's set. B excludes an edited night whose stored hypnogram is a
byte-exact replay of the CURRENT V2 — right for B's question, but
version-dependent by construction: change the recipe and a night can enter or
leave the exclusion, silently swapping the denominator underneath a
before/after. The stagelock cursor set comes from `cursors` and does not move
when the recipe moves.
Read-only properties are unchanged: DB.swift is untouched, the open is still
SQLITE_OPEN_READONLY | immutable=1, there is no write surface, and the database
path is still a required argument.
Verification. 13 new unit tests over the pure label-array primitives, no
database needed (swift test in Tools/SleepBench). Measured on a real 36-session
database with 15 stage-locked human-authored references (9,163 epochs), against
the previous commit as a before/after:
V2 4-class kappa 0.691 -> 0.689, accuracy 78.2% -> 78.2%
stage bias, pp wake -7.26 -> -7.13
light -7.54 -> -6.10
deep +6.65 -> +6.65
rem +8.15 -> +6.57
first-REM latency median 72.2 -> 83.2 min, minimum 10.0 -> 43.0 min
healthy-stratum wake fraction 9.40% -> 9.43% (+0.03 pp)
Kappa moved -0.002 and would have called that change nothing. The latency
minimum moving off 10 minutes, and the healthy-stratum wake fraction holding to
+0.03 pp, are the evidence — and neither was measurable in this repository
before this commit.
Section E's reference set keys on `stagelock` so the denominator cannot move between two builds — the right call, and the reason section B's recipe-dependent exclusion is not reused there. But a `stagelock` cursor proves only that the stages arrived through `edit_sleep_stages`; it does not prove they differ from what V2 emits. On the database this was developed against, 3 of the 15 stage-locked reference nights replay BYTE-EXACT from V2 and a 4th agrees at 99.09%. Those rows carry no information about V2: they hand the incumbent a guaranteed perfect night and charge every alternative recipe for the same nights. The effect is large enough to invert a verdict. Scoring PR #348's parameter set on this database gives 4-class kappa 0.640 against the incumbent's 0.689 on the contaminated 15-night set — #348 loses — and 0.705 against 0.604 on the same set with the three byte-exact nights held out — #348 wins by +0.101. Section E exists precisely to judge a #348-style tune, so a 0.085-kappa thumb on the scale in favour of "change nothing" is the one bias it must not have. Section E cannot simply drop the rows the way section B does without reintroducing the version-dependence it was built to avoid, so the harness names them instead: a new E.-1 audit lists every stage-locked night whose stored hypnogram replays from V2 at >= 95%, flags the byte-exact ones, reports what fraction of the reference set they are, and prints the exact `--exclude` line to pin the same held-out set across both builds of a comparison. Excluding by explicit timestamp keeps the set frozen; excluding by "matches this build's V2" would not. The audit asserts no mechanism for the match, only that a row cannot be an independent reference for the recipe it replays. Verification: `swift test` in Tools/SleepBench = 13 tests, 0 failures; release build clean; run against a 37-session device database copy, E.-1 names the four nights above and every downstream section is unchanged when `--exclude` is not passed. Tools-only — no analytics, no stager, no Android twin, no goldens. Refs #348, #437.
`HealthAlertBanner` was mounted only in the classic `TodayView`. Liquid Today ships as the iOS default (`noop.liquidTodayEnabled` = true), so when `AppModel.applyIllnessSignal` raised `healthAlert` on `.raised` / `.alreadyUnwell`, the screen the user actually opens showed nothing — the signal survived only in the Health tab's HeadsUpCard and the once-a-day local notification. Mount the same leaf in the Liquid section column, directly under the header scene, matching the classic placement under its top bar. It sits above the reorderable block so the Arrange sheet can't move it out of sight, and it renders nothing when there's no alert. Same amber, non-diagnostic treatment (unchanged `HealthAlertBanner` / `NoopCard` + `StrandPalette.statusWarning`). Same class of regression as #992 runtime, B1 backfill and #543 Charge carry. Verified: NOOPiOS + Strand (macOS) both build; screenshotted the seeded `--demo-screen liquidtoday` simulator build with and without an alert — banner renders under the header above the hero, and the no-alert case leaves the wordmark-to-hero spacing unchanged (no phantom gap).
…8 that survives measurement
PR #348 re-tuned seven things about SleepStagerV2 on DREAMT; PR #437 reverted all of them 48 h later
because one healthy night went from 6 % to 23 % awake ("kappa doesn't guard stage-fraction calibration").
Re-litigating that revert component-by-component, on a reference set de-contaminated with the
`--exclude` flag added in af44563, six of the seven components measure neutral-to-negative and stay
reverted. One does not: the AWAKE transition row.
Restored from #348, on both platforms:
"awake": deep 0.01 -> 0.0, rem 0.02 -> 0.0, light 0.27 -> 0.10, awake 0.70 -> 0.90
This is a physiological claim, not a fitted constant: sleep onset descends through N1/N2, so wake never
transitions straight into N3 or REM, and the freed mass makes a WASO episode span several epochs rather
than flicker back to sleep after one. The `ln(max(v, 1e-9))` viterbi floor that makes a zeroed entry
safe already landed with #348 and survived #437, so no other change is needed to support it.
MEASUREMENT (Tools/SleepBench, one wearer's 36 recorded nights).
The only reference here that V2 cannot contaminate is the strap's own band `sleep_state` — the v18 @81
nibble, WHOOP's verdict rather than a re-derivation of NOOP's. Over 21 banded nights / 15 554 epochs:
sleep/wake kappa 0.105 -> 0.118 wake sensitivity 16.0 % -> 17.6 %
accuracy 79.1 % -> 79.2 %
The #437 guard holds — this is the number that must not blow out, and does not:
healthy-stratum wake fraction (n = 20) 9.43 % -> 9.96 % (#348 entire: 32.66 %)
healthy deep 22.09 % -> 22.11 %
first-REM latency MAE 53.9 -> 41.6 min
Why the other six stay reverted, each measured alone against the same band reference / wake guard:
base priors (deep .15, awake .34) healthy wake 9.43 % -> 17.76 % the #437 blow-out, confirmed
motion gate (jerk 75/35, boost 4) healthy wake 9.43 % -> 15.92 % a SECOND wake channel, not free
emission coefficients band kappa 0.105 -> 0.094
deep gate .25 -> .40 healthy deep 22.09 % -> 25.47 % worsens an existing over-call
awake dead-zone 0.30 band kappa 0.105 -> 0.099
deep/rem/light transition rows band kappa 0.105 -> 0.101, healthy REM 29.58 % -> 33.11 %
LIMITS, stated plainly. n = 1 wearer. #348's out-of-cohort evidence was +0.028 (AAUWSS) and +0.006
(Walch); its headline +0.17 is DREAMT in-sample, on the cohort its constants were tuned to. 13 of this
database's 15 stage-locked nights are byte-exact or >= 95 % V2 replays and cannot serve as an independent
reference for V2 at all, leaving 2 usable hand-labelled nights (kappa 0.857 -> 0.863 there, a tie); the
band comparison above is the load-bearing one precisely because it does not depend on those labels.
Verification: StrandAnalytics `swift test` 1248 tests, 0 failures. Android
`:app:testFullDebugUnitTest --rerun-tasks` 3239 tests, 0 failures, 0 errors (counted from JUnit XML),
SleepStagerV2Test 12/12. Both frozen goldens pass UNCHANGED — they are insensitive to this row, which
is why this adds a direct pin for it on both platforms (twin of the existing deep-row pin), asserting
the zeros and that a zeroed entry still reaches the lattice as a finite log-weight.
Six accuracy findings reversed in a single review when they were re-measured instead of inherited: a contamination count, three agreement statistics, the sign of a bias, and the direction of a stage error. One rested on a mechanism that cannot occur, because one transaction writes both streams it claimed had diverged. These were process failures, not arithmetic ones. Adds docs/VALIDATION_PROTOCOL.md: eight rules with a paste-in PR checklist. Pre-register the prediction, the threshold, and the evaluation domain before measuring; never score against a reference the scorer produced; re-derive numbers rather than citing them; match the instrument to the question; report a temporal held-out split; check a mechanism's base rate and whether it is even reachable; ship definitions with numbers; and treat "not measurable on this data" as a result. The quantitative claims in the doc were measured against one wearer's live data while writing it, not assumed: - Evaluation domain moved sleep/wake kappa from 0.095 to 0.795 on identical nights, reference and predictor -- the only difference was scoring every band epoch versus only epochs inside the detected sessions. SleepBench's section C uses the in-session domain, so its kappa is not comparable to a whole-record one. - A coverage test separated a truncated day from 18 complete ones with zero false positives, where statistical outlier tests on the scored value flagged 23-32% of all days and still missed it. - A remembered "wrist ticks are 2-6x phone steps" invariant held on 57% of 30 qualifying days, with every violation on the low side. - A one-parameter HR threshold fitted on an early window lost 5.7 pp balanced accuracy held out, and its optimum moved more than 10 bpm in three weeks. Also records that the stored hypnogram vocabulary contains both "wake" and "awake", so stage comparisons written against a single literal misfile one of them. Links the protocol from both CONTRIBUTING files.
This was referenced Jul 31, 2026
Merged
Owner
|
The document is good and I want it in. The PR as it stands cannot be merged, and the reason has nothing to do with its content. The branch is carrying your fork's mainThe body says "Docs only — The other 187 are 100 commits of unrelated work — Oura live-API import with OAuth2 and Keychain token storage, One thing to flag plainly, not as an accusation: It is also The fixRe-cut from this repo's On the document itselfIts repo-specific claims check out — One correction to make before re-cutting. R3 says the benchmark behind the shipped sleep-staging default "exists only as a numeric claim inside a source comment. No dataset loader, no script, and no test in this repository reproduces it." That was true when you wrote it and is not true now — Two smaller notes for the re-cut, neither blocking:
Re-open against current |
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
2 participants
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.
Why
Six accuracy findings about sleep scoring reversed in a single review when a fresh reader re-measured them instead of inheriting them — a contamination count, three agreement statistics, the sign of a bias, and the direction of a stage error. One of them rested on a mechanism that cannot physically occur, because a single transaction writes both of the streams it claimed had diverged.
None were arithmetic mistakes. They were process failures, and the same process produces them again. This adds
docs/VALIDATION_PROTOCOL.md— eight rules and a paste-in PR checklist — so that a scoring change has a defined bar to clear before it is described as an improvement.The rules, in one line each
E.-1every time and use the exclusion list it prints.The numbers in the doc were measured, not assumed
Everything quantitative in it was re-derived against live wearable data while writing it:
sleep_state), same predictor. The only difference was scoring every band epoch versus only epochs inside the detected sleep sessions. That single unstated choice swings kappa further than most effects anyone is trying to measure. Worth knowing:Tools/SleepBenchsection C uses the in-session domain, so its kappa is not comparable to a whole-record figure.Also recorded
The stored hypnogram vocabulary contains both
wakeandawakeas distinct stage strings. Any consumer written asstage == "wake"silently misfiles the other spelling as sleep. Android already has acanonicalStage()normaliser with tests; Swift does not. The protocol calls this out under R7; the harness fix is filed separately rather than bundled here, since changing the harness mid-audit would change the instrument.Scope
Docs only —
docs/VALIDATION_PROTOCOL.mdplus a link from bothCONTRIBUTING.mdfiles. No code, no behaviour change. No health data: the doc carries no dates, no absolute biometric values, and no database.The bar it sets is deliberately narrow — a scoring change with no held-out number can still ship behind a flag; it just cannot be called an improvement in the changelog, a release note, or a source comment.