From 0ea1c218eafab372f1abd9c46c0f1614ccbddabb Mon Sep 17 00:00:00 2001 From: dev_agent Date: Wed, 7 Oct 2026 02:53:09 -0400 Subject: [PATCH] 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 --- web/src/discover/DiscoverPage.test.tsx | 49 ++++++++++++++++++--- web/src/discover/DiscoverPage.tsx | 9 ++-- web/src/discover/api.ts | 61 ++++++++++++++------------ 3 files changed, 82 insertions(+), 37 deletions(-) diff --git a/web/src/discover/DiscoverPage.test.tsx b/web/src/discover/DiscoverPage.test.tsx index 1a76239..193fc41 100644 --- a/web/src/discover/DiscoverPage.test.tsx +++ b/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 () => { - const server = stubServer([BOWLS]) + 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 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(within(alert).getByRole('button', { name: 'Retry' })).toBeInTheDocument() - expect(sent(server.fetchMock)).toEqual([]) + expect(alert).toHaveTextContent("Couldn't save that choice.") + expect(sent(server.fetchMock)).toEqual(['POST /api/v1/suggestions/s1/accept']) + // 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 () => { diff --git a/web/src/discover/DiscoverPage.tsx b/web/src/discover/DiscoverPage.tsx index 114d566..491b967 100644 --- a/web/src/discover/DiscoverPage.tsx +++ b/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 }) => { - leaving.current = variables.id + (variables: { quest: SideQuest; choice: Choice }) => { + 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 diff --git a/web/src/discover/api.ts b/web/src/discover/api.ts index 52f67ab..3bb846c 100644 --- a/web/src/discover/api.ts +++ b/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 -// added to the Quest log. -type Decided = { quest: SideQuest; added: boolean } +// 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 } // useDecide records Try it (POST /api/v1/suggestions/{id}/accept) or Pass -// (POST /api/v1/suggestions/{id}/decline). 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. +// (POST /api/v1/suggestions/{id}/decline) on the Side quest the household +// decided on, whether or not a read of the deck has since taken it away. +// 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 -// out of the Quest log only the dinner it added. 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. +// 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. export function useDecide() { const { apiFetch } = useApi() const queryClient = useQueryClient() return useMutation({ - mutationFn: async ({ id, choice }: { id: string; choice: Choice }): Promise => { + mutationFn: async ({ quest: { id }, choice }: { quest: SideQuest; choice: Choice }): Promise => { const action = choice === 'try' ? 'accept' : 'decline' const path = `${SUGGESTIONS_PATH}/${encodeURIComponent(id)}/${action}` const body = await apiFetch(path, { method: 'POST' }) @@ -171,30 +176,30 @@ export function useDecide() { } return body.replenish_job_id }, - onMutate: async ({ id, choice }): Promise => { + onMutate: async ({ quest, choice }): Promise => { // 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(DECK_KEY)?.find((q) => q.id === id) - if (quest === undefined) throw new Error(`suggestion ${id} is not in the deck`) - queryClient.setQueryData(DECK_KEY, (deck) => deck?.filter((q) => q.id !== id)) + const deck = queryClient.getQueryData(DECK_KEY) + const taken = deck !== undefined && deck.some((q) => q.id === quest.id) + if (taken) queryClient.setQueryData(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(QUEST_LOG_KEY) - const added = choice === 'try' && dinners !== undefined && !dinners.some((d) => d.id === quest.dinner_id) - if (added) queryClient.setQueryData(QUEST_LOG_KEY, dinners.concat({ id: quest.dinner_id, name: quest.name })) - return { quest, added } + if (choice !== 'try' || dinners === undefined || dinners.some((d) => d.id === quest.dinner_id)) return { taken, adding: null } + const adding = Symbol(quest.dinner_id) + queryClient.setQueryData(QUEST_LOG_KEY, dinners.concat({ id: quest.dinner_id, name: quest.name, adding })) + return { taken, adding } }, - onError: (_error, _variables, context) => { - // onMutate refusing leaves nothing to undo. - if (context === undefined) return - const { quest, added } = context - queryClient.setQueryData(DECK_KEY, (deck) => deck && [quest, ...deck.filter((q) => q.id !== quest.id)]) - if (added) { - queryClient.setQueryData(QUEST_LOG_KEY, (dinners) => dinners?.filter((d) => d.id !== quest.dinner_id)) - } + onError: (_error, { quest }, context) => { + const { taken, adding } = context! + if (taken) queryClient.setQueryData(DECK_KEY, (deck) => deck && [quest, ...deck.filter((q) => q.id !== quest.id)]) + if (adding === null) return + // A read of the Quest log that landed meanwhile has replaced the entry. + queryClient.setQueryData(QUEST_LOG_KEY, (dinners) => dinners?.filter((d) => d.adding !== adding)) + 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.