diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 0b266bb..ffee938 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -357,13 +357,19 @@ automatic** (D-021). left as it is and the job fails `confirm_required`. The job reads the plan's revision before draft_week runs; if the plan was edited (a swap, or another draft stored) by the time the draft is stored, the plan is - left as it is and the job fails `plan_changed`. A pool under seven + left as it is and the job fails `plan_changed`. The pool is read again + in the same transaction that stores the draft; if a dinner the draft + planned has left it meanwhile (archived, `tried`, or a Quest log dinner + stored on another current or later week's plan, as by a draft of + another week), nothing is stored and the job fails `pool_changed`, + which drafting again resolves. A pool under seven dinners fails the job `not_enough_dinners`, draft_week failing `ai_failed`. `PUT /api/v1/plans/{weekStart}/days/{date}` takes `{dinner_id}`, a Regular or Quest log dinner, swaps the day, recomputes `is_new`, moves a `synced` plan to `changed`, and answers 200 with the plan. A date outside the week is 400 `invalid_date`, an unknown dinner or - a week with no plan 404, and any other dinner 409 `dinner_not_plannable`. + a week with no plan 404, any other dinner 409 `dinner_not_plannable`, and + a dinner already on another day of the week 409 `dinner_already_planned`. - Rotation check (`plan.Service`, D-021). `MarkTried(date)`, which the `rotation_check` schedule calls for the household-local day just past, moves every `quest_log` dinner planned on that date, on a plan of any diff --git a/internal/plan/draft.go b/internal/plan/draft.go index f64a569..5ef6cbd 100644 --- a/internal/plan/draft.go +++ b/internal/plan/draft.go @@ -36,9 +36,9 @@ func (r draftResult) PublicJSON() ([]byte, error) { return json.Marshal(r) } // Regulars and the Quest log dinners no other current or later plan has, // given the household context and last week's dinners, and the draft is // stored as the week's plan if the plan is still as it was before -// draft_week ran. A pool under seven dinners fails with -// ReasonNotEnoughDinners and draft_week failing with ReasonAIFailed; no -// week is made up instead. +// draft_week ran and every dinner it planned is still in the pool. A pool +// under seven dinners fails with ReasonNotEnoughDinners and draft_week +// failing with ReasonAIFailed; no week is made up instead. func (s *Service) runDraft(ctx context.Context, p DraftJob, _ func(draftResult) error) (draftResult, error) { weekStart, err := ParseWeekStart(p.WeekStart) if err != nil { @@ -68,7 +68,7 @@ func (s *Service) runDraft(ctx context.Context, p DraftJob, _ func(draftResult) for _, w := range week.Warnings { s.logger.InfoContext(ctx, "week draft warning", "week_start", p.WeekStart, "warning", w) } - planID, err := s.storeDraft(ctx, weekStart, p.Confirm, base, week) + planID, err := s.storeDraft(ctx, weekStart, household.WeekStart(in.Context.Today), p.Confirm, base, week) if err != nil { return draftResult{}, err } @@ -142,11 +142,28 @@ func (s *Service) request(ctx context.Context, weekStart store.LocalDate) (skill // draft was asked for without confirm is left as it is, failing with // ReasonConfirmRequired, and so is a plan no longer at revision base (the // revision before draft_week ran, or noPlan), failing with -// ReasonPlanChanged. -func (s *Service) storeDraft(ctx context.Context, weekStart store.LocalDate, confirm bool, base int64, week skills.Week) (string, error) { +// ReasonPlanChanged. The pool is read again, as of thisWeek like the pool +// draft_week was given: a day whose dinner is no longer in it, or is in it +// as a Regular rather than the Quest log or the other way round, leaves +// everything as it is, failing with ReasonPoolChanged. +func (s *Service) storeDraft(ctx context.Context, weekStart, thisWeek store.LocalDate, confirm bool, base int64, week skills.Week) (string, error) { var planID string err := s.st.WithTx(ctx, func(tx *sql.Tx) error { q := db.New(tx) + rows, err := q.ListDraftPool(ctx, db.ListDraftPoolParams{FromWeekStart: thisWeek.String(), WeekStart: weekStart.String()}) + if err != nil { + return fmt.Errorf("plan: list draft pool: %w", err) + } + pool := make(map[string]string, len(rows)) + for _, d := range rows { + pool[d.ID] = d.Status + } + for _, d := range week.Days { + status, ok := pool[d.DinnerID] + if !ok || (status == dinners.StatusQuestLog) != d.IsNew { + return &jobs.Failure{Code: ReasonPoolChanged} + } + } now := store.Now().String() note := sql.NullString{String: week.Note, Valid: week.Note != ""} cur, err := q.GetWeekPlan(ctx, weekStart.String()) diff --git a/internal/plan/plan.go b/internal/plan/plan.go index ec9647f..15291d5 100644 --- a/internal/plan/plan.go +++ b/internal/plan/plan.go @@ -53,10 +53,15 @@ const ( // ReasonPlanChanged: the plan was edited while draft_week ran, so the // draft does not replace the edit. ReasonPlanChanged = "plan_changed" + // ReasonPoolChanged: a dinner the draft planned left the pool while + // draft_week ran (archived, tried, or its Quest log dinner planned on + // another week), so the draft is not stored; drafting again uses the + // pool as it is now. + ReasonPoolChanged = "pool_changed" ) // Reasons are every draft failure reason. -var Reasons = []string{ReasonNotEnoughDinners, ReasonAIFailed, ReasonConfirmRequired, ReasonPlanChanged} +var Reasons = []string{ReasonNotEnoughDinners, ReasonAIFailed, ReasonConfirmRequired, ReasonPlanChanged, ReasonPoolChanged} // ContextBudget is the character budget of the household context // draft_week is given (see harness.BuildContext). @@ -71,6 +76,10 @@ var ErrNotFound = errors.New("plan not found") // ErrDinnerNotFound is a swap to an unknown dinner. var ErrDinnerNotFound = errors.New("dinner not found") +// ErrDinnerAlreadyPlanned is a swap to a dinner already on another day of +// the week. +var ErrDinnerAlreadyPlanned = errors.New("the dinner is already on another day of this week") + // ErrConfirmRequired is a draft over a synced or changed plan asked for // without confirm. var ErrConfirmRequired = errors.New("the plan was pushed to Calendar; drafting over it needs confirm") @@ -337,9 +346,10 @@ func (s *Service) StartDraft(ctx context.Context, weekStart store.LocalDate, con // Swap plans dinnerID on date of the week starting weekStart, marking the // day new when the dinner is in the Quest log. Swapping a synced plan // makes it changed. The week must have a plan (else ErrNotFound), date -// must be one of its days (else *InvalidDayError), and the dinner must be -// a Regular or in the Quest log (else ErrDinnerNotFound or -// *DinnerNotPlannableError). It returns the plan as swapped. +// must be one of its days (else *InvalidDayError), the dinner must be a +// Regular or in the Quest log (else ErrDinnerNotFound or +// *DinnerNotPlannableError), and it must not be on another day of the +// week (else ErrDinnerAlreadyPlanned). It returns the plan as swapped. func (s *Service) Swap(ctx context.Context, weekStart store.LocalDate, date, dinnerID string) (Plan, error) { dates, err := household.WeekDays(weekStart) if err != nil { @@ -369,6 +379,13 @@ func (s *Service) Swap(ctx context.Context, weekStart store.LocalDate, date, din if !slices.Contains(plannable, d.Status) { return &DinnerNotPlannableError{Status: d.Status} } + days, err := q.ListPlanDays(ctx, cur.ID) + if err != nil { + return fmt.Errorf("plan: read days of %s: %w", cur.ID, err) + } + if slices.ContainsFunc(days, func(r db.ListPlanDaysRow) bool { return r.Date != day.String() && r.Dinner.ID == d.ID }) { + return ErrDinnerAlreadyPlanned + } now := store.Now().String() n, err := q.SwapPlanDay(ctx, db.SwapPlanDayParams{ DinnerID: sql.NullString{String: d.ID, Valid: true}, diff --git a/internal/plan/plan_test.go b/internal/plan/plan_test.go index a437bb6..8795f70 100644 --- a/internal/plan/plan_test.go +++ b/internal/plan/plan_test.go @@ -174,9 +174,15 @@ func (e *env) reply(note string, names ...string) { // weekReply is the draft_week reply planning the dinners named names on // weekDates, in order. func (e *env) weekReply(note string, names ...string) llm.GenerateResult { + return e.weekReplyOn(weekDates, note, names...) +} + +// weekReplyOn is the draft_week reply planning the dinners named names on +// dates, in order. +func (e *env) weekReplyOn(dates []string, note string, names ...string) llm.GenerateResult { var days []string for i, name := range names { - days = append(days, fmt.Sprintf(`{"date":%q,"dinner_id":%q}`, weekDates[i], e.ids[name])) + days = append(days, fmt.Sprintf(`{"date":%q,"dinner_id":%q}`, dates[i], e.ids[name])) } js := fmt.Sprintf(`{"days":[%s],"note":%q}`, strings.Join(days, ","), note) return llm.GenerateResult{Response: llm.Response{Text: js, JSON: json.RawMessage(js)}} @@ -398,15 +404,16 @@ func TestStartByAcrossDaylightSavingTimeEnd(t *testing.T) { t.Errorf("%s start_by = %s, %v; want %s", dates[tc.day], b, err, tc.want) } } - // Swapped to Beef Chili, Sunday starts at 13:30 EST. - if _, err := e.svc.Swap(t.Context(), dst, "2026-11-01", e.ids["Beef Chili"]); err != nil { + // Swapped to Short Ribs (240 minutes), Sunday starts at 13:30 EST. + ribs := e.dinner(t, "Short Ribs", "regular", "crockpot", 240) + if _, err := e.svc.Swap(t.Context(), dst, "2026-11-01", ribs); err != nil { t.Fatal(err) } if p, err = e.svc.Get(t.Context(), dst); err != nil { t.Fatal(err) } if b, err := json.Marshal(p.Days[6].StartBy); err != nil || string(b) != `"2026-11-01T13:30:00-05:00"` { - t.Errorf("Sunday Beef Chili start_by = %s, %v; want 13:30 EST", b, err) + t.Errorf("Sunday Short Ribs start_by = %s, %v; want 13:30 EST", b, err) } } @@ -466,6 +473,70 @@ func TestDraftPoolLeavesOutQuestLogPlannedOnAnotherWeek(t *testing.T) { } } +func TestDraftFailsWhenPoolDinnerLeavesWhileDrafting(t *testing.T) { + for _, status := range []string{"archived", "tried"} { + t.Run(status, func(t *testing.T) { + e := newEnv(t) + e.start(t) + // Pad Thai leaves the Quest log while draft_week plans it. + started, release := make(chan struct{}), make(chan struct{}) + r := e.weekReply("", fullWeek...) + r.Started, r.Release = started, release + e.model.QueueGenerate(r) + d, err := e.svc.StartDraft(t.Context(), week, false) + if err != nil { + t.Fatal(err) + } + <-started + if _, err := e.st.Writer.ExecContext(t.Context(), `UPDATE dinners SET status = ? WHERE id = ?`, status, e.ids["Pad Thai"]); err != nil { + t.Fatal(err) + } + close(release) + job := e.waitJob(t, d.JobID) + if job.Status != jobs.StatusFailed || *job.Error != plan.ReasonPoolChanged { + t.Fatalf("job %s %v, want failed %s", job.Status, job.Error, plan.ReasonPoolChanged) + } + if _, err := e.svc.Get(t.Context(), week); !errors.Is(err, plan.ErrNotFound) { + t.Errorf("Get = %v, want ErrNotFound: no partial plan", err) + } + }) + } +} + +func TestDraftsOfTwoWeeksDoNotShareAQuestLogDinner(t *testing.T) { + e := newEnv(t) + e.start(t) + // This week's draft plans Pad Thai; while draft_week runs, the other + // worker drafts a later week with Pad Thai too and stores it first. + started, release := make(chan struct{}), make(chan struct{}) + r := e.weekReply("", fullWeek...) + r.Started, r.Release = started, release + e.model.QueueGenerate(r) + d, err := e.svc.StartDraft(t.Context(), week, false) + if err != nil { + t.Fatal(err) + } + <-started + later := mustDate("2099-01-05") + laterDates := []string{"2099-01-05", "2099-01-06", "2099-01-07", "2099-01-08", "2099-01-09", "2099-01-10", "2099-01-11"} + e.model.QueueGenerate(e.weekReplyOn(laterDates, "", fullWeek...)) + other, err := e.svc.StartDraft(t.Context(), later, false) + if err != nil { + t.Fatal(err) + } + if job := e.waitJob(t, other.JobID); job.Status != jobs.StatusSucceeded { + t.Fatalf("later week's draft %s %v", job.Status, job.Error) + } + close(release) + job := e.waitJob(t, d.JobID) + if job.Status != jobs.StatusFailed || *job.Error != plan.ReasonPoolChanged { + t.Fatalf("job %s %v, want failed %s", job.Status, job.Error, plan.ReasonPoolChanged) + } + if _, err := e.svc.Get(t.Context(), week); !errors.Is(err, plan.ErrNotFound) { + t.Errorf("Get = %v, want ErrNotFound: no partial plan", err) + } +} + func TestDraftFailsWithNotEnoughDinners(t *testing.T) { e := newEnv(t) e.start(t) @@ -633,7 +704,8 @@ func TestDraftFailsWhenPlanSwappedWhileDrafting(t *testing.T) { t.Fatal(err) } <-started - if _, err := e.svc.Swap(t.Context(), week, "2026-10-14", e.ids["Sheet Pan Salmon"]); err != nil { + e.ids["Fish Tacos"] = e.dinner(t, "Fish Tacos", "regular", "stovetop", 25) + if _, err := e.svc.Swap(t.Context(), week, "2026-10-14", e.ids["Fish Tacos"]); err != nil { t.Fatal(err) } close(release) @@ -646,7 +718,7 @@ func TestDraftFailsWhenPlanSwappedWhileDrafting(t *testing.T) { t.Fatal(err) } want := slices.Clone(fullWeek) - want[2] = "Sheet Pan Salmon" + want[2] = "Fish Tacos" if *p.AINote != "first" || !slices.Equal(dayNames(p), want) { t.Errorf("plan = %q %v, want the first draft with the swap %v", *p.AINote, dayNames(p), want) } @@ -822,7 +894,8 @@ func TestSwapStatusTransitions(t *testing.T) { e.reply("", fullWeek...) e.mustDraft(t, false) e.setStatus(t, tc.from) - p, err := e.svc.Swap(t.Context(), week, "2026-10-12", e.ids["Pad Thai"]) + fishTacos := e.dinner(t, "Fish Tacos", "regular", "stovetop", 25) + p, err := e.svc.Swap(t.Context(), week, "2026-10-12", fishTacos) if err != nil { t.Fatal(err) } @@ -852,6 +925,7 @@ func TestSwapRefusals(t *testing.T) { }{ {"no plan", mustDate("2026-10-19"), "2026-10-19", e.ids["Pad Thai"], func(err error) bool { return errors.Is(err, plan.ErrNotFound) }}, {"unknown dinner", week, "2026-10-12", "no-such-dinner", func(err error) bool { return errors.Is(err, plan.ErrDinnerNotFound) }}, + {"dinner on another day", week, "2026-10-12", e.ids["Pot Roast"], func(err error) bool { return errors.Is(err, plan.ErrDinnerAlreadyPlanned) }}, {"suggested dinner", week, "2026-10-12", suggested, func(err error) bool { return errors.As(err, ¬Plannable) }}, {"archived dinner", week, "2026-10-12", archived, func(err error) bool { return errors.As(err, ¬Plannable) }}, {"day of another week", week, "2026-10-19", e.ids["Pad Thai"], func(err error) bool { return errors.As(err, &badDay) }}, diff --git a/internal/server/plans.go b/internal/server/plans.go index 949f1a5..b15d345 100644 --- a/internal/server/plans.go +++ b/internal/server/plans.go @@ -107,8 +107,8 @@ func weekStartOrFail(w http.ResponseWriter, r *http.Request, logger *slog.Logger // writePlanError maps a plan.Service error to its API error: a week with // no plan or an unknown dinner is 404, a date outside the week is 400, and // drafting over a synced plan without confirm, a confirmed draft while an -// unconfirmed one runs, or planning a dinner that is neither a Regular nor -// in the Quest log, is 409. +// unconfirmed one runs, planning a dinner that is neither a Regular nor in +// the Quest log, or one already on another day of the week, is 409. func writePlanError(w http.ResponseWriter, r *http.Request, logger *slog.Logger, action string, err error) { var day *plan.InvalidDayError var week *plan.InvalidWeekError @@ -120,6 +120,8 @@ func writePlanError(w http.ResponseWriter, r *http.Request, logger *slog.Logger, writeError(w, r, logger, http.StatusBadRequest, "invalid_week", week.Error()) case errors.As(err, ¬Plannable): writeError(w, r, logger, http.StatusConflict, "dinner_not_plannable", notPlannable.Error()) + case errors.Is(err, plan.ErrDinnerAlreadyPlanned): + writeError(w, r, logger, http.StatusConflict, "dinner_already_planned", err.Error()) case errors.Is(err, plan.ErrConfirmRequired): writeError(w, r, logger, http.StatusConflict, "confirm_required", err.Error()) case errors.Is(err, plan.ErrDraftRunning): diff --git a/internal/server/plans_test.go b/internal/server/plans_test.go index 00b2f2f..9f6142c 100644 --- a/internal/server/plans_test.go +++ b/internal/server/plans_test.go @@ -309,6 +309,7 @@ func TestPutPlanDay(t *testing.T) { {"2026-10-12/days/2026-10-32", `{"dinner_id":"` + e.questLog + `"}`, http.StatusBadRequest, "invalid_date"}, {"2026-10-12/days/2026-10-14", `{"dinner_id":"no-such-dinner"}`, http.StatusNotFound, "not_found"}, {"2026-10-19/days/2026-10-20", `{"dinner_id":"` + e.questLog + `"}`, http.StatusNotFound, "not_found"}, + {"2026-10-12/days/2026-10-15", `{"dinner_id":"` + e.questLog + `"}`, http.StatusConflict, "dinner_already_planned"}, } { rec, body := callDinners(t, e.h, http.MethodPut, plansPath+tc.target, tc.body) if rec.Code != tc.status || errorCode(body) != tc.code { diff --git a/web/src/plan/PlanPage.test.tsx b/web/src/plan/PlanPage.test.tsx index 329dcc2..9e48c68 100644 --- a/web/src/plan/PlanPage.test.tsx +++ b/web/src/plan/PlanPage.test.tsx @@ -972,6 +972,31 @@ describe.each([ expect(screen.queryByRole('dialog')).not.toBeInTheDocument() await waitFor(() => expect(posts(fetchMock)).toEqual([`${DRAFT} {}`])) }) + it('reads the Quest log again once a draft finishes, dropping the dinners it planned', async () => { + let plan = planBody('draft') + let job = draftJob('running') + let questLog = [dinner('q1', 'Lemon orzo', 'easy', 25)] + stubServer({ + ...lists, + [`GET ${THIS_WEEK}`]: () => jsonResponse(200, plan), + [`POST ${DRAFT}`]: () => jsonResponse(202, { jobId: 'j1' }), + [`GET ${JOB}`]: () => jsonResponse(200, job), + [`GET ${QUEST_LOG}`]: () => jsonResponse(200, { dinners: questLog }), + }) + await renderPage() + + const list = await screen.findByRole('list', { name: layout === 'desktop' ? 'Quest log' : 'Quest log dinners' }) + expect(await within(list).findByText('Lemon orzo')).toBeInTheDocument() + fireEvent.click(screen.getByRole('button', { name: 'Rebuild week' })) + expect(await screen.findByRole('region', { name: 'Drafting this week' })).toBeInTheDocument() + + plan = planBody('draft', ['Lemon orzo', ...NAMES.slice(1)]) + questLog = [] + job = draftJob('succeeded') + const week = await screen.findByRole('list', { name: 'Dinners this week' }, { timeout: 3000 }) + expect(await within(week).findByText('Lemon orzo', {}, { timeout: 3000 })).toBeInTheDocument() + await waitFor(() => expect(within(list).queryByText('Lemon orzo')).not.toBeInTheDocument()) + }) }) // Dragging a dinner from desktop's panel or tablet's strip onto a day swaps diff --git a/web/src/plan/PlanPage.tsx b/web/src/plan/PlanPage.tsx index 884ca54..3867949 100644 --- a/web/src/plan/PlanPage.tsx +++ b/web/src/plan/PlanPage.tsx @@ -2,6 +2,7 @@ import { useQueryClient } from '@tanstack/react-query' import { CalendarDays, Check, ChevronLeft, ChevronRight, Plus, RefreshCw, ShoppingCart } from 'lucide-react' import { useEffect, useId, useMemo, useState, type ReactNode } from 'react' import { useJob } from '../api/index.ts' +import { QUEST_LOG_KEY } from '../discover/api.ts' import { Confirm } from '../settings/LeaveGuard.tsx' import { methodLabels } from '../settings/RegularsSettingsPage.tsx' import { formatClockTime, useHouseholdSettings } from '../settings/api.ts' @@ -57,6 +58,7 @@ const draftFailures: Record = { ai_failed: "The AI assistant couldn't draft it.", confirm_required: 'The plan was pushed to Calendar while it was drafting.', plan_changed: 'The plan was edited while it was drafting.', + pool_changed: 'A dinner it picked was archived, tried or planned for another week while it was drafting.', internal_error: 'Something went wrong on the server.', } @@ -239,12 +241,14 @@ function WeekBoard({ // Once the job ends the plan is read again: a succeeded draft is followed // until that read lands; a failed one stays shown, and may have met a - // plan changed meanwhile. + // plan changed meanwhile. The Quest log is read again too, as after a + // swap: the draft may have planned some of it. const [readJobId, setReadJobId] = useState(null) const ended = job.status === 'succeeded' || job.status === 'failed' useEffect(() => { if (!ended) return let active = true + void queryClient.invalidateQueries({ queryKey: QUEST_LOG_KEY }) void queryClient.invalidateQueries({ queryKey: planKey(weekStart) }).then(() => { if (active) setReadJobId(jobId) })