diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 0472353..1d12443 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -259,6 +259,19 @@ 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; 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`. `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..6e629a4 100644 --- a/internal/db/dinners.sql.go +++ b/internal/db/dinners.sql.go @@ -344,6 +344,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 +496,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 +670,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..4ce3309 100644 --- a/internal/db/querier.go +++ b/internal/db/querier.go @@ -175,6 +175,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 +213,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 +320,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..96ac308 --- /dev/null +++ b/internal/dinners/backfill.go @@ -0,0 +1,145 @@ +package dinners + +import ( + "context" + "database/sql" + "encoding/json" + "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" +) + +// 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. +func (s *Service) StartPhotoBackfill(ctx context.Context) (string, error) { + id, err := s.backfillJob.Enqueue(ctx, BackfillJob{}) + if err != nil { + return "", fmt.Errorf("dinners: enqueue photo backfill: %w", err) + } + return id, nil +} + +// 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. Only a failure to list the dinners, or the job's context +// ending, fails the job. +func (s *Service) runBackfillJob(ctx context.Context, p BackfillJob, _ 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 := false + for _, row := range rows { + out, err := s.backfillDinner(ctx, row) + if err != nil { + return backfillResult{}, err + } + grounded = grounded || out.Outcome == BackfillGrounded + res.Dinners = append(res.Dinners, out) + } + if grounded { + s.wake() + } + return res, nil +} + +// backfillDinner grounds row, a dinner ListPhotoBackfillDinners listed, as +// runBackfillJob describes. Its error is only the job's context ending. +func (s *Service) backfillDinner(ctx context.Context, row db.Dinner) (BackfillOutcome, 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{}, 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, + }) + return s.backfillRecorded(ctx, out, BackfillFailed, reason, n, err), 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, + }) + return s.backfillRecorded(ctx, out, BackfillGrounded, "", n, err), 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. +func (s *Service) backfillRecorded(ctx context.Context, out BackfillOutcome, outcome, reason string, n int64, err error) BackfillOutcome { + 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 +} diff --git a/internal/dinners/backfill_test.go b/internal/dinners/backfill_test.go new file mode 100644 index 0000000..3ad08a4 --- /dev/null +++ b/internal/dinners/backfill_test.go @@ -0,0 +1,241 @@ +package dinners_test + +import ( + "database/sql" + "encoding/json" + "errors" + "slices" + "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/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) + } +} + +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) + } +} diff --git a/internal/dinners/dinners.go b/internal/dinners/dinners.go index 45308e5..e2a57ef 100644 --- a/internal/dinners/dinners.go +++ b/internal/dinners/dinners.go @@ -138,6 +138,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 +164,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 +186,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 } diff --git a/internal/dinners/export_test.go b/internal/dinners/export_test.go index 0f4a731..ae83d86 100644 --- a/internal/dinners/export_test.go +++ b/internal/dinners/export_test.go @@ -74,3 +74,10 @@ 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. +func (s *Service) RunBackfillJob(ctx context.Context, p BackfillJob) ([]BackfillOutcome, error) { + r, err := s.runBackfillJob(ctx, p, nil) + return r.Dinners, err +} 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..f541136 100644 --- a/migrations/queries/dinners.sql +++ b/migrations/queries/dinners.sql @@ -181,3 +181,53 @@ 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: 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);