From 3882f670ca4921cf137fcd74cb24131d91e4ad09 Mon Sep 17 00:00:00 2001 From: dev_agent Date: Sun, 11 Oct 2026 21:58:52 -0400 Subject: [PATCH 1/2] Photo backfill UI: fix-recipe failures and Find missing photos (Maestro 7a468d2d, Feature 23 task 2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Settings → Regular dinners gains a "Find missing photos" trigger that calls POST /api/v1/settings/photo-backfill (202 {id}) and follows the backfill_dinner_photos job with useJob, showing progress, a summary of the outcomes, or why it stopped. When the job ends, the dinners, plans and home queries are read again so found photos show everywhere. The fix-recipe sheet now shows the dinner's grounding_error as a typed reason and offers "Search again" (the same backfill), reporting this dinner's outcome; it is not offered for recipe_unusable, since the backfill never replaces a pasted link. Regular now parses grounding_error. Task: 7a468d2d-4a9b-45f2-b626-a7bed5f671df Co-Authored-By: Claude Opus 5.5 --- web/src/dinners/DinnerDetailActions.test.tsx | 1 + web/src/dinners/api.ts | 12 +- web/src/plan/PlanPage.test.tsx | 1 + .../settings/RegularsSettingsPage.module.css | 24 +++ .../settings/RegularsSettingsPage.test.tsx | 189 ++++++++++++++++++ web/src/settings/RegularsSettingsPage.tsx | 156 ++++++++++++++- web/src/settings/photoBackfill.test.tsx | 63 ++++++ web/src/settings/photoBackfill.ts | 110 ++++++++++ 8 files changed, 544 insertions(+), 12 deletions(-) create mode 100644 web/src/settings/photoBackfill.test.tsx create mode 100644 web/src/settings/photoBackfill.ts diff --git a/web/src/dinners/DinnerDetailActions.test.tsx b/web/src/dinners/DinnerDetailActions.test.tsx index dc967a4..e2e27c6 100644 --- a/web/src/dinners/DinnerDetailActions.test.tsx +++ b/web/src/dinners/DinnerDetailActions.test.tsx @@ -52,6 +52,7 @@ function listed(is_favorite: boolean) { is_favorite, photo_url: null, grounding_status: 'grounded', + grounding_error: null, } } diff --git a/web/src/dinners/api.ts b/web/src/dinners/api.ts index 2c0c081..c01745d 100644 --- a/web/src/dinners/api.ts +++ b/web/src/dinners/api.ts @@ -20,8 +20,10 @@ export type Difficulty = (typeof DIFFICULTIES)[number] export type GroundingStatus = (typeof GROUNDING_STATUSES)[number] // Regular is the part of a dinner the Regulars screens read. photo_id is -// the stored photo's id, from photo_url. pending is true while an -// optimistic add waits for the server; its id is not the server's. +// the stored photo's id, from photo_url. grounding_error is why its +// grounding failed, a reason code, set only while grounding_status is +// failed. pending is true while an optimistic add waits for the server; +// its id is not the server's. export type Regular = { id: string name: string @@ -31,6 +33,7 @@ export type Regular = { is_favorite: boolean photo_id: string | undefined grounding_status: GroundingStatus + grounding_error: string | null pending?: true } @@ -122,6 +125,10 @@ export function parseDinner(where: string, body: unknown): Regular { is_favorite: body.is_favorite, photo_id: photoId(where, body.photo_url), grounding_status: oneOf(where, 'grounding_status', GROUNDING_STATUSES, body.grounding_status), + grounding_error: nullable(body.grounding_error, (v) => { + if (typeof v !== 'string') throw new Error(`${where}: grounding_error is not a string`) + return v + }), } } @@ -225,6 +232,7 @@ export function useAddRegular() { is_favorite: false, photo_id: undefined, grounding_status: 'pending', + grounding_error: null, pending: true, } await changeOptimistically(queryClient, (list) => [...list, optimistic]) diff --git a/web/src/plan/PlanPage.test.tsx b/web/src/plan/PlanPage.test.tsx index 77c9867..76d97d4 100644 --- a/web/src/plan/PlanPage.test.tsx +++ b/web/src/plan/PlanPage.test.tsx @@ -114,6 +114,7 @@ function dinner(id: string, name: string, difficulty: string | null, minutes: nu is_favorite: false, photo_url: null, grounding_status: 'grounded', + grounding_error: null, } } diff --git a/web/src/settings/RegularsSettingsPage.module.css b/web/src/settings/RegularsSettingsPage.module.css index 0954766..b5de520 100644 --- a/web/src/settings/RegularsSettingsPage.module.css +++ b/web/src/settings/RegularsSettingsPage.module.css @@ -300,3 +300,27 @@ .lock { display: contents; } + +/* Find missing photos: its hint, button and progress, padded like a sheet. */ +.card.backfill { + display: flex; + flex-direction: column; + gap: 12px; + padding: 16px; +} + +/* Why a Regular's recipe was not found. */ +.reason { + margin: 0; + padding: 12px 14px; + border-radius: 18px; + background: var(--color-neutral-100); + color: var(--color-neutral-900); + font-family: var(--font-body); + font-size: 14px; + line-height: 1.4; +} + +.reasonLabel { + font-weight: 700; +} diff --git a/web/src/settings/RegularsSettingsPage.test.tsx b/web/src/settings/RegularsSettingsPage.test.tsx index afae963..b92b132 100644 --- a/web/src/settings/RegularsSettingsPage.test.tsx +++ b/web/src/settings/RegularsSettingsPage.test.tsx @@ -496,6 +496,195 @@ describe('RegularsSettingsPage', () => { }) }) + describe('Photo backfill', () => { + const BACKFILL = 'POST /api/v1/settings/photo-backfill' + const JOB = 'GET /api/v1/jobs/j9' + + // backfillJob is a GET /api/v1/jobs/j9 answer for the photo backfill. + function backfillJob(status: string, result: unknown = null, error: string | null = null) { + return jsonResponse(200, { id: 'j9', kind: 'backfill_dinner_photos', status, result, error }) + } + + // polls answers each poll of the job with the next answer, then the last + // one again; an answer may change the server first. + function polls(...answers: (() => Response)[]) { + let next = 0 + return () => answers[Math.min(next++, answers.length - 1)]!() + } + + function fixSheet() { + return within(screen.getByRole('dialog', { name: 'Fix recipe' })) + } + + const NO_CANDIDATES = { 'GET /api/v1/dinners/d3/recipe-candidates': () => jsonResponse(200, { candidates: [] }) } + + it('shows why the recipe was not found in the fix-recipe sheet', async () => { + stubServer([PIZZA], NO_CANDIDATES) + await renderPage() + + openRow('Pizza night') + expect(fixSheet().getByText('No recipe page the search found had a recipe we could use.')).toBeInTheDocument() + expect(fixSheet().getByRole('button', { name: 'Search again' })).toBeEnabled() + }) + + it('offers no search again for a pasted link that had no usable recipe', async () => { + const linked = { ...PIZZA, grounding_error: 'recipe_unusable' } + stubServer([linked], NO_CANDIDATES) + await renderPage() + + openRow('Pizza night') + expect(fixSheet().getByText('The pasted recipe link has no recipe we could use.')).toBeInTheDocument() + expect(fixSheet().queryByRole('button', { name: 'Search again' })).not.toBeInTheDocument() + }) + + it('searches again from the fix-recipe sheet and shows the new reason when it still fails', async () => { + const server = stubServer([PIZZA], { + ...NO_CANDIDATES, + [BACKFILL]: () => jsonResponse(202, { id: 'j9' }), + [JOB]: polls( + () => backfillJob('running'), + () => { + server.regulars = [{ ...PIZZA, grounding_error: 'search_failed' }] + return backfillJob('succeeded', { dinners: [{ dinner_id: 'd3', outcome: 'failed', reason: 'search_failed' }] }) + }, + ), + }) + await renderPage() + + openRow('Pizza night') + fireEvent.click(fixSheet().getByRole('button', { name: 'Search again' })) + expect(await fixSheet().findByRole('button', { name: 'Searching again…' })).toBeDisabled() + expect(fixSheet().getByText('Searching again… this can take a few minutes.')).toBeInTheDocument() + expect(sent(server.fetchMock)).toEqual([{ request: BACKFILL, body: undefined }]) + + expect(await fixSheet().findByText('Searched again: still no recipe found.', {}, { timeout: 3000 })).toBeInTheDocument() + expect(await fixSheet().findByText("The recipe search couldn't be reached.")).toBeInTheDocument() + expect(fixSheet().getByRole('button', { name: 'Search again' })).toBeEnabled() + }) + + it('closes the fix-recipe sheet once searching again finds the recipe and its photo', async () => { + const server = stubServer([PIZZA], { + ...NO_CANDIDATES, + [BACKFILL]: () => jsonResponse(202, { id: 'j9' }), + [JOB]: () => { + server.regulars = [{ ...PIZZA, grounding_status: 'grounded', grounding_error: null, photo_url: '/media/photos/p8-400.jpg' }] + return backfillJob('succeeded', { dinners: [{ dinner_id: 'd3', outcome: 'grounded', reason: null }] }) + }, + }) + await renderPage() + + openRow('Pizza night') + fireEvent.click(fixSheet().getByRole('button', { name: 'Search again' })) + await waitFor(() => expect(screen.queryByRole('dialog')).not.toBeInTheDocument()) + expect(within(row('Pizza night')).getByText('Recipe found')).toBeInTheDocument() + expect(within(row('Pizza night')).getByRole('img', { name: 'Pizza night' })).toHaveAttribute('src', '/media/photos/p8-400.jpg') + }) + + it('says a dinner the search left out keeps its pasted recipe link', async () => { + const linked = { ...PIZZA, grounding_error: 'ai_failed' } + stubServer([linked], { + ...NO_CANDIDATES, + [BACKFILL]: () => jsonResponse(202, { id: 'j9' }), + [JOB]: () => backfillJob('succeeded', { dinners: [] }), + }) + await renderPage() + + openRow('Pizza night') + fireEvent.click(fixSheet().getByRole('button', { name: 'Search again' })) + expect(await fixSheet().findByText('Not searched again: it keeps its pasted recipe link.')).toBeInTheDocument() + }) + + it('finds missing photos: the placeholder shows until the backfill finds a photo', async () => { + const found = { ...PIZZA, grounding_status: 'grounded', grounding_error: null, photo_url: '/media/photos/p8-400.jpg' } + const server = stubServer([TACOS, PIZZA], { + [BACKFILL]: () => jsonResponse(202, { id: 'j9' }), + [JOB]: polls( + () => backfillJob('queued'), + () => { + server.regulars = [TACOS, found] + return backfillJob('succeeded', { dinners: [{ dinner_id: 'd3', outcome: 'grounded', reason: null }] }) + }, + ), + }) + await renderPage() + + // The photoless dinner shows the striped placeholder, not an image. + const placeholder = within(row('Pizza night')).getByRole('img', { name: 'Pizza night' }) + expect(placeholder).not.toHaveAttribute('src') + expect(placeholder).toHaveTextContent('dish photo') + + const trigger = screen.getByRole('button', { name: 'Find missing photos' }) + expectMinHitArea(trigger) + fireEvent.click(trigger) + expect(await screen.findByRole('button', { name: 'Finding photos…' })).toBeDisabled() + expect(await screen.findByText('Finding missing photos… this can take a few minutes.')).toBeInTheDocument() + expect(within(row('Pizza night')).getByRole('img', { name: 'Pizza night' })).not.toHaveAttribute('src') + expect(sent(server.fetchMock)).toEqual([{ request: BACKFILL, body: undefined }]) + + expect(await screen.findByText('Photo search done: 1 photo found.', {}, { timeout: 3000 })).toBeInTheDocument() + await waitFor(() => + expect(within(row('Pizza night')).getByRole('img', { name: 'Pizza night' })).toHaveAttribute('src', '/media/photos/p8-400.jpg'), + ) + expect(screen.getByRole('button', { name: 'Find missing photos' })).toBeEnabled() + expect(reads(server.fetchMock)).toBe(2) + }) + + it('sums up what the backfill did', async () => { + stubServer([PIZZA], { + [BACKFILL]: () => jsonResponse(202, { id: 'j9' }), + [JOB]: () => + backfillJob('succeeded', { + dinners: [ + { dinner_id: 'd3', outcome: 'grounded', reason: null }, + { dinner_id: 'd5', outcome: 'grounded', reason: null }, + { dinner_id: 'd6', outcome: 'failed', reason: 'no_recipe_found' }, + { dinner_id: 'd7', outcome: 'skipped', reason: null }, + ], + }), + }) + await renderPage() + + fireEvent.click(screen.getByRole('button', { name: 'Find missing photos' })) + expect( + await screen.findByText( + 'Photo search done: 2 photos found · 1 dinner still needs a recipe · 1 dinner changed meanwhile, left as it was.', + ), + ).toBeInTheDocument() + }) + + it('says when no dinner was missing a photo', async () => { + stubServer([TACOS], { + [BACKFILL]: () => jsonResponse(202, { id: 'j9' }), + [JOB]: () => backfillJob('succeeded', { dinners: [] }), + }) + await renderPage() + + fireEvent.click(screen.getByRole('button', { name: 'Find missing photos' })) + expect(await screen.findByText('No dinners were missing a photo.')).toBeInTheDocument() + }) + + it('says why the backfill stopped', async () => { + stubServer([PIZZA], { + [BACKFILL]: () => jsonResponse(202, { id: 'j9' }), + [JOB]: () => backfillJob('failed', { dinners: [] }, 'internal_error'), + }) + await renderPage() + + fireEvent.click(screen.getByRole('button', { name: 'Find missing photos' })) + expect(await screen.findByRole('alert')).toHaveTextContent('Photo search stopped: Something went wrong on the server.') + expect(screen.getByRole('button', { name: 'Find missing photos' })).toBeEnabled() + }) + + it('says why the backfill could not start', async () => { + stubServer([PIZZA], { [BACKFILL]: () => errorResponse(403, 'csrf_failed') }) + await renderPage() + + fireEvent.click(screen.getByRole('button', { name: 'Find missing photos' })) + expect(await screen.findByRole('alert')).toHaveTextContent('Photo search not started: csrf_failed message') + expect(screen.getByRole('button', { name: 'Find missing photos' })).toBeEnabled() + }) + }) + it('refreshes grounding status while any Regular is finding its recipe, then stops', async () => { // Only intervals are faked: TanStack Query polls with setInterval, and // Testing Library's waits keep their real timers. diff --git a/web/src/settings/RegularsSettingsPage.tsx b/web/src/settings/RegularsSettingsPage.tsx index c816b49..d700d22 100644 --- a/web/src/settings/RegularsSettingsPage.tsx +++ b/web/src/settings/RegularsSettingsPage.tsx @@ -34,6 +34,7 @@ import { Sheet } from '../ui/Sheet.tsx' import { Skeleton } from '../ui/Skeleton.tsx' import { Tag } from '../ui/Tag.tsx' import { TextInput } from '../ui/TextInput.tsx' +import { usePhotoBackfill, type BackfillResult, type PhotoBackfill } from './photoBackfill.ts' import styles from './RegularsSettingsPage.module.css' // The most minutes a dinner may take, as the dinners API allows: a day. @@ -57,7 +58,8 @@ const LINK = /^https?:\/\//i // method and minutes, favorite heart and whether its recipe was found. The // field at the top adds one by name, or by pasted recipe link with a name // the household gives it; tapping one -// edits or removes it, or fixes its recipe when none was found. +// edits or removes it, or fixes its recipe when none was found. Find +// missing photos searches again for every dinner left without one. export function RegularsSettingsPage() { const navigate = useNavigate() return ( @@ -86,6 +88,7 @@ function RegularsManager() { const remove = useRemoveRegular() const favorite = useToggleFavorite() const { refusals, track } = useRefusals() + const backfill = usePhotoBackfill() const [draft, setDraft] = useState('') const [linkName, setLinkName] = useState('') const [open, setOpen] = useState(null) @@ -94,8 +97,10 @@ function RegularsManager() { const isLink = LINK.test(draft.trim()) const ready = draft.trim() !== '' && (!isLink || linkName.trim() !== '') const list = regulars.data ?? [] - // A Regular removed meanwhile closes its sheet. + // A Regular removed meanwhile closes its sheet, and one whose recipe was + // found meanwhile closes its fix-recipe sheet. const opened = open && list.find((regular) => regular.id === open.id && !regular.pending) + const fixing = opened && open.sheet === 'fix' && opened.grounding_status === 'failed' ? opened : undefined function onSubmit(event: FormEvent) { event.preventDefault() @@ -188,6 +193,7 @@ function RegularsManager() { )} )} + {regulars.isSuccess && } {opened && open.sheet === 'edit' && ( onRemove(opened)} /> )} - {opened && open.sheet === 'fix' && ( + {fixing && ( setOpen(null)} - onEdit={() => setOpen({ sheet: 'edit', id: opened.id })} + onEdit={() => setOpen({ sheet: 'edit', id: fixing.id })} /> )} @@ -231,6 +238,88 @@ function GroundingBadge({ status }: { status: GroundingStatus }) { return {badge.text} } +// groundingReasons says why a dinner's recipe was not found, by its +// grounding_error reason code. +const groundingReasons: Record = { + no_recipe_found: 'No recipe page the search found had a recipe we could use.', + recipe_unusable: 'The pasted recipe link has no recipe we could use.', + search_not_configured: 'Recipe search is not set up. An admin can set it up in Settings.', + search_failed: "The recipe search couldn't be reached.", + ai_failed: "The AI assistant couldn't read the recipe.", + interrupted: 'The server stopped while finding the recipe.', + grounding_error: 'Something went wrong on the server.', +} + +// groundingReason says why a dinner's recipe was not found; an unknown +// reason code is shown as it is. +export function groundingReason(code: string): string { + return groundingReasons[code] ?? code +} + +// backfillJobFailures says why a photo backfill job failed, by its failure +// code; an unknown code is shown as it is. +const backfillJobFailures: Record = { + internal_error: 'Something went wrong on the server.', +} + +// backfillProblem says what went wrong with a photo backfill, or null. +function backfillProblem(backfill: PhotoBackfill): string | null { + if (backfill.startError) return `Photo search not started: ${backfill.startError.message}` + if (backfill.status === 'failed' && backfill.error) { + return `Photo search stopped: ${backfillJobFailures[backfill.error.message] ?? backfill.error.message}` + } + if (backfill.error) return `Couldn't check on the photo search: ${backfill.error.message}` + return null +} + +function count(n: number, one: string, many: string): string { + return `${n} ${n === 1 ? one : many}` +} + +// backfillSummary says what a finished photo backfill did. +function backfillSummary(result: BackfillResult): string { + const tally = (outcome: string) => result.dinners.filter((dinner) => dinner.outcome === outcome).length + if (result.dinners.length === 0) return 'No dinners were missing a photo.' + const parts = [count(tally('grounded'), 'photo found', 'photos found')] + const failed = tally('failed') + if (failed > 0) parts.push(`${count(failed, 'dinner still needs', 'dinners still need')} a recipe`) + const skipped = tally('skipped') + if (skipped > 0) parts.push(`${count(skipped, 'dinner', 'dinners')} changed meanwhile, left as it was`) + return `Photo search done: ${parts.join(' · ')}.` +} + +// FindMissingPhotos searches again, by name, for a recipe and photo for +// every dinner left without a photo because its recipe was not found. +function FindMissingPhotos({ backfill }: { backfill: PhotoBackfill }) { + const headingId = useId() + const problem = backfillProblem(backfill) + return ( +
+

