Integration fix 2: conditional recipe repairs and no pasted links in logs (Feature 12 review F1, F2) (#96)

Co-authored-by: orchestrator-agent <20+orchestrator-agent@code.cubecraftcreations.com>
2026-10-06 20:26:56 -04:00
parent cc7875c76c
commit 5dff7825d8
9 changed files with 509 additions and 61 deletions
@@ -76,6 +76,18 @@ 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 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")
+
+// ErrArchived is grounding an archived dinner by hand.
+var ErrArchived = errors.New("dinner archived")
+
+// ErrGroundConflict is a candidate or link grounded for a dinner that was
+// renamed or grounded some other way while it was grounding.
+var ErrGroundConflict = errors.New("dinner changed while its recipe was grounding")
+
 // GroundJob is the KindGroundDinner payload: the dinner to ground and its
 // grounding_revision when the job was enqueued.
 type GroundJob struct {
@@ -323,11 +335,17 @@ type ManualRecipe struct {
 // Ground grounds dinner id as f says, recording updatedBy. A candidate or
 // link is grounded with recipes.Grounder.GroundURL and sets
 // grounding_status grounded, with the recipe's photo, method, minutes and
-// difficulty. A manual recipe is stored as typed — no model runs — and
-// sets grounding_status manual with its method and minutes; it has no
-// photo, and the dinner keeps its difficulty. An invalid f is a
-// *ValidationError; a page that cannot ground the dinner is a
-// *recipes.ErrUngroundable and changes nothing.
+// difficulty; one a member changed while it was grounding is kept. A
+// manual recipe is stored as typed — no model runs — and sets
+// grounding_status manual with its method and minutes; it has no photo,
+// and the dinner keeps its difficulty. An invalid f is a *ValidationError
+// and an archived dinner ErrArchived. A failure grounding a candidate or
+// 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. 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")
@@ -339,9 +357,16 @@ func (s *Service) Ground(ctx context.Context, id string, f Fix, updatedBy string
 	if manual != nil {
 		return s.groundManual(ctx, id, *manual, updatedBy)
 	}
-	if _, err := s.get(ctx, id); err != nil {
+	cur, err := s.get(ctx, id)
+	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
+	}
 	groundCtx, cancel := context.WithTimeout(ctx, groundTimeout)
 	defer cancel()
 	gr, err := s.grounder.GroundURL(groundCtx, link)
@@ -349,30 +374,57 @@ func (s *Service) Ground(ctx context.Context, id string, f Fix, updatedBy string
 		if ctx.Err() == nil && errors.Is(groundCtx.Err(), context.DeadlineExceeded) {
 			return Dinner{}, ErrGroundTimeout
 		}
-		return Dinner{}, fmt.Errorf("dinners: ground %s: %w", id, err)
+		return Dinner{}, fmt.Errorf("%w: dinner %s: %w", ErrGroundFailed, id, err)
 	}
 	row, err := db.New(s.st.Writer).GroundDinner(ctx, db.GroundDinnerParams{
-		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},
-		GroundingStatus: GroundingGrounded,
-		UpdatedBy:       sql.NullString{String: updatedBy, Valid: true},
-		UpdatedAt:       store.Now().String(),
-		ID:              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},
+		GroundingStatus:           GroundingGrounded,
+		UpdatedBy:                 sql.NullString{String: updatedBy, Valid: true},
+		UpdatedAt:                 store.Now().String(),
+		ID:                        cur.ID,
+		ObservedName:              cur.Name,
+		ObservedNormalizedName:    cur.NormalizedName,
+		ObservedGroundingStatus:   cur.GroundingStatus,
+		ObservedGroundingRevision: cur.GroundingRevision,
+		ObservedRecipeID:          cur.RecipeID,
+		ObservedMethod:            cur.Method,
+		ObservedMinutesTotal:      cur.MinutesTotal,
+		ObservedDifficulty:        cur.Difficulty,
 	})
 	if errors.Is(err, sql.ErrNoRows) {
-		return Dinner{}, ErrNotFound
+		return Dinner{}, s.groundRejected(ctx, id)
 	}
 	if err != nil {
-		return Dinner{}, fmt.Errorf("dinners: ground %s: %w", id, err)
+		return Dinner{}, fmt.Errorf("%w: dinner %s: record: %w", ErrGroundFailed, id, err)
 	}
 	return fromRow(row), nil
 }

+// groundRejected is the error for dinner id when it was not as observed
+// before grounding: ErrNotFound, ErrArchived or ErrGroundConflict. Failing
+// to read it wraps ErrGroundFailed.
+func (s *Service) groundRejected(ctx context.Context, id string) error {
+	cur, err := s.get(ctx, id)
+	if errors.Is(err, ErrNotFound) {
+		return err
+	}
+	if err != nil {
+		return fmt.Errorf("%w: dinner %s: %w", ErrGroundFailed, id, err)
+	}
+	if cur.Status == StatusArchived {
+		return ErrArchived
+	}
+	return ErrGroundConflict
+}
+
 // 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 {
@@ -384,6 +436,9 @@ func (s *Service) groundManual(ctx context.Context, id string, m ManualRecipe, u
 		if err != nil {
 			return fmt.Errorf("dinners: read %s: %w", id, err)
 		}
+		if cur.Status == StatusArchived {
+			return ErrArchived
+		}
 		ld, err := json.Marshal(manualJSONLD{
 			Context:      "https://schema.org",
 			Type:         "Recipe",
@@ -439,23 +494,34 @@ func (s *Service) groundManual(ctx context.Context, id string, m ManualRecipe, u
 			}
 		}
 		row, err = q.GroundDinner(ctx, db.GroundDinnerParams{
-			RecipeID:        sql.NullString{String: rec.ID, Valid: true},
-			Method:          method,
-			MinutesTotal:    minutes,
-			Difficulty:      cur.Difficulty,
-			GroundingStatus: GroundingManual,
-			UpdatedBy:       sql.NullString{String: updatedBy, Valid: true},
-			UpdatedAt:       now,
-			ID:              cur.ID,
+			RecipeID:                  sql.NullString{String: rec.ID, Valid: true},
+			Method:                    method,
+			MinutesTotal:              minutes,
+			Difficulty:                cur.Difficulty,
+			GroundingStatus:           GroundingManual,
+			UpdatedBy:                 sql.NullString{String: updatedBy, Valid: true},
+			UpdatedAt:                 now,
+			ID:                        cur.ID,
+			ObservedName:              cur.Name,
+			ObservedNormalizedName:    cur.NormalizedName,
+			ObservedGroundingStatus:   cur.GroundingStatus,
+			ObservedGroundingRevision: cur.GroundingRevision,
+			ObservedRecipeID:          cur.RecipeID,
+			ObservedMethod:            cur.Method,
+			ObservedMinutesTotal:      cur.MinutesTotal,
+			ObservedDifficulty:        cur.Difficulty,
 		})
 		if err != nil {
 			return fmt.Errorf("dinners: ground %s: %w", id, err)
 		}
 		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
 }

@@ -1106,3 +1106,204 @@ func TestGroundTimesOut(t *testing.T) {
 		t.Errorf("grounding_status = %s, want failed as before", row.GroundingStatus)
 	}
 }
+
+// TestFixInFlightWhenDinnerChanges: a link still grounding when its dinner
+// is renamed, archived or grounded some other way writes nothing; the
+// change made meanwhile is kept.
+func TestFixInFlightWhenDinnerChanges(t *testing.T) {
+	t.Parallel()
+	tests := []struct {
+		name   string
+		change func(t *testing.T, e *env, id string)
+		want   error
+		// kept checks the dinner shows the change, not the link's recipe.
+		kept func(row db.Dinner) bool
+	}{
+		{"renamed", func(t *testing.T, e *env, id string) {
+			if _, err := e.svc.Update(t.Context(), id, dinners.Patch{Name: ptr("Chicken curry")}, member); err != nil {
+				t.Fatal(err)
+			}
+		}, dinners.ErrGroundConflict, func(row db.Dinner) bool {
+			return row.Name == "Chicken curry" && row.GroundingStatus == dinners.GroundingFailed && !row.RecipeID.Valid
+		}},
+		{"renamed to the same normalized name", func(t *testing.T, e *env, id string) {
+			if _, err := e.svc.Update(t.Context(), id, dinners.Patch{Name: ptr("Easy Grandma's stew")}, member); err != nil {
+				t.Fatal(err)
+			}
+		}, dinners.ErrGroundConflict, func(row db.Dinner) bool {
+			return row.Name == "Easy Grandma's stew" && row.GroundingStatus == dinners.GroundingFailed && !row.RecipeID.Valid
+		}},
+		{"archived", func(t *testing.T, e *env, id string) {
+			if _, err := e.svc.Archive(t.Context(), id, member); err != nil {
+				t.Fatal(err)
+			}
+		}, dinners.ErrArchived, func(row db.Dinner) bool {
+			return row.Status == dinners.StatusArchived && row.GroundingStatus == dinners.GroundingFailed && !row.RecipeID.Valid
+		}},
+		{"grounded by hand", func(t *testing.T, e *env, id string) {
+			if _, err := e.svc.Ground(t.Context(), id, dinners.Fix{Manual: &dinners.ManualRecipe{
+				Ingredients: []string{"1 lb beef"}, Minutes: ptr(240), Method: ptr("crockpot"),
+			}}, member); err != nil {
+				t.Fatal(err)
+			}
+		}, dinners.ErrGroundConflict, func(row db.Dinner) bool {
+			return row.GroundingStatus == dinners.GroundingManual && row.Method.String == "crockpot"
+		}},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(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{})
+
+			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)
+			tt.change(t, e, id)
+			close(e.pages.Gate)
+
+			if err := <-done; !errors.Is(err, tt.want) {
+				t.Fatalf("err = %v, want %v", err, tt.want)
+			}
+			if row := e.dinner(t, id); !tt.kept(row) {
+				t.Errorf("dinner = %+v, want the %s change kept and the link's recipe not written", row, tt.name)
+			}
+		})
+	}
+}
+
+// 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()
+	// 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, tt.edit, member); err != nil {
+				t.Fatal(err)
+			}
+			close(e.pages.Gate)
+
+			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)
+			}
+		})
+	}
+}
+
+// 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 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.
+func TestFixArchivedDinner(t *testing.T) {
+	t.Parallel()
+	e := newEnv(t)
+	id := failedDinner(t, e, "Grandma's stew")
+	if _, err := e.svc.Archive(t.Context(), id, member); err != nil {
+		t.Fatal(err)
+	}
+	before := e.dinner(t, id)
+	for _, f := range []dinners.Fix{
+		{RecipeURL: ptr(stew)},
+		{Manual: &dinners.ManualRecipe{Ingredients: []string{"1 lb beef"}, Minutes: ptr(240), Method: ptr("crockpot")}},
+	} {
+		if _, err := e.svc.Ground(t.Context(), id, f, member); !errors.Is(err, dinners.ErrArchived) {
+			t.Errorf("fix %+v: err = %v, want ErrArchived", f, err)
+		}
+	}
+	if got := e.pages.Fetched(); len(got) != 0 {
+		t.Errorf("fetched %q for an archived dinner", got)
+	}
+	if after := e.dinner(t, id); after != before {
+		t.Errorf("archived dinner changed: %+v, was %+v", after, before)
+	}
+}
+
+// waitFetched waits until rawURL has been fetched.
+func (e *env) waitFetched(t *testing.T, rawURL string) {
+	t.Helper()
+	deadline := time.Now().Add(10 * time.Second)
+	for !slices.Contains(e.pages.Fetched(), rawURL) {
+		if time.Now().After(deadline) {
+			t.Fatalf("%s never fetched", rawURL)
+		}
+		time.Sleep(5 * time.Millisecond)
+	}
+}