Skip to content

Android: Missing Backend for Rhythm Screen - #2381

Merged
ryanbr merged 2 commits into
ryanbr:mainfrom
andigandhi:egc
Sep 23, 2026
Merged

ryanbr merged 2 commits into
ryanbr:mainfrom
andigandhi:egc

Conversation

@andigandhi

Copy link
Copy Markdown

What this PR does

The Rhythm screen was fully implemented but showed no data because it was hardcoded with empty inputs (night = null, windows = emptyList()).

This commit adds the missing data-loading layer (RhythmRoute) that mirrors macOS RhythmHost.load():

  • Add anyRegisteredWhoopId() to WhoopRepository to load data from any registered WHOOP device, not just the currently active one. This allows Rhythm to work offline with historical data.

  • Use sleepSessionsMerged() to search across both imported (raw) and computed ("-noop") sleep sessions, matching Swift allSleepSessions() which reads from both namespaces.

  • Select the most recent session via maxByOrNull { it.endTs } instead of lastOrNull(), ensuring we load the actual most recent sleep data rather than the last item in an unsorted list.

  • Window the night into 5-minute slices, gate each on stillness, and run the pure RhythmScreener engine over each window.

  • Use 14-day lookback window to match iOS allSleepSessions(days: 14).

Rhythm now displays data when sleep sessions exist in the database, even when no strap is currently connected as shown below.

Screenshot_20260921-205838_NOOP Staging

Type of change

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

How it was tested

It was tested on a real Android device using real life data.

Checklist

  • Swift package tests pass for any package I touched (swift test in Packages/<name>)
  • Android unit tests pass if I touched android/ (./gradlew testFullDebugUnitTest)
  • No new build warnings introduced
  • UI changes use only StrandDesign tokens — no hardcoded colors, fonts, or spacing
  • 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

Addresses #2380

@ryanbr

ryanbr commented Sep 22, 2026

Copy link
Copy Markdown
Owner

Reviewed at 17c6ad5. The diagnosis is right and the screen genuinely was dead: hardcoded night = null, windows = emptyList(), so all the UI work behind it never had anything to draw. Two good catches on the way, too. maxByOrNull { it.endTs } over lastOrNull() on a list with no guaranteed order is the kind of thing that works until it does not, and matching iOS's 14-day lookback rather than inventing a window is the right instinct.

I would hold it on one thing: the read path does not do what it says it does, and the plumbing it needs already exists.

The twins are already there

Swift's allSleepSessions unions rawPhysiologyReadIds and rawComputedReadIds. Android already has that exact pair, written for this:

  • sleepSessionsUnion: "ALL imported sleep BLOCKS across every registered WHOOP (active first, archived included, canonical last) ... Mirrors Swift Repository.unionSleepSessions"
  • computedSleepSessionsUnion: "Mirrors Swift Repository.unionComputedSleepSessions"

This uses sleepSessionsMerged(deviceId) instead, which resolves through importedSourceIds, and that is [deviceId, "my-whoop"]: the active strap and the canonical spine, nothing else. A night recorded under a third strap's id is invisible to it. So anyRegisteredWhoopId's doc, "matching Swift allSleepSessions behavior which reads across all registered devices", describes the function you want rather than the one being called: it reads across two.

Which makes the new resolver unnecessary

anyRegisteredWhoopId falls back to whoopIds.firstOrNull() over an unordered pairedDevices(), so on a two-strap install it can return a retired strap. That matters most in the case the feature exists for: the fallback only runs when activeDeviceId is null, which is exactly the offline, nothing-connected path Rhythm is meant to serve. Pick the wrong id there and importedSourceIds unions the wrong pair.

Swapping to the union functions makes all of that moot, because rawWhoopSourceIdsFor includes every registered WHOOP plus canonical regardless of which id it is handed. It is robust to a bad active id in a way importedSourceIds is not, which is why it exists.

So the change I would ask for is small and subtractive: drop anyRegisteredWhoopId, read through sleepSessionsUnion + computedSleepSessionsUnion, and the parity claim in the description becomes literally true. You would also match the imported-wins-per-day split Swift does after its union, which is worth a look while you are in there.

On tests

Unlike your Deep Timeline fix, this one is testable and I would ask for it. anyRegisteredWhoopId is a repository resolver, and MultiWhoopSourceUnionTest exists specifically to pin resolvers of this shape with a Proxy-based fake DAO, no Compose harness required. Its class doc is worth reading first: it pins the trap that passing a single literal id collapses a union and re-drops the second strap, which is a near relative of what is happening here.

If the union functions replace the resolver then there may be nothing new left to test, which is the better outcome of the two.

Not blocking

The screenshot is a good inclusion, and "tested on a real Android device using real life data" is the right claim to make for a screen with no test harness. The feature is wanted and the UI half is right; it is the read path that wants the existing plumbing rather than new plumbing.

@andigandhi

Copy link
Copy Markdown
Author

Thanks for the improvements! I had some troubles getting the sleep sessions detected.
Currently I'm not able to test the new Commit on a real phone, so take this Pull Request as WIP. Will update if I was able to test it.

@andigandhi

Copy link
Copy Markdown
Author

It still works with live data from last night on a real Android phone.

The Rhythm screen was fully implemented but showed no data because it
was hardcoded with empty inputs (night = null, windows = emptyList()).

This commit adds the missing data-loading layer (RhythmRoute) that
mirrors macOS RhythmHost.load():

- Add anyRegisteredWhoopId() to WhoopRepository to load data from any
  registered WHOOP device, not just the currently active one. This
  allows Rhythm to work offline with historical data.

- Use sleepSessionsMerged() to search across both imported (raw) and
  computed ("-noop") sleep sessions, matching Swift allSleepSessions()
  which reads from both namespaces.

- Select the most recent session via maxByOrNull { it.endTs } instead
  of lastOrNull(), ensuring we load the actual most recent sleep data
  rather than the last item in an unsorted list.

- Window the night into 5-minute slices, gate each on stillness, and
  run the pure RhythmScreener engine over each window.

- Use 14-day lookback window to match iOS allSleepSessions(days: 14).

Rhythm now displays data when sleep sessions exist in the database,
even when no strap is currently connected.
@ryanbr
ryanbr merged commit 1eeb1ff into ryanbr:main Sep 23, 2026
3 checks passed
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