diff --git a/internal/ai/skills/normalize_recipe.go b/internal/ai/skills/normalize_recipe.go index e370d30..dce18ba 100644 --- a/internal/ai/skills/normalize_recipe.go +++ b/internal/ai/skills/normalize_recipe.go @@ -3,6 +3,7 @@ package skills import ( + "bytes" "context" "encoding/json" "errors" @@ -27,14 +28,17 @@ type RecipeText struct { // Steps are the instructions, in order; there may be none. Steps []string // PrepMinutes, CookMinutes and TotalMinutes are the JSON-LD times, - // nil when the page gives none. + // nil when the page gives none. A TotalMinutes that is present must be + // positive: a recipe cannot take no time, so NormalizeRecipe rejects a + // zero total rather than guess one. PrepMinutes *int CookMinutes *int TotalMinutes *int } // Ingredient is one normalized ingredient line. Qty, Unit and Note are -// empty when the line gives none. +// empty when the line gives none; the model must still give all four, so a +// reply leaving one out or null does not decode. type Ingredient struct { Name string `json:"name"` Qty string `json:"qty"` @@ -60,8 +64,8 @@ type NormalizedRecipe struct { // same page must normalize the same way every time. scope names the recipe, // e.g. its source URL (see harness.Run). // -// Values the code can derive win over the model's: a positive JSON-LD -// TotalMinutes is minutes_total; a title or step mentioning a slow cooker +// Values the code can derive win over the model's: the JSON-LD +// TotalMinutes, when given, is minutes_total; a title or step mentioning a slow cooker // or crock pot makes the method crockpot; and a difficulty the model left // out is derived from the step count and minutes. func NormalizeRecipe(ctx context.Context, cfg harness.Config, scope string, in RecipeText) (NormalizedRecipe, error) { @@ -100,6 +104,9 @@ func (normalizeRecipe) PromptInput(in RecipeText) (harness.PromptInput, error) { if len(in.Ingredients) == 0 { return harness.PromptInput{}, errors.New("recipe has no ingredient lines") } + if in.TotalMinutes != nil && *in.TotalMinutes <= 0 { + return harness.PromptInput{}, fmt.Errorf("recipe total time is %d minutes; a given total must be positive", *in.TotalMinutes) + } p := recipePrompt{Title: title} for i, line := range in.Ingredients { line = oneLine(line) @@ -161,6 +168,54 @@ func (normalizeRecipe) Schema() llm.JSONSchema { }`)} } +// UnmarshalJSON decodes an ingredient, requiring each of its four fields +// as the schema does: encoding/json would otherwise read a missing or null +// one as empty. +func (ing *Ingredient) UnmarshalJSON(data []byte) error { + if err := requireFields(data, nil, "name", "qty", "unit", "note"); err != nil { + return fmt.Errorf("ingredient %s", err) + } + type plain Ingredient + return json.Unmarshal(data, (*plain)(ing)) +} + +// UnmarshalJSON decodes the output, requiring each of its fields as the +// schema does. Only difficulty and minutes_total may be null. +func (r *NormalizedRecipe) UnmarshalJSON(data []byte) error { + if err := requireFields(data, []string{"difficulty", "minutes_total"}, "ingredients", "method", "difficulty", "minutes_total"); err != nil { + return fmt.Errorf("output %s", err) + } + type plain NormalizedRecipe + return json.Unmarshal(data, (*plain)(r)) +} + +// requireFields checks that the JSON object data has exactly the fields +// named, none of them null unless listed in nullable. +func requireFields(data []byte, nullable []string, fields ...string) error { + var obj map[string]json.RawMessage + if err := json.Unmarshal(data, &obj); err != nil { + return err + } + if obj == nil { + return errors.New("is null, not an object") + } + for _, f := range fields { + v, ok := obj[f] + if !ok { + return fmt.Errorf("has no %q field; give every field the schema requires", f) + } + if bytes.Equal(bytes.TrimSpace(v), []byte("null")) && !slices.Contains(nullable, f) { + return fmt.Errorf("field %q is null, which the schema does not allow", f) + } + } + for f := range obj { + if !slices.Contains(fields, f) { + return fmt.Errorf("has unknown field %q", f) + } + } + return nil +} + var ( methods = []string{"stovetop", "oven", "crockpot", "grill", "other"} difficulties = []string{"easy", "medium", "hard"} @@ -194,22 +249,26 @@ func (s normalizeRecipe) Validate(out NormalizedRecipe) []string { } // orderProblems finds ingredients out of input order: an ingredient whose -// name shares no word with its own line but does with another line. -// Ingredients that match no line at all, e.g. a renamed one, are accepted. +// name has more of its words in another line than in its own. Sharing a +// word is not enough: "sesame oil" on the line "2 tbsp olive oil" has one +// of its two words there but both in "1 tbsp sesame oil", so it is out of +// order. Ingredients that match no line at all, e.g. a renamed one, are +// accepted, as are ties, e.g. a repeated line. func orderProblems(ings []Ingredient, lines []string) []string { var problems []string for i, ing := range ings { name := words(ing.Name) - if len(name) == 0 || sharesWord(name, words(lines[i])) { - continue - } + own := matchedWords(name, words(lines[i])) + best, bestLine := own, i for j, line := range lines { - if j != i && sharesWord(name, words(line)) { - problems = append(problems, fmt.Sprintf("ingredient %d %q matches line %d %q, not line %d %q; keep the ingredients in line order", - i+1, ing.Name, j+1, line, i+1, lines[i])) - break + if n := matchedWords(name, words(line)); n > best { + best, bestLine = n, j } } + if bestLine != i { + problems = append(problems, fmt.Sprintf("ingredient %d %q matches line %d %q, not line %d %q; keep the ingredients in line order", + i+1, ing.Name, bestLine+1, lines[bestLine], i+1, lines[i])) + } } return problems } @@ -225,17 +284,16 @@ func words(s string) []string { return out } -// sharesWord reports whether a word of a and a word of b are the same up -// to a suffix, as "tomato" and "tomatoes" are. -func sharesWord(a, b []string) bool { - for _, x := range a { - for _, y := range b { - if strings.HasPrefix(x, y) || strings.HasPrefix(y, x) { - return true - } +// matchedWords counts the words of name that are in line, up to a suffix, +// as "tomato" and "tomatoes" are the same word. +func matchedWords(name, line []string) int { + n := 0 + for _, x := range name { + if slices.ContainsFunc(line, func(y string) bool { return strings.HasPrefix(x, y) || strings.HasPrefix(y, x) }) { + n++ } } - return false + return n } // slowCooker matches a mention of a slow cooker or crock pot. @@ -269,9 +327,10 @@ func (s normalizeRecipe) PostProcess(out NormalizedRecipe) (NormalizedRecipe, er } // jsonLDMinutes returns a copy of the JSON-LD total time, or nil when the -// page gives no positive one. +// page gives none. PromptInput has already rejected a total that is not +// positive. func jsonLDMinutes(in RecipeText) *int { - if in.TotalMinutes == nil || *in.TotalMinutes <= 0 { + if in.TotalMinutes == nil { return nil } m := *in.TotalMinutes diff --git a/internal/ai/skills/normalize_recipe_test.go b/internal/ai/skills/normalize_recipe_test.go index 4cba44a..04a95ab 100644 --- a/internal/ai/skills/normalize_recipe_test.go +++ b/internal/ai/skills/normalize_recipe_test.go @@ -246,6 +246,99 @@ func TestNormalizeRecipeUnrepairedFails(t *testing.T) { } } +// stirFry has two ingredient lines that share the word "oil". +var stirFry = RecipeText{ + Title: "Vegetable Stir Fry", + Ingredients: []string{"2 tbsp olive oil", "1 tbsp sesame oil"}, + Steps: []string{"Stir-fry the vegetables in the olive oil, then finish with the sesame oil."}, +} + +const stirFryReply = `{"ingredients":[ + {"name":"olive oil","qty":"2","unit":"tbsp","note":""}, + {"name":"sesame oil","qty":"1","unit":"tbsp","note":""}], + "method":"stovetop","difficulty":"easy","minutes_total":15}` + +func TestNormalizeRecipeSwapSharingAWordRepaired(t *testing.T) { + swapped := `{"ingredients":[ + {"name":"sesame oil","qty":"1","unit":"tbsp","note":""}, + {"name":"olive oil","qty":"2","unit":"tbsp","note":""}], + "method":"stovetop","difficulty":"easy","minutes_total":15}` + var f llm.Fake + f.QueueGenerate(reply(swapped), reply(stirFryReply)) + out, reqs, err := normalize(t, &f, stirFry, 2) + if err != nil { + t.Fatal(err) + } + assertOutput(t, out, NormalizedRecipe{ + Ingredients: []Ingredient{ + {Name: "olive oil", Qty: "2", Unit: "tbsp"}, + {Name: "sesame oil", Qty: "1", Unit: "tbsp"}, + }, + Method: "stovetop", + Difficulty: "easy", + MinutesTotal: minutes(15), + }) + for _, want := range []string{ + `ingredient 1 "sesame oil" matches line 2 "1 tbsp sesame oil", not line 1 "2 tbsp olive oil"; keep the ingredients in line order`, + `ingredient 2 "olive oil" matches line 1 "2 tbsp olive oil", not line 2 "1 tbsp sesame oil"; keep the ingredients in line order`, + } { + if repair := reqs[1].Messages[2].Content; !strings.Contains(repair, want) { + t.Fatalf("repair message = %q, want it to contain %q", repair, want) + } + } +} + +func TestNormalizeRecipeOmittedFieldRepaired(t *testing.T) { + for name, tc := range map[string]struct{ first, want string }{ + "omitted qty": { + strings.Replace(stirFryReply, `"qty":"1",`, ``, 1), + `ingredient has no "qty" field; give every field the schema requires`, + }, + "null note": { + strings.Replace(stirFryReply, `"tbsp","note":""}]`, `"tbsp","note":null}]`, 1), + `ingredient field "note" is null, which the schema does not allow`, + }, + "omitted unit": { + strings.Replace(stirFryReply, `"unit":"tbsp",`, ``, 1), + `ingredient has no "unit" field; give every field the schema requires`, + }, + "omitted minutes_total": { + strings.Replace(stirFryReply, `,"minutes_total":15`, ``, 1), + `output has no "minutes_total" field; give every field the schema requires`, + }, + "unknown field": { + strings.Replace(stirFryReply, `"note":""}]`, `"note":"","brand":"x"}]`, 1), + `ingredient has unknown field "brand"`, + }, + } { + t.Run(name, func(t *testing.T) { + if tc.first == stirFryReply { + t.Fatal("replacement did not change the reply") + } + var f llm.Fake + f.QueueGenerate(reply(tc.first), reply(stirFryReply)) + _, reqs, err := normalize(t, &f, stirFry, 2) + if err != nil { + t.Fatal(err) + } + if repair := reqs[1].Messages[2].Content; !strings.Contains(repair, tc.want) { + t.Fatalf("repair message = %q, want it to contain %q", repair, tc.want) + } + }) + } +} + +func TestNormalizeRecipeZeroTotalRejected(t *testing.T) { + in := garlicPasta + in.TotalMinutes = minutes(0) + var f llm.Fake + _, _, err := normalize(t, &f, in, 0) + want := "recipe total time is 0 minutes; a given total must be positive" + if err == nil || !strings.Contains(err.Error(), want) { + t.Fatalf("err = %v, want it to contain %q", err, want) + } +} + func TestValidate(t *testing.T) { s := normalizeRecipe{in: RecipeText{ Title: "Tomato soup", @@ -274,6 +367,13 @@ func TestValidate(t *testing.T) { `ingredient 2 "tomatoes" matches line 1 "4 ripe tomatoes", not line 2 "1 onion, diced"; keep the ingredients in line order`, }, }, + "swapped sharing a word": { + NormalizedRecipe{Ingredients: []Ingredient{{Name: "diced onion"}, {Name: "ripe tomato"}}, Method: "oven"}, + []string{ + `ingredient 1 "diced onion" matches line 2 "1 onion, diced", not line 1 "4 ripe tomatoes"; keep the ingredients in line order`, + `ingredient 2 "ripe tomato" matches line 1 "4 ripe tomatoes", not line 2 "1 onion, diced"; keep the ingredients in line order`, + }, + }, "too many and bad enums": { NormalizedRecipe{ Ingredients: []Ingredient{{Name: "tomato"}, {Name: "onion"}, {Name: ""}}, @@ -305,6 +405,7 @@ func TestPromptInputRejects(t *testing.T) { }{ "no title": {RecipeText{Title: " ", Ingredients: []string{"1 egg"}}, "recipe has no title"}, "no ingredients": {RecipeText{Title: "Egg"}, "recipe has no ingredient lines"}, + "negative total": {RecipeText{Title: "Egg", Ingredients: []string{"1 egg"}, TotalMinutes: minutes(-5)}, "recipe total time is -5 minutes; a given total must be positive"}, "blank line": {RecipeText{Title: "Egg", Ingredients: []string{"1 egg", "\t"}}, "ingredient line 2 is blank"}, "blank step": {RecipeText{Title: "Egg", Ingredients: []string{"1 egg"}, Steps: []string{""}}, "step 1 is blank"}, } {