From 7bbd1e2b0f1e9f53af0d2b22c6cf63f6e4b59ac3 Mon Sep 17 00:00:00 2001 From: dev_agent Date: Fri, 9 Oct 2026 20:23:46 -0400 Subject: [PATCH] Announce a batch's Side quests once, when it completes A scheduled or manual batch now holds every grounding round's Side quests and stores them all, with one new_suggestions, in a single transaction once its last round is done. The notification counts and names every Side quest the batch stored, instead of only the first round's (MQ-172-1), and a batch that fails at any round, including a later round's normalize_recipe, stores nothing and sends nothing, so no Side quest is left in the deck unannounced (MQ-172-2). suggest.store stays the only new_suggestions emitter; a notifying batch now calls it once. A replenish batch, which never notifies, still stores round by round so the deck fills as the household swipes. Maestro task 459aab71-5c3d-4853-9855-236a3602b1e0 Co-Authored-By: Claude Opus 5.5 --- docs/ARCHITECTURE.md | 41 +++++----- internal/suggest/batch.go | 67 ++++++++++------ internal/suggest/suggest_test.go | 127 +++++++++++++++++++++---------- 3 files changed, 153 insertions(+), 82 deletions(-) diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 8b757e0..c4db4a0 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -309,20 +309,24 @@ automatic** (D-021). `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`, + left. A Side quest no recipe grounds is dropped. A `replenish` batch + stores each round's grounded ones before the next; a `scheduled` or + `manual` batch, which notifies, holds them and stores them all in one + transaction once its last round is done. Each is stored as 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 + `failed`. A failed `scheduled` or `manual` batch has stored nothing; a + failed `replenish` batch's 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 @@ -833,13 +837,14 @@ Fake-clock tests of a schedule live in `internal/scheduler`, where - `new_suggestions:{YYYY-Www}` for a `scheduled` batch, the ISO week it was scheduled for (carried in its job, so a batch caught up late is still keyed by its own week), or `new_suggestions:{batchId}` for a - `manual` one (URL `/discover`), when the batch first adds Side quests - to the deck, in the transaction storing them, so they are announced - even when a later round or recording the batch's end fails. Its title - counts that round's Side quests ("3 new Side quests are in") and its - body names them; a later round's emit stores nothing. A batch that - fails before storing any, as when `suggest_side_quests` fails (the - failure is in `ai_runs`), sends nothing. A `replenish` batch never + `manual` one (URL `/discover`), once, when the batch's last round is + done, in the one transaction that stores all its Side quests, so they + and their notification commit together or not at all. Its title + counts every Side quest the batch stored ("5 new Side quests are in") + and its body names them. A batch that fails at any round, as when + `suggest_side_quests` or a later round's `normalize_recipe` fails (the + failure is in `ai_runs`), stores no Side quest and sends nothing, so + no stored Side quest goes unannounced. A `replenish` batch never notifies. This is the only place `new_suggestions` is emitted; the `suggestions_weekly` schedule does not emit it itself. - `day_of_reminder:{date}` (URL `/dinner/{id}`) through diff --git a/internal/suggest/batch.go b/internal/suggest/batch.go index 62b2fad..a6e130c 100644 --- a/internal/suggest/batch.go +++ b/internal/suggest/batch.go @@ -53,19 +53,22 @@ func (r batchResult) PublicJSON() ([]byte, error) { return json.Marshal(r) } // 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. +// instead. The batch ends done or failed. 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. // -// A scheduled or manual batch emits new_suggestions (see noticeOf) in the -// transaction that first adds Side quests to the deck, so the -// household is told of every Side quest stored, whether or not the batch -// then ends done: a later round, or recording the batch's end, may fail. -// A replenish batch, which tops up the deck as the household swipes, never -// notifies. See store. +// When a batch stores its Side quests depends on whether it notifies (see +// noticeOf). A scheduled or manual batch notifies: it holds every round's +// grounded Side quests and, once the last round is done, stores them all +// in one transaction with one new_suggestions counting and naming every +// Side quest stored. So the household is told of exactly what the batch +// added, once; and a batch that fails at any round, including a later +// round's normalize_recipe, stores nothing and sends nothing, leaving no +// Side quest in the deck that was never announced. A replenish batch, +// which tops up the deck as the household swipes and never notifies, +// stores each round's Side quests before the next round starts, so the +// deck fills as it goes; those stored before a failure stay in the deck. +// See store. func (s *Service) runBatch(ctx context.Context, p BatchJob, publish func(batchResult) error) (batchResult, error) { batch, err := db.New(s.st.Reader).GetSuggestionBatch(ctx, p.BatchID) if err != nil { @@ -102,8 +105,9 @@ func (s *Service) runBatch(ctx context.Context, p BatchJob, publish func(batchRe } // fill does runBatch's work for job of a batch with trigger, returning a -// *jobs.Failure for a failure the household may see. Unless the batch is a -// replenish one, store emits new_suggestions with the Side quests it adds. +// *jobs.Failure for a failure the household may see. A batch with a notice +// is stored once, when its last round is done; any other batch round by +// round. func (s *Service) fill(ctx context.Context, trigger string, job BatchJob, publish func(batchResult) error) (batchResult, error) { batchID := job.BatchID res := batchResult{BatchID: batchID, Stage: StageSuggesting, SuggestionIDs: []string{}} @@ -131,28 +135,46 @@ func (s *Service) fill(ctx context.Context, trigger string, job BatchJob, publis return batchResult{}, err } left := out.Suggestions - for len(res.SuggestionIDs) < BatchSize && len(left) > 0 { - round := left[:min(BatchSize-len(res.SuggestionIDs), len(left))] + // held are a notifying batch's grounded Side quests, not yet stored. + var held []groundedQuest + for len(res.SuggestionIDs)+len(held) < BatchSize && len(left) > 0 { + round := left[:min(BatchSize-len(res.SuggestionIDs)-len(held), 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, note, grounded) - if err != nil { + if note != nil { + held = append(held, grounded...) + } else if err := s.storeInto(ctx, &res, nil, grounded); 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 } } + if note != nil { + if err := s.storeInto(ctx, &res, note, held); err != nil { + return batchResult{}, err + } + } res.Stage = StageDone return res, nil } +// storeInto stores quests for res's batch with note (see store) and adds +// the suggestions stored, and the Side quests left out, to res. +func (s *Service) storeInto(ctx context.Context, res *batchResult, note *notice, quests []groundedQuest) error { + ids, err := s.store(ctx, res.BatchID, note, quests) + if err != nil { + return err + } + res.Dropped += len(quests) - len(ids) + res.SuggestionIDs = append(res.SuggestionIDs, ids...) + return nil +} + // notice is how a batch's new_suggestions is keyed: by id, as // new_suggestions:{id}, for the week starting on week. type notice struct { @@ -265,8 +287,9 @@ func (s *Service) ground(ctx context.Context, quests []skills.SideQuest) ([]grou // note, counting and naming those stored, is emitted in the same // transaction, so the Side quests and their notification commit together // or not at all: a notification that cannot be stored fails the store, and -// the batch with it. The event's key makes the first round to store Side -// quests the one that notifies; a later round's emit stores nothing. +// the batch with it. A notifying batch calls store once, with all its +// Side quests, so the count is the batch's (see runBatch). This is the +// only place new_suggestions is emitted. func (s *Service) store(ctx context.Context, batchID string, note *notice, quests []groundedQuest) ([]string, error) { ids := []string{} var stored []string diff --git a/internal/suggest/suggest_test.go b/internal/suggest/suggest_test.go index 2c932c3..2caa7cf 100644 --- a/internal/suggest/suggest_test.go +++ b/internal/suggest/suggest_test.go @@ -495,53 +495,58 @@ func TestBatchEmitsNewSuggestions(t *testing.T) { } } -// TestBatchNotifiesAsSideQuestsAreStored: new_suggestions is stored with -// the first round to add Side quests, so a batch whose later round fails -// still tells the household of those in the deck, once; a later round that -// succeeds adds no second notification; a replenish batch notifies nothing. -func TestBatchNotifiesAsSideQuestsAreStored(t *testing.T) { +// twoRoundBatch scripts a suggest_side_quests reply of BatchSize+1 Side +// quests, Fajita Dish A to F, whose first round grounds all but B, so a +// second round grounds F. Unless lastGrounds, normalize_recipe fails for +// F. +func (e *env) twoRoundBatch(t *testing.T, lastGrounds bool) { + t.Helper() + var quests []string + for i := range suggest.BatchSize + 1 { + name := fmt.Sprintf("Fajita Dish %c", 'A'+i) + e.search.Results[name] = nil + switch { + case i == suggest.BatchSize && !lastGrounds: + // A page is found, but no normalize_recipe reply is scripted + // for it, so the AI call grounding it fails. + 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)) + case i != 1: + e.recipe(t, name) + } + quests = append(quests, sideQuest(name, "Like your Chicken Fajitas.", "Chicken Fajitas")) + } + e.reply(quests...) +} + +// TestBatchNotifiesOnceWithEverySideQuestStored: a batch that fills across +// two grounding rounds stores new_suggestions once, when it completes, +// counting and naming every Side quest it stored, not just the first +// round's; a replenish batch notifies nothing. +func TestBatchNotifiesOnceWithEverySideQuestStored(t *testing.T) { for _, tc := range []struct { - trigger string - lastFails bool - notify bool + trigger string + notify bool }{ - {suggest.TriggerScheduled, true, true}, - {suggest.TriggerManual, true, true}, - {suggest.TriggerManual, false, true}, - {suggest.TriggerReplenish, true, false}, + {suggest.TriggerScheduled, true}, + {suggest.TriggerManual, true}, + {suggest.TriggerReplenish, false}, } { - t.Run(fmt.Sprintf("%s last round fails %v", tc.trigger, tc.lastFails), func(t *testing.T) { + t.Run(tc.trigger, func(t *testing.T) { e := newEnv(t) e.start(t) - // The first round grounds BatchSize Side quests but B; the - // second round grounds F. - var quests []string - for i := range suggest.BatchSize + 1 { - name := fmt.Sprintf("Fajita Dish %c", 'A'+i) - e.search.Results[name] = nil - if i != 1 { - e.recipe(t, name) - } - quests = append(quests, sideQuest(name, "Like your Chicken Fajitas.", "Chicken Fajitas")) - } - e.reply(quests...) - if tc.lastFails { - if _, err := e.st.Writer.ExecContext(t.Context(), `CREATE TRIGGER refuse_f BEFORE INSERT ON dinners WHEN NEW.name = 'Fajita Dish F' BEGIN SELECT RAISE(ABORT, 'F refused'); END`); err != nil { - t.Fatal(err) - } - } + e.twoRoundBatch(t, true) b, key := e.startBatch(t, tc.trigger) - job := e.waitJob(t, b.JobID) - want, deck := jobs.StatusSucceeded, suggest.BatchSize - if tc.lastFails { - want, deck = jobs.StatusFailed, suggest.BatchSize-1 + if job := e.waitJob(t, b.JobID); job.Status != jobs.StatusSucceeded { + t.Fatalf("batch job %s, want succeeded", job.Status) } - if job.Status != want { - t.Fatalf("batch job %s, want %s", job.Status, want) - } - if got := e.deckNames(t); len(got) != deck { - t.Fatalf("deck = %v, want %d Side quests", got, deck) + deck := e.deckNames(t) + if want := []string{"Fajita Dish A", "Fajita Dish C", "Fajita Dish D", "Fajita Dish E", "Fajita Dish F"}; !slices.Equal(deck, want) { + t.Fatalf("deck = %v, want %v", deck, want) } rows := notifytest.Rows(t, e.st, notify.NewSuggestions) if !tc.notify { @@ -550,9 +555,47 @@ func TestBatchNotifiesAsSideQuestsAreStored(t *testing.T) { } return } - if len(rows) != 1 || rows[0].Key != key || rows[0].Payload.Title != "4 new Side quests are in" || - rows[0].Payload.Body != "Fajita Dish A, Fajita Dish C, Fajita Dish D + 1 more" { - t.Errorf("new_suggestions rows = %+v, want one keyed %s counting and naming the first round", rows, key) + want := fmt.Sprintf("%d new Side quests are in", len(deck)) + if len(rows) != 1 || rows[0].Key != key || rows[0].Payload.Title != want || + rows[0].Payload.Body != "Fajita Dish A, Fajita Dish C, Fajita Dish D + 2 more" { + t.Errorf("new_suggestions rows = %+v, want one keyed %s titled %q", rows, key, want) + } + }) + } +} + +// TestBatchFailingInALaterRoundStoresNothing: when normalize_recipe fails +// in a batch's second grounding round, a scheduled or manual batch stores +// none of the first round's Side quests and sends no new_suggestions, so +// no Side quest is left in the deck unannounced; a replenish batch, which +// never notifies, keeps the first round's in the deck. +func TestBatchFailingInALaterRoundStoresNothing(t *testing.T) { + for _, tc := range []struct { + trigger string + deck int + }{ + {suggest.TriggerScheduled, 0}, + {suggest.TriggerManual, 0}, + {suggest.TriggerReplenish, suggest.BatchSize - 1}, + } { + t.Run(tc.trigger, func(t *testing.T) { + e := newEnv(t) + e.start(t) + e.twoRoundBatch(t, false) + + b, _ := e.startBatch(t, tc.trigger) + job := e.waitJob(t, b.JobID) + if job.Status != jobs.StatusFailed || job.Error == nil || *job.Error != suggest.ReasonAIFailed { + t.Fatalf("batch job %s, error %+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) != tc.deck { + t.Errorf("deck = %v, want %d Side quests", got, tc.deck) + } + if rows := notifytest.Rows(t, e.st, notify.NewSuggestions); len(rows) != 0 { + t.Errorf("new_suggestions rows = %+v, want none", rows) } }) }