WHOOP import: key journal entries to the wake day, not the onset evening (#136)#158
Merged
Conversation
…ing (#136) WHOOP's journal_entries.csv keys each entry only by cycle_start (the evening you fell asleep), but parseCycles/parseSleeps and the native in-app journal all key by the WAKE day. So every imported journal entry landed one day EARLY and never matched the recovery/sleep outcome it belongs to -- in Insights the whole imported history collapsed into "Without" (the entry's "with" day never lined up with an outcome). Fix: build a cycle_start -> wake-day map from the cycles CSV (reusing the existing wake-day rule -- wake_onset, else cycle_end, else start) and key each journal entry to its cycle's wake day, falling back to the onset day only when the cycle isn't in the export. Both platforms. NON-DESTRUCTIVE by design: no existing data is deleted. New/fresh imports are correct immediately; to correct already-imported history, a user removes + re-adds their WHOOP import (the existing, user-initiated flow that clears the imported device) -- we never silently delete their journal. Tests (Android): an entry with onset 2026-06-05 evening + wake 2026-06-06 keys to the 6th (not the 5th) with answeredYes intact; fallback keys to the onset day when the cycle is absent. 2366 unit tests green. iOS mirrors via the shared WhoopDayKeying helper.
Follow-up to the wake-day keying fix: without cleanup, an existing user re-importing
would keep the pre-fix onset-keyed rows AND add the new wake-keyed ones (different day
= different PK) -> duplicates.
Fix it safely, NOT with a wholesale delete: before upserting, clear this device's
journal across EXACTLY the day span being re-written ([min, max] of the imported
entries' days), then upsert. Bounded to the imported range, so journal OUTSIDE it --
e.g. older history from an earlier, wider export -- is never touched. This is the same
"re-import replaces this period" behaviour daily/sleep already have, and it can't lose
data the way clearing the whole device would.
Both platforms: new deleteJournalRange (deviceId + day-range scoped, mirrors the
existing dailyMetric range delete) on WhoopDao/WhoopRepository and the iOS store, called
from the importer. Native ("noop-journal") log is a separate device, never touched.
Residual (negligible): the very first night's onset row sits one day below the write
range, so it isn't cleared -- one day, only for users who imported pre-fix and re-import
without removing. Not worth reaching below the span we actually write. Android 2366 green.
Owner
Author
|
Update — the re-import duplication tradeoff is now fixed, safely. The earlier version left a tradeoff: an existing user re-importing (without first removing the import) would keep the old onset-keyed rows and add the new wake-keyed ones → duplicates. Fixed now with a bounded range replace, not a wholesale delete:
Residual (negligible): the very first night's onset row sits one day below the write range, so it's not cleared — one day, only for users who imported pre-fix and re-import without removing. Deliberately not reaching below the span we actually write. Android 2366 unit tests green. |
The dedupe cleared the journal range and re-inserted in two separate transactions, so a crash between them would leave the range deleted-but-not-repopulated -- data loss, which is exactly what this whole change was trying to avoid. Fold both into ONE transaction: Android via a @transaction replaceJournalRange on WhoopDao (delete range + upsert), iOS via a single syncWrite in the store's replaceJournalRange (DELETE then the same INSERT ... ON CONFLICT upsertJournal uses). A crash now rolls the whole thing back -- the range is never left empty. Both importers call replaceJournalRange instead of delete-then-upsert. Android 2366 green.
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 #136.
The bug
Imported WHOOP journal entries all read as "Without" in Insights — new (native) entries correlate, old (imported) ones don't. Not because they're unparsed (they are), but because they land on the wrong day.
WHOOP's
journal_entries.csvkeys each entry only bycycle_start— the onset evening — whileparseCycles/parseSleepsand the native in-app journal all key by the wake day. So every imported entry sat one day early and never matched the recovery/sleep outcome it belongs to; Insights walks each outcome (wake) day, finds no matching "yes" entry, and buckets it into "Without." The old code even flagged this as "a minor correlation-only offset" — it isn't minor; it zeroes out the entire imported journal.The fix
Build a
cycle_start → wake-daymap from the cycles CSV (reusing the existing wake-day rule:wake_onset ?: cycle_end ?: start) and key each journal entry to its cycle's wake day — the same day the outcome and native journal use. Falls back to the onset day only when the cycle isn't in the export. Both platforms (WhoopCsvImporter.kt+WhoopImporter.swift, via the sharedWhoopDayKeying).Non-destructive by design
No existing data is deleted. New/fresh imports are correct immediately. To correct already-imported history, a user removes + re-adds their WHOOP import — the existing, user-initiated flow that clears the imported device. We deliberately do not silently delete anyone's journal on import (an export that's ever shorter than a previous one would lose entries — not worth the risk).
Tests
Android: an entry with onset
2026-06-05evening and wake2026-06-06keys to the 6th (not the 5th), withansweredYesintact; a second test pins the onset-day fallback when the cycle is absent. Full suite 2366 green. iOS keying mirrors the same logic through the shared, already-testedWhoopDayKeying.wakeDayKey(app-target keying isn't locally unit-testable — validated at release CI).Out of scope
The secondary design question — whether a day with no journal entry should be "Without" vs. excluded (unknown) — is left for a separate discussion; this PR fixes the day-key misalignment only.