Tighten normalize_recipe output checks

Address review findings on the normalize_recipe skill:

- Ingredient order: an ingredient is out of order when its name has more
  of its words in another line than in its own, not merely when it
  shares no word with its own line. Swapping "olive oil" and
  "sesame oil" now fails validation and triggers the repair turn.
- Required fields: Ingredient and NormalizedRecipe decode through
  UnmarshalJSON that requires every schema field and rejects unknown
  ones; qty, unit and note may not be omitted or null. Such a reply
  fails to decode and triggers the repair turn.
- Zero total: a present JSON-LD TotalMinutes that is not positive is
  rejected by PromptInput before any model call, instead of falling
  back to the model's minutes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

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

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"},
 	} {