Skip to content

Distinguish "connected, no data" from "not connected" (#612) - #826

Merged
ryanbr merged 5 commits into
ryanbr:mainfrom
digitalerdude:fix/612-connected-no-data
Aug 2, 2026
Merged

ryanbr merged 5 commits into
ryanbr:mainfrom
digitalerdude:fix/612-connected-no-data

Conversation

@digitalerdude

@digitalerdude digitalerdude commented Jul 26, 2026 •

Copy link
Copy Markdown

Problem

#612 describes a strap that connects fine but never actually hands over data
(no live HR, offloads keep coming back empty) — and every surface still
honestly says "connected", while the Recording chip specifically does not:
RecordingState.resolve only checks connected in its first branch
(.recording). Fall through that, and a strap that's genuinely connected but
has never once synced — a fresh pairing, or an established WHOOP-4/generic
strap whose last several offloads are a sustained empty streak — reads
"Not recording. Strap not connected. Tap to connect.", which is simply
false.

Part 1 of #612 (surfacing the age of the last banked night on the baseline
card) already shipped. This is part 2, the "connected, but no data" signal,
scoped specifically to the gap that isn't already covered by #580's
.historyExperimental override — which still takes priority for a WHOOP
5/MG past its 2-consecutive-empty-offload threshold, unchanged by this PR.

What changed

  • LiveState.sustainedEmptyOffload (new @Published bool, both platforms):
    exposes the WHOOP-4/generic empty-offload streak that EmptySyncTracker
    (and its Kotlin twin) already tracked, but which previously only reached
    the UI as free text inside lastSyncError. Set on every HISTORY_COMPLETE
    (so a productive sync clears it right away), reset on disconnect (the
    display flag only — the underlying streak counter deliberately survives a
    reconnect, unchanged existing behavior).
  • RecordingState.connectedNoData (Swift + Kotlin, identical case/priority):
    connected && heartRate == nil && (lastSyncedAt == nil || sustainedEmptyOffload).
    Label stays "Connected" (still true), detail: "No live heart rate or synced
    history yet this session."

Explicitly out of scope here

  • No new/more sensitive trigger — the existing 3×/2× consecutive-empty
    thresholds are untouched.
  • No clickable single-bond README hint. That's a layer above the pure,
    testable RecordingState enum; a smaller follow-up.
  • Android's RecordingStatusChip/recordingStateFor are currently not
    wired into any screen
    — pre-existing, unrelated to this PR. The Kotlin
    mirror here keeps parity for whenever that does get wired, per
    RecordingState's own doc-comment promise of a 1:1 Kotlin mirror.

Verification

  • xcodebuild … -scheme Strand -destination 'platform=macOS' build and
    -scheme NOOPiOS -destination 'generic/platform=iOS Simulator' build — both clean.
  • StrandTests, 52/52 (TodayExplainabilityTests + SyncChipStateTests;
    needed -testLanguage en -testRegion US — this machine's system locale is
    German, which the pre-existing copy tests are sensitive to regardless of
    this change).
  • Android's testFullDebugUnitTest, 50/50 (TodayExplainabilityTest +
    SyncChipStateTest) — now that fix(android): fehlende DeviceRegistryDao-Overrides in den drei Test-Fakes nachziehen #828 fixed the FakeRegistryDao gap that
    previously blocked running any Android unit test locally.
  • Not verified on real hardware this session (no strap connected). Both
    the regression case (a healthy WHOOP 5.0 session still reading "Recording")
    and the new .connectedNoData case itself need a live check before this
    leaves draft.
  • No Android device+strap test setup — the Android side of this PR is
    compile/unit-test verified only.
  • main was merged in to resolve reported conflicts (purely additive, only
    Localizable.xcstrings needed manual resolution). The check (i18n audit)
    CI failure this surfaces is pre-existing on main itself (a release-notes
    literal in AppChangelog.kt, unrelated to this PR) — see the PR comment
    thread for detail.

The strap-recording chip only checked `connected` in its first branch
(`.recording`). A strap that is genuinely connected but has never once
synced — a fresh pairing, or a WHOOP-4/generic strap whose recent offloads
are a sustained streak of empty — fell through to `.notRecording` and showed
"Strap not connected. Tap to connect.", which is simply false: the link is up.
That's the exact symptom ryanbr#612 describes ("the app doesn't say what's really
happening") for the case that isn't already covered by ryanbr#580's
`.historyExperimental` override (which still takes priority for a WHOOP 5/MG
past its 2-empty-offload threshold — unchanged here).

