From 9ed5cb119232bf67c07820454942ab9a8d5e2fd7 Mon Sep 17 00:00:00 2001 From: dev_agent Date: Sun, 4 Oct 2026 17:08:52 -0400 Subject: [PATCH 1/2] Add skill harness with one repair retry internal/ai/harness defines the generic Skill interface (Name, Version, Render, Schema, Validate, PostProcess) and Run, which renders the prompt, calls Provider.Generate with the skill's schema and the household temperature, strictly decodes and validates the reply, and on failure sends exactly one repair turn quoting the problems. A second failure returns a *BadOutputError matching llm.ErrBadOutput with both problem lists. Provider errors other than ErrBadOutput pass through unchanged. Maestro task 38e7bf98-f1d1-4301-b73b-7ec4b51e8b68. Co-Authored-By: Claude Opus 5.5 --- internal/ai/harness/echo_test.go | 51 +++++++++ internal/ai/harness/harness.go | 168 ++++++++++++++++++++++++++++ internal/ai/harness/harness_test.go | 160 ++++++++++++++++++++++++++ 3 files changed, 379 insertions(+) create mode 100644 internal/ai/harness/echo_test.go create mode 100644 internal/ai/harness/harness.go create mode 100644 internal/ai/harness/harness_test.go diff --git a/internal/ai/harness/echo_test.go b/internal/ai/harness/echo_test.go new file mode 100644 index 0000000..f30975d --- /dev/null +++ b/internal/ai/harness/echo_test.go @@ -0,0 +1,51 @@ +package harness + +import ( + "encoding/json" + "errors" + "strings" + + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/ai/llm" +) + +// echoSkill is the trivial test skill: it asks the model to repeat a word. +type echoSkill struct{} + +type echoInput struct{ Word string } + +type echoOutput struct { + Echo string `json:"echo"` +} + +var _ Skill[echoInput, echoOutput] = echoSkill{} + +func (echoSkill) Name() string { return "echo" } + +func (echoSkill) Version() int { return 1 } + +func (echoSkill) Render(in echoInput) (Prompt, error) { + if in.Word == "" { + return Prompt{}, errors.New("no word to echo") + } + return Prompt{System: "You repeat words.", User: "Repeat: " + in.Word}, nil +} + +func (echoSkill) Schema() llm.JSONSchema { + return llm.JSONSchema{Name: "echo", Schema: json.RawMessage( + `{"type":"object","properties":{"echo":{"type":"string"}},"required":["echo"],"additionalProperties":false}`)} +} + +func (echoSkill) Validate(out echoOutput) []string { + if out.Echo == "" { + return []string{"echo must not be empty"} + } + if out.Echo != strings.ToLower(out.Echo) { + return []string{"echo must be lower case"} + } + return nil +} + +func (echoSkill) PostProcess(out echoOutput) (echoOutput, error) { + out.Echo = strings.TrimSpace(out.Echo) + return out, nil +} diff --git a/internal/ai/harness/harness.go b/internal/ai/harness/harness.go new file mode 100644 index 0000000..c7015b1 --- /dev/null +++ b/internal/ai/harness/harness.go @@ -0,0 +1,168 @@ +// Package harness runs skills: versioned, typed prompts whose structured +// output Go validates. A run renders the skill's prompt, asks the provider +// for output matching the skill's schema, decodes and validates the reply, +// and on failure sends exactly one repair turn quoting the problems. If the +// repair fails too the run fails with llm.ErrBadOutput; there is no silent +// fallback (D-007). +package harness + +import ( + "bytes" + "context" + "encoding/json" + "errors" + "fmt" + "io" + "strings" + + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/ai/llm" +) + +// Prompt is a rendered skill prompt. Prompt layering builds on it later. +type Prompt struct { + // System is the system prompt; empty means none. + System string + // User is the user message. It must not be empty. + User string +} + +// Skill is one versioned AI task with input In and structured output Out. +type Skill[In, Out any] interface { + // Name identifies the skill, e.g. "side_quests". + Name() string + // Version is bumped whenever the prompt, schema or validation changes. + Version() int + // Render builds the prompt for in. + Render(in In) (Prompt, error) + // Schema is the JSON Schema the reply must match; Out decodes from it. + Schema() llm.JSONSchema + // Validate returns every problem with out, or none when it is usable. + // The problems are sent to the model in the repair turn, so each must + // say what to fix. + Validate(out Out) []string + // PostProcess finishes a valid out, e.g. normalising names. Its error + // fails the run without a repair turn. + PostProcess(out Out) (Out, error) +} + +// Config is what a run needs from the household's AI settings. +type Config struct { + // Provider answers the requests. + Provider llm.Provider + // Temperature is the household's sampling temperature, sent with + // every request. + Temperature float64 +} + +// BadOutputError is a run whose reply and repair reply were both unusable. +// It matches llm.ErrBadOutput. +type BadOutputError struct { + // Skill and Version identify the skill that ran. + Skill string + Version int + // First lists the problems with the first reply, Repair those with + // the reply to the repair turn. + First []string + Repair []string +} + +func (e *BadOutputError) Error() string { + return fmt.Sprintf("harness: skill %s v%d: %s: first reply: %s; repair reply: %s", + e.Skill, e.Version, llm.ErrBadOutput, strings.Join(e.First, "; "), strings.Join(e.Repair, "; ")) +} + +func (e *BadOutputError) Unwrap() error { return llm.ErrBadOutput } + +// Run runs skill s on in with cfg and returns its validated, post-processed +// output. A provider error other than llm.ErrBadOutput is returned +// unchanged and never repaired. +func Run[In, Out any](ctx context.Context, cfg Config, s Skill[In, Out], in In) (Out, error) { + var zero Out + prompt, err := s.Render(in) + if err != nil { + return zero, fmt.Errorf("harness: skill %s v%d: render: %w", s.Name(), s.Version(), err) + } + if prompt.User == "" { + return zero, fmt.Errorf("harness: skill %s v%d: render: empty user message", s.Name(), s.Version()) + } + schema := s.Schema() + temperature := cfg.Temperature + req := llm.Request{ + System: prompt.System, + Messages: []llm.Message{{Role: llm.RoleUser, Content: prompt.User}}, + Temperature: &temperature, + Schema: &schema, + } + + out, reply, first, err := attempt(ctx, cfg.Provider, s, req) + if err != nil { + return zero, err + } + if len(first) > 0 { + if reply != "" { + req.Messages = append(req.Messages, llm.Message{Role: llm.RoleAssistant, Content: reply}) + } + req.Messages = append(req.Messages, llm.Message{Role: llm.RoleUser, Content: repairMessage(schema.Name, first)}) + var second []string + out, _, second, err = attempt(ctx, cfg.Provider, s, req) + if err != nil { + return zero, err + } + if len(second) > 0 { + return zero, &BadOutputError{Skill: s.Name(), Version: s.Version(), First: first, Repair: second} + } + } + + out, err = s.PostProcess(out) + if err != nil { + return zero, fmt.Errorf("harness: skill %s v%d: post-process: %w", s.Name(), s.Version(), err) + } + return out, nil +} + +// attempt sends req once and decodes and validates the reply. It returns +// the reply text for the repair turn and the problems found; err is set +// only for a provider failure, which is not repairable. +func attempt[In, Out any](ctx context.Context, p llm.Provider, s Skill[In, Out], req llm.Request) (out Out, reply string, problems []string, err error) { + resp, err := p.Generate(ctx, req) + reply = resp.Text + if reply == "" { + reply = string(resp.JSON) + } + if err != nil { + if errors.Is(err, llm.ErrBadOutput) { + return out, reply, []string{err.Error()}, nil + } + return out, "", nil, err + } + if err := decode(resp.JSON, &out); err != nil { + return out, reply, []string{err.Error()}, nil + } + return out, reply, s.Validate(out), nil +} + +// decode strictly decodes data into out: one JSON value, no unknown fields. +func decode(data json.RawMessage, out any) error { + if len(data) == 0 { + return errors.New("reply holds no JSON") + } + dec := json.NewDecoder(bytes.NewReader(data)) + dec.DisallowUnknownFields() + if err := dec.Decode(out); err != nil { + return fmt.Errorf("reply does not decode: %v", err) + } + if err := dec.Decode(new(json.RawMessage)); err != io.EOF { + return errors.New("reply has data after the JSON value") + } + return nil +} + +// repairMessage asks the model to fix problems in its last reply. +func repairMessage(schemaName string, problems []string) string { + var b strings.Builder + fmt.Fprintf(&b, "Your previous reply was not usable. Reply again with JSON matching the %s schema, fixing these problems:\n", schemaName) + for _, p := range problems { + fmt.Fprintf(&b, "- %s\n", p) + } + return b.String() +} diff --git a/internal/ai/harness/harness_test.go b/internal/ai/harness/harness_test.go new file mode 100644 index 0000000..40b1c4d --- /dev/null +++ b/internal/ai/harness/harness_test.go @@ -0,0 +1,160 @@ +package harness + +import ( + "context" + "encoding/json" + "errors" + "slices" + "strings" + "testing" + + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/ai/llm" +) + +func reply(js string) llm.GenerateResult { + return llm.GenerateResult{Response: llm.Response{Text: js, JSON: json.RawMessage(js)}} +} + +func run(t *testing.T, f *llm.Fake) (echoOutput, error) { + t.Helper() + return Run(context.Background(), Config{Provider: f, Temperature: 0.7}, echoSkill{}, echoInput{Word: "taco"}) +} + +func assertCalls(t *testing.T, f *llm.Fake, want int) []llm.Request { + t.Helper() + reqs := f.Requests() + if len(reqs) != want { + t.Fatalf("Generate called %d times, want %d", len(reqs), want) + } + if g, _, _ := f.Remaining(); g != 0 { + t.Fatalf("%d scripted Generate results unused", g) + } + return reqs +} + +func TestRunValidFirstTime(t *testing.T) { + var f llm.Fake + f.QueueGenerate(reply(`{"echo":" taco "}`)) + out, err := run(t, &f) + if err != nil { + t.Fatal(err) + } + if out.Echo != "taco" { + t.Fatalf("Echo = %q, want post-processed %q", out.Echo, "taco") + } + req := assertCalls(t, &f, 1)[0] + if req.System != "You repeat words." || len(req.Messages) != 1 || + req.Messages[0] != (llm.Message{Role: llm.RoleUser, Content: "Repeat: taco"}) { + t.Fatalf("request prompt = %+v", req) + } + if req.Temperature == nil || *req.Temperature != 0.7 { + t.Fatalf("Temperature = %v, want 0.7", req.Temperature) + } + if req.Schema == nil || req.Schema.Name != "echo" || string(req.Schema.Schema) != string(echoSkill{}.Schema().Schema) { + t.Fatalf("Schema = %+v", req.Schema) + } +} + +func TestRunInvalidThenValid(t *testing.T) { + var f llm.Fake + f.QueueGenerate(reply(`{"echo":"TACO"}`), reply(`{"echo":"taco"}`)) + out, err := run(t, &f) + if err != nil { + t.Fatal(err) + } + if out.Echo != "taco" { + t.Fatalf("Echo = %q", out.Echo) + } + reqs := assertCalls(t, &f, 2) + msgs := reqs[1].Messages + if len(msgs) != 3 || msgs[0] != reqs[0].Messages[0] || + msgs[1] != (llm.Message{Role: llm.RoleAssistant, Content: `{"echo":"TACO"}`}) || msgs[2].Role != llm.RoleUser { + t.Fatalf("repair messages = %+v", msgs) + } + if !strings.Contains(msgs[2].Content, "echo must be lower case") { + t.Fatalf("repair message %q lacks the validator error", msgs[2].Content) + } + if reqs[1].System != reqs[0].System || reqs[1].Schema == nil || *reqs[1].Temperature != 0.7 { + t.Fatalf("repair request lost system, schema or temperature: %+v", reqs[1]) + } +} + +func TestRunDecodeFailureIsRepaired(t *testing.T) { + for name, first := range map[string]llm.GenerateResult{ + "unknown field": reply(`{"echo":"taco","extra":1}`), + "wrong type": reply(`{"echo":5}`), + "trailing data": reply(`{"echo":"taco"}{}`), + "no JSON": {Response: llm.Response{Text: "taco"}}, + "adapter refuse": {Response: llm.Response{Text: "nope"}, Err: llm.ErrBadOutput}, + } { + t.Run(name, func(t *testing.T) { + var f llm.Fake + f.QueueGenerate(first, reply(`{"echo":"taco"}`)) + if out, err := run(t, &f); err != nil || out.Echo != "taco" { + t.Fatalf("Run = %+v, %v", out, err) + } + assertCalls(t, &f, 2) + }) + } +} + +func TestRunInvalidTwice(t *testing.T) { + var f llm.Fake + f.QueueGenerate(reply(`{"echo":"TACO"}`), reply(`{"echo":""}`)) + _, err := run(t, &f) + if !errors.Is(err, llm.ErrBadOutput) { + t.Fatalf("err = %v, want ErrBadOutput", err) + } + var bad *BadOutputError + if !errors.As(err, &bad) { + t.Fatalf("err = %T, want *BadOutputError", err) + } + if bad.Skill != "echo" || bad.Version != 1 || + !slices.Equal(bad.First, []string{"echo must be lower case"}) || + !slices.Equal(bad.Repair, []string{"echo must not be empty"}) { + t.Fatalf("BadOutputError = %+v", bad) + } + for _, want := range []string{"echo must be lower case", "echo must not be empty"} { + if !strings.Contains(err.Error(), want) { + t.Fatalf("error %q lacks %q", err, want) + } + } + assertCalls(t, &f, 2) +} + +func TestRunProviderErrorPassesThrough(t *testing.T) { + for name, perr := range map[string]error{ + "auth": llm.ErrAuth, + "rate limited": &llm.RateLimitedError{}, + "timeout": llm.ErrTimeout, + "provider": llm.NewProviderError(500, []byte("boom"), ""), + } { + t.Run(name, func(t *testing.T) { + var f llm.Fake + f.QueueGenerate(llm.GenerateResult{Err: perr}) + _, err := run(t, &f) + if err != perr { + t.Fatalf("err = %v, want the provider error unchanged", err) + } + assertCalls(t, &f, 1) + }) + } +} + +func TestRunProviderErrorOnRepairPassesThrough(t *testing.T) { + var f llm.Fake + f.QueueGenerate(reply(`{"echo":"TACO"}`), llm.GenerateResult{Err: llm.ErrTimeout}) + if _, err := run(t, &f); err != llm.ErrTimeout { + t.Fatalf("err = %v, want ErrTimeout unchanged", err) + } + assertCalls(t, &f, 2) +} + +func TestRunRenderError(t *testing.T) { + var f llm.Fake + _, err := Run(context.Background(), Config{Provider: &f}, echoSkill{}, echoInput{}) + if err == nil || errors.Is(err, llm.ErrBadOutput) { + t.Fatalf("err = %v, want a render error", err) + } + assertCalls(t, &f, 0) +} -- 2.55.0 From 7be4879f6dd6a4cffa62b1c8102e977217781330 Mon Sep 17 00:00:00 2001 From: dev_agent Date: Mon, 5 Oct 2026 07:49:23 -0400 Subject: [PATCH 2/2] Validate post-processed skill output and quote failed JSON in repair The harness now validates a skill's output again after PostProcess, so output that post-processing makes invalid (e.g. a whitespace-only echo trimmed to empty) enters the single repair turn instead of returning as a success. The repair turn now quotes Response.JSON as the previous assistant reply whenever structured output was returned, falling back to Response.Text only when there is none. Co-Authored-By: Claude Opus 5.5 --- internal/ai/harness/harness.go | 30 ++++++++++-------- internal/ai/harness/harness_test.go | 47 +++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 12 deletions(-) diff --git a/internal/ai/harness/harness.go b/internal/ai/harness/harness.go index c7015b1..bc0dcb6 100644 --- a/internal/ai/harness/harness.go +++ b/internal/ai/harness/harness.go @@ -40,8 +40,9 @@ type Skill[In, Out any] interface { // The problems are sent to the model in the repair turn, so each must // say what to fix. Validate(out Out) []string - // PostProcess finishes a valid out, e.g. normalising names. Its error - // fails the run without a repair turn. + // PostProcess finishes a valid out, e.g. normalising names. Its result + // is validated again; problems found then are repaired like any other. + // Its error fails the run without a repair turn. PostProcess(out Out) (Out, error) } @@ -112,22 +113,20 @@ func Run[In, Out any](ctx context.Context, cfg Config, s Skill[In, Out], in In) return zero, &BadOutputError{Skill: s.Name(), Version: s.Version(), First: first, Repair: second} } } - - out, err = s.PostProcess(out) - if err != nil { - return zero, fmt.Errorf("harness: skill %s v%d: post-process: %w", s.Name(), s.Version(), err) - } return out, nil } -// attempt sends req once and decodes and validates the reply. It returns -// the reply text for the repair turn and the problems found; err is set -// only for a provider failure, which is not repairable. +// attempt sends req once, then decodes, validates, post-processes and +// validates the reply again. It returns the reply for the repair turn and +// the problems found; err is set only for a provider or post-process +// failure, neither of which is repairable. func attempt[In, Out any](ctx context.Context, p llm.Provider, s Skill[In, Out], req llm.Request) (out Out, reply string, problems []string, err error) { resp, err := p.Generate(ctx, req) - reply = resp.Text + // The repair turn must quote what failed: the structured output when + // there is any, otherwise the text. + reply = string(resp.JSON) if reply == "" { - reply = string(resp.JSON) + reply = resp.Text } if err != nil { if errors.Is(err, llm.ErrBadOutput) { @@ -138,6 +137,13 @@ func attempt[In, Out any](ctx context.Context, p llm.Provider, s Skill[In, Out], if err := decode(resp.JSON, &out); err != nil { return out, reply, []string{err.Error()}, nil } + if problems := s.Validate(out); len(problems) > 0 { + return out, reply, problems, nil + } + out, err = s.PostProcess(out) + if err != nil { + return out, "", nil, fmt.Errorf("harness: skill %s v%d: post-process: %w", s.Name(), s.Version(), err) + } return out, reply, s.Validate(out), nil } diff --git a/internal/ai/harness/harness_test.go b/internal/ai/harness/harness_test.go index 40b1c4d..7c68000 100644 --- a/internal/ai/harness/harness_test.go +++ b/internal/ai/harness/harness_test.go @@ -79,6 +79,53 @@ func TestRunInvalidThenValid(t *testing.T) { } } +func TestRunPostProcessedOutputIsValidated(t *testing.T) { + var f llm.Fake + f.QueueGenerate(reply(`{"echo":" "}`), reply(`{"echo":"taco"}`)) + out, err := run(t, &f) + if err != nil { + t.Fatal(err) + } + if out.Echo != "taco" { + t.Fatalf("Echo = %q", out.Echo) + } + msgs := assertCalls(t, &f, 2)[1].Messages + if len(msgs) != 3 || msgs[1] != (llm.Message{Role: llm.RoleAssistant, Content: `{"echo":" "}`}) || + !strings.Contains(msgs[2].Content, "echo must not be empty") { + t.Fatalf("repair messages = %+v", msgs) + } +} + +func TestRunWhitespaceTwiceIsBadOutput(t *testing.T) { + var f llm.Fake + f.QueueGenerate(reply(`{"echo":" "}`), reply(`{"echo":" "}`)) + _, err := run(t, &f) + var bad *BadOutputError + if !errors.Is(err, llm.ErrBadOutput) || !errors.As(err, &bad) { + t.Fatalf("err = %v, want *BadOutputError", err) + } + if !slices.Equal(bad.First, []string{"echo must not be empty"}) || + !slices.Equal(bad.Repair, []string{"echo must not be empty"}) { + t.Fatalf("BadOutputError = %+v", bad) + } + assertCalls(t, &f, 2) +} + +func TestRunRepairQuotesJSONOverText(t *testing.T) { + var f llm.Fake + f.QueueGenerate( + llm.GenerateResult{Response: llm.Response{Text: "Here you go!", JSON: json.RawMessage(`{"echo":"TACO"}`)}}, + reply(`{"echo":"taco"}`), + ) + if out, err := run(t, &f); err != nil || out.Echo != "taco" { + t.Fatalf("Run = %+v, %v", out, err) + } + msgs := assertCalls(t, &f, 2)[1].Messages + if len(msgs) != 3 || msgs[1] != (llm.Message{Role: llm.RoleAssistant, Content: `{"echo":"TACO"}`}) { + t.Fatalf("repair messages = %+v, want the failed JSON as the assistant reply", msgs) + } +} + func TestRunDecodeFailureIsRepaired(t *testing.T) { for name, first := range map[string]llm.GenerateResult{ "unknown field": reply(`{"echo":"taco","extra":1}`), -- 2.55.0