Skip to content

Workouts: read HR under the recording strap's id, not a hardcoded default (multi-WHOOP) - #512

Merged
ryanbr merged 1 commit into
mainfrom
fix/workout-hr-device-key-510
Jul 16, 2026
Merged

ryanbr merged 1 commit into
mainfrom
fix/workout-hr-device-key-510

Conversation

@ryanbr

@ryanbr ryanbr commented Jul 16, 2026 •

Copy link
Copy Markdown
Owner

What

fillWorkoutHrFromStrap (the Today/Workouts display fill from #77) read every workout's HR window under a hardcoded "my-whoop" default. A strap-native workout recorded on a second WHOOP — registered under whoop-<mac>, with its hrSample rows banked under that id — therefore read an empty window, so stats.n < minSamples returned the row unchanged: its Avg HR / Max HR were never reconciled from the strap trace, and a null Effort (strain) was never recomputed.

Single-WHOOP installs keep the canonical my-whoop id, which matched the default. This only bit multi-WHOOP users.

Not calories. This fill only writes avgHr / maxHr / strain; energyKcal is computed by the detector under the day owner (which already resolves multi-device correctly). And because failure returns the row unchanged, detected rows keep their stored Avg/Max — the user-visible win here is the Effort recompute (and Avg-HR reconcile / manual-row HR fill) landing on the correct strap.

Fix

Resolve the HR read key per row, from the device that actually recorded the workout:

  • Strap-native rows (source == "manual" or a detected "<id>-noop") were charted/zoned/scored from their own strap trace → key on that strap. Strip the computed -noop suffix to reach the raw hrSample id (a detected bout lives under "<id>-noop"; its HR under "<id>").
  • Imported rows (Apple / HC / WHOOP CSV / activity file) carry no strap HR; Add compact Android widget #77 fills them from the strapDeviceId default (my-whoop), byte-identical to today. Which strap was worn during an imported session on a multi-strap install is undetermined, so that case is deliberately left as-is.

Classification lives in one place (isStrapNativeWorkout) shared by the fill and the new workoutHrDeviceId helper.

Why it's safe

  • Single-device behaviour is byte-identical: a detected my-whoop-noop row strips to my-whoop (== the old default); imported rows still read my-whoop.
  • The fill never nulls a present value (failure returns the row unchanged); no change to the strain math, the minSamples/cap accounting, or the imported-fill path.

Test

WorkoutHrDeviceKeyTest (JVM, 6 cases): pins the strap-native vs imported classification and the per-row key resolution — including that a 2nd-WHOOP detected row reads its own strap and that imported rows follow the supplied strap (preserving #77). compileFullDebugKotlin + the test pass locally.

Scope / follow-ups

  • Kotlin only. The Swift twin reconcileWorkoutHrWithTrace reads under the Repository's single active deviceId, and its read-model WorkoutRow (WhoopStore, Codable) carries no deviceId — parity means threading the source device through the workout fetch → dedup → reconcile pipeline. Separate, app-compile-verified follow-up (app-target Swift has no default CI).
  • Surfaced by missing Effort Score and no HR+Calories for Workouts #510 (a single-WHOOP report — so this is not confirmed to be that reporter's root cause, which is still under investigation via a debug export). This PR fixes the multi-WHOOP class the review turned up.

@ryanbr
ryanbr force-pushed the fix/workout-hr-device-key-510 branch from de57641 to e12b9be Compare July 16, 2026 09:49
…ault

fillWorkoutHrFromStrap read every workout's HR window under a hardcoded
"my-whoop". A strap-native workout recorded on a SECOND WHOOP (id
"whoop-<mac>", its hrSample rows banked under that id) therefore read an
empty window, so `stats.n < minSamples` returned the row UNCHANGED: its
Avg HR / Max HR were never reconciled from the strap trace and a null Effort
(strain) was never recomputed. Single-WHOOP installs keep the canonical
"my-whoop" id, so they matched the default and were unaffected — this only
bit multi-WHOOP users.

(Calories/energyKcal are NOT touched by this fill — they're computed by the
detector under the day owner, which already resolves multi-device correctly.
The fill only sets avgHr / maxHr / strain.)

Resolve the HR read key per row from the recording device instead:
 - STRAP-NATIVE rows (source "manual" / detected "<id>-noop") were charted
   and zoned from their OWN strap's trace, so key on that strap — strip the
   computed "-noop" suffix to reach the raw hrSample id.
 - IMPORTED rows (Apple Health / Health Connect / WHOOP CSV / activity file)
   carry no strap HR; #77 still fills them from the strapDeviceId default, so
   their behaviour is byte-identical (which strap was worn during an imported
   session on a multi-strap install is undetermined and left as-is).

Single-device behaviour is unchanged (detected "my-whoop-noop" strips to
"my-whoop", same as the old default). New JVM test pins the classification
and the per-row key resolution. Surfaced by #510 (a single-WHOOP report whose
root cause is still under investigation); this fixes the multi-WHOOP class.

Kotlin only; the Swift twin (reconcileWorkoutHrWithTrace) reads under the
Repository's single active id and its read-model WorkoutRow carries no
deviceId — a parity follow-up will thread the source device through the
workout fetch.
@ryanbr
ryanbr force-pushed the fix/workout-hr-device-key-510 branch from e12b9be to 8098a5b Compare July 16, 2026 09:51
@ryanbr
ryanbr merged commit 28f5b28 into main Jul 16, 2026
1 check failed
@ryanbr
ryanbr deleted the fix/workout-hr-device-key-510 branch July 16, 2026 09:55
ryanbr added a commit that referenced this pull request Jul 16, 2026
…#514)

The 9.0.1 release generated its AppChangelog title as a raw Kotlin literal
(title = "German, French & Spanish, ..."), unlike every other entry which
uses uiString(R.string.l10n_app_changelog_...). The full-tree i18n audit flags
the raw literal, and since i18n-coverage runs only on pull_request (never on
the release's push to main), it landed unaudited and now red-checks every open
PR (#507, #512, #513) on a line none of them touched.

Localize it like the prior entries: title -> uiString(R.string.l10n_app_changelog_german_french_spanish_pull_to_sync_1109bda2),
with the string added to values/ (en) + the de/es/fr focus locales. Key follows
the established scheme (first-6-word slug + sha1(title)[:8]; verified it
reproduces the 9.0.0/8.7.0/8.6.2 keys).

i18n_audit.py --ci: OK no hardcoded literals. compileFullDebugKotlin: OK.
Apple side is unaffected (SwiftUI auto-extracts literals).
ryanbr added a commit that referenced this pull request Jul 16, 2026
…ve id (#513)

Swift twin of #512. reconcileWorkoutHrWithTrace read every workout's HR window
under the Repository's single active deviceId, so a bout auto-detected on a
SECOND WHOOP (its hrSample rows banked under that strap's id) read the active
strap's empty window and its Avg HR / Effort went un-reconciled. Single-WHOOP
installs matched the active id, so only multi-WHOOP users were affected.

Resolve the read key per row via the new nonisolated static
Repository.workoutHrDeviceId(source:activeStrapId:): a DETECTED row's own
`source` IS its computed strap id ("<base>-noop"), so strip the suffix to read
HR under the raw "<base>". MANUAL and IMPORTED rows carry no strap id in
`source`, so they reconcile against the active strap as before (single-device:
"<base>-noop" strips to the active id, so the read is byte-identical).

Display-only projection (never persisted). New WorkoutHrDeviceKeyTests pins the
resolution. Parity note: the Kotlin twin also keys MANUAL rows on their stored
deviceId; the Swift WorkoutRow read-model carries no deviceId, so a manual
session created on a non-active strap reconciles here against the active strap
— a minor, display-only divergence; the detected-dominant multi-WHOOP bug is
fixed on both platforms.
ryanbr added a commit that referenced this pull request Jul 17, 2026
Numbers are main's current 292/203 +1, matching the previous staging commit's
pattern, so the .staging app upgrades cleanly over the last testing build (290)
and stays above main's released high-water.

Carries everything merged since the 9.0.1 release:
 - Workouts read HR under the recording strap, not a hardcoded id (#512 Kotlin,
   #513 Swift twin)
 - Oura 0x60/0x80 IBI decoders corrected to the ring's real byte layout (#511)
 - Whoop5Config enable_sig12 corrected to ASCII '1' (#522)
 - Sleep time edits save as one validated window (#525)
 - "Recompute this night" for deleted sleep windows (#526)
 - Stress marker values stay readable (#507)
 - Faster history sync + faster Bluetooth link, both EXPERIMENTAL and default-off
   (#536/#537/#538) - the whole point of this build is field data on those
 - i18n changelog-title fix + main-push audit (#514/#535)

versionName stays 9.0.2 here for the staging identity only; the real 9.0.2
release still bumps main separately (and must be raised past 293 first).
DX23876 pushed a commit to DX23876/noop that referenced this pull request Jul 22, 2026
…ault (ryanbr#512)

fillWorkoutHrFromStrap read every workout's HR window under a hardcoded
"my-whoop". A strap-native workout recorded on a SECOND WHOOP (id
"whoop-<mac>", its hrSample rows banked under that id) therefore read an
empty window, so `stats.n < minSamples` returned the row UNCHANGED: its
Avg HR / Max HR were never reconciled from the strap trace and a null Effort
(strain) was never recomputed. Single-WHOOP installs keep the canonical
"my-whoop" id, so they matched the default and were unaffected — this only
bit multi-WHOOP users.

(Calories/energyKcal are NOT touched by this fill — they're computed by the
detector under the day owner, which already resolves multi-device correctly.
The fill only sets avgHr / maxHr / strain.)

Resolve the HR read key per row from the recording device instead:
 - STRAP-NATIVE rows (source "manual" / detected "<id>-noop") were charted
   and zoned from their OWN strap's trace, so key on that strap — strip the
   computed "-noop" suffix to reach the raw hrSample id.
 - IMPORTED rows (Apple Health / Health Connect / WHOOP CSV / activity file)
   carry no strap HR; ryanbr#77 still fills them from the strapDeviceId default, so
   their behaviour is byte-identical (which strap was worn during an imported
   session on a multi-strap install is undetermined and left as-is).

Single-device behaviour is unchanged (detected "my-whoop-noop" strips to
"my-whoop", same as the old default). New JVM test pins the classification
and the per-row key resolution. Surfaced by ryanbr#510 (a single-WHOOP report whose
root cause is still under investigation); this fixes the multi-WHOOP class.

Kotlin only; the Swift twin (reconcileWorkoutHrWithTrace) reads under the
Repository's single active id and its read-model WorkoutRow carries no
deviceId — a parity follow-up will thread the source device through the
workout fetch.
DX23876 pushed a commit to DX23876/noop that referenced this pull request Jul 22, 2026
…ryanbr#514)

The 9.0.1 release generated its AppChangelog title as a raw Kotlin literal
(title = "German, French & Spanish, ..."), unlike every other entry which
uses uiString(R.string.l10n_app_changelog_...). The full-tree i18n audit flags
the raw literal, and since i18n-coverage runs only on pull_request (never on
the release's push to main), it landed unaudited and now red-checks every open
PR (ryanbr#507, ryanbr#512, ryanbr#513) on a line none of them touched.

Localize it like the prior entries: title -> uiString(R.string.l10n_app_changelog_german_french_spanish_pull_to_sync_1109bda2),
with the string added to values/ (en) + the de/es/fr focus locales. Key follows
the established scheme (first-6-word slug + sha1(title)[:8]; verified it
reproduces the 9.0.0/8.7.0/8.6.2 keys).

i18n_audit.py --ci: OK no hardcoded literals. compileFullDebugKotlin: OK.
Apple side is unaffected (SwiftUI auto-extracts literals).
DX23876 pushed a commit to DX23876/noop that referenced this pull request Jul 22, 2026
…ve id (ryanbr#513)

Swift twin of ryanbr#512. reconcileWorkoutHrWithTrace read every workout's HR window
under the Repository's single active deviceId, so a bout auto-detected on a
SECOND WHOOP (its hrSample rows banked under that strap's id) read the active
strap's empty window and its Avg HR / Effort went un-reconciled. Single-WHOOP
installs matched the active id, so only multi-WHOOP users were affected.

Resolve the read key per row via the new nonisolated static
Repository.workoutHrDeviceId(source:activeStrapId:): a DETECTED row's own
`source` IS its computed strap id ("<base>-noop"), so strip the suffix to read
HR under the raw "<base>". MANUAL and IMPORTED rows carry no strap id in
`source`, so they reconcile against the active strap as before (single-device:
"<base>-noop" strips to the active id, so the read is byte-identical).

Display-only projection (never persisted). New WorkoutHrDeviceKeyTests pins the
resolution. Parity note: the Kotlin twin also keys MANUAL rows on their stored
deviceId; the Swift WorkoutRow read-model carries no deviceId, so a manual
session created on a non-active strap reconciles here against the active strap
— a minor, display-only divergence; the detected-dominant multi-WHOOP bug is
fixed on both platforms.
ryanbr added a commit that referenced this pull request Jul 24, 2026
…ed my-whoop (#767, #717) (#777)

Workout auto-detection silently stopped for live-BLE users while sleep,
charge and everything else kept syncing. Root cause: AutoWorkoutNudge read
HR via repo.hrSamples("my-whoop", ...) — a hardcoded id — but a live strap
banks its raw under its OWN fresh id ("whoop-<uuid>", #908), so the read
returned nothing and detect() bailed at 'hr.size < 2'. The mid-July
active-strap-id migration (#175/#512/#513) moved sleep/charge/health/iOS
reads to the active-strap union but missed this one.

Fix: read the hrSamplesUnion(activeDeviceId) — importedSourceIds(active) ∪
my-whoop, deduped — exactly what every other Today read and iOS
Repository.autoDetectCandidate() already use. Android-only; iOS was never
affected (it already reads the union). No behaviour change for single-WHOOP
installs, whose active id resolves to my-whoop (one id, identical read).

App-target Android; not compiled here (aapt2 x86-only on this host) - run
./gradlew compileFullDebugKotlin before merge.
ryanbr added a commit that referenced this pull request Aug 16, 2026
…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 added a commit that referenced this pull request Aug 16, 2026
…on the Kotlin #512-class paths (#1304)

The Swift side is already union-safe everywhere, so every remaining #512-class bug was 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 strap banked under "whoop-<uuid>". Thread the active strap id instead:

- CoupledScreen: sleep + habitual midsleep via sleepSessionsUnion/computedSleepSessionsUnion/habitualMidsleepSec(vm.activeStrapId), matching SleepScreen.
- TodayScreen: the sleepToday hero unions, and the battery estimate reads the active strap's own SoC.
- WhoopConnectionService: notification recentDaysMergedFlow + battery read use NoopApplication.activeDeviceId (as at :606).
- AiCoach: daysMerged/rrIntervals/Lab markers read the active strap (from CoachViewModel), matching Swift AICoach; journal stays canonical (user-global).

Verified NOT #512-class and left alone: stress + sleep_performance/need are imported metrics (canonical by definition); ShortcutHealthImport.forbiddenSources is a write guard. DataSourcesScreen badge under-count is a cosmetic follow-up (no union-aware count variant on either platform).

Parity confirmed by reading each Swift twin (allSleepSessions / habitualMidsleepSec() / repo.sleeps / AICoach repo.deviceId) — all already union; Kotlin-only change. Locked by MultiWhoopSourceUnionTest; 319 data+ai unit tests green.
simoncad7 pushed a commit to simoncad7/noop that referenced this pull request Aug 17, 2026
…ault (ryanbr#512)

fillWorkoutHrFromStrap read every workout's HR window under a hardcoded
"my-whoop". A strap-native workout recorded on a SECOND WHOOP (id
"whoop-<mac>", its hrSample rows banked under that id) therefore read an
empty window, so `stats.n < minSamples` returned the row UNCHANGED: its
Avg HR / Max HR were never reconciled from the strap trace and a null Effort
(strain) was never recomputed. Single-WHOOP installs keep the canonical
"my-whoop" id, so they matched the default and were unaffected — this only
bit multi-WHOOP users.

(Calories/energyKcal are NOT touched by this fill — they're computed by the
detector under the day owner, which already resolves multi-device correctly.
The fill only sets avgHr / maxHr / strain.)

Resolve the HR read key per row from the recording device instead:
 - STRAP-NATIVE rows (source "manual" / detected "<id>-noop") were charted
   and zoned from their OWN strap's trace, so key on that strap — strip the
   computed "-noop" suffix to reach the raw hrSample id.
 - IMPORTED rows (Apple Health / Health Connect / WHOOP CSV / activity file)
   carry no strap HR; ryanbr#77 still fills them from the strapDeviceId default, so
   their behaviour is byte-identical (which strap was worn during an imported
   session on a multi-strap install is undetermined and left as-is).

Single-device behaviour is unchanged (detected "my-whoop-noop" strips to
"my-whoop", same as the old default). New JVM test pins the classification
and the per-row key resolution. Surfaced by ryanbr#510 (a single-WHOOP report whose
root cause is still under investigation); this fixes the multi-WHOOP class.

Kotlin only; the Swift twin (reconcileWorkoutHrWithTrace) reads under the
Repository's single active id and its read-model WorkoutRow carries no
deviceId — a parity follow-up will thread the source device through the
workout fetch.
simoncad7 pushed a commit to simoncad7/noop that referenced this pull request Aug 17, 2026
…ryanbr#514)

The 9.0.1 release generated its AppChangelog title as a raw Kotlin literal
(title = "German, French & Spanish, ..."), unlike every other entry which
uses uiString(R.string.l10n_app_changelog_...). The full-tree i18n audit flags
the raw literal, and since i18n-coverage runs only on pull_request (never on
the release's push to main), it landed unaudited and now red-checks every open
PR (ryanbr#507, ryanbr#512, ryanbr#513) on a line none of them touched.

Localize it like the prior entries: title -> uiString(R.string.l10n_app_changelog_german_french_spanish_pull_to_sync_1109bda2),
with the string added to values/ (en) + the de/es/fr focus locales. Key follows
the established scheme (first-6-word slug + sha1(title)[:8]; verified it
reproduces the 9.0.0/8.7.0/8.6.2 keys).

i18n_audit.py --ci: OK no hardcoded literals. compileFullDebugKotlin: OK.
Apple side is unaffected (SwiftUI auto-extracts literals).
simoncad7 pushed a commit to simoncad7/noop that referenced this pull request Aug 17, 2026
…ve id (ryanbr#513)

Swift twin of ryanbr#512. reconcileWorkoutHrWithTrace read every workout's HR window
under the Repository's single active deviceId, so a bout auto-detected on a
SECOND WHOOP (its hrSample rows banked under that strap's id) read the active
strap's empty window and its Avg HR / Effort went un-reconciled. Single-WHOOP
installs matched the active id, so only multi-WHOOP users were affected.

Resolve the read key per row via the new nonisolated static
Repository.workoutHrDeviceId(source:activeStrapId:): a DETECTED row's own
`source` IS its computed strap id ("<base>-noop"), so strip the suffix to read
HR under the raw "<base>". MANUAL and IMPORTED rows carry no strap id in
`source`, so they reconcile against the active strap as before (single-device:
"<base>-noop" strips to the active id, so the read is byte-identical).

Display-only projection (never persisted). New WorkoutHrDeviceKeyTests pins the
resolution. Parity note: the Kotlin twin also keys MANUAL rows on their stored
deviceId; the Swift WorkoutRow read-model carries no deviceId, so a manual
session created on a non-active strap reconciles here against the active strap
— a minor, display-only divergence; the detected-dominant multi-WHOOP bug is
fixed on both platforms.
simoncad7 pushed a commit to simoncad7/noop that referenced this pull request Aug 17, 2026
…ed my-whoop (ryanbr#767, ryanbr#717) (ryanbr#777)

Workout auto-detection silently stopped for live-BLE users while sleep,
charge and everything else kept syncing. Root cause: AutoWorkoutNudge read
HR via repo.hrSamples("my-whoop", ...) — a hardcoded id — but a live strap
banks its raw under its OWN fresh id ("whoop-<uuid>", ryanbr#908), so the read
returned nothing and detect() bailed at 'hr.size < 2'. The mid-July
active-strap-id migration (ryanbr#175/ryanbr#512/ryanbr#513) moved sleep/charge/health/iOS
reads to the active-strap union but missed this one.

Fix: read the hrSamplesUnion(activeDeviceId) — importedSourceIds(active) ∪
my-whoop, deduped — exactly what every other Today read and iOS
Repository.autoDetectCandidate() already use. Android-only; iOS was never
affected (it already reads the union). No behaviour change for single-WHOOP
installs, whose active id resolves to my-whoop (one id, identical read).

App-target Android; not compiled here (aapt2 x86-only on this host) - run
./gradlew compileFullDebugKotlin before merge.
simoncad7 pushed a commit to simoncad7/noop that referenced this pull request Aug 17, 2026
…on the Kotlin ryanbr#512-class paths (ryanbr#1304)

The Swift side is already union-safe everywhere, so every remaining ryanbr#512-class bug was 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 strap banked under "whoop-<uuid>". Thread the active strap id instead:

- CoupledScreen: sleep + habitual midsleep via sleepSessionsUnion/computedSleepSessionsUnion/habitualMidsleepSec(vm.activeStrapId), matching SleepScreen.
- TodayScreen: the sleepToday hero unions, and the battery estimate reads the active strap's own SoC.
- WhoopConnectionService: notification recentDaysMergedFlow + battery read use NoopApplication.activeDeviceId (as at :606).
- AiCoach: daysMerged/rrIntervals/Lab markers read the active strap (from CoachViewModel), matching Swift AICoach; journal stays canonical (user-global).

Verified NOT ryanbr#512-class and left alone: stress + sleep_performance/need are imported metrics (canonical by definition); ShortcutHealthImport.forbiddenSources is a write guard. DataSourcesScreen badge under-count is a cosmetic follow-up (no union-aware count variant on either platform).

Parity confirmed by reading each Swift twin (allSleepSessions / habitualMidsleepSec() / repo.sleeps / AICoach repo.deviceId) — all already union; Kotlin-only change. Locked by MultiWhoopSourceUnionTest; 319 data+ai unit tests green.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant