feat(health): add menstrual cycle tracker#718
Conversation
Store period starts locally and use them to anchor cycle estimates without introducing fertility or medical claims. Keep Swift and Android storage, UI, and analytics behavior aligned.
ReviewGood feature — well-structured, cross-platform parity, good test coverage, and careful scoping (no fertility/medical claims, data stays local). A few things to address before merging: Blockers
Non-blocking (follow-up)
Nits
Everything else looks solid: source isolation ( |
|
Thanks for the review — the requested changes and confirmations are now on the PR branch.
Verification completed:
No BLE path was changed, so strap hardware testing is not applicable. The focused follow-up is in |
|
Re-reviewed. Both blockers are fixed — verified on the branch rather than taken on trust. 1. Android phase localization — 2. Better than you claimed on localization. The PR notes say German, Spanish and French. The new Swift strings actually carry eight locales — de, es, fr, it, pt-PT, ru, zh-Hans, zh-Hant — including The conflict is trivial
No CI has run on this branch yet, presumably because of the conflict. The i18n audit is the gate that matters here given ~40 new strings across four Android locales and eight iOS ones, so please confirm it goes green after the rebase. One new thing, and it is the one worth discussingThe shipped copy reads:
The first two clauses are exactly right — there is no network path, no telemetry, and no platform-health export (I checked: nothing in the Health Connect / HealthKit paths references cycle data). But "never shared" is doing more work than the code supports, in two user-initiated paths:
Nothing covert is happening, and for heart-rate samples I would not raise it. For menstrual data the reading matters: someone who exports a backup to attach to a bug report here, or hands a CSV to a coach, would be sharing their period history without having been told. That is a decision for @ryanbr rather than you, and either resolution is defensible:
I would lean to the first, since silently dropping a source from a backup creates its own surprise on restore. Still outstanding from the first review
Otherwise this is good work — the source isolation under |
|
Correction to my previous comment — I got half of the privacy point wrong, in the direction that made your design look worse than it is. I said the cycle data travels in two user-initiated paths. It is one. Wrong: the CSV/JSON export. I claimed That is not luck, it is the isolation you built on purpose — Still correct: the So the accurate picture is narrower and much better than I first described:
The remaining question is the same but smaller: "never shared" is absolute, and someone attaching a Apologies for the noise — I should have traced |
|
Two things left. Rebase is already done on my side — the 4 conflicts are append-only in the Android 1. pt-PT — 55 keys. The gate fails on this now. Worth saying: pt-PT became a gated locale after you ran the audit, so this is branch staleness rather than something you missed.
The "format mismatch" errors (4 Android, 3 iOS) are the same problem, not extra work — a missing translation reads as an empty value and fails the placeholder check. Verified: adding one entry dropped missing 23→22 and mismatches 3→2 together. Keep 2. Privacy copy. Cross-platform portability (CSV route carries no period starts) is a separate follow-up, not this PR. |
Store period starts locally and use them to anchor cycle estimates without introducing fertility or medical claims.
Keep Swift and Android storage, UI, and analytics behavior aligned.
What this PR does
Adds an offline menstrual cycle tracker to the Health screen on Apple and Android.
metricSeriestable under the isolatednoop-cyclesource.Type of change
How it was tested
Packages/WhoopStore:swift test— 270 tests passed.MetricSeriesStoreTests.testDeletePointUsesFullNaturalKeyAndIsIdempotentverifies physical deletion, source isolation, and idempotency.Packages/StrandDesign:swift test— 32 tests passed.Packages/StrandAnalytics:swift test --filter CyclePhaseEngineTests— 11 tests passed../gradlew testFullDebugUnitTest— full unit suite passed.CycleTrackingStoreTestcovers source/key parity, ISO dates, ordering, idempotent logging, and deletion.CyclePhaseEngineTestcovers logged-anchor behavior and period-window estimates../gradlew compileFullDebugKotlinpassed.xcodebuild -project Strand.xcodeproj -scheme Strand -destination 'platform=macOS' CODE_SIGNING_ALLOWED=NO buildpassed.python3 tools/i18n_audit.py --ci mainpassed for Apple and Android.git diff --checkpassed.NOOPiOS: generic iOS device build passed, including the embedded widgets, Watch app, and complications.Checklist
swift testinPackages/<name>)android/(./gradlew testFullDebugUnitTest)StrandDesigntokens — no hardcoded colors, fonts, or spacingdocs/CONTRIBUTING.mdStrand.xcodeproj/) or any secrets/keystoresRelated issues
None.