Roll back only the decision's own Quest log entry and send stale-card decisions

A refused Try it now removes the Quest log entry it tagged when it added
it, so a read of the Quest log that landed during the decision is kept,
and reads the Quest log again since the decision may have been saved.

A decision is sent for the card thrown out even when a read of the deck
took it off during the exit, so Retry on that refusal posts the decision
again instead of failing before sending it.

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

web/src/discover/DiscoverPage.test.tsx

@@ -676,8 +676,40 @@ describe('DiscoverPage', () => {
       expect(screen.getByText('Quest log · 2')).toBeInTheDocument()
     })
 
-    it('shows a decision refused because the deck changed during the exit, with Retry', async () => {
+    it('takes out of the Quest log only the entry a refused Try it added, after a read of the Quest log lands during it', async () => {
-      const server = stubServer([BOWLS])
+      const accept = deferred()
+      const questLog = [{ id: 'q0', name: 'Quest 0' }]
+      const server = stubServer([BOWLS, TIKKA, ORZO, PAELLA], {
+        'POST /api/v1/suggestions/s1/accept': () => accept.promise,
+        [`GET ${QUEST_LOG}`]: () => jsonResponse(200, { dinners: questLog }),
+      })
+      const queryClient = createQueryClient()
+      await renderPage(queryClient)
+      await screen.findByRole('article', { name: 'Korean beef bowls' })
+      await screen.findByText('Quest log · 1')
+
+      fireEvent.click(screen.getByRole('button', { name: 'Try it' }))
+      await waitFor(() => expect(sent(server.fetchMock)).toEqual(['POST /api/v1/suggestions/s1/accept']))
+      expect(screen.getByText('Quest log · 2')).toBeInTheDocument()
+
+      // The decision is saved, and a read of the Quest log lands with its
+      // dinner in it; then the answer to the decision is lost.
+      questLog.push({ id: 'dinner-s1', name: 'Korean beef bowls' })
+      await act(async () => await queryClient.invalidateQueries({ queryKey: ['dinners', 'quest_log'] }))
+      accept.resolve(errorResponse(500, 'internal'))
+      await screen.findByRole('alert')
+      expect(screen.getByText('Quest log · 2')).toBeInTheDocument()
+      // The answer was lost, so the Quest log is read again.
+      const questLogReads = () => server.fetchMock.mock.calls.filter(([input]) => String(input) === QUEST_LOG).length
+      await waitFor(() => expect(questLogReads()).toBe(3))
+      expect(screen.getByText('Quest log · 2')).toBeInTheDocument()
+    })
+
+    it('sends a decision for the card thrown out even when a read of the deck took it away during the exit, and Retry sends it again', async () => {
+      let refuse = true
+      const server = stubServer([BOWLS], {
+        'POST /api/v1/suggestions/s1/accept': () => (refuse ? errorResponse(500, 'internal') : accepted(BOWLS)),
+      })
       const queryClient = createQueryClient()
       const animations = stubAnimate({ autoFinish: false })
       await renderPage(queryClient)
@@ -692,9 +724,16 @@ describe('DiscoverPage', () => {
       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(alert).toHaveTextContent("Couldn't save that choice.")
-      expect(within(alert).getByRole('button', { name: 'Retry' })).toBeInTheDocument()
+      expect(sent(server.fetchMock)).toEqual(['POST /api/v1/suggestions/s1/accept'])
-      expect(sent(server.fetchMock)).toEqual([])
+      // The read had taken the card away: refused, it is not put back.
+      expect(screen.queryByRole('article')).not.toBeInTheDocument()
+
+      refuse = false
+      fireEvent.click(within(alert).getByRole('button', { name: 'Retry' }))
+      await waitFor(() => expect(screen.queryByRole('alert')).not.toBeInTheDocument())
+      expect(sent(server.fetchMock)).toEqual(['POST /api/v1/suggestions/s1/accept', 'POST /api/v1/suggestions/s1/accept'])
+      expect(screen.getByRole('heading', { name: 'All caught up' })).toBeInTheDocument()
     })
 
     it('reads the deck again once a saved decision leaves fewer than 3 cards', async () => {

web/src/discover/DiscoverPage.tsx

@@ -17,6 +17,7 @@ import {
   useQuestLog,
   useStartBatch,
   type Choice,
+  type SideQuest,
 } from './api.ts'
 import { DeckCard, DECK_DEPTH, type SwipeHandlers } from './DeckCard.tsx'
 import {
@@ -152,8 +153,8 @@ export function DiscoverPage() {
   // send records a decision. A refused one puts the card back; a saved one
   // that started a replenish batch follows its job.
   const send = useCallback(
-    (variables: { id: string; choice: Choice }) => {
+    (variables: { quest: SideQuest; choice: Choice }) => {
-      leaving.current = variables.id
+      leaving.current = variables.quest.id
       mutateDecision(variables, {
         onSuccess: (replenishJobId) => {
           if (replenishJobId !== null) setBatchJobId(replenishJobId)
@@ -172,7 +173,7 @@ export function DiscoverPage() {
     [mutateDecision],
   )
   const submit = useCallback(
-    (variables: { id: string; choice: Choice }) => {
+    (variables: { quest: SideQuest; choice: Choice }) => {
       if (lock()) send(variables)
     },
     [lock, send],
@@ -217,7 +218,7 @@ export function DiscoverPage() {
         () => {
           // The Quest log pill is missing while the Quest log fails to load.
           if (choice === 'try' && !reduced && questLogRef.current !== null) bump(questLogRef.current)
-          send({ id: top.id, choice })
+          send({ quest: top, choice })
         },
         (error: unknown) => {
           if (!isAbort(error)) throw error

web/src/discover/api.ts

@@ -38,7 +38,9 @@ export const BATCH_JOB_KIND = 'suggestion_batch'
 export type BatchResult = { batch_id: string; stage: string; suggestion_ids: string[]; dropped: number }
 
 // QuestLogDinner is a dinner in the Quest log, as the Quest log pill lists it.
-export type QuestLogDinner = { id: string; name: string }
+// adding marks the entry a Try it put in before it was saved, so its
+// rollback takes out that entry and no other.
+export type QuestLogDinner = { id: string; name: string; adding?: symbol }
 
 // REFILL_BELOW is the deck size, the depths the deck shows, under which a
 // saved decision reads the deck again.
@@ -135,26 +137,29 @@ export function useStartBatch() {
 }
 
 // Decided is the context a decision's optimistic update leaves for its
-// rollback: the Side quest taken off the deck, and whether its dinner was
+// rollback: whether the Side quest was taken off the deck, and the mark of
-// added to the Quest log.
+// the Quest log entry it added, if it added one.
-type Decided = { quest: SideQuest; added: boolean }
+type Decided = { taken: boolean; adding: symbol | null }
 
 // useDecide records Try it (POST /api/v1/suggestions/{id}/accept) or Pass
-// (POST /api/v1/suggestions/{id}/decline). The answer must be that
+// (POST /api/v1/suggestions/{id}/decline) on the Side quest the household
-// suggestion, decided that way; the mutation's data is the id of the
+// decided on, whether or not a read of the deck has since taken it away.
-// replenish job the decision started, or null.
+// The answer must be that suggestion, decided that way; the mutation's data
+// is the id of the 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 unless it is there already. A
-// refused decision puts the Side quest back on top of the deck and takes
+// refused decision puts the Side quest back on top of the deck if it took
-// out of the Quest log only the dinner it added. A saved one reads the Quest
+// it off, takes out of the Quest log only the entry it added, and reads the
-// log again, and the deck too once fewer than REFILL_BELOW Side quests are
+// Quest log again, since the decision may have been saved all the same. A
-// left in it or a read of the deck started before the decision was saved.
+// 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()
   return useMutation({
-    mutationFn: async ({ id, choice }: { id: string; choice: Choice }): Promise<string | null> => {
+    mutationFn: async ({ quest: { id }, choice }: { quest: SideQuest; choice: Choice }): Promise<string | null> => {
       const action = choice === 'try' ? 'accept' : 'decline'
       const path = `${SUGGESTIONS_PATH}/${encodeURIComponent(id)}/${action}`
       const body = await apiFetch<unknown>(path, { method: 'POST' })
@@ -171,30 +176,30 @@ export function useDecide() {
       }
       return body.replenish_job_id
     },
-    onMutate: async ({ id, choice }): Promise<Decided> => {
+    onMutate: async ({ quest, choice }): Promise<Decided> => {
       // A read already under way would answer with the Side quest still
       // pending, undoing the update.
       await Promise.all([queryClient.cancelQueries({ queryKey: DECK_KEY }), queryClient.cancelQueries({ queryKey: QUEST_LOG_KEY })])
-      const quest = queryClient.getQueryData<SideQuest[]>(DECK_KEY)?.find((q) => q.id === id)
+      const deck = queryClient.getQueryData<SideQuest[]>(DECK_KEY)
-      if (quest === undefined) throw new Error(`suggestion ${id} is not in the deck`)
+      const taken = deck !== undefined && deck.some((q) => q.id === quest.id)
-      queryClient.setQueryData<SideQuest[]>(DECK_KEY, (deck) => deck?.filter((q) => q.id !== id))
+      if (taken) queryClient.setQueryData<SideQuest[]>(DECK_KEY, deck.filter((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)
-      const added = choice === 'try' && dinners !== undefined && !dinners.some((d) => d.id === quest.dinner_id)
+      if (choice !== 'try' || dinners === undefined || dinners.some((d) => d.id === quest.dinner_id)) return { taken, adding: null }
-      if (added) queryClient.setQueryData<QuestLogDinner[]>(QUEST_LOG_KEY, dinners.concat({ id: quest.dinner_id, name: quest.name }))
+      const adding = Symbol(quest.dinner_id)
-      return { quest, added }
+      queryClient.setQueryData<QuestLogDinner[]>(QUEST_LOG_KEY, dinners.concat({ id: quest.dinner_id, name: quest.name, adding }))
+      return { taken, adding }
     },
-    onError: (_error, _variables, context) => {
+    onError: (_error, { quest }, context) => {
-      // onMutate refusing leaves nothing to undo.
+      const { taken, adding } = context!
-      if (context === undefined) return
+      if (taken) queryClient.setQueryData<SideQuest[]>(DECK_KEY, (deck) => deck && [quest, ...deck.filter((q) => q.id !== quest.id)])
-      const { quest, added } = context
+      if (adding === null) return
-      queryClient.setQueryData<SideQuest[]>(DECK_KEY, (deck) => deck && [quest, ...deck.filter((q) => q.id !== quest.id)])
+      // A read of the Quest log that landed meanwhile has replaced the entry.
-      if (added) {
+      queryClient.setQueryData<QuestLogDinner[]>(QUEST_LOG_KEY, (dinners) => dinners?.filter((d) => d.adding !== adding))
-        queryClient.setQueryData<QuestLogDinner[]>(QUEST_LOG_KEY, (dinners) => dinners?.filter((d) => d.id !== quest.dinner_id))
+      void queryClient.invalidateQueries({ queryKey: QUEST_LOG_KEY })
-      }
     },
-    onSuccess: async (_replenishJobId, { id }) => {
+    onSuccess: async (_replenishJobId, { quest: { id } }) => {
       // A read that landed while the decision was in flight may have put
       // the Side quest back, and one still under way may yet: invalidating
       // cancels it for a read made after the decision was saved.