internal/recipes/ground.go

@@ -102,11 +102,11 @@ func NewGrounder(st *store.Store, search SearchSource, pages PageFetcher, photos
 }
 
 // Ground searches for query and grounds dishName in the best recipe found.
-// When one of the top RankDepth candidates is already stored, the first
+// The candidates are ranked (see Rank) and tried best first: a candidate
-// such is returned without fetching anything. Otherwise the candidates are
+// already stored is returned as it is, and otherwise the first whose photo
-// ranked (see Rank) and tried best first: the first whose photo can be
+// can be cached and whose ingredients normalize is stored — the recipe, its
-// cached and whose ingredients normalize is stored — the recipe, its
+// ingredients, steps and photo in one transaction — and returned. A stored
-// ingredients, steps and photo in one transaction — and returned.
+// recipe is reused only when it ranks above every candidate that grounds.
 //
 // When no candidate grounds the dish the error is an *ErrUngroundable with
 // each reason. A failure that is not the candidates' — the search, the
@@ -126,12 +126,6 @@ func (g *Grounder) Ground(ctx context.Context, dishName, query string) (Grounded
 	if len(cands) == 0 {
 		return Grounded{}, &ErrUngroundable{Target: dishName, Reasons: []error{ErrNoCandidates}}
 	}
-	for _, c := range cands {
-		if gr, ok, err := g.stored(ctx, c.URL); err != nil || ok {
-			return gr, err
-		}
-	}
-
 	ranked, rankErr := Rank(ctx, g.pages, dishName, cands)
 	if err := ctx.Err(); err != nil {
 		return Grounded{}, err
@@ -141,6 +135,9 @@ func (g *Grounder) Ground(ctx context.Context, dishName, query string) (Grounded
 		reasons = append(reasons, rankErr)
 	}
 	for _, r := range ranked {
+		if gr, ok, err := g.stored(ctx, r.Candidate.URL); err != nil || ok {
+			return gr, err
+		}
 		gr, reason, err := g.ground(ctx, r.Candidate.URL, r.Recipe)
 		if err != nil {
 			return Grounded{}, fmt.Errorf("recipes: ground %q: %w", dishName, err)

internal/recipes/ground_test.go

@@ -328,10 +328,15 @@ func TestGroundReusesStoredRecipe(t *testing.T) {
 		again.Difficulty != first.Difficulty || *again.MinutesTotal != *first.MinutesTotal || again.SiteName != first.SiteName {
 		t.Errorf("reused grounding = %+v, want %+v", again, first)
 	}
-	if env.pages.fetchCount() != pages || env.images.requests.Load() != images || len(env.llm.Requests()) != calls {
+	if env.images.requests.Load() != images || len(env.llm.Requests()) != calls {
-		t.Errorf("reuse fetched pages %d→%d, images %d→%d, model calls %d→%d; want none",
+		t.Errorf("reuse fetched images %d→%d, model calls %d→%d; want none",
-			pages, env.pages.fetchCount(), images, env.images.requests.Load(), calls, len(env.llm.Requests()))
+			images, env.images.requests.Load(), calls, len(env.llm.Requests()))
 	}
+	// Ranking fetches both candidates' pages; only the stored one ranks.
+	if n := env.pages.fetchCount() - pages; n != 2 {
+		t.Errorf("reuse fetched %d pages, want the 2 candidates ranked", n)
+	}
+	pages = env.pages.fetchCount()
 
 	byURL, err := env.g.GroundURL(t.Context(), "https://b.example/curry")
 	if err != nil || byURL.RecipeID != first.RecipeID {
@@ -345,6 +350,44 @@ func TestGroundReusesStoredRecipe(t *testing.T) {
 	}
 }
 
+func TestGroundStoredRecipeDoesNotOutrankRelevantOne(t *testing.T) {
+	t.Parallel()
+	search := &fakeSearch{results: map[string][]Candidate{
+		"brownies":      {{URL: "https://b.example/brownies"}},
+		"chicken curry": {{URL: "https://a.example/curry"}, {URL: "https://b.example/brownies"}},
+	}}
+	env := newGroundEnv(t,
+		map[string][]byte{
+			"https://a.example/curry":    groundPage("Chicken Curry", "/img/curry.jpg"),
+			"https://b.example/brownies": groundPage("Fudgy Brownies", "/img/brownies.jpg"),
+		},
+		map[string]photoFile{
+			"/img/curry.jpg":    jpegFile(t, 640, 480),
+			"/img/brownies.jpg": jpegFile(t, 640, 480),
+		},
+		search)
+	env.llm.QueueGenerate(normalizedReply(), normalizedReply())
+
+	brownies, err := env.g.Ground(t.Context(), "Brownies", "brownies")
+	if err != nil {
+		t.Fatalf("Ground brownies: %v", err)
+	}
+
+	// Search lists the relevant new recipe first and the unrelated stored
+	// one second: the relevant one is grounded.
+	curry, err := env.g.Ground(t.Context(), "Chicken Curry", "chicken curry")
+	if err != nil {
+		t.Fatalf("Ground chicken curry: %v", err)
+	}
+	if curry.SourceURL != "https://a.example/curry" || curry.RecipeID == brownies.RecipeID {
+		t.Errorf("grounded Chicken Curry in %s (recipe %s), want https://a.example/curry, not the stored brownies %s",
+			curry.SourceURL, curry.RecipeID, brownies.RecipeID)
+	}
+	if n := countRows(t, env.st, "recipes"); n != 2 {
+		t.Errorf("%d recipes rows, want 2", n)
+	}
+}
+
 func TestGroundURL(t *testing.T) {
 	t.Parallel()
 	env := newGroundEnv(t,