Keep decided Side quests off the deck and show refused decisions

A saved decision now reads the deck again whenever a read started before
the save is still under way, so a late answer can no longer put the
decided Side quest back. Optimistic Try it adds its dinner to the Quest
log only when it is not there yet, and a refusal removes only that
addition. A refused decision's ErrorBanner with Retry now shows whatever
the deck has come to, including when a read emptied it during the exit.

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

web/src/discover/DiscoverPage.test.tsx

@@ -8,6 +8,7 @@ import { declaredStyle } from '../test/cssRules.ts'
 import { errorResponse, fakeLocation, jsonResponse, ME } from '../test/fetchStub.ts'
 import { expectMinHitArea } from '../test/hitArea.ts'
 import { mockViewport } from '../test/matchMedia.ts'
+import { DECK_KEY } from './api.ts'
 import { DiscoverPage } from './DiscoverPage.tsx'
 
 const DECK = '/api/v1/suggestions/deck'
@@ -102,13 +103,13 @@ async function finishRunning(animations: FakeAnimation[]) {
 }
 
 // renderPage loads /me (for the CSRF token every write needs) and renders
-// the Discover page.
+// the Discover page with queryClient.
-async function renderPage() {
+async function renderPage(queryClient = createQueryClient()) {
   const api = createApiClient(fakeLocation('/discover'), createNetworkStatus(window), createVerifiedSession(localStorage))
   await api.loadMe()
   return render(
     <ApiProvider client={api}>
-      <QueryClientProvider client={createQueryClient()}>
+      <QueryClientProvider client={queryClient}>
         <DiscoverPage />
       </QueryClientProvider>
     </ApiProvider>,
@@ -610,6 +611,92 @@ describe('DiscoverPage', () => {
       expect(screen.getByText('Quest log · 7')).toBeInTheDocument()
     })
 
+    it('keeps a saved decision off the deck when a read started during it answers late', async () => {
+      const decline = deferred()
+      let late: ReturnType<typeof deferred> | null = null
+      const server = stubServer([BOWLS, TIKKA, ORZO, PAELLA], {
+        'POST /api/v1/suggestions/s1/decline': () => decline.promise,
+        [`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: 'Pass' }))
+      await waitFor(() => expect(sent(server.fetchMock)).toEqual(['POST /api/v1/suggestions/s1/decline']))
+      expect(topCard()).toHaveAccessibleName('Crock pot chicken tikka masala')
+
+      // A read of the deck starts before the decision is saved, and answers
+      // only after: with the Side quest still pending.
+      late = deferred()
+      const stale = late
+      await act(async () => void queryClient.invalidateQueries({ queryKey: DECK_KEY }))
+      expect(deckReads(server.fetchMock)).toBe(2)
+      late = null
+      server.deck = [TIKKA, ORZO, PAELLA]
+      decline.resolve(declined(BOWLS))
+      await waitFor(() => expect(screen.getByRole('button', { name: 'Pass' })).toBeEnabled())
+      await act(async () => stale.resolve(jsonResponse(200, { suggestions: [BOWLS, TIKKA, ORZO, PAELLA] })))
+
+      // The late answer is dropped, and the deck is read again after the save.
+      await act(async () => await new Promise((resolve) => setTimeout(resolve, 0)))
+      expect(screen.queryByRole('article', { name: 'Korean beef bowls' })).not.toBeInTheDocument()
+      await waitFor(() => expect(deckReads(server.fetchMock)).toBe(3))
+      expect(screen.getAllByRole('article', { hidden: true }).map((card) => card.querySelector('h2')!.textContent)).toEqual([
+        'Crock pot chicken tikka masala',
+        'Lemon garlic shrimp orzo',
+        'Sheet pan paella',
+      ])
+    })
+
+    it('leaves the Quest log count alone for a dinner already in it, on Try it and on its refusal', async () => {
+      const accept = deferred()
+      stubServer([BOWLS, TIKKA, ORZO, PAELLA], {
+        'POST /api/v1/suggestions/s1/accept': () => accept.promise,
+        // A read of the Quest log already has the dinner the stale deck offers.
+        [`GET ${QUEST_LOG}`]: () =>
+          jsonResponse(200, {
+            dinners: [
+              { id: 'q0', name: 'Quest 0' },
+              { id: 'dinner-s1', name: 'Korean beef bowls' },
+            ],
+          }),
+      })
+      await renderPage()
+      await screen.findByRole('article', { name: 'Korean beef bowls' })
+      await screen.findByText('Quest log · 2')
+
+      fireEvent.click(screen.getByRole('button', { name: 'Try it' }))
+      await waitFor(() => expect(topCard()).toHaveAccessibleName('Crock pot chicken tikka masala'))
+      expect(screen.getByText('Quest log · 2')).toBeInTheDocument()
+
+      accept.resolve(errorResponse(500, 'internal'))
+      await screen.findByRole('alert')
+      expect(topCard()).toHaveAccessibleName('Korean beef bowls')
+      expect(screen.getByText('Quest log · 2')).toBeInTheDocument()
+    })
+
+    it('shows a decision refused because the deck changed during the exit, with Retry', async () => {
+      const server = stubServer([BOWLS])
+      const queryClient = createQueryClient()
+      const animations = stubAnimate({ autoFinish: false })
+      await renderPage(queryClient)
+      await screen.findByRole('article', { name: 'Korean beef bowls' })
+
+      // While the card leaves, a read of the deck takes the last card away.
+      fireEvent.click(screen.getByRole('button', { name: 'Try it' }))
+      server.deck = []
+      await act(async () => await queryClient.invalidateQueries({ queryKey: DECK_KEY }))
+      await screen.findByRole('heading', { name: 'All caught up' })
+
+      await finishRunning(animations)
+      await finishRunning(animations)
+      const alert = await screen.findByRole('alert')
+      expect(alert).toHaveTextContent("Couldn't save that choice. suggestion s1 is not in the deck")
+      expect(within(alert).getByRole('button', { name: 'Retry' })).toBeInTheDocument()
+      expect(sent(server.fetchMock)).toEqual([])
+    })
+
     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/DiscoverPage.tsx

@@ -362,11 +362,6 @@ export function DiscoverPage() {
             <Icon icon={Heart} size="nav" />
           </IconButton>
         </div>
-        {decision.isError && (
-          <ErrorBanner onRetry={() => submit(decision.variables!)}>
-            Couldn't save that choice. {decision.error.message}
-          </ErrorBanner>
-        )}
       </>
     )
   }
@@ -384,6 +379,10 @@ export function DiscoverPage() {
         <ErrorBanner onRetry={() => void questLog.refetch()}>Couldn't load the Quest log. {questLog.error.message}</ErrorBanner>
       )}
       {body}
+      {/* A refused decision is shown whatever the deck has come to since. */}
+      {decision.isError && (
+        <ErrorBanner onRetry={() => submit(decision.variables!)}>Couldn't save that choice. {decision.error.message}</ErrorBanner>
+      )}
     </div>
   )
 }

web/src/discover/api.ts

@@ -135,8 +135,9 @@ export function useStartBatch() {
 }
 
 // Decided is the context a decision's optimistic update leaves for its
-// rollback: the Side quest taken off the deck.
+// rollback: the Side quest taken off the deck, and whether its dinner was
-type Decided = { quest: SideQuest }
+// added to the Quest log.
+type Decided = { quest: SideQuest; added: boolean }
 
 // useDecide records Try it (POST /api/v1/suggestions/{id}/accept) or Pass
 // (POST /api/v1/suggestions/{id}/decline). The answer must be that
@@ -144,10 +145,11 @@ type Decided = { quest: SideQuest }
 // replenish job the decision started, or null.
 //
 // The decision is applied at once: the Side quest leaves the deck and, on
-// Try it, its dinner joins the Quest log. A refused decision puts the Side
+// Try it, its dinner joins the Quest log unless it is there already. A
-// quest back on top of the deck and takes its dinner out of the Quest log
+// refused decision puts the Side quest back on top of the deck and takes
-// again. A saved one reads the Quest log again, and the deck too once fewer
+// out of the Quest log only the dinner it added. A saved one reads the Quest
-// than REFILL_BELOW Side quests are left in it.
+// 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()
@@ -176,28 +178,30 @@ export function useDecide() {
       const quest = queryClient.getQueryData<SideQuest[]>(DECK_KEY)?.find((q) => q.id === id)
       if (quest === undefined) throw new Error(`suggestion ${id} is not in the deck`)
       queryClient.setQueryData<SideQuest[]>(DECK_KEY, (deck) => deck?.filter((q) => q.id !== id))
-      if (choice === 'try') {
+      // A stale deck can offer a Side quest whose dinner a read of the Quest
-        queryClient.setQueryData<QuestLogDinner[]>(QUEST_LOG_KEY, (dinners) =>
+      // log already has.
-          dinners?.concat({ id: quest.dinner_id, name: quest.name }),
+      const dinners = queryClient.getQueryData<QuestLogDinner[]>(QUEST_LOG_KEY)
-        )
+      const added = choice === 'try' && dinners !== undefined && !dinners.some((d) => d.id === quest.dinner_id)
-      }
+      if (added) queryClient.setQueryData<QuestLogDinner[]>(QUEST_LOG_KEY, dinners.concat({ id: quest.dinner_id, name: quest.name }))
-      return { quest }
+      return { quest, added }
     },
-    onError: (_error, { choice }, context) => {
+    onError: (_error, _variables, context) => {
       // onMutate refusing leaves nothing to undo.
       if (context === undefined) return
-      const { quest } = context
+      const { quest, added } = context
       queryClient.setQueryData<SideQuest[]>(DECK_KEY, (deck) => deck && [quest, ...deck.filter((q) => q.id !== quest.id)])
-      if (choice === 'try') {
+      if (added) {
         queryClient.setQueryData<QuestLogDinner[]>(QUEST_LOG_KEY, (dinners) => dinners?.filter((d) => d.id !== quest.dinner_id))
       }
     },
     onSuccess: async (_replenishJobId, { id }) => {
       // A read that landed while the decision was in flight may have put
-      // the Side quest back.
+      // the Side quest back, and one still under way may yet: invalidating
+      // cancels it for a read made after the decision was saved.
+      const reading = queryClient.isFetching({ queryKey: DECK_KEY }) > 0
       const deck = queryClient.setQueryData<SideQuest[]>(DECK_KEY, (current) => current?.filter((q) => q.id !== id))
       await Promise.all([
-        deck !== undefined && deck.length < REFILL_BELOW ? queryClient.invalidateQueries({ queryKey: DECK_KEY }) : undefined,
+        reading || (deck !== undefined && deck.length < REFILL_BELOW) ? queryClient.invalidateQueries({ queryKey: DECK_KEY }) : undefined,
         queryClient.invalidateQueries({ queryKey: QUEST_LOG_KEY }),
       ])
     },