From dd485bfcaf41c8aa4f239d4e544812bd8611859f Mon Sep 17 00:00:00 2001 From: Aswin C Date: Sat, 7 Nov 2026 22:27:57 +0530 Subject: [PATCH 1/3] fix(analytics): ask the NPS survey after finishes across restarts, carry the old wait only for answers v0.0.30 carried the old survey's 90-day wait over for every phone that marked it shown, including ones that never displayed it, and the finish moment lived in memory for two hours. Together they kept every real listener from being asked. --- app/README.md | 2 +- .../cx/aswin/boxlore/BoxLoreApplication.kt | 2 + core/catalog/README.md | 2 +- .../catalog/EngagementPromptCoordinator.kt | 33 ++++++--- .../EngagementPromptCoordinatorSurveyTest.kt | 18 +++-- core/playback/README.md | 2 +- .../boxlore/core/playback/PlaybackFinishes.kt | 8 ++- .../core/playback/PlaybackFinishesTest.kt | 17 +++++ core/prefs/README.md | 2 +- .../core/prefs/EngagementPreferences.kt | 3 + .../boxlore/core/prefs/NpsSurveySchedule.kt | 28 ++++---- .../boxlore/core/prefs/NpsSurveyStore.kt | 48 ++++++++++--- .../core/prefs/UserPreferencesRepository.kt | 2 + .../core/prefs/NpsSurveyScheduleTest.kt | 19 ++--- .../boxlore/core/prefs/NpsSurveyStoreTest.kt | 69 +++++++++++++++++-- feature/home/README.md | 2 +- .../feature/home/HomeViewModelLoadData.kt | 5 +- release-notes/upcoming.md | 1 + 18 files changed, 201 insertions(+), 62 deletions(-) diff --git a/app/README.md b/app/README.md index f9e98350d..83012ea43 100644 --- a/app/README.md +++ b/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-`, 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. diff --git a/app/src/main/java/cx/aswin/boxlore/BoxLoreApplication.kt b/app/src/main/java/cx/aswin/boxlore/BoxLoreApplication.kt index 16262f27f..e2006abc6 100644 --- a/app/src/main/java/cx/aswin/boxlore/BoxLoreApplication.kt +++ b/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 = diff --git a/core/catalog/README.md b/core/catalog/README.md index deeec014e..cbf849421 100644 --- a/core/catalog/README.md +++ b/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`. diff --git a/core/catalog/src/main/java/cx/aswin/boxlore/core/catalog/EngagementPromptCoordinator.kt b/core/catalog/src/main/java/cx/aswin/boxlore/core/catalog/EngagementPromptCoordinator.kt index 361fdf767..7789c41f8 100644 --- a/core/catalog/src/main/java/cx/aswin/boxlore/core/catalog/EngagementPromptCoordinator.kt +++ b/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,32 @@ 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. */ + suspend fun lastFinishedAtMs(inThisRunAtMs: Long?): Long? = listOfNotNull(inThisRunAtMs, surveyStore.record().lastFinishedAtMs).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 +105,6 @@ class EngagementPromptCoordinator( installedAtMs = installedAtMs, finishedSinceInstall = finishedSinceInstall, lastFinishedAtMs = lastFinishedAtMs, - processStartedAtMs = processStartedAtMs, record = surveyStore.record(), inQuietDays = userPrefs.isInReviewQuietPeriod(installedAtMs, nowMs), cooldownElapsed = userPrefs.isEngagementCooldownElapsed(), @@ -111,7 +123,6 @@ class EngagementPromptCoordinator( installedAtMs = installedAtMs, finishedSinceInstall = finishedSinceInstall, lastFinishedAtMs = nowMs, - processStartedAtMs = processStartedAtMs, record = surveyStore.record(), inQuietDays = userPrefs.isInReviewQuietPeriod(installedAtMs, nowMs), cooldownElapsed = userPrefs.isEngagementCooldownElapsed(), diff --git a/core/catalog/src/test/java/cx/aswin/boxlore/core/catalog/EngagementPromptCoordinatorSurveyTest.kt b/core/catalog/src/test/java/cx/aswin/boxlore/core/catalog/EngagementPromptCoordinatorSurveyTest.kt index fe7d67901..63d526ecc 100644 --- a/core/catalog/src/test/java/cx/aswin/boxlore/core/catalog/EngagementPromptCoordinatorSurveyTest.kt +++ b/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,12 @@ 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)) assertFalse( "not again within 90 days", nextRun.shouldShowNpsSurvey(false, now - 30 * day, 6, lastFinishedAtMs = now + 1, nowMs = now + 2), diff --git a/core/playback/README.md b/core/playback/README.md index 4b049c4fa..db09d3ef8 100644 --- a/core/playback/README.md +++ b/core/playback/README.md @@ -141,7 +141,7 @@ Files under `core/data/service` are compatibility stubs for old service class na - `playback_session_ended` goes out once per listen; `playback_paused` is still built but off by default. Only buffering of 5 seconds or more is sent alone as `playback_buffering`. - Errors go out once per cause per episode until a listen starts (`PlaybackErrorOnce`, which remembers every episode tried since), also to error tracking with links and tokens removed. - `ListenNoteKeeper` writes a note when a listen starts and every 30 seconds while it plays (`boxlore_listen_note`), and clears it when the listen ends; `ListenInterruptions.reportLeftover`, called once at app start, sends `playback_interrupted` for a note Android's ending of the app left behind and clears it. -- Each timed play (`PlaybackTelemetrySession`, saved through `ListeningSessionRecordLogic`) also records its listening time (`wallMs`: each natural position advance divided by the playback speed, `ListeningSessionRecordLogic.listenedWallMs`, so buffering and pauses never count), how it started (`playSource`: a Lore or Smart Queue queue context, else the analytics glossary's canonical entry point), where it was heard (`service.ListeningOutputRoute`, set on the session by the service as `listeningOutput`: Cast, the car, Bluetooth, wired or USB headphones or the speaker; Android 13+ reads the media route, older versions the connected outputs in Android's routing order) and the episode's release date (queue, then catalog; seconds or ms stored as ms, `releaseDateMs`). A recorded play adds its show to the played shows (`PlayedShowDao.recordPlay`, with the art of the episode's history row). A progress save from playback dates the first play and a finish by listening (`ListeningHistoryUpsertLogic.buildProgressSaveEntity`); the database keeps earlier dates, and only playback dates a finish. Sessions carry the listening device ID the service sets (`listeningDeviceId`, from `BoxcastPrefs.getOrCreateListeningDeviceId`). Undo of a history removal puts back the dates and the sessions' and daily summaries' new fields, the summaries' richer totals included, their time with a known release date (Room v42) too (`PlaybackHistoryMappings`). `ListeningHistoryItem` carries `firstPlayedAt` and `finishedAt`. `PlaybackFinishes.lastAtMs` is when the player on this phone last played an episode to the end in this run (both play-to-end paths set it before the row is written); a replay sets it again though the row keeps its first `finishedAt`, and marking played, sync and restores never set it. Home uses it for the NPS survey and the review prompt (`PlaybackFinishesTest`). +- Each timed play (`PlaybackTelemetrySession`, saved through `ListeningSessionRecordLogic`) also records its listening time (`wallMs`: each natural position advance divided by the playback speed, `ListeningSessionRecordLogic.listenedWallMs`, so buffering and pauses never count), how it started (`playSource`: a Lore or Smart Queue queue context, else the analytics glossary's canonical entry point), where it was heard (`service.ListeningOutputRoute`, set on the session by the service as `listeningOutput`: Cast, the car, Bluetooth, wired or USB headphones or the speaker; Android 13+ reads the media route, older versions the connected outputs in Android's routing order) and the episode's release date (queue, then catalog; seconds or ms stored as ms, `releaseDateMs`). A recorded play adds its show to the played shows (`PlayedShowDao.recordPlay`, with the art of the episode's history row). A progress save from playback dates the first play and a finish by listening (`ListeningHistoryUpsertLogic.buildProgressSaveEntity`); the database keeps earlier dates, and only playback dates a finish. Sessions carry the listening device ID the service sets (`listeningDeviceId`, from `BoxcastPrefs.getOrCreateListeningDeviceId`). Undo of a history removal puts back the dates and the sessions' and daily summaries' new fields, the summaries' richer totals included, their time with a known release date (Room v42) too (`PlaybackHistoryMappings`). `ListeningHistoryItem` carries `firstPlayedAt` and `finishedAt`. `PlaybackFinishes.lastAtMs` is when the player on this phone last played an episode to the end in this run (both play-to-end paths set it before the row is written); a replay sets it again though the row keeps its first `finishedAt`, and marking played, sync and restores never set it. Each finish also goes to `PlaybackFinishes.onRecorded` (the app keeps it on disk, so the moment outlives a restart). Home uses it for the NPS survey and the review prompt (`PlaybackFinishesTest`). - `PlaybackRepository` polls MediaController position only while app UI is foregrounded; the playback service is the sole owner of active-session progress persistence and telemetry. Service writes capture live Player state before a playlist can be cleared and serialize sequence-stamped snapshots through one coordinator, so an older suspended write cannot replace a later seek/restart. Replayed completed episodes reset completion via `reopenProgress` when intentional progress occurs, and `ensureCurrentHistoryRow` refreshes `lastPlayedAt` immediately on play initiation so quick plays (<10s) are discoverable on cold restart. Service-only entry points such as Android Auto atomically seed missing history rows. When the UI returns to an empty player, it refreshes from Room instead of replacing a valid restored duration with `TIME_UNSET`/zero. This adds no background UI polling or duplicate periodic Room writes. - `BoxLorePlaybackService` is a Media3 `MediaLibraryService`; it resolves shared dependencies lazily after application startup and keeps its local and remote players service-scoped so transfers continue while the Activity is absent. A local session that stays paused and backgrounded for 15 minutes saves progress, rechecks eligibility after persistence, and stops the service; resumed playback, cancellation, buffering intent, foreground UI, and Cast sessions bypass that teardown. - The local ExoPlayer opportunistically enables platform audio offload only when the device can preserve playback-speed behavior. `WAKE_MODE_NETWORK` remains enabled for reliable screen-off streaming. diff --git a/core/playback/src/main/java/cx/aswin/boxlore/core/playback/PlaybackFinishes.kt b/core/playback/src/main/java/cx/aswin/boxlore/core/playback/PlaybackFinishes.kt index fae693c96..168b193a3 100644 --- a/core/playback/src/main/java/cx/aswin/boxlore/core/playback/PlaybackFinishes.kt +++ b/core/playback/src/main/java/cx/aswin/boxlore/core/playback/PlaybackFinishes.kt @@ -3,15 +3,21 @@ package cx.aswin.boxlore.core.playback /** * When the player on this phone last played an episode to the end, in this run of boxlore. Replays * count each time; marking an episode played, a sync from another device or a backup never does. - * Home asks the NPS survey and the review prompt only shortly after one. + * Home asks the NPS survey and the review prompt only shortly after one. [onRecorded] (set by the app) + * also keeps each finish on disk, so the moment outlives a restart. */ object PlaybackFinishes { @Volatile var lastAtMs: Long? = null private set + /** Called with each finish's time, as it is recorded; must not block (the app saves it in the background). */ + @Volatile + var onRecorded: ((Long) -> Unit)? = null + internal fun record(atMs: Long) { lastAtMs = atMs + onRecorded?.invoke(atMs) } /** Test hook: no finish in this run. */ diff --git a/core/playback/src/test/java/cx/aswin/boxlore/core/playback/PlaybackFinishesTest.kt b/core/playback/src/test/java/cx/aswin/boxlore/core/playback/PlaybackFinishesTest.kt index ab63c3d3f..d6e7c76cf 100644 --- a/core/playback/src/test/java/cx/aswin/boxlore/core/playback/PlaybackFinishesTest.kt +++ b/core/playback/src/test/java/cx/aswin/boxlore/core/playback/PlaybackFinishesTest.kt @@ -9,6 +9,7 @@ import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.test.UnconfinedTestDispatcher import kotlinx.coroutines.test.runTest import org.junit.After +import org.junit.Assert.assertEquals import org.junit.Assert.assertNotNull import org.junit.Assert.assertNull import org.junit.Assert.assertTrue @@ -41,6 +42,7 @@ class PlaybackFinishesTest { fun tearDown() { database.close() PlaybackFinishes.resetForTest() + PlaybackFinishes.onRecorded = null } @Test @@ -62,6 +64,21 @@ class PlaybackFinishesTest { assertTrue(database.listeningHistoryDao().getHistoryItem(ID)!!.finishedAt == first) } + @Test + fun everyFinishIsHandedOnToBeKeptAcrossRestarts() = runTest(testDispatcher) { + // Regression: the finish lived in memory only, so closing boxlore after an episode lost the + // moment the NPS survey waits for. + val kept = mutableListOf() + PlaybackFinishes.onRecorded = { kept += it } + database.listeningHistoryDao().upsert(history()) + val progress = progressCoordinator() + + progress.saveProgressSnapshot(snapshot(positionMs = 60_000L, sequence = 1L)) + assertTrue("progress mid-episode hands nothing on", kept.isEmpty()) + progress.saveProgressSnapshot(snapshot(positionMs = 1_799_000L, sequence = 2L)) + assertEquals(listOf(PlaybackFinishes.lastAtMs), kept) + } + private fun progressCoordinator() = PlaybackProgressCoordinator( mainDispatcher = testDispatcher, database = database, diff --git a/core/prefs/README.md b/core/prefs/README.md index 396a06614..0ead4506a 100644 --- a/core/prefs/README.md +++ b/core/prefs/README.md @@ -84,7 +84,7 @@ src/main/java/cx/aswin/boxlore/core/prefs/ - `subscription_manual_order` and `home_pinned_podcast_ids` live in the same `user_preferences` DataStore (do not rename the file). Unsubscribe drops that id from both lists. - `seen_listening_story` (`seenListeningStoryStream` / `setSeenListeningStory`, added 2026-10-31): the last monthly listening story opened, as `yyyy-MM`, so Listening history stops calling it New. Local to the phone. - Review prompt (`EngagementPreferences`): a milestone (5, 15 or 30 episodes finished in boxlore on this install; Home counts them) is stored as `review_milestone_pending` and, once a prompt shows, as `review_milestone_consumed`, so the same milestone never asks again. `shouldShowReviewPrompt` and `isInReviewQuietPeriod` take Android's install time (a backup doesn't carry it) and keep a 2-day quiet period (`REVIEW_QUIET_DAYS`) after it and after `library_imported_at` (`markLibraryImported`). `review_first_launch_at` (the old lazily set install day) is no longer read. The pending milestone follows the count, so one a backup brought onto a phone with no listening behind it is dropped; on the first sync after updating, an install already prompted counts milestone 5 (or the one it has reached) as used (`ReviewPromptPreferencesTest`). `has_logged_first_play` is read only so an updated install doesn't send `first_episode_played` again; the player claims it now. `UserPreferencesRepositoryTest` covers the quiet days after install and import and a used milestone never coming back. -- NPS survey: `NpsSurveySchedule` decides when it may appear: right after the player on this phone plays an episode to the end in this run of boxlore (within 2 hours and never in the future; never on an open alone, a sync or a mark-played), with 3 episodes finished here, a week since install, not in the first run, outside the quiet days and the shared 14-day cooldown, and 90 days after it last appeared. `NpsSurveyStore` keeps when it last appeared and how many times boxlore has started in its own file, `boxlore_nps_survey`, which backups leave out, so a restored phone is asked afresh. Once per install `carryOverLegacy` makes an install that Android updated from an older version and that showed the old popover survey (`nps_survey_fired`) wait 90 days from the update; a fresh install, restored from a backup or not and however late it first opens, ignores the copied flag. `nps_survey_fired` and `nps_last_score` stay in `user_preferences`, because the milestone review prompt waits for the survey and skips low scores; the old pending and Play hand-off keys are no longer used. `unmet` names the rules a moment misses, for the debug screen, and `resetSchedule` (debug) forgets the last showing. `NpsSurveyScheduleTest` and `NpsSurveyStoreTest` cover each rule. +- NPS survey: `NpsSurveySchedule` decides when it may appear: within 6 hours of the player on this phone playing an episode to the end (`FINISH_MOMENT_WINDOW_MS`; never in the future; never on an open alone, a sync or a mark-played; the finish is kept in `NpsSurveyStore` across restarts, because the next episode often plays straight on and listeners often close boxlore after one, and with a 2-hour, this-run-only moment almost nobody was asked), with 3 episodes finished here, a week since install, not in the first run, outside the quiet days and the shared 14-day cooldown, and 90 days after it last appeared. `NpsSurveyStore` keeps when it last appeared, how many times boxlore has started and the last finish (`recordFinish`, the latest kept) in its own file, `boxlore_nps_survey`, which backups leave out, so a restored phone is asked afresh. Once per install `carryOverLegacy` makes an install that Android updated from an older version wait 90 days from the update only when its listener answered the old popover survey (`nps_last_score` kept); one that only showed it, had it dismissed, or marked it shown before it ever appeared (`nps_survey_fired`) is asked like a new install. 0.0.30 carried the wait over for every `nps_survey_fired` phone, which kept most regular listeners from being asked; a one-time revision (`legacy_revised`) lifts that wait where the listener never answered, and keeps a real 0.0.30 showing (it wrote the shared prompt time at the same moment). A fresh install, restored from a backup or not and however late it first opens, ignores the copied flags. `nps_survey_fired` and `nps_last_score` stay in `user_preferences`, because the milestone review prompt waits for the survey and skips low scores; the old pending and Play hand-off keys are no longer used. `unmet` names the rules a moment misses, for the debug screen, and `resetSchedule` (debug) forgets the last showing. `NpsSurveyScheduleTest` and `NpsSurveyStoreTest` cover each rule, the finish kept across restarts and the carry-over and its revision. `lastEngagementPromptAtMs` reads the shared prompt time. - `legacy_rss_repair_*` keys are a crash-recovery contract; keep them until every shipping build that can begin the repair has aged out. ## Testing notes diff --git a/core/prefs/src/main/java/cx/aswin/boxlore/core/prefs/EngagementPreferences.kt b/core/prefs/src/main/java/cx/aswin/boxlore/core/prefs/EngagementPreferences.kt index eb420259b..d39760565 100644 --- a/core/prefs/src/main/java/cx/aswin/boxlore/core/prefs/EngagementPreferences.kt +++ b/core/prefs/src/main/java/cx/aswin/boxlore/core/prefs/EngagementPreferences.kt @@ -276,6 +276,9 @@ internal class EngagementPreferences(private val dataStore: DataStore= processStartedAtMs && nowMs - lastFinishedAtMs in 0L..FINISH_MOMENT_WINDOW_MS + /** An episode finished here within the window, and not in the future. */ + fun isFinishMoment(lastFinishedAtMs: Long?, nowMs: Long,): Boolean = + lastFinishedAtMs != null && nowMs - lastFinishedAtMs in 0L..FINISH_MOMENT_WINDOW_MS fun isDue(moment: NpsSurveyMoment): Boolean = unmet(moment).isEmpty() @@ -45,7 +49,7 @@ object NpsSurveySchedule { private fun isMet(rule: NpsSurveyRule, moment: NpsSurveyMoment): Boolean { val now = moment.nowMs return when (rule) { - NpsSurveyRule.FinishMoment -> isFinishMoment(moment.lastFinishedAtMs, moment.processStartedAtMs, now) + NpsSurveyRule.FinishMoment -> isFinishMoment(moment.lastFinishedAtMs, now) NpsSurveyRule.FinishedEpisodes -> moment.finishedSinceInstall >= MIN_FINISHED_EPISODES NpsSurveyRule.DaysInstalled -> moment.installedAtMs > 0L && now - moment.installedAtMs >= MIN_DAYS_INSTALLED * DAY_MS NpsSurveyRule.FirstRun -> moment.record.launches >= 2 diff --git a/core/prefs/src/main/java/cx/aswin/boxlore/core/prefs/NpsSurveyStore.kt b/core/prefs/src/main/java/cx/aswin/boxlore/core/prefs/NpsSurveyStore.kt index bcdc73341..71eaea94b 100644 --- a/core/prefs/src/main/java/cx/aswin/boxlore/core/prefs/NpsSurveyStore.kt +++ b/core/prefs/src/main/java/cx/aswin/boxlore/core/prefs/NpsSurveyStore.kt @@ -16,15 +16,23 @@ import kotlinx.coroutines.flow.first /** The NPS survey's history on this install. The backup rules leave the file out, so a restored phone is asked afresh. */ internal val Context.npsSurveyDataStore: DataStore by preferencesDataStore(name = NpsSurveyStore.FILE_NAME) -/** What the NPS survey's schedule reads: when it last appeared here, and how many times boxlore has started here. */ -data class NpsSurveyRecord(val lastShownAtMs: Long?, val launches: Int,) +/** + * What the NPS survey's schedule reads: when it last appeared here, how many times boxlore has started + * here, and when the player here last played an episode to the end. + */ +data class NpsSurveyRecord(val lastShownAtMs: Long?, val launches: Int, val lastFinishedAtMs: Long? = null,) class NpsSurveyStore internal constructor(private val dataStore: DataStore,) { constructor(context: Context) : this(context.applicationContext.npsSurveyDataStore) suspend fun record(): NpsSurveyRecord { val prefs = dataStore.data.catch { if (it is IOException) emit(emptyPreferences()) else throw it }.first() - return NpsSurveyRecord(lastShownAtMs = prefs[LAST_SHOWN_AT], launches = prefs[LAUNCHES] ?: 0) + return NpsSurveyRecord(lastShownAtMs = prefs[LAST_SHOWN_AT], launches = prefs[LAUNCHES] ?: 0, lastFinishedAtMs = prefs[LAST_FINISHED_AT]) + } + + /** The player here played an episode to the end at [atMs]; keeps the latest, so the finish outlives a restart. */ + suspend fun recordFinish(atMs: Long) { + dataStore.edit { prefs -> if (atMs > (prefs[LAST_FINISHED_AT] ?: 0L)) prefs[LAST_FINISHED_AT] = atMs } } /** Counts one start of boxlore on this install, so the survey never appears in the first one. */ @@ -47,15 +55,30 @@ class NpsSurveyStore internal constructor(private val dataStore: DataStore - if (prefs[LEGACY_CHECKED] == true) return@edit - prefs[LEGACY_CHECKED] = true - if (legacyShown && updatedInstall && prefs[LAST_SHOWN_AT] == null) prefs[LAST_SHOWN_AT] = nowMs + if (prefs[LEGACY_CHECKED] != true) { + prefs[LEGACY_CHECKED] = true + prefs[LEGACY_REVISED] = true + if (legacyAnswered && updatedInstall && prefs[LAST_SHOWN_AT] == null) prefs[LAST_SHOWN_AT] = nowMs + return@edit + } + if (prefs[LEGACY_REVISED] == true) return@edit + prefs[LEGACY_REVISED] = true + val shownAt = prefs[LAST_SHOWN_AT] ?: return@edit + val realShowing = lastPromptAtMs != null && kotlin.math.abs(lastPromptAtMs - shownAt) <= SAME_MOMENT_MS + val carriedOver = legacyShown && updatedInstall && !realShowing + if (carriedOver && !legacyAnswered) prefs.remove(LAST_SHOWN_AT) } } @@ -64,5 +87,10 @@ class NpsSurveyStore internal constructor(private val dataStore: DataStore + prefs[androidx.datastore.preferences.core.booleanPreferencesKey("legacy_checked")] = true + prefs[androidx.datastore.preferences.core.longPreferencesKey("last_shown_at")] = lastShownAt + } + } + + private suspend fun carry( + legacyShown: Boolean, + legacyAnswered: Boolean = false, + updatedInstall: Boolean = true, + lastPromptAt: Long? = null, + nowMs: Long = now, + ) = store.carryOverLegacy(legacyShown, legacyAnswered, updatedInstall, lastPromptAt, nowMs) + @Test fun aRestoredPhoneIgnoresTheBackedUpFlagHoweverLateItFirstOpens() = runTest { // Installed fresh (never updated) with the old flag from a backup, first opened days later: // regression, the old 24-hour guess counted this as an updated install and waited 90 days. - store.carryOverLegacy(legacyShown = true, updatedInstall = false, nowMs = now + 2 * day) + carry(legacyShown = true, legacyAnswered = true, updatedInstall = false, nowMs = now + 2 * day) assertNull(store.record().lastShownAtMs) } @Test fun theCarryOverRunsOnce() = runTest { - store.carryOverLegacy(legacyShown = false, updatedInstall = true, nowMs = now) - store.carryOverLegacy(legacyShown = true, updatedInstall = true, nowMs = now + day) + carry(legacyShown = false) + carry(legacyShown = true, legacyAnswered = true, nowMs = now + day) assertNull(store.record().lastShownAtMs) } @Test fun theCarryOverNeverMovesARealShowing() = runTest { store.markShown(now - 10 * day) - store.carryOverLegacy(legacyShown = true, updatedInstall = true, nowMs = now) + carry(legacyShown = true, legacyAnswered = true) assertEquals(now - 10 * day, store.record().lastShownAtMs) } diff --git a/feature/home/README.md b/feature/home/README.md index 51778b000..7202c6f8f 100644 --- a/feature/home/README.md +++ b/feature/home/README.md @@ -130,7 +130,7 @@ Main Kotlin files should remain below 1000 lines; extracted Home feed, ViewModel ## Testing notes - Review prompt: Home counts only episodes played to the end after this install (`countFinishedSince` on `HomeListeningHistoryItem.finishedAt`, with `HomeViewModel.installedAtMs` from Android's install time; synced listening from another phone after that counts too), so marked-played episodes and a restore of older listening never trigger it, and the sheet's "You've finished N episodes" uses the same count. A decided prompt waits in `ReviewPromptRecorder` and is recorded only once its sheet is on screen with Home resumed (`onReviewPromptDisplayed`: milestone, cooldown and `engagement_prompt_shown`); deciding only reserves the session (`reserveSessionForPrompt`), so a prompt decided behind another screen isn't used up if boxlore closes first (`ReviewPromptRecorderTest`). It also appears only at a finish moment (the player here played an episode to the end in this run, `PlaybackFinishes` in `:core:playback`), never on an open alone, a sync or a mark-played. The category chips send `home_surface_tapped` (`category_filter`). `HomeListeningHistoryItemTest` covers the count. -- NPS survey: Home owns it. At a finish moment with nothing playing and no other sheet, `EngagementPromptCoordinator.shouldShowNpsSurvey` (`NpsSurveySchedule` in `:core:prefs`) decides it and `NpsSurveyController` opens it; `NpsSurveyHost` shows it only once nothing plays, drops it silently if it hasn't appeared within the finish moment's 2 hours, and it counts as shown (`survey shown`, the schedule used up) only on screen with Home resumed. Steps: a 0–10 score (two staggered rows of same-size circles, "Not likely" over the 0 and "Very likely" under the 10), one follow-up by band (0–6 "What's the one thing we should fix first?", 7–8 "What would turn this up to a 10?", 9–10 "What should we queue up next?" with the ideas tag), then thanks; 9s and 10s get "Rate on Google Play", which opens the store page (`store_review_opened`, `nps_survey`). The score is sent at once as a partial answer and Send or Skip completes the same submission; every send lists all four questions in the survey's order, as PostHog's own SDKs do; closing before a score is `survey dismissed`, closing on the follow-up keeps the score. Answers are capped at 500 characters. Wording can be changed from the `nps-survey-copy` flag's payload (any `NpsSurveyCopy` key; an empty `ideaTag` hides the tag). Long-pressing Home's Feedback icon opens it by hand without using up the schedule, and the debug screen's Flags tab says what the automatic survey still waits for and can reset its timing (last showing, shared cooldown, this session's prompt; `NpsSurveyDebugTest`). Echo stands on the sheet's top edge and plays one clip per step from its manifest (`survey-ask`, `survey-notes`, `survey-think`, `survey-idea`, `survey-thanks-calm`, `survey-thanks`, `survey-celebrate`), then rests with its eyes alive; reduced motion shows the resting still. +- NPS survey: Home owns it. At a finish moment with nothing playing and no other sheet, `EngagementPromptCoordinator.shouldShowNpsSurvey` (`NpsSurveySchedule` in `:core:prefs`) decides it and `NpsSurveyController` opens it; `NpsSurveyHost` shows it only once nothing plays, drops it silently if it hasn't appeared within the finish moment's 6 hours (the finish comes from `EngagementPromptCoordinator.lastFinishedAtMs`, so one before a restart counts), and it counts as shown (`survey shown`, the schedule used up) only on screen with Home resumed. Steps: a 0–10 score (two staggered rows of same-size circles, "Not likely" over the 0 and "Very likely" under the 10), one follow-up by band (0–6 "What's the one thing we should fix first?", 7–8 "What would turn this up to a 10?", 9–10 "What should we queue up next?" with the ideas tag), then thanks; 9s and 10s get "Rate on Google Play", which opens the store page (`store_review_opened`, `nps_survey`). The score is sent at once as a partial answer and Send or Skip completes the same submission; every send lists all four questions in the survey's order, as PostHog's own SDKs do; closing before a score is `survey dismissed`, closing on the follow-up keeps the score. Answers are capped at 500 characters. Wording can be changed from the `nps-survey-copy` flag's payload (any `NpsSurveyCopy` key; an empty `ideaTag` hides the tag). Long-pressing Home's Feedback icon opens it by hand without using up the schedule, and the debug screen's Flags tab says what the automatic survey still waits for and can reset its timing (last showing, shared cooldown, this session's prompt; `NpsSurveyDebugTest`). Echo stands on the sheet's top edge and plays one clip per step from its manifest (`survey-ask`, `survey-notes`, `survey-think`, `survey-idea`, `survey-thanks-calm`, `survey-thanks`, `survey-celebrate`), then rests with its eyes alive; reduced motion shows the resting still. - NPS survey layout: colours are theme roles only (score circles `primaryContainer`; the sheet white in light themes, `surfaceContainerHigh` in dark; the answer box `surfaceContainerHighest`, its edge and label `primary` when focused), so every preset, custom colour, wallpaper colour and Pure style applies. With the keyboard up the sheet rises above it, Echo shrinks to 132dp and the score chip and helper make room; windows under 600dp tall start Echo at 132dp, and the content scrolls when it still doesn't fit. On expanded and folded tablets it is a centered dialog. Before a score and on the thanks it closes like any sheet; on the follow-up only Skip, Send or Back close it, so a typed answer isn't lost to a stray tap. `NpsSurveySheetAdaptiveTest` keeps one survey, its step and its typed answer through phone ↔ tablet transitions across the 600dp and 480dp gates, 840dp, portrait/landscape and a half-opened fold in both directions with no event sent; checks the outside tap on the dialog before a score and on the follow-up, Back on the dialog's follow-up, the scroll position across 839 → 840dp, a survey waiting while something plays and one dropped after its window, and a short landscape phone reaching every score and Not now. `NpsSurveyControllerTest` covers the events and steps; `NpsSurveySheetPolicyTest` the Echo sequences against the manifest, Echo's sizes and the score circles. - `HomeRecommendationsApplyTest` covers `applyHomeRecommendations`: a failed request keeps the restored picks and saves nothing; fresh picks replace them once per episode ID and title and are saved with their fallback flag; an empty answer that did not fail clears them. - `HomeSerialLogicTest` covers serial progress IDs and picks, and an oldest-first show switched to its feed continuing after the episode its old show was playing, found under the ID the show lists. `HomeSelectedPodcastLogicTest` checks such a show's last play, and `HomeCatchUpLogicTest` that its latest episode played under the old ID reads as played. diff --git a/feature/home/src/main/java/cx/aswin/boxlore/feature/home/HomeViewModelLoadData.kt b/feature/home/src/main/java/cx/aswin/boxlore/feature/home/HomeViewModelLoadData.kt index 37f322e98..924872bea 100644 --- a/feature/home/src/main/java/cx/aswin/boxlore/feature/home/HomeViewModelLoadData.kt +++ b/feature/home/src/main/java/cx/aswin/boxlore/feature/home/HomeViewModelLoadData.kt @@ -212,8 +212,9 @@ internal fun HomeViewModel.loadData() { // Prompts count only episodes finished here, so a restore or import can't trigger them. val completedCount = allHistory.countFinishedSince(installedAtMs) - // Prompts appear only at a finish moment: the player here played an episode to the end in this run. - val lastFinishedAtMs = cx.aswin.boxlore.core.playback.PlaybackFinishes.lastAtMs + // Prompts appear only at a finish moment: the player here played an episode to the end + // recently, in this run or before a restart. + val lastFinishedAtMs = engagementCoordinator.lastFinishedAtMs(cx.aswin.boxlore.core.playback.PlaybackFinishes.lastAtMs) if (!isPromptSheetOpen) { val isPlayingNow = playerState.value.isPlaying if (engagementCoordinator.shouldShowNpsSurvey(isPlayingNow, installedAtMs, completedCount, lastFinishedAtMs)) { diff --git a/release-notes/upcoming.md b/release-notes/upcoming.md index 8a97ce125..da2c7a48f 100644 --- a/release-notes/upcoming.md +++ b/release-notes/upcoming.md @@ -38,3 +38,4 @@ Announcement title: - Tapping a link no app on your phone can open, like a host's page or a funding link, now says so instead of closing boxlore. - Playback controls on the lock screen, in notifications and in Samsung's Now Bar show rewind, play and forward in that order, even when nothing is queued next, with a Next episode button beside them when there is. - Fixed a crash that could close boxlore in the background while the player notification loaded an episode's artwork. +- Echo's occasional question about how likely you are to recommend boxlore reaches listeners it had been skipping since the last update. It can now also ask when you come back within a few hours of finishing an episode. From bc017584b6ea684ec14a62ec0a92e968f96d350c Mon Sep 17 00:00:00 2001 From: Aswin C Date: Sat, 7 Nov 2026 22:37:23 +0530 Subject: [PATCH 2/3] fix(analytics): ignore future-dated finishes; note the review prompt's moment --- .../core/catalog/EngagementPromptCoordinator.kt | 8 ++++++-- .../EngagementPromptCoordinatorSurveyTest.kt | 4 +++- .../cx/aswin/boxlore/core/prefs/NpsSurveyStore.kt | 13 ++++++++++--- .../aswin/boxlore/core/prefs/NpsSurveyStoreTest.kt | 14 +++++++++++--- feature/home/README.md | 2 +- 5 files changed, 31 insertions(+), 10 deletions(-) diff --git a/core/catalog/src/main/java/cx/aswin/boxlore/core/catalog/EngagementPromptCoordinator.kt b/core/catalog/src/main/java/cx/aswin/boxlore/core/catalog/EngagementPromptCoordinator.kt index 7789c41f8..bd43c4433 100644 --- a/core/catalog/src/main/java/cx/aswin/boxlore/core/catalog/EngagementPromptCoordinator.kt +++ b/core/catalog/src/main/java/cx/aswin/boxlore/core/catalog/EngagementPromptCoordinator.kt @@ -70,8 +70,12 @@ class EngagementPromptCoordinator( 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. */ - suspend fun lastFinishedAtMs(inThisRunAtMs: Long?): Long? = listOfNotNull(inThisRunAtMs, surveyStore.record().lastFinishedAtMs).maxOrNull() + /** + * 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 for a diff --git a/core/catalog/src/test/java/cx/aswin/boxlore/core/catalog/EngagementPromptCoordinatorSurveyTest.kt b/core/catalog/src/test/java/cx/aswin/boxlore/core/catalog/EngagementPromptCoordinatorSurveyTest.kt index 63d526ecc..ce76db8c6 100644 --- a/core/catalog/src/test/java/cx/aswin/boxlore/core/catalog/EngagementPromptCoordinatorSurveyTest.kt +++ b/core/catalog/src/test/java/cx/aswin/boxlore/core/catalog/EngagementPromptCoordinatorSurveyTest.kt @@ -70,7 +70,9 @@ class EngagementPromptCoordinatorSurveyTest { settle() val nextRun = EngagementPromptCoordinator(prefs, store, scope = scope) assertEquals(now - 60_000, nextRun.lastFinishedAtMs(inThisRunAtMs = null)) - assertEquals(now + 1, nextRun.lastFinishedAtMs(inThisRunAtMs = now + 1)) + 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), diff --git a/core/prefs/src/main/java/cx/aswin/boxlore/core/prefs/NpsSurveyStore.kt b/core/prefs/src/main/java/cx/aswin/boxlore/core/prefs/NpsSurveyStore.kt index 71eaea94b..9cf69a2af 100644 --- a/core/prefs/src/main/java/cx/aswin/boxlore/core/prefs/NpsSurveyStore.kt +++ b/core/prefs/src/main/java/cx/aswin/boxlore/core/prefs/NpsSurveyStore.kt @@ -30,9 +30,16 @@ class NpsSurveyStore internal constructor(private val dataStore: DataStore if (atMs > (prefs[LAST_FINISHED_AT] ?: 0L)) prefs[LAST_FINISHED_AT] = atMs } + /** + * The player here played an episode to the end at [atMs]; keeps the latest, so the finish outlives a + * restart. A kept finish later than [nowMs] (the clock was set ahead, then put right) is replaced, + * so it can never hold the moment back. + */ + suspend fun recordFinish(atMs: Long, nowMs: Long = System.currentTimeMillis()) { + dataStore.edit { prefs -> + val kept = prefs[LAST_FINISHED_AT] + if (kept == null || atMs > kept || kept > nowMs) prefs[LAST_FINISHED_AT] = atMs + } } /** Counts one start of boxlore on this install, so the survey never appears in the first one. */ diff --git a/core/prefs/src/test/java/cx/aswin/boxlore/core/prefs/NpsSurveyStoreTest.kt b/core/prefs/src/test/java/cx/aswin/boxlore/core/prefs/NpsSurveyStoreTest.kt index dd4502a1a..20dc713a7 100644 --- a/core/prefs/src/test/java/cx/aswin/boxlore/core/prefs/NpsSurveyStoreTest.kt +++ b/core/prefs/src/test/java/cx/aswin/boxlore/core/prefs/NpsSurveyStoreTest.kt @@ -46,12 +46,20 @@ class NpsSurveyStoreTest { @Test fun keepsTheLatestFinishAcrossRestarts() = runTest { - store.recordFinish(now - day) - store.recordFinish(now) - store.recordFinish(now - 2 * day) + store.recordFinish(now - day, nowMs = now) + store.recordFinish(now, nowMs = now) + store.recordFinish(now - 2 * day, nowMs = now) assertEquals(now, NpsSurveyStore(context).record().lastFinishedAtMs) } + @Test + fun aFinishKeptFromTheFutureIsReplacedByTheNextRealOne() = runTest { + // A clock set ahead, then put right: the kept finish must not outrank every real one after it. + store.recordFinish(now + 30 * day, nowMs = now + 30 * day) + store.recordFinish(now, nowMs = now) + assertEquals(now, store.record().lastFinishedAtMs) + } + @Test fun anUpdatedInstallWhoseListenerAnsweredTheOldSurveyWaitsFromTheUpdate() = runTest { carry(legacyShown = true, legacyAnswered = true) diff --git a/feature/home/README.md b/feature/home/README.md index 7202c6f8f..457f1fa12 100644 --- a/feature/home/README.md +++ b/feature/home/README.md @@ -129,7 +129,7 @@ Main Kotlin files should remain below 1000 lines; extracted Home feed, ViewModel ## Testing notes -- Review prompt: Home counts only episodes played to the end after this install (`countFinishedSince` on `HomeListeningHistoryItem.finishedAt`, with `HomeViewModel.installedAtMs` from Android's install time; synced listening from another phone after that counts too), so marked-played episodes and a restore of older listening never trigger it, and the sheet's "You've finished N episodes" uses the same count. A decided prompt waits in `ReviewPromptRecorder` and is recorded only once its sheet is on screen with Home resumed (`onReviewPromptDisplayed`: milestone, cooldown and `engagement_prompt_shown`); deciding only reserves the session (`reserveSessionForPrompt`), so a prompt decided behind another screen isn't used up if boxlore closes first (`ReviewPromptRecorderTest`). It also appears only at a finish moment (the player here played an episode to the end in this run, `PlaybackFinishes` in `:core:playback`), never on an open alone, a sync or a mark-played. The category chips send `home_surface_tapped` (`category_filter`). `HomeListeningHistoryItemTest` covers the count. +- Review prompt: Home counts only episodes played to the end after this install (`countFinishedSince` on `HomeListeningHistoryItem.finishedAt`, with `HomeViewModel.installedAtMs` from Android's install time; synced listening from another phone after that counts too), so marked-played episodes and a restore of older listening never trigger it, and the sheet's "You've finished N episodes" uses the same count. A decided prompt waits in `ReviewPromptRecorder` and is recorded only once its sheet is on screen with Home resumed (`onReviewPromptDisplayed`: milestone, cooldown and `engagement_prompt_shown`); deciding only reserves the session (`reserveSessionForPrompt`), so a prompt decided behind another screen isn't used up if boxlore closes first (`ReviewPromptRecorderTest`). It also appears only at a finish moment (the player here played an episode to the end within the last 6 hours, in this run or before a restart: `PlaybackFinishes` in `:core:playback`, kept by `EngagementPromptCoordinator.recordFinish`; marking played, sync and restores never count), never on an open alone, a sync or a mark-played. The category chips send `home_surface_tapped` (`category_filter`). `HomeListeningHistoryItemTest` covers the count. - NPS survey: Home owns it. At a finish moment with nothing playing and no other sheet, `EngagementPromptCoordinator.shouldShowNpsSurvey` (`NpsSurveySchedule` in `:core:prefs`) decides it and `NpsSurveyController` opens it; `NpsSurveyHost` shows it only once nothing plays, drops it silently if it hasn't appeared within the finish moment's 6 hours (the finish comes from `EngagementPromptCoordinator.lastFinishedAtMs`, so one before a restart counts), and it counts as shown (`survey shown`, the schedule used up) only on screen with Home resumed. Steps: a 0–10 score (two staggered rows of same-size circles, "Not likely" over the 0 and "Very likely" under the 10), one follow-up by band (0–6 "What's the one thing we should fix first?", 7–8 "What would turn this up to a 10?", 9–10 "What should we queue up next?" with the ideas tag), then thanks; 9s and 10s get "Rate on Google Play", which opens the store page (`store_review_opened`, `nps_survey`). The score is sent at once as a partial answer and Send or Skip completes the same submission; every send lists all four questions in the survey's order, as PostHog's own SDKs do; closing before a score is `survey dismissed`, closing on the follow-up keeps the score. Answers are capped at 500 characters. Wording can be changed from the `nps-survey-copy` flag's payload (any `NpsSurveyCopy` key; an empty `ideaTag` hides the tag). Long-pressing Home's Feedback icon opens it by hand without using up the schedule, and the debug screen's Flags tab says what the automatic survey still waits for and can reset its timing (last showing, shared cooldown, this session's prompt; `NpsSurveyDebugTest`). Echo stands on the sheet's top edge and plays one clip per step from its manifest (`survey-ask`, `survey-notes`, `survey-think`, `survey-idea`, `survey-thanks-calm`, `survey-thanks`, `survey-celebrate`), then rests with its eyes alive; reduced motion shows the resting still. - NPS survey layout: colours are theme roles only (score circles `primaryContainer`; the sheet white in light themes, `surfaceContainerHigh` in dark; the answer box `surfaceContainerHighest`, its edge and label `primary` when focused), so every preset, custom colour, wallpaper colour and Pure style applies. With the keyboard up the sheet rises above it, Echo shrinks to 132dp and the score chip and helper make room; windows under 600dp tall start Echo at 132dp, and the content scrolls when it still doesn't fit. On expanded and folded tablets it is a centered dialog. Before a score and on the thanks it closes like any sheet; on the follow-up only Skip, Send or Back close it, so a typed answer isn't lost to a stray tap. `NpsSurveySheetAdaptiveTest` keeps one survey, its step and its typed answer through phone ↔ tablet transitions across the 600dp and 480dp gates, 840dp, portrait/landscape and a half-opened fold in both directions with no event sent; checks the outside tap on the dialog before a score and on the follow-up, Back on the dialog's follow-up, the scroll position across 839 → 840dp, a survey waiting while something plays and one dropped after its window, and a short landscape phone reaching every score and Not now. `NpsSurveyControllerTest` covers the events and steps; `NpsSurveySheetPolicyTest` the Echo sequences against the manifest, Echo's sizes and the score circles. - `HomeRecommendationsApplyTest` covers `applyHomeRecommendations`: a failed request keeps the restored picks and saves nothing; fresh picks replace them once per episode ID and title and are saved with their fallback flag; an empty answer that did not fail clears them. From dd75a50958a961f8b62e585dd8b7110f78478a45 Mon Sep 17 00:00:00 2001 From: Aswin C Date: Sat, 7 Nov 2026 22:48:45 +0530 Subject: [PATCH 3/3] fix(analytics): never keep a finish later than now --- .../java/cx/aswin/boxlore/core/prefs/NpsSurveyStore.kt | 5 +++-- .../cx/aswin/boxlore/core/prefs/NpsSurveyStoreTest.kt | 8 ++++++++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/core/prefs/src/main/java/cx/aswin/boxlore/core/prefs/NpsSurveyStore.kt b/core/prefs/src/main/java/cx/aswin/boxlore/core/prefs/NpsSurveyStore.kt index 9cf69a2af..0d5cf94fc 100644 --- a/core/prefs/src/main/java/cx/aswin/boxlore/core/prefs/NpsSurveyStore.kt +++ b/core/prefs/src/main/java/cx/aswin/boxlore/core/prefs/NpsSurveyStore.kt @@ -32,11 +32,12 @@ class NpsSurveyStore internal constructor(private val dataStore: DataStore + if (atMs > nowMs) return@edit val kept = prefs[LAST_FINISHED_AT] if (kept == null || atMs > kept || kept > nowMs) prefs[LAST_FINISHED_AT] = atMs } diff --git a/core/prefs/src/test/java/cx/aswin/boxlore/core/prefs/NpsSurveyStoreTest.kt b/core/prefs/src/test/java/cx/aswin/boxlore/core/prefs/NpsSurveyStoreTest.kt index 20dc713a7..b7a6c5497 100644 --- a/core/prefs/src/test/java/cx/aswin/boxlore/core/prefs/NpsSurveyStoreTest.kt +++ b/core/prefs/src/test/java/cx/aswin/boxlore/core/prefs/NpsSurveyStoreTest.kt @@ -60,6 +60,14 @@ class NpsSurveyStoreTest { assertEquals(now, store.record().lastFinishedAtMs) } + @Test + fun aFinishFromTheFutureNeverReplacesAKeptOne() = runTest { + // The clock stepped back between the finish and its save: the earlier real finish must stay. + store.recordFinish(now - 60_000, nowMs = now) + store.recordFinish(now + day, nowMs = now) + assertEquals(now - 60_000, store.record().lastFinishedAtMs) + } + @Test fun anUpdatedInstallWhoseListenerAnsweredTheOldSurveyWaitsFromTheUpdate() = runTest { carry(legacyShown = true, legacyAnswered = true)