From 871a56680081c51eedd638b833cb79e19af9b111 Mon Sep 17 00:00:00 2001 From: dev_agent Date: Mon, 5 Oct 2026 15:18:29 -0400 Subject: [PATCH] Rank stored candidates instead of reusing them first Ground returned the first search result that was already stored before fetching or ranking anything, so an unrelated stored recipe could win over a relevant new one. Ground now ranks every candidate and walks the ranking best first, reusing a stored recipe only at its ranked place. Co-Authored-By: Claude Opus 5.5 --- internal/recipes/ground.go | 19 ++++++------- internal/recipes/ground_test.go | 49 +++++++++++++++++++++++++++++++-- 2 files changed, 54 insertions(+), 14 deletions(-) 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,