Skip to content

fix(whoopstore): re-home legacy 'my-whoop' alias rows onto the registry device id - #2753

Open
pranav6226 wants to merge 5 commits into
ryanbr:mainfrom
pranav6226:fix-rr-owner-alias
Open

pranav6226 wants to merge 5 commits into
ryanbr:mainfrom
pranav6226:fix-rr-owner-alias

Conversation

@pranav6226

Copy link
Copy Markdown

What this PR does

An install that started on the legacy single-device flow and later paired the
same strap in the multi-device registry holds one strap's rows under two
device ids: the v15 seed id my-whoop (the write id is session-scoped, so
sessions launched while the seed row was active banked under it) and the
registry's whoop-… serial id. Scoring resolves one owner per day, so a
split day only ever sees one side:

  • seed-owned days have HR but read 0 RR rows — sleep stages, HRV never computes;
  • registry-owned days have RR but a partial HR slice — the stager returns NO-NIGHT.

Neither side alone can score a night, so an affected install shows live HR
only, indefinitely, with healthy sync throughout.

The fix heals the store in place:

  • Migration v48 (v48-legacy-whoop-alias-rehome, data-only, no schema
    change) runs DeviceRegistryStore.rehomeLegacyWhoopAlias at upgrade time.
  • The same guarded heal re-runs at every bootstrap (BLEManager
    bootstrap path), so a split created after the upgrade also self-heals.
  • The merge reuses adoptSerialIdentity's semantics: the active registry id
    wins primary-key clashes, the -noop computed sibling travels with it, and
    the seed rows are dropped after the move.

Guards make it a no-op unless the two ids provably denote the same single
strap: the seed row exists; the active device is a registry WHOOP id other
than the seed; there are exactly two WHOOP rows in total; the seed never
adopted a different peripheral than the active device; and the seed actually
holds banked rows.

Refs #814 — that PR's read spine UNIONs the two ids at read time; this change
removes the split at the data level so the union stops finding two owners.
Maintainers decide if/when the report that led here is resolved; no closing
keyword intended.

Type of change

  • Bug fix
  • New feature
  • Refactor / cleanup
  • Documentation
  • CI / tooling

How it was tested

New LegacyWhoopAliasRehomeTests (7 tests) covering the merge and every
guard: split store merges onto the active serial id; no-op when the seed is
the active device; no-op when another device is active; refusal when the seed
adopted a different peripheral; refusal with a third WHOOP row; no-op when
the seed holds no rows; and the v48 migration healing a split install
end-to-end.

Package suite (Linux, per docs/BUILD.md snapshot-SQLite recipe):
cd Packages/WhoopStore && swift test — 628 tests executed, 1 skipped,
0 failures, including the 7 above. (macOS CI swift-packages.yml is the
authority; the Linux run is a convenience check. The run used the pinned
GRDB 6.29.3 sources supplied from a local path — the sandbox's git fetch
of the dependency does not complete — with every GRDB file hash-verified
against the upstream v6.29.3 tree before use.)

On hardware: WHOOP MG, firmware 50.42.1.0, iOS build carrying this fix. The
test install had a real split store — two nights unscored (NO-NIGHT), HRV
never computed, sync healthy. On first launch post-upgrade the rows merged
and the same nights scored: sleep detected, HRV present, and later nights
score normally. The bootstrap call site is in the BLE-adjacent launch path
and was exercised on the device, not just in the package suite.

Cross-platform parity (called out per AGENTS.md)

The Kotlin twin — DeviceRegistryStore.rehomeLegacyWhoopAlias as Room
MIGRATION_41_42 plus the same bootstrap call — is required for full parity
and is not in this PR; the Swift function doc says so in terms. The
split originates in the legacy seed-id install flow; whether Room installs
can reach the same state is a maintainer call. Both schema_oracle.json
copies gain v48-legacy-whoop-alias-rehome so the shared canonical
migration list stays in step. Happy to add the Room twin to this PR or as a
follow-up, whichever the maintainers prefer.

Checklist

  • Swift package tests pass for any package I touched (swift test in Packages/<name>)
  • Android unit tests pass if I touched android/ (only the shared schema-oracle fixture under android/app/src/test/resources/ — no Kotlin changed)
  • No new build warnings introduced
  • UI changes use only StrandDesign tokens — no hardcoded colors, fonts, or spacing (no UI changed)
  • No hardcoded hex frame bytes; protocol facts live in the schema / decoders
  • Follows the conventions in docs/CONTRIBUTING.md
  • I did not commit generated output (Strand.xcodeproj/) or any secrets/keystores