+ Missing photos +

+ +

+ Search again for a recipe and photo for every dinner whose recipe wasn't found. Dinners with a pasted + recipe link keep it. +

+ +

+ {backfill.active && 'Finding missing photos… this can take a few minutes.'} + {backfill.status === 'succeeded' && backfill.result && backfillSummary(backfill.result)} +

+ {problem && ( +

+ {problem} +

+ )} + +
+ ) +} + type RegularRowProps = { regular: Regular onOpen: () => void @@ -447,6 +536,7 @@ const fixPaths = [ type FixRecipeSheetProps = { regular: Regular + backfill: PhotoBackfill onClose: () => void onEdit: () => void } @@ -459,10 +549,19 @@ function lines(text: string): string[] { .filter((line) => line !== '') } -// FixRecipeSheet grounds a Regular whose recipe was not found: in one of -// up to three recipe pages found for its name, in a pasted recipe link, or -// in ingredients and steps typed in. -function FixRecipeSheet({ regular, onClose, onEdit }: FixRecipeSheetProps) { +// retryOutcomes says what searching again did for this dinner. +const retryOutcomes = { + grounded: 'Searched again: a recipe and photo were found.', + failed: 'Searched again: still no recipe found.', + skipped: 'It changed while it was searched again, so it was left as it is.', +} as const + +// FixRecipeSheet shows why a Regular's recipe was not found and grounds it: +// by searching its name again (a photo backfill), in one of up to three +// recipe pages found for its name, in a pasted recipe link, or in +// ingredients and steps typed in. Searching again never replaces a pasted +// link, so a dinner whose link had no usable recipe is not offered it. +function FixRecipeSheet({ regular, backfill, onClose, onEdit }: FixRecipeSheetProps) { const [path, setPath] = useState('pick') const ground = useGroundRegular() @@ -475,6 +574,15 @@ function FixRecipeSheet({ regular, onClose, onEdit }: FixRecipeSheetProps) {

We couldn't find a recipe for {regular.name}. Pick a match below, paste a recipe link or type it in.

+ {regular.grounding_error !== null && ( +

+ Why: + {groundingReason(regular.grounding_error)} +

+ )} + {regular.grounding_error !== 'recipe_unusable' && ( + + )} dinner.dinner_id === id) + let status: string | null = null + if (backfill.active) status = 'Searching again… this can take a few minutes.' + else if (backfill.status === 'succeeded') { + status = tried ? retryOutcomes[tried.outcome] : 'Not searched again: it keeps its pasted recipe link.' + } + return ( + <> + +

+ {status} +

+ {problem && ( +

+ {problem} +

+ )} + + ) +} + function PickCandidate({ id, onPick }: { id: string; onPick: (candidateId: string) => void }) { const candidates = useRecipeCandidates(id) if (candidates.isPending) return diff --git a/web/src/settings/photoBackfill.test.tsx b/web/src/settings/photoBackfill.test.tsx new file mode 100644 index 0000000..e599084 --- /dev/null +++ b/web/src/settings/photoBackfill.test.tsx @@ -0,0 +1,63 @@ +import { QueryClientProvider } from '@tanstack/react-query' +import { act, renderHook, waitFor } from '@testing-library/react' +import type { ReactNode } from 'react' +import { afterEach, describe, expect, it, vi } from 'vitest' +import { ApiProvider, createApiClient, createQueryClient, createVerifiedSession } from '../api/index.ts' +import { HOME_KEY } from '../home/api.ts' +import { planKey } from '../plan/api.ts' +import { createNetworkStatus } from '../pwa/networkStatus.ts' +import { fakeLocation, jsonResponse, ME, stubFetch } from '../test/fetchStub.ts' +import { parseBackfillResult, usePhotoBackfill } from './photoBackfill.ts' + +afterEach(() => { + vi.unstubAllGlobals() +}) + +const RESULT = { dinners: [{ dinner_id: 'd3', outcome: 'grounded', reason: null }] } + +function backfillJob(status: string, result: unknown) { + return jsonResponse(200, { id: 'j9', kind: 'backfill_dinner_photos', status, result, error: null }) +} + +describe('usePhotoBackfill', () => { + it('reads the plans, home and dinners again once the backfill ends, so a found photo shows everywhere', async () => { + const fetchMock = stubFetch( + jsonResponse(200, ME), + jsonResponse(202, { id: 'j9' }), + backfillJob('running', null), + backfillJob('succeeded', RESULT), + ) + const api = createApiClient(fakeLocation(), createNetworkStatus(window), createVerifiedSession(localStorage)) + await api.loadMe() + const queryClient = createQueryClient() + const plan = planKey('2026-10-05') + for (const key of [plan, HOME_KEY, ['dinners', 'quest_log']]) queryClient.setQueryData(key, {}) + const wrapper = ({ children }: { children: ReactNode }) => ( + + {children} + + ) + const { result } = renderHook(() => usePhotoBackfill(), { wrapper }) + + act(() => result.current.start()) + await waitFor(() => expect(result.current.status).toBe('running')) + expect(result.current.active).toBe(true) + expect(queryClient.getQueryState(plan)?.isInvalidated).toBe(false) + + await waitFor(() => expect(result.current.status).toBe('succeeded'), { timeout: 3000 }) + expect(result.current).toMatchObject({ active: false, error: null, result: RESULT }) + expect(fetchMock).toHaveBeenCalledTimes(4) + for (const key of [plan, HOME_KEY, ['dinners', 'quest_log']]) { + expect(queryClient.getQueryState(key)?.isInvalidated).toBe(true) + } + }) +}) + +describe('parseBackfillResult', () => { + it('refuses a result that is not a photo backfill', () => { + expect(() => parseBackfillResult('job', { dinners: [{ dinner_id: 'd3', outcome: 'lost', reason: null }] })).toThrow( + 'job: outcome "lost" is not one of grounded, failed, skipped', + ) + expect(() => parseBackfillResult('job', { stage: 'x' })).toThrow('job: result is not a photo backfill') + }) +}) diff --git a/web/src/settings/photoBackfill.ts b/web/src/settings/photoBackfill.ts new file mode 100644 index 0000000..efbcbb6 --- /dev/null +++ b/web/src/settings/photoBackfill.ts @@ -0,0 +1,110 @@ +import { useMutation, useQueryClient } from '@tanstack/react-query' +import { useEffect, useMemo, useState } from 'react' +import { useApi, useJob, type JobStatus } from '../api/index.ts' +import { isRecord } from '../api/errors.ts' +import { oneOf } from '../dinners/api.ts' +import { HOME_KEY } from '../home/api.ts' + +export const PHOTO_BACKFILL_PATH = '/api/v1/settings/photo-backfill' + +// BACKFILL_JOB_KIND is the kind of the job a photo backfill runs as. +export const BACKFILL_JOB_KIND = 'backfill_dinner_photos' + +export const BACKFILL_OUTCOMES = ['grounded', 'failed', 'skipped'] as const +export type BackfillOutcome = (typeof BACKFILL_OUTCOMES)[number] + +// BackfillDinner is what a photo backfill did for one dinner. reason is +// set only when it failed: the reason code now its grounding_error. +export type BackfillDinner = { dinner_id: string; outcome: BackfillOutcome; reason: string | null } + +// BackfillResult is the photo backfill job's result: each dinner it tried, +// in the order it tried them. +export type BackfillResult = { dinners: BackfillDinner[] } + +// Every query showing a dinner's photo, read again once a backfill ends: +// the dinners lists and details, the plans and the home dashboard. +const PHOTO_KEYS = [['dinners'], ['plans'], HOME_KEY] as const + +// parseBackfillResult checks a photo backfill job's result, which useJob +// passes on unread. +export function parseBackfillResult(where: string, body: unknown): BackfillResult { + if (!isRecord(body) || !Array.isArray(body.dinners)) throw new Error(`${where}: result is not a photo backfill`) + return { + dinners: body.dinners.map((dinner): BackfillDinner => { + if (!isRecord(dinner) || typeof dinner.dinner_id !== 'string' || (dinner.reason !== null && typeof dinner.reason !== 'string')) { + throw new Error(`${where}: result has a dinner that is not a backfill outcome`) + } + return { + dinner_id: dinner.dinner_id, + outcome: oneOf(where, 'outcome', BACKFILL_OUTCOMES, dinner.outcome), + reason: dinner.reason, + } + }), + } +} + +// PhotoBackfill is a photo backfill started from this screen, as it goes. +export type PhotoBackfill = { + // start asks the server for a backfill over every dinner without a + // photo. While one is already queued or running the server answers with + // that job, so starting never runs two at once. + start: () => void + // active is true from start until the job ends, except while a poll's + // failure leaves its progress unknown: starting again then asks the + // server for the job it is running and polls it anew. + active: boolean + // Why the backfill could not be started. + startError: Error | null + // The job's status, undefined until it is started and first polled. + status: JobStatus | undefined + // The job's result, once it has one. + result: BackfillResult | null + // The job's failure, a poll's failure, or a result that is not a backfill. + error: Error | null +} + +// usePhotoBackfill starts a photo backfill (POST +// /api/v1/settings/photo-backfill, 202 {id}) and follows its job with +// useJob. When the job ends, every query showing a dinner's photo is read +// again, so dinners it grounded show their photo everywhere. +export function usePhotoBackfill(): PhotoBackfill { + const { apiFetch } = useApi() + const queryClient = useQueryClient() + const [jobId, setJobId] = useState(null) + const job = useJob(BACKFILL_JOB_KIND, jobId) + const begin = useMutation({ + mutationFn: async (): Promise => { + const body = await apiFetch(PHOTO_BACKFILL_PATH, { method: 'POST' }) + if (!isRecord(body) || typeof body.id !== 'string') throw new Error(`POST ${PHOTO_BACKFILL_PATH}: response has no job id`) + return body.id + }, + onSuccess: (id) => { + setJobId(id) + job.poll() + }, + }) + + const ended = job.status === 'succeeded' || job.status === 'failed' + useEffect(() => { + if (!ended) return + for (const queryKey of PHOTO_KEYS) void queryClient.invalidateQueries({ queryKey }) + }, [ended, jobId, queryClient]) + + const parsed = useMemo(() => { + if (job.result === null) return { result: null, error: null } + try { + return { result: parseBackfillResult(`GET /api/v1/jobs/${jobId}`, job.result), error: null } + } catch (error) { + return { result: null, error: error as Error } + } + }, [job.result, jobId]) + + return { + start: () => begin.mutate(), + active: begin.isPending || (jobId !== null && !ended && job.error === null), + startError: begin.error, + status: job.status, + result: parsed.result, + error: job.error ?? parsed.error, + } +} -- 2.55.0 From 6510ad12c0785300ada8ad140fac8b626fc666a0 Mon Sep 17 00:00:00 2001 From: dev_agent Date: Sun, 11 Oct 2026 22:21:06 -0400 Subject: [PATCH 2/2] Offer Search again only where the photo backfill runs (Maestro 7a468d2d, review F1/F2) F1: every dinner response now carries photo_backfill_eligible, computed by the exported dinners.PhotoBackfillEligible: the condition ListPhotoBackfillDinners lists by and GroundFailedDinner and RefailDinnerGrounding write under (unarchived, failed, no photo, no pasted recipe_url). A test holds the flag to ListPhotoBackfillDinners for an eligible dinner and for grounded, linked and archived ones. The fix-recipe sheet shows "Search again" only when the flag is set, so a dinner with a pasted link is never offered it, whatever its reason. F2: a dinner absent from the backfill result now reads "This dinner was not included in that search." instead of claiming a pasted link. Task: 7a468d2d-4a9b-45f2-b626-a7bed5f671df Co-Authored-By: Claude Opus 5.5 --- docs/ARCHITECTURE.md | 3 ++ internal/dinners/backfill.go | 11 +++++ internal/dinners/backfill_test.go | 47 +++++++++++++++++++ internal/dinners/dinners.go | 7 +++ web/src/dinners/DinnerDetailActions.test.tsx | 1 + web/src/dinners/api.ts | 10 +++- web/src/plan/PlanPage.test.tsx | 1 + .../settings/RegularsSettingsPage.test.tsx | 25 +++++++--- web/src/settings/RegularsSettingsPage.tsx | 9 ++-- web/src/test/dinners.ts | 2 + 10 files changed, 105 insertions(+), 11 deletions(-) diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 9ab4786..ac36359 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -276,6 +276,9 @@ automatic** (D-021). `grounded`, `failed` (with its reason) or `skipped`. A dinner whose write fails is `failed` with `grounding_error` and left as it was; the others still run, then the job fails with `internal_error`, keeping that result. + Every dinner response carries `photo_backfill_eligible`, whether the + backfill grounds that dinner, computed by the same condition it lists + dinners by, so a client offers a search only where one runs. `GET /api/v1/dinners/{id}/recipe-candidates` searches for the dinner's name and returns `{candidates: [{candidate_id, title, site_name, photo_url, minutes}]}`, at most 3, ranked as `Ground` diff --git a/internal/dinners/backfill.go b/internal/dinners/backfill.go index 0c06c00..ffde974 100644 --- a/internal/dinners/backfill.go +++ b/internal/dinners/backfill.go @@ -25,6 +25,17 @@ const ( BackfillSkipped = "skipped" ) +// PhotoBackfillEligible reports whether a photo backfill grounds r: the +// condition ListPhotoBackfillDinners lists dinners by, and GroundFailedDinner +// and RefailDinnerGrounding write under, for one dinner. The dinner is +// unarchived, failed, with no photo and no pasted recipe_url. +func PhotoBackfillEligible(r db.Dinner) bool { + return r.Status != StatusArchived && + r.GroundingStatus == GroundingFailed && + !r.PhotoID.Valid && + !r.RecipeUrl.Valid +} + // BackfillJob is the KindBackfillPhotos payload. DinnerID narrows the // backfill to that dinner; empty backfills every dinner without a photo. type BackfillJob struct { diff --git a/internal/dinners/backfill_test.go b/internal/dinners/backfill_test.go index 6d22243..c1ec585 100644 --- a/internal/dinners/backfill_test.go +++ b/internal/dinners/backfill_test.go @@ -192,6 +192,53 @@ func TestBackfillSkipsDinnersNotMissingAPhoto(t *testing.T) { } } +// TestPhotoBackfillEligibleIsWhatTheBackfillLists holds the API's +// photo_backfill_eligible to ListPhotoBackfillDinners for a dinner the +// backfill grounds and for each kind it never does. +func TestPhotoBackfillEligibleIsWhatTheBackfillLists(t *testing.T) { + t.Parallel() + e := newEnv(t) + e.model.QueueGenerate(recipestest.NormalizedReply("stovetop", 40)) + grounded, err := e.svc.CreateRegular(t.Context(), dinners.NewRegular{Name: ptr("Chicken curry")}, member) + if err != nil { + t.Fatal(err) + } + e.waitJob(t, *grounded.GroundingJobID) + linked, err := e.svc.CreateRegular(t.Context(), dinners.NewRegular{Name: ptr("Mystery pie"), RecipeURL: ptr("https://d.example/pie")}, member) + if err != nil { + t.Fatal(err) + } + e.waitJob(t, *linked.GroundingJobID) + failed := e.failedDinner(t, "Abalone surprise") + archived, err := e.svc.Archive(t.Context(), e.failedDinner(t, "Old stew"), member) + if err != nil { + t.Fatal(err) + } + + shown := map[string]dinners.Dinner{archived.ID: archived} + regulars, err := e.svc.List(t.Context(), dinners.StatusRegular) + if err != nil { + t.Fatal(err) + } + for _, d := range regulars { + shown[d.ID] = d + } + q := db.New(e.st.Reader) + for id, want := range map[string]bool{grounded.ID: false, linked.ID: false, failed: true, archived.ID: false} { + listed, err := q.ListPhotoBackfillDinners(t.Context(), sql.NullString{String: id, Valid: true}) + if err != nil { + t.Fatal(err) + } + d, ok := shown[id] + if !ok { + t.Fatalf("dinner %s not shown", id) + } + if got := len(listed) == 1; got != want || d.PhotoBackfillEligible != want { + t.Errorf("%s: listed %t, photo_backfill_eligible %t, want both %t", d.Name, got, d.PhotoBackfillEligible, want) + } + } +} + func TestBackfillOfOneDinnerGroundsOnlyIt(t *testing.T) { t.Parallel() e := newUnstartedEnv(t) diff --git a/internal/dinners/dinners.go b/internal/dinners/dinners.go index e2a57ef..38ed541 100644 --- a/internal/dinners/dinners.go +++ b/internal/dinners/dinners.go @@ -79,6 +79,11 @@ type Dinner struct { UpdatedBy *string `json:"updated_by"` CreatedAt string `json:"created_at"` UpdatedAt string `json:"updated_at"` + + // PhotoBackfillEligible is whether a photo backfill grounds the dinner + // (PhotoBackfillEligible): false for one with a pasted recipe link, + // which a search never replaces. + PhotoBackfillEligible bool `json:"photo_backfill_eligible"` } // NewRegular is a Regular to add: its Name, which the household always @@ -712,6 +717,8 @@ func fromRow(r db.Dinner) Dinner { UpdatedBy: nullString(r.UpdatedBy), CreatedAt: r.CreatedAt, UpdatedAt: r.UpdatedAt, + + PhotoBackfillEligible: PhotoBackfillEligible(r), } if r.MinutesTotal.Valid { d.MinutesTotal = &r.MinutesTotal.Int64 diff --git a/web/src/dinners/DinnerDetailActions.test.tsx b/web/src/dinners/DinnerDetailActions.test.tsx index e2e27c6..9e49828 100644 --- a/web/src/dinners/DinnerDetailActions.test.tsx +++ b/web/src/dinners/DinnerDetailActions.test.tsx @@ -53,6 +53,7 @@ function listed(is_favorite: boolean) { photo_url: null, grounding_status: 'grounded', grounding_error: null, + photo_backfill_eligible: false, } } diff --git a/web/src/dinners/api.ts b/web/src/dinners/api.ts index c01745d..00de235 100644 --- a/web/src/dinners/api.ts +++ b/web/src/dinners/api.ts @@ -22,7 +22,9 @@ export type GroundingStatus = (typeof GROUNDING_STATUSES)[number] // Regular is the part of a dinner the Regulars screens read. photo_id is // the stored photo's id, from photo_url. grounding_error is why its // grounding failed, a reason code, set only while grounding_status is -// failed. pending is true while an optimistic add waits for the server; +// failed. photo_backfill_eligible is whether a photo backfill (Search +// again) searches for it: never for a dinner with a pasted recipe link. +// pending is true while an optimistic add waits for the server; // its id is not the server's. export type Regular = { id: string @@ -34,6 +36,7 @@ export type Regular = { photo_id: string | undefined grounding_status: GroundingStatus grounding_error: string | null + photo_backfill_eligible: boolean pending?: true } @@ -116,6 +119,9 @@ export function parseDinner(where: string, body: unknown): Regular { throw new Error(`${where}: response is not a dinner`) } if (typeof body.is_favorite !== 'boolean') throw new Error(`${where}: is_favorite is not a boolean`) + if (typeof body.photo_backfill_eligible !== 'boolean') { + throw new Error(`${where}: photo_backfill_eligible is not a boolean`) + } return { id: body.id, name: body.name, @@ -129,6 +135,7 @@ export function parseDinner(where: string, body: unknown): Regular { if (typeof v !== 'string') throw new Error(`${where}: grounding_error is not a string`) return v }), + photo_backfill_eligible: body.photo_backfill_eligible, } } @@ -233,6 +240,7 @@ export function useAddRegular() { photo_id: undefined, grounding_status: 'pending', grounding_error: null, + photo_backfill_eligible: false, pending: true, } await changeOptimistically(queryClient, (list) => [...list, optimistic]) diff --git a/web/src/plan/PlanPage.test.tsx b/web/src/plan/PlanPage.test.tsx index 76d97d4..be09c8d 100644 --- a/web/src/plan/PlanPage.test.tsx +++ b/web/src/plan/PlanPage.test.tsx @@ -115,6 +115,7 @@ function dinner(id: string, name: string, difficulty: string | null, minutes: nu photo_url: null, grounding_status: 'grounded', grounding_error: null, + photo_backfill_eligible: false, } } diff --git a/web/src/settings/RegularsSettingsPage.test.tsx b/web/src/settings/RegularsSettingsPage.test.tsx index b92b132..216fb2a 100644 --- a/web/src/settings/RegularsSettingsPage.test.tsx +++ b/web/src/settings/RegularsSettingsPage.test.tsx @@ -85,7 +85,11 @@ const TACOS = dinnerBody('d1', 'Beef tacos', { photo_url: '/media/photos/p1-400.jpg', }) const CHILI = dinnerBody('d2', 'Chili', { method: 'crockpot', minutes_total: 480, grounding_status: 'pending' }) -const PIZZA = dinnerBody('d3', 'Pizza night', { grounding_status: 'failed', grounding_error: 'no_recipe_found' }) +const PIZZA = dinnerBody('d3', 'Pizza night', { + grounding_status: 'failed', + grounding_error: 'no_recipe_found', + photo_backfill_eligible: true, +}) const ROAST = dinnerBody('d4', 'Pot roast', { method: 'oven', grounding_status: 'manual' }) function rows() { @@ -528,7 +532,7 @@ describe('RegularsSettingsPage', () => { }) it('offers no search again for a pasted link that had no usable recipe', async () => { - const linked = { ...PIZZA, grounding_error: 'recipe_unusable' } + const linked = { ...PIZZA, grounding_error: 'recipe_unusable', photo_backfill_eligible: false } stubServer([linked], NO_CANDIDATES) await renderPage() @@ -537,6 +541,16 @@ describe('RegularsSettingsPage', () => { expect(fixSheet().queryByRole('button', { name: 'Search again' })).not.toBeInTheDocument() }) + it('offers no search again for a dinner with a pasted link whatever its reason', async () => { + const linked = { ...PIZZA, grounding_error: 'ai_failed', photo_backfill_eligible: false } + stubServer([linked], NO_CANDIDATES) + await renderPage() + + openRow('Pizza night') + expect(fixSheet().getByText("The AI assistant couldn't read the recipe.")).toBeInTheDocument() + expect(fixSheet().queryByRole('button', { name: 'Search again' })).not.toBeInTheDocument() + }) + it('searches again from the fix-recipe sheet and shows the new reason when it still fails', async () => { const server = stubServer([PIZZA], { ...NO_CANDIDATES, @@ -580,9 +594,8 @@ describe('RegularsSettingsPage', () => { expect(within(row('Pizza night')).getByRole('img', { name: 'Pizza night' })).toHaveAttribute('src', '/media/photos/p8-400.jpg') }) - it('says a dinner the search left out keeps its pasted recipe link', async () => { - const linked = { ...PIZZA, grounding_error: 'ai_failed' } - stubServer([linked], { + it('says when a dinner was not included in the search', async () => { + stubServer([PIZZA], { ...NO_CANDIDATES, [BACKFILL]: () => jsonResponse(202, { id: 'j9' }), [JOB]: () => backfillJob('succeeded', { dinners: [] }), @@ -591,7 +604,7 @@ describe('RegularsSettingsPage', () => { openRow('Pizza night') fireEvent.click(fixSheet().getByRole('button', { name: 'Search again' })) - expect(await fixSheet().findByText('Not searched again: it keeps its pasted recipe link.')).toBeInTheDocument() + expect(await fixSheet().findByText('This dinner was not included in that search.')).toBeInTheDocument() }) it('finds missing photos: the placeholder shows until the backfill finds a photo', async () => { diff --git a/web/src/settings/RegularsSettingsPage.tsx b/web/src/settings/RegularsSettingsPage.tsx index d700d22..bd0153c 100644 --- a/web/src/settings/RegularsSettingsPage.tsx +++ b/web/src/settings/RegularsSettingsPage.tsx @@ -559,8 +559,9 @@ const retryOutcomes = { // FixRecipeSheet shows why a Regular's recipe was not found and grounds it: // by searching its name again (a photo backfill), in one of up to three // recipe pages found for its name, in a pasted recipe link, or in -// ingredients and steps typed in. Searching again never replaces a pasted -// link, so a dinner whose link had no usable recipe is not offered it. +// ingredients and steps typed in. Searching again is offered only for a +// dinner the backfill searches (photo_backfill_eligible): it never replaces +// a pasted link. function FixRecipeSheet({ regular, backfill, onClose, onEdit }: FixRecipeSheetProps) { const [path, setPath] = useState('pick') const ground = useGroundRegular() @@ -580,7 +581,7 @@ function FixRecipeSheet({ regular, backfill, onClose, onEdit }: FixRecipeSheetPr {groundingReason(regular.grounding_error)}

)} - {regular.grounding_error !== 'recipe_unusable' && ( + {regular.photo_backfill_eligible && ( )} diff --git a/web/src/test/dinners.ts b/web/src/test/dinners.ts index f316391..1407ac3 100644 --- a/web/src/test/dinners.ts +++ b/web/src/test/dinners.ts @@ -13,6 +13,7 @@ export type DinnerBody = { photo_url: string | null grounding_status: string grounding_error: string | null + photo_backfill_eligible: boolean updated_by: string | null created_at: string updated_at: string @@ -35,6 +36,7 @@ export function dinnerBody(id: string, name: string, overrides: Partial