Fix cache-miss spinner and animate screen state changes

.claude/skills/circuit-screen/SKILL.md

@@ -81,8 +81,10 @@ return state.foldToState(
 `Success(isRefreshing = true)` rather than dropping back to `Loading`. Do not add a `retain`ed var
 beside it to do that job — that pattern predates `ContentState` and is gone.
 
-Inside `foldToState`, **`hasLoaded` is the discriminator, not `data.isEmpty()`**: an empty list is a
-real answer, and reading it as "nothing yet" hangs a spinner over an empty screen. See the
+Inside `foldToState`, **`hasAnswer` is the discriminator, not `data.isEmpty()`**: an empty list is a
+real answer, and reading it as "nothing yet" hangs a spinner over an empty screen. The one empty list
+that isn't an answer is an empty read from cache while its request is still in flight. The
+repository holds it back (`asOutcomes(fetching = refresh) { … }`) and `hasAnswer` refuses it. See the
 Architecture section of `CLAUDE.md`.
 
 ### Screen persistence

CLAUDE.md

@@ -160,7 +160,8 @@ full pattern when adding a screen.
 Repositories return `Flow<Outcome<T>>`, not Store's own `StoreReadResponse`. **Store 5 is an
 implementation detail of `:repository`** — it is an `implementation` dependency there, and nothing
 above that module compiles against it. `io.github.solcott:dataresult-store5` does the translation
-in `asOutcomes()`, at the end of each repository's chain.
+in `asOutcomes(fetching = refresh) { … }`, at the end of each repository's chain. It sits *inside*
+the `flatMapLatest { refresh -> … }` so it can see whether a fetch is coming.
 
 The types come from [kmp-dataresult](https://github.com/solcott/kmp-dataresult), shared with the
 `Countries` project. Resolving them needs `gpr.user`/`gpr.key` in `~/.gradle/gradle.properties` (a
@@ -169,7 +170,10 @@ classic PAT with `read:packages`) — GitHub Packages authenticates even public
 The path from a repository to a screen state is three steps:
 
 1. `Flow<Outcome<T>>` — `Loading`, `Data(value, origin)` or `Error(cause, origin)`. Store's
-   `Initial` and `Loading` both become `Outcome.Loading`; `NoNewData` is dropped.
+   `Initial` and `Loading` both become `Outcome.Loading`; `NoNewData` is dropped. An empty first
+   read from cache while a fetch is pending is held back until the fetch answers. Room returns `[]`
+   for a key it has never fetched, and that's a cache miss, not an empty result.
+   `getFavoritesAsFlow` passes no `fetching`, because nothing is coming to answer instead.
 2. `produceRetainedContentState(initial, keys) { … }` from `libs.uistateCircuit`
    (`io.github.solcott.uistate.circuit`) folds those emissions into a retained `ContentState<T>`.
    **`ContentState.data` is what holds the last loaded value** — the separate retained
@@ -187,10 +191,14 @@ the example.
 
 Two things are easy to get wrong:
 
-- **Check `hasLoaded`, never `data.isEmpty()`**, to tell "nothing has loaded yet" from "loaded and
-  empty". `hasLoaded` reads `origin`, which stays null until the first value arrives. Testing the
-  data for emptiness leaves a spinner over a legitimately empty tab forever.
-- **Having data beats being in flight.** `foldToState` checks `hasLoaded` first, which is what
+- **Check `hasAnswer`, never `data.isEmpty()`**, to tell "nothing has loaded yet" from "loaded and
+  empty". `hasLoaded` reads `origin`, which stays null until the first value arrives. `hasAnswer`
+  also refuses an empty read *from cache* while its request is in flight or has failed. That's a key
+  the database has never seen, not an empty result. The repositories already hold that read back
+  with `asOutcomes(fetching = refresh) { … }`, and `hasAnswer` is the backstop. Testing the data for
+  emptiness leaves a spinner over a legitimately empty tab forever. `hasAnswer` can't, because it is
+  only false while a request is outstanding or has failed.
+- **Having data beats being in flight.** `foldToState` checks `hasAnswer` first, which is what
   renders a background refresh as `isRefreshing` over the existing grid instead of dropping back to
   a full-screen spinner. `domain`'s `refreshKeepsPreviousAreas` test pins this — don't reorder those
   branches.

domain/src/commonMain/kotlin/com/scottolcott/recipe/domain/presenter/Content.kt

@@ -0,0 +1,28 @@
+package com.scottolcott.recipe.domain.presenter
+
+import io.github.solcott.uistate.ContentState
+import io.github.solcott.uistate.errorOrNull
+import io.github.solcott.uistate.isLoading
+
+/**
+ * [foldToState] for a screen that shows one optional item, such as a recipe by id.
+ *
+ * Simpler than the list version, because a non-null item is always an answer: the one read
+ * `hasAnswer` exists to refuse -- an empty read from cache while its request is still outstanding
+ * -- is a null here, and a null is never content. So having the item wins over being in flight (a
+ * refresh renders as content that is refreshing, not a spinner); no item while in flight is still
+ * loading; and no item once settled is an error -- the request failed, or the source answered that
+ * there is no such item.
+ */
+internal inline fun <T : Any, S> ContentState<T?>.foldToState(
+  onLoading: () -> S,
+  onError: (message: String) -> S,
+  onContent: (item: T, isRefreshing: Boolean) -> S,
+): S {
+  val item = data
+  return when {
+    item != null -> onContent(item, isLoading)
+    isLoading -> onLoading()
+    else -> onError(errorOrNull.toMessage())
+  }
+}

domain/src/commonMain/kotlin/com/scottolcott/recipe/domain/presenter/ListContent.kt

@@ -3,7 +3,7 @@ package com.scottolcott.recipe.domain.presenter
 import io.github.solcott.dataresult.DataError
 import io.github.solcott.uistate.ContentState
 import io.github.solcott.uistate.errorOrNull
-import io.github.solcott.uistate.hasLoaded
+import io.github.solcott.uistate.hasAnswer
 import io.github.solcott.uistate.isLoading
 
 /**
@@ -17,17 +17,20 @@ import io.github.solcott.uistate.isLoading
  * keeps the last list it loaded, so a background refresh reports it as content that is refreshing
  * rather than dropping the grid for a spinner.
  *
- * [ContentState.hasLoaded] is what separates the first two branches, not `data.isEmpty()` — an
- * empty list is a real answer, and reading it as "nothing yet" would leave a spinner over a
- * legitimately empty tab forever.
+ * `hasAnswer` is what separates the first two branches, not `data.isEmpty()` — an empty list is a
+ * real answer, and reading it as "nothing yet" would leave a spinner over a legitimately empty tab
+ * forever. What it adds over `hasLoaded` is the one empty list that is *not* an answer: an empty
+ * read from cache while its request is still in flight or has failed. That is a key the database
+ * has never seen, so it gets the spinner, or the error, rather than "nothing found". The
+ * repositories already hold that read back with `asOutcomes(fetching = …)`; this is the backstop.
  */
 internal inline fun <T, S> ContentState<List<T>>.foldToState(
   onLoading: () -> S,
   onError: (message: String) -> S,
   onContent: (items: List<T>, isRefreshing: Boolean) -> S,
 ): S =
   when {
-    hasLoaded -> onContent(data, isLoading)
+    hasAnswer(List<T>::isEmpty) -> onContent(data, isLoading)
     isLoading -> onLoading()
     else -> onError(errorOrNull.toMessage())
   }

domain/src/commonMain/kotlin/com/scottolcott/recipe/domain/presenter/RecipeDetailsPresenter.kt

@@ -5,6 +5,8 @@ import androidx.compose.runtime.getValue
 import androidx.compose.runtime.mutableIntStateOf
 import androidx.compose.runtime.remember
 import androidx.compose.runtime.rememberCoroutineScope
+import androidx.compose.runtime.rememberUpdatedState
+import androidx.compose.runtime.retain.retain
 import androidx.compose.runtime.setValue
 import com.scottolcott.recipe.domain.presenter.RecipesScreen.ByArea
 import com.scottolcott.recipe.domain.presenter.RecipesScreen.ByCategory
@@ -22,8 +24,6 @@ import dev.zacsweers.metro.AppScope
 import dev.zacsweers.metro.Inject
 import dev.zacsweers.redacted.annotations.Redacted
 import io.github.solcott.uistate.circuit.produceRetainedContentState
-import io.github.solcott.uistate.errorOrNull
-import io.github.solcott.uistate.isLoading
 import kotlinx.coroutines.launch
 
 @CircuitInject(RecipeDetailsScreen::class, AppScope::class)
@@ -37,54 +37,78 @@ internal constructor(
   @Composable
   override fun present(): RecipeDetailsState {
     val coroutineScope = rememberCoroutineScope()
-    var retryTrigger by remember { mutableIntStateOf(0) }
+    var retryTrigger by retain { mutableIntStateOf(0) }
     val state =
       produceRetainedContentState<Recipe?>(null, retryTrigger) {
         recipeRepository.getById(screen.id)
       }
-    // Unlike the old response-shaped state, this keeps the recipe on screen through a refresh
-    // rather than blanking it whenever the store goes back to the network.
-    val recipe: Recipe? = state.data
-    return RecipeDetailsState(
-      recipe,
-      loading = state.isLoading,
-      error = state.errorOrNull != null,
-    ) { event ->
-      when (event) {
-        RecipeDetailsEvent.ToggleFavorite ->
-          coroutineScope.launch {
-            if (recipe != null) {
+    // Read through updated state: the sink below is remembered once, so closing over a plain local
+    // would pin it to the recipe from the first composition -- which is always null.
+    val latestRecipe by rememberUpdatedState(state.data)
+    val successEventSink: (RecipeDetailsEvent.Success) -> Unit = remember {
+      { event ->
+        when (event) {
+          RecipeDetailsEvent.Success.ToggleFavorite ->
+            coroutineScope.launch {
+              val recipe = latestRecipe ?: return@launch
               if (recipe.favorite) {
                 recipeRepository.removeFavorite(screen.id)
               } else {
                 recipeRepository.addFavorite(screen.id)
               }
             }
-          }
-        RecipeDetailsEvent.RetryClicked -> retryTrigger++
-        is RecipeDetailsEvent.CategoryClicked -> navigator.goTo(ByCategory(event.category))
+          is RecipeDetailsEvent.Success.CategoryClicked ->
+            navigator.goTo(ByCategory(event.category))
 
-        is RecipeDetailsEvent.AreaClicked -> navigator.goTo(ByArea(event.area))
+          is RecipeDetailsEvent.Success.AreaClicked -> navigator.goTo(ByArea(event.area))
+        }
       }
     }
+
+    val errorEventSink: (RecipeDetailsEvent.Error) -> Unit = remember {
+      { event ->
+        when (event) {
+          RecipeDetailsEvent.Error.RetryClicked -> retryTrigger++
+        }
+      }
+    }
+    return state.foldToState(
+      onLoading = { RecipeDetailsState.Loading },
+      onError = { message -> RecipeDetailsState.Error(message, errorEventSink) },
+      onContent = { recipe, isRefreshing ->
+        RecipeDetailsState.Success(recipe, isRefreshing, successEventSink)
+      },
+    )
   }
 }
 
-data class RecipeDetailsState(
-  val recipe: Recipe?,
-  val loading: Boolean,
-  val error: Boolean,
-  @Redacted val eventSink: (RecipeDetailsEvent) -> Unit,
-) : CircuitUiState
+sealed interface RecipeDetailsState : CircuitUiState {
+  data object Loading : RecipeDetailsState
+
+  data class Error(
+    val message: String,
+    @Redacted val eventSink: (RecipeDetailsEvent.Error) -> Unit,
+  ) : RecipeDetailsState
+
+  data class Success(
+    val recipe: Recipe,
+    val isRefreshing: Boolean,
+    @Redacted val eventSink: (RecipeDetailsEvent.Success) -> Unit,
+  ) : RecipeDetailsState
+}
 
 sealed interface RecipeDetailsEvent : CircuitUiEvent {
-  data object ToggleFavorite : RecipeDetailsEvent
+  sealed interface Success : RecipeDetailsEvent {
+    data class CategoryClicked(val category: String) : Success
 
-  data object RetryClicked : RecipeDetailsEvent
+    data class AreaClicked(val area: String) : Success
 
-  data class CategoryClicked(val category: String) : RecipeDetailsEvent
+    data object ToggleFavorite : Success
+  }
 
-  data class AreaClicked(val area: String) : RecipeDetailsEvent
+  sealed interface Error : RecipeDetailsEvent {
+    data object RetryClicked : Error
+  }
 }
 
 @CircuitSerializable(AppScope::class) data class RecipeDetailsScreen(val id: RecipeId) : Screen