Repository navigation
fix(multi-whoop): read the active strap, not a hardcoded "my-whoop", on the Kotlin #512-class paths (#1304) - #1374
Merged
Merged
Conversation
…on the Kotlin #512-class paths (#1304) Sweep result for #1304: the Swift side is already union-safe everywhere (Repository / CsvExport / MetricCatalog), so every remaining #512-class bug is a Kotlin CALLER handing the literal "my-whoop" into a union-aware repo method — which collapses the union (importedSourceIdsFor("my-whoop") == ["my-whoop"]) and re-drops a 2nd strap's data (banked under "whoop-<uuid>"). Thread the active strap id instead: - CoupledScreen: last-night sleep + habitual midsleep now use sleepSessionsUnion / computedSleepSessionsUnion / habitualMidsleepSec(vm.activeStrapId), matching SleepScreen. - TodayScreen: the sleepToday hero unions (like the hrBucketsUnion read just above it), and the battery runtime estimate reads the active strap's own SoC. - WhoopConnectionService: the FGS notification's recentDaysMergedFlow and the battery read use (application as NoopApplication).activeDeviceId (same accessor already used at :606). - AiCoach: daysMerged / rrIntervals / Lab Book markers now read the active strap (threaded from CoachViewModel), matching Swift AICoach (repo.deviceId). Journal stays canonical "my-whoop" (user-global, not strap telemetry) — matching Swift AICoach.journalEntries(). Deliberately NOT changed (verified NOT #512-class): the "stress" and sleep_performance/need reads are IMPORTED metrics that live under the canonical "my-whoop" by definition (a 2nd BLE strap produces none; Swift's union collapses to the same thing); ShortcutHealthImport's forbiddenSources is a write-target guard, not a read. DataSourcesScreen's WHOOP badge counts under-report a 2nd strap but only cosmetically (no union-aware count variant exists yet) — left as a minor follow-up. Test: MultiWhoopSourceUnionTest pins the collapse-vs-union contract these callers now depend on (pure companion resolvers, twin of WorkoutHrDeviceKeyTest). 319 data+ai unit tests green.
ryanbr
force-pushed
the
fix/multi-whoop-read-paths-1304
branch
from
August 16, 2026 06:37
2f37c7b to
3beef9e
Compare
This was referenced Aug 16, 2026
ryanbr
added a commit
that referenced
this pull request
Aug 16, 2026
…p union (#1304) The Data Sources "WHOOP data" badge counted under a hardcoded "my-whoop", under-reporting a strap banked under "whoop-<uuid>" (cosmetic follow-up to #1374). Count across the active-strap union instead: - days: daysMerged(vm.activeStrapId).size — the exact twin of Swift's repo.days (mergeActivityFileSteps(mergeDaily(imported, computed)); apple is not folded in on either side), so the Android count matches iOS, including a strap-only user's computed-only ("-noop") days. - workouts: workoutsUnion(...).size (deduped union). - HR: latestHrSampleTsUnion(...) (newest across the union; null -> no HR). Single-WHOOP / import-only (activeStrapId == "my-whoop") collapses to one id, byte-identical. Kotlin-only: Swift's badge already counts via the unioned repo.days (verified the refresh() assignment) and dataVolumeSnapshot() unions — iOS never under-counted. Locked by MultiWhoopSourceUnionTest; 321 data+ai tests green.
simoncad7
pushed a commit
to simoncad7/noop
that referenced
this pull request
Aug 17, 2026
…p union (ryanbr#1304) The Data Sources "WHOOP data" badge counted under a hardcoded "my-whoop", under-reporting a strap banked under "whoop-<uuid>" (cosmetic follow-up to ryanbr#1374). Count across the active-strap union instead: - days: daysMerged(vm.activeStrapId).size — the exact twin of Swift's repo.days (mergeActivityFileSteps(mergeDaily(imported, computed)); apple is not folded in on either side), so the Android count matches iOS, including a strap-only user's computed-only ("-noop") days. - workouts: workoutsUnion(...).size (deduped union). - HR: latestHrSampleTsUnion(...) (newest across the union; null -> no HR). Single-WHOOP / import-only (activeStrapId == "my-whoop") collapses to one id, byte-identical. Kotlin-only: Swift's badge already counts via the unioned repo.days (verified the refresh() assignment) and dataVolumeSnapshot() unions — iOS never under-counted. Locked by MultiWhoopSourceUnionTest; 321 data+ai tests 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.
Part of #1304 (the #512-class sweep).
What the sweep found
The Swift side is already union-safe everywhere (
Repository.series/workoutRows/habitualMidsleepSec,CsvExport's active∪canonical,MetricCatalogis display-only). So every remaining #512-class bug is a Kotlin caller handing the literal"my-whoop"into a union-aware repo method — which collapses the union (importedSourceIdsFor("my-whoop") == ["my-whoop"]) and re-drops a 2nd strap's data (banked underwhoop-<uuid>).The fix — thread the active strap id
CoupledScreensleep + habitualsleepSessions("my-whoop")/habitualMidsleepSec("my-whoop")sleepSessionsUnion/computedSleepSessionsUnion/habitualMidsleepSec(vm.activeStrapId)— matchesSleepScreenTodayScreensleepToday herosleepSessions("my-whoop")sleepSessionsUnion(viewModel.activeStrapId)(like thehrBucketsUnionread one line above)TodayScreenbattery estimatebatterySamples("my-whoop")batterySamples(viewModel.activeStrapId)WhoopConnectionServicenotification + batteryrecentDaysMergedFlow("my-whoop")/batterySamples("my-whoop")(application as NoopApplication).activeDeviceId— same accessor already used at:606AiCoachdaily + R-R + Lab markersdaysMerged/rrIntervals/labMarkers("my-whoop")CoachViewModel) — matches SwiftAICoach(repo.deviceId)AiCoachjournal stays canonical"my-whoop"(user-global, not strap telemetry) — matching SwiftAICoach.journalEntries().Deliberately NOT changed (verified not #512-class)
stressandsleep_performance/sleep_need_min(TodayScreen/StressScreen/TrendsReport, CoupledScreen:127) — these are imported metrics that live under the canonicalmy-whoopby definition (a 2nd BLE strap produces none; Swift'sseries()union collapses to the same thing). The DemoSeeder comment confirms stress is "derived EXACTLY as a real WHOOP import derives it… under my-whoop." Touching them would fabricate a bug, not fix one.ShortcutHealthImport.forbiddenSources— a write-target guard (target is the constantapple-health), never enumerates strap ids. Red herring; the guard holds.DataSourcesScreenWHOOP badge counts — under-report a 2nd strap but only cosmetically (no union-aware count variant exists). Left as a minor follow-up, noted in Multi-WHOOP: sweep the hardcoded "my-whoop" assumptions (the #512 class) #1304.Test & verification
MultiWhoopSourceUnionTestpins the collapse-vs-union contract every fixed caller now depends on (pure companion resolvers, twin ofWorkoutHrDeviceKeyTest).:app:compileFullDebugKotlinclean; 319com.noop.data+com.noop.aiunit tests green (run locally — Android tests aren't in default CI).Follow-ups for #1304:
DataSourcesScreencounts (needs union-aware count variants), and the companion #1193 (serial identity) which would shrink this surface at the source.