Circadian: the input is heart rate, not motion — say so, and pin what the gate costs (#982)

[ryanbr]

Addresses the CircadianEngine half of #982 (@vishk23). Docs and tests only — no behaviour change.

The documentation was wrong

ActivityBin.activity was documented as "the motion volume in that bin (e.g. StepsEstimateEngine.dayMotionIntensity per hour)". It has never been fed that. The only production caller pools per-hour mean heart rate in bpm:

sums[hour] += b.bpm                                        // AppModel.swift:1624
CircadianEngine.ActivityBin(hour: …, activity: sums[h] / Double(counts[h]))   // :1628

HR is the right input here — WHOOP 4.0 motion is documented as too sparse to even stage sleep (#345) — so the caller is fine and the doc was simply describing a different design. Corrected on both platforms, with the reason recorded.

Why it matters, and what the gate actually costs

minRelativeAmplitude gates on amplitude / |mesor|, and HR arrives with a ~45–75 bpm DC offset that a motion volume does not have. So the real bar is an absolute 0.10 × mesor bpm — roughly 6.5 bpm at a 65 bpm mesor, 4.5 at 45.

I did NOT re-tune it, and the reason is a correction to the issue

#982 raises this as the gate penalising the fittest wearers. That direction does not hold. Because the bar scales with the mesor, a low-resting wearer faces a lower absolute requirement, not a higher one:

mesor 45 -> needs >= 4.5 bpm
mesor 65 -> needs >= 6.5 bpm

The same 5 bpm swing is arrhythmic at 65 and readable at 45. The concern only applies to someone whose amplitude is disproportionately small for their mesor — which is a single wearer’s observation, not an established pattern — and by @vishk23’s own account nobody is currently silenced by the gate.

Given that, swapping in an invented bpm constant would put unvalidated tuning into everyone’s analytics on n=1 evidence, which is exactly what CLAUDE.md’s rule about deriving signals warns against. The shape of the gate is still arguably wrong for this domain; the case for any particular replacement value has not been made.

So the deliverable is: correct the record, make the trade executable

A parity test pair pins what the gate costs at each mesor, including the pair that demonstrates the direction:

typicalMesorNeedsAboutSixAndAHalfBpmOfSwing   65 bpm: 8 readable, 5 not
theSameSwingIsReadableAtALowerMesor           5 bpm: unreadable at 65, readable at 45; 4 not

Whoever does have the data to re-tune this now starts from a measured fact rather than an argument — and if someone changes the constant, these fail and say what changed.

Verification

• Parity pair, byte-identical fixtures across Swift and Kotlin; both run in CI (test (StrandAnalytics), Android build-and-test).
• Expected values derived from the ratios and cross-checked against goodDaysForFit yielding .solid on the readable path, so the non-unreadable assertions are meaningful rather than vacuous.
• Kotlin compiles with no new errors; swiftc -parse clean; doc-comment lint and i18n audit both 0.
• No production code changed, so there is nothing to device-test.

Not in scope

The other two #982 items — the unsurfaced chargeConfidence/effortConfidence/restConfidence tier, and the cadence test pinning solidEffortReadings / minHourHRSamples / maximumContinuousGapSeconds — are separate and both worth doing. I verified the confidence tier is genuinely unread on main (declaration, init, computation, one test assertion, zero production readers).

Worth noting for anyone reading #982 against this tree: four fields it names (highStressMinutes, hrOnlyFallback, activityMaskedHours, daysBelowBand) do not exist on main — they are added by #988, so that sweep was run against a branch, not main.

[ryanbr]

Re-reviewed. Nothing found — the first pass today where that is the honest answer, so here is what was actually checked rather than a bare assertion.

Executed the shipped engine, not my arithmetic. CircadianEngine.kt imports only kotlin.math, so it compiles standalone. Ran the four fixtures against the real object:

  pass  cosinor recovers mesor 65
  pass  cosinor recovers amplitude 8
  pass  65/8 readable   (0.123)
  pass  65/5 unreadable (0.077)
  pass  45/5 readable   (0.111)
  pass  45/4 unreadable (0.089)
  -> same 5bpm swing: 65=UNREADABLE  45=SOLID

That last line is the finding this PR exists to record, confirmed empirically rather than derived: the identical 5 bpm swing is arrhythmic at a 65 bpm mesor and SOLID at 45. The gate favours the low-resting wearer.

It also settles that the tests are not vacuous — the cosinor recovers the injected parameters to 1e-6, so each fixture really does land where the ratio says, and every case sits well clear of the 0.10 boundary (0.077 / 0.089 / 0.111 / 0.123), so no float wobble can flip one.

The Swift twin is not executable here, but the two engines are byte-parity twins with existing tests pinning that, and the fixtures are identical — so the Kotlin result carries.

The "no behaviour change" claim is verified, not asserted. Stripping comment lines from the production diff leaves nothing: both CircadianEngine.swift and CircadianEngine.kt changed in comments only, and minRelativeAmplitude = 0.10 is untouched on both sides.

Also clear: no stacked doc comments (lint 0 — worth checking, since I inserted a KDoc above an existing //-commented constant block), existing suites unaffected, and CI ran both test (StrandAnalytics) and Android build-and-test.

The one thing I would still call soft is judgement, not correctness: whether to leave the gate relative at all. I have argued not to re-tune it on n=1 evidence and pinned the cost so the decision is informed — but that is a position, not a proof, and @vishk23 has the wearer data I do not.