[criticalAY]

Note

Assisted-by: Opus 5.5

Purpose / Description

Deck Picker crashed if the backend failed while deleting a deck, deleting empty cards, rebuilding or emptying a filtered deck, selecting a new deck or refreshing the list. The ViewModel now catches these errors and shows them, as David asked on #21831.

• Depends on feat: Deckpicker progress adoption #21831.

Fixes

• Follow-up to feat: Deckpicker progress adoption #21831

Approach

check commit messages

How Has This Been Tested?

added tests, the error ones crashed before the fix

Learning (optional, can help others)

NA

Checklist

Please, go through these checks before submitting the PR.

• You have a descriptive commit message with a short title (first line, max 50 chars).
• You have commented your code, particularly in hard-to-understand areas
• You have performed a self-review of your own code
• UI changes: include screenshots of all affected screens (in particular showing any new or changed strings)
• UI Changes: You have tested your change using the Google Accessibility Scanner

[github-actions]

Snapshot diff report: Previous regressions were resolved by the latest commit.

[criticalAY]

Legit failure, fixed it

[mikehardy]

Assisted-by: cursor-grok-4.6-medium

The stopped-lifecycle and database-corruption tests from the last request are on this tip. ViewModel failures now go through MessageQueue into showException instead of crashing.

Rebased d8d16bd onto main (7e29d24) cleanly and did not push. Local tip is 695da54f. Lint (lintAll, ktlintCheck, lint-rules:test) passed in 55s. Unit (jacocoUnitTestReport) failed once on CardContentProviderStartupTest and a flaky ReviewerNoParamTest; both passed alone, and neither class is in this diff. DeckPickerErrorTest was 9/9. Emulator tests on emulator-5554 (API 36, playRelease) passed. The XML noise was the usual phone-sized failures plus a tablet assumption in DeckPickerImportTest, which this diff does not touch. ReviewRemindersTest was 9/9.

showException is the same dialog path runCatching already used, including corruption recovery. Delete, empty cards, filtered rebuild and empty, select, and deck-list reload catch into a queue that DeckPicker drains after CREATED and withStarted. Rebuild refresh stays on the ViewModel flow. The Activity no longer also calls updateDeckList and tryShowStudyOptionsPanel after join.

some acceptable findings, no action required, just leaving for posterity: Approach is "check commit messages"; launchCatchingIO still uses the string onError SharedFlow.

[david-allison]

@criticalAY Tests are failing

[david-allison]

GPT-6.1

Subject: [PATCH] PR 22504: test replacement refresh expansion race

diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerReplacementRefreshReviewTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerReplacementRefreshReviewTest.kt
new file mode 100644
index 0000000000..3acce2a009
--- /dev/null
+++ b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerReplacementRefreshReviewTest.kt
@@ -0,0 +1,112 @@
+// SPDX-License-Identifier: GPL-3.0-or-later
+
+package com.ichi2.anki
+
+import androidx.lifecycle.viewModelScope
+import androidx.test.ext.junit.runners.AndroidJUnit4
+import anki.decks.DeckTreeNode
+import com.ichi2.anki.deckpicker.DeckPickerViewModel
+import com.ichi2.anki.libanki.sched.DeckNode
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.ExperimentalForInheritanceCoroutinesApi
+import kotlinx.coroutines.cancel
+import kotlinx.coroutines.flow.MutableStateFlow
+import kotlinx.coroutines.runBlocking
+import net.ankiweb.rsdroid.Backend
+import net.ankiweb.rsdroid.BackendFactory
+import org.junit.After
+import org.junit.Test
+import org.junit.runner.RunWith
+import java.util.concurrent.CountDownLatch
+import java.util.concurrent.TimeUnit
+import java.util.concurrent.atomic.AtomicBoolean
+import kotlin.concurrent.thread
+import kotlin.test.assertEquals
+import kotlin.test.assertTrue
+
+@OptIn(ExperimentalForInheritanceCoroutinesApi::class)
+@RunWith(AndroidJUnit4::class)
+class DeckPickerReplacementRefreshReviewTest : RobolectricTest() {
+    @After
+    fun resetBackend() = BackendFactory.setOverride(null)
+
+    @Test
+    fun `refresh replacing the one awaited by a toggle must not publish stale expansion`() {
+        val readingFirst = CountDownLatch(1)
+        val releaseFirst = CountDownLatch(1)
+        val pauseFirst = AtomicBoolean(false)
+        runBlocking { CollectionManager.discardBackend() }
+        BackendFactory.setOverride {
+            object : Backend() {
+                override fun deckTree(now: Long): DeckTreeNode {
+                    if (pauseFirst.compareAndSet(true, false)) {
+                        readingFirst.countDown()
+                        check(releaseFirst.await(10, TimeUnit.SECONDS))
+                    }
+                    return super.deckTree(now)
+                }
+            }
+        }
+        val parent = addDeck("Parent")
+        addDeck("Parent::Child")
+        val model = DeckPickerViewModel()
+        val initial = model.reloadDeckCounts()
+        advanceRobolectricLooperUntil { initial.isCompleted }
+        val initiallyCollapsed = model.dueTree!!.find(parent)!!.collapsed
+        val treeField = model.javaClass.getDeclaredField("flowOfDeckDueTree").apply { isAccessible = true }
+        @Suppress("UNCHECKED_CAST")
+        val original = treeField.get(model) as MutableStateFlow<DeckNode?>
+        val publishingReplacement = CountDownLatch(1)
+        val releaseReplacement = CountDownLatch(1)
+        val gatePublication = AtomicBoolean(true)
+        treeField.set(model, object : MutableStateFlow<DeckNode?> by original {
+            override var value: DeckNode?
+                get() = original.value
+                set(value) {
+                    if (gatePublication.compareAndSet(true, false)) {
+                        publishingReplacement.countDown()
+                        check(releaseReplacement.await(10, TimeUnit.SECONDS))
+                    }
+                    original.value = value
+                }
+        })
+        val previousQueue = CollectionManager.setTestDispatcher(Dispatchers.IO.limitedParallelism(1), useReentrantLock = false)
+        try {
+            addDeck("New deck that makes the replacement snapshot different")
+            pauseFirst.set(true)
+            val first = model.reloadDeckCounts()
+            assertTrue(readingFirst.await(5, TimeUnit.SECONDS))
+            // This action waits for the first refresh, which a subsequent refresh cancels.
+            val toggle = model.toggleDeckExpand(parent)
+            val replacement = model.reloadDeckCounts()
+            releaseFirst.countDown()
+            // A shared lock may let the toggle finish before the replacement reads its tree.
+            // Give that correct implementation a chance to drain Main as well.
+            if (!publishingReplacement.await(2, TimeUnit.SECONDS)) {
+                advanceRobolectricLooperUntil { publishingReplacement.count == 0L }
+            }
+            val toggleCompleted = CountDownLatch(1)
+            toggle.invokeOnCompletion { toggleCompleted.countDown() }
+            val releaser = thread(name = "release-replacement-refresh") {
+                // A correct implementation can serialize the toggle behind this refresh.
+                // Permit that ordering while exposing the stale publication in the current code.
+                toggleCompleted.await(2, TimeUnit.SECONDS)
+                releaseReplacement.countDown()
+            }
+            advanceRobolectricLooperUntil { first.isCompleted && toggle.isCompleted && replacement.isCompleted }
+            releaser.join()
+            assertEquals(!initiallyCollapsed, col.decks.get(parent)!!.collapsed)
+            assertEquals(
+                col.decks.get(parent)!!.collapsed,
+                model.dueTree!!.find(parent)!!.collapsed,
+                "The replacement refresh must not undo the saved expansion in the displayed tree",
+            )
+        } finally {
+            releaseFirst.countDown()
+            releaseReplacement.countDown()
+            treeField.set(model, original)
+            model.viewModelScope.cancel()
+            CollectionManager.setTestDispatcher(previousQueue)
+        }
+    }
+}