diff --git a/cmd/server/main.go b/cmd/server/main.go index bc87216..f1307ce 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -133,7 +133,7 @@ func run(cfg config.Config, logger *slog.Logger) error { return err } - shopSvc, err := shopping.NewService(db, archiveTitler(ai, guard), logger) + shopSvc, err := shopping.NewService(db, archiveTitler(ai, guard), runner, logger) if err != nil { return err } @@ -280,7 +280,7 @@ func newJobRunner(db *store.Store, logger *slog.Logger) (*jobs.Runner, *jobKinds // service registers its own kinds when it is built, before the runner // starts (dinners.NewService registers dinners.KindGroundDinner and // suggest.NewService suggest.KindSuggestionBatch, plan.NewService -// plan.KindDraftWeek). +// plan.KindDraftWeek, shopping.NewService shopping.KindTitleSupplyRun). func registerJobKinds(runner *jobs.Runner) (*jobKinds, error) { builtins, err := jobs.RegisterBuiltins(runner) if err != nil { diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 9f61026..c3c3dac 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -476,14 +476,20 @@ automatic** (D-021). household temperature with no household context. A title must be 1–48 characters on one line, with no emoji and no surrounding quotation marks; a reply breaking a rule gets the one repair turn, never a - truncation. `shopping.Generate` (and `GeneratePushed`) runs it after - storing a list that is still untitled — a new list, or one a failed - title left untitled — outside the generation's transaction, and writes - `shopping_lists.title` only while the list is untitled and not frozen. - A title is written once; a remake keeps it. A failed title never fails - the generation: the list stays untitled (the UI shows the week label), - the failure is logged as a warning and recorded in `ai_runs`, and the - week's next generation tries again. + truncation. `shopping.Generate` (and `GeneratePushed`), on storing a + list that is still untitled — a new list, or one a failed title left + untitled — enqueues a `title_supply_run` job in the generation's + transaction and returns without waiting for it, so neither Push to + Calendar nor `POST /shopping/{weekStart}/generate` waits for a model + call; the list reads untitled until the job stores its title. The job's + scope is `@/`, so runs share a title only for the same names (a renamed dinner + changes the names, not the revision). It writes `shopping_lists.title` + only while the list is untitled and not frozen. A title is written once; + a remake keeps it and enqueues no job. A failed title never fails the + generation: the list stays untitled (the UI shows the week label), the + failure is logged as a warning and recorded in `ai_runs`, the job fails + `title_failed`, and the week's next generation tries again. ## Recipe grounding diff --git a/internal/server/plans_test.go b/internal/server/plans_test.go index d8cb13c..fd6d3d7 100644 --- a/internal/server/plans_test.go +++ b/internal/server/plans_test.go @@ -37,6 +37,7 @@ type plansEnv struct { h http.Handler st *store.Store svc *plan.Service + runner *jobs.Runner questLog string // google is the fake Google the plan service pushes to; it starts // unconnected. @@ -47,6 +48,13 @@ type plansEnv struct { } func newPlansRouter(t *testing.T) plansEnv { + t.Helper() + return newPlansRouterTitled(t, untitled) +} + +// newPlansRouterTitled is newPlansRouter whose Supply runs are titled by +// title. +func newPlansRouterTitled(t *testing.T, title shopping.Titler) plansEnv { t.Helper() st := newTestStore(t) ctx := t.Context() @@ -80,7 +88,7 @@ func newPlansRouter(t *testing.T) plansEnv { if err != nil { t.Fatal(err) } - shop, err := shopping.NewService(st, untitled, logger) + shop, err := shopping.NewService(st, title, runner, logger) if err != nil { t.Fatal(err) } @@ -119,7 +127,7 @@ func newPlansRouter(t *testing.T) plansEnv { t.Fatal(err) } } - return plansEnv{h: h, st: st, svc: svc, questLog: dinner("Pad Thai", "quest_log", "stovetop", 35), + return plansEnv{h: h, st: st, svc: svc, runner: runner, questLog: dinner("Pad Thai", "quest_log", "stovetop", 35), google: google, gcal: googleSvc, logs: logs} } diff --git a/internal/server/server_test.go b/internal/server/server_test.go index 16a88b3..c735e54 100644 --- a/internal/server/server_test.go +++ b/internal/server/server_test.go @@ -214,7 +214,13 @@ func newTestPlan(t *testing.T) *plan.Service { // database, whose Supply runs stay untitled. func newTestShopping(t *testing.T) *shopping.Service { t.Helper() - svc, err := shopping.NewService(newTestStore(t), untitled, slog.New(slog.NewTextHandler(io.Discard, nil))) + st := newTestStore(t) + logger := slog.New(slog.NewTextHandler(io.Discard, nil)) + runner, err := jobs.NewRunner(st, logger, 1) + if err != nil { + t.Fatal(err) + } + svc, err := shopping.NewService(st, untitled, runner, logger) if err != nil { t.Fatal(err) } diff --git a/internal/server/shopping_test.go b/internal/server/shopping_test.go index 9ef7d42..350b100 100644 --- a/internal/server/shopping_test.go +++ b/internal/server/shopping_test.go @@ -1,6 +1,7 @@ package server import ( + "context" "database/sql" "encoding/json" "net/http" @@ -8,7 +9,9 @@ import ( "slices" "strings" "testing" + "time" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/ai/skills" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/db" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/plan" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/store" @@ -378,6 +381,128 @@ func TestPushPlanMakesSupplyRun(t *testing.T) { } } +// heldTitler is a shopping.Titler that reports each call on entered and +// answers title only once release is closed. +type heldTitler struct { + entered chan struct{} + release chan struct{} +} + +func newHeldTitler() heldTitler { + return heldTitler{entered: make(chan struct{}, 4), release: make(chan struct{})} +} + +func (h heldTitler) title(ctx context.Context, _ string, _ skills.ArchiveTitleRequest) (skills.ArchiveTitle, error) { + h.entered <- struct{}{} + select { + case <-h.release: + return skills.ArchiveTitle{Title: "Chili and Friends"}, nil + case <-ctx.Done(): + return skills.ArchiveTitle{}, ctx.Err() + } +} + +// startRunner starts e's job runner, stopping it when the test ends. e's +// logs must not be read while it runs. +func (e plansEnv) startRunner(t *testing.T) { + t.Helper() + ctx, cancel := context.WithCancel(context.Background()) + if err := e.runner.Start(ctx); err != nil { + cancel() + t.Fatal(err) + } + t.Cleanup(func() { + cancel() + e.runner.Wait() + }) +} + +// waitEntered waits for the title call to begin. +func (h heldTitler) waitEntered(t *testing.T) { + t.Helper() + select { + case <-h.entered: + case <-time.After(10 * time.Second): + t.Fatal("archive_title was never called") + } +} + +// waitTitled waits for the week's Supply run to have a title through GET +// and returns it. +func (e plansEnv) waitTitled(t *testing.T) string { + t.Helper() + deadline := time.Now().Add(10 * time.Second) + for { + var l supplyRunJSON + callShopping(t, e, http.MethodGet, shoppingPath+"2026-10-12", "", &l) + if l.Title != nil { + return *l.Title + } + if time.Now().After(deadline) { + t.Fatal("the Supply run was never titled") + } + time.Sleep(5 * time.Millisecond) + } +} + +// TestPushPlanDoesNotWaitForTitle: Push to Calendar answers while the +// Supply run's title is still being written; the list reads untitled until +// the title job stores it. +func TestPushPlanDoesNotWaitForTitle(t *testing.T) { + t.Parallel() + held := newHeldTitler() + e := newPlansRouterTitled(t, held.title) + e.groundWeek(t) + e.connectCalendar(t, true) + e.startRunner(t) + + rec, body := callDinners(t, e.h, http.MethodPost, plansPath+"2026-10-12/push-calendar", "") + if rec.Code != http.StatusOK || body["status"] != "synced" { + t.Fatalf("push = %d %s", rec.Code, rec.Body) + } + if want := map[string]any{"status": "generated", "error": nil}; !reflect.DeepEqual(body["supply_run"], want) { + t.Errorf("supply_run = %v, want %v", body["supply_run"], want) + } + held.waitEntered(t) + var l supplyRunJSON + callShopping(t, e, http.MethodGet, shoppingPath+"2026-10-12", "", &l) + if l.Title != nil { + t.Errorf("title = %q while archive_title is still running, want null", *l.Title) + } + + close(held.release) + if got := e.waitTitled(t); got != "Chili and Friends" { + t.Errorf("title = %q, want the title job's", got) + } +} + +// TestSupplyRunGenerateDoesNotWaitForTitle: POST generate answers with the +// list untitled while its title is still being written. +func TestSupplyRunGenerateDoesNotWaitForTitle(t *testing.T) { + t.Parallel() + held := newHeldTitler() + e := newPlansRouterTitled(t, held.title) + e.groundWeek(t) + e.startRunner(t) + + var l supplyRunJSON + if status, body := callShopping(t, e, http.MethodPost, shoppingPath+"2026-10-12/generate", "", &l); status != http.StatusOK { + t.Fatalf("generate = %d %v", status, body) + } + if l.Title != nil { + t.Errorf("title = %q while archive_title is still running, want null", *l.Title) + } + if !reflect.DeepEqual(l.render(), weekLines) { + t.Errorf("generated = %v, want %v", l.render(), weekLines) + } + held.waitEntered(t) + + close(held.release) + if got := e.waitTitled(t); got != "Chili and Friends" { + t.Errorf("title = %q, want the title job's", got) + } +} + // TestFailedPushMakesNoSupplyRun: a push Calendar refused makes no Supply // run. func TestFailedPushMakesNoSupplyRun(t *testing.T) { diff --git a/internal/shopping/shopping.go b/internal/shopping/shopping.go index 4c5c90d..1ed9097 100644 --- a/internal/shopping/shopping.go +++ b/internal/shopping/shopping.go @@ -6,12 +6,15 @@ // the plan changes. It is deterministic and runs no model: an item is one // ingredient of the dinner's recipe exactly as recipe grounding stored it // (see Generate). The one model call is the Supply run's title, written by -// archive_title when the list is generated untitled (see Generate). +// archive_title in a KindTitleSupplyRun job when the list is generated +// untitled (see Generate). package shopping import ( "context" + "crypto/sha256" "database/sql" + "encoding/json" "errors" "fmt" "log/slog" @@ -21,6 +24,7 @@ import ( "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/db" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/dinners" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/household" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/jobs" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/store" ) @@ -39,6 +43,15 @@ const ( SkipNoIngredients = "no_ingredients" ) +// KindTitleSupplyRun is the job kind that titles a Supply run with +// archive_title. Only Generate and GeneratePushed enqueue it. +const KindTitleSupplyRun = "title_supply_run" + +// ReasonTitleFailed is the failure code of a KindTitleSupplyRun job that +// stored no title: archive_title failed, or the Supply run was frozen or +// replaced first. The Supply run stays untitled. +const ReasonTitleFailed = "title_failed" + // ErrPlanNotFound is a generation for a week with no plan. var ErrPlanNotFound = errors.New("the week has no plan") @@ -57,30 +70,44 @@ var ErrListNotFound = errors.New("the week has no Supply run") // ErrItemNotFound is a change to an item on no Supply run. var ErrItemNotFound = errors.New("no Supply run has that item") -// Titler runs archive_title on in; scope names the week and its dinners. +// Titler runs archive_title on in; scope names the week, its plan revision +// and its dinners. // In production it is skills.TitleArchive with the household's // harness.Config. type Titler func(ctx context.Context, scope string, in skills.ArchiveTitleRequest) (skills.ArchiveTitle, error) // Service generates Supply runs. type Service struct { - st *store.Store - title Titler - logger *slog.Logger + st *store.Store + title Titler + titleJob *jobs.Kind[TitleJob] + logger *slog.Logger } // NewService returns a service over st that titles Supply runs with title -// and logs to logger. -func NewService(st *store.Store, title Titler, logger *slog.Logger) (*Service, error) { +// in KindTitleSupplyRun jobs, which it registers on runner, and logs to +// logger. runner must not have started yet. +func NewService(st *store.Store, title Titler, runner *jobs.Runner, logger *slog.Logger) (*Service, error) { switch { case st == nil: return nil, errors.New("shopping: nil store") case title == nil: return nil, errors.New("shopping: nil titler") + case runner == nil: + return nil, errors.New("shopping: nil job runner") case logger == nil: return nil, errors.New("shopping: nil logger") } - return &Service{st: st, title: title, logger: logger}, nil + s := &Service{st: st, title: title, logger: logger} + kind, err := jobs.Register(runner, KindTitleSupplyRun, s.runTitleJob, ReasonTitleFailed) + if err != nil { + return nil, err + } + if err := kind.ServiceOnly(); err != nil { + return nil, err + } + s.titleJob = kind + return s, nil } // List is a Supply run. @@ -158,6 +185,9 @@ type Generated struct { // Skipped are the plan days whose dinner has no ingredients to shop // for, by date. Skipped []SkippedDay `json:"skipped"` + // TitleJobID is the KindTitleSupplyRun job titling the list; nil when + // the list already has its title. + TitleJobID *string `json:"title_job_id"` } // Generate makes the Supply run of the week starting weekStart from the @@ -182,12 +212,15 @@ type Generated struct { // new and needed; an item no longer on the plan is removed, and with it a // group no longer on the plan. // -// A list still untitled once it is stored, as a new list always is, is -// then titled by archive_title from the names of the plan's dinners in day -// order, outside the transaction. A title is written once: a remake keeps -// it. Titling never fails the generation: a title that cannot be written -// leaves the list untitled, as it is returned, the failure is logged, and -// the next generation of the week, Generate or GeneratePushed, tries again. +// A list still untitled once it is stored, as a new list always is, gets a +// KindTitleSupplyRun job, enqueued in the same transaction, that titles it +// with archive_title from the names of the plan's dinners in day order. +// Generate returns without waiting for the job: the list is returned +// untitled, with the job's id. A title is written once: a remake keeps it +// and enqueues no job. Titling never fails the generation: a title that +// cannot be written leaves the list untitled, the failure is logged and +// the job failed with ReasonTitleFailed, and the next generation of the +// week, Generate or GeneratePushed, tries again. // // A week with no plan is ErrPlanNotFound; a list frozen into the Supply // archive is ErrFrozen. A fetched recipe's ingredient without a name is an @@ -216,11 +249,7 @@ func (s *Service) generate(ctx context.Context, weekStart store.LocalDate, check if _, err := household.WeekDays(weekStart); err != nil { return Generated{}, fmt.Errorf("shopping: generate: %w", err) } - var ( - out Generated - revision int64 - names []string // the plan's dinners, in day order - ) + var out Generated err := s.st.WithTx(ctx, func(tx *sql.Tx) error { q := db.New(tx) plan, err := q.GetWeekPlan(ctx, weekStart.String()) @@ -237,7 +266,6 @@ func (s *Service) generate(ctx context.Context, weekStart store.LocalDate, check if err != nil { return err } - revision, names = plan.Revision, dinnerNames now := store.Now().String() list, err := q.GetShoppingList(ctx, weekStart.String()) if err != nil && !errors.Is(err, sql.ErrNoRows) { @@ -263,43 +291,77 @@ func (s *Service) generate(ctx context.Context, weekStart store.LocalDate, check return err } out = Generated{List: listOf(list, groups), Skipped: skipped} + if list.Title.Valid { + return nil + } + id, err := s.titleJob.EnqueueTx(ctx, tx, TitleJob{ + ListID: list.ID, WeekStart: list.WeekStart, Revision: plan.Revision, Dinners: dinnerNames, + }) + if err != nil { + return fmt.Errorf("shopping: enqueue title: %w", err) + } + out.TitleJobID = &id return nil }) if err != nil { return Generated{}, err } - if out.List.Title == nil { - s.titleList(ctx, &out.List, revision, names) + if out.TitleJobID != nil { + s.titleJob.Wake() } return out, nil } -// titleList titles l, an untitled Supply run made from dinners, the names -// of its plan's dinners in day order at plan revision, with archive_title, -// and stores the title on it. A title that cannot be written leaves l -// untitled; the failure is logged, never returned. -func (s *Service) titleList(ctx context.Context, l *List, revision int64, dinners []string) { - title, err := s.writeTitle(ctx, l, revision, dinners) - if err != nil { - s.logger.WarnContext(ctx, "Supply run left untitled; its next generation titles it again", - "week", l.WeekStart, "list", l.ID, "error", err) - return - } - l.Title = &title +// TitleJob is a KindTitleSupplyRun job's payload: the untitled Supply run +// ListID of the week WeekStart, made from the plan at Revision, whose +// dinners in day order are Dinners. +type TitleJob struct { + ListID string `json:"list_id"` + WeekStart string `json:"week_start"` + Revision int64 `json:"revision"` + Dinners []string `json:"dinners"` } -// writeTitle runs archive_title for l and stores its title. When l was -// titled by another generation meanwhile, that title stands and is -// returned. -func (s *Service) writeTitle(ctx context.Context, l *List, revision int64, dinners []string) (string, error) { - scope := fmt.Sprintf("%s@%d", l.WeekStart, revision) - out, err := s.title(ctx, scope, skills.ArchiveTitleRequest{Dinners: dinners}) +// titleResult is a KindTitleSupplyRun job's result: the Supply run and the +// title it has. +type titleResult struct { + ListID string `json:"list_id"` + Title string `json:"title"` +} + +// PublicJSON returns the Supply run's id and title. +func (r titleResult) PublicJSON() ([]byte, error) { return json.Marshal(r) } + +// runTitleJob titles the Supply run of p with archive_title and stores the +// title. A title that cannot be stored leaves the list untitled: the +// failure is logged and the job fails with ReasonTitleFailed. +func (s *Service) runTitleJob(ctx context.Context, p TitleJob, _ func(titleResult) error) (titleResult, error) { + title, err := s.writeTitle(ctx, p) + if err != nil { + s.logger.WarnContext(ctx, "Supply run left untitled; its next generation titles it again", + "week", p.WeekStart, "list", p.ListID, "error", err) + return titleResult{}, &jobs.Failure{Code: ReasonTitleFailed} + } + return titleResult{ListID: p.ListID, Title: title}, nil +} + +// writeTitle runs archive_title for p's Supply run and stores its title. +// When the list was titled by another job, before this one ran or +// meanwhile, that title stands and is returned. +func (s *Service) writeTitle(ctx context.Context, p TitleJob) (string, error) { + q := db.New(s.st.Writer) + title, err := s.currentTitle(ctx, q, p) + if err != nil || title != "" { + return title, err + } + out, err := s.title(ctx, titleScope(p), skills.ArchiveTitleRequest{Dinners: p.Dinners}) if err != nil { return "", fmt.Errorf("shopping: title: %w", err) } - q := db.New(s.st.Writer) - n, err := q.SetShoppingListTitle(ctx, db.SetShoppingListTitleParams{ - Title: sql.NullString{String: out.Title, Valid: true}, UpdatedAt: store.Now().String(), ID: l.ID, + write, cancel := jobs.FinalWriteContext(ctx) + defer cancel() + n, err := q.SetShoppingListTitle(write, db.SetShoppingListTitleParams{ + Title: sql.NullString{String: out.Title, Valid: true}, UpdatedAt: store.Now().String(), ID: p.ListID, }) if err != nil { return "", fmt.Errorf("shopping: store title: %w", err) @@ -307,14 +369,37 @@ func (s *Service) writeTitle(ctx context.Context, l *List, revision int64, dinne if n == 1 { return out.Title, nil } - list, err := q.GetShoppingList(ctx, l.WeekStart) - if err != nil { - return "", fmt.Errorf("shopping: store title: read list: %w", err) + title, err = s.currentTitle(write, q, p) + if err == nil && title == "" { + err = errors.New("shopping: store title: the Supply run is still untitled") } - if list.ID != l.ID || !list.Title.Valid { + return title, err +} + +// currentTitle returns the title of p's Supply run, or "" while it is +// untitled. A list frozen or replaced untitled is an error: it can no +// longer be titled. +func (s *Service) currentTitle(ctx context.Context, q *db.Queries, p TitleJob) (string, error) { + list, err := q.GetShoppingList(ctx, p.WeekStart) + if err != nil && !errors.Is(err, sql.ErrNoRows) { + return "", fmt.Errorf("shopping: title: read list: %w", err) + } + switch { + case err == nil && list.ID == p.ListID && list.Title.Valid: + return list.Title.String, nil + case err != nil || list.ID != p.ListID || frozen(list): return "", errors.New("shopping: store title: the Supply run was frozen or replaced before its title was stored") } - return list.Title.String, nil + return "", nil +} + +// titleScope is the archive_title scope of p: its week and plan revision, +// and a fingerprint of its dinners in day order, so runs share a title only +// when they title the same dinner names. A renamed dinner changes the +// names but not the plan revision. +func titleScope(p TitleJob) string { + names, _ := json.Marshal(p.Dinners) // a []string always encodes + return fmt.Sprintf("%s@%d/%x", p.WeekStart, p.Revision, sha256.Sum256(names)) } // Get returns the Supply run of the week starting weekStart, as stored: a diff --git a/internal/shopping/shopping_test.go b/internal/shopping/shopping_test.go index 3da14e6..a746343 100644 --- a/internal/shopping/shopping_test.go +++ b/internal/shopping/shopping_test.go @@ -12,6 +12,7 @@ import ( "strings" "sync" "testing" + "time" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/ai/harness" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/ai/llm" @@ -46,11 +47,15 @@ type env struct { planID string // ids are the dinners' ids by name. ids map[string]string + // runner runs the title jobs, two at a time, once start starts it. + runner *jobs.Runner // model answers archive_title, which titles the Supply runs; with // nothing queued it fails, and the lists stay untitled. model *llm.Fake // beforeTitle, when set, runs as archive_title is asked for a title. beforeTitle func() + // beforeModel, when set, runs as the model is sent a request. + beforeModel func(llm.Request) // log holds what the service logged. log *syncBuffer } @@ -95,7 +100,7 @@ func newEnv(t *testing.T) *env { } cfg := harness.Config{ Settings: llm.Settings{Provider: "anthropic", Model: "claude-test", Temperature: 0.7}, - NewProvider: func(llm.Settings, string) (llm.Provider, error) { return e.model, nil }, + NewProvider: func(llm.Settings, string) (llm.Provider, error) { return hookedModel{e}, nil }, Store: st, Guard: guard, } @@ -105,13 +110,70 @@ func newEnv(t *testing.T) *env { } return skills.TitleArchive(ctx, cfg, scope, in) } - e.svc, err = shopping.NewService(st, title, slog.New(slog.NewTextHandler(e.log, nil))) + logger := slog.New(slog.NewTextHandler(e.log, nil)) + e.runner, err = jobs.NewRunner(st, logger, 2) + if err != nil { + t.Fatal(err) + } + e.svc, err = shopping.NewService(st, title, e.runner, logger) if err != nil { t.Fatal(err) } return e } +// hookedModel is the env's model, running beforeModel first. +type hookedModel struct{ e *env } + +func (m hookedModel) Generate(ctx context.Context, req llm.Request) (llm.Response, error) { + if m.e.beforeModel != nil { + m.e.beforeModel(req) + } + return m.e.model.Generate(ctx, req) +} + +func (m hookedModel) ListModels(ctx context.Context) ([]llm.Model, error) { + return m.e.model.ListModels(ctx) +} + +func (m hookedModel) Verify(ctx context.Context) error { return m.e.model.Verify(ctx) } + +// start starts the env's job runner, stopping it when the test ends. +func (e *env) start(t *testing.T) { + t.Helper() + ctx, cancel := context.WithCancel(context.Background()) + if err := e.runner.Start(ctx); err != nil { + cancel() + t.Fatal(err) + } + t.Cleanup(func() { + cancel() + e.runner.Wait() + }) +} + +// waitTitle waits for g's title job to finish and returns it. +func (e *env) waitTitle(t *testing.T, g shopping.Generated) jobs.Job { + t.Helper() + if g.TitleJobID == nil { + t.Fatal("generation enqueued no title job") + } + deadline := time.Now().Add(10 * time.Second) + for { + job, err := e.runner.Get(t.Context(), *g.TitleJobID) + if err != nil { + t.Fatal(err) + } + if job.Status == jobs.StatusSucceeded || job.Status == jobs.StatusFailed { + return job + } + if time.Now().After(deadline) { + t.Fatalf("title job %s still %s", job.ID, job.Status) + } + time.Sleep(5 * time.Millisecond) + } +} + // ing is a recipe ingredient as recipe grounding stores it: an empty // qty or unit is NULL. type ing struct { @@ -395,8 +457,13 @@ func TestGenerateSevenDayPlan(t *testing.T) { } // Generating again from the same plan changes nothing: same rows, same - // ids, no extra rows. + // ids, no extra rows. The list is still untitled, so it gets another + // title job. again := e.generate(t) + if again.TitleJobID == nil || g.TitleJobID == nil || *again.TitleJobID == *g.TitleJobID { + t.Errorf("title jobs = %v then %v, want one per generation of the untitled list", deref(g.TitleJobID), deref(again.TitleJobID)) + } + again.TitleJobID = g.TitleJobID if !reflect.DeepEqual(again, g) { t.Errorf("second generation =\n %+v\nwant the first\n %+v", again, g) } diff --git a/internal/shopping/title_test.go b/internal/shopping/title_test.go index 951d588..00cf399 100644 --- a/internal/shopping/title_test.go +++ b/internal/shopping/title_test.go @@ -4,12 +4,16 @@ import ( "database/sql" "encoding/json" "fmt" + "slices" "strings" + "sync" "testing" + "time" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/ai/harness" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/ai/llm" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/db" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/jobs" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/shopping" ) @@ -20,10 +24,11 @@ func titled(title string) llm.GenerateResult { } // titleWeek stores three dinners, one with no recipe, and plans them on -// the week's first three days. +// the week's first three days; its job runner is started. func titleWeek(t *testing.T) *env { t.Helper() e := newEnv(t) + e.start(t) e.dinner(t, "Beef Chili", ing{"ground beef", "1", "lb"}) e.dinner(t, "Fish Tacos") e.dinner(t, "Pad Thai", ing{"rice noodles", "8", "oz"}) @@ -88,16 +93,29 @@ func deref(s *string) any { return *s } -// TestGenerateTitlesNewList: a new list is titled by archive_title from -// the plan's dinners in day order, a dinner with no group included, and -// the title is stored and returned. +// wantJob fails t unless job ended with status and, when failed, code. +func wantJob(t *testing.T, job jobs.Job, status jobs.Status, code string) { + t.Helper() + if job.Status != status || (code != "" && (job.Error == nil || *job.Error != code)) { + t.Errorf("title job = %s %v, want %s %s", job.Status, deref(job.Error), status, code) + } +} + +// TestGenerateTitlesNewList: a new list is returned untitled with a title +// job, which titles it by archive_title from the plan's dinners in day +// order, a dinner with no group included, and stores the title. func TestGenerateTitlesNewList(t *testing.T) { e := titleWeek(t) e.model.QueueGenerate(titled("Noodles, Tacos and Chili")) g := e.generate(t) + wantUntitled(t, "returned title", g.List.Title) + job := e.waitTitle(t, g) - wantTitle(t, "returned title", g.List.Title, "Noodles, Tacos and Chili") + wantJob(t, job, jobs.StatusSucceeded, "") + if want := `{"list_id":"` + g.List.ID + `","title":"Noodles, Tacos and Chili"}`; string(job.Result) != want { + t.Errorf("job result = %s, want %s", job.Result, want) + } wantTitle(t, "stored title", e.storedTitle(t), "Noodles, Tacos and Chili") got, err := e.svc.Get(t.Context(), week) if err != nil { @@ -125,9 +143,8 @@ func TestGenerateTitleTooLongRepaired(t *testing.T) { long := "Pad Thai Monday, Fish Taco Tuesday and Chili Wednesday" e.model.QueueGenerate(titled(long), titled("Noodles, Tacos and Chili")) - g := e.generate(t) + wantJob(t, e.waitTitle(t, e.generate(t)), jobs.StatusSucceeded, "") - wantTitle(t, "returned title", g.List.Title, "Noodles, Tacos and Chili") wantTitle(t, "stored title", e.storedTitle(t), "Noodles, Tacos and Chili") reqs := e.model.Requests() if len(reqs) != 2 || !strings.Contains(reqs[1].Messages[2].Content, "has 54 characters; shorten it to at most 48") { @@ -140,7 +157,7 @@ func TestGenerateTitleTooLongRepaired(t *testing.T) { // TestGenerateTitleFailureKeepsList: archive_title giving an emoji twice // fails, and the list is still made, whole and untitled, with the failure -// logged and recorded in ai_runs. +// logged, recorded in ai_runs and failing the title job. func TestGenerateTitleFailureKeepsList(t *testing.T) { e := titleWeek(t) e.model.QueueGenerate(titled("Taco Week 🌮"), titled("Taco Week 🌮")) @@ -159,6 +176,7 @@ func TestGenerateTitleFailureKeepsList(t *testing.T) { if len(g.Skipped) != 1 || g.Skipped[0].DinnerName != "Fish Tacos" { t.Errorf("skipped = %+v, want Fish Tacos", g.Skipped) } + wantJob(t, e.waitTitle(t, g), jobs.StatusFailed, shopping.ReasonTitleFailed) wantUntitled(t, "returned title", g.List.Title) wantUntitled(t, "stored title", e.storedTitle(t)) if g.List.Status != shopping.StatusCurrent { @@ -182,7 +200,8 @@ func TestGenerateRetriesTitle(t *testing.T) { e := titleWeek(t) e.model.QueueGenerate(llm.GenerateResult{Err: fmt.Errorf("%w: model down", llm.ErrProvider)}) first := e.generate(t) - wantUntitled(t, "first title", first.List.Title) + wantJob(t, e.waitTitle(t, first), jobs.StatusFailed, shopping.ReasonTitleFailed) + wantUntitled(t, "first title", e.storedTitle(t)) if !strings.Contains(e.log.String(), "model down") { t.Errorf("log lacks the failure:\n%s", e.log.String()) } @@ -199,12 +218,15 @@ func TestGenerateRetriesTitle(t *testing.T) { if second.List.ID != first.List.ID { t.Fatalf("second generation made list %s, want list %s again", second.List.ID, first.List.ID) } - wantTitle(t, "retried title", second.List.Title, "Noodles, Tacos and Chili") - wantTitle(t, "stored title", e.storedTitle(t), "Noodles, Tacos and Chili") + wantJob(t, e.waitTitle(t, second), jobs.StatusSucceeded, "") + wantTitle(t, "retried title", e.storedTitle(t), "Noodles, Tacos and Chili") e.swap(t, "2026-10-12", "Beef Chili") third := e.generate(t) wantTitle(t, "title after a remake", third.List.Title, "Noodles, Tacos and Chili") + if third.TitleJobID != nil { + t.Errorf("a remake of a titled list enqueued title job %s", *third.TitleJobID) + } if n := len(e.model.Requests()); n != 2 { t.Errorf("%d model calls, want 2: a titled list is not titled again", n) } @@ -222,11 +244,70 @@ func TestGenerateTitleNotStoredOnFrozenList(t *testing.T) { } } - g := e.generate(t) + wantJob(t, e.waitTitle(t, e.generate(t)), jobs.StatusFailed, shopping.ReasonTitleFailed) - wantUntitled(t, "returned title", g.List.Title) wantUntitled(t, "stored title", e.storedTitle(t)) if !strings.Contains(e.log.String(), "the Supply run was frozen or replaced before its title was stored") { t.Errorf("log lacks the failure:\n%s", e.log.String()) } } + +// TestGenerateTitleOverlapAcrossRename: a dinner renamed between two +// generations of an untitled list changes the names to title but not the +// plan revision. The two title jobs, running at once, each send the model +// their own names; neither is given the other's title. +func TestGenerateTitleOverlapAcrossRename(t *testing.T) { + e := titleWeek(t) + e.model.QueueGenerate(titled("Noodles, Tacos and Chili"), titled("Drunken Noodles Week")) + // Each model call waits for the other, so the runs overlap. + var arrived sync.WaitGroup + arrived.Add(2) + e.beforeModel = func(llm.Request) { + arrived.Done() + done := make(chan struct{}) + go func() { arrived.Wait(); close(done) }() + select { + case <-done: + case <-time.After(5 * time.Second): + t.Error("the title runs did not overlap: one model call never came") + } + } + revision := func() int64 { + p, err := db.New(e.st.Reader).GetWeekPlan(t.Context(), week.String()) + if err != nil { + t.Fatal(err) + } + return p.Revision + } + + before := revision() + first := e.generate(t) + if _, err := e.st.Writer.Exec(`UPDATE dinners SET name = 'Drunken Noodles', normalized_name = 'drunken noodles' WHERE id = ?`, e.ids["Pad Thai"]); err != nil { + t.Fatal(err) + } + second := e.generate(t) + if revision() != before { + t.Fatalf("plan revision moved from %d to %d: the rename must leave it", before, revision()) + } + + wantJob(t, e.waitTitle(t, first), jobs.StatusSucceeded, "") + wantJob(t, e.waitTitle(t, second), jobs.StatusSucceeded, "") + var users []string + for _, r := range e.model.Requests() { + users = append(users, r.Messages[0].Content) + } + slices.Sort(users) + want := []string{ + "Title the week whose dinners are, in day order from Monday:\n- Drunken Noodles\n- Fish Tacos\n- Beef Chili", + "Title the week whose dinners are, in day order from Monday:\n- Pad Thai\n- Fish Tacos\n- Beef Chili", + } + if !slices.Equal(users, want) { + t.Errorf("model was sent %q, want each generation's own names %q", users, want) + } + if s := e.runStatuses(t); len(s) != 2 { + t.Errorf("archive_title runs = %q, want two", s) + } + if got := e.storedTitle(t); got == nil || (*got != "Noodles, Tacos and Chili" && *got != "Drunken Noodles Week") { + t.Errorf("stored title = %v, want one of the two titles", deref(got)) + } +}