Report the latest failed suggestion batch on the deck
The deck reported only a pending or running batch, so a Discover page opened after a batch failed (or was interrupted by a restart) had no job to follow and showed "All caught up" or the remaining cards with no error. GET /api/v1/suggestions/deck now also returns failed_batch: the newest batch when it failed, read in the same snapshot after orphaned batches are failed, so at most one of active_batch and failed_batch is set and a later batch supersedes the failure. The page follows the failed batch's job like the active one, so its failure shows with the existing Retry, which asks for a new batch. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -78,6 +78,9 @@ type Querier interface {
|
||||
// Returns sql.ErrNoRows until the row exists.
|
||||
GetIntegrationsGoogle(ctx context.Context) (IntegrationsGoogle, error)
|
||||
GetJob(ctx context.Context, id string) (Job, error)
|
||||
// The newest batch, whatever its status. Returns sql.ErrNoRows when there
|
||||
// is none.
|
||||
GetLatestSuggestionBatch(ctx context.Context) (SuggestionBatch, error)
|
||||
GetPhoto(ctx context.Context, id string) (Photo, error)
|
||||
// Finds stored image bytes by content hash. The schema does not make sha256
|
||||
// unique, so the earliest-created (lowest id) match is returned.
|
||||
|
||||
@@ -187,6 +187,28 @@ func (q *Queries) GetActiveSuggestionBatch(ctx context.Context) (SuggestionBatch
|
||||
return i, err
|
||||
}
|
||||
|
||||
const getLatestSuggestionBatch = `-- name: GetLatestSuggestionBatch :one
|
||||
SELECT id, "trigger", status, ai_run_id, created_at, updated_at FROM suggestion_batches
|
||||
ORDER BY id DESC
|
||||
LIMIT 1
|
||||
`
|
||||
|
||||
// The newest batch, whatever its status. Returns sql.ErrNoRows when there
|
||||
// is none.
|
||||
func (q *Queries) GetLatestSuggestionBatch(ctx context.Context) (SuggestionBatch, error) {
|
||||
row := q.db.QueryRowContext(ctx, getLatestSuggestionBatch)
|
||||
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
|
||||
|
||||
@@ -174,6 +174,9 @@ func TestSuggestionDeckActiveBatch(t *testing.T) {
|
||||
if active, ok := body["active_batch"]; !ok || active != nil {
|
||||
t.Fatalf("deck = %v, want active_batch null", body)
|
||||
}
|
||||
if failed, ok := body["failed_batch"]; !ok || failed != nil {
|
||||
t.Fatalf("deck = %v, want failed_batch null", body)
|
||||
}
|
||||
|
||||
rec, started := callDinners(t, h, http.MethodPost, suggestionsPath+"/batches", "")
|
||||
if rec.Code != http.StatusAccepted {
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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))
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user