From ba8a6318eca71f6793248c8a2d976c5471c7a050 Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Wed, 9 Sep 2026 11:11:25 -0400 Subject: [PATCH 01/24] feat(notifications): model push-triggered work as PushAction --- .../flipcash/app/notifications/PushAction.kt | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 apps/flipcash/shared/notifications/src/main/kotlin/com/flipcash/app/notifications/PushAction.kt diff --git a/apps/flipcash/shared/notifications/src/main/kotlin/com/flipcash/app/notifications/PushAction.kt b/apps/flipcash/shared/notifications/src/main/kotlin/com/flipcash/app/notifications/PushAction.kt new file mode 100644 index 000000000..bb0449f8e --- /dev/null +++ b/apps/flipcash/shared/notifications/src/main/kotlin/com/flipcash/app/notifications/PushAction.kt @@ -0,0 +1,32 @@ +package com.flipcash.app.notifications + +import com.flipcash.services.models.NotificationPayload +import com.flipcash.services.models.chat.ChatId + +/** + * A single unit of work a received push asks the app to perform. + * + * Modelling this as data rather than as control flow inside + * [NotificationService.onMessageReceived] is what makes the push handling + * rules unit-testable without Robolectric or a live FirebaseMessagingService. + */ +sealed interface PushAction { + /** Server-side feed sync. Safe to request redundantly. */ + data object RefreshFeed : PushAction + + /** Fetch and persist full message history for one chat. */ + data class LoadMessages(val chatId: ChatId) : PushAction + + /** Refresh token/mint state. */ + data object UpdateTokens : PushAction + + /** Refresh the contact list. */ + data object SyncContacts : PushAction + + /** Post a user-visible notification. Absent for a silent push. */ + data class PostNotification( + val title: String, + val body: String?, + val payload: NotificationPayload?, + ) : PushAction +} From 106a80da87d335492b8c30aebc36b286cd0e49e9 Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Wed, 9 Sep 2026 11:13:11 -0400 Subject: [PATCH 02/24] feat(notifications): extract push handling rules into a pure planner --- .../app/notifications/PushHandlingPlanner.kt | 56 +++++++++++ .../notifications/PushHandlingPlannerTest.kt | 98 +++++++++++++++++++ 2 files changed, 154 insertions(+) create mode 100644 apps/flipcash/shared/notifications/src/main/kotlin/com/flipcash/app/notifications/PushHandlingPlanner.kt create mode 100644 apps/flipcash/shared/notifications/src/test/kotlin/com/flipcash/app/notifications/PushHandlingPlannerTest.kt diff --git a/apps/flipcash/shared/notifications/src/main/kotlin/com/flipcash/app/notifications/PushHandlingPlanner.kt b/apps/flipcash/shared/notifications/src/main/kotlin/com/flipcash/app/notifications/PushHandlingPlanner.kt new file mode 100644 index 000000000..e28e45fa0 --- /dev/null +++ b/apps/flipcash/shared/notifications/src/main/kotlin/com/flipcash/app/notifications/PushHandlingPlanner.kt @@ -0,0 +1,56 @@ +package com.flipcash.app.notifications + +import com.flipcash.services.models.NavigationTrigger +import com.flipcash.services.models.NotificationCategory +import com.flipcash.services.models.NotificationPayload + +/** + * Decides what a received push should cause the app to do. + * + * Pure by construction: no Android types, no coroutines, no injection. Every + * rule about push handling is testable here, which is the point. + * + * @param title resolved push title, null for a data-only push + * @param body resolved push body, may be null even for a visible push + * @param payload decoded [NotificationPayload], null when absent or undecodable + * @param silentSyncEnabled whether the `PushSilentSync` feature flag is on + */ +fun planPushHandling( + title: String?, + body: String?, + payload: NotificationPayload?, + silentSyncEnabled: Boolean, +): List { + // Today a data-only push is dropped before its payload is read. Task 4 + // replaces this early return with the silent branch. + if (title == null) return emptyList() + + val actions = mutableListOf() + actions += syncActionsFor(payload) + actions += PushAction.PostNotification(title, body, payload) + return actions +} + +/** The sync work implied by [payload], independent of visibility. */ +private fun syncActionsFor(payload: NotificationPayload?): List { + if (payload == null) return emptyList() + + val actions = mutableListOf() + + if (payload.navigation is NavigationTrigger.CurrencyInfo) { + actions += PushAction.UpdateTokens + } + + if (payload.category == NotificationCategory.CONTACT_JOIN) { + actions += PushAction.RefreshFeed + actions += PushAction.SyncContacts + } + + val navigation = payload.navigation + if (navigation is NavigationTrigger.Chat.ById) { + if (PushAction.RefreshFeed !in actions) actions += PushAction.RefreshFeed + actions += PushAction.LoadMessages(navigation.chatId) + } + + return actions +} diff --git a/apps/flipcash/shared/notifications/src/test/kotlin/com/flipcash/app/notifications/PushHandlingPlannerTest.kt b/apps/flipcash/shared/notifications/src/test/kotlin/com/flipcash/app/notifications/PushHandlingPlannerTest.kt new file mode 100644 index 000000000..9a123255d --- /dev/null +++ b/apps/flipcash/shared/notifications/src/test/kotlin/com/flipcash/app/notifications/PushHandlingPlannerTest.kt @@ -0,0 +1,98 @@ +package com.flipcash.app.notifications + +import com.flipcash.services.models.NavigationTrigger +import com.flipcash.services.models.NotificationCategory +import com.flipcash.services.models.NotificationPayload +import com.flipcash.services.models.chat.ChatId +import com.getcode.solana.keys.Mint +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertTrue + +class PushHandlingPlannerTest { + + private fun payload( + navigation: NavigationTrigger? = null, + category: NotificationCategory = NotificationCategory.DEFAULT, + ) = NotificationPayload( + navigation = navigation, + category = category, + ) + + // region Today's behaviour: a titleless push is dropped entirely + + @Test + fun `no title yields no actions when silent sync is disabled`() { + val actions = planPushHandling( + title = null, + body = "ignored", + payload = payload(navigation = NavigationTrigger.CurrencyInfo(mint = TEST_MINT)), + silentSyncEnabled = false, + ) + assertEquals(emptyList(), actions) + } + + @Test + fun `no title drops chat sync too when silent sync is disabled`() { + val actions = planPushHandling( + title = null, + body = null, + payload = payload(navigation = NavigationTrigger.Chat.ById(ChatId("aa01"))), + silentSyncEnabled = false, + ) + assertEquals(emptyList(), actions) + } + + // endregion + + // region Today's behaviour: a titled push + + @Test + fun `currency info push updates tokens and posts`() { + val p = payload(navigation = NavigationTrigger.CurrencyInfo(mint = TEST_MINT)) + val actions = planPushHandling("Title", "Body", p, silentSyncEnabled = false) + assertTrue(PushAction.UpdateTokens in actions) + assertTrue(actions.last() is PushAction.PostNotification) + } + + @Test + fun `contact join push refreshes feed and syncs contacts`() { + val p = payload(category = NotificationCategory.CONTACT_JOIN) + val actions = planPushHandling("Title", null, p, silentSyncEnabled = false) + assertTrue(PushAction.RefreshFeed in actions) + assertTrue(PushAction.SyncContacts in actions) + } + + @Test + fun `chat push refreshes feed then loads that chat`() { + val chatId = ChatId("aa07") + val p = payload(navigation = NavigationTrigger.Chat.ById(chatId)) + val actions = planPushHandling("Title", "Body", p, silentSyncEnabled = false) + assertEquals( + listOf(PushAction.RefreshFeed, PushAction.LoadMessages(chatId)), + actions.filterNot { it is PushAction.PostNotification }, + ) + } + + @Test + fun `push with no payload still posts the notification`() { + val actions = planPushHandling("Title", "Body", payload = null, silentSyncEnabled = false) + assertEquals( + listOf(PushAction.PostNotification("Title", "Body", null)), + actions, + ) + } + + @Test + fun `post notification is always last so sync starts first`() { + val p = payload(navigation = NavigationTrigger.Chat.ById(ChatId("0c"))) + val actions = planPushHandling("Title", "Body", p, silentSyncEnabled = false) + assertTrue(actions.last() is PushAction.PostNotification) + } + + // endregion + + companion object { + private val TEST_MINT = Mint.usdc + } +} From 0b902c064cd0718c5479be77b1e1a617a4ae3b54 Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Wed, 9 Sep 2026 11:13:57 -0400 Subject: [PATCH 03/24] refactor(notifications): route onMessageReceived through the planner --- .../app/notifications/NotificationService.kt | 65 ++++++++++++------- 1 file changed, 43 insertions(+), 22 deletions(-) diff --git a/apps/flipcash/shared/notifications/src/main/kotlin/com/flipcash/app/notifications/NotificationService.kt b/apps/flipcash/shared/notifications/src/main/kotlin/com/flipcash/app/notifications/NotificationService.kt index 02805ef5c..a51344f0b 100644 --- a/apps/flipcash/shared/notifications/src/main/kotlin/com/flipcash/app/notifications/NotificationService.kt +++ b/apps/flipcash/shared/notifications/src/main/kotlin/com/flipcash/app/notifications/NotificationService.kt @@ -135,52 +135,73 @@ class NotificationService : FirebaseMessagingService(), val title = message.data[KEY_TITLE]?.ifEmpty { message.notification?.title } val body = message.data[KEY_BODY]?.ifEmpty { message.notification?.body } + val payload = message.data.getOrDefault(KEY_PAYLOAD, "") + .takeIf { it.isNotEmpty() } + ?.let { NotificationPayload.fromEncoded(it) } + + val actions = planPushHandling( + title = title, + body = body, + payload = payload, + silentSyncEnabled = false, + ) + trace( message = "onMessageReceived", type = TraceType.Process, metadata = { "title" to title "body" to body + "actions" to actions.size } ) - if (title == null) return + if (actions.isEmpty()) return - val payload = message.data.getOrDefault(KEY_PAYLOAD, "") - .takeIf { it.isNotEmpty() } - ?.let { NotificationPayload.fromEncoded(it) } + execute(actions) + } - if (payload?.navigation is NavigationTrigger.CurrencyInfo) { + /** Runs a planned action list. Sync work starts immediately; posting a + * notification waits for authentication, as it always has. */ + private fun execute(actions: List) { + if (PushAction.UpdateTokens in actions) { launch { tokenCoordinator.update() } } - if (payload?.category == NotificationCategory.CONTACT_JOIN) { - launch { - chatCoordinator.refreshFeed() - } + val chatActions = actions.filter { + it is PushAction.RefreshFeed || it is PushAction.LoadMessages } - - when (val trigger = payload?.navigation) { - is NavigationTrigger.Chat.ById -> { - launch { - chatCoordinator.refreshFeed() - chatCoordinator.loadMessages(chatId = trigger.chatId) + if (chatActions.isNotEmpty()) { + launch { + chatActions.forEach { action -> + when (action) { + is PushAction.RefreshFeed -> chatCoordinator.refreshFeed() + is PushAction.LoadMessages -> chatCoordinator.loadMessages(chatId = action.chatId) + else -> Unit + } } } - else -> Unit } + val post = actions.filterIsInstance().firstOrNull() + val syncContacts = actions.any { it is PushAction.SyncContacts } + + if (post == null && !syncContacts) return + authenticateIfNeeded { launch { try { - if (payload?.category == NotificationCategory.CONTACT_JOIN) { - launch { contactCoordinator.sync() } + if (syncContacts) launch { contactCoordinator.sync() } + if (post != null) { + val resolvedTitle = + applySubstitutions(post.title, post.payload?.titleSubstitutions.orEmpty()) + val resolvedBody = post.body?.let { + applySubstitutions(it, post.payload?.bodySubstitutions.orEmpty()) + } + postNotification(resolvedTitle, resolvedBody, post.payload) } - val resolvedTitle = applySubstitutions(title, payload?.titleSubstitutions.orEmpty()) - val resolvedBody = body?.let { applySubstitutions(it, payload?.bodySubstitutions.orEmpty()) } - postNotification(resolvedTitle, resolvedBody, payload) } catch (e: Exception) { - trace(tag = "NotificationService", message = "Failed to post notification", error = e) + trace(tag = "NotificationService", message = "Failed to handle push", error = e) } } } From ddae046d3a1f652ebb92db8007893857f369b340 Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Wed, 9 Sep 2026 11:16:31 -0400 Subject: [PATCH 04/24] feat(notifications): handle data-only pushes behind PushSilentSync flag --- .../flipcash/app/featureflags/FeatureFlag.kt | 13 ++++++ .../shared/notifications/build.gradle.kts | 1 + .../app/notifications/NotificationService.kt | 8 +++- .../app/notifications/PushHandlingPlanner.kt | 6 +-- .../notifications/PushHandlingPlannerTest.kt | 43 +++++++++++++++++++ 5 files changed, 67 insertions(+), 4 deletions(-) diff --git a/apps/flipcash/shared/featureflags/src/main/kotlin/com/flipcash/app/featureflags/FeatureFlag.kt b/apps/flipcash/shared/featureflags/src/main/kotlin/com/flipcash/app/featureflags/FeatureFlag.kt index 08703a32d..927c7b790 100644 --- a/apps/flipcash/shared/featureflags/src/main/kotlin/com/flipcash/app/featureflags/FeatureFlag.kt +++ b/apps/flipcash/shared/featureflags/src/main/kotlin/com/flipcash/app/featureflags/FeatureFlag.kt @@ -117,6 +117,17 @@ sealed interface FeatureFlag { override val persistLogOut: Boolean = false } + @FeatureFlagMarker + data object PushSilentSync: FeatureFlag { + override val key: String = "push_silent_sync_enabled" + override val default: Boolean = false + override val launched: Boolean = false + override val visible: Boolean = true + override val persistLogOut: Boolean = false + override val onboarding: Boolean = false + override val minTrack: FeatureTrack = FeatureTrack.Internal + } + companion object { val entries: List> get() = FeatureFlagEntries.entries @@ -139,6 +150,7 @@ val FeatureFlag<*>.title: String FeatureFlag.ContactPickerMode -> "Contact Picker Mode" FeatureFlag.ShowNetworkState -> "Network Offline Indicator" FeatureFlag.FrostedTipCard -> "Frosted Tip Card" + FeatureFlag.PushSilentSync -> "Push Silent Sync" } val FeatureFlag<*>.message: String @@ -152,6 +164,7 @@ val FeatureFlag<*>.message: String FeatureFlag.ContactPickerMode -> "When enabled, contacts will be accessed via the system contact picker instead of requesting full READ_CONTACTS permission" FeatureFlag.ShowNetworkState -> "When enabled, you'll gain the ability to see the network state on the Scanner when offline" FeatureFlag.FrostedTipCard -> "When enabled, the tip card in the scanner renders as frosted glass over a blurred snapshot of the camera instead of a solid card" + FeatureFlag.PushSilentSync -> "When enabled, data-only pushes trigger background sync work without posting a visible notification" } diff --git a/apps/flipcash/shared/notifications/build.gradle.kts b/apps/flipcash/shared/notifications/build.gradle.kts index 3488b3360..d16d684bd 100644 --- a/apps/flipcash/shared/notifications/build.gradle.kts +++ b/apps/flipcash/shared/notifications/build.gradle.kts @@ -11,6 +11,7 @@ dependencies { implementation(project(":apps:flipcash:shared:authentication")) implementation(project(":apps:flipcash:shared:chat")) implementation(project(":apps:flipcash:shared:contacts")) + implementation(project(":apps:flipcash:shared:featureflags")) implementation(project(":apps:flipcash:shared:persistence:sources")) implementation(project(":apps:flipcash:shared:phone")) implementation(project(":apps:flipcash:shared:push")) diff --git a/apps/flipcash/shared/notifications/src/main/kotlin/com/flipcash/app/notifications/NotificationService.kt b/apps/flipcash/shared/notifications/src/main/kotlin/com/flipcash/app/notifications/NotificationService.kt index a51344f0b..c4efcf4cf 100644 --- a/apps/flipcash/shared/notifications/src/main/kotlin/com/flipcash/app/notifications/NotificationService.kt +++ b/apps/flipcash/shared/notifications/src/main/kotlin/com/flipcash/app/notifications/NotificationService.kt @@ -26,6 +26,8 @@ import com.flipcash.app.core.media.MediaUrlResolver import com.flipcash.app.auth.AuthManager import com.flipcash.app.contacts.ContactCoordinator import com.flipcash.app.contacts.ContactResolver +import com.flipcash.app.featureflags.FeatureFlag +import com.flipcash.app.featureflags.FeatureFlagController import com.flipcash.app.core.util.Linkify import com.flipcash.shared.chat.ChatCoordinator import com.flipcash.app.tokens.TokenCoordinator @@ -54,6 +56,7 @@ import dagger.hilt.android.AndroidEntryPoint import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.launch +import kotlinx.coroutines.runBlocking import kotlinx.coroutines.withTimeoutOrNull import java.security.SecureRandom import javax.inject.Inject @@ -108,6 +111,9 @@ class NotificationService : FirebaseMessagingService(), @Inject lateinit var userProfileDataSource: UserProfileDataSource + @Inject + lateinit var featureFlags: FeatureFlagController + // TODO(firebase-messaging): 25.1.0 deprecated onNewToken in favor of FID-based onRegistered(). // Migrate once Firebase ships a stable guide and the backend accepts FID registration. // Tracking: https://github.com/firebase/firebase-android-sdk/issues/8087 @@ -143,7 +149,7 @@ class NotificationService : FirebaseMessagingService(), title = title, body = body, payload = payload, - silentSyncEnabled = false, + silentSyncEnabled = runBlocking { featureFlags.get(FeatureFlag.PushSilentSync) }, ) trace( diff --git a/apps/flipcash/shared/notifications/src/main/kotlin/com/flipcash/app/notifications/PushHandlingPlanner.kt b/apps/flipcash/shared/notifications/src/main/kotlin/com/flipcash/app/notifications/PushHandlingPlanner.kt index e28e45fa0..e908092d3 100644 --- a/apps/flipcash/shared/notifications/src/main/kotlin/com/flipcash/app/notifications/PushHandlingPlanner.kt +++ b/apps/flipcash/shared/notifications/src/main/kotlin/com/flipcash/app/notifications/PushHandlingPlanner.kt @@ -21,9 +21,9 @@ fun planPushHandling( payload: NotificationPayload?, silentSyncEnabled: Boolean, ): List { - // Today a data-only push is dropped before its payload is read. Task 4 - // replaces this early return with the silent branch. - if (title == null) return emptyList() + if (title == null) { + return if (silentSyncEnabled) syncActionsFor(payload) else emptyList() + } val actions = mutableListOf() actions += syncActionsFor(payload) diff --git a/apps/flipcash/shared/notifications/src/test/kotlin/com/flipcash/app/notifications/PushHandlingPlannerTest.kt b/apps/flipcash/shared/notifications/src/test/kotlin/com/flipcash/app/notifications/PushHandlingPlannerTest.kt index 9a123255d..69c9c3630 100644 --- a/apps/flipcash/shared/notifications/src/test/kotlin/com/flipcash/app/notifications/PushHandlingPlannerTest.kt +++ b/apps/flipcash/shared/notifications/src/test/kotlin/com/flipcash/app/notifications/PushHandlingPlannerTest.kt @@ -92,6 +92,49 @@ class PushHandlingPlannerTest { // endregion + // region Silent sync enabled + + @Test + fun `no title still syncs chat when silent sync is enabled`() { + val chatId = ChatId("aa09") + val actions = planPushHandling( + title = null, + body = null, + payload = payload(navigation = NavigationTrigger.Chat.ById(chatId)), + silentSyncEnabled = true, + ) + assertEquals(listOf(PushAction.RefreshFeed, PushAction.LoadMessages(chatId)), actions) + } + + @Test + fun `silent push never posts a notification`() { + val actions = planPushHandling( + title = null, + body = null, + payload = payload(navigation = NavigationTrigger.CurrencyInfo(mint = TEST_MINT)), + silentSyncEnabled = true, + ) + assertTrue(actions.none { it is PushAction.PostNotification }) + assertEquals(listOf(PushAction.UpdateTokens), actions) + } + + @Test + fun `silent push with no payload does nothing`() { + val actions = planPushHandling(null, null, payload = null, silentSyncEnabled = true) + assertEquals(emptyList(), actions) + } + + @Test + fun `enabling silent sync does not change a titled push`() { + val p = payload(navigation = NavigationTrigger.Chat.ById(ChatId("0c"))) + assertEquals( + planPushHandling("Title", "Body", p, silentSyncEnabled = false), + planPushHandling("Title", "Body", p, silentSyncEnabled = true), + ) + } + + // endregion + companion object { private val TEST_MINT = Mint.usdc } From f718f1ca78f051e8c89333232396435e9f1f3df2 Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Wed, 9 Sep 2026 11:17:36 -0400 Subject: [PATCH 05/24] feat(notifications): report the app's own standby bucket --- .../notifications/StandbyBucketReporter.kt | 27 +++++++++++++++++++ .../StandbyBucketReporterTest.kt | 22 +++++++++++++++ 2 files changed, 49 insertions(+) create mode 100644 apps/flipcash/shared/notifications/src/main/kotlin/com/flipcash/app/notifications/StandbyBucketReporter.kt create mode 100644 apps/flipcash/shared/notifications/src/test/kotlin/com/flipcash/app/notifications/StandbyBucketReporterTest.kt diff --git a/apps/flipcash/shared/notifications/src/main/kotlin/com/flipcash/app/notifications/StandbyBucketReporter.kt b/apps/flipcash/shared/notifications/src/main/kotlin/com/flipcash/app/notifications/StandbyBucketReporter.kt new file mode 100644 index 000000000..3845a6159 --- /dev/null +++ b/apps/flipcash/shared/notifications/src/main/kotlin/com/flipcash/app/notifications/StandbyBucketReporter.kt @@ -0,0 +1,27 @@ +package com.flipcash.app.notifications + +import android.app.usage.UsageStatsManager +import android.content.Context +import androidx.core.content.getSystemService + +/** + * Stable string label for a raw [UsageStatsManager] standby bucket constant. + * + * Separated from the system lookup so the mapping is testable without a + * device, and so an unrecognised bucket is preserved rather than collapsed + * into "unknown" — a new bucket constant would otherwise vanish silently. + */ +fun bucketLabel(bucket: Int): String = when (bucket) { + UsageStatsManager.STANDBY_BUCKET_ACTIVE -> "active" + UsageStatsManager.STANDBY_BUCKET_WORKING_SET -> "working_set" + UsageStatsManager.STANDBY_BUCKET_FREQUENT -> "frequent" + UsageStatsManager.STANDBY_BUCKET_RARE -> "rare" + UsageStatsManager.STANDBY_BUCKET_RESTRICTED -> "restricted" + else -> "unknown_$bucket" +} + +/** The calling app's current standby bucket, or "unavailable" if it cannot be read. */ +fun Context.currentStandbyBucket(): String { + val manager = getSystemService() ?: return "unavailable" + return runCatching { bucketLabel(manager.appStandbyBucket) }.getOrElse { "unavailable" } +} diff --git a/apps/flipcash/shared/notifications/src/test/kotlin/com/flipcash/app/notifications/StandbyBucketReporterTest.kt b/apps/flipcash/shared/notifications/src/test/kotlin/com/flipcash/app/notifications/StandbyBucketReporterTest.kt new file mode 100644 index 000000000..47f01b6ec --- /dev/null +++ b/apps/flipcash/shared/notifications/src/test/kotlin/com/flipcash/app/notifications/StandbyBucketReporterTest.kt @@ -0,0 +1,22 @@ +package com.flipcash.app.notifications + +import android.app.usage.UsageStatsManager +import kotlin.test.Test +import kotlin.test.assertEquals + +class StandbyBucketReporterTest { + + @Test + fun `maps every documented bucket to a stable label`() { + assertEquals("active", bucketLabel(UsageStatsManager.STANDBY_BUCKET_ACTIVE)) + assertEquals("working_set", bucketLabel(UsageStatsManager.STANDBY_BUCKET_WORKING_SET)) + assertEquals("frequent", bucketLabel(UsageStatsManager.STANDBY_BUCKET_FREQUENT)) + assertEquals("rare", bucketLabel(UsageStatsManager.STANDBY_BUCKET_RARE)) + assertEquals("restricted", bucketLabel(UsageStatsManager.STANDBY_BUCKET_RESTRICTED)) + } + + @Test + fun `maps an unknown bucket to its raw value rather than losing it`() { + assertEquals("unknown_999", bucketLabel(999)) + } +} From 71c2d398436cfd8afdf748acec43a37b7e2ca298 Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Wed, 9 Sep 2026 11:18:13 -0400 Subject: [PATCH 06/24] feat(notifications): trace push delivery latency, bucket and priority --- .../com/flipcash/app/notifications/NotificationService.kt | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/apps/flipcash/shared/notifications/src/main/kotlin/com/flipcash/app/notifications/NotificationService.kt b/apps/flipcash/shared/notifications/src/main/kotlin/com/flipcash/app/notifications/NotificationService.kt index c4efcf4cf..67472c3e6 100644 --- a/apps/flipcash/shared/notifications/src/main/kotlin/com/flipcash/app/notifications/NotificationService.kt +++ b/apps/flipcash/shared/notifications/src/main/kotlin/com/flipcash/app/notifications/NotificationService.kt @@ -152,6 +152,9 @@ class NotificationService : FirebaseMessagingService(), silentSyncEnabled = runBlocking { featureFlags.get(FeatureFlag.PushSilentSync) }, ) + val latencyMs = System.currentTimeMillis() - message.sentTime + val bucket = applicationContext.currentStandbyBucket() + trace( message = "onMessageReceived", type = TraceType.Process, @@ -159,6 +162,11 @@ class NotificationService : FirebaseMessagingService(), "title" to title "body" to body "actions" to actions.size + "silent" to (title == null) + "bucket" to bucket + "latency_ms" to latencyMs + "priority" to message.priority + "original_priority" to message.originalPriority } ) From 2471f7b6e7df077fc2c52fe8598be99d1fec649f Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Wed, 9 Sep 2026 15:24:00 -0400 Subject: [PATCH 07/24] fix(notifications): record push trace fields that were being dropped MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MetadataBuilder declares `infix fun String.to(value: Any)`. Passing the nullable title and body resolved to kotlin.to instead, which builds a Pair and discards it, so neither field ever reached the log — including when the push carried a body. A device trace confirms it: a push sent with push_notification_body=sanity-silent-001 logged onMessageReceived | actions=0, silent=true, bucket=active, latency_ms=-329 with no body= at all. It compiles clean, so nothing flagged it. Replace both with non-null values. Body content stays out: TraceType.Process is forwarded to breadcrumb sinks, and message text does not belong in Bugsnag. has_body carries what the spike needs, and spike_seq gives the bucket matrix a per-push correlation id so a dropped push and a late one are distinguishable. --- .../app/notifications/NotificationService.kt | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/apps/flipcash/shared/notifications/src/main/kotlin/com/flipcash/app/notifications/NotificationService.kt b/apps/flipcash/shared/notifications/src/main/kotlin/com/flipcash/app/notifications/NotificationService.kt index 67472c3e6..261f10ede 100644 --- a/apps/flipcash/shared/notifications/src/main/kotlin/com/flipcash/app/notifications/NotificationService.kt +++ b/apps/flipcash/shared/notifications/src/main/kotlin/com/flipcash/app/notifications/NotificationService.kt @@ -71,6 +71,11 @@ class NotificationService : FirebaseMessagingService(), private const val KEY_BODY = "push_notification_body" private const val KEY_PAYLOAD = "flipcash_payload" + // Spike-only correlation id. The bucket matrix sends a known sequence + // number with every push so a missing delivery and a late delivery can + // be told apart in the log; nothing in production sets it. + private const val KEY_SPIKE_SEQ = "spike_seq" + // Upper bound on how long we'll wait for a remote avatar before posting // without one. A memory/disk cache hit returns well under this; the // bound only caps the cold-cache network fetch so the notification isn't @@ -159,8 +164,11 @@ class NotificationService : FirebaseMessagingService(), message = "onMessageReceived", type = TraceType.Process, metadata = { - "title" to title - "body" to body + // MetadataBuilder.to takes a non-null Any, so passing a String? + // silently resolves to kotlin.to, builds a Pair and discards it. + // Every value here must be non-null or it will not be recorded. + "seq" to message.data[KEY_SPIKE_SEQ].orEmpty() + "has_body" to (body != null) "actions" to actions.size "silent" to (title == null) "bucket" to bucket From 2cc1f13fc4866460bc661d26776a4b5a27bd5c1d Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Thu, 10 Sep 2026 15:12:36 -0400 Subject: [PATCH 08/24] refactor(notifications): make the push event switch a table MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit planPushHandling branched once per rule, so the shared event taxonomy would have grown a branch per event class. The category dimension is now a map from NotificationCategory to the actions it implies, and adding an event class is an entry in it. The key becomes the new field once flipcash.push.v1.Payload carries one; the shape does not change with it. Navigation stays a `when` because each arm reads the chat id or mint off the trigger it matched, so its actions cannot be written down ahead of time. The two sources are concatenated and deduplicated, which replaces the explicit `RefreshFeed !in actions` guard: a contact-join push that also names a chat asks for a feed refresh from both sides and gets one. Two characterization tests went in first and stayed green across the change — the deduplicated contact-join-plus-chat ordering, and a category with no entry planning no sync of its own. --- .../app/notifications/PushHandlingPlanner.kt | 72 ++++++++++++------- .../notifications/PushHandlingPlannerTest.kt | 57 +++++++++++---- 2 files changed, 91 insertions(+), 38 deletions(-) diff --git a/apps/flipcash/shared/notifications/src/main/kotlin/com/flipcash/app/notifications/PushHandlingPlanner.kt b/apps/flipcash/shared/notifications/src/main/kotlin/com/flipcash/app/notifications/PushHandlingPlanner.kt index e908092d3..190f849f1 100644 --- a/apps/flipcash/shared/notifications/src/main/kotlin/com/flipcash/app/notifications/PushHandlingPlanner.kt +++ b/apps/flipcash/shared/notifications/src/main/kotlin/com/flipcash/app/notifications/PushHandlingPlanner.kt @@ -13,44 +13,64 @@ import com.flipcash.services.models.NotificationPayload * @param title resolved push title, null for a data-only push * @param body resolved push body, may be null even for a visible push * @param payload decoded [NotificationPayload], null when absent or undecodable - * @param silentSyncEnabled whether the `PushSilentSync` feature flag is on + * @param silentSyncEnabled reads the `PushSilentSync` feature flag. Passed as a + * function because only a data-only push consults it, and the call site's + * read is a blocking DataStore lookup on the FCM dispatch thread — a visible + * push should not pay for a flag that cannot change its outcome. */ fun planPushHandling( title: String?, body: String?, payload: NotificationPayload?, - silentSyncEnabled: Boolean, + silentSyncEnabled: () -> Boolean, ): List { if (title == null) { - return if (silentSyncEnabled) syncActionsFor(payload) else emptyList() + return if (silentSyncEnabled()) syncActionsFor(payload) else emptyList() } - val actions = mutableListOf() - actions += syncActionsFor(payload) - actions += PushAction.PostNotification(title, body, payload) - return actions + return syncActionsFor(payload) + PushAction.PostNotification(title, body, payload) } -/** The sync work implied by [payload], independent of visibility. */ -private fun syncActionsFor(payload: NotificationPayload?): List { - if (payload == null) return emptyList() - - val actions = mutableListOf() - - if (payload.navigation is NavigationTrigger.CurrencyInfo) { - actions += PushAction.UpdateTokens - } - - if (payload.category == NotificationCategory.CONTACT_JOIN) { - actions += PushAction.RefreshFeed - actions += PushAction.SyncContacts - } +/** + * The sync work an event class implies, independent of where the push + * navigates. + * + * Adding an event class is an entry in this table rather than a branch in + * [syncActionsFor]. The key is [NotificationCategory] until + * `flipcash.push.v1.Payload` carries the event field the shared taxonomy needs; + * when it does, the key type changes and the shape does not. + * + * A category absent from the table plans no sync of its own, which is every + * category but one today. + */ +private val syncByCategory: Map> = mapOf( + NotificationCategory.CONTACT_JOIN to listOf(PushAction.RefreshFeed, PushAction.SyncContacts), +) - val navigation = payload.navigation - if (navigation is NavigationTrigger.Chat.ById) { - if (PushAction.RefreshFeed !in actions) actions += PushAction.RefreshFeed - actions += PushAction.LoadMessages(navigation.chatId) +/** + * The sync work a navigation target implies. + * + * This stays a `when` rather than joining the table above: each arm reads data + * off the trigger it matched, so the actions cannot be written down in advance. + */ +private fun syncForNavigation(navigation: NavigationTrigger?): List = + when (navigation) { + is NavigationTrigger.CurrencyInfo -> listOf(PushAction.UpdateTokens) + is NavigationTrigger.Chat.ById -> + listOf(PushAction.RefreshFeed, PushAction.LoadMessages(navigation.chatId)) + is NavigationTrigger.Chat.ByContact -> emptyList() + null -> emptyList() } - return actions +/** + * The sync work implied by [payload], independent of visibility. + * + * `distinct()` is what lets the two sources overlap without the caller + * knowing: a contact-join push that also names a chat asks for + * [PushAction.RefreshFeed] from both and gets one. + */ +private fun syncActionsFor(payload: NotificationPayload?): List { + if (payload == null) return emptyList() + return (syncByCategory[payload.category].orEmpty() + syncForNavigation(payload.navigation)) + .distinct() } diff --git a/apps/flipcash/shared/notifications/src/test/kotlin/com/flipcash/app/notifications/PushHandlingPlannerTest.kt b/apps/flipcash/shared/notifications/src/test/kotlin/com/flipcash/app/notifications/PushHandlingPlannerTest.kt index 69c9c3630..6278aff0e 100644 --- a/apps/flipcash/shared/notifications/src/test/kotlin/com/flipcash/app/notifications/PushHandlingPlannerTest.kt +++ b/apps/flipcash/shared/notifications/src/test/kotlin/com/flipcash/app/notifications/PushHandlingPlannerTest.kt @@ -27,7 +27,7 @@ class PushHandlingPlannerTest { title = null, body = "ignored", payload = payload(navigation = NavigationTrigger.CurrencyInfo(mint = TEST_MINT)), - silentSyncEnabled = false, + silentSyncEnabled = { false }, ) assertEquals(emptyList(), actions) } @@ -38,7 +38,7 @@ class PushHandlingPlannerTest { title = null, body = null, payload = payload(navigation = NavigationTrigger.Chat.ById(ChatId("aa01"))), - silentSyncEnabled = false, + silentSyncEnabled = { false }, ) assertEquals(emptyList(), actions) } @@ -50,7 +50,7 @@ class PushHandlingPlannerTest { @Test fun `currency info push updates tokens and posts`() { val p = payload(navigation = NavigationTrigger.CurrencyInfo(mint = TEST_MINT)) - val actions = planPushHandling("Title", "Body", p, silentSyncEnabled = false) + val actions = planPushHandling("Title", "Body", p, silentSyncEnabled = { false }) assertTrue(PushAction.UpdateTokens in actions) assertTrue(actions.last() is PushAction.PostNotification) } @@ -58,7 +58,7 @@ class PushHandlingPlannerTest { @Test fun `contact join push refreshes feed and syncs contacts`() { val p = payload(category = NotificationCategory.CONTACT_JOIN) - val actions = planPushHandling("Title", null, p, silentSyncEnabled = false) + val actions = planPushHandling("Title", null, p, silentSyncEnabled = { false }) assertTrue(PushAction.RefreshFeed in actions) assertTrue(PushAction.SyncContacts in actions) } @@ -67,16 +67,37 @@ class PushHandlingPlannerTest { fun `chat push refreshes feed then loads that chat`() { val chatId = ChatId("aa07") val p = payload(navigation = NavigationTrigger.Chat.ById(chatId)) - val actions = planPushHandling("Title", "Body", p, silentSyncEnabled = false) + val actions = planPushHandling("Title", "Body", p, silentSyncEnabled = { false }) assertEquals( listOf(PushAction.RefreshFeed, PushAction.LoadMessages(chatId)), actions.filterNot { it is PushAction.PostNotification }, ) } + @Test + fun `a contact join that also names a chat refreshes the feed once`() { + val chatId = ChatId("aa11") + val p = payload( + navigation = NavigationTrigger.Chat.ById(chatId), + category = NotificationCategory.CONTACT_JOIN, + ) + val actions = planPushHandling("Title", null, p, silentSyncEnabled = { false }) + assertEquals( + listOf(PushAction.RefreshFeed, PushAction.SyncContacts, PushAction.LoadMessages(chatId)), + actions.filterNot { it is PushAction.PostNotification }, + ) + } + + @Test + fun `a category with no sync of its own plans nothing`() { + val p = payload(category = NotificationCategory.GAIN) + val actions = planPushHandling("Title", null, p, silentSyncEnabled = { false }) + assertEquals(listOf(PushAction.PostNotification("Title", null, p)), actions) + } + @Test fun `push with no payload still posts the notification`() { - val actions = planPushHandling("Title", "Body", payload = null, silentSyncEnabled = false) + val actions = planPushHandling("Title", "Body", payload = null, silentSyncEnabled = { false }) assertEquals( listOf(PushAction.PostNotification("Title", "Body", null)), actions, @@ -86,7 +107,7 @@ class PushHandlingPlannerTest { @Test fun `post notification is always last so sync starts first`() { val p = payload(navigation = NavigationTrigger.Chat.ById(ChatId("0c"))) - val actions = planPushHandling("Title", "Body", p, silentSyncEnabled = false) + val actions = planPushHandling("Title", "Body", p, silentSyncEnabled = { false }) assertTrue(actions.last() is PushAction.PostNotification) } @@ -101,7 +122,7 @@ class PushHandlingPlannerTest { title = null, body = null, payload = payload(navigation = NavigationTrigger.Chat.ById(chatId)), - silentSyncEnabled = true, + silentSyncEnabled = { true }, ) assertEquals(listOf(PushAction.RefreshFeed, PushAction.LoadMessages(chatId)), actions) } @@ -112,7 +133,7 @@ class PushHandlingPlannerTest { title = null, body = null, payload = payload(navigation = NavigationTrigger.CurrencyInfo(mint = TEST_MINT)), - silentSyncEnabled = true, + silentSyncEnabled = { true }, ) assertTrue(actions.none { it is PushAction.PostNotification }) assertEquals(listOf(PushAction.UpdateTokens), actions) @@ -120,7 +141,7 @@ class PushHandlingPlannerTest { @Test fun `silent push with no payload does nothing`() { - val actions = planPushHandling(null, null, payload = null, silentSyncEnabled = true) + val actions = planPushHandling(null, null, payload = null, silentSyncEnabled = { true }) assertEquals(emptyList(), actions) } @@ -128,9 +149,21 @@ class PushHandlingPlannerTest { fun `enabling silent sync does not change a titled push`() { val p = payload(navigation = NavigationTrigger.Chat.ById(ChatId("0c"))) assertEquals( - planPushHandling("Title", "Body", p, silentSyncEnabled = false), - planPushHandling("Title", "Body", p, silentSyncEnabled = true), + planPushHandling("Title", "Body", p, silentSyncEnabled = { false }), + planPushHandling("Title", "Body", p, silentSyncEnabled = { true }), + ) + } + + @Test + fun `a titled push never reads the silent sync flag`() { + var reads = 0 + planPushHandling( + title = "Title", + body = "Body", + payload = payload(navigation = NavigationTrigger.Chat.ById(ChatId("aa13"))), + silentSyncEnabled = { reads++; true }, ) + assertEquals(0, reads) } // endregion From 5ddb3756bc4dd3052307d52b8aea37434f1caec2 Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Thu, 10 Sep 2026 15:15:34 -0400 Subject: [PATCH 09/24] fix(notifications): only read the silent-sync flag when it can change the outcome MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit FeatureFlagController.get is suspend over DataStore, and the call site wrapped it in runBlocking on the FCM dispatch thread. Every push paid that read, including the visible ones, which are all of production traffic today — and a visible push posts its notification and plans its sync whatever the flag says. Passing the read as a lambda keeps the rule in planPushHandling, which is the only place that knows a title means the flag is irrelevant. A test asserts the lambda is never invoked for a titled push, so the property is pinned rather than implied by reading the branch. Also rewords the spike_seq comment, since the correlation key lands with the harness rather than staying on the spike branch. --- .../flipcash/app/notifications/NotificationService.kt | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/apps/flipcash/shared/notifications/src/main/kotlin/com/flipcash/app/notifications/NotificationService.kt b/apps/flipcash/shared/notifications/src/main/kotlin/com/flipcash/app/notifications/NotificationService.kt index 261f10ede..1b8381c96 100644 --- a/apps/flipcash/shared/notifications/src/main/kotlin/com/flipcash/app/notifications/NotificationService.kt +++ b/apps/flipcash/shared/notifications/src/main/kotlin/com/flipcash/app/notifications/NotificationService.kt @@ -71,9 +71,10 @@ class NotificationService : FirebaseMessagingService(), private const val KEY_BODY = "push_notification_body" private const val KEY_PAYLOAD = "flipcash_payload" - // Spike-only correlation id. The bucket matrix sends a known sequence - // number with every push so a missing delivery and a late delivery can - // be told apart in the log; nothing in production sets it. + // Correlation id for measurement runs. scripts/spike/ sends a known + // sequence number with every push so a missing delivery and a late one + // can be told apart in the log rather than reading as the same silence. + // No production sender sets it, so `seq` is empty in the field. private const val KEY_SPIKE_SEQ = "spike_seq" // Upper bound on how long we'll wait for a remote avatar before posting @@ -154,7 +155,7 @@ class NotificationService : FirebaseMessagingService(), title = title, body = body, payload = payload, - silentSyncEnabled = runBlocking { featureFlags.get(FeatureFlag.PushSilentSync) }, + silentSyncEnabled = { runBlocking { featureFlags.get(FeatureFlag.PushSilentSync) } }, ) val latencyMs = System.currentTimeMillis() - message.sentTime From be0e48d3190bf5a71a0cf5d91a301a4bdd8a7ab9 Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Thu, 10 Sep 2026 15:15:56 -0400 Subject: [PATCH 10/24] chore(spike): add the push delivery measurement harness scripts/spike/run-bucket-matrix.sh drives one cell of the standby-bucket matrix: put the app in a bucket, optionally force or wait out Doze, send N high-priority pushes at a fixed interval with a sequence number in each, and capture logcat plus the power state around them. parse-bucket-log.py turns the capture into per-push delivery latency and the actions the planner chose; standby-bucket-probe.sh reads the bucket without running a cell. It lands with the feature rather than being deleted with the branch because the next delivery question needs the same instrument, and because the results document cites these scripts by path for how each number was produced. Raw captures stay out of the repo: a full device logcat carries unrelated personal content, so docs/spikes/raw/ is gitignored and only the summarised results are committed. maestro/spike_login.yaml gets the device back to a logged-in state after the data wipe a bucket reset needs. --- .gitignore | 4 + maestro/spike_login.yaml | 47 ++++ scripts/spike/parse-bucket-log.py | 154 +++++++++++++ scripts/spike/run-bucket-matrix.sh | 312 ++++++++++++++++++++++++++ scripts/spike/standby-bucket-probe.sh | 27 +++ 5 files changed, 544 insertions(+) create mode 100644 maestro/spike_login.yaml create mode 100755 scripts/spike/parse-bucket-log.py create mode 100755 scripts/spike/run-bucket-matrix.sh create mode 100755 scripts/spike/standby-bucket-probe.sh diff --git a/.gitignore b/.gitignore index 14409355f..875a5863c 100644 --- a/.gitignore +++ b/.gitignore @@ -43,3 +43,7 @@ maestro/screenshots/results/ maestro/screenshots/diffs/ docs/superpowers/ + +# Raw spike capture: full device logcat, so it can carry unrelated personal +# content. Keep the summarised results, not the source logs. +docs/spikes/raw/ diff --git a/maestro/spike_login.yaml b/maestro/spike_login.yaml new file mode 100644 index 000000000..e3042eb83 --- /dev/null +++ b/maestro/spike_login.yaml @@ -0,0 +1,47 @@ +appId: com.flipcash.app.android +name: "Spike: log in with PushSilentSync enabled" +tags: + - spike +--- +# Login plus flag enablement in one launch, so the flag is set in onCreate +# and no relaunch is needed to make it stick. Run with: +# BETA_FLAGS=push_silent_sync_enabled DEVICE= maestro/run.sh maestro/spike_login.yaml +# +# The device must be unlocked. A locked screen lets launchApp succeed behind +# the keyguard, so the failure surfaces later as "Element not found: Log in". +# +# This inlines subflows/login.yaml instead of calling it, because a real device +# with a Google account raises a Password Manager autofill sheet over the seed +# screen that the CI emulator never shows. The "No thanks" taps are optional so +# the flow still runs where no sheet appears. +- clearState +- launchApp: + arguments: + isUiTest: true + betaFlags: ${BETA_FLAGS} +- extendedWaitUntil: + visible: "Log in" + timeout: 30000 +- tapOn: "Log in" +- tapOn: + text: "No thanks" + optional: true +- extendedWaitUntil: + visible: + id: seed_input_screen + timeout: 20000 +- tapOn: + id: "seed_input_field" +- tapOn: + text: "No thanks" + optional: true +- inputText: ${SEED_PHRASE} +- tapOn: + text: "(?i)Log ?In" +- tapOn: + text: "No thanks" + optional: true +- extendedWaitUntil: + visible: + id: "wallet_screen" + timeout: 60000 diff --git a/scripts/spike/parse-bucket-log.py b/scripts/spike/parse-bucket-log.py new file mode 100755 index 000000000..506b913c1 --- /dev/null +++ b/scripts/spike/parse-bucket-log.py @@ -0,0 +1,154 @@ +#!/usr/bin/env python3 +"""Summarise one bucket-matrix run. + +Usage: scripts/spike/parse-bucket-log.py + +Joins the sends file against the onMessageReceived traces on the spike_seq id, +so a push that never arrived and a push that arrived late are different +outcomes rather than the same silence. Written against a trace line observed on +device, not a guessed format: + + 09-09 15:13:37.101 D/LoggingKt | trace (15895): onMessageReceived | \ +seq=active-silent-001, has_body=false, actions=0, silent=true, bucket=active, \ +latency_ms=-329, priority=1, original_priority=1 +""" +import datetime +import gzip +import re +import statistics +import sys + +TRACE = re.compile(r"onMessageReceived \| (.+)$") +FIELD = re.compile(r"(\w+)=([^,]+)") +# `adb logcat -v time` stamps lines "MM-DD HH:MM:SS.mmm" with no year. +STAMP = re.compile(r"^(\d{2})-(\d{2}) (\d{2}):(\d{2}):(\d{2})\.(\d{3})") + + +def read_log(path, year): + """Return (first trace per seq, epoch-ms of the last stamped line). + + One pass: the captures run to tens of MB, and a `<(gunzip -c ...)` argument + is a FIFO that cannot be read twice. + """ + opener = gzip.open if path.endswith(".gz") else open + seen = {} + last = None + with opener(path, "rt", errors="replace") as fh: + for line in fh: + stamp = STAMP.match(line) + if stamp: + last = stamp + m = TRACE.search(line) + if not m: + continue + fields = dict(FIELD.findall(m.group(1))) + seq = fields.get("seq", "") + if seq: + # First arrival wins; FCM can redeliver. + seen.setdefault(seq, fields) + return seen, stamp_to_epoch_ms(last, year) + + +def stamp_to_epoch_ms(match, year): + """Epoch-ms for one `adb logcat -v time` stamp, which carries no year. + + A run whose capture stops early makes the sends after that point look like + delivery failures. They are not measurements at all, so they have to be + separable from a push that was inside the window and never arrived. + """ + if not match: + return None + mo, d, h, mi, sec, ms = (int(g) for g in match.groups()) + try: + when = datetime.datetime(year, mo, d, h, mi, sec, ms * 1000) + except ValueError: # 02-29 against a non-leap year, etc. + return None + return int(when.timestamp() * 1000) + + +def pct(values, p): + if not values: + return None + ordered = sorted(values) + idx = min(len(ordered) - 1, int(round((p / 100) * (len(ordered) - 1)))) + return ordered[idx] + + +def main(): + if len(sys.argv) != 3: + sys.exit(__doc__) + log_path, sends_path = sys.argv[1], sys.argv[2] + + sent = [] + with open(sends_path) as fh: + for line in fh: + parts = line.split() + if len(parts) == 2: + sent.append((parts[1], int(parts[0]))) + + year = (datetime.datetime.fromtimestamp(sent[0][1] / 1000).year if sent + else datetime.datetime.now().year) + seen, end = read_log(log_path, year) + delivered = [q for q, _ in sent if q in seen] + # Only a send the capture was still running for can be called missing. + missing = [q for q, at in sent if q not in seen and (end is None or at <= end)] + unobserved = [q for q, at in sent if q not in seen and end is not None and at > end] + unsolicited = [q for q in seen if q not in dict(sent)] + + latencies = [] + downgraded = 0 + buckets = set() + for seq in delivered: + f = seen[seq] + try: + latencies.append(int(f["latency_ms"])) + except (KeyError, ValueError): + pass + if f.get("priority") != f.get("original_priority"): + downgraded += 1 + buckets.add(f.get("bucket", "?")) + + measured = len(delivered) + len(missing) + print(f"sent: {len(sent)}") + print(f"delivered: {len(delivered)}" + + (f" ({100 * len(delivered) / measured:.0f}% of {measured} measured)" if measured else "")) + print(f"missing: {len(missing)}" + + (f" {', '.join(missing)}" if missing else "")) + if unobserved: + print(f"unobserved: {len(unobserved)} {', '.join(unobserved)}" + " (capture ended before these were due — not delivery failures)") + if unsolicited: + print(f"unexpected: {len(unsolicited)} {', '.join(sorted(unsolicited))}") + print(f"bucket(s) reported by app: {', '.join(sorted(buckets)) or 'n/a'}") + print(f"priority downgraded: {downgraded}/{len(delivered)}") + + # Group by the bucket the app reported, not the one the run asked for. + # `am set-standby-bucket` is a request the OS re-evaluates mid-run, so the + # two diverge and only the reported one describes the data point. + by_bucket = {} + for seq in delivered: + f = seen[seq] + try: + by_bucket.setdefault(f.get("bucket", "?"), []).append(int(f["latency_ms"])) + except (KeyError, ValueError): + pass + if len(by_bucket) > 1: + floor = min(min(v) for v in by_bucket.values()) + print("per reported bucket, skew-corrected:") + for b in sorted(by_bucket): + v = sorted(x - floor for x in by_bucket[b]) + print(f" {b:12s} n={len(v):3d} median={v[len(v) // 2]}ms worst={v[-1]}ms") + + if latencies: + # latency_ms is device clock minus FCM sentTime, so it carries the + # device/server clock skew. A small negative floor is skew, not a push + # arriving before it was sent. + print(f"latency_ms min={min(latencies)} median={int(statistics.median(latencies))}" + f" p95={pct(latencies, 95)} max={max(latencies)}") + print(f" (skew floor {min(latencies)} ms; subtract it for a delivery-only figure)") + else: + print("latency_ms: none recorded") + + +if __name__ == "__main__": + main() diff --git a/scripts/spike/run-bucket-matrix.sh b/scripts/spike/run-bucket-matrix.sh new file mode 100755 index 000000000..6d288c951 --- /dev/null +++ b/scripts/spike/run-bucket-matrix.sh @@ -0,0 +1,312 @@ +#!/usr/bin/env bash +# Send data-only pushes into one standby bucket and record what arrives. +# +# Usage: scripts/spike/run-bucket-matrix.sh [count] [interval_s] [mode] +# bucket: active | working_set | frequent | rare | restricted +# count: pushes to send (default 20) +# interval_s: seconds between sends (default 180, so 20 sends span an hour) +# mode: silent (default, data-only, no title) | visible (control, with title) +# +# Each send carries a sequence id in the spike_seq data key, which the +# onMessageReceived trace logs as seq=. That is what makes a missing push +# distinguishable from a late one: correlation is per-push, not per-window. +# It is NOT carried in push_notification_body: the trace never logged the body +# (MetadataBuilder.to takes a non-null Any, so a String? resolves to kotlin.to +# and is discarded), and putting message text in a Bugsnag breadcrumb is a leak. +# +# Set DEVICE to an adb serial when more than one device is attached. +# +# DOZE=1 runs the cell under deep Doze on battery instead of the default +# charging, never-idle state. Doze is the one condition under which buckets are +# documented to withhold work, so a cell measured while charging cannot speak to +# it. The mode holds the device there for the whole run: +# - `dumpsys battery unplug` makes the framework see battery. The cable stays +# in, so the run does not depend on a charge level, but every power decision +# above the driver is taken as if unplugged. Reachability over Wi-Fi adb is +# therefore not required, though it is the more faithful setup. +# - `deviceidle force-idle` needs the screen off and the framework unplugged, +# which is why both precede it. +# - Idle is re-asserted before each send: a delivery can pull the device into +# a maintenance window, and a cell that silently left Doze after push 3 +# measures the same thing the charging cells already did. +# Both overrides are released on exit, including on failure. +# +# DOZE=natural waits for deep idle instead of forcing it. `force-idle` applies +# Doze's restriction set but skips the gating that normally precedes it — the +# screen-off timer, and the significant-motion detector that resets the whole +# countdown when the phone is picked up. A cell that reached idle on its own is +# the stronger claim, and it is the one the forced cell explicitly does not make. +# What the mode does differently: +# - Nothing is overridden. The device must be PHYSICALLY unplugged, which is +# why the mode requires adb over Wi-Fi and refuses to run on USB. +# - It polls `deviceidle get deep` until it reads IDLE, up to IDLE_TIMEOUT +# (default 2h) at IDLE_POLL intervals (default 60s), then starts sending. +# - Idle is NOT re-asserted between sends. A delivery that pulls the device +# into a maintenance window is the behaviour under test, not a defect in the +# cell, so the per-send `deep=` in the .power sidecar is the result rather +# than something the runner corrects. +# The phone must be left still for the whole run. Motion restarts Doze's +# countdown, and nothing in this script can see that happen — only the deep= +# samples will show it, after the fact. +# The mode tolerates the Wi-Fi link dropping, which it will: adb calls retry for +# ADB_GRACE seconds and logcat reattaches from where it stopped, so a reset +# connection costs an annotated gap in the capture rather than the cell. Three +# consecutive sends with the device unreachable still fail the run — at that +# point the pushes are going somewhere nothing is recording. +set -euo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +REPO="$(cd "$SCRIPT_DIR/../.." && pwd)" + +TOKEN="${1:?usage: run-bucket-matrix.sh [count] [interval_s] [mode]}" +BUCKET="${2:?bucket required}" +COUNT="${3:-20}" +INTERVAL="${4:-180}" +MODE="${5:-silent}" +# Seconds to keep the log open after the last send, to catch stragglers. +# Lower it only for a smoke test; a real bucket run needs the full window. +HOLD="${HOLD:-300}" +# 0 (charging, never idle) | 1 (forced deep idle) | natural (idle reached) +DOZE="${DOZE:-0}" +# DOZE=natural only: how long to wait for deep idle, and how often to look. +# Each poll is an adb round trip, which nudges the device awake, so keep the +# cadence coarse — the wait is measured in tens of minutes either way. +IDLE_TIMEOUT="${IDLE_TIMEOUT:-7200}" +IDLE_POLL="${IDLE_POLL:-60}" + +# PAYLOAD is a base64 flipcash.push.v1.Payload, sent as the flipcash_payload +# data key. Without it every cell measures delivery and nothing else, because +# planPushHandling derives all sync work from that payload and returns an empty +# action list when it is absent — which is what the first seven cells did, all +# of them recording actions=0 in the trace they were read from. The two-byte +# encoding `IAU=` is category=CONTACT_JOIN and nothing else, which plans +# RefreshFeed + SyncContacts without naming a chat or a contact. +# +# Note the flag interaction: planPushHandling consults PushSilentSync only when +# the title is null, so MODE=silent measures the flag and the sync path +# together, while MODE=visible exercises the sync path whatever the flag says. +PAYLOAD="${PAYLOAD:-}" + +PACKAGE="com.flipcash.app.android" +# Target one device via ANDROID_SERIAL rather than an adb wrapper function: +# wrapping adb makes $! the wrapper subshell's pid, so the later kill misses the +# real logcat, which then keeps the script's stdout pipe open forever. +if [ -n "${DEVICE:-}" ]; then + export ANDROID_SERIAL="$DEVICE" +fi +STAMP="$(date +%Y%m%dT%H%M%S)" +OUT_DIR="$REPO/docs/spikes/raw" +# A natural-idle cell is marked in the filename. The forced and charging cells +# keep the original naming so the captures already committed still match it. +SUFFIX="" +[ "$DOZE" = "natural" ] && SUFFIX="-natdoze" +LOG="$OUT_DIR/${BUCKET}-${MODE}${SUFFIX}-${STAMP}.log" +SENDS="$OUT_DIR/${BUCKET}-${MODE}${SUFFIX}-${STAMP}.sends" +POWER="$OUT_DIR/${BUCKET}-${MODE}${SUFFIX}-${STAMP}.power" +mkdir -p "$OUT_DIR" + +# Fail before the run rather than after it. A disconnected device makes every +# adb call print "device not found" to stderr and carry on, which once produced +# three cells that reported success against no device at all. +require_device() { + adb shell true >/dev/null 2>&1 \ + || { echo "FATAL: no device (${ANDROID_SERIAL:-default}) — nothing was measured" >&2; exit 1; } +} +require_device + +MODEL="$(adb shell getprop ro.product.model | tr -d '\r')" +RELEASE="$(adb shell getprop ro.build.version.release | tr -d '\r')" +SDK="$(adb shell getprop ro.build.version.sdk | tr -d '\r')" + +# Wi-Fi adb drops, and a drop is not a result. Over screen-off Doze the phone +# resets its adbd connection: TCP 5555 stays open and the device re-authorizes +# within seconds, but every adb call issued in that window returns "device +# offline", and under set -e that took the first natural-Doze cell down at send +# 5 of 20 with the device still sitting in unforced deep idle. So one-shot adb +# calls retry, and only a device that stays unreachable for ADB_GRACE seconds is +# treated as gone. +ADB_GRACE="${ADB_GRACE:-180}" +adb_try() { + local deadline=$((SECONDS + ADB_GRACE)) out + while :; do + if out="$(adb "$@" 2>/dev/null)"; then printf '%s' "$out"; return 0; fi + [ "$SECONDS" -ge "$deadline" ] && return 1 + adb reconnect >/dev/null 2>&1 || true + sleep 5 + done +} + +# logcat dies with the connection too, and it is the capture — a cell whose log +# stops at send 5 reads as fifteen undelivered pushes. Reattach from the last +# stamped line rather than re-dumping the buffer, so the outage window is +# recovered; the parse step keeps the first trace per seq, so the overlap that +# creates is discarded rather than counted twice. Each gap is marked in the log. +logcat_forever() { + local last="" + while :; do + if [ -n "$last" ]; then + adb logcat -v time -T "$last" >> "$LOG" 2>/dev/null & + else + adb logcat -v time >> "$LOG" 2>/dev/null & + fi + echo $! > "$LOGCAT_CHILD" + wait $! 2>/dev/null || true + echo "# logcat detached $(date +%s000)" >> "$LOG" + last="$(grep -oE '^[0-9]{2}-[0-9]{2} [0-9]{2}:[0-9]{2}:[0-9]{2}\.[0-9]{3}' "$LOG" | tail -1)" + sleep 5 + adb_try shell true >/dev/null || continue + echo "# logcat reattached $(date +%s000)" >> "$LOG" + done +} + +# Power state is sampled, not assumed. The first six cells were all written up +# as "charging, therefore Doze-exempt" on the strength of spot checks taken +# outside the capture; a reader had to take that on trust. These fields make the +# claim checkable from the artefacts alone, and catch a run that drifted out of +# the state it was supposed to be measuring. +power_sample() { + local label="$1" + local plugged deep light bucket + if ! plugged="$(adb_try shell dumpsys battery | awk -F': ' ' + /AC powered/ {ac=$2} + /USB powered/ {usb=$2} + /Wireless powered/ {wl=$2} + END {printf "ac=%s,usb=%s,wireless=%s", ac, usb, wl}' | tr -d '\r')"; then + echo "$(date +%s000) ${label} unreachable" + return 1 + fi + deep="$(adb_try shell dumpsys deviceidle get deep | tr -d '\r')" || deep="?" + light="$(adb_try shell dumpsys deviceidle get light | tr -d '\r')" || light="?" + bucket="$(adb_try shell am get-standby-bucket "$PACKAGE" | tr -d '\r')" || bucket="?" + echo "$(date +%s000) ${label} plugged=[${plugged}] deep=${deep} light=${light} bucket=${bucket}" +} + +release_overrides() { + if [ "$DOZE" = "1" ]; then + adb shell dumpsys deviceidle unforce >/dev/null 2>&1 || true + adb shell dumpsys battery reset >/dev/null 2>&1 || true + fi +} + +adb shell am set-standby-bucket "$PACKAGE" "$BUCKET" +echo "bucket code now: $(adb shell am get-standby-bucket "$PACKAGE" | tr -d '\r')" +echo "device: $MODEL, Android $RELEASE (API $SDK)" + +# Background the app and let the system settle before measuring. Skipped under +# DOZE=natural: HOME is a wake key, so on a screen-off device it turns the +# display on and restarts the idle countdown the mode is waiting on — and a +# device that is already asleep has already backgrounded the app. +if [ "$DOZE" != "natural" ]; then + adb shell input keyevent KEYCODE_HOME + sleep 5 +fi + +if [ "$DOZE" = "1" ]; then + trap 'release_overrides' EXIT + adb shell input keyevent KEYCODE_SLEEP + sleep 3 + adb shell dumpsys battery unplug >/dev/null + adb shell dumpsys deviceidle force-idle >/dev/null + DEEP="$(adb shell dumpsys deviceidle get deep | tr -d '\r')" + [ "$DEEP" = "IDLE" ] \ + || { echo "FATAL: deep idle is $DEEP, not IDLE — the cell would measure the charging case again" >&2; exit 1; } + echo "deep idle: $DEEP (framework sees battery; cable may stay in)" +elif [ "$DOZE" = "natural" ]; then + # A USB link is a charger. Refuse rather than quietly measure the plugged + # case, which is exactly how the first six cells came to be Doze-exempt. + case "${ANDROID_SERIAL:-}" in + *:*) : ;; + *) echo "FATAL: DOZE=natural needs adb over Wi-Fi — set DEVICE to host:port" >&2; exit 1 ;; + esac + PLUGGED="$(adb shell dumpsys battery | awk -F': ' ' + /AC powered/{ac=$2} /USB powered/{usb=$2} /Wireless powered/{wl=$2} + END {print ac usb wl}' | tr -d '\r')" + case "$PLUGGED" in + *true*) echo "FATAL: device is still on a charger ($PLUGGED) — unplug it; Doze will not start" >&2; exit 1 ;; + esac + adb shell input keyevent KEYCODE_SLEEP + echo "waiting for deep idle (up to ${IDLE_TIMEOUT}s, polling every ${IDLE_POLL}s)" + echo "leave the phone still — motion restarts the countdown" + WAITED=0 + while :; do + DEEP="$(adb shell dumpsys deviceidle get deep 2>/dev/null | tr -d '\r')" + [ "$DEEP" = "IDLE" ] && break + [ "$WAITED" -ge "$IDLE_TIMEOUT" ] \ + && { echo "FATAL: still $DEEP after ${WAITED}s — nothing was measured" >&2; exit 1; } + echo " ${WAITED}s: deep=$DEEP" + sleep "$IDLE_POLL" + WAITED=$((WAITED + IDLE_POLL)) + done + echo "deep idle reached after ${WAITED}s, unforced, on battery" +fi + +adb logcat -c +{ + echo "# device: $MODEL, Android $RELEASE (API $SDK)" + echo "# bucket: $BUCKET mode: $MODE count: $COUNT interval: ${INTERVAL}s hold: ${HOLD}s doze: $DOZE" + echo "# power: $(power_sample start)" +} > "$LOG" +LOGCAT_CHILD="$(mktemp -t bucketlogcat)" +logcat_forever & +LOGCAT_PID=$! +# Kill the supervisor first, or it respawns the child it is watching. +stop_logcat() { + kill "$LOGCAT_PID" 2>/dev/null || true + [ -s "$LOGCAT_CHILD" ] && kill "$(cat "$LOGCAT_CHILD")" 2>/dev/null || true + rm -f "$LOGCAT_CHILD" +} +if [ "$DOZE" = "1" ]; then + trap 'stop_logcat; release_overrides' EXIT +else + trap 'stop_logcat' EXIT +fi + +: > "$SENDS" +: > "$POWER" +power_sample start >> "$POWER" +MISSES=0 +for i in $(seq 1 "$COUNT"); do + SEQ=$(printf "%s-%s-%03d" "$BUCKET" "$MODE" "$i") + if [ "$MODE" = "visible" ]; then + DATA=$(jq -n --arg s "$SEQ" \ + '{spike_seq: $s, push_notification_title: "Spike", push_notification_body: $s}') + else + DATA=$(jq -n --arg s "$SEQ" '{spike_seq: $s}') + fi + if [ -n "$PAYLOAD" ]; then + DATA=$(jq -n --argjson d "$DATA" --arg p "$PAYLOAD" '$d + {flipcash_payload: $p}') + fi + if [ "$DOZE" = "1" ]; then + # Re-assert rather than assert-once: step-idle can advance to a + # maintenance window on its own, and force-idle is idempotent. + adb shell dumpsys deviceidle force-idle >/dev/null 2>&1 || true + fi + if power_sample "pre-$SEQ" >> "$POWER"; then + MISSES=0 + else + MISSES=$((MISSES + 1)) + echo "device unreachable at $SEQ (${MISSES} in a row)" >&2 + [ "$MISSES" -ge 3 ] && { + echo "FATAL: device unreachable across $MISSES consecutive sends — this cell is incomplete" >&2 + exit 1 + } + fi + echo "$(date +%s000) $SEQ" >> "$SENDS" + "$REPO/scripts/fcm.sh" "$TOKEN" "$DATA" > /dev/null 2>&1 \ + && echo "sent $SEQ" || echo "SEND FAILED $SEQ" + [ "$i" -lt "$COUNT" ] && sleep "$INTERVAL" +done + +echo "sends done; holding ${HOLD}s for stragglers" +sleep "$HOLD" +power_sample end >> "$POWER" +kill $LOGCAT_PID 2>/dev/null || true + +# The device can vanish mid-run — a cable, a reboot, a sleeping host. Say so, +# because the log then just stops and the cell reads as a partial delivery +# failure instead of an interrupted capture. +adb_try shell true >/dev/null \ + || { echo "FATAL: device disappeared during the run — this cell is incomplete" >&2; exit 1; } +echo "log: $LOG" +echo "sends: $SENDS" +echo "power: $POWER" diff --git a/scripts/spike/standby-bucket-probe.sh b/scripts/spike/standby-bucket-probe.sh new file mode 100755 index 000000000..827ff4046 --- /dev/null +++ b/scripts/spike/standby-bucket-probe.sh @@ -0,0 +1,27 @@ +#!/usr/bin/env bash +# Force the app into a standby bucket, then watch how the next pushes land. +# +# Usage: scripts/spike/standby-bucket-probe.sh +# bucket: active | working_set | frequent | rare | restricted +# +# Send the pushes themselves from the server or the FCM console while this +# runs. It does not send them; it only controls the variable and records +# the result. +set -euo pipefail + +PACKAGE="com.flipcash.app.android" +BUCKET="${1:?usage: standby-bucket-probe.sh }" +OUT="docs/spikes/raw/${BUCKET}-$(date +%Y%m%dT%H%M%S).log" + +mkdir -p "$(dirname "$OUT")" + +adb shell am set-standby-bucket "$PACKAGE" "$BUCKET" +echo "bucket now: $(adb shell am get-standby-bucket "$PACKAGE")" + +# Background the app and let the system settle before measuring. +adb shell input keyevent KEYCODE_HOME +sleep 5 + +adb logcat -c +echo "recording to $OUT — send test pushes now, Ctrl-C when done" +adb logcat -v time | grep --line-buffered "onMessageReceived" | tee "$OUT" From 02f2d3a250d4c3d70c956334cce3f16a1dc3b874 Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Thu, 10 Sep 2026 15:16:07 -0400 Subject: [PATCH 11/24] docs(spikes): add the standby-bucket delivery results 139 of 139 high-priority pushes were delivered across every bucket including restricted, with no priority downgrades, so bucket is not the delivery risk the design assumed. What the matrix did surface is a CPU one: three of the runs ended in ActivityManager killing the process for exceeding 2% CPU over a 300 s window while cached, and regressing the kills against the log volume inside their own windows puts one push at roughly 6.7 s of CPU. The document is the evidence for both halves of the push-preload design, and it cites scripts/spike/ for how each figure was produced, so it lands in the same change as the harness and the planner it describes. --- .../2026-09-08-standby-bucket-results.md | 611 ++++++++++++++++++ 1 file changed, 611 insertions(+) create mode 100644 docs/spikes/2026-09-08-standby-bucket-results.md diff --git a/docs/spikes/2026-09-08-standby-bucket-results.md b/docs/spikes/2026-09-08-standby-bucket-results.md new file mode 100644 index 000000000..c7d962b2d --- /dev/null +++ b/docs/spikes/2026-09-08-standby-bucket-results.md @@ -0,0 +1,611 @@ +# Spike results: do App Standby Buckets delay or drop silent pushes? + +Plan: `docs/superpowers/plans/` in the orchestrator repo. +Spec: `docs/superpowers/specs/2026-09-08-push-preload-research.md`. +Raw traces and send manifests: `docs/spikes/raw/` — gitignored and local to the machine +that ran the matrix, because the captures are ~41 MB of logcat carrying unrelated device +chatter. Everything the conclusions rest on is quoted here. + +**Status: all six charging cells collected, plus the deep-Doze cell they were missing.** +Device `SM02G4061915766`, model `Seeker`, Android 16 (API 36), throughout. + +**Correction, 2026-09-10: every cell below measured delivery and nothing else.** The runner sent +`{spike_seq: …}` with no `flipcash_payload`, and the app derives all push-triggered work from that +key, so no cell exercised the push handling path — the trace reads `actions=0` on all 139 sends. +The delivery and latency figures are unaffected. Every claim about work is restated or withdrawn +below; see [The matrix never exercised the push handling +path](#the-matrix-never-exercised-the-push-handling-path). + +Each cell: 20 high-priority pushes, 180 s apart, then a 300 s straggler hold. Latency comes +from `onMessageReceived`'s trace line, joined to the send manifest on `spike_seq`, so a push +that never arrived and a push that arrived late are different outcomes rather than the same +silence. + +--- + +## Delivery + +**139 observable sends, 139 delivered, 0 dropped, 0 priority downgrades.** + +| Cell | Delivered | Priority downgraded | Bucket the app actually saw | +|---|---|---|---| +| `active` / silent | 20 / 20 | 0 | `active` for 4 sends, then `working_set` | +| `working_set` / silent | 19 / 19 observable | 0 | `working_set` throughout | +| `frequent` / silent | 20 / 20 | 0 | `frequent` throughout | +| `rare` / silent | 20 / 20 | 0 | `rare` throughout | +| `restricted` / silent | 20 / 20 | 0 | `restricted` throughout | +| `active` / visible | 20 / 20 | 0 | `active` throughout | +| `restricted` / silent, deep Doze | 20 / 20 | 0 | `restricted` throughout | + +The `working_set` run's 20th send is not a miss. Logcat capture stopped at 18:44:02 and that +push was not due until 18:44:10 — the window was never observed, so it is excluded rather +than counted as a drop. + +A seventh capture exists, `active-silent-20260909T151759.log.gz`, holding three sends from an +aborted first attempt. It is excluded from every figure here. + +## Latency + +The device clock runs behind the host that timestamps the sends, which puts a floor of about +−330 ms under every reading. Each run is corrected against its own floor (−322 to −397 ms +across the seven), which is what `parse-bucket-log.py` does for a single run; earlier revisions +of this document used one global floor and so quoted figures a few ms off these. + +The floor is each run's own fastest sample, which is a noisy estimator of the true skew. Two +runs whose raw distributions match can end up tens of milliseconds apart after correction +purely because one happened to catch a faster outlier. Differences of that size between rows +should not be read as real. + +| Bucket the app reported | n | median | p95 | worst | +|---|---|---|---|---| +| `restricted` | 20 | 61 ms | 233 ms | 444 ms | +| `active` | 24 | 96 ms | 203 ms | 116 s (see below) | +| `working_set` | 35 | 104 ms | 322 ms | 543 ms | +| `frequent` | 20 | 122 ms | 545 ms | 749 ms | +| `rare` | 20 | 138 ms | 529 ms | 836 ms | +| `restricted`, deep Doze | 20 | 131 ms | 316 ms | 317 ms | + +The first five rows are charging cells, grouped by the bucket the app reported rather than the +one the run requested — see below. The `active` row mixes 4 silent observations with 20 visible +ones; every other row is silent only. The last row is the deep-Doze cell, kept separate because +its power state differs. + +**Bucket depth did not gate delivery.** The most throttled bucket Android has, `restricted`, +delivered all 20 and posted the lowest median in the matrix. A 61-to-138 ms spread across five +buckets, with per-bucket p95s that overlap each other, is not a bucket effect — it is the +noise floor of the measurement. High priority is documented to bypass Doze and bucket +deferral, and on this device it does. + +## A visible notification holds `active`; a silent push does not + +`am set-standby-bucket active` is a request the OS re-evaluates, not a latch. In the silent +cell the runner re-asserted it every 45 seconds and the app still reported `working_set` from +the fifth push onward — roughly twelve minutes in. + +The visible cell ran the same pinner against the same app and held `active` for all 20 sends, +just over an hour. The difference between the two runs is the notification: a push the user +can see is itself a signal that keeps the app in `active`, and a silent one is not. + +Two consequences: + +- **The silent `active` cell has n=4, not n=20.** Sixteen of its sends are `working_set` + observations wearing the wrong label, which is why they are counted under `working_set` + above. +- **Any measurement that assumes a pinned bucket is measuring something else.** The per-send + bucket label in the trace is what makes this visible; without it the run would have produced + twenty clean-looking `active` rows that were mostly not active. + +Analysis has to key on the reported bucket rather than the requested one. This run's parser +does. + +## One 116-second delivery, unexplained + +`active-visible-001` was sent at 21:53:26 and reached `onMessageReceived` at 21:55:23.568, +which the app's own trace records as `latency_ms=115608`. Every other send in the matrix +landed inside 850 ms. + +Two candidate explanations were checked against the capture and both fail: + +- **Not the app freezer.** The delivery is preceded by + `sync unfroze 6266 com.flipcash.app.android for 3`, but that line precedes *every* delivery + in the cell — the process is frozen between pushes and unfrozen to receive each one. The + other 19 unfreezes cost ~100 ms. +- **Not an FCM retry.** GMS arms a `FcmRetry` alarm alongside this delivery, but it does so on + every delivery in every cell (29 to 45 per capture), so its presence says nothing about this + message. + +What is distinctive is position: this was the first push of a cell, sent six seconds after the +cell started and one second after logcat began recording. But the first pushes of the +`frequent` and `restricted` cells cost 384 ms and −122 ms, so "first of a cell" is not +sufficient on its own. + +It stays on the record as one unexplained outlier in 119, not averaged away. A 116-second +delay is past the point where preloaded content would still be waiting when the user opens the +app, so if it recurs on battery it stops being a curiosity. + +## The first six cells were measured on a charging device + +`dumpsys battery` reported USB powered, 100%, and `dumpsys deviceidle get deep` reported +`ACTIVE` — deep Doze never engaged for any of the six. + +That was built into the harness rather than overlooked: the runner drove the device over USB +adb, and a device on USB is charging and therefore Doze-exempt. So those six describe delivery +to a plugged-in phone, which is close to the best case and not the case the preload design has +to survive. + +Two things closed the gap. `adb tcpip` took the runner off the cable — the connection needs an +`adb kill-server` after it, because the server holds stale state from the restart of `adbd` and +reports "No route to host" against a device that is plainly listening. And the runner now +samples power and idle state before every send into a `.power` sidecar, so the state a cell ran +in is a property of the capture rather than of a spot check taken beside it. + +## `restricted` under deep Doze, on battery + +**20 / 20 delivered, no priority downgrades, and the chat feed fetched successfully every +time.** `deep=IDLE` on all 20 pre-send samples; the only `IDLE_MAINTENANCE` reading is the +final one, taken after the straggler hold. The app reported `restricted` for all 20. + +Delivery timing is indistinguishable from the same cell on mains: + +| push → `onMessageReceived` | median | p95 | worst | +|---|---|---|---| +| deep Doze, battery | 131 ms | 316 ms | 317 ms | +| charging | 61 ms | 233 ms | 444 ms | + +Do not read the median gap as a Doze penalty. The two runs' *uncorrected* medians are −266 and +−267 ms — the same number — and the whole difference comes from their skew floors, −397 ms +against −328 ms. The tail moves the other way, with Doze's worst case 317 ms against 444 ms. +Both cells sit inside the noise of the matrix. + +**The push woke the process, and the process reached the network.** Waking a frozen process +proves nothing on its own if the network is still shut. On each of the 20 pushes the app brought +its gRPC channel from `CONNECTING` to `READY`, issued `GetDmChatFeedRequest`, and logged +`The request was processed successfully`: + +| push → first successful RPC | n | median | p95 | worst | +|---|---|---|---|---| +| deep Doze, battery | 20 | 373 ms | 455 ms | 521 ms | +| charging | 21 | 524 ms | 1208 ms | 1891 ms | + +So on this device, a `restricted`-bucket app in deep Doze on battery went from a silent push to +a completed chat-feed fetch in under 530 ms, worst case. + +**That is not the preload path, and an earlier revision of this document said it was.** The trace +reads `actions=0` on all 20 of these pushes, so no `PushAction` ran and no push handling took +place. The fetch is the event stream reconnecting once the push woke the process. What the cell +establishes is the precondition for a preload — a push in the most throttled bucket, under forced +deep idle, wakes the process, and the process can then reach the network — rather than the preload +itself. + +One error per push appears in both cells — `An error occurred while processing the request`, +about 20 s after delivery, following `EventStreamingController: Stream error: Event stream +timed out`. It occurs 20 times in the Doze cell and 21 times in the charging cell, once per +push in each, so it is the event stream's own reconnect behaviour and not something Doze did. + +### What this cell does not establish + +- **Forced idle is not naturally-entered Doze.** `deviceidle force-idle` applies the same + restriction set but skips the motion and screen gating that normally precedes it. A phone + that reached deep idle on its own, in a pocket, over hours, is a longer test than this one. + This has since been run — see the natural-idle cell below. Delivery held; the app's network + activity did not, though not for the reason first recorded here, since neither cell exercised + push handling at all. +- **The battery was unplugged at the framework level, not physically.** Every power decision + above the driver was taken as if on battery, which is where Doze decisions are made, but this + says nothing about behaviour at a low charge level or under thermal pressure. +- **Wi-Fi adb held a live socket to the device throughout**, so the radio was up for the whole + hour. A device with no debugger attached may let the radio idle harder. +- **One device, one OEM, one Android version.** Samsung's power management is its own; this + result should not be read as an Android-wide guarantee. + +## `restricted` under Doze the device entered on its own + +The forced cell's first caveat was that `force-idle` skips the gating real Doze applies. That +cell has now been re-run with nothing overridden: phone physically unplugged, screen off, left +untouched until the framework reached deep idle by itself, over Wi-Fi adb. All 22 power samples +read `plugged=[ac=false,usb=false,wireless=false]`, and `dumpsys deviceidle` reported +`mForceIdle=false` throughout. + +Raw: `raw/restricted-silent-natdoze-20260910T102101.{log,sends,power}`. + +**Delivery survives the stronger condition.** 20 sent, 20 delivered, none missing, none +downgraded, every one reporting `bucket=restricted`. + +**Latency does not.** Three consecutive pushes were held and then released together: + +| seq | sent at | held | delivered at | +|---|---|---|---| +| 011 | +1816s | 182 ms | +1815.8s | +| 012 | +1999s | 397.9s | +2396.5s | +| 013 | +2182s | 216.3s | +2397.9s | +| 014 | +2363s | 35.0s | +2397.6s | +| 015 | +2545s | 0 ms | +2544.6s | + +Three pushes sent three minutes apart arriving within 1.4 seconds of each other is a maintenance +window flushing a queue, not three independent delays. The `.power` sidecar agrees from the other +side: 21 of 22 samples read `deep=IDLE`, and the one that does not is the pre-send sample for 015, +which reads `deep=IDLE_MAINTENANCE`. + +Split on that boundary, against this run's own skew floor of −405 ms: + +| push → `onMessageReceived` | n | median | p95 | worst | +|---|---|---|---|---| +| between windows | 17 | 182 ms | 506 ms | 616 ms | +| held for a window | 3 | — | — | 397.9s | +| forced idle, for comparison | 20 | 131 ms | 316 ms | 317 ms | + +Between windows, natural Doze looks exactly like forced Doze. The difference is entirely the tail, +and forcing is what hid it: `DOZE=1` re-asserts `force-idle` before every send, which suppresses +the maintenance windows this cell exists to observe. The forced cell did not measure a device that +had no deferral; it measured a device that was not allowed to flush. + +### The app received all 20 and did no network work on any of them + +The forced cell's stronger claim was that the app completed a chat-feed fetch on every push. That +does not reproduce. Counting log lines from the app's own pid, the same process (6266) in both +cells: + +| tag | forced idle | natural idle | +|---|---|---| +| `LoggingKt \| trace` (deliveries) | 61 | 20 | +| `[RpcLogging]` | 314 | 0 | +| `[gRPC]` | 151 | 0 | +| `[event-streaming]` | 178 | 0 | +| `[BIDI]` | 60 | 0 | +| total app-pid lines | 4,957 | 391 | + +`onMessageReceived` ran 20 times. No gRPC channel was opened, no RPC was issued, and no RPC +completed — including for 015, which arrived *during* the maintenance window. + +**Neither cell asked the app to do anything.** Both read `actions=0` on every send, so the +difference between them is not one of push handling. What differs is that the forced capture's +event stream was alive and cycling — 41 `Stream error`, 15 `Event stream timed out` — and the +natural capture contains none of that machinery at all. + +**The silence is not network denial.** Denied network leaves failed attempts behind. Pid 6266 +logged no network activity of any kind across the hour: no attempt, no failure. The seven +`UNAVAILABLE` and four `NetworkException` lines in the capture belong to other processes — +location reporting, the sync service, Play services — and none to the app. It did not try. + +**The control does not settle it either.** That run was itself `actions=0`, so it tested nothing +about push handling; what it shows is that a push to an awake device wakes the process into doing +work, which was never the point in doubt. An earlier revision of this document offered it as proof +that the app was healthy and the absence was therefore Doze. It does not support that. Whether the +forced/natural difference is Doze or app state — an event stream alive at 00:30 and gone by 10:20 — +is not resolved by anything in these two captures. + +Incidentally the control prices the wake fan-out two parallel investigations have flagged: one +silent push cost eight successful RPCs, none of them asked for by the push. + +### The aborted first attempt + +`raw/restricted-silent-natdoze-20260910T095802.*` is a 5-send capture from the same condition, +kept because it is valid as far as it goes: 5/5 delivered, all five pre-send samples on battery at +`deep=IDLE`. It ended at send 5 when Wi-Fi adb dropped the session and `set -e` took the runner +down — the phone was still in unforced deep idle when it died. The harness now survives that; see +the defects section. + +## The matrix never exercised the push handling path + +Every trace line the matrix produced carries an `actions=` field, and across all seven cells it +reads `actions=0`. 139 sends, no exceptions, plus the awake control run afterwards. + +`NotificationService.onMessageReceived` derives every unit of work from the `flipcash_payload` +data key. `planPushHandling` hands that to `syncActionsFor`, which returns an empty list when the +payload is null (`PushHandlingPlanner.kt:36`), and `onMessageReceived` returns before dispatching +anything. The runner sent `{spike_seq: …}` and nothing else, so the payload was null on every push +in every cell. + +The matrix was built to tell a missing push from a late one, and it does that correctly — the +`spike_seq` correlation, the delivery counts and the latency figures are all unaffected. What it +cannot speak to is what the app does with a push, because it never sent one the app was built to +act on. The field naming that miss was in every line being read. + +Three consequences, two for the cells above and one for the design. + +**The forced cell's RPCs were real, and were not the preload.** They are locked to the sends: 40 +successful RPCs, exactly two per push, each pair 450–650 ms after its own push trace, at the 180 s +send cadence. That is not background noise, and calling it ambient would be wrong. But with +`actions=0` no `PushAction` ran, so the fetch came from the event stream reconnecting once the +push woke the process — a wake effect, not push handling. + +**The natural cell's silence was an app that was never asked.** Not a network it was denied: see +the section above for why the two look different in the capture. + +**Silent preload is gated on a flag that ships off.** `planPushHandling` consults `PushSilentSync` +only when the title is null, and the flag's default is `false` (`FeatureFlag.kt:123`). A data-only +push does nothing on a default build, however well formed its payload. The flag is on for the +device under test — established by sending one silent push carrying a payload and reading +`actions=2` back, against `actions=0` for all 139 committed sends. + +This also constrains how the remaining cells can be run. A visible push cannot measure Doze: +posting the notification lights the screen, and screen-on ends deep idle. One visible smoke push +took the device from `IDLE` to `ACTIVE`. So a Doze cell has to be silent, which makes it depend on +`PushSilentSync` being on — the flag is part of the measurement setup, not an incidental detail. + +## Re-run with a payload the app acts on — four samples, then the phone went away + +`raw/restricted-silent-natdoze-20260910T113729.*`. Twelve silent pushes were planned, 180 s apart, +`restricted`, natural deep idle on battery, carrying `flipcash_payload=IAU=` — a two-byte +`flipcash.push.v1.Payload` holding `category=CONTACT_JOIN` and nothing else, which plans +`RefreshFeed` + `SyncContacts` without naming a chat or a contact. + +**The cell aborted at send seven.** Wireless adb went unreachable at sends five, six and seven, and +the runner's three-consecutive-failure guard stopped it. Sends five and six went out but were never +observed, so the traced sample is four. + +**All four handled pushes behaved identically, and the handler completed.** + +| Reading | Value | +|---|---| +| Traced pushes | 4, all `actions=2`, all pid 6266 | +| Power state at every sample | `deep=IDLE`, 5 of 5 | +| `GetDmChatFeedRequest` | 8 — two per push | +| `The request was processed successfully` | 8 — two per push | +| Contact RPCs | 8 — two per push, all `UNAVAILABLE: contact list service disabled` | +| Trace line to both feed RPCs complete | 0.51–0.65 s | + +The contact failures are not network failures. `contact list service disabled` is an application +answer from the backend, which means the RPC left the device, reached the server and came back. So +both halves of the planned action list ran: `RefreshFeed` fetched, and `SyncContacts` was refused +for a reason that has nothing to do with Doze. + +**What four samples support:** the push handling path is not blocked under Doze the device entered +on its own, on a `restricted`-bucket app that is not on the Doze allowlist. That is the thing seven +earlier cells and 139 sends never tested. + +**What they do not support:** any rate. Four consecutive successes cannot distinguish "always works" +from "works until the maintenance window moves", and the cell was designed for twelve partly to see +whether behaviour degrades across a longer idle. The re-run below supplies the missing eight. The adb +drops that ended this cell look like Wi-Fi power management during idle, which is the same failure the +`9b21c6b76` retry loop was added for and did not survive. + +## The same cell, finished — twelve samples, and the app got killed halfway through + +`raw/restricted-silent-natdoze-20260910T131820.*`. Identical settings to the aborted run: +`restricted`, silent, 180 s apart, natural deep idle on battery, `flipcash_payload=IAU=`. Twelve +sends, twelve traced. + +| Reading | Value | +|---|---| +| Delivered | 12 of 12, 0 missing | +| Priority downgraded | 0 of 12 | +| `actions=2` | 12 of 12 | +| Power state | `deep=IDLE`, `light=OVERRIDE`, `bucket=45` at all fourteen samples — `start`, twelve `pre-`, `end` | +| Latency | min -335, median -81, p95 695, max 939 ms (skew floor -335 ms) | +| Feed RPCs per push | 4 `GetDmChatFeedRequest`, 2 completions, uniform across all twelve | +| Contact RPCs per push | 2, all `UNAVAILABLE: contact list service disabled` | + +Four request lines per push against two completions is twice what the four-sample cell logged. The +shape is two requests within 5 ms of the trace line, then two more that each complete: pushes one +through eight had 5.4 s between the pairs, pushes nine through twelve had 0.35 s. That is two +callers rather than a retry: the push's `RefreshFeed` and the event stream's reconnect handler both +call `FeedSyncDelegate.syncFeed()`, which cancels any sync already in flight before launching its +own, so the loser is cancelled after it has issued its RPCs. See below. + +Combined with the aborted cell, that is sixteen payload-carrying pushes at `deep=IDLE` in +`restricted`, all delivered, all planning two actions, all reaching the network. + +### The app was SIGKILLed mid-cell, and the next push brought it back + +Between send five and send six the platform killed the app: + +``` +13:31:22.390 I/ActivityManager: Killing 1307:com.flipcash.app.android/u0a309 (adj 905): + excessive cpu 13300 during 300059 dur=854340751 limit=2 +``` + +`adj 905` is the cached bucket, so this is the background CPU killer: 13.3 s of CPU inside a 300 s +window, about 4.4%, against a 2% limit. It happened three times across the two payload cells — +pid 6266 at 11:50:15 (7,240 ms), pid 1307 at 13:31:22 (13,300 ms), pid 3633 at 13:56:15 (6,730 ms). +The last of those landed during the straggler hold after send twelve, so it cost no samples; the +first ended the four-sample cell. + +What happened next is the finding. Send six arrived 129 s after the kill, into no process at all: + +``` +13:33:30.790 START_DEBUG: ProcessRecord{8f1da92 3633:com.flipcash.app.android/u0a309} +13:33:31.468 onMessageReceived | seq=restricted-silent-006, actions=2, silent=true, bucket=restricted +13:33:34.848 authState change Unknown => Authenticating +13:33:34.864 database init start ZaFTTj4o +13:33:34.878 database init end +13:33:35.014 authState change Authenticating => Ready +13:33:38.074 Request: [GetDmChatFeedRequest ... +``` + +A data-only push, to a `restricted`-bucket app, under deep idle the device entered on its own, cold- +started the process, opened the Room database, completed soft login and fetched. Sends seven through +twelve then ran in pid 3633 exactly as one through five had run in 1307. + +The cost is startup latency, not delivery: 6.6 s from the push to the first RPC on the cold process, +against 14 ms warm. For a preload that is the difference between content being ready and content +arriving while the user is already looking at the screen. + +### What burned the CPU: the wake, not the stream + +The bill is push-driven almost in full. Binned into 30 s buckets, the process logs a ~415-line burst +at each push and nothing at all in between — the quiet stretches are two minutes wide and completely +empty. The stream traffic that looked like a free-running reconnect loop is per-push and one-to-one: +12 `onMessageReceived`, 24 `flipcash-stream => CONNECTING` (two per push), 12 `=> READY`, 11 `Event +stream down, syncing feed and reconnecting`, 10 `Event stream timed out`. Every one of them falls +inside a burst. + +Regressing each kill's CPU against the app-owned log lines inside its own 300 s window: + +| pid | window | CPU | app lines | ms/line | +|---|---|---|---|---| +| 1307 | 13:26:22–13:31:22 | 13,300 ms | 861 (two bursts) | 15.45 | +| 3633 | 13:51:15–13:56:15 | 6,730 ms | 431 (one burst) | 15.61 | +| 6266 | 11:45:15–11:50:15 (aborted cell) | 7,240 ms | 808 | 8.96 | + +The two windows from the same run agree to within 1%. Solving them for a per-burst cost and an idle +rate gives **~6.7 s of CPU per push and ~0.5 ms per idle second, about 0.05%** — a fortieth of the +limit. Cached and untouched, this app costs nothing measurable. + +That reverses the cadence caveat. 6.7 s is 2.24% of a five-minute window on its own, so **one push +per five minutes already exceeds the 2% limit**, and break-even is a push every ~336 s. The 180 s +cadence decides how fast the kill arrives, not whether it arrives. + +Within a burst the work is front-loaded. Taking pid 3633's 13:36:31 push, 435 lines over ~88 s: + +| Phase | Wall | Lines | +|---|---|---| +| Wake → stream `READY` (reconnect, 162 SQLite statements) | 6.8 s | 205 | +| Feed sync + contact sync | 0.9 s | 207 | +| Stream alive, 5 s pings | 39.4 s | 7 | +| Stream times out, second reconnect cycle | 41.1 s | 16 | + +95% of the logged work is in the first 7.7 s. Holding the stream open is nearly free; +re-establishing it is not. + +### Why a cached process holds a stream open at all + +`RealChatCoordinator.onStop` does tear the stream down — `stopHeartbeat()` then `close()`. It cannot +run here. `onStop` requires the process lifecycle to have reached STARTED, and a push-woken process +never does: the capture has zero `Lifecycle resumed` and zero `teardown complete` lines across 40 +minutes. pid 3633 was created for an FCM broadcast and never had a UI. + +The stream gets opened anyway. Soft login during push handling fires `Events.OnLoggedIn`, which +reaches `onUserLoggedIn`, which calls `open()` and `startHeartbeat { syncFeed() }` with no lifecycle +check; the connectivity observer in `wireDelegateRouting()` calls `open()` on the same terms. A +headless process therefore acquires a stream and a supervisor that nothing in that process can stop. + +The freezer turns that into per-wake cost. A cached process is SIGSTOPped, so the 30 s heartbeat tick +and the stream's own ping timeout both come due while it is suspended and both fire on the next thaw. +Every burst opens the same way: + +``` +13:36:31.815 flipcash-stream => CONNECTING +13:36:31.815 Event stream down, syncing feed and reconnecting +13:36:31.816 event-stream: Timed out, signaling error for reconnect +``` + +The backoff policies are not at fault: `OpenStream` and `EventStreamDelegate.reopenBackoff` both cap +at 30 s and neither spins. The cost is that each wake pays a fresh gRPC connect plus a full +`syncFeed()` from `onReconnect`, on top of the `RefreshFeed` the push asked for. + +Four changes follow, in cost order: + +1. Gate `open()` and `startHeartbeat()` on the process lifecycle having reached STARTED, in + `onUserLoggedIn` and in the connectivity observer. A push-woken process then does its push work + and never touches the stream, which removes the reconnect, the pings and the second reconnect + cycle. +2. Make the heartbeat freeze-aware — compare wall clock across the `withTimeoutOrNull` wait, so a + 180 s gap on a 30 s tick reads as *we were frozen* rather than *the server dropped us*. +3. Coalesce `syncFeed()` within a short window instead of cancelling and relaunching, so two callers + cost one round trip. +4. Skip the contact `FullUpload` when the service is disabled. It ships the entire phone book on + every push and is refused every time. + +**One caveat on the 6.7 s.** The capture build had `TraceManager.includeRpcBodies = true`, so +`LoggingClientCallListener` builds a full proto `toString()` of every request and response — about +55 KB per burst, including the whole contact list and the whole feed with blurhashes and CDN URLs — +and `SQLiteTime` logs every statement at VERBOSE, ~285 per burst. A release build constructs none of +those strings. Treat 6.7 s as an upper bound and re-measure with bodies off before quoting it as a +production figure. The structure it describes — a headless process reconnecting a stream and +full-resyncing on every wake — does not depend on the logging. + +### What this says about option B's Android half + +- **Reliability, as far as twelve samples can say it.** No degradation across 33 minutes of + continuous idle, and no drop when the process underneath changed. +- **Process death is not a delivery failure here.** FCM re-created the process and the handler ran. + The preload design does not need the app to stay resident. +- **The CPU killer is a real constraint, and it is the push path that trips it.** Three kills in + two cells, at 2.2–4.4% average CPU while cached. Each push costs ~6.7 s of CPU, which is 2.24% of + a five-minute window on its own, against a measured idle cost of ~0.05%. The cadence accelerates + the kill rather than causing it, and the ten `Event stream timed out` errors and 24 + `flipcash-stream => CONNECTING` transitions are not work running alongside the pushes — they are + two reconnects per push, triggered by the wake. Preload work would land inside a window that is + already over budget. +- **`actions=2` is the plan, not the outcome.** It is what `planPushHandling` returned. The feed + RPCs prove `RefreshFeed` ran; `SyncContacts` was refused by the backend. Neither is instrumented + to report completion. + +One device, one OEM, one Android version — unchanged. + +## Where this leaves the kill criterion + +The criterion was: *rare-bucket delivery materially better than documented weakens the argument +against option A; delivery as poor as documented settles it.* + +Measured, delivery is better than documented — not just in `rare` but in `restricted`, and with +no drops anywhere. On the evidence collected, bucket assignment is not a reason to reject +option A. + +**The measurement the argument turns on has now been taken twice, and delivery agrees both +times.** The most throttled bucket Android has, in deep idle, on battery, delivered all 20 +pushes — with idle forced, and again with idle reached naturally. Bucket assignment is not a +reason to reject option A on this device. + +**What neither cell supports is any claim about the preload running.** Both read `actions=0` +throughout, so the forced cell's chat-feed fetch was the event stream reconnecting after the wake +and the natural cell's silence was an app that was never asked to do anything. Sizing option A +against either would be sizing it against the wrong measurement. + +**The re-run is the first cell to put the question properly, and across both attempts it says the +path is not blocked.** Sixteen pushes handled at `deep=IDLE`, sixteen times `actions=2`, feed RPCs +that completed every time. Twelve of those ran consecutively across 33 minutes of unbroken idle with +no degradation, which is as close to a rate as this rig produces. + +**What the finished cell added was a constraint the matrix was not looking for.** The platform +SIGKILLed the app mid-cell for excessive background CPU, three times across the two payload cells, +and delivery survived it — the next push cold-started the process, which opened the database and +authenticated before fetching. Delivery is not what the CPU killer threatens. Latency is: 6.6 s to +the first RPC cold against 14 ms warm. + +Generality is unchanged — one device, one OEM, one Android version. The other open question has +shrunk: the natural cell's silence was an app with nothing to do, and the re-run shows that same +process reaching the network under natural idle once the payload asks it to. The forced cell's +unprompted reconnect traffic has closed too, and it was never about Doze: a push-woken process opens +the event stream during soft login and never reaches the lifecycle state that would close it, so +every thaw fires the heartbeat and the stream's ping timeout together and pays a reconnect plus a +full feed sync. Note that the +spec's objection has narrowed rather than closed: high-priority FCM was never the missing +ingredient, since every push in every cell was sent `priority: "high"` and none was downgraded. + +## Harness defects found and fixed + +- **`run-bucket-matrix.sh` exited 0 when the device was gone**, producing three empty cells + that looked like completed work. It now fails fast on a missing device, and checks again + after the straggler hold so a mid-run disconnect is reported rather than read as a delivery + failure. +- **`adb logcat` hung for ~16 minutes** after the device vanished, which is why the first + `working_set` cell reports finishing at 19:00:39 when its last trace is 18:44. +- **The device model and Android version were unrecorded.** The runner now writes them into + each log's header before logcat starts appending. +- **`parse-bucket-log.py` read the log twice**, which silently returned zero deliveries for a + `<(gunzip -c …)` argument, since a process-substitution FIFO can only be read once. It now + makes a single pass and opens `.gz` natively. + +- **Nothing recorded the power state a cell ran in**, so the charging caveat on the first six + cells rests on spot checks taken beside the captures rather than on the captures themselves. + The runner now writes AC/USB/wireless power, deep and light idle state, and the standby bucket + to a `.power` sidecar before every send, and `DOZE=1` aborts the run if the device does not + reach `IDLE` instead of quietly measuring the charging case again. + +**A dropped Wi-Fi adb session killed the cell rather than a sample.** The first natural-idle +attempt died at send 5 of 20 on `adb: device offline`, with the phone still in unforced deep idle +on battery — TCP 5555 was open when probed and the device re-authorized within ten seconds on its +own, so adbd had reset the session and nothing about the device had changed. Under `set -e` a +failed `dumpsys` inside `power_sample` took the run down, and logcat, which is the capture, died +with it. `adb_try` now retries one-shot calls for `ADB_GRACE` seconds and `power_sample` degrades +to `unreachable`; three consecutive unreachable sends still fail the run. logcat runs under a +supervisor that reattaches from the last stamped line and marks the gap. Fixed in `9b21c6b76`; the +re-run recorded zero gaps. + +**The runner sent a payload the app is built to ignore**, which is why seven cells measured +delivery and nothing else. `PAYLOAD` now injects a base64 `flipcash.push.v1.Payload` as the +`flipcash_payload` data key. Checked against the device before use: visible plus payload reads +`actions=3`, silent plus payload reads `actions=2`, against `actions=0` for all 139 committed +sends. This was the costliest defect in the harness — the `actions=` field that exposes it was +present in every trace line the earlier cells were read from. + +**A visible push cannot be used to measure Doze.** Posting the notification lights the screen and +screen-on ends deep idle; one visible smoke push took the device from `IDLE` to `ACTIVE`. Doze +cells have to be silent, and therefore depend on `PushSilentSync` being on. + +**`DOZE=natural` woke the device it was waiting on.** The setup pressed `KEYCODE_HOME` to +background the app. HOME is a wake key, so on a screen-off phone it lit the display and restarted +the idle countdown. Skipped under `natural`, where the device is already asleep and the app already +backgrounded. From 11ae878e2f4a78894fdc19855fab691ef3f960fa Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Thu, 10 Sep 2026 15:29:41 -0400 Subject: [PATCH 12/24] feat(spike): measure per-push CPU from /proc instead of log volume The 6.7 s per push in the results document comes from regressing two kills against how many lines the app logged inside their own 300 s windows. That infers the quantity the killer reads; it does not read it. ActivityManager takes utime+stime out of /proc through ProcessCpuTracker and compares the delta against 2% of the window, so sampling those two fields around a push measures what actually decides the kill. The script brackets each push with a /proc sample, waits out the ~90 s burst the captures show, and starts with one push-free window of the same length for the idle term. Budget arithmetic needs both: 2% of 300 s is 6000 ms, and idle CPU spends some of it before any push arrives. It does not use deep Doze. The kill is gated on the process being cached with the framework seeing battery, which `dumpsys battery unplug` plus a screen-off, adj>=900 process gives without the Wi-Fi adb drops that ended two earlier cells. A pid change across a sample is reported rather than averaged in: a process killed partway through work it had not finished is not a reading of what that work costs. --- scripts/spike/measure-push-cpu.sh | 146 ++++++++++++++++++++++++++++++ 1 file changed, 146 insertions(+) create mode 100755 scripts/spike/measure-push-cpu.sh diff --git a/scripts/spike/measure-push-cpu.sh b/scripts/spike/measure-push-cpu.sh new file mode 100755 index 000000000..d7b67c54b --- /dev/null +++ b/scripts/spike/measure-push-cpu.sh @@ -0,0 +1,146 @@ +#!/usr/bin/env bash +# Measure what one push costs the app in CPU, and derive how many fit in the +# background CPU budget. +# +# Usage: scripts/spike/measure-push-cpu.sh [count] [settle_s] +# count: pushes to measure (default 10) +# settle_s: seconds to let each push's work finish before sampling (default 120) +# +# Why /proc rather than the log-line regression in the results document: the +# killer in ActivityManager reads utime+stime out of /proc via ProcessCpuTracker +# and compares the delta against 2% of a 300 s window. Sampling the same two +# fields measures the quantity that decides the kill, instead of inferring it +# from how many lines the app logged inside a window that happened to end in +# one. The regression put a push at ~6.7 s; this is the direct reading. +# +# The measurement needs three conditions, none of which is deep Doze: +# - The framework must see battery. The check is gated on it, and a charging +# device is never killed however much CPU it burns. `dumpsys battery unplug` +# is enough; the cable can stay in. +# - The screen must be off and the process cached (oom_score_adj >= 900). The +# observed kills were at adj 905. A process that is top-sleeping or in the +# previous slot is not a candidate and is also not frozen, so it keeps +# paying timer work the cached case does not. +# - Each push needs to finish. The captured bursts ran ~90 s from wake to the +# last line, so a settle window shorter than that measures part of a push. +# Deep Doze is deliberately not required. It adds the Wi-Fi adb drops that took +# down two earlier cells and it does not change what the killer reads. +# +# The run starts with one quiet window of the same length and no push, which +# gives the idle term. Budget arithmetic needs both: the 2% allowance is +# 6000 ms per 300 s window, and idle CPU spends part of it before any push +# arrives. +# +# A pid change between two samples means the process died inside that window — +# usually the killer, which is the outcome being sized. The sample is dropped +# from the mean and reported separately, because a killed process stops +# accruing partway through work it had not finished. +# +# Set DEVICE to an adb serial when more than one device is attached. PAYLOAD is +# the same base64 flipcash.push.v1.Payload the bucket runner sends, and the +# default `IAU=` is category=CONTACT_JOIN, which plans RefreshFeed + +# SyncContacts. PushSilentSync must be ON for a data-only push to plan anything +# at all; with it off this script measures an empty action list. +set -euo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +REPO="$(cd "$SCRIPT_DIR/../.." && pwd)" + +TOKEN="${1:?usage: measure-push-cpu.sh [count] [settle_s]}" +COUNT="${2:-10}" +SETTLE="${3:-120}" +PAYLOAD="${PAYLOAD:-IAU=}" +PACKAGE="com.flipcash.app.android" + +if [ -n "${DEVICE:-}" ]; then + export ANDROID_SERIAL="$DEVICE" +fi + +adb shell true >/dev/null 2>&1 \ + || { echo "FATAL: no device (${ANDROID_SERIAL:-default}) — nothing was measured" >&2; exit 1; } + +STAMP="$(date +%Y%m%dT%H%M%S)" +OUT_DIR="$REPO/docs/spikes/raw" +mkdir -p "$OUT_DIR" +OUT="$OUT_DIR/push-cpu-${STAMP}.cpu" + +MODEL="$(adb shell getprop ro.product.model | tr -d '\r')" +SDK="$(adb shell getprop ro.build.version.sdk | tr -d '\r')" +HZ="$(adb shell getconf CLK_TCK | tr -d '\r')" + +restore() { + adb shell "dumpsys battery reset" >/dev/null 2>&1 || true +} +trap restore EXIT + +adb shell "dumpsys battery unplug" >/dev/null +adb shell "input keyevent KEYCODE_SLEEP" >/dev/null || true + +# One round trip returns pid, cached-ness and the two counters together. Read +# apart they can straddle a process death and produce a negative delta. +sample() { + # utime and stime are fields 14 and 15 of /proc/pid/stat. The comm field is + # parenthesised and can hold spaces, so everything up to the closing paren + # is cut before counting: state becomes field 1, which puts utime at 12. + adb shell "pid=\$(pidof $PACKAGE); \ + if [ -z \"\$pid\" ]; then echo 'dead 0 0 0'; else \ + echo \"\$pid \$(cat /proc/\$pid/oom_score_adj) \ + \$(sed 's/.*) //' /proc/\$pid/stat | awk '{print \$12, \$13}')\"; fi" | tr -d '\r' +} + +{ + echo "# model=$MODEL sdk=$SDK clk_tck=$HZ settle_s=$SETTLE payload=$PAYLOAD" + echo "# window pid_before adj_before pid_after adj_after ticks ms" +} > "$OUT" + +# Ticks are per-core sums across the process's threads, so the value is CPU +# time and not wall time; that is what the killer compares too. +ticks_to_ms() { echo "$(( $1 * 1000 / HZ ))"; } + +measure() { + local label="$1" send="$2" + local before after p0 a0 u0 s0 p1 a1 u1 s1 ticks + before="$(sample)"; read -r p0 a0 u0 s0 <<< "$before" + [ "$p0" = "dead" ] && { echo "$label - - - - - dead_before" >> "$OUT"; return; } + if [ "$send" = "yes" ]; then + "$REPO/scripts/fcm.sh" "$TOKEN" \ + "{\"spike_seq\":\"cpu-$label\",\"flipcash_payload\":\"$PAYLOAD\"}" >/dev/null + fi + sleep "$SETTLE" + after="$(sample)"; read -r p1 a1 u1 s1 <<< "$after" + if [ "$p1" = "dead" ] || [ "$p1" != "$p0" ]; then + echo "$label $p0 $a0 ${p1} ${a1} - died" >> "$OUT" + return + fi + ticks=$(( (u1 + s1) - (u0 + s0) )) + echo "$label $p0 $a0 $p1 $a1 $ticks $(ticks_to_ms "$ticks")" >> "$OUT" +} + +measure "idle" no +for i in $(seq -f '%03g' 1 "$COUNT"); do + measure "$i" yes +done + +# 2% of a 300 s window is 6000 ms. Subtract what idle spends over the same +# window before dividing the remainder by the cost of one push: a budget that +# ignores the idle term overstates how many pushes fit. +python3 - "$OUT" "$SETTLE" <<'PY' +import statistics, sys +path, settle = sys.argv[1], float(sys.argv[2]) +rows = [l.split() for l in open(path) if not l.startswith('#')] +idle = [r for r in rows if r[0] == 'idle' and r[-1].isdigit()] +push = [int(r[-1]) for r in rows if r[0] != 'idle' and r[-1].isdigit()] +died = [r[0] for r in rows if r[-1] == 'died'] +if not push: + print(f"no complete push samples; {len(died)} died: {died}"); sys.exit(0) +idle_ms = int(idle[0][-1]) if idle else 0 +idle_300 = idle_ms * 300 / settle +mean, med = statistics.mean(push), statistics.median(push) +print(f"samples: n={len(push)} died={len(died)} {died if died else ''}") +print(f"idle: {idle_ms} ms over {settle:.0f} s -> {idle_300:.0f} ms per 300 s window") +print(f"push: mean {mean:.0f} ms median {med:.0f} ms min {min(push)} max {max(push)}") +budget = 6000 - idle_300 +print(f"budget: (6000 - {idle_300:.0f}) / {mean:.0f} = {budget/mean:.2f} pushes per 5 min") +PY + +echo "raw: $OUT" From 76c911f95215d0fedf6af3fe22514c48ac7931f7 Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Thu, 10 Sep 2026 15:44:44 -0400 Subject: [PATCH 13/24] docs(spikes): read the idle CPU term directly, and size the budget on it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 2% killer is a 6000 ms allowance per 300 s window, so how many pushes fit is that budget minus idle, divided by the cost of one push. The regression fit both terms to log volume. Sampling /proc//stat once a minute for 906 s with the app cached at adj 910, screen off and the phone unplugged, the counters do not move once: 0 ticks, against a 10 ms tick. The process is frozen — cgroup.freeze reads 1 — so it is not sleeping cheaply, it is not running. That is a different quantity from the regression's ~0.5 ms per idle second, which was fitted across windows with pushes arriving every 180 s and so attributed each burst's thaw and timer catch-up to the gap it sat in. With nothing arriving the floor is zero, which leaves the budget with one term. At the 6.7 s per push the regression gives, that is 0.90 pushes per five minutes and a break-even cadence of 335 s: the app cannot absorb one push per window. A preload has to get a push under 6000 ms to survive one, and under 1500 ms for the four per five minutes a chat preload implies. The per-push term is still the regression's figure and still an upper bound — includeRpcBodies was on for the capture build. The section says what would settle it and names the variant to re-measure on. --- .../2026-09-08-standby-bucket-results.md | 71 +++++++++++++++++++ 1 file changed, 71 insertions(+) diff --git a/docs/spikes/2026-09-08-standby-bucket-results.md b/docs/spikes/2026-09-08-standby-bucket-results.md index c7d962b2d..9a0bbf441 100644 --- a/docs/spikes/2026-09-08-standby-bucket-results.md +++ b/docs/spikes/2026-09-08-standby-bucket-results.md @@ -609,3 +609,74 @@ cells have to be silent, and therefore depend on `PushSilentSync` being on. background the app. HOME is a wake key, so on a screen-off phone it lit the display and restarted the idle countdown. Skipped under `natural`, where the device is already asleep and the app already backgrounded. + +## Sizing the preload against the 2% ceiling + +The killer's rule is a budget: 2% of a 300 s window is **6000 ms of CPU**, and a cached process that +spends more than that is SIGKILLed. How many pushes fit is that budget minus what the process costs +while nothing is happening, divided by what one push costs. The regression above produced both terms +from log volume. One of them has now been read directly. + +### An untouched cached process costs nothing, because it is frozen + +Sampling `/proc//stat` utime+stime once a minute for **906 s**, with the app cached, the screen +off and the phone physically unplugged, the counters do not move at all: + +| | | +|---|---| +| Samples | 16, at 60 s | +| Span | 906 s | +| `oom_score_adj` | 910 throughout (the kills were at 905) | +| `cgroup.freeze` | `/sys/fs/cgroup/apps/uid_10309/pid_5303/cgroup.freeze` = 1 | +| utime + stime | 2221 + 199 ticks at the first sample, and at all fifteen after it | +| Delta | **0 ticks** | + +`CLK_TCK` is 100, so a tick is 10 ms and zero ticks over 906 s bounds idle at under 11 µs per second +— under 10 ms of the 6000 ms allowance, whichever 300 s window you take. The mechanism is in the +third row: the process is frozen, so it is not sleeping cheaply, it is not running at all. + +**That is not the quantity the regression called idle.** Its ~0.5 ms per idle second was fitted +across windows in which pushes arrived every 180 s, so it is the residue either side of a burst — +thaw, timer catch-up, the stream's ping timer — attributed to the gap it sat in. Measured with +nothing arriving, the floor is zero. The practical consequence is that the budget has one term: the +whole 6000 ms is available to push work, and the answer depends only on what a push costs. + +### The answer, at the cost we have + +The per-push term is still the regression's **~6.7 s**, and it is an upper bound rather than a +reading. `TraceManager.includeRpcBodies` was on for the capture build — it follows `BuildConfig.DEBUG` +and `ReleaseStage.Internal` — so `LoggingClientCallListener` built a full proto `toString()` for every +RPC in every burst, roughly 55 KB of string per push that production never builds. + +Taken at that bound, with idle at zero: + +| | | +|---|---| +| Budget per 300 s window | 6000 ms | +| Cost of one push | ~6700 ms | +| Pushes per five minutes | **0.90** | +| Break-even cadence | 335 s | + +**The app cannot absorb even one push per five minutes.** That is the same conclusion the regression +reached, and removing the idle term does not soften it: idle was never spending the budget, so there +is nothing there to reclaim. A push has to come in under 6000 ms of CPU to survive one per window at +all, under 3000 ms for two, and under 1500 ms for the four-per-five-minutes a chat preload would +imply. The measured cost misses the first of those thresholds by 12% and the last by 4.5x. + +### What this does not establish + +The per-push number was not re-measured. `scripts/spike/measure-push-cpu.sh` is the instrument for +it: the same `/proc` bracket used above, applied around each send, with a push-free window first for +the idle term. It needs the device's FCM registration token, which `scripts/fcm.sh` takes as its +first argument, and it has not been run. + +Two things that run would settle. The first is how much of 6.7 s is the RPC-body logging: the +`benchmark` variant is `initWith(debug)` with `isDebuggable = false`, which turns `BuildConfig.DEBUG` +off, and it shares the `contributors` signing key with `debug`, so it installs over the existing +build and keeps the login and the stored flag value. Running the same cell on both variants +attributes the difference. The second is whether the four fixes listed above move the number, since +each of them targets work inside the first 7.7 s of a burst, which is where 95% of it is. + +Generality is unchanged from the rest of this document: one device, one OEM, one Android version. +The idle measurement adds one caveat of its own — the phone was at `deep=INACTIVE`, not in Doze. +Doze would only make idle cheaper, so the zero holds as a floor for the deeper states too. From 1ef6434bded4ef41b10260fef6fefbd7fe9c63c0 Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Thu, 10 Sep 2026 15:49:52 -0400 Subject: [PATCH 14/24] chore(deps): bump flipcash2-client-protocol to 0.5.0 0.5.0 carries flipcash2-protobuf-api#92, which adds `messaging.v1.Message message = 3` to `push.v1.ChatMetadata`. That field is what lets a chat push carry its own message body instead of naming a chat to fetch. `ocp-client-protocol` is unchanged. --- gradle/libs.versions.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 1822713f3..9967d745b 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -69,7 +69,7 @@ protovalidate-kt = "0.1.2" # 0.3.0 is the first release of either package to ship R8 keep rules for its generated # messages, which is what lets proguard-rules.pro drop its own. ocp-client-protocol = "0.3.0" -flipcash2-client-protocol = "0.4.1" +flipcash2-client-protocol = "0.5.0" # The Android port is the ONLY libphonenumber this app depends on, deliberately. Google's # `com.googlecode` artifact used to sit alongside it; the two ship separate copies of the metadata, From c5f025fa961941427e4dcb79e69bd6d901b5bc20 Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Thu, 10 Sep 2026 15:56:49 -0400 Subject: [PATCH 15/24] feat(chat): persist a push-carried message without an RPC `push.v1.ChatMetadata.message` arrived in flipcash2-client-protocol 0.5.0, so a chat push can now carry the message itself rather than only naming the chat it belongs to. `asPayload` maps it onto `PushChatMetadata.message` behind the proto's `hasMessage()` presence check, and `applyPushedMessage` writes it through the same `ChatMessageDataSource.upsert` the event stream uses. That upsert already drops a copy whose `event_sequence` is not newer than the stored row, which is what makes re-delivery of the same push harmless and lets a later `loadMessages` converge on the same transcript. Two things it deliberately does not do: - advance the event-log cursor. A push carries one message, not a page, so seating the cursor at its sequence would let a catch-up resume from a frontier it never fetched and skip what it missed in between. - rewind the feed row. `last_message_id` and `last_activity_epoch_ms` only move forward, guarded by the new `getLastMessageId` read. The field is optional and the message body is size-limited, so a chat push will still sometimes arrive without one. Nothing here replaces the fetch path. --- .../com/flipcash/shared/chat/ChatCoordinator.kt | 9 +++++++++ .../chat/internal/delegates/MessagingDelegate.kt | 14 ++++++++++++++ .../app/persistence/dao/ChatMetadataDao.kt | 3 +++ .../persistence/sources/ChatMetadataDataSource.kt | 3 +++ .../internal/network/extensions/ProtobufToLocal.kt | 1 + .../services/models/NotificationPayload.kt | 7 +++++++ 6 files changed, 37 insertions(+) diff --git a/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/ChatCoordinator.kt b/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/ChatCoordinator.kt index c1828277d..d09195b2d 100644 --- a/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/ChatCoordinator.kt +++ b/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/ChatCoordinator.kt @@ -160,6 +160,15 @@ interface MessagingOperations { /** Fetches the full message history for [chatId] from the server and persists locally. */ suspend fun loadMessages(chatId: ChatId) + /** + * Persists a message the server delivered inside a push payload, without an RPC. + * + * Idempotent on `eventSequence`: a copy that is not newer than the stored row is + * dropped, so re-delivery of the same push and a following [loadMessages] converge + * on the same transcript. + */ + suspend fun applyPushedMessage(chatId: ChatId, message: ChatMessage) + /** * Sends a text message to [chatId]. Returns the server-confirmed [ChatMessage]. * diff --git a/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/internal/delegates/MessagingDelegate.kt b/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/internal/delegates/MessagingDelegate.kt index ce2a871c2..779d01cb0 100644 --- a/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/internal/delegates/MessagingDelegate.kt +++ b/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/internal/delegates/MessagingDelegate.kt @@ -193,6 +193,20 @@ class MessagingDelegate @Inject constructor( } } + override suspend fun applyPushedMessage(chatId: ChatId, message: ChatMessage) { + // The DAO's upsert already drops a copy older than the stored row, so the only + // ordering this has to protect is the metadata below it. + messageDataSource.upsert(chatId, listOf(message)) + + // Deliberately not advancing the event-log cursor. A push carries one message, not + // a page, so seating the cursor at its sequence would let a later catch-up resume + // from a frontier it never actually fetched and skip whatever it missed in between. + if (message.messageId > (metadataDataSource.getLastMessageId(chatId) ?: 0L)) { + metadataDataSource.updateLastMessageId(chatId, message.messageId) + metadataDataSource.updateLastActivity(chatId, message.timestamp.toEpochMilliseconds()) + } + } + override suspend fun sendMessage( chatId: ChatId, content: String, diff --git a/apps/flipcash/shared/persistence/db/src/main/kotlin/com/flipcash/app/persistence/dao/ChatMetadataDao.kt b/apps/flipcash/shared/persistence/db/src/main/kotlin/com/flipcash/app/persistence/dao/ChatMetadataDao.kt index 0ca8e5b43..7841c95e7 100644 --- a/apps/flipcash/shared/persistence/db/src/main/kotlin/com/flipcash/app/persistence/dao/ChatMetadataDao.kt +++ b/apps/flipcash/shared/persistence/db/src/main/kotlin/com/flipcash/app/persistence/dao/ChatMetadataDao.kt @@ -73,6 +73,9 @@ interface ChatMetadataDao { @Query("UPDATE chat_metadata SET last_activity_epoch_ms = :epochMs WHERE chat_id_hex = :chatIdHex") suspend fun updateLastActivity(chatIdHex: String, epochMs: Long) + @Query("SELECT last_message_id FROM chat_metadata WHERE chat_id_hex = :chatIdHex") + suspend fun getLastMessageId(chatIdHex: String): Long? + @Query("UPDATE chat_metadata SET last_message_id = :messageId WHERE chat_id_hex = :chatIdHex") suspend fun updateLastMessageId(chatIdHex: String, messageId: Long) diff --git a/apps/flipcash/shared/persistence/sources/src/main/kotlin/com/flipcash/app/persistence/sources/ChatMetadataDataSource.kt b/apps/flipcash/shared/persistence/sources/src/main/kotlin/com/flipcash/app/persistence/sources/ChatMetadataDataSource.kt index 71e00eb65..7c1bec788 100644 --- a/apps/flipcash/shared/persistence/sources/src/main/kotlin/com/flipcash/app/persistence/sources/ChatMetadataDataSource.kt +++ b/apps/flipcash/shared/persistence/sources/src/main/kotlin/com/flipcash/app/persistence/sources/ChatMetadataDataSource.kt @@ -40,6 +40,9 @@ class ChatMetadataDataSource @Inject constructor( db?.chatMetadataDao()?.updateLastActivity(mapper.chatIdHex(chatId), epochMs) } + suspend fun getLastMessageId(chatId: ChatId): Long? = + db?.chatMetadataDao()?.getLastMessageId(mapper.chatIdHex(chatId)) + suspend fun updateLastMessageId(chatId: ChatId, messageId: Long) { db?.chatMetadataDao()?.updateLastMessageId(mapper.chatIdHex(chatId), messageId) } diff --git a/services/flipcash/src/main/kotlin/com/flipcash/services/internal/network/extensions/ProtobufToLocal.kt b/services/flipcash/src/main/kotlin/com/flipcash/services/internal/network/extensions/ProtobufToLocal.kt index 0bccc3a3c..4797be0b8 100644 --- a/services/flipcash/src/main/kotlin/com/flipcash/services/internal/network/extensions/ProtobufToLocal.kt +++ b/services/flipcash/src/main/kotlin/com/flipcash/services/internal/network/extensions/ProtobufToLocal.kt @@ -110,6 +110,7 @@ internal fun PushModels.Payload.asPayload(): NotificationPayload { PushChatMetadata( sendingUserId = if (chatMetadata.hasSendingUserId()) chatMetadata.sendingUserId.toId() else null, chatType = chatMetadata.type.toChatType(), + message = if (chatMetadata.hasMessage()) chatMetadata.message.toChatMessage() else null, ) } else null diff --git a/services/flipcash/src/main/kotlin/com/flipcash/services/models/NotificationPayload.kt b/services/flipcash/src/main/kotlin/com/flipcash/services/models/NotificationPayload.kt index 658bab11a..9e840814d 100644 --- a/services/flipcash/src/main/kotlin/com/flipcash/services/models/NotificationPayload.kt +++ b/services/flipcash/src/main/kotlin/com/flipcash/services/models/NotificationPayload.kt @@ -3,6 +3,7 @@ package com.flipcash.services.models import com.codeinc.flipcash.gen.push.v1.Model import com.flipcash.services.internal.network.extensions.asPayload import com.flipcash.services.models.chat.ChatId +import com.flipcash.services.models.chat.ChatMessage import com.flipcash.services.models.chat.ChatType import com.getcode.opencode.model.core.ID import com.getcode.solana.keys.Mint @@ -34,10 +35,16 @@ sealed interface Substitution { * * [sendingUserId] is null for system messages or notifications not tied to a user, * mirroring the proto's optional `sending_user_id` field. + * + * [message] is the pushed message itself, present only when the push is for a + * message and the server chose to inline it. It is optional in the proto and + * the body has a size limit, so a chat push can still arrive without one; the + * fetch path stays the fallback rather than being replaced. */ data class PushChatMetadata( val sendingUserId: ID?, val chatType: ChatType, + val message: ChatMessage? = null, ) data class NotificationPayload( From 427c84cb4126e1da31b6f2e4a0385af121738f74 Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Thu, 10 Sep 2026 16:09:58 -0400 Subject: [PATCH 16/24] feat(notifications): plan the carried message instead of a fetch A push at a named chat planned `RefreshFeed` then `LoadMessages`, and `LoadMessages` is a `GetMessages` round trip on the wake path. Splitting one push-woken burst by trace phase puts roughly half its lines under wake and stream setup and roughly half under the sync it triggered (docs/spikes/2026-09-08-standby-bucket-results.md), so both are material against the 2%-over-five-minutes CPU ceiling and the split between them is unmeasured. This removes one of the two round trips the sync half makes. When the payload inlines the message, `syncForNavigation` now plans `PushAction.ApplyMessage` in its place, which is a local write. When it does not, the plan is the `LoadMessages` it has always been. The fallback is not defensive: `chat_metadata.message` is optional in the proto and the body is size-limited, so the server will omit it. `RefreshFeed` stays. Unread counts come from the feed, not from the message row, so dropping it would change what the user sees rather than only what the push costs. Also corrects the table's comment: `flipcash.push.v1.Payload.category` is the event taxonomy and no separate event field is coming, so the map is already keyed on the taxonomy. Adding an event class stays an entry rather than a branch. --- .../app/notifications/NotificationService.kt | 8 ++- .../flipcash/app/notifications/PushAction.kt | 10 +++ .../app/notifications/PushHandlingPlanner.kt | 27 ++++--- .../notifications/PushHandlingPlannerTest.kt | 71 +++++++++++++++++++ 4 files changed, 106 insertions(+), 10 deletions(-) diff --git a/apps/flipcash/shared/notifications/src/main/kotlin/com/flipcash/app/notifications/NotificationService.kt b/apps/flipcash/shared/notifications/src/main/kotlin/com/flipcash/app/notifications/NotificationService.kt index 1b8381c96..c340cd7a5 100644 --- a/apps/flipcash/shared/notifications/src/main/kotlin/com/flipcash/app/notifications/NotificationService.kt +++ b/apps/flipcash/shared/notifications/src/main/kotlin/com/flipcash/app/notifications/NotificationService.kt @@ -192,7 +192,9 @@ class NotificationService : FirebaseMessagingService(), } val chatActions = actions.filter { - it is PushAction.RefreshFeed || it is PushAction.LoadMessages + it is PushAction.RefreshFeed || + it is PushAction.LoadMessages || + it is PushAction.ApplyMessage } if (chatActions.isNotEmpty()) { launch { @@ -200,6 +202,10 @@ class NotificationService : FirebaseMessagingService(), when (action) { is PushAction.RefreshFeed -> chatCoordinator.refreshFeed() is PushAction.LoadMessages -> chatCoordinator.loadMessages(chatId = action.chatId) + is PushAction.ApplyMessage -> chatCoordinator.applyPushedMessage( + chatId = action.chatId, + message = action.message, + ) else -> Unit } } diff --git a/apps/flipcash/shared/notifications/src/main/kotlin/com/flipcash/app/notifications/PushAction.kt b/apps/flipcash/shared/notifications/src/main/kotlin/com/flipcash/app/notifications/PushAction.kt index bb0449f8e..21f108414 100644 --- a/apps/flipcash/shared/notifications/src/main/kotlin/com/flipcash/app/notifications/PushAction.kt +++ b/apps/flipcash/shared/notifications/src/main/kotlin/com/flipcash/app/notifications/PushAction.kt @@ -2,6 +2,7 @@ package com.flipcash.app.notifications import com.flipcash.services.models.NotificationPayload import com.flipcash.services.models.chat.ChatId +import com.flipcash.services.models.chat.ChatMessage /** * A single unit of work a received push asks the app to perform. @@ -17,6 +18,15 @@ sealed interface PushAction { /** Fetch and persist full message history for one chat. */ data class LoadMessages(val chatId: ChatId) : PushAction + /** + * Persist a message the push carried, instead of fetching it. + * + * Planned in place of [LoadMessages] when the payload inlines the message. + * The write is local, so this is the one sync action a push can satisfy + * without a network round trip. + */ + data class ApplyMessage(val chatId: ChatId, val message: ChatMessage) : PushAction + /** Refresh token/mint state. */ data object UpdateTokens : PushAction diff --git a/apps/flipcash/shared/notifications/src/main/kotlin/com/flipcash/app/notifications/PushHandlingPlanner.kt b/apps/flipcash/shared/notifications/src/main/kotlin/com/flipcash/app/notifications/PushHandlingPlanner.kt index 190f849f1..3ca1d2ffa 100644 --- a/apps/flipcash/shared/notifications/src/main/kotlin/com/flipcash/app/notifications/PushHandlingPlanner.kt +++ b/apps/flipcash/shared/notifications/src/main/kotlin/com/flipcash/app/notifications/PushHandlingPlanner.kt @@ -35,10 +35,10 @@ fun planPushHandling( * The sync work an event class implies, independent of where the push * navigates. * - * Adding an event class is an entry in this table rather than a branch in - * [syncActionsFor]. The key is [NotificationCategory] until - * `flipcash.push.v1.Payload` carries the event field the shared taxonomy needs; - * when it does, the key type changes and the shape does not. + * `flipcash.push.v1.Payload.category` is the event taxonomy — there is no + * separate event field coming, so this table is keyed on the taxonomy already. + * Adding an event class is an entry here rather than a branch in + * [syncActionsFor]. * * A category absent from the table plans no sync of its own, which is every * category but one today. @@ -52,12 +52,21 @@ private val syncByCategory: Map> = mapOf( * * This stays a `when` rather than joining the table above: each arm reads data * off the trigger it matched, so the actions cannot be written down in advance. + * + * A push at a named chat resolves to one of two shapes. When the payload + * inlines the message, [PushAction.ApplyMessage] writes it locally; when it + * does not — the field is optional and the body is size-limited — the plan + * falls back to the [PushAction.LoadMessages] fetch it has always used. */ -private fun syncForNavigation(navigation: NavigationTrigger?): List = - when (navigation) { +private fun syncForNavigation(payload: NotificationPayload): List = + when (val navigation = payload.navigation) { is NavigationTrigger.CurrencyInfo -> listOf(PushAction.UpdateTokens) - is NavigationTrigger.Chat.ById -> - listOf(PushAction.RefreshFeed, PushAction.LoadMessages(navigation.chatId)) + is NavigationTrigger.Chat.ById -> listOf( + PushAction.RefreshFeed, + payload.chatMetadata?.message + ?.let { PushAction.ApplyMessage(navigation.chatId, it) } + ?: PushAction.LoadMessages(navigation.chatId), + ) is NavigationTrigger.Chat.ByContact -> emptyList() null -> emptyList() } @@ -71,6 +80,6 @@ private fun syncForNavigation(navigation: NavigationTrigger?): List */ private fun syncActionsFor(payload: NotificationPayload?): List { if (payload == null) return emptyList() - return (syncByCategory[payload.category].orEmpty() + syncForNavigation(payload.navigation)) + return (syncByCategory[payload.category].orEmpty() + syncForNavigation(payload)) .distinct() } diff --git a/apps/flipcash/shared/notifications/src/test/kotlin/com/flipcash/app/notifications/PushHandlingPlannerTest.kt b/apps/flipcash/shared/notifications/src/test/kotlin/com/flipcash/app/notifications/PushHandlingPlannerTest.kt index 6278aff0e..96cbda22a 100644 --- a/apps/flipcash/shared/notifications/src/test/kotlin/com/flipcash/app/notifications/PushHandlingPlannerTest.kt +++ b/apps/flipcash/shared/notifications/src/test/kotlin/com/flipcash/app/notifications/PushHandlingPlannerTest.kt @@ -3,8 +3,13 @@ package com.flipcash.app.notifications import com.flipcash.services.models.NavigationTrigger import com.flipcash.services.models.NotificationCategory import com.flipcash.services.models.NotificationPayload +import com.flipcash.services.models.PushChatMetadata import com.flipcash.services.models.chat.ChatId +import com.flipcash.services.models.chat.ChatMessage +import com.flipcash.services.models.chat.ChatType +import com.flipcash.services.models.chat.MessageContent import com.getcode.solana.keys.Mint +import kotlin.time.Instant import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertTrue @@ -14,9 +19,26 @@ class PushHandlingPlannerTest { private fun payload( navigation: NavigationTrigger? = null, category: NotificationCategory = NotificationCategory.DEFAULT, + chatMetadata: PushChatMetadata? = null, ) = NotificationPayload( navigation = navigation, category = category, + chatMetadata = chatMetadata, + ) + + private fun inlinedMessage(messageId: Long = 42L, eventSequence: Long = 7L) = ChatMessage( + messageId = messageId, + senderId = null, + content = listOf(MessageContent.Text("hello")), + timestamp = Instant.fromEpochSeconds(1_757_000_000), + unreadSeq = 1L, + eventSequence = eventSequence, + ) + + private fun chatMetadata(message: ChatMessage?) = PushChatMetadata( + sendingUserId = null, + chatType = ChatType.CONTACT_DM, + message = message, ) // region Today's behaviour: a titleless push is dropped entirely @@ -74,6 +96,55 @@ class PushHandlingPlannerTest { ) } + @Test + fun `a chat push carrying its message applies it instead of fetching`() { + val chatId = ChatId("aa08") + val message = inlinedMessage() + val p = payload( + navigation = NavigationTrigger.Chat.ById(chatId), + chatMetadata = chatMetadata(message), + ) + val actions = planPushHandling("Title", "Body", p, silentSyncEnabled = { false }) + assertEquals( + listOf(PushAction.RefreshFeed, PushAction.ApplyMessage(chatId, message)), + actions.filterNot { it is PushAction.PostNotification }, + ) + } + + @Test + fun `chat metadata without a message falls back to fetching`() { + val chatId = ChatId("aa09") + val p = payload( + navigation = NavigationTrigger.Chat.ById(chatId), + chatMetadata = chatMetadata(message = null), + ) + val actions = planPushHandling("Title", "Body", p, silentSyncEnabled = { false }) + assertEquals( + listOf(PushAction.RefreshFeed, PushAction.LoadMessages(chatId)), + actions.filterNot { it is PushAction.PostNotification }, + ) + } + + @Test + fun `an inlined message on a push that names no chat plans nothing to apply`() { + val p = payload(chatMetadata = chatMetadata(inlinedMessage())) + val actions = planPushHandling("Title", null, p, silentSyncEnabled = { false }) + assertEquals(listOf(PushAction.PostNotification("Title", null, p)), actions) + } + + @Test + fun `a silent chat push with an inlined message needs no network`() { + val chatId = ChatId("aa10") + val message = inlinedMessage() + val p = payload( + navigation = NavigationTrigger.Chat.ById(chatId), + chatMetadata = chatMetadata(message), + ) + val actions = planPushHandling(null, null, p, silentSyncEnabled = { true }) + assertTrue(PushAction.ApplyMessage(chatId, message) in actions) + assertTrue(actions.none { it is PushAction.LoadMessages }) + } + @Test fun `a contact join that also names a chat refreshes the feed once`() { val chatId = ChatId("aa11") From 64e6ecabbeafea8f9d8c7ec9b27843cbff5243d0 Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Thu, 10 Sep 2026 16:00:42 -0400 Subject: [PATCH 17/24] docs(spikes): retire the fitted idle term where it is still stated as a finding MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The regression section and the option-B bullet both reported ~0.5 ms per idle second as a measured cost, ~200 lines before the section that refutes it. A direct `/proc` read of a cached process shows zero ticks over 906 s, because `cgroup.freeze` is 1 — the residual was burst edges attributed to the gap beside them, not an idle rate. Both now say what survived measurement and point forward to the reading. Also makes the break-even cadence read 335 s in both places; it was 336 s here and 335 s in the budget table, the same division at different precision. --- .../2026-09-08-standby-bucket-results.md | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/docs/spikes/2026-09-08-standby-bucket-results.md b/docs/spikes/2026-09-08-standby-bucket-results.md index 9a0bbf441..5a29e0f29 100644 --- a/docs/spikes/2026-09-08-standby-bucket-results.md +++ b/docs/spikes/2026-09-08-standby-bucket-results.md @@ -439,11 +439,13 @@ Regressing each kill's CPU against the app-owned log lines inside its own 300 s | 6266 | 11:45:15–11:50:15 (aborted cell) | 7,240 ms | 808 | 8.96 | The two windows from the same run agree to within 1%. Solving them for a per-burst cost and an idle -rate gives **~6.7 s of CPU per push and ~0.5 ms per idle second, about 0.05%** — a fortieth of the -limit. Cached and untouched, this app costs nothing measurable. +rate gives **~6.7 s of CPU per push**, plus a residual of ~0.5 ms per idle second. Only the first +term survived direct measurement — a frozen process reads zero ticks, and the residual turned out to +be burst edges attributed to the gap beside them. See *An untouched cached process costs nothing, +because it is frozen* below. That reverses the cadence caveat. 6.7 s is 2.24% of a five-minute window on its own, so **one push -per five minutes already exceeds the 2% limit**, and break-even is a push every ~336 s. The 180 s +per five minutes already exceeds the 2% limit**, and break-even is a push every ~335 s. The 180 s cadence decides how fast the kill arrives, not whether it arrives. Within a burst the work is front-loaded. Taking pid 3633's 13:36:31 push, 435 lines over ~88 s: @@ -513,11 +515,11 @@ full-resyncing on every wake — does not depend on the logging. The preload design does not need the app to stay resident. - **The CPU killer is a real constraint, and it is the push path that trips it.** Three kills in two cells, at 2.2–4.4% average CPU while cached. Each push costs ~6.7 s of CPU, which is 2.24% of - a five-minute window on its own, against a measured idle cost of ~0.05%. The cadence accelerates - the kill rather than causing it, and the ten `Event stream timed out` errors and 24 - `flipcash-stream => CONNECTING` transitions are not work running alongside the pushes — they are - two reconnects per push, triggered by the wake. Preload work would land inside a window that is - already over budget. + a five-minute window on its own, against an idle cost of zero, since a cached process is frozen + rather than merely quiet. The cadence accelerates the kill rather than causing it, and the ten + `Event stream timed out` errors and 24 `flipcash-stream => CONNECTING` transitions are not work + running alongside the pushes — they are two reconnects per push, triggered by the wake. Preload + work would land inside a window that is already over budget. - **`actions=2` is the plan, not the outcome.** It is what `planPushHandling` returned. The feed RPCs prove `RefreshFeed` ran; `SyncContacts` was refused by the backend. Neither is instrumented to report completion. From 0a8796ac26225203d00b7ef090e971d1c648f120 Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Thu, 10 Sep 2026 16:08:13 -0400 Subject: [PATCH 18/24] refactor(notifications): drop the PushSilentSync flag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Silent preload was gated on `PushSilentSync`, default off, so a data-only push planned nothing on a default build. `planPushHandling` now plans the same sync work either way and a title only adds `PostNotification` on top, which also takes the blocking DataStore read off the FCM dispatch thread — the reason the flag was passed as a `() -> Boolean` rather than read as a value. The two tests asserting that a titleless push planned nothing go with it, replaced by their inverse plus the invariant that a title changes the notification and not the sync plan. The spike doc's flag paragraphs move to past tense: its numbers were measured with the flag on, and a re-run now needs no flag setup. --- .../flipcash/app/featureflags/FeatureFlag.kt | 13 --- .../app/notifications/NotificationService.kt | 7 -- .../app/notifications/PushHandlingPlanner.kt | 12 +-- .../notifications/PushHandlingPlannerTest.kt | 80 +++++-------------- .../2026-09-08-standby-bucket-results.md | 21 ++--- 5 files changed, 34 insertions(+), 99 deletions(-) diff --git a/apps/flipcash/shared/featureflags/src/main/kotlin/com/flipcash/app/featureflags/FeatureFlag.kt b/apps/flipcash/shared/featureflags/src/main/kotlin/com/flipcash/app/featureflags/FeatureFlag.kt index 927c7b790..08703a32d 100644 --- a/apps/flipcash/shared/featureflags/src/main/kotlin/com/flipcash/app/featureflags/FeatureFlag.kt +++ b/apps/flipcash/shared/featureflags/src/main/kotlin/com/flipcash/app/featureflags/FeatureFlag.kt @@ -117,17 +117,6 @@ sealed interface FeatureFlag { override val persistLogOut: Boolean = false } - @FeatureFlagMarker - data object PushSilentSync: FeatureFlag { - override val key: String = "push_silent_sync_enabled" - override val default: Boolean = false - override val launched: Boolean = false - override val visible: Boolean = true - override val persistLogOut: Boolean = false - override val onboarding: Boolean = false - override val minTrack: FeatureTrack = FeatureTrack.Internal - } - companion object { val entries: List> get() = FeatureFlagEntries.entries @@ -150,7 +139,6 @@ val FeatureFlag<*>.title: String FeatureFlag.ContactPickerMode -> "Contact Picker Mode" FeatureFlag.ShowNetworkState -> "Network Offline Indicator" FeatureFlag.FrostedTipCard -> "Frosted Tip Card" - FeatureFlag.PushSilentSync -> "Push Silent Sync" } val FeatureFlag<*>.message: String @@ -164,7 +152,6 @@ val FeatureFlag<*>.message: String FeatureFlag.ContactPickerMode -> "When enabled, contacts will be accessed via the system contact picker instead of requesting full READ_CONTACTS permission" FeatureFlag.ShowNetworkState -> "When enabled, you'll gain the ability to see the network state on the Scanner when offline" FeatureFlag.FrostedTipCard -> "When enabled, the tip card in the scanner renders as frosted glass over a blurred snapshot of the camera instead of a solid card" - FeatureFlag.PushSilentSync -> "When enabled, data-only pushes trigger background sync work without posting a visible notification" } diff --git a/apps/flipcash/shared/notifications/src/main/kotlin/com/flipcash/app/notifications/NotificationService.kt b/apps/flipcash/shared/notifications/src/main/kotlin/com/flipcash/app/notifications/NotificationService.kt index c340cd7a5..526af3cbc 100644 --- a/apps/flipcash/shared/notifications/src/main/kotlin/com/flipcash/app/notifications/NotificationService.kt +++ b/apps/flipcash/shared/notifications/src/main/kotlin/com/flipcash/app/notifications/NotificationService.kt @@ -26,8 +26,6 @@ import com.flipcash.app.core.media.MediaUrlResolver import com.flipcash.app.auth.AuthManager import com.flipcash.app.contacts.ContactCoordinator import com.flipcash.app.contacts.ContactResolver -import com.flipcash.app.featureflags.FeatureFlag -import com.flipcash.app.featureflags.FeatureFlagController import com.flipcash.app.core.util.Linkify import com.flipcash.shared.chat.ChatCoordinator import com.flipcash.app.tokens.TokenCoordinator @@ -56,7 +54,6 @@ import dagger.hilt.android.AndroidEntryPoint import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.launch -import kotlinx.coroutines.runBlocking import kotlinx.coroutines.withTimeoutOrNull import java.security.SecureRandom import javax.inject.Inject @@ -117,9 +114,6 @@ class NotificationService : FirebaseMessagingService(), @Inject lateinit var userProfileDataSource: UserProfileDataSource - @Inject - lateinit var featureFlags: FeatureFlagController - // TODO(firebase-messaging): 25.1.0 deprecated onNewToken in favor of FID-based onRegistered(). // Migrate once Firebase ships a stable guide and the backend accepts FID registration. // Tracking: https://github.com/firebase/firebase-android-sdk/issues/8087 @@ -155,7 +149,6 @@ class NotificationService : FirebaseMessagingService(), title = title, body = body, payload = payload, - silentSyncEnabled = { runBlocking { featureFlags.get(FeatureFlag.PushSilentSync) } }, ) val latencyMs = System.currentTimeMillis() - message.sentTime diff --git a/apps/flipcash/shared/notifications/src/main/kotlin/com/flipcash/app/notifications/PushHandlingPlanner.kt b/apps/flipcash/shared/notifications/src/main/kotlin/com/flipcash/app/notifications/PushHandlingPlanner.kt index 3ca1d2ffa..180a5e634 100644 --- a/apps/flipcash/shared/notifications/src/main/kotlin/com/flipcash/app/notifications/PushHandlingPlanner.kt +++ b/apps/flipcash/shared/notifications/src/main/kotlin/com/flipcash/app/notifications/PushHandlingPlanner.kt @@ -13,22 +13,16 @@ import com.flipcash.services.models.NotificationPayload * @param title resolved push title, null for a data-only push * @param body resolved push body, may be null even for a visible push * @param payload decoded [NotificationPayload], null when absent or undecodable - * @param silentSyncEnabled reads the `PushSilentSync` feature flag. Passed as a - * function because only a data-only push consults it, and the call site's - * read is a blocking DataStore lookup on the FCM dispatch thread — a visible - * push should not pay for a flag that cannot change its outcome. */ fun planPushHandling( title: String?, body: String?, payload: NotificationPayload?, - silentSyncEnabled: () -> Boolean, ): List { - if (title == null) { - return if (silentSyncEnabled()) syncActionsFor(payload) else emptyList() - } + val sync = syncActionsFor(payload) - return syncActionsFor(payload) + PushAction.PostNotification(title, body, payload) + // The sync half is the same either way; a title only adds the notification on top. + return if (title == null) sync else sync + PushAction.PostNotification(title, body, payload) } /** diff --git a/apps/flipcash/shared/notifications/src/test/kotlin/com/flipcash/app/notifications/PushHandlingPlannerTest.kt b/apps/flipcash/shared/notifications/src/test/kotlin/com/flipcash/app/notifications/PushHandlingPlannerTest.kt index 96cbda22a..c5387421e 100644 --- a/apps/flipcash/shared/notifications/src/test/kotlin/com/flipcash/app/notifications/PushHandlingPlannerTest.kt +++ b/apps/flipcash/shared/notifications/src/test/kotlin/com/flipcash/app/notifications/PushHandlingPlannerTest.kt @@ -41,38 +41,12 @@ class PushHandlingPlannerTest { message = message, ) - // region Today's behaviour: a titleless push is dropped entirely - - @Test - fun `no title yields no actions when silent sync is disabled`() { - val actions = planPushHandling( - title = null, - body = "ignored", - payload = payload(navigation = NavigationTrigger.CurrencyInfo(mint = TEST_MINT)), - silentSyncEnabled = { false }, - ) - assertEquals(emptyList(), actions) - } - - @Test - fun `no title drops chat sync too when silent sync is disabled`() { - val actions = planPushHandling( - title = null, - body = null, - payload = payload(navigation = NavigationTrigger.Chat.ById(ChatId("aa01"))), - silentSyncEnabled = { false }, - ) - assertEquals(emptyList(), actions) - } - - // endregion - - // region Today's behaviour: a titled push + // region A titled push @Test fun `currency info push updates tokens and posts`() { val p = payload(navigation = NavigationTrigger.CurrencyInfo(mint = TEST_MINT)) - val actions = planPushHandling("Title", "Body", p, silentSyncEnabled = { false }) + val actions = planPushHandling("Title", "Body", p) assertTrue(PushAction.UpdateTokens in actions) assertTrue(actions.last() is PushAction.PostNotification) } @@ -80,7 +54,7 @@ class PushHandlingPlannerTest { @Test fun `contact join push refreshes feed and syncs contacts`() { val p = payload(category = NotificationCategory.CONTACT_JOIN) - val actions = planPushHandling("Title", null, p, silentSyncEnabled = { false }) + val actions = planPushHandling("Title", null, p) assertTrue(PushAction.RefreshFeed in actions) assertTrue(PushAction.SyncContacts in actions) } @@ -89,7 +63,7 @@ class PushHandlingPlannerTest { fun `chat push refreshes feed then loads that chat`() { val chatId = ChatId("aa07") val p = payload(navigation = NavigationTrigger.Chat.ById(chatId)) - val actions = planPushHandling("Title", "Body", p, silentSyncEnabled = { false }) + val actions = planPushHandling("Title", "Body", p) assertEquals( listOf(PushAction.RefreshFeed, PushAction.LoadMessages(chatId)), actions.filterNot { it is PushAction.PostNotification }, @@ -104,7 +78,7 @@ class PushHandlingPlannerTest { navigation = NavigationTrigger.Chat.ById(chatId), chatMetadata = chatMetadata(message), ) - val actions = planPushHandling("Title", "Body", p, silentSyncEnabled = { false }) + val actions = planPushHandling("Title", "Body", p) assertEquals( listOf(PushAction.RefreshFeed, PushAction.ApplyMessage(chatId, message)), actions.filterNot { it is PushAction.PostNotification }, @@ -118,7 +92,7 @@ class PushHandlingPlannerTest { navigation = NavigationTrigger.Chat.ById(chatId), chatMetadata = chatMetadata(message = null), ) - val actions = planPushHandling("Title", "Body", p, silentSyncEnabled = { false }) + val actions = planPushHandling("Title", "Body", p) assertEquals( listOf(PushAction.RefreshFeed, PushAction.LoadMessages(chatId)), actions.filterNot { it is PushAction.PostNotification }, @@ -128,7 +102,7 @@ class PushHandlingPlannerTest { @Test fun `an inlined message on a push that names no chat plans nothing to apply`() { val p = payload(chatMetadata = chatMetadata(inlinedMessage())) - val actions = planPushHandling("Title", null, p, silentSyncEnabled = { false }) + val actions = planPushHandling("Title", null, p) assertEquals(listOf(PushAction.PostNotification("Title", null, p)), actions) } @@ -140,7 +114,7 @@ class PushHandlingPlannerTest { navigation = NavigationTrigger.Chat.ById(chatId), chatMetadata = chatMetadata(message), ) - val actions = planPushHandling(null, null, p, silentSyncEnabled = { true }) + val actions = planPushHandling(null, null, p) assertTrue(PushAction.ApplyMessage(chatId, message) in actions) assertTrue(actions.none { it is PushAction.LoadMessages }) } @@ -152,7 +126,7 @@ class PushHandlingPlannerTest { navigation = NavigationTrigger.Chat.ById(chatId), category = NotificationCategory.CONTACT_JOIN, ) - val actions = planPushHandling("Title", null, p, silentSyncEnabled = { false }) + val actions = planPushHandling("Title", null, p) assertEquals( listOf(PushAction.RefreshFeed, PushAction.SyncContacts, PushAction.LoadMessages(chatId)), actions.filterNot { it is PushAction.PostNotification }, @@ -162,13 +136,13 @@ class PushHandlingPlannerTest { @Test fun `a category with no sync of its own plans nothing`() { val p = payload(category = NotificationCategory.GAIN) - val actions = planPushHandling("Title", null, p, silentSyncEnabled = { false }) + val actions = planPushHandling("Title", null, p) assertEquals(listOf(PushAction.PostNotification("Title", null, p)), actions) } @Test fun `push with no payload still posts the notification`() { - val actions = planPushHandling("Title", "Body", payload = null, silentSyncEnabled = { false }) + val actions = planPushHandling("Title", "Body", payload = null) assertEquals( listOf(PushAction.PostNotification("Title", "Body", null)), actions, @@ -178,63 +152,49 @@ class PushHandlingPlannerTest { @Test fun `post notification is always last so sync starts first`() { val p = payload(navigation = NavigationTrigger.Chat.ById(ChatId("0c"))) - val actions = planPushHandling("Title", "Body", p, silentSyncEnabled = { false }) + val actions = planPushHandling("Title", "Body", p) assertTrue(actions.last() is PushAction.PostNotification) } // endregion - // region Silent sync enabled + // region A data-only push @Test - fun `no title still syncs chat when silent sync is enabled`() { + fun `a data only push syncs chat without posting`() { val chatId = ChatId("aa09") val actions = planPushHandling( title = null, body = null, payload = payload(navigation = NavigationTrigger.Chat.ById(chatId)), - silentSyncEnabled = { true }, ) assertEquals(listOf(PushAction.RefreshFeed, PushAction.LoadMessages(chatId)), actions) } @Test - fun `silent push never posts a notification`() { + fun `a data only push never posts a notification`() { val actions = planPushHandling( title = null, body = null, payload = payload(navigation = NavigationTrigger.CurrencyInfo(mint = TEST_MINT)), - silentSyncEnabled = { true }, ) assertTrue(actions.none { it is PushAction.PostNotification }) assertEquals(listOf(PushAction.UpdateTokens), actions) } @Test - fun `silent push with no payload does nothing`() { - val actions = planPushHandling(null, null, payload = null, silentSyncEnabled = { true }) + fun `a data only push with no payload does nothing`() { + val actions = planPushHandling(null, null, payload = null) assertEquals(emptyList(), actions) } @Test - fun `enabling silent sync does not change a titled push`() { + fun `a title changes only the notification, not the sync plan`() { val p = payload(navigation = NavigationTrigger.Chat.ById(ChatId("0c"))) assertEquals( - planPushHandling("Title", "Body", p, silentSyncEnabled = { false }), - planPushHandling("Title", "Body", p, silentSyncEnabled = { true }), - ) - } - - @Test - fun `a titled push never reads the silent sync flag`() { - var reads = 0 - planPushHandling( - title = "Title", - body = "Body", - payload = payload(navigation = NavigationTrigger.Chat.ById(ChatId("aa13"))), - silentSyncEnabled = { reads++; true }, + planPushHandling(null, null, p), + planPushHandling("Title", "Body", p).filterNot { it is PushAction.PostNotification }, ) - assertEquals(0, reads) } // endregion diff --git a/docs/spikes/2026-09-08-standby-bucket-results.md b/docs/spikes/2026-09-08-standby-bucket-results.md index 5a29e0f29..5741dfa90 100644 --- a/docs/spikes/2026-09-08-standby-bucket-results.md +++ b/docs/spikes/2026-09-08-standby-bucket-results.md @@ -312,16 +312,17 @@ push woke the process — a wake effect, not push handling. **The natural cell's silence was an app that was never asked.** Not a network it was denied: see the section above for why the two look different in the capture. -**Silent preload is gated on a flag that ships off.** `planPushHandling` consults `PushSilentSync` -only when the title is null, and the flag's default is `false` (`FeatureFlag.kt:123`). A data-only -push does nothing on a default build, however well formed its payload. The flag is on for the -device under test — established by sending one silent push carrying a payload and reading -`actions=2` back, against `actions=0` for all 139 committed sends. - -This also constrains how the remaining cells can be run. A visible push cannot measure Doze: +**Silent preload was gated on a flag while these cells ran.** `planPushHandling` consulted +`PushSilentSync` when the title was null, and the flag defaulted to `false`, so a data-only push +did nothing on a default build however well formed its payload. The flag was on for the device +under test — established by sending one silent push carrying a payload and reading `actions=2` +back, against `actions=0` for all 139 committed sends. The flag has since been removed and a +data-only push plans its sync unconditionally, so a re-run needs no flag setup; every number below +was still measured with it on. + +This also constrained how the remaining cells could be run. A visible push cannot measure Doze: posting the notification lights the screen, and screen-on ends deep idle. One visible smoke push -took the device from `IDLE` to `ACTIVE`. So a Doze cell has to be silent, which makes it depend on -`PushSilentSync` being on — the flag is part of the measurement setup, not an incidental detail. +took the device from `IDLE` to `ACTIVE`. So a Doze cell has to be silent. ## Re-run with a payload the app acts on — four samples, then the phone went away @@ -605,7 +606,7 @@ present in every trace line the earlier cells were read from. **A visible push cannot be used to measure Doze.** Posting the notification lights the screen and screen-on ends deep idle; one visible smoke push took the device from `IDLE` to `ACTIVE`. Doze -cells have to be silent, and therefore depend on `PushSilentSync` being on. +cells have to be silent, which on the build these cells ran against meant `PushSilentSync` on. **`DOZE=natural` woke the device it was waiting on.** The setup pressed `KEYCODE_HOME` to background the app. HOME is a wake key, so on a screen-off phone it lit the display and restarted From a6e53b01dd638c6f875f339372e076685aacd45f Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Thu, 10 Sep 2026 20:27:02 -0400 Subject: [PATCH 19/24] test(chat): cover the write a push-carried message performs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `applyPushedMessage` was argued in review comments and asserted nowhere. Seven tests now pin the three properties the planner cannot see: the message reaches the same `ChatMessageDataSource.upsert` a fetched page uses, the event-log cursor is left where it was, and the feed row only moves forward. The cursor case is the one worth having. A push carries one message rather than a page, so seating `updateLatestEventSequence` at its sequence would let a later catch-up resume from a frontier it never fetched — a silent gap in the transcript, not a visible failure. Checked against a mutant: dropping the forward-only guard, emptying the upsert list and adding the cursor write fails four of the seven, and the three that survive are the ones those mutations do not reach. --- .../shared/chat/MessagingPushedMessageTest.kt | 135 ++++++++++++++++++ 1 file changed, 135 insertions(+) create mode 100644 apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/MessagingPushedMessageTest.kt diff --git a/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/MessagingPushedMessageTest.kt b/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/MessagingPushedMessageTest.kt new file mode 100644 index 000000000..9eb95bb98 --- /dev/null +++ b/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/MessagingPushedMessageTest.kt @@ -0,0 +1,135 @@ +package com.flipcash.shared.chat + +import com.flipcash.app.persistence.sources.ChatMessageDataSource +import com.flipcash.app.persistence.sources.ChatMetadataDataSource +import com.flipcash.services.controllers.ChatMessagingController +import com.flipcash.services.models.chat.ChatId +import com.flipcash.services.models.chat.ChatMessage +import com.flipcash.services.models.chat.MessageContent +import com.flipcash.shared.chat.internal.delegates.MessagingDelegate +import io.mockk.coEvery +import io.mockk.coVerify +import io.mockk.mockk +import kotlinx.coroutines.test.runTest +import org.junit.Test +import kotlin.time.Instant + +/** + * Covers the write a push-carried message performs in place of a fetch. + * + * Three properties matter here, and none of them are visible from the planner: the write + * reaches the same data source a fetched page would, the event cursor stays where it was, + * and the feed row only moves forward. + */ +class MessagingPushedMessageTest { + + private val chatId = ChatId("aabbccdd") + + private fun message(id: Long, eventSequence: Long, epochSeconds: Long = 1_757_000_000) = + ChatMessage( + messageId = id, + senderId = listOf(4, 5, 6), + content = listOf(MessageContent.Text("msg-$id")), + timestamp = Instant.fromEpochSeconds(epochSeconds), + unreadSeq = 0, + eventSequence = eventSequence, + ) + + private fun delegateWith( + metadataDataSource: ChatMetadataDataSource, + messageDataSource: ChatMessageDataSource = mockk(relaxed = true), + messagingController: ChatMessagingController = mockk(relaxed = true), + ) = MessagingDelegate( + chatController = mockk(relaxed = true), + messagingController = messagingController, + metadataDataSource = metadataDataSource, + messageDataSource = messageDataSource, + memberDataSource = mockk(relaxed = true), + notificationManager = mockk(relaxed = true), + userManager = mockk(relaxed = true), + stateHolder = mockk(relaxed = true), + analytics = mockk(relaxed = true), + ) + + @Test + fun `a pushed message is written through the same source a fetched page uses`() = runTest { + val pushed = message(id = 12, eventSequence = 9) + val messageDataSource = mockk(relaxed = true) + val metadataDataSource = mockk(relaxed = true) + coEvery { metadataDataSource.getLastMessageId(chatId) } returns 11 + + delegateWith(metadataDataSource, messageDataSource).applyPushedMessage(chatId, pushed) + + coVerify(exactly = 1) { messageDataSource.upsert(chatId, listOf(pushed)) } + } + + @Test + fun `applying a pushed message makes no network call`() = runTest { + val messagingController = mockk(relaxed = true) + val metadataDataSource = mockk(relaxed = true) + coEvery { metadataDataSource.getLastMessageId(chatId) } returns 0 + + delegateWith(metadataDataSource, messagingController = messagingController) + .applyPushedMessage(chatId, message(id = 1, eventSequence = 3)) + + coVerify(exactly = 0) { messagingController.getMessages(any(), any()) } + } + + @Test + fun `a pushed message does not seat the event cursor`() = runTest { + val metadataDataSource = mockk(relaxed = true) + coEvery { metadataDataSource.getLastMessageId(chatId) } returns 0 + + delegateWith(metadataDataSource).applyPushedMessage(chatId, message(id = 12, eventSequence = 99)) + + // A push carries one message, not a page. Seating the cursor at its sequence would let a + // later catch-up resume from a frontier it never fetched. + coVerify(exactly = 0) { metadataDataSource.updateLatestEventSequence(chatId, any()) } + } + + @Test + fun `a newer pushed message moves the feed row forward`() = runTest { + val metadataDataSource = mockk(relaxed = true) + coEvery { metadataDataSource.getLastMessageId(chatId) } returns 11 + + delegateWith(metadataDataSource) + .applyPushedMessage(chatId, message(id = 12, eventSequence = 9, epochSeconds = 1_757_000_042)) + + coVerify(exactly = 1) { metadataDataSource.updateLastMessageId(chatId, 12) } + coVerify(exactly = 1) { metadataDataSource.updateLastActivity(chatId, 1_757_000_042_000) } + } + + @Test + fun `a re-delivered push does not rewind the feed row`() = runTest { + val metadataDataSource = mockk(relaxed = true) + coEvery { metadataDataSource.getLastMessageId(chatId) } returns 20 + + delegateWith(metadataDataSource).applyPushedMessage(chatId, message(id = 12, eventSequence = 9)) + + coVerify(exactly = 0) { metadataDataSource.updateLastMessageId(chatId, any()) } + coVerify(exactly = 0) { metadataDataSource.updateLastActivity(chatId, any()) } + } + + @Test + fun `the same push applied twice moves the feed row once`() = runTest { + val metadataDataSource = mockk(relaxed = true) + coEvery { metadataDataSource.getLastMessageId(chatId) } returns 11 andThen 12 + val pushed = message(id = 12, eventSequence = 9) + val delegate = delegateWith(metadataDataSource) + + delegate.applyPushedMessage(chatId, pushed) + delegate.applyPushedMessage(chatId, pushed) + + coVerify(exactly = 1) { metadataDataSource.updateLastMessageId(chatId, 12) } + } + + @Test + fun `a chat with no stored message id accepts the first push`() = runTest { + val metadataDataSource = mockk(relaxed = true) + coEvery { metadataDataSource.getLastMessageId(chatId) } returns null + + delegateWith(metadataDataSource).applyPushedMessage(chatId, message(id = 1, eventSequence = 3)) + + coVerify(exactly = 1) { metadataDataSource.updateLastMessageId(chatId, 1) } + } +} From 82b4ead6dd56fe746879ebf1c3bb3402918c0363 Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Thu, 10 Sep 2026 20:34:10 -0400 Subject: [PATCH 20/24] test(persistence): pin upsert's event_sequence guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The same message reaches `ChatMessageDao.upsert` from a fetched page, the event stream and now a push that carried it, with no ordering between them. The guard is what makes arrival order stop mattering, and it had no test of its own. Eight cases against real Room rather than a mock DAO, because the behaviour under test is what SQLite stores after a REPLACE. Three of them are edges the comparison makes rather than the guard's headline: a copy at the same sequence writes through, since the comparison is strict; a copy at sequence 0 bypasses the guard entirely and can overwrite a stamped row; and a dropped write leaves `pending_client_id_hex` on the row it lost to, which is what keeps an optimistic message matchable to the server's echo. Checked against a mutant: deleting the guard fails the two cases that assert a drop and leaves the other six passing, which is the right split — the rest describe behaviour the guard is not responsible for. --- .../dao/ChatMessageUpsertGuardTest.kt | 170 ++++++++++++++++++ 1 file changed, 170 insertions(+) create mode 100644 apps/flipcash/shared/persistence/db/src/test/kotlin/com/flipcash/app/persistence/dao/ChatMessageUpsertGuardTest.kt diff --git a/apps/flipcash/shared/persistence/db/src/test/kotlin/com/flipcash/app/persistence/dao/ChatMessageUpsertGuardTest.kt b/apps/flipcash/shared/persistence/db/src/test/kotlin/com/flipcash/app/persistence/dao/ChatMessageUpsertGuardTest.kt new file mode 100644 index 000000000..059f700f0 --- /dev/null +++ b/apps/flipcash/shared/persistence/db/src/test/kotlin/com/flipcash/app/persistence/dao/ChatMessageUpsertGuardTest.kt @@ -0,0 +1,170 @@ +package com.flipcash.app.persistence.dao + +import android.content.Context +import androidx.room.Room +import androidx.test.core.app.ApplicationProvider +import com.flipcash.app.persistence.FlipcashDatabase +import com.flipcash.app.persistence.converters.MessageContentSerialized +import com.flipcash.app.persistence.entities.ChatMessageEntity +import com.flipcash.app.persistence.entities.MessageStatus +import kotlinx.coroutines.test.runTest +import org.junit.After +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import kotlin.test.assertEquals + +/** + * Covers `upsert`'s last-writer-wins guard on `event_sequence`. + * + * The same message reaches this DAO from three directions — a fetched page, the event stream, + * and now a push that carried it — with no ordering between them. The guard is what makes the + * order they arrive in stop mattering, so its edges are worth pinning: which comparison drops a + * write, which lets one through, and the sequence-0 passthrough that bypasses it entirely. + */ +@RunWith(RobolectricTestRunner::class) +class ChatMessageUpsertGuardTest { + + private lateinit var db: FlipcashDatabase + private lateinit var dao: ChatMessageDao + + @Before + fun setUp() { + val context = ApplicationProvider.getApplicationContext() + db = Room.inMemoryDatabaseBuilder(context, FlipcashDatabase::class.java) + .allowMainThreadQueries() + .build() + dao = db.chatMessageDao() + } + + @After + fun tearDown() { + db.close() + } + + private fun message( + messageId: Long = 7, + body: String, + eventSequence: Long, + ) = ChatMessageEntity( + chatIdHex = CHAT_HEX, + messageId = messageId, + senderIdHex = SENDER_HEX, + contentJson = listOf(MessageContentSerialized.Text(body)), + timestampEpochMs = messageId * 1_000, + unreadSeq = messageId, + eventSequence = eventSequence, + ) + + private suspend fun storedBody(messageId: Long = 7): String? = + (dao.getMessage(CHAT_HEX, messageId)?.contentJson?.firstOrNull() as? MessageContentSerialized.Text) + ?.text + + @Test + fun `a copy older than the stored row is dropped`() = runTest { + dao.upsert(message(body = "newer", eventSequence = 9)) + dao.upsert(message(body = "older", eventSequence = 4)) + + assertEquals("newer", storedBody()) + assertEquals(9L, dao.getEventSequence(CHAT_HEX, 7)) + } + + @Test + fun `a copy newer than the stored row replaces it`() = runTest { + dao.upsert(message(body = "older", eventSequence = 4)) + dao.upsert(message(body = "newer", eventSequence = 9)) + + assertEquals("newer", storedBody()) + assertEquals(9L, dao.getEventSequence(CHAT_HEX, 7)) + } + + /** + * The comparison is strict, so a re-delivery at the same sequence writes through rather than + * being skipped. That is the case a re-delivered push hits, and it converges because the two + * copies are the same message — not because the guard stopped the second one. + */ + @Test + fun `a copy at the same sequence writes through`() = runTest { + dao.upsert(message(body = "first", eventSequence = 9)) + dao.upsert(message(body = "second", eventSequence = 9)) + + assertEquals("second", storedBody()) + } + + /** + * Sequence 0 means "unstamped" — a legacy row, or an optimistic row the server has not + * echoed yet — and the guard lets it past unconditionally. A source that sends 0 for a + * message the server did stamp therefore overwrites a newer stored copy. + */ + @Test + fun `an unstamped copy bypasses the guard and overwrites a stamped row`() = runTest { + dao.upsert(message(body = "stamped", eventSequence = 9)) + dao.upsert(message(body = "unstamped", eventSequence = 0)) + + assertEquals("unstamped", storedBody()) + assertEquals(0L, dao.getEventSequence(CHAT_HEX, 7)) + } + + @Test + fun `a stamped copy replaces an unstamped stored row`() = runTest { + dao.upsert(message(body = "unstamped", eventSequence = 0)) + dao.upsert(message(body = "stamped", eventSequence = 9)) + + assertEquals("stamped", storedBody()) + assertEquals(9L, dao.getEventSequence(CHAT_HEX, 7)) + } + + /** + * The path a push-carried message takes: `applyPushedMessage` hands the DAO a single-element + * list, so the guard has to hold through the list overload and not only the single-entity one. + */ + @Test + fun `the list overload guards each entity on its own`() = runTest { + dao.upsert(listOf(message(messageId = 1, body = "one newer", eventSequence = 9))) + dao.upsert(listOf(message(messageId = 2, body = "two older", eventSequence = 4))) + + dao.upsert( + listOf( + message(messageId = 1, body = "one stale", eventSequence = 4), + message(messageId = 2, body = "two fresh", eventSequence = 9), + ) + ) + + assertEquals("one newer", storedBody(messageId = 1)) + assertEquals("two fresh", storedBody(messageId = 2)) + } + + @Test + fun `the guard is scoped to one chat`() = runTest { + dao.upsert(message(body = "here newer", eventSequence = 9)) + dao.upsert(message(body = "elsewhere older", eventSequence = 4).copy(chatIdHex = OTHER_HEX)) + + assertEquals("here newer", storedBody()) + assertEquals(4L, dao.getEventSequence(OTHER_HEX, 7)) + } + + /** + * A dropped write must not take the pending id with it. The row a stale copy loses to may + * still be the optimistic one awaiting confirmation, and losing `pending_client_id_hex` + * strands it — nothing can match the server's echo back to it afterwards. + */ + @Test + fun `a dropped copy leaves the pending id on the stored row`() = runTest { + dao.upsert(message(body = "newer", eventSequence = 9).copy( + status = MessageStatus.SENDING, + pendingClientIdHex = CLIENT_HEX, + )) + + dao.upsert(message(body = "older", eventSequence = 4)) + + assertEquals(CLIENT_HEX, dao.getPendingClientId(CHAT_HEX, 7)) + } + + private companion object { + const val CHAT_HEX = "aabb" + const val OTHER_HEX = "ccdd" + const val SENDER_HEX = "1122" + const val CLIENT_HEX = "eeff" + } +} From 3ebd3a2bf03fd42945c979d27b65e8f7cd8ae3d6 Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Thu, 10 Sep 2026 20:43:53 -0400 Subject: [PATCH 21/24] docs(persistence): say why the event-sequence guard is strict MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit messaging.v1 tells clients to ignore a copy whose event_sequence is at or below the version they hold. `ChatMessageDao.upsert` drops only a strictly older copy, and the difference is load-bearing: `confirmPendingMessage` stamps the server's event_sequence onto the optimistic row while leaving the content written locally, so the server's canonical copy of that message arrives at a sequence equal to the one already stored. Treating equal as "ignore" would pin the optimistic content. Also records that the sequence-0 passthrough is not reachable from the server. `event_sequence` carries `(validate.rules).uint64.gte = 1`, and every Message the server emits — GetMessages, the send echo, the event stream, and the message inlined in a push — is built by the same `Message.ToProto()` off a store-assigned sequence, so only a locally built row can arrive unstamped. Comments only. --- .../flipcash/app/persistence/dao/ChatMessageDao.kt | 12 ++++++++++-- .../persistence/dao/ChatMessageUpsertGuardTest.kt | 14 ++++++++++---- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/apps/flipcash/shared/persistence/db/src/main/kotlin/com/flipcash/app/persistence/dao/ChatMessageDao.kt b/apps/flipcash/shared/persistence/db/src/main/kotlin/com/flipcash/app/persistence/dao/ChatMessageDao.kt index a134687a6..f8151989a 100644 --- a/apps/flipcash/shared/persistence/db/src/main/kotlin/com/flipcash/app/persistence/dao/ChatMessageDao.kt +++ b/apps/flipcash/shared/persistence/db/src/main/kotlin/com/flipcash/app/persistence/dao/ChatMessageDao.kt @@ -87,8 +87,16 @@ interface ChatMessageDao { @Transaction suspend fun upsert(entity: ChatMessageEntity) { - // Event-sequence guard: skip if stored sequence is newer (last-writer-wins). - // Passthrough when eventSequence == 0 (legacy messages). + // Event-sequence guard: skip if the stored sequence is strictly newer (last-writer-wins). + // messaging.v1 tells clients to ignore a copy at or below the version they hold; the + // comparison here is strict instead, because a confirmed send needs the equal case. + // confirmPendingMessage stamps the server's event_sequence onto the optimistic row without + // replacing the content written locally, so the server's canonical copy of that message + // arrives at a sequence equal to the one already stored. Dropping it would pin the + // optimistic content forever. + // + // Passthrough when eventSequence == 0: a legacy row or an optimistic one the server has + // not echoed yet. The server cannot send 0 — messaging.v1 constrains event_sequence to >= 1. if (entity.eventSequence > 0) { val stored = getEventSequence(entity.chatIdHex, entity.messageId) if (stored != null && stored > entity.eventSequence) return diff --git a/apps/flipcash/shared/persistence/db/src/test/kotlin/com/flipcash/app/persistence/dao/ChatMessageUpsertGuardTest.kt b/apps/flipcash/shared/persistence/db/src/test/kotlin/com/flipcash/app/persistence/dao/ChatMessageUpsertGuardTest.kt index 059f700f0..670ea2d51 100644 --- a/apps/flipcash/shared/persistence/db/src/test/kotlin/com/flipcash/app/persistence/dao/ChatMessageUpsertGuardTest.kt +++ b/apps/flipcash/shared/persistence/db/src/test/kotlin/com/flipcash/app/persistence/dao/ChatMessageUpsertGuardTest.kt @@ -80,9 +80,13 @@ class ChatMessageUpsertGuardTest { } /** - * The comparison is strict, so a re-delivery at the same sequence writes through rather than - * being skipped. That is the case a re-delivered push hits, and it converges because the two - * copies are the same message — not because the guard stopped the second one. + * The comparison is strict, so a copy at the same sequence writes through rather than being + * skipped. messaging.v1 tells clients to ignore a copy at or below the held version; the + * equal case is kept because a confirmed send depends on it — confirmPendingMessage stamps + * the server's event_sequence onto the optimistic row but leaves its locally written content, + * and the canonical copy that supersedes it carries that same sequence. A re-delivered push + * hits the same path and converges because the two copies are the same message, not because + * the guard stopped the second one. */ @Test fun `a copy at the same sequence writes through`() = runTest { @@ -95,7 +99,9 @@ class ChatMessageUpsertGuardTest { /** * Sequence 0 means "unstamped" — a legacy row, or an optimistic row the server has not * echoed yet — and the guard lets it past unconditionally. A source that sends 0 for a - * message the server did stamp therefore overwrites a newer stored copy. + * message the server did stamp therefore overwrites a newer stored copy. The server is not + * such a source: messaging.v1 constrains event_sequence to >= 1, so this is reachable only + * from a locally built row. */ @Test fun `an unstamped copy bypasses the guard and overwrites a stamped row`() = runTest { From b3988eeca7241ac2fb28f7c1e35e9998bc8c5258 Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Thu, 10 Sep 2026 23:54:40 -0400 Subject: [PATCH 22/24] fix(spike): end the CPU cell when the killer takes the process measure() returns immediately once the process is dead, so after a kill the loop filled the rest of the file with `dead_before` rows in a couple of seconds. A run that died on push 4 of 8 still printed eight windows and finished early without saying why. Check for the pid before each window and break, naming the window the cell stopped at. Also drops the stale note about the feature flag, which no longer exists, and records why the window filter gates on the process being cached at the start of a window rather than at both ends: handling a push is itself what promotes the process to the previous-app slot, so requiring both ends discards every push window by construction. --- scripts/spike/measure-push-cpu.sh | 55 ++++++++++++++++++++++++++++--- 1 file changed, 51 insertions(+), 4 deletions(-) diff --git a/scripts/spike/measure-push-cpu.sh b/scripts/spike/measure-push-cpu.sh index d7b67c54b..eed2ac37e 100755 --- a/scripts/spike/measure-push-cpu.sh +++ b/scripts/spike/measure-push-cpu.sh @@ -39,8 +39,7 @@ # Set DEVICE to an adb serial when more than one device is attached. PAYLOAD is # the same base64 flipcash.push.v1.Payload the bucket runner sends, and the # default `IAU=` is category=CONTACT_JOIN, which plans RefreshFeed + -# SyncContacts. PushSilentSync must be ON for a data-only push to plan anything -# at all; with it off this script measures an empty action list. +# SyncContacts. set -euo pipefail SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" @@ -74,8 +73,34 @@ restore() { trap restore EXIT adb shell "dumpsys battery unplug" >/dev/null + +# HOME before SLEEP, and then wait for the process to actually fall out of the +# top slot. Turning the screen off on its own is not enough: the app stays the +# top of its task as `top-sleeping` at adj 0, which is neither cached nor frozen +# and keeps paying the timer work the cached case does not. The first run of this +# script measured that state for eleven windows without noticing, because it +# recorded the adj columns instead of gating on them. +# WAKEUP first: HOME sent to a sleeping device does nothing, which is how the +# first run stayed top-sleeping through the HOME it thought had backgrounded it. +adb shell "input keyevent KEYCODE_WAKEUP" >/dev/null || true +sleep 2 +adb shell "input keyevent KEYCODE_HOME" >/dev/null || true +sleep 3 adb shell "input keyevent KEYCODE_SLEEP" >/dev/null || true +adj_now() { adb shell "cat /proc/\$(pidof $PACKAGE)/oom_score_adj" 2>/dev/null | tr -d '\r'; } + +waited=0 +until adj="$(adj_now)"; [ -n "$adj" ] && [ "$adj" -ge 900 ] 2>/dev/null; do + [ "$waited" -ge 300 ] && { + echo "FATAL: $PACKAGE never reached adj >= 900 (last: ${adj:-unknown}) — nothing was measured" >&2 + exit 1 + } + sleep 5 + waited=$(( waited + 5 )) +done +echo "cached at adj $adj after ${waited} s" >&2 + # One round trip returns pid, cached-ness and the two counters together. Read # apart they can straddle a process death and produce a negative delta. sample() { @@ -118,6 +143,14 @@ measure() { measure "idle" no for i in $(seq -f '%03g' 1 "$COUNT"); do + # A kill ends the cell. Without this the loop keeps calling measure(), which + # returns immediately on a dead process, so the remaining windows land in the + # file as `dead_before` in a couple of seconds and the run looks like it + # completed its full count. Stop and say which push was the last one. + if [ -z "$(adb shell "pidof $PACKAGE" 2>/dev/null | tr -d '\r')" ]; then + echo "process gone before window $i - cell ends here" >&2 + break + fi measure "$i" yes done @@ -128,9 +161,23 @@ python3 - "$OUT" "$SETTLE" <<'PY' import statistics, sys path, settle = sys.argv[1], float(sys.argv[2]) rows = [l.split() for l in open(path) if not l.startswith('#')] -idle = [r for r in rows if r[0] == 'idle' and r[-1].isdigit()] -push = [int(r[-1]) for r in rows if r[0] != 'idle' and r[-1].isdigit()] + +# The window has to *start* cached. That is the state the killer's budget applies +# to, and the state a push has to thaw the process out of, so it is what decides +# whether the reading is the cold cost or a warm one. It cannot also *end* cached: +# handling a push promotes the process into the previous-app slot at adj 700, +# which it holds for about two minutes after the last activity. Requiring both +# ends discards every push window by construction. +def cached(r): + return r[2].isdigit() and int(r[2]) >= 900 + +complete = [r for r in rows if r[-1].isdigit()] +uncached = [r[0] for r in complete if not cached(r)] +idle = [r for r in complete if r[0] == 'idle' and cached(r)] +push = [int(r[-1]) for r in complete if r[0] != 'idle' and cached(r)] died = [r[0] for r in rows if r[-1] == 'died'] +if uncached: + print(f"dropped: {len(uncached)} window(s) that did not start cached: {uncached}") if not push: print(f"no complete push samples; {len(died)} died: {died}"); sys.exit(0) idle_ms = int(idle[0][-1]) if idle else 0 From 08a259085f20fa43445cea555bc054e9e9999433 Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Thu, 10 Sep 2026 23:59:04 -0400 Subject: [PATCH 23/24] docs(spike): size the preload against a measured push cost The 6.7 s per push in this document was inferred from log volume. Bracketing /proc//stat around each send puts it at ~2.2 s across twelve windows in three cells, with 87% of a burst inside its first two seconds. The arithmetic that follows from 6000/2200 is wrong anyway, and the cells say so: 240 s spacing killed the app twice while 120 s survived ten pushes and 420 s survived five. The kill record reconciles it. AMS charged 6230 ms where the three measured windows sum to 6210, so the instrument was right; what was wrong was the window. Those three sends are 240 s apart and AMS calls the interval between them 300043, because the window is 300 s of uptimeMillis(), which does not advance across suspend. A cadence that never lets the process freeze also never lets the device suspend, which is why the tightest spacing is as safe as the widest. Records the resulting rule: two pushes per 300 s of uptime, which the cells bracket at one push per 420 s of wall clock, with less margin than the division suggests. Also corrects the idle table, which gave adj 905 as the kill band. The kills here were at 700 and 900; AMS checks setProcState >= PROCESS_STATE_HOME, so the promotion a push earns does not put the process out of reach. --- .../2026-09-08-standby-bucket-results.md | 153 ++++++++++++++---- 1 file changed, 119 insertions(+), 34 deletions(-) diff --git a/docs/spikes/2026-09-08-standby-bucket-results.md b/docs/spikes/2026-09-08-standby-bucket-results.md index 5741dfa90..52a30a39d 100644 --- a/docs/spikes/2026-09-08-standby-bucket-results.md +++ b/docs/spikes/2026-09-08-standby-bucket-results.md @@ -629,7 +629,7 @@ off and the phone physically unplugged, the counters do not move at all: |---|---| | Samples | 16, at 60 s | | Span | 906 s | -| `oom_score_adj` | 910 throughout (the kills were at 905) | +| `oom_score_adj` | 910 throughout (kills have been seen at 700, 900 and 905) | | `cgroup.freeze` | `/sys/fs/cgroup/apps/uid_10309/pid_5303/cgroup.freeze` = 1 | | utime + stime | 2221 + 199 ticks at the first sample, and at all fifteen after it | | Delta | **0 ticks** | @@ -642,44 +642,129 @@ third row: the process is frozen, so it is not sleeping cheaply, it is not runni across windows in which pushes arrived every 180 s, so it is the residue either side of a burst — thaw, timer catch-up, the stream's ping timer — attributed to the gap it sat in. Measured with nothing arriving, the floor is zero. The practical consequence is that the budget has one term: the -whole 6000 ms is available to push work, and the answer depends only on what a push costs. +whole 6000 ms is available to push work, and what remains is the cost of one push and how many of +them a window holds. -### The answer, at the cost we have +### The answer, at the measured cost -The per-push term is still the regression's **~6.7 s**, and it is an upper bound rather than a -reading. `TraceManager.includeRpcBodies` was on for the capture build — it follows `BuildConfig.DEBUG` -and `ReleaseStage.Internal` — so `LoggingClientCallListener` built a full proto `toString()` for every -RPC in every burst, roughly 55 KB of string per push that production never builds. +The instrument has now been run. `scripts/spike/measure-push-cpu.sh` brackets `/proc//stat` +utime+stime around each send, takes a push-free window first for the idle term, and gates each window +on the process having actually reached cached before the push lands. -Taken at that bound, with idle at zero: +Twelve endpoint brackets across three cells, plus one burst sampled at 2 s: -| | | -|---|---| -| Budget per 300 s window | 6000 ms | -| Cost of one push | ~6700 ms | -| Pushes per five minutes | **0.90** | -| Break-even cadence | 335 s | +| Cell | Spacing | n | Per-push (ms) | Mean | +|---|---|---|---|---| +| 240 s, first attempt | 240 s | 4 | 1810, 2200, 1830, 2180 | 2005 | +| 240 s, repeat | 240 s | 3 | 1930, 1990, 2290 | 2070 | +| 420 s | 420 s | 5 | 2680, 2210, 2430, 1970, 2360 | 2330 | +| 2 s profile | single push | 1 | 2030 total | — | + +**A push costs about 2.2 s of CPU, not the 6.7 s the regression inferred.** That number came from log +volume; read off the counters it is a third of the size. Bodies are still on for every figure here — +this is a `debug` install, so `includeRpcBodies` is true and each RPC still builds a full proto +`toString()` — so 2.2 s is also an upper bound on production, just a much tighter one. + +The profile says where it goes: cumulative 1760 ms at t=2 s of a 2030 ms total, then roughly 10 ms +per 2 s out to t≈120 s. **87% of a burst is in its first two seconds.** The tail is the adj-700 +previous-app decay winding down, not work worth trimming. + +### The 300 s window is uptime, not wall clock + +Dividing the budget by the cost gives 6000 / 2200 = 2.7 pushes per five minutes. The device +disagrees, and not in the direction a margin would explain: + +| Cell | Process between pushes | Pushes | Outcome | +|---|---|---|---| +| 120 s spacing | never frozen, adj 0 then 700 | 10 | survived | +| 240 s spacing | cached and frozen | 5 | killed on #5 | +| 240 s spacing, repeat | cached and frozen | 4 | killed on #4 | +| 420 s spacing | cached and frozen | 5 | survived | + +The 120 s row is the voided first attempt, whose readings are unusable because it never backgrounded +the app. Whether the process survived is not a reading, so that row still counts, and it is the row +that makes the pattern impossible to read as a rate: ten pushes at the tightest spacing survived +while four at an intermediate spacing did not. No rate expressed in wall-clock seconds produces that +ordering. The kill record from the repeat cell +resolves it: + +``` +am_kill: [0,12639,com.flipcash.app.android,900,excessive cpu 6230 during 300043 dur=1750357 limit=2,249060] +``` + +6230 ms charged. The three windows measured for that process sum to 1930 + 1990 + 2290 = **6210 ms**. +The instrument and AMS agree to 0.3%, which retires the discrepancy this document previously could +not account for. But those three bursts are 240 s apart, so first to last is at least 480 s of wall +clock, and AMS calls the interval between them `300043`. **The window is 300 s of `uptimeMillis()`, +which does not advance across suspend.** + +How much wall clock a window covers is then a question about how much the phone suspends. Two +readings of `dumpsys batterystats`, 421 s apart with the device asleep and no pushes arriving, differ +by 142 s of uptime: **33.7%**, or a 300 s window stretched across 890 s. Over the whole 14 h on +battery the figure is 38.4%. Neither is the number that applies during a push cadence, because each +push wakes the device and buys back uptime. + +The cells bracket that number better than either aggregate does. Three sends 240 s apart were charged +to one window, so the window covers more than 480 s of wall clock. The 420 s cell survived, so fewer +than three of its sends fit, putting the window at 840 s or less. A 300 s uptime window during a +cadence like this therefore spans between roughly 480 s and 840 s, and every cell follows: -**The app cannot absorb even one push per five minutes.** That is the same conclusion the regression -reached, and removing the idle term does not soften it: idle was never spending the budget, so there -is nothing there to reclaim. A push has to come in under 6000 ms of CPU to survive one per window at -all, under 3000 ms for two, and under 1500 ms for the four-per-five-minutes a chat preload would -imply. The measured cost misses the first of those thresholds by 12% and the last by 4.5x. +| Cell | Wall clock per window | Pushes inside it | CPU | vs 6000 ms | +|---|---|---|---|---| +| 120 s, never frozen — device stays awake, so uptime ≈ realtime | 300 s | 2.5 | ~5500 ms | under, survived | +| 240 s, frozen | 480-840 s | 2.0-3.5 | 4400-7700 ms | over at the observed 3, killed twice | +| 420 s, frozen | 480-840 s | 1.1-2.0 | 2500-4400 ms | under, survived | + +Keeping the process out of the freezer is also what keeps the device out of suspend, so the tightest +cadence is safe for the same reason the widest one is: neither manages to compress three bursts into +a single window. + +**One correction to the idle table further up.** It recorded the kills as being at `adj 905`, which +reads as though the killer only reaches fully cached processes. It does not. The two kills in these +cells name `adj 700` and `adj 900`, and AMS checks any process at `setProcState >= PROCESS_STATE_HOME`, +which +includes the previous-app slot. Handling a push promotes the process from 900 to 700 for about 135 s; +that promotion does not buy it immunity. + +### What this means for the preload + +The budget is 6000 ms per 300 s of uptime and a push costs ~2.2 s, so the app can absorb **two pushes +per uptime window**. The third is what killed it in both 240 s cells. + +Turning that into a cadence needs the suspend ratio, which is a property of the user's device and +day rather than of the app. Two pushes per 840 s is **one push every seven minutes**, which is what +420 s delivered: five consecutive pushes, no kill, and by the same bracket 2500-4400 ms against a +6000 ms budget. 240 s killed in both attempts. Treat 420 s as the fastest cadence observed to be +safe, and note the margin is thinner than the ratio 6000/2200 suggests — at the unfavourable end of +the bracket, 420 s is already using 73% of the budget. + +A chat preload driven by message arrival will exceed that on any active conversation, which leaves +two options, and the burst shape chooses between them. Coalescing server-side costs one burst per +window whatever the message volume. Trimming the handler has 87% of its target inside the first two +seconds, in the sync RPC fan-out, and almost nothing in the tail — so it is a fan-out problem, not a +timer problem. ### What this does not establish -The per-push number was not re-measured. `scripts/spike/measure-push-cpu.sh` is the instrument for -it: the same `/proc` bracket used above, applied around each send, with a push-free window first for -the idle term. It needs the device's FCM registration token, which `scripts/fcm.sh` takes as its -first argument, and it has not been run. - -Two things that run would settle. The first is how much of 6.7 s is the RPC-body logging: the -`benchmark` variant is `initWith(debug)` with `isDebuggable = false`, which turns `BuildConfig.DEBUG` -off, and it shares the `contributors` signing key with `debug`, so it installs over the existing -build and keeps the login and the stored flag value. Running the same cell on both variants -attributes the difference. The second is whether the four fixes listed above move the number, since -each of them targets work inside the first 7.7 s of a burst, which is where 95% of it is. - -Generality is unchanged from the rest of this document: one device, one OEM, one Android version. -The idle measurement adds one caveat of its own — the phone was at `deep=INACTIVE`, not in Doze. -Doze would only make idle cheaper, so the zero holds as a floor for the deeper states too. +Bodies were on for all of it. Production sets `includeRpcBodies` false, so the real per-push cost is +below 2.2 s by a margin nothing here measures. The `benchmark` variant would attribute it: it is +`initWith(debug)` with `isDebuggable = false` and shares the `contributors` signing key, so it +installs over the existing build and keeps the login. That run replaces the app on the test device +and was not made. + +The four fixes listed above are unmeasured against the new baseline. Each targets work inside the +first seconds of a burst, which is still where the cost sits, but 2.2 s leaves less to reclaim than +6.7 s did. + +The first 240 s cell's kill charged 7190 ms where its four preceding windows sum to 8020. A window +boundary falling inside a burst rather than between bursts would explain it, but that was not +confirmed; only the repeat cell's charge lines up closely enough to stand as evidence by itself. + +The suspend ratio during a cadence was bracketed, not measured. The 480-840 s span comes from which +cells lived and died, so it is only as tight as the two spacings tried; a cell between them would +narrow it. The direct readings either side — 33.7% over a quiet window, 38.4% over the battery's +lifetime — are both from periods that are not a push cadence. + +Generality is unchanged from the rest of this document: one device, one OEM, one Android version. The +uptime ratio travels worst of all the numbers here, since it is set by how much a given phone +suspends, which varies by device, by user, and by whatever else is installed. From 4e0a542dd6d62d92cb0462ecd45e4ae693bf0889 Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Fri, 11 Sep 2026 11:22:06 -0400 Subject: [PATCH 24/24] refactor(chat): resolve stream messages from events only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ChatUpdate.new_messages is deprecated in favour of the sequenced, gap-detectable events field — the proto states new messages now arrive as events, and the backend sends new_messages empty. The fallback branch in applyUpdate was therefore unreachable. Drop newMessages from the ChatUpdate domain model, its protobuf mapper, and the EventStreamingController trace, which now reports the events count. ChatCoordinatorEventsTest's two fallback cases covered behaviour that no longer exists, so they go. ReceivedEventTest, ReceivedCounterTest and ChatCoordinatorEagerBalanceTest build their fixtures on events instead — they cover receipts, counters and eager balance rather than the removed field. Their event sequences run contiguously from 1 so the gap detector stays out of the way. --- .../internal/delegates/EventStreamDelegate.kt | 19 +++---- .../chat/ChatCoordinatorEagerBalanceTest.kt | 16 +++++- .../shared/chat/ChatCoordinatorEventsTest.kt | 51 +------------------ .../shared/chat/ReceivedCounterTest.kt | 19 +++++-- .../flipcash/shared/chat/ReceivedEventTest.kt | 19 +++++-- .../controllers/EventStreamingController.kt | 2 +- .../network/extensions/ProtobufToLocal.kt | 2 - .../services/models/chat/ChatUpdate.kt | 2 - .../services/models/chat/DomainModelsTest.kt | 15 ++++-- 9 files changed, 66 insertions(+), 79 deletions(-) diff --git a/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/internal/delegates/EventStreamDelegate.kt b/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/internal/delegates/EventStreamDelegate.kt index f2123b6bf..387aaf37b 100644 --- a/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/internal/delegates/EventStreamDelegate.kt +++ b/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/internal/delegates/EventStreamDelegate.kt @@ -51,8 +51,8 @@ import kotlin.time.Duration.Companion.seconds * local persistence and in-memory state. * * Responsibilities: - * - **Message persistence** — resolves messages from `ChatUpdate.events` (preferred) - * or the deprecated `newMessages` field, then upserts to Room. + * - **Message persistence** — resolves messages from `ChatUpdate.events`, then + * upserts to Room. * - **Gap-aware event sequencing** — uses [EventSequenceTracker] to maintain a * contiguous frontier. Only the highest contiguous sequence is persisted; if a * gap is detected, a timed [getDelta][performDeltaSync] backfill is scheduled. @@ -290,17 +290,12 @@ class EventStreamDelegate @Inject constructor( private suspend fun applyUpdate(update: ChatUpdate) { val chatId = update.chatId - // --- Resolve messages: prefer events, fall back to deprecated newMessages --- + // --- Resolve messages from the event log --- - val resolvedMessages = if (update.events.isNotEmpty()) { - update.events - .flatMap { event -> event.mutations.map { it.message } } - .sortedBy { it.eventSequence } - .distinctBy { it.messageId } - } else { - @Suppress("DEPRECATION") - update.newMessages - } + val resolvedMessages = update.events + .flatMap { event -> event.mutations.map { it.message } } + .sortedBy { it.eventSequence } + .distinctBy { it.messageId } trace( tag = TAG, diff --git a/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/ChatCoordinatorEagerBalanceTest.kt b/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/ChatCoordinatorEagerBalanceTest.kt index 33542d431..af581eae0 100644 --- a/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/ChatCoordinatorEagerBalanceTest.kt +++ b/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/ChatCoordinatorEagerBalanceTest.kt @@ -9,8 +9,10 @@ import com.flipcash.app.tokens.TokenCoordinator import com.flipcash.services.controllers.ChatController import com.flipcash.services.controllers.ChatMessagingController import com.flipcash.services.controllers.EventStreamingController +import com.flipcash.services.models.chat.ChatEvent import com.flipcash.services.models.chat.ChatId import com.flipcash.services.models.chat.ChatMessage +import com.flipcash.services.models.chat.ChatMutation import com.flipcash.services.models.chat.ChatUpdate import com.flipcash.services.models.chat.MessageContent import com.flipcash.shared.chat.internal.ChatIdGenerator @@ -156,12 +158,24 @@ class ChatCoordinatorEagerBalanceTest { unreadSeq = 0, ) + // Each update carries its messages as event-log events. Sequences run + // contiguously from 1 across the whole test so the gap detector stays quiet + // and these tests exercise only the behaviour they name. + private var nextEventSequence = 1L + private fun chatUpdate(vararg messages: ChatMessage) = ChatUpdate( chatId = chatId, - newMessages = messages.toList(), pointerUpdates = emptyList(), typingNotifications = emptyList(), metadataUpdates = emptyList(), + events = messages.map { message -> + ChatEvent( + sequence = nextEventSequence++, + count = 1, + ts = message.timestamp, + mutations = listOf(ChatMutation.MessageSent(message)), + ) + }, ) private suspend fun triggerCollection() { diff --git a/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/ChatCoordinatorEventsTest.kt b/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/ChatCoordinatorEventsTest.kt index 3a44e72b1..c9757aec7 100644 --- a/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/ChatCoordinatorEventsTest.kt +++ b/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/ChatCoordinatorEventsTest.kt @@ -161,56 +161,7 @@ class ChatCoordinatorEventsTest { coordinator.onUserLoggedIn(mockk(relaxed = true)) } - // region Events vs newMessages - - @Test - fun `events are preferred over deprecated newMessages`() = runTest(testDispatchers.dispatcher) { - triggerCollection() - - val eventMsg = textMessage(id = 1, eventSequence = 1) - val deprecatedMsg = textMessage(id = 99) - - @Suppress("DEPRECATION") - val update = ChatUpdate( - chatId = chatId, - newMessages = listOf(deprecatedMsg), - events = listOf(chatEvent(1, eventMsg)), - ) - chatUpdatesChannel.send(update) - advanceTimeBy(1_000.milliseconds) - runCurrent() - - // Should upsert the event message, not the deprecated one - coVerify { - messageDataSource.upsert(chatId, match { messages -> - messages.size == 1 && messages[0].messageId == 1L - }) - } - coordinator.teardown() - } - - @Test - fun `falls back to newMessages when events is empty`() = runTest(testDispatchers.dispatcher) { - triggerCollection() - - val msg = textMessage(id = 42) - @Suppress("DEPRECATION") - val update = ChatUpdate( - chatId = chatId, - newMessages = listOf(msg), - events = emptyList(), - ) - chatUpdatesChannel.send(update) - advanceTimeBy(1_000.milliseconds) - runCurrent() - - coVerify { - messageDataSource.upsert(chatId, match { messages -> - messages.size == 1 && messages[0].messageId == 42L - }) - } - coordinator.teardown() - } + // region Event message resolution @Test fun `multiple events are flattened and deduped by messageId`() = runTest(testDispatchers.dispatcher) { diff --git a/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/ReceivedCounterTest.kt b/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/ReceivedCounterTest.kt index 218ee6473..6bf34ce80 100644 --- a/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/ReceivedCounterTest.kt +++ b/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/ReceivedCounterTest.kt @@ -11,8 +11,10 @@ import com.flipcash.app.tokens.TokenCoordinator import com.flipcash.services.controllers.ChatController import com.flipcash.services.controllers.ChatMessagingController import com.flipcash.services.controllers.EventStreamingController +import com.flipcash.services.models.chat.ChatEvent import com.flipcash.services.models.chat.ChatId import com.flipcash.services.models.chat.ChatMessage +import com.flipcash.services.models.chat.ChatMutation import com.flipcash.services.models.chat.ChatUpdate import com.flipcash.services.models.chat.MessageContent import com.flipcash.services.user.UserManager @@ -180,15 +182,24 @@ class ReceivedCounterTest { unreadSeq = 0, ) - // newMessages is deprecated in favour of `events`, but applyUpdate still falls - // back to it and every existing chat test builds updates this way. Matching the - // existing harness keeps these tests readable next to their neighbours. + // Each update carries its messages as event-log events. Sequences run + // contiguously from 1 across the whole test so the gap detector stays quiet + // and these tests exercise only the behaviour they name. + private var nextEventSequence = 1L + private fun chatUpdate(vararg messages: ChatMessage) = ChatUpdate( chatId = chatId, - newMessages = messages.toList(), pointerUpdates = emptyList(), typingNotifications = emptyList(), metadataUpdates = emptyList(), + events = messages.map { message -> + ChatEvent( + sequence = nextEventSequence++, + count = 1, + ts = message.timestamp, + mutations = listOf(ChatMutation.MessageSent(message)), + ) + }, ) private suspend fun TestScope.deliver(vararg messages: ChatMessage) { diff --git a/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/ReceivedEventTest.kt b/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/ReceivedEventTest.kt index ba845a3f6..8ffa9ce54 100644 --- a/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/ReceivedEventTest.kt +++ b/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/ReceivedEventTest.kt @@ -10,8 +10,10 @@ import com.flipcash.app.tokens.TokenCoordinator import com.flipcash.services.controllers.ChatController import com.flipcash.services.controllers.ChatMessagingController import com.flipcash.services.controllers.EventStreamingController +import com.flipcash.services.models.chat.ChatEvent import com.flipcash.services.models.chat.ChatId import com.flipcash.services.models.chat.ChatMessage +import com.flipcash.services.models.chat.ChatMutation import com.flipcash.services.models.chat.ChatType import com.flipcash.services.models.chat.ChatUpdate import com.flipcash.services.models.chat.MessageContent @@ -183,15 +185,24 @@ class ReceivedEventTest { unreadSeq = 0, ) - // newMessages is deprecated in favour of `events`, but applyUpdate still falls - // back to it and every existing chat test builds updates this way. Matching the - // existing harness keeps these tests readable next to their neighbours. + // Each update carries its messages as event-log events. Sequences run + // contiguously from 1 across the whole test so the gap detector stays quiet + // and these tests exercise only the behaviour they name. + private var nextEventSequence = 1L + private fun chatUpdate(vararg messages: ChatMessage) = ChatUpdate( chatId = chatId, - newMessages = messages.toList(), pointerUpdates = emptyList(), typingNotifications = emptyList(), metadataUpdates = emptyList(), + events = messages.map { message -> + ChatEvent( + sequence = nextEventSequence++, + count = 1, + ts = message.timestamp, + mutations = listOf(ChatMutation.MessageSent(message)), + ) + }, ) private suspend fun TestScope.deliver(vararg messages: ChatMessage) { diff --git a/services/flipcash/src/main/kotlin/com/flipcash/services/controllers/EventStreamingController.kt b/services/flipcash/src/main/kotlin/com/flipcash/services/controllers/EventStreamingController.kt index 4a0269142..32aa71cc9 100644 --- a/services/flipcash/src/main/kotlin/com/flipcash/services/controllers/EventStreamingController.kt +++ b/services/flipcash/src/main/kotlin/com/flipcash/services/controllers/EventStreamingController.kt @@ -68,7 +68,7 @@ class EventStreamingController @Inject constructor( scope = scope, owner = owner, onEvent = { update -> - trace("EventStreamingController: Received chat update, messages=${update.newMessages.size}") + trace("EventStreamingController: Received chat update, events=${update.events.size}") _chatUpdates.trySend(update) }, onBlobUpdate = { update -> diff --git a/services/flipcash/src/main/kotlin/com/flipcash/services/internal/network/extensions/ProtobufToLocal.kt b/services/flipcash/src/main/kotlin/com/flipcash/services/internal/network/extensions/ProtobufToLocal.kt index 4797be0b8..c6a67f9dc 100644 --- a/services/flipcash/src/main/kotlin/com/flipcash/services/internal/network/extensions/ProtobufToLocal.kt +++ b/services/flipcash/src/main/kotlin/com/flipcash/services/internal/network/extensions/ProtobufToLocal.kt @@ -406,13 +406,11 @@ internal fun ChatModel.Metadata.toChatMetadata(): ChatMetadata { // -- EventModel.ChatUpdate -- -@Suppress("DEPRECATION") internal fun EventModel.ChatUpdate.toChatUpdate( metadataMapper: (ChatModel.Metadata) -> ChatMetadata = { it.toChatMetadata() }, ): ChatUpdate { return ChatUpdate( chatId = chat.toChatId(), - newMessages = if (hasNewMessages()) newMessages.messagesList.map { it.toChatMessage() } else emptyList(), pointerUpdates = if (hasPointerUpdates()) pointerUpdates.pointersList.map { it.toPointer() } else emptyList(), typingNotifications = if (hasIsTypingNotifications()) isTypingNotifications.isTypingNotificationsList.map { it.toTypingNotification() } else emptyList(), metadataUpdates = metadataUpdatesList.map { it.toMetadataUpdate(metadataMapper) }, diff --git a/services/flipcash/src/main/kotlin/com/flipcash/services/models/chat/ChatUpdate.kt b/services/flipcash/src/main/kotlin/com/flipcash/services/models/chat/ChatUpdate.kt index 5d541ae4b..e20312423 100644 --- a/services/flipcash/src/main/kotlin/com/flipcash/services/models/chat/ChatUpdate.kt +++ b/services/flipcash/src/main/kotlin/com/flipcash/services/models/chat/ChatUpdate.kt @@ -2,8 +2,6 @@ package com.flipcash.services.models.chat data class ChatUpdate( val chatId: ChatId, - @Deprecated("Use events instead", replaceWith = ReplaceWith("events")) - val newMessages: List = emptyList(), val pointerUpdates: List = emptyList(), val typingNotifications: List = emptyList(), val metadataUpdates: List = emptyList(), diff --git a/services/flipcash/src/test/kotlin/com/flipcash/services/models/chat/DomainModelsTest.kt b/services/flipcash/src/test/kotlin/com/flipcash/services/models/chat/DomainModelsTest.kt index c910523a0..b61b65685 100644 --- a/services/flipcash/src/test/kotlin/com/flipcash/services/models/chat/DomainModelsTest.kt +++ b/services/flipcash/src/test/kotlin/com/flipcash/services/models/chat/DomainModelsTest.kt @@ -55,8 +55,17 @@ class DomainModelsTest { val chatId = ChatId(ByteArray(32)) val update = ChatUpdate( chatId = chatId, - newMessages = listOf( - ChatMessage(1, null, listOf(MessageContent.Text("hi")), Instant.fromEpochSeconds(0), 1) + events = listOf( + ChatEvent( + sequence = 1, + count = 1, + ts = Instant.fromEpochSeconds(0), + mutations = listOf( + ChatMutation.MessageSent( + ChatMessage(1, null, listOf(MessageContent.Text("hi")), Instant.fromEpochSeconds(0), 1) + ) + ), + ) ), pointerUpdates = listOf( MessagePointer(PointerType.READ, listOf(1.toByte()), 5, Instant.fromEpochSeconds(0)) @@ -69,7 +78,7 @@ class DomainModelsTest { ), ) - assertEquals(1, update.newMessages.size) + assertEquals(1, update.events.size) assertEquals(1, update.pointerUpdates.size) assertEquals(1, update.typingNotifications.size) assertEquals(1, update.metadataUpdates.size)