fix(insights): decouple the forced warm sections so a slow comprehensive step cannot starve the rest

The on-demand force-warm ran the comprehensive insight first and only ran
the per-status (7) and generic-metric (~30) assessment passes when that
step returned generated or cached. A slow or stalled comprehensive
generation therefore aborted the entire warm — observed in production as a
102 s run ending with assessments_warmed: 0 — leaving every card cold and
the user back on the lazy 30-60 s on-first-click path.

Give the comprehensive step its own bounded budget and treat a timeout,
error, or skip as non-fatal: the per-status and generic-metric passes now
always run afterwards. Each per-card generator resolves its own provider
chain and bounds its own provider call, so running them after a failed
comprehensive costs a cheap chain-resolve when no provider is configured
and is capped per card otherwise. The result reports each section's
outcome independently, and a timed-out comprehensive is reported distinctly
from a failed one so a partial warm is observable.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
(cherry picked from commit f52f8e9c3b32590f3b085d082418ebdd3a923f1d)

src/lib/jobs/__tests__/insight-pregenerate.test.ts

@@ -415,15 +415,22 @@ describe("forceWarmUser — on-demand single-user warm (v1.8.7.1)", () => {
     });
   });
 
-  it("does NOT warm the per-status or generic caches when the comprehensive pass skipped (no provider)", async () => {
+  it("still runs the per-status + generic warm even when the comprehensive pass skipped (no provider)", async () => {
+    // v1.9.0 — the three sections are decoupled. A skipped comprehensive
+    // (no provider) must NOT short-circuit the per-status + generic passes:
+    // each generator resolves its own provider chain and no-ops cheaply when
+    // none is configured, so running them is safe and is what warms the cards
+    // when the comprehensive briefing is the only thing missing a provider.
     const { prisma } = makePrisma([]);
     const generate = vi
       .fn()
       .mockResolvedValue({ status: "skipped", reason: "no-provider" });
+    // Each generator independently reports no-provider — nothing warmed, but
+    // it WAS invoked (it is the generator, not the caller, that decides).
     const statusGenerators = Array.from({ length: 7 }, () =>
       vi.fn().mockResolvedValue({ hasProvider: false, cached: true }),
     );
-    const warmGenericMetrics = vi.fn().mockResolvedValue(5);
+    const warmGenericMetrics = vi.fn().mockResolvedValue(0);
 
     const result = await forceWarmUser(prisma as never, "u1", "de", {
       generate,
@@ -432,9 +439,10 @@ describe("forceWarmUser — on-demand single-user warm (v1.8.7.1)", () => {
     });
 
     for (const g of statusGenerators) {
-      expect(g).not.toHaveBeenCalled();
+      expect(g).toHaveBeenCalledTimes(1);
+      expect(g).toHaveBeenCalledWith("u1", { locale: "de", force: true });
     }
-    expect(warmGenericMetrics).not.toHaveBeenCalled();
+    expect(warmGenericMetrics).toHaveBeenCalledWith("u1", ["de"]);
     expect(result).toMatchObject({
       comprehensive: "skipped",
       assessmentsWarmed: 0,
@@ -442,6 +450,84 @@ describe("forceWarmUser — on-demand single-user warm (v1.8.7.1)", () => {
     });
   });
 
+  it("warms the per-status + generic caches even when the comprehensive pass fails", async () => {
+    // v1.9.0 — a failed comprehensive (provider chain unhealthy this run)
+    // is non-fatal: the cheaper per-status (7) + generic (~30) cards still
+    // warm so the user's first click is a cache read, not a 30–60 s lazy
+    // generation. This is the prod regression the stability wave closes.
+    const { prisma } = makePrisma([]);
+    const generate = vi
+      .fn()
+      .mockResolvedValue({ status: "failed", reason: "all-providers-failed" });
+    const statusGenerators = Array.from({ length: 7 }, () =>
+      vi.fn().mockResolvedValue({ hasProvider: true, cached: false }),
+    );
+    const warmGenericMetrics = vi.fn().mockResolvedValue(4);
+
+    const result = await forceWarmUser(prisma as never, "u1", "de", {
+      generate,
+      statusGenerators,
+      warmGenericMetrics,
+    });
+
+    for (const g of statusGenerators) {
+      expect(g).toHaveBeenCalledTimes(1);
+    }
+    expect(warmGenericMetrics).toHaveBeenCalledWith("u1", ["de"]);
+    expect(result).toMatchObject({
+      comprehensive: "failed",
+      assessmentsWarmed: 7,
+      metricAssessmentsWarmed: 4,
+    });
+  });
+
+  it("reports a timed-out comprehensive distinctly and still warms the rest", async () => {
+    // A comprehensive generation that exceeds its bounded budget is
+    // abandoned and reported as `timeout`, but the per-status + generic
+    // passes run regardless. The prod incident was a 102 s comprehensive
+    // that aborted the whole warm with `assessments_warmed: 0`.
+    const { prisma } = makePrisma([]);
+    const generate = vi.fn().mockImplementation(
+      () =>
+        new Promise((resolve) => {
+          // Never settles within the bounded budget; the fake timers below
+          // advance past it so the warm continues.
+          setTimeout(
+            () => resolve({ status: "generated", providerType: "x" }),
+            120_000,
+          );
+        }),
+    );
+    const statusGenerators = Array.from({ length: 7 }, () =>
+      vi.fn().mockResolvedValue({ hasProvider: true, cached: false }),
+    );
+    const warmGenericMetrics = vi.fn().mockResolvedValue(3);
+
+    vi.useFakeTimers();
+    try {
+      const promise = forceWarmUser(prisma as never, "u1", "de", {
+        generate,
+        statusGenerators,
+        warmGenericMetrics,
+      });
+      // Advance past the comprehensive budget so withTimeout fires.
+      await vi.advanceTimersByTimeAsync(60_000);
+      const result = await promise;
+
+      for (const g of statusGenerators) {
+        expect(g).toHaveBeenCalledTimes(1);
+      }
+      expect(warmGenericMetrics).toHaveBeenCalledWith("u1", ["de"]);
+      expect(result).toMatchObject({
+        comprehensive: "timeout",
+        assessmentsWarmed: 7,
+        metricAssessmentsWarmed: 3,
+      });
+    } finally {
+      vi.useRealTimers();
+    }
+  });
+
   it("short-circuits to a no-op when the briefing surface is disabled globally", async () => {
     getAssistantFlags.mockResolvedValue({ enabled: false, briefing: false });
     const { prisma } = makePrisma([]);

src/lib/jobs/insight-pregenerate.ts

@@ -68,6 +68,7 @@ import {
   getMetricStatusMeta,
   type MetricStatusMetricId,
 } from "@/lib/insights/metric-status-registry";
+import { withTimeout } from "@/lib/insights/with-timeout";
 import {
   INSIGHT_PREGENERATE_QUEUE,
   enqueueForceWarm,
@@ -109,10 +110,28 @@ const PREGENERATE_BUDGET_WINDOW_MS = 20 * 60 * 60 * 1000;
  */
 const WARM_PASS_CONCURRENCY = 3;
 
+/**
+ * Bounded budget for the comprehensive step of a forced full warm. The
+ * comprehensive insight is the single heaviest generation (full feature
+ * extraction + a 1500-token completion) and a slow or stalled provider
+ * could pin the whole warm for a minute-plus, after which the entire job
+ * aborted with the per-status + generic-metric passes never run. Capping
+ * it lets the forced path abandon a slow comprehensive and still warm the
+ * cheaper per-status (7) + generic-metric (~30) assessments, so the user's
+ * cards are warm even when the briefing generation stalls. Set a hair
+ * below the queue's own retry horizon so a single tick stays bounded.
+ */
+const FORCE_WARM_COMPREHENSIVE_TIMEOUT_MS = 45_000;
+
 /** Per-user result of one forced full warm. */
 export interface ForceWarmResult {
-  /** Outcome of the comprehensive-insight generation. */
-  comprehensive: GenerateOutcome["status"];
+  /**
+   * Outcome of the comprehensive-insight generation. `"timeout"` is
+   * distinct from `"failed"`: the comprehensive step exceeded its own
+   * bounded budget (see `FORCE_WARM_COMPREHENSIVE_TIMEOUT_MS`) and was
+   * abandoned so the per-status + generic-metric passes could still run.
+   */
+  comprehensive: GenerateOutcome["status"] | "timeout";
   /** Count of specialised per-status assessments written warm. */
   assessmentsWarmed: number;
   /** Count of generic per-HealthKit-metric assessments written warm. */
@@ -493,11 +512,20 @@ export async function runInsightPregenerate(
  *     the queue `singletonKey`.
  *
  * Unlike the nightly cron (de+en), the forced path warms only the caller's
- * active `locale` so a single-locale user pays half the provider cost. A
- * `failed`/`skipped` comprehensive outcome short-circuits the per-status
- * warm exactly as the cron does — re-running seven (then ~30) generators
- * against an unhealthy or provider-less chain would only multiply the
- * failure.
+ * active `locale` so a single-locale user pays half the provider cost.
+ *
+ * The three sections are decoupled: the comprehensive step runs under its
+ * own bounded budget (`FORCE_WARM_COMPREHENSIVE_TIMEOUT_MS`) and its
+ * failure / timeout / skip is non-fatal — the per-status (7) and
+ * generic-metric (~30) warm passes ALWAYS run afterwards. The earlier
+ * design short-circuited both passes on a non-`generated`/`cached`
+ * comprehensive, so a single slow briefing generation left every card cold
+ * and the user back on the lazy 30–60 s on-first-click path. The per-card
+ * generators each resolve their own provider chain and bound their own
+ * provider call, so a missing provider costs a cheap chain-resolve and a
+ * degraded one is capped per card — running them after a failed
+ * comprehensive can never multiply a stall. The result reports each
+ * section's outcome independently so a partial warm is observable.
  */
 export async function forceWarmUser(
   prisma: PrismaClient,
@@ -536,18 +564,40 @@ export async function forceWarmUser(
 
   const locales: ReadonlyArray<"de" | "en"> = [locale];
 
-  const outcome = await generate(userId, { locale, force: true });
-  result.comprehensive = outcome.status;
-
-  if (outcome.status === "generated" || outcome.status === "cached") {
-    result.assessmentsWarmed = await warmPerStatusCaches(
-      userId,
-      locales,
-      statusGenerators,
-    );
-    result.metricAssessmentsWarmed = await warmGenericMetrics(userId, locales);
+  // The comprehensive step gets its own bounded budget and its failure is
+  // non-fatal. A slow or stalled briefing generation must not short-circuit
+  // the per-status + generic-metric passes — those are the ~37 cards the
+  // user actually clicks first, and warming them is independent of the
+  // comprehensive insight (each generator resolves its own provider chain
+  // and writes its own cache row). A timeout / error / skipped comprehensive
+  // now falls through to the warm passes instead of aborting the whole job.
+  const comprehensive = await withTimeout(
+    () => generate(userId, { locale, force: true }),
+    FORCE_WARM_COMPREHENSIVE_TIMEOUT_MS,
+    null,
+  );
+  if (comprehensive.timedOut) {
+    result.comprehensive = "timeout";
+  } else if (comprehensive.errored || comprehensive.value === null) {
+    result.comprehensive = "failed";
+  } else {
+    result.comprehensive = comprehensive.value.status;
   }
 
+  // Always run the per-status + generic-metric warm passes, regardless of
+  // the comprehensive outcome. Each generator independently no-ops to its
+  // no-key fallback when no provider is configured (no LLM call) and serves
+  // its fallback without persisting on a per-card timeout, so running them
+  // after a failed comprehensive is safe and bounded by their own per-card
+  // budgets. The only outcome that has nothing to warm is a missing provider,
+  // and the generators detect that themselves at near-zero cost.
+  result.assessmentsWarmed = await warmPerStatusCaches(
+    userId,
+    locales,
+    statusGenerators,
+  );
+  result.metricAssessmentsWarmed = await warmGenericMetrics(userId, locales);
+
   annotate({
     action: { name: "insights.pregenerate.force" },
     meta: {