fix(analytics): ask the NPS survey after finishes across restarts, carry the old wait only for answers

app/README.md

@@ -88,7 +88,7 @@ The application module owns the Android app shell: `BoxLoreApplication`, `MainAc
 - **Saved feed addresses** (P4 PR 14): `AppContainer` wires the one address writer. `subscriptionRepository` gets `FeedAddressWiring` (re-registration through `fcm/PodcastNotificationRegistrationWorker`, `feedTelemetry`, and pending registrations in SharedPreferences `boxlore_notification_reregistrations`); `subscribedEpisodeCatalog` follows moved feeds on every refresh path (`FeedMoveFollower`, behind `feed-fix-moved-feed-off`); `autoDownloadCoordinator` saves recovered addresses through `ensureHttpsFeedUrl`; cloud pulls save addresses through `subscriptionRepository.feedAddresses`; `subscriptionForegroundSync` gets `FeedAddressUpkeep` for the daily address check and broken-feed reports (report times in SharedPreferences `boxlore_feed_reports`, never sent while `connectivityStatus` says offline), behind `address-check-off`. `PodcastNotificationRegistrationWorker` registers a Podcast Index show's notifications again after its address changed and waits for Firebase to acknowledge the node and topic: unique work `podcast-notification-registration-<show ID>`, REPLACE (it reads the row when it runs), network-gated, exponential backoff from 30 s, at most 8 runs. It records each outcome (`settle`): a show it gives up on stays pending and the next foreground pass queues it again; `rss:` shows keep their own journal worker.
 - **Feed-first show pages** (P4 PR 16): `AppContainer.podcastRepository` gets `FeedFirstWiring`: the stored copies (`localEpisodeCatalogRepository.showPages`), `connectivityStatus` for the offline rung, one broken-feed report limiter (`feedReportLimiter` over the `boxlore_feed_reports` stamps, the same instance `FeedAddressUpkeep.reportLimiter` gives the daily address check, so both claim inside one daily limit), `feedTelemetry` for `feed_fetch_result`, and `isFollowed` (`podcasts.isSubscribed`), so the show page leaves followed shows' reports to their own refresh. The page reads the feed only while `feed_first` is on (`RolloutSwitches.feedFirst`, fixed per process).
 - **Feed address repair after the update** (P4 PR 15, `sync/UpgradeRepairWorker`): a thin WorkManager shell around `:core:catalog` `UpgradeAddressRepair` (`AppContainer.upgradeAddressRepair`, reached through `SharedAppDependenciesHolder`). Every launch, headless ones included, `BoxLoreApplication` calls `UpgradeRepairWorker.scheduleIfNeeded`, which enqueues unique work `feed-address-upgrade-repair` (KEEP, network-gated, exponential backoff from 30 s) only while the stored state says work is left: followed shows with repair steps left, or step 4 not yet recorded in `noBackupFilesDir/upgrade_repair_notifications`. A fresh install, or one with only `rss:` follows, settles without network and enqueues nothing. A restore brings Room but not WorkManager's queue or the step 4 record, so the next launch enqueues the repair again and every followed show's notifications are registered again. A run maps `DONE` and `PAUSED` (`upgrade-repair-off`) to success and `RETRY` to a retry; a launch's chain stops after 10 runs and the next launch enqueues it again. `AppContainer` wires `/v2/shows/feeds` (`AddressCheckAnswer.fetchWith`), the one address writer, the catalog re-read, `PodcastNotificationRegistrationWorker.request` (returns once WorkManager has saved the request, so step 4 never records one lost to process death), `userSyncCoordinator.requestIdentityCatchUp` (once every followed show was read again, the next sync runs the identity catch-up again), `connectivityStatus` and `feedTelemetry`.
-- The NPS survey is Home's own sheet (`feature/home`), reporting to a PostHog `api` survey; PostHog draws no surveys (`surveys = false`), so the SDK can update freely. PostHog push is off too (`capturePushNotificationSubscriptions`, `capturePushNotificationOpened`, on by default in SDK 3.7x), so the FCM token and notification opens never reach PostHog. `BoxLoreApplication` makes `EngagementPromptCoordinator` with the install-only `NpsSurveyStore` (`datastore/boxlore_nps_survey.preferences_pb`, left out of legacy backup, cloud backup and device transfer: `BackgroundCheckBackupRulesTest`) and counts each start with `onAppLaunched`.
+- The NPS survey is Home's own sheet (`feature/home`), reporting to a PostHog `api` survey; PostHog draws no surveys (`surveys = false`), so the SDK can update freely. PostHog push is off too (`capturePushNotificationSubscriptions`, `capturePushNotificationOpened`, on by default in SDK 3.7x), so the FCM token and notification opens never reach PostHog. `BoxLoreApplication` makes `EngagementPromptCoordinator` with the install-only `NpsSurveyStore` (`datastore/boxlore_nps_survey.preferences_pb`, left out of legacy backup, cloud backup and device transfer: `BackgroundCheckBackupRulesTest`) and counts each start with `onAppLaunched`. It sets `PlaybackFinishes.onRecorded` to `EngagementPromptCoordinator.recordFinish`, so each finish is also kept on disk and the survey's moment outlives a restart.
 - JVM `ArtworkPredictiveBackBackgroundTest` checks actual native rendering of the Back wrapper with changing artwork backgrounds beneath the saved app theme.
 - JVM `ArtworkNavigationHostThemeTest` uses a real NavHostController and the production slide transitions to verify source capture before first loader composition, stable saved-theme fallback, Back loader inheritance and separation of Home/regular-page colors from transient loader accents.
 - `ArtworkNavigationHostTheme` captures source colors via a synchronous destination listener, paints the slide/fade underlay with the departing page’s background, and gives entering loaders temporary inherited colors. Regular pages (including Home) and their navbar use the saved app theme as one unit from the first frame, while only their loaders ease back after the slide; detail loaders keep their own entry-scoped handoff until artwork is resolved. Back also starts with the departing palette before restoring the returning page’s own colors, including rapid returns while its composition is still retained; cold starts, opt-out and theme changes reset the fallback safely. The root Scaffold and predictive-Back wrapper use the current page background so shrinking the page does not reveal the saved-theme surface. Navigation routes, duration, gesture handling and playback remain unchanged.

app/src/main/java/cx/aswin/boxlore/BoxLoreApplication.kt

@@ -187,6 +187,8 @@ class BoxLoreApplication :
         )
         engagementPromptCoordinator =
             EngagementPromptCoordinator(userPreferencesRepository, cx.aswin.boxlore.core.prefs.NpsSurveyStore(this))
+        // Each finish is also kept on disk, so the NPS survey's moment outlives a restart.
+        cx.aswin.boxlore.core.playback.PlaybackFinishes.onRecorded = engagementPromptCoordinator::recordFinish
         applicationScope.launch {
             // Updated after installing: this install ran an older version (a fresh or restored install has not).
             val updatedInstall =

core/catalog/README.md

@@ -174,7 +174,7 @@ Main Kotlin files should remain below 1000 lines; extracted helpers keep reposit
 
 ## Testing notes
 
-- Install referrer: Play's `utm_source=(not set)` placeholder is channel `unknown`, not `utm` (`InstallReferrerManagerTest`). `EngagementPromptCoordinator` allows one prompt per session and only at a finish moment (an episode finished since it was made at startup, `isFinishMoment`): `shouldShowNpsSurvey` applies `NpsSurveySchedule` with nothing playing, `onSurveyDisplayed` uses up the session, the cooldown and the survey's 90 days, and `reserveSessionForPrompt` holds the session for a prompt that waits for its screen. The NPS Play hand-off is gone: 9s and 10s get a Play Store button in the survey instead (`EngagementPromptCoordinatorSurveyTest`). `UserSyncCoordinator` calls `onFirstAccountPull` (by default `markLibraryImported`) before a phone's first pull for an account, a sign-in or an account switch, so another phone's synced listening starts the quiet days too (`UserSyncCoordinatorTest`).
+- Install referrer: Play's `utm_source=(not set)` placeholder is channel `unknown`, not `utm` (`InstallReferrerManagerTest`). `EngagementPromptCoordinator` allows one prompt per session and only at a finish moment (`isFinishMoment`: an episode played to the end here within 6 hours; `lastFinishedAtMs` takes the player's own finish in this run or the one `recordFinish` kept before a restart): `shouldShowNpsSurvey` applies `NpsSurveySchedule` with nothing playing, `onSurveyDisplayed` uses up the session, the cooldown and the survey's 90 days, and `reserveSessionForPrompt` holds the session for a prompt that waits for its screen. The NPS Play hand-off is gone: 9s and 10s get a Play Store button in the survey instead (`EngagementPromptCoordinatorSurveyTest`, which also covers a finish kept across a restart). `onAppLaunched` carries the old survey's wait over only for a listener who answered it (`NpsSurveyStore.carryOverLegacy`). `UserSyncCoordinator` calls `onFirstAccountPull` (by default `markLibraryImported`) before a phone's first pull for an account, a sign-in or an account switch, so another phone's synced listening starts the quiet days too (`UserSyncCoordinatorTest`).
 - Saved feed addresses: `FeedAddressUpdaterTest` (Robolectric Room: one update writes both addresses, keeps the old one, resets every validator, marks the row dirty, re-registers and reports once; undo; `rss:` and non-HTTPS refused; a changed row left alone; fills, cloud addresses and unfollowed shows aren't corrections and an unfollowed row stays clean; a failed registration request of any kind keeps the address; a registration stays pending until acknowledged and is asked for again, also after the process died; no address written when its request couldn't be saved; a redirect from an address other than the saved one is never adopted; a re-follow keeps a corrected address), `FeedAddressDailyCheckTest` (501 followed shows make two calls, nothing again the same day; only confirmed moves, read before they are saved; an unreadable registry address never saved and tried again the next day; a swapped show stamped by its swap; never back to the address the saved one replaced, whatever replaced it; follows changed on the phone wait; a failed call or an unavailable registry stamps nothing, so the `/podcast` fallback runs once, and never for a show the check reached; the switch), `FeedAddressRecordedAnswersTest` (the API's own `/v2/shows/feeds` and `/v2/feed-report` answers, `fixtures/v2`, through the app's client: a registry move, a dump address, an unknown show, an http-only show, an unavailable registry and a refused report), `FeedReportsTest` and `SharedPreferencesFeedReportStampsTest` (once per show a day, 20 a day in all, limiters sharing one store counting each other's reports, a failed or refused send counting nothing, kept across processes, never offline, for `rss:` shows, without a read or with the switch off; only a readable new address offered back), `PodcastNotificationRegistrarTest` (the same node and topic, the shared node never written without its address, nothing without notifications, retry and refused outcomes, a cloud pull that changes only the address re-registering on the same node and topic, a pulled previous address never undoing a registry correction, and a merge never writing an older copy's address over a newer one), `SubscribedEpisodeCatalogTest` (moves followed on every refresh path, never for `rss:` shows or with the switch off, caller-handled requests left alone), and `DirectFeedRecoveryTest` (every failed read reported, the once-a-day `/podcast` re-check sharing the daily stamp, the report's address tried before the `/podcast` re-check, a passing failure keeping its address, a candidate replacing only the address that failed).
 - `PodcastRepositoryPlayerPrefsTest` checks the player preferences (and their legacy-file migration) open on first use, not while the repository is constructed on the main thread at startup.
 - Unit tests live under `core/catalog/src/test`.

core/catalog/src/main/java/cx/aswin/boxlore/core/catalog/EngagementPromptCoordinator.kt

@@ -15,13 +15,12 @@ import kotlinx.coroutines.launch
 
 /**
  * Coordinates proactive engagement prompts (NPS survey, Play review) so only one surfaces per
- * session, and only at a finish moment: an episode finished since boxlore started
- * ([processStartedAtMs], when this coordinator is made at startup), never on an open alone.
+ * session, and only at a finish moment: shortly after the player here played an episode to the end
+ * ([NpsSurveySchedule.isFinishMoment]; the finish is kept across restarts), never on an open alone.
  */
 class EngagementPromptCoordinator(
     private val userPrefs: UserPreferencesRepository,
     private val surveyStore: NpsSurveyStore,
-    private val processStartedAtMs: Long = System.currentTimeMillis(),
     private val scope: CoroutineScope =
         CoroutineScope(
             SupervisorJob() +
@@ -62,18 +61,36 @@ class EngagementPromptCoordinator(
         }
     }
 
-    /** An episode finished in this run of boxlore, recently: the only time a prompt may appear. */
+    /** An episode finished here recently: the only time a prompt may appear. */
     fun isFinishMoment(lastFinishedAtMs: Long?, nowMs: Long = System.currentTimeMillis()): Boolean =
-        NpsSurveySchedule.isFinishMoment(lastFinishedAtMs, processStartedAtMs, nowMs)
+        NpsSurveySchedule.isFinishMoment(lastFinishedAtMs, nowMs)
+
+    /** Keeps the player's finish at [atMs] on disk (from `PlaybackFinishes`), so the moment outlives a restart. */
+    fun recordFinish(atMs: Long) {
+        launchSafely("Failed to keep the finish") { surveyStore.recordFinish(atMs) }
+    }
+
+    /**
+     * The latest finish here: [inThisRunAtMs] (the player's own, this run) or the one kept from before a
+     * restart. A finish later than [nowMs] is left out, so a clock that ran ahead can't hide a real one.
+     */
+    suspend fun lastFinishedAtMs(inThisRunAtMs: Long?, nowMs: Long = System.currentTimeMillis()): Long? =
+        listOfNotNull(inThisRunAtMs, surveyStore.record().lastFinishedAtMs).filter { it <= nowMs }.maxOrNull()
 
     /**
-     * Counts this start of boxlore and, once per install, carries over an older version's survey
-     * ([NpsSurveyStore.carryOverLegacy]; [updatedInstall] when Android updated boxlore after installing
-     * it). Called once at startup.
+     * Counts this start of boxlore and, once per install, carries over an older version's survey for a
+     * listener who answered it ([NpsSurveyStore.carryOverLegacy]; [updatedInstall] when Android updated
+     * boxlore after installing it). Called once at startup.
      */
     suspend fun onAppLaunched(updatedInstall: Boolean, nowMs: Long = System.currentTimeMillis()) {
         surveyStore.recordLaunch()
-        surveyStore.carryOverLegacy(userPrefs.hasNpsSurveyFired(), updatedInstall, nowMs)
+        surveyStore.carryOverLegacy(
+            legacyShown = userPrefs.hasNpsSurveyFired(),
+            legacyAnswered = userPrefs.npsLastScore() != null,
+            updatedInstall = updatedInstall,
+            lastPromptAtMs = userPrefs.lastEngagementPromptAtMs(),
+            nowMs = nowMs,
+        )
     }
 
     /** Whether the NPS survey should appear now, by [NpsSurveySchedule] with nothing playing and no prompt yet this session. */
@@ -92,7 +109,6 @@ class EngagementPromptCoordinator(
                 installedAtMs = installedAtMs,
                 finishedSinceInstall = finishedSinceInstall,
                 lastFinishedAtMs = lastFinishedAtMs,
-                processStartedAtMs = processStartedAtMs,
                 record = surveyStore.record(),
                 inQuietDays = userPrefs.isInReviewQuietPeriod(installedAtMs, nowMs),
                 cooldownElapsed = userPrefs.isEngagementCooldownElapsed(),
@@ -111,7 +127,6 @@ class EngagementPromptCoordinator(
                 installedAtMs = installedAtMs,
                 finishedSinceInstall = finishedSinceInstall,
                 lastFinishedAtMs = nowMs,
-                processStartedAtMs = processStartedAtMs,
                 record = surveyStore.record(),
                 inQuietDays = userPrefs.isInReviewQuietPeriod(installedAtMs, nowMs),
                 cooldownElapsed = userPrefs.isEngagementCooldownElapsed(),

core/catalog/src/test/java/cx/aswin/boxlore/core/catalog/EngagementPromptCoordinatorSurveyTest.kt

@@ -21,8 +21,9 @@ import org.robolectric.RobolectricTestRunner
 import org.robolectric.annotation.Config
 
 /**
- * The coordinator asks the NPS survey only at a finish moment in this run, with nothing playing and no
- * prompt yet this session; once it was on screen it waits 90 days, and the review prompt may follow.
+ * The coordinator asks the NPS survey only at a finish moment (kept across restarts), with nothing
+ * playing and no prompt yet this session; once it was on screen it waits 90 days, and the review
+ * prompt may follow.
  * (One story in one test: the survey's install-only store lives for the whole test run.)
  */
 @RunWith(RobolectricTestRunner::class)
@@ -37,9 +38,8 @@ class EngagementPromptCoordinatorSurveyTest {
         val prefs = UserPreferencesRepository(context)
         val store = NpsSurveyStore(context)
         val now = System.currentTimeMillis()
-        val processStart = now - 10 * 60 * 1000
         val scope = CoroutineScope(SupervisorJob())
-        val coordinator = EngagementPromptCoordinator(prefs, store, processStartedAtMs = processStart, scope = scope)
+        val coordinator = EngagementPromptCoordinator(prefs, store, scope = scope)
         suspend fun settle() = scope.coroutineContext[Job]!!.children.forEach { it.join() }
         suspend fun due(lastFinishedAt: Long?, playing: Boolean = false) =
             coordinator.shouldShowNpsSurvey(playing, installedAtMs = now - 30 * day, finishedSinceInstall = 5, lastFinishedAtMs = lastFinishedAt, nowMs = now)
@@ -49,7 +49,8 @@ class EngagementPromptCoordinatorSurveyTest {
         // A restored or older install that never showed the old survey starts with no history.
         assertNull(store.record().lastShownAtMs)
 
-        assertFalse("an open alone is not the moment", due(lastFinishedAt = processStart - 1))
+        assertFalse("an open alone is not the moment", due(lastFinishedAt = null))
+        assertFalse("nor a finish long ago", due(lastFinishedAt = now - day))
         assertFalse("never during playback", due(lastFinishedAt = now - 60_000, playing = true))
         assertTrue(due(lastFinishedAt = now - 60_000))
         assertTrue(coordinator.isFinishMoment(now - 60_000, now))
@@ -64,7 +65,14 @@ class EngagementPromptCoordinatorSurveyTest {
         assertTrue("the review prompt may follow later", prefs.hasNpsSurveyFired())
         assertEquals(9, prefs.npsLastScore())
 
-        val nextRun = EngagementPromptCoordinator(prefs, store, processStartedAtMs = now, scope = scope)
+        // A finish kept on disk is still the moment after a restart (regression: it lived in memory only).
+        coordinator.recordFinish(now - 60_000)
+        settle()
+        val nextRun = EngagementPromptCoordinator(prefs, store, scope = scope)
+        assertEquals(now - 60_000, nextRun.lastFinishedAtMs(inThisRunAtMs = null))
+        assertEquals(now + 1, nextRun.lastFinishedAtMs(inThisRunAtMs = now + 1, nowMs = now + 2))
+        // A finish from a clock that ran ahead is left out.
+        assertEquals(now - 60_000, nextRun.lastFinishedAtMs(inThisRunAtMs = now + day, nowMs = now))
         assertFalse(
             "not again within 90 days",
             nextRun.shouldShowNpsSurvey(false, now - 30 * day, 6, lastFinishedAtMs = now + 1, nowMs = now + 2),