Log every unexpected recipe grounding failure by fixed reason

Wrap an initial dinner read failure on the candidate and link path, and
any manual-recipe read, store or write failure, in ErrGroundFailed so the
handler logs only reason=ground_failed and the dinner id, never database
text. Not-found, archived and conflict answers stay distinct.

Strengthen TestFixInFlightKeepsMemberEdits: start from populated method,
minutes and difficulty, edit each in flight in turn, and require the edit
kept and the untouched populated fields replaced by the recipe's values.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

internal/dinners/grounding.go

@@ -76,7 +76,7 @@ var groundTimeout = GroundTimeout
 // runs past GroundTimeout; the dinner is left as it was.
 var ErrGroundTimeout = errors.New("dinners: grounding the recipe took too long")
 
-// ErrGroundFailed wraps any failure grounding a candidate or link by hand.
+// ErrGroundFailed wraps any unexpected failure grounding a dinner by hand.
 // Its text may quote the link and dependency errors, so it is never
 // logged.
 var ErrGroundFailed = errors.New("dinners: grounding the recipe failed")
@@ -343,8 +343,9 @@ type ManualRecipe struct {
 // link, or recording it on the dinner, wraps ErrGroundFailed; a page that
 // cannot ground the dinner is also a *recipes.ErrUngroundable. A dinner
 // renamed, archived or grounded some other way while a candidate or link
-// was grounding is ErrGroundConflict or ErrArchived. None of these changes
-// the dinner.
+// was grounding is ErrGroundConflict or ErrArchived. A missing dinner is
+// ErrNotFound; any other failure, by candidate, link or manual recipe,
+// wraps ErrGroundFailed. None of these changes the dinner.
 func (s *Service) Ground(ctx context.Context, id string, f Fix, updatedBy string) (Dinner, error) {
 	if updatedBy == "" {
 		return Dinner{}, errors.New("dinners: ground: no member to record as updated_by")
@@ -357,9 +358,12 @@ func (s *Service) Ground(ctx context.Context, id string, f Fix, updatedBy string
 		return s.groundManual(ctx, id, *manual, updatedBy)
 	}
 	cur, err := s.get(ctx, id)
-	if err != nil {
+	if errors.Is(err, ErrNotFound) {
 		return Dinner{}, err
 	}
+	if err != nil {
+		return Dinner{}, fmt.Errorf("%w: dinner %s: %w", ErrGroundFailed, id, err)
+	}
 	if cur.Status == StatusArchived {
 		return Dinner{}, ErrArchived
 	}
@@ -418,7 +422,9 @@ func (s *Service) groundRejected(ctx context.Context, id string) error {
 }
 
 // groundManual stores m as dinner id's recipe, with its ingredients and
-// steps, and records it on the dinner, all in one transaction.
+// steps, and records it on the dinner, all in one transaction. A missing
+// or archived dinner is ErrNotFound or ErrArchived; any other failure
+// wraps ErrGroundFailed.
 func (s *Service) groundManual(ctx context.Context, id string, m ManualRecipe, updatedBy string) (Dinner, error) {
 	var row db.Dinner
 	err := s.st.WithTx(ctx, func(tx *sql.Tx) error {
@@ -510,9 +516,12 @@ func (s *Service) groundManual(ctx context.Context, id string, m ManualRecipe, u
 		}
 		return nil
 	})
-	if err != nil {
+	if errors.Is(err, ErrNotFound) || errors.Is(err, ErrArchived) {
 		return Dinner{}, err
 	}
+	if err != nil {
+		return Dinner{}, fmt.Errorf("%w: dinner %s: %w", ErrGroundFailed, id, err)
+	}
 	return fromRow(row), nil
 }
 

internal/dinners/grounding_test.go

@@ -1177,38 +1177,98 @@ func TestFixInFlightWhenDinnerChanges(t *testing.T) {
 	}
 }
 
-// TestFixInFlightKeepsMemberEdits: a method or difficulty a member sets
-// while a link is grounding is kept; what they left alone takes the link's
-// recipe.
+// TestFixInFlightKeepsMemberEdits: a method, minutes or difficulty a
+// member sets while a link is grounding is kept; a field they left alone
+// takes the link's recipe, even one already set before grounding began.
 func TestFixInFlightKeepsMemberEdits(t *testing.T) {
 	t.Parallel()
-	e := newEnv(t)
-	id := failedDinner(t, e, "Grandma's stew")
-	e.model.QueueGenerate(recipestest.NormalizedReply("oven", 150))
-	e.pages.Gate = make(chan struct{})
+	// Every field is set before grounding, then one is changed in flight.
+	before := dinners.Patch{Method: ptr("stovetop"), MinutesTotal: ptr(45), Difficulty: ptr("easy")}
+	tests := []struct {
+		name  string
+		edit  dinners.Patch
+		field string
+	}{
+		{"method", dinners.Patch{Method: ptr("grill")}, "method"},
+		{"minutes", dinners.Patch{MinutesTotal: ptr(20)}, "minutes_total"},
+		{"difficulty", dinners.Patch{Difficulty: ptr("medium")}, "difficulty"},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			e := newEnv(t)
+			id := failedDinner(t, e, "Grandma's stew")
+			if _, err := e.svc.Update(t.Context(), id, before, member); err != nil {
+				t.Fatal(err)
+			}
+			e.model.QueueGenerate(recipestest.NormalizedReply("oven", 150))
+			e.pages.Gate = make(chan struct{})
 
-	done := make(chan error, 1)
-	go func() {
-		_, err := e.svc.Ground(context.Background(), id, dinners.Fix{RecipeURL: ptr(stew)}, member)
-		done <- err
-	}()
-	e.waitFetched(t, stew)
-	if _, err := e.svc.Update(t.Context(), id, dinners.Patch{Method: ptr("grill"), Difficulty: ptr("easy")}, member); err != nil {
-		t.Fatal(err)
-	}
-	close(e.pages.Gate)
+			done := make(chan error, 1)
+			go func() {
+				_, err := e.svc.Ground(context.Background(), id, dinners.Fix{RecipeURL: ptr(stew)}, member)
+				done <- err
+			}()
+			e.waitFetched(t, stew)
+			if _, err := e.svc.Update(t.Context(), id, tt.edit, member); err != nil {
+				t.Fatal(err)
+			}
+			close(e.pages.Gate)
 
-	if err := <-done; err != nil {
-		t.Fatal(err)
+			if err := <-done; err != nil {
+				t.Fatal(err)
+			}
+			row := e.dinner(t, id)
+			if row.GroundingStatus != dinners.GroundingGrounded || !row.RecipeID.Valid {
+				t.Fatalf("dinner = %+v, want grounded in the link's recipe", row)
+			}
+			var recipe fields
+			if err := e.st.Reader.QueryRowContext(t.Context(),
+				`SELECT method, minutes_total, difficulty FROM recipes WHERE id = ?`, row.RecipeID.String,
+			).Scan(&recipe.method, &recipe.minutes, &recipe.difficulty); err != nil {
+				t.Fatal(err)
+			}
+			old, edited := patchFields(before), patchFields(tt.edit)
+			want := recipe
+			switch tt.field {
+			case "method":
+				want.method = edited.method
+			case "minutes_total":
+				want.minutes = edited.minutes
+			case "difficulty":
+				want.difficulty = edited.difficulty
+			}
+			if recipe.method == old.method || recipe.minutes == old.minutes || recipe.difficulty == old.difficulty {
+				t.Fatalf("recipe %+v shares a value with the dinner before grounding %+v", recipe, old)
+			}
+			got := fields{row.Method.String, row.MinutesTotal.Int64, row.Difficulty.String}
+			if got != want {
+				t.Errorf("method, minutes, difficulty = %+v, want %+v: the %s edit kept, the rest from the recipe", got, want, tt.field)
+			}
+		})
 	}
-	row := e.dinner(t, id)
-	if row.GroundingStatus != dinners.GroundingGrounded || !row.RecipeID.Valid {
-		t.Errorf("dinner = %+v, want grounded in the link's recipe", row)
+}
+
+// fields are a dinner's or recipe's method, minutes_total and difficulty.
+type fields struct {
+	method     string
+	minutes    int64
+	difficulty string
+}
+
+// patchFields returns the fields p sets, zero where it sets none.
+func patchFields(p dinners.Patch) fields {
+	var f fields
+	if p.Method != nil {
+		f.method = *p.Method
 	}
-	if row.Method.String != "grill" || row.Difficulty.String != "easy" || row.MinutesTotal.Int64 != 150 {
-		t.Errorf("method, difficulty, minutes = %q, %q, %d; want grill and easy kept, 150 from the recipe",
-			row.Method.String, row.Difficulty.String, row.MinutesTotal.Int64)
+	if p.MinutesTotal != nil {
+		f.minutes = int64(*p.MinutesTotal)
 	}
+	if p.Difficulty != nil {
+		f.difficulty = *p.Difficulty
+	}
+	return f
 }
 
 // TestFixArchivedDinner: an archived dinner cannot be grounded by hand.

internal/server/dinners.go

@@ -202,7 +202,8 @@ func actorOrFail(w http.ResponseWriter, r *http.Request, logger *slog.Logger) (a
 // up is 409 not_configured; a search or model failure is 502; grounding by
 // hand that runs past dinners.GroundTimeout is 504 ground_timeout; grounding
 // an archived dinner is 409 dinner_archived and one changed while grounding
-// 409 ground_conflict. Any other grounding failure is 500, logged by reason
+// 409 ground_conflict. Any other grounding failure, by candidate, link or
+// manual recipe, wraps dinners.ErrGroundFailed and is 500, logged by reason
 // and dinner id only.
 func writeDinnerError(w http.ResponseWriter, r *http.Request, logger *slog.Logger, action string, err error) {
 	var invalid *dinners.ValidationError

internal/server/grounding_test.go

@@ -279,6 +279,45 @@ func TestPostGroundViaRecipeURLDinnerWriteFailure(t *testing.T) {
 	}
 }
 
+// TestPostGroundUnexpectedFailures: grounding by hand that fails reading
+// the dinner, or storing a manual recipe, is a 500 logged by reason and
+// dinner id, never with the database error.
+func TestPostGroundUnexpectedFailures(t *testing.T) {
+	t.Parallel()
+	const (
+		// Renaming a column every dinner read selects fails the read.
+		breakRead = `ALTER TABLE dinners RENAME COLUMN cuisine TO cuisine_gone`
+		// The trigger's message stands in for the database error.
+		breakStore = `CREATE TRIGGER refuse_recipes BEFORE INSERT ON recipes BEGIN SELECT RAISE(ABORT, 'disk full'); END`
+		manual     = `{"manual":{"ingredients":["1 lb beef"],"steps":["Brown."],"minutes":240,"method":"crockpot"}}`
+	)
+	for _, tc := range []struct {
+		name, sabotage, fix, leak string
+	}{
+		{"link read", breakRead, `{"recipe_url":"` + stewPage + `"}`, "cuisine"},
+		{"manual read", breakRead, manual, "cuisine"},
+		{"manual store", breakStore, manual, "disk full"},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+			e := newDinnersEnv(t).withRecipes(t)
+			id := createDinner(t, e.h, `{"name":"Sunday stew"}`)
+			if _, err := e.st.Writer.ExecContext(t.Context(), tc.sabotage); err != nil {
+				t.Fatal(err)
+			}
+
+			rec, body := callDinners(t, e.h, http.MethodPost, dinnersPath+"/"+id+"/ground", tc.fix)
+			if rec.Code != http.StatusInternalServerError || errorCode(body) != "internal" {
+				t.Errorf("POST = %d %s, want 500 internal", rec.Code, rec.Body)
+			}
+			logs := e.logs.String()
+			if strings.Contains(logs, tc.leak) || !strings.Contains(logs, "reason=ground_failed") || !strings.Contains(logs, "dinner_id="+id) {
+				t.Errorf("logs = %q, want reason=ground_failed and the dinner id, without the database error", logs)
+			}
+		})
+	}
+}
+
 // TestPostGroundArchived: grounding an archived dinner by hand is a 409.
 func TestPostGroundArchived(t *testing.T) {
 	t.Parallel()