Adds `LiveState.sustainedEmptyOffload`, exposing the WHOOP-4/generic
empty-offload streak (`EmptySyncTracker` / Android's twin) that was
previously only reachable as free text in `lastSyncError`. Set on every
HISTORY_COMPLETE (so a productive sync clears it immediately), reset on
disconnect (display flag only — the underlying streak counter deliberately
survives a reconnect, unchanged). `RecordingState` gains `.connectedNoData`:
connected, no live HR, and either never-synced or sustained-empty. Kotlin
gets the identical case, param and priority order in `TodayScoring.kt`,
per `RecordingState`'s own doc-comment promise of a 1:1 Kotlin mirror —
though note Android's `RecordingStatusChip`/`recordingStateFor` are
currently unwired into any screen (pre-existing, unrelated gap; will flag
separately).

Deliberately not done: no new, more sensitive trigger (the existing 3×/2×
consecutive-empty thresholds are unchanged), and no clickable single-bond
README hint (belongs a layer above the pure `RecordingState` enum — a
smaller follow-up).

Verified: xcodebuild build for Strand (macOS) and NOOPiOS.
StrandTests/TodayExplainabilityTests (46/46, forcing -testLanguage en
-testRegion US — this Mac's system locale is German, which the existing
copy tests are sensitive to regardless of this change).
`./gradlew compileFullDebugKotlin` clean. Android's `testFullDebugUnitTest`
could not be run locally — an unrelated, pre-existing gap (three
FakeRegistryDao test fakes are missing ~25 methods DeviceRegistryDao now
declares) fails test compilation for the whole module; filed separately,
not touched here.

Not verified on real hardware this session (no strap connected). Both the
regression case (a healthy WHOOP 5.0 session still reading "Recording") and
the new `.connectedNoData` case (a fresh pairing, or a WHOOP-4/generic
strap's 3rd consecutive empty offload) need a live check before this leaves
draft. No Android device+strap test setup either way — that side is
compile/unit-test verified only.

@ryanbr ryanbr left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Solid — the state genuinely was lying ("Strap not connected" on a live link), both platforms move together, and scoping around #580's .historyExperimental override rather than competing with it is the right call. I checked the priority: historySyncExperimental is still tested before resolve, so that path is untouched as you say. Conditions are identical on both sides.

One thing to fix: the new copy is missing from the String Catalog.

"Strap not connected. Tap to connect."                    -> in catalog
"History sync is experimental on 5.0."                    -> in catalog
"No live heart rate or synced history yet this session."  -> ABSENT
"Connected. No live heart rate or synced history…"        -> ABSENT

LocalizedStringKey / String(localized:) is the right mechanism, so English is fine — but de/es/fr/pt-PT will show these two in English while every neighbouring chip string is translated. Both siblings are already in Localizable.xcstrings, so the convention is clear.

Worth knowing why CI stayed green: i18n_audit checks that each entry already in the catalog has every language. It cannot see a string that never made it into the catalog at all. Same blind-spot class as the Text(variable) gap.

The Kotlin literals are fine as-is — the whole enum is unlocalized there already and, as you note, the chip is not wired into any screen yet.

…review)

RecordingState.connectedNoData's detail and accessibilityText used
String(localized:), the right mechanism, but the entries never got added to
Strand/Resources/Localizable.xcstrings — so de/es/fr/pt-PT (and the other
languages already carried by every neighbouring chip string) fell back to
English. i18n_audit's CI gate couldn't see this: it verifies translation
completeness for keys already IN the catalog, not for strings that never
made it in at all (the same blind spot as the Text(variable) gap ryanbr#245's
review caught).

Verified: Tools/i18n_audit.py --ci origin/main clean, StrandTests
TodayExplainabilityTests 46/46 (-testLanguage en -testRegion US).
@digitalerdude

Copy link
Copy Markdown
Author

Fixed in 3f0c437 — added the two missing entries to Strand/Resources/Localizable.xcstrings (de/es/fr/it/pt-PT/ru/zh-Hans/zh-Hant, matching the terminology already used by the neighboring chip strings). Tools/i18n_audit.py --ci origin/main is clean and TodayExplainabilityTests still passes 46/46.

@digitalerdude
digitalerdude marked this pull request as ready for review July 27, 2026 13:58
# Conflicts:
#	Strand/Resources/Localizable.xcstrings
@digitalerdude

Copy link
Copy Markdown
Author

Merged `main` in (814109b) to resolve the reported conflicts — purely additive on both sides, no real content conflicts (only `Localizable.xcstrings`, resolved by unioning both sides' new keys).

Two things that fell out of the merge, both worth noting:

  • The `check` CI failure (i18n audit) is pre-existing on `main` itself, not from this branch: `AppChangelog.kt:42`'s "HRV accuracy fix, Oura sleep and motion, richer 5/MG decoding, and a stoppable sync" (the 9.2.0 release's What's New title) is a raw Kotlin literal that never made it into the string catalog — confirmed with `git show main:android/app/src/main/java/com/noop/ui/AppChangelog.kt`, verbatim on `main` before this merge touched it. Not fixing it here (out of scope for Surface "no new nights for N days" instead of silently falling back to calibrating #612), but flagging it since it'll show red on this PR regardless.
  • Now that main has the FakeRegistryDao fix (fix(android): fehlende DeviceRegistryDao-Overrides in den drei Test-Fakes nachziehen #828) and the aapt2 verification-metadata fix, I could actually run the full test suites instead of just confirming compilation: `StrandTests` 52/52, and Android's `testFullDebugUnitTest` (TodayExplainabilityTest + SyncChipStateTest) 50/50. Updating the verification section below with real numbers rather than "compiles but couldn't run."

The 9.2.0 release generated its Android AppChangelog title as a raw Kotlin
literal instead of uiString(R.string....), the same pattern as ryanbr#514. The
i18n audit's regression gate now catches it on every open PR that merges
against the current main, including this one, even though this branch
never touched that file.

Localize it like the other entries: uiString(R.string.
l10n_app_changelog_hrv_accuracy_fix_oura_sleep_and_4ed31e39), with the
string added to values/ (en) + de/es/fr/pt-rPT (focus locales) and zh
(tracked under the ratcheting non-focus-locale allowance). Key follows the
established scheme (first-6-word slug + sha1(title)[:8]).

i18n_audit.py --ci: OK, no new hardcoded literals, no new locale gaps.
compileFullDebugKotlin: OK.
# Conflicts:
#	android/app/src/main/res/values-fr/strings.xml
@ryanbr

ryanbr commented Aug 2, 2026

Copy link
Copy Markdown
Owner

Reviewed. This is ready as far as I can tell — the diagnosis is right, the fix is placed correctly, and the twins actually match. Notes below are verification context, not requested changes.

The bug is still live on main

RecordingState.resolve today:

if connected && heartRate != nil { return .recording }
if let at = lastSyncedAt { return .lastSynced(minutesAgo: mins) }
return .notRecording

A genuinely connected strap with no live HR and no prior sync falls to .notRecording, which renders "Not recording. Strap not connected. Tap to connect." That is false, and it is false in exactly the situation where a user most needs an accurate read — a fresh pairing that is not handing anything over.

What I checked rather than assumed

Placement. .connectedNoData sits after .recording and before .lastSynced. That ordering is load-bearing: a WHOOP-4 with a KNOWN lastSyncedAt but a sustained empty streak would never reach the new case if it were placed after, so the sustainedEmptyOffload half of the condition would be dead. As written it fires.

.notRecording stays honest. After the change it is reachable only when genuinely disconnected, so the fix narrows the false case without swallowing the true one.

The twins really are twins. Same condition, same branch order, verbatim title/detail strings, same Accent tone (matching HistoryExperimental, which is also a connected state), same defaulted parameter so existing call sites are unaffected.

The flag semantics match the description, on both platforms: set on EVERY HISTORY_COMPLETE so a productive sync clears it immediately rather than waiting for a streak to lapse, and reset on disconnect as a display flag only, with the underlying EmptySyncTracker counter left alone. The comments say so on both sides and the code does what they say.

Not making the new state clickable is right. .notRecording's tap means "Tap to connect", which is meaningless for a strap that is already connected.

Verification

Your checks ran 2026-07-28, and main has moved 75 commits since — including BLEManager.swift +547, WhoopBleClient.kt +378 and TodayView.swift +210 in the files this touches. GitHub does not re-run checks when the base moves, so those ticks certified an older tree. No app-build ran either (it was disabled then), so the Swift here had never been compiled by CI.

I dispatched app-build.yml with pr: 826, which merges current main live rather than trusting the frozen merge ref:

build (Strand, macOS)                       success
build (NOOPiOS, iOS Simulator, macos-26)    success

So the Swift half is verified against today's tree. The 07-28 Android CI run was green but predates that drift; a trivial push would refresh it, though I would not hold the PR for it given the Kotlin change is small and self-contained.

StrandTests never runs in CI at all (swift-packages.yml covers Packages/**, app-build is compile-only), so your TodayExplainabilityTests additions are author-verified only. Standing repo gap, not yours.

One pre-existing asymmetry, noted not blocked

Android renders state.title / state.detail as raw English — StatePill(title = state.title) and Text(state.detail) — so the whole recording chip is unlocalized there, while Swift localizes it through the catalog (your +104 xcstrings lines). This PR adds one more untranslated Android string, which is consistent with NotRecording / Recording / LastSynced already sitting there as literals. Worth a separate pass to extract the set, not something to fix inside a behaviour PR.

@ryanbr
ryanbr merged commit 7a9a6dc into ryanbr:main Aug 2, 2026
3 checks passed
ryanbr added a commit that referenced this pull request Aug 2, 2026
…ocale (#612) (#1047)

The Today/Live recording chip renders its status word and detail line in English
on every Android device, whatever the phone's language. A German user reads
"Not recording. Strap not connected. Tap to connect." while the iPhone beside
them reads "Strap nicht verbunden. Tippe zum Verbinden."

The cause is a platform asymmetry in the twin. Swift's RecordingState.label and
.detail return LocalizedStringKey, so their literals are catalogue KEYS that
SwiftUI resolves. Kotlin's return plain String, and TodayScreen rendered
state.title / state.detail straight into StatePill and Text, so the literals
WERE the UI. Every branch was affected, not just the one #826 added.

i18n_audit could not see it: the literals live in a sealed-class getter rather
than a Compose call argument, so scan_android never looked at them. Confirmed by
probing the scanner directly — "Not recording" and "Strap not connected. Tap to
connect." both come back MISSED.

TodayScoring.kt is deliberately untouched. Its title/detail are the English
parity contract, asserted verbatim against the Swift twin in
TodayExplainabilityTest, and making them resource-backed would drag a Context
into a pure mapper and break those tests. The resource lookup therefore lives at
the render site, which is also where Swift does it.

No new translation: all nine strings already exist in Strand's catalogue, fully
translated in every locale Android ships, so the values are copied 1:1 rather
than invented. `%lld` becomes `%1$d`; the format signature is identical across
all six files.

Not fixed here: ScoreState has the same asymmetry (Swift LocalizedStringKey?,
Kotlin raw String). Its copy carries dates and night counts, so it needs plural
and date handling this change does not, and it deserves its own pass.
simoncad7 pushed a commit to simoncad7/noop that referenced this pull request Aug 17, 2026
…yanbr#826)

* Distinguish "connected, no data" from "not connected" (ryanbr#612)

The strap-recording chip only checked `connected` in its first branch
(`.recording`). A strap that is genuinely connected but has never once
synced — a fresh pairing, or a WHOOP-4/generic strap whose recent offloads
are a sustained streak of empty — fell through to `.notRecording` and showed
"Strap not connected. Tap to connect.", which is simply false: the link is up.
That's the exact symptom ryanbr#612 describes ("the app doesn't say what's really
happening") for the case that isn't already covered by ryanbr#580's
`.historyExperimental` override (which still takes priority for a WHOOP 5/MG
past its 2-empty-offload threshold — unchanged here).

Adds `LiveState.sustainedEmptyOffload`, exposing the WHOOP-4/generic
empty-offload streak (`EmptySyncTracker` / Android's twin) that was
previously only reachable as free text in `lastSyncError`. Set on every
HISTORY_COMPLETE (so a productive sync clears it immediately), reset on
disconnect (display flag only — the underlying streak counter deliberately
survives a reconnect, unchanged). `RecordingState` gains `.connectedNoData`:
connected, no live HR, and either never-synced or sustained-empty. Kotlin
gets the identical case, param and priority order in `TodayScoring.kt`,
per `RecordingState`'s own doc-comment promise of a 1:1 Kotlin mirror —
though note Android's `RecordingStatusChip`/`recordingStateFor` are
currently unwired into any screen (pre-existing, unrelated gap; will flag
separately).

Deliberately not done: no new, more sensitive trigger (the existing 3×/2×
consecutive-empty thresholds are unchanged), and no clickable single-bond
README hint (belongs a layer above the pure `RecordingState` enum — a
smaller follow-up).

Verified: xcodebuild build for Strand (macOS) and NOOPiOS.
StrandTests/TodayExplainabilityTests (46/46, forcing -testLanguage en
-testRegion US — this Mac's system locale is German, which the existing
copy tests are sensitive to regardless of this change).
`./gradlew compileFullDebugKotlin` clean. Android's `testFullDebugUnitTest`
could not be run locally — an unrelated, pre-existing gap (three
FakeRegistryDao test fakes are missing ~25 methods DeviceRegistryDao now
declares) fails test compilation for the whole module; filed separately,
not touched here.

Not verified on real hardware this session (no strap connected). Both the
regression case (a healthy WHOOP 5.0 session still reading "Recording") and
the new `.connectedNoData` case (a fresh pairing, or a WHOOP-4/generic
strap's 3rd consecutive empty offload) need a live check before this leaves
draft. No Android device+strap test setup either way — that side is
compile/unit-test verified only.

* Add the missing String Catalog entries for connectedNoData (ryanbr#612 review)

RecordingState.connectedNoData's detail and accessibilityText used
String(localized:), the right mechanism, but the entries never got added to
Strand/Resources/Localizable.xcstrings — so de/es/fr/pt-PT (and the other
languages already carried by every neighbouring chip string) fell back to
English. i18n_audit's CI gate couldn't see this: it verifies translation
completeness for keys already IN the catalog, not for strings that never
made it in at all (the same blind spot as the Text(variable) gap ryanbr#245's
review caught).

Verified: Tools/i18n_audit.py --ci origin/main clean, StrandTests
TodayExplainabilityTests 46/46 (-testLanguage en -testRegion US).

* i18n: localize the 9.2.0 changelog title (unblock the coverage gate)

The 9.2.0 release generated its Android AppChangelog title as a raw Kotlin
literal instead of uiString(R.string....), the same pattern as ryanbr#514. The
i18n audit's regression gate now catches it on every open PR that merges
against the current main, including this one, even though this branch
never touched that file.

Localize it like the other entries: uiString(R.string.
l10n_app_changelog_hrv_accuracy_fix_oura_sleep_and_4ed31e39), with the
string added to values/ (en) + de/es/fr/pt-rPT (focus locales) and zh
(tracked under the ratcheting non-focus-locale allowance). Key follows the
established scheme (first-6-word slug + sha1(title)[:8]).

i18n_audit.py --ci: OK, no new hardcoded literals, no new locale gaps.
compileFullDebugKotlin: OK.
simoncad7 pushed a commit to simoncad7/noop that referenced this pull request Aug 17, 2026
…ocale (ryanbr#612) (ryanbr#1047)

The Today/Live recording chip renders its status word and detail line in English
on every Android device, whatever the phone's language. A German user reads
"Not recording. Strap not connected. Tap to connect." while the iPhone beside
them reads "Strap nicht verbunden. Tippe zum Verbinden."

The cause is a platform asymmetry in the twin. Swift's RecordingState.label and
.detail return LocalizedStringKey, so their literals are catalogue KEYS that
SwiftUI resolves. Kotlin's return plain String, and TodayScreen rendered
state.title / state.detail straight into StatePill and Text, so the literals
WERE the UI. Every branch was affected, not just the one ryanbr#826 added.

i18n_audit could not see it: the literals live in a sealed-class getter rather
than a Compose call argument, so scan_android never looked at them. Confirmed by
probing the scanner directly — "Not recording" and "Strap not connected. Tap to
connect." both come back MISSED.

TodayScoring.kt is deliberately untouched. Its title/detail are the English
parity contract, asserted verbatim against the Swift twin in
TodayExplainabilityTest, and making them resource-backed would drag a Context
into a pure mapper and break those tests. The resource lookup therefore lives at
the render site, which is also where Swift does it.

No new translation: all nine strings already exist in Strand's catalogue, fully
translated in every locale Android ships, so the values are copied 1:1 rather
than invented. `%lld` becomes `%1$d`; the format signature is identical across
all six files.

Not fixed here: ScoreState has the same asymmetry (Swift LocalizedStringKey?,
Kotlin raw String). Its copy carries dates and night counts, so it needs plural
and date handling this change does not, and it deserves its own pass.
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