Feature 14 integration fix: revalidate the draft pool, refuse duplicate week dinners, refresh the Quest log (#122)
Co-authored-by: orchestrator-agent <20+orchestrator-agent@code.cubecraftcreations.com>
This commit was merged in pull request #122.
This commit is contained in:
+23
-6
@@ -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())
|
||||
|
||||
+21
-4
@@ -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},
|
||||
|
||||
@@ -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) }},
|
||||
|
||||
@@ -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):
|
||||
|
||||
@@ -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 {
|
||||
|
||||
Reference in New Issue
Block a user