From 6510ad12c0785300ada8ad140fac8b626fc666a0 Mon Sep 17 00:00:00 2001 From: dev_agent Date: Sun, 11 Oct 2026 22:21:06 -0400 Subject: [PATCH] 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