fix(app): keep the onboarding view model in the activity's store

app/README.md

@@ -16,6 +16,7 @@
 ## Composition root and startup
 
 - `AppContainer` is created once in `BoxLoreApplication.onCreate` and wires database, network, auth, RSS, catalog, ranking, playback, queue, downloads, prefs, support and analytics in the [object graph order](../ARCHITECTURE.md#identity-and-storage).
+- `ui/rememberOnboardingViewModel` keeps the one `OnboardingViewModel` in the activity's `ViewModelStore`, so every onboarding flow shares it and it is cleared when the activity finishes; never create it with `remember`.
 - The one `PlaybackRepository` is built lazily. Cast state waits in `DeferredCastSessionState` until it exists, so widget, worker and notification starts never build playback on the main thread.
 - The container gives core modules ports instead of reverse dependencies: `feedTelemetry` for feed events, `rolloutSwitches`, and a `DownloadServiceLauncherHolder` that starts `MediaDownloadService`.
 - `onCreate` order: API sessions first, then the Coil image loader (before widgets and services), Media3 Cast from `BoxLoreCastOptionsProvider`, the shared `UserPreferencesRepository`, the container and its holders. PostHog starts later on a background thread through `PostHogStartup`, after `AnalyticsHelper.initPolicy`.

app/src/main/java/cx/aswin/boxlore/ui/BoxLoreAppRoot.kt

@@ -240,8 +240,9 @@ fun BoxLoreAppRoot(
         FcmTopicHelper.reconcileAfterRestoreIfNeeded(activity, subscriptionRepository)
     }
 
+    // One instance for every onboarding flow, held by the activity so it is cleared when the activity finishes.
     val onboardingViewModel =
-        remember {
+        rememberOnboardingViewModel(owner = activity) {
             cx.aswin.boxlore.feature.onboarding.OnboardingViewModel(
                 application,
                 podcastRepository,

app/src/main/java/cx/aswin/boxlore/ui/OnboardingViewModelScope.kt

@@ -0,0 +1,38 @@
+package cx.aswin.boxlore.ui
+
+import androidx.compose.runtime.Composable
+import androidx.lifecycle.ViewModel
+import androidx.lifecycle.ViewModelProvider
+import androidx.lifecycle.ViewModelStoreOwner
+import androidx.lifecycle.viewmodel.compose.viewModel
+import cx.aswin.boxlore.feature.onboarding.OnboardingViewModel
+
+/** The key the activity's onboarding view model is stored under. */
+internal const val ONBOARDING_VIEW_MODEL_KEY = "boxlore-onboarding"
+
+/**
+ * The activity's one [OnboardingViewModel], kept in [owner]'s view model store rather than in
+ * composition.
+ *
+ * Every onboarding path shares this instance: search, Echo (AI), genre picks, the import sheet,
+ * Home's AI onboarding entry and the sign-in detour through Settings. Shows picked in one flow
+ * therefore carry over to the others, and the instance survives the activity being recreated.
+ *
+ * Because the store owns it, `onCleared` runs when the activity finishes for good: Echo requests
+ * are cancelled, deck audio is released, and an onboarding left unfinished is reported as
+ * abandoned. Created with `remember`, it was never cleared, so none of that happened.
+ *
+ * [build] runs once per store; later calls return the stored instance.
+ */
+@Composable
+internal fun rememberOnboardingViewModel(
+    owner: ViewModelStoreOwner,
+    build: () -> OnboardingViewModel,
+): OnboardingViewModel = viewModel(
+    viewModelStoreOwner = owner,
+    key = ONBOARDING_VIEW_MODEL_KEY,
+    factory = object : ViewModelProvider.Factory {
+        @Suppress("UNCHECKED_CAST")
+        override fun <T : ViewModel> create(modelClass: Class<T>): T = build() as T
+    },
+)

app/src/test/java/cx/aswin/boxlore/ui/OnboardingViewModelRecreationTest.kt

@@ -0,0 +1,111 @@
+package cx.aswin.boxlore.ui
+
+import android.app.Application
+import android.os.Looper
+import androidx.activity.ComponentActivity
+import androidx.activity.compose.setContent
+import androidx.room.Room
+import androidx.test.core.app.ApplicationProvider
+import cx.aswin.boxlore.core.analytics.AnalyticsHelper
+import cx.aswin.boxlore.core.catalog.PodcastRepository
+import cx.aswin.boxlore.core.catalog.SubscriptionRepository
+import cx.aswin.boxlore.core.database.BoxLoreDatabase
+import cx.aswin.boxlore.core.prefs.BoxcastPrefs
+import cx.aswin.boxlore.core.prefs.UserPreferencesRepository
+import cx.aswin.boxlore.core.rss.RssPodcastRepository
+import cx.aswin.boxlore.core.testing.TestFixtures
+import cx.aswin.boxlore.feature.onboarding.OnboardingViewModel
+import org.junit.After
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertNotNull
+import org.junit.Assert.assertSame
+import org.junit.Assert.assertTrue
+import org.junit.Before
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.robolectric.Robolectric
+import org.robolectric.RobolectricTestRunner
+import org.robolectric.Shadows.shadowOf
+import org.robolectric.android.controller.ActivityController
+import org.robolectric.annotation.Config
+
+/**
+ * A real activity recreated mid-onboarding keeps its onboarding view model and the shows picked so
+ * far, and reports nothing; only finishing it for good clears the view model and reports the
+ * unfinished onboarding as abandoned.
+ */
+@RunWith(RobolectricTestRunner::class)
+@Config(sdk = [34], application = Application::class)
+class OnboardingViewModelRecreationTest {
+    private val context: Application = ApplicationProvider.getApplicationContext()
+    private lateinit var database: BoxLoreDatabase
+    private lateinit var controller: ActivityController<ComponentActivity>
+    private val events = mutableListOf<Pair<String, Map<String, Any>>>()
+    private lateinit var restoreAnalytics: () -> Unit
+    private var builds = 0
+
+    @Before
+    fun setUp() {
+        database = Room.inMemoryDatabaseBuilder(context, BoxLoreDatabase::class.java).allowMainThreadQueries().build()
+        RssPodcastRepository.clearInstanceForTests()
+        BoxcastPrefs(context).setOnboardingCompleted(false)
+        restoreAnalytics = AnalyticsHelper.installRecordingSink(events)
+        controller = Robolectric.buildActivity(ComponentActivity::class.java).setup()
+    }
+
+    @After
+    fun tearDown() {
+        restoreAnalytics()
+        database.close()
+        RssPodcastRepository.clearInstanceForTests()
+    }
+
+    private fun build(): OnboardingViewModel {
+        builds++
+        val podcastRepository =
+            PodcastRepository(
+                baseUrl = "http://localhost/",
+                publicKey = "test-app-key",
+                context = context,
+                rssRepository = RssPodcastRepository.createForTests(context = context, database = database),
+            )
+        return OnboardingViewModel(context, podcastRepository, SubscriptionRepository(database.podcastDao()), UserPreferencesRepository(context))
+    }
+
+    /** Sets the activity's content the way the app root does and returns the view model it gets. */
+    private fun showOnboarding(): OnboardingViewModel {
+        var viewModel: OnboardingViewModel? = null
+        val activity = controller.get()
+        activity.setContent { viewModel = rememberOnboardingViewModel(owner = activity, build = ::build) }
+        shadowOf(Looper.getMainLooper()).idle()
+        assertNotNull(viewModel)
+        return viewModel!!
+    }
+
+    @Test
+    fun `recreating the activity mid-onboarding keeps the view model and its picks and reports nothing`() {
+        val before = showOnboarding()
+        before.onWelcomeScreenViewed()
+        before.setShowsSelected(listOf(TestFixtures.podcast(id = "show-1"), TestFixtures.podcast(id = "show-2")), selected = true)
+
+        controller.recreate()
+        val after = showOnboarding()
+
+        assertSame(before, after)
+        assertEquals(1, builds)
+        assertEquals(setOf("show-1", "show-2"), after.uiState.value.selectedPodcasts.keys)
+        assertTrue(events.none { it.first == "onboarding_abandoned" })
+    }
+
+    @Test
+    fun `finishing the activity after a recreation reports the unfinished onboarding once`() {
+        showOnboarding().onWelcomeScreenViewed()
+        controller.recreate()
+        showOnboarding()
+
+        controller.get().finish()
+        controller.pause().stop().destroy()
+
+        assertEquals(1, events.count { it.first == "onboarding_abandoned" })
+    }
+}

app/src/test/java/cx/aswin/boxlore/ui/OnboardingViewModelScopeTest.kt

@@ -0,0 +1,142 @@
+package cx.aswin.boxlore.ui
+
+import android.app.Application
+import androidx.compose.runtime.getValue
+import androidx.compose.runtime.mutableStateOf
+import androidx.compose.runtime.setValue
+import androidx.compose.ui.test.junit4.createComposeRule
+import androidx.lifecycle.ViewModelStore
+import androidx.lifecycle.ViewModelStoreOwner
+import androidx.lifecycle.viewModelScope
+import androidx.room.Room
+import androidx.test.core.app.ApplicationProvider
+import cx.aswin.boxlore.core.analytics.AnalyticsHelper
+import cx.aswin.boxlore.core.catalog.PodcastRepository
+import cx.aswin.boxlore.core.catalog.SubscriptionRepository
+import cx.aswin.boxlore.core.database.BoxLoreDatabase
+import cx.aswin.boxlore.core.prefs.BoxcastPrefs
+import cx.aswin.boxlore.core.prefs.UserPreferencesRepository
+import cx.aswin.boxlore.core.rss.RssPodcastRepository
+import cx.aswin.boxlore.feature.onboarding.OnboardingViewModel
+import cx.aswin.boxlore.feature.onboarding.markOnboardingCompletedSilent
+import kotlinx.coroutines.isActive
+import org.junit.After
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertFalse
+import org.junit.Assert.assertSame
+import org.junit.Assert.assertTrue
+import org.junit.Before
+import org.junit.Rule
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.robolectric.RobolectricTestRunner
+import org.robolectric.annotation.Config
+
+/**
+ * The onboarding view model lives in the activity's view model store: every onboarding flow shares
+ * one instance (so picks carry over between search, Echo and the import sheet), and finishing the
+ * activity clears it. Created with `remember` it was never cleared, so an unfinished onboarding was
+ * never reported as abandoned and its Echo work was never cancelled.
+ */
+@RunWith(RobolectricTestRunner::class)
+@Config(sdk = [34], application = Application::class)
+class OnboardingViewModelScopeTest {
+    @get:Rule val composeRule = createComposeRule()
+
+    private val context: Application = ApplicationProvider.getApplicationContext()
+    private lateinit var database: BoxLoreDatabase
+    private val store = ViewModelStore()
+    private val activityOwner = object : ViewModelStoreOwner {
+        override val viewModelStore: ViewModelStore = store
+    }
+    private val events = mutableListOf<Pair<String, Map<String, Any>>>()
+    private lateinit var restoreAnalytics: () -> Unit
+    private var builds = 0
+
+    @Before
+    fun setUp() {
+        database = Room.inMemoryDatabaseBuilder(context, BoxLoreDatabase::class.java).allowMainThreadQueries().build()
+        RssPodcastRepository.clearInstanceForTests()
+        BoxcastPrefs(context).setOnboardingCompleted(false)
+        restoreAnalytics = AnalyticsHelper.installRecordingSink(events)
+    }
+
+    @After
+    fun tearDown() {
+        store.clear()
+        restoreAnalytics()
+        database.close()
+        RssPodcastRepository.clearInstanceForTests()
+    }
+
+    private fun build(): OnboardingViewModel {
+        builds++
+        val podcastRepository =
+            PodcastRepository(
+                baseUrl = "http://localhost/",
+                publicKey = "test-app-key",
+                context = context,
+                rssRepository = RssPodcastRepository.createForTests(context = context, database = database),
+            )
+        return OnboardingViewModel(context, podcastRepository, SubscriptionRepository(database.podcastDao()), UserPreferencesRepository(context))
+    }
+
+    @Test
+    fun `every onboarding flow gets the same instance while the activity lives`() {
+        var showImportSheet by mutableStateOf(false)
+        var showOnboarding by mutableStateOf(true)
+        val seen = mutableListOf<OnboardingViewModel>()
+        composeRule.setContent {
+            // Separate call sites stand for the onboarding route, the import sheet and Home's AI entry.
+            if (showOnboarding) seen += rememberOnboardingViewModel(activityOwner, ::build)
+            if (showImportSheet) seen += rememberOnboardingViewModel(activityOwner, ::build)
+        }
+
+        composeRule.runOnIdle { showImportSheet = true }
+        // Leaving onboarding (e.g. for the sign-in detour) and coming back keeps the same instance.
+        composeRule.runOnIdle { showOnboarding = false }
+        composeRule.runOnIdle { showOnboarding = true }
+        composeRule.waitForIdle()
+
+        assertEquals(1, builds)
+        assertTrue(seen.size >= 3)
+        seen.forEach { assertSame(seen.first(), it) }
+    }
+
+    @Test
+    fun `finishing the activity mid-onboarding reports it abandoned once and stops its work`() {
+        lateinit var viewModel: OnboardingViewModel
+        composeRule.setContent { viewModel = rememberOnboardingViewModel(activityOwner, ::build) }
+        composeRule.runOnIdle { viewModel.onWelcomeScreenViewed() }
+
+        composeRule.runOnIdle { store.clear() }
+
+        val abandoned = events.filter { it.first == "onboarding_abandoned" }
+        assertEquals(1, abandoned.size)
+        assertTrue(abandoned.single().second.containsKey("last_step"))
+        assertFalse(viewModel.viewModelScope.isActive)
+    }
+
+    @Test
+    fun `a finished onboarding is not reported when the activity finishes`() {
+        lateinit var viewModel: OnboardingViewModel
+        composeRule.setContent { viewModel = rememberOnboardingViewModel(activityOwner, ::build) }
+        composeRule.runOnIdle {
+            viewModel.onWelcomeScreenViewed()
+            viewModel.markOnboardingCompletedSilent {}
+        }
+
+        composeRule.runOnIdle { store.clear() }
+
+        assertTrue(events.none { it.first == "onboarding_abandoned" })
+    }
+
+    @Test
+    fun `an onboarding never started is not reported when the activity finishes`() {
+        composeRule.setContent { rememberOnboardingViewModel(activityOwner, ::build) }
+
+        composeRule.runOnIdle { store.clear() }
+
+        assertTrue(events.none { it.first == "onboarding_abandoned" })
+    }
+}

feature/onboarding/README.md

@@ -72,7 +72,7 @@ src/main/res/values/                   # echo_onboarding.xml, onboarding_suggest
 
 ## Threading / lifecycle
 
-- `OnboardingViewModel` is constructed directly in the app root's composition (`remember`), not through a `ViewModelStore`, so it outlives the onboarding route. Its `onCleared` cancels Echo and deck requests, releases the clip pool and records abandonment.
+- `OnboardingViewModel` is one instance per activity, kept in the activity's `ViewModelStore` by `:app` (`rememberOnboardingViewModel`). Every onboarding flow shares it (search, Echo, genres, the import sheet, Home's AI entry), so picks carry over between them. Its `onCleared` runs when the activity finishes: it cancels Echo and deck requests, releases the clip pool and reports an unfinished onboarding as abandoned.
 - Echo turn, synthesis, deck and similar-shows jobs cancel on Back, Search, the topic fallback, restart and skip. Cancelled work never replaces a restored question or shows an error; cancellation is rethrown, not reported.
 - Genre synthesis runs `async` on `Dispatchers.IO` and catches failure inside the child (`orFallbackOnFailure`), because an exception escaping an `async` child fails its parent.
 - Subscribing a show asks the application-scoped `subscriptionForegroundSync` for a catalog ingest, so leaving onboarding cannot cancel it.