Related issues

Refs #814

…ry device id

Rows written under the session-scoped 'my-whoop' alias split a strap's
data across two device ids, so per-day single-owner scoring never saw
HR+RR together: sleep staging returned staged-none and HRV never
computed. Migration v48 plus a guarded per-launch re-check in
bootstrapStore re-homes the alias rows onto the registry serial id;
stored data self-heals on first launch. Adds LegacyWhoopAliasRehomeTests.
@ryanbr

ryanbr commented Oct 13, 2026

Copy link
Copy Markdown
Owner

Approved the workflow runs. Swift Packages CI is green, so the v48 migration and all seven LegacyWhoopAliasRehomeTests hold up. Tools Python CI is red, and it is not the schema oracle. It is the parity repository-acceptance suite, which runs inside the linux-capture job, 2 failures out of 50.

1. The Kotlin twin is a claim the ledger cannot resolve

ScanError(rule='dead-twin-reference',
          path='Packages/WhoopStore/Sources/WhoopStore/DeviceRegistryStore.swift',
          line=282,
          text='Kotlin target DeviceRegistryStore.rehomeLegacyWhoopAlias does not resolve')
ScanError(rule='unresolved-attached-function-claim',
          path='Packages/WhoopStore/Sources/WhoopStore/DeviceRegistryStore.swift',
          line=282,
          text='attached twin claim DeviceRegistryStore.rehomeLegacyWhoopAlias does not resolve to an inventory function')

That is the line reading Kotlin twin: DeviceRegistryStore.rehomeLegacyWhoopAlias (Room MIGRATION_41_42 + the same bootstrap call), required for parity, not part of this change. You called it out yourself, so nothing here is a surprise, but naming a Kotlin target is a claim the scanner checks rather than a note it reads, and there is no rehomeLegacyWhoopAlias anywhere under android/app/src/main. Either the Kotlin side lands with it, or the claim comes out of the comment and the function is declared Apple-only for now.

2. The parity authority is stale

functions is checked in at 4538 while the current sources compute 4540, with a different sha256. Tools/parity_twin_map.json is not in this PR, so the derived authority was never refreshed after two new public declarations inside Packages/, which is in Swift parity scope. A --refresh-derived pass as its own metadata-only commit settles it.

3. The Android schema oracle asserts something Android does not do

v48-legacy-whoop-alias-rehome was added to both copies, Packages/WhoopStore/Tests/WhoopStoreTests/Resources/schema_oracle.json and android/app/src/test/resources/schema_oracle.json, while this PR carries no Android source at all. So Android's oracle now declares a migration Android never performs, and Android CI passed regardless, which means nothing on that side catches the gap. That one worries me more than the two above, because it is silent. I would revert the Android line until there is an Android migration behind it, and keep the Apple copy.

Worth saying separately

The split you describe is not Apple-only. Android seeds the same my-whoop row and its read spine resolves the same union, so an Android install reaches the identical two-id state, and as written this heals only one of the two platforms. If you would rather not take the Kotlin twin on in this PR, splitting it out and saying so in the comment is fine, but the Android oracle line should travel with the Kotlin work rather than ahead of it.

For what it is worth, the diagnosis reads correct to me and the guard set is the right shape: requiring exactly two WHOOP rows, refusing a seed that adopted a different peripheral, and requiring the seed to actually hold rows are each the guard I would want before a merge of this kind touches anybody's history.

The doc comment's 'Kotlin twin:' line parsed as an attached twin claim
for a function that does not exist, failing the repository acceptance
suite with dead-twin-reference. Reword it to plain prose and add
platform_specific dispositions for rehomeLegacyWhoopAlias,
rehomeLegacyWhoopAliasIfNeeded and legacyWhoopId. The Room re-home is
a follow-up, called out in the PR.
The Android oracle declared v48-legacy-whoop-alias-rehome, a migration
Android does not perform; nothing on that side catches the gap. The
entry travels with the future Room re-home instead of ahead of it.
The WhoopStore (Apple) oracle entry stays.
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.

2 participants