Keep a decided Side quest off the deck after an uncertain decision failure

A decision now follows the reads of the deck that land while it is in
flight. When it fails, the Side quest is put back on top of the deck only
if no such read has left it out, so a card the server already decided is
not offered again.

A failure that is not an API error answer, such as a lost response, may
have come after the server saved the decision, so the deck is read again
to reconcile it. An API error answer still puts the card back on top.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

web/src/discover/DiscoverPage.test.tsx

@@ -736,6 +736,41 @@ describe('DiscoverPage', () => {
       expect(screen.getByRole('heading', { name: 'All caught up' })).toBeInTheDocument()
     })
 
+    it('keeps a card off the deck when a read during its decision left it out and the answer is lost, and reads the deck again', async () => {
+      let lose!: (error: Error) => void
+      const accept = new Promise<Response>((_, reject) => (lose = reject))
+      let late: ReturnType<typeof deferred> | null = null
+      const server = stubServer([BOWLS, TIKKA, ORZO, PAELLA], {
+        'POST /api/v1/suggestions/s1/accept': () => accept,
+        [`GET ${DECK}`]: () => late?.promise ?? jsonResponse(200, { suggestions: server.deck }),
+      })
+      const queryClient = createQueryClient()
+      await renderPage(queryClient)
+      await screen.findByRole('article', { name: 'Korean beef bowls' })
+
+      fireEvent.click(screen.getByRole('button', { name: 'Try it' }))
+      await waitFor(() => expect(sent(server.fetchMock)).toEqual(['POST /api/v1/suggestions/s1/accept']))
+
+      // The decision is saved, and a read of the deck started during it
+      // answers without the Side quest; then the answer to the decision is
+      // lost on the way back.
+      late = deferred()
+      const delayed = late
+      await act(async () => void queryClient.invalidateQueries({ queryKey: DECK_KEY }))
+      late = null
+      server.deck = [TIKKA, ORZO, PAELLA]
+      await act(async () => delayed.resolve(jsonResponse(200, { suggestions: server.deck })))
+      const reads = deckReads(server.fetchMock)
+      await act(async () => lose(new TypeError('Failed to fetch')))
+
+      await screen.findByRole('alert')
+      expect(screen.queryByRole('article', { name: 'Korean beef bowls' })).not.toBeInTheDocument()
+      // The decision may have been saved: the deck is read again.
+      await waitFor(() => expect(deckReads(server.fetchMock)).toBe(reads + 1))
+      expect(screen.queryByRole('article', { name: 'Korean beef bowls' })).not.toBeInTheDocument()
+      expect(topCard()).toHaveAccessibleName('Crock pot chicken tikka masala')
+    })
+
     it('reads the deck again once a saved decision leaves fewer than 3 cards', async () => {
       const server = stubServer([BOWLS, TIKKA, ORZO], {
         'POST /api/v1/suggestions/s1/decline': () => {

web/src/discover/api.ts

@@ -1,5 +1,5 @@
-import { useMutation, useQuery, useQueryClient } from '@tanstack/react-query'
-import { useApi } from '../api/index.ts'
+import { hashKey, useMutation, useQuery, useQueryClient } from '@tanstack/react-query'
+import { ApiError, useApi } from '../api/index.ts'
 import { isRecord } from '../api/errors.ts'
 import { DIFFICULTIES, DINNERS_PATH, oneOf, photoId, type Difficulty } from '../dinners/api.ts'
 
@@ -137,9 +137,14 @@ export function useStartBatch() {
 }
 
 // Decided is the context a decision's optimistic update leaves for its
-// rollback: whether the Side quest was taken off the deck, and the mark of
-// the Quest log entry it added, if it added one.
-type Decided = { taken: boolean; adding: symbol | null }
+// rollback: whether the Side quest was taken off the deck, the mark of the
+// Quest log entry it added, if it added one, and what the reads of the deck
+// that landed since have said of the Side quest.
+type Decided = { taken: boolean; adding: symbol | null; reads: DeckReads }
+
+// DeckReads follows the reads of the deck that land while a decision is in
+// flight: gone is whether the latest left the Side quest out.
+type DeckReads = { gone: boolean; stop: () => void }
 
 // useDecide records Try it (POST /api/v1/suggestions/{id}/accept) or Pass
 // (POST /api/v1/suggestions/{id}/decline) on the Side quest the household
@@ -150,11 +155,13 @@ type Decided = { taken: boolean; adding: symbol | null }
 // The decision is applied at once: the Side quest leaves the deck and, on
 // Try it, its dinner joins the Quest log unless it is there already. A
 // refused decision puts the Side quest back on top of the deck if it took
-// it off, takes out of the Quest log only the entry it added, and reads the
-// Quest log again, since the decision may have been saved all the same. A
-// saved one reads the Quest log again, and the deck too once fewer than
-// REFILL_BELOW Side quests are left in it or a read of the deck started
-// before the decision was saved.
+// it off and no read of the deck since has left it out, takes out of the
+// Quest log only the entry it added, and reads the Quest log again, since
+// the decision may have been saved all the same. A failure without an
+// answer from the server, which may have saved it, reads the deck again
+// too. A saved one reads the Quest log again, and the deck too once fewer
+// than REFILL_BELOW Side quests are left in it or a read of the deck
+// started before the decision was saved.
 export function useDecide() {
   const { apiFetch } = useApi()
   const queryClient = useQueryClient()
@@ -183,17 +190,26 @@ export function useDecide() {
       const deck = queryClient.getQueryData<SideQuest[]>(DECK_KEY)
       const taken = deck !== undefined && deck.some((q) => q.id === quest.id)
       if (taken) queryClient.setQueryData<SideQuest[]>(DECK_KEY, deck.filter((q) => q.id !== quest.id))
+      const reads: DeckReads = { gone: false, stop: () => {} }
+      const deckHash = hashKey(DECK_KEY)
+      reads.stop = queryClient.getQueryCache().subscribe((event) => {
+        if (event.type !== 'updated' || event.query.queryHash !== deckHash || event.action.type !== 'success' || event.action.manual) return
+        reads.gone = !(event.action.data as SideQuest[]).some((q) => q.id === quest.id)
+      })
       // A stale deck can offer a Side quest whose dinner a read of the Quest
       // log already has.
       const dinners = queryClient.getQueryData<QuestLogDinner[]>(QUEST_LOG_KEY)
-      if (choice !== 'try' || dinners === undefined || dinners.some((d) => d.id === quest.dinner_id)) return { taken, adding: null }
+      if (choice !== 'try' || dinners === undefined || dinners.some((d) => d.id === quest.dinner_id)) return { taken, adding: null, reads }
       const adding = Symbol(quest.dinner_id)
       queryClient.setQueryData<QuestLogDinner[]>(QUEST_LOG_KEY, dinners.concat({ id: quest.dinner_id, name: quest.name, adding }))
-      return { taken, adding }
+      return { taken, adding, reads }
     },
-    onError: (_error, { quest }, context) => {
-      const { taken, adding } = context!
-      if (taken) queryClient.setQueryData<SideQuest[]>(DECK_KEY, (deck) => deck && [quest, ...deck.filter((q) => q.id !== quest.id)])
+    onError: (error, { quest }, context) => {
+      const { taken, adding, reads } = context!
+      if (taken && !reads.gone) queryClient.setQueryData<SideQuest[]>(DECK_KEY, (deck) => deck && [quest, ...deck.filter((q) => q.id !== quest.id)])
+      // An API error answer is the server refusing the decision; anything
+      // else may have come after it saved it.
+      if (!(error instanceof ApiError)) void queryClient.invalidateQueries({ queryKey: DECK_KEY })
       if (adding === null) return
       // A read of the Quest log that landed meanwhile has replaced the entry.
       queryClient.setQueryData<QuestLogDinner[]>(QUEST_LOG_KEY, (dinners) => dinners?.filter((d) => d.adding !== adding))
@@ -210,5 +226,6 @@ export function useDecide() {
         queryClient.invalidateQueries({ queryKey: QUEST_LOG_KEY }),
       ])
     },
+    onSettled: (_data, _error, _variables, context) => context?.reads.stop(),
   })
 }