From 74611da62bb7d181e6eaa9e11edd77135afa6039 Mon Sep 17 00:00:00 2001 From: dev_agent Date: Wed, 7 Oct 2026 02:31:55 -0400 Subject: [PATCH 1/9] Apply Discover decisions optimistically and refill the deck Try it and Pass are applied when the card's exit finishes: the Side quest leaves the deck and, on Try it, its dinner joins the Quest log count before the server answers. A refused decision puts the card back on top of the deck, reverts the count and shows the ErrorBanner with Retry. A saved decision reads the deck again once fewer than 3 cards are left, and follows the replenish job it started, showing skeleton cards rather than "All caught up" while that job runs. Co-Authored-By: Claude Opus 5.5 --- web/src/discover/DiscoverPage.test.tsx | 200 ++++++++++++++++++++++++- web/src/discover/DiscoverPage.tsx | 39 +++-- web/src/discover/api.ts | 63 +++++++- 3 files changed, 273 insertions(+), 29 deletions(-) diff --git a/web/src/discover/DiscoverPage.test.tsx b/web/src/discover/DiscoverPage.test.tsx index a98ecb0..3ee3d1b 100644 --- a/web/src/discover/DiscoverPage.test.tsx +++ b/web/src/discover/DiscoverPage.test.tsx @@ -41,16 +41,16 @@ const ORZO = suggestionBody('s3', 'Lemon garlic shrimp orzo', { difficulty: 'har type Responder = () => Response | Promise // stubServer stands in for the API: GET of the deck answers the current -// server.deck and the Quest log has questLog dinners; every other request -// is answered by responders, keyed "METHOD path". Any other request fails -// the test. +// server.deck and the Quest log the current server.questLog dinners; every +// other request is answered by responders, keyed "METHOD path". Any other +// request fails the test. function stubServer(deck: unknown[], responders: Record = {}, questLog = 6) { - const server = { deck, fetchMock: undefined as unknown as Mock } + const server = { deck, questLog, fetchMock: undefined as unknown as Mock } const all: Record = { 'GET /api/v1/me': () => jsonResponse(200, ME), [`GET ${DECK}`]: () => jsonResponse(200, { suggestions: server.deck }), [`GET ${QUEST_LOG}`]: () => - jsonResponse(200, { dinners: Array.from({ length: questLog }, (_, i) => ({ id: `q${i}`, name: `Quest ${i}` })) }), + jsonResponse(200, { dinners: Array.from({ length: server.questLog }, (_, i) => ({ id: `q${i}`, name: `Quest ${i}` })) }), ...responders, } server.fetchMock = vi.fn((input, init) => { @@ -70,6 +70,11 @@ function sent(fetchMock: Mock) { .map(([input, init]) => `${init?.method} ${String(input)}`) } +// deckReads counts the GET /api/v1/suggestions/deck requests. +function deckReads(fetchMock: Mock) { + return fetchMock.mock.calls.filter(([input, init]) => (init?.method ?? 'GET') === 'GET' && String(input) === DECK).length +} + // jobPolls counts the GET /api/v1/jobs/j1 requests. function jobPolls(fetchMock: Mock) { return fetchMock.mock.calls.filter(([input, init]) => (init?.method ?? 'GET') === 'GET' && String(input) === '/api/v1/jobs/j1').length @@ -505,6 +510,179 @@ describe('DiscoverPage', () => { expect(screen.getByRole('heading', { name: 'All caught up' })).toBeInTheDocument() }) + describe('optimistic decisions', () => { + const PAELLA = suggestionBody('s4', 'Sheet pan paella') + const SOUP = suggestionBody('s5', 'Tortellini soup') + const accepted = (quest: typeof BOWLS, replenish: string | null = null) => + jsonResponse(200, { suggestion: { ...quest, status: 'accepted' }, replenish_job_id: replenish }) + const declined = (quest: typeof BOWLS) => jsonResponse(200, { suggestion: { ...quest, status: 'declined' }, replenish_job_id: null }) + // topCard is the deck's top card, the one read out. + const topCard = () => screen.getAllByRole('article')[0] + + it('Try it takes the card off the deck and bumps the Quest log count once the exit finishes, before the answer', async () => { + const accept = deferred() + const server = stubServer([BOWLS, TIKKA, ORZO, PAELLA], { 'POST /api/v1/suggestions/s1/accept': () => accept.promise }) + const animations = stubAnimate({ autoFinish: false }) + await renderPage() + await screen.findByRole('article', { name: 'Korean beef bowls' }) + await screen.findByText('Quest log · 6') + + // Nothing is applied on the press: the card is still leaving. + fireEvent.click(screen.getByRole('button', { name: 'Try it' })) + expect(screen.getByRole('article', { name: 'Korean beef bowls' })).toBeInTheDocument() + expect(screen.getByText('Quest log · 6')).toBeInTheDocument() + expect(sent(server.fetchMock)).toEqual([]) + + // Once it is gone the decision is applied, and sent. + await finishRunning(animations) + await finishRunning(animations) + await screen.findByText('Quest log · 7') + expect(topCard()).toHaveAccessibleName('Crock pot chicken tikka masala') + expect(screen.queryByRole('article', { name: 'Korean beef bowls' })).not.toBeInTheDocument() + expect(sent(server.fetchMock)).toEqual(['POST /api/v1/suggestions/s1/accept']) + expect(screen.getByRole('button', { name: 'Try it' })).toBeDisabled() + + server.deck = [TIKKA, ORZO, PAELLA] + server.questLog = 7 + accept.resolve(accepted(BOWLS)) + await waitFor(() => expect(screen.getByRole('button', { name: 'Try it' })).toBeEnabled()) + expect(screen.getByText('Quest log · 7')).toBeInTheDocument() + expect(topCard()).toHaveAccessibleName('Crock pot chicken tikka masala') + // Three Side quests are left: the deck is not read again. + expect(deckReads(server.fetchMock)).toBe(1) + expect(screen.queryByRole('alert')).not.toBeInTheDocument() + }) + + it('Pass takes the card off the deck before the answer, leaving the Quest log count', async () => { + const decline = deferred() + const server = stubServer([BOWLS, TIKKA, ORZO, PAELLA], { 'POST /api/v1/suggestions/s1/decline': () => decline.promise }) + await renderPage() + await screen.findByRole('article', { name: 'Korean beef bowls' }) + await screen.findByText('Quest log · 6') + + fireEvent.click(screen.getByRole('button', { name: 'Pass' })) + await waitFor(() => expect(topCard()).toHaveAccessibleName('Crock pot chicken tikka masala')) + expect(screen.queryByRole('article', { name: 'Korean beef bowls' })).not.toBeInTheDocument() + expect(screen.getByText('Quest log · 6')).toBeInTheDocument() + expect(sent(server.fetchMock)).toEqual(['POST /api/v1/suggestions/s1/decline']) + + server.deck = [TIKKA, ORZO, PAELLA] + decline.resolve(declined(BOWLS)) + await waitFor(() => expect(screen.getByRole('button', { name: 'Pass' })).toBeEnabled()) + expect(screen.getByText('Quest log · 6')).toBeInTheDocument() + expect(deckReads(server.fetchMock)).toBe(1) + }) + + it('puts a refused Try it back on top of the deck, reverts the Quest log count and offers Retry', async () => { + const accept = deferred() + let refuse = true + const server = stubServer([BOWLS, TIKKA, ORZO, PAELLA], { + 'POST /api/v1/suggestions/s1/accept': () => (refuse ? accept.promise : accepted(BOWLS)), + }) + await renderPage() + await screen.findByRole('article', { name: 'Korean beef bowls' }) + await screen.findByText('Quest log · 6') + + fireEvent.click(screen.getByRole('button', { name: 'Try it' })) + await screen.findByText('Quest log · 7') + expect(topCard()).toHaveAccessibleName('Crock pot chicken tikka masala') + + accept.resolve(errorResponse(500, 'internal')) + const alert = await screen.findByRole('alert') + expect(alert).toHaveTextContent("Couldn't save that choice.") + expect(topCard()).toHaveAccessibleName('Korean beef bowls') + expect(screen.getAllByRole('article', { hidden: true }).map((card) => card.querySelector('h2')!.textContent)).toEqual([ + 'Korean beef bowls', + 'Crock pot chicken tikka masala', + 'Lemon garlic shrimp orzo', + ]) + expect(screen.getByText('Quest log · 6')).toBeInTheDocument() + expect(screen.getByRole('button', { name: 'Try it' })).toBeEnabled() + + // Retry sends the same decision again, applied again at once. + refuse = false + server.deck = [TIKKA, ORZO, PAELLA] + server.questLog = 7 + 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(topCard()).toHaveAccessibleName('Crock pot chicken tikka masala') + expect(screen.getByText('Quest log · 7')).toBeInTheDocument() + }) + + 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': () => { + server.deck = [TIKKA, ORZO, PAELLA, SOUP] + return declined(BOWLS) + }, + }) + await renderPage() + await screen.findByRole('article', { name: 'Korean beef bowls' }) + expect(deckReads(server.fetchMock)).toBe(1) + + fireEvent.click(screen.getByRole('button', { name: 'Pass' })) + await waitFor(() => expect(deckReads(server.fetchMock)).toBe(2)) + await waitFor(() => + 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('shows skeleton cards, not "All caught up", while the replenish job a decision started runs', async () => { + const polls = [ + { id: 'j1', kind: 'suggestion_batch', status: 'running', result: { batch_id: 'b1', stage: 'suggesting', suggestion_ids: [], dropped: 0 }, error: null }, + { id: 'j1', kind: 'suggestion_batch', status: 'succeeded', result: { batch_id: 'b1', stage: 'done', suggestion_ids: ['s2'], dropped: 0 }, error: null }, + ] + const server = stubServer([BOWLS], { + 'POST /api/v1/suggestions/s1/accept': () => { + server.deck = [] + return accepted(BOWLS, 'j1') + }, + 'GET /api/v1/jobs/j1': () => { + const job = polls.shift()! + if (job.status === 'succeeded') server.deck = [TIKKA] + return jsonResponse(200, job) + }, + }) + await renderPage() + await screen.findByRole('article', { name: 'Korean beef bowls' }) + + fireEvent.click(screen.getByRole('button', { name: 'Try it' })) + expect(await screen.findByText('Finding new side quests…')).toBeInTheDocument() + expect(screen.getAllByTestId('skeleton-card')).toHaveLength(3) + expect(screen.getByRole('region', { name: 'Side quests loading' })).toHaveAttribute('aria-busy', 'true') + expect(screen.queryByRole('heading', { name: 'All caught up' })).not.toBeInTheDocument() + + await screen.findByRole('article', { name: 'Crock pot chicken tikka masala' }, { timeout: 3000 }) + expect(screen.queryByTestId('skeleton-card')).not.toBeInTheDocument() + expect(screen.queryByRole('heading', { name: 'All caught up' })).not.toBeInTheDocument() + // The decision's replenish is followed; no batch is asked for. + expect(sent(server.fetchMock)).toEqual(['POST /api/v1/suggestions/s1/accept']) + }) + + it('shows skeleton cards, not "All caught up", while the decision that emptied the deck is unanswered', async () => { + const accept = deferred() + const server = stubServer([BOWLS], { 'POST /api/v1/suggestions/s1/accept': () => accept.promise }) + await renderPage() + await screen.findByRole('article', { name: 'Korean beef bowls' }) + + fireEvent.click(screen.getByRole('button', { name: 'Try it' })) + await waitFor(() => expect(screen.getAllByTestId('skeleton-card')).toHaveLength(3)) + expect(screen.queryByRole('heading', { name: 'All caught up' })).not.toBeInTheDocument() + expect(screen.queryByText('Finding new side quests…')).not.toBeInTheDocument() + + // Saved without a replenish, the deck is caught up. + server.deck = [] + accept.resolve(accepted(BOWLS)) + await screen.findByRole('heading', { name: 'All caught up' }) + expect(screen.queryByTestId('skeleton-card')).not.toBeInTheDocument() + }) + }) + describe('motion', () => { const accepted = (quest: typeof BOWLS) => jsonResponse(200, { suggestion: { ...quest, status: 'accepted' }, replenish_job_id: null }) const declined = (quest: typeof BOWLS) => jsonResponse(200, { suggestion: { ...quest, status: 'declined' }, replenish_job_id: null }) @@ -990,14 +1168,16 @@ describe('DiscoverPage', () => { 'POST /api/v1/suggestions/s1/accept': () => (++calls === 1 ? errorResponse(500, 'internal') : retry.promise), }) await renderPage() - const card = await screen.findByRole('article', { name: 'Korean beef bowls' }) + await screen.findByRole('article', { name: 'Korean beef bowls' }) fireEvent.click(screen.getByRole('button', { name: 'Try it' })) const alert = await screen.findByRole('alert') + // The refused Side quest is back on top of the deck. + const card = screen.getByRole('article', { name: 'Korean beef bowls' }) press(card) drag(card, 60, 0) fireEvent.click(within(alert).getByRole('button', { name: 'Retry' })) - await waitFor(() => expect(screen.getByRole('button', { name: 'Try it' })).toBeDisabled()) + expect(screen.getByRole('button', { name: 'Try it' })).toBeDisabled() drag(card, -160, 0) expect(card.style.transform).toBe('translate(60px,0px) rotate(3.3333333333333335deg)') const started = vi.mocked(Element.prototype.animate).mock.calls.length @@ -1005,7 +1185,11 @@ describe('DiscoverPage', () => { expect(card.style.transform).toBe('') expect(within(card).getByText('Try it!').style.opacity).toBe('') expect(vi.mocked(Element.prototype.animate).mock.calls.length).toBe(started) - expect(sent(server.fetchMock)).toEqual(['POST /api/v1/suggestions/s1/accept', 'POST /api/v1/suggestions/s1/accept']) + await waitFor(() => + expect(sent(server.fetchMock)).toEqual(['POST /api/v1/suggestions/s1/accept', 'POST /api/v1/suggestions/s1/accept']), + ) + // The retried Side quest leaves the deck while the Retry is out. + await screen.findByRole('article', { name: 'Crock pot chicken tikka masala' }) retry.resolve(accepted(BOWLS)) await waitFor(() => expect(screen.getByRole('button', { name: 'Try it' })).toBeEnabled()) }) diff --git a/web/src/discover/DiscoverPage.tsx b/web/src/discover/DiscoverPage.tsx index 12535ec..2fadb80 100644 --- a/web/src/discover/DiscoverPage.tsx +++ b/web/src/discover/DiscoverPage.tsx @@ -88,7 +88,8 @@ function QuestLogPill({ dinners, ref }: { dinners: { id: string; name: string }[ // DiscoverPage is mock 1a: the deck of Side quests to Try it or Pass, one // at a time from the top card. A decision from the buttons, the ← / → keys // or a swipe of the top card throws it out (motion.ts) and sends the -// decision once it is gone; Retry sends a refused one again. Input is +// decision once it is gone, taking the card off the deck at once (see +// useDecide); Retry sends a refused one again. Input is // locked from the press until the decision settles: the lock is a ref, set // before anything starts, so a second press before the next render does // nothing, and busy mirrors it to disable the buttons. @@ -98,22 +99,27 @@ export function DiscoverPage() { const questLog = useQuestLog() const decision = useDecide() const startBatch = useStartBatch() - const job = useJob(BATCH_JOB_KIND, startBatch.data?.job_id ?? null) + // The job of the batch last known to be finding Side quests: the one Get + // more now started or joined, or the replenish a decision started. + const [batchJobId, setBatchJobId] = useState(null) + const job = useJob(BATCH_JOB_KIND, batchJobId) const top = deck.data?.[0] const [busy, setBusy] = useState(false) - const jobRunning = startBatch.isSuccess && job.status !== 'succeeded' && job.status !== 'failed' + const jobRunning = batchJobId !== null && job.status !== 'succeeded' && job.status !== 'failed' const finding = startBatch.isPending || jobRunning + const { mutate: mutateStartBatch } = startBatch + const getMore = useCallback(() => mutateStartBatch(undefined, { onSuccess: (b) => setBatchJobId(b.job_id) }), [mutateStartBatch]) // The batch's result as far as it has gone, or why it cannot be read. const batch = useMemo(() => { if (job.result === null) return { stored: 0, error: null } try { - return { stored: parseBatchResult(`GET /api/v1/jobs/${startBatch.data?.job_id}`, job.result).suggestion_ids.length, error: null } + return { stored: parseBatchResult(`GET /api/v1/jobs/${batchJobId}`, job.result).suggestion_ids.length, error: null } } catch (error) { return { stored: 0, error: error as Error } } - }, [job.result, startBatch.data?.job_id]) + }, [job.result, batchJobId]) // The job's failure, the latest poll's failure, an answer that is not the // batch's job, or an unreadable result. const batchError = job.error ?? batch.error @@ -122,8 +128,8 @@ export function DiscoverPage() { const passRef = useRef(null) const tryRef = useRef(null) const questLogRef = useRef(null) - // The top card's exit while it runs or its decision is unsettled, and the - // Side quest the last decision sent was for, until the deck moves past it. + // The top card's exit until its decision settles, and the Side quest the + // last decision sent was for, until the deck moves past it. const exit = useRef(null) const leaving = useRef(null) const drag = useRef(null) @@ -143,17 +149,21 @@ export function DiscoverPage() { setBusy(true) return true }, []) - // send records a decision. A refused one puts the card back. + // 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 mutateDecision(variables, { + onSuccess: (replenishJobId) => { + if (replenishJobId !== null) setBatchJobId(replenishJobId) + }, onError: () => { exit.current?.cancel() - exit.current = null leaving.current = null }, onSettled: () => { + exit.current = null deciding.current = false setBusy(false) }, @@ -278,7 +288,6 @@ export function DiscoverPage() { useLayoutEffect(() => { if (leaving.current === null || leaving.current === topId) return leaving.current = null - exit.current = null if (topId === undefined || prefersReducedMotion()) return promote([...deckRef.current!.querySelectorAll(':scope > article')]) }, [topId]) @@ -307,7 +316,9 @@ export function DiscoverPage() { let body if (deck.isError) { body = void deck.refetch()}>Couldn't load your side quests. {deck.error.message} - } else if (deck.isPending || (deck.data.length === 0 && finding)) { + } else if (deck.isPending || (deck.data.length === 0 && (finding || decision.isPending))) { + // An empty deck is not caught up while a batch finds more Side quests, + // or while the last decision may yet start one or come back refused. body = (
@@ -325,12 +336,12 @@ export function DiscoverPage() { body = ( <> {startBatch.isError && ( - startBatch.mutate()}>Couldn't ask for more side quests. {startBatch.error.message} + Couldn't ask for more side quests. {startBatch.error.message} )} {batchError !== null && ( - startBatch.mutate()}>Couldn't find new side quests. {batchError.message} + Couldn't find new side quests. {batchError.message} )} - startBatch.mutate() }}> + New side quests arrive Sunday. diff --git a/web/src/discover/api.ts b/web/src/discover/api.ts index 9767eb7..ede8008 100644 --- a/web/src/discover/api.ts +++ b/web/src/discover/api.ts @@ -12,10 +12,12 @@ export const QUEST_LOG_PATH = `${DINNERS_PATH}?status=quest_log` export const DECK_KEY = ['suggestions', 'deck'] as const export const QUEST_LOG_KEY = ['dinners', 'quest_log'] as const -// SideQuest is a pending suggestion as the deck shows it. photo_id is the -// stored photo's id, from photo_url; reason is the "Why this?". +// SideQuest is a pending suggestion as the deck shows it. dinner_id is the +// dinner Try it moves to the Quest log; photo_id is the stored photo's id, +// from photo_url; reason is the "Why this?". export type SideQuest = { id: string + dinner_id: string name: string photo_id: string | undefined difficulty: Difficulty | null @@ -38,6 +40,10 @@ export type BatchResult = { batch_id: string; stage: string; suggestion_ids: str // QuestLogDinner is a dinner in the Quest log, as the Quest log pill lists it. export type QuestLogDinner = { id: string; name: string } +// REFILL_BELOW is the deck size, the depths the deck shows, under which a +// saved decision reads the deck again. +export const REFILL_BELOW = 3 + // DECISION_STATUS is the status a suggestion has once each choice is saved. const DECISION_STATUS: Record = { try: 'accepted', pass: 'declined' } @@ -61,6 +67,7 @@ function parseSideQuest(where: string, body: unknown): SideQuest { if ( !isRecord(body) || typeof body.id !== 'string' || + typeof body.dinner_id !== 'string' || typeof body.name !== 'string' || typeof body.reason !== 'string' ) { @@ -68,6 +75,7 @@ function parseSideQuest(where: string, body: unknown): SideQuest { } return { id: body.id, + dinner_id: body.dinner_id, name: body.name, photo_id: photoId(where, body.photo_url), difficulty: body.difficulty === null ? null : oneOf(where, 'difficulty', DIFFICULTIES, body.difficulty), @@ -126,14 +134,25 @@ export function useStartBatch() { }) } +// Decided is the context a decision's optimistic update leaves for its +// rollback: the Side quest taken off the deck. +type Decided = { quest: SideQuest } + // useDecide records Try it (POST /api/v1/suggestions/{id}/accept) or Pass -// (POST /api/v1/suggestions/{id}/decline), then reads the deck and the -// Quest log again. The answer must be that suggestion, decided that way. +// (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. +// +// 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 +// quest back on top of the deck and takes its dinner out of the Quest log +// again. A saved one reads the Quest log again, and the deck too once fewer +// than REFILL_BELOW Side quests are left in it. export function useDecide() { const { apiFetch } = useApi() const queryClient = useQueryClient() return useMutation({ - mutationFn: async ({ id, choice }: { id: string; choice: Choice }) => { + mutationFn: async ({ id, choice }: { id: string; choice: Choice }): Promise => { const action = choice === 'try' ? 'accept' : 'decline' const path = `${SUGGESTIONS_PATH}/${encodeURIComponent(id)}/${action}` const body = await apiFetch(path, { method: 'POST' }) @@ -145,10 +164,40 @@ export function useDecide() { ) { throw new Error(`POST ${path}: response is not the ${DECISION_STATUS[choice]} suggestion`) } + if (body.replenish_job_id !== null && typeof body.replenish_job_id !== 'string') { + throw new Error(`POST ${path}: response has no replenish_job_id`) + } + return body.replenish_job_id }, - onSuccess: async () => { + onMutate: async ({ id, 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)) + if (choice === 'try') { + queryClient.setQueryData(QUEST_LOG_KEY, (dinners) => + dinners?.concat({ id: quest.dinner_id, name: quest.name }), + ) + } + return { quest } + }, + onError: (_error, { choice }, context) => { + // onMutate refusing leaves nothing to undo. + if (context === undefined) return + const { quest } = context + queryClient.setQueryData(DECK_KEY, (deck) => deck && [quest, ...deck.filter((q) => q.id !== quest.id)]) + if (choice === 'try') { + queryClient.setQueryData(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. + const deck = queryClient.setQueryData(DECK_KEY, (current) => current?.filter((q) => q.id !== id)) await Promise.all([ - queryClient.invalidateQueries({ queryKey: DECK_KEY }), + deck !== undefined && deck.length < REFILL_BELOW ? queryClient.invalidateQueries({ queryKey: DECK_KEY }) : undefined, queryClient.invalidateQueries({ queryKey: QUEST_LOG_KEY }), ]) }, -- 2.55.0 From 6c105416a9357ebacc23e0dce8af76ed8d7f9e6b Mon Sep 17 00:00:00 2001 From: dev_agent Date: Wed, 7 Oct 2026 02:43:06 -0400 Subject: [PATCH 2/9] 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 --- web/src/discover/DiscoverPage.test.tsx | 93 +++++++++++++++++++++++++- web/src/discover/DiscoverPage.tsx | 9 ++- web/src/discover/api.ts | 38 ++++++----- 3 files changed, 115 insertions(+), 25 deletions(-) diff --git a/web/src/discover/DiscoverPage.test.tsx b/web/src/discover/DiscoverPage.test.tsx index 3ee3d1b..1a76239 100644 --- a/web/src/discover/DiscoverPage.test.tsx +++ b/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. -async function renderPage() { +// the Discover page with queryClient. +async function renderPage(queryClient = createQueryClient()) { const api = createApiClient(fakeLocation('/discover'), createNetworkStatus(window), createVerifiedSession(localStorage)) await api.loadMe() return render( - + , @@ -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 | 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': () => { diff --git a/web/src/discover/DiscoverPage.tsx b/web/src/discover/DiscoverPage.tsx index 2fadb80..114d566 100644 --- a/web/src/discover/DiscoverPage.tsx +++ b/web/src/discover/DiscoverPage.tsx @@ -362,11 +362,6 @@ export function DiscoverPage() {
- {decision.isError && ( - submit(decision.variables!)}> - Couldn't save that choice. {decision.error.message} - - )} ) } @@ -384,6 +379,10 @@ export function DiscoverPage() { void questLog.refetch()}>Couldn't load the Quest log. {questLog.error.message} )} {body} + {/* A refused decision is shown whatever the deck has come to since. */} + {decision.isError && ( + submit(decision.variables!)}>Couldn't save that choice. {decision.error.message} + )} ) } diff --git a/web/src/discover/api.ts b/web/src/discover/api.ts index ede8008..52f67ab 100644 --- a/web/src/discover/api.ts +++ b/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. -type Decided = { quest: SideQuest } +// rollback: the Side quest taken off the deck, and whether its dinner was +// 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 -// quest back on top of the deck and takes its dinner out of the Quest log -// again. A saved one reads the Quest log again, and the deck too once fewer -// than REFILL_BELOW Side quests are left in it. +// 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. export function useDecide() { const { apiFetch } = useApi() const queryClient = useQueryClient() @@ -176,28 +178,30 @@ export function useDecide() { 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)) - if (choice === 'try') { - queryClient.setQueryData(QUEST_LOG_KEY, (dinners) => - dinners?.concat({ id: quest.dinner_id, name: quest.name }), - ) - } - return { quest } + // 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 } }, - 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(DECK_KEY, (deck) => deck && [quest, ...deck.filter((q) => q.id !== quest.id)]) - if (choice === 'try') { + if (added) { queryClient.setQueryData(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(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 }), ]) }, -- 2.55.0 From 0ea1c218eafab372f1abd9c46c0f1614ccbddabb Mon Sep 17 00:00:00 2001 From: dev_agent Date: Wed, 7 Oct 2026 02:53:09 -0400 Subject: [PATCH 3/9] 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. -- 2.55.0 From 1c59e5d9d78530a41797b67a4416313881673bfb Mon Sep 17 00:00:00 2001 From: dev_agent Date: Wed, 7 Oct 2026 03:02:04 -0400 Subject: [PATCH 4/9] 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 --- web/src/discover/DiscoverPage.test.tsx | 35 +++++++++++++++++++ web/src/discover/api.ts | 47 ++++++++++++++++++-------- 2 files changed, 67 insertions(+), 15 deletions(-) diff --git a/web/src/discover/DiscoverPage.test.tsx b/web/src/discover/DiscoverPage.test.tsx index 193fc41..8e2e31e 100644 --- a/web/src/discover/DiscoverPage.test.tsx +++ b/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((_, reject) => (lose = reject)) + let late: ReturnType | 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': () => { diff --git a/web/src/discover/api.ts b/web/src/discover/api.ts index 3bb846c..c169e7d 100644 --- a/web/src/discover/api.ts +++ b/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(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)) + 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(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(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(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(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(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(), }) } -- 2.55.0 From 3319434bd1b2f673a8abbe939cc4d0758acacc3c Mon Sep 17 00:00:00 2001 From: dev_agent Date: Wed, 7 Oct 2026 03:10:39 -0400 Subject: [PATCH 5/9] Reconcile the deck after a decision failure that may not leave the card pending Only an API error that refuses the request itself (a 4xx other than 404 or 409) puts a failed Side quest back on top of the deck. A 409 (decided by another session or no longer suggested), a 404, a 5xx or a failure without an answer now reads the deck again instead, keeping the card off it until that read answers. Co-Authored-By: Claude Opus 5.5 --- web/src/discover/DiscoverPage.test.tsx | 54 +++++++++++++++++++++++++- web/src/discover/api.ts | 31 +++++++++------ 2 files changed, 72 insertions(+), 13 deletions(-) diff --git a/web/src/discover/DiscoverPage.test.tsx b/web/src/discover/DiscoverPage.test.tsx index 8e2e31e..67cf6dc 100644 --- a/web/src/discover/DiscoverPage.test.tsx +++ b/web/src/discover/DiscoverPage.test.tsx @@ -588,10 +588,12 @@ describe('DiscoverPage', () => { await screen.findByText('Quest log · 7') expect(topCard()).toHaveAccessibleName('Crock pot chicken tikka masala') - accept.resolve(errorResponse(500, 'internal')) + // The server refuses the request itself: the Side quest is still pending. + accept.resolve(errorResponse(400, 'invalid_request')) const alert = await screen.findByRole('alert') expect(alert).toHaveTextContent("Couldn't save that choice.") expect(topCard()).toHaveAccessibleName('Korean beef bowls') + expect(deckReads(server.fetchMock)).toBe(1) expect(screen.getAllByRole('article', { hidden: true }).map((card) => card.querySelector('h2')!.textContent)).toEqual([ 'Korean beef bowls', 'Crock pot chicken tikka masala', @@ -771,6 +773,56 @@ describe('DiscoverPage', () => { expect(topCard()).toHaveAccessibleName('Crock pot chicken tikka masala') }) + it('keeps a card off the deck when another session already decided it (409), and reads the deck again', async () => { + const accept = deferred() + let read: ReturnType | null = null + const server = stubServer([BOWLS, TIKKA, ORZO, PAELLA], { + 'POST /api/v1/suggestions/s1/accept': () => accept.promise, + [`GET ${DECK}`]: () => read?.promise ?? jsonResponse(200, { suggestions: server.deck }), + }) + await renderPage() + 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'])) + + // Another session passed on it first; no read of the deck lands + // during the decision. + read = deferred() + const pending = read + server.deck = [TIKKA, ORZO, PAELLA] + await act(async () => accept.resolve(errorResponse(409, 'suggestion_decided'))) + await screen.findByRole('alert') + expect(screen.queryByRole('article', { name: 'Korean beef bowls' })).not.toBeInTheDocument() + await waitFor(() => expect(deckReads(server.fetchMock)).toBe(2)) + expect(screen.queryByRole('article', { name: 'Korean beef bowls' })).not.toBeInTheDocument() + + await act(async () => pending.resolve(jsonResponse(200, { suggestions: server.deck }))) + expect(screen.queryByRole('article', { name: 'Korean beef bowls' })).not.toBeInTheDocument() + expect(topCard()).toHaveAccessibleName('Crock pot chicken tikka masala') + }) + + it('reads the deck again after a 500 to a decision, rather than putting the card back', async () => { + const decline = deferred() + const server = stubServer([BOWLS, TIKKA, ORZO, PAELLA], { + 'POST /api/v1/suggestions/s1/decline': () => decline.promise, + }) + await renderPage() + 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'])) + + // The decision was saved, and the answer to it is a 500 all the same. + server.deck = [TIKKA, ORZO, PAELLA] + await act(async () => decline.resolve(errorResponse(500, 'internal'))) + await screen.findByRole('alert') + expect(screen.queryByRole('article', { name: 'Korean beef bowls' })).not.toBeInTheDocument() + await waitFor(() => expect(deckReads(server.fetchMock)).toBe(2)) + 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': () => { diff --git a/web/src/discover/api.ts b/web/src/discover/api.ts index c169e7d..d7ee63f 100644 --- a/web/src/discover/api.ts +++ b/web/src/discover/api.ts @@ -146,6 +146,14 @@ type Decided = { taken: boolean; adding: symbol | null; reads: DeckReads } // flight: gone is whether the latest left the Side quest out. type DeckReads = { gone: boolean; stop: () => void } +// stillPending is whether a failed decision's error shows its Side quest +// still pending: an API error answer refusing the request itself. A 404 or +// 409 says it is gone or decided already, a 5xx answer may have come after +// the decision was saved, and a failure without an answer may have too. +function stillPending(error: Error): boolean { + return error instanceof ApiError && error.status >= 400 && error.status < 500 && error.status !== 404 && error.status !== 409 +} + // useDecide records Try it (POST /api/v1/suggestions/{id}/accept) or Pass // (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. @@ -154,14 +162,15 @@ type DeckReads = { gone: boolean; stop: () => void } // // 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 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. +// failed decision 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. One the server refused with the Side quest still pending (see +// stillPending) puts it back on top of the deck if it took it off and no +// read of the deck since has left it out; any other failure reads the deck +// again instead, leaving the Side quest off it until that read answers. 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() @@ -206,10 +215,8 @@ export function useDecide() { }, onError: (error, { quest }, context) => { const { taken, adding, reads } = context! - if (taken && !reads.gone) queryClient.setQueryData(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 (!stillPending(error)) void queryClient.invalidateQueries({ queryKey: DECK_KEY }) + else if (taken && !reads.gone) 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)) -- 2.55.0 From f26cfd512924083ffac94f263cae54bd7846758a Mon Sep 17 00:00:00 2001 From: dev_agent Date: Wed, 7 Oct 2026 03:21:54 -0400 Subject: [PATCH 6/9] Reconcile every Discover decision from the server before the next Replace the hand-maintained optimistic bookkeeping in useDecide with one rule: a decision takes its card off the deck (and, on Try it, adds its dinner to the Quest log) when the exit finishes; however it settles, the deck and the Quest log are read again and the decision stays pending until those reads land, so the input lock keeps the next decision out. No decision cancels a read. A failed decision whose Side quest the reconciled deck still has goes back on top with a Retry; otherwise the banner says it was decided already, with no Retry. ErrorBanner's Retry is now optional. Co-Authored-By: Claude Opus 5.5 --- web/src/discover/DiscoverPage.test.tsx | 109 ++++++++++++++++++++----- web/src/discover/DiscoverPage.tsx | 33 +++++--- web/src/discover/api.ts | 102 ++++++----------------- web/src/ui/ErrorBanner.test.tsx | 6 ++ web/src/ui/ErrorBanner.tsx | 8 +- 5 files changed, 147 insertions(+), 111 deletions(-) diff --git a/web/src/discover/DiscoverPage.test.tsx b/web/src/discover/DiscoverPage.test.tsx index 67cf6dc..180846f 100644 --- a/web/src/discover/DiscoverPage.test.tsx +++ b/web/src/discover/DiscoverPage.test.tsx @@ -549,8 +549,8 @@ describe('DiscoverPage', () => { await waitFor(() => expect(screen.getByRole('button', { name: 'Try it' })).toBeEnabled()) expect(screen.getByText('Quest log · 7')).toBeInTheDocument() expect(topCard()).toHaveAccessibleName('Crock pot chicken tikka masala') - // Three Side quests are left: the deck is not read again. - expect(deckReads(server.fetchMock)).toBe(1) + // The deck is read again once the decision is saved. + expect(deckReads(server.fetchMock)).toBe(2) expect(screen.queryByRole('alert')).not.toBeInTheDocument() }) @@ -571,7 +571,7 @@ describe('DiscoverPage', () => { decline.resolve(declined(BOWLS)) await waitFor(() => expect(screen.getByRole('button', { name: 'Pass' })).toBeEnabled()) expect(screen.getByText('Quest log · 6')).toBeInTheDocument() - expect(deckReads(server.fetchMock)).toBe(1) + expect(deckReads(server.fetchMock)).toBe(2) }) it('puts a refused Try it back on top of the deck, reverts the Quest log count and offers Retry', async () => { @@ -588,12 +588,14 @@ describe('DiscoverPage', () => { await screen.findByText('Quest log · 7') expect(topCard()).toHaveAccessibleName('Crock pot chicken tikka masala') - // The server refuses the request itself: the Side quest is still pending. + // The server refuses the request itself: the deck read again after it + // still has the Side quest pending, in the server's order. + server.deck = [TIKKA, BOWLS, ORZO, PAELLA] accept.resolve(errorResponse(400, 'invalid_request')) const alert = await screen.findByRole('alert') expect(alert).toHaveTextContent("Couldn't save that choice.") expect(topCard()).toHaveAccessibleName('Korean beef bowls') - expect(deckReads(server.fetchMock)).toBe(1) + expect(deckReads(server.fetchMock)).toBe(2) expect(screen.getAllByRole('article', { hidden: true }).map((card) => card.querySelector('h2')!.textContent)).toEqual([ 'Korean beef bowls', 'Crock pot chicken tikka masala', @@ -707,10 +709,9 @@ describe('DiscoverPage', () => { 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 + it('sends a decision for the card thrown out even when a read of the deck took it away during the exit, offering no Retry for it', async () => { const server = stubServer([BOWLS], { - 'POST /api/v1/suggestions/s1/accept': () => (refuse ? errorResponse(500, 'internal') : accepted(BOWLS)), + 'POST /api/v1/suggestions/s1/accept': () => errorResponse(500, 'internal'), }) const queryClient = createQueryClient() const animations = stubAnimate({ autoFinish: false }) @@ -726,15 +727,12 @@ describe('DiscoverPage', () => { await finishRunning(animations) await finishRunning(animations) const alert = await screen.findByRole('alert') - 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. + // The deck read after it confirms the Side quest is decided: the card + // is not put back, and there is nothing to retry. + expect(alert).toHaveTextContent('That side quest has been decided already.') + expect(within(alert).queryByRole('button')).not.toBeInTheDocument() 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() }) @@ -792,14 +790,78 @@ describe('DiscoverPage', () => { const pending = read server.deck = [TIKKA, ORZO, PAELLA] await act(async () => accept.resolve(errorResponse(409, 'suggestion_decided'))) - await screen.findByRole('alert') - expect(screen.queryByRole('article', { name: 'Korean beef bowls' })).not.toBeInTheDocument() await waitFor(() => expect(deckReads(server.fetchMock)).toBe(2)) expect(screen.queryByRole('article', { name: 'Korean beef bowls' })).not.toBeInTheDocument() + // The decision is not over until that read lands. + expect(screen.queryByRole('alert')).not.toBeInTheDocument() + expect(screen.getByRole('button', { name: 'Try it' })).toBeDisabled() await act(async () => pending.resolve(jsonResponse(200, { suggestions: server.deck }))) + const alert = await screen.findByRole('alert') expect(screen.queryByRole('article', { name: 'Korean beef bowls' })).not.toBeInTheDocument() expect(topCard()).toHaveAccessibleName('Crock pot chicken tikka masala') + // It was decided: there is no decision to retry. + expect(alert).toHaveTextContent('That side quest has been decided already.') + expect(within(alert).queryByRole('button')).not.toBeInTheDocument() + expect(screen.getByRole('button', { name: 'Try it' })).toBeEnabled() + }) + + it('reads the Quest log again after a Pass another session beat with Try it (409), counting its dinner', async () => { + const server = stubServer([BOWLS, TIKKA, ORZO, PAELLA], { + 'POST /api/v1/suggestions/s1/decline': () => { + // Another session tried it first: its dinner is in the Quest log. + server.deck = [TIKKA, ORZO, PAELLA] + server.questLog = 7 + return errorResponse(409, 'suggestion_decided') + }, + }) + await renderPage() + await screen.findByRole('article', { name: 'Korean beef bowls' }) + await screen.findByText('Quest log · 6') + + fireEvent.click(screen.getByRole('button', { name: 'Pass' })) + const alert = await screen.findByRole('alert') + expect(alert).toHaveTextContent('That side quest has been decided already.') + expect(screen.getByText('Quest log · 7')).toBeInTheDocument() + expect(topCard()).toHaveAccessibleName('Crock pot chicken tikka masala') + }) + + it('locks the next decision until the reads after a failed one land, then puts the still-pending card back on top with a Retry', async () => { + let read: ReturnType | null = null + const server = stubServer([BOWLS, TIKKA, ORZO, PAELLA], { + 'POST /api/v1/suggestions/s1/accept': () => { + read = deferred() + return errorResponse(500, 'internal') + }, + [`GET ${DECK}`]: () => read?.promise ?? jsonResponse(200, { suggestions: server.deck }), + }) + await renderPage() + await screen.findByRole('article', { name: 'Korean beef bowls' }) + + // The decision failed and was not saved; the deck read after it is slow. + fireEvent.click(screen.getByRole('button', { name: 'Try it' })) + await waitFor(() => expect(deckReads(server.fetchMock)).toBe(2)) + const slow = read! + read = null + expect(topCard()).toHaveAccessibleName('Crock pot chicken tikka masala') + + // Deciding the next card does nothing while that read is out. + expect(screen.getByRole('button', { name: 'Pass' })).toBeDisabled() + fireEvent.click(screen.getByRole('button', { name: 'Pass' })) + fireEvent.keyDown(window, { key: 'ArrowLeft' }) + await act(async () => await new Promise((resolve) => setTimeout(resolve, 0))) + expect(sent(server.fetchMock)).toEqual(['POST /api/v1/suggestions/s1/accept']) + expect(screen.queryByRole('alert')).not.toBeInTheDocument() + + await act(async () => slow.resolve(jsonResponse(200, { suggestions: server.deck }))) + const alert = await screen.findByRole('alert') + expect(alert).toHaveTextContent("Couldn't save that choice.") + expect(topCard()).toHaveAccessibleName('Korean beef bowls') + expect(screen.getByRole('button', { name: 'Try it' })).toBeEnabled() + + // Retry sends it again. + fireEvent.click(within(alert).getByRole('button', { name: 'Retry' })) + await waitFor(() => expect(sent(server.fetchMock)).toEqual(['POST /api/v1/suggestions/s1/accept', 'POST /api/v1/suggestions/s1/accept'])) }) it('reads the deck again after a 500 to a decision, rather than putting the card back', async () => { @@ -1380,12 +1442,16 @@ describe('DiscoverPage', () => { const server = stubServer([BOWLS, TIKKA], { 'POST /api/v1/suggestions/s1/accept': () => (++calls === 1 ? errorResponse(500, 'internal') : retry.promise), }) - await renderPage() + const queryClient = createQueryClient() + await renderPage(queryClient) await screen.findByRole('article', { name: 'Korean beef bowls' }) fireEvent.click(screen.getByRole('button', { name: 'Try it' })) const alert = await screen.findByRole('alert') - // The refused Side quest is back on top of the deck. - const card = screen.getByRole('article', { name: 'Korean beef bowls' }) + // A read of the deck puts the refused Side quest back in the server's + // order, still pending, behind the card dragged during its Retry. + server.deck = [TIKKA, BOWLS] + await act(async () => await queryClient.invalidateQueries({ queryKey: DECK_KEY })) + const card = await screen.findByRole('article', { name: 'Crock pot chicken tikka masala' }) press(card) drag(card, 60, 0) @@ -1402,7 +1468,8 @@ describe('DiscoverPage', () => { expect(sent(server.fetchMock)).toEqual(['POST /api/v1/suggestions/s1/accept', 'POST /api/v1/suggestions/s1/accept']), ) // The retried Side quest leaves the deck while the Retry is out. - await screen.findByRole('article', { name: 'Crock pot chicken tikka masala' }) + await waitFor(() => expect(screen.queryByRole('article', { name: 'Korean beef bowls' })).not.toBeInTheDocument()) + server.deck = [TIKKA] retry.resolve(accepted(BOWLS)) await waitFor(() => expect(screen.getByRole('button', { name: 'Try it' })).toBeEnabled()) }) diff --git a/web/src/discover/DiscoverPage.tsx b/web/src/discover/DiscoverPage.tsx index 491b967..46c8bb9 100644 --- a/web/src/discover/DiscoverPage.tsx +++ b/web/src/discover/DiscoverPage.tsx @@ -90,10 +90,12 @@ function QuestLogPill({ dinners, ref }: { dinners: { id: string; name: string }[ // at a time from the top card. A decision from the buttons, the ← / → keys // or a swipe of the top card throws it out (motion.ts) and sends the // decision once it is gone, taking the card off the deck at once (see -// useDecide); Retry sends a refused one again. Input is -// locked from the press until the decision settles: the lock is a ref, set -// before anything starts, so a second press before the next render does -// nothing, and busy mirrors it to disable the buttons. +// useDecide); Retry sends a failed one again while the deck read since still +// has its Side quest pending. Input is locked from the press until the +// decision settles, which is once the deck and the Quest log have been read +// again after it, so no decision starts under those reads. The lock is a +// ref, set before anything starts, so a second press before the next render +// does nothing, and busy mirrors it to disable the buttons. export function DiscoverPage() { const queryClient = useQueryClient() const deck = useDeck() @@ -150,8 +152,9 @@ export function DiscoverPage() { setBusy(true) return true }, []) - // send records a decision. A refused one puts the card back; a saved one - // that started a replenish batch follows its job. + // send records a decision. A failed one stops the card's exit, so a card + // useDecide put back on the deck is at rest; a saved one that started a + // replenish batch follows its job. const send = useCallback( (variables: { quest: SideQuest; choice: Choice }) => { leaving.current = variables.quest.id @@ -314,6 +317,11 @@ export function DiscoverPage() { void queryClient.invalidateQueries({ queryKey: DECK_KEY }) }, [queryClient, job.status, stored]) + // A failed decision can be retried only while the deck, read again after + // it, still has its Side quest pending: anything else decided it already. + const failed = decision.isError ? decision.variables.quest.id : null + const retryable = failed !== null && !deck.isError && deck.data !== undefined && deck.data.some((q) => q.id === failed) + let body if (deck.isError) { body = void deck.refetch()}>Couldn't load your side quests. {deck.error.message} @@ -380,10 +388,15 @@ export function DiscoverPage() { void questLog.refetch()}>Couldn't load the Quest log. {questLog.error.message} )} {body} - {/* A refused decision is shown whatever the deck has come to since. */} - {decision.isError && ( - submit(decision.variables!)}>Couldn't save that choice. {decision.error.message} - )} + {/* A failed decision is shown whatever the deck has come to since. */} + {decision.isError && + (retryable ? ( + submit(decision.variables)}>Couldn't save that choice. {decision.error.message} + ) : deck.isError ? ( + Couldn't save that choice. {decision.error.message} + ) : ( + That side quest has been decided already. {decision.error.message} + ))} ) } diff --git a/web/src/discover/api.ts b/web/src/discover/api.ts index d7ee63f..47b78eb 100644 --- a/web/src/discover/api.ts +++ b/web/src/discover/api.ts @@ -1,5 +1,5 @@ -import { hashKey, useMutation, useQuery, useQueryClient } from '@tanstack/react-query' -import { ApiError, useApi } from '../api/index.ts' +import { useMutation, useQuery, useQueryClient } from '@tanstack/react-query' +import { useApi } from '../api/index.ts' import { isRecord } from '../api/errors.ts' import { DIFFICULTIES, DINNERS_PATH, oneOf, photoId, type Difficulty } from '../dinners/api.ts' @@ -38,13 +38,7 @@ 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. -// 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. -export const REFILL_BELOW = 3 +export type QuestLogDinner = { id: string; name: string } // DECISION_STATUS is the status a suggestion has once each choice is saved. const DECISION_STATUS: Record = { try: 'accepted', pass: 'declined' } @@ -136,24 +130,6 @@ 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, 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 } - -// stillPending is whether a failed decision's error shows its Side quest -// still pending: an API error answer refusing the request itself. A 404 or -// 409 says it is gone or decided already, a 5xx answer may have come after -// the decision was saved, and a failure without an answer may have too. -function stillPending(error: Error): boolean { - return error instanceof ApiError && error.status >= 400 && error.status < 500 && error.status !== 404 && error.status !== 409 -} - // useDecide records Try it (POST /api/v1/suggestions/{id}/accept) or Pass // (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. @@ -161,16 +137,14 @@ function stillPending(error: Error): boolean { // 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 -// failed decision 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. One the server refused with the Side quest still pending (see -// stillPending) puts it back on top of the deck if it took it off and no -// read of the deck since has left it out; any other failure reads the deck -// again instead, leaving the Side quest off it until that read answers. 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. +// Try it, its dinner joins the Quest log unless it is there already. +// Nothing else is worked out here: the server is the record of both. However +// the decision ends, saved or failed, the deck and the Quest log are read +// again, and the decision is pending until those reads land, so the next one +// (which DiscoverPage locks out until then) cannot start under them. No +// decision cancels a read; one under way when the decision ends is replaced +// by the read made after it. A failed decision whose Side quest that read +// still has is still pending, and goes back on top of the deck. export function useDecide() { const { apiFetch } = useApi() const queryClient = useQueryClient() @@ -192,47 +166,21 @@ export function useDecide() { } return body.replenish_job_id }, - 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 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)) - 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) + onMutate: ({ quest, choice }) => { + queryClient.setQueryData(DECK_KEY, (deck) => deck?.filter((q) => q.id !== quest.id)) + if (choice !== 'try') return + // A stale deck can offer a Side quest whose dinner the Quest log has. + queryClient.setQueryData(QUEST_LOG_KEY, (dinners) => + dinners === undefined || dinners.some((d) => d.id === quest.dinner_id) ? dinners : dinners.concat({ id: quest.dinner_id, name: quest.name }), + ) + }, + onSettled: async (_replenishJobId, error, { quest: { id } }) => { + await Promise.all([queryClient.invalidateQueries({ queryKey: DECK_KEY }), queryClient.invalidateQueries({ queryKey: QUEST_LOG_KEY })]) + if (error === null) return + queryClient.setQueryData(DECK_KEY, (deck) => { + const pending = deck?.find((q) => q.id === id) + return pending === undefined ? deck : [pending, ...deck!.filter((q) => q !== pending)] }) - // 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) - 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(QUEST_LOG_KEY, dinners.concat({ id: quest.dinner_id, name: quest.name, adding })) - return { taken, adding, reads } }, - onError: (error, { quest }, context) => { - const { taken, adding, reads } = context! - if (!stillPending(error)) void queryClient.invalidateQueries({ queryKey: DECK_KEY }) - else if (taken && !reads.gone) 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, { 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. - const reading = queryClient.isFetching({ queryKey: DECK_KEY }) > 0 - const deck = queryClient.setQueryData(DECK_KEY, (current) => current?.filter((q) => q.id !== id)) - await Promise.all([ - reading || (deck !== undefined && deck.length < REFILL_BELOW) ? queryClient.invalidateQueries({ queryKey: DECK_KEY }) : undefined, - queryClient.invalidateQueries({ queryKey: QUEST_LOG_KEY }), - ]) - }, - onSettled: (_data, _error, _variables, context) => context?.reads.stop(), }) } diff --git a/web/src/ui/ErrorBanner.test.tsx b/web/src/ui/ErrorBanner.test.tsx index 811b5e5..71a91f0 100644 --- a/web/src/ui/ErrorBanner.test.tsx +++ b/web/src/ui/ErrorBanner.test.tsx @@ -24,6 +24,12 @@ describe('ErrorBanner', () => { expect(screen.getByRole('button', { name: 'Rebuild week' })).toBeInTheDocument() }) + it('has no button without onRetry', () => { + render(That was done already.) + expect(screen.getByRole('alert')).toHaveTextContent('That was done already.') + expect(screen.queryByRole('button')).not.toBeInTheDocument() + }) + it('is a neutral-100 card with grape-700 text', () => { render( {}}>Offline) expect(declaredStyle(screen.getByRole('alert')).background).toBe('var(--color-neutral-100)') diff --git a/web/src/ui/ErrorBanner.tsx b/web/src/ui/ErrorBanner.tsx index 1274bf9..5537f43 100644 --- a/web/src/ui/ErrorBanner.tsx +++ b/web/src/ui/ErrorBanner.tsx @@ -6,16 +6,18 @@ import styles from './ErrorBanner.module.css' export type ErrorBannerProps = { // What went wrong, in the caller's words. children: ReactNode - onRetry: () => void + // What Retry does; without it there is nothing to retry and no button. + onRetry?: () => void retryLabel?: string } -// Muted neutral-100 card with grape-700 text and a Retry button. +// Muted neutral-100 card with grape-700 text and, given onRetry, a Retry +// button. export function ErrorBanner({ children, onRetry, retryLabel = 'Retry' }: ErrorBannerProps) { return (

{children}

- + {onRetry !== undefined && } ) } -- 2.55.0 From 8b1839971041b3426283c210e9035fab28b1012f Mon Sep 17 00:00:00 2001 From: dev_agent Date: Wed, 7 Oct 2026 03:36:19 -0400 Subject: [PATCH 7/9] Settle a failed Discover decision on the deck read it makes itself A failed decision now reads the deck as its own fetch and waits for it, and that read's answer travels on the DecisionError: still pending puts the Side quest back on top with a Retry, decided shows the informational banner, and a failed read shows the error without Retry. The page no longer classifies from whatever the deck cache holds. A batch poll's deck read no longer cancels a read already out (which may be a decision's): it waits for it, then reads again, as that read may predate what the batch reported. Co-Authored-By: Claude Opus 5.5 --- web/src/discover/DiscoverPage.test.tsx | 84 +++++++++++++++++++++++ web/src/discover/DiscoverPage.tsx | 28 +++++--- web/src/discover/api.ts | 92 ++++++++++++++++++-------- 3 files changed, 166 insertions(+), 38 deletions(-) diff --git a/web/src/discover/DiscoverPage.test.tsx b/web/src/discover/DiscoverPage.test.tsx index 180846f..1120491 100644 --- a/web/src/discover/DiscoverPage.test.tsx +++ b/web/src/discover/DiscoverPage.test.tsx @@ -864,6 +864,90 @@ describe('DiscoverPage', () => { await waitFor(() => expect(sent(server.fetchMock)).toEqual(['POST /api/v1/suggestions/s1/accept', 'POST /api/v1/suggestions/s1/accept'])) }) + it('settles a failed decision on its own read of the deck when a batch poll reads the deck again during it', async () => { + const job = (suggestionIds: string[]) => + jsonResponse(200, { + id: 'j1', + kind: 'suggestion_batch', + status: 'running', + result: { batch_id: 'b1', stage: 'suggesting', suggestion_ids: suggestionIds, dropped: 0 }, + error: null, + }) + let poll: ReturnType | null = null + let read: ReturnType | null = null + let refuse = true + const server = stubServer([BOWLS, TIKKA, ORZO, PAELLA], { + 'POST /api/v1/suggestions/s1/decline': () => { + server.deck = [TIKKA, ORZO, PAELLA] + return jsonResponse(200, { suggestion: { ...BOWLS, status: 'declined' }, replenish_job_id: 'j1' }) + }, + 'POST /api/v1/suggestions/s2/accept': () => { + if (!refuse) return accepted(TIKKA) + read = deferred() + return errorResponse(500, 'internal') + }, + 'GET /api/v1/jobs/j1': () => poll?.promise ?? job([]), + [`GET ${DECK}`]: () => read?.promise ?? jsonResponse(200, { suggestions: server.deck }), + }) + const queryClient = createQueryClient() + await renderPage(queryClient) + await screen.findByRole('article', { name: 'Korean beef bowls' }) + + // A Pass starts a replenish batch, whose job is polled. + fireEvent.click(screen.getByRole('button', { name: 'Pass' })) + await waitFor(() => expect(jobPolls(server.fetchMock)).toBe(1)) + await waitFor(() => expect(screen.getByRole('button', { name: 'Try it' })).toBeEnabled()) + expect(topCard()).toHaveAccessibleName('Crock pot chicken tikka masala') + poll = deferred() + const polled = poll + await waitFor(() => expect(jobPolls(server.fetchMock)).toBe(2), { timeout: 2000 }) + + // Try it fails without being saved; the deck read it makes is slow. + const reads = deckReads(server.fetchMock) + fireEvent.click(screen.getByRole('button', { name: 'Try it' })) + await waitFor(() => expect(deckReads(server.fetchMock)).toBe(reads + 1)) + const reconciling = read! + read = deferred() + const after = read + expect(screen.getByRole('button', { name: 'Try it' })).toBeDisabled() + + // The batch stores a Side quest while that read is out: its read of + // the deck waits for the decision's rather than replacing it. + const SOUP = suggestionBody('s5', 'Tortellini soup') + server.deck = [TIKKA, ORZO, PAELLA, SOUP] + await act(async () => polled.resolve(job(['s5']))) + await act(async () => await new Promise((resolve) => setTimeout(resolve, 0))) + expect(deckReads(server.fetchMock)).toBe(reads + 1) + expect(screen.getByRole('button', { name: 'Try it' })).toBeDisabled() + + // The decision's read has the Side quest still pending: the card goes + // back on top with a Retry, and the decision settles on that read. + await act(async () => reconciling.resolve(jsonResponse(200, { suggestions: server.deck }))) + await waitFor(() => expect(screen.getByRole('button', { name: 'Try it' })).toBeEnabled()) + const alert = screen.getByRole('alert') + expect(alert).toHaveTextContent("Couldn't save that choice.") + expect(within(alert).getByRole('button', { name: 'Retry' })).toBeInTheDocument() + expect(topCard()).toHaveAccessibleName('Crock pot chicken tikka masala') + + // The batch's read goes out after it, and brings its Side quest. + await waitFor(() => expect(deckReads(server.fetchMock)).toBe(reads + 2)) + read = null + await act(async () => after.resolve(jsonResponse(200, { suggestions: server.deck }))) + await waitFor(() => expect(queryClient.getQueryData<{ id: string }[]>(DECK_KEY)?.map((q) => q.id)).toEqual(['s2', 's3', 's4', 's5'])) + expect(topCard()).toHaveAccessibleName('Crock pot chicken tikka masala') + + // Retry sends it again. + refuse = false + fireEvent.click(within(alert).getByRole('button', { name: 'Retry' })) + await waitFor(() => + expect(sent(server.fetchMock)).toEqual([ + 'POST /api/v1/suggestions/s1/decline', + 'POST /api/v1/suggestions/s2/accept', + 'POST /api/v1/suggestions/s2/accept', + ]), + ) + }) + it('reads the deck again after a 500 to a decision, rather than putting the card back', async () => { const decline = deferred() const server = stubServer([BOWLS, TIKKA, ORZO, PAELLA], { diff --git a/web/src/discover/DiscoverPage.tsx b/web/src/discover/DiscoverPage.tsx index 46c8bb9..1ab836c 100644 --- a/web/src/discover/DiscoverPage.tsx +++ b/web/src/discover/DiscoverPage.tsx @@ -90,10 +90,10 @@ function QuestLogPill({ dinners, ref }: { dinners: { id: string; name: string }[ // at a time from the top card. A decision from the buttons, the ← / → keys // or a swipe of the top card throws it out (motion.ts) and sends the // decision once it is gone, taking the card off the deck at once (see -// useDecide); Retry sends a failed one again while the deck read since still -// has its Side quest pending. Input is locked from the press until the -// decision settles, which is once the deck and the Quest log have been read -// again after it, so no decision starts under those reads. The lock is a +// useDecide); Retry sends a failed one again while the deck it read after +// failing still had its Side quest pending. Input is locked from the press +// until the decision settles, which is once the deck and the Quest log have +// been read again after it, so no decision starts under those reads. The lock is a // ref, set before anything starts, so a second press before the next render // does nothing, and busy mirrors it to disable the buttons. export function DiscoverPage() { @@ -310,17 +310,23 @@ export function DiscoverPage() { }, [decide]) // The batch stores its Side quests as it goes, so the deck is read again - // whenever it reports more, and once it ends. + // whenever it reports more, and once it ends. A read already out may be a + // decision's, which must not be cut short: the batch's read waits for it, + // then goes out after it, as it may predate what the batch reported. const { stored } = batch useEffect(() => { if (job.status === undefined) return - void queryClient.invalidateQueries({ queryKey: DECK_KEY }) + const after = queryClient.isFetching({ queryKey: DECK_KEY }) > 0 + void queryClient + .invalidateQueries({ queryKey: DECK_KEY }, { cancelRefetch: false }) + .then(() => (after ? queryClient.invalidateQueries({ queryKey: DECK_KEY }, { cancelRefetch: false }) : undefined)) }, [queryClient, job.status, stored]) - // A failed decision can be retried only while the deck, read again after - // it, still has its Side quest pending: anything else decided it already. - const failed = decision.isError ? decision.variables.quest.id : null - const retryable = failed !== null && !deck.isError && deck.data !== undefined && deck.data.some((q) => q.id === failed) + // A failed decision can be retried only while the deck it read after + // failing still has its Side quest pending: anything else decided it + // already. + const pending = decision.isError ? decision.error.pending : undefined + const retryable = pending !== undefined && pending !== null let body if (deck.isError) { @@ -392,7 +398,7 @@ export function DiscoverPage() { {decision.isError && (retryable ? ( submit(decision.variables)}>Couldn't save that choice. {decision.error.message} - ) : deck.isError ? ( + ) : pending === undefined ? ( Couldn't save that choice. {decision.error.message} ) : ( That side quest has been decided already. {decision.error.message} diff --git a/web/src/discover/api.ts b/web/src/discover/api.ts index 47b78eb..9ee08c4 100644 --- a/web/src/discover/api.ts +++ b/web/src/discover/api.ts @@ -130,6 +130,20 @@ export function useStartBatch() { }) } +// DecisionError is a failed decision, with what the deck read after it +// found: pending is its Side quest while the server still has it pending, +// null once it is decided, and undefined when that read failed too (the +// deck query holds the read's error). +export class DecisionError extends Error { + readonly pending: SideQuest | null | undefined + + constructor(cause: unknown, pending: SideQuest | null | undefined) { + super(cause instanceof Error ? cause.message : String(cause), { cause }) + this.name = 'DecisionError' + this.pending = pending + } +} + // useDecide records Try it (POST /api/v1/suggestions/{id}/accept) or Pass // (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. @@ -141,30 +155,51 @@ export function useStartBatch() { // Nothing else is worked out here: the server is the record of both. However // the decision ends, saved or failed, the deck and the Quest log are read // again, and the decision is pending until those reads land, so the next one -// (which DiscoverPage locks out until then) cannot start under them. No -// decision cancels a read; one under way when the decision ends is replaced -// by the read made after it. A failed decision whose Side quest that read -// still has is still pending, and goes back on top of the deck. +// (which DiscoverPage locks out until then) cannot start under them. A read +// under way when the decision ends is replaced by the read made after it, +// and DiscoverPage's batch reads wait for that one rather than replace it. +// A failed decision is a DecisionError, settled on the read it made and +// waited for itself: a Side quest that read still has is still pending, and +// goes back on top of the deck. export function useDecide() { const { apiFetch } = useApi() const queryClient = useQueryClient() - return useMutation({ - 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' }) - if ( - !isRecord(body) || - !isRecord(body.suggestion) || - body.suggestion.id !== id || - body.suggestion.status !== DECISION_STATUS[choice] - ) { - throw new Error(`POST ${path}: response is not the ${DECISION_STATUS[choice]} suggestion`) + + // send POSTs the decision and checks the answer. + async function send(id: string, choice: Choice): Promise { + const action = choice === 'try' ? 'accept' : 'decline' + const path = `${SUGGESTIONS_PATH}/${encodeURIComponent(id)}/${action}` + const body = await apiFetch(path, { method: 'POST' }) + if ( + !isRecord(body) || + !isRecord(body.suggestion) || + body.suggestion.id !== id || + body.suggestion.status !== DECISION_STATUS[choice] + ) { + throw new Error(`POST ${path}: response is not the ${DECISION_STATUS[choice]} suggestion`) + } + if (body.replenish_job_id !== null && typeof body.replenish_job_id !== 'string') { + throw new Error(`POST ${path}: response has no replenish_job_id`) + } + return body.replenish_job_id + } + + return useMutation({ + mutationFn: async ({ quest, choice }) => { + try { + return await send(quest.id, choice) + } catch (error) { + // The deck is read again as the decision's own fetch, replacing any + // read already out; its answer, not the cache's, settles it. + let read: SideQuest[] + try { + read = await queryClient.getQueryCache().build(queryClient, { queryKey: DECK_KEY }).fetch(undefined, { cancelRefetch: true }) + } catch { + // The deck query holds the read's error, shown with the deck. + throw new DecisionError(error, undefined) + } + throw new DecisionError(error, read.find((q) => q.id === quest.id) ?? null) } - if (body.replenish_job_id !== null && typeof body.replenish_job_id !== 'string') { - throw new Error(`POST ${path}: response has no replenish_job_id`) - } - return body.replenish_job_id }, onMutate: ({ quest, choice }) => { queryClient.setQueryData(DECK_KEY, (deck) => deck?.filter((q) => q.id !== quest.id)) @@ -174,13 +209,16 @@ export function useDecide() { dinners === undefined || dinners.some((d) => d.id === quest.dinner_id) ? dinners : dinners.concat({ id: quest.dinner_id, name: quest.name }), ) }, - onSettled: async (_replenishJobId, error, { quest: { id } }) => { - await Promise.all([queryClient.invalidateQueries({ queryKey: DECK_KEY }), queryClient.invalidateQueries({ queryKey: QUEST_LOG_KEY })]) - if (error === null) return - queryClient.setQueryData(DECK_KEY, (deck) => { - const pending = deck?.find((q) => q.id === id) - return pending === undefined ? deck : [pending, ...deck!.filter((q) => q !== pending)] - }) + onSettled: async (_replenishJobId, error) => { + if (error === null) { + await Promise.all([queryClient.invalidateQueries({ queryKey: DECK_KEY }), queryClient.invalidateQueries({ queryKey: QUEST_LOG_KEY })]) + return + } + // A failed decision has read the deck already. + await queryClient.invalidateQueries({ queryKey: QUEST_LOG_KEY }) + const { pending } = error + if (pending === null || pending === undefined) return + queryClient.setQueryData(DECK_KEY, (deck) => [pending, ...(deck ?? []).filter((q) => q.id !== pending.id)]) }, }) } -- 2.55.0 From 701021d3277cd0df0a9156ec5f8ae55d70063cee Mon Sep 17 00:00:00 2001 From: dev_agent Date: Wed, 7 Oct 2026 03:45:51 -0400 Subject: [PATCH 8/9] Settle a failed Discover decision on the deck's newest read A failed decision still waits for the deck read it makes itself, but no longer carries that read's answer on its error. Whether its Side quest is still pending is now asked of the deck query's newest successful read (stillPending), both when the decision settles and when the page offers Retry. A read that lands later and has the Side quest decided keeps it off the deck and withdraws the Retry; a deck whose latest read failed offers no Retry. Co-Authored-By: Claude Opus 5.5 --- web/src/discover/DiscoverPage.test.tsx | 54 ++++++++++++++++++++++++++ web/src/discover/DiscoverPage.tsx | 12 +++--- web/src/discover/api.ts | 53 ++++++++++++------------- 3 files changed, 84 insertions(+), 35 deletions(-) diff --git a/web/src/discover/DiscoverPage.test.tsx b/web/src/discover/DiscoverPage.test.tsx index 1120491..5ff2518 100644 --- a/web/src/discover/DiscoverPage.test.tsx +++ b/web/src/discover/DiscoverPage.test.tsx @@ -948,6 +948,60 @@ describe('DiscoverPage', () => { ) }) + it('does not put a failed decision back on the deck when a later read, landing before it settles, has it decided', async () => { + let log: ReturnType | null = null + const server = stubServer([BOWLS, TIKKA, ORZO, PAELLA], { + 'POST /api/v1/suggestions/s1/accept': () => { + log = deferred() + return errorResponse(500, 'internal') + }, + [`GET ${QUEST_LOG}`]: () => log?.promise ?? jsonResponse(200, { dinners: [] }), + }) + const queryClient = createQueryClient() + await renderPage(queryClient) + await screen.findByRole('article', { name: 'Korean beef bowls' }) + + // The decision fails; the deck read after it still has the Side quest + // pending, and the Quest log read after it is slow. + fireEvent.click(screen.getByRole('button', { name: 'Try it' })) + await waitFor(() => expect(log).not.toBeNull()) + const slow = log! + log = null + await waitFor(() => expect(deckReads(server.fetchMock)).toBe(2)) + + // Another session decides it, and a read of the deck lands meanwhile. + server.deck = [TIKKA, ORZO, PAELLA] + await act(async () => await queryClient.refetchQueries({ queryKey: DECK_KEY })) + await act(async () => slow.resolve(jsonResponse(200, { dinners: [] }))) + + const alert = await screen.findByRole('alert') + expect(screen.queryByRole('article', { name: 'Korean beef bowls' })).not.toBeInTheDocument() + expect(topCard()).toHaveAccessibleName('Crock pot chicken tikka masala') + expect(alert).toHaveTextContent('That side quest has been decided already.') + expect(within(alert).queryByRole('button', { name: 'Retry' })).not.toBeInTheDocument() + }) + + it('withdraws the Retry for a failed decision once a later read of the deck has it decided', async () => { + const server = stubServer([BOWLS, TIKKA, ORZO, PAELLA], { + 'POST /api/v1/suggestions/s1/accept': () => errorResponse(500, 'internal'), + }) + const queryClient = createQueryClient() + await renderPage(queryClient) + await screen.findByRole('article', { name: 'Korean beef bowls' }) + + fireEvent.click(screen.getByRole('button', { name: 'Try it' })) + const alert = await screen.findByRole('alert') + expect(within(alert).getByRole('button', { name: 'Retry' })).toBeInTheDocument() + expect(topCard()).toHaveAccessibleName('Korean beef bowls') + + // Another session decides it, and the next read of the deck says so. + server.deck = [TIKKA, ORZO, PAELLA] + await act(async () => await queryClient.refetchQueries({ queryKey: DECK_KEY })) + await waitFor(() => expect(topCard()).toHaveAccessibleName('Crock pot chicken tikka masala')) + expect(screen.getByRole('alert')).toHaveTextContent('That side quest has been decided already.') + expect(screen.queryByRole('button', { name: 'Retry' })).not.toBeInTheDocument() + }) + it('reads the deck again after a 500 to a decision, rather than putting the card back', async () => { const decline = deferred() const server = stubServer([BOWLS, TIKKA, ORZO, PAELLA], { diff --git a/web/src/discover/DiscoverPage.tsx b/web/src/discover/DiscoverPage.tsx index 1ab836c..b3e3a0f 100644 --- a/web/src/discover/DiscoverPage.tsx +++ b/web/src/discover/DiscoverPage.tsx @@ -12,6 +12,7 @@ import { BATCH_JOB_KIND, DECK_KEY, parseBatchResult, + stillPending, useDeck, useDecide, useQuestLog, @@ -90,8 +91,8 @@ function QuestLogPill({ dinners, ref }: { dinners: { id: string; name: string }[ // at a time from the top card. A decision from the buttons, the ← / → keys // or a swipe of the top card throws it out (motion.ts) and sends the // decision once it is gone, taking the card off the deck at once (see -// useDecide); Retry sends a failed one again while the deck it read after -// failing still had its Side quest pending. Input is locked from the press +// useDecide); Retry sends a failed one again while the deck's newest read +// still has its Side quest pending. Input is locked from the press // until the decision settles, which is once the deck and the Quest log have // been read again after it, so no decision starts under those reads. The lock is a // ref, set before anything starts, so a second press before the next render @@ -322,10 +323,9 @@ export function DiscoverPage() { .then(() => (after ? queryClient.invalidateQueries({ queryKey: DECK_KEY }, { cancelRefetch: false }) : undefined)) }, [queryClient, job.status, stored]) - // A failed decision can be retried only while the deck it read after - // failing still has its Side quest pending: anything else decided it - // already. - const pending = decision.isError ? decision.error.pending : undefined + // A failed decision can be retried only while the deck's newest read + // still has its Side quest pending: anything else decided it already. + const pending = decision.isError ? stillPending(deck, decision.variables.quest.id) : undefined const retryable = pending !== undefined && pending !== null let body diff --git a/web/src/discover/api.ts b/web/src/discover/api.ts index 9ee08c4..1251e5a 100644 --- a/web/src/discover/api.ts +++ b/web/src/discover/api.ts @@ -130,18 +130,13 @@ export function useStartBatch() { }) } -// DecisionError is a failed decision, with what the deck read after it -// found: pending is its Side quest while the server still has it pending, -// null once it is decided, and undefined when that read failed too (the -// deck query holds the read's error). -export class DecisionError extends Error { - readonly pending: SideQuest | null | undefined - - constructor(cause: unknown, pending: SideQuest | null | undefined) { - super(cause instanceof Error ? cause.message : String(cause), { cause }) - this.name = 'DecisionError' - this.pending = pending - } +// stillPending says whether the deck's newest read has the Side quest id +// pending, after a failed decision on it: the Side quest when it does, null +// when it does not, and undefined when that read failed, which says neither. +// deck is the deck query's state, from the cache or useDeck. +export function stillPending(deck: { status: string; data: SideQuest[] | undefined } | undefined, id: string): SideQuest | null | undefined { + if (deck?.status !== 'success') return undefined + return deck.data!.find((q) => q.id === id) ?? null } // useDecide records Try it (POST /api/v1/suggestions/{id}/accept) or Pass @@ -158,9 +153,10 @@ export class DecisionError extends Error { // (which DiscoverPage locks out until then) cannot start under them. A read // under way when the decision ends is replaced by the read made after it, // and DiscoverPage's batch reads wait for that one rather than replace it. -// A failed decision is a DecisionError, settled on the read it made and -// waited for itself: a Side quest that read still has is still pending, and -// goes back on top of the deck. +// A failed decision waits for the deck read it makes itself. Whether its +// Side quest is still pending is then the deck query's to say, from its +// newest successful read: once the reads have landed, a Side quest the deck +// still has goes back on top of it. export function useDecide() { const { apiFetch } = useApi() const queryClient = useQueryClient() @@ -184,21 +180,20 @@ export function useDecide() { return body.replenish_job_id } - return useMutation({ + return useMutation({ mutationFn: async ({ quest, choice }) => { try { return await send(quest.id, choice) } catch (error) { // The deck is read again as the decision's own fetch, replacing any - // read already out; its answer, not the cache's, settles it. - let read: SideQuest[] - try { - read = await queryClient.getQueryCache().build(queryClient, { queryKey: DECK_KEY }).fetch(undefined, { cancelRefetch: true }) - } catch { - // The deck query holds the read's error, shown with the deck. - throw new DecisionError(error, undefined) - } - throw new DecisionError(error, read.find((q) => q.id === quest.id) ?? null) + // read already out. A failed read is the deck query's error, shown + // with the deck; the decision's own error is the one thrown. + await queryClient + .getQueryCache() + .build(queryClient, { queryKey: DECK_KEY }) + .fetch(undefined, { cancelRefetch: true }) + .catch(() => undefined) + throw error } }, onMutate: ({ quest, choice }) => { @@ -209,16 +204,16 @@ export function useDecide() { dinners === undefined || dinners.some((d) => d.id === quest.dinner_id) ? dinners : dinners.concat({ id: quest.dinner_id, name: quest.name }), ) }, - onSettled: async (_replenishJobId, error) => { + onSettled: async (_replenishJobId, error, { quest }) => { if (error === null) { await Promise.all([queryClient.invalidateQueries({ queryKey: DECK_KEY }), queryClient.invalidateQueries({ queryKey: QUEST_LOG_KEY })]) return } - // A failed decision has read the deck already. + // A failed decision has read the deck already; any read since is newer. await queryClient.invalidateQueries({ queryKey: QUEST_LOG_KEY }) - const { pending } = error + const pending = stillPending(queryClient.getQueryState(DECK_KEY), quest.id) if (pending === null || pending === undefined) return - queryClient.setQueryData(DECK_KEY, (deck) => [pending, ...(deck ?? []).filter((q) => q.id !== pending.id)]) + queryClient.setQueryData(DECK_KEY, (deck) => [pending, ...deck!.filter((q) => q.id !== pending.id)]) }, }) } -- 2.55.0 From 6fd607f1250e4da6cdacb98d4c1a9f5894342e69 Mon Sep 17 00:00:00 2001 From: dev_agent Date: Wed, 7 Oct 2026 03:57:53 -0400 Subject: [PATCH 9/9] Keep a Side quest off the shown deck while its decision is pending The deck shown is now the deck's newest read less the Side quests whose decisions are in flight, taken from the pending decide mutations, instead of removing the card from the deck cache in onMutate. A read of the deck landing before the decision is answered can no longer bring the exited card back; settlement still reconciles from the server read as before. Co-Authored-By: Claude Opus 5.5 --- web/src/discover/DiscoverPage.test.tsx | 72 ++++++++++++++++++++++++++ web/src/discover/DiscoverPage.tsx | 11 ++-- web/src/discover/api.ts | 36 +++++++++---- 3 files changed, 105 insertions(+), 14 deletions(-) diff --git a/web/src/discover/DiscoverPage.test.tsx b/web/src/discover/DiscoverPage.test.tsx index 5ff2518..66d941c 100644 --- a/web/src/discover/DiscoverPage.test.tsx +++ b/web/src/discover/DiscoverPage.test.tsx @@ -653,6 +653,78 @@ describe('DiscoverPage', () => { ]) }) + it('keeps the card off the deck while its decision is unanswered when a read of the deck lands with it still pending, then reads the deck again once it is saved', async () => { + const decline = deferred() + const server = stubServer([BOWLS, TIKKA, ORZO, PAELLA], { 'POST /api/v1/suggestions/s1/decline': () => decline.promise }) + 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 lands before the decision is saved, with the Side + // quest still pending: the card stays off the deck. + await act(async () => await queryClient.refetchQueries({ queryKey: DECK_KEY })) + await act(async () => await new Promise((resolve) => setTimeout(resolve, 0))) + expect(deckReads(server.fetchMock)).toBe(2) + expect(queryClient.getQueryData<{ id: string }[]>(DECK_KEY)?.map((q) => q.id)).toEqual(['s1', 's2', 's3', 's4']) + 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', + ]) + + // Once it is saved, the deck read after it is the deck shown. + server.deck = [TIKKA, ORZO, PAELLA] + decline.resolve(declined(BOWLS)) + await waitFor(() => expect(screen.getByRole('button', { name: 'Pass' })).toBeEnabled()) + expect(deckReads(server.fetchMock)).toBe(3) + expect(screen.queryByRole('article', { name: 'Korean beef bowls' })).not.toBeInTheDocument() + 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', + ]) + expect(screen.queryByRole('alert')).not.toBeInTheDocument() + }) + + it('keeps the card off the deck while its decision is unanswered when a read of the deck lands with it still pending, then puts it back on top with a Retry once it is refused', async () => { + const accept = deferred() + const server = stubServer([BOWLS, TIKKA, ORZO, PAELLA], { 'POST /api/v1/suggestions/s1/accept': () => accept.promise }) + 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'])) + expect(topCard()).toHaveAccessibleName('Crock pot chicken tikka masala') + + // A read of the deck lands mid-decision, in the server's order. + server.deck = [TIKKA, BOWLS, ORZO, PAELLA] + await act(async () => await queryClient.refetchQueries({ queryKey: DECK_KEY })) + await act(async () => await new Promise((resolve) => setTimeout(resolve, 0))) + expect(deckReads(server.fetchMock)).toBe(2) + 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', + ]) + + // The server refuses it; the deck read after it still has it pending. + accept.resolve(errorResponse(400, 'invalid_request')) + const alert = await screen.findByRole('alert') + expect(alert).toHaveTextContent("Couldn't save that choice.") + expect(within(alert).getByRole('button', { name: 'Retry' })).toBeInTheDocument() + expect(deckReads(server.fetchMock)).toBe(3) + expect(screen.getAllByRole('article', { hidden: true }).map((card) => card.querySelector('h2')!.textContent)).toEqual([ + 'Korean beef bowls', + 'Crock pot chicken tikka masala', + 'Lemon garlic shrimp orzo', + ]) + }) + 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], { diff --git a/web/src/discover/DiscoverPage.tsx b/web/src/discover/DiscoverPage.tsx index b3e3a0f..078286b 100644 --- a/web/src/discover/DiscoverPage.tsx +++ b/web/src/discover/DiscoverPage.tsx @@ -13,6 +13,7 @@ import { DECK_KEY, parseBatchResult, stillPending, + useDeciding, useDeck, useDecide, useQuestLog, @@ -100,6 +101,10 @@ function QuestLogPill({ dinners, ref }: { dinners: { id: string; name: string }[ export function DiscoverPage() { const queryClient = useQueryClient() const deck = useDeck() + // The deck shown: the deck's newest read, less the Side quests being + // decided, which a read landing mid-decision may still have pending. + const inFlight = useDeciding() + const cards = deck.data?.filter((quest) => !inFlight.has(quest.id)) const questLog = useQuestLog() const decision = useDecide() const startBatch = useStartBatch() @@ -108,7 +113,7 @@ export function DiscoverPage() { const [batchJobId, setBatchJobId] = useState(null) const job = useJob(BATCH_JOB_KIND, batchJobId) - const top = deck.data?.[0] + const top = cards?.[0] const [busy, setBusy] = useState(false) const jobRunning = batchJobId !== null && job.status !== 'succeeded' && job.status !== 'failed' const finding = startBatch.isPending || jobRunning @@ -331,7 +336,7 @@ export function DiscoverPage() { let body if (deck.isError) { body = void deck.refetch()}>Couldn't load your side quests. {deck.error.message} - } else if (deck.isPending || (deck.data.length === 0 && (finding || decision.isPending))) { + } else if (deck.isPending || (cards!.length === 0 && (finding || decision.isPending))) { // An empty deck is not caught up while a batch finds more Side quests, // or while the last decision may yet start one or come back refused. body = ( @@ -365,7 +370,7 @@ export function DiscoverPage() { body = ( <>
- {deck.data.slice(0, DECK_DEPTH.length).map((quest, depth) => ( + {cards!.slice(0, DECK_DEPTH.length).map((quest, depth) => ( ))}
diff --git a/web/src/discover/api.ts b/web/src/discover/api.ts index 1251e5a..c233de3 100644 --- a/web/src/discover/api.ts +++ b/web/src/discover/api.ts @@ -1,4 +1,4 @@ -import { useMutation, useQuery, useQueryClient } from '@tanstack/react-query' +import { useMutation, useMutationState, useQuery, useQueryClient } from '@tanstack/react-query' import { useApi } from '../api/index.ts' import { isRecord } from '../api/errors.ts' import { DIFFICULTIES, DINNERS_PATH, oneOf, photoId, type Difficulty } from '../dinners/api.ts' @@ -8,9 +8,11 @@ export const DECK_PATH = `${SUGGESTIONS_PATH}/deck` export const BATCHES_PATH = `${SUGGESTIONS_PATH}/batches` export const QUEST_LOG_PATH = `${DINNERS_PATH}?status=quest_log` -// DECK_KEY is the deck's query; QUEST_LOG_KEY the Quest log's. +// DECK_KEY is the deck's query; QUEST_LOG_KEY the Quest log's; +// DECIDE_KEY the decisions'. export const DECK_KEY = ['suggestions', 'deck'] as const export const QUEST_LOG_KEY = ['dinners', 'quest_log'] as const +const DECIDE_KEY = ['suggestions', 'decide'] as const // SideQuest is a pending suggestion as the deck shows it. dinner_id is the // dinner Try it moves to the Quest log; photo_id is the stored photo's id, @@ -139,20 +141,32 @@ export function stillPending(deck: { status: string; data: SideQuest[] | undefin return deck.data!.find((q) => q.id === id) ?? null } +// useDeciding is the ids of the Side quests whose decisions are pending: +// sent and not yet settled. The deck shown leaves them out, whatever a read +// of the deck has said meanwhile. +export function useDeciding(): Set { + const ids = useMutationState({ + filters: { mutationKey: DECIDE_KEY, status: 'pending' }, + select: (mutation) => (mutation.state.variables as { quest: SideQuest }).quest.id, + }) + return new Set(ids) +} + // useDecide records Try it (POST /api/v1/suggestions/{id}/accept) or Pass // (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. -// Nothing else is worked out here: the server is the record of both. However -// the decision ends, saved or failed, the deck and the Quest log are read -// again, and the decision is pending until those reads land, so the next one -// (which DiscoverPage locks out until then) cannot start under them. A read -// under way when the decision ends is replaced by the read made after it, -// and DiscoverPage's batch reads wait for that one rather than replace it. +// The decision is applied at once: the Side quest leaves the deck shown +// while it is pending (see useDeciding) and, on Try it, its dinner joins the +// Quest log unless it is there already. Nothing else is worked out here: +// the server is the record of both. However the decision ends, saved or +// failed, the deck and the Quest log are read again, and the decision is +// pending until those reads land, so the next one (which DiscoverPage locks +// out until then) cannot start under them. A read under way when the +// decision ends is replaced by the read made after it, and DiscoverPage's +// batch reads wait for that one rather than replace it. // A failed decision waits for the deck read it makes itself. Whether its // Side quest is still pending is then the deck query's to say, from its // newest successful read: once the reads have landed, a Side quest the deck @@ -181,6 +195,7 @@ export function useDecide() { } return useMutation({ + mutationKey: DECIDE_KEY, mutationFn: async ({ quest, choice }) => { try { return await send(quest.id, choice) @@ -197,7 +212,6 @@ export function useDecide() { } }, onMutate: ({ quest, choice }) => { - queryClient.setQueryData(DECK_KEY, (deck) => deck?.filter((q) => q.id !== quest.id)) if (choice !== 'try') return // A stale deck can offer a Side quest whose dinner the Quest log has. queryClient.setQueryData(QUEST_LOG_KEY, (dinners) => -- 2.55.0