From 9dd95429d8779a72b0b53e1f6c68307d74d32c53 Mon Sep 17 00:00:00 2001 From: Khoa Truong Date: Mon, 10 Aug 2026 12:55:39 -0700 Subject: [PATCH 1/4] fix(ui,coach): grow Today tiles with font scale; retry + explain network failures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two unrelated field bugs from the same report (Pixel 10 Pro XL). **Today Activity tile clipped the calories row.** `TodayTileMetrics.height` was a fixed `168.dp` while everything inside a tile is sized in `sp`. The Activity tile's three label/value pairs need `3 x (12+18)sp + 6dp`, and only ~108dp survives the 16dp padding, the eyebrow row and the 8dp spacer — so they stop fitting at roughly **fontScale 1.15** and the third row (CAL) is sliced off. This is NOT a density/DPI problem, despite looking like one: dp and sp both scale with density, so a 420dpi Pixel 8 and a 480dpi Pixel 10 Pro XL lay out identically at the same font-size setting. That is why the earlier issue-#24 fix verified clean on the Pixel 8. Shrinking the text a second time would only move the breaking point, so the container grows instead — height is now derived from the font scale, sampled at 16.sp (Android 14+ scales each sp size on its own non-linear curve, so `fontScale` alone under-reports it) and clamped to 1.0..1.6. Every tile reads the same value, so the grid stays uniform and the Sleep/Chart/Gauge tiles get the same protection. **Coach turns died on a single DNS blip, with unreadable errors.** `ResponsesHttp` made exactly one attempt and surfaced the raw JDK string, so a momentary resolver failure printed `Unable to resolve host "generativelanguage.googleapis.com": No address associated with hostname` and the next attempt printed the single word `timeout` — neither hinting at the actual cause (an active VPN or a Private DNS entry that can't resolve Google hosts). Now retries up to twice with backoff, but ONLY for `UnknownHostException` and `ConnectException` — failures that provably never left the device. Read timeouts are deliberately not retried: OkHttp reports connect and read timeouts as the same exception, and re-sending a request the model already ran would bill the user's key twice. Transport errors also map to copy that names the host and points at VPN / Private DNS, keeping the raw text in parentheses for bug reports. One transport, so all four providers benefit. Runtime-verified on emulator-5554 (API 35) at Pixel 10 Pro XL geometry (1344x2992 @ 480dpi): - density 560 / fontScale 1.0 -> no vertical clip, only graceful horizontal ellipsis; confirms density is not the trigger. - density 480 / fontScale 1.3 with the height temporarily re-pinned to 168.dp -> reproduces the reported clip exactly. - density 480 / fontScale 1.3 and 2.0 with the fix -> all three values visible. - Coach error reproduced end-to-end via airplane mode with a Gemini key: the bubble now reads "Couldn't look up generativelanguage.googleapis.com ... check ... whether a VPN or a Private DNS setting is blocking it." 6 new unit tests; full suite green. --- .../coach/openai/OpenAIResponsesClient.kt | 41 ++++++++++++++--- .../coach/orchestration/CoachTurnError.kt | 36 ++++++++++++++- .../com/pulseloop/ui/components/TodayTiles.kt | 46 +++++++++++++++++-- .../com/pulseloop/coach/CoachTurnErrorTest.kt | 45 ++++++++++++++++++ 4 files changed, 155 insertions(+), 13 deletions(-) diff --git a/app/src/main/java/com/pulseloop/coach/openai/OpenAIResponsesClient.kt b/app/src/main/java/com/pulseloop/coach/openai/OpenAIResponsesClient.kt index e5c71cb..0ae7948 100644 --- a/app/src/main/java/com/pulseloop/coach/openai/OpenAIResponsesClient.kt +++ b/app/src/main/java/com/pulseloop/coach/openai/OpenAIResponsesClient.kt @@ -56,16 +56,43 @@ internal object ResponsesHttp { .url(url) .post(okhttp3.RequestBody.create(jsonMediaType, body)) for ((name, value) in headers) builder.header(name, value) + val request = builder.build() - val response = try { - client.newCall(builder.build()).execute() - } catch (e: Exception) { - throw ResponsesError.Transport(e) + var lastFailure: Exception? = null + for (attempt in 0..MAX_UNSENT_RETRIES) { + val response = try { + client.newCall(request).execute() + } catch (e: Exception) { + lastFailure = e + // Only retry failures that provably never reached the provider: DNS resolution and + // TCP connect. A momentary DNS miss (radio handover, a VPN or private-DNS resolver + // still coming up) otherwise kills the whole turn and burns the user's message. + // + // A read timeout is deliberately NOT retried. OkHttp reports connect and read + // timeouts as the same SocketTimeoutException, so we cannot tell "never sent" from + // "sent, answer lost" — and re-sending the latter bills the user's API key for a + // generation that already ran. + if (!isProvablyUnsent(e) || attempt == MAX_UNSENT_RETRIES) throw ResponsesError.Transport(e) + Thread.sleep(RETRY_BACKOFF_MS shl attempt) // 400ms, 800ms + continue + } + val text = response.body?.string() ?: "" + if (!response.isSuccessful) throw ResponsesError.Http(response.code, text) + return text } - val text = response.body?.string() ?: "" - if (!response.isSuccessful) throw ResponsesError.Http(response.code, text) - return text + // Unreachable — the loop either returns or throws — but keeps the compiler happy. + throw ResponsesError.Transport(lastFailure ?: IllegalStateException("request never ran")) } + + /** + * True when [e] means the request never left the device, so re-sending it is side-effect free. + * `UnknownHostException` is DNS; `ConnectException` is a refused/unreachable TCP connect. + */ + internal fun isProvablyUnsent(e: Throwable): Boolean = + e is java.net.UnknownHostException || e is java.net.ConnectException + + private const val MAX_UNSENT_RETRIES = 2 + private const val RETRY_BACKOFF_MS = 400L } /** One parsed Responses-API function tool spec, provider-neutral. */ diff --git a/app/src/main/java/com/pulseloop/coach/orchestration/CoachTurnError.kt b/app/src/main/java/com/pulseloop/coach/orchestration/CoachTurnError.kt index b210bd6..c1f604e 100644 --- a/app/src/main/java/com/pulseloop/coach/orchestration/CoachTurnError.kt +++ b/app/src/main/java/com/pulseloop/coach/orchestration/CoachTurnError.kt @@ -52,7 +52,7 @@ data class CoachTurnError( reason = "No API key is configured for the selected provider. Add one in Settings → AI Coach.") is ResponsesError.Transport -> CoachTurnError( code = "Network", - reason = error.underlying.message ?: "The network request failed.") + reason = transportReason(error.underlying)) is ResponsesError.Http -> CoachTurnError( code = "HTTP ${error.status}", reason = cleanReason(error.body, error.status)) @@ -63,6 +63,40 @@ data class CoachTurnError( else -> CoachTurnError(code = "Error", reason = error.message ?: "Something went wrong.") } + /** + * Turns a raw transport exception into something the user can act on. + * + * The JDK's own strings are useless in a chat bubble — a blocked resolver surfaces as + * `Unable to resolve host "generativelanguage.googleapis.com": No address associated with + * hostname`, and a socket timeout as the single word `timeout`. Neither hints that the + * usual causes are an active VPN, a Private DNS entry that doesn't resolve Google hosts, or + * simply no connectivity. The underlying text is still appended so a bug report keeps it. + */ + internal fun transportReason(underlying: Throwable): String { + val detail = underlying.message?.trim().orEmpty() + return when (underlying) { + is java.net.UnknownHostException -> { + val host = hostFrom(detail) + "Couldn't look up ${host ?: "the provider"}. Your device can't resolve it right " + + "now — check your connection, and whether a VPN or a Private DNS setting is " + + "blocking it. (${detail.ifEmpty { "unknown host" }})" + } + is java.net.SocketTimeoutException -> + "The provider didn't respond in time. Check your connection and try again — a " + + "VPN or a weak signal will do this." + is javax.net.ssl.SSLException -> + "The secure connection to the provider failed. This is usually a VPN, a proxy, " + + "or a network that intercepts traffic. (${detail.ifEmpty { "TLS error" }})" + is java.net.ConnectException -> + "Couldn't connect to the provider. (${detail.ifEmpty { "connection refused" }})" + else -> detail.ifEmpty { "The network request failed." } + } + } + + /** Pulls `example.com` out of `Unable to resolve host "example.com": …`, if present. */ + private fun hostFrom(message: String): String? = + Regex("\"([^\"]+)\"").find(message)?.groupValues?.getOrNull(1)?.takeIf { it.isNotBlank() } + /** Extracts a readable message from a provider error body, which is * usually JSON like `{"error":{"message":"..."}}` (OpenAI/OpenRouter) or * `{"error":{"message":"...","status":"..."}}` (Gemini). Falls back to diff --git a/app/src/main/java/com/pulseloop/ui/components/TodayTiles.kt b/app/src/main/java/com/pulseloop/ui/components/TodayTiles.kt index f5fee39..1b59bfa 100644 --- a/app/src/main/java/com/pulseloop/ui/components/TodayTiles.kt +++ b/app/src/main/java/com/pulseloop/ui/components/TodayTiles.kt @@ -17,10 +17,12 @@ import androidx.compose.ui.draw.clip import androidx.compose.ui.draw.shadow import androidx.compose.ui.graphics.Brush import androidx.compose.ui.graphics.Color +import androidx.compose.ui.platform.LocalDensity import androidx.compose.ui.text.PlatformTextStyle import androidx.compose.ui.text.TextStyle import androidx.compose.ui.text.font.FontWeight import androidx.compose.ui.text.style.TextOverflow +import androidx.compose.ui.unit.Dp import androidx.compose.ui.unit.dp import androidx.compose.ui.unit.sp import com.pulseloop.ui.theme.PulseColors @@ -33,8 +35,37 @@ import com.pulseloop.ui.theme.PulseColors /** Shared sizing so every Today tile is identical (TodayTileMetrics in Swift). */ object TodayTileMetrics { - val height = 168.dp + /** The design height, at the default font scale (1.0). */ + val baseHeight = 168.dp val corner = 20.dp + + /** + * The tile height, grown with the user's font-size setting. + * + * The grid is a fixed-height design, but everything inside a tile is sized in `sp` — so a tile + * stops fitting its own contents the moment the user raises the font scale. That is what clipped + * the Activity tile's third row (calories, issue #24 round two): the three label/value pairs need + * `3 × (12 + 18)sp + 2 × 3dp`, and only ~108dp of the 168dp tile is left after the 16dp padding, + * the eyebrow row, and the 8dp spacer. They stop fitting at roughly **fontScale 1.15**. + * + * **This is not a density/DPI problem.** dp and sp both scale by density, so a 420dpi Pixel 8 and + * a 480dpi Pixel 10 Pro XL lay out identically at the same font scale — which is why this passed + * verification on the Pixel 8. Only `fontScale` moves sp relative to dp. Shrinking the text again + * would just move the breaking point; the container has to grow instead. + * + * Sampling the scale at 16.sp (the Activity value size) rather than reading `fontScale` directly + * matters on Android 14+, where font scaling is non-linear and every sp size has its own curve — + * `fontScale` alone would under-report the growth of the text that actually overflows. + * + * Clamped at the bottom so a small-text user still gets the designed layout, and at the top so an + * accessibility-max setting can't produce an absurd grid. The clamp is safe: at the 2.0 ceiling the + * values block needs 186dp and a 1.6×-clamped tile still offers ~199dp. + */ + val height: Dp + @Composable get() { + val scale = with(LocalDensity.current) { 16.sp.toDp() / 16.dp } + return baseHeight * scale.coerceIn(1f, 1.6f) + } } /** @@ -110,10 +141,15 @@ fun ActivityTile( strokeWidth = 9.dp, ringSpacing = 4.dp, ) - // Three metrics (steps/distance/calories) share the fixed-height tile. At 22.sp the - // third value overflowed and clipped the calories row (issue #24). Keep each pair - // compact — 16.sp value, tight line heights, and no font padding (Compose's default - // includeFontPadding adds several dp per line) — so all three fit with margin. + // Three metrics (steps/distance/calories) share one tile. At 22.sp the third value + // overflowed and clipped the calories row (issue #24), so each pair is compact — + // 16.sp value, tight line heights, and no font padding (Compose's default + // includeFontPadding adds several dp per line). + // + // That alone was not enough: this block is measured in sp while the tile was a fixed + // 168.dp, so calories clipped again on a device with a raised font scale. Shrinking the + // text further would only move the breaking point — [TodayTileMetrics.height] now grows + // the tile with the font scale instead. Don't re-pin the height to a dp literal. val compact = TextStyle(platformStyle = PlatformTextStyle(includeFontPadding = false)) Column(verticalArrangement = Arrangement.spacedBy(3.dp), modifier = Modifier.weight(1f)) { values.forEach { value -> diff --git a/app/src/test/java/com/pulseloop/coach/CoachTurnErrorTest.kt b/app/src/test/java/com/pulseloop/coach/CoachTurnErrorTest.kt index a16aaf9..043df24 100644 --- a/app/src/test/java/com/pulseloop/coach/CoachTurnErrorTest.kt +++ b/app/src/test/java/com/pulseloop/coach/CoachTurnErrorTest.kt @@ -1,6 +1,7 @@ package com.pulseloop.coach.orchestration import com.pulseloop.coach.openai.ResponsesError +import com.pulseloop.coach.openai.ResponsesHttp import org.junit.Assert.* import org.junit.Test @@ -57,6 +58,50 @@ class CoachTurnErrorTest { assertEquals("timeout", e.reason) } + /** + * The real-world failure this copy exists for: a Pixel with an active VPN produced + * `Unable to resolve host "generativelanguage.googleapis.com": No address associated with + * hostname` verbatim in the chat bubble. The bubble must name the host and point at the cause. + */ + @Test + fun testUnknownHostNamesTheHostAndSuggestsVpnOrDns() { + val underlying = java.net.UnknownHostException( + "Unable to resolve host \"generativelanguage.googleapis.com\": " + + "No address associated with hostname", + ) + val e = CoachTurnError.from(ResponsesError.Transport(underlying)) + assertEquals("Network", e.code) + assertTrue(e.reason.contains("generativelanguage.googleapis.com")) + assertTrue(e.reason.contains("VPN")) + assertTrue(e.reason.contains("Private DNS")) + // The raw text survives for bug reports. + assertTrue(e.reason.contains("No address associated with hostname")) + } + + @Test + fun testSocketTimeoutIsNotJustTheWordTimeout() { + val e = CoachTurnError.from(ResponsesError.Transport(java.net.SocketTimeoutException("timeout"))) + assertEquals("Network", e.code) + assertNotEquals("timeout", e.reason) + assertTrue(e.reason.contains("didn't respond in time")) + } + + @Test + fun testUnknownHostWithoutAQuotedHostStillReads() { + val e = CoachTurnError.from(ResponsesError.Transport(java.net.UnknownHostException(""))) + assertTrue(e.reason.contains("the provider")) + } + + /** DNS and TCP connect never reached the provider, so re-sending is free. A read timeout may + * have already billed a generation — retrying it would charge the user twice. */ + @Test + fun testOnlyProvablyUnsentFailuresAreRetryable() { + assertTrue(ResponsesHttp.isProvablyUnsent(java.net.UnknownHostException("dns"))) + assertTrue(ResponsesHttp.isProvablyUnsent(java.net.ConnectException("refused"))) + assertFalse(ResponsesHttp.isProvablyUnsent(java.net.SocketTimeoutException("timeout"))) + assertFalse(ResponsesHttp.isProvablyUnsent(java.io.IOException("closed"))) + } + @Test fun testUnknownErrorFallsBackToMessage() { val e = CoachTurnError.from(IllegalStateException("weird")) From 2d36a10fc8b7f4ae5de2d0a784cceb25715ac516 Mon Sep 17 00:00:00 2001 From: Khoa Truong Date: Mon, 10 Aug 2026 12:55:56 -0700 Subject: [PATCH 2/4] docs(health-connect): design + phase plan; re-triage iOS #80 from SKIP to ADAPT MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit iOS PR #80 (Apple Health sync) was triaged SKIP with the note that Health Connect is the Android analogue "if ever wanted". It is wanted: the app has no Health Connect code at all, so ring data can't reach any consumer outside PulseLoop. Re-triaged to ADAPT/XL and queued. No code — this is the document a later session starts from. It covers what iOS writes to HealthKit today (the parity target), why not shipping on Google Play does not block this, the target design, and a seven-phase plan with per-phase verification. Two things worth surfacing from the research: - **The Play Store is a non-issue.** The "Health apps declaration" is a publishing-review gate, not a runtime one; Health Connect permissions are ordinary `android.permission.health.*` runtime permissions. Gadgetbridge — checked out at the parent repo root — ships a full integration sideload-only with zero workarounds, and is therefore the reference implementation here rather than iOS. The real caveats are that Android 13 and below need the Health Connect APK from Play, and that a privacy-rationale Activity is mandatory regardless of distribution. - **Two Android-specific identity traps** that would silently duplicate records: `SleepStageBlockEntity.id` is regenerated on every re-sync (iOS's block ids are stable, so its scheme does not port), and `MeasurementEntity.id` is a random UUID for live rows but a stable `history::` for history rows. Scope settled with the maintainer: write-only, iOS parity first (phases 1-4), then the types Health Connect supports and HealthKit made awkward — blood pressure, glucose, respiratory rate, VO2 max, resting HR, nutrition (phase 5). Stress and fatigue have no Health Connect record type, same gap as HealthKit. --- docs/health-connect-integration.md | 464 +++++++++++++++++++++++++++++ docs/ios-sync.md | 11 +- 2 files changed, 474 insertions(+), 1 deletion(-) create mode 100644 docs/health-connect-integration.md diff --git a/docs/health-connect-integration.md b/docs/health-connect-integration.md new file mode 100644 index 0000000..e1af497 --- /dev/null +++ b/docs/health-connect-integration.md @@ -0,0 +1,464 @@ +# Health Connect integration — design and implementation plan + +Status: **not started.** This document exists so a future session can pick the work up cold. +Implementation begins at Phase 0. + +## Context + +The iOS app ships a complete one-way Apple HealthKit export, added in iOS PR #80. +`docs/ios-sync.md` triaged that PR as **SKIP** with the note *"HealthKit — intentional iOS-only +divergence. Android analogue is **Health Connect**; use this as the reference design if ever +wanted"*, and the "Intentional divergences" section lists "HealthKit-adjacent integrations" as +iOS-only. This document revisits that decision. + +The Android app currently has **zero** Health Connect code — no dependency, no manifest entry, no +`` element. Ring data is therefore trapped inside PulseLoop and can't reach any Health +Connect consumer (Fitbit-style dashboards, Home Assistant, other fitness apps). + +The outcome we want: a **write-only** Health Connect export mirroring what HealthKit gets on iOS, +delivered as mergeable slices, followed by a phase covering the metrics Health Connect supports but +HealthKit made awkward. + +Paths below are relative to this repo (`android/`) unless prefixed ``, which means the iOS +repo — this repo's parent directory. `Gadgetbridge/` is also at the parent root. + +--- + +## 1. What iOS writes to HealthKit (the baseline to match) + +Source: `/PulseLoop/Health/HealthSyncService.swift` (598 L), `+Workouts.swift`, +`+Nutrition.swift`, `HealthKitTypeMappings.swift`, `HealthSyncPublisher.swift`, +`/PulseLoop/Settings/AppleHealthPrefsStore.swift`, +`/PulseLoop/Views/Settings/AppleHealthSettingsView.swift`. + +### Writes (the `toShare` set) + +| Category | HealthKit type | Source | Notes | +|---|---|---|---| +| Vitals | `.heartRate` | `Measurement` rows, `kind == .heartRate` | count/min, instantaneous | +| | `.oxygenSaturation` | `.spo2` | percent → 0…1 fraction | +| | `.heartRateVariabilitySDNN` | `.hrv` | ms | +| | `.bodyTemperature` | `.temperature` | °C — Apple's wrist-temp type is read-only to third parties | +| Daily activity | `.stepCount`, `.activeEnergyBurned`, `.distanceWalkingRunning` | `ActivityDaily` | one day-spanning sample each; workout kcal/distance **netted out** so the Move ring doesn't double count | +| Sleep | `.sleepAnalysis` category | `SleepBlock` per stage | deep / core (light) / REM / awake / unspecified; carries `HKMetadataKeyTimeZone` | +| Workouts | `HKWorkout` + `HKWorkoutRoute` | `ActivitySession` | `HKWorkoutBuilder`, child energy + distance samples, GPS route from accepted `ActivityGpsPoint`s | +| Nutrition | 7 dietary types (energy, protein, carbs, fat, fiber, sugar, sodium) | `MealEntry` | only when the nutrition feature's own master toggle is on | + +**Deliberately not written** (`HealthKitTypeMappings.swift:48-56`): stress and fatigue (no HealthKit +equivalent), blood pressure (needs `HKCorrelation` pairing), blood sugar, respiratory rate, VO₂max — +all documented as follow-ups that never happened. + +### Reads + +Profile characteristics only — date of birth, biological sex, latest height, latest weight — +consumed solely by the "Import from Apple Health" button on `ProfileSettingsView`. No +`HKAnchoredObjectQuery`, no observers, no importing of other apps' samples. + +### Mechanics worth copying verbatim + +- **Upsert, not delete-and-rewrite.** Every sample carries a deterministic + `HKMetadataKeySyncIdentifier` (`pl-m--`, `pl-act--`, + `pl-wk-`, …) plus an `HKMetadataKeySyncVersion`. Re-exporting the same logical row + replaces it. +- **Watermarks in UserDefaults**, not the database (`AppleHealthSyncState`). Vitals watermark on + `Measurement.createdAt` — deliberately *not* the sample timestamp, so late-arriving ring history is + still picked up. Aggregates watermark on `updatedAt`. Advanced per chunk so an interrupted backfill + resumes. +- **Debounced trigger.** `HealthSyncPublisher` observes the app-wide `PulseDataChange` token and + exports 15 s after the last change. No background task, no timer. +- **First-enable backfill dialog**: "Sync all history" / "Only new data from now on". Exports are + blocked entirely while `backfillChoice == .notAsked`. +- **Per-datatype toggles** (HR, SpO₂, HRV, temperature, sleep, steps & activity, workouts, + nutrition), all default on under a master toggle that defaults **off**. +- **"Remove PulseLoop data from Apple Health"** deletes everything scoped to our own `HKSource`, then + clears watermarks. Individual workout/meal deletion hooks fire when the local row is deleted. +- Batching: 1 000 vitals samples per save, with a per-object fallback on batch failure, and the + watermark only advances past rows that actually landed. + +--- + +## 2. Does not shipping on Google Play matter? + +**No, not for what we're building.** The one real consequence is a per-user prerequisite on +Android 13 and below, not a restriction on us. + +**Why it's fine:** + +- Health Connect permissions are ordinary Android runtime permissions in the + `android.permission.health.*` group. They're declared in the manifest and granted by the user in + the Health Connect permission sheet. There is no server-side allowlist keyed to Play approval for + the standard (non-medical) data types. +- The **Play Console "Health apps declaration" is a publishing-time review gate.** Google's own + wording ties the failure mode to Play-published apps: *"If your health app is published in the Play + store and released to the public, but you didn't request for data type accesses, your end users + receive [an error] dialog."* Not publishing means not being reviewed. +- **Direct proof by existence:** Gadgetbridge is F-Droid / sideload only, has never been on Play, and + ships a full production Health Connect integration writing 22 permissions' worth of data types. Its + source is checked out at the parent repo root (`Gadgetbridge/`) and contains zero Play-Store, + declaration-form, or sideloading workarounds. **It is the reference implementation for this plan.** + +**The real caveats, in order of importance:** + +1. **Android 13 and below need the Health Connect app from Play.** Health Connect is part of AOSP + from Android 14 onward. Below that it's a separate APK (`com.google.android.apps.healthdata`) + distributed through the Play Store. `minSdk = 26`, so a meaningful slice of users hit this. Handle + it at runtime: `HealthConnectClient.getSdkStatus()` returns `SDK_UNAVAILABLE` or + `SDK_UNAVAILABLE_PROVIDER_UPDATE_REQUIRED`, and the settings screen shows an explanatory row with + a Play deep-link instead of a broken toggle. This is a user prerequisite, not a build constraint. +2. **A privacy-rationale Activity is mandatory regardless of distribution.** Health Connect's + permission sheet shows a "privacy policy" link that fires + `androidx.health.ACTION_SHOW_PERMISSIONS_RATIONALE` (pre-14) or + `android.intent.action.VIEW_PERMISSION_USAGE` + `category.HEALTH_PERMISSIONS` (14+). Gadgetbridge + satisfies this with a two-`TextView` in-app screen and **no hosted URL**. Do the same. +3. **Medical records (FHIR) data types are a different regime** with genuine extra approval. We don't + touch them. +4. **Restricted read permissions we're avoiding anyway.** `READ_HEALTH_DATA_HISTORY` (reading other + apps' data older than 30 days) and `READ_HEALTH_DATA_IN_BACKGROUND` get heightened Play scrutiny. + Write-only sidesteps both. Note that *writing* historically-dated records has no such restriction + — full backfill is fine. +5. **Unrelated but worth knowing:** Google's developer-verification requirement for sideloaded apps + begins rolling out September 2026 (Brazil, Indonesia, Singapore, Thailand first; global 2027+). + That affects how users install PulseLoop at all, not Health Connect specifically, and is out of + scope here. + +**Bottom line:** build it exactly as if we were shipping on Play (manifest permissions, rationale +activity, `` block) and simply never file the declaration form. If PulseLoop is ever +submitted to Play, the declaration becomes a form-filling exercise, not a rework. + +--- + +## 3. Target design + +New package `app/src/main/java/com/pulseloop/health/`, a sibling of the existing `strava/` +integration package. + +### Scope decisions (settled) + +- **Write-only.** No `READ_*` permissions at all. iOS's profile import can't fully port regardless — + Health Connect has no date-of-birth or biological-sex data type. +- **iOS parity first** (Phases 1–4), then the extras Health Connect supports and HealthKit didn't + (Phase 5). +- **Mergeable slices**, one PR per phase. + +### Architectural decisions + +**`clientRecordId` on every record.** This is Health Connect's native upsert: `insertRecords` with a +matching `clientRecordId` and a `clientRecordVersion` ≥ the stored one **replaces** the record. It +maps one-to-one onto iOS's `HKMetadataKeySyncIdentifier` scheme, so reuse the same id shapes. +Gadgetbridge only uses this for three of its record types and eats duplicate risk elsewhere — we +should use it everywhere. + +> **`clientRecordVersion` must never be the metric value.** Gadgetbridge's comment in +> `syncers/HealthConnectSyncer.kt` explains why: a downward correction would carry a lower version +> and be silently ignored, freezing the record at its stale maximum. Use `1` for immutable vitals and +> the row's `updatedAt` (or the run's wall-clock) for mutable aggregates. + +**Watermarks in SharedPreferences, not Room.** Mirrors `AppleHealthPrefsStore` and avoids a DB +migration entirely. Follow the `MetricPrefsStore` pattern (`ui/dashboard/MetricPrefsStore.kt`) — a +JSON blob plus a `StateFlow`. Two separate keys so frequent watermark writes don't rewrite the +preference blob. + +**Export is a pure DB → Health Connect pass**, driven by watermarks, never by events. It doesn't +matter when or how the data landed. This is deliberate: + +> While mapping the sync pipeline for this plan I noticed `EventPersistenceSubscriber` is only +> constructed inside the `PulseLoopApp` **composable** (`ui/PulseLoopApp.kt:73`), while +> `RingSyncWorker` runs BLE with the app backgrounded. If WorkManager cold-starts the process, no +> subscriber exists to persist what the worker fetches. That is a pre-existing question outside this +> plan's scope — but it is the reason the exporter must read the DB rather than hang off the event +> bus. + +**Trigger** — a `HealthConnectExportWorker` (`CoroutineWorker`) enqueued as unique one-time work with +`setInitialDelay(15s)` and `ExistingWorkPolicy.REPLACE`. The REPLACE-on-delay pattern *is* the +debounce (exactly what Gadgetbridge's `NewDataReceiver` does with a 10 s window, and the Android +equivalent of iOS's 15 s `HealthSyncPublisher`). Enqueued from three places: + +- `EventPersistenceSubscriber`'s `SyncProgress("done")` branch (`service/EventPersistenceSubscriber.kt:277`) +- the end of `RingSyncWorker.doWork()`, so background-only syncs still export +- the manual "Export now" button + +No foreground service. A plain worker gets ~10 minutes; watermarks advance per chunk, so a long +backfill just resumes on the next run. This avoids adding `FOREGROUND_SERVICE_DATA_SYNC` and the +`androidx.work.impl.foreground.SystemForegroundService` manifest override that Gadgetbridge needs. + +### Data-type mapping + +**Phases 1–4 (iOS parity).** Ranges marked ⚠️ are Health Connect platform limits, not our choice — +violating them throws. + +| PulseLoop source | Health Connect record | Permission | Units / conversion | Guard | +|---|---|---|---|---| +| `MeasurementKind.HEART_RATE` | `HeartRateRecord` (**series**) | `WRITE_HEART_RATE` | bpm `Long` | 20…300, drop 0 | +| `SPO2` | `OxygenSaturationRecord` | `WRITE_OXYGEN_SATURATION` | `Percentage(0…100)` — **not** the 0…1 fraction HealthKit wants | 50…100 | +| `HRV` | `HeartRateVariabilityRmssdRecord` | `WRITE_HEART_RATE_VARIABILITY` | ms `Double` | ⚠️ **1…200** — narrower than iOS's 0…1000; a 211 crashed Gadgetbridge (their issue #6190) | +| `TEMPERATURE` | `BodyTemperatureRecord` | `WRITE_BODY_TEMPERATURE` | `Temperature.celsius` | 25…45 | +| `STRESS`, `FATIGUE` | — | — | — | no Health Connect record type exists (same gap as HealthKit) | +| `SleepSessionEntity` + `SleepStageBlockEntity` | `SleepSessionRecord` with `stages` | `WRITE_SLEEP` | — | stages sorted, non-overlapping, inside session bounds | +| `ActivityDailyEntity.steps` | `StepsRecord` | `WRITE_STEPS` | `Long` | > 0 | +| `.calories` (net of workouts) | `ActiveCaloriesBurnedRecord` | `WRITE_ACTIVE_CALORIES_BURNED` | `Energy.kilocalories` | > 0 | +| `.distanceMeters` (net of workouts) | `DistanceRecord` | `WRITE_DISTANCE` | `Length.meters` | > 0 | +| `ActivitySessionEntity` | `ExerciseSessionRecord` | `WRITE_EXERCISE` | type map below | `endedAt > startedAt`, not future | +| `ActivityGpsPointEntity` | `ExerciseRoute` (embedded) | `WRITE_EXERCISE_ROUTE` | lat/lon/alt/accuracy | `accepted` only, ≥ 2 points, **no duplicate timestamps** (HC rejects), decimate on 1 MB overflow | +| session `calories` / `distanceMeters` | sibling `ActiveCaloriesBurnedRecord` / `DistanceRecord` over the session window | as above | kcal / m | > 0 | + +**Phase 5 (beyond iOS — Health Connect has types HealthKit lacked or made awkward):** + +| PulseLoop source | Health Connect record | Notes | +|---|---|---| +| `BLOOD_PRESSURE_SYSTOLIC` + `BLOOD_PRESSURE_DIASTOLIC` | `BloodPressureRecord` | One record carries both — much simpler than HealthKit's `HKCorrelation`, which is why iOS skipped it. Requires **pairing the two `MeasurementEntity` rows by timestamp**. | +| `BLOOD_SUGAR` | `BloodGlucoseRecord` | `BloodGlucose.milligramsPerDeciliter`, ⚠️ ≤ 900.91 mg/dL (= 50 mmol/L) | +| `RESPIRATORY_RATE` | `RespiratoryRateRecord` | breaths/min, 0…1000 | +| `VO2MAX` | `Vo2MaxRecord` | 0…100, `MEASUREMENT_METHOD_OTHER` | +| `RestingHRBaselineService` / `UserProfileEntity.hrRestingBaseline` | `RestingHeartRateRecord` | ⚠️ 1…300 | +| `MealEntryEntity` | `NutritionRecord` | One record carries energy + all macros — simpler than HealthKit's 7 separate types | + +**Exercise type map** — `ActivityMeta.ORDER` (`ui/components/ActivityMeta.kt:23`) → Health Connect +constants, following the shape of the existing `strava/StravaSportMapping.kt`: + +``` +walk -> EXERCISE_TYPE_WALKING gym -> EXERCISE_TYPE_STRENGTH_TRAINING +run -> EXERCISE_TYPE_RUNNING squash -> EXERCISE_TYPE_SQUASH +cycle -> EXERCISE_TYPE_BIKING yoga -> EXERCISE_TYPE_YOGA +hike -> EXERCISE_TYPE_HIKING dance -> EXERCISE_TYPE_DANCING +sport -> EXERCISE_TYPE_OTHER_WORKOUT else -> EXERCISE_TYPE_OTHER_WORKOUT +``` + +### `clientRecordId` scheme + +Ported directly from `/PulseLoop/Health/HealthKitTypeMappings.swift:100-139`: + +``` +pl-hr- HeartRateRecord series bucket version = max(createdAt) in bucket +pl-m-- instantaneous vitals version = 1 +pl-sleep- SleepSessionRecord version = session.updatedAt +pl-act-- daily aggregates version = row.updatedAt +pl-wk- ExerciseSessionRecord version = session.updatedAt +pl-wk-- workout child records version = session.updatedAt +``` + +Two identity traps specific to Android, both of which would silently create duplicates: + +1. **`SleepStageBlockEntity.id` is a fresh random UUID on every re-sync** — `upsertSleepSessionAtomic` + replaces the blocks. Never key on it (iOS can, because its block ids are stable). Health Connect's + model helps here: one `SleepSessionRecord` holds all stages, so key on the session's `date`, which + is uniquely indexed and stable. +2. **`MeasurementEntity.id` is a random UUID for live rows** and a stable `history::` for + history rows. Derive the id from `kindRaw` + `timestamp` instead, so a reading that arrives once + live and once via history collapses onto one record. + +### Heart rate needs bucketing + +`HeartRateRecord` is a **series** record — Google's guidance is explicitly *"avoid creating single, +long-duration records; structure data into smaller records"*. Do not write one record per sample. + +Bucket by local hour, `clientRecordId = pl-hr-`. The subtlety: an hour that gains +samples on a later sync must re-upsert the *whole* hour, so after selecting new rows by watermark, +re-query every touched hour in full by `timestamp` and rebuild the record. `clientRecordVersion` = +the max `createdAt` in the bucket, so a later, fuller version always wins. + +Gadgetbridge additionally splits a series on a local-date change, a > 15 min gap, and at 1 000 +samples — worth copying — and bumps `endTime` by 1 s when start == end, because Health Connect +requires a positive duration. + +### Robustness constants (measured from Gadgetbridge's production code) + +- `CHUNK_SIZE = 200` records per `insertRecords` call. +- 5 retries, exponential backoff from 1 s (1 / 2 / 4 / 8 / 16 s). `SecurityException` aborts + immediately — never retry a permission failure. +- **1 MB per-record platform limit**, not exposed by any API. The only variable-size record we build + is a GPS route inside `ExerciseSessionRecord`. Parse the limit out of the exception message + (`"single record size limit: 1000000, was: 1700644"`) and uniformly decimate the route to ~90 % of + the limit, preserving first and last points, never duplicating a timestamp. +- Day-sliced backfill so a large history never blows the memory limit. +- Advance the watermark **only** to a timestamp that actually reached Health Connect, and **never** + rewind. + +--- + +## 4. Phases + +Each phase is a self-contained PR: builds, tests pass, runtime-verified on `emulator-5554` before the +next starts. Branch `feat/health-connect-`; commits `feat(health): …`; no `Co-Authored-By` +trailer. + +### Phase 0 — Foundation (no data written yet) + +- `app/build.gradle.kts`: `implementation("androidx.health.connect:connect-client:1.1.0")`. No version + catalog in this repo — add the coordinate as a literal alongside the others. Add + `` **only if** the manifest merger + complains (`minSdk = 26` should already satisfy it; Gadgetbridge needs it because it's on 23). +- `AndroidManifest.xml`: the `WRITE_*` permissions for Phases 1–4 only (add Phase 5's when Phase 5 + lands — a narrower first permission sheet is better UX); the `` block for + `com.google.android.apps.healthdata` plus the rationale intent; a new `HealthConnectRationaleActivity` + with the `androidx.health.ACTION_SHOW_PERMISSIONS_RATIONALE` intent-filter; and the + `ViewPermissionUsageActivity` alias guarded by `android.permission.START_VIEW_PERMISSION_USAGE` for + API 34+. +- `health/HealthConnectAvailability.kt` — wraps `HealthConnectClient.getSdkStatus()`, distinguishing + `SDK_AVAILABLE` / `PROVIDER_UPDATE_REQUIRED` / `UNAVAILABLE` so the UI can say something useful. +- `health/HealthConnectPermissions.kt` — permission sets grouped by logical data type, derived from + record classes via `HealthPermission.getWritePermission(X::class)`. Never hardcode the strings. +- `health/HealthConnectPrefsStore.kt` — `enabled` (default **false**), per-type toggles (default true), + `backfillChoice`, watermarks, `lastSyncAt`, `lastSyncSummary`, and the last-granted permission set. + Tolerant JSON decode so a new key never wipes an existing blob. +- `ui/screens/SettingsSubScreens.kt` — a `HealthConnectSettingsScreen` modeled on the existing + `StravaSettingsScreen` (line 2252); a row in `SettingsScreen.kt`; a route in `PulseLoopApp.kt`. + The master toggle launches + `rememberLauncherForActivityResult(PermissionController.createRequestPermissionResultContract())` — + `MainActivity` is a `ComponentActivity`, so no `FragmentActivity` is needed. +- Partial grants are first-class: any granted permission counts as connected; each pass re-checks its + own record class against the granted set. + +**Verify:** settings screen renders on API 35 (built-in provider) and on an API 30 image without the +Health Connect app (shows the install prompt, no crash); granting permissions shows PulseLoop in the +Health Connect app's connected-apps list; tapping the privacy-policy link in the permission sheet +opens the rationale screen. + +### Phase 1 — Vitals + the export engine + +The big one; every later phase only adds a mapper. + +- `health/HealthConnectTypeMappings.kt` — **pure**: id builders, plausibility guards, stage/exercise + constant maps. Keep it free of `HealthConnectClient` so it's trivially unit-testable. +- `health/HealthConnectExporter.kt` — watermark loop, chunking, retry/backoff, per-pass `try/catch` so + one failing type doesn't sink the others, run summary. +- `health/exporters/VitalsExporter.kt` — HR (bucketed series) + SpO₂ / HRV / temperature (instantaneous). +- `health/HealthConnectExportWorker.kt` + enqueue helper. +- New DAO queries in `data/dao/Daos.kt`: `MeasurementDao.createdSince(kind, watermark)` and an + hour-window re-read. Note there is currently **no** `createdAt`-based query — only + `range(kind, start, end)` by `timestamp`. +- Wire the trigger into `EventPersistenceSubscriber.kt:277` and `RingSyncWorker.doWork()`. +- Exclude `sourceRaw == "demo"` / `"mock"` rows, mirroring iOS. +- First-enable backfill dialog ("Sync all history" / "Only new data from now on") and a hard gate: no + export runs while the choice is unanswered. + +**Verify:** inject known measurements via on-device `sqlite3`, run the worker +(`adb shell cmd jobscheduler run -f com.pulseloop `), and confirm the exact values, units, and +timestamps in the Health Connect app's data browser. Then re-run the export and confirm the record +count is **unchanged** — that's the `clientRecordId` upsert working. + +### Phase 2 — Sleep + +`health/exporters/SleepExporter.kt` — one `SleepSessionRecord` per `SleepSessionEntity`, stages from +`SleepStageBlockEntity` mapped `DEEP→STAGE_TYPE_DEEP`, `LIGHT→STAGE_TYPE_LIGHT`, `REM→STAGE_TYPE_REM`, +`AWAKE→STAGE_TYPE_AWAKE`, `UNKNOWN→STAGE_TYPE_UNKNOWN`. Sort, clamp to session bounds, drop overlaps. +`clientRecordId = pl-sleep-` — **not** the block id (see the identity traps above). + +**Verify:** a re-synced night updates in place rather than producing a second session; a night that +grows (later blocks arrive) shows the longer span with no orphan. + +### Phase 3 — Daily activity + +`health/exporters/ActivityExporter.kt` — `StepsRecord`, `ActiveCaloriesBurnedRecord`, `DistanceRecord` +per day, spanning `startOfDay … min(endOfDay, now)`. Port iOS's `workoutNetting` +(`/PulseLoop/Health/HealthSyncService.swift:315-331`): subtract finished-workout kcal, and +distance only for walk/run-type sessions, so Health Connect consumers don't double-count against +Phase 4's records. + +**Verify:** a day with a recorded workout shows daily totals *plus* the workout, without the workout's +calories appearing twice. + +### Phase 4 — Workouts + GPS route + +`health/exporters/WorkoutExporter.kt` — `ExerciseSessionRecord` with the type map, `title` from +`ActivityMeta.label`, embedded `ExerciseRoute` from accepted `ActivityGpsPointEntity` rows, plus +sibling energy/distance records. Route sanitisation: drop points outside the session window, +non-finite or out-of-range coordinates, and duplicate timestamps; require ≥ 2 points; skip the route +entirely if `WRITE_EXERCISE_ROUTE` wasn't granted (the session still writes). Implement the 1 MB +decimation fallback. + +Deletion hooks: when a session is deleted locally (UI trash icon, coach `delete_activity_session`), +call `deleteRecords(ExerciseSessionRecord::class, clientRecordIdsList = listOf("pl-wk-"))`. + +**Verify:** record a real GPS walk on the emulator (`adb emu geo fix`), finish it, confirm the route +renders in Health Connect; delete it locally and confirm it disappears from Health Connect. + +### Phase 5 — Beyond iOS + +Blood pressure (pair systolic/diastolic rows by timestamp into one `BloodPressureRecord`), glucose, +respiratory rate, VO₂max, resting HR, and `NutritionRecord` from `MealEntryEntity`. Add the matching +manifest permissions in this phase, not earlier. + +### Phase 6 — Lifecycle, removal, docs + +- **"Remove PulseLoop data from Health Connect"** — `deleteRecords` by record type over our own + records, then clear all watermarks. Mirrors iOS's `removeAllExportedData`. +- **Revocation detection** — store the last-granted permission set; on app start and on + settings-screen open, diff against `permissionController.getGrantedPermissions()`. If everything was + revoked, offer to reset the watermarks so a later re-grant re-exports (Gadgetbridge's + `HealthConnectResetDialogFragment` pattern). A `SecurityException` from `insertRecords` should also + trigger a re-check. +- **`DataArchiveService` restore** should stamp watermarks to now, so an imported archive doesn't + re-export the whole history (iOS does this at `DataArchiveService.swift:458-460`). No `ALL_TABLES` + change is needed — state lives in SharedPreferences, not Room. +- Update `docs/ios-sync.md`: fill in the Android commit for the port-queue row, and move + "HealthKit-adjacent integrations" out of the "iOS-only intentional divergences" list once Phase 1 + ships. + +--- + +## 5. Verification + +Repo convention is **"Runtime-verified on `emulator-5554`"** with specifics — what was tapped, what +the DB showed, `sqlite3` / `dumpsys` / `adb` output. Per phase: + +1. `./gradlew testDebugUnitTest` — pure mappers, id determinism, plausibility bounds, HR bucketing, + sleep-stage clamping, route sanitisation, watermark monotonicity. +2. `./gradlew assembleDebug`, install on an API 35 emulator (built-in Health Connect). +3. Grant permissions, run the export, inspect the actual records in the Health Connect app. +4. **Re-run the export and confirm no duplicates** — the single most important check. +5. Check an API 30 emulator without the Health Connect APK degrades gracefully. +6. Before the final merge: `assembleRelease` and smoke-test on device. Release builds have + `isMinifyEnabled = true`; confirm R8 hasn't stripped anything the Health Connect client reflects on. + +### Testing approach + +This repo has JUnit 4 + `kotlinx-coroutines-test` and *no* MockK, Mockito, or Robolectric, with +`isReturnDefaultValues = true`. Gadgetbridge works within exactly the same constraints, and its two +techniques port directly: + +- **Declare the pure `convertSample`-equivalent functions `internal`, not `private`**, so same-module + tests can call them without touching a client. This is an API-shaping decision to make in Phase 1, + not retrofit later. +- **Hand-roll a `CapturingClient : HealthConnectClient`** that collects `insertRecords` calls and + throws `NotImplementedError()` on the other ~12 interface methods. ~15 lines, no mocking framework, + and it lets `runBlocking` drive a whole exporter pass. See + `Gadgetbridge/app/src/test/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/syncers/HeartRateSyncerTest.kt`. + +Confirm early in Phase 1 that constructing `androidx.health.connect.client` record classes works under +plain JVM unit tests. Gadgetbridge does it, so it should — but verify before building the test suite +around it. + +--- + +## 6. Reference files + +| Concern | Read this | +|---|---| +| iOS behaviour to match | `/PulseLoop/Health/HealthSyncService.swift`, `HealthKitTypeMappings.swift`, `/PulseLoop/Settings/AppleHealthPrefsStore.swift` | +| Client availability, permissions | `Gadgetbridge/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/util/healthconnect/HealthConnectClientProvider.kt`, `HealthConnectPermissionManager.kt` | +| Orchestrator: slicing, cursors, insert + retry, route shrink | `Gadgetbridge/.../util/healthconnect/HealthConnectUtils.kt` (979 L) | +| Syncer abstraction + `clientRecordMetadata` | `Gadgetbridge/.../util/healthconnect/syncers/HealthConnectSyncer.kt`, `AbstractTimeSampleSyncer.kt` | +| Sleep identity across re-syncs | `Gadgetbridge/.../util/healthconnect/syncers/SleepSyncer.kt` | +| Workouts + route + companion records | `Gadgetbridge/.../util/healthconnect/syncers/RecordedWorkoutSyncer.kt` | +| Manifest requirements | `Gadgetbridge/app/src/main/AndroidManifest.xml:116-139`, `:1153-1175`, `:1313-1324` | +| Android integration precedent | `app/src/main/java/com/pulseloop/strava/` | +| Trigger point | `app/src/main/java/com/pulseloop/service/EventPersistenceSubscriber.kt:277` | +| Settings-screen precedent | `app/src/main/java/com/pulseloop/ui/screens/SettingsSubScreens.kt:2252` (`StravaSettingsScreen`) | + +--- + +## 7. Open items + +- **HRV semantics.** Health Connect only has `HeartRateVariabilityRmssdRecord` (RMSSD); iOS writes + SDNN. The rings' reported HRV metric is vendor-specific and undocumented, so we'd be labelling it + RMSSD without proof. Worth a one-line caveat in the settings screen footer, and worth checking the + CRP/Colmi decompiles before Phase 1 ships. The ⚠️ 1–200 ms clamp will also silently drop readings + iOS accepts. +- **Skin vs body temperature.** `SkinTemperatureRecord` is semantically right for a ring but is a + feature-gated (`FEATURE_SKIN_TEMPERATURE`) baseline-plus-deltas series requiring a rolling baseline + (Gadgetbridge maintains a 3-day rolling average defaulting to 33 °C). `BodyTemperatureRecord` matches + iOS and is far simpler. Start with body temperature; skin temperature is a candidate for a later + phase. +- **Background persistence gap** (pre-existing, noted above): `EventPersistenceSubscriber` lives in the + composable while `RingSyncWorker` runs backgrounded. Not this plan's problem, but it bounds how much + data a background-only user actually accumulates to export. diff --git a/docs/ios-sync.md b/docs/ios-sync.md index dc85511..804acd6 100644 --- a/docs/ios-sync.md +++ b/docs/ios-sync.md @@ -113,6 +113,7 @@ seeded-data mode). **SKIP** — no portable behavior, Android has its own indepe | ☑ | [#98](https://github.com/saksham2001/PulseLoopiOS/pull/98) `ac01555` | ~07-27 | On-device daily calorie estimation (Mifflin-St Jeor BMR + Keytel/MET active energy, HR-gated) for rings that don't report calories | **PORT** | M | `0ca53a1` | | ☑ | [#130](https://github.com/saksham2001/PulseLoopiOS/pull/130) `cf5c0f4` | ~08-04 | RWfit ring family (dual 0x7E/0xAB protocol, full metric set, service-UUID recognition) | **ADAPT** | L–XL | Backed out of PR #45, then **rebuilt from `decompiled-rwfit-official/`** on `feat/rwfit-vendor-rebuild`. Legacy `0x7E` path complete; JieLi `0xAB` framing complete but its history bodies are not decoded yet. **No hardware validation.** See below. | | ☑ | [#131](https://github.com/saksham2001/PulseLoopiOS/pull/131) `88c0f6b` | ~08-08 | Sleep hypnogram label alignment + press-and-hold stage scrubber (+ sync spinner rewrite, iOS-only) | **ADAPT** | S–M | `802789d` | +| ☐ | [#80](https://github.com/saksham2001/PulseLoopiOS/pull/80) `c1275ad` | 07-11 | **Apple Health sync → Health Connect** (per-type toggles, vitals/sleep/activity/workout export, backfill choice, remove-all). Re-triaged 2026-08-09 from SKIP: the *behaviour* ports even though HealthKit doesn't. Write-only; profile import can't port (Health Connect has no DOB/sex type). Design + 7-phase plan in [`health-connect-integration.md`](health-connect-integration.md); reference implementation is `Gadgetbridge/` at the parent repo root, not iOS. Not blocked by the Play Store — the declaration form is a publishing gate, and Gadgetbridge ships this sideload-only. | **ADAPT** | XL | | ## Port priority — open items (as of 2026-08-08) @@ -131,6 +132,14 @@ seeded-data mode). **SKIP** — no portable behavior, Android has its own indepe > then recombined. > > Next triage after those: `git -C log --first-parent --oneline 88c0f6b..main`. +> +> **Newly queued, independent of the two above:** **#80 → Health Connect** (re-triaged +> 2026-08-09 from SKIP to ADAPT/XL). Design and a 7-phase implementation plan are written up in +> [`health-connect-integration.md`](health-connect-integration.md); no code yet. Start at Phase 0. +> Unlike every other row in this ledger, the reference implementation is **not** iOS — it's +> `Gadgetbridge/` at the parent repo root, which ships this sideload-only and solves the Android +> -specific problems (record identity, series bucketing, rate limits, the 1 MB record cap) that +> HealthKit doesn't have. > **▶ RESUME HERE (next session):** Tier 1 and Tier 2 both fully clear — **#65 is DONE**, re-triaged > into #65a–f, all landed 2026-07-17/18: **#65a** persistence (`daed897`), **#65b** usage tracking @@ -1261,7 +1270,7 @@ main-thread access from a background worker, and Room calls on the right dispatc | [#47](https://github.com/saksham2001/PulseLoopiOS/pull/47) [#46](https://github.com/saksham2001/PulseLoopiOS/pull/46) [#39](https://github.com/saksham2001/PulseLoopiOS/pull/39) | Release-IPA CI workflow + fixes | iOS CI | | [#45](https://github.com/saksham2001/PulseLoopiOS/pull/45) [#37](https://github.com/saksham2001/PulseLoopiOS/pull/37) [#28](https://github.com/saksham2001/PulseLoopiOS/pull/28) [#23](https://github.com/saksham2001/PulseLoopiOS/pull/23) | Sideloading guide, iOS-vs-Android refresh, MkDocs site, README updates | Docs | | [#7](https://github.com/saksham2001/PulseLoopiOS/pull/7) `c9897c9` | OSS setup (templates, SwiftLint, CI) | Repo governance; Android repo has its own | -| [#80](https://github.com/saksham2001/PulseLoopiOS/pull/80) `c1275ad` | Apple Health sync (per-type toggles, workout export, profile import) | HealthKit — intentional iOS-only divergence. Android analogue is **Health Connect**; use this as the reference design if ever wanted | +| [#80](https://github.com/saksham2001/PulseLoopiOS/pull/80) `c1275ad` | Apple Health sync (per-type toggles, workout export, profile import) | HealthKit itself is iOS-only, but the **behaviour now has an Android home**: re-triaged 2026-08-09 as **ADAPT** → Health Connect. Design + phase plan in [`health-connect-integration.md`](health-connect-integration.md); tracked in the port queue above. This row stays here only for the HealthKit-specific parts (profile import can't fully port — Health Connect has no date-of-birth or biological-sex data type) | | [#81](https://github.com/saksham2001/PulseLoopiOS/pull/81) `32dfbe3` | Automated contributor recognition (Action + script + README) | Repo governance; Android repo has its own | | [#89](https://github.com/saksham2001/PulseLoopiOS/pull/89) `0a8ab4e` | iOS-26 Liquid Glass rendering correctness + Dynamic Type a11y | Glass is an iOS visual language (standing SKIP); portable reactivity bit folds into #88 | | `25e49fd` `577c5f3` `35d1aa7` `ee42b10` `b3697c0` `0f500fc` | Direct commits: docs/screenshots/tagline/YCBT-spec/Discord/jring-URLs | Docs | From 85a69c29de936d70fe8f3220735b87db87099752 Mon Sep 17 00:00:00 2001 From: Khoa Truong Date: Mon, 10 Aug 2026 12:58:02 -0700 Subject: [PATCH 3/4] build: bump versionCode to 35 --- app/build.gradle.kts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/build.gradle.kts b/app/build.gradle.kts index b0621e5..6139531 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -19,7 +19,7 @@ android { // versionCode/versionName are overridable from Gradle properties so the release CI // can drive them straight from the git tag (e.g. -PappVersionCode=5 -PappVersionName=1.0.0). // Local builds fall back to the literals below. - versionCode = (project.findProperty("appVersionCode") as String?)?.toIntOrNull() ?: 34 + versionCode = (project.findProperty("appVersionCode") as String?)?.toIntOrNull() ?: 35 versionName = (project.findProperty("appVersionName") as String?) ?: "2.5.0" testInstrumentationRunner = "androidx.test.runner.AndroidJUnitRunner" From 0cc2c59eb57a35cc265b14713110473ca0df7c87 Mon Sep 17 00:00:00 2001 From: Khoa Truong Date: Mon, 10 Aug 2026 13:21:59 -0700 Subject: [PATCH 4/4] fix(coach): wrap body-read failures as Transport; keep timeout detail; cancellable retry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-ups on this branch's own network-error work. **A read failure during the body stream bypassed the new error copy.** `response.body?.string()` sat outside the try, so anything thrown after the response headers arrived — a mid-stream disconnect, or a read timeout firing while the body was still coming — escaped as a raw exception, never became `ResponsesError.Transport`, and fell through `CoachTurnError.from` to its generic `else` branch. For a timeout that reprinted the JDK's one-word `timeout`: exactly the bug this branch set out to fix, on one of the two paths that produce it. The call and the body read now share one try, inside `use` so the response is closed on every path including a mid-read failure. That required separating outcomes from failures: an HTTP status is an *answer* from the provider, not a transport error, so `ResponsesError` is caught and rethrown ahead of the catch-all rather than being retried or re-wrapped as Transport. **The timeout branch dropped its detail.** Every sibling branch appends the raw text, and the KDoc promises it. OkHttp reports connect and read timeouts as the same exception and only the message tells them apart (`failed to connect to … after 30000ms` vs `timeout`), so that was the one string a bug report needed. Restored, and the copy no longer asserts the read-timeout reading. **`Thread.sleep` -> `delay`.** `post` is now suspend; all four call sites are already directly inside `override suspend fun send`, so no caller changed. Previously a cancelled turn (user leaves the coach screen, WorkManager stops the summary worker) left an IO thread parked and then fired the remaining doomed attempts anyway. Also drops the now-dead `lastFailure` accumulator. **Documented what the taller tile does not fix.** The font-scale height only helps tiles whose content is a plain sp-measured column — Activity, Sleep, Chart. `GaugeTile`/`BpRingColumn` pin `VitalRingGauge` to a dp literal and derive centre font sizes from it inside a `Box(modifier.size(size))`, so their centre text still overflows its ring at a high font scale. Pre-existing and separate; noted in the KDoc so it isn't rediscovered. `baseHeight` made private. Adds mockwebserver (matching the okhttp version) and `ResponsesHttpTest`, which drives the transport over a real socket instead of hand-building the error wrappers — the reason the body-read gap was invisible to the existing tests. Verified as a real regression test: against the previous commit, `testFailureWhileStreamingTheBodyIsStillATransportError` fails. 875 tests green; assembleDebug green. --- app/build.gradle.kts | 3 + .../coach/openai/OpenAIResponsesClient.kt | 35 ++++-- .../coach/orchestration/CoachTurnError.kt | 8 +- .../com/pulseloop/ui/components/TodayTiles.kt | 9 +- .../com/pulseloop/coach/CoachTurnErrorTest.kt | 20 +++- .../com/pulseloop/coach/ResponsesHttpTest.kt | 113 ++++++++++++++++++ 6 files changed, 172 insertions(+), 16 deletions(-) create mode 100644 app/src/test/java/com/pulseloop/coach/ResponsesHttpTest.kt diff --git a/app/build.gradle.kts b/app/build.gradle.kts index 6139531..167932c 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -171,4 +171,7 @@ dependencies { // Testing testImplementation("junit:junit:4.13.2") testImplementation("org.jetbrains.kotlinx:kotlinx-coroutines-test:1.9.0") + // Drives ResponsesHttp against a real socket so the retry/transport-mapping rules are tested + // end-to-end rather than by hand-constructing the error wrappers. Matches the okhttp version. + testImplementation("com.squareup.okhttp3:mockwebserver:4.12.0") } diff --git a/app/src/main/java/com/pulseloop/coach/openai/OpenAIResponsesClient.kt b/app/src/main/java/com/pulseloop/coach/openai/OpenAIResponsesClient.kt index 0ae7948..71b02d7 100644 --- a/app/src/main/java/com/pulseloop/coach/openai/OpenAIResponsesClient.kt +++ b/app/src/main/java/com/pulseloop/coach/openai/OpenAIResponsesClient.kt @@ -1,6 +1,7 @@ package com.pulseloop.coach.openai import com.pulseloop.coach.attachments.CoachImagePayload +import kotlinx.coroutines.delay import kotlinx.serialization.Serializable import kotlinx.serialization.json.* import okhttp3.MediaType.Companion.toMediaType @@ -51,19 +52,30 @@ internal object ResponsesHttp { * [ResponsesError.Transport] on network failure and [ResponsesError.Http] * (with the error body) on a non-2xx status. */ - fun post(url: String, body: ByteArray, headers: Map = emptyMap()): String { + suspend fun post(url: String, body: ByteArray, headers: Map = emptyMap()): String { val builder = okhttp3.Request.Builder() .url(url) .post(okhttp3.RequestBody.create(jsonMediaType, body)) for ((name, value) in headers) builder.header(name, value) val request = builder.build() - var lastFailure: Exception? = null for (attempt in 0..MAX_UNSENT_RETRIES) { - val response = try { - client.newCall(request).execute() + try { + // Both the call and the body read live inside the try. A read timeout can fire + // while the body is still streaming, and if that escaped uncaught it would reach + // CoachTurnError as a bare SocketTimeoutException — bypassing the transport copy + // and printing the JDK's one-word "timeout" again, the exact bug this fixes. + // `use` closes the response on every path, including a mid-read failure. + return client.newCall(request).execute().use { response -> + val text = response.body?.string() ?: "" + if (!response.isSuccessful) throw ResponsesError.Http(response.code, text) + text + } + } catch (e: ResponsesError) { + // An HTTP status is an answer from the provider, not a transport failure. Never + // retried, and never re-wrapped as Transport by the catch below. + throw e } catch (e: Exception) { - lastFailure = e // Only retry failures that provably never reached the provider: DNS resolution and // TCP connect. A momentary DNS miss (radio handover, a VPN or private-DNS resolver // still coming up) otherwise kills the whole turn and burns the user's message. @@ -73,15 +85,14 @@ internal object ResponsesHttp { // "sent, answer lost" — and re-sending the latter bills the user's API key for a // generation that already ran. if (!isProvablyUnsent(e) || attempt == MAX_UNSENT_RETRIES) throw ResponsesError.Transport(e) - Thread.sleep(RETRY_BACKOFF_MS shl attempt) // 400ms, 800ms - continue + // delay(), not Thread.sleep(): the turn is cancellable (the user leaves the coach + // screen, WorkManager stops the summary worker), and a blocking sleep would keep an + // IO thread parked and then fire the remaining doomed attempts anyway. + delay(RETRY_BACKOFF_MS shl attempt) // 400ms, 800ms } - val text = response.body?.string() ?: "" - if (!response.isSuccessful) throw ResponsesError.Http(response.code, text) - return text } - // Unreachable — the loop either returns or throws — but keeps the compiler happy. - throw ResponsesError.Transport(lastFailure ?: IllegalStateException("request never ran")) + // Unreachable — the final attempt either returns or throws — but keeps the compiler happy. + throw IllegalStateException("request never ran") } /** diff --git a/app/src/main/java/com/pulseloop/coach/orchestration/CoachTurnError.kt b/app/src/main/java/com/pulseloop/coach/orchestration/CoachTurnError.kt index c1f604e..2c4c179 100644 --- a/app/src/main/java/com/pulseloop/coach/orchestration/CoachTurnError.kt +++ b/app/src/main/java/com/pulseloop/coach/orchestration/CoachTurnError.kt @@ -81,9 +81,13 @@ data class CoachTurnError( "now — check your connection, and whether a VPN or a Private DNS setting is " + "blocking it. (${detail.ifEmpty { "unknown host" }})" } + // OkHttp reports connect and read timeouts as this same exception, and only the + // message tells them apart ("failed to connect to … after 30000ms" vs "timeout"). + // The copy therefore stays neutral about which one happened, and the raw text is + // appended like every other branch so a bug report can still distinguish them. is java.net.SocketTimeoutException -> - "The provider didn't respond in time. Check your connection and try again — a " + - "VPN or a weak signal will do this." + "The provider took too long to answer. Check your connection and try again — a " + + "VPN or a weak signal will do this. (${detail.ifEmpty { "timeout" }})" is javax.net.ssl.SSLException -> "The secure connection to the provider failed. This is usually a VPN, a proxy, " + "or a network that intercepts traffic. (${detail.ifEmpty { "TLS error" }})" diff --git a/app/src/main/java/com/pulseloop/ui/components/TodayTiles.kt b/app/src/main/java/com/pulseloop/ui/components/TodayTiles.kt index 1b59bfa..e3d5ef0 100644 --- a/app/src/main/java/com/pulseloop/ui/components/TodayTiles.kt +++ b/app/src/main/java/com/pulseloop/ui/components/TodayTiles.kt @@ -36,7 +36,7 @@ import com.pulseloop.ui.theme.PulseColors /** Shared sizing so every Today tile is identical (TodayTileMetrics in Swift). */ object TodayTileMetrics { /** The design height, at the default font scale (1.0). */ - val baseHeight = 168.dp + private val baseHeight = 168.dp val corner = 20.dp /** @@ -60,6 +60,13 @@ object TodayTileMetrics { * Clamped at the bottom so a small-text user still gets the designed layout, and at the top so an * accessibility-max setting can't produce an absurd grid. The clamp is safe: at the 2.0 ceiling the * values block needs 186dp and a 1.6×-clamped tile still offers ~199dp. + * + * **Only helps tiles whose content is a plain sp-measured column** — Activity, Sleep, Chart. The + * gauge tiles are unaffected by this: `GaugeTile` and `BpRingColumn` pin `VitalRingGauge` to a dp + * literal (108.dp / 66.dp) and derive the centre font sizes from it (`size.value * 0.30f`), inside + * a `Box(modifier.size(size))` — so their centre text still overflows its ring at a high font + * scale while the tile around it has spare room. Pre-existing and separate; fixing it means making + * the gauge size font-scale-aware too, not making the tile taller. */ val height: Dp @Composable get() { diff --git a/app/src/test/java/com/pulseloop/coach/CoachTurnErrorTest.kt b/app/src/test/java/com/pulseloop/coach/CoachTurnErrorTest.kt index 043df24..c54e21a 100644 --- a/app/src/test/java/com/pulseloop/coach/CoachTurnErrorTest.kt +++ b/app/src/test/java/com/pulseloop/coach/CoachTurnErrorTest.kt @@ -83,7 +83,25 @@ class CoachTurnErrorTest { val e = CoachTurnError.from(ResponsesError.Transport(java.net.SocketTimeoutException("timeout"))) assertEquals("Network", e.code) assertNotEquals("timeout", e.reason) - assertTrue(e.reason.contains("didn't respond in time")) + assertTrue(e.reason.contains("took too long to answer")) + } + + /** + * OkHttp reports connect and read timeouts as the same exception, so the raw message is the + * only thing that tells a bug report which one happened. Every other transport branch appends + * it; this one used to drop it. + */ + @Test + fun testSocketTimeoutKeepsTheRawTextForBugReports() { + val connect = CoachTurnError.from( + ResponsesError.Transport( + java.net.SocketTimeoutException("failed to connect to api.openai.com after 30000ms"), + ), + ) + assertTrue(connect.reason.contains("failed to connect to api.openai.com after 30000ms")) + + val read = CoachTurnError.from(ResponsesError.Transport(java.net.SocketTimeoutException("timeout"))) + assertTrue(read.reason.contains("(timeout)")) } @Test diff --git a/app/src/test/java/com/pulseloop/coach/ResponsesHttpTest.kt b/app/src/test/java/com/pulseloop/coach/ResponsesHttpTest.kt new file mode 100644 index 0000000..05f3487 --- /dev/null +++ b/app/src/test/java/com/pulseloop/coach/ResponsesHttpTest.kt @@ -0,0 +1,113 @@ +package com.pulseloop.coach.openai + +import com.pulseloop.coach.orchestration.CoachTurnError +import kotlinx.coroutines.runBlocking +import okhttp3.mockwebserver.MockResponse +import okhttp3.mockwebserver.MockWebServer +import okhttp3.mockwebserver.SocketPolicy +import org.junit.After +import org.junit.Assert.* +import org.junit.Before +import org.junit.Test + +/** + * Drives [ResponsesHttp.post] against a real socket. The hand-built `ResponsesError.Transport(…)` + * tests in [CoachTurnErrorTest] cover the *mapping*; these cover whether the transport actually + * produces that wrapper on the paths users hit. + */ +class ResponsesHttpTest { + private lateinit var server: MockWebServer + + @Before + fun setUp() { + server = MockWebServer() + server.start() + } + + @After + fun tearDown() { + server.shutdown() + } + + private fun url() = server.url("/v1/responses").toString() + + private fun post() = runBlocking { ResponsesHttp.post(url(), "{}".toByteArray()) } + + @Test + fun testSuccessfulResponseReturnsTheBody() { + server.enqueue(MockResponse().setBody("""{"ok":true}""")) + assertEquals("""{"ok":true}""", post()) + assertEquals(1, server.requestCount) + } + + /** + * A non-2xx is an *answer*, not a transport failure: it must surface as [ResponsesError.Http] + * with the body intact, and must not be retried or re-wrapped as Transport by the retry + * loop's catch-all. + */ + @Test + fun testHttpErrorIsNotRetriedAndKeepsItsBody() { + server.enqueue(MockResponse().setResponseCode(429).setBody("""{"error":{"message":"slow down"}}""")) + val thrown = try { + post(); null + } catch (e: ResponsesError) { + e + } + val http = thrown as? ResponsesError.Http + assertNotNull("expected ResponsesError.Http, got $thrown", http) + assertEquals(429, http!!.status) + assertTrue(http.body.contains("slow down")) + assertEquals("one attempt only — an HTTP status is an answer", 1, server.requestCount) + } + + /** + * The regression this file exists for. The body read used to sit *outside* the try, so any + * failure after the response headers arrived — a mid-stream disconnect, or a read timeout that + * fires while the body is still coming — escaped unwrapped and `CoachTurnError` fell through to + * its generic branch, printing the raw JDK string again (for a timeout, the one word "timeout") + * instead of the transport copy. + * + * Uses a mid-body disconnect rather than a stalled socket so the test doesn't have to wait out + * the client's 60 s read timeout; both take the identical path through the body read. + */ + @Test + fun testFailureWhileStreamingTheBodyIsStillATransportError() { + server.enqueue( + MockResponse() + .setBody("""{"partial":""") + .setSocketPolicy(SocketPolicy.DISCONNECT_DURING_RESPONSE_BODY), + ) + val thrown = try { + post(); null + } catch (e: Exception) { + e + } + assertTrue( + "expected ResponsesError.Transport, got $thrown", + thrown is ResponsesError.Transport, + ) + // And it therefore reaches the user through the Network branch rather than the generic + // "else -> error.message" fallback that produced the unreadable strings. + assertEquals("Network", CoachTurnError.from(thrown as ResponsesError).code) + assertEquals("a body-read failure is not retried", 1, server.requestCount) + } + + /** + * DNS never left the device, so re-sending is free — three attempts total (1 + 2 retries), then + * the failure surfaces as Transport. Uses a host that cannot resolve rather than the server. + */ + @Test + fun testUnresolvableHostRetriesThenFailsAsTransport() { + val thrown = try { + runBlocking { + ResponsesHttp.post("https://pulseloop.invalid/v1/responses", "{}".toByteArray()) + } + null + } catch (e: Exception) { + e + } + val transport = thrown as? ResponsesError.Transport + assertNotNull("expected ResponsesError.Transport, got $thrown", transport) + assertTrue(transport!!.underlying is java.net.UnknownHostException) + } +}