From 4c34467f8b521b3a395435aafd591139606bc2d9 Mon Sep 17 00:00:00 2001 From: orchestrator-agent <20+orchestrator-agent@code.cubecraftcreations.com> Date: Wed, 7 Oct 2026 00:56:34 -0400 Subject: [PATCH] Suggestion batches, deck and Try it / Pass API (#102) Co-authored-by: orchestrator-agent <20+orchestrator-agent@code.cubecraftcreations.com> --- cmd/server/main.go | 22 +- docs/ARCHITECTURE.md | 60 +- internal/ai/harness/context_test.go | 6 +- internal/db/household_context.sql.go | 10 +- internal/db/models.go | 1 + internal/db/querier.go | 39 +- internal/db/suggestions.sql.go | 409 ++++++++ internal/jobs/jobs.go | 139 ++- internal/jobs/jobs_test.go | 132 +++ internal/server/aisettings_test.go | 2 +- internal/server/dinners_test.go | 2 +- internal/server/login_test.go | 2 +- internal/server/media_test.go | 2 +- internal/server/origin_test.go | 2 +- internal/server/recipesearch_test.go | 2 +- internal/server/server.go | 14 +- internal/server/server_test.go | 48 +- internal/server/settings_test.go | 2 +- internal/server/suggestions.go | 82 ++ internal/server/suggestions_test.go | 189 ++++ internal/store/migrate_test.go | 84 +- internal/store/repository_test.go | 2 +- internal/store/schema_test.go | 1 + internal/suggest/batch.go | 309 ++++++ internal/suggest/suggest.go | 419 ++++++++ internal/suggest/suggest_test.go | 1144 ++++++++++++++++++++++ migrations/0012_suggestion_name.sql | 37 + migrations/queries/household_context.sql | 10 +- migrations/queries/suggestions.sql | 103 ++ 29 files changed, 3201 insertions(+), 73 deletions(-) create mode 100644 internal/db/suggestions.sql.go create mode 100644 internal/server/suggestions.go create mode 100644 internal/server/suggestions_test.go create mode 100644 internal/suggest/batch.go create mode 100644 internal/suggest/suggest.go create mode 100644 internal/suggest/suggest_test.go create mode 100644 migrations/0012_suggestion_name.sql create mode 100644 migrations/queries/suggestions.sql diff --git a/cmd/server/main.go b/cmd/server/main.go index 3935c0a..ca0834e 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -27,6 +27,7 @@ import ( "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/secretbox" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/server" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/store" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/suggest" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/migrations" ) @@ -106,6 +107,10 @@ func run(cfg config.Config, logger *slog.Logger) error { if err != nil { return err } + suggestSvc, err := suggest.NewService(db, grounder, sideQuests(ai, guard), runner, logger) + if err != nil { + return err + } sessions, err := auth.NewSessionManager(db, cfg.SessionIdle, cfg.SessionMax) if err != nil { @@ -123,7 +128,7 @@ func run(cfg config.Config, logger *slog.Logger) error { return err } - router, err := server.NewRouter(logger, version, cfg.TrustedProxy, authFlow, sessions, settings, search, ai, dinnerSvc, runner, photos, frontend.Assets()) + router, err := server.NewRouter(logger, version, cfg.TrustedProxy, authFlow, sessions, settings, search, ai, dinnerSvc, suggestSvc, runner, photos, frontend.Assets()) if err != nil { return err } @@ -247,7 +252,8 @@ func newJobRunner(db *store.Store, logger *slog.Logger) (*jobs.Runner, *jobKinds // registerJobKinds registers the framework's job kinds on runner. A feature // service registers its own kinds when it is built, before the runner -// starts (dinners.NewService registers dinners.KindGroundDinner). +// starts (dinners.NewService registers dinners.KindGroundDinner and +// suggest.NewService suggest.KindSuggestionBatch). func registerJobKinds(runner *jobs.Runner) (*jobKinds, error) { builtins, err := jobs.RegisterBuiltins(runner) if err != nil { @@ -268,6 +274,18 @@ func normalizer(ai *aisettings.Service, guard *harness.Guard) recipes.Normalizer } } +// sideQuests runs suggest_side_quests with the AI settings current at each +// run, sharing guard. +func sideQuests(ai *aisettings.Service, guard *harness.Guard) suggest.Suggester { + return func(ctx context.Context, scope string, in skills.SideQuestsRequest) (skills.SideQuests, error) { + cfg, err := ai.HarnessConfig(ctx, guard) + if err != nil { + return skills.SideQuests{}, err + } + return skills.SuggestSideQuests(ctx, cfg, scope, in) + } +} + // logListening logs the startup line. Only the public URL's origin is logged // so no credential-bearing URL component can reach the log. func logListening(logger *slog.Logger, cfg config.Config) { diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index e46c340..3227230 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -98,7 +98,7 @@ Columns ending in `_ct` hold AES-256-GCM ciphertext (via `secretbox`). | `photos` | source_url, local_path, width, height, sha256, fetched_at. | | `dinners` | The meal record and its lifecycle: name, normalized_name, status (`suggested` / `quest_log` / `tried` / `regular` / `archived`), origin, method, minutes_total, difficulty, cuisine, is_favorite, recipe_id, recipe_url (a pasted recipe link awaiting grounding), photo_id, grounding_status (`pending` / `grounded` / `failed` / `manual`), grounding_error (why grounding failed), grounding_revision (raised when a pending dinner is renamed, so only the latest `ground_dinner` job may write it), updated_by. | | `suggestion_batches` | trigger, status, ai_run_id. | -| `suggestions` | dinner_id, batch_id, reason, cited_regular_ids (JSON), status, decided_at. | +| `suggestions` | dinner_id, batch_id, name (the name the dish was suggested under, kept whatever its dinner is later renamed, so a Pass stays an immutable record every later batch dedupes against), reason, cited_regular_ids (JSON), status, decided_at. | | `week_plans` | week_start (unique), status (`draft` / `synced` / `changed`), ai_note, ai_run_id, calendar_synced_at. | | `plan_days` | plan_id, date (unique per plan), dinner_id, is_new, calendar_event_id. | | `rotation_reviews` | dinner_id + plan_date (unique), status, decision (`keep` / `drop` / `not_made`), decided_at, updated_by. | @@ -127,7 +127,7 @@ suggested (deck) ──Try it──▶ quest_log ──planned date passes── │ └──────── explicit decision ─────┤ │ ├──▶ regular │ └──▶ archived - └──Pass──▶ archived (name kept for dedupe) + └──Pass──▶ archived (suggested name kept for dedupe) ``` From `tried`, an explicit decision moves the dinner to `regular`, to @@ -260,6 +260,62 @@ automatic** (D-021). dinner, is 500, logged with a fixed reason and the dinner id only — never the link, which can carry a personal token, or dependency error text. +- Side quest suggestions (`suggest.Service`). `POST + /api/v1/suggestions/batches` creates a `manual` batch and enqueues its + `suggestion_batch` job, answering 202 `{batch_id, job_id, trigger, + status}` without waiting; while a batch is `pending` or `running` it + answers 202 with that batch instead, so at most one runs at a time (the + check, the batch and its job share one write transaction). The job reads the + household context, the deck's names and every declined name, runs + `suggest_side_quests` for 5 Side quests (scope: the batch id), then + grounds the survivors in the model's order with `Grounder.GroundMany`, + as many as are still wanted at a time, until 5 are kept or none are + left. A Side quest no recipe grounds is dropped; each round's grounded + ones are stored before the next — a dinner with status `suggested`, + origin `side_quest`, grounding_status `grounded` with the recipe's + recipe_id, photo_id, method, minutes_total and difficulty and the + model's cuisine, and a `pending` suggestion with the name it is + suggested under, the reason and `cited_regular_ids` — unless, since the + job read its input, a dinner it repeats was added, a suggestion it + repeats passed, or a Regular it cites renamed or archived (checked in + the storing transaction). The job publishes + `{batch_id, stage, suggestion_ids, dropped}` as it goes (`suggesting`, + `grounding`, `done`). It fails with `no_regulars`, `ai_failed` (the + skill or `normalize_recipe` failed; no Side quest is made up instead, + D-017), `search_not_configured` or `search_failed`, and the batch is set + `failed`; Side quests stored before the failure stay in the deck. If + recording the batch's end fails, its job fails; the job runner retries + a job's final status write with backoff until it lands, each attempt + bounded at 5 seconds; at shutdown an attempt that fails or times out is + not retried, and the next start fails the job as interrupted. Recording + the batch's end is bounded the same way (one attempt, 5 seconds, made + even during shutdown), so a busy writer cannot hold a stopping worker; + a batch whose end timed out is left `running` with its job. Each time a + batch is started, before the active batch is selected, every batch + still `pending` or `running` with no job queued or running (the process + stopped, or recording its end failed) is set `failed`, so it does not + block the next. + `GET /api/v1/suggestions/deck` returns `{suggestions: [{id, dinner_id, + name, photo_url, difficulty, method, minutes_total, cuisine, reason, + cited_regular_ids, status, dinner_status, decided_at, created_at}]}`: + the `pending` suggestions whose dinner is still `suggested`, oldest + first. `POST /api/v1/suggestions/{id}/accept` (Try it) sets the + suggestion `accepted` and moves its dinner to `quest_log`; `POST + /api/v1/suggestions/{id}/decline` (Pass) sets it `declined` and archives + the dinner; every later batch, and the household context's passed list, + reads the suggestion's own name as declined, so renaming the archived + dinner changes nothing. Both record + the session's subject as the dinner's `updated_by` and answer 200 + `{suggestion, replenish_job_id}`. Repeating a decision changes nothing + and answers 200; the opposite decision is 409 `suggestion_decided`, a + suggestion whose dinner left the deck some other way 409 + `dinner_not_suggested`, and an unknown id 404. A decision that leaves + fewer than 5 in the deck starts a `replenish` batch, whose job id it + returns, unless a batch is already pending or running + (`replenish_job_id` is then null). The batch and its job are written in + the decision's own transaction, so a decision is never recorded without + the replenish it calls for; when they cannot be written, neither is the + decision, and a retry decides again. - `/health` — liveness. - Everything else serves the embedded PWA with SPA fallback. diff --git a/internal/ai/harness/context_test.go b/internal/ai/harness/context_test.go index b121eb7..ef369b6 100644 --- a/internal/ai/harness/context_test.go +++ b/internal/ai/harness/context_test.go @@ -82,9 +82,9 @@ func (s *seeder) pass(name, decidedAt string) { batch := s.id("batch") s.exec(`INSERT INTO suggestion_batches (id, trigger, status, created_at, updated_at) VALUES (?, 'manual', 'done', ?, ?)`, batch, decidedAt, decidedAt) - s.exec(`INSERT INTO suggestions (id, dinner_id, batch_id, reason, cited_regular_ids, status, - decided_at, created_at, updated_at) VALUES (?, ?, ?, 'why', '[]', 'declined', ?, ?, ?)`, - s.id("suggestion"), s.dinner(name, "archived", "", "", 0, false), batch, decidedAt, decidedAt, decidedAt) + s.exec(`INSERT INTO suggestions (id, dinner_id, batch_id, name, reason, cited_regular_ids, status, + decided_at, created_at, updated_at) VALUES (?, ?, ?, ?, 'why', '[]', 'declined', ?, ?, ?)`, + s.id("suggestion"), s.dinner(name, "archived", "", "", 0, false), batch, name, decidedAt, decidedAt, decidedAt) } // plan inserts the week plan starting weekStart with one dinner per date; diff --git a/internal/db/household_context.sql.go b/internal/db/household_context.sql.go index d924b27..e26740a 100644 --- a/internal/db/household_context.sql.go +++ b/internal/db/household_context.sql.go @@ -54,16 +54,16 @@ func (q *Queries) ListPlannedDinners(ctx context.Context, arg ListPlannedDinners const listRecentDeclinedNames = `-- name: ListRecentDeclinedNames :many -SELECT dinners.name +SELECT name FROM suggestions -JOIN dinners ON dinners.id = suggestions.dinner_id -WHERE suggestions.status = 'declined' -ORDER BY suggestions.decided_at DESC, suggestions.id DESC +WHERE status = 'declined' +ORDER BY decided_at DESC, id DESC LIMIT ? ` // Reads for the AI household context block (internal/ai/harness). -// The names of the most recently declined suggestions, newest first. +// The names the most recently declined suggestions were suggested under, +// newest first; renaming their dinners does not change them. func (q *Queries) ListRecentDeclinedNames(ctx context.Context, limit int64) ([]string, error) { rows, err := q.db.QueryContext(ctx, listRecentDeclinedNames, limit) if err != nil { diff --git a/internal/db/models.go b/internal/db/models.go index b026169..a75716a 100644 --- a/internal/db/models.go +++ b/internal/db/models.go @@ -289,6 +289,7 @@ type Suggestion struct { ID string DinnerID string BatchID string + Name string Reason string CitedRegularIds string Status string diff --git a/internal/db/querier.go b/internal/db/querier.go index 0a27a86..b6094eb 100644 --- a/internal/db/querier.go +++ b/internal/db/querier.go @@ -21,6 +21,8 @@ type Querier interface { // Drops references to the runs created before the cutoff, so the purge // can delete them. ClearWeekPlanAIRunsBefore(ctx context.Context, cutoff string) (int64, error) + // How many suggestions ListDeck returns. + CountDeck(ctx context.Context) (int64, error) // How many Regulars the household has; archived dinners are not counted. CountRegularDinners(ctx context.Context) (int64, error) CreateAIRun(ctx context.Context, arg CreateAIRunParams) error @@ -32,6 +34,13 @@ type Querier interface { CreatePhoto(ctx context.Context, arg CreatePhotoParams) (Photo, error) CreateRecipeIngredient(ctx context.Context, arg CreateRecipeIngredientParams) error CreateRecipeStep(ctx context.Context, arg CreateRecipeStepParams) error + // name is the name the dish is suggested under, kept whatever its dinner is + // later renamed. + CreateSuggestion(ctx context.Context, arg CreateSuggestionParams) error + CreateSuggestionBatch(ctx context.Context, arg CreateSuggestionBatchParams) (SuggestionBatch, error) + // Records the household's decision on a pending suggestion; one already + // decided is left as it is (0 rows). + DecideSuggestion(ctx context.Context, arg DecideSuggestionParams) (int64, error) DeleteAIRunsBefore(ctx context.Context, cutoff string) (int64, error) DeleteRecipeIngredients(ctx context.Context, recipeID string) error DeleteRecipeSteps(ctx context.Context, recipeID string) error @@ -42,6 +51,10 @@ type Querier interface { // archived dinner is left as it is; archiving stopped its grounding. Run at // boot, after the job runner fails the jobs a previous process left running. FailOrphanedPendingDinnerGroundings(ctx context.Context, arg FailOrphanedPendingDinnerGroundingsParams) (int64, error) + // Fails every batch still pending or running whose job is no longer queued + // or running: the process stopped, or recording the batch's end failed, + // before its handler recorded a result. Run before reading the active batch. + FailOrphanedSuggestionBatches(ctx context.Context, arg FailOrphanedSuggestionBatchesParams) (int64, error) // Records why the background grounding of an unarchived dinner still // pending at the grounding_revision it was grounded for failed; a dinner // grounded some other way, renamed or archived meanwhile is left as it is @@ -49,8 +62,14 @@ type Querier interface { FailPendingDinnerGrounding(ctx context.Context, arg FailPendingDinnerGroundingParams) (int64, error) // Fails every job left running, at boot before any worker starts. FailRunningJobs(ctx context.Context, arg FailRunningJobsParams) (int64, error) + // Records a pending or running batch as done or failed; a batch already + // finished is left as it is (0 rows). + FinishSuggestionBatch(ctx context.Context, arg FinishSuggestionBatchParams) (int64, error) // Returns sql.ErrNoRows until the row exists. GetAISettings(ctx context.Context) (AiSetting, error) + // The batch still pending or running, oldest first. Returns sql.ErrNoRows + // when there is none. + GetActiveSuggestionBatch(ctx context.Context) (SuggestionBatch, error) // Returns sql.ErrNoRows until the row exists. GetChatSettings(ctx context.Context) (ChatSetting, error) GetDinner(ctx context.Context, id string) (Dinner, error) @@ -67,6 +86,10 @@ type Querier interface { GetRecipeBySourceURL(ctx context.Context, sourceUrl string) (Recipe, error) // Returns sql.ErrNoRows until the row exists. GetRecipeSearchSettings(ctx context.Context) (RecipeSearchSetting, error) + // A suggestion with its dinner. Returns sql.ErrNoRows for an unknown id. + GetSuggestion(ctx context.Context, id string) (GetSuggestionRow, error) + // The id of the job that runs batch batch_id. + GetSuggestionBatchJobID(ctx context.Context, arg GetSuggestionBatchJobIDParams) (string, error) // Records a dinner's grounding by hand (grounded or manual) and clears any // grounding_error and pasted recipe_url: the recipe chosen replaces the // link. It writes only an unarchived dinner still as it was observed @@ -82,22 +105,36 @@ type Querier interface { // minutes_total or difficulty already set is kept: a pending dinner has // them only when a member edited them while it was grounding. GroundPendingDinner(ctx context.Context, arg GroundPendingDinnerParams) (int64, error) + // The pending suggestions whose dinner is still suggested, oldest first, + // with their dinners. + ListDeck(ctx context.Context) ([]ListDeckRow, error) + // The name every declined suggestion was suggested under, oldest decision + // first; renaming its dinner does not change it. + ListDeclinedSuggestionNames(ctx context.Context) ([]string, error) ListDinnersByStatus(ctx context.Context, status string) ([]Dinner, error) // The planned dinners dated from from_date up to but not including // to_date, oldest first. Days with no dinner are left out. ListPlannedDinners(ctx context.Context, arg ListPlannedDinnersParams) ([]ListPlannedDinnersRow, error) // Reads for the AI household context block (internal/ai/harness). - // The names of the most recently declined suggestions, newest first. + // The names the most recently declined suggestions were suggested under, + // newest first; renaming their dinners does not change them. ListRecentDeclinedNames(ctx context.Context, limit int64) ([]string, error) ListRecipeIngredients(ctx context.Context, recipeID string) ([]RecipeIngredient, error) ListRecipeSteps(ctx context.Context, recipeID string) ([]RecipeStep, error) // Every dinner a new or renamed dinner must not repeat, with the // source_url of the recipe it is grounded in (NULL while it has none). ListUnarchivedDinners(ctx context.Context) ([]ListUnarchivedDinnersRow, error) + // Moves a dinner still suggested to status (quest_log or archived), + // recording updated_by; a dinner no longer suggested is left as it is + // (0 rows). + MoveSuggestedDinner(ctx context.Context, arg MoveSuggestedDinnerParams) (int64, error) // Stores a partial result of a running job. PublishJobResult(ctx context.Context, arg PublishJobResultParams) (int64, error) // An unknown id returns sql.ErrNoRows. SetDinnerFavorite(ctx context.Context, arg SetDinnerFavoriteParams) (Dinner, error) + // Marks a pending batch running; a batch in any other status is left as + // it is (0 rows). + StartSuggestionBatch(ctx context.Context, arg StartSuggestionBatchParams) (int64, error) SucceedJob(ctx context.Context, arg SucceedJobParams) (int64, error) // An unknown id returns sql.ErrNoRows. ToggleDinnerFavorite(ctx context.Context, arg ToggleDinnerFavoriteParams) (Dinner, error) diff --git a/internal/db/suggestions.sql.go b/internal/db/suggestions.sql.go new file mode 100644 index 0000000..4106a2a --- /dev/null +++ b/internal/db/suggestions.sql.go @@ -0,0 +1,409 @@ +// Code generated by sqlc. DO NOT EDIT. +// versions: +// sqlc v1.31.1 +// source: suggestions.sql + +package db + +import ( + "context" + "database/sql" +) + +const countDeck = `-- name: CountDeck :one +SELECT COUNT(*) +FROM suggestions +JOIN dinners ON dinners.id = suggestions.dinner_id +WHERE suggestions.status = 'pending' AND dinners.status = 'suggested' +` + +// How many suggestions ListDeck returns. +func (q *Queries) CountDeck(ctx context.Context) (int64, error) { + row := q.db.QueryRowContext(ctx, countDeck) + var count int64 + err := row.Scan(&count) + return count, err +} + +const createSuggestion = `-- name: CreateSuggestion :exec +INSERT INTO suggestions ( + id, dinner_id, batch_id, name, reason, cited_regular_ids, status, created_at, updated_at +) VALUES ( + ?, ?, ?, ?, ?, ?, 'pending', ?, ? +) +` + +type CreateSuggestionParams struct { + ID string + DinnerID string + BatchID string + Name string + Reason string + CitedRegularIds string + CreatedAt string + UpdatedAt string +} + +// name is the name the dish is suggested under, kept whatever its dinner is +// later renamed. +func (q *Queries) CreateSuggestion(ctx context.Context, arg CreateSuggestionParams) error { + _, err := q.db.ExecContext(ctx, createSuggestion, + arg.ID, + arg.DinnerID, + arg.BatchID, + arg.Name, + arg.Reason, + arg.CitedRegularIds, + arg.CreatedAt, + arg.UpdatedAt, + ) + return err +} + +const createSuggestionBatch = `-- name: CreateSuggestionBatch :one +INSERT INTO suggestion_batches (id, trigger, status, created_at, updated_at) +VALUES (?, ?, 'pending', ?, ?) +RETURNING id, "trigger", status, ai_run_id, created_at, updated_at +` + +type CreateSuggestionBatchParams struct { + ID string + Trigger string + CreatedAt string + UpdatedAt string +} + +func (q *Queries) CreateSuggestionBatch(ctx context.Context, arg CreateSuggestionBatchParams) (SuggestionBatch, error) { + row := q.db.QueryRowContext(ctx, createSuggestionBatch, + arg.ID, + arg.Trigger, + arg.CreatedAt, + arg.UpdatedAt, + ) + var i SuggestionBatch + err := row.Scan( + &i.ID, + &i.Trigger, + &i.Status, + &i.AiRunID, + &i.CreatedAt, + &i.UpdatedAt, + ) + return i, err +} + +const decideSuggestion = `-- name: DecideSuggestion :execrows +UPDATE suggestions +SET status = ?1, decided_at = ?2, updated_at = ?2 +WHERE id = ?3 AND status = 'pending' +` + +type DecideSuggestionParams struct { + Status string + Now sql.NullString + ID string +} + +// Records the household's decision on a pending suggestion; one already +// decided is left as it is (0 rows). +func (q *Queries) DecideSuggestion(ctx context.Context, arg DecideSuggestionParams) (int64, error) { + result, err := q.db.ExecContext(ctx, decideSuggestion, arg.Status, arg.Now, arg.ID) + if err != nil { + return 0, err + } + return result.RowsAffected() +} + +const failOrphanedSuggestionBatches = `-- name: FailOrphanedSuggestionBatches :execrows +UPDATE suggestion_batches +SET status = 'failed', updated_at = ?1 +WHERE status IN ('pending', 'running') + AND NOT EXISTS ( + SELECT 1 FROM jobs + WHERE jobs.kind = ?2 + AND jobs.status IN ('queued', 'running') + AND json_extract(jobs.payload, '$.batch_id') = suggestion_batches.id + ) +` + +type FailOrphanedSuggestionBatchesParams struct { + UpdatedAt string + Kind string +} + +// Fails every batch still pending or running whose job is no longer queued +// or running: the process stopped, or recording the batch's end failed, +// before its handler recorded a result. Run before reading the active batch. +func (q *Queries) FailOrphanedSuggestionBatches(ctx context.Context, arg FailOrphanedSuggestionBatchesParams) (int64, error) { + result, err := q.db.ExecContext(ctx, failOrphanedSuggestionBatches, arg.UpdatedAt, arg.Kind) + if err != nil { + return 0, err + } + return result.RowsAffected() +} + +const finishSuggestionBatch = `-- name: FinishSuggestionBatch :execrows +UPDATE suggestion_batches +SET status = ?, updated_at = ? +WHERE id = ? AND status IN ('pending', 'running') +` + +type FinishSuggestionBatchParams struct { + Status string + UpdatedAt string + ID string +} + +// Records a pending or running batch as done or failed; a batch already +// finished is left as it is (0 rows). +func (q *Queries) FinishSuggestionBatch(ctx context.Context, arg FinishSuggestionBatchParams) (int64, error) { + result, err := q.db.ExecContext(ctx, finishSuggestionBatch, arg.Status, arg.UpdatedAt, arg.ID) + if err != nil { + return 0, err + } + return result.RowsAffected() +} + +const getActiveSuggestionBatch = `-- name: GetActiveSuggestionBatch :one +SELECT id, "trigger", status, ai_run_id, created_at, updated_at FROM suggestion_batches +WHERE status IN ('pending', 'running') +ORDER BY id +LIMIT 1 +` + +// The batch still pending or running, oldest first. Returns sql.ErrNoRows +// when there is none. +func (q *Queries) GetActiveSuggestionBatch(ctx context.Context) (SuggestionBatch, error) { + row := q.db.QueryRowContext(ctx, getActiveSuggestionBatch) + var i SuggestionBatch + err := row.Scan( + &i.ID, + &i.Trigger, + &i.Status, + &i.AiRunID, + &i.CreatedAt, + &i.UpdatedAt, + ) + return i, err +} + +const getSuggestion = `-- name: GetSuggestion :one +SELECT suggestions.id, suggestions.dinner_id, suggestions.batch_id, suggestions.name, suggestions.reason, suggestions.cited_regular_ids, suggestions.status, suggestions.decided_at, suggestions.created_at, suggestions.updated_at, dinners.id, dinners.name, dinners.normalized_name, dinners.status, dinners.origin, dinners.method, dinners.minutes_total, dinners.difficulty, dinners.cuisine, dinners.is_favorite, dinners.recipe_id, dinners.photo_id, dinners.grounding_status, dinners.updated_by, dinners.created_at, dinners.updated_at, dinners.recipe_url, dinners.grounding_error, dinners.grounding_revision +FROM suggestions +JOIN dinners ON dinners.id = suggestions.dinner_id +WHERE suggestions.id = ? +` + +type GetSuggestionRow struct { + Suggestion Suggestion + Dinner Dinner +} + +// A suggestion with its dinner. Returns sql.ErrNoRows for an unknown id. +func (q *Queries) GetSuggestion(ctx context.Context, id string) (GetSuggestionRow, error) { + row := q.db.QueryRowContext(ctx, getSuggestion, id) + var i GetSuggestionRow + err := row.Scan( + &i.Suggestion.ID, + &i.Suggestion.DinnerID, + &i.Suggestion.BatchID, + &i.Suggestion.Name, + &i.Suggestion.Reason, + &i.Suggestion.CitedRegularIds, + &i.Suggestion.Status, + &i.Suggestion.DecidedAt, + &i.Suggestion.CreatedAt, + &i.Suggestion.UpdatedAt, + &i.Dinner.ID, + &i.Dinner.Name, + &i.Dinner.NormalizedName, + &i.Dinner.Status, + &i.Dinner.Origin, + &i.Dinner.Method, + &i.Dinner.MinutesTotal, + &i.Dinner.Difficulty, + &i.Dinner.Cuisine, + &i.Dinner.IsFavorite, + &i.Dinner.RecipeID, + &i.Dinner.PhotoID, + &i.Dinner.GroundingStatus, + &i.Dinner.UpdatedBy, + &i.Dinner.CreatedAt, + &i.Dinner.UpdatedAt, + &i.Dinner.RecipeUrl, + &i.Dinner.GroundingError, + &i.Dinner.GroundingRevision, + ) + return i, err +} + +const getSuggestionBatchJobID = `-- name: GetSuggestionBatchJobID :one +SELECT id FROM jobs +WHERE kind = ?1 + AND json_extract(payload, '$.batch_id') = ?2 +ORDER BY id +LIMIT 1 +` + +type GetSuggestionBatchJobIDParams struct { + Kind string + BatchID string +} + +// The id of the job that runs batch batch_id. +func (q *Queries) GetSuggestionBatchJobID(ctx context.Context, arg GetSuggestionBatchJobIDParams) (string, error) { + row := q.db.QueryRowContext(ctx, getSuggestionBatchJobID, arg.Kind, arg.BatchID) + var id string + err := row.Scan(&id) + return id, err +} + +const listDeck = `-- name: ListDeck :many +SELECT suggestions.id, suggestions.dinner_id, suggestions.batch_id, suggestions.name, suggestions.reason, suggestions.cited_regular_ids, suggestions.status, suggestions.decided_at, suggestions.created_at, suggestions.updated_at, dinners.id, dinners.name, dinners.normalized_name, dinners.status, dinners.origin, dinners.method, dinners.minutes_total, dinners.difficulty, dinners.cuisine, dinners.is_favorite, dinners.recipe_id, dinners.photo_id, dinners.grounding_status, dinners.updated_by, dinners.created_at, dinners.updated_at, dinners.recipe_url, dinners.grounding_error, dinners.grounding_revision +FROM suggestions +JOIN dinners ON dinners.id = suggestions.dinner_id +WHERE suggestions.status = 'pending' AND dinners.status = 'suggested' +ORDER BY suggestions.created_at, suggestions.id +` + +type ListDeckRow struct { + Suggestion Suggestion + Dinner Dinner +} + +// The pending suggestions whose dinner is still suggested, oldest first, +// with their dinners. +func (q *Queries) ListDeck(ctx context.Context) ([]ListDeckRow, error) { + rows, err := q.db.QueryContext(ctx, listDeck) + if err != nil { + return nil, err + } + defer rows.Close() + var items []ListDeckRow + for rows.Next() { + var i ListDeckRow + if err := rows.Scan( + &i.Suggestion.ID, + &i.Suggestion.DinnerID, + &i.Suggestion.BatchID, + &i.Suggestion.Name, + &i.Suggestion.Reason, + &i.Suggestion.CitedRegularIds, + &i.Suggestion.Status, + &i.Suggestion.DecidedAt, + &i.Suggestion.CreatedAt, + &i.Suggestion.UpdatedAt, + &i.Dinner.ID, + &i.Dinner.Name, + &i.Dinner.NormalizedName, + &i.Dinner.Status, + &i.Dinner.Origin, + &i.Dinner.Method, + &i.Dinner.MinutesTotal, + &i.Dinner.Difficulty, + &i.Dinner.Cuisine, + &i.Dinner.IsFavorite, + &i.Dinner.RecipeID, + &i.Dinner.PhotoID, + &i.Dinner.GroundingStatus, + &i.Dinner.UpdatedBy, + &i.Dinner.CreatedAt, + &i.Dinner.UpdatedAt, + &i.Dinner.RecipeUrl, + &i.Dinner.GroundingError, + &i.Dinner.GroundingRevision, + ); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Close(); err != nil { + return nil, err + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + +const listDeclinedSuggestionNames = `-- name: ListDeclinedSuggestionNames :many +SELECT name +FROM suggestions +WHERE status = 'declined' +ORDER BY decided_at, id +` + +// The name every declined suggestion was suggested under, oldest decision +// first; renaming its dinner does not change it. +func (q *Queries) ListDeclinedSuggestionNames(ctx context.Context) ([]string, error) { + rows, err := q.db.QueryContext(ctx, listDeclinedSuggestionNames) + if err != nil { + return nil, err + } + defer rows.Close() + var items []string + for rows.Next() { + var name string + if err := rows.Scan(&name); err != nil { + return nil, err + } + items = append(items, name) + } + if err := rows.Close(); err != nil { + return nil, err + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + +const moveSuggestedDinner = `-- name: MoveSuggestedDinner :execrows +UPDATE dinners +SET status = ?1, updated_by = ?2, updated_at = ?3 +WHERE id = ?4 AND status = 'suggested' +` + +type MoveSuggestedDinnerParams struct { + Status string + UpdatedBy sql.NullString + UpdatedAt string + ID string +} + +// Moves a dinner still suggested to status (quest_log or archived), +// recording updated_by; a dinner no longer suggested is left as it is +// (0 rows). +func (q *Queries) MoveSuggestedDinner(ctx context.Context, arg MoveSuggestedDinnerParams) (int64, error) { + result, err := q.db.ExecContext(ctx, moveSuggestedDinner, + arg.Status, + arg.UpdatedBy, + arg.UpdatedAt, + arg.ID, + ) + if err != nil { + return 0, err + } + return result.RowsAffected() +} + +const startSuggestionBatch = `-- name: StartSuggestionBatch :execrows +UPDATE suggestion_batches +SET status = 'running', updated_at = ? +WHERE id = ? AND status = 'pending' +` + +type StartSuggestionBatchParams struct { + UpdatedAt string + ID string +} + +// Marks a pending batch running; a batch in any other status is left as +// it is (0 rows). +func (q *Queries) StartSuggestionBatch(ctx context.Context, arg StartSuggestionBatchParams) (int64, error) { + result, err := q.db.ExecContext(ctx, startSuggestionBatch, arg.UpdatedAt, arg.ID) + if err != nil { + return 0, err + } + return result.RowsAffected() +} diff --git a/internal/jobs/jobs.go b/internal/jobs/jobs.go index 337addc..12cb947 100644 --- a/internal/jobs/jobs.go +++ b/internal/jobs/jobs.go @@ -54,14 +54,30 @@ const ( CodeUnknownKind = "unknown_kind" ) -// Claim retry backoff. A worker whose claim fails with a database error -// retries after claimRetryMin, doubling up to claimRetryMax, so a job queued -// when the error clears is claimed without waiting for a wake token. +// Retry backoff. A worker whose claim or final job write fails with a +// database error retries after retryMin, doubling up to retryMax, so a job +// queued when the error clears is claimed without waiting for a wake token, +// and a job the worker has finished is never left running. const ( - claimRetryMin = 50 * time.Millisecond - claimRetryMax = 5 * time.Second + retryMin = 50 * time.Millisecond + retryMax = 5 * time.Second ) +// FinalWriteTimeout bounds each attempt at a job's final write, and at a +// handler's own last write, so a writer connection that stays busy cannot +// hold a stopping worker, and with it shutdown, open. +const FinalWriteTimeout = 5 * time.Second + +// FinalWriteContext returns the context for one attempt at a final write +// of a job: it is not cancelled with ctx, so a job that finishes during +// shutdown is still recorded, but it ends after FinalWriteTimeout. A +// handler makes its own last write, the one recording what the job did, +// with it too, and returns the error when the attempt times out, so its +// worker can stop; the next Start fails the job as interrupted. +func FinalWriteContext(ctx context.Context) (context.Context, context.CancelFunc) { + return context.WithTimeout(context.WithoutCancel(ctx), FinalWriteTimeout) +} + // codePattern is the form of every failure code: lower-case snake case. var codePattern = regexp.MustCompile(`^[a-z][a-z0-9_]{0,63}$`) @@ -259,6 +275,21 @@ func (k *Kind[P]) Enqueue(ctx context.Context, payload P) (string, error) { return k.r.enqueue(ctx, k.name, data) } +// EnqueueTx stores a queued job with payload in tx and returns its id, so +// the job exists exactly when what else tx writes does. Once tx has +// committed, call Wake so an idle worker runs it. +func (k *Kind[P]) EnqueueTx(ctx context.Context, tx *sql.Tx, payload P) (string, error) { + data, err := json.Marshal(payload) + if err != nil { + return "", fmt.Errorf("jobs: enqueue %s: encode payload: %w", k.name, err) + } + return createJob(ctx, db.New(tx), k.name, data) +} + +// Wake tells an idle worker to look for a job, as Enqueue does itself: +// call it after committing the transaction a job was enqueued in. +func (k *Kind[P]) Wake() { k.r.wakeOne() } + // Enqueue stores a queued job of kind with its JSON payload and returns its // id, as Kind.Enqueue does for a kind known at compile time. It returns // ErrUnknownKind for a kind with no registered handler and @@ -280,9 +311,20 @@ func (r *Runner) Enqueue(ctx context.Context, kind string, payload json.RawMessa // enqueue stores a queued job of kind with its encoded payload and wakes a // worker. func (r *Runner) enqueue(ctx context.Context, kind string, payload []byte) (string, error) { + id, err := createJob(ctx, db.New(r.st.Writer), kind, payload) + if err != nil { + return "", err + } + r.wakeOne() + return id, nil +} + +// createJob stores a queued job of kind with its encoded payload through q +// and returns its id. +func createJob(ctx context.Context, q *db.Queries, kind string, payload []byte) (string, error) { id := store.NewID() now := store.Now().String() - if err := db.New(r.st.Writer).CreateJob(ctx, db.CreateJobParams{ + if err := q.CreateJob(ctx, db.CreateJobParams{ ID: id, Kind: kind, Payload: string(payload), @@ -291,11 +333,15 @@ func (r *Runner) enqueue(ctx context.Context, kind string, payload []byte) (stri }); err != nil { return "", fmt.Errorf("jobs: enqueue %s: %w", kind, err) } + return id, nil +} + +// wakeOne gives an idle worker a token to look for a queued job. +func (r *Runner) wakeOne() { select { case r.wake <- struct{}{}: default: // every worker already has a token and will look again } - return id, nil } // Start fails every job a previous process left running with @@ -357,16 +403,16 @@ func (r *Runner) Get(ctx context.Context, id string) (Job, error) { // already queued is not stranded by a transient error. func (r *Runner) work(ctx context.Context) { defer r.wg.Done() - backoff := claimRetryMin + backoff := retryMin for ctx.Err() == nil { job, err := r.claim(ctx, sql.NullString{String: store.Now().String(), Valid: true}) switch { case err == nil: - backoff = claimRetryMin + backoff = retryMin r.run(ctx, job) continue case errors.Is(err, sql.ErrNoRows): - backoff = claimRetryMin + backoff = retryMin select { case <-r.wake: case <-ctx.Done(): @@ -382,7 +428,7 @@ func (r *Runner) work(ctx context.Context) { case <-ctx.Done(): timer.Stop() } - backoff = min(2*backoff, claimRetryMax) + backoff = min(2*backoff, retryMax) } } @@ -390,14 +436,13 @@ func (r *Runner) work(ctx context.Context) { // ctx has been cancelled, so a handler that finishes during shutdown is // recorded. func (r *Runner) run(ctx context.Context, job db.Job) { - write := context.WithoutCancel(ctx) logger := r.logger.With("job_id", job.ID, "kind", job.Kind) q := db.New(r.st.Writer) h, ok := r.handlers[job.Kind] // read-only once started if !ok { logger.ErrorContext(ctx, "job failed", "code", CodeUnknownKind, "reason", "no handler registered for the kind") - r.fail(write, logger, job.ID, CodeUnknownKind) + r.fail(ctx, logger, job.ID, CodeUnknownKind) return } @@ -419,7 +464,7 @@ func (r *Runner) run(ctx context.Context, job db.Job) { result, code, reason := h.run(ctx, job.Payload, publish) if code != "" { if ctx.Err() != nil { - logger.WarnContext(write, "job interrupted by shutdown; it fails at the next start") + logger.WarnContext(context.WithoutCancel(ctx), "job interrupted by shutdown; it fails at the next start") return } level := slog.LevelInfo @@ -427,35 +472,61 @@ func (r *Runner) run(ctx context.Context, job db.Job) { level = slog.LevelError } logger.Log(ctx, level, "job failed", "code", code, "reason", reason) - r.fail(write, logger, job.ID, code) + r.fail(ctx, logger, job.ID, code) return } - n, err := q.SucceedJob(write, db.SucceedJobParams{ - Result: sql.NullString{String: string(result), Valid: true}, - Now: sql.NullString{String: store.Now().String(), Valid: true}, - ID: job.ID, + r.record(ctx, logger, "record job success", func(write context.Context) (int64, error) { + return q.SucceedJob(write, db.SucceedJobParams{ + Result: sql.NullString{String: string(result), Valid: true}, + Now: sql.NullString{String: store.Now().String(), Valid: true}, + ID: job.ID, + }) }) - if err == nil && n != 1 { - err = errors.New("job is not running") - } - if err != nil { - logger.ErrorContext(write, "record job success", "error", err) - } } // fail records the job as failed with code. The record errors it logs come // from the database, never from a handler. func (r *Runner) fail(ctx context.Context, logger *slog.Logger, id, code string) { - n, err := db.New(r.st.Writer).FailJob(ctx, db.FailJobParams{ - Error: sql.NullString{String: code, Valid: true}, - Now: sql.NullString{String: store.Now().String(), Valid: true}, - ID: id, + r.record(ctx, logger, "record job failure", func(write context.Context) (int64, error) { + return db.New(r.st.Writer).FailJob(write, db.FailJobParams{ + Error: sql.NullString{String: code, Valid: true}, + Now: sql.NullString{String: store.Now().String(), Valid: true}, + ID: id, + }) }) - if err == nil && n != 1 { - err = errors.New("job is not running") - } - if err != nil { - logger.ErrorContext(ctx, "record job failure", "error", err) +} + +// record makes a job's final write, which must change the one running job. +// A database error is retried with backoff until the write lands, so a +// worker never moves on from a job it has finished while leaving it +// running. Each attempt is made with FinalWriteContext, so even when ctx +// has been cancelled, and is bounded by FinalWriteTimeout; once ctx is +// cancelled, a failed or timed-out attempt is not retried and the job +// stays running, so the next Start fails it as interrupted. +func (r *Runner) record(ctx context.Context, logger *slog.Logger, what string, write func(context.Context) (int64, error)) { + backoff := retryMin + for { + attempt, cancel := FinalWriteContext(ctx) + n, err := write(attempt) + cancel() + if err == nil { + if n != 1 { + logger.ErrorContext(ctx, what, "error", "job is not running") + } + return + } + if ctx.Err() != nil { + logger.ErrorContext(context.WithoutCancel(ctx), what+" failed during shutdown; it fails at the next start", "error", err) + return + } + logger.ErrorContext(ctx, what+" failed; retrying", "error", err, "retry_in", backoff) + timer := time.NewTimer(backoff) + select { + case <-timer.C: + case <-ctx.Done(): + timer.Stop() + } + backoff = min(2*backoff, retryMax) } } diff --git a/internal/jobs/jobs_test.go b/internal/jobs/jobs_test.go index 1ab0822..ece6b13 100644 --- a/internal/jobs/jobs_test.go +++ b/internal/jobs/jobs_test.go @@ -187,6 +187,46 @@ func TestJobSucceeds(t *testing.T) { } } +func TestEnqueueTxStoresJobOnlyWithItsTransaction(t *testing.T) { + t.Parallel() + st := migratedStore(t) + r := newRunner(t, st, 1) + count := register(t, r, "count", func(_ context.Context, p countPayload, _ func(countResult) error) (countResult, error) { + return countResult{Done: p.Steps}, nil + }) + start(t, r) + ctx := context.Background() + + var rolledBack string + refused := errors.New("refused") + if err := st.WithTx(ctx, func(tx *sql.Tx) error { + var err error + rolledBack, err = count.EnqueueTx(ctx, tx, countPayload{Steps: 1}) + if err != nil { + return err + } + return refused + }); !errors.Is(err, refused) { + t.Fatalf("WithTx = %v, want the refusal", err) + } + if _, err := r.Get(ctx, rolledBack); !errors.Is(err, ErrNotFound) { + t.Errorf("job of a rolled-back transaction: Get = %v, want ErrNotFound", err) + } + + var id string + if err := st.WithTx(ctx, func(tx *sql.Tx) error { + var err error + id, err = count.EnqueueTx(ctx, tx, countPayload{Steps: 2}) + return err + }); err != nil { + t.Fatal(err) + } + count.Wake() + if job := waitFor(t, r, id, finished); job.Status != StatusSucceeded || string(job.Result) != `{"done":2}` { + t.Errorf("job = %+v (result %s), want succeeded with {\"done\":2}", job, job.Result) + } +} + func TestJobFailsWithFailureCode(t *testing.T) { t.Parallel() st := migratedStore(t) @@ -569,6 +609,98 @@ func TestClaimErrorIsRetried(t *testing.T) { } } +func TestFinalWriteErrorIsRetried(t *testing.T) { + st := migratedStore(t) + logs := &logBuffer{} + r := newRunnerLogging(t, st, 1, logs) + k := register(t, r, "count", func(ctx context.Context, p countPayload, publish func(countResult) error) (countResult, error) { + return countResult{Done: p.Steps}, nil + }) + if _, err := st.Writer.ExecContext(t.Context(), `CREATE TRIGGER refuse_end BEFORE UPDATE OF status ON jobs WHEN NEW.status IN ('succeeded', 'failed') BEGIN SELECT RAISE(ABORT, 'end refused'); END`); err != nil { + t.Fatal(err) + } + start(t, r) + id := enqueue(t, k, countPayload{Steps: 3}) + + deadline := time.Now().Add(5 * time.Second) + for !strings.Contains(logs.String(), "record job success") { + if time.Now().After(deadline) { + t.Fatal("job success never recorded") + } + time.Sleep(5 * time.Millisecond) + } + if _, err := st.Writer.ExecContext(t.Context(), `DROP TRIGGER refuse_end`); err != nil { + t.Fatal(err) + } + + job := waitFor(t, r, id, finished) + if job.Status != StatusSucceeded || string(job.Result) != `{"done":3}` { + t.Fatalf("job = %+v (result %s), want succeeded with {\"done\":3}", job, job.Result) + } + if out := logs.String(); !strings.Contains(out, "record job success failed; retrying") { + t.Fatalf("final write error not logged as retried: %s", out) + } +} + +func TestBlockedFinalWriteDoesNotHoldShutdown(t *testing.T) { + st := migratedStore(t) + r := newRunner(t, st, 1) + started, finish := make(chan struct{}), make(chan struct{}) + k := register(t, r, "count", func(ctx context.Context, p countPayload, publish func(countResult) error) (countResult, error) { + close(started) + <-finish + return countResult{Done: p.Steps}, nil + }) + stop := start(t, r) + id := enqueue(t, k, countPayload{Steps: 3}) + <-started + + // Hold the only writer connection, so the job's final write waits for it. + conn, err := st.Writer.Conn(t.Context()) + if err != nil { + t.Fatal(err) + } + waits := st.Writer.Stats().WaitCount + close(finish) + deadline := time.Now().Add(5 * time.Second) + for st.Writer.Stats().WaitCount == waits { + if time.Now().After(deadline) { + t.Fatal("final write never waited for the writer") + } + time.Sleep(5 * time.Millisecond) + } + + stopped := make(chan struct{}) + go func() { + stop() + close(stopped) + }() + // The final write's attempt ends after 5 seconds; well past that, the + // worker is waiting for the writer with no bound. + select { + case <-stopped: + case <-time.After(15 * time.Second): + conn.Close() + <-stopped + t.Fatal("worker waited for a held writer through shutdown") + } + if err := conn.Close(); err != nil { + t.Fatal(err) + } + + if job := waitFor(t, r, id, func(Job) bool { return true }); job.Status != StatusRunning { + t.Fatalf("after shutdown job = %+v, want left running", job) + } + second := newRunner(t, st, 1) + register(t, second, "count", func(context.Context, countPayload, func(countResult) error) (countResult, error) { + return countResult{}, nil + }) + start(t, second) + if job := waitFor(t, second, id, finished); job.Status != StatusFailed || job.Error == nil || *job.Error != CodeInterrupted { + t.Errorf("interrupted job = %+v, want failed with %q", job, CodeInterrupted) + } +} + func TestRunnerEnqueueByKindName(t *testing.T) { t.Parallel() st := migratedStore(t) diff --git a/internal/server/aisettings_test.go b/internal/server/aisettings_test.go index 4c58c43..bc4dd39 100644 --- a/internal/server/aisettings_test.go +++ b/internal/server/aisettings_test.go @@ -28,7 +28,7 @@ const ( func newAIRouter(t *testing.T) http.Handler { t.Helper() logger := slog.New(slog.NewTextHandler(io.Discard, nil)) - r, err := NewRouter(logger, "dev", nil, stubAuth{}, stubSessions{}, newTestSettings(t), newTestSearch(t), newTestAI(t), newTestDinners(t), + r, err := NewRouter(logger, "dev", nil, stubAuth{}, stubSessions{}, newTestSettings(t), newTestSearch(t), newTestAI(t), newTestDinners(t), newTestSuggest(t), newTestRunner(t), newTestPhotos(t), fstest.MapFS{"index.html": {Data: []byte(fixtureIndex)}}) if err != nil { t.Fatal(err) diff --git a/internal/server/dinners_test.go b/internal/server/dinners_test.go index 49e617c..a8c312f 100644 --- a/internal/server/dinners_test.go +++ b/internal/server/dinners_test.go @@ -86,7 +86,7 @@ func newDinnersEnv(t *testing.T) *dinnersEnv { t.Fatal(err) } e.h, err = NewRouter(logger, "dev", nil, stubAuth{}, stubSessions{}, newTestSettings(t), newTestSearch(t), newTestAI(t), - svc, runner, newTestPhotos(t), fstest.MapFS{"index.html": {Data: []byte(fixtureIndex)}}) + svc, newTestSuggest(t), runner, newTestPhotos(t), fstest.MapFS{"index.html": {Data: []byte(fixtureIndex)}}) if err != nil { t.Fatal(err) } diff --git a/internal/server/login_test.go b/internal/server/login_test.go index b801274..24f6a37 100644 --- a/internal/server/login_test.go +++ b/internal/server/login_test.go @@ -83,7 +83,7 @@ func newLoginRouter(t *testing.T, issuer string) http.Handler { t.Fatal(err) } r, err := NewRouter(logger, "dev", trusted, flow, sessions, newTestSettings(t), - newTestSearch(t), newTestAI(t), newTestDinners(t), newTestRunner(t), newTestPhotos(t), fstest.MapFS{"index.html": {Data: []byte(fixtureIndex)}}) + newTestSearch(t), newTestAI(t), newTestDinners(t), newTestSuggest(t), newTestRunner(t), newTestPhotos(t), fstest.MapFS{"index.html": {Data: []byte(fixtureIndex)}}) if err != nil { t.Fatal(err) } diff --git a/internal/server/media_test.go b/internal/server/media_test.go index e1aed7b..c2ce862 100644 --- a/internal/server/media_test.go +++ b/internal/server/media_test.go @@ -51,7 +51,7 @@ func newMediaRouter(t *testing.T) (r http.Handler, id, orphan string) { t.Fatal(err) } logger := slog.New(slog.NewTextHandler(io.Discard, nil)) - r, err = NewRouter(logger, "dev", nil, stubAuth{}, stubSessions{}, newTestSettings(t), newTestSearch(t), newTestAI(t), newTestDinners(t), newTestRunner(t), + r, err = NewRouter(logger, "dev", nil, stubAuth{}, stubSessions{}, newTestSettings(t), newTestSearch(t), newTestAI(t), newTestDinners(t), newTestSuggest(t), newTestRunner(t), photos, fstest.MapFS{"index.html": {Data: []byte(fixtureIndex)}}) if err != nil { t.Fatal(err) diff --git a/internal/server/origin_test.go b/internal/server/origin_test.go index ef97b38..7242e74 100644 --- a/internal/server/origin_test.go +++ b/internal/server/origin_test.go @@ -45,7 +45,7 @@ func newOriginRouter(t *testing.T, logs io.Writer) http.Handler { } logger := slog.New(forwarded.LogHandler(slog.NewJSONHandler(logs, nil))) r, err := NewRouter(logger, "dev", trusted, probeAuth{}, stubSessions{}, newTestSettings(t), - newTestSearch(t), newTestAI(t), newTestDinners(t), newTestRunner(t), newTestPhotos(t), fstest.MapFS{"index.html": {Data: []byte(fixtureIndex)}}) + newTestSearch(t), newTestAI(t), newTestDinners(t), newTestSuggest(t), newTestRunner(t), newTestPhotos(t), fstest.MapFS{"index.html": {Data: []byte(fixtureIndex)}}) if err != nil { t.Fatal(err) } diff --git a/internal/server/recipesearch_test.go b/internal/server/recipesearch_test.go index ecc0d69..5e20f89 100644 --- a/internal/server/recipesearch_test.go +++ b/internal/server/recipesearch_test.go @@ -22,7 +22,7 @@ const ( func newRecipeSearchRouter(t *testing.T, search *recipes.SearchSettingsService) http.Handler { t.Helper() logger := slog.New(slog.NewTextHandler(io.Discard, nil)) - r, err := NewRouter(logger, "dev", nil, stubAuth{}, stubSessions{}, newTestSettings(t), search, newTestAI(t), newTestDinners(t), newTestRunner(t), + r, err := NewRouter(logger, "dev", nil, stubAuth{}, stubSessions{}, newTestSettings(t), search, newTestAI(t), newTestDinners(t), newTestSuggest(t), newTestRunner(t), newTestPhotos(t), fstest.MapFS{"index.html": {Data: []byte(fixtureIndex)}}) if err != nil { t.Fatal(err) diff --git a/internal/server/server.go b/internal/server/server.go index 8eaf64c..c81b05e 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -17,6 +17,7 @@ import ( "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/recipes" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/suggest" ) // reservedPrefixes are owned by the API router and are never answered with the @@ -38,12 +39,14 @@ type RouteMounter interface { // dinnerSvc serves GET and POST /api/v1/dinners, PATCH and DELETE // /api/v1/dinners/{id}, POST /api/v1/dinners/{id}/favorite, GET // /api/v1/dinners/{id}/recipe-candidates and POST /api/v1/dinners/{id}/ground; -// runner serves +// suggestSvc serves POST /api/v1/suggestions/batches, GET +// /api/v1/suggestions/deck and POST /api/v1/suggestions/{id}/accept and +// /decline; runner serves // POST /api/v1/jobs and GET /api/v1/jobs/{id}; photos serves // GET /media/photos/{id}-{size}.jpg; assets is the web build // served for every other GET. /health, /auth and the web build are public. // Requests to /api/v1 other than GET also need the session's CSRF token in CSRFHeader. -func NewRouter(logger *slog.Logger, version string, trusted forwarded.Trusted, auth RouteMounter, sessions SessionLookup, settings *household.SettingsService, search *recipes.SearchSettingsService, ai *aisettings.Service, dinnerSvc *dinners.Service, runner *jobs.Runner, photos PhotoFiles, assets fs.FS) (http.Handler, error) { +func NewRouter(logger *slog.Logger, version string, trusted forwarded.Trusted, auth RouteMounter, sessions SessionLookup, settings *household.SettingsService, search *recipes.SearchSettingsService, ai *aisettings.Service, dinnerSvc *dinners.Service, suggestSvc *suggest.Service, runner *jobs.Runner, photos PhotoFiles, assets fs.FS) (http.Handler, error) { if settings == nil { return nil, errors.New("server: nil household settings service") } @@ -56,6 +59,9 @@ func NewRouter(logger *slog.Logger, version string, trusted forwarded.Trusted, a if dinnerSvc == nil { return nil, errors.New("server: nil dinners service") } + if suggestSvc == nil { + return nil, errors.New("server: nil suggestions service") + } if runner == nil { return nil, errors.New("server: nil job runner") } @@ -106,6 +112,10 @@ func NewRouter(logger *slog.Logger, version string, trusted forwarded.Trusted, a apiV1.Post("/dinners/{id}/favorite", postDinnerFavorite(logger, dinnerSvc)) apiV1.Get("/dinners/{id}/recipe-candidates", getDinnerCandidates(logger, dinnerSvc)) apiV1.Post("/dinners/{id}/ground", postDinnerGround(logger, dinnerSvc)) + apiV1.Post("/suggestions/batches", postSuggestionBatch(logger, suggestSvc)) + apiV1.Get("/suggestions/deck", getSuggestionDeck(logger, suggestSvc)) + apiV1.Post("/suggestions/{id}/accept", postSuggestionDecision(logger, suggestSvc, true)) + apiV1.Post("/suggestions/{id}/decline", postSuggestionDecision(logger, suggestSvc, false)) apiV1.Post("/jobs", postJob(logger, runner)) apiV1.Get("/jobs/{id}", getJob(logger, runner)) media.Get("/photos/{name}", getPhoto(logger, photos)) diff --git a/internal/server/server_test.go b/internal/server/server_test.go index 5f121ce..05a3807 100644 --- a/internal/server/server_test.go +++ b/internal/server/server_test.go @@ -5,6 +5,7 @@ import ( "crypto/rand" "encoding/base64" "encoding/json" + "errors" "io" "log/slog" "maps" @@ -18,6 +19,7 @@ import ( "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/ai/aisettings" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/ai/llm" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/ai/skills" "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" @@ -25,6 +27,7 @@ import ( "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/recipes/recipestest" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/secretbox" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/store" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/suggest" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/migrations" ) @@ -46,7 +49,7 @@ func newTestRouterWith(t *testing.T, version string, settings *household.Setting "index.html": {Data: []byte(fixtureIndex)}, "assets/index-AbC123.js": {Data: []byte(fixtureAsset)}, } - r, err := NewRouter(logger, version, nil, stubAuth{}, stubSessions{}, settings, newTestSearch(t), newTestAI(t), newTestDinners(t), runner, newTestPhotos(t), assets) + r, err := NewRouter(logger, version, nil, stubAuth{}, stubSessions{}, settings, newTestSearch(t), newTestAI(t), newTestDinners(t), newTestSuggest(t), runner, newTestPhotos(t), assets) if err != nil { t.Fatal(err) } @@ -160,6 +163,28 @@ func newTestDinners(t *testing.T) *dinners.Service { return svc } +// newTestSuggest returns a suggestions service over a migrated temp +// database, with its own unstarted job runner, a grounder that finds +// nothing and a suggester that always fails. +func newTestSuggest(t *testing.T) *suggest.Service { + t.Helper() + st := newTestStore(t) + logger := slog.New(slog.NewTextHandler(io.Discard, nil)) + runner, err := jobs.NewRunner(st, logger, 1) + if err != nil { + t.Fatal(err) + } + g := recipestest.NewGrounder(t, st, t.TempDir(), &recipestest.Search{}, &recipestest.Pages{}, &recipestest.Images{}, &llm.Fake{}) + fail := func(context.Context, string, skills.SideQuestsRequest) (skills.SideQuests, error) { + return skills.SideQuests{}, errors.New("no model in this test") + } + svc, err := suggest.NewService(st, g, fail, runner, logger) + if err != nil { + t.Fatal(err) + } + return svc +} + // newTestBox returns a secret box under a random key. func newTestBox(t *testing.T) *secretbox.Box { t.Helper() @@ -191,7 +216,7 @@ func TestNewRouterRequiresAISettings(t *testing.T) { t.Parallel() logger := slog.New(slog.NewTextHandler(io.Discard, nil)) - if _, err := NewRouter(logger, "dev", nil, stubAuth{}, stubSessions{}, newTestSettings(t), newTestSearch(t), nil, newTestDinners(t), newTestRunner(t), newTestPhotos(t), fstest.MapFS{"index.html": {}}); err == nil { + if _, err := NewRouter(logger, "dev", nil, stubAuth{}, stubSessions{}, newTestSettings(t), newTestSearch(t), nil, newTestDinners(t), newTestSuggest(t), newTestRunner(t), newTestPhotos(t), fstest.MapFS{"index.html": {}}); err == nil { t.Fatal("NewRouter accepted a nil AI settings service") } } @@ -200,7 +225,7 @@ func TestNewRouterRequiresSearchSettings(t *testing.T) { t.Parallel() logger := slog.New(slog.NewTextHandler(io.Discard, nil)) - if _, err := NewRouter(logger, "dev", nil, stubAuth{}, stubSessions{}, newTestSettings(t), nil, newTestAI(t), newTestDinners(t), newTestRunner(t), newTestPhotos(t), fstest.MapFS{"index.html": {}}); err == nil { + if _, err := NewRouter(logger, "dev", nil, stubAuth{}, stubSessions{}, newTestSettings(t), nil, newTestAI(t), newTestDinners(t), newTestSuggest(t), newTestRunner(t), newTestPhotos(t), fstest.MapFS{"index.html": {}}); err == nil { t.Fatal("NewRouter accepted a nil recipe search settings service") } } @@ -209,7 +234,7 @@ func TestNewRouterRequiresPhotoFiles(t *testing.T) { t.Parallel() logger := slog.New(slog.NewTextHandler(io.Discard, nil)) - if _, err := NewRouter(logger, "dev", nil, stubAuth{}, stubSessions{}, newTestSettings(t), newTestSearch(t), newTestAI(t), newTestDinners(t), newTestRunner(t), nil, fstest.MapFS{"index.html": {}}); err == nil { + if _, err := NewRouter(logger, "dev", nil, stubAuth{}, stubSessions{}, newTestSettings(t), newTestSearch(t), newTestAI(t), newTestDinners(t), newTestSuggest(t), newTestRunner(t), nil, fstest.MapFS{"index.html": {}}); err == nil { t.Fatal("NewRouter accepted nil photo files") } } @@ -218,7 +243,7 @@ func TestNewRouterRequiresSettings(t *testing.T) { t.Parallel() logger := slog.New(slog.NewTextHandler(io.Discard, nil)) - if _, err := NewRouter(logger, "dev", nil, stubAuth{}, stubSessions{}, nil, newTestSearch(t), newTestAI(t), newTestDinners(t), newTestRunner(t), newTestPhotos(t), fstest.MapFS{"index.html": {}}); err == nil { + if _, err := NewRouter(logger, "dev", nil, stubAuth{}, stubSessions{}, nil, newTestSearch(t), newTestAI(t), newTestDinners(t), newTestSuggest(t), newTestRunner(t), newTestPhotos(t), fstest.MapFS{"index.html": {}}); err == nil { t.Fatal("NewRouter accepted a nil settings service") } } @@ -227,16 +252,25 @@ func TestNewRouterRequiresDinners(t *testing.T) { t.Parallel() logger := slog.New(slog.NewTextHandler(io.Discard, nil)) - if _, err := NewRouter(logger, "dev", nil, stubAuth{}, stubSessions{}, newTestSettings(t), newTestSearch(t), newTestAI(t), nil, newTestRunner(t), newTestPhotos(t), fstest.MapFS{"index.html": {}}); err == nil { + if _, err := NewRouter(logger, "dev", nil, stubAuth{}, stubSessions{}, newTestSettings(t), newTestSearch(t), newTestAI(t), nil, newTestSuggest(t), newTestRunner(t), newTestPhotos(t), fstest.MapFS{"index.html": {}}); err == nil { t.Fatal("NewRouter accepted a nil dinners service") } } +func TestNewRouterRequiresSuggestions(t *testing.T) { + t.Parallel() + + logger := slog.New(slog.NewTextHandler(io.Discard, nil)) + if _, err := NewRouter(logger, "dev", nil, stubAuth{}, stubSessions{}, newTestSettings(t), newTestSearch(t), newTestAI(t), newTestDinners(t), nil, newTestRunner(t), newTestPhotos(t), fstest.MapFS{"index.html": {}}); err == nil { + t.Fatal("NewRouter accepted a nil suggestions service") + } +} + func TestNewRouterRequiresJobRunner(t *testing.T) { t.Parallel() logger := slog.New(slog.NewTextHandler(io.Discard, nil)) - if _, err := NewRouter(logger, "dev", nil, stubAuth{}, stubSessions{}, newTestSettings(t), newTestSearch(t), newTestAI(t), newTestDinners(t), nil, newTestPhotos(t), fstest.MapFS{"index.html": {}}); err == nil { + if _, err := NewRouter(logger, "dev", nil, stubAuth{}, stubSessions{}, newTestSettings(t), newTestSearch(t), newTestAI(t), newTestDinners(t), newTestSuggest(t), nil, newTestPhotos(t), fstest.MapFS{"index.html": {}}); err == nil { t.Fatal("NewRouter accepted a nil job runner") } } diff --git a/internal/server/settings_test.go b/internal/server/settings_test.go index 9570fbc..6586253 100644 --- a/internal/server/settings_test.go +++ b/internal/server/settings_test.go @@ -277,7 +277,7 @@ func TestHouseholdSettingsWithRealSession(t *testing.T) { } logger := slog.New(slog.NewTextHandler(io.Discard, nil)) router, err := NewRouter(logger, "dev", nil, stubAuth{}, sessions, household.NewSettingsService(st), - newTestSearch(t), newTestAI(t), newTestDinners(t), newTestRunner(t), newTestPhotos(t), fstest.MapFS{"index.html": {Data: []byte(fixtureIndex)}}) + newTestSearch(t), newTestAI(t), newTestDinners(t), newTestSuggest(t), newTestRunner(t), newTestPhotos(t), fstest.MapFS{"index.html": {Data: []byte(fixtureIndex)}}) if err != nil { t.Fatal(err) } diff --git a/internal/server/suggestions.go b/internal/server/suggestions.go new file mode 100644 index 0000000..dbe1b84 --- /dev/null +++ b/internal/server/suggestions.go @@ -0,0 +1,82 @@ +package server + +import ( + "errors" + "log/slog" + "net/http" + + "github.com/go-chi/chi/v5" + + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/suggest" +) + +type deck struct { + Suggestions []suggest.Suggestion `json:"suggestions"` +} + +// postSuggestionBatch starts a manual suggestion batch and answers 202 with +// its batch_id and the job_id that GET /jobs/{id} reports on, without +// waiting for the job. When a batch is already pending or running it +// answers 202 with that batch instead of starting another. +func postSuggestionBatch(logger *slog.Logger, svc *suggest.Service) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + b, err := svc.StartBatch(r.Context(), suggest.TriggerManual) + if err != nil { + writeSuggestionError(w, r, logger, "start suggestion batch", err) + return + } + writeJSON(w, r, logger, http.StatusAccepted, b) + } +} + +// getSuggestionDeck answers GET /suggestions/deck with the pending +// suggestions, oldest first. +func getSuggestionDeck(logger *slog.Logger, svc *suggest.Service) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + list, err := svc.Deck(r.Context()) + if err != nil { + writeSuggestionError(w, r, logger, "list suggestion deck", err) + return + } + writeJSON(w, r, logger, http.StatusOK, deck{Suggestions: list}) + } +} + +// postSuggestionDecision records Try it (accept) or Pass (decline) on a +// suggestion, recording the session's actor as its dinner's updated_by. +func postSuggestionDecision(logger *slog.Logger, svc *suggest.Service, accept bool) http.HandlerFunc { + decide, action := svc.Decline, "decline suggestion" + if accept { + decide, action = svc.Accept, "accept suggestion" + } + return func(w http.ResponseWriter, r *http.Request) { + actor, ok := actorOrFail(w, r, logger) + if !ok { + return + } + d, err := decide(r.Context(), chi.URLParam(r, "id"), actor.Subject) + if err != nil { + writeSuggestionError(w, r, logger, action, err) + return + } + writeJSON(w, r, logger, http.StatusOK, d) + } +} + +// writeSuggestionError maps a suggest.Service error to its API error: an +// unknown suggestion is 404, and a decision on one already decided the +// other way, or whose dinner left the deck, is 409. +func writeSuggestionError(w http.ResponseWriter, r *http.Request, logger *slog.Logger, action string, err error) { + var decided *suggest.DecidedError + switch { + case errors.As(err, &decided): + writeError(w, r, logger, http.StatusConflict, "suggestion_decided", decided.Error()) + case errors.Is(err, suggest.ErrDinnerNotSuggested): + writeError(w, r, logger, http.StatusConflict, "dinner_not_suggested", err.Error()) + case errors.Is(err, suggest.ErrNotFound): + writeError(w, r, logger, http.StatusNotFound, "not_found", "suggestion not found") + default: + logger.ErrorContext(r.Context(), action, "error", err) + writeError(w, r, logger, http.StatusInternalServerError, "internal", "internal error") + } +} diff --git a/internal/server/suggestions_test.go b/internal/server/suggestions_test.go new file mode 100644 index 0000000..7f33f3f --- /dev/null +++ b/internal/server/suggestions_test.go @@ -0,0 +1,189 @@ +package server + +import ( + "context" + "database/sql" + "errors" + "fmt" + "io" + "log/slog" + "net/http" + "net/http/httptest" + "testing" + "testing/fstest" + + "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/ai/skills" + "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/recipes/recipestest" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/store" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/suggest" +) + +const suggestionsPath = "/api/v1/suggestions" + +// newSuggestionsRouter returns a router whose suggestions service has an +// unstarted job runner, so batches stay pending, and a deck seeded with +// pending suggestions of dinners named names, in order, whose ids it +// returns. +func newSuggestionsRouter(t *testing.T, names ...string) (http.Handler, *store.Store, []string) { + t.Helper() + st := newTestStore(t) + logger := slog.New(slog.NewTextHandler(io.Discard, nil)) + runner, err := jobs.NewRunner(st, logger, 1) + if err != nil { + t.Fatal(err) + } + g := recipestest.NewGrounder(t, st, t.TempDir(), &recipestest.Search{}, &recipestest.Pages{}, &recipestest.Images{}, &llm.Fake{}) + fail := func(context.Context, string, skills.SideQuestsRequest) (skills.SideQuests, error) { + return skills.SideQuests{}, errors.New("no model in this test") + } + svc, err := suggest.NewService(st, g, fail, runner, logger) + if err != nil { + t.Fatal(err) + } + h, err := NewRouter(logger, "dev", nil, stubAuth{}, stubSessions{}, newTestSettings(t), newTestSearch(t), newTestAI(t), + newTestDinners(t), svc, runner, newTestPhotos(t), fstest.MapFS{"index.html": {Data: []byte(fixtureIndex)}}) + if err != nil { + t.Fatal(err) + } + + q := db.New(st.Writer) + ctx := t.Context() + now := store.Now().String() + batch, err := q.CreateSuggestionBatch(ctx, db.CreateSuggestionBatchParams{ID: store.NewID(), Trigger: "manual", CreatedAt: now, UpdatedAt: now}) + if err != nil { + t.Fatal(err) + } + if _, err := q.FinishSuggestionBatch(ctx, db.FinishSuggestionBatchParams{Status: "done", UpdatedAt: now, ID: batch.ID}); err != nil { + t.Fatal(err) + } + var ids []string + for i, name := range names { + d, err := q.CreateDinner(ctx, db.CreateDinnerParams{ + ID: store.NewID(), Name: name, NormalizedName: harness.Normalize(name), Status: "suggested", Origin: "side_quest", + Difficulty: sql.NullString{String: "easy", Valid: true}, GroundingStatus: "grounded", CreatedAt: now, UpdatedAt: now, + }) + if err != nil { + t.Fatal(err) + } + id := store.NewID() + if err := q.CreateSuggestion(ctx, db.CreateSuggestionParams{ + ID: id, DinnerID: d.ID, BatchID: batch.ID, Name: name, Reason: fmt.Sprintf("Reason %d", i), CitedRegularIds: "[]", + CreatedAt: store.Now().String(), UpdatedAt: now, + }); err != nil { + t.Fatal(err) + } + ids = append(ids, id) + } + return h, st, ids +} + +func TestSuggestionDeck(t *testing.T) { + t.Parallel() + h, _, ids := newSuggestionsRouter(t, "Pho", "Ramen") + + rec, body := callDinners(t, h, http.MethodGet, suggestionsPath+"/deck", "") + if rec.Code != http.StatusOK { + t.Fatalf("GET deck = %d: %s", rec.Code, rec.Body) + } + list, _ := body["suggestions"].([]any) + if len(list) != 2 { + t.Fatalf("deck = %v, want 2 suggestions", body) + } + for i, name := range []string{"Pho", "Ramen"} { + s := list[i].(map[string]any) + if s["id"] != ids[i] || s["name"] != name || s["difficulty"] != "easy" || s["reason"] != fmt.Sprintf("Reason %d", i) || s["status"] != "pending" { + t.Errorf("deck[%d] = %v, want %s", i, s, name) + } + if _, ok := s["photo_url"]; !ok { + t.Errorf("deck[%d] has no photo_url key", i) + } + } +} + +func TestSuggestionDecisions(t *testing.T) { + t.Parallel() + h, _, ids := newSuggestionsRouter(t, "Pho", "Ramen", "Laksa", "Bibimbap", "Katsu", "Tonkatsu", "Udon") + + for _, tc := range []struct { + target string + status int + code string + sugStatus string + dinnerStatus string + }{ + {ids[0] + "/accept", http.StatusOK, "", "accepted", "quest_log"}, + {ids[0] + "/accept", http.StatusOK, "", "accepted", "quest_log"}, + {ids[0] + "/decline", http.StatusConflict, "suggestion_decided", "", ""}, + {ids[1] + "/decline", http.StatusOK, "", "declined", "archived"}, + {ids[1] + "/decline", http.StatusOK, "", "declined", "archived"}, + {ids[1] + "/accept", http.StatusConflict, "suggestion_decided", "", ""}, + {"nope/accept", http.StatusNotFound, "not_found", "", ""}, + {"nope/decline", http.StatusNotFound, "not_found", "", ""}, + } { + rec, body := callDinners(t, h, http.MethodPost, suggestionsPath+"/"+tc.target, "") + if rec.Code != tc.status { + t.Fatalf("POST %s = %d, want %d: %s", tc.target, rec.Code, tc.status, rec.Body) + } + if tc.code != "" { + if errorCode(body) != tc.code { + t.Errorf("POST %s error = %v, want %s", tc.target, body, tc.code) + } + continue + } + s, _ := body["suggestion"].(map[string]any) + if s["status"] != tc.sugStatus || s["dinner_status"] != tc.dinnerStatus { + t.Errorf("POST %s = %v, want %s with dinner %s", tc.target, body, tc.sugStatus, tc.dinnerStatus) + } + if body["replenish_job_id"] != nil { + t.Errorf("POST %s replenished with 5 left: %v", tc.target, body["replenish_job_id"]) + } + } + + // A third decision leaves 4: a replenish batch starts. + rec, body := callDinners(t, h, http.MethodPost, suggestionsPath+"/"+ids[2]+"/decline", "") + if rec.Code != http.StatusOK { + t.Fatalf("decline = %d: %s", rec.Code, rec.Body) + } + if id, _ := body["replenish_job_id"].(string); id == "" { + t.Errorf("decline leaving 4 = %v, want a replenish_job_id", body) + } +} + +func TestPostSuggestionBatch(t *testing.T) { + t.Parallel() + h, st, _ := newSuggestionsRouter(t) + + rec, first := callDinners(t, h, http.MethodPost, suggestionsPath+"/batches", "") + if rec.Code != http.StatusAccepted { + t.Fatalf("POST batches = %d: %s", rec.Code, rec.Body) + } + if first["batch_id"] == nil || first["job_id"] == nil || first["trigger"] != "manual" || first["status"] != "pending" { + t.Errorf("POST batches = %v, want a pending manual batch with its job", first) + } + var kind string + if err := st.Reader.QueryRow(`SELECT kind FROM jobs WHERE id = ?`, first["job_id"]).Scan(&kind); err != nil || kind != suggest.KindSuggestionBatch { + t.Errorf("job kind = %q (%v), want %s", kind, err, suggest.KindSuggestionBatch) + } + + rec, again := callDinners(t, h, http.MethodPost, suggestionsPath+"/batches", "") + if rec.Code != http.StatusAccepted || again["batch_id"] != first["batch_id"] || again["job_id"] != first["job_id"] { + t.Errorf("second POST batches = %d %v, want the pending batch %v", rec.Code, again, first) + } +} + +func TestSuggestionWritesNeedCSRF(t *testing.T) { + t.Parallel() + h, _, ids := newSuggestionsRouter(t, "Pho") + for _, target := range []string{"/batches", "/" + ids[0] + "/accept", "/" + ids[0] + "/decline"} { + req := httptest.NewRequest(http.MethodPost, suggestionsPath+target, nil) + req.AddCookie(validSessionCookie) + rec, body := serve(t, h, req) + if rec.Code != http.StatusForbidden || errorCode(body) != "csrf" { + t.Errorf("POST %s without CSRF = %d %v, want 403 csrf", target, rec.Code, body) + } + } +} diff --git a/internal/store/migrate_test.go b/internal/store/migrate_test.go index 94b8ec3..4952f3b 100644 --- a/internal/store/migrate_test.go +++ b/internal/store/migrate_test.go @@ -3,6 +3,7 @@ package store import ( "context" "io/fs" + "reflect" "slices" "strings" "testing" @@ -175,8 +176,8 @@ func TestMigrateEmbeddedMigrations(t *testing.T) { if err := Migrate(context.Background(), s.Writer, migrations.FS); err != nil { t.Fatal(err) } - if got := versions(migrationRows(t, s)); got != "0000_bootstrap,0001_init,0002_household_settings_updated_by,0003_jobs_status,0004_ai_provider_presets,0005_ai_provider_xai,0006_ai_runs_status,0007_recipe_grounding,0008_dinner_recipe_url,0009_dinner_grounding_error,0010_dinner_grounding_revision,0011_ground_dinner_job_revision" { - t.Errorf("versions = %q, want 0000_bootstrap,0001_init,0002_household_settings_updated_by,0003_jobs_status,0004_ai_provider_presets,0005_ai_provider_xai,0006_ai_runs_status,0007_recipe_grounding,0008_dinner_recipe_url,0009_dinner_grounding_error,0010_dinner_grounding_revision,0011_ground_dinner_job_revision", got) + if got := versions(migrationRows(t, s)); got != "0000_bootstrap,0001_init,0002_household_settings_updated_by,0003_jobs_status,0004_ai_provider_presets,0005_ai_provider_xai,0006_ai_runs_status,0007_recipe_grounding,0008_dinner_recipe_url,0009_dinner_grounding_error,0010_dinner_grounding_revision,0011_ground_dinner_job_revision,0012_suggestion_name" { + t.Errorf("versions = %q, want 0000_bootstrap,0001_init,0002_household_settings_updated_by,0003_jobs_status,0004_ai_provider_presets,0005_ai_provider_xai,0006_ai_runs_status,0007_recipe_grounding,0008_dinner_recipe_url,0009_dinner_grounding_error,0010_dinner_grounding_revision,0011_ground_dinner_job_revision,0012_suggestion_name", got) } } @@ -196,8 +197,8 @@ func TestMigrateBootstrapThenEmbeddedInitApplies(t *testing.T) { if err := Migrate(ctx, s.Writer, migrations.FS); err != nil { t.Fatal(err) } - if got := versions(migrationRows(t, s)); got != "0000_bootstrap,0001_init,0002_household_settings_updated_by,0003_jobs_status,0004_ai_provider_presets,0005_ai_provider_xai,0006_ai_runs_status,0007_recipe_grounding,0008_dinner_recipe_url,0009_dinner_grounding_error,0010_dinner_grounding_revision,0011_ground_dinner_job_revision" { - t.Errorf("versions = %q, want 0000_bootstrap,0001_init,0002_household_settings_updated_by,0003_jobs_status,0004_ai_provider_presets,0005_ai_provider_xai,0006_ai_runs_status,0007_recipe_grounding,0008_dinner_recipe_url,0009_dinner_grounding_error,0010_dinner_grounding_revision,0011_ground_dinner_job_revision", got) + if got := versions(migrationRows(t, s)); got != "0000_bootstrap,0001_init,0002_household_settings_updated_by,0003_jobs_status,0004_ai_provider_presets,0005_ai_provider_xai,0006_ai_runs_status,0007_recipe_grounding,0008_dinner_recipe_url,0009_dinner_grounding_error,0010_dinner_grounding_revision,0011_ground_dinner_job_revision,0012_suggestion_name" { + t.Errorf("versions = %q, want 0000_bootstrap,0001_init,0002_household_settings_updated_by,0003_jobs_status,0004_ai_provider_presets,0005_ai_provider_xai,0006_ai_runs_status,0007_recipe_grounding,0008_dinner_recipe_url,0009_dinner_grounding_error,0010_dinner_grounding_revision,0011_ground_dinner_job_revision,0012_suggestion_name", got) } if !tableExists(t, s, "dinners") { t.Error("table from 0001_init was not created") @@ -557,3 +558,78 @@ func TestMigrateAIRunsStatus(t *testing.T) { }) } } + +// TestMigrateSuggestionName upgrades a database at 0011 holding a pending +// and a declined suggestion and checks 0012 keeps both, gives each its +// dinner's name, and refuses a suggestion with no name. +func TestMigrateSuggestionName(t *testing.T) { + ctx := context.Background() + s := openTestStore(t) + before := fstest.MapFS{} + for _, name := range []string{ + "0000_bootstrap.sql", "0001_init.sql", "0002_household_settings_updated_by.sql", "0003_jobs_status.sql", + "0004_ai_provider_presets.sql", "0005_ai_provider_xai.sql", "0006_ai_runs_status.sql", "0007_recipe_grounding.sql", + "0008_dinner_recipe_url.sql", "0009_dinner_grounding_error.sql", "0010_dinner_grounding_revision.sql", + "0011_ground_dinner_job_revision.sql", + } { + body, err := fs.ReadFile(migrations.FS, name) + if err != nil { + t.Fatal(err) + } + before[name] = &fstest.MapFile{Data: body} + } + if err := Migrate(ctx, s.Writer, before); err != nil { + t.Fatal(err) + } + for _, stmt := range []string{ + `INSERT INTO dinners (id, name, normalized_name, status, origin, is_favorite, grounding_status, created_at, updated_at) + VALUES ('dinner-1', 'Pho', 'pho', 'suggested', 'side_quest', 0, 'grounded', 'c', 'u'), + ('dinner-2', 'Ramen', 'ramen', 'archived', 'side_quest', 0, 'grounded', 'c', 'u')`, + `INSERT INTO suggestion_batches (id, trigger, status, created_at, updated_at) + VALUES ('batch-1', 'manual', 'done', 'c', 'u')`, + `INSERT INTO suggestions (id, dinner_id, batch_id, reason, cited_regular_ids, status, decided_at, created_at, updated_at) + VALUES ('sugg-1', 'dinner-1', 'batch-1', 'why 1', '[]', 'pending', NULL, 'c1', 'u1'), + ('sugg-2', 'dinner-2', 'batch-1', 'why 2', '["dinner-1"]', 'declined', 'd2', 'c2', 'u2')`, + } { + if _, err := s.Writer.Exec(stmt); err != nil { + t.Fatalf("seed: %v", err) + } + } + + if err := Migrate(ctx, s.Writer, migrations.FS); err != nil { + t.Fatal(err) + } + + rows, err := s.Reader.Query(`SELECT id, dinner_id, batch_id, name, reason, cited_regular_ids, status, COALESCE(decided_at, ''), created_at, updated_at + FROM suggestions ORDER BY id`) + if err != nil { + t.Fatal(err) + } + var got [][]string + for rows.Next() { + r := make([]string, 10) + if err := rows.Scan(&r[0], &r[1], &r[2], &r[3], &r[4], &r[5], &r[6], &r[7], &r[8], &r[9]); err != nil { + t.Fatal(err) + } + got = append(got, r) + } + if err := rows.Close(); err != nil { + t.Fatal(err) + } + want := [][]string{ + {"sugg-1", "dinner-1", "batch-1", "Pho", "why 1", "[]", "pending", "", "c1", "u1"}, + {"sugg-2", "dinner-2", "batch-1", "Ramen", "why 2", `["dinner-1"]`, "declined", "d2", "c2", "u2"}, + } + if !reflect.DeepEqual(got, want) { + t.Errorf("suggestions after 0012 =\n %v\nwant\n %v", got, want) + } + if _, err := s.Writer.Exec(`UPDATE suggestions SET name = '' WHERE id = 'sugg-1'`); err == nil || !strings.Contains(err.Error(), "CHECK constraint failed") { + t.Errorf("empty suggestion name: error %v, want CHECK constraint failure", err) + } + if _, err := s.Writer.Exec(`UPDATE suggestions SET dinner_id = 'nope' WHERE id = 'sugg-1'`); err == nil || !strings.Contains(err.Error(), "FOREIGN KEY constraint failed") { + t.Errorf("unknown dinner: error %v, want FOREIGN KEY constraint failure", err) + } + if tableExists(t, s, "suggestions_copy") { + t.Error("suggestions_copy left behind") + } +} diff --git a/internal/store/repository_test.go b/internal/store/repository_test.go index 8ae01b2..58101e8 100644 --- a/internal/store/repository_test.go +++ b/internal/store/repository_test.go @@ -95,7 +95,7 @@ func TestRepositoryInsertReadUpdate(t *testing.T) { parents: []string{"dinners", "suggestion_batches"}, row: repoRow{"suggestions", map[string]any{ "id": "sugg-1", "dinner_id": "dinner-1", "batch_id": "batch-1", - "reason": "You make chili often.", "cited_regular_ids": `["dinner-1"]`, + "name": "White Chicken Chili", "reason": "You make chili often.", "cited_regular_ids": `["dinner-1"]`, "status": "pending", "decided_at": nil, "created_at": repoTime, "updated_at": repoTime, }}, diff --git a/internal/store/schema_test.go b/internal/store/schema_test.go index eb735b8..510a752 100644 --- a/internal/store/schema_test.go +++ b/internal/store/schema_test.go @@ -110,6 +110,7 @@ func testRow(table string, i int) map[string]any { case "suggestions": r["dinner_id"] = rowID("dinners", 0) r["batch_id"] = rowID("suggestion_batches", 0) + r["name"] = fmt.Sprintf("Side quest %d", i) r["reason"] = "Like your chili." r["cited_regular_ids"] = "[]" r["status"] = "pending" diff --git a/internal/suggest/batch.go b/internal/suggest/batch.go new file mode 100644 index 0000000..00aeccb --- /dev/null +++ b/internal/suggest/batch.go @@ -0,0 +1,309 @@ +package suggest + +import ( + "context" + "database/sql" + "encoding/json" + "errors" + "fmt" + "time" + + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/ai/harness" + "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/dinners" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/jobs" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/recipes" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/store" +) + +// BatchJob is the KindSuggestionBatch payload: the batch to run. +type BatchJob struct { + BatchID string `json:"batch_id"` +} + +// Batch stages, as the job's result reports them. +const ( + StageSuggesting = "suggesting" + StageGrounding = "grounding" + StageDone = "done" +) + +// batchResult is the KindSuggestionBatch result, published as the batch +// goes: its stage, the suggestions stored so far, and how many Side +// quests were dropped, by suggest_side_quests' rules, because no recipe +// grounds them, or because they repeat a dinner added meanwhile. +type batchResult struct { + BatchID string `json:"batch_id"` + Stage string `json:"stage"` + SuggestionIDs []string `json:"suggestion_ids"` + Dropped int `json:"dropped"` +} + +func (r batchResult) PublicJSON() ([]byte, error) { return json.Marshal(r) } + +// runBatch runs batch p.BatchID: it asks suggest_side_quests for BatchSize +// Side quests given the household context, the deck and every declined +// name, then grounds them in the model's order, at most the number still +// wanted at a time, until BatchSize are kept or none are left. A Side quest +// no recipe grounds is dropped; any other grounding failure fails the +// batch, as does suggest_side_quests failing, with no Side quest made up +// instead. Each round's grounded Side quests are stored before the next +// round starts, so the deck fills as the batch goes. The batch ends done +// or failed; Side quests stored before a failure stay in the deck. When +// recording the batch's end fails, so does the job; when it times out at +// shutdown, the job is left running and the batch with it, for the next +// start to fail. +func (s *Service) runBatch(ctx context.Context, p BatchJob, publish func(batchResult) error) (batchResult, error) { + n, err := db.New(s.st.Writer).StartSuggestionBatch(ctx, db.StartSuggestionBatchParams{ + UpdatedAt: store.Now().String(), + ID: p.BatchID, + }) + if err != nil { + return batchResult{}, fmt.Errorf("suggest: start batch %s: %w", p.BatchID, err) + } + if n == 0 { + return batchResult{}, fmt.Errorf("suggest: batch %s is not pending", p.BatchID) + } + res, err := s.fill(ctx, p.BatchID, publish) + if err == nil { + if err := s.finishBatch(ctx, p.BatchID, BatchDone); err != nil { + return batchResult{}, err + } + return res, nil + } + finishErr := s.finishBatch(ctx, p.BatchID, BatchFailed) + if ctx.Err() != nil { + return batchResult{}, errors.Join(ctx.Err(), finishErr) + } + var failure *jobs.Failure + if errors.As(err, &failure) { + // Only the code is logged: the error may quote a model's or + // search provider's response. + s.logger.InfoContext(ctx, "suggestion batch failed", "batch_id", p.BatchID, "reason", failure.Code) + } + return batchResult{}, errors.Join(err, finishErr) +} + +// fill does runBatch's work, returning a *jobs.Failure for a failure the +// household may see. +func (s *Service) fill(ctx context.Context, batchID string, publish func(batchResult) error) (batchResult, error) { + res := batchResult{BatchID: batchID, Stage: StageSuggesting, SuggestionIDs: []string{}} + if err := publish(res); err != nil { + return batchResult{}, err + } + in, err := s.request(ctx) + if err != nil { + return batchResult{}, err + } + out, err := s.suggest(ctx, batchID, in) + if err != nil { + if ctx.Err() != nil { + return batchResult{}, ctx.Err() + } + return batchResult{}, &jobs.Failure{Code: ReasonAIFailed} + } + res.Stage = StageGrounding + res.Dropped = len(out.Dropped) + if err := publish(res); err != nil { + return batchResult{}, err + } + left := out.Suggestions + for len(res.SuggestionIDs) < BatchSize && len(left) > 0 { + round := left[:min(BatchSize-len(res.SuggestionIDs), len(left))] + left = left[len(round):] + grounded, err := s.ground(ctx, round) + if err != nil { + return batchResult{}, err + } + res.Dropped += len(round) - len(grounded) + ids, err := s.store(ctx, batchID, grounded) + if err != nil { + return batchResult{}, err + } + res.Dropped += len(grounded) - len(ids) + res.SuggestionIDs = append(res.SuggestionIDs, ids...) + if err := publish(res); err != nil { + return batchResult{}, err + } + } + res.Stage = StageDone + return res, nil +} + +// request reads what suggest_side_quests is given: the household context, +// the deck's names and every declined name. A household with no Regulars +// fails with ReasonNoRegulars. +func (s *Service) request(ctx context.Context) (skills.SideQuestsRequest, error) { + hctx, err := harness.BuildContext(ctx, s.st, time.Now(), ContextBudget) + if err != nil { + return skills.SideQuestsRequest{}, fmt.Errorf("suggest: %w", err) + } + if len(hctx.Regulars) == 0 { + return skills.SideQuestsRequest{}, &jobs.Failure{Code: ReasonNoRegulars} + } + q := db.New(s.st.Reader) + deck, err := q.ListDeck(ctx) + if err != nil { + return skills.SideQuestsRequest{}, fmt.Errorf("suggest: list deck: %w", err) + } + pending := []string{} + for _, row := range deck { + pending = append(pending, row.Dinner.Name) + } + declined, err := q.ListDeclinedSuggestionNames(ctx) + if err != nil { + return skills.SideQuestsRequest{}, fmt.Errorf("suggest: list declined names: %w", err) + } + return skills.SideQuestsRequest{Context: hctx, Needed: BatchSize, Pending: pending, Declined: declined}, nil +} + +// groundedQuest is a Side quest with the recipe that grounds it. +type groundedQuest struct { + quest skills.SideQuest + grounded recipes.Grounded +} + +// ground grounds quests by their search queries and returns those a +// recipe grounds, in order. A Side quest no recipe grounds is left out; +// any other failure fails the batch. +func (s *Service) ground(ctx context.Context, quests []skills.SideQuest) ([]groundedQuest, error) { + reqs := make([]recipes.GroundRequest, len(quests)) + for i, q := range quests { + reqs[i] = recipes.GroundRequest{DishName: q.Name, Query: q.SearchQuery} + } + results := s.grounder.GroundMany(ctx, reqs) + if err := ctx.Err(); err != nil { + return nil, err + } + var out []groundedQuest + for i, r := range results { + var ungroundable *recipes.ErrUngroundable + switch { + case r.Err == nil: + out = append(out, groundedQuest{quest: quests[i], grounded: r.Grounded}) + case errors.As(r.Err, &ungroundable): + case errors.Is(r.Err, recipes.ErrSearchNotConfigured): + return nil, &jobs.Failure{Code: ReasonSearchNotConfigured} + case errors.Is(r.Err, recipes.ErrSearchFailed): + return nil, &jobs.Failure{Code: ReasonSearchFailed} + case errors.Is(r.Err, recipes.ErrNormalizeFailed): + return nil, &jobs.Failure{Code: ReasonAIFailed} + default: + return nil, fmt.Errorf("suggest: ground %q: %w", quests[i].Name, r.Err) + } + } + return out, nil +} + +// store stores each grounded Side quest of batchID as a dinner with status +// suggested and origin side_quest, grounded in its recipe, and a pending +// suggestion citing its Regulars by id, all in one transaction, and +// returns the suggestion ids. A Side quest is left out when, since +// suggest_side_quests ran, a dinner it repeats was added, a suggestion it +// repeats passed, or a Regular it cites renamed or archived. Declined +// names are read in the same transaction as the store, so a Pass committed +// before it is never missed. +func (s *Service) store(ctx context.Context, batchID string, quests []groundedQuest) ([]string, error) { + ids := []string{} + err := s.st.WithTx(ctx, func(tx *sql.Tx) error { + q := db.New(tx) + existing, err := q.ListUnarchivedDinners(ctx) + if err != nil { + return fmt.Errorf("suggest: list dinners for dedupe: %w", err) + } + names, err := q.ListDeclinedSuggestionNames(ctx) + if err != nil { + return fmt.Errorf("suggest: list declined names for dedupe: %w", err) + } + regulars := map[string][]string{} + for _, row := range existing { + names = append(names, row.Dinner.Name) + if row.Dinner.Status == dinners.StatusRegular { + regulars[row.Dinner.Name] = append(regulars[row.Dinner.Name], row.Dinner.ID) + } + } + for _, gq := range quests { + cited, ok := citedIDs(gq.quest.CitedRegulars, regulars) + if !ok || repeats(gq.quest.Name, names) { + s.logger.InfoContext(ctx, "suggested dinner dropped: changed while grounding", "batch_id", batchID) + continue + } + citedJSON, err := json.Marshal(cited) + if err != nil { + return fmt.Errorf("suggest: encode cited Regulars: %w", err) + } + now := store.Now().String() + gr := gq.grounded + d, err := q.CreateDinner(ctx, db.CreateDinnerParams{ + ID: store.NewID(), + Name: gq.quest.Name, + NormalizedName: harness.Normalize(gq.quest.Name), + Status: dinners.StatusSuggested, + Origin: "side_quest", + Method: sql.NullString{String: gr.Method, Valid: true}, + MinutesTotal: nullInt(gr.MinutesTotal), + Difficulty: sql.NullString{String: gr.Difficulty, Valid: true}, + Cuisine: sql.NullString{String: gq.quest.Cuisine, Valid: true}, + RecipeID: sql.NullString{String: gr.RecipeID, Valid: true}, + PhotoID: sql.NullString{String: gr.PhotoID, Valid: true}, + GroundingStatus: dinners.GroundingGrounded, + CreatedAt: now, + UpdatedAt: now, + }) + if err != nil { + return fmt.Errorf("suggest: create dinner: %w", err) + } + id := store.NewID() + if err := q.CreateSuggestion(ctx, db.CreateSuggestionParams{ + ID: id, + DinnerID: d.ID, + BatchID: batchID, + Name: d.Name, + Reason: gq.quest.Reason, + CitedRegularIds: string(citedJSON), + CreatedAt: now, + UpdatedAt: now, + }); err != nil { + return fmt.Errorf("suggest: create suggestion: %w", err) + } + names = append(names, d.Name) + ids = append(ids, id) + } + return nil + }) + if err != nil { + return nil, err + } + return ids, nil +} + +// citedIDs returns the ids of the Regulars named cited, false when one +// names no Regular or more than one. +func citedIDs(cited []string, regulars map[string][]string) ([]string, bool) { + ids := make([]string, len(cited)) + for i, name := range cited { + if len(regulars[name]) != 1 { + return nil, false + } + ids[i] = regulars[name][0] + } + return ids, true +} + +// repeats reports whether name is harness.Similar to any of names. +func repeats(name string, names []string) bool { + for _, n := range names { + if harness.Similar(name, n) { + return true + } + } + return false +} + +func nullInt(v *int) sql.NullInt64 { + if v == nil { + return sql.NullInt64{} + } + return sql.NullInt64{Int64: int64(*v), Valid: true} +} diff --git a/internal/suggest/suggest.go b/internal/suggest/suggest.go new file mode 100644 index 0000000..efed277 --- /dev/null +++ b/internal/suggest/suggest.go @@ -0,0 +1,419 @@ +// Package suggest keeps the household's Side quest deck: batches of new +// dinners the AI suggests, each grounded in a recipe, and the household's +// Try it / Pass decision on each. A batch runs as a job: suggest_side_quests +// proposes Side quests, each is grounded with the recipe grounding pipeline +// (dropping those no recipe grounds), and every one kept is stored as a +// dinner with status suggested and origin side_quest and a pending +// suggestion. Only one batch is pending or running at a time. +package suggest + +import ( + "context" + "database/sql" + "encoding/json" + "errors" + "fmt" + "log/slog" + "slices" + + "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/dinners" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/jobs" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/recipes" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/store" +) + +// KindSuggestionBatch is the job that runs a suggestion batch. +const KindSuggestionBatch = "suggestion_batch" + +const ( + // BatchSize is how many Side quests a batch keeps at most. + BatchSize = 5 + // ReplenishBelow is the deck size under which a decision starts a + // replenish batch. + ReplenishBelow = 5 + // ContextBudget is the character budget of the household context + // suggest_side_quests is given (see harness.BuildContext). + ContextBudget = 16000 +) + +// Batch triggers (the suggestion_batches.trigger column). +const ( + TriggerScheduled = "scheduled" + TriggerManual = "manual" + TriggerReplenish = "replenish" +) + +// Batch statuses (the suggestion_batches.status column). +const ( + BatchPending = "pending" + BatchRunning = "running" + BatchDone = "done" + BatchFailed = "failed" +) + +// Suggestion statuses (the suggestions.status column). +const ( + StatusPending = "pending" + StatusAccepted = "accepted" + StatusDeclined = "declined" +) + +// Reasons a batch failed, stored as the KindSuggestionBatch job's failure +// code. Any other failure, such as the database's, is the runner's +// internal_error. +const ( + // ReasonNoRegulars: the household has no Regulars for Side quests to + // be based on. + ReasonNoRegulars = "no_regulars" + // ReasonAIFailed: suggest_side_quests, or normalize_recipe while + // grounding, failed; no Side quest is made up instead (D-017). + ReasonAIFailed = "ai_failed" + // ReasonSearchNotConfigured: recipe search is not set up. + ReasonSearchNotConfigured = "search_not_configured" + // ReasonSearchFailed: the search provider failed. + ReasonSearchFailed = "search_failed" +) + +// Reasons are every batch failure reason. +var Reasons = []string{ReasonNoRegulars, ReasonAIFailed, ReasonSearchNotConfigured, ReasonSearchFailed} + +var triggers = []string{TriggerScheduled, TriggerManual, TriggerReplenish} + +// ErrNotFound is returned for an unknown suggestion id. +var ErrNotFound = errors.New("suggestion not found") + +// ErrDinnerNotSuggested is a decision on a pending suggestion whose dinner +// was archived or moved out of the deck some other way. +var ErrDinnerNotSuggested = errors.New("the suggestion's dinner is no longer suggested") + +// DecidedError is a decision on a suggestion already decided the other +// way. +type DecidedError struct { + // Status is the suggestion's decision: accepted or declined. + Status string +} + +func (e *DecidedError) Error() string { + return "the suggestion was already " + e.Status +} + +// Suggester runs suggest_side_quests on in; scope names the batch. In +// production it is skills.SuggestSideQuests with the household's +// harness.Config. +type Suggester func(ctx context.Context, scope string, in skills.SideQuestsRequest) (skills.SideQuests, error) + +// Service runs suggestion batches and records decisions. +type Service struct { + st *store.Store + grounder *recipes.Grounder + suggest Suggester + logger *slog.Logger + batchJob *jobs.Kind[BatchJob] +} + +// NewService returns a service over st that suggests with suggest, grounds +// with grounder and registers the KindSuggestionBatch job on runner, which +// must not have started yet. +func NewService(st *store.Store, grounder *recipes.Grounder, suggest Suggester, runner *jobs.Runner, logger *slog.Logger) (*Service, error) { + switch { + case st == nil: + return nil, errors.New("suggest: nil store") + case grounder == nil: + return nil, errors.New("suggest: nil grounder") + case suggest == nil: + return nil, errors.New("suggest: nil suggester") + case runner == nil: + return nil, errors.New("suggest: nil job runner") + case logger == nil: + return nil, errors.New("suggest: nil logger") + } + s := &Service{st: st, grounder: grounder, suggest: suggest, logger: logger} + kind, err := jobs.Register(runner, KindSuggestionBatch, s.runBatch, Reasons...) + if err != nil { + return nil, err + } + s.batchJob = kind + return s, nil +} + +// Batch is a suggestion batch with the id of the job running it. +type Batch struct { + ID string `json:"batch_id"` + JobID string `json:"job_id"` + Trigger string `json:"trigger"` + Status string `json:"status"` + // Created says whether this call created the batch rather than found + // it already pending or running. + Created bool `json:"-"` +} + +// StartBatch creates a batch with trigger and enqueues the +// KindSuggestionBatch job that runs it, unless a batch is already pending +// or running, which it returns instead with Created false. +func (s *Service) StartBatch(ctx context.Context, trigger string) (Batch, error) { + if !slices.Contains(triggers, trigger) { + return Batch{}, fmt.Errorf("suggest: unknown batch trigger %q", trigger) + } + var b Batch + err := s.st.WithTx(ctx, func(tx *sql.Tx) error { + var err error + b, err = s.startBatch(ctx, tx, trigger) + return err + }) + if err != nil { + return Batch{}, err + } + if b.Created { + s.batchJob.Wake() + } + return b, nil +} + +// startBatch does StartBatch's work in tx: the batch and the job that runs +// it are committed together, so a batch is never seen without its job. A +// batch still pending or running whose job is no longer queued or running +// is failed first, so a batch whose job ended without recording its end +// (the process stopped, or the write failed) never blocks a later one. The +// caller wakes the job runner once tx has committed and Created is true. +func (s *Service) startBatch(ctx context.Context, tx *sql.Tx, trigger string) (Batch, error) { + q := db.New(tx) + n, err := q.FailOrphanedSuggestionBatches(ctx, db.FailOrphanedSuggestionBatchesParams{ + UpdatedAt: store.Now().String(), + Kind: KindSuggestionBatch, + }) + if err != nil { + return Batch{}, fmt.Errorf("suggest: fail orphaned batches: %w", err) + } + if n > 0 { + s.logger.WarnContext(ctx, "failed suggestion batches whose job ended", "count", n) + } + cur, err := q.GetActiveSuggestionBatch(ctx) + if err == nil { + jobID, err := q.GetSuggestionBatchJobID(ctx, db.GetSuggestionBatchJobIDParams{Kind: KindSuggestionBatch, BatchID: cur.ID}) + if err != nil { + return Batch{}, fmt.Errorf("suggest: read job of batch %s: %w", cur.ID, err) + } + return Batch{ID: cur.ID, JobID: jobID, Trigger: cur.Trigger, Status: cur.Status}, nil + } + if !errors.Is(err, sql.ErrNoRows) { + return Batch{}, fmt.Errorf("suggest: read active batch: %w", err) + } + now := store.Now().String() + row, err := q.CreateSuggestionBatch(ctx, db.CreateSuggestionBatchParams{ + ID: store.NewID(), + Trigger: trigger, + CreatedAt: now, + UpdatedAt: now, + }) + if err != nil { + return Batch{}, fmt.Errorf("suggest: create batch: %w", err) + } + jobID, err := s.batchJob.EnqueueTx(ctx, tx, BatchJob{BatchID: row.ID}) + if err != nil { + return Batch{}, fmt.Errorf("suggest: enqueue batch %s: %w", row.ID, err) + } + return Batch{ID: row.ID, JobID: jobID, Trigger: row.Trigger, Status: row.Status, Created: true}, nil +} + +// finishBatch records running batch id as status, with +// jobs.FinalWriteContext: the write is made even when ctx has been +// cancelled, but a writer that stays busy cannot hold its worker past +// jobs.FinalWriteTimeout. Its job's handler returns any error, so the job +// never ends succeeded with the batch still running; the runner keeps only +// a code, so the error is logged too. +func (s *Service) finishBatch(ctx context.Context, id, status string) error { + write, cancel := jobs.FinalWriteContext(ctx) + defer cancel() + n, err := db.New(s.st.Writer).FinishSuggestionBatch(write, db.FinishSuggestionBatchParams{ + Status: status, + UpdatedAt: store.Now().String(), + ID: id, + }) + if err == nil && n != 1 { + err = errors.New("batch is not running") + } + if err != nil { + s.logger.ErrorContext(ctx, "record suggestion batch status", "batch_id", id, "status", status, "error", err) + return fmt.Errorf("suggest: record batch %s %s: %w", id, status, err) + } + return nil +} + +// Suggestion is a Side quest suggestion as the API shows it, with its +// dinner. +type Suggestion struct { + ID string `json:"id"` + DinnerID string `json:"dinner_id"` + Name string `json:"name"` + PhotoURL *string `json:"photo_url"` + Difficulty *string `json:"difficulty"` + Method *string `json:"method"` + MinutesTotal *int64 `json:"minutes_total"` + Cuisine *string `json:"cuisine"` + // Reason is the "Why this?". + Reason string `json:"reason"` + CitedRegularIDs []string `json:"cited_regular_ids"` + Status string `json:"status"` + DinnerStatus string `json:"dinner_status"` + DecidedAt *string `json:"decided_at"` + CreatedAt string `json:"created_at"` +} + +// Deck returns the pending suggestions whose dinner is still suggested, +// oldest first. +func (s *Service) Deck(ctx context.Context) ([]Suggestion, error) { + rows, err := db.New(s.st.Reader).ListDeck(ctx) + if err != nil { + return nil, fmt.Errorf("suggest: list deck: %w", err) + } + out := make([]Suggestion, len(rows)) + for i, row := range rows { + out[i], err = fromRow(row.Suggestion, row.Dinner) + if err != nil { + return nil, err + } + } + return out, nil +} + +// Decision is a suggestion once decided, with the id of the replenish +// batch's job when the decision started one. +type Decision struct { + Suggestion Suggestion `json:"suggestion"` + ReplenishJobID *string `json:"replenish_job_id"` +} + +// Accept records Try it on suggestion id, moving its dinner to the Quest +// log and recording updatedBy on it. +func (s *Service) Accept(ctx context.Context, id, updatedBy string) (Decision, error) { + return s.decide(ctx, id, StatusAccepted, dinners.StatusQuestLog, updatedBy) +} + +// Decline records Pass on suggestion id, archiving its dinner and +// recording updatedBy on it. The suggestion keeps the name its dish was +// suggested under, whatever the dinner is later renamed, and every later +// batch reads it as declined, so the dish is never suggested again. +func (s *Service) Decline(ctx context.Context, id, updatedBy string) (Decision, error) { + return s.decide(ctx, id, StatusDeclined, dinners.StatusArchived, updatedBy) +} + +// decide records decision on suggestion id and moves its dinner to +// dinnerStatus. Repeating a decision changes nothing and returns the +// suggestion as it is; the opposite decision is a *DecidedError. A +// decision that leaves the deck under ReplenishBelow starts a replenish +// batch unless one is already pending or running, in the decision's own +// transaction: the decision is never recorded without the replenish it +// calls for, and a failure to start the batch records neither. +func (s *Service) decide(ctx context.Context, id, decision, dinnerStatus, updatedBy string) (Decision, error) { + if updatedBy == "" { + return Decision{}, errors.New("suggest: decide: no member to record as updated_by") + } + var row db.GetSuggestionRow + var replenish *Batch + err := s.st.WithTx(ctx, func(tx *sql.Tx) error { + q := db.New(tx) + cur, err := q.GetSuggestion(ctx, id) + if errors.Is(err, sql.ErrNoRows) { + return ErrNotFound + } + if err != nil { + return fmt.Errorf("suggest: read suggestion %s: %w", id, err) + } + switch cur.Suggestion.Status { + case decision: + row = cur + return nil + case StatusPending: + default: + return &DecidedError{Status: cur.Suggestion.Status} + } + if cur.Dinner.Status != dinners.StatusSuggested { + return ErrDinnerNotSuggested + } + now := store.Now().String() + if _, err := q.DecideSuggestion(ctx, db.DecideSuggestionParams{ + Status: decision, + Now: sql.NullString{String: now, Valid: true}, + ID: id, + }); err != nil { + return fmt.Errorf("suggest: decide %s: %w", id, err) + } + if _, err := q.MoveSuggestedDinner(ctx, db.MoveSuggestedDinnerParams{ + Status: dinnerStatus, + UpdatedBy: sql.NullString{String: updatedBy, Valid: true}, + UpdatedAt: now, + ID: cur.Dinner.ID, + }); err != nil { + return fmt.Errorf("suggest: move dinner %s: %w", cur.Dinner.ID, err) + } + row, err = q.GetSuggestion(ctx, id) + if err != nil { + return fmt.Errorf("suggest: read suggestion %s: %w", id, err) + } + left, err := q.CountDeck(ctx) + if err != nil { + return fmt.Errorf("suggest: count deck: %w", err) + } + if left >= ReplenishBelow { + return nil + } + b, err := s.startBatch(ctx, tx, TriggerReplenish) + if err != nil { + return err + } + replenish = &b + return nil + }) + if err != nil { + return Decision{}, err + } + created := replenish != nil && replenish.Created + if created { + s.batchJob.Wake() + } + sug, err := fromRow(row.Suggestion, row.Dinner) + if err != nil { + return Decision{}, err + } + d := Decision{Suggestion: sug} + if created { + d.ReplenishJobID = &replenish.JobID + } + return d, nil +} + +func fromRow(sg db.Suggestion, d db.Dinner) (Suggestion, error) { + out := Suggestion{ + ID: sg.ID, + DinnerID: d.ID, + Name: d.Name, + Difficulty: nullString(d.Difficulty), + Method: nullString(d.Method), + Cuisine: nullString(d.Cuisine), + Reason: sg.Reason, + Status: sg.Status, + DinnerStatus: d.Status, + DecidedAt: nullString(sg.DecidedAt), + CreatedAt: sg.CreatedAt, + } + if err := json.Unmarshal([]byte(sg.CitedRegularIds), &out.CitedRegularIDs); err != nil || out.CitedRegularIDs == nil { + return Suggestion{}, fmt.Errorf("suggest: suggestion %s cited_regular_ids is not a JSON array: %q", sg.ID, sg.CitedRegularIds) + } + if d.MinutesTotal.Valid { + out.MinutesTotal = &d.MinutesTotal.Int64 + } + if d.PhotoID.Valid { + u := dinners.PhotoURL(d.PhotoID.String) + out.PhotoURL = &u + } + return out, nil +} + +func nullString(s sql.NullString) *string { + if !s.Valid { + return nil + } + return &s.String +} diff --git a/internal/suggest/suggest_test.go b/internal/suggest/suggest_test.go new file mode 100644 index 0000000..157f1a5 --- /dev/null +++ b/internal/suggest/suggest_test.go @@ -0,0 +1,1144 @@ +package suggest_test + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "io" + "log/slog" + "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/ai/skills" + "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/recipes" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/recipes/recipestest" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/store" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/suggest" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/migrations" +) + +const member = "member-1" + +type env struct { + svc *suggest.Service + st *store.Store + runner *jobs.Runner + search *recipestest.Search + pages *recipestest.Pages + images *recipestest.Images + // model answers normalize_recipe while grounding; side answers + // suggest_side_quests. + model *llm.Fake + side *llm.Fake + // regulars are the Regulars' ids by name. + regulars map[string]string + grounder *recipes.Grounder + + // logs collects the service's and job runner's log output. + logs *logBuffer + + mu sync.Mutex + inputs []skills.SideQuestsRequest + // stub, when set, answers in place of suggest_side_quests. + stub func(in skills.SideQuestsRequest) skills.SideQuests +} + +// newEnv returns a suggestions service over a migrated store with the +// Regulars Beef Chili and Chicken Fajitas and an unstarted job runner. It +// suggests through the real harness and suggest_side_quests with the +// scripted e.side, and grounds with fake search, pages and images and the +// scripted e.model. +func newEnv(t *testing.T) *env { + t.Helper() + ctx := t.Context() + dataDir := t.TempDir() + st, err := store.Open(ctx, dataDir) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { + if err := st.Close(); err != nil { + t.Error(err) + } + }) + if err := store.Migrate(ctx, st.Writer, migrations.FS); err != nil { + t.Fatal(err) + } + if err := household.SeedSettings(ctx, st, "America/New_York"); err != nil { + t.Fatal(err) + } + e := &env{ + st: st, + search: &recipestest.Search{Results: map[string][]recipes.Candidate{}}, + pages: &recipestest.Pages{Bodies: map[string][]byte{}}, + images: &recipestest.Images{JPEGs: map[string][]byte{}}, + model: &llm.Fake{}, + side: &llm.Fake{}, + regulars: map[string]string{}, + logs: &logBuffer{}, + } + for _, name := range []string{"Beef Chili", "Chicken Fajitas"} { + e.regulars[name] = e.dinner(t, name, "regular") + } + g := recipestest.NewGrounder(t, st, dataDir, e.search, e.pages, e.images, e.model) + e.grounder = g + guard, err := harness.NewGuard(harness.DefaultMaxCalls) + if err != nil { + t.Fatal(err) + } + cfg := harness.Config{ + Settings: llm.Settings{Provider: "anthropic", Model: "claude-test", Temperature: 0.7}, + NewProvider: func(llm.Settings, string) (llm.Provider, error) { return e.side, nil }, + Store: st, + Guard: guard, + } + suggester := func(ctx context.Context, scope string, in skills.SideQuestsRequest) (skills.SideQuests, error) { + e.mu.Lock() + e.inputs = append(e.inputs, in) + stub := e.stub + e.mu.Unlock() + if stub != nil { + return stub(in), nil + } + return skills.SuggestSideQuests(ctx, cfg, scope, in) + } + logger := slog.New(slog.NewTextHandler(e.logs, nil)) + e.runner, err = jobs.NewRunner(st, logger, 1) + if err != nil { + t.Fatal(err) + } + e.svc, err = suggest.NewService(st, g, suggester, e.runner, logger) + if err != nil { + t.Fatal(err) + } + return e +} + +// logBuffer collects log output written by concurrent goroutines. +type logBuffer struct { + mu sync.Mutex + buf strings.Builder +} + +func (b *logBuffer) Write(p []byte) (int, error) { + b.mu.Lock() + defer b.mu.Unlock() + return b.buf.Write(p) +} + +func (b *logBuffer) String() string { + b.mu.Lock() + defer b.mu.Unlock() + return b.buf.String() +} + +// 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() + }) +} + +// dinner stores a dinner named name with status and returns its id. +func (e *env) dinner(t *testing.T, name, status string) string { + t.Helper() + now := store.Now().String() + origin := "regular" + if status == "suggested" { + origin = "side_quest" + } + d, err := db.New(e.st.Writer).CreateDinner(t.Context(), db.CreateDinnerParams{ + ID: store.NewID(), + Name: name, + NormalizedName: harness.Normalize(name), + Status: status, + Origin: origin, + GroundingStatus: "manual", + CreatedAt: now, + UpdatedAt: now, + }) + if err != nil { + t.Fatal(err) + } + return d.ID +} + +// seedDeck stores a finished batch holding pending suggestions of +// dinners named names, in order, and returns their ids. +func (e *env) seedDeck(t *testing.T, names ...string) []string { + t.Helper() + q := db.New(e.st.Writer) + now := store.Now().String() + b, err := q.CreateSuggestionBatch(t.Context(), db.CreateSuggestionBatchParams{ID: store.NewID(), Trigger: "manual", CreatedAt: now, UpdatedAt: now}) + if err != nil { + t.Fatal(err) + } + if _, err := q.FinishSuggestionBatch(t.Context(), db.FinishSuggestionBatchParams{Status: "done", UpdatedAt: now, ID: b.ID}); err != nil { + t.Fatal(err) + } + var ids []string + for _, name := range names { + id := store.NewID() + if err := q.CreateSuggestion(t.Context(), db.CreateSuggestionParams{ + ID: id, + DinnerID: e.dinner(t, name, "suggested"), + BatchID: b.ID, + Name: name, + Reason: "Like your Beef Chili.", + CitedRegularIds: fmt.Sprintf("[%q]", e.regulars["Beef Chili"]), + CreatedAt: store.Now().String(), + UpdatedAt: now, + }); err != nil { + t.Fatal(err) + } + ids = append(ids, id) + } + return ids +} + +// recipe makes the search for name find one page that grounds it, and +// scripts the normalize_recipe reply grounding it takes. +func (e *env) recipe(t *testing.T, name string) { + t.Helper() + slug := strings.ReplaceAll(strings.ToLower(name), " ", "-") + page, photo := "https://r.example/"+slug, "https://r.example/"+slug+".jpg" + e.search.Results[name] = []recipes.Candidate{{URL: page}} + e.pages.Bodies[page] = recipestest.RecipePage(name, "R Kitchen", photo, 35) + e.images.JPEGs[photo] = recipestest.JPEG(t, 400, 320, uint8(len(e.images.JPEGs)+1)) + e.model.QueueGenerate(recipestest.NormalizedReply("stovetop", 35)) +} + +// sideQuest renders one Side quest of a suggest_side_quests reply whose +// search query is its name. +func sideQuest(name, reason string, cited ...string) string { + c, _ := json.Marshal(cited) + r, _ := json.Marshal(reason) + return fmt.Sprintf(`{"name":%q,"cuisine":"Fusion","method":"stovetop","est_minutes":35,"difficulty":"easy","reason":%s,"cited_regulars":%s,"search_query":%q}`, + name, r, c, name) +} + +// reply scripts one suggest_side_quests reply holding quests. +func (e *env) reply(quests ...string) { + js := `{"suggestions":[` + strings.Join(quests, ",") + `]}` + e.side.QueueGenerate(llm.GenerateResult{Response: llm.Response{Text: js, JSON: json.RawMessage(js)}}) +} + +// waitJob waits for job id to finish and returns it. +func (e *env) waitJob(t *testing.T, id string) jobs.Job { + t.Helper() + deadline := time.Now().Add(10 * time.Second) + for { + job, err := e.runner.Get(t.Context(), id) + if err != nil { + t.Fatal(err) + } + if job.Status == jobs.StatusSucceeded || job.Status == jobs.StatusFailed { + return job + } + if time.Now().After(deadline) { + t.Fatalf("job %s still %s", id, job.Status) + } + time.Sleep(5 * time.Millisecond) + } +} + +type batchResult struct { + BatchID string `json:"batch_id"` + Stage string `json:"stage"` + SuggestionIDs []string `json:"suggestion_ids"` + Dropped int `json:"dropped"` +} + +// runBatch starts a manual batch, waits for it to succeed and returns its +// result. +func (e *env) runBatch(t *testing.T) batchResult { + t.Helper() + b, err := e.svc.StartBatch(t.Context(), suggest.TriggerManual) + if err != nil { + t.Fatal(err) + } + if !b.Created { + t.Fatalf("batch %s was already active", b.ID) + } + job := e.waitJob(t, b.JobID) + if job.Status != jobs.StatusSucceeded { + t.Fatalf("batch job %s, error %v", job.Status, *job.Error) + } + var res batchResult + if err := json.Unmarshal(job.Result, &res); err != nil { + t.Fatal(err) + } + if res.BatchID != b.ID || res.Stage != suggest.StageDone { + t.Fatalf("batch result = %+v, want batch %s done", res, b.ID) + } + if got := e.batchStatus(t, b.ID); got != suggest.BatchDone { + t.Fatalf("batch status = %s, want done", got) + } + return res +} + +func (e *env) batchStatus(t *testing.T, id string) string { + t.Helper() + var status string + if err := e.st.Reader.QueryRowContext(t.Context(), `SELECT status FROM suggestion_batches WHERE id = ?`, id).Scan(&status); err != nil { + t.Fatal(err) + } + return status +} + +func (e *env) activeBatches(t *testing.T) int { + t.Helper() + var n int + if err := e.st.Reader.QueryRowContext(t.Context(), `SELECT COUNT(*) FROM suggestion_batches WHERE status IN ('pending', 'running')`).Scan(&n); err != nil { + t.Fatal(err) + } + return n +} + +func (e *env) dinnerStatus(t *testing.T, id string) string { + t.Helper() + row, err := db.New(e.st.Reader).GetDinner(t.Context(), id) + if err != nil { + t.Fatal(err) + } + return row.Status +} + +// deckNames returns the names Deck lists, in order. +func (e *env) deckNames(t *testing.T) []string { + t.Helper() + deck, err := e.svc.Deck(t.Context()) + if err != nil { + t.Fatal(err) + } + names := []string{} + for _, s := range deck { + names = append(names, s.Name) + } + return names +} + +func TestBatchStoresGroundedSideQuests(t *testing.T) { + e := newEnv(t) + e.start(t) + e.recipe(t, "Chicken Shawarma Bowls") + e.recipe(t, "Chili Verde") + e.reply( + sideQuest("Chicken Shawarma Bowls", "Spiced chicken, like your Chicken Fajitas.", "Chicken Fajitas"), + sideQuest("Mystery Stew", "Hearty like your Beef Chili.", "Beef Chili"), // no recipe grounds it + sideQuest("Chili Verde", "Your Beef Chili gone green.", "Beef Chili"), + sideQuest("Lamb Tagine", "Slow-cooked like your Pot Roast.", "Pot Roast"), // cites no Regular + ) + + res := e.runBatch(t) + if len(res.SuggestionIDs) != 2 || res.Dropped != 2 { + t.Fatalf("result = %+v, want 2 suggestions and 2 dropped", res) + } + if g, _, _ := e.model.Remaining(); g != 0 { + t.Errorf("%d normalize_recipe replies unused", g) + } + if q := e.search.Queries(); slices.Contains(q, "Lamb Tagine") { + t.Errorf("searched %q for a Side quest the skill dropped", q) + } + + deck, err := e.svc.Deck(t.Context()) + if err != nil { + t.Fatal(err) + } + if len(deck) != 2 { + t.Fatalf("deck has %d suggestions, want 2", len(deck)) + } + want := []struct{ name, reason, regular string }{ + {"Chicken Shawarma Bowls", "Spiced chicken, like your Chicken Fajitas.", "Chicken Fajitas"}, + {"Chili Verde", "Your Beef Chili gone green.", "Beef Chili"}, + } + for i, w := range want { + s := deck[i] + if s.ID != res.SuggestionIDs[i] || s.Name != w.name || s.Reason != w.reason || s.Status != suggest.StatusPending { + t.Errorf("deck[%d] = %+v, want %s pending with reason %q", i, s, w.name, w.reason) + } + if !slices.Equal(s.CitedRegularIDs, []string{e.regulars[w.regular]}) { + t.Errorf("deck[%d] cites %v, want %s's id", i, s.CitedRegularIDs, w.regular) + } + if s.PhotoURL == nil || !strings.HasPrefix(*s.PhotoURL, "/media/photos/") || s.Difficulty == nil { + t.Errorf("deck[%d] photo %v, difficulty %v; want both", i, s.PhotoURL, s.Difficulty) + } + row, err := db.New(e.st.Reader).GetDinner(t.Context(), s.DinnerID) + if err != nil { + t.Fatal(err) + } + if row.Status != "suggested" || row.Origin != "side_quest" || row.GroundingStatus != "grounded" || !row.RecipeID.Valid || row.Cuisine.String != "Fusion" { + t.Errorf("dinner %s = %+v, want suggested side_quest grounded in a recipe", w.name, row) + } + } + var dinners int + if err := e.st.Reader.QueryRowContext(t.Context(), `SELECT COUNT(*) FROM dinners WHERE name IN ('Mystery Stew', 'Lamb Tagine')`).Scan(&dinners); err != nil { + t.Fatal(err) + } + if dinners != 0 { + t.Errorf("%d dropped Side quests stored as dinners, want none", dinners) + } +} + +func TestBatchKeepsAtMostBatchSize(t *testing.T) { + e := newEnv(t) + e.start(t) + var quests []string + for i := range suggest.BatchSize + 2 { + name := fmt.Sprintf("Fajita Dish %c", 'A'+i) + e.search.Results[name] = nil // ungroundable unless recipe is called + quests = append(quests, sideQuest(name, "Like your Chicken Fajitas.", "Chicken Fajitas")) + } + // The first round grounds BatchSize Side quests; one fails, so the + // second round grounds one more, and the last is never grounded. + for i := range suggest.BatchSize + 1 { + if i != 1 { + e.recipe(t, fmt.Sprintf("Fajita Dish %c", 'A'+i)) + } + } + e.reply(quests...) + + res := e.runBatch(t) + if len(res.SuggestionIDs) != suggest.BatchSize { + t.Fatalf("kept %d, want %d", len(res.SuggestionIDs), suggest.BatchSize) + } + if slices.Contains(e.search.Queries(), fmt.Sprintf("Fajita Dish %c", 'A'+suggest.BatchSize+1)) { + t.Error("grounded a Side quest after the batch was full") + } + names := e.deckNames(t) + if names[0] != "Fajita Dish A" || names[1] != "Fajita Dish C" || names[len(names)-1] != fmt.Sprintf("Fajita Dish %c", 'A'+suggest.BatchSize) { + t.Errorf("deck = %v, want the model's order without B", names) + } +} + +func TestDeckIsOldestFirst(t *testing.T) { + e := newEnv(t) + e.start(t) + e.recipe(t, "Chicken Shawarma Bowls") + e.recipe(t, "Chili Verde") + e.reply( + sideQuest("Chicken Shawarma Bowls", "Spiced chicken, like your Chicken Fajitas.", "Chicken Fajitas"), + sideQuest("Chili Verde", "Your Beef Chili gone green.", "Beef Chili"), + ) + e.runBatch(t) + e.recipe(t, "Beef Bulgogi") + e.reply(sideQuest("Beef Bulgogi", "Sweet beef for Beef Chili fans.", "Beef Chili")) + e.runBatch(t) + + want := []string{"Chicken Shawarma Bowls", "Chili Verde", "Beef Bulgogi"} + if got := e.deckNames(t); !slices.Equal(got, want) { + t.Errorf("deck = %v, want %v", got, want) + } + // The second batch was told the first one's Side quests are pending. + if got := e.inputs[1].Pending; !slices.Equal(got, want[:2]) { + t.Errorf("second batch pending = %v, want %v", got, want[:2]) + } +} + +func TestDeckLeavesOutDinnersNoLongerSuggested(t *testing.T) { + e := newEnv(t) + ids := e.seedDeck(t, "Pho", "Ramen") + sug, err := e.svc.Deck(t.Context()) + if err != nil { + t.Fatal(err) + } + if _, err := db.New(e.st.Writer).ArchiveDinner(t.Context(), db.ArchiveDinnerParams{ID: sug[0].DinnerID, UpdatedAt: store.Now().String()}); err != nil { + t.Fatal(err) + } + if got := e.deckNames(t); !slices.Equal(got, []string{"Ramen"}) { + t.Errorf("deck = %v, want [Ramen]", got) + } + if _, err := e.svc.Accept(t.Context(), ids[0], member); !errors.Is(err, suggest.ErrDinnerNotSuggested) { + t.Errorf("accept a suggestion whose dinner was archived: %v, want ErrDinnerNotSuggested", err) + } +} + +func TestAcceptIsIdempotent(t *testing.T) { + e := newEnv(t) + ids := e.seedDeck(t, "Pho", "Ramen", "Laksa", "Bibimbap", "Katsu", "Tonkatsu") + + first, err := e.svc.Accept(t.Context(), ids[0], member) + if err != nil { + t.Fatal(err) + } + s := first.Suggestion + if s.Status != suggest.StatusAccepted || s.DinnerStatus != "quest_log" || s.DecidedAt == nil { + t.Fatalf("accepted = %+v, want accepted with its dinner in the Quest log", s) + } + row, err := db.New(e.st.Reader).GetDinner(t.Context(), s.DinnerID) + if err != nil { + t.Fatal(err) + } + if row.Status != "quest_log" || row.UpdatedBy.String != member { + t.Errorf("dinner status %s, updated_by %q; want quest_log by %s", row.Status, row.UpdatedBy.String, member) + } + + again, err := e.svc.Accept(t.Context(), ids[0], "member-2") + if err != nil { + t.Fatal(err) + } + if *again.Suggestion.DecidedAt != *s.DecidedAt || again.ReplenishJobID != nil { + t.Errorf("repeat accept = %+v, want the first decision unchanged", again) + } + if row, _ := db.New(e.st.Reader).GetDinner(t.Context(), s.DinnerID); row.UpdatedBy.String != member { + t.Errorf("repeat accept recorded updated_by %q", row.UpdatedBy.String) + } + + var decided *suggest.DecidedError + if _, err := e.svc.Decline(t.Context(), ids[0], member); !errors.As(err, &decided) || decided.Status != suggest.StatusAccepted { + t.Fatalf("decline an accepted suggestion: %v, want *DecidedError accepted", err) + } + if got := e.dinnerStatus(t, s.DinnerID); got != "quest_log" { + t.Errorf("dinner status after refused decline = %s, want quest_log", got) + } + if got := e.deckNames(t); slices.Contains(got, "Pho") { + t.Errorf("deck %v still holds the accepted suggestion", got) + } +} + +func TestDeclineIsIdempotent(t *testing.T) { + e := newEnv(t) + ids := e.seedDeck(t, "Pho", "Ramen", "Laksa", "Bibimbap", "Katsu", "Tonkatsu") + + first, err := e.svc.Decline(t.Context(), ids[1], member) + if err != nil { + t.Fatal(err) + } + s := first.Suggestion + if s.Status != suggest.StatusDeclined || s.DinnerStatus != "archived" || s.DecidedAt == nil { + t.Fatalf("declined = %+v, want declined with its dinner archived", s) + } + again, err := e.svc.Decline(t.Context(), ids[1], member) + if err != nil { + t.Fatal(err) + } + if *again.Suggestion.DecidedAt != *s.DecidedAt || again.ReplenishJobID != nil { + t.Errorf("repeat decline = %+v, want the first decision unchanged", again) + } + var decided *suggest.DecidedError + if _, err := e.svc.Accept(t.Context(), ids[1], member); !errors.As(err, &decided) || decided.Status != suggest.StatusDeclined { + t.Fatalf("accept a declined suggestion: %v, want *DecidedError declined", err) + } + if got := e.dinnerStatus(t, s.DinnerID); got != "archived" { + t.Errorf("dinner status after refused accept = %s, want archived", got) + } + names, err := db.New(e.st.Reader).ListDeclinedSuggestionNames(t.Context()) + if err != nil { + t.Fatal(err) + } + if !slices.Equal(names, []string{"Ramen"}) { + t.Errorf("declined names = %v, want [Ramen]", names) + } +} + +func TestDecideUnknownSuggestion(t *testing.T) { + e := newEnv(t) + if _, err := e.svc.Accept(t.Context(), "nope", member); !errors.Is(err, suggest.ErrNotFound) { + t.Errorf("accept unknown: %v, want ErrNotFound", err) + } + if _, err := e.svc.Decline(t.Context(), "nope", member); !errors.Is(err, suggest.ErrNotFound) { + t.Errorf("decline unknown: %v, want ErrNotFound", err) + } +} + +func TestReplenishWhenDeckFallsBelowFive(t *testing.T) { + e := newEnv(t) // runner not started: a replenish batch stays pending + ids := e.seedDeck(t, "Pho", "Ramen", "Laksa", "Bibimbap", "Katsu", "Tonkatsu") + + d, err := e.svc.Accept(t.Context(), ids[0], member) // 5 left + if err != nil { + t.Fatal(err) + } + if d.ReplenishJobID != nil || e.activeBatches(t) != 0 { + t.Fatalf("replenished with 5 left: job %v, %d active batches", d.ReplenishJobID, e.activeBatches(t)) + } + + d, err = e.svc.Decline(t.Context(), ids[1], member) // 4 left + if err != nil { + t.Fatal(err) + } + if d.ReplenishJobID == nil { + t.Fatal("no replenish batch with 4 left") + } + job, err := e.runner.Get(t.Context(), *d.ReplenishJobID) + if err != nil { + t.Fatal(err) + } + if job.Kind != suggest.KindSuggestionBatch || job.Status != jobs.StatusQueued { + t.Errorf("replenish job = %+v, want a queued %s", job, suggest.KindSuggestionBatch) + } + var trigger string + if err := e.st.Reader.QueryRowContext(t.Context(), `SELECT trigger FROM suggestion_batches WHERE status = 'pending'`).Scan(&trigger); err != nil { + t.Fatal(err) + } + if trigger != suggest.TriggerReplenish { + t.Errorf("batch trigger = %s, want replenish", trigger) + } +} + +func TestReplenishIsSingleFlight(t *testing.T) { + e := newEnv(t) // runner not started: the replenish batch stays pending + ids := e.seedDeck(t, "Pho", "Ramen", "Laksa") + + first, err := e.svc.Decline(t.Context(), ids[0], member) + if err != nil { + t.Fatal(err) + } + if first.ReplenishJobID == nil { + t.Fatal("no replenish batch with 2 left") + } + second, err := e.svc.Accept(t.Context(), ids[1], member) + if err != nil { + t.Fatal(err) + } + if second.ReplenishJobID != nil { + t.Errorf("second decision enqueued replenish job %s while one is pending", *second.ReplenishJobID) + } + manual, err := e.svc.StartBatch(t.Context(), suggest.TriggerManual) + if err != nil { + t.Fatal(err) + } + if manual.Created || manual.JobID != *first.ReplenishJobID || manual.Trigger != suggest.TriggerReplenish { + t.Errorf("manual batch = %+v, want the pending replenish batch with job %s", manual, *first.ReplenishJobID) + } + if n := e.activeBatches(t); n != 1 { + t.Errorf("%d active batches, want 1", n) + } + var jobsQueued int + if err := e.st.Reader.QueryRowContext(t.Context(), `SELECT COUNT(*) FROM jobs WHERE kind = ?`, suggest.KindSuggestionBatch).Scan(&jobsQueued); err != nil { + t.Fatal(err) + } + if jobsQueued != 1 { + t.Errorf("%d batch jobs, want 1", jobsQueued) + } +} + +func TestStartBatchIsSingleFlightUnderConcurrency(t *testing.T) { + e := newEnv(t) + var wg sync.WaitGroup + batches := make([]suggest.Batch, 8) + for i := range batches { + wg.Add(1) + go func() { + defer wg.Done() + b, err := e.svc.StartBatch(context.Background(), suggest.TriggerManual) + if err != nil { + t.Error(err) + } + batches[i] = b + }() + } + wg.Wait() + created := 0 + for _, b := range batches { + if b.Created { + created++ + } + if b.ID != batches[0].ID || b.JobID != batches[0].JobID || b.JobID == "" { + t.Errorf("batch %+v, want %s with job %s", b, batches[0].ID, batches[0].JobID) + } + } + if created != 1 { + t.Errorf("%d calls created a batch, want 1", created) + } +} + +func TestLaterBatchNeverResuggestsDeclined(t *testing.T) { + e := newEnv(t) + e.start(t) + shawarma := sideQuest("Chicken Shawarma Bowls", "Spiced chicken, like your Chicken Fajitas.", "Chicken Fajitas") + e.recipe(t, "Chicken Shawarma Bowls") + e.reply(shawarma) + res := e.runBatch(t) + + // Passing it leaves the deck empty, so a replenish batch runs; the + // model suggests the passed dish again, respelled, with a new one. + e.recipe(t, "Beef Ragu") + e.reply( + sideQuest("chicken shawarma bowls!", "Spiced chicken, like your Chicken Fajitas.", "Chicken Fajitas"), + sideQuest("Beef Ragu", "Slow beef like your Beef Chili.", "Beef Chili"), + ) + d, err := e.svc.Decline(t.Context(), res.SuggestionIDs[0], member) + if err != nil { + t.Fatal(err) + } + if d.ReplenishJobID == nil { + t.Fatal("passing the last Side quest started no replenish batch") + } + job := e.waitJob(t, *d.ReplenishJobID) + if job.Status != jobs.StatusSucceeded { + t.Fatalf("replenish job %s: %v", job.Status, *job.Error) + } + var replenished batchResult + if err := json.Unmarshal(job.Result, &replenished); err != nil { + t.Fatal(err) + } + if len(replenished.SuggestionIDs) != 1 || replenished.Dropped != 1 { + t.Errorf("replenish result = %+v, want 1 kept and the passed dish dropped", replenished) + } + if got := e.deckNames(t); !slices.Equal(got, []string{"Beef Ragu"}) { + t.Errorf("deck = %v, want [Beef Ragu]", got) + } + e.mu.Lock() + in := e.inputs[1] + e.mu.Unlock() + if !slices.Equal(in.Declined, []string{"Chicken Shawarma Bowls"}) || !slices.Equal(in.Context.Passed, []string{"Chicken Shawarma Bowls"}) { + t.Errorf("replenish batch declined %v, passed %v; want the passed dish in both", in.Declined, in.Context.Passed) + } +} + +// TestBatchNeverStoresDishPassedWhileRunning passes a Side quest after a +// batch has read its input but before it stores, with suggest_side_quests +// answering from that input: the batch must not store the passed dish. +func TestBatchNeverStoresDishPassedWhileRunning(t *testing.T) { + e := newEnv(t) + e.start(t) + ids := e.seedDeck(t, "Chicken Shawarma Bowls") + e.recipe(t, "Chicken Shawarma Bowls") + e.recipe(t, "Beef Ragu") + held, release := make(chan struct{}), make(chan struct{}) + stub := func(skills.SideQuestsRequest) skills.SideQuests { + close(held) + <-release + quest := func(name, cited string) skills.SideQuest { + return skills.SideQuest{Name: name, Cuisine: "Fusion", Method: "stovetop", EstMinutes: 35, Difficulty: "easy", + Reason: "Like your " + cited + ".", CitedRegulars: []string{cited}, SearchQuery: name} + } + return skills.SideQuests{Suggestions: []skills.SideQuest{ + quest("Chicken Shawarma Bowls", "Chicken Fajitas"), + quest("Beef Ragu", "Beef Chili"), + }} + } + e.mu.Lock() + e.stub = stub + e.mu.Unlock() + + b, err := e.svc.StartBatch(t.Context(), suggest.TriggerManual) + if err != nil { + t.Fatal(err) + } + <-held + if _, err := e.svc.Decline(t.Context(), ids[0], member); err != nil { + close(release) + t.Fatal(err) + } + close(release) + job := e.waitJob(t, b.JobID) + if job.Status != jobs.StatusSucceeded { + t.Fatalf("batch job %s: %v", job.Status, *job.Error) + } + var res batchResult + if err := json.Unmarshal(job.Result, &res); err != nil { + t.Fatal(err) + } + if len(res.SuggestionIDs) != 1 || res.Dropped != 1 { + t.Errorf("batch result = %+v, want 1 kept and the passed dish dropped", res) + } + if got := e.deckNames(t); !slices.Equal(got, []string{"Beef Ragu"}) { + t.Errorf("deck = %v, want [Beef Ragu]", got) + } +} + +// TestLaterBatchNeverResuggestsRenamedPass passes a Side quest, renames +// its archived dinner, and checks the next batch still refuses the name it +// was suggested under. +func TestLaterBatchNeverResuggestsRenamedPass(t *testing.T) { + e := newEnv(t) // runner not started: the replenish batch waits for the rename + logger := slog.New(slog.NewTextHandler(io.Discard, nil)) + dinnerSvc, err := dinners.NewService(e.st, e.grounder, e.runner, logger) + if err != nil { + t.Fatal(err) + } + ids := e.seedDeck(t, "Chicken Shawarma Bowls") + d, err := e.svc.Decline(t.Context(), ids[0], member) + if err != nil { + t.Fatal(err) + } + if d.ReplenishJobID == nil { + t.Fatal("passing the last Side quest started no replenish batch") + } + renamed := "Leftover Wrap" + if _, err := dinnerSvc.Update(t.Context(), d.Suggestion.DinnerID, dinners.Patch{Name: &renamed}, member); err != nil { + t.Fatal(err) + } + + // The respelled passed dish would ground, so only the declined names + // keep it out of the deck. + e.recipe(t, "chicken shawarma bowls!") + e.recipe(t, "Beef Ragu") + e.reply( + sideQuest("chicken shawarma bowls!", "Spiced chicken, like your Chicken Fajitas.", "Chicken Fajitas"), + sideQuest("Beef Ragu", "Slow beef like your Beef Chili.", "Beef Chili"), + ) + e.start(t) + job := e.waitJob(t, *d.ReplenishJobID) + if job.Status != jobs.StatusSucceeded { + t.Fatalf("replenish job %s: %v", job.Status, *job.Error) + } + if got := e.deckNames(t); !slices.Equal(got, []string{"Beef Ragu"}) { + t.Errorf("deck = %v, want [Beef Ragu]", got) + } + e.mu.Lock() + in := e.inputs[0] + e.mu.Unlock() + if !slices.Equal(in.Declined, []string{"Chicken Shawarma Bowls"}) || !slices.Equal(in.Context.Passed, []string{"Chicken Shawarma Bowls"}) { + t.Errorf("replenish batch declined %v, passed %v; want the name it was suggested under in both", in.Declined, in.Context.Passed) + } +} + +// TestDecisionNeverCommitsWithoutItsReplenish makes the replenish batch's +// job fail to store: the decision must not be recorded either, so the +// retry decides again and starts the replenish. +func TestDecisionNeverCommitsWithoutItsReplenish(t *testing.T) { + e := newEnv(t) // runner not started: the replenish batch stays pending + ids := e.seedDeck(t, "Pho", "Ramen", "Laksa", "Bibimbap", "Katsu") + if _, err := e.st.Writer.ExecContext(t.Context(), `CREATE TRIGGER refuse_jobs BEFORE INSERT ON jobs BEGIN SELECT RAISE(ABORT, 'jobs refused'); END`); err != nil { + t.Fatal(err) + } + if _, err := e.svc.Decline(t.Context(), ids[0], member); err == nil { + t.Fatal("decline succeeded with no replenish job stored") + } + if got := e.deckNames(t); len(got) != 5 { + t.Errorf("deck after the failed decline = %v, want all 5 still pending", got) + } + if n := e.activeBatches(t); n != 0 { + t.Errorf("%d active batches after the failed decline, want 0", n) + } + + if _, err := e.st.Writer.ExecContext(t.Context(), `DROP TRIGGER refuse_jobs`); err != nil { + t.Fatal(err) + } + d, err := e.svc.Decline(t.Context(), ids[0], member) + if err != nil { + t.Fatal(err) + } + if d.Suggestion.Status != suggest.StatusDeclined || d.ReplenishJobID == nil { + t.Fatalf("retried decline = %+v, want declined with a replenish job", d) + } + job, err := e.runner.Get(t.Context(), *d.ReplenishJobID) + if err != nil { + t.Fatal(err) + } + if job.Kind != suggest.KindSuggestionBatch || job.Status != jobs.StatusQueued { + t.Errorf("replenish job = %+v, want a queued %s", job, suggest.KindSuggestionBatch) + } + if n := e.activeBatches(t); n != 1 { + t.Errorf("%d active batches, want 1", n) + } +} + +func TestBatchFailsWhenAIFails(t *testing.T) { + e := newEnv(t) + e.start(t) + e.side.QueueGenerate(llm.GenerateResult{Err: errors.New("provider down")}) + + b, err := e.svc.StartBatch(t.Context(), suggest.TriggerManual) + if err != nil { + t.Fatal(err) + } + job := e.waitJob(t, b.JobID) + if job.Status != jobs.StatusFailed || *job.Error != suggest.ReasonAIFailed { + t.Fatalf("job = %s %v, want failed %s", job.Status, job.Error, suggest.ReasonAIFailed) + } + if got := e.batchStatus(t, b.ID); got != suggest.BatchFailed { + t.Errorf("batch status = %s, want failed", got) + } + if got := e.deckNames(t); len(got) != 0 { + t.Errorf("deck = %v after a failed batch, want empty", got) + } + // A failed batch does not block the next one. + next, err := e.svc.StartBatch(t.Context(), suggest.TriggerManual) + if err != nil { + t.Fatal(err) + } + if !next.Created || next.ID == b.ID { + t.Errorf("next batch = %+v, want a new one", next) + } +} + +func TestBatchFailsWhenSearchFails(t *testing.T) { + e := newEnv(t) + e.start(t) + e.search.Err = errors.New("search down") + e.reply(sideQuest("Chili Verde", "Your Beef Chili gone green.", "Beef Chili")) + + b, err := e.svc.StartBatch(t.Context(), suggest.TriggerManual) + if err != nil { + t.Fatal(err) + } + job := e.waitJob(t, b.JobID) + if job.Status != jobs.StatusFailed || *job.Error != suggest.ReasonSearchFailed { + t.Fatalf("job = %s %v, want failed %s", job.Status, job.Error, suggest.ReasonSearchFailed) + } + if got := e.deckNames(t); len(got) != 0 { + t.Errorf("deck = %v, want empty", got) + } +} + +func TestBatchFailsWithNoRegulars(t *testing.T) { + e := newEnv(t) + e.start(t) + for _, id := range e.regulars { + if _, err := db.New(e.st.Writer).ArchiveDinner(t.Context(), db.ArchiveDinnerParams{ID: id, UpdatedAt: store.Now().String()}); err != nil { + t.Fatal(err) + } + } + b, err := e.svc.StartBatch(t.Context(), suggest.TriggerManual) + if err != nil { + t.Fatal(err) + } + job := e.waitJob(t, b.JobID) + if job.Status != jobs.StatusFailed || *job.Error != suggest.ReasonNoRegulars { + t.Fatalf("job = %s %v, want failed %s", job.Status, job.Error, suggest.ReasonNoRegulars) + } + if reqs := e.side.Requests(); len(reqs) != 0 { + t.Errorf("model called %d times with no Regulars", len(reqs)) + } +} + +func TestStartBatchFailsOrphanedBatch(t *testing.T) { + e := newEnv(t) + now := store.Now().String() + orphan, err := db.New(e.st.Writer).CreateSuggestionBatch(t.Context(), db.CreateSuggestionBatchParams{ID: store.NewID(), Trigger: "manual", CreatedAt: now, UpdatedAt: now}) + if err != nil { + t.Fatal(err) + } + b, err := e.svc.StartBatch(t.Context(), suggest.TriggerManual) + if err != nil { + t.Fatal(err) + } + if !b.Created || b.ID == orphan.ID { + t.Fatalf("batch = %+v, want a new one past the orphan", b) + } + if got := e.batchStatus(t, orphan.ID); got != suggest.BatchFailed { + t.Errorf("orphan status = %s, want failed", got) + } + // A batch whose job is still queued is not an orphan. + again, err := e.svc.StartBatch(t.Context(), suggest.TriggerManual) + if err != nil { + t.Fatal(err) + } + if again.Created || again.ID != b.ID || e.batchStatus(t, b.ID) != suggest.BatchPending { + t.Errorf("second batch = %+v, status %s; want batch %s still pending", again, e.batchStatus(t, b.ID), b.ID) + } +} + +func TestBatchJobFailsWhenRecordingItsEndFails(t *testing.T) { + e := newEnv(t) + e.start(t) + e.recipe(t, "Chili Verde") + e.reply(sideQuest("Chili Verde", "Your Beef Chili gone green.", "Beef Chili")) + if _, err := e.st.Writer.ExecContext(t.Context(), `CREATE TRIGGER refuse_done BEFORE UPDATE OF status ON suggestion_batches WHEN NEW.status = 'done' BEGIN SELECT RAISE(ABORT, 'done refused'); END`); err != nil { + t.Fatal(err) + } + + b, err := e.svc.StartBatch(t.Context(), suggest.TriggerManual) + if err != nil { + t.Fatal(err) + } + job := e.waitJob(t, b.JobID) + if job.Status != jobs.StatusFailed { + t.Fatalf("job %s with its batch %s, want failed", job.Status, e.batchStatus(t, b.ID)) + } + + if _, err := e.st.Writer.ExecContext(t.Context(), `DROP TRIGGER refuse_done`); err != nil { + t.Fatal(err) + } + next, err := e.svc.StartBatch(t.Context(), suggest.TriggerManual) + if err != nil { + t.Fatal(err) + } + if !next.Created || next.ID == b.ID { + t.Errorf("next batch = %+v, want a new one", next) + } + if got := e.batchStatus(t, b.ID); got != suggest.BatchFailed { + t.Errorf("batch status = %s with its job failed, want failed", got) + } +} + +func TestBatchRecoversWhenRecordingItsEndAndItsJobsEndFail(t *testing.T) { + e := newEnv(t) + e.start(t) + e.recipe(t, "Chili Verde") + e.reply(sideQuest("Chili Verde", "Your Beef Chili gone green.", "Beef Chili")) + for _, trigger := range []string{ + `CREATE TRIGGER refuse_done BEFORE UPDATE OF status ON suggestion_batches WHEN NEW.status = 'done' BEGIN SELECT RAISE(ABORT, 'done refused'); END`, + `CREATE TRIGGER refuse_job_end BEFORE UPDATE OF status ON jobs WHEN NEW.status IN ('succeeded', 'failed') BEGIN SELECT RAISE(ABORT, 'job end refused'); END`, + } { + if _, err := e.st.Writer.ExecContext(t.Context(), trigger); err != nil { + t.Fatal(err) + } + } + + b, err := e.svc.StartBatch(t.Context(), suggest.TriggerManual) + if err != nil { + t.Fatal(err) + } + deadline := time.Now().Add(10 * time.Second) + for !strings.Contains(e.logs.String(), "record job failure") { + if time.Now().After(deadline) { + t.Fatalf("job end never recorded; batch %s", e.batchStatus(t, b.ID)) + } + time.Sleep(5 * time.Millisecond) + } + // While neither end is recorded the batch is still the active one. + again, err := e.svc.StartBatch(t.Context(), suggest.TriggerManual) + if err != nil { + t.Fatal(err) + } + if again.Created || again.ID != b.ID { + t.Fatalf("batch = %+v while batch %s is running, want it", again, b.ID) + } + + // The database recovers; the service is not restarted. + for _, trigger := range []string{"refuse_done", "refuse_job_end"} { + if _, err := e.st.Writer.ExecContext(t.Context(), "DROP TRIGGER "+trigger); err != nil { + t.Fatal(err) + } + } + job := e.waitJob(t, b.JobID) + if job.Status != jobs.StatusFailed { + t.Fatalf("job %s, want failed", job.Status) + } + next, err := e.svc.StartBatch(t.Context(), suggest.TriggerManual) + if err != nil { + t.Fatal(err) + } + if !next.Created || next.ID == b.ID { + t.Errorf("next batch = %+v, want a new one", next) + } + if got := e.batchStatus(t, b.ID); got != suggest.BatchFailed { + t.Errorf("batch status = %s with its job failed, want failed", got) + } +} + +func TestBlockedBatchEndDoesNotHoldShutdown(t *testing.T) { + e := newEnv(t) + ctx, cancel := context.WithCancel(context.Background()) + if err := e.runner.Start(ctx); err != nil { + cancel() + t.Fatal(err) + } + stopped := make(chan struct{}) + t.Cleanup(func() { + cancel() + <-stopped + }) + e.search.Gate = make(chan struct{}) + e.search.Err = errors.New("search down") + e.reply(sideQuest("Chili Verde", "Your Beef Chili gone green.", "Beef Chili")) + + b, err := e.svc.StartBatch(t.Context(), suggest.TriggerManual) + if err != nil { + t.Fatal(err) + } + deadline := time.Now().Add(10 * time.Second) + for len(e.search.Queries()) == 0 { + if time.Now().After(deadline) { + t.Fatal("batch never searched") + } + time.Sleep(5 * time.Millisecond) + } + + // Hold the only writer connection, so recording the batch's end + // waits for it once the search fails. + conn, err := e.st.Writer.Conn(t.Context()) + if err != nil { + t.Fatal(err) + } + waits := e.st.Writer.Stats().WaitCount + close(e.search.Gate) + for e.st.Writer.Stats().WaitCount == waits { + if time.Now().After(deadline) { + conn.Close() + t.Fatal("batch end never waited for the writer") + } + time.Sleep(5 * time.Millisecond) + } + + go func() { + e.runner.Wait() + close(stopped) + }() + cancel() + // Each final write's attempt ends after jobs.FinalWriteTimeout; well + // past that, the worker is waiting for the writer with no bound. + select { + case <-stopped: + case <-time.After(jobs.FinalWriteTimeout + 10*time.Second): + conn.Close() + <-stopped + t.Fatal("worker waited for a held writer through shutdown") + } + if err := conn.Close(); err != nil { + t.Fatal(err) + } + if got := e.batchStatus(t, b.ID); got != suggest.BatchRunning { + t.Fatalf("after shutdown batch status = %s, want left running", got) + } + + // The next process fails the job as interrupted, and the batch no + // longer blocks the next one. + logger := slog.New(slog.NewTextHandler(io.Discard, nil)) + next, err := jobs.NewRunner(e.st, logger, 1) + if err != nil { + t.Fatal(err) + } + refuse := func(context.Context, string, skills.SideQuestsRequest) (skills.SideQuests, error) { + return skills.SideQuests{}, errors.New("not wanted") + } + svc, err := suggest.NewService(e.st, e.grounder, refuse, next, logger) + if err != nil { + t.Fatal(err) + } + nextCtx, nextCancel := context.WithCancel(context.Background()) + if err := next.Start(nextCtx); err != nil { + nextCancel() + t.Fatal(err) + } + t.Cleanup(func() { + nextCancel() + next.Wait() + }) + job, err := next.Get(t.Context(), b.JobID) + if err != nil { + t.Fatal(err) + } + if job.Status != jobs.StatusFailed || job.Error == nil || *job.Error != jobs.CodeInterrupted { + t.Fatalf("interrupted job = %+v, want failed with %q", job, jobs.CodeInterrupted) + } + again, err := svc.StartBatch(t.Context(), suggest.TriggerManual) + if err != nil { + t.Fatal(err) + } + if !again.Created || again.ID == b.ID { + t.Errorf("next batch = %+v, want a new one", again) + } + if got := e.batchStatus(t, b.ID); got != suggest.BatchFailed { + t.Errorf("interrupted batch status = %s, want failed", got) + } +} + +func TestStartBatchRefusesUnknownTrigger(t *testing.T) { + e := newEnv(t) + if _, err := e.svc.StartBatch(t.Context(), "whenever"); err == nil { + t.Fatal("unknown trigger accepted") + } +} diff --git a/migrations/0012_suggestion_name.sql b/migrations/0012_suggestion_name.sql new file mode 100644 index 0000000..d54cc3b --- /dev/null +++ b/migrations/0012_suggestion_name.sql @@ -0,0 +1,37 @@ +-- 0012_suggestion_name: a suggestion keeps the name its dish was suggested +-- under. Its dinner can be renamed, even once archived by a Pass, so the +-- declined names every later batch refuses are read from here, never from +-- the dinner. A suggestion stored before this takes its dinner's name now. +-- SQLite cannot add a NOT NULL column without a default, so the table is +-- rebuilt as 0005 does: copy the rows aside, drop the table, create it +-- again, copy the rows back and drop the copy. Nothing references +-- suggestions. + +CREATE TABLE suggestions_copy AS SELECT * FROM suggestions; + +DROP TABLE suggestions; + +CREATE TABLE suggestions ( + id TEXT NOT NULL PRIMARY KEY, + dinner_id TEXT NOT NULL REFERENCES dinners (id), + batch_id TEXT NOT NULL REFERENCES suggestion_batches (id), + name TEXT NOT NULL CHECK (name <> ''), + reason TEXT NOT NULL, + cited_regular_ids TEXT NOT NULL, -- JSON array of dinner ids + status TEXT NOT NULL CHECK (status IN ('pending', 'accepted', 'declined')), + decided_at TEXT, + created_at TEXT NOT NULL, + updated_at TEXT NOT NULL +); + +INSERT INTO suggestions ( + id, dinner_id, batch_id, name, reason, cited_regular_ids, status, decided_at, created_at, updated_at +) +SELECT + suggestions_copy.id, suggestions_copy.dinner_id, suggestions_copy.batch_id, dinners.name, + suggestions_copy.reason, suggestions_copy.cited_regular_ids, suggestions_copy.status, + suggestions_copy.decided_at, suggestions_copy.created_at, suggestions_copy.updated_at +FROM suggestions_copy +JOIN dinners ON dinners.id = suggestions_copy.dinner_id; + +DROP TABLE suggestions_copy; diff --git a/migrations/queries/household_context.sql b/migrations/queries/household_context.sql index 37ff2d7..fbea66d 100644 --- a/migrations/queries/household_context.sql +++ b/migrations/queries/household_context.sql @@ -1,12 +1,12 @@ -- Reads for the AI household context block (internal/ai/harness). -- name: ListRecentDeclinedNames :many --- The names of the most recently declined suggestions, newest first. -SELECT dinners.name +-- The names the most recently declined suggestions were suggested under, +-- newest first; renaming their dinners does not change them. +SELECT name FROM suggestions -JOIN dinners ON dinners.id = suggestions.dinner_id -WHERE suggestions.status = 'declined' -ORDER BY suggestions.decided_at DESC, suggestions.id DESC +WHERE status = 'declined' +ORDER BY decided_at DESC, id DESC LIMIT ?; -- name: ListPlannedDinners :many diff --git a/migrations/queries/suggestions.sql b/migrations/queries/suggestions.sql new file mode 100644 index 0000000..3a42470 --- /dev/null +++ b/migrations/queries/suggestions.sql @@ -0,0 +1,103 @@ +-- name: CreateSuggestionBatch :one +INSERT INTO suggestion_batches (id, trigger, status, created_at, updated_at) +VALUES (?, ?, 'pending', ?, ?) +RETURNING *; + +-- name: GetActiveSuggestionBatch :one +-- The batch still pending or running, oldest first. Returns sql.ErrNoRows +-- when there is none. +SELECT * FROM suggestion_batches +WHERE status IN ('pending', 'running') +ORDER BY id +LIMIT 1; + +-- name: GetSuggestionBatchJobID :one +-- The id of the job that runs batch batch_id. +SELECT id FROM jobs +WHERE kind = sqlc.arg(kind) + AND json_extract(payload, '$.batch_id') = sqlc.arg(batch_id) +ORDER BY id +LIMIT 1; + +-- name: StartSuggestionBatch :execrows +-- Marks a pending batch running; a batch in any other status is left as +-- it is (0 rows). +UPDATE suggestion_batches +SET status = 'running', updated_at = ? +WHERE id = ? AND status = 'pending'; + +-- name: FinishSuggestionBatch :execrows +-- Records a pending or running batch as done or failed; a batch already +-- finished is left as it is (0 rows). +UPDATE suggestion_batches +SET status = ?, updated_at = ? +WHERE id = ? AND status IN ('pending', 'running'); + +-- name: FailOrphanedSuggestionBatches :execrows +-- Fails every batch still pending or running whose job is no longer queued +-- or running: the process stopped, or recording the batch's end failed, +-- before its handler recorded a result. Run before reading the active batch. +UPDATE suggestion_batches +SET status = 'failed', updated_at = sqlc.arg(updated_at) +WHERE status IN ('pending', 'running') + AND NOT EXISTS ( + SELECT 1 FROM jobs + WHERE jobs.kind = sqlc.arg(kind) + AND jobs.status IN ('queued', 'running') + AND json_extract(jobs.payload, '$.batch_id') = suggestion_batches.id + ); + +-- name: CreateSuggestion :exec +-- name is the name the dish is suggested under, kept whatever its dinner is +-- later renamed. +INSERT INTO suggestions ( + id, dinner_id, batch_id, name, reason, cited_regular_ids, status, created_at, updated_at +) VALUES ( + ?, ?, ?, ?, ?, ?, 'pending', ?, ? +); + +-- name: GetSuggestion :one +-- A suggestion with its dinner. Returns sql.ErrNoRows for an unknown id. +SELECT sqlc.embed(suggestions), sqlc.embed(dinners) +FROM suggestions +JOIN dinners ON dinners.id = suggestions.dinner_id +WHERE suggestions.id = ?; + +-- name: ListDeck :many +-- The pending suggestions whose dinner is still suggested, oldest first, +-- with their dinners. +SELECT sqlc.embed(suggestions), sqlc.embed(dinners) +FROM suggestions +JOIN dinners ON dinners.id = suggestions.dinner_id +WHERE suggestions.status = 'pending' AND dinners.status = 'suggested' +ORDER BY suggestions.created_at, suggestions.id; + +-- name: CountDeck :one +-- How many suggestions ListDeck returns. +SELECT COUNT(*) +FROM suggestions +JOIN dinners ON dinners.id = suggestions.dinner_id +WHERE suggestions.status = 'pending' AND dinners.status = 'suggested'; + +-- name: ListDeclinedSuggestionNames :many +-- The name every declined suggestion was suggested under, oldest decision +-- first; renaming its dinner does not change it. +SELECT name +FROM suggestions +WHERE status = 'declined' +ORDER BY decided_at, id; + +-- name: DecideSuggestion :execrows +-- Records the household's decision on a pending suggestion; one already +-- decided is left as it is (0 rows). +UPDATE suggestions +SET status = sqlc.arg(status), decided_at = sqlc.arg(now), updated_at = sqlc.arg(now) +WHERE id = sqlc.arg(id) AND status = 'pending'; + +-- name: MoveSuggestedDinner :execrows +-- Moves a dinner still suggested to status (quest_log or archived), +-- recording updated_by; a dinner no longer suggested is left as it is +-- (0 rows). +UPDATE dinners +SET status = sqlc.arg(status), updated_by = sqlc.arg(updated_by), updated_at = sqlc.arg(updated_at) +WHERE id = sqlc.arg(id) AND status = 'suggested';