diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 0472353..33f12dc 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -259,6 +259,29 @@ automatic** (D-021). unarchived dinner still `pending` with no `ground_dinner` job for its revision queued or running is set `failed` with `interrupted`, so no dinner stays pending with nothing left to ground it. + Photo backfill. `POST /api/v1/settings/photo-backfill` (CSRF-checked) + enqueues a `backfill_dinner_photos` job and answers 202 `{id}` without + waiting for it; while one is queued or running it enqueues nothing and + answers with that job's id, so two passes never overlap, across restarts + too. The kind is refused by `POST /api/v1/jobs`. The job + grounds, one at a time with `Ground` on its name, every unarchived dinner + that is `failed` with no photo and no pasted `recipe_url` — grounded, + `manual` and `pending` dinners are never listed, and a pasted link is + never replaced by a search. Success sets grounding_status `grounded` as + the `ground_dinner` job does (the recipe keeps its source URL and site); + failure leaves the dinner `failed` with the new `grounding_error`, for + the fix-recipe flow. One dinner's failure never stops the others; a + dinner renamed, archived or grounded by hand meanwhile is left as it is. + Its result is `{dinners: [{dinner_id, outcome, reason}]}`, outcome + `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. + The responses returning a `dinners.Dinner` representation (the list; the + create, update, archive, favorite and grounding responses; the idea + responses) carry `photo_backfill_eligible`, and + `GET /api/v1/dinners/{id}` does not; it says 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/db/dinners.sql.go b/internal/db/dinners.sql.go index 986c0aa..be82f27 100644 --- a/internal/db/dinners.sql.go +++ b/internal/db/dinners.sql.go @@ -214,6 +214,26 @@ func (q *Queries) FailPendingDinnerGrounding(ctx context.Context, arg FailPendin return result.RowsAffected() } +const getActivePhotoBackfillJob = `-- name: GetActivePhotoBackfillJob :one +SELECT id +FROM jobs +WHERE kind = ?1 + AND status IN ('queued', 'running') + AND json_extract(payload, '$.dinner_id') IS NULL +ORDER BY id +LIMIT 1 +` + +// The id of the full photo backfill job (no dinner_id) of kind queued or +// running. StartPhotoBackfill enqueues one only when there is none, so +// there is at most one. Returns sql.ErrNoRows when there is none. +func (q *Queries) GetActivePhotoBackfillJob(ctx context.Context, kind string) (string, error) { + row := q.db.QueryRowContext(ctx, getActivePhotoBackfillJob, kind) + var id string + err := row.Scan(&id) + return id, err +} + const getDinner = `-- name: GetDinner :one SELECT id, name, normalized_name, status, origin, method, minutes_total, difficulty, cuisine, is_favorite, recipe_id, photo_id, grounding_status, updated_by, created_at, updated_at, recipe_url, grounding_error, grounding_revision FROM dinners WHERE id = ? @@ -344,6 +364,58 @@ func (q *Queries) GroundDinner(ctx context.Context, arg GroundDinnerParams) (Din return i, err } +const groundFailedDinner = `-- name: GroundFailedDinner :execrows +UPDATE dinners +SET recipe_id = ?1, + photo_id = ?2, + method = COALESCE(method, ?3), + minutes_total = COALESCE(minutes_total, ?4), + difficulty = COALESCE(difficulty, ?5), + grounding_status = 'grounded', + grounding_error = NULL, + updated_at = ?6 +WHERE id = ?7 + AND status <> 'archived' + AND grounding_status = 'failed' + AND photo_id IS NULL + AND recipe_url IS NULL + AND grounding_revision = ?8 +` + +type GroundFailedDinnerParams struct { + RecipeID sql.NullString + PhotoID sql.NullString + Method sql.NullString + MinutesTotal sql.NullInt64 + Difficulty sql.NullString + UpdatedAt string + ID string + GroundingRevision int64 +} + +// Records a photo backfill's grounding of a dinner it listed, while the +// dinner is still unarchived, failed, photoless, without a recipe_url and at +// the grounding_revision it was listed at; a dinner grounded by hand, +// renamed or archived meanwhile is left as it is (0 rows). Like +// GroundPendingDinner it keeps a method, minutes_total or difficulty a +// member already set. +func (q *Queries) GroundFailedDinner(ctx context.Context, arg GroundFailedDinnerParams) (int64, error) { + result, err := q.db.ExecContext(ctx, groundFailedDinner, + arg.RecipeID, + arg.PhotoID, + arg.Method, + arg.MinutesTotal, + arg.Difficulty, + arg.UpdatedAt, + arg.ID, + arg.GroundingRevision, + ) + if err != nil { + return 0, err + } + return result.RowsAffected() +} + const groundPendingDinner = `-- name: GroundPendingDinner :execrows UPDATE dinners SET recipe_id = ?1, @@ -444,6 +516,64 @@ func (q *Queries) ListDinnersByStatus(ctx context.Context, status string) ([]Din return items, nil } +const listPhotoBackfillDinners = `-- name: ListPhotoBackfillDinners :many +SELECT id, name, normalized_name, status, origin, method, minutes_total, difficulty, cuisine, is_favorite, recipe_id, photo_id, grounding_status, updated_by, created_at, updated_at, recipe_url, grounding_error, grounding_revision FROM dinners +WHERE status <> 'archived' + AND grounding_status = 'failed' + AND photo_id IS NULL + AND recipe_url IS NULL + AND id = COALESCE(?1, id) +ORDER BY normalized_name, id +` + +// The dinners a photo backfill grounds by name: unarchived, failed, with no +// photo and no pasted recipe_url (the household's own link is never +// replaced by a search), ordered by normalized name. A non-NULL id narrows +// the list to that dinner. Grounded, manual and pending dinners are never +// listed: they have a recipe, or a ground_dinner job grounding them. +func (q *Queries) ListPhotoBackfillDinners(ctx context.Context, id sql.NullString) ([]Dinner, error) { + rows, err := q.db.QueryContext(ctx, listPhotoBackfillDinners, id) + if err != nil { + return nil, err + } + defer rows.Close() + var items []Dinner + for rows.Next() { + var i Dinner + if err := rows.Scan( + &i.ID, + &i.Name, + &i.NormalizedName, + &i.Status, + &i.Origin, + &i.Method, + &i.MinutesTotal, + &i.Difficulty, + &i.Cuisine, + &i.IsFavorite, + &i.RecipeID, + &i.PhotoID, + &i.GroundingStatus, + &i.UpdatedBy, + &i.CreatedAt, + &i.UpdatedAt, + &i.RecipeUrl, + &i.GroundingError, + &i.GroundingRevision, + ); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Close(); err != nil { + return nil, err + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + const listUnarchivedDinners = `-- name: ListUnarchivedDinners :many SELECT dinners.id, dinners.name, dinners.normalized_name, dinners.status, dinners.origin, dinners.method, dinners.minutes_total, dinners.difficulty, dinners.cuisine, dinners.is_favorite, dinners.recipe_id, dinners.photo_id, dinners.grounding_status, dinners.updated_by, dinners.created_at, dinners.updated_at, dinners.recipe_url, dinners.grounding_error, dinners.grounding_revision, recipes.source_url AS recipe_source_url FROM dinners @@ -560,6 +690,40 @@ func (q *Queries) ListUnplannedQuestLogDinners(ctx context.Context, fromWeekStar return items, nil } +const refailDinnerGrounding = `-- name: RefailDinnerGrounding :execrows +UPDATE dinners +SET grounding_error = ?1, + updated_at = ?2 +WHERE id = ?3 + AND status <> 'archived' + AND grounding_status = 'failed' + AND photo_id IS NULL + AND recipe_url IS NULL + AND grounding_revision = ?4 +` + +type RefailDinnerGroundingParams struct { + GroundingError sql.NullString + UpdatedAt string + ID string + GroundingRevision int64 +} + +// Records why a photo backfill could not ground a dinner it listed, under +// the same conditions as GroundFailedDinner (0 rows otherwise). +func (q *Queries) RefailDinnerGrounding(ctx context.Context, arg RefailDinnerGroundingParams) (int64, error) { + result, err := q.db.ExecContext(ctx, refailDinnerGrounding, + arg.GroundingError, + arg.UpdatedAt, + arg.ID, + arg.GroundingRevision, + ) + if err != nil { + return 0, err + } + return result.RowsAffected() +} + const setDinnerFavorite = `-- name: SetDinnerFavorite :one UPDATE dinners SET is_favorite = ?, diff --git a/internal/db/querier.go b/internal/db/querier.go index d1623b1..e19b54e 100644 --- a/internal/db/querier.go +++ b/internal/db/querier.go @@ -117,6 +117,10 @@ type Querier interface { // when there is none, so there is at most one. Returns sql.ErrNoRows when // there is none. GetActiveDraftJob(ctx context.Context, arg GetActiveDraftJobParams) (GetActiveDraftJobRow, error) + // The id of the full photo backfill job (no dinner_id) of kind queued or + // running. StartPhotoBackfill enqueues one only when there is none, so + // there is at most one. Returns sql.ErrNoRows when there is none. + GetActivePhotoBackfillJob(ctx context.Context, kind string) (string, error) // The batch still pending or running, oldest first. Returns sql.ErrNoRows // when there is none. GetActiveSuggestionBatch(ctx context.Context) (SuggestionBatch, error) @@ -175,6 +179,13 @@ type Querier interface { // unknown id, returns sql.ErrNoRows. A method, minutes_total or difficulty // no longer as observed is kept: a member edited it while it was grounding. GroundDinner(ctx context.Context, arg GroundDinnerParams) (Dinner, error) + // Records a photo backfill's grounding of a dinner it listed, while the + // dinner is still unarchived, failed, photoless, without a recipe_url and at + // the grounding_revision it was listed at; a dinner grounded by hand, + // renamed or archived meanwhile is left as it is (0 rows). Like + // GroundPendingDinner it keeps a method, minutes_total or difficulty a + // member already set. + GroundFailedDinner(ctx context.Context, arg GroundFailedDinnerParams) (int64, error) // Records the background grounding of an unarchived dinner still pending at // the grounding_revision it was grounded for; a dinner grounded some other // way, renamed or archived meanwhile is left as it is (0 rows). A method, @@ -206,6 +217,12 @@ type Querier interface { // The pending Rotation checks whose dinner is still tried, oldest plan // date first, with their dinners. ListPendingRotationReviews(ctx context.Context) ([]ListPendingRotationReviewsRow, error) + // The dinners a photo backfill grounds by name: unarchived, failed, with no + // photo and no pasted recipe_url (the household's own link is never + // replaced by a search), ordered by normalized name. A non-NULL id narrows + // the list to that dinner. Grounded, manual and pending dinners are never + // listed: they have a recipe, or a ground_dinner job grounding them. + ListPhotoBackfillDinners(ctx context.Context, id sql.NullString) ([]Dinner, error) // The days of plan plan_id with their dinners, Monday first. A day with // no dinner is left out. ListPlanDays(ctx context.Context, planID string) ([]ListPlanDaysRow, error) @@ -307,6 +324,9 @@ type Querier interface { // Records a new draft over plan id: its status and note, raising its // revision. RedraftWeekPlan(ctx context.Context, arg RedraftWeekPlanParams) error + // Records why a photo backfill could not ground a dinner it listed, under + // the same conditions as GroundFailedDinner (0 rows otherwise). + RefailDinnerGrounding(ctx context.Context, arg RefailDinnerGroundingParams) (int64, error) // Records a generation of list id from plan plan_id. RegenerateShoppingList(ctx context.Context, arg RegenerateShoppingListParams) error // Records a failed delivery and when to try again. The lease must still be diff --git a/internal/dinners/backfill.go b/internal/dinners/backfill.go new file mode 100644 index 0000000..ffde974 --- /dev/null +++ b/internal/dinners/backfill.go @@ -0,0 +1,199 @@ +package dinners + +import ( + "context" + "database/sql" + "encoding/json" + "errors" + "fmt" + + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/db" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/store" +) + +// KindBackfillPhotos is the job that grounds, by name, the dinners left +// without a photo: those whose grounding failed and that have no pasted +// recipe_url (ListPhotoBackfillDinners). +const KindBackfillPhotos = "backfill_dinner_photos" + +// Photo backfill outcomes for one dinner. +const ( + BackfillGrounded = "grounded" + BackfillFailed = "failed" + // BackfillSkipped: the dinner was grounded by hand, renamed or archived + // while the backfill grounded it, so the backfill left it as it is. + 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 { + DinnerID string `json:"dinner_id,omitempty"` +} + +// BackfillOutcome is what a photo backfill did for one dinner. Reason is +// set only for BackfillFailed: the Reason recorded as its grounding_error. +type BackfillOutcome struct { + DinnerID string `json:"dinner_id"` + Outcome string `json:"outcome"` + Reason *string `json:"reason"` +} + +// backfillResult is the KindBackfillPhotos result: the outcome of each +// dinner the backfill tried, in the order it tried them. +type backfillResult struct { + Dinners []BackfillOutcome `json:"dinners"` +} + +func (r backfillResult) PublicJSON() ([]byte, error) { return json.Marshal(r) } + +// StartPhotoBackfill enqueues a KindBackfillPhotos job over every dinner +// without a photo and returns its id, without waiting for it. While a full +// backfill is queued or running it enqueues nothing and returns that job's +// id instead, so two passes never ground the same dinner at once; the jobs +// table holds the claim, so it holds across restarts too. +func (s *Service) StartPhotoBackfill(ctx context.Context) (string, error) { + var id string + created := false + err := s.st.WithTx(ctx, func(tx *sql.Tx) error { + active, err := db.New(tx).GetActivePhotoBackfillJob(ctx, KindBackfillPhotos) + switch { + case errors.Is(err, sql.ErrNoRows): + case err != nil: + return fmt.Errorf("dinners: read active photo backfill: %w", err) + default: + id = active + return nil + } + id, err = s.backfillJob.EnqueueTx(ctx, tx, BackfillJob{}) + if err != nil { + return fmt.Errorf("dinners: enqueue photo backfill: %w", err) + } + created = true + return nil + }) + if err != nil { + return "", err + } + if created { + s.backfillJob.Wake() + } + return id, nil +} + +// errBackfillNotRecorded fails a photo backfill that could not record the +// outcome of one or more of its dinners. +var errBackfillNotRecorded = errors.New("dinners: photo backfill: dinner outcomes not recorded") + +// runBackfillJob grounds each dinner ListPhotoBackfillDinners lists for p, +// one at a time, with recipes.Grounder.Ground on its name: the first ranked +// candidate whose photo caches and whose ingredients normalize grounds it, +// with the recipe's source URL and site kept as any grounding keeps them. +// Success records the recipe and photo with grounding_status grounded; +// failure leaves the dinner failed with the new Reason as its +// grounding_error, for the household to fix its recipe by hand. One +// dinner's failure, its search's, model's or database write's, never stops +// the others. A failure to list the dinners, or the job's context ending, +// fails the job at once. A write that failed fails the job once every +// dinner has been tried, as a grounding job's failed write does, with the +// outcomes published as its result, so the job never reports done for a +// dinner whose outcome is not stored. +func (s *Service) runBackfillJob(ctx context.Context, p BackfillJob, publish func(backfillResult) error) (backfillResult, error) { + q := db.New(s.st.Reader) + rows, err := q.ListPhotoBackfillDinners(ctx, sql.NullString{String: p.DinnerID, Valid: p.DinnerID != ""}) + if err != nil { + return backfillResult{}, fmt.Errorf("dinners: photo backfill: list: %w", err) + } + res := backfillResult{Dinners: make([]BackfillOutcome, 0, len(rows))} + grounded, unrecorded := false, false + for _, row := range rows { + out, recorded, err := s.backfillDinner(ctx, row) + if err != nil { + return backfillResult{}, err + } + grounded = grounded || out.Outcome == BackfillGrounded + unrecorded = unrecorded || !recorded + res.Dinners = append(res.Dinners, out) + } + if grounded { + s.wake() + } + if unrecorded { + if err := publish(res); err != nil { + return backfillResult{}, errors.Join(errBackfillNotRecorded, err) + } + return backfillResult{}, errBackfillNotRecorded + } + return res, nil +} + +// backfillDinner grounds row, a dinner ListPhotoBackfillDinners listed, as +// runBackfillJob describes, and reports whether its outcome was recorded. +// Its error is only the job's context ending. +func (s *Service) backfillDinner(ctx context.Context, row db.Dinner) (BackfillOutcome, bool, error) { + out := BackfillOutcome{DinnerID: row.ID} + w := db.New(s.st.Writer) + gr, err := s.grounder.Ground(ctx, row.Name, row.Name) + if ctxErr := ctx.Err(); ctxErr != nil { + return BackfillOutcome{}, false, ctxErr + } + if err != nil { + reason := reasonFor(err, false) + // Only the reason is logged: the error may quote a dependency's + // response, which jobs never store, return or log. + s.logger.InfoContext(ctx, "dinner photo backfill failed", "dinner_id", row.ID, "reason", reason) + n, err := w.RefailDinnerGrounding(ctx, db.RefailDinnerGroundingParams{ + GroundingError: sql.NullString{String: reason, Valid: true}, + UpdatedAt: store.Now().String(), + ID: row.ID, + GroundingRevision: row.GroundingRevision, + }) + out, recorded := s.backfillRecorded(ctx, out, BackfillFailed, reason, n, err) + return out, recorded, nil + } + n, err := w.GroundFailedDinner(ctx, db.GroundFailedDinnerParams{ + RecipeID: sql.NullString{String: gr.RecipeID, Valid: true}, + PhotoID: sql.NullString{String: gr.PhotoID, Valid: true}, + Method: sql.NullString{String: gr.Method, Valid: true}, + MinutesTotal: nullInt(gr.MinutesTotal), + Difficulty: sql.NullString{String: gr.Difficulty, Valid: true}, + UpdatedAt: store.Now().String(), + ID: row.ID, + GroundingRevision: row.GroundingRevision, + }) + out, recorded := s.backfillRecorded(ctx, out, BackfillGrounded, "", n, err) + return out, recorded, nil +} + +// backfillRecorded is out once the backfill recorded outcome (with reason, +// for BackfillFailed) on the dinner, writing n rows with err: a dinner that +// changed meanwhile (no row) is BackfillSkipped, and one whose write failed +// is BackfillFailed with ReasonGroundingError, the dinner left as it was, +// and not recorded. +func (s *Service) backfillRecorded(ctx context.Context, out BackfillOutcome, outcome, reason string, n int64, err error) (BackfillOutcome, bool) { + switch { + case err != nil: + // The error is a database error, never a dependency's text. + s.logger.ErrorContext(ctx, "dinner photo backfill not recorded", "dinner_id", out.DinnerID, "error", err) + out.Outcome, reason = BackfillFailed, ReasonGroundingError + case n == 0: + out.Outcome, reason = BackfillSkipped, "" + default: + out.Outcome = outcome + } + if reason != "" { + out.Reason = &reason + } + return out, err == nil +} diff --git a/internal/dinners/backfill_test.go b/internal/dinners/backfill_test.go new file mode 100644 index 0000000..c1ec585 --- /dev/null +++ b/internal/dinners/backfill_test.go @@ -0,0 +1,425 @@ +package dinners_test + +import ( + "database/sql" + "encoding/json" + "errors" + "slices" + "sync" + "testing" + "time" + + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/ai/harness" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/db" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/dinners" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/jobs" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/recipes" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/recipes/recipestest" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/store" +) + +// failedDinner stores a Regular named name whose grounding failed with +// no_recipe_found, without a photo or recipe_url, and returns its id. +func (e *env) failedDinner(t *testing.T, name string) string { + t.Helper() + q := db.New(e.st.Writer) + now := store.Now().String() + row, err := q.CreateDinner(t.Context(), db.CreateDinnerParams{ + ID: store.NewID(), + Name: name, + NormalizedName: harness.Normalize(name), + Status: dinners.StatusRegular, + Origin: "regular", + GroundingStatus: dinners.GroundingPending, + UpdatedBy: sql.NullString{String: member, Valid: true}, + CreatedAt: now, + UpdatedAt: now, + }) + if err != nil { + t.Fatal(err) + } + if _, err := q.FailPendingDinnerGrounding(t.Context(), db.FailPendingDinnerGroundingParams{ + GroundingError: sql.NullString{String: dinners.ReasonNoRecipe, Valid: true}, + UpdatedAt: now, + ID: row.ID, + GroundingRevision: row.GroundingRevision, + }); err != nil { + t.Fatal(err) + } + return row.ID +} + +func outcome(id, out string, reason *string) dinners.BackfillOutcome { + return dinners.BackfillOutcome{DinnerID: id, Outcome: out, Reason: reason} +} + +func checkOutcomes(t *testing.T, got, want []dinners.BackfillOutcome) { + t.Helper() + gj, _ := json.Marshal(got) + wj, _ := json.Marshal(want) + if string(gj) != string(wj) { + t.Fatalf("outcomes = %s, want %s", gj, wj) + } +} + +func TestBackfillGroundsFirstCandidateWithAPhoto(t *testing.T) { + t.Parallel() + e := newUnstartedEnv(t) + // curryA ranks first but its photo is gone; curryB's photo caches. + delete(e.images.JPEGs, "https://a.example/curry.jpg") + e.model.QueueGenerate(recipestest.NormalizedReply("stovetop", 25)) + id := e.failedDinner(t, "Chicken curry") + + got, err := e.svc.RunBackfillJob(t.Context(), dinners.BackfillJob{}) + if err != nil { + t.Fatal(err) + } + checkOutcomes(t, got, []dinners.BackfillOutcome{outcome(id, dinners.BackfillGrounded, nil)}) + + row := e.dinner(t, id) + if row.GroundingStatus != dinners.GroundingGrounded || !row.PhotoID.Valid || !row.RecipeID.Valid || row.GroundingError.Valid { + t.Fatalf("dinner = %+v, want grounded with a photo and no error", row) + } + rec, err := db.New(e.st.Reader).GetRecipe(t.Context(), row.RecipeID.String) + if err != nil { + t.Fatal(err) + } + if rec.SourceUrl != curryB || rec.SiteName.String != "B Kitchen" || rec.PhotoID != row.PhotoID { + t.Errorf("recipe = %s from %q photo %s, want %s from B Kitchen photo %s", rec.SourceUrl, rec.SiteName.String, rec.PhotoID.String, curryB, row.PhotoID.String) + } + if got := e.search.Queries(); !slices.Equal(got, []string{"Chicken curry"}) { + t.Errorf("searched %q, want the dinner's name", got) + } +} + +func TestBackfillReusesCachedRecipeAndPhoto(t *testing.T) { + t.Parallel() + e := newUnstartedEnv(t) + e.search.Results["Curry chicken"] = []recipes.Candidate{{URL: curryA}} + // One reply: the second dinner must reuse the recipe the first stored. + e.model.QueueGenerate(recipestest.NormalizedReply("stovetop", 40)) + first := e.failedDinner(t, "Chicken curry") + second := e.failedDinner(t, "Curry chicken") + + got, err := e.svc.RunBackfillJob(t.Context(), dinners.BackfillJob{}) + if err != nil { + t.Fatal(err) + } + checkOutcomes(t, got, []dinners.BackfillOutcome{ + outcome(first, dinners.BackfillGrounded, nil), + outcome(second, dinners.BackfillGrounded, nil), + }) + a, b := e.dinner(t, first), e.dinner(t, second) + if a.RecipeID != b.RecipeID || a.PhotoID != b.PhotoID || !a.PhotoID.Valid { + t.Errorf("dinners grounded in recipe %s photo %s and recipe %s photo %s, want the same cached ones", + a.RecipeID.String, a.PhotoID.String, b.RecipeID.String, b.PhotoID.String) + } + if n := len(e.model.Requests()); n != 1 { + t.Errorf("normalize_recipe ran %d times, want 1", n) + } + if n := slices.Index(e.pages.Fetched(), curryA); n < 0 || slices.Contains(e.pages.Fetched()[n+1:], curryA) { + t.Errorf("fetched %q, want %s fetched once", e.pages.Fetched(), curryA) + } +} + +func TestBackfillIsolatesEachDinnersFailure(t *testing.T) { + t.Parallel() + e := newUnstartedEnv(t) + e.search.Fail = map[string]error{"Broken search": errors.New("search provider down")} + e.model.QueueGenerate(recipestest.NormalizedReply("stovetop", 40)) + // Listed by normalized name: both failures come before the curry. + broken := e.failedDinner(t, "Broken search") + curry := e.failedDinner(t, "Chicken curry") + absent := e.failedDinner(t, "Abalone surprise") + + got, err := e.svc.RunBackfillJob(t.Context(), dinners.BackfillJob{}) + if err != nil { + t.Fatal(err) + } + checkOutcomes(t, got, []dinners.BackfillOutcome{ + outcome(absent, dinners.BackfillFailed, ptr(dinners.ReasonNoRecipe)), + outcome(broken, dinners.BackfillFailed, ptr(dinners.ReasonSearchFailed)), + outcome(curry, dinners.BackfillGrounded, nil), + }) + for id, reason := range map[string]string{absent: dinners.ReasonNoRecipe, broken: dinners.ReasonSearchFailed} { + row := e.dinner(t, id) + if row.GroundingStatus != dinners.GroundingFailed || row.GroundingError.String != reason || row.PhotoID.Valid { + t.Errorf("dinner %s = %+v, want failed with %s and no photo", row.Name, row, reason) + } + } + if row := e.dinner(t, curry); row.GroundingStatus != dinners.GroundingGrounded || !row.PhotoID.Valid { + t.Errorf("curry = %+v, want grounded with a photo", row) + } +} + +func TestBackfillSkipsDinnersNotMissingAPhoto(t *testing.T) { + t.Parallel() + e := newEnv(t) + e.model.QueueGenerate(recipestest.NormalizedReply("stovetop", 40)) + c, err := e.svc.CreateRegular(t.Context(), dinners.NewRegular{Name: ptr("Chicken curry")}, member) + if err != nil { + t.Fatal(err) + } + e.waitJob(t, *c.GroundingJobID) + if row := e.dinner(t, c.ID); row.GroundingStatus != dinners.GroundingGrounded { + t.Fatalf("dinner = %+v, want grounded", row) + } + // A failed dinner with a pasted link keeps that link for a fix by hand. + 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) + if row := e.dinner(t, linked.ID); row.GroundingStatus != dinners.GroundingFailed { + t.Fatalf("linked dinner = %+v, want failed", row) + } + archived := e.failedDinner(t, "Old stew") + if _, err := e.svc.Archive(t.Context(), archived, member); err != nil { + t.Fatal(err) + } + searched := len(e.search.Queries()) + + id, err := e.svc.StartPhotoBackfill(t.Context()) + if err != nil { + t.Fatal(err) + } + job := e.waitJob(t, id) + if job.Kind != dinners.KindBackfillPhotos || string(job.Result) != `{"dinners":[]}` { + t.Fatalf("job = %+v (result %s), want a backfill with no dinners", job, job.Result) + } + if got := e.search.Queries()[searched:]; len(got) != 0 { + t.Errorf("backfill searched %q, want nothing", got) + } +} + +// 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) + e.model.QueueGenerate(recipestest.NormalizedReply("stovetop", 40)) + curry := e.failedDinner(t, "Chicken curry") + other := e.failedDinner(t, "Abalone surprise") + + got, err := e.svc.RunBackfillJob(t.Context(), dinners.BackfillJob{DinnerID: curry}) + if err != nil { + t.Fatal(err) + } + checkOutcomes(t, got, []dinners.BackfillOutcome{outcome(curry, dinners.BackfillGrounded, nil)}) + if row := e.dinner(t, other); row.GroundingStatus != dinners.GroundingFailed || row.PhotoID.Valid { + t.Errorf("other dinner = %+v, want left failed", row) + } + if got := e.search.Queries(); !slices.Equal(got, []string{"Chicken curry"}) { + t.Errorf("searched %q, want only the dinner's name", got) + } +} + +func TestBackfillLeavesDinnerChangedWhileGrounding(t *testing.T) { + t.Parallel() + e := newUnstartedEnv(t) + id := e.failedDinner(t, "Chicken curry") + gate := make(chan struct{}) + e.search.Gate = gate + e.model.QueueGenerate(recipestest.NormalizedReply("stovetop", 40)) + + done := make(chan []dinners.BackfillOutcome) + go func() { + got, err := e.svc.RunBackfillJob(t.Context(), dinners.BackfillJob{DinnerID: id}) + if err != nil { + t.Error(err) + } + done <- got + }() + // The backfill listed the dinner and waits at its search. + for len(e.search.Queries()) == 0 { + time.Sleep(time.Millisecond) + } + if _, err := e.svc.Update(t.Context(), id, dinners.Patch{Name: ptr("Chicken korma")}, member); err != nil { + t.Fatal(err) + } + close(gate) + checkOutcomes(t, <-done, []dinners.BackfillOutcome{outcome(id, dinners.BackfillSkipped, nil)}) + if row := e.dinner(t, id); row.GroundingStatus != dinners.GroundingFailed || row.PhotoID.Valid { + t.Errorf("dinner = %+v, want left failed without a photo", row) + } +} + +// triggerAtOnce calls svc.StartPhotoBackfill from n goroutines released +// together and returns the job ids they got. +func triggerAtOnce(t *testing.T, svc *dinners.Service, n int) []string { + t.Helper() + ids := make([]string, n) + errs := make([]error, n) + release := make(chan struct{}) + var wg sync.WaitGroup + for i := range n { + wg.Add(1) + go func() { + defer wg.Done() + <-release + ids[i], errs[i] = svc.StartPhotoBackfill(t.Context()) + }() + } + close(release) + wg.Wait() + if err := errors.Join(errs...); err != nil { + t.Fatal(err) + } + return ids +} + +// backfillJobs counts the KindBackfillPhotos jobs in st. +func backfillJobs(t *testing.T, st *store.Store) int { + t.Helper() + var n int + if err := st.Reader.QueryRowContext(t.Context(), `SELECT count(*) FROM jobs WHERE kind = ?`, dinners.KindBackfillPhotos).Scan(&n); err != nil { + t.Fatal(err) + } + return n +} + +func TestStartPhotoBackfillIsSingleFlight(t *testing.T) { + t.Parallel() + const triggers = 8 + dataDir := t.TempDir() + st := openStore(t, dataDir) + e := newUnstartedEnvOn(t, st, dataDir) + e.failedDinner(t, "Chicken curry") + gate := make(chan struct{}) + e.search.Gate = gate + + // Queued: simultaneous triggers all get the one pass they enqueued. + ids := triggerAtOnce(t, e.svc, triggers) + first := ids[0] + for _, id := range ids { + if id != first { + t.Fatalf("simultaneous triggers = %q, want one job id", ids) + } + } + if n := backfillJobs(t, st); n != 1 { + t.Fatalf("%d backfill jobs after simultaneous triggers, want 1", n) + } + + // After a restart the queued pass is still the one: the claim is the + // stored job, not the process's memory. + if err := st.Close(); err != nil { + t.Fatal(err) + } + st = openStore(t, dataDir) + restarted := newUnstartedEnvOn(t, st, dataDir) + restarted.search.Gate = gate + restarted.model.QueueGenerate(recipestest.NormalizedReply("stovetop", 40)) + if again, err := restarted.svc.StartPhotoBackfill(t.Context()); err != nil || again != first { + t.Fatalf("trigger after restart = %q, %v; want the queued %q", again, err, first) + } + + // Running: the pass waits at its search, and simultaneous triggers all + // return it. + restarted.start(t) + for len(restarted.search.Queries()) == 0 { + time.Sleep(time.Millisecond) + } + for _, id := range triggerAtOnce(t, restarted.svc, triggers) { + if id != first { + t.Fatalf("trigger while running = %q, want the running %q", id, first) + } + } + if n := backfillJobs(t, st); n != 1 { + t.Fatalf("%d backfill jobs, want 1", n) + } + + // Finished: the next trigger starts a new pass. + close(gate) + if job := restarted.waitJob(t, first); job.Status != jobs.StatusSucceeded { + t.Fatalf("job = %+v (result %s), want succeeded", job, job.Result) + } + next, err := restarted.svc.StartPhotoBackfill(t.Context()) + if err != nil { + t.Fatal(err) + } + if next == first { + t.Fatalf("trigger after the pass finished = %q, want a new job", next) + } + restarted.waitJob(t, next) +} + +func TestBackfillFailsJobWhenAWriteFails(t *testing.T) { + t.Parallel() + e := newEnv(t) + e.model.QueueGenerate(recipestest.NormalizedReply("stovetop", 40)) + absent := e.failedDinner(t, "Abalone surprise") + curry := e.failedDinner(t, "Chicken curry") + if _, err := e.st.Writer.ExecContext(t.Context(), `CREATE TRIGGER refuse_grounded BEFORE UPDATE OF grounding_status ON dinners + WHEN NEW.id = '`+curry+`' AND NEW.grounding_status = 'grounded' + BEGIN SELECT RAISE(ABORT, 'grounded refused'); END`); err != nil { + t.Fatal(err) + } + + id, err := e.svc.StartPhotoBackfill(t.Context()) + if err != nil { + t.Fatal(err) + } + job := e.waitJob(t, id) + if job.Status != jobs.StatusFailed || job.Error == nil || *job.Error != jobs.CodeInternal { + t.Fatalf("job = %+v, want failed with %s", job, jobs.CodeInternal) + } + var got struct { + Dinners []dinners.BackfillOutcome `json:"dinners"` + } + if err := json.Unmarshal(job.Result, &got); err != nil { + t.Fatalf("result %s: %v", job.Result, err) + } + // The other dinner was still tried and recorded. + checkOutcomes(t, got.Dinners, []dinners.BackfillOutcome{ + outcome(absent, dinners.BackfillFailed, ptr(dinners.ReasonNoRecipe)), + outcome(curry, dinners.BackfillFailed, ptr(dinners.ReasonGroundingError)), + }) + if row := e.dinner(t, curry); row.GroundingStatus != dinners.GroundingFailed || row.PhotoID.Valid { + t.Errorf("curry = %+v, want left failed without a photo", row) + } +} diff --git a/internal/dinners/dinners.go b/internal/dinners/dinners.go index 45308e5..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 @@ -138,6 +143,8 @@ type Service struct { grounder *recipes.Grounder logger *slog.Logger groundJob *jobs.Kind[GroundJob] + // backfillJob enqueues KindBackfillPhotos jobs. + backfillJob *jobs.Kind[BackfillJob] // wake is called after every committed change to a dinner's method or // minutes, which can move its crock-pot start reminder. wake func() @@ -162,8 +169,8 @@ type Service struct { // NewService returns a service over st that grounds dinners with grounder, // calls wake (the scheduler's Wake) after every committed change to a -// dinner's method or minutes, and registers the KindGroundDinner job on -// runner, which must not have started yet. +// dinner's method or minutes, and registers the KindGroundDinner and +// KindBackfillPhotos jobs on runner, which must not have started yet. func NewService(st *store.Store, grounder *recipes.Grounder, runner *jobs.Runner, wake func(), logger *slog.Logger) (*Service, error) { switch { case st == nil: @@ -184,6 +191,14 @@ func NewService(st *store.Store, grounder *recipes.Grounder, runner *jobs.Runner return nil, err } s.groundJob = kind + backfill, err := jobs.Register(runner, KindBackfillPhotos, s.runBackfillJob) + if err != nil { + return nil, err + } + if err := backfill.ServiceOnly(); err != nil { + return nil, err + } + s.backfillJob = backfill return s, nil } @@ -702,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/internal/dinners/export_test.go b/internal/dinners/export_test.go index 0f4a731..dfdf5ee 100644 --- a/internal/dinners/export_test.go +++ b/internal/dinners/export_test.go @@ -74,3 +74,18 @@ func (s *Service) RunGroundJob(ctx context.Context, p GroundJob) (string, error) // SafeSourceURL is safeSourceURL. var SafeSourceURL = safeSourceURL + +// RunBackfillJob runs the KindBackfillPhotos handler for p in the caller's +// goroutine, as the job runner would, and returns its outcomes: on an +// error, those it last published. +func (s *Service) RunBackfillJob(ctx context.Context, p BackfillJob) ([]BackfillOutcome, error) { + var published backfillResult + r, err := s.runBackfillJob(ctx, p, func(partial backfillResult) error { + published = partial + return nil + }) + if err != nil { + return published.Dinners, err + } + return r.Dinners, nil +} diff --git a/internal/dinners/grounding_test.go b/internal/dinners/grounding_test.go index 11fb4c0..890f2a9 100644 --- a/internal/dinners/grounding_test.go +++ b/internal/dinners/grounding_test.go @@ -39,6 +39,7 @@ type env struct { runner *jobs.Runner search *recipestest.Search pages *recipestest.Pages + images *recipestest.Images model *llm.Fake } @@ -108,7 +109,7 @@ func newUnstartedEnvOn(t *testing.T, st *store.Store, dataDir string) *env { if err != nil { t.Fatal(err) } - return &env{svc: svc, st: st, runner: runner, search: search, pages: pages, model: model} + return &env{svc: svc, st: st, runner: runner, search: search, pages: pages, images: images, model: model} } // start starts the env's job runner, stopping it when the test ends. diff --git a/internal/server/dinners.go b/internal/server/dinners.go index 5f0d54e..4934594 100644 --- a/internal/server/dinners.go +++ b/internal/server/dinners.go @@ -112,6 +112,20 @@ func postDinner(logger *slog.Logger, svc *dinners.Service) http.HandlerFunc { } } +// postPhotoBackfill starts a photo backfill over every dinner without a +// photo and answers 202 with its job id, which GET /jobs/{id} reports on, +// without waiting for the job. +func postPhotoBackfill(logger *slog.Logger, svc *dinners.Service) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + id, err := svc.StartPhotoBackfill(r.Context()) + if err != nil { + writeDinnerError(w, r, logger, "start photo backfill", err) + return + } + writeJSON(w, r, logger, http.StatusAccepted, jobCreated{ID: id}) + } +} + // patchDinner edits a dinner, recording the session's actor as updated_by. func patchDinner(logger *slog.Logger, svc *dinners.Service) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { diff --git a/internal/server/grounding_test.go b/internal/server/grounding_test.go index dfac21a..5be3a66 100644 --- a/internal/server/grounding_test.go +++ b/internal/server/grounding_test.go @@ -556,3 +556,43 @@ func TestPostDinnerEnqueueFailureAnswersFailedDinner(t *testing.T) { t.Errorf("listed = %v, want failed", got) } } + +func TestPostPhotoBackfillGroundsFailedDinners(t *testing.T) { + t.Parallel() + e := newDinnersEnv(t).withRecipes(t) + e.start(t) + + // No model reply is scripted, so grounding on creation fails. + rec, created := callDinners(t, e.h, http.MethodPost, dinnersPath, `{"name":"Chicken curry"}`) + if rec.Code != http.StatusCreated { + t.Fatalf("POST = %d: %s", rec.Code, rec.Body) + } + id := created["id"].(string) + waitJob(t, e.h, created["grounding_job_id"].(string)) + if d := listedDinner(t, e.h, id); d["grounding_status"] != "failed" || d["photo_url"] != nil { + t.Fatalf("dinner = %v, want failed without a photo", d) + } + + e.model.QueueGenerate(recipestest.NormalizedReply("stovetop", 40)) + rec, started := callDinners(t, e.h, http.MethodPost, "/api/v1/settings/photo-backfill", "") + jobID, _ := started["id"].(string) + if rec.Code != http.StatusAccepted || jobID == "" { + t.Fatalf("POST photo-backfill = %d: %s, want 202 with a job id", rec.Code, rec.Body) + } + job := waitJob(t, e.h, jobID) + want := fmt.Sprintf(`map[dinners:[map[dinner_id:%s outcome:grounded reason:]]]`, id) + if job["status"] != "succeeded" || job["kind"] != "backfill_dinner_photos" || fmt.Sprint(job["result"]) != want { + t.Fatalf("job = %v, want backfill_dinner_photos succeeded with %s", job, want) + } + d := listedDinner(t, e.h, id) + photo, _ := d["photo_url"].(string) + if d["grounding_status"] != "grounded" || d["grounding_error"] != nil || !strings.HasSuffix(photo, "-400.jpg") { + t.Errorf("dinner = %v, want grounded with a photo", d) + } + + // The backfill is started only by its own endpoint. + rec, _ = callDinners(t, e.h, http.MethodPost, "/api/v1/jobs", `{"kind":"backfill_dinner_photos","payload":{}}`) + if rec.Code != http.StatusBadRequest || !strings.Contains(rec.Body.String(), "service_only_kind") { + t.Errorf("POST /jobs backfill = %d: %s, want 400 service_only_kind", rec.Code, rec.Body) + } +} diff --git a/internal/server/server.go b/internal/server/server.go index 3b0352e..c7fce9b 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -162,6 +162,7 @@ func NewRouter(logger *slog.Logger, version string, trusted forwarded.Trusted, a apiV1.Get("/settings/chat-webhook", getChatWebhook(logger, chatSvc)) apiV1.Put("/settings/chat-webhook", putChatWebhook(logger, chatSvc)) apiV1.Post("/settings/chat-webhook/test", postChatWebhookTest(logger, chatSvc)) + apiV1.Post("/settings/photo-backfill", postPhotoBackfill(logger, dinnerSvc)) apiV1.Get("/dinners", getDinners(logger, dinnerSvc)) apiV1.Post("/dinners", postDinner(logger, dinnerSvc)) apiV1.Get("/dinners/{id}", getDinner(logger, dinnerSvc)) diff --git a/migrations/queries/dinners.sql b/migrations/queries/dinners.sql index 98795bf..8bed5a9 100644 --- a/migrations/queries/dinners.sql +++ b/migrations/queries/dinners.sql @@ -181,3 +181,65 @@ WHERE grounding_status = 'pending' AND json_extract(jobs.payload, '$.dinner_id') = dinners.id AND json_extract(jobs.payload, '$.revision') = dinners.grounding_revision ); + +-- name: ListPhotoBackfillDinners :many +-- The dinners a photo backfill grounds by name: unarchived, failed, with no +-- photo and no pasted recipe_url (the household's own link is never +-- replaced by a search), ordered by normalized name. A non-NULL id narrows +-- the list to that dinner. Grounded, manual and pending dinners are never +-- listed: they have a recipe, or a ground_dinner job grounding them. +SELECT * FROM dinners +WHERE status <> 'archived' + AND grounding_status = 'failed' + AND photo_id IS NULL + AND recipe_url IS NULL + AND id = COALESCE(sqlc.narg(id), id) +ORDER BY normalized_name, id; + +-- name: GetActivePhotoBackfillJob :one +-- The id of the full photo backfill job (no dinner_id) of kind queued or +-- running. StartPhotoBackfill enqueues one only when there is none, so +-- there is at most one. Returns sql.ErrNoRows when there is none. +SELECT id +FROM jobs +WHERE kind = sqlc.arg(kind) + AND status IN ('queued', 'running') + AND json_extract(payload, '$.dinner_id') IS NULL +ORDER BY id +LIMIT 1; + +-- name: GroundFailedDinner :execrows +-- Records a photo backfill's grounding of a dinner it listed, while the +-- dinner is still unarchived, failed, photoless, without a recipe_url and at +-- the grounding_revision it was listed at; a dinner grounded by hand, +-- renamed or archived meanwhile is left as it is (0 rows). Like +-- GroundPendingDinner it keeps a method, minutes_total or difficulty a +-- member already set. +UPDATE dinners +SET recipe_id = sqlc.arg(recipe_id), + photo_id = sqlc.arg(photo_id), + method = COALESCE(method, sqlc.arg(method)), + minutes_total = COALESCE(minutes_total, sqlc.arg(minutes_total)), + difficulty = COALESCE(difficulty, sqlc.arg(difficulty)), + grounding_status = 'grounded', + grounding_error = NULL, + updated_at = sqlc.arg(updated_at) +WHERE id = sqlc.arg(id) + AND status <> 'archived' + AND grounding_status = 'failed' + AND photo_id IS NULL + AND recipe_url IS NULL + AND grounding_revision = sqlc.arg(grounding_revision); + +-- name: RefailDinnerGrounding :execrows +-- Records why a photo backfill could not ground a dinner it listed, under +-- the same conditions as GroundFailedDinner (0 rows otherwise). +UPDATE dinners +SET grounding_error = sqlc.arg(grounding_error), + updated_at = sqlc.arg(updated_at) +WHERE id = sqlc.arg(id) + AND status <> 'archived' + AND grounding_status = 'failed' + AND photo_id IS NULL + AND recipe_url IS NULL + AND grounding_revision = sqlc.arg(grounding_revision); diff --git a/web/src/dinners/DinnerDetailActions.test.tsx b/web/src/dinners/DinnerDetailActions.test.tsx index dc967a4..9e49828 100644 --- a/web/src/dinners/DinnerDetailActions.test.tsx +++ b/web/src/dinners/DinnerDetailActions.test.tsx @@ -52,6 +52,8 @@ function listed(is_favorite: boolean) { is_favorite, 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 2c0c081..00de235 100644 --- a/web/src/dinners/api.ts +++ b/web/src/dinners/api.ts @@ -20,8 +20,12 @@ 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. 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 name: string @@ -31,6 +35,8 @@ export type Regular = { is_favorite: boolean photo_id: string | undefined grounding_status: GroundingStatus + grounding_error: string | null + photo_backfill_eligible: boolean pending?: true } @@ -113,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, @@ -122,6 +131,11 @@ 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 + }), + photo_backfill_eligible: body.photo_backfill_eligible, } } @@ -225,6 +239,8 @@ export function useAddRegular() { is_favorite: false, 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 77c9867..be09c8d 100644 --- a/web/src/plan/PlanPage.test.tsx +++ b/web/src/plan/PlanPage.test.tsx @@ -114,6 +114,8 @@ function dinner(id: string, name: string, difficulty: string | null, minutes: nu is_favorite: false, photo_url: null, grounding_status: 'grounded', + grounding_error: null, + photo_backfill_eligible: false, } } 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..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() { @@ -496,6 +500,204 @@ 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', photo_backfill_eligible: false } + 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('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, + [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 when a dinner was not included in the search', async () => { + stubServer([PIZZA], { + ...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('This dinner was not included in that search.')).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..bd0153c 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,20 @@ 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 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() @@ -475,6 +575,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.photo_backfill_eligible && ( + + )} 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] : 'This dinner was not included in that search.' + } + 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, + } +} 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