Feature 13 integration fix 2: report a failed batch on deck load (#112)

Co-authored-by: orchestrator-agent <20+orchestrator-agent@code.cubecraftcreations.com>

internal/suggest/suggest.go

@@ -201,6 +201,28 @@ func (s *Service) activeBatch(ctx context.Context, q *db.Queries) (*Batch, error
 	return &Batch{ID: cur.ID, JobID: jobID, Trigger: cur.Trigger, Status: cur.Status}, nil
 }
 
+// failedBatch returns the newest batch in q's transaction when it failed,
+// or nil when it did not or there is none: a later batch, done or under
+// way, supersedes an earlier failure. Run after activeBatch, so a batch
+// whose job ended without recording its end reads as failed.
+func failedBatch(ctx context.Context, q *db.Queries) (*Batch, error) {
+	cur, err := q.GetLatestSuggestionBatch(ctx)
+	if errors.Is(err, sql.ErrNoRows) {
+		return nil, nil
+	}
+	if err != nil {
+		return nil, fmt.Errorf("suggest: read latest batch: %w", err)
+	}
+	if cur.Status != BatchFailed {
+		return nil, nil
+	}
+	jobID, err := q.GetSuggestionBatchJobID(ctx, db.GetSuggestionBatchJobIDParams{Kind: KindSuggestionBatch, BatchID: cur.ID})
+	if err != nil {
+		return nil, 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
+}
+
 // 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. The
 // caller wakes the job runner once tx has committed and Created is true.
@@ -275,16 +297,22 @@ type Suggestion struct {
 }
 
 // Deck is the household's deck: the pending suggestions whose dinner is
-// still suggested, oldest first, and the batch pending or running, if any,
-// so a client can follow the job producing more.
+// still suggested, oldest first; the batch pending or running, if any, so
+// a client can follow the job producing more; and the newest batch when it
+// failed, so a client that was not following its job still learns of the
+// failure. At most one of ActiveBatch and FailedBatch is set: a batch is
+// only started when none is pending or running, so the active batch is
+// always the newest.
 type Deck struct {
 	Suggestions []Suggestion `json:"suggestions"`
 	ActiveBatch *Batch       `json:"active_batch"`
+	FailedBatch *Batch       `json:"failed_batch"`
 }
 
-// Deck reads the deck and its active batch in one transaction, so both come
-// from the same snapshot. It is a write transaction because reading the
-// active batch first fails any batch whose job has ended (see activeBatch).
+// Deck reads the deck and its active and failed batches in one
+// transaction, so all come from the same snapshot. It is a write
+// transaction because reading the active batch first fails any batch whose
+// job has ended (see activeBatch), which is then the failed batch.
 func (s *Service) Deck(ctx context.Context) (Deck, error) {
 	var deck Deck
 	err := s.st.WithTx(ctx, func(tx *sql.Tx) error {
@@ -301,6 +329,10 @@ func (s *Service) Deck(ctx context.Context) (Deck, error) {
 			}
 		}
 		deck.ActiveBatch, err = s.activeBatch(ctx, q)
+		if err != nil || deck.ActiveBatch != nil {
+			return err
+		}
+		deck.FailedBatch, err = failedBatch(ctx, q)
 		return err
 	})
 	if err != nil {

internal/suggest/suggest_test.go

@@ -463,6 +463,9 @@ func TestDeckReportsTheActiveBatch(t *testing.T) {
 	if deck.ActiveBatch != nil {
 		t.Fatalf("active batch = %+v with none pending or running, want nil", deck.ActiveBatch)
 	}
+	if deck.FailedBatch != nil {
+		t.Fatalf("failed batch = %+v with none failed, want nil", deck.FailedBatch)
+	}
 	b, err := e.svc.StartBatch(t.Context(), suggest.TriggerManual)
 	if err != nil {
 		t.Fatal(err)
@@ -478,6 +481,42 @@ func TestDeckReportsTheActiveBatch(t *testing.T) {
 	if deck.ActiveBatch == nil || *deck.ActiveBatch != want {
 		t.Errorf("active batch = %+v, want %+v", deck.ActiveBatch, want)
 	}
+	if deck.FailedBatch != nil {
+		t.Errorf("failed batch = %+v with a batch pending, want nil", deck.FailedBatch)
+	}
+}
+
+func TestDeckReportsTheLatestFailedBatch(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)
+	}
+	if job := e.waitJob(t, b.JobID); job.Status != jobs.StatusFailed {
+		t.Fatalf("job status = %s, want failed", job.Status)
+	}
+	deck, err := e.svc.Deck(t.Context())
+	if err != nil {
+		t.Fatal(err)
+	}
+	want := suggest.Batch{ID: b.ID, JobID: b.JobID, Trigger: suggest.TriggerManual, Status: suggest.BatchFailed}
+	if deck.ActiveBatch != nil || deck.FailedBatch == nil || *deck.FailedBatch != want {
+		t.Fatalf("active batch = %+v, failed batch = %+v; want no active batch and failed %+v", deck.ActiveBatch, deck.FailedBatch, want)
+	}
+
+	// A later batch that succeeds supersedes the failure.
+	e.recipe(t, "Chili Verde")
+	e.reply(sideQuest("Chili Verde", "Your Beef Chili gone green.", "Beef Chili"))
+	e.runBatch(t)
+	deck, err = e.svc.Deck(t.Context())
+	if err != nil {
+		t.Fatal(err)
+	}
+	if deck.ActiveBatch != nil || deck.FailedBatch != nil {
+		t.Errorf("active batch = %+v, failed batch = %+v after a batch succeeded, want neither", deck.ActiveBatch, deck.FailedBatch)
+	}
 }
 
 func TestDeckFailsBatchInterruptedByRestart(t *testing.T) {
@@ -508,6 +547,10 @@ func TestDeckFailsBatchInterruptedByRestart(t *testing.T) {
 	if deck.ActiveBatch != nil {
 		t.Errorf("active batch = %+v after its job failed, want nil", deck.ActiveBatch)
 	}
+	want := suggest.Batch{ID: b.ID, JobID: b.JobID, Trigger: suggest.TriggerManual, Status: suggest.BatchFailed}
+	if deck.FailedBatch == nil || *deck.FailedBatch != want {
+		t.Errorf("failed batch = %+v after its job was interrupted, want %+v", deck.FailedBatch, want)
+	}
 	if len(deck.Suggestions) != 1 {
 		t.Errorf("deck has %d suggestions, want 1", len(deck.Suggestions))
 	}