From 5e60eaf9c0451d66aa8e7f17fddcf1e4cffeff44 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 4 Jun 2026 16:30:53 +0200 Subject: [PATCH] fix(insights): serve the daily briefing read-only so the overview never blocks on the LLM MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The hero strip, daily-briefing card and trends row all read the advisor payload through `useInsightsAdvisorQuery`, which POSTed `/api/insights/generate` without `force`. That POST generates inline on a cache miss, pinning the page-load path on the full provider chain (up to the client's 8 s abort) before the briefing paints — even though a read-only lift of `User.insightsCachedText` costs nothing. Add a read-only GET to the route that serves the cached payload immediately and, when the cache is stale or missing and a provider is configured, enqueues an out-of-band warm on the existing `insight-pregenerate` queue. The read mount now uses the GET; user-initiated regeneration keeps the POST force path. Mirrors the stale-while-revalidate posture of the per-metric status and period-narrative reads — no provider call ever sits on a navigation request. --- .../insights/generate/__tests__/route.test.ts | 72 ++++++++++++++- src/app/api/insights/generate/route.ts | 88 +++++++++++++++++++ .../insights/use-insights-advisor.ts | 21 +++-- 3 files changed, 174 insertions(+), 7 deletions(-) diff --git a/src/app/api/insights/generate/__tests__/route.test.ts b/src/app/api/insights/generate/__tests__/route.test.ts index bd1c7d9c3..abf9f5669 100644 --- a/src/app/api/insights/generate/__tests__/route.test.ts +++ b/src/app/api/insights/generate/__tests__/route.test.ts @@ -76,6 +76,16 @@ vi.mock("@/lib/rate-limit", () => ({ checkRateLimit: vi.fn(async () => ({ allowed: true })), })); +// The read-only GET probes the provider chain (no completion) and +// enqueues an out-of-band warm on a stale / missing cache. +vi.mock("@/lib/insights/status-provider", () => ({ + hasUsableStatusProvider: vi.fn(async () => true), +})); + +vi.mock("@/lib/jobs/insight-pregenerate-shared", () => ({ + enqueueForceWarm: vi.fn(async () => undefined), +})); + vi.mock("@/lib/logging/context", () => ({ annotate: vi.fn(), })); @@ -99,10 +109,12 @@ vi.mock("@/lib/i18n/server-locale", () => ({ resolveServerLocale: vi.fn(async () => "en"), })); -import { POST, resolveInsightsRateLimit } from "../route"; +import { GET, POST, resolveInsightsRateLimit } from "../route"; import { resolveProvider } from "@/lib/ai/provider"; import { checkRateLimit } from "@/lib/rate-limit"; import { prisma } from "@/lib/db"; +import { hasUsableStatusProvider } from "@/lib/insights/status-provider"; +import { enqueueForceWarm } from "@/lib/jobs/insight-pregenerate-shared"; import { clearLastWorkingProviderCache } from "@/lib/ai/provider-runner"; import { extractFeatures, @@ -404,3 +416,61 @@ describe("POST /api/insights/generate — payload-size hard downgrade (H1)", () expect(annotated, "annotate event with insights_payload_too_large").toBeTruthy(); }); }); + +describe("GET /api/insights/generate — read-only advisor read", () => { + it("serves the cached briefing without ever calling the provider", async () => { + const cached = { dailyBriefing: { paragraph: "ok", keyFindings: [] } }; + vi.mocked(prisma.user.findUnique).mockResolvedValueOnce({ + insightsCachedAt: new Date(), + insightsCachedText: JSON.stringify(cached), + locale: "en", + } as never); + + const res = await (GET as () => Promise)(); + expect(res.status).toBe(200); + const body = (await res.json()) as { + data: { insights: unknown; cached: boolean }; + }; + expect(body.data.cached).toBe(true); + expect(body.data.insights).toEqual(cached); + // No completion is ever run on the read path. + expect(resolveProvider).not.toHaveBeenCalled(); + // A fresh cache (just now) does not trigger a warm. + expect(enqueueForceWarm).not.toHaveBeenCalled(); + }); + + it("enqueues an out-of-band warm when the cache is stale and a provider exists", async () => { + const stale = new Date(Date.now() - 48 * 60 * 60 * 1000); + vi.mocked(prisma.user.findUnique).mockResolvedValueOnce({ + insightsCachedAt: stale, + insightsCachedText: JSON.stringify({ dailyBriefing: null }), + locale: "en", + } as never); + + const res = await (GET as () => Promise)(); + expect(res.status).toBe(200); + expect(enqueueForceWarm).toHaveBeenCalledWith({ + userId: "u-1", + locale: "en", + }); + expect(resolveProvider).not.toHaveBeenCalled(); + }); + + it("returns an empty payload (no warm) on a cold cache without a provider", async () => { + vi.mocked(hasUsableStatusProvider).mockResolvedValueOnce(false); + vi.mocked(prisma.user.findUnique).mockResolvedValueOnce({ + insightsCachedAt: null, + insightsCachedText: null, + locale: "en", + } as never); + + const res = await (GET as () => Promise)(); + expect(res.status).toBe(200); + const body = (await res.json()) as { + data: { insights: unknown; cached: boolean }; + }; + expect(body.data.cached).toBe(false); + expect(body.data.insights).toBeNull(); + expect(enqueueForceWarm).not.toHaveBeenCalled(); + }); +}); diff --git a/src/app/api/insights/generate/route.ts b/src/app/api/insights/generate/route.ts index 73b33cd9a..d2c8b9a7a 100644 --- a/src/app/api/insights/generate/route.ts +++ b/src/app/api/insights/generate/route.ts @@ -41,6 +41,18 @@ import { requireAssistantSurface } from "@/lib/feature-flags"; import { invalidateUserInsights } from "@/lib/cache/invalidate"; import { annotate } from "@/lib/logging/context"; import { resolveServerLocale } from "@/lib/i18n/server-locale"; +import { hasUsableStatusProvider } from "@/lib/insights/status-provider"; +import { enqueueForceWarm } from "@/lib/jobs/insight-pregenerate-shared"; + +export const dynamic = "force-dynamic"; + +/** + * Briefing freshness window for the read-only GET. A cached briefing read + * this recently is fresh and triggers no warm; older / missing enqueues an + * out-of-band regeneration. Mirrors the 24 h cache-hit window the POST path + * honours so the two stay consistent. + */ +const BRIEFING_FRESH_MS = 24 * 60 * 60 * 1000; const DEFAULT_INSIGHTS_RATE_LIMIT_PER_HOUR = 10; @@ -184,6 +196,82 @@ async function buildComparisonSnapshotForUser( return { baseline, metrics }; } +/** + * GET /api/insights/generate — read-only advisor read. + * + * The advisor consumers (hero strip, daily-briefing card, trends row) used + * to mount against the POST handler, which generates inline on a cache miss + * and blocks the page-load path on the full provider chain (up to the + * client's 8 s abort). This read-only GET serves the cached payload from + * `User.insightsCachedText` immediately — NEVER calling the provider — and, + * when the cache is stale / missing AND a provider is configured, enqueues + * an out-of-band warm (the same `insight-pregenerate` queue the nightly + * cron and the "prepare assessments" button use). The next read reflects + * the fresh briefing. User-initiated regeneration stays on the POST path. + * + * `userId` is narrowed from the session / Bearer — never a body field. + */ +export const GET = apiHandler(async () => { + const { user } = await requireAuth(); + // Same surface gate as the POST + the read-only status routes: a user + // with assessments enabled but Coach disabled still reads the cached + // briefing. + await requireAssistantSurface("coach"); + const userId = user.id; + + const dbUser = await prisma.user.findUnique({ + where: { id: userId }, + select: { + insightsCachedAt: true, + insightsCachedText: true, + locale: true, + }, + }); + + const cachedAt = dbUser?.insightsCachedAt ?? null; + const isFresh = + cachedAt !== null && + Date.now() - cachedAt.getTime() < BRIEFING_FRESH_MS; + + // Read-only: never block on the provider. Warm out of band only when the + // cached briefing is stale / missing AND a provider is configured (a + // provider-less account costs one cheap chain-resolve and shows the + // empty / connect-AI state instead of a wasted enqueue). + let revalidating = false; + if (!isFresh && (await hasUsableStatusProvider(userId))) { + const locale = (dbUser?.locale ?? user.locale) === "en" ? "en" : "de"; + void enqueueForceWarm({ userId, locale }); + revalidating = true; + } + + if (dbUser?.insightsCachedText) { + try { + const cached = JSON.parse(dbUser.insightsCachedText); + const legacyPayload = isLegacyInsightPayload(cached); + annotate({ + action: { name: "insights.generate.read" }, + meta: { cached: true, legacyPayload, revalidating }, + }); + return apiSuccess({ + insights: cached, + cached: true, + cachedAt, + legacyPayload, + }); + } catch { + // Invalid cache row — fall through to the empty payload below. The + // warm enqueue above (when a provider exists) repairs it for the + // next read. + } + } + + annotate({ + action: { name: "insights.generate.read" }, + meta: { cached: false, revalidating }, + }); + return apiSuccess({ insights: null, cached: false, legacyPayload: false }); +}); + export const POST = apiHandler(async (request: NextRequest) => { const { user } = await requireAuth(); // v1.4.31 — the advisor + daily briefing surfaces ride this diff --git a/src/components/insights/use-insights-advisor.ts b/src/components/insights/use-insights-advisor.ts index 775077698..aad5d43ec 100644 --- a/src/components/insights/use-insights-advisor.ts +++ b/src/components/insights/use-insights-advisor.ts @@ -73,12 +73,21 @@ async function fetchAdvisor( ); let res: Response; try { - res = await fetch("/api/insights/generate", { - method: "POST", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify(options.force ? { force: true } : {}), - signal: controller.signal, - }); + // Read path (no `force`): the GET serves the cached briefing read-only + // and enqueues an out-of-band warm on a stale / missing cache — it + // never blocks the page-load path on the provider chain. Only the + // user-initiated regenerate (`force`) POSTs to generate inline. + res = options.force + ? await fetch("/api/insights/generate", { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ force: true }), + signal: controller.signal, + }) + : await fetch("/api/insights/generate", { + method: "GET", + signal: controller.signal, + }); } catch (err) { if (err instanceof DOMException && err.name === "AbortError") { // Graceful empty payload — the UI surfaces the empty / regen