Fix stale Regular grounding and enforce three Regulars for onboarding

A background grounding result now applies only while the dinner is still pending under the normalized name the job searched: GroundPendingDinner and FailPendingDinnerGrounding match normalized_name, so a rename makes the old job's success or failure a no-op. Renaming a dinner still pending enqueues a fresh grounding job for the new name; if that cannot be enqueued, the dinner's grounding is failed so it is not left pending with no job. Completing onboarding (PATCH /api/v1/settings/household with onboarding_completed true) is refused with 400 invalid_settings until the household has at least three Regulars (status regular; archived and Quest log dinners do not count), and onboarding_completed_at is set only then. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-06 18:47:41 -04:00
parent 9baabad84c
commit 171c9c2d11
9 changed files with 355 additions and 55 deletions
@@ -176,27 +176,41 @@ func (s *Service) CreateRegular(ctx context.Context, r NewRegular, updatedBy str
 	if err != nil {
 		return Created{}, err
 	}
-	jobID, err := s.groundJob.Enqueue(ctx, GroundJob{DinnerID: d.ID})
-	if err == nil {
-		return Created{Dinner: d, GroundingJobID: &jobID}, nil
+	jobID, err := s.enqueueGrounding(ctx, d.ID, harness.Normalize(d.Name))
+	if err != nil {
+		return Created{}, err
 	}
-	// The dinner is stored; record that it will not be grounded so it is
-	// not left pending with no job. The error is a database error, never a
-	// dependency's text.
-	s.logger.ErrorContext(ctx, "dinner grounding not enqueued", "dinner_id", d.ID, "error", err)
-	write := context.WithoutCancel(ctx)
-	if _, err := db.New(s.st.Writer).FailPendingDinnerGrounding(write, db.FailPendingDinnerGroundingParams{
+	if jobID == nil {
+		row, err := db.New(s.st.Reader).GetDinner(context.WithoutCancel(ctx), d.ID)
+		if err != nil {
+			return Created{}, fmt.Errorf("dinners: read %s: %w", d.ID, err)
+		}
+		return Created{Dinner: fromRow(row)}, nil
+	}
+	return Created{Dinner: d, GroundingJobID: jobID}, nil
+}
+
+// enqueueGrounding enqueues the KindGroundDinner job for pending dinner id,
+// whose normalized name is normalizedName, and returns its id. When the job
+// cannot be enqueued, the dinner's grounding is failed with
+// ReasonGroundingError, so it is not left pending with no job, and the id
+// is nil.
+func (s *Service) enqueueGrounding(ctx context.Context, id, normalizedName string) (*string, error) {
+	jobID, err := s.groundJob.Enqueue(ctx, GroundJob{DinnerID: id})
+	if err == nil {
+		return &jobID, nil
+	}
+	// The error is a database error, never a dependency's text.
+	s.logger.ErrorContext(ctx, "dinner grounding not enqueued", "dinner_id", id, "error", err)
+	if _, err := db.New(s.st.Writer).FailPendingDinnerGrounding(context.WithoutCancel(ctx), db.FailPendingDinnerGroundingParams{
 		GroundingError: sql.NullString{String: ReasonGroundingError, Valid: true},
 		UpdatedAt:      store.Now().String(),
-		ID:             d.ID,
+		ID:             id,
+		NormalizedName: normalizedName,
 	}); err != nil {
-		return Created{}, fmt.Errorf("dinners: record failed grounding of %s: %w", d.ID, err)
+		return nil, fmt.Errorf("dinners: record failed grounding of %s: %w", id, err)
 	}
-	row, err := db.New(s.st.Reader).GetDinner(write, d.ID)
-	if err != nil {
-		return Created{}, fmt.Errorf("dinners: read %s: %w", d.ID, err)
-	}
-	return Created{Dinner: fromRow(row)}, nil
+	return nil, nil
 }

 func (s *Service) createRegular(ctx context.Context, r NewRegular, updatedBy string) (Dinner, error) {
@@ -279,7 +293,9 @@ func (s *Service) List(ctx context.Context, status string) ([]Dinner, error) {

 // Update applies p to dinner id, recording updatedBy. An invalid patch is a
 // *ValidationError and a new name repeating another unarchived dinner a
-// *DuplicateError; either changes nothing.
+// *DuplicateError; either changes nothing. Renaming a dinner still pending
+// enqueues a fresh KindGroundDinner job for the new name: the job already
+// running was for the old name, and its result no longer applies.
 func (s *Service) Update(ctx context.Context, id string, p Patch, updatedBy string) (Dinner, error) {
 	if updatedBy == "" {
 		return Dinner{}, errors.New("dinners: update: no member to record as updated_by")
@@ -290,6 +306,7 @@ func (s *Service) Update(ctx context.Context, id string, p Patch, updatedBy stri
 	}

 	var row db.Dinner
+	var regrounding bool
 	err = s.st.WithTx(ctx, func(tx *sql.Tx) error {
 		q := db.New(tx)
 		cur, err := q.GetDinner(ctx, id)
@@ -335,11 +352,24 @@ func (s *Service) Update(ctx context.Context, id string, p Patch, updatedBy stri
 		if err != nil {
 			return fmt.Errorf("dinners: update %s: %w", id, err)
 		}
+		regrounding = row.GroundingStatus == GroundingPending && row.NormalizedName != cur.NormalizedName
 		return nil
 	})
 	if err != nil {
 		return Dinner{}, err
 	}
+	if regrounding {
+		jobID, err := s.enqueueGrounding(ctx, row.ID, row.NormalizedName)
+		if err != nil {
+			return Dinner{}, err
+		}
+		if jobID == nil {
+			row, err = db.New(s.st.Reader).GetDinner(context.WithoutCancel(ctx), row.ID)
+			if err != nil {
+				return Dinner{}, fmt.Errorf("dinners: read %s: %w", id, err)
+			}
+		}
+	}
 	return fromRow(row), nil
 }

@@ -95,8 +95,10 @@ func (r groundResult) PublicJSON() ([]byte, error) { return json.Marshal(r) }
 // recipe_url with GroundURL, else its name with Ground. Success records
 // the recipe, photo, method, minutes and difficulty with grounding_status
 // grounded; failure records grounding_status failed with its Reason, which
-// is also the job's failure code. A dinner no longer pending (grounded by
-// hand meanwhile) is left as it is.
+// is also the job's failure code. Either is recorded only while the dinner
+// is still pending under the normalized name read here: a dinner grounded
+// by hand meanwhile is left as it is, and one renamed meanwhile is left
+// pending for the job its rename enqueued.
 func (s *Service) runGroundJob(ctx context.Context, p GroundJob, _ func(groundResult) error) (groundResult, error) {
 	row, err := db.New(s.st.Reader).GetDinner(ctx, p.DinnerID)
 	if err != nil {
@@ -125,6 +127,7 @@ func (s *Service) runGroundJob(ctx context.Context, p GroundJob, _ func(groundRe
 			GroundingError: sql.NullString{String: reason, Valid: true},
 			UpdatedAt:      store.Now().String(),
 			ID:             row.ID,
+			NormalizedName: row.NormalizedName,
 		}); failErr != nil {
 			return groundResult{}, fmt.Errorf("dinners: ground: record failure of %s: %w", row.ID, failErr)
 		}
@@ -135,13 +138,14 @@ func (s *Service) runGroundJob(ctx context.Context, p GroundJob, _ func(groundRe
 	}

 	n, err := db.New(s.st.Writer).GroundPendingDinner(ctx, db.GroundPendingDinnerParams{
-		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,
+		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,
+		NormalizedName: row.NormalizedName,
 	})
 	if err != nil {
 		return groundResult{}, fmt.Errorf("dinners: ground: record %s: %w", row.ID, err)
@@ -286,6 +286,120 @@ func TestManualFixWhilePendingIsKept(t *testing.T) {
 	}
 }

+// TestRenameWhilePendingGroundsNewName: a Regular renamed while its job is
+// still grounding the old name never gets the old name's result, whether
+// that job succeeds or fails; the job the rename enqueues grounds the new
+// name.
+func TestRenameWhilePendingGroundsNewName(t *testing.T) {
+	t.Parallel()
+	tests := []struct {
+		name    string
+		oldName string
+		replies []llm.GenerateResult
+	}{
+		{"old name found", "Chicken curry", []llm.GenerateResult{
+			recipestest.NormalizedReply("stovetop", 40), recipestest.NormalizedReply("oven", 150),
+		}},
+		{"old name not found", "Unknown dish", []llm.GenerateResult{recipestest.NormalizedReply("oven", 150)}},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			e := newEnv(t)
+			e.search.Results["Beef stew"] = []recipes.Candidate{{URL: stew}}
+			e.search.Gate = make(chan struct{})
+			for _, r := range tt.replies {
+				e.model.QueueGenerate(r)
+			}
+
+			c, err := e.svc.CreateRegular(t.Context(), dinners.NewRegular{Name: ptr(tt.oldName)}, member)
+			if err != nil {
+				t.Fatal(err)
+			}
+			// Rename only once the job has read the old name and is searching it.
+			e.waitQuery(t, tt.oldName)
+			d, err := e.svc.Update(t.Context(), c.ID, dinners.Patch{Name: ptr("Beef stew")}, member)
+			if err != nil {
+				t.Fatal(err)
+			}
+			if d.GroundingStatus != dinners.GroundingPending {
+				t.Fatalf("renamed = %+v, want still pending", d)
+			}
+			close(e.search.Gate)
+
+			first := e.waitJob(t, *c.GroundingJobID)
+			if string(first.Result) == `{"dinner_id":"`+c.ID+`","grounding_status":"grounded"}` {
+				t.Fatalf("old name's job reported grounded: %+v (result %s)", first, first.Result)
+			}
+			row := e.waitGrounding(t, c.ID)
+			if row.GroundingStatus != dinners.GroundingGrounded {
+				t.Fatalf("dinner = %+v, want grounded for the new name", row)
+			}
+			rec, err := db.New(e.st.Reader).GetRecipe(t.Context(), row.RecipeID.String)
+			if err != nil {
+				t.Fatal(err)
+			}
+			if rec.SourceUrl != stew || rec.PhotoID != row.PhotoID || row.Method.String != "oven" || row.MinutesTotal.Int64 != 150 {
+				t.Errorf("dinner = %+v recipe %s, want grounded in %s", row, rec.SourceUrl, stew)
+			}
+			if got := e.search.Queries(); !slices.Equal(got, []string{tt.oldName, "Beef stew"}) {
+				t.Errorf("searched %q, want the old name then the new", got)
+			}
+		})
+	}
+}
+
+// TestRenameWhilePendingEnqueueFailure: a pending Regular renamed when the
+// job for its new name cannot be enqueued is failed, not left pending.
+func TestRenameWhilePendingEnqueueFailure(t *testing.T) {
+	t.Parallel()
+	e := newUnstartedEnv(t)
+	c, err := e.svc.CreateRegular(t.Context(), dinners.NewRegular{Name: ptr("Chicken curry")}, member)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if _, err := e.st.Writer.ExecContext(t.Context(),
+		`CREATE TRIGGER refuse_jobs BEFORE INSERT ON jobs BEGIN SELECT RAISE(ABORT, 'no jobs'); END`); err != nil {
+		t.Fatal(err)
+	}
+	d, err := e.svc.Update(t.Context(), c.ID, dinners.Patch{Name: ptr("Beef stew")}, member)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if d.Name != "Beef stew" || d.GroundingStatus != dinners.GroundingFailed ||
+		d.GroundingError == nil || *d.GroundingError != dinners.ReasonGroundingError {
+		t.Fatalf("renamed = %+v, want Beef stew failed (grounding_error)", d)
+	}
+}
+
+// waitQuery waits until the fake search has been asked query.
+func (e *env) waitQuery(t *testing.T, query string) {
+	t.Helper()
+	deadline := time.Now().Add(10 * time.Second)
+	for !slices.Contains(e.search.Queries(), query) {
+		if time.Now().After(deadline) {
+			t.Fatalf("search never asked %q; asked %q", query, e.search.Queries())
+		}
+		time.Sleep(5 * time.Millisecond)
+	}
+}
+
+// waitGrounding waits for dinner id to be no longer pending and returns it.
+func (e *env) waitGrounding(t *testing.T, id string) db.Dinner {
+	t.Helper()
+	deadline := time.Now().Add(10 * time.Second)
+	for {
+		row := e.dinner(t, id)
+		if row.GroundingStatus != dinners.GroundingPending {
+			return row
+		}
+		if time.Now().After(deadline) {
+			t.Fatalf("dinner %s still pending", id)
+		}
+		time.Sleep(5 * time.Millisecond)
+	}
+}
+
 // failedDinner creates a Regular whose background grounding fails.
 func failedDinner(t *testing.T, e *env, name string) string {
 	t.Helper()