From 24d91b5bf65c4795b114546650eaadff9ace4c19 Mon Sep 17 00:00:00 2001 From: dev_agent Date: Mon, 5 Oct 2026 15:35:37 -0400 Subject: [PATCH] Rank stored candidates on their stored recipe without refetching Ground no longer fetches the page of a candidate whose source URL is already stored: the stored recipe, its photo, ingredients and steps are scored as a fetched page would be, so ranking still decides between it and new candidates, and a stored page that is now unavailable can still be reused. Rank's fetch-and-extract is split out behind a loader so Rank and Ground share one scoring path. The reuse test removes the stored page and checks that only the new candidate is fetched; the ranking regression test lists the unrelated stored recipe first so search order cannot pass it. Co-Authored-By: Claude Opus 5.5 --- internal/recipes/ground.go | 118 +++++++++++++++++++++++++++----- internal/recipes/ground_test.go | 21 ++++-- internal/recipes/rank.go | 29 +++++--- 3 files changed, 136 insertions(+), 32 deletions(-) diff --git a/internal/recipes/ground.go b/internal/recipes/ground.go index d10ca8f..59b304e 100644 --- a/internal/recipes/ground.go +++ b/internal/recipes/ground.go @@ -3,6 +3,7 @@ package recipes import ( "context" "database/sql" + "encoding/json" "errors" "fmt" "strings" @@ -102,11 +103,13 @@ func NewGrounder(st *store.Store, search SearchSource, pages PageFetcher, photos } // Ground searches for query and grounds dishName in the best recipe found. -// 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. +// The candidates are ranked as Rank does, except that a candidate already +// stored is scored on its stored recipe and never fetched. They are tried +// best first: a stored candidate 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,7 +129,23 @@ func (g *Grounder) Ground(ctx context.Context, dishName, query string) (Grounded if len(cands) == 0 { return Grounded{}, &ErrUngroundable{Target: dishName, Reasons: []error{ErrNoCandidates}} } - ranked, rankErr := Rank(ctx, g.pages, dishName, cands) + cands = cands[:min(len(cands), RankDepth)] + stored := make(map[string]storedRecipe) + for _, c := range cands { + sr, ok, err := g.storedRecipe(ctx, c.URL) + if err != nil { + return Grounded{}, fmt.Errorf("recipes: ground %q: %w", dishName, err) + } + if ok { + stored[c.URL] = sr + } + } + ranked, rankErr := rank(ctx, dishName, cands, func(ctx context.Context, c Candidate) (Recipe, error) { + if sr, ok := stored[c.URL]; ok { + return sr.recipe, nil + } + return fetchRecipe(ctx, g.pages, c.URL) + }) if err := ctx.Err(); err != nil { return Grounded{}, err } @@ -135,8 +154,8 @@ 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 + if sr, ok := stored[r.Candidate.URL]; ok { + return sr.grounded, nil } gr, reason, err := g.ground(ctx, r.Candidate.URL, r.Recipe) if err != nil { @@ -227,19 +246,76 @@ func (g *Grounder) GroundMany(ctx context.Context, reqs []GroundRequest) []Groun // stored returns the grounding of the recipe stored for sourceURL, and // false when there is none. func (g *Grounder) stored(ctx context.Context, sourceURL string) (Grounded, bool, error) { - row, err := db.New(g.st.Reader).GetRecipeBySourceURL(ctx, sourceURL) - if errors.Is(err, sql.ErrNoRows) { - return Grounded{}, false, nil - } - if err != nil { - return Grounded{}, false, fmt.Errorf("recipes: look up recipe %s: %w", sourceURL, err) - } - if !row.PhotoID.Valid || !row.Method.Valid || !row.Difficulty.Valid { - return Grounded{}, false, fmt.Errorf("recipes: stored recipe %s for %s has no photo, method or difficulty", row.ID, sourceURL) + row, ok, err := g.storedRow(ctx, sourceURL) + if err != nil || !ok { + return Grounded{}, false, err } return groundedFrom(row), true, nil } +// storedRow returns the recipe row stored for sourceURL, and false when +// there is none. +func (g *Grounder) storedRow(ctx context.Context, sourceURL string) (db.Recipe, bool, error) { + row, err := db.New(g.st.Reader).GetRecipeBySourceURL(ctx, sourceURL) + if errors.Is(err, sql.ErrNoRows) { + return db.Recipe{}, false, nil + } + if err != nil { + return db.Recipe{}, false, fmt.Errorf("recipes: look up recipe %s: %w", sourceURL, err) + } + if !row.PhotoID.Valid || !row.Method.Valid || !row.Difficulty.Valid { + return db.Recipe{}, false, fmt.Errorf("recipes: stored recipe %s for %s has no photo, method or difficulty", row.ID, sourceURL) + } + return row, true, nil +} + +// storedRecipe is a recipe already stored: its grounding, and the recipe +// rebuilt from what was stored so it can be ranked without its page. +type storedRecipe struct { + grounded Grounded + recipe Recipe +} + +// storedRecipe returns the recipe stored for sourceURL with its photo, +// ingredients and steps, and false when there is none. +func (g *Grounder) storedRecipe(ctx context.Context, sourceURL string) (storedRecipe, bool, error) { + row, ok, err := g.storedRow(ctx, sourceURL) + if err != nil || !ok { + return storedRecipe{}, false, err + } + gr := groundedFrom(row) + q := db.New(g.st.Reader) + photo, err := q.GetPhoto(ctx, gr.PhotoID) + if err != nil { + return storedRecipe{}, false, fmt.Errorf("recipes: load photo %s of recipe %s: %w", gr.PhotoID, gr.RecipeID, err) + } + ings, err := q.ListRecipeIngredients(ctx, gr.RecipeID) + if err != nil { + return storedRecipe{}, false, fmt.Errorf("recipes: load ingredients of recipe %s: %w", gr.RecipeID, err) + } + steps, err := q.ListRecipeSteps(ctx, gr.RecipeID) + if err != nil { + return storedRecipe{}, false, fmt.Errorf("recipes: load steps of recipe %s: %w", gr.RecipeID, err) + } + r := Recipe{ + Title: row.Title, + SiteName: row.SiteName.String, + ImageURL: photo.SourceUrl, + PrepMinutes: intFrom(row.PrepMinutes), + CookMinutes: intFrom(row.CookMinutes), + TotalMinutes: intFrom(row.TotalMinutes), + Yield: row.Yield.String, + JSONLD: json.RawMessage(row.RawJsonld), + } + for _, ing := range ings { + r.Ingredients = append(r.Ingredients, ing.RawText) + } + for _, st := range steps { + r.Steps = append(r.Steps, st.Text) + } + return storedRecipe{grounded: gr, recipe: r}, true, nil +} + // ground downloads r's photo, normalizes it and stores it as sourceURL's // recipe. A problem with this recipe is returned as reason, so the caller // can try another; err is a failure that is not the recipe's. @@ -374,6 +450,14 @@ func groundedFrom(row db.Recipe) Grounded { return gr } +func intFrom(v sql.NullInt64) *int { + if !v.Valid { + return nil + } + n := int(v.Int64) + return &n +} + func nullInt(v *int) sql.NullInt64 { if v == nil { return sql.NullInt64{} diff --git a/internal/recipes/ground_test.go b/internal/recipes/ground_test.go index bec4655..8bde81a 100644 --- a/internal/recipes/ground_test.go +++ b/internal/recipes/ground_test.go @@ -319,7 +319,9 @@ func TestGroundReusesStoredRecipe(t *testing.T) { } pages, images, calls := env.pages.fetchCount(), env.images.requests.Load(), len(env.llm.Requests()) - // The model has no reply left: a second normalization would fail. + // The stored page is gone and the model has no reply left: only the + // stored recipe can ground the dish. + delete(env.pages.pages, "https://b.example/curry") again, err := env.g.Ground(t.Context(), "Chicken Curry", "chicken curry") if err != nil { t.Fatalf("second Ground: %v", err) @@ -332,9 +334,13 @@ func TestGroundReusesStoredRecipe(t *testing.T) { 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) + // Ranking fetches only the candidate not stored; the stored one ranks + // on its stored recipe. + env.pages.mu.Lock() + fetched := append([]string(nil), env.pages.fetched[pages:]...) + env.pages.mu.Unlock() + if len(fetched) != 1 || fetched[0] != "https://a.example/other" { + t.Errorf("reuse fetched %v, want only https://a.example/other", fetched) } pages = env.pages.fetchCount() @@ -354,7 +360,7 @@ 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"}}, + "chicken curry": {{URL: "https://b.example/brownies"}, {URL: "https://a.example/curry"}}, }} env := newGroundEnv(t, map[string][]byte{ @@ -373,8 +379,9 @@ func TestGroundStoredRecipeDoesNotOutrankRelevantOne(t *testing.T) { t.Fatalf("Ground brownies: %v", err) } - // Search lists the relevant new recipe first and the unrelated stored - // one second: the relevant one is grounded. + // Search lists the unrelated stored recipe first and the relevant new + // one second: ranking, not search order or storage, grounds the + // relevant one. curry, err := env.g.Ground(t.Context(), "Chicken Curry", "chicken curry") if err != nil { t.Fatalf("Ground chicken curry: %v", err) diff --git a/internal/recipes/rank.go b/internal/recipes/rank.go index 07310e9..69ed9ce 100644 --- a/internal/recipes/rank.go +++ b/internal/recipes/rank.go @@ -57,6 +57,14 @@ type Ranked struct { // unranked candidate's URL and reason. A cancelled ctx returns only its // error. func Rank(ctx context.Context, f PageFetcher, dish string, cands []Candidate) ([]Ranked, error) { + return rank(ctx, dish, cands, func(ctx context.Context, c Candidate) (Recipe, error) { + return fetchRecipe(ctx, f, c.URL) + }) +} + +// rank is Rank with each candidate's recipe got by load rather than always +// fetched. +func rank(ctx context.Context, dish string, cands []Candidate, load func(context.Context, Candidate) (Recipe, error)) ([]Ranked, error) { if len(cands) > RankDepth { cands = cands[:RankDepth] } @@ -70,7 +78,7 @@ func Rank(ctx context.Context, f PageFetcher, dish string, cands []Candidate) ([ wg.Add(1) go func() { defer wg.Done() - r, err := rankOne(ctx, f, dish, c) + r, err := rankOne(ctx, load, dish, c) outcomes[i] = outcome{r, err} }() } @@ -98,14 +106,10 @@ func Rank(ctx context.Context, f PageFetcher, dish string, cands []Candidate) ([ return ranked, nil } -// rankOne fetches, extracts and scores one candidate, or says why it is +// rankOne loads and scores one candidate's recipe, or says why it is // rejected. -func rankOne(ctx context.Context, f PageFetcher, dish string, c Candidate) (Ranked, error) { - page, err := f.Fetch(ctx, c.URL) - if err != nil { - return Ranked{}, err - } - r, err := Extract(page.Body, page.URL) +func rankOne(ctx context.Context, load func(context.Context, Candidate) (Recipe, error), dish string, c Candidate) (Ranked, error) { + r, err := load(ctx, c) if err != nil { return Ranked{}, err } @@ -115,6 +119,15 @@ func rankOne(ctx context.Context, f PageFetcher, dish string, c Candidate) (Rank return Ranked{Candidate: c, Recipe: r, Score: harness.Similarity(r.Title, dish) + completeness(r)}, nil } +// fetchRecipe fetches the page at rawURL and extracts its recipe. +func fetchRecipe(ctx context.Context, f PageFetcher, rawURL string) (Recipe, error) { + page, err := f.Fetch(ctx, rawURL) + if err != nil { + return Recipe{}, err + } + return Extract(page.Body, page.URL) +} + // usable says why r cannot ground a dish: it has no ingredients or no // image, or a total time that is not positive. func usable(r Recipe) error {