diff --git a/internal/recipes/ground.go b/internal/recipes/ground.go index e5b163e..d10ca8f 100644 --- a/internal/recipes/ground.go +++ b/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 -// such is returned without fetching anything. Otherwise the candidates are -// ranked (see Rank) and tried best first: the first whose photo can be -// cached and whose ingredients normalize is stored — the recipe, its -// ingredients, steps and photo in one transaction — and returned. +// The candidates are ranked (see Rank) and tried best first: a candidate +// already stored is returned as it is, and otherwise the first whose photo +// can be cached and whose ingredients normalize is stored — the recipe, its +// ingredients, steps and photo in one transaction — and returned. A stored +// 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) diff --git a/internal/recipes/ground_test.go b/internal/recipes/ground_test.go index e3cec25..bec4655 100644 --- a/internal/recipes/ground_test.go +++ b/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 { - t.Errorf("reuse fetched pages %d→%d, images %d→%d, model calls %d→%d; want none", - pages, env.pages.fetchCount(), images, env.images.requests.Load(), calls, len(env.llm.Requests())) + if env.images.requests.Load() != images || len(env.llm.Requests()) != calls { + t.Errorf("reuse fetched images %d→%d, model calls %d→%d; want none", + 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,