Repository navigation
Raw sensor export reads the active strap, not the legacy id (#175) - #924
Merged
Merged
Conversation
RawSensorExport.export defaulted deviceId to "my-whoop" and its single caller in SettingsScreen took the default. After a strap re-add the active id is no longer the canonical one, so the export wrote the OLD strap's streams — silently, into the CSV people attach to bug reports. The diagnostic most likely to be read by someone debugging a re-added strap was the one describing a different strap. This is the #172/#175 shape verbatim: a defaulted strap id that no caller overrides, which compiles, passes every test, and only misbehaves after a re-add. ResolvedSeriesCallSiteAuditTest exists because eight surfaces had it. The sibling export twelve hundred lines up (WhoopCsvExporter.exportZip at SettingsScreen:603) already threads vm.activeStrapId, so the codebase knew the rule and this call site missed it. Fixed by removing the default rather than only passing the argument: with `deviceId: String` required, a future caller cannot silently inherit the wrong strap, and the compiler enforces what an audit test would otherwise have to notice. Checked first that export() has exactly one caller, so requiring it breaks nothing. Ported from tanarchytan/noop e4f508f, which fixed the same call site by passing the argument; credited in the code comment. Not swept further here: WhoopRepository still defaults strapDeviceId on several reads (dismissedDetected, dismissedSleeps, relabelDetected, freshness). Those are a wider audit than this one-line fix and deserve their own pass, since each needs its callers checked individually.
ryanbr
force-pushed
the
fix/raw-export-active-strap
branch
from
July 28, 2026 05:42
344b9c6 to
cd4cc53
Compare
The Apple twin of the same defect. exportRawSensorCSV passed model.deviceId, a hardcoded let "my-whoop", where every other strap read in the app uses repo.deviceId — seeded with that id and then re-pointed to the registry's active strap once the store opens. After a remove+re-add the two diverge and the CSV described the wrong strap. Read on the MainActor before the Task hop, matching LiveSessionRunner.
simoncad7
pushed a commit
to simoncad7/noop
that referenced
this pull request
Aug 17, 2026
…) (ryanbr#924) * Raw sensor export reads the active strap, not the legacy id (ryanbr#175) RawSensorExport.export defaulted deviceId to "my-whoop" and its single caller in SettingsScreen took the default. After a strap re-add the active id is no longer the canonical one, so the export wrote the OLD strap's streams — silently, into the CSV people attach to bug reports. The diagnostic most likely to be read by someone debugging a re-added strap was the one describing a different strap. This is the ryanbr#172/ryanbr#175 shape verbatim: a defaulted strap id that no caller overrides, which compiles, passes every test, and only misbehaves after a re-add. ResolvedSeriesCallSiteAuditTest exists because eight surfaces had it. The sibling export twelve hundred lines up (WhoopCsvExporter.exportZip at SettingsScreen:603) already threads vm.activeStrapId, so the codebase knew the rule and this call site missed it. Fixed by removing the default rather than only passing the argument: with `deviceId: String` required, a future caller cannot silently inherit the wrong strap, and the compiler enforces what an audit test would otherwise have to notice. Checked first that export() has exactly one caller, so requiring it breaks nothing. Ported from tanarchytan/noop e4f508f, which fixed the same call site by passing the argument; credited in the code comment. Not swept further here: WhoopRepository still defaults strapDeviceId on several reads (dismissedDetected, dismissedSleeps, relabelDetected, freshness). Those are a wider audit than this one-line fix and deserve their own pass, since each needs its callers checked individually. * Raw sensor CSV export: use the active strap on iOS too The Apple twin of the same defect. exportRawSensorCSV passed model.deviceId, a hardcoded let "my-whoop", where every other strap read in the app uses repo.deviceId — seeded with that id and then re-pointed to the registry's active strap once the store opens. After a remove+re-add the two diverge and the CSV described the wrong strap. Read on the MainActor before the Task hop, matching LiveSessionRunner.
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.
The raw sensor CSV — the file people attach to bug reports — could describe a strap the user
isn't wearing. Fixed on both platforms.
Android.
RawSensorExport.exporttookdeviceId: String = "my-whoop"and its one caller(
SettingsScreen.kt:2082) took the default. After a remove+re-add the strap gets a freshwhoop-<uuid>that the collector writes today's raw under, so the export read the legacy id'sstreams instead. The sibling export four hundred lines up already threads
vm.activeStrapId.The default is removed rather than corrected, so the compiler asks at every future call site
instead of the next author having to remember. Verified there was exactly one caller first.
iOS/macOS. Same defect, worse form:
exportRawSensorCSVpassedmodel.deviceId, which is ahardcoded
let "my-whoop", where ~20 other strap reads userepo.deviceId— seeded with that idand then re-pointed to the registry's active strap by
adoptActiveDeviceId. Read on the MainActorbefore the
Taskhop, matchingLiveSessionRunner.This is #814 on the diagnostic path, and the #172/#175 shape that
ResolvedSeriesCallSiteAuditTestexists to catch.
Verification. Android compiles and its unit tests pass in CI. The Swift side is app-target
(
Strand/), which nothing compiles whileapp-build.ymlis disabled — so it is stated plainly: itis uncompiled. What supports it is that
LiveSessionRunner.persist()is the same four lines alreadyshipping — capture
repo.deviceId,Task {,await repo.storeHandle(), pass the captured id — andthe enclosing function mutates
@Statesynchronously, so it is MainActor-isolated. Same isolationshape as working code, but a local
xcodebuildbefore merge is still the thing that proves it.Not changed, deliberately. Two other
model.deviceIdreaders exist —WatchSessionBridge.swift:150and
StrandiOSApp.swift:58— andWhoopRepositorystill defaultsstrapDeviceIdon four Androidreads. Each needs its own caller audit; bundling them here would hide this one.
Android half ported from
tanarchytan/noope4f508f, which fixed the caller; the removed defaultand the Apple twin are additional.