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

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

docs/ARCHITECTURE.md

@@ -311,10 +311,13 @@ automatic** (D-021).
   `GET /api/v1/suggestions/deck` returns `{suggestions: [{id, dinner_id,
   name, photo_url, difficulty, method, minutes_total, cuisine, reason,
   cited_regular_ids, status, dinner_status, decided_at, created_at}],
-  active_batch}`: the `pending` suggestions whose dinner is still
+  active_batch, failed_batch}`: the `pending` suggestions whose dinner is
-  `suggested`, oldest first, and the batch `pending` or `running`
+  still `suggested`, oldest first; the batch `pending` or `running`
-  (`{batch_id, job_id, trigger, status}`, or null), read in one snapshot,
+  (`{batch_id, job_id, trigger, status}`, or null); and the newest batch
-  so a client that did not start the batch can follow its job. `POST /api/v1/suggestions/{id}/accept` (Try it) sets the
+  when it `failed` (same shape, or null; a later batch, done or under
+  way, supersedes the failure), read in one snapshot, so a client that
+  did not start the batch can follow its job and learns of a failure it
+  was not following. At most one of the two is set. `POST /api/v1/suggestions/{id}/accept` (Try it) sets the
   suggestion `accepted` and moves its dinner to `quest_log`; `POST
   /api/v1/suggestions/{id}/decline` (Pass) sets it `declined` and archives
   the dinner; every later batch, and the household context's passed list,

internal/db/querier.go

@@ -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.

internal/db/suggestions.sql.go

@@ -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

internal/server/suggestions_test.go

@@ -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 {

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,
+// still suggested, oldest first; the batch pending or running, if any, so
-// so a client can follow the job producing more.
+// 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
+// Deck reads the deck and its active and failed batches in one
-// from the same snapshot. It is a write transaction because reading the
+// transaction, so all come from the same snapshot. It is a write
-// active batch first fails any batch whose job has ended (see activeBatch).
+// 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))
 	}

migrations/queries/suggestions.sql

@@ -11,6 +11,13 @@ WHERE status IN ('pending', 'running')
 ORDER BY id
 LIMIT 1;
 
+-- name: GetLatestSuggestionBatch :one
+-- The newest batch, whatever its status. Returns sql.ErrNoRows when there
+-- is none.
+SELECT * FROM suggestion_batches
+ORDER BY id DESC
+LIMIT 1;
+
 -- name: GetSuggestionBatchJobID :one
 -- The id of the job that runs batch batch_id.
 SELECT id FROM jobs

web/src/App.test.tsx

@@ -107,7 +107,7 @@ describe('App routes', () => {
     mockViewport(390)
     stubFetchByPath({
       ...SIGNED_IN,
-      '/api/v1/suggestions/deck': () => jsonResponse(200, { suggestions: [], active_batch: null }),
+      '/api/v1/suggestions/deck': () => jsonResponse(200, { suggestions: [], active_batch: null, failed_batch: null }),
       '/api/v1/dinners?status=quest_log': () => jsonResponse(200, { dinners: [] }),
     })
     renderAt('/discover')

web/src/discover/DiscoverPage.test.tsx

@@ -50,15 +50,21 @@ const ORZO = suggestionBody('s3', 'Lemon garlic shrimp orzo', { difficulty: 'har
 type Responder = () => Response | Promise<Response>
 
 // stubServer stands in for the API: GET of the deck answers the current
-// server.deck and server.activeBatch, and the Quest log the current
+// server.deck, server.activeBatch and server.failedBatch, and the Quest log the current
 // server.questLog dinners; every
 // other request is answered by responders, keyed "METHOD path". Any other
 // request fails the test.
 function stubServer(deck: unknown[], responders: Record<string, Responder> = {}, questLog = 6) {
-  const server = { deck, questLog, activeBatch: null as { batch_id: string; job_id: string } | null, fetchMock: undefined as unknown as Mock<typeof fetch> }
+  const server = {
+    deck,
+    questLog,
+    activeBatch: null as { batch_id: string; job_id: string } | null,
+    failedBatch: null as { batch_id: string; job_id: string } | null,
+    fetchMock: undefined as unknown as Mock<typeof fetch>,
+  }
   const all: Record<string, Responder> = {
     'GET /api/v1/me': () => jsonResponse(200, ME),
-    [`GET ${DECK}`]: () => jsonResponse(200, { suggestions: server.deck, active_batch: server.activeBatch }),
+    [`GET ${DECK}`]: () => jsonResponse(200, { suggestions: server.deck, active_batch: server.activeBatch, failed_batch: server.failedBatch }),
     [`GET ${QUEST_LOG}`]: () =>
       jsonResponse(200, { dinners: Array.from({ length: server.questLog }, (_, i) => questLogBody(`q${i}`, `Quest ${i}`)) }),
     ...responders,
@@ -378,7 +384,7 @@ describe('DiscoverPage', () => {
 
   it('shows a deck that fails to load with a retry', async () => {
     let fail = true
-    stubServer([BOWLS], { [`GET ${DECK}`]: () => (fail ? errorResponse(500, 'internal') : jsonResponse(200, { suggestions: [BOWLS], active_batch: null })) })
+    stubServer([BOWLS], { [`GET ${DECK}`]: () => (fail ? errorResponse(500, 'internal') : jsonResponse(200, { suggestions: [BOWLS], active_batch: null, failed_batch: null })) })
     await renderPage()
     const alert = await screen.findByRole('alert', {}, { timeout: 5000 })
     expect(alert).toHaveTextContent("Couldn't load your side quests.")
@@ -525,11 +531,21 @@ describe('DiscoverPage', () => {
       const server = stubServer([BOWLS, TIKKA], {
         'POST /api/v1/suggestions/s1/decline': () => {
           server.deck = [TIKKA]
+          server.activeBatch = { batch_id: 'b-j1', job_id: 'j1' }
           return declined(BOWLS, 'j1')
         },
-        'GET /api/v1/jobs/j1': () => jsonResponse(200, batchJob('j1', 'failed', [], 'model unavailable')),
+        'GET /api/v1/jobs/j1': () => {
-        'POST /api/v1/suggestions/batches': () => jsonResponse(202, { batch_id: 'b-j2', job_id: 'j2', trigger: 'manual', status: 'pending' }),
+          server.activeBatch = null
+          server.failedBatch = { batch_id: 'b-j1', job_id: 'j1' }
+          return jsonResponse(200, batchJob('j1', 'failed', [], 'model unavailable'))
+        },
+        'POST /api/v1/suggestions/batches': () => {
+          server.failedBatch = null
+          server.activeBatch = { batch_id: 'b-j2', job_id: 'j2' }
+          return jsonResponse(202, { batch_id: 'b-j2', job_id: 'j2', trigger: 'manual', status: 'pending' })
+        },
         'GET /api/v1/jobs/j2': () => {
+          server.activeBatch = null
           server.deck = [TIKKA, ORZO]
           return jsonResponse(200, batchJob('j2', 'succeeded', ['s3']))
         },
@@ -580,6 +596,78 @@ describe('DiscoverPage', () => {
       expect(sent(server.fetchMock)).toEqual(['POST /api/v1/suggestions/s1/decline'])
     })
 
+    it.each([
+      ['the AI failed', 'an empty deck', 'ai_failed', []],
+      ['the AI failed', 'cards left', 'ai_failed', [BOWLS, TIKKA]],
+      ['a restart interrupted it', 'an empty deck', 'interrupted by restart', []],
+      ['a restart interrupted it', 'cards left', 'interrupted by restart', [BOWLS, TIKKA]],
+    ])('shows the failed batch the deck reports when the page opens after %s, with %s, and a retry that asks again', async (_, __, reason, deck) => {
+      const server = stubServer(deck, {
+        'GET /api/v1/jobs/j1': () => jsonResponse(200, batchJob('j1', 'failed', [], reason)),
+        'POST /api/v1/suggestions/batches': () => {
+          server.failedBatch = null
+          server.activeBatch = { batch_id: 'b-j2', job_id: 'j2' }
+          return jsonResponse(202, { batch_id: 'b-j2', job_id: 'j2', trigger: 'manual', status: 'pending' })
+        },
+        'GET /api/v1/jobs/j2': () => {
+          server.activeBatch = null
+          server.deck = [...deck, ORZO]
+          return jsonResponse(200, batchJob('j2', 'succeeded', ['s3']))
+        },
+      })
+      // The batch failed while the page was closed.
+      server.failedBatch = { batch_id: 'b-j1', job_id: 'j1' }
+      await renderPage()
+
+      const alert = await screen.findByRole('alert')
+      expect(alert).toHaveTextContent(`Couldn't find new side quests. ${reason}`)
+      if (deck.length === 0) {
+        expect(screen.getByRole('heading', { name: 'All caught up' })).toBeInTheDocument()
+      } else {
+        expect(screen.getByRole('article', { name: 'Korean beef bowls' })).toBeInTheDocument()
+        expect(screen.getAllByTestId('blank-card')).toHaveLength(1)
+      }
+      // A failed batch is not finding anything.
+      expect(screen.queryByText('Finding new side quests…')).not.toBeInTheDocument()
+      expect(screen.queryByTestId('skeleton-card')).not.toBeInTheDocument()
+
+      fireEvent.click(within(alert).getByRole('button', { name: 'Retry' }))
+      // The new batch's Side quest joins the deck.
+      await waitFor(() => expect(screen.getAllByRole('article', { hidden: true })).toHaveLength(deck.length + 1), { timeout: 3000 })
+      await waitFor(() => expect(screen.queryByRole('alert')).not.toBeInTheDocument())
+      expect(sent(server.fetchMock)).toEqual(['POST /api/v1/suggestions/batches'])
+    })
+
+    it.each([
+      ['once its job has been polled', false],
+      ['before its job has been polled', true],
+    ])('drops a failed batch %s once a read of the deck reports a later batch another session finished', async (_, slowPoll) => {
+      const poll = deferred()
+      const server = stubServer([BOWLS, TIKKA], {
+        'GET /api/v1/jobs/j1': () => (slowPoll ? poll.promise : jsonResponse(200, batchJob('j1', 'failed', [], 'ai_failed'))),
+      })
+      server.failedBatch = { batch_id: 'b-j1', job_id: 'j1' }
+      const queryClient = createQueryClient()
+      await renderPage(queryClient)
+      await screen.findByRole('article', { name: 'Korean beef bowls' })
+      if (slowPoll) await waitFor(() => expect(jobPolls(server.fetchMock)).toBe(1))
+      else expect(await screen.findByRole('alert')).toHaveTextContent("Couldn't find new side quests. ai_failed")
+
+      // Another session's batch j2 succeeds, superseding j1's failure.
+      server.failedBatch = null
+      server.deck = [BOWLS, TIKKA, ORZO]
+      const reads = deckReads(server.fetchMock)
+      await act(() => queryClient.invalidateQueries({ queryKey: DECK_KEY }))
+      await waitFor(() => expect(deckReads(server.fetchMock)).toBe(reads + 1))
+      await waitFor(() => expect(screen.getAllByRole('article', { hidden: true })).toHaveLength(3))
+      // A poll of j1 still out when the deck moved on does not bring it back.
+      await act(async () => poll.resolve(jsonResponse(200, batchJob('j1', 'failed', [], 'ai_failed'))))
+
+      expect(screen.queryByRole('alert')).not.toBeInTheDocument()
+      expect(screen.queryByText('Finding new side quests…')).not.toBeInTheDocument()
+      expect(sent(server.fetchMock)).toEqual([])
+    })
+
     it('follows the batch the deck reports running when the page opens, with skeleton cards rather than "All caught up"', async () => {
       const jobs = [batchJob('j1', 'running'), batchJob('j1', 'succeeded', ['s1'])]
       const server = stubServer([], {
@@ -1054,7 +1142,7 @@ describe('DiscoverPage', () => {
       let late: ReturnType<typeof deferred> | null = null
       const server = stubServer([BOWLS, TIKKA, ORZO, PAELLA], {
         'POST /api/v1/suggestions/s1/decline': () => decline.promise,
-        [`GET ${DECK}`]: () => late?.promise ?? jsonResponse(200, { suggestions: server.deck, active_batch: null }),
+        [`GET ${DECK}`]: () => late?.promise ?? jsonResponse(200, { suggestions: server.deck, active_batch: null, failed_batch: null }),
       })
       const queryClient = createQueryClient()
       await renderPage(queryClient)
@@ -1074,7 +1162,7 @@ describe('DiscoverPage', () => {
       server.deck = [TIKKA, ORZO, PAELLA]
       decline.resolve(declined(BOWLS))
       await waitFor(() => expect(screen.getByRole('button', { name: 'Pass' })).toBeEnabled())
-      await act(async () => stale.resolve(jsonResponse(200, { suggestions: [BOWLS, TIKKA, ORZO, PAELLA], active_batch: null })))
+      await act(async () => stale.resolve(jsonResponse(200, { suggestions: [BOWLS, TIKKA, ORZO, PAELLA], active_batch: null, failed_batch: null })))
 
       // The late answer is dropped, and the deck is read again after the save.
       await act(async () => await new Promise((resolve) => setTimeout(resolve, 0)))
@@ -1248,7 +1336,7 @@ describe('DiscoverPage', () => {
       let late: ReturnType<typeof deferred> | null = null
       const server = stubServer([BOWLS, TIKKA, ORZO, PAELLA], {
         'POST /api/v1/suggestions/s1/accept': () => accept,
-        [`GET ${DECK}`]: () => late?.promise ?? jsonResponse(200, { suggestions: server.deck, active_batch: null }),
+        [`GET ${DECK}`]: () => late?.promise ?? jsonResponse(200, { suggestions: server.deck, active_batch: null, failed_batch: null }),
       })
       const queryClient = createQueryClient()
       await renderPage(queryClient)
@@ -1265,7 +1353,7 @@ describe('DiscoverPage', () => {
       await act(async () => void queryClient.invalidateQueries({ queryKey: DECK_KEY }))
       late = null
       server.deck = [TIKKA, ORZO, PAELLA]
-      await act(async () => delayed.resolve(jsonResponse(200, { suggestions: server.deck, active_batch: null })))
+      await act(async () => delayed.resolve(jsonResponse(200, { suggestions: server.deck, active_batch: null, failed_batch: null })))
       const reads = deckReads(server.fetchMock)
       await act(async () => lose(new TypeError('Failed to fetch')))
 
@@ -1282,7 +1370,7 @@ describe('DiscoverPage', () => {
       let read: ReturnType<typeof deferred> | null = null
       const server = stubServer([BOWLS, TIKKA, ORZO, PAELLA], {
         'POST /api/v1/suggestions/s1/accept': () => accept.promise,
-        [`GET ${DECK}`]: () => read?.promise ?? jsonResponse(200, { suggestions: server.deck, active_batch: null }),
+        [`GET ${DECK}`]: () => read?.promise ?? jsonResponse(200, { suggestions: server.deck, active_batch: null, failed_batch: null }),
       })
       await renderPage()
       await screen.findByRole('article', { name: 'Korean beef bowls' })
@@ -1302,7 +1390,7 @@ describe('DiscoverPage', () => {
       expect(screen.queryByRole('alert')).not.toBeInTheDocument()
       expect(screen.getByRole('button', { name: 'Try it' })).toBeDisabled()
 
-      await act(async () => pending.resolve(jsonResponse(200, { suggestions: server.deck, active_batch: null })))
+      await act(async () => pending.resolve(jsonResponse(200, { suggestions: server.deck, active_batch: null, failed_batch: null })))
       const alert = await screen.findByRole('alert')
       expect(screen.queryByRole('article', { name: 'Korean beef bowls' })).not.toBeInTheDocument()
       expect(topCard()).toHaveAccessibleName('Crock pot chicken tikka masala')
@@ -1339,7 +1427,7 @@ describe('DiscoverPage', () => {
           read = deferred()
           return errorResponse(500, 'internal')
         },
-        [`GET ${DECK}`]: () => read?.promise ?? jsonResponse(200, { suggestions: server.deck, active_batch: null }),
+        [`GET ${DECK}`]: () => read?.promise ?? jsonResponse(200, { suggestions: server.deck, active_batch: null, failed_batch: null }),
       })
       await renderPage()
       await screen.findByRole('article', { name: 'Korean beef bowls' })
@@ -1359,7 +1447,7 @@ describe('DiscoverPage', () => {
       expect(sent(server.fetchMock)).toEqual(['POST /api/v1/suggestions/s1/accept'])
       expect(screen.queryByRole('alert')).not.toBeInTheDocument()
 
-      await act(async () => slow.resolve(jsonResponse(200, { suggestions: server.deck, active_batch: null })))
+      await act(async () => slow.resolve(jsonResponse(200, { suggestions: server.deck, active_batch: null, failed_batch: null })))
       const alert = await screen.findByRole('alert')
       expect(alert).toHaveTextContent("Couldn't save that choice.")
       expect(topCard()).toHaveAccessibleName('Korean beef bowls')
@@ -1393,7 +1481,7 @@ describe('DiscoverPage', () => {
           return errorResponse(500, 'internal')
         },
         'GET /api/v1/jobs/j1': () => poll?.promise ?? job([]),
-        [`GET ${DECK}`]: () => read?.promise ?? jsonResponse(200, { suggestions: server.deck, active_batch: null }),
+        [`GET ${DECK}`]: () => read?.promise ?? jsonResponse(200, { suggestions: server.deck, active_batch: null, failed_batch: null }),
       })
       const queryClient = createQueryClient()
       await renderPage(queryClient)
@@ -1428,7 +1516,7 @@ describe('DiscoverPage', () => {
 
       // The decision's read has the Side quest still pending: the card goes
       // back on top with a Retry, and the decision settles on that read.
-      await act(async () => reconciling.resolve(jsonResponse(200, { suggestions: server.deck, active_batch: null })))
+      await act(async () => reconciling.resolve(jsonResponse(200, { suggestions: server.deck, active_batch: null, failed_batch: null })))
       await waitFor(() => expect(screen.getByRole('button', { name: 'Try it' })).toBeEnabled())
       const alert = screen.getByRole('alert')
       expect(alert).toHaveTextContent("Couldn't save that choice.")
@@ -1438,7 +1526,7 @@ describe('DiscoverPage', () => {
       // The batch's read goes out after it, and brings its Side quest.
       await waitFor(() => expect(deckReads(server.fetchMock)).toBe(reads + 2))
       read = null
-      await act(async () => after.resolve(jsonResponse(200, { suggestions: server.deck, active_batch: null })))
+      await act(async () => after.resolve(jsonResponse(200, { suggestions: server.deck, active_batch: null, failed_batch: null })))
       await waitFor(() => expect(queryClient.getQueryData<{ suggestions: { id: string }[] }>(DECK_KEY)?.suggestions.map((q) => q.id)).toEqual(['s2', 's3', 's4', 's5']))
       expect(topCard()).toHaveAccessibleName('Crock pot chicken tikka masala')
 

web/src/discover/DiscoverPage.tsx

@@ -95,25 +95,31 @@ export function DiscoverPage() {
   const questLog = useQuestLog()
   const decision = useDecide()
   const startBatch = useStartBatch()
-  // The job of the batch last known to be finding Side quests: the one Get
+  // The job of the batch last known to be finding Side quests, or to have
-  // more now started or joined, the replenish a decision started or joined,
+  // failed to: the one Get more now started or joined, the replenish a
-  // or the deck's active batch, which may have been started elsewhere or
+  // decision started or joined, or the deck's active or failed batch, which
-  // before the page opened.
+  // may have been started elsewhere or before the page opened.
   const [batchJobId, setBatchJobId] = useState<string | null>(null)
-  // A read of the deck reporting a new active batch follows its job from
-  // that render on; one reporting none leaves the last job followed, so its
-  // end and any failure stay shown.
-  const activeJobId = deck.data?.activeBatch?.job_id ?? null
-  const [seenActiveJobId, setSeenActiveJobId] = useState<string | null>(null)
-  if (activeJobId !== seenActiveJobId) {
-    setSeenActiveJobId(activeJobId)
-    if (activeJobId !== null) setBatchJobId(activeJobId)
-  }
   const job = useJob<unknown>(BATCH_JOB_KIND, batchJobId)
+  // A read of the deck reporting a new active or failed batch follows its
+  // job from that render on. One reporting neither leaves the last job
+  // followed, so its end stays shown — unless that job is known to have
+  // failed, by its poll or the deck's last read: the deck reports the newest
+  // batch's failure, so a later batch has superseded it, and it goes.
+  const failedJobId = deck.data?.failedBatch?.job_id ?? null
+  const deckJobId = deck.data?.activeBatch?.job_id ?? failedJobId
+  const [seenRead, setSeenRead] = useState({ at: 0, jobId: null as string | null, failedJobId: null as string | null })
+  if (deck.dataUpdatedAt !== seenRead.at) {
+    setSeenRead({ at: deck.dataUpdatedAt, jobId: deckJobId, failedJobId })
+    if (deckJobId !== null && deckJobId !== seenRead.jobId) setBatchJobId(deckJobId)
+    else if (deckJobId === null && batchJobId !== null && (job.status === 'failed' || batchJobId === seenRead.failedJobId)) setBatchJobId(null)
+  }
 
   const top = cards?.[0]
   const [busy, setBusy] = useState(false)
-  const jobRunning = batchJobId !== null && job.status !== 'succeeded' && job.status !== 'failed'
+  // The deck's failed batch is not finding anything, even before its job's
+  // first poll says why it failed.
+  const jobRunning = batchJobId !== null && batchJobId !== failedJobId && job.status !== 'succeeded' && job.status !== 'failed'
   const finding = startBatch.isPending || jobRunning
   const { mutate: mutateStartBatch } = startBatch
   const getMore = useCallback(() => mutateStartBatch(undefined, { onSuccess: (b) => setBatchJobId(b.job_id) }), [mutateStartBatch])

web/src/discover/api.ts

@@ -30,13 +30,15 @@ export type SideQuest = {
 export type Choice = 'try' | 'pass'
 
 // Batch is a POST /api/v1/suggestions/batches answer, or the deck's active
-// batch: the batch and the job running it, which GET /api/v1/jobs/{id}
+// or failed batch: the batch and the job running it, which GET
-// reports on.
+// /api/v1/jobs/{id} reports on.
 export type Batch = { batch_id: string; job_id: string }
 
 // Deck is the deck as GET /api/v1/suggestions/deck reads it: the pending
-// Side quests, oldest first, and the batch pending or running, if any.
+// Side quests, oldest first; the batch pending or running, if any; and the
-export type Deck = { suggestions: SideQuest[]; activeBatch: Batch | null }
+// newest batch when it failed, which a later batch supersedes. At most one
+// of the two batches is set.
+export type Deck = { suggestions: SideQuest[]; activeBatch: Batch | null; failedBatch: Batch | null }
 
 function isBatch(body: unknown): body is Batch {
   return isRecord(body) && typeof body.batch_id === 'string' && typeof body.job_id === 'string'
@@ -91,8 +93,9 @@ function parseSideQuest(where: string, body: unknown): SideQuest {
   }
 }
 
-// useDeck is the pending Side quests, oldest first, and the batch pending
+// useDeck is the pending Side quests, oldest first, the batch pending or
-// or running (GET /api/v1/suggestions/deck).
+// running and the newest batch when it failed (GET
+// /api/v1/suggestions/deck).
 export function useDeck() {
   const { apiFetch } = useApi()
   return useQuery({
@@ -102,9 +105,11 @@ export function useDeck() {
       const body = await apiFetch<unknown>(DECK_PATH)
       if (!isRecord(body) || !Array.isArray(body.suggestions)) throw new Error(`${where}: response has no suggestions list`)
       if (body.active_batch !== null && !isBatch(body.active_batch)) throw new Error(`${where}: response has no active_batch`)
+      if (body.failed_batch !== null && !isBatch(body.failed_batch)) throw new Error(`${where}: response has no failed_batch`)
       return {
         suggestions: body.suggestions.map((suggestion) => parseSideQuest(where, suggestion)),
         activeBatch: body.active_batch === null ? null : { batch_id: body.active_batch.batch_id, job_id: body.active_batch.job_id },
+        failedBatch: body.failed_batch === null ? null : { batch_id: body.failed_batch.batch_id, job_id: body.failed_batch.job_id },
       }
     },
   })