Repository navigation
FIT import: double total_cycles → steps for foot sports (fixes #568 halving) - #584
Merged
Merged
Conversation
An imported walk/run showed exactly HALF its steps (a 2300-step walk imported as 1150). FIT message field 10 is total_cycles, which for foot sports is STRIDES — one stride = two steps — but both parsers read it raw as the step count. iOS's own comment already said field 10 is "total_cycles; converted to steps only for foot activities", but the ×2 conversion was never applied on either platform. Apply ×2 for foot sports in both buildResult()s: - android/ActivityFileImporter.kt - Packages/StrandImport/FitParser.swift Tests updated to the doubled value (fixture field 10 = 1175 strides → 2350 steps), both platforms. Scope note: this is the FIT-standard interpretation (Suunto/Garmin record cycles=strides); a non-standard device that put actual steps in field 10 would now double — but that's off-standard, and the exact-half report + the existing "converted to steps" intent confirm the mainstream case. Tested: Android ./gradlew testFullDebugUnitTest --tests ActivityFileImporterTest -> 10/10. iOS FitParser is in StrandImport (CI-covered by swift-packages.yml; GRDB wall blocks a local Linux build) — the change mirrors the Android one I compiled + ran.
ryanbr
force-pushed
the
fix/fit-steps-cycles-doubling
branch
from
July 18, 2026 22:42
ba8405d to
0b657d9
Compare
ryanbr
added a commit
that referenced
this pull request
Jul 18, 2026
…trapping init (#588) mergeActivityFileSteps built its day lookup with `Dictionary(uniqueKeysWithValues: base.map { ($0.day, $0) })`, which TRAPS (crashes) if `base` ever carries two rows for the same day. Two reasons to make it graceful: - Parity: the Kotlin twin (WhoopRepository.mergeActivityFileSteps) already does last-wins (`for (row in base) byDay[row.day] = row`) — no crash. - Convention: elsewhere in this same file the day-keyed dictionaries use the safe `Dictionary(…, uniquingKeysWith: { _, last in last })` form; this one was the outlier. `mergeDaily` de-dups by day today, so the trap is likely unreachable now — but using a trapping initializer on merged external data is fragile and diverges from Android. Swap to the two-arg last-wins init. Found while investigating #568 (unrelated to the steps-count fix in #584). App-target Swift, so no local build (GRDB/Combine wall) — the two-arg Dictionary init form was type-checked in isolation; `mergeActivityFileSteps` is covered by VitalSourceResolutionTests on macOS CI.
DX23876
pushed a commit
to DX23876/noop
that referenced
this pull request
Jul 22, 2026
…yanbr#584) An imported walk/run showed exactly HALF its steps (a 2300-step walk imported as 1150). FIT message field 10 is total_cycles, which for foot sports is STRIDES — one stride = two steps — but both parsers read it raw as the step count. iOS's own comment already said field 10 is "total_cycles; converted to steps only for foot activities", but the ×2 conversion was never applied on either platform. Apply ×2 for foot sports in both buildResult()s: - android/ActivityFileImporter.kt - Packages/StrandImport/FitParser.swift Tests updated to the doubled value (fixture field 10 = 1175 strides → 2350 steps), both platforms. Scope note: this is the FIT-standard interpretation (Suunto/Garmin record cycles=strides); a non-standard device that put actual steps in field 10 would now double — but that's off-standard, and the exact-half report + the existing "converted to steps" intent confirm the mainstream case. Tested: Android ./gradlew testFullDebugUnitTest --tests ActivityFileImporterTest -> 10/10. iOS FitParser is in StrandImport (CI-covered by swift-packages.yml; GRDB wall blocks a local Linux build) — the change mirrors the Android one I compiled + ran.
DX23876
pushed a commit
to DX23876/noop
that referenced
this pull request
Jul 22, 2026
…trapping init (ryanbr#588) mergeActivityFileSteps built its day lookup with `Dictionary(uniqueKeysWithValues: base.map { ($0.day, $0) })`, which TRAPS (crashes) if `base` ever carries two rows for the same day. Two reasons to make it graceful: - Parity: the Kotlin twin (WhoopRepository.mergeActivityFileSteps) already does last-wins (`for (row in base) byDay[row.day] = row`) — no crash. - Convention: elsewhere in this same file the day-keyed dictionaries use the safe `Dictionary(…, uniquingKeysWith: { _, last in last })` form; this one was the outlier. `mergeDaily` de-dups by day today, so the trap is likely unreachable now — but using a trapping initializer on merged external data is fragile and diverges from Android. Swap to the two-arg last-wins init. Found while investigating ryanbr#568 (unrelated to the steps-count fix in ryanbr#584). App-target Swift, so no local build (GRDB/Combine wall) — the two-arg Dictionary init form was type-checked in isolation; `mergeActivityFileSteps` is covered by VitalSourceResolutionTests on macOS CI.
simoncad7
pushed a commit
to simoncad7/noop
that referenced
this pull request
Aug 17, 2026
…yanbr#584) An imported walk/run showed exactly HALF its steps (a 2300-step walk imported as 1150). FIT message field 10 is total_cycles, which for foot sports is STRIDES — one stride = two steps — but both parsers read it raw as the step count. iOS's own comment already said field 10 is "total_cycles; converted to steps only for foot activities", but the ×2 conversion was never applied on either platform. Apply ×2 for foot sports in both buildResult()s: - android/ActivityFileImporter.kt - Packages/StrandImport/FitParser.swift Tests updated to the doubled value (fixture field 10 = 1175 strides → 2350 steps), both platforms. Scope note: this is the FIT-standard interpretation (Suunto/Garmin record cycles=strides); a non-standard device that put actual steps in field 10 would now double — but that's off-standard, and the exact-half report + the existing "converted to steps" intent confirm the mainstream case. Tested: Android ./gradlew testFullDebugUnitTest --tests ActivityFileImporterTest -> 10/10. iOS FitParser is in StrandImport (CI-covered by swift-packages.yml; GRDB wall blocks a local Linux build) — the change mirrors the Android one I compiled + ran.
simoncad7
pushed a commit
to simoncad7/noop
that referenced
this pull request
Aug 17, 2026
…trapping init (ryanbr#588) mergeActivityFileSteps built its day lookup with `Dictionary(uniqueKeysWithValues: base.map { ($0.day, $0) })`, which TRAPS (crashes) if `base` ever carries two rows for the same day. Two reasons to make it graceful: - Parity: the Kotlin twin (WhoopRepository.mergeActivityFileSteps) already does last-wins (`for (row in base) byDay[row.day] = row`) — no crash. - Convention: elsewhere in this same file the day-keyed dictionaries use the safe `Dictionary(…, uniquingKeysWith: { _, last in last })` form; this one was the outlier. `mergeDaily` de-dups by day today, so the trap is likely unreachable now — but using a trapping initializer on merged external data is fragile and diverges from Android. Swap to the two-arg last-wins init. Found while investigating ryanbr#568 (unrelated to the steps-count fix in ryanbr#584). App-target Swift, so no local build (GRDB/Combine wall) — the two-arg Dictionary init form was type-checked in isolation; `mergeActivityFileSteps` is covered by VitalSourceResolutionTests on macOS CI.
This was referenced Aug 20, 2026
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 the halving half of #568 (@pilleuspulcher-blip): an imported walk/run showed exactly half its steps (a 2300-step Suunto walk imported as 1150).
Cause
The FIT parser reads message field 10 as the step count. But field 10 is
total_cycles— for foot sports that's strides, and one stride = two steps. Both parsers read it raw, so strides were shown as steps → exactly half. iOS's own comment even names it: "total_cycles; converted to steps only for foot activities in buildResult()" — the ×2 conversion was intended but never implemented, on either platform.Fix
Apply
×2for foot sports in bothbuildResult()s:android/…/ActivityFileImporter.ktPackages/StrandImport/…/FitParser.swiftBoth step tests updated to the doubled value (fixture field 10 = 1175 strides → 2350 steps).
Scope / caveat
This is the FIT-standard interpretation — Suunto/Garmin record
total_cycles= strides, so ×2 is the real step count. A non-standard device that wrote actual steps into field 10 would now be doubled, but that's off-spec; the reporter's exact-half discrepancy plus the pre-existing "converted to steps" intent confirm the mainstream case.Verification
./gradlew testFullDebugUnitTest --tests "com.noop.ingest.ActivityFileImporterTest"→ 10/10 (compiled + run locally).FitParserlives inStrandImport, which is covered byswift-packages.ymlCI — so the iOS half gets validated there. It can't build on Linux (GRDB/sqlite wall), so I couldn't run it locally, but the edit + test update mirror the Android ones exactly.Not included
The second bug in #568 — imported activity steps hijacking/duplicating the daily total — is deliberately not in this PR. It's not a clean fix: the daily-steps write feeds a deliberate design (
RegistryDayOwnerSource#137 ranks activity-file as a day-steps owner),WorkoutRowhas no steps field, and the reporter's "sum them" is a double-count risk. That one needs a design decision and is better tracked separately.