Skip to content

feat(health): stamp nightly vitals inside the night they describe, not at its wake - #2281

Draft
Iskrata wants to merge 3 commits into
ryanbr:mainfrom
Iskrata:feat/health-vitals-in-sleep-window
Draft

Iskrata wants to merge 3 commits into
ryanbr:mainfrom
Iskrata:feat/health-vitals-in-sleep-window

Conversation

@Iskrata

@Iskrata Iskrata commented Sep 16, 2026

Copy link
Copy Markdown

What this PR does

Resting HR, HRV, SpO₂ and respiratory rate were each written to Apple Health as a single instant at the day's latest wake. That timestamp sits exactly on the sleep window's edge, so a reader that picks vitals by that window can't count on seeing it. Bevel, for example, shows no Recovery when HRV or resting HR isn't "captured during your sleep window". The latest wake could also come from a nap after the main sleep.

They're now stamped at the midpoint of the longest bridged night that ended on that day. That night comes from the same plan the sleep write exports: writeSleep's grouping is factored into sleepPlan(sessions:) so the two can't disagree. Keys don't change, so the next write-back replaces the wake-stamped samples.

HealthWriteback.vitalsInstantByDay is a pure function, covered in HealthWritebackTests. It carries a platform_specific parity disposition, since only the HealthKit write-back calls it.

Type of change

  • Bug fix

How it was tested

  • swift test in Packages/StrandImport: 329 tests pass, including 4 new ones (midpoint, longest night beats a later nap, per-day attribution, empty span).
  • Parity-governance suite (the four modules CI runs) OK; parity_ratchet.py --offline shows 0 errors.
  • NOOPiOS builds for device and is installed on an iPhone 16 with a WHOOP 5.0. Draft until Apple Health on that phone shows the vitals inside the night.

Checklist

  • Swift package tests pass for any package I touched (swift test in Packages/StrandImport)
  • No new build warnings introduced
  • Follows the conventions in docs/CONTRIBUTING.md
  • I did not commit generated output (Strand.xcodeproj/) or any secrets/keystores

Related issues

Refs #1021

@ryanbr

ryanbr commented Sep 17, 2026

Copy link
Copy Markdown
Owner

Reviewed the draft. The load-bearing claim checks out, and there are two connections worth making.

The replacement claim holds

"Keys don't change, so the next write-back replaces the wake-stamped samples" is the part that decides whether existing users get a clean migration or two samples per night, so I checked it rather than took it. The delete predicate is bySource AND byKey on HKMetadataKeyExternalUUID, with no date component:

let byKey = HKQuery.predicateForObjects(withMetadataKey: HKMetadataKeyExternalUUID, allowedValues: keys)
let pred = NSCompoundPredicate(andPredicateWithSubpredicates: [bySource, byKey])

So moving the instant cannot orphan the old sample: it is found by key wherever it sits, deleted, and rewritten at the midpoint. Confirmed.

Factoring sleepPlan(sessions:) so the sleep write and the vitals stamps cannot disagree is the right shape, and it is the specific thing that would otherwise rot: two copies of "which night is this" drift the moment either side changes.

This is half of #2264

@andremiliano filed #2264 on the same write-back, and its first complaint is exactly this: "one sample per night, stamped at the wake instant ... it sits exactly at the sleep session's end boundary rather than inside the night". You have fixed the placement.

The other half is DENSITY: still one zero-length sample per night, where an Apple Watch writes many short-window samples through the night. Worth cross-referencing so the two are not solved twice or, worse, solved in ways that disagree about which instant a night's value belongs to.

No conflict with what just landed

1a312a99d merged a change to the same writeVitals, carrying a computed avgSdnn across an imported row so a CSV import stops turning a correct SDNN into RMSSD. That touches the imported merge loop; yours replaces the wakeByDay lookup a few lines below. Different lines, and GitHub still reports mergeable. Flagging it only so a rebase does not surprise you.

The disposition is the right mechanism

The platform_specific entry for vitalsInstantByDay is correct, and worth saying out loud because I got this wrong myself earlier today: I had concluded a new one-sided Swift function could not be recorded at all, having only found that add-unpaired-function is not baselineable through the compact baseline. It is dispositionable through Tools/parity_dispositions.json, which is what you have done, with an identity, a hash and a rationale naming why only one platform calls it. That is the route, and your rationale is specific enough to be checkable later.

Staying a draft is the right call

Holding until Apple Health on the device actually shows the vitals inside the night is exactly right. The reason this bug existed is that a plausible-looking timestamp was never checked against what a reader does with it, and the fix has the same failure mode until someone looks at real Health data.

One question while you are there: does a reader that keys off the sleep window accept a sample at the exact midpoint of a night that was BRIDGED across brief wakes, where the midpoint could land inside a wake gap rather than inside a sleep segment? The midpoint of the span is not necessarily inside any of its segments.

…t at its wake

Resting HR, HRV, SpO2 and respiratory rate were each written as one instant at the day's latest wake,
which sits exactly on the sleep window's boundary. A reader that selects them by that window cannot
count on a boundary sample: Bevel, for one, shows no Recovery when HRV or resting HR is not captured
during the sleep window. The latest wake could also belong to a nap after the main sleep.

They are now stamped at the midpoint of the longest bridged night that woke on that day, from the same
night plan the sleep write exports (factored out of `writeSleep` so the two cannot disagree). Keys are
unchanged, so the next write-back replaces the wake-stamped samples. `vitalsInstantByDay` is pure and
covered in `HealthWritebackTests`.
@Iskrata
Iskrata force-pushed the feat/health-vitals-in-sleep-window branch from 2dd04ab to e343538 Compare September 17, 2026 09:24
@Iskrata

Iskrata commented Sep 17, 2026

Copy link
Copy Markdown
Author

Thanks for checking the key-based replacement rather than taking it on trust.

On the bridged-night question: yes, it could. The plan entry is the bridged night, and its .inBed sample spans spanStart…spanEnd, so the midpoint was always inside the in-bed window. But it could land in the .awake interval between two fragments, or in a wake segment of one, and so be outside every asleep sample, where a reader keying off asleep samples would not see it. e343538 fixes that: an instant that falls outside every asleep interval moves to the nearest asleep second. A night whose stages carry no timing (only .unspecified) keeps its midpoint. Two tests pin both cases.

On #2264: agreed, this is the placement half. Density isn't attempted here. Beat-to-beat data reaches Health through #2282 (heartbeat series), which a reader can window itself. If denser scalar vitals are still wanted, I'd do it as a follow-up that picks the night through the same vitalsInstantByDay, so the two can't disagree about which night a value belongs to.

Rebased onto current main (the branch point was 17 commits behind). The 1a312a99d change you flagged applied without conflicts, and parity_ratchet.py --offline reports 0 errors. #2282 is stacked on this branch and has been rebased onto it. If this one lands as a squash, I'll rebase #2282 again rather than let it carry these commits a second time.

Staying a draft until Health on the device shows the vitals inside the night. I'll post that evidence here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants