perf(wallet): stop the login spinner waiting on three network races

apps/flipcash/core/src/main/kotlin/com/flipcash/app/core/updater/NetworkUpdater.kt

@@ -10,16 +10,31 @@ import kotlin.time.Duration.Companion.seconds
 
 abstract class NetworkUpdater {
     private var job: Job? = null
+    private var activeKey: Any? = null
 
     protected abstract suspend fun doUpdate()
 
+    /**
+     * Starts the polling loop, or leaves an equivalent one already running alone.
+     *
+     * The idempotence matters because callers arrive from lifecycle edges that can fire twice in
+     * quick succession — `SessionController.onAppInForeground` is invoked both by the transition
+     * into `AuthState.Ready` and by `ON_RESUME`, and at login those land together. Restarting
+     * unconditionally cancelled the in-flight [doUpdate] and re-served [startIn], so the first
+     * fetch of a fresh session was pushed further out by the very event that asked for it.
+     *
+     * Backgrounding and logout still reset the loop, through [stop].
+     */
     fun poll(
         key: Any? = null,
         scope: CoroutineScope,
         frequency: Duration,
         startIn: Duration = 0.seconds,
     ) {
+        if (job?.isActive == true && activeKey == key) return
+
         stop()
+        activeKey = key
         job = scope.launch {
             delay(startIn)
             while (isActive) {
@@ -32,5 +47,6 @@ abstract class NetworkUpdater {
     fun stop() {
         job?.cancel()
         job = null
+        activeKey = null
     }
 }

apps/flipcash/core/src/test/kotlin/com/flipcash/app/core/updater/NetworkUpdaterTest.kt

@@ -0,0 +1,86 @@
+package com.flipcash.app.core.updater
+
+import kotlinx.coroutines.ExperimentalCoroutinesApi
+import kotlinx.coroutines.test.advanceTimeBy
+import kotlinx.coroutines.test.runTest
+import kotlin.test.Test
+import kotlin.test.assertEquals
+import kotlin.time.Duration.Companion.milliseconds
+import kotlin.time.Duration.Companion.seconds
+
+/**
+ * `SessionController.onAppInForeground` is invoked both by the transition into `AuthState.Ready`
+ * and by `ON_RESUME`, and at login those arrive together. A poller that restarted on every call
+ * cancelled the fetch the first call had started and re-served its start delay, so the second
+ * event pushed the first fetch of a fresh session further away.
+ *
+ * Time is advanced in bounded steps rather than with `advanceUntilIdle`, which does not run
+ * `backgroundScope`'s tasks — and an unbounded advance would not terminate against a poll loop.
+ */
+@OptIn(ExperimentalCoroutinesApi::class)
+class NetworkUpdaterTest {
+
+    private class CountingUpdater : NetworkUpdater() {
+        var updates = 0
+            private set
+
+        override suspend fun doUpdate() {
+            updates++
+        }
+    }
+
+    @Test
+    fun `a repeated poll leaves the running loop alone`() = runTest {
+        val updater = CountingUpdater()
+
+        updater.poll(scope = backgroundScope, frequency = 20.seconds)
+        advanceTimeBy(1.milliseconds)
+        assertEquals(1, updater.updates, "the first poll fetches immediately")
+
+        updater.poll(scope = backgroundScope, frequency = 20.seconds)
+        advanceTimeBy(1.seconds)
+        assertEquals(1, updater.updates, "the duplicate must not re-fetch off-cadence")
+
+        advanceTimeBy(20.seconds)
+        assertEquals(2, updater.updates, "and the original cadence is intact")
+    }
+
+    @Test
+    fun `a repeated poll does not re-serve the start delay`() = runTest {
+        val updater = CountingUpdater()
+
+        updater.poll(scope = backgroundScope, frequency = 20.seconds, startIn = 2.seconds)
+        advanceTimeBy(1.seconds)
+        updater.poll(scope = backgroundScope, frequency = 20.seconds, startIn = 2.seconds)
+
+        // t = 2.5s. On the original schedule the fetch has landed; a restart would have pushed it
+        // to t = 3s.
+        advanceTimeBy(1500.milliseconds)
+        assertEquals(1, updater.updates, "the fetch lands on the original schedule, not a second late")
+    }
+
+    @Test
+    fun `stopping releases the loop so the next foreground restarts it`() = runTest {
+        val updater = CountingUpdater()
+
+        updater.poll(scope = backgroundScope, frequency = 20.seconds)
+        advanceTimeBy(1.milliseconds)
+        updater.stop()
+
+        updater.poll(scope = backgroundScope, frequency = 20.seconds)
+        advanceTimeBy(1.milliseconds)
+        assertEquals(2, updater.updates)
+    }
+
+    @Test
+    fun `a poll under a different key replaces the running loop`() = runTest {
+        val updater = CountingUpdater()
+
+        updater.poll(key = "a", scope = backgroundScope, frequency = 20.seconds)
+        advanceTimeBy(1.milliseconds)
+
+        updater.poll(key = "b", scope = backgroundScope, frequency = 20.seconds)
+        advanceTimeBy(1.milliseconds)
+        assertEquals(2, updater.updates)
+    }
+}

apps/flipcash/features/balance/src/main/kotlin/com/flipcash/app/balance/internal/WalletViewModel.kt

@@ -21,7 +21,6 @@ import dagger.hilt.android.lifecycle.HiltViewModel
 import kotlinx.coroutines.flow.combine
 import kotlinx.coroutines.flow.filter
 import kotlinx.coroutines.flow.filterIsInstance
-import kotlinx.coroutines.flow.filterNotNull
 import kotlinx.coroutines.flow.flatMapLatest
 import kotlinx.coroutines.flow.launchIn
 import kotlinx.coroutines.flow.mapNotNull
@@ -49,8 +48,22 @@ internal class WalletViewModel @Inject constructor(
          * Onboarding milestones, or `null` while they are still unknown. The distinction matters:
          * an empty/incomplete checklist is what draws the new-user tutorial, and every milestone
          * reads as incomplete before its source has reported.
+         *
+         * Published off local reads (the feed's own cache and the held balance), so this lands
+         * without waiting on the network. The tip milestone inside it is the one that needs a
+         * server round-trip; [isTipMilestoneResolved] says whether it can be believed yet.
          */
         val onboardingItems: List<TutorialItem>? = null,
+        /**
+         * Whether [TutorialItem.ScanTipCard]'s answer is trustworthy.
+         *
+         * The tip milestone is read off the chat cache, which reports every account as never
+         * having tipped until its history has been reconciled with the server — a wait that
+         * scales with how many conversations the account has. Only the tutorial depends on that
+         * answer, so only the tutorial waits for it (see [isNewUserTutorialComplete]); the rest
+         * of the tab draws off state it already has, as iOS does.
+         */
+        val isTipMilestoneResolved: Boolean = false,
         /**
          * Preview of the most recent unified cross-token activity — at most [RECENT_PREVIEW_COUNT]
          * rows. The coordinator owns the mapping and enforces the limit; the full paged history is a
@@ -64,9 +77,15 @@ internal class WalletViewModel @Inject constructor(
         val hasReceivedMoney: Boolean
             get() = onboardingItems?.find { it is TutorialItem.AddMoney }?.isCompleted == true
 
-        /** Treated as complete while unknown, so the tutorial is never the thing we guess at. */
+        /**
+         * Treated as complete while unknown, so the tutorial is never the thing we guess at.
+         *
+         * "Unknown" covers the un-reconciled chat cache as well as absent milestones: until
+         * [isTipMilestoneResolved], `ScanTipCard` reads incomplete for everyone, and drawing that
+         * would tell an established tipper to go and scan a tip card.
+         */
         val isNewUserTutorialComplete: Boolean
-            get() = onboardingItems?.all { it.isCompleted } != false
+            get() = !isTipMilestoneResolved || onboardingItems?.all { it.isCompleted } != false
 
         /**
          * Whether the activity half of the tab is still settling.
@@ -79,11 +98,11 @@ internal class WalletViewModel @Inject constructor(
          * mistake for a new account — and neither is a held balance, which is a live read of the
          * account rather than of the cache.
          *
-         * The two caches are separate and settle separately. The `feedSyncState` clause covers the
-         * activity feed; the *chat* cache backing the tip milestone is covered by
-         * [onboardingItems] staying null, because the milestone flow withholds an answer until its
-         * own history has hydrated. A held balance deliberately does not short-circuit that one:
-         * it says nothing about whether the account has tipped.
+         * Scoped to the activity feed on purpose. The *chat* cache backing the tip milestone
+         * settles separately and far later — its hydration waits on a per-conversation backfill —
+         * and the only thing that reads it is the tutorial, which withholds itself while unsure
+         * (see [isNewUserTutorialComplete]). Holding the whole tab for it meant the balance and
+         * the card deck, both long since resolved, waited on an answer neither of them uses.
          */
         val isAwaitingActivity: Boolean
             get() = onboardingItems == null ||
@@ -94,6 +113,7 @@ internal class WalletViewModel @Inject constructor(
         data class OnOnboardingItemsUpdated(
             val items: List<TutorialItem>,
             val holdsBalance: Boolean,
+            val isTipMilestoneResolved: Boolean,
         ): Event
         data class OnTransactionsUpdated(val transactions: List<TransactionListItem>) : Event
         data class OnPreferredOnRampProviderChanged(val provider: OnRampProvider.Defined?) : Event
@@ -145,20 +165,21 @@ internal class WalletViewModel @Inject constructor(
             tokenCoordinator.hasAnyBalance,
         ) { hasReceivedMoney, hasTipped, holdsBalance ->
             // A null tip milestone means the chat cache has not been reconciled yet, and an
-            // un-hydrated cache reports every account as never having tipped. Emitting nothing
-            // keeps [State.onboardingItems] null, which holds the whole tab on its spinner rather
-            // than drawing "Scan a Tip Card" as outstanding to someone who already did it.
-            hasTipped?.let {
-                Event.OnOnboardingItemsUpdated(
-                    items = listOf(
-                        TutorialItem.AddMoney(isCompleted = hasReceivedMoney || holdsBalance),
-                        TutorialItem.ScanTipCard(isCompleted = it),
-                    ),
-                    holdsBalance = holdsBalance,
-                )
-            }
+            // un-hydrated cache reports every account as never having tipped. That is carried as
+            // `isTipMilestoneResolved` rather than by withholding the emission: the other
+            // milestone and the action tiles are answerable from local state immediately, and
+            // holding them back put the chat backfill on the critical path for the whole tab.
+            // "Scan a Tip Card" is still never drawn as outstanding to someone who already did it
+            // — [State.isNewUserTutorialComplete] reads complete until this resolves.
+            Event.OnOnboardingItemsUpdated(
+                items = listOf(
+                    TutorialItem.AddMoney(isCompleted = hasReceivedMoney || holdsBalance),
+                    TutorialItem.ScanTipCard(isCompleted = hasTipped == true),
+                ),
+                holdsBalance = holdsBalance,
+                isTipMilestoneResolved = hasTipped != null,
+            )
         }
-            .filterNotNull()
             .onEach { dispatchEvent(it) }
             .launchIn(viewModelScope)
     }
@@ -177,7 +198,11 @@ internal class WalletViewModel @Inject constructor(
                     state.copy(feedSyncState = event.syncState)
                 }
                 is Event.OnOnboardingItemsUpdated -> { state ->
-                    state.copy(onboardingItems = event.items, holdsBalance = event.holdsBalance)
+                    state.copy(
+                        onboardingItems = event.items,
+                        holdsBalance = event.holdsBalance,
+                        isTipMilestoneResolved = event.isTipMilestoneResolved,
+                    )
                 }
                 is Event.OnTransactionsUpdated -> { state ->
                     state.copy(transactions = event.transactions)

apps/flipcash/features/balance/src/test/kotlin/com/flipcash/app/balance/internal/WalletLoadingStateTest.kt

@@ -104,6 +104,7 @@ class WalletLoadingStateTest {
     fun `tutorial shows once a milestone is known to be outstanding`() {
         val state = WalletViewModel.State(
             onboardingItems = milestones(addedMoney = true, tipped = false),
+            isTipMilestoneResolved = true,
             feedSyncState = FeedSyncState.Synced,
         )
         assertFalse(state.isNewUserTutorialComplete)
@@ -113,11 +114,35 @@ class WalletLoadingStateTest {
     fun `tutorial is complete when every milestone is`() {
         val state = WalletViewModel.State(
             onboardingItems = milestones(addedMoney = true, tipped = true),
+            isTipMilestoneResolved = true,
             feedSyncState = FeedSyncState.Synced,
         )
         assertTrue(state.isNewUserTutorialComplete)
     }
 
+    /**
+     * The chat cache reports every account as never having tipped until it has been reconciled,
+     * and reconciling it waits on a per-conversation backfill. The tutorial is the only thing that
+     * reads the answer, so it is the only thing that waits.
+     */
+    @Test
+    fun `an unresolved tip milestone withholds the tutorial without holding the tab`() {
+        val state = WalletViewModel.State(
+            onboardingItems = milestones(addedMoney = true, tipped = false),
+            isTipMilestoneResolved = false,
+            feedSyncState = FeedSyncState.Synced,
+        )
+        assertTrue(
+            state.isNewUserTutorialComplete,
+            "an un-reconciled chat cache must not be drawn as an outstanding milestone",
+        )
+        assertFalse(
+            state.isAwaitingActivity,
+            "the balance and the card deck do not read the tip milestone and must not wait for it",
+        )
+        assertTrue(state.hasReceivedMoney, "the action tiles read local state and are answerable")
+    }
+
     @Test
     fun `hasReceivedMoney is false while unknown, gating the action tiles`() {
         assertFalse(WalletViewModel.State().hasReceivedMoney)
@@ -132,6 +157,7 @@ class WalletLoadingStateTest {
     fun `a held balance completes the add-money milestone without a feed row`() {
         val state = WalletViewModel.State(
             onboardingItems = milestones(addedMoney = true, tipped = true),
+            isTipMilestoneResolved = true,
             feedSyncState = FeedSyncState.Synced,
             holdsBalance = true,
         )