fix(analytics): wire primary-session resting HR to fix nap floor dist…

[moyasserr]

What this PR does

Wires primarySessionRestingHR into restingHRDaily on both Swift and Kotlin AnalyticsEngine implementations.

Previously, restingHRDaily took the minimum (.min()) resting HR across all sleep sessions of the day. A daytime nap or brief artifact would drag down the entire day's resting HR by 4–8 bpm, artificially lowering daily cardiovascular strain and distorting the recovery baseline.

This change calculates resting HR as the sample mean of the primary (longest) overnight sleep session, using the existing PrimarySessionRestingHR logic that was previously run only as shadow instrumentation. If primary session data is sparse (< 30 valid samples), it cleanly falls back to the previous session-minimum behavior.

Type of change

• Bug fix
• New feature
• Refactor / cleanup
• Documentation
• CI / tooling

How it was tested

• Parity Governance Tools:
  • python3 Tools/parity_ledger.py: Passed cleanly with zero new findings; decreased test-only-callsite debt on both Packages/StrandAnalytics and android/analytics.
  • python3 Tools/parity_ratchet.py --base origin/main --offline: Passed with errors=0.
• Unit & Algorithmic Tests:
  • Verified against existing twin suites: PrimarySessionRestingHRTests.swift and PrimarySessionRestingHRTest.kt (multi-session nap vs. main night fixtures).
  • Validated edge cases: single-session nights, multi-session days with daytime naps, and sparse-sample sessions falling back to .min().

Checklist

• Swift package tests pass for any package I touched (swift test in Packages/<name>)
• Android unit tests pass if I touched android/ (./gradlew testFullDebugUnitTest)
• No new build warnings introduced
• UI changes use only StrandDesign tokens — no hardcoded colors, fonts, or spacing
• No hardcoded hex frame bytes; protocol facts live in the schema / decoders
• Follows the conventions in docs/CONTRIBUTING.md
• I did not commit generated output (Strand.xcodeproj/) or any secrets/keystores

Related issues

Refs #1169

[ryanbr]

Reviewed at f697002. Thanks for picking this up. The parity half is careful and I have no complaint with it: Swift max(by:) and Kotlin maxByOrNull both keep the first maximum on ties, and Int($0.rounded()) against roundToInt() differ only on negative halves, which a resting HR never produces. The two engines will agree.

My hesitation is not in the diff. It is in the helper this wires up.

PrimarySessionRestingHR's own doc block, which this PR leaves in place, is headed "Deliberately PURE and UNWIRED". It states that "nothing consumes it yet", that switching the consumers would be "a re-baselining of core scores", that "the issue itself says needs a larger multi-participant, pre-declared holdout first", and that this is "out of scope here".

This PR performs the switch that paragraph defers. Merged as it stands, the repo would carry a doc asserting that nothing consumes the helper, sitting directly above a helper that now sets the headline resting HR, which in turn feeds recovery, strain, workout detection and energy. Two readouts of one fact, able to disagree, in documentation form.

Three concrete things behind that:

1. No new evidence. The helper's doc records the original validation as a single participant over five nights. The testing section here lists parity tools and the helper's existing unit tests. Nothing shadow-run, nothing multi-participant. That bar was set by the issue and by the helper's own doc, not by me.
2. No test files changed. This is a behaviour change to a stored, score-feeding metric with nothing pinning the new wiring. A test that goes red when the fallback order is reordered would be the minimum.
3. Scope. The providedPrimaryRHR clause, matching provided sessions on exact start and end, is extra logic beyond the stated purpose and deserves its own reasoning.

What I would rather merge: keep this wiring exactly as you have written it, but put it behind a default-off Test Centre toggle in the shape #2361 uses for the Oura notification mask. Then both definitions can be compared over real nights before either becomes the scored one. That turns a re-baselining into an A/B, which is the bar the helper's own doc set, and it makes the eventual flip evidence-backed rather than argued. I am happy to write the toggle if you would rather not.

One housekeeping item whichever way this goes: the last line of the description pairs a closing verb with the issue number. Could you switch it to Refs #1169? As written, a merge would retire an issue whose validation question is still open.

[ryanbr]

Follow-up, and it is about sequencing rather than anything new in your diff.

#2284 changes what a sleep session's restingHR IS: from the lowest 5-minute bin mean to the mean HR across the session's deep-sleep segments (SleepStager.sessionDeepSleepRestingHR), on both platforms, with a one-time full-history rescore.

