From 0c5b95ec3f3ab063c75082a2e213bd2e8d0d0faf Mon Sep 17 00:00:00 2001 From: Khoa Truong Date: Mon, 21 Sep 2026 16:39:56 -0700 Subject: [PATCH 1/9] fix(coach): OpenAI rejected the coach schema before the model saw it (#77) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit strict: true makes OpenAI validate the JSON schema itself, and two things in CoachResponseSchema failed that validation, so every native-OpenAI coach turn 400'd with "Invalid schema for response_format 'coach_response'": - the chart object omitted additionalProperties: false (the reported error) - required listed four properties where strict mode demands all eleven, optionality expressed as a null union rather than a missing key Gemini is unaffected: cleanSchema strips additionalProperties and rewrites the null unions to its own nullable flag. Also on this ticket: Gemini's 503 "high demand" (the -latest aliases routing to a congested pool) now gets a bounded retry with backoff — a 503 means the request was never served, so a retry cannot double-bill; a 429 still surfaces immediately, since a quota bucket refills daily, not in seconds. And the model picker drops the retired gpt-4o / gpt-4o-mini / o4-mini slugs. --- .../pulseloop/coach/gemini/GeminiClient.kt | 39 ++++++++++++- .../coach/orchestration/CoachOrchestrator.kt | 29 ++++++++-- .../ui/screens/SettingsSubScreens.kt | 5 +- .../com/pulseloop/coach/CoachSchemaTest.kt | 51 +++++++++++++++++ .../com/pulseloop/coach/GeminiClientTest.kt | 57 +++++++++++++++++++ 5 files changed, 174 insertions(+), 7 deletions(-) diff --git a/app/src/main/java/com/pulseloop/coach/gemini/GeminiClient.kt b/app/src/main/java/com/pulseloop/coach/gemini/GeminiClient.kt index 666c73f5..a80efd2c 100644 --- a/app/src/main/java/com/pulseloop/coach/gemini/GeminiClient.kt +++ b/app/src/main/java/com/pulseloop/coach/gemini/GeminiClient.kt @@ -10,6 +10,7 @@ import com.pulseloop.coach.openai.ResponsesError import com.pulseloop.coach.openai.ResponsesHttp import com.pulseloop.coach.openai.ResponsesToolSpecs import com.pulseloop.coach.openai.TextContent +import kotlinx.coroutines.delay import kotlinx.serialization.json.* import java.util.UUID @@ -66,7 +67,7 @@ class GeminiClient( val geminiBody = buildRequestBody(req) val bodyBytes = json.encodeToString(JsonObject.serializer(), geminiBody).toByteArray() - val body = ResponsesHttp.post("$baseURL/$model:generateContent?key=$apiKey", bodyBytes) + val body = postWithOverloadRetry("$baseURL/$model:generateContent?key=$apiKey", bodyBytes) val root = try { json.parseToJsonElement(body).jsonObject @@ -76,6 +77,42 @@ class GeminiClient( return ingestResponse(root) } + /** + * POST with a bounded retry on Gemini's **503 "high demand"** (issue #77). The + * `-latest` model aliases route to whatever pool is congested that hour, so a coach + * turn could fail on first contact for days while Google's own dashboard showed the + * account nowhere near quota. A 503 means the request was never served — nothing was + * generated and nothing billed — so a retry cannot double-charge, unlike a read + * timeout, which [ResponsesHttp] deliberately never retries for exactly that reason. + * + * A **429 is not retried**: it means the key's quota bucket said no, and on the free + * tier that bucket refills daily, not in seconds. [ResponsesHttp] raises it as-is and + * the turn fails with the provider's own message. + * + * [post] is a seam so a test can fail attempts without a network stack. + */ + internal suspend fun postWithOverloadRetry( + url: String, + bodyBytes: ByteArray, + backoffMs: Long = OVERLOAD_BACKOFF_MS, + post: suspend (String, ByteArray) -> String = { u, b -> ResponsesHttp.post(u, b) }, + ): String { + for (attempt in 0..MAX_OVERLOAD_RETRIES) { + try { + return post(url, bodyBytes) + } catch (e: ResponsesError.Http) { + if (e.status != 503 || attempt == MAX_OVERLOAD_RETRIES) throw e + delay(backoffMs shl attempt) // 2 s, then 8 s + } + } + throw IllegalStateException("overload retry loop never returned") + } + + private companion object { + const val MAX_OVERLOAD_RETRIES = 2 + const val OVERLOAD_BACKOFF_MS = 2_000L + } + // ── Request assembly (internal for unit tests) ─────────────────────── /** Translates one Responses-API request into the Gemini `generateContent` diff --git a/app/src/main/java/com/pulseloop/coach/orchestration/CoachOrchestrator.kt b/app/src/main/java/com/pulseloop/coach/orchestration/CoachOrchestrator.kt index 81904f9a..03df919e 100644 --- a/app/src/main/java/com/pulseloop/coach/orchestration/CoachOrchestrator.kt +++ b/app/src/main/java/com/pulseloop/coach/orchestration/CoachOrchestrator.kt @@ -294,7 +294,11 @@ object CoachResponseSchema { "items" to JsonObject(mapOf("type" to JsonPrimitive("string"))), )), "chart" to JsonObject(mapOf( - "type" to JsonPrimitive("object"), + // Nullable: most answers carry no chart. Under `strict: true` OpenAI requires + // every property to be listed in `required`, with optionality expressed as a + // null union — not by omitting the key (issue #77: the 400 was the missing + // `additionalProperties`, and an incomplete `required` was the next one). + "type" to JsonArray(listOf(JsonPrimitive("object"), JsonPrimitive("null"))), "properties" to JsonObject(mapOf( "chart_type" to JsonObject(mapOf("type" to JsonPrimitive("string"))), "title" to JsonObject(mapOf("type" to JsonPrimitive("string"))), @@ -313,13 +317,21 @@ object CoachResponseSchema { )), )), )), + "required" to JsonArray(listOf( + "chart_type", "title", "x_label", "y_label", "points", + ).map { JsonPrimitive(it) }), + "additionalProperties" to JsonPrimitive(false), )), "confidence" to JsonObject(mapOf( "type" to JsonPrimitive("string"), "enum" to JsonArray(listOf("low", "medium", "high").map { JsonPrimitive(it) }), )), - "safety_note" to JsonObject(mapOf("type" to JsonPrimitive("string"))), - "data_quality_note" to JsonObject(mapOf("type" to JsonPrimitive("string"))), + "safety_note" to JsonObject(mapOf( + "type" to JsonArray(listOf(JsonPrimitive("string"), JsonPrimitive("null"))), + )), + "data_quality_note" to JsonObject(mapOf( + "type" to JsonArray(listOf(JsonPrimitive("string"), JsonPrimitive("null"))), + )), "sources" to JsonObject(mapOf( "type" to JsonPrimitive("array"), "items" to JsonObject(mapOf( @@ -329,7 +341,9 @@ object CoachResponseSchema { "url" to JsonObject(mapOf("type" to JsonPrimitive("string"))), "publisher" to JsonObject(mapOf("type" to JsonPrimitive("string"))), )), - "required" to JsonArray(listOf(JsonPrimitive("title"), JsonPrimitive("url"))), + "required" to JsonArray(listOf( + JsonPrimitive("title"), JsonPrimitive("url"), JsonPrimitive("publisher"), + )), "additionalProperties" to JsonPrimitive(false), )), )), @@ -342,8 +356,13 @@ object CoachResponseSchema { "items" to JsonObject(mapOf("type" to JsonPrimitive("string"))), )), )), + // OpenAI strict structured outputs (issue #77): `required` must list **every** + // property — optionality is a null union, not a missing key — and every object + // needs `additionalProperties: false`. CoachResponseParser still only demands the + // four load-bearing keys, so a compliant reply decodes with the rest defaulted. "required" to JsonArray(listOf( - "response_type", "title", "summary", "confidence" + "response_type", "title", "summary", "bullets", "chart", "safety_note", + "data_quality_note", "sources", "follow_up_chips", "actions_taken", "confidence", ).map { JsonPrimitive(it) }), "additionalProperties" to JsonPrimitive(false), )) diff --git a/app/src/main/java/com/pulseloop/ui/screens/SettingsSubScreens.kt b/app/src/main/java/com/pulseloop/ui/screens/SettingsSubScreens.kt index bd57254a..ebb22175 100644 --- a/app/src/main/java/com/pulseloop/ui/screens/SettingsSubScreens.kt +++ b/app/src/main/java/com/pulseloop/ui/screens/SettingsSubScreens.kt @@ -201,7 +201,10 @@ fun CoachSettingsScreen(onBack: () -> Unit) { } } - val models = listOf("gpt-5.4", "gpt-4o", "gpt-4o-mini", "o4-mini") + // Retired slugs removed (issue #77): gpt-4o / gpt-4o-mini / o4-mini are rejected by the + // Responses API, and a picker entry that cannot work is worse than a shorter list. New + // models go here as OpenAI ships them. + val models = listOf("gpt-5.4") SettingsSubScreen(title = "AI Coach", onBack = onBack) { // AI Coach section — ported from CoachSettingsSection.swift diff --git a/app/src/test/java/com/pulseloop/coach/CoachSchemaTest.kt b/app/src/test/java/com/pulseloop/coach/CoachSchemaTest.kt index 5d7fa8ec..97424dba 100644 --- a/app/src/test/java/com/pulseloop/coach/CoachSchemaTest.kt +++ b/app/src/test/java/com/pulseloop/coach/CoachSchemaTest.kt @@ -1,6 +1,7 @@ package com.pulseloop.coach.schema import com.pulseloop.coach.orchestration.CoachResponseParser +import com.pulseloop.coach.orchestration.CoachResponseSchema import kotlinx.serialization.json.* import org.junit.Assert.* import org.junit.Test @@ -76,6 +77,56 @@ class CoachSchemaTest { assertTrue(text.contains("HR: 72 bpm")) } + @Test + fun `the coach_response schema satisfies OpenAI strict structured outputs`() { + // Issue #77: the coach chat sends `strict: true`, under which OpenAI validates the + // SCHEMA itself before the model ever sees the request — every object must declare + // `additionalProperties: false`, and every property must be listed in `required` + // (optionality is a null union, not a missing key). `chart` shipped without either, + // so every native-OpenAI coach turn 400'd with: + // Invalid schema for response_format 'coach_response': In context=('properties', + // 'chart'), 'additionalProperties' is required to be supplied and to be false. + // This walk re-checks the whole tree so the next schema field can't reintroduce it. + fun walk(node: JsonObject, path: String) { + val type = node["type"] + val typeNames = when (type) { + is JsonPrimitive -> listOf(type.content) + is JsonArray -> type.mapNotNull { (it as? JsonPrimitive)?.contentOrNull } + else -> emptyList() + } + if ("object" in typeNames) { + val additional = node["additionalProperties"] + assertTrue( + "$path must declare additionalProperties: false under strict mode", + additional is JsonPrimitive && additional.content == "false", + ) + val required = (node["required"] as? JsonArray) + ?.mapNotNull { (it as? JsonPrimitive)?.contentOrNull } + ?: emptyList() + val props = node["properties"] as? JsonObject + assertNotNull("$path is an object with no properties", props) + assertEquals( + "$path: strict mode requires EVERY property to be listed in required", + props!!.keys, + required.toSet(), + ) + } + (node["properties"] as? JsonObject)?.forEach { (name, value) -> + val sub = value as? JsonObject ?: return@forEach + val subType = sub["type"] + if (subType is JsonArray) { + assertTrue( + "$path.properties.$name: a nullable union must include null", + subType.any { (it as? JsonPrimitive)?.contentOrNull == "null" }, + ) + } + walk(sub, "$path.properties.$name") + (sub["items"] as? JsonObject)?.let { walk(it, "$path.properties.$name.items") } + } + } + walk(CoachResponseSchema.schema, "coach_response") + } + @Test fun testPlainTextNoBullets() { val response = CoachResponse(summary = "Just a summary.") diff --git a/app/src/test/java/com/pulseloop/coach/GeminiClientTest.kt b/app/src/test/java/com/pulseloop/coach/GeminiClientTest.kt index f50b162a..0fa36de0 100644 --- a/app/src/test/java/com/pulseloop/coach/GeminiClientTest.kt +++ b/app/src/test/java/com/pulseloop/coach/GeminiClientTest.kt @@ -1,6 +1,7 @@ package com.pulseloop.coach.gemini import com.pulseloop.coach.openai.FunctionCallOutput +import com.pulseloop.coach.openai.ResponsesError import kotlinx.serialization.json.* import org.junit.Assert.* import org.junit.Test @@ -334,4 +335,60 @@ class GeminiClientTest { assertTrue(note.startsWith("Web search sources")) assertTrue(note.contains("https://example.com")) } + + // ── Overload retry (issue #77) ───────────────────────────────────── + + @Test + fun `a 503 is retried and the turn survives`() = kotlinx.coroutines.runBlocking { + // gemini-flash-latest routes to whatever pool is congested; "high demand" 503s killed + // every coach turn for days on end. A 503 means the request was never served, so + // retrying cannot double-bill — unlike a read timeout, which stays never-retried. + val client = GeminiClient("key") + var attempts = 0 + val result = client.postWithOverloadRetry( + "https://example.test/generateContent", ByteArray(0), backoffMs = 1, + ) { _, _ -> + if (attempts++ < 2) throw ResponsesError.Http(503, "This model is currently experiencing high demand.") + "ok" + } + assertEquals("ok", result) + assertEquals(3, attempts) + } + + @Test + fun `the last 503 still surfaces`() = kotlinx.coroutines.runBlocking { + val client = GeminiClient("key") + var attempts = 0 + try { + client.postWithOverloadRetry( + "https://example.test/generateContent", ByteArray(0), backoffMs = 1, + ) { _, _ -> + attempts++ + throw ResponsesError.Http(503, "still overloaded") + } + fail("a persistently overloaded model must surface, not hang") + } catch (e: ResponsesError.Http) { + assertEquals(503, e.status) + } + assertEquals(3, attempts) + } + + @Test + fun `a 429 is never retried`() = kotlinx.coroutines.runBlocking { + // Quota buckets refill daily, not in seconds — retrying only burns the turn's wall time. + val client = GeminiClient("key") + var attempts = 0 + try { + client.postWithOverloadRetry( + "https://example.test/generateContent", ByteArray(0), backoffMs = 1, + ) { _, _ -> + attempts++ + throw ResponsesError.Http(429, "You exceeded your current quota.") + } + fail("429 must surface immediately") + } catch (e: ResponsesError.Http) { + assertEquals(429, e.status) + } + assertEquals(1, attempts) + } } From fb3cb80ad27e821556f108b41c18ec2e5c6e7463 Mon Sep 17 00:00:00 2001 From: Khoa Truong Date: Mon, 21 Sep 2026 16:54:30 -0700 Subject: [PATCH 2/9] feat(activity): day navigation on the dashboard (#76) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sleep and the vital detail screens could step back through stored days; the Activity tab could not, so once a day rolled over its steps, distance and calories were out of reach — and a deleted block could not be re-checked later (the exact check the reporter wanted on #70's deletion). The header mirrors Sleep's: chevrons stepping a clamped [0, maxDayOffset] window (a week past the earliest stored day, one-year floor), Today / Yesterday / weekday / date label. The summary card and the RECORDS card follow the shown day; the weekly-goal widget keeps reading live-today, as a week widget should. deleteBucket no longer resets the shown day — verifying a past-day deletion was the point. New ActivityDailyDao.earliestDay bounds the pager, same rule as sleep's. --- .../main/java/com/pulseloop/data/dao/Daos.kt | 4 + .../pulseloop/ui/screens/ActivityScreen.kt | 84 ++++++++++++++++++- .../com/pulseloop/ui/viewmodels/ViewModels.kt | 59 ++++++++++++- 3 files changed, 143 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/com/pulseloop/data/dao/Daos.kt b/app/src/main/java/com/pulseloop/data/dao/Daos.kt index dbcbfde3..18eff4f4 100644 --- a/app/src/main/java/com/pulseloop/data/dao/Daos.kt +++ b/app/src/main/java/com/pulseloop/data/dao/Daos.kt @@ -159,6 +159,10 @@ interface ActivityDailyDao { @Query("SELECT * FROM activity_daily WHERE source NOT IN ('demo','mock') ORDER BY date DESC LIMIT :limit") suspend fun recentReal(limit: Int = 7): List + /** Earliest tracked day key (local midnight millis) — bounds how far Activity day navigation can page back (issue #76). */ + @Query("SELECT MIN(date) FROM activity_daily") + suspend fun earliestDay(): Long? + /** Whether the ring has ever synced a day. */ @Query("SELECT EXISTS(SELECT 1 FROM activity_daily WHERE source NOT IN ('demo','mock'))") suspend fun hasReal(): Boolean diff --git a/app/src/main/java/com/pulseloop/ui/screens/ActivityScreen.kt b/app/src/main/java/com/pulseloop/ui/screens/ActivityScreen.kt index a729ec61..c67fff46 100644 --- a/app/src/main/java/com/pulseloop/ui/screens/ActivityScreen.kt +++ b/app/src/main/java/com/pulseloop/ui/screens/ActivityScreen.kt @@ -14,6 +14,8 @@ import androidx.compose.foundation.shape.RoundedCornerShape import androidx.compose.material.icons.Icons import androidx.compose.material.icons.automirrored.filled.KeyboardArrowRight import androidx.compose.material.icons.filled.CalendarMonth +import androidx.compose.material.icons.filled.ChevronLeft +import androidx.compose.material.icons.filled.ChevronRight import androidx.compose.material.icons.filled.DeleteOutline import androidx.compose.material.icons.filled.ExpandLess import androidx.compose.material.icons.filled.ExpandMore @@ -130,6 +132,10 @@ fun ActivityScreen( ), verticalArrangement = Arrangement.spacedBy(16.dp), ) { + // Day navigation (issue #76): the summary + RECORDS cards follow the shown day, so a + // past day's steps/distance/calories are reachable the way Sleep already allows. + item { ActivityDayNavHeader(state, viewModel) } + item { DailyActivitySummaryCard(state, units) } item { @@ -228,8 +234,8 @@ fun ActivityScreen( ActivityRecordsCard( viewModel = viewModel, units = units, - dayStart = com.pulseloop.util.TimeUtil.startOfTodayLocal(), - dayLabel = "today", + dayStart = shownDayStart(state), + dayLabel = shownDayLabel(state), ) } item { Spacer(Modifier.height(64.dp)) } @@ -277,12 +283,84 @@ private fun weeklySteps(state: ActivityViewModel.ActivityState, todayIdx: Int): } } +// ─────────────────── Day navigation (issue #76) ─────────────────── + +/** Local-midnight key of the day the dashboard is showing. */ +private fun shownDayStart(state: ActivityViewModel.ActivityState): Long = + if (state.shownDay != 0L) state.shownDay else com.pulseloop.util.TimeUtil.startOfTodayLocal() + +/** Label for the RECORDS card's empty text: "today" as before, else the shown date. */ +private fun shownDayLabel(state: ActivityViewModel.ActivityState): String { + val today = com.pulseloop.util.TimeUtil.startOfTodayLocal() + return if (shownDayStart(state) == today) "today" + else dayHeaderLabel(shownDayStart(state)).lowercase() +} + +/** ‹ older · date · newer › stepper above the dashboard — the Sleep header's shape (iOS #84), + * minus the swipe/picker: Activity needs only to reach days the store already holds. */ +@Composable +private fun ActivityDayNavHeader(state: ActivityViewModel.ActivityState, viewModel: ActivityViewModel?) { + val canOlder = state.dayOffset < state.maxDayOffset + val canNewer = state.dayOffset > 0 + Row( + Modifier.fillMaxWidth(), + verticalAlignment = Alignment.CenterVertically, + horizontalArrangement = Arrangement.SpaceBetween, + ) { + ActivityNavChevron(Icons.Filled.ChevronLeft, "Previous day", canOlder) { viewModel?.stepDay(older = true) } + Text( + dayHeaderLabel(shownDayStart(state)), + fontSize = 16.sp, fontWeight = FontWeight.SemiBold, color = PulseColors.textPrimary, + modifier = Modifier.weight(1f), + textAlign = androidx.compose.ui.text.style.TextAlign.Center, + ) + ActivityNavChevron(Icons.Filled.ChevronRight, "Next day", canNewer) { viewModel?.stepDay(older = false) } + } +} + +@Composable +private fun ActivityNavChevron( + icon: androidx.compose.ui.graphics.vector.ImageVector, + label: String, + enabled: Boolean, + onClick: () -> Unit, +) { + Box( + Modifier + .size(44.dp) + .clip(CircleShape) + .then(if (enabled) Modifier.clickable(onClick = onClick) else Modifier), + contentAlignment = Alignment.Center, + ) { + Icon( + icon, contentDescription = label, + tint = if (enabled) PulseColors.textPrimary else PulseColors.textMuted.copy(alpha = 0.4f), + ) + } +} + +/** Same phrasing as Sleep's header: Today / Yesterday / weekday / date. */ +private fun dayHeaderLabel(dayMillis: Long): String { + val today = com.pulseloop.util.TimeUtil.startOfTodayLocal() + val daysAgo = ((today - dayMillis) / 86_400_000L).toInt() + val date = Instant.ofEpochMilli(dayMillis).atZone(ZoneId.systemDefault()) + return when { + daysAgo <= 0 -> "Today" + daysAgo == 1 -> "Yesterday" + daysAgo < 7 -> date.format(DateTimeFormatter.ofPattern("EEEE")) + date.year == java.time.Year.now().value -> date.format(DateTimeFormatter.ofPattern("EEE, MMM d")) + else -> date.format(DateTimeFormatter.ofPattern("MMM d, yyyy")) + } +} + // ─────────────────── Daily summary card (stats + rings) ─────────────────── @Composable private fun DailyActivitySummaryCard(state: ActivityViewModel.ActivityState, units: UnitSystem) { val shape = RoundedCornerShape(20.dp) - val today = state.today + // The shown day's totals, not always-today: day navigation must move this card (issue #76). + // WeeklyGoalCard keeps reading state.today — a week widget is about the live week. + val today = state.daySummary ?: state.today val distValue = today?.distanceMeters?.let { Formats.distance(UnitConverter.distance(it, units)) } Row( Modifier diff --git a/app/src/main/java/com/pulseloop/ui/viewmodels/ViewModels.kt b/app/src/main/java/com/pulseloop/ui/viewmodels/ViewModels.kt index b7506068..16da9f47 100644 --- a/app/src/main/java/com/pulseloop/ui/viewmodels/ViewModels.kt +++ b/app/src/main/java/com/pulseloop/ui/viewmodels/ViewModels.kt @@ -401,12 +401,23 @@ class SleepViewModel(private val db: PulseLoopDatabase) : ViewModel() { /** * ActivityViewModel — reads Room data for the Activity screen. */ +@OptIn(kotlinx.coroutines.ExperimentalCoroutinesApi::class) class ActivityViewModel(db: PulseLoopDatabase) : ViewModel() { data class ActivityState( val recentDays: List = emptyList(), /** All finished sessions, newest first (drives Today + the history sheet). */ val finishedWorkouts: List = emptyList(), val today: ActivityDailyEntity? = null, + // ── Day navigation (issue #76 — mirrors SleepViewModel's, iOS #84) ── + /** How many days back the shown day is: 0 = today, clamped to [0, maxDayOffset]. */ + val dayOffset: Int = 0, + /** How far the chevrons may page: a week past the earliest stored day, one-year floor. + * 0 (locked to today) when nothing is stored yet. */ + val maxDayOffset: Int = 0, + /** Local-midnight key of the shown day — drives the RECORDS card's day query. */ + val shownDay: Long = 0, + /** The shown day's totals (the summary card); `today` stays live for the week widget. */ + val daySummary: ActivityDailyEntity? = null, val stepGoal: Int = UserGoalEntity.DEFAULT_STEPS, val activeMinutesGoal: Int = 45, val distanceGoalMeters: Double = UserGoalEntity.DEFAULT_DISTANCE_METERS, @@ -418,11 +429,15 @@ class ActivityViewModel(db: PulseLoopDatabase) : ViewModel() { private val _state = MutableStateFlow(ActivityState()) val state: StateFlow = _state.asStateFlow() private val todayStart = MutableStateFlow(TimeUtil.startOfTodayLocal()) + /** The day the dashboard is looking at; [currentDayValues] handles today's own rollover. */ + private val shownDayStart = MutableStateFlow(TimeUtil.startOfTodayLocal()) private val db = db fun refreshCurrentDay() { todayStart.value = TimeUtil.startOfTodayLocal() + // A resume lands the user back on today, where they left the app from. + jumpToOffset(0) } init { @@ -436,11 +451,17 @@ class ActivityViewModel(db: PulseLoopDatabase) : ViewModel() { _state.update { it.copy(today = day) } } } + viewModelScope.launch { + shownDayStart.flatMapLatest { day -> db.activityDailyDao().byDayFlow(day) }.collect { day -> + _state.update { it.copy(daySummary = day) } + } + } viewModelScope.launch { db.activitySessionDao().recentFlow(200).collect { sessions -> _state.update { it.copy(finishedWorkouts = sessions.filter { s -> s.statusRaw == "finished" }) } } } + viewModelScope.launch { refreshMaxDayOffset() } // Reactive: goals saved from onboarding/Settings show up without a manual reload. viewModelScope.launch { try { @@ -464,10 +485,46 @@ class ActivityViewModel(db: PulseLoopDatabase) : ViewModel() { suspend fun deleteBucket(startEpoch: Long): List { val day = TimeUtil.startOfDayLocal(startEpoch) try { ActivityBucketDeletion.delete(db, startEpoch) } catch (_: Exception) {} - refreshCurrentDay() + // The daily-total flows are reactive; only the live "today" pointer needs a nudge, and + // only when the deleted day IS today — a past-day deletion must not yank the dashboard + // back from the day the user is inspecting (issue #76). + if (day == TimeUtil.startOfTodayLocal()) todayStart.value = day return bucketsForDay(day) } + // ── Day navigation (issue #76 — mirrors SleepViewModel's, iOS #84) ── + + /** Step to an older (older = true) or newer day, clamped to [0, maxDayOffset]. */ + fun stepDay(older: Boolean) = jumpToOffset(_state.value.dayOffset + if (older) 1 else -1) + + /** Jump to the day at [dayMillis] (a local-midnight key), clamped to the valid range. */ + fun jumpToDay(dayMillis: Long) { + val today = TimeUtil.startOfTodayLocal() + jumpToOffset(((today - TimeUtil.startOfDayLocal(dayMillis)) / 86_400_000L).toInt()) + } + + /** Return the dashboard to today. */ + fun resetToToday() = jumpToOffset(0) + + private fun jumpToOffset(target: Int) { + val clamped = target.coerceIn(0, _state.value.maxDayOffset) + val shown = TimeUtil.startOfTodayLocal() - clamped * 86_400_000L + shownDayStart.value = shown + _state.update { it.copy(dayOffset = clamped, shownDay = shown) } + } + + /** Recompute the pager bound: a week before the earliest stored day, one-year floor. + * Mirrors SleepViewModel.maxDayOffset; 0 (locked to today) when nothing is stored. */ + private suspend fun refreshMaxDayOffset() { + val earliest = try { db.activityDailyDao().earliestDay() } catch (_: Exception) { null } + val maxOffset = if (earliest == null) 0 else { + val today = TimeUtil.startOfTodayLocal() + val floor = maxOf(earliest - 7 * 86_400_000L, today - 365 * 86_400_000L) + ((today - floor) / 86_400_000L).toInt().coerceAtLeast(0) + } + _state.update { it.copy(maxDayOffset = maxOffset) } + } + suspend fun reloadGoals() { try { db.userGoalDao().get()?.let { goal -> From 35e72343cd8998bf749d7148598cb71b91b0a4d7 Mon Sep 17 00:00:00 2001 From: Khoa Truong Date: Mon, 21 Sep 2026 17:18:31 -0700 Subject: [PATCH 3/9] feat(sleep): delete one ring record from the night (#78) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A night can arrive as several ring records merged into one session, and the merge is usually right — but the ring opens a record on a still wrist, so the evening before the wearer got into bed can land as a full phantom session and the night runs an hour high. The RECORDS card (issue #68) shows the parts; now each row gets a delete. Three rules, mirroring the activity deletion's (issue #70): - Tombstone the blocks, not the session row: the write path re-derives the waking day from the blocks the ring re-sends, so the row alone would rebuild on the next sync. A block's minute-grid startAt is the stable identity — the same record re-sent reproduces it — keyed by waking day (sleep::), and upsertSleepSessionAtomic consults the tombstones before writing. - Re-derive the session from the blocks that remain (bounds, asleep minutes, score), never hand-patch; empty means the row goes and the day reads as unslept. The stage-score banding moves to SleepInsights.sleepStageScore so the restatement and the write path share one source of truth. - sleepRecordRuns re-splits the survivors, so the card reseals itself. Also fixes the card's now-reachable singular: a one-record night says "1 part", not "1 parts". --- .../com/pulseloop/data/SleepRecordDeletion.kt | 122 ++++++++++++++++++ .../main/java/com/pulseloop/data/dao/Daos.kt | 37 ++++++ .../service/EventPersistenceSubscriber.kt | 23 ++-- .../com/pulseloop/service/SleepInsights.kt | 17 +++ .../com/pulseloop/ui/screens/SleepScreen.kt | 56 +++++++- .../com/pulseloop/ui/viewmodels/ViewModels.kt | 11 ++ .../pulseloop/data/SleepRecordDeletionTest.kt | 99 ++++++++++++++ 7 files changed, 351 insertions(+), 14 deletions(-) create mode 100644 app/src/main/java/com/pulseloop/data/SleepRecordDeletion.kt create mode 100644 app/src/test/java/com/pulseloop/data/SleepRecordDeletionTest.kt diff --git a/app/src/main/java/com/pulseloop/data/SleepRecordDeletion.kt b/app/src/main/java/com/pulseloop/data/SleepRecordDeletion.kt new file mode 100644 index 00000000..43ccd70c --- /dev/null +++ b/app/src/main/java/com/pulseloop/data/SleepRecordDeletion.kt @@ -0,0 +1,122 @@ +package com.pulseloop.data + +import androidx.room.withTransaction +import com.pulseloop.data.entity.SleepSessionEntity +import com.pulseloop.data.entity.SleepStageBlockEntity +import com.pulseloop.ring.SleepStage + +/** + * Deleting one ring sleep record (issue #78) — the sleep sibling of [ActivityBucketDeletion]. + * + * A night can arrive as several ring records minutes apart, merged into one stored session because + * the merge is the night (#63). Mostly that merge is right — but the ring opens a record on a still + * wrist, so an evening on the sofa can land as a full phantom "session" before the wearer is even + * in bed, and the night's total runs an hour high. The wearer knows which run is wrong; the sensor + * cannot. This is the escape hatch. + * + * Three rules, mirroring the activity deletion's, and all of them live here rather than in any + * caller: + * + * * **Tombstone the blocks, not the session row.** Sleep's write path never upserts a session by + * id — `EventPersistenceSubscriber` re-derives the whole waking day from the raw stage blocks + * the ring re-sends, so deleting the row alone would let the next sync of that night rebuild it + * exactly. A block is minute-grid RLE over the record's stages, so the same record re-sent + * reproduces its blocks with the same `startAt` values; `MeasurementDeletionDao.sleepBlockId` + * keys on the waking day plus block start — the same stable-identity role the activity + * tombstone's `startEpoch` plays. The re-derive path must consult those tombstones before + * writing (it does — see `upsertSleepSessionAtomic`). + * * **Re-derive, never hand-patch.** The surviving session's bounds, asleep minutes and score are + * recomputed from the blocks that remain — the same numbers `reconcileWakingDay` produces — so + * what the UI shows after a delete is what the next sync would agree with. If nothing remains, + * the session row goes entirely and the day reads as unslept rather than reverting to the ring's + * figure on the next pass. + * * **The merge reseals itself.** The survivors are still distinct records with their own gap, so + * `sleepRecordRuns` re-splits the remaining blocks and the RECORDS card shows the shorter list + * with no stored "these were one record" state to maintain. + */ +object SleepRecordDeletion { + + /** + * Delete the ring record whose blocks carry [recordStartAt] from session [sessionId], remember + * the blocks, and restate the session from what remains. + * + * Returns true when a record was actually removed. + */ + suspend fun delete( + db: PulseLoopDatabase, + sessionId: String, + recordStartAt: Long, + ): Boolean = db.withTransaction { + val session = db.sleepSessionDao().byId(sessionId) ?: return@withTransaction false + val blocks = db.sleepStageBlockDao().forSession(sessionId) + // The run the user pointed at is the stretch of blocks stamped with that record's declared + // start (issue #68). `0` means a pre-#68 row with no stamp: fall back to "everything from + // the run boundary until the next stamped start", the same fallback `sleepRecordRuns` uses + // to split those nights for display. + val targets = if (recordStartAt == 0L) { + emptyList() + } else { + blocks.filter { it.recordStartAt == recordStartAt }.ifEmpty { + blocks.filter { it.recordStartAt == 0L && it.startAt >= recordStartAt } + .takeWhile { it.startAt < nextStampedStart(blocks, recordStartAt) } + } + } + if (targets.isEmpty()) return@withTransaction false + + // The waking day the tombstone keys under is this session's own date: every record the day + // reconciles is assigned that same `date`, so a re-send of the record lands on the same key. + val day = session.date + db.measurementDeletionDao().recordSleepBlocks(day, targets.map { it.startAt }) + targets.forEach { db.sleepStageBlockDao().deleteByStart(sessionId, it.startAt) } + + restateSession(db, session, blocks - targets.toSet()) + true + } + + /** The next stamped record boundary after [after], or end-of-night — bounds the legacy fallback. */ + private fun nextStampedStart(blocks: List, after: Long): Long = + blocks.map { it.recordStartAt }.filter { it > after }.minOrNull() ?: Long.MAX_VALUE + + /** + * Rewrite [session] from the blocks it still has — bounds, asleep minutes and score, the same + * formulas `reconcileWakingDay` applies (blocks re-keyed to the new bounds the same way). Empty + * means gone: the row is deleted and the day reads as unslept. + */ + private suspend fun restateSession( + db: PulseLoopDatabase, + session: SleepSessionEntity, + remaining: List, + ) { + if (remaining.isEmpty()) { + db.sleepSessionDao().deleteById(session.id) + return + } + val ordered = remaining.sortedBy { it.startAt } + val startAt = ordered.first().startAt + val endAt = ordered.maxOf { it.startAt + it.durationMinutes * 60_000L } + val totalMin = com.pulseloop.service.asleepMinutes(ordered) + val deepMin = ordered.filter { it.stageRaw == SleepStage.DEEP.name } + .sumOf { it.durationMinutes } + val now = System.currentTimeMillis() + db.sleepSessionDao().upsert( + session.copy( + startAt = startAt, + endAt = endAt, + totalMinutes = totalMin, + score = com.pulseloop.service.sleepStageScore(deepMin, totalMin), updatedAt = now, + ), + ) + // Re-key the surviving blocks to the restated bounds: `startMinute` is relative to the + // session start, so it moves with the new first block. + db.sleepStageBlockDao().deleteBySession(session.id) + ordered.forEach { + db.sleepStageBlockDao().insert( + it.copy( + id = java.util.UUID.randomUUID().toString(), + sessionId = session.id, + startMinute = ((it.startAt - startAt) / 60_000L).toInt().coerceAtLeast(0), + ), + ) + } + } +} diff --git a/app/src/main/java/com/pulseloop/data/dao/Daos.kt b/app/src/main/java/com/pulseloop/data/dao/Daos.kt index 18eff4f4..08e09303 100644 --- a/app/src/main/java/com/pulseloop/data/dao/Daos.kt +++ b/app/src/main/java/com/pulseloop/data/dao/Daos.kt @@ -322,6 +322,9 @@ interface SleepSessionDao { @Query("SELECT * FROM sleep_sessions WHERE date = :day ORDER BY (sourceRaw = 'demo') ASC, totalMinutes DESC LIMIT 1") suspend fun byDay(day: Long): SleepSessionEntity? + @Query("SELECT * FROM sleep_sessions WHERE id = :id LIMIT 1") + suspend fun byId(id: String): SleepSessionEntity? + /** All sessions for a day, earliest first — feeds the Day-view carousel. */ @Query("SELECT * FROM sleep_sessions WHERE date = :day ORDER BY startAt ASC") suspend fun allByDay(day: Long): List @@ -412,6 +415,10 @@ interface SleepStageBlockDao { @Query("DELETE FROM sleep_stage_blocks WHERE sessionId = :sessionId") suspend fun deleteBySession(sessionId: String) + /** Remove one run-length block — the delete affordance on the sleep RECORDS card (issue #78). */ + @Query("DELETE FROM sleep_stage_blocks WHERE sessionId = :sessionId AND startAt = :startAt") + suspend fun deleteByStart(sessionId: String, startAt: Long) + @Query("DELETE FROM sleep_stage_blocks") suspend fun clear() @@ -719,6 +726,28 @@ interface MeasurementDeletionDao { @Query("SELECT EXISTS(SELECT 1 FROM measurement_deletions WHERE measurementId = :id)") suspend fun isActivityBucketDeleted(id: String): Boolean + /** + * Remember a deleted ring sleep record (issue #78) — one tombstone per stage block. + * + * Sleep's write path never upserts a session by id: [com.pulseloop.service.EventPersistenceSubscriber] + * re-derives the whole waking day from the raw stage blocks the ring re-sends, so deleting a + * session row alone would let the next sync of that night rebuild it exactly. A block is + * minute-grid RLE over the record's stages, so the same record re-sent reproduces its blocks + * with the same `startAt` values — which is the stable identity, the same role the bucket + * startEpoch plays above. Keyed by the waking day as well, because the same wall-clock minute + * legitimately recurs on different days. + */ + suspend fun recordSleepBlocks(dayStart: Long, startAts: List) { + if (startAts.isEmpty()) return + insertAll(startAts.map { + MeasurementDeletionEntity( + measurementId = sleepBlockId(dayStart, it), + kindRaw = SLEEP_RECORD_KIND, + timestamp = it, + ) + }) + } + companion object { /** The prefix `EventPersistenceSubscriber.historyMeasurementId` builds its stable ids from. * A measurement whose id starts with this is one the ring can hand us again. */ @@ -735,7 +764,15 @@ interface MeasurementDeletionDao { * are not measurements — so it is deliberately a name no kind can collide with. */ const val ACTIVITY_KIND = "ACTIVITY_BUCKET" + /** Key prefix for a deleted ring sleep record's blocks (issue #78) — see [recordSleepBlocks]. */ + const val SLEEP_ID_PREFIX = "sleep:" + /** `kindRaw` for a sleep-record tombstone — a name no measurement kind can collide with. */ + const val SLEEP_RECORD_KIND = "SLEEP_RECORD_BLOCK" + /** The tombstone key for the bucket starting at [startEpoch]. */ fun activityBucketId(startEpoch: Long): String = "$ACTIVITY_ID_PREFIX$startEpoch" + + /** The tombstone key for the sleep stage block starting at [startAt] on waking day [dayStart]. */ + fun sleepBlockId(dayStart: Long, startAt: Long): String = "$SLEEP_ID_PREFIX$dayStart:$startAt" } } diff --git a/app/src/main/java/com/pulseloop/service/EventPersistenceSubscriber.kt b/app/src/main/java/com/pulseloop/service/EventPersistenceSubscriber.kt index 9aae681a..dd32c1bd 100644 --- a/app/src/main/java/com/pulseloop/service/EventPersistenceSubscriber.kt +++ b/app/src/main/java/com/pulseloop/service/EventPersistenceSubscriber.kt @@ -3,6 +3,7 @@ package com.pulseloop.service import android.content.Context import androidx.room.withTransaction import com.pulseloop.data.PulseLoopDatabase +import com.pulseloop.data.dao.MeasurementDeletionDao import com.pulseloop.data.entity.* import com.pulseloop.health.HealthConnectExportWorker import com.pulseloop.ring.* @@ -593,6 +594,14 @@ class EventPersistenceSubscriber( // (issue #63): see [completeSessionSurvivors] for why "the session it describes" is a // contiguous run of blocks and not every block of every row the packet touches. val replacements = buildStageBlocks("", ts, stages) + // Tombstone check (issue #78): a deleted ring record's blocks come back with the same + // `startAt` values on every re-send of that night, so the re-derive must drop them + // here or `SleepRecordDeletion`'s work lasts exactly one sync. + .filterNot { block -> + db.measurementDeletionDao().isDeleted( + MeasurementDeletionDao.sleepBlockId(dayStart, block.startAt), + ) + } val dayBlocks = replaceOverlappingSleepBlocks( existing = if (completeSession) { completeSessionSurvivors(existingBlocks, ts, packetEnd) @@ -767,16 +776,10 @@ class EventPersistenceSubscriber( * Medical research: optimal deep sleep = 15-25% of total. * Matches the official app's scoring. */ - private fun computeSleepScore(deepMin: Int, totalMin: Int): Int? { - if (totalMin == 0) return null - val deepPct = (deepMin.toFloat() / totalMin * 100).toInt() - return when { - deepPct >= 20 -> 90 - deepPct >= 15 -> 75 - deepPct >= 10 -> 60 - else -> 40 - } - } + private fun computeSleepScore(deepMin: Int, totalMin: Int): Int? = + // Delegates since issue #78: SleepRecordDeletion restates deleted-from sessions with the + // same banding, so one function owns it. + sleepStageScore(deepMin, totalMin) // ── Calorie estimation recompute (iOS #98) ─────────────────────────────────── diff --git a/app/src/main/java/com/pulseloop/service/SleepInsights.kt b/app/src/main/java/com/pulseloop/service/SleepInsights.kt index 799d4e7c..fd157dd4 100644 --- a/app/src/main/java/com/pulseloop/service/SleepInsights.kt +++ b/app/src/main/java/com/pulseloop/service/SleepInsights.kt @@ -141,6 +141,23 @@ fun asleepMinutes(blocks: List): Int = .sumOf { it.durationMinutes } .coerceAtLeast(0) +/** + * The banded stage score stored on a session row — deep-percentage bands, nothing else. The one + * source of truth for it: `EventPersistenceSubscriber` stamps it on every reconcile, and + * `SleepRecordDeletion` restates with the same function so a post-deletion row agrees with what + * the next sync would write (issue #78). + */ +fun sleepStageScore(deepMin: Int, totalMin: Int): Int? { + if (totalMin == 0) return null + val deepPct = (deepMin.toFloat() / totalMin * 100).toInt() + return when { + deepPct >= 20 -> 90 + deepPct >= 15 -> 75 + deepPct >= 10 -> 60 + else -> 40 + } +} + /** * How long the session covers on the clock, first stage block to last — which since issue #63 is a * different number from [SleepSessionEntity.totalMinutes], the time actually asleep. diff --git a/app/src/main/java/com/pulseloop/ui/screens/SleepScreen.kt b/app/src/main/java/com/pulseloop/ui/screens/SleepScreen.kt index 9eea1a3a..2e60421c 100644 --- a/app/src/main/java/com/pulseloop/ui/screens/SleepScreen.kt +++ b/app/src/main/java/com/pulseloop/ui/screens/SleepScreen.kt @@ -14,15 +14,19 @@ import androidx.compose.foundation.shape.RoundedCornerShape import androidx.compose.material.icons.Icons import androidx.compose.material.icons.filled.ChevronLeft import androidx.compose.material.icons.filled.ChevronRight +import androidx.compose.material.icons.filled.DeleteOutline +import androidx.compose.material3.AlertDialog import androidx.compose.material3.DatePicker import androidx.compose.material3.DatePickerDialog import androidx.compose.material3.ExperimentalMaterial3Api import androidx.compose.material3.Icon +import androidx.compose.material3.IconButton import androidx.compose.material3.SelectableDates import androidx.compose.material3.Text import androidx.compose.material3.TextButton import androidx.compose.material3.rememberDatePickerState import androidx.compose.runtime.* +import kotlinx.coroutines.launch import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.draw.clip @@ -124,7 +128,7 @@ private fun androidx.compose.foundation.lazy.LazyListScope.dayItems( item { SleepStageSummaryCards(deep = "—", light = "—", awake = "—") } } // Single session: render exactly as before, no carousel chrome. - sessions.size == 1 -> sessionPageItems(sessions[0], state.dayBlocks[sessions[0].id] ?: emptyList()) + sessions.size == 1 -> sessionPageItems(sessions[0], state.dayBlocks[sessions[0].id] ?: emptyList(), viewModel) // Multiple sessions (night + naps): horizontal paged carousel with dot indicators. else -> item { SleepCarousel(sessions, state.dayBlocks) } } @@ -146,6 +150,7 @@ private fun androidx.compose.foundation.lazy.LazyListScope.dayItems( private fun androidx.compose.foundation.lazy.LazyListScope.sessionPageItems( session: SleepSessionEntity, blocks: List, + viewModel: SleepViewModel? = null, ) { item { SessionHero(session, blocks) } item { @@ -159,7 +164,7 @@ private fun androidx.compose.foundation.lazy.LazyListScope.sessionPageItems( // takes its share of the list's 16 dp spacing, which put a 32 dp hole in every single-record // night. val runs = com.pulseloop.service.sleepRecordRuns(blocks) - if (runs.size > 1) item { SleepRecordsCard(runs) } + if (runs.size > 1) item { SleepRecordsCard(session, runs, viewModel) } item { val byStage = blocks.groupBy { it.stageRaw }.mapValues { (_, b) -> b.sumOf { it.durationMinutes } } SleepStageSummaryCards( @@ -978,10 +983,18 @@ private const val PILL_OFFSET_ABOVE_LANE_DP = 30f * and the vendor app keeps each as its own row. Shown underneath rather than instead. */ @Composable -private fun SleepRecordsCard(runs: List) { +private fun SleepRecordsCard( + session: SleepSessionEntity, + runs: List, + viewModel: SleepViewModel?, +) { + val scope = androidx.compose.runtime.rememberCoroutineScope() + // Two-step delete (the vitals/activity pattern): a row's trash icon arms the confirm dialog. + var pendingRun by remember { mutableStateOf(null) } VisualizationCard( eyebrow = "Records", - title = "The ring recorded this night in ${runs.size} parts", + title = if (runs.size == 1) "The ring recorded this night in 1 part" + else "The ring recorded this night in ${runs.size} parts", legend = false, ) { Column(verticalArrangement = Arrangement.spacedBy(10.dp)) { @@ -1005,6 +1018,17 @@ private fun SleepRecordsCard(runs: List) { color = PulseColors.textMuted, ) } + // The escape hatch (issue #78): the ring opened a record on a still wrist — + // say, the evening before the wearer got into bed — and the night runs an + // hour high. The wearer knows which run is wrong; the sensor cannot. + IconButton(onClick = { pendingRun = run }) { + Icon( + Icons.Filled.DeleteOutline, + contentDescription = "Delete this record", + tint = PulseColors.textMuted, + modifier = Modifier.size(18.dp), + ) + } } if (index < runs.lastIndex) { val gapMinutes = ((runs[index + 1].startAt - run.endAt) / 60_000L).toInt() @@ -1018,4 +1042,28 @@ private fun SleepRecordsCard(runs: List) { } } } + + pendingRun?.let { run -> + AlertDialog( + onDismissRequest = { pendingRun = null }, + title = { Text("Delete this record?") }, + text = { + Text( + "${SleepFormat.clockTime(run.startAt)} – ${SleepFormat.clockTime(run.endAt)} " + + "leaves tonight's totals and the hypnogram, and it won't come back on re-sync.", + ) + }, + confirmButton = { + TextButton(onClick = { + pendingRun = null + scope.launch { + viewModel?.deleteSleepRecord(session.id, run.startAt) + } + }) { Text("Delete", color = PulseColors.danger) } + }, + dismissButton = { + TextButton(onClick = { pendingRun = null }) { Text("Keep") } + }, + ) + } } diff --git a/app/src/main/java/com/pulseloop/ui/viewmodels/ViewModels.kt b/app/src/main/java/com/pulseloop/ui/viewmodels/ViewModels.kt index 16da9f47..edc7d080 100644 --- a/app/src/main/java/com/pulseloop/ui/viewmodels/ViewModels.kt +++ b/app/src/main/java/com/pulseloop/ui/viewmodels/ViewModels.kt @@ -381,6 +381,17 @@ class SleepViewModel(private val db: PulseLoopDatabase) : ViewModel() { /** Step to an older (older = true) or newer day, clamped to [0, maxDayOffset]. */ fun stepDay(older: Boolean) = jumpToOffset(_state.value.dayOffset + if (older) 1 else -1) + /** + * Delete one ring record (issue #78) and rebuild the day. Returns whether anything was + * removed, so the UI can only confirm on a real deletion. + */ + suspend fun deleteSleepRecord(sessionId: String, recordStartAt: Long): Boolean = + try { + val removed = com.pulseloop.data.SleepRecordDeletion.delete(db, sessionId, recordStartAt) + if (removed) rebuild(_state.value.range) + removed + } catch (_: Exception) { false } + /** Jump to the day at [dayMillis] (a local-midnight key), clamped to the valid range. */ fun jumpToDay(dayMillis: Long) { val today = TimeUtil.startOfTodayLocal() diff --git a/app/src/test/java/com/pulseloop/data/SleepRecordDeletionTest.kt b/app/src/test/java/com/pulseloop/data/SleepRecordDeletionTest.kt new file mode 100644 index 00000000..88c31680 --- /dev/null +++ b/app/src/test/java/com/pulseloop/data/SleepRecordDeletionTest.kt @@ -0,0 +1,99 @@ +package com.pulseloop.data + +import com.pulseloop.data.dao.MeasurementDeletionDao +import com.pulseloop.data.entity.MeasurementDeletionEntity +import kotlinx.coroutines.test.runTest +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * The tombstone rule behind deleting one ring sleep record (issue #78), tested against the DAO's + * own default implementation — Room only supplies the queries, and where the bug would be is the + * *identity*: the write path re-derives sessions from the blocks the ring re-sends, so a delete + * only sticks if the tombstone answers for exactly the blocks that re-send reproduces. + */ +class SleepRecordDeletionTest { + + private class FakeDeletionDao : MeasurementDeletionDao { + val rows = mutableMapOf() + override suspend fun isDeleted(id: String) = id in rows + override suspend fun isSpotDeleted(kind: String, from: Long, to: Long) = false + override suspend fun isActivityBucketDeleted(id: String) = id in rows + override suspend fun insertAll(rows: List) { + rows.forEach { this.rows[it.measurementId] = it } + } + } + + // The scenario from the issue: a record opened 23:08 that is really sofa time, tombstoned by + // `SleepRecordDeletion` under the session's waking day. Minutes on the same grid `buildStageBlocks` + // reproduces when the ring re-sends that record. + private val wakingDay = 1_760_000_000_000L - (1_760_000_000_000L % 86_400_000L) + private val deletedStarts = listOf( + wakingDay + 0L, // 23:08 run: RLE blocks at their minute-grid starts + wakingDay + 17 * 60_000L, + wakingDay + 41 * 60_000L, + ) + + private suspend fun tombstone(dao: FakeDeletionDao) { + dao.recordSleepBlocks(wakingDay, deletedStarts) + } + + /** The contract the whole feature hangs on: the re-sent record's blocks are all suppressed. */ + @Test + fun `a re-sent record's blocks are all tombstoned`() = runTest { + val dao = FakeDeletionDao() + tombstone(dao) + + deletedStarts.forEach { start -> + assertTrue( + "the re-derive must suppress the block at $start or the delete lasts one sync", + dao.isDeleted(MeasurementDeletionDao.sleepBlockId(wakingDay, start)), + ) + } + } + + /** Only the deleted record goes: its neighbours must survive the re-derive. */ + @Test + fun `a different record's blocks are not suppressed`() = runTest { + val dao = FakeDeletionDao() + tombstone(dao) + + val neighbourStarts = listOf( + wakingDay + 78 * 60_000L, + wakingDay + 96 * 60_000L, + ) + neighbourStarts.forEach { start -> + assertFalse( + "the neighbour record at $start must survive", + dao.isDeleted(MeasurementDeletionDao.sleepBlockId(wakingDay, start)), + ) + } + } + + /** The same wall-clock minute on another night is different data, not a resurrected block. */ + @Test + fun `the same minute-of-day on a different waking day is not suppressed`() = runTest { + val dao = FakeDeletionDao() + tombstone(dao) + + val nextNight = wakingDay + 86_400_000L + deletedStarts.forEach { start -> + assertFalse( + "day+minute keying must not leak across nights", + dao.isDeleted(MeasurementDeletionDao.sleepBlockId(nextNight, start)), + ) + } + } + + /** One tombstone per block start, replaced in place on a re-delete — the table must not grow. */ + @Test + fun `re-deleting the same record replaces its tombstones rather than growing the table`() = runTest { + val dao = FakeDeletionDao() + tombstone(dao) + tombstone(dao) + + assertEquals(deletedStarts.size, dao.rows.size) + } +} From b6c1da946bdc64580c97aa6e0e30dcf2e1df1e0d Mon Sep 17 00:00:00 2001 From: Khoa Truong Date: Mon, 21 Sep 2026 17:29:22 -0700 Subject: [PATCH 4/9] feat(sleep): opt-in quiet-hours gate on import (#79) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A still wrist reads as sleep, so an evening on the sofa can land as a phantom session and run the night an hour high (the same night #78's delete handles, caught before import instead of after). Opt-in, off by default, on the Wearable screen: sleep the ring opens outside a daily window is declined before any write. A time window rather than the reporter's first choice of Android quiet Modes: a Mode gate needs notification-policy access plus a recorded interruption-filter history to compare imports against, and the sofa case is mostly caught by a plain window. The Modes version stays open as a follow-up; the reporter offered to build it. Properties worth the defaults: the gate reads per record, so a settings change takes effect on the next packet; it never deletes (already-imported nights are untouched); and drop-on-import is one-way — what the gate skips while narrower stays skipped, which is why it is opt-in. Window boundaries are start-inclusive/end-exclusive, a start==end window covers the whole day, and the default 22:00→07:00 wraps midnight. --- .../service/EventPersistenceSubscriber.kt | 11 +++ .../com/pulseloop/settings/QuietHoursPrefs.kt | 68 +++++++++++++++ .../ui/screens/SettingsSubScreens.kt | 82 +++++++++++++++++++ .../pulseloop/settings/QuietHoursPrefsTest.kt | 59 +++++++++++++ 4 files changed, 220 insertions(+) create mode 100644 app/src/main/java/com/pulseloop/settings/QuietHoursPrefs.kt create mode 100644 app/src/test/java/com/pulseloop/settings/QuietHoursPrefsTest.kt diff --git a/app/src/main/java/com/pulseloop/service/EventPersistenceSubscriber.kt b/app/src/main/java/com/pulseloop/service/EventPersistenceSubscriber.kt index dd32c1bd..39c5ef61 100644 --- a/app/src/main/java/com/pulseloop/service/EventPersistenceSubscriber.kt +++ b/app/src/main/java/com/pulseloop/service/EventPersistenceSubscriber.kt @@ -6,6 +6,7 @@ import com.pulseloop.data.PulseLoopDatabase import com.pulseloop.data.dao.MeasurementDeletionDao import com.pulseloop.data.entity.* import com.pulseloop.health.HealthConnectExportWorker +import com.pulseloop.settings.QuietHoursPrefs import com.pulseloop.ring.* import kotlinx.coroutines.* @@ -569,6 +570,16 @@ class EventPersistenceSubscriber( private suspend fun upsertSleepSession(ts: Long, stages: List, completeSession: Boolean) { if (stages.isEmpty() || stages.size > MAX_SLEEP_TIMELINE_MINUTES) return + // Quiet-hours gate (issue #79), opt-in and off by default: a record the ring opens outside + // the window is declined here, before any write — the still-wrist/sofa case. Already- + // imported nights are untouched (this never deletes), and the gate is read per record so a + // settings change takes effect on the next packet, not the next launch. + val quiet = QuietHoursPrefs(context) + if (quiet.enabled && + !QuietHoursPrefs.covers(quiet.startMinutes, quiet.endMinutes, QuietHoursPrefs.minuteOfDay(ts)) + ) { + return + } db.withTransaction { upsertSleepSessionAtomic(ts, stages, completeSession) } } diff --git a/app/src/main/java/com/pulseloop/settings/QuietHoursPrefs.kt b/app/src/main/java/com/pulseloop/settings/QuietHoursPrefs.kt new file mode 100644 index 00000000..2a4e4ba3 --- /dev/null +++ b/app/src/main/java/com/pulseloop/settings/QuietHoursPrefs.kt @@ -0,0 +1,68 @@ +package com.pulseloop.settings + +import android.content.Context +import java.time.Instant +import java.time.ZoneId + +/** + * The opt-in quiet-hours gate behind issue #79: ignore sleep the ring opens outside a daily + * window. A still wrist reads as sleep, so an evening on the sofa lands as a phantom session and + * the night runs an hour high; the wearer knows their bedtime, the sensor guesses. + * + * Deliberately a time window rather than Android's quiet Modes (the reporter's first choice): + * a Mode gate needs notification-policy access plus a recorded interruption-filter history to + * compare imports against, and the sofa case is mostly caught by a plain window. The Modes + * version stays open as a follow-up — the reporter offered to build it. + * + * **The filter is drop-on-import, and that is not recoverable.** A record the gate declines is + * never written, so widening the window later cannot resurrect what was skipped while it was + * narrower — the same one-way property the deletion tombstones have, chosen for the same reason: + * the alternative (import everything, hide outside the window) makes the ring's own figure and + * the displayed one permanently disagree. Off by default for exactly this reason. + */ +class QuietHoursPrefs(context: Context) { + + private val prefs = context.getSharedPreferences(PREFS_NAME, Context.MODE_PRIVATE) + + /** Off by default: the ring's own behaviour is the right default for everyone else. */ + var enabled: Boolean + get() = prefs.getBoolean(KEY_ENABLED, false) + set(value) { prefs.edit().putBoolean(KEY_ENABLED, value).apply() } + + /** Window start, minutes of local day (default 22:00). */ + var startMinutes: Int + get() = prefs.getInt(KEY_START, DEFAULT_START) + set(value) { prefs.edit().putInt(KEY_START, value.coerceIn(0, 24 * 60 - 1)).apply() } + + /** Window end, minutes of local day (default 07:00). */ + var endMinutes: Int + get() = prefs.getInt(KEY_END, DEFAULT_END) + set(value) { prefs.edit().putInt(KEY_END, value.coerceIn(0, 24 * 60)).apply() } + + companion object { + private const val PREFS_NAME = "sleep_quiet_hours" + private const val KEY_ENABLED = "enabled" + private const val KEY_START = "startMinutes" + private const val KEY_END = "endMinutes" + const val DEFAULT_START = 22 * 60 + const val DEFAULT_END = 7 * 60 + + /** + * Does a record opened at [minuteOfDay] (local) fall inside the window [start]→[end]? + * Inclusive at the start, exclusive at the end — a 07:00 record belongs to the day. + * A window whose start equals its end covers the whole day, which disables the gate + * without a second flag. + */ + fun covers(start: Int, end: Int, minuteOfDay: Int): Boolean { + if (start == end) return true + return if (start < end) minuteOfDay in start until end + else minuteOfDay >= start || minuteOfDay < end + } + + /** Local minute-of-day of an epoch-millis instant. */ + fun minuteOfDay(ts: Long, zone: ZoneId = ZoneId.systemDefault()): Int { + val time = Instant.ofEpochMilli(ts).atZone(zone) + return time.hour * 60 + time.minute + } + } +} diff --git a/app/src/main/java/com/pulseloop/ui/screens/SettingsSubScreens.kt b/app/src/main/java/com/pulseloop/ui/screens/SettingsSubScreens.kt index ebb22175..5fc731cd 100644 --- a/app/src/main/java/com/pulseloop/ui/screens/SettingsSubScreens.kt +++ b/app/src/main/java/com/pulseloop/ui/screens/SettingsSubScreens.kt @@ -74,6 +74,7 @@ import com.pulseloop.service.VitalColorToken import com.pulseloop.service.VitalSample import com.pulseloop.service.ZoneSeverity import com.pulseloop.settings.ApiKeyStore +import com.pulseloop.settings.QuietHoursPrefs import com.pulseloop.settings.UnitSystem import com.pulseloop.ui.components.DeviceHeroStatus import com.pulseloop.ui.components.ZoneLineChart @@ -1608,6 +1609,83 @@ fun MeasurementSettingsScreen(coordinator: RingSyncCoordinator?, onBack: () -> U // MARK: - Wearable +/** + * The opt-in quiet-hours gate (issue #79): sleep the ring opens outside the window is ignored on + * import. The rationale lives on [QuietHoursPrefs]; this is just the controls — a switch and the + * two window ends, on the Wearable screen because it governs what a sync is allowed to store. + */ +@OptIn(ExperimentalMaterial3Api::class) +@Composable +private fun QuietHoursCard() { + val context = LocalContext.current + val prefs = remember { QuietHoursPrefs(context) } + var enabled by remember { mutableStateOf(prefs.enabled) } + var start by remember { mutableStateOf(prefs.startMinutes) } + var end by remember { mutableStateOf(prefs.endMinutes) } + // "start" | "end" while a picker is open. + var picking by remember { mutableStateOf(null) } + + Card(Modifier.fillMaxWidth()) { + Column(Modifier.padding(16.dp)) { + Row( + Modifier.fillMaxWidth(), + horizontalArrangement = Arrangement.SpaceBetween, + verticalAlignment = Alignment.CenterVertically, + ) { + Text("Sleep quiet hours", style = MaterialTheme.typography.titleMedium, fontWeight = FontWeight.SemiBold) + Switch(checked = enabled, onCheckedChange = { enabled = it; prefs.enabled = it }) + } + Text( + "When on, sleep the ring opens outside the window is ignored on import — a still " + + "wrist reads as sleep, so an evening on the sofa can log a phantom session and " + + "run the night high. Already-imported nights are never touched, and what the " + + "gate skips while it is narrower stays skipped.", + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + ) + if (enabled) { + Spacer(Modifier.height(12.dp)) + Row(Modifier.fillMaxWidth(), horizontalArrangement = Arrangement.spacedBy(12.dp)) { + OutlinedButton(onClick = { picking = "start" }, modifier = Modifier.weight(1f)) { + Text("From ${formatMinutesOfDay(start)}") + } + OutlinedButton(onClick = { picking = "end" }, modifier = Modifier.weight(1f)) { + Text("Until ${formatMinutesOfDay(end)}") + } + } + } + } + } + + picking?.let { which -> + val initial = if (which == "start") start else end + val pickerState = rememberTimePickerState( + initialHour = initial / 60, + initialMinute = initial % 60, + is24Hour = true, + ) + AlertDialog( + onDismissRequest = { picking = null }, + title = { Text(if (which == "start") "Quiet from" else "Quiet until") }, + text = { TimePicker(state = pickerState) }, + confirmButton = { + TextButton(onClick = { + val minutes = pickerState.hour * 60 + pickerState.minute + if (which == "start") { start = minutes; prefs.startMinutes = minutes } + else { end = minutes; prefs.endMinutes = minutes } + picking = null + }) { Text("Done") } + }, + dismissButton = { + TextButton(onClick = { picking = null }) { Text("Cancel") } + }, + ) + } +} + +private fun formatMinutesOfDay(minutes: Int): String = + "%02d:%02d".format(minutes / 60, minutes % 60) + /** * Wearable detail screen (iOS WearableSettingsView) — the hero card opens this. Connection * state, exact model name, firmware, sync/find/disconnect actions, plus the old inline "Ring" @@ -1735,6 +1813,10 @@ fun WearableSettingsScreen( BatteryHistorySection(db) } + if (device != null) { + QuietHoursCard() + } + if (device != null) { Card(Modifier.fillMaxWidth()) { Column(Modifier.padding(16.dp)) { diff --git a/app/src/test/java/com/pulseloop/settings/QuietHoursPrefsTest.kt b/app/src/test/java/com/pulseloop/settings/QuietHoursPrefsTest.kt new file mode 100644 index 00000000..de88b81a --- /dev/null +++ b/app/src/test/java/com/pulseloop/settings/QuietHoursPrefsTest.kt @@ -0,0 +1,59 @@ +package com.pulseloop.settings + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * The window arithmetic behind the quiet-hours gate (issue #79). The import gate is one call to + * [QuietHoursPrefs.covers] — the boundaries and the midnight wrap are where a mistake would drop + * real nights or keep phantom ones. + */ +class QuietHoursPrefsTest { + + /** Default 22:00 → 07:00 wraps midnight; a 23:08 record is the sofa case it exists for. */ + @Test + fun `the default window wraps midnight`() { + assertTrue(QuietHoursPrefs.covers(22 * 60, 7 * 60, 23 * 60 + 8)) + assertTrue(QuietHoursPrefs.covers(22 * 60, 7 * 60, 3 * 60)) + assertFalse(QuietHoursPrefs.covers(22 * 60, 7 * 60, 14 * 60)) + } + + /** Inclusive at the start, exclusive at the end — a 07:00 record belongs to the day. */ + @Test + fun `boundaries are start-inclusive and end-exclusive`() { + assertTrue(QuietHoursPrefs.covers(22 * 60, 7 * 60, 22 * 60)) + assertFalse(QuietHoursPrefs.covers(22 * 60, 7 * 60, 7 * 60)) + assertTrue(QuietHoursPrefs.covers(9 * 60, 18 * 60, 9 * 60)) + assertFalse(QuietHoursPrefs.covers(9 * 60, 18 * 60, 18 * 60)) + } + + /** A plain daytime window (no wrap) still works — shift workers nap in the day. */ + @Test + fun `a non-wrapping window behaves`() { + assertTrue(QuietHoursPrefs.covers(9 * 60, 18 * 60, 12 * 60)) + assertFalse(QuietHoursPrefs.covers(9 * 60, 18 * 60, 21 * 60)) + assertFalse(QuietHoursPrefs.covers(9 * 60, 18 * 60, 6 * 60)) + } + + /** start == end means the whole day — the gate is on but the window filters nothing. */ + @Test + fun `an equal window covers the whole day`() { + for (minute in listOf(0, 7 * 60, 12 * 60, 23 * 60 + 59)) { + assertTrue(QuietHoursPrefs.covers(8 * 60, 8 * 60, minute)) + } + } + + /** The gate reads wall-clock local time, not epoch minutes. */ + @Test + fun `minuteOfDay uses the local wall clock`() { + // 2026-09-21T23:08:00Z is a different local wall clock per zone; assert against the zone + // arithmetic itself rather than a fixed zone, so the test holds on any machine. + val ts = 1_789_000_000_000L + val zone = java.time.ZoneId.of("UTC") + val expected = java.time.Instant.ofEpochMilli(ts).atZone(zone).hour * 60 + + java.time.Instant.ofEpochMilli(ts).atZone(zone).minute + assertEquals(expected.toLong(), QuietHoursPrefs.minuteOfDay(ts, zone).toLong()) + } +} From 2f5d247441b23396cad0d9543827f3798ded90f2 Mon Sep 17 00:00:00 2001 From: Khoa Truong Date: Mon, 21 Sep 2026 19:54:44 -0700 Subject: [PATCH 5/9] fix(sleep): Awake minutes include the between-record gaps (#81) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The ring reports a brief night-waking two ways — an AWAKE stage, or closing the record and reopening a new one. The second form appears on the RECORDS card as "Awake Xm between" but contributed nothing to the Awake card, so one night read 4m awake on one card and 0m on the other (five wakings, four reopened, one staged). awakeMinutes() counts staged blocks plus inter-record gaps at the same 2-minute real-boundary threshold sleepRecordRuns draws — the one-minute seam (issue #63) stays excluded, or every unsplit night grows a spurious minute. The Day-view Awake card (single session + carousel) and the aggregate's per-night average both use it. --- .../com/pulseloop/service/SleepInsights.kt | 27 ++++++ .../com/pulseloop/ui/screens/SleepScreen.kt | 6 +- .../com/pulseloop/ui/viewmodels/ViewModels.kt | 2 +- .../com/pulseloop/service/AwakeMinutesTest.kt | 92 +++++++++++++++++++ 4 files changed, 124 insertions(+), 3 deletions(-) create mode 100644 app/src/test/java/com/pulseloop/service/AwakeMinutesTest.kt diff --git a/app/src/main/java/com/pulseloop/service/SleepInsights.kt b/app/src/main/java/com/pulseloop/service/SleepInsights.kt index fd157dd4..84975695 100644 --- a/app/src/main/java/com/pulseloop/service/SleepInsights.kt +++ b/app/src/main/java/com/pulseloop/service/SleepInsights.kt @@ -141,6 +141,33 @@ fun asleepMinutes(blocks: List): Int = .sumOf { it.durationMinutes } .coerceAtLeast(0) +/** + * Awake minutes for a session, including the short gaps between its ring records (issue #81). + * + * The ring reports a brief night-waking two ways: as an explicit `AWAKE` stage block, and — when it + * catches the waking by closing the record and opening a new one rather than staging it — as the + * gap between two records of the same session. The second form is real not-asleep minutes the + * RECORDS card already displays as "Awake Xm between", so the Awake figure counts it too; otherwise + * the same night can read 4m awake on one card and 0m on the other (five wakings, four caught by + * reopening, one staged). + * + * The one-minute seam stays excluded: this ring closes one record and opens the next a single + * minute later (issue #63), which is the same width as the minute-grid rounding seam *within* one + * record — counting those grows a spurious minute on every unsplit night. Two minutes is the same + * real-boundary threshold [sleepRecordRuns] draws its line at. + */ +fun awakeMinutes(blocks: List): Int { + val staged = blocks + .filter { it.stageRaw == SleepStage.AWAKE.name } + .sumOf { it.durationMinutes } + val runs = sleepRecordRuns(blocks) + val between = (0 until runs.lastIndex).sumOf { i -> + val gapMinutes = ((runs[i + 1].startAt - runs[i].endAt) / 60_000L).toInt() + if (gapMinutes >= 2) gapMinutes else 0 + } + return staged + between +} + /** * The banded stage score stored on a session row — deep-percentage bands, nothing else. The one * source of truth for it: `EventPersistenceSubscriber` stamps it on every reconcile, and diff --git a/app/src/main/java/com/pulseloop/ui/screens/SleepScreen.kt b/app/src/main/java/com/pulseloop/ui/screens/SleepScreen.kt index 2e60421c..f711287e 100644 --- a/app/src/main/java/com/pulseloop/ui/screens/SleepScreen.kt +++ b/app/src/main/java/com/pulseloop/ui/screens/SleepScreen.kt @@ -170,7 +170,9 @@ private fun androidx.compose.foundation.lazy.LazyListScope.sessionPageItems( SleepStageSummaryCards( deep = SleepFormat.duration(byStage["DEEP"] ?: 0), light = SleepFormat.duration(byStage["LIGHT"] ?: 0), - awake = SleepFormat.duration(byStage["AWAKE"] ?: 0), + // Awake counts the between-record gaps too (issue #81) — the same minutes this page's + // RECORDS card labels "Awake Xm between". + awake = SleepFormat.duration(com.pulseloop.service.awakeMinutes(blocks)), ) } } @@ -222,7 +224,7 @@ private fun SleepCarousel( SleepStageSummaryCards( deep = SleepFormat.duration(byStage["DEEP"] ?: 0), light = SleepFormat.duration(byStage["LIGHT"] ?: 0), - awake = SleepFormat.duration(byStage["AWAKE"] ?: 0), + awake = SleepFormat.duration(com.pulseloop.service.awakeMinutes(blocks)), ) } } diff --git a/app/src/main/java/com/pulseloop/ui/viewmodels/ViewModels.kt b/app/src/main/java/com/pulseloop/ui/viewmodels/ViewModels.kt index edc7d080..cea07ac8 100644 --- a/app/src/main/java/com/pulseloop/ui/viewmodels/ViewModels.kt +++ b/app/src/main/java/com/pulseloop/ui/viewmodels/ViewModels.kt @@ -323,7 +323,7 @@ class SleepViewModel(private val db: PulseLoopDatabase) : ViewModel() { val stageAvg = if (valid.isEmpty()) null else Triple( valid.sumOf { s -> lookup(s.id).filter { it.stageRaw == "DEEP" }.sumOf { b -> b.durationMinutes } } / valid.size, valid.sumOf { s -> lookup(s.id).filter { it.stageRaw == "LIGHT" }.sumOf { b -> b.durationMinutes } } / valid.size, - valid.sumOf { s -> lookup(s.id).filter { it.stageRaw == "AWAKE" }.sumOf { b -> b.durationMinutes } } / valid.size, + valid.sumOf { s -> com.pulseloop.service.awakeMinutes(lookup(s.id)) } / valid.size, ) val bars = when (range) { SleepRangeKey.YEAR -> SleepInsights.buildMonthBuckets(anchor, collapsedSessions, lookup) diff --git a/app/src/test/java/com/pulseloop/service/AwakeMinutesTest.kt b/app/src/test/java/com/pulseloop/service/AwakeMinutesTest.kt new file mode 100644 index 00000000..3bc38956 --- /dev/null +++ b/app/src/test/java/com/pulseloop/service/AwakeMinutesTest.kt @@ -0,0 +1,92 @@ +package com.pulseloop.service + +import com.pulseloop.data.entity.SleepStageBlockEntity +import com.pulseloop.ring.SleepStage +import org.junit.Assert.assertEquals +import org.junit.Test + +/** + * The Awake figure that includes the between-record gaps (issue #81). + * + * The ring catches a brief waking either by staging it (`AWAKE` block) or by closing the record and + * opening a new one — the second form exists on screen only as the RECORDS card's "Awake Xm + * between", so the Awake stat must count it or the two displays disagree for the same night. The + * one-minute seam must stay out: it is the rounding artifact every unsplit night carries, not a + * waking. + */ +class AwakeMinutesTest { + + private val base = 1_725_408_720_000L + + private fun block( + startMinute: Int, + minutes: Int, + stage: SleepStage = SleepStage.LIGHT, + recordStartMinute: Int? = null, + ) = SleepStageBlockEntity( + id = "b$startMinute", + sessionId = "s", + startAt = base + startMinute * 60_000L, + startMinute = startMinute, + durationMinutes = minutes, + stageRaw = stage.name, + recordStartAt = recordStartMinute?.let { base + it * 60_000L } ?: 0L, + ) + + @Test + fun `an unsplit night with staged awake counts the stages and nothing else`() { + val blocks = listOf( + block(0, 60, SleepStage.LIGHT), + block(60, 5, SleepStage.AWAKE), + block(65, 120, SleepStage.DEEP), + ) + + assertEquals(5, awakeMinutes(blocks)) + } + + @Test + fun `a one-minute record seam is not awake time`() { + // Same record re-opened a minute later (the #63 firmware behavior): two runs, 1 min gap. + val blocks = listOf( + block(0, 60, SleepStage.LIGHT, recordStartMinute = 0), + block(61, 60, SleepStage.LIGHT, recordStartMinute = 61), + ) + + assertEquals(0, awakeMinutes(blocks)) + } + + @Test + fun `a real between-record waking counts as awake`() { + // The sofa-adjacent case: the ring closes the record when the wearer gets up and reopens + // when they settle — the 4 minutes in between exist as a gap, not a stage. + val blocks = listOf( + block(0, 120, SleepStage.LIGHT, recordStartMinute = 0), + block(124, 240, SleepStage.DEEP, recordStartMinute = 124), + ) + + assertEquals(4, awakeMinutes(blocks)) + } + + /** The night from the issue: five wakings, four caught by reopening, one staged. */ + @Test + fun `the reported night reconciles — gaps and the one staged waking both count`() { + val blocks = listOf( + // Four reopened wakings of 2-3 minutes between five records... + block(0, 30, SleepStage.LIGHT, recordStartMinute = 0), + block(33, 25, SleepStage.LIGHT, recordStartMinute = 33), // gap 3 + block(60, 40, SleepStage.LIGHT, recordStartMinute = 60), // gap 2 + block(102, 35, SleepStage.LIGHT, recordStartMinute = 102), // gap 2 + block(140, 20, SleepStage.LIGHT, recordStartMinute = 140), // gap 3 + // ...and the one waking the ring actually staged. + block(160, 2, SleepStage.AWAKE, recordStartMinute = 140), + block(162, 180, SleepStage.DEEP, recordStartMinute = 140), + ) + + assertEquals(2 + (3 + 2 + 2 + 3), awakeMinutes(blocks)) + } + + @Test + fun `no blocks means no awake minutes`() { + assertEquals(0, awakeMinutes(emptyList())) + } +} From 65fc87584198485f0f913a7b3a590452bd547481 Mon Sep 17 00:00:00 2001 From: Khoa Truong Date: Thu, 24 Sep 2026 18:33:19 -0700 Subject: [PATCH 6/9] fix(sleep): a deleted middle record no longer reads as hours awake (#78, #81) Deleting a middle record restated the survivors as one session, so the hole it left was counted by awakeMinutes() as a between-record waking: deleting 00:25-03:19 from the reported night showed ~3h20m Awake. The next sync would also have disagreed, since reconcileWakingDay splits at the 60-minute session gap. SleepRecordDeletion now re-segments the survivors the same way; the best-overlapping segment keeps the row id, others take the write path's sleep-- id so a re-sync matches rather than twins them. The Awake card and the RECORDS card's "Awake Xm between" line now read one rule (betweenRecordAwakeMinutes), so the one-minute seam no longer shows as "Awake 1m between" while counting 0. Multi-session days (night + nap) had no RECORDS card and so no delete; a phantom session the merge didn't join to the night couldn't be removed. The carousel pages now show it, even for a one-record session. A failed delete shows a toast instead of silently doing nothing. --- .../com/pulseloop/data/SleepRecordDeletion.kt | 93 ++++++++++++++----- .../com/pulseloop/service/SleepInsights.kt | 16 +++- .../com/pulseloop/ui/screens/SleepScreen.kt | 27 +++++- .../pulseloop/data/SleepRecordDeletionTest.kt | 56 +++++++++++ 4 files changed, 161 insertions(+), 31 deletions(-) diff --git a/app/src/main/java/com/pulseloop/data/SleepRecordDeletion.kt b/app/src/main/java/com/pulseloop/data/SleepRecordDeletion.kt index 43ccd70c..7926df81 100644 --- a/app/src/main/java/com/pulseloop/data/SleepRecordDeletion.kt +++ b/app/src/main/java/com/pulseloop/data/SleepRecordDeletion.kt @@ -81,42 +81,87 @@ object SleepRecordDeletion { * Rewrite [session] from the blocks it still has — bounds, asleep minutes and score, the same * formulas `reconcileWakingDay` applies (blocks re-keyed to the new bounds the same way). Empty * means gone: the row is deleted and the day reads as unslept. + * + * The survivors are re-segmented by [com.pulseloop.service.SleepSegmentation] first, exactly as + * the next sync would do it. Deleting a *middle* record can leave an hour-plus hole — which the + * write path reads as two sessions, not one night with a long waking. Restating it as a single + * row would both disagree with the next sync and let `awakeMinutes` count the hole as a + * between-record waking (issue #81), turning a deleted 00:25–03:19 into three hours awake. */ private suspend fun restateSession( db: PulseLoopDatabase, session: SleepSessionEntity, remaining: List, ) { - if (remaining.isEmpty()) { + val plan = planRestate(session, remaining) + if (plan.isEmpty()) { db.sleepSessionDao().deleteById(session.id) return } - val ordered = remaining.sortedBy { it.startAt } - val startAt = ordered.first().startAt - val endAt = ordered.maxOf { it.startAt + it.durationMinutes * 60_000L } - val totalMin = com.pulseloop.service.asleepMinutes(ordered) - val deepMin = ordered.filter { it.stageRaw == SleepStage.DEEP.name } - .sumOf { it.durationMinutes } - val now = System.currentTimeMillis() - db.sleepSessionDao().upsert( - session.copy( - startAt = startAt, - endAt = endAt, - totalMinutes = totalMin, - score = com.pulseloop.service.sleepStageScore(deepMin, totalMin), updatedAt = now, - ), - ) - // Re-key the surviving blocks to the restated bounds: `startMinute` is relative to the - // session start, so it moves with the new first block. db.sleepStageBlockDao().deleteBySession(session.id) - ordered.forEach { - db.sleepStageBlockDao().insert( - it.copy( - id = java.util.UUID.randomUUID().toString(), - sessionId = session.id, - startMinute = ((it.startAt - startAt) / 60_000L).toInt().coerceAtLeast(0), + val now = System.currentTimeMillis() + for (row in plan) { + val totalMin = com.pulseloop.service.asleepMinutes(row.blocks) + val deepMin = row.blocks.filter { it.stageRaw == SleepStage.DEEP.name } + .sumOf { it.durationMinutes } + // Parent session BEFORE its blocks (FK sleep_stage_blocks -> sleep_sessions.id). + db.sleepSessionDao().upsert( + session.copy( + id = row.id, + startAt = row.startAt, + endAt = row.endAt, + totalMinutes = totalMin, + score = com.pulseloop.service.sleepStageScore(deepMin, totalMin), + updatedAt = now, ), ) + // Re-key the surviving blocks to the restated bounds: `startMinute` is relative to the + // session start, so it moves with the new first block. + row.blocks.forEach { + db.sleepStageBlockDao().insert( + it.copy( + id = java.util.UUID.randomUUID().toString(), + sessionId = row.id, + startMinute = ((it.startAt - row.startAt) / 60_000L).toInt().coerceAtLeast(0), + ), + ) + } + } + } + + /** One session row the restate writes: its id, bounds and blocks (sorted). */ + internal data class RestatedRow( + val id: String, + val startAt: Long, + val endAt: Long, + val blocks: List, + ) + + /** + * Split [remaining] the way `reconcileWakingDay` would and assign row ids: the segment that + * overlaps [session]'s old bounds most keeps its id (so the Day view stays on the same row); + * any other segment gets the id the write path mints for a new segment, `sleep--`, + * so the next sync matches it rather than inserting a twin. Empty in → empty out. + */ + internal fun planRestate( + session: SleepSessionEntity, + remaining: List, + ): List { + val segments = com.pulseloop.service.SleepSegmentation.segment(remaining).map { g -> + val sorted = g.sortedBy { it.startAt } + RestatedRow( + id = "", + startAt = sorted.first().startAt, + endAt = sorted.maxOf { it.startAt + it.durationMinutes * 60_000L }, + blocks = sorted, + ) + } + if (segments.isEmpty()) return emptyList() + fun overlap(r: RestatedRow) = + maxOf(0L, minOf(r.endAt, session.endAt) - maxOf(r.startAt, session.startAt)) + val keeper = segments.maxBy { overlap(it) } + return segments.map { + it.copy(id = if (it === keeper) session.id else "sleep-${session.date}-${it.startAt}") } } } diff --git a/app/src/main/java/com/pulseloop/service/SleepInsights.kt b/app/src/main/java/com/pulseloop/service/SleepInsights.kt index 84975695..00759931 100644 --- a/app/src/main/java/com/pulseloop/service/SleepInsights.kt +++ b/app/src/main/java/com/pulseloop/service/SleepInsights.kt @@ -162,12 +162,24 @@ fun awakeMinutes(blocks: List): Int { .sumOf { it.durationMinutes } val runs = sleepRecordRuns(blocks) val between = (0 until runs.lastIndex).sumOf { i -> - val gapMinutes = ((runs[i + 1].startAt - runs[i].endAt) / 60_000L).toInt() - if (gapMinutes >= 2) gapMinutes else 0 + betweenRecordAwakeMinutes(runs[i], runs[i + 1]) } return staged + between } +/** + * The waking between two consecutive ring records of one session, in minutes — 0 for the one-minute + * seam (issue #63). The one rule both the Awake figure ([awakeMinutes]) and the RECORDS card's + * "Awake Xm between" line read, so the two cannot disagree about the same gap (issue #81). + * + * No upper cap is needed: a session never holds a gap of [SleepSegmentation.SESSION_GAP_MINUTES] + * or more — the write path and `SleepRecordDeletion` both split there. + */ +fun betweenRecordAwakeMinutes(earlier: SleepRecordRun, later: SleepRecordRun): Int { + val gapMinutes = ((later.startAt - earlier.endAt) / 60_000L).toInt() + return if (gapMinutes >= 2) gapMinutes else 0 +} + /** * The banded stage score stored on a session row — deep-percentage bands, nothing else. The one * source of truth for it: `EventPersistenceSubscriber` stamps it on every reconcile, and diff --git a/app/src/main/java/com/pulseloop/ui/screens/SleepScreen.kt b/app/src/main/java/com/pulseloop/ui/screens/SleepScreen.kt index f711287e..31fcc87d 100644 --- a/app/src/main/java/com/pulseloop/ui/screens/SleepScreen.kt +++ b/app/src/main/java/com/pulseloop/ui/screens/SleepScreen.kt @@ -130,7 +130,7 @@ private fun androidx.compose.foundation.lazy.LazyListScope.dayItems( // Single session: render exactly as before, no carousel chrome. sessions.size == 1 -> sessionPageItems(sessions[0], state.dayBlocks[sessions[0].id] ?: emptyList(), viewModel) // Multiple sessions (night + naps): horizontal paged carousel with dot indicators. - else -> item { SleepCarousel(sessions, state.dayBlocks) } + else -> item { SleepCarousel(sessions, state.dayBlocks, viewModel) } } item { @@ -195,6 +195,7 @@ private fun SessionHero(session: SleepSessionEntity, blocks: List, blocksBySession: Map>, + viewModel: SleepViewModel? = null, ) { // Reset to the first page whenever the day's session set changes (a different day / fewer // pages): keying the composable recreates the pager state. @@ -220,6 +221,11 @@ private fun SleepCarousel( VisualizationCard(eyebrow = "Stages", title = "Sleep architecture", legend = true) { SleepHypnogram(blocks = blocks, spanMin = s.spanMinutes, startTs = s.startAt) } + // Unlike the single-session page, shown even for a one-record session: on a + // multi-session day a phantom (the sofa evening the merge did not join to the + // night) is its own page, and this card is the only place to delete it (#78). + val runs = com.pulseloop.service.sleepRecordRuns(blocks) + if (runs.isNotEmpty()) SleepRecordsCard(s, runs, viewModel) val byStage = blocks.groupBy { it.stageRaw }.mapValues { (_, b) -> b.sumOf { it.durationMinutes } } SleepStageSummaryCards( deep = SleepFormat.duration(byStage["DEEP"] ?: 0), @@ -991,11 +997,13 @@ private fun SleepRecordsCard( viewModel: SleepViewModel?, ) { val scope = androidx.compose.runtime.rememberCoroutineScope() + val context = androidx.compose.ui.platform.LocalContext.current // Two-step delete (the vitals/activity pattern): a row's trash icon arms the confirm dialog. var pendingRun by remember { mutableStateOf(null) } VisualizationCard( eyebrow = "Records", - title = if (runs.size == 1) "The ring recorded this night in 1 part" + // One run only happens on a carousel page, which may be a nap rather than a night. + title = if (runs.size == 1) "The ring recorded this as one record" else "The ring recorded this night in ${runs.size} parts", legend = false, ) { @@ -1032,8 +1040,12 @@ private fun SleepRecordsCard( ) } } - if (index < runs.lastIndex) { - val gapMinutes = ((runs[index + 1].startAt - run.endAt) / 60_000L).toInt() + // Only a gap the Awake card also counts (issue #81): the one-minute seam between + // two records is not a waking, so it gets no "Awake" line here either. + val gapMinutes = if (index < runs.lastIndex) { + com.pulseloop.service.betweenRecordAwakeMinutes(run, runs[index + 1]) + } else 0 + if (gapMinutes > 0) { Text( "Awake ${SleepFormat.duration(gapMinutes)} between", fontSize = 12.sp, @@ -1059,7 +1071,12 @@ private fun SleepRecordsCard( TextButton(onClick = { pendingRun = null scope.launch { - viewModel?.deleteSleepRecord(session.id, run.startAt) + val removed = viewModel?.deleteSleepRecord(session.id, run.startAt) ?: false + if (!removed) { + android.widget.Toast.makeText( + context, "Couldn't delete this record", android.widget.Toast.LENGTH_SHORT, + ).show() + } } }) { Text("Delete", color = PulseColors.danger) } }, diff --git a/app/src/test/java/com/pulseloop/data/SleepRecordDeletionTest.kt b/app/src/test/java/com/pulseloop/data/SleepRecordDeletionTest.kt index 88c31680..997e9dd9 100644 --- a/app/src/test/java/com/pulseloop/data/SleepRecordDeletionTest.kt +++ b/app/src/test/java/com/pulseloop/data/SleepRecordDeletionTest.kt @@ -96,4 +96,60 @@ class SleepRecordDeletionTest { assertEquals(deletedStarts.size, dao.rows.size) } + + // ── Restating the survivors ([SleepRecordDeletion.planRestate]) ───────── + + private fun block(startMinute: Int, minutes: Int, recordStartMinute: Int) = + com.pulseloop.data.entity.SleepStageBlockEntity( + id = "b$startMinute", + sessionId = "night", + startAt = wakingDay + startMinute * 60_000L, + startMinute = startMinute, + durationMinutes = minutes, + stageRaw = com.pulseloop.ring.SleepStage.LIGHT.name, + recordStartAt = wakingDay + recordStartMinute * 60_000L, + ) + + // The reported night, minutes from 23:08: 23:08–00:05, 00:25–03:19, 03:26–07:08. + private val first = block(0, 57, 0) + private val middle = block(77, 174, 77) + private val last = block(258, 222, 258) + private val session = com.pulseloop.data.entity.SleepSessionEntity( + id = "night", date = wakingDay, + startAt = first.startAt, endAt = last.startAt + last.durationMinutes * 60_000L, + totalMinutes = 453, + ) + + /** The reported case: drop the sofa record, the rest stays one session on the same row. */ + @Test + fun `deleting the first record restates one row under the same id`() { + val plan = SleepRecordDeletion.planRestate(session, listOf(middle, last)) + assertEquals(1, plan.size) + assertEquals("night", plan[0].id) + assertEquals(middle.startAt, plan[0].startAt) + } + + /** + * Deleting the middle record leaves a 3 h 21 m hole. The next sync re-segments at the 60-minute + * session gap, so the restate must too — kept as one row, `awakeMinutes` would have counted the + * whole hole as a between-record waking (#81). + */ + @Test + fun `deleting a middle record past the session gap splits the night like a re-sync would`() { + val plan = SleepRecordDeletion.planRestate(session, listOf(first, last)) + assertEquals(2, plan.size) + // The longer survivor overlaps the old bounds most and keeps the row id... + assertEquals("night", plan[1].id) + assertEquals(listOf(last.startAt), plan[1].blocks.map { it.startAt }) + // ...the other takes the id the write path mints, so a re-sync matches rather than twins it. + assertEquals("sleep-$wakingDay-${first.startAt}", plan[0].id) + plan.forEach { row -> + assertEquals(0, com.pulseloop.service.awakeMinutes(row.blocks)) + } + } + + @Test + fun `deleting every record restates nothing`() { + assertTrue(SleepRecordDeletion.planRestate(session, emptyList()).isEmpty()) + } } From a1ae0e348e46d39290b755fc17386268a4408aa2 Mon Sep 17 00:00:00 2001 From: Khoa Truong Date: Thu, 24 Sep 2026 18:33:33 -0700 Subject: [PATCH 7/9] fix(sleep): quiet-hours gate trims records to the window instead of dropping them (#79) The gate accepted or declined a whole record by its start minute. The ring often opens one record on the sofa and runs it straight into the real night, so the reported 23:08 start either kept the sofa hour (it is inside the default 22:00-07:00) or, with a later start set, threw the whole night away - permanently, since the gate is drop-on-import. keptMinutes() now keeps the longest contiguous stretch of the record inside the window, and the record is imported from there. The settings copy tells the wearer to set the start to their real bedtime and says naps outside the window are skipped. --- .../service/EventPersistenceSubscriber.kt | 18 ++++--- .../com/pulseloop/settings/QuietHoursPrefs.kt | 39 +++++++++++++- .../ui/screens/SettingsSubScreens.kt | 9 ++-- .../pulseloop/settings/QuietHoursPrefsTest.kt | 51 ++++++++++++++++++- 4 files changed, 104 insertions(+), 13 deletions(-) diff --git a/app/src/main/java/com/pulseloop/service/EventPersistenceSubscriber.kt b/app/src/main/java/com/pulseloop/service/EventPersistenceSubscriber.kt index 39c5ef61..b3c4a36b 100644 --- a/app/src/main/java/com/pulseloop/service/EventPersistenceSubscriber.kt +++ b/app/src/main/java/com/pulseloop/service/EventPersistenceSubscriber.kt @@ -570,14 +570,18 @@ class EventPersistenceSubscriber( private suspend fun upsertSleepSession(ts: Long, stages: List, completeSession: Boolean) { if (stages.isEmpty() || stages.size > MAX_SLEEP_TIMELINE_MINUTES) return - // Quiet-hours gate (issue #79), opt-in and off by default: a record the ring opens outside - // the window is declined here, before any write — the still-wrist/sofa case. Already- - // imported nights are untouched (this never deletes), and the gate is read per record so a - // settings change takes effect on the next packet, not the next launch. + // Quiet-hours gate (issue #79), opt-in and off by default: the record is trimmed to the + // minutes inside the window before any write — the still-wrist/sofa case — rather than + // judged by its start, because the ring often runs the sofa hour and the real night as one + // record. Already-imported nights are untouched (this never deletes), and the gate is read + // per record so a settings change takes effect on the next packet, not the next launch. val quiet = QuietHoursPrefs(context) - if (quiet.enabled && - !QuietHoursPrefs.covers(quiet.startMinutes, quiet.endMinutes, QuietHoursPrefs.minuteOfDay(ts)) - ) { + if (quiet.enabled) { + val kept = QuietHoursPrefs.keptMinutes(ts, stages.size, quiet.startMinutes, quiet.endMinutes) + ?: return + val keptTs = ts + kept.first * 60_000L + val keptStages = stages.subList(kept.first, kept.last + 1) + db.withTransaction { upsertSleepSessionAtomic(keptTs, keptStages, completeSession) } return } db.withTransaction { upsertSleepSessionAtomic(ts, stages, completeSession) } diff --git a/app/src/main/java/com/pulseloop/settings/QuietHoursPrefs.kt b/app/src/main/java/com/pulseloop/settings/QuietHoursPrefs.kt index 2a4e4ba3..b027f319 100644 --- a/app/src/main/java/com/pulseloop/settings/QuietHoursPrefs.kt +++ b/app/src/main/java/com/pulseloop/settings/QuietHoursPrefs.kt @@ -14,7 +14,12 @@ import java.time.ZoneId * compare imports against, and the sofa case is mostly caught by a plain window. The Modes * version stays open as a follow-up — the reporter offered to build it. * - * **The filter is drop-on-import, and that is not recoverable.** A record the gate declines is + * **Minute-level, not record-level.** A record is trimmed to the minutes that fall inside the + * window ([keptMinutes]) rather than accepted or declined by its start minute. The ring often + * opens one record on the sofa and runs it straight on into the real night; judging that record + * by its 23:08 start would either keep the sofa hour or throw the whole night away. + * + * **The filter is drop-on-import, and that is not recoverable.** Minutes the gate trims are * never written, so widening the window later cannot resurrect what was skipped while it was * narrower — the same one-way property the deletion tombstones have, chosen for the same reason: * the alternative (import everything, hide outside the window) makes the ring's own figure and @@ -59,6 +64,38 @@ class QuietHoursPrefs(context: Context) { else minuteOfDay >= start || minuteOfDay < end } + /** + * Which minutes of a ring record opened at [ts] with [minutes] stage minutes to keep: the + * longest contiguous stretch whose minutes fall inside [start]→[end] (see [covers]), as + * indices into the record's stages. Null when no minute is inside — the record is dropped. + * + * Longest-stretch rather than every in-window minute because the window is one daily + * interval: a record longer than the gap between two windows (a nap run through to the + * evening) could touch two of them, and keeping both would bridge the hole with a record + * that was never contiguous. + */ + fun keptMinutes( + ts: Long, + minutes: Int, + start: Int, + end: Int, + zone: ZoneId = ZoneId.systemDefault(), + ): IntRange? { + if (minutes <= 0) return null + var best: IntRange? = null + var runStart = -1 + for (i in 0..minutes) { + val inside = i < minutes && covers(start, end, minuteOfDay(ts + i * 60_000L, zone)) + if (inside && runStart < 0) runStart = i + if (!inside && runStart >= 0) { + val run = runStart until i + if (best == null || run.count() > best.count()) best = run + runStart = -1 + } + } + return best + } + /** Local minute-of-day of an epoch-millis instant. */ fun minuteOfDay(ts: Long, zone: ZoneId = ZoneId.systemDefault()): Int { val time = Instant.ofEpochMilli(ts).atZone(zone) diff --git a/app/src/main/java/com/pulseloop/ui/screens/SettingsSubScreens.kt b/app/src/main/java/com/pulseloop/ui/screens/SettingsSubScreens.kt index 5fc731cd..f7be4f6d 100644 --- a/app/src/main/java/com/pulseloop/ui/screens/SettingsSubScreens.kt +++ b/app/src/main/java/com/pulseloop/ui/screens/SettingsSubScreens.kt @@ -1636,10 +1636,11 @@ private fun QuietHoursCard() { Switch(checked = enabled, onCheckedChange = { enabled = it; prefs.enabled = it }) } Text( - "When on, sleep the ring opens outside the window is ignored on import — a still " + - "wrist reads as sleep, so an evening on the sofa can log a phantom session and " + - "run the night high. Already-imported nights are never touched, and what the " + - "gate skips while it is narrower stays skipped.", + "When on, sleep the ring records outside the window is trimmed off on import — a " + + "still wrist reads as sleep, so an evening on the sofa can log a phantom session " + + "and run the night high. Set the start to when you actually go to bed. Naps " + + "outside the window are skipped too. Already-imported nights are never touched, " + + "and what the gate skips while it is narrower stays skipped.", style = MaterialTheme.typography.bodySmall, color = MaterialTheme.colorScheme.onSurfaceVariant, ) diff --git a/app/src/test/java/com/pulseloop/settings/QuietHoursPrefsTest.kt b/app/src/test/java/com/pulseloop/settings/QuietHoursPrefsTest.kt index de88b81a..1b2ce8ab 100644 --- a/app/src/test/java/com/pulseloop/settings/QuietHoursPrefsTest.kt +++ b/app/src/test/java/com/pulseloop/settings/QuietHoursPrefsTest.kt @@ -12,7 +12,8 @@ import org.junit.Test */ class QuietHoursPrefsTest { - /** Default 22:00 → 07:00 wraps midnight; a 23:08 record is the sofa case it exists for. */ + /** Default 22:00 → 07:00 wraps midnight. Note it *keeps* 23:08 — the reported sofa start — + * which is why the settings copy tells the wearer to set the start to their real bedtime. */ @Test fun `the default window wraps midnight`() { assertTrue(QuietHoursPrefs.covers(22 * 60, 7 * 60, 23 * 60 + 8)) @@ -56,4 +57,52 @@ class QuietHoursPrefsTest { java.time.Instant.ofEpochMilli(ts).atZone(zone).minute assertEquals(expected.toLong(), QuietHoursPrefs.minuteOfDay(ts, zone).toLong()) } + + // ── Minute-level trim ([QuietHoursPrefs.keptMinutes]) ──────────────────── + + private val utc = java.time.ZoneId.of("UTC") + /** 2026-09-17 at [h]:[m] UTC, epoch millis. */ + private fun at(h: Int, m: Int) = + java.time.ZonedDateTime.of(2026, 9, 17, h, m, 0, 0, utc).toInstant().toEpochMilli() + + /** + * The reported night as one unsplit record: 23:08 on the sofa straight through to 07:08. Judged + * by its start the whole night would stand or fall together; trimmed, the sofa minutes before + * a 00:05 start and the minutes after 07:00 go, and the real night stays. + */ + @Test + fun `a sofa start running into the night is trimmed, not dropped`() { + val minutes = 8 * 60 // 23:08 → 07:08 + val kept = QuietHoursPrefs.keptMinutes(at(23, 8), minutes, 0 * 60 + 5, 7 * 60, utc) + assertEquals(57 until 57 + (6 * 60 + 55), kept) // 00:05 → 07:00 + } + + @Test + fun `a record wholly outside the window is dropped`() { + assertEquals(null, QuietHoursPrefs.keptMinutes(at(14, 0), 45, 22 * 60, 7 * 60, utc)) + } + + @Test + fun `a record wholly inside the window is kept whole`() { + assertEquals(0 until 300, QuietHoursPrefs.keptMinutes(at(23, 30), 300, 22 * 60, 7 * 60, utc)) + } + + @Test + fun `an equal window keeps every minute`() { + assertEquals(0 until 90, QuietHoursPrefs.keptMinutes(at(15, 0), 90, 8 * 60, 8 * 60, utc)) + } + + /** A record touching two windows keeps the longer stretch, never both across the hole. */ + @Test + fun `a record touching two windows keeps the longer stretch`() { + // Window 12:00 → 13:00 daily; a record 12:30 → next day 12:10 (MAX timeline allows it in + // principle): 30 minutes today vs 10 tomorrow. + val kept = QuietHoursPrefs.keptMinutes(at(12, 30), 24 * 60 - 20, 12 * 60, 13 * 60, utc) + assertEquals(0 until 30, kept) + } + + @Test + fun `an empty record keeps nothing`() { + assertEquals(null, QuietHoursPrefs.keptMinutes(at(23, 0), 0, 22 * 60, 7 * 60, utc)) + } } From 5258b2d2c69096ec843f4fb642cc82cd73522b85 Mon Sep 17 00:00:00 2001 From: Khoa Truong Date: Thu, 24 Sep 2026 18:33:40 -0700 Subject: [PATCH 8/9] fix(coach): a stored retired model reads as the default (#77) Dropping gpt-4o / gpt-4o-mini / o4-mini from the picker left anyone who had one selected stuck on it, with nothing in the list showing what was stored. OpenAIModel.normalize() maps the retired slugs (and blank) to the default in ApiKeyStore.model; a typed unknown slug is left alone. The picker now lists OpenAIModel's entries instead of a second hand-kept list, which had already drifted (gpt-5.4-mini and gpt-5.5 were missing). The Gemini 503 retry backs off 2 s then 4 s (2000 shl 1), not 8 s as the comment said. --- .../pulseloop/coach/config/CoachModelPresets.kt | 8 ++++++++ .../com/pulseloop/coach/gemini/GeminiClient.kt | 2 +- .../java/com/pulseloop/settings/ApiKeyStore.kt | 3 ++- .../pulseloop/ui/screens/SettingsSubScreens.kt | 9 ++++----- .../com/pulseloop/coach/CoachModelPresetsTest.kt | 16 ++++++++++++++++ 5 files changed, 31 insertions(+), 7 deletions(-) diff --git a/app/src/main/java/com/pulseloop/coach/config/CoachModelPresets.kt b/app/src/main/java/com/pulseloop/coach/config/CoachModelPresets.kt index f2159e1c..0d3ab5d4 100644 --- a/app/src/main/java/com/pulseloop/coach/config/CoachModelPresets.kt +++ b/app/src/main/java/com/pulseloop/coach/config/CoachModelPresets.kt @@ -97,5 +97,13 @@ enum class OpenAIModel(val slug: String, val blurb: String) { companion object { val DEFAULT = GPT_54 + + /** Slugs the Settings picker used to offer and no longer does (issue #77). A stored + * selection of one would keep failing with no visible way out, so it reads as [DEFAULT]. */ + val RETIRED_SLUGS = setOf("gpt-4o", "gpt-4o-mini", "o4-mini") + + /** [slug], or [DEFAULT]'s slug when it is blank or retired. */ + fun normalize(slug: String): String = + if (slug.isBlank() || slug in RETIRED_SLUGS) DEFAULT.slug else slug } } diff --git a/app/src/main/java/com/pulseloop/coach/gemini/GeminiClient.kt b/app/src/main/java/com/pulseloop/coach/gemini/GeminiClient.kt index a80efd2c..9c69115d 100644 --- a/app/src/main/java/com/pulseloop/coach/gemini/GeminiClient.kt +++ b/app/src/main/java/com/pulseloop/coach/gemini/GeminiClient.kt @@ -102,7 +102,7 @@ class GeminiClient( return post(url, bodyBytes) } catch (e: ResponsesError.Http) { if (e.status != 503 || attempt == MAX_OVERLOAD_RETRIES) throw e - delay(backoffMs shl attempt) // 2 s, then 8 s + delay(backoffMs shl attempt) // 2 s, then 4 s } } throw IllegalStateException("overload retry loop never returned") diff --git a/app/src/main/java/com/pulseloop/settings/ApiKeyStore.kt b/app/src/main/java/com/pulseloop/settings/ApiKeyStore.kt index fbb6d503..a679e4ba 100644 --- a/app/src/main/java/com/pulseloop/settings/ApiKeyStore.kt +++ b/app/src/main/java/com/pulseloop/settings/ApiKeyStore.kt @@ -27,7 +27,8 @@ class ApiKeyStore(context: Context) { val hasApiKey: Boolean get() = apiKey.isNotBlank() var model: String - get() = prefs.getString(KEY_MODEL, "gpt-5.4") ?: "gpt-5.4" + // Normalized so a retired slug stored by an older picker (issue #77) reads as the default. + get() = com.pulseloop.coach.config.OpenAIModel.normalize(prefs.getString(KEY_MODEL, "") ?: "") set(value) { prefs.edit().putString(KEY_MODEL, value).apply() } var coachEnabled: Boolean diff --git a/app/src/main/java/com/pulseloop/ui/screens/SettingsSubScreens.kt b/app/src/main/java/com/pulseloop/ui/screens/SettingsSubScreens.kt index f7be4f6d..2bfbe4ac 100644 --- a/app/src/main/java/com/pulseloop/ui/screens/SettingsSubScreens.kt +++ b/app/src/main/java/com/pulseloop/ui/screens/SettingsSubScreens.kt @@ -202,10 +202,9 @@ fun CoachSettingsScreen(onBack: () -> Unit) { } } - // Retired slugs removed (issue #77): gpt-4o / gpt-4o-mini / o4-mini are rejected by the - // Responses API, and a picker entry that cannot work is worse than a shorter list. New - // models go here as OpenAI ships them. - val models = listOf("gpt-5.4") + // The curated OpenAI picks live in one place, [OpenAIModel]; the retired gpt-4o / gpt-4o-mini + // / o4-mini are gone from it (issue #77), and a stored one reads as the default. + val models = com.pulseloop.coach.config.OpenAIModel.entries.map { it.slug to it.blurb } SettingsSubScreen(title = "AI Coach", onBack = onBack) { // AI Coach section — ported from CoachSettingsSection.swift @@ -677,7 +676,7 @@ fun CoachSettingsScreen(onBack: () -> Unit) { } else -> { // OpenAI (and legacy modes): the original model picker + key field. - ModelDropdown("Model", selectedModel, models.map { it to "" }) { + ModelDropdown("Model", selectedModel, models) { selectedModel = it; keyStore.model = it } KeyField( diff --git a/app/src/test/java/com/pulseloop/coach/CoachModelPresetsTest.kt b/app/src/test/java/com/pulseloop/coach/CoachModelPresetsTest.kt index cbcb7d77..92f04744 100644 --- a/app/src/test/java/com/pulseloop/coach/CoachModelPresetsTest.kt +++ b/app/src/test/java/com/pulseloop/coach/CoachModelPresetsTest.kt @@ -52,4 +52,20 @@ class CoachModelPresetsTest { // ApiKeyStore.model defaults to "gpt-5.4"; the preset default must agree. assertEquals("gpt-5.4", OpenAIModel.DEFAULT.slug) } + + /** Issue #77: a stored retired slug must not strand the user on a model the picker no longer offers. */ + @Test + fun `retired OpenAI slugs normalize to the default`() { + for (slug in listOf("gpt-4o", "gpt-4o-mini", "o4-mini", "")) { + assertEquals(OpenAIModel.DEFAULT.slug, OpenAIModel.normalize(slug)) + } + assertEquals("gpt-5.5", OpenAIModel.normalize("gpt-5.5")) + // A typed/unknown slug is the user's choice — only the retired set is rewritten. + assertEquals("gpt-6-preview", OpenAIModel.normalize("gpt-6-preview")) + } + + @Test + fun `no preset is a retired slug`() { + OpenAIModel.entries.forEach { assertFalse(it.slug in OpenAIModel.RETIRED_SLUGS) } + } } From 8770aa2c9ef62c40705e328ba9cc4928828fa06b Mon Sep 17 00:00:00 2001 From: Khoa Truong Date: Thu, 24 Sep 2026 18:33:40 -0700 Subject: [PATCH 9/9] fix(activity): refresh the day pager's bound on resume (#76) maxDayOffset was computed once in init, so days synced (or a midnight rolled) since the ViewModel was built stayed out of the chevrons' reach. --- app/src/main/java/com/pulseloop/ui/viewmodels/ViewModels.kt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/app/src/main/java/com/pulseloop/ui/viewmodels/ViewModels.kt b/app/src/main/java/com/pulseloop/ui/viewmodels/ViewModels.kt index cea07ac8..7fc6c98a 100644 --- a/app/src/main/java/com/pulseloop/ui/viewmodels/ViewModels.kt +++ b/app/src/main/java/com/pulseloop/ui/viewmodels/ViewModels.kt @@ -449,6 +449,9 @@ class ActivityViewModel(db: PulseLoopDatabase) : ViewModel() { todayStart.value = TimeUtil.startOfTodayLocal() // A resume lands the user back on today, where they left the app from. jumpToOffset(0) + // Re-read the pager bound too: it was computed once at init, so days synced (or a + // midnight rolled) since then would stay out of reach until the ViewModel was rebuilt. + viewModelScope.launch { refreshMaxDayOffset() } } init {