Repository navigation
Description
Severity: P1 — a production merge path silently nils out a stored per-session steps value; the loss is persisted and permanent.
Verified against current main (f2271cd).
Problem
WorkoutDetector.backfillWorkout (ryanbr#510) merges a detected bout into an existing real WorkoutRow by reconstructing the row field-by-field instead of copying it. The reconstruction enumerates every WorkoutRow field except steps (added later by upstream ryanbr#1058 with an = nil init default), so the rebuilt row always carries steps = nil — even when the stored row had a real value. The upsert then writes steps = excluded.steps, overwriting the stored count with nil.
The doc comment directly above promises the opposite: "backfill ONLY the avgHr/maxHr/energyKcal/strain fields real doesn't already have … never touching a field that's already present, whether typed by the user, imported, or filled by an earlier pass."
Worse, dropping steps also breaks the function's own change-detection contract ("real unchanged (==) means it already had everything"): for a row with steps != nil, backfilled != hit is true even when all four backfill fields were already present, so didBackfill fires spuriously and the very write that destroys the steps is guaranteed to happen.
Who is hit
Any real workout row that (a) has steps set — i.e. activity-file foot-sport imports per ryanbr#1058 — and (b) time-overlaps a detected bout on a later analytics run (the normal case when the user wore the strap during that imported activity). One re-score later, the session's step count is gone; the day's step total (recomputed as SUM over sessions) drops with it.
Evidence (current main)
- Field dropped in the rebuild:
Packages/StrandAnalytics/Sources/StrandAnalytics/WorkoutDetector.swift:378–387— theWorkoutRow(...)initializer call passesstartTs/endTs/sport/source/durationS/energyKcal/avgHr/maxHr/strain/distanceM/zonesJSON/notesand omitssteps, which defaults tonil(Packages/WhoopStore/Sources/WhoopStore/JournalWorkoutAppleCache.swift:43–46, "Activity-file import: a second file imported for the same day OVERWRITES the day's step count instead of adding to it ryanbr/noop#1058: per-session steps"). - Contradicted contract:
WorkoutDetector.swift:373–376— "never touching a field that's already present". - Production call site:
Strand/Data/IntelligenceEngine.swift:1529–1547— every detected bout overlapping a real row (any source) is merged viabackfillWorkout; changed rows are collected inbackfilledByDevice. - Persistence of the loss:
IntelligenceEngine.swift:1997–1999upserts those rows;JournalWorkoutAppleCache.swift:152–163doesON CONFLICT … DO UPDATE SET … steps = excluded.steps—nilclobbers the stored value. - Kotlin twin does it right:
android/app/src/main/java/com/noop/analytics/IntelligenceEngine.kt:1739–1744usesreal.copy(avgHr = …, maxHr = …, energyKcal = …, strain = …), sosteps(androutePolyline) survive untouched. This is therefore also cross-platform drift: the same input data yields different stored rows on iOS/macOS vs Android, violating the parity contract inCLAUDE.md("Analytics and stored data must be byte-identical across Swift and Kotlin").
steps is the only field lost on the Swift side (all other WorkoutRow fields are forwarded), but the pattern is the root cause: any future field added to WorkoutRow with a defaulted init parameter will be silently dropped the same way.
Fix
Mirror Kotlin's copy semantics instead of reconstructing: keep the existing row and replace only the four computed fields — e.g. make backfillWorkout build the result from real with avgHr = real.avgHr ?? avgBpm, maxHr = real.maxHr ?? peakHR, energyKcal = real.energyKcal ?? caloriesKcal, strain = real.strain ?? strain, forwarding every other field including steps (simplest: add a with(avgHr:maxHr:energyKcal:strain:)-style copy helper on WorkoutRow, or pass steps: real.steps explicitly plus a test that fails when a new field is forgotten). Add a regression test: a row with steps set and all four backfill fields present must come back == unchanged.
Swift-side fix restores parity with Android (no Kotlin change needed).
Activity
bhelm commentedon Aug 18, 2026
Upstream fix proposed
ryanbr/noop#1434 preserves
stepswhen Swift reconstructs a workout during backfill, matching Kotlin's existing copy behavior.Deterministic regression tests cover both partial backfill and an already-complete row. GitHub currently reports the PR as cleanly mergeable, with all reported checks passing.
Keeping this issue open until the upstream change is merged.
bhelm commentedon Aug 19, 2026
Resolved upstream by ryanbr#1434, merged as . The Swift backfill now preserves stored workout steps, with deterministic regression coverage for partial and already-complete rows. Closing this fork finding.
bhelm commentedon Aug 19, 2026
Correction to the closing note: upstream merge SHA is c0da6a2.