That lands one layer below this PR and largely on the same complaint. This PR replaces restingHRDaily at AnalyticsEngine.swift:699 with a primary-session mean. #2284 changes the per-session value at :519 that restingHRDaily = ...restingHR.min() then takes the minimum of. If that one lands first, the shipped daily figure becomes the minimum across deep-sleep means rather than across nightly floors, which absorbs most of the gap this PR is closing.

The practical consequence for your numbers: the MAE improvements in your description (6.0 to 2.0 dev, 7.5 to 0.8 holdout) were measured against the floor statistic. Afterwards the comparison baseline is a different statistic, so those figures would need re-deriving before they mean what they currently say.

It may turn out the remaining gap is small enough that the wiring is not worth a re-baselining. It may equally turn out the primary-session half still matters, because the other PR does nothing about the part of the issue where a short low-HR nap can replace the main overnight session through .min(). That half is genuinely yours and nothing else in flight addresses it.

So none of this is a reason to withdraw anything. It is a reason not to merge the two independently. My earlier suggestion stands: land the wiring behind a default-off Test Centre toggle so both definitions can be compared on real nights, and if the session-level change goes in first, compare against that.

Nothing here changes the earlier review. The last line of your description still pairs a closing verb with an issue number and wants switching to Refs, and the helper's own doc block still states that nothing consumes it yet.

[ryanbr]

Sequencing decided, and it affects this PR more than it affects #2284: that one goes in first.

Why that way round. #2284's argument is internal to the codebase, that the recovery baseline mixes the imported export's resting HR with computed lowest-bin values roughly 8 bpm lower, so a wearer's own nights read as unusually low and inflate Charge. That is demonstrable without any external reference. This PR's argument IS the comparison, which is a harder thing to carry on one participant, and it sits one layer above the statistic #2284 changes.

What that means for your numbers. #2284 replaces a sleep session's restingHR with the mean across its deep-sleep segments, so restingHRDaily = ...restingHR.min() becomes the minimum across deep-sleep means rather than across nightly floors. Your MAE figures, 6.0 down to 2.0 on dev and 7.5 down to 0.8 on holdout, were measured against the floor. After #2284 lands the comparison baseline is a different statistic, so those figures would need re-deriving before they mean what the description currently says.

I want to be clear this is not a polite way of turning the PR down. The half that is genuinely yours survives #2284 untouched: nothing in it addresses .min() letting a short low-HR nap displace the main overnight session, which is a real part of #1169 and nothing else in flight covers it. If the remaining gap after #2284 is small, that is worth knowing; if the nap-displacement half still moves the number, that is worth knowing too, and only a re-derivation tells you which.

So the practical path: wait for #2284, re-run your dev and holdout split against the new statistic, and bring the numbers back. My earlier suggestion still stands as the way to land it either way, which is to put the wiring behind a default-off Test Centre toggle so both definitions can be compared on real nights rather than argued.

Two things from the first review are unchanged and independent of all this: the last line of the description still pairs a closing verb with an issue number and wants switching to Refs, and PrimarySessionRestingHR's own doc block still states that nothing consumes it yet.

[moyasserr]

Thanks for the thorough review and clear context on sequencing with #2284, @ryanbr

I agree with this path:

1. Housekeeping: Updated the description to Refs #1169.
2. Test Centre Toggle: Putting the primary-session resting HR wiring behind a default-off Test Centre toggle (in the style of feat(oura): Test Centre toggle to send the official app's SetNotification mask ff (packed-notification A/B) #2361) makes complete sense. It avoids an abrupt re-baselining while enabling real-night A/B comparisons.
3. Sequencing with fix(rhr): report resting HR as the deep-sleep mean, not the night's lowest 5-min bin #2284: Happy to wait for fix(rhr): report resting HR as the deep-sleep mean, not the night's lowest 5-min bin #2284 to land first. Once the session-level deep-sleep mean is in place, we can re-derive the comparison numbers against the new baseline and evaluate how the nap displacement protection interacts with it.
4. Docs & Tests: When adding the toggle, we'll update the PrimarySessionRestingHR doc block and add unit tests pinning the wiring and fallback order.

Regarding the toggle implementation: if you'd like to push the Test Centre toggle directly to this branch, you are more than welcome to. Otherwise, I can implement the toggle and matching tests once #2284 merges. Let me know your preference!