Apple Health import: write steps to DailyMetric — iOS + Android (#89)#124
Merged
Conversation
Apple Health steps never reached DailyMetric.steps, so they didn't surface in the
sourced-daily step arbitration (Android FusionDayAdapter via dailyColumn("steps")
= d.steps; iOS via metricValue = d.steps). Every OTHER Apple-derivable daily field
was written to DailyMetric, but the steps column was silently omitted from the
construction on BOTH platforms.
Fix: set steps in the DailyMetric row the Apple Health importer builds.
- Android: add "steps = d.steps?.let { Math.round(it).toInt() }" to the
construction, AND add "|| d.steps != null" to the hasDailyMetric gate so a
steps-only day (no HR/HRV/sleep) still creates the row.
- iOS: add "steps: d.steps.map { Int($0) }" to the construction (no gate there --
it already maps every day).
Reimplements #97's intent correctly: #97 only added the gate clause, but the
constructor still never set steps, so it created bare all-null rows and left
DailyMetric.steps null -> #89 stayed broken. This sets the field (the actual gap).
No double-count: Apple steps win by arbitration tier (as intended), and the
aggregator already takes MAX-per-source, never a cross-source sum (#589).
Verified: Android :app:compileFullDebugKotlin passes; the parse-side step
aggregation is already covered by AppleHealthImporterToleranceTest. iOS rides the
next build (app target can't compile on WSL); the added field mirrors the 8
sibling fields in the same constructor.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #89 (Apple Health steps not counted) on both platforms. Supersedes #97.
Root cause
Apple Health steps never reached
DailyMetric.steps, so they never surfaced in the sourced-daily step arbitration:FusionDayAdapter→WhoopRepository.dailyColumn("steps")=d.stepsmetricValue=d.steps(Repository.swift:1792)The importer wrote every other Apple-derivable daily field into
DailyMetric, but thestepscolumn was silently omitted from the constructor on both platforms — soDailyMetric.stepswas always null for Apple Health.Fix
Set
stepsin theDailyMetricthe importer builds:AppleHealthImporter.kt): addsteps = d.steps?.let { Math.round(it).toInt() }to the construction, and|| d.steps != nullto thehasDailyMetricgate so a steps-only day (no HR/HRV/sleep) still creates the row.AppleHealthImport.swift): addsteps: d.steps.map { Int($0) }to the construction (no gate — it already maps every day).Why this over #97
#97 added only the gate clause (
|| d.steps != null) but the constructor still never setsteps— so it created bare all-nullDailyMetricrows and leftDailyMetric.stepsnull. #89 stayed broken. The actual gap was the missing constructor field; this sets it (plus the gate, for steps-only days).No double-count: Apple steps win by arbitration tier (intended), and the aggregator already takes max-per-source, never a cross-source sum (#589).
Verification
:app:compileFullDebugKotlinpasses; the parse-side step aggregation is already covered byAppleHealthImporterToleranceTest(stepsDoNotDoubleCountAcrossSources).privateday-aggregate + a Room repo with no JVM test seam, so covering it would need a refactor larger than the one-line fix. Flagging that explicitly rather than hiding it.