Repository navigation
Port the SDNN index to Android - #1535
Conversation
Android integration verificationTested end to end on a real Android staging installation after merging this exact reviewed commit into
This verifies the production Android calculation, persistence, Room schema, and |
|
Clarification on historical population: the Room 31→32 migration itself performs no SQL/value backfill; it only adds nullable |
|
Reviewed. This is the Android twin the schema oracle has been asking for by name, and it holds up — I checked the parity chain end to end rather than trusting the description, and tested the merged result rather than the branch alone. The parity question that decides itYour Kotlin buckets samples in one pass; Swift re-filters the whole night per segment. Those are only equivalent if cleaning happens after windowing — Malik rejection compares each interval to its predecessor, so if Swift cleaned across the night first, its boundary intervals would carry a neighbour yours don't, and every segment edge would drift.
The remaining shape differences are equivalences, not divergences: Swift walks every segment from first to last including empty spans (which yield nil sdnn and are skipped), you skip them by construction; Swift's explicit Everything else I checked
Verification I ranThe branch is based on
Your note about there being no Room runtime migration harness is the right call to make explicitly. An additive nullable column is the safest migration shape there is, and claiming an exercised on-device 31-to-32 upgrade without a harness would have been the wrong thing to write. One pre-existing thing, not yoursWhile reading the Apple mapping I noticed the daily step total diverges across platforms on the same import:
Ready to merge from my side. |
…1540) Follow-up to #1535, found reviewing its Apple import mapping and not introduced by it. The same Apple Health export stored a different step count on each platform: Apple steps: d.steps.map { Int($0) } truncates Android steps = d.steps?.let { Math.round(it).toInt() } rounds A fractional daily total is reachable rather than theoretical: the aggregator accumulates per source (`stepsBySource[...] += v`) before the winning source's figure is taken, and nothing constrains a source to whole counts. On such a day Apple stored a total one lower than Android from the same file. Rounding is the side to converge on, and the truncation was an oversight rather than a policy: every other count and rate converted on the SAME rows already rounds -- avgHr, maxHr and walkingHr on the AppleDaily row, restingHr on the DailyMetric row -- and Android rounds throughout. `steps` was the only field on the row that did not, so Apple was inconsistent with itself before it was inconsistent with Android. Both sites now route through one `AppleHealthImport.stepsInt`, extracted because the conversion sat inside a file-IO function with no test seam at all -- the same shape #1535 used for `dailyMetricRow`, so the test exercises the production conversion rather than a copy of it. Swift's `.rounded()` is half-away-from-zero and Kotlin's `Math.round` is half-up. They differ only for negatives, which a step count cannot be, and the test pins that agreement rather than assuming it. No Android change: it already rounds. No backfill -- this corrects future imports, and re-importing an affected export now stores the value both platforms agree on. The total's DERIVATION was already identical on both sides (max of stepsBySource, #589); only the final Double-to-Int step differed. `map { Int($0) }` now appears nowhere in the Swift tree, so these two sites were the only ones. Verified: app-build green on both legs at the merged head, Test Strand 1191 tests 0 failures (1188 before, so all three new tests ran); doc lint clean.
Curated notes for docs/releases/v10.6.0.md covering the 83 merged PRs since v10.5.0, then Tools/appchangelog-gen.py run over them to insert the entry into both AppChangelog.swift and AppChangelog.kt and localize the card title into all six Android locales. Generated rather than hand-written on purpose: the release workflow only runs appchangelog-gen when it performs the version bump itself, so running it here means the in-app card can be reviewed and lint-checked before anything is published instead of arriving as a CI warning nobody reads. Headline items are the selectable Banister TRIMP Effort scale (#1562, #1563), the background re-score that can now finish rather than restarting forever (#1557, #1559), the Oura ring being handed out of daytime-HR mode so its own sleep suite can run (#1526, #1550), ramp-aware sleep with the night's HR line (#1551, #1552), live-workout pause/discard plus SDNN on Android (#1533, #1535), and the WHOOP 4.0 device key no longer reaching a shared strap log (#1610). Third-party contributors credited by handle per CLAUDE.md, covering both merged PRs and the reports behind the fixes. The version itself is deliberately NOT bumped here: the release workflow skips appchangelog-gen when the source version already equals the target, so pre-bumping would ship a release whose in-app What's New still advertised the previous one. Verified: compileFullDebugKotlin, lintVitalFullRelease -PstagingRelease (the ExtraTranslation gate that compile and the i18n audit both miss), i18n_audit --ci, doc_comment_lint, and the 108 Tools tests.
Why
Android currently stores nightly RMSSD but lacks the separate five-minute SDNN index already computed and persisted on iOS. This closes that data and schema gap so native Android nights and cross-platform backups can carry the same distinct metric.
Scope
DailyMetric.avgSdnnand preserve it through merge/coalescing pathsavgHrvcompatibility field and the new semantically correctavgSdnnavgHrv; it must not be relabelled as SDNNExisting rows remain null. There is deliberately no RMSSD-to-SDNN backfill.
Verification
Migration verification is static plus Room KSP schema verification. The repository currently has no Room runtime migration-test harness, so this PR does not claim an exercised on-device SQLite 31-to-32 upgrade.
Non-goals
No UI, score, baseline, Health Connect export, or trust-gating behavior changes. Shared trust hardening remains a separate cross-platform concern.
Closes bhelm#18