From 556f442724d5520fc32cc91da8809c8cf860b656 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sun, 28 Jun 2026 12:42:35 +0200 Subject: [PATCH 1/2] fix(insights): honor the per-user AI timeout and keep the last good narrative MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The daily briefing and the per-metric status assessments ignored the per-user response-timeout setting: they hardcoded their per-surface AI_BUDGETS default, so raising the timeout for a slow self-hosted or local backend had no effect and the surfaces still aborted early. Only the coach threaded the setting through. Resolve the effective timeout from User.aiResponseTimeoutSeconds in one shared helper and thread it onto every non-coach generation call: - runStatusCompletion is the single chokepoint every status / reference surface funnels through (per-metric cards, the batched assessment, the derived assessments, the period narratives), so resolving it there applies the setting to all of them at once — on both the upstream call's own timeout and the outer wall-clock cap that previously clipped a raised value at 60 s. - The comprehensive briefing generator and the user-initiated regenerate route thread it onto the main generation, the JSON retry, the grounding retry, and the paragraph re-roll. On a failed generation the briefing keeps its last good text — no cache row is written, so the previous payload stays intact — and records a dated, append-only failure marker (no migration; reuses the audit-log JSON pattern). The read path compares the marker against the last successful generation so it can tell a held briefing from a current one. --- .../insights/generate/__tests__/route.test.ts | 5 + src/app/api/insights/generate/route.ts | 46 +++++- .../ai/__tests__/effective-timeout.test.ts | 26 ++++ src/lib/ai/effective-timeout.ts | 25 ++++ .../__tests__/briefing-failure-marker.test.ts | 92 ++++++++++++ .../comprehensive-timeout-honored.test.ts | 140 ++++++++++++++++++ src/lib/insights/briefing-failure-marker.ts | 89 +++++++++++ src/lib/insights/comprehensive-generate.ts | 42 +++++- src/lib/insights/status-provider.ts | 28 +++- 9 files changed, 484 insertions(+), 9 deletions(-) create mode 100644 src/lib/ai/__tests__/effective-timeout.test.ts create mode 100644 src/lib/ai/effective-timeout.ts create mode 100644 src/lib/insights/__tests__/briefing-failure-marker.test.ts create mode 100644 src/lib/insights/__tests__/comprehensive-timeout-honored.test.ts create mode 100644 src/lib/insights/briefing-failure-marker.ts diff --git a/src/app/api/insights/generate/__tests__/route.test.ts b/src/app/api/insights/generate/__tests__/route.test.ts index 355250d4a..550817bd4 100644 --- a/src/app/api/insights/generate/__tests__/route.test.ts +++ b/src/app/api/insights/generate/__tests__/route.test.ts @@ -48,6 +48,11 @@ vi.mock("@/lib/db", () => ({ // generation. The test prisma mock has to surface the call so // the cache-invalidation test can assert against it. deleteMany: vi.fn(async () => ({ count: 0 })), + // v1.25 — the GET read path consults the briefing-failure marker + // (`readBriefingFailure`) and the failure paths append one + // (`recordBriefingFailure`). Default to no marker. + findFirst: vi.fn(async () => null), + create: vi.fn(async () => ({})), }, // v1.12.1 — the server-managed consent gate reads the latest active // receipt before egress. These fixtures resolve to an admin-openai diff --git a/src/app/api/insights/generate/route.ts b/src/app/api/insights/generate/route.ts index 72c5af606..5f15fe2da 100644 --- a/src/app/api/insights/generate/route.ts +++ b/src/app/api/insights/generate/route.ts @@ -61,6 +61,11 @@ import { type InsightResult, } from "@/lib/ai/types"; import { AI_BUDGETS } from "@/lib/ai/ai-budgets"; +import { resolveEffectiveTimeoutMs } from "@/lib/ai/effective-timeout"; +import { + recordBriefingFailure, + readBriefingFailure, +} from "@/lib/insights/briefing-failure-marker"; import { resolveProvider, resolveProviderChain } from "@/lib/ai/provider"; import { AllProvidersFailedError, @@ -307,6 +312,15 @@ export const GET = apiHandler(async (request: NextRequest) => { // connect-an-AI-provider hint instead of implying the read is live. const hasProvider = await hasUsableStatusProvider(userId); + // v1.25 — surface whether the most recent generation attempt failed (a + // marker newer than the last successful generation). The briefing keeps its + // last good text on failure, so this is the only honest signal that a shown + // briefing is held rather than freshly refreshed — and, when there is no + // last good text, that the empty state should read "couldn't generate" with + // a retry rather than the generic "no briefing yet". + const generationFailed = + (await readBriefingFailure({ userId, since: cachedAt })) !== null; + // 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 @@ -351,6 +365,8 @@ export const GET = apiHandler(async (request: NextRequest) => { // the served briefing can never refresh, so the UI pairs its age // with a connect-provider affordance. hasProvider, + // v1.25 — true when the last generation attempt failed (held text). + generationFailed, }); } catch { // Invalid cache row — fall through to the empty payload below. The @@ -370,6 +386,9 @@ export const GET = apiHandler(async (request: NextRequest) => { revalidating, // v1.18.9 (#4) — see the cached branch above. hasProvider, + // v1.25 — no last-good text AND the last attempt failed: the UI shows a + // "couldn't generate" empty state with a retry instead of the generic one. + generationFailed, }); }); @@ -403,9 +422,17 @@ export const POST = apiHandler((request: NextRequest) => heightCm: true, // v1.22 (W6) — first name for the sparse, hash-gated briefing opener. displayName: true, + // v1.25 — per-user response-timeout (seconds), threaded onto the + // generation calls so a slow self-hosted backend honours the setting. + aiResponseTimeoutSeconds: true, }, }); + const effectiveTimeoutMs = resolveEffectiveTimeoutMs( + dbUser?.aiResponseTimeoutSeconds, + AI_BUDGETS.comprehensive.timeoutMs, + ); + const locale = await resolveServerLocale({ request, userLocale: dbUser?.locale ?? user.locale ?? null, @@ -717,7 +744,8 @@ export const POST = apiHandler((request: NextRequest) => // v1.21.5 — wider upstream budget so the reasoning-heavy briefing // generation is not aborted at the client's 60 s default on large // accounts (which returned an empty briefing + trend narrative). - timeoutMs: AI_BUDGETS.comprehensive.timeoutMs, + // v1.25 — honours the per-user response-timeout setting. + timeoutMs: effectiveTimeoutMs, // The reply is parsed with `JSON.parse` below, so opt the OpenAI / // Codex chains into their strict JSON mode (gated on this flag). responseFormat: "json", @@ -727,6 +755,18 @@ export const POST = apiHandler((request: NextRequest) => workingProviderType = fallback.workingProvider.providerType; fallbackHopCount = fallback.fallbackHops.length; } catch (e) { + // v1.25 — record a dated failure marker (no cache row is written on this + // path, so the last good briefing stays intact). The read path pairs the + // preserved text with a discreet "couldn't refresh" hint, or shows a + // retry empty state when there is no last good text. + void recordBriefingFailure({ + userId, + reason: + e instanceof AllProvidersFailedError + ? "all-providers-failed" + : "provider-error", + locale, + }); if (e instanceof AllProvidersFailedError) { annotate({ meta: { @@ -826,6 +866,7 @@ export const POST = apiHandler((request: NextRequest) => // breaks `await res.json()` on the client side. 422 stays passthrough // so the React Query mutation can read the JSON body and surface a // readable message. Same fix pattern as v1.4.5 ai/test. + void recordBriefingFailure({ userId, reason: "invalid-json", locale }); return apiError("AI response was not valid JSON", 422); } @@ -856,7 +897,8 @@ export const POST = apiHandler((request: NextRequest) => temperature: 0.3, maxTokens: 1500, // v1.21.5 — wider upstream budget; see the first generation call. - timeoutMs: AI_BUDGETS.comprehensive.timeoutMs, + // v1.25 — honours the per-user response-timeout setting. + timeoutMs: effectiveTimeoutMs, // Parsed with `JSON.parse` below — opt OpenAI / Codex into JSON mode. responseFormat: "json", }), diff --git a/src/lib/ai/__tests__/effective-timeout.test.ts b/src/lib/ai/__tests__/effective-timeout.test.ts new file mode 100644 index 000000000..96a95bf13 --- /dev/null +++ b/src/lib/ai/__tests__/effective-timeout.test.ts @@ -0,0 +1,26 @@ +import { describe, it, expect } from "vitest"; +import { resolveEffectiveTimeoutMs } from "../effective-timeout"; + +const BUDGET = 120_000; + +describe("resolveEffectiveTimeoutMs", () => { + it("converts a positive per-user setting from seconds to ms", () => { + expect(resolveEffectiveTimeoutMs(300, BUDGET)).toBe(300_000); + expect(resolveEffectiveTimeoutMs(10, BUDGET)).toBe(10_000); + }); + + it("falls back to the budget default when the setting is unset", () => { + expect(resolveEffectiveTimeoutMs(null, BUDGET)).toBe(BUDGET); + expect(resolveEffectiveTimeoutMs(undefined, BUDGET)).toBe(BUDGET); + }); + + it("falls back to the budget default for a non-positive value", () => { + expect(resolveEffectiveTimeoutMs(0, BUDGET)).toBe(BUDGET); + expect(resolveEffectiveTimeoutMs(-5, BUDGET)).toBe(BUDGET); + }); + + it("honours different budget defaults per surface", () => { + expect(resolveEffectiveTimeoutMs(null, 60_000)).toBe(60_000); + expect(resolveEffectiveTimeoutMs(45, 60_000)).toBe(45_000); + }); +}); diff --git a/src/lib/ai/effective-timeout.ts b/src/lib/ai/effective-timeout.ts new file mode 100644 index 000000000..1055c558e --- /dev/null +++ b/src/lib/ai/effective-timeout.ts @@ -0,0 +1,25 @@ +/** + * Resolve the upstream provider timeout (ms) for a non-chat generation surface. + * + * The per-user `User.aiResponseTimeoutSeconds` setting (Settings → AI) is the + * operator's lever for slow self-hosted / local backends whose first request + * loads the model. Coach already threads it onto its provider call; the + * generated-narrative surfaces (daily briefing, status cards, period + * narratives, …) historically hardcoded their per-surface `AI_BUDGETS.*` + * default and ignored the setting, so raising the timeout had no effect on + * them. This helper is the one place that converts the stored seconds into the + * milliseconds a `CompletionParams.timeoutMs` expects, falling back to the + * surface's budget default when the user has not set a value. + * + * Semantics mirror Coach: a positive stored value wins (seconds → ms), and any + * unset / non-positive value yields the surface default. The setting is bounded + * to 10–600 s at write-time, so no read-time clamp is needed here. + */ +export function resolveEffectiveTimeoutMs( + aiResponseTimeoutSeconds: number | null | undefined, + budgetDefaultMs: number, +): number { + return aiResponseTimeoutSeconds != null && aiResponseTimeoutSeconds > 0 + ? aiResponseTimeoutSeconds * 1000 + : budgetDefaultMs; +} diff --git a/src/lib/insights/__tests__/briefing-failure-marker.test.ts b/src/lib/insights/__tests__/briefing-failure-marker.test.ts new file mode 100644 index 000000000..e50ff58d3 --- /dev/null +++ b/src/lib/insights/__tests__/briefing-failure-marker.test.ts @@ -0,0 +1,92 @@ +/** + * v1.25 — the briefing failure marker drives the read path's "couldn't + * refresh" signal WITHOUT touching the cached briefing text. These tests pin + * that a marker newer than the last successful generation reads as a failure, + * an older one is treated as superseded, and a write failure never throws. + */ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +const findFirst = vi.fn(); +const create = vi.fn(); + +vi.mock("@/lib/db", () => ({ + prisma: { + auditLog: { + findFirst: (...a: unknown[]) => findFirst(...a), + create: (...a: unknown[]) => create(...a), + }, + }, +})); + +import { + readBriefingFailure, + recordBriefingFailure, + BRIEFING_FAILURE_ACTION, +} from "../briefing-failure-marker"; + +beforeEach(() => { + vi.clearAllMocks(); +}); + +describe("readBriefingFailure", () => { + it("returns the failure when the marker is newer than the last success", async () => { + findFirst.mockResolvedValue({ + createdAt: new Date("2026-06-28T10:00:00Z"), + details: JSON.stringify({ reason: "provider-error" }), + }); + + const result = await readBriefingFailure({ + userId: "u1", + since: new Date("2026-06-28T09:00:00Z"), + }); + + expect(result).toEqual( + expect.objectContaining({ reason: "provider-error" }), + ); + }); + + it("treats a marker older than the last success as superseded", async () => { + findFirst.mockResolvedValue({ + createdAt: new Date("2026-06-28T08:00:00Z"), + details: JSON.stringify({ reason: "provider-error" }), + }); + + const result = await readBriefingFailure({ + userId: "u1", + since: new Date("2026-06-28T09:00:00Z"), + }); + + expect(result).toBeNull(); + }); + + it("returns null when no marker exists", async () => { + findFirst.mockResolvedValue(null); + expect(await readBriefingFailure({ userId: "u1", since: null })).toBeNull(); + }); +}); + +describe("recordBriefingFailure", () => { + it("writes an append-only marker row under the failure action", async () => { + create.mockResolvedValue({}); + await recordBriefingFailure({ + userId: "u1", + reason: "timeout", + locale: "en", + }); + expect(create).toHaveBeenCalledWith( + expect.objectContaining({ + data: expect.objectContaining({ + userId: "u1", + action: BRIEFING_FAILURE_ACTION, + }), + }), + ); + }); + + it("never throws when the write fails", async () => { + create.mockRejectedValue(new Error("db down")); + await expect( + recordBriefingFailure({ userId: "u1", reason: "timeout" }), + ).resolves.toBeUndefined(); + }); +}); diff --git a/src/lib/insights/__tests__/comprehensive-timeout-honored.test.ts b/src/lib/insights/__tests__/comprehensive-timeout-honored.test.ts new file mode 100644 index 000000000..6da5f93a6 --- /dev/null +++ b/src/lib/insights/__tests__/comprehensive-timeout-honored.test.ts @@ -0,0 +1,140 @@ +/** + * v1.25 — the comprehensive briefing generator honours the per-user + * `aiResponseTimeoutSeconds` setting and keeps its last good text on failure. + * + * Before this, every generation call hardcoded `AI_BUDGETS.comprehensive + * .timeoutMs`, so raising the timeout for a slow self-hosted backend had no + * effect on the briefing. These tests pin: a set value is threaded onto the + * provider call (seconds → ms); an unset value falls back to the budget; and a + * terminal failure writes NO cache row (last good preserved) while recording a + * failure marker the read path can surface. + */ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { AI_BUDGETS } from "@/lib/ai/ai-budgets"; + +const findUnique = vi.fn(); +const userUpdate = vi.fn(); +const resolveProviderChain = vi.fn(); +const resolveProvider = vi.fn(); +const runRawCompletionWithFallback = vi.fn(); +const extractFeatures = vi.fn(); +const recordBriefingFailure = vi.fn(); + +vi.mock("@/lib/db", () => ({ + prisma: { + user: { + findUnique: (...a: unknown[]) => findUnique(...a), + update: (...a: unknown[]) => userUpdate(...a), + }, + auditLog: { deleteMany: vi.fn(), create: vi.fn() }, + }, +})); +vi.mock("@/lib/ai/provider", () => ({ + resolveProviderChain: (...a: unknown[]) => resolveProviderChain(...a), + resolveProvider: (...a: unknown[]) => resolveProvider(...a), +})); +vi.mock("@/lib/ai/provider-runner", () => ({ + AllProvidersFailedError: class extends Error {}, + runRawCompletionWithFallback: (...a: unknown[]) => + runRawCompletionWithFallback(...a), +})); +vi.mock("@/lib/insights/features", () => ({ + FeaturesPayloadTooLargeError: class extends Error { + sizeBytes = 0; + }, + extractFeatures: (...a: unknown[]) => extractFeatures(...a), + BRIEFING_FEATURE_WINDOW_DAYS: 400, +})); +vi.mock("@/lib/insights/illness-cycle-briefing", () => ({ + buildBriefingIllnessCycleContext: vi.fn().mockResolvedValue(null), + buildBriefingIllnessCyclePrompt: vi.fn().mockReturnValue(""), +})); +vi.mock("@/lib/insights/glp1-plateau", () => ({ + detectGlp1Plateau: vi.fn(async () => null), + buildGlp1PlateauPrompt: vi.fn(() => ""), +})); +vi.mock("@/lib/ai/coach/about-me", () => ({ + getSelfContextTextForUser: vi.fn(async () => null), + buildAboutMeInsightBlock: vi.fn(() => ""), +})); +vi.mock("@/lib/cache/invalidate", () => ({ + invalidateUserInsights: vi.fn(), +})); +vi.mock("@/lib/insights/briefing-failure-marker", () => ({ + recordBriefingFailure: (...a: unknown[]) => recordBriefingFailure(...a), +})); + +import { generateComprehensiveInsight } from "../comprehensive-generate"; + +const FEATURES = { weight: { count: 12, latest: 81.4, mean30: 82.1 } }; +const VALID = JSON.stringify({ dailyBriefing: { paragraph: "ok" } }); + +function mockUser(overrides: Record = {}) { + findUnique.mockResolvedValue({ + insightsPrivacyMode: "aggregated", + insightsCachedAt: null, + insightsCachedText: null, + insightsExcludeMetrics: [], + insightsSnapshotHash: null, + insightsBriefingRerollDate: null, + aiResponseTimeoutSeconds: null, + ...overrides, + }); +} + +beforeEach(() => { + vi.clearAllMocks(); + resolveProviderChain.mockResolvedValue([ + { providerType: "openai", instance: {} }, + ]); + resolveProvider.mockResolvedValue({ type: "none" }); + extractFeatures.mockResolvedValue(FEATURES); + userUpdate.mockResolvedValue({}); + mockUser(); +}); + +describe("comprehensive timeout honouring", () => { + it("threads the per-user timeout (seconds → ms) onto the provider call", async () => { + mockUser({ aiResponseTimeoutSeconds: 300 }); + runRawCompletionWithFallback.mockResolvedValue({ + result: { content: VALID, tokensUsed: 10, model: "m" }, + workingProvider: { providerType: "openai" }, + fallbackHops: [], + }); + + await generateComprehensiveInsight("u1", { locale: "de" }); + + const params = runRawCompletionWithFallback.mock.calls[0][0].params; + expect(params.timeoutMs).toBe(300_000); + expect(params.timeoutMs).not.toBe(AI_BUDGETS.comprehensive.timeoutMs); + }); + + it("falls back to the comprehensive budget when the setting is unset", async () => { + mockUser({ aiResponseTimeoutSeconds: null }); + runRawCompletionWithFallback.mockResolvedValue({ + result: { content: VALID, tokensUsed: 10, model: "m" }, + workingProvider: { providerType: "openai" }, + fallbackHops: [], + }); + + await generateComprehensiveInsight("u1", { locale: "de" }); + + const params = runRawCompletionWithFallback.mock.calls[0][0].params; + expect(params.timeoutMs).toBe(AI_BUDGETS.comprehensive.timeoutMs); + }); + + it("keeps the last good text and records a failure marker on a provider error", async () => { + mockUser({ aiResponseTimeoutSeconds: 300 }); + runRawCompletionWithFallback.mockRejectedValue(new Error("boom")); + + const outcome = await generateComprehensiveInsight("u1", { locale: "de" }); + + expect(outcome.status).toBe("failed"); + // No cache row written → the prior `insightsCachedText` (last good) survives. + expect(userUpdate).not.toHaveBeenCalled(); + // A dated failure marker is recorded so the read path can surface a hint. + expect(recordBriefingFailure).toHaveBeenCalledWith( + expect.objectContaining({ userId: "u1", locale: "de" }), + ); + }); +}); diff --git a/src/lib/insights/briefing-failure-marker.ts b/src/lib/insights/briefing-failure-marker.ts new file mode 100644 index 000000000..ad11c863d --- /dev/null +++ b/src/lib/insights/briefing-failure-marker.ts @@ -0,0 +1,89 @@ +import { prisma } from "@/lib/db"; + +/** + * Dated failure marker for the daily-briefing generation. + * + * The briefing keeps its last good payload on a failed refresh: the generator + * writes NO `User.insightsCachedText` row when a provider times out or errors, + * so the previous briefing stays intact and the surface never blanks. What was + * missing is a signal the READ path can use to tell the difference between "no + * refresh was due" and "the last refresh attempt failed" — without that, a + * stale-but-shown briefing reads as current and a never-generated one shows a + * generic empty state instead of an honest "couldn't generate". + * + * This records an append-only `auditLog` row (no migration, no overwrite of the + * cached text) carrying the calendar day, the reason, and the locale. The read + * path compares the latest marker's timestamp against the last successful + * generation (`User.insightsCachedAt`): a marker newer than the last success + * means the most recent attempt failed. + */ +export const BRIEFING_FAILURE_ACTION = "insights.briefing-failure"; + +/** UTC YYYY-MM-DD calendar-day key for the marker payload. */ +function dateKey(at: Date = new Date()): string { + return at.toISOString().slice(0, 10); +} + +/** + * Append a failure marker for `userId`. Best-effort: a write failure here must + * never propagate out of the generation's own failure path. + */ +export async function recordBriefingFailure(args: { + userId: string; + reason: string; + locale?: string; +}): Promise { + try { + await prisma.auditLog.create({ + data: { + userId: args.userId, + action: BRIEFING_FAILURE_ACTION, + details: JSON.stringify({ + dateKey: dateKey(), + reason: args.reason, + locale: args.locale ?? null, + triedAt: new Date().toISOString(), + }), + }, + }); + } catch { + // Marker is best-effort — never throw out of the failure path. + } +} + +export interface BriefingFailureState { + triedAt: string; + reason: string; +} + +/** + * Read the latest briefing-failure marker for `userId`, but only when it is + * newer than the last successful generation (`since`). Returns `null` when the + * most recent attempt succeeded (or there is no marker at all), so the read + * path can render a discreet "couldn't refresh" hint exactly when it applies. + */ +export async function readBriefingFailure(args: { + userId: string; + since: Date | null; +}): Promise { + const latest = await prisma.auditLog.findFirst({ + where: { userId: args.userId, action: BRIEFING_FAILURE_ACTION }, + orderBy: { createdAt: "desc" }, + select: { createdAt: true, details: true }, + }); + if (!latest) return null; + // A failure older than (or equal to) the last successful generation has been + // superseded — the surface is showing fresh text, not a held one. + if (args.since && latest.createdAt <= args.since) return null; + + let reason = "unknown"; + if (latest.details) { + try { + const parsed = JSON.parse(latest.details) as { reason?: unknown }; + if (typeof parsed.reason === "string") reason = parsed.reason; + } catch { + // Malformed marker payload — still report the failure, generic reason. + } + } + return { triedAt: latest.createdAt.toISOString(), reason }; +} diff --git a/src/lib/insights/comprehensive-generate.ts b/src/lib/insights/comprehensive-generate.ts index eb697390f..60cbca84b 100644 --- a/src/lib/insights/comprehensive-generate.ts +++ b/src/lib/insights/comprehensive-generate.ts @@ -93,6 +93,8 @@ import { } from "@/lib/insights/metric-status-registry"; import { annotate } from "@/lib/logging/context"; import { AI_BUDGETS } from "@/lib/ai/ai-budgets"; +import { resolveEffectiveTimeoutMs } from "@/lib/ai/effective-timeout"; +import { recordBriefingFailure } from "@/lib/insights/briefing-failure-marker"; const CACHE_TTL_MS = 24 * 60 * 60 * 1000; @@ -200,6 +202,13 @@ async function rerollBriefingParagraph(args: { * care / workouts) the reroll may now cite. */ features?: AggregatedFeatures | null; + /** + * The upstream provider timeout for this re-roll, resolved from the user's + * response-timeout setting (falling back to the comprehensive budget). The + * caller resolves it once and threads it in so the re-roll honours the same + * slow-backend allowance the main generation does. + */ + effectiveTimeoutMs: number; }): Promise<{ text: string; providerType: string } | null> { let cached: Record; try { @@ -232,7 +241,8 @@ async function rerollBriefingParagraph(args: { maxTokens: AI_BUDGETS.comprehensive.maxTokens, // v1.21.5 — wider upstream budget so the reasoning-heavy briefing // generation is not clipped at the client's 60 s default mid-stream. - timeoutMs: AI_BUDGETS.comprehensive.timeoutMs, + // v1.25 — honours the per-user response-timeout setting (see caller). + timeoutMs: args.effectiveTimeoutMs, responseFormat: "json", }), }); @@ -730,9 +740,22 @@ export async function generateComprehensiveInsight( gender: true, // v1.22 (W6) — first name for the sparse, hash-gated briefing opener. displayName: true, + // v1.25 — per-user response-timeout (seconds). Threaded onto every + // provider call below so a raised value for a slow self-hosted backend + // actually applies to the briefing, not just to Coach. + aiResponseTimeoutSeconds: true, }, }); + // Resolve the upstream provider timeout once from the user's setting, + // falling back to the comprehensive budget. Every generation call in this + // function (main, JSON retry, grounding retry, and the paragraph re-roll) + // uses it so the whole briefing path honours the operator's allowance. + const effectiveTimeoutMs = resolveEffectiveTimeoutMs( + dbUser?.aiResponseTimeoutSeconds, + AI_BUDGETS.comprehensive.timeoutMs, + ); + if ( !force && dbUser?.insightsCachedAt && @@ -930,6 +953,7 @@ export async function generateComprehensiveInsight( locale, signals: features.signalsOfDay ?? null, features, + effectiveTimeoutMs, }); if (rerolled) { await prisma.user.update({ @@ -986,7 +1010,8 @@ export async function generateComprehensiveInsight( maxTokens: AI_BUDGETS.comprehensive.maxTokens, // v1.21.5 — wider upstream budget so the reasoning-heavy briefing // generation is not clipped at the client's 60 s default mid-stream. - timeoutMs: AI_BUDGETS.comprehensive.timeoutMs, + // v1.25 — honours the per-user response-timeout setting. + timeoutMs: effectiveTimeoutMs, // v1.18.7 — structured surface: opt the non-OpenAI chains into their // strongest JSON mode (Ollama `format`, Anthropic `{` prefill) so a // first-pass JSON miss is rarer; stripJsonFences stays the net. @@ -1019,6 +1044,11 @@ export async function generateComprehensiveInsight( provider_message: err.message?.slice(0, 240) ?? null, }, }); + // v1.25 — record a dated failure marker so the read path can surface a + // discreet "couldn't refresh" hint alongside the preserved last-good + // briefing. No cache row is written here, so `insightsCachedText` (the + // last good payload) stays intact and the surface never blanks. + void recordBriefingFailure({ userId, reason, locale }); return { status: "failed", reason }; } @@ -1050,17 +1080,20 @@ export async function generateComprehensiveInsight( temperature: AI_BUDGETS.comprehensive.temperature, maxTokens: AI_BUDGETS.comprehensive.maxTokens, // v1.21.5 — wider upstream budget; see the first generation call. - timeoutMs: AI_BUDGETS.comprehensive.timeoutMs, + // v1.25 — honours the per-user response-timeout setting. + timeoutMs: effectiveTimeoutMs, responseFormat: "json", }), }); result = retry.result; workingProviderType = retry.workingProvider.providerType; } catch { + void recordBriefingFailure({ userId, reason: "invalid-json", locale }); return { status: "failed", reason: "invalid-json" }; } insights = parseComprehensiveResult(result.content); if (insights === null) { + void recordBriefingFailure({ userId, reason: "invalid-json", locale }); return { status: "failed", reason: "invalid-json" }; } } @@ -1092,7 +1125,8 @@ export async function generateComprehensiveInsight( temperature: AI_BUDGETS.comprehensive.temperature, maxTokens: AI_BUDGETS.comprehensive.maxTokens, // v1.21.5 — wider upstream budget; see the first generation call. - timeoutMs: AI_BUDGETS.comprehensive.timeoutMs, + // v1.25 — honours the per-user response-timeout setting. + timeoutMs: effectiveTimeoutMs, responseFormat: "json", }), }); diff --git a/src/lib/insights/status-provider.ts b/src/lib/insights/status-provider.ts index cb1015677..107a48466 100644 --- a/src/lib/insights/status-provider.ts +++ b/src/lib/insights/status-provider.ts @@ -13,6 +13,8 @@ import { annotate } from "@/lib/logging/context"; import { AI_BUDGETS, REFERENCE_AI_SEED } from "@/lib/ai/ai-budgets"; import { singleUserTurn } from "@/lib/ai/types"; import { STATUS_PROVIDER_TIMEOUT_MS, withTimeout } from "./with-timeout"; +import { prisma } from "@/lib/db"; +import { resolveEffectiveTimeoutMs } from "@/lib/ai/effective-timeout"; /** * Shared provider plumbing for the seven `*-status.ts` generators. @@ -134,8 +136,9 @@ export async function statusConsentBlocksGeneration( } /** - * Run a status generation across the user's provider chain, bounded by - * `STATUS_PROVIDER_TIMEOUT_MS`. The result discriminates between + * Run a status generation across the user's provider chain, bounded by the + * per-user response-timeout setting (falling back to `STATUS_PROVIDER_TIMEOUT_MS` + * when unset). The result discriminates between * no-provider / timeout / provider-error / success so the caller can * decide what to persist — only `ok` is ever cached as the day's * assessment. @@ -168,6 +171,24 @@ export async function runStatusCompletion( return { kind: "none" }; } + // Honour the per-user response-timeout setting the operator dials in for a + // slow self-hosted / local backend (Settings → AI). This is the single + // chokepoint every status / reference surface funnels through — per-metric + // cards, the batched assessment, the derived assessments, and the period + // narratives — so resolving it here threads the setting onto all of them at + // once. A positive stored value wins (seconds → ms); unset falls back to the + // status-path budget. Applied to BOTH the upstream call's own `timeoutMs` + // and the outer `withTimeout` cap so a raised value is not silently clipped + // by the 60 s wall-clock that previously bounded the path. + const settingsRow = await prisma.user.findUnique({ + where: { id: userId }, + select: { aiResponseTimeoutSeconds: true }, + }); + const effectiveTimeoutMs = resolveEffectiveTimeoutMs( + settingsRow?.aiResponseTimeoutSeconds, + STATUS_PROVIDER_TIMEOUT_MS, + ); + const raced = await withTimeout( () => runRawCompletionWithFallback({ @@ -184,9 +205,10 @@ export async function runStatusCompletion( // Status cards are JSON by default; the narrative opts out via // `"text"`. responseFormat: args.responseFormat === "text" ? undefined : "json", + timeoutMs: effectiveTimeoutMs, }), }), - STATUS_PROVIDER_TIMEOUT_MS, + effectiveTimeoutMs, null, ); From 423aecc695b2164cc78891377d65400fee37b72b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sun, 28 Jun 2026 12:42:43 +0200 Subject: [PATCH 2/2] feat(insights): add a retry affordance for failed briefing generation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Surface the generation-failure signal in the daily briefing card. When a held briefing is shown but the last refresh failed, the footer states it plainly and offers a retry that runs the existing explicit regenerate — no warm-on-mount. When there is no briefing to fall back on, the empty state reads "couldn't generate" with the same retry instead of the generic "no briefing yet". The hint is suppressed when no AI provider is configured, where that hint already owns the surface and a retry is futile. New copy lands in all six locales. --- messages/de.json | 6 +- messages/en.json | 6 +- messages/es.json | 6 +- messages/fr.json | 6 +- messages/it.json | 6 +- messages/pl.json | 6 +- src/app/insights/page.tsx | 6 ++ .../__tests__/daily-briefing.test.tsx | 51 +++++++++++++++++ src/components/insights/daily-briefing.tsx | 57 +++++++++++++++++-- .../insights/use-insights-advisor.ts | 19 +++++++ 10 files changed, 159 insertions(+), 10 deletions(-) diff --git a/messages/de.json b/messages/de.json index ce28bb09c..7c58dd950 100644 --- a/messages/de.json +++ b/messages/de.json @@ -2037,7 +2037,11 @@ "noProviderTitle": "Kein KI-Anbieter verbunden", "noProviderDescription": "Das Tagesbriefing fasst deine Werte in Klartext zusammen. Dafür braucht HealthLog einen KI-Anbieter — nutze deinen eigenen Schlüssel (OpenAI, Anthropic) oder ein lokales Modell. Deine Daten bleiben dabei bei dir.", "noProviderAction": "Zu den KI-Einstellungen", - "staleNoProviderHint": "Dieses Briefing kann nicht aktualisiert werden — es ist kein KI-Anbieter verbunden." + "staleNoProviderHint": "Dieses Briefing kann nicht aktualisiert werden — es ist kein KI-Anbieter verbunden.", + "refreshFailedHint": "Briefing konnte nicht aktualisiert werden — es wird das letzte angezeigt.", + "retryAction": "Erneut versuchen", + "failedTitle": "Briefing konnte nicht erstellt werden", + "failedDescription": "Der KI-Anbieter hat das Zeitlimit überschritten oder einen Fehler zurückgegeben. Versuche es erneut oder erhöhe in den KI-Einstellungen das Antwort-Zeitlimit für ein langsames lokales Modell." }, "trendsRow": { "title": "Trends", diff --git a/messages/en.json b/messages/en.json index 14fa8b085..05c423d34 100644 --- a/messages/en.json +++ b/messages/en.json @@ -2037,7 +2037,11 @@ "noProviderTitle": "No AI provider connected", "noProviderDescription": "The daily briefing turns your readings into plain-language summary. HealthLog needs an AI provider for that — use your own key (OpenAI, Anthropic) or a local model. Your data stays with you.", "noProviderAction": "Open AI settings", - "staleNoProviderHint": "This briefing can't refresh — no AI provider is connected." + "staleNoProviderHint": "This briefing can't refresh — no AI provider is connected.", + "refreshFailedHint": "Couldn't refresh the briefing — showing the last one.", + "retryAction": "Retry", + "failedTitle": "Couldn't generate the briefing", + "failedDescription": "The AI provider timed out or returned an error. Try again, or raise the response timeout in AI settings for a slow local model." }, "trendsRow": { "title": "Trends", diff --git a/messages/es.json b/messages/es.json index ce2ee6de9..8d631f350 100644 --- a/messages/es.json +++ b/messages/es.json @@ -2037,7 +2037,11 @@ "noProviderTitle": "Ningún proveedor de IA conectado", "noProviderDescription": "El briefing diario resume tus lecturas en lenguaje claro. HealthLog necesita un proveedor de IA para ello: usa tu propia clave (OpenAI, Anthropic) o un modelo local. Tus datos siguen siendo tuyos.", "noProviderAction": "Abrir ajustes de IA", - "staleNoProviderHint": "Este briefing no puede actualizarse: no hay ningún proveedor de IA conectado." + "staleNoProviderHint": "Este briefing no puede actualizarse: no hay ningún proveedor de IA conectado.", + "refreshFailedHint": "No se pudo actualizar el briefing: se muestra el anterior.", + "retryAction": "Reintentar", + "failedTitle": "No se pudo generar el briefing", + "failedDescription": "El proveedor de IA agotó el tiempo de espera o devolvió un error. Inténtalo de nuevo o aumenta el tiempo de espera de respuesta en los ajustes de IA para un modelo local lento." }, "trendsRow": { "title": "Tendencias", diff --git a/messages/fr.json b/messages/fr.json index 69f085b74..f236c24bd 100644 --- a/messages/fr.json +++ b/messages/fr.json @@ -2037,7 +2037,11 @@ "noProviderTitle": "Aucun fournisseur d'IA connecté", "noProviderDescription": "Le briefing quotidien résume tes mesures en langage clair. HealthLog a besoin d'un fournisseur d'IA pour cela : utilise ta propre clé (OpenAI, Anthropic) ou un modèle local. Tes données restent chez toi.", "noProviderAction": "Ouvrir les réglages IA", - "staleNoProviderHint": "Ce briefing ne peut pas être actualisé — aucun fournisseur d'IA n'est connecté." + "staleNoProviderHint": "Ce briefing ne peut pas être actualisé — aucun fournisseur d'IA n'est connecté.", + "refreshFailedHint": "Impossible d'actualiser le briefing — affichage du précédent.", + "retryAction": "Réessayer", + "failedTitle": "Impossible de générer le briefing", + "failedDescription": "Le fournisseur d'IA a dépassé le délai ou renvoyé une erreur. Réessayez ou augmentez le délai de réponse dans les paramètres d'IA pour un modèle local lent." }, "trendsRow": { "title": "Tendances", diff --git a/messages/it.json b/messages/it.json index 60e5288d4..b5bdb8722 100644 --- a/messages/it.json +++ b/messages/it.json @@ -2037,7 +2037,11 @@ "noProviderTitle": "Nessun provider IA collegato", "noProviderDescription": "Il briefing giornaliero riassume le tue misurazioni in linguaggio chiaro. HealthLog ha bisogno di un provider IA per questo: usa la tua chiave (OpenAI, Anthropic) o un modello locale. I tuoi dati restano tuoi.", "noProviderAction": "Apri impostazioni IA", - "staleNoProviderHint": "Questo briefing non può aggiornarsi: nessun provider IA è collegato." + "staleNoProviderHint": "Questo briefing non può aggiornarsi: nessun provider IA è collegato.", + "refreshFailedHint": "Impossibile aggiornare il briefing: viene mostrato l'ultimo.", + "retryAction": "Riprova", + "failedTitle": "Impossibile generare il briefing", + "failedDescription": "Il provider IA ha superato il tempo limite o ha restituito un errore. Riprova oppure aumenta il timeout di risposta nelle impostazioni IA per un modello locale lento." }, "trendsRow": { "title": "Tendenze", diff --git a/messages/pl.json b/messages/pl.json index 5631b6f90..18f0c7064 100644 --- a/messages/pl.json +++ b/messages/pl.json @@ -2037,7 +2037,11 @@ "noProviderTitle": "Brak połączonego dostawcy AI", "noProviderDescription": "Codzienny briefing podsumowuje Twoje pomiary prostym językiem. HealthLog potrzebuje do tego dostawcy AI — użyj własnego klucza (OpenAI, Anthropic) lub modelu lokalnego. Twoje dane pozostają u Ciebie.", "noProviderAction": "Otwórz ustawienia AI", - "staleNoProviderHint": "Tego briefingu nie można odświeżyć — nie połączono żadnego dostawcy AI." + "staleNoProviderHint": "Tego briefingu nie można odświeżyć — nie połączono żadnego dostawcy AI.", + "refreshFailedHint": "Nie udało się odświeżyć briefingu — pokazujemy poprzedni.", + "retryAction": "Spróbuj ponownie", + "failedTitle": "Nie udało się wygenerować briefingu", + "failedDescription": "Dostawca AI przekroczył limit czasu lub zwrócił błąd. Spróbuj ponownie albo zwiększ limit czasu odpowiedzi w ustawieniach AI dla wolnego modelu lokalnego." }, "trendsRow": { "title": "Trendy", diff --git a/src/app/insights/page.tsx b/src/app/insights/page.tsx index 000d31c73..cb5ce21ee 100644 --- a/src/app/insights/page.tsx +++ b/src/app/insights/page.tsx @@ -447,6 +447,12 @@ export default function InsightsPage() { // briefing is actually rendered (the empty-state path owns the // `noProvider` branch above). noProviderStale={briefingPayload !== null && !advisor.hasProvider} + // v1.25 — the last generation attempt failed. On a held briefing this + // adds a discreet "couldn't refresh — retry" footer hint; on an empty + // card it swaps the generic empty state for an honest "couldn't + // generate" one. Suppressed when no provider is configured (that hint + // owns the surface and a retry would be futile). + generationFailed={advisor.generationFailed && advisor.hasProvider} // v1.21.2 (A4) — server-resolved recall + forward-look off the dashboard // snapshot. Both strings are already localised; null leaves the card's // memory block unrendered. diff --git a/src/components/insights/__tests__/daily-briefing.test.tsx b/src/components/insights/__tests__/daily-briefing.test.tsx index a7fa34620..5b1903f7e 100644 --- a/src/components/insights/__tests__/daily-briefing.test.tsx +++ b/src/components/insights/__tests__/daily-briefing.test.tsx @@ -238,4 +238,55 @@ describe("", () => { expect(html).toContain('data-slot="daily-briefing-findings"'); expect(html).not.toContain('data-slot="daily-briefing-no-provider"'); }); + + // v1.25 — generation-failed affordances. The briefing keeps its last good + // text; the card adds an honest "couldn't refresh" footer hint on a held + // briefing and a "couldn't generate — retry" empty state when there is none. + it("renders the refresh-failed footer hint on a held briefing", () => { + const html = render( + {}} + generationFailed + />, + ); + expect(html).toContain('data-slot="daily-briefing-refresh-failed"'); + // The rendered apostrophe is HTML-escaped; match the unambiguous tail. + expect(html).toContain("refresh the briefing"); + expect(html).toContain('data-slot="daily-briefing-refresh-failed-retry"'); + }); + + it("does NOT render the refresh-failed hint when generation did not fail", () => { + const html = render( + {}} />, + ); + expect(html).not.toContain('data-slot="daily-briefing-refresh-failed"'); + }); + + it("suppresses the refresh-failed hint when no provider is connected", () => { + const html = render( + {}} + generationFailed + noProviderStale + />, + ); + expect(html).not.toContain('data-slot="daily-briefing-refresh-failed"'); + expect(html).toContain('data-slot="daily-briefing-stale-no-provider"'); + }); + + it("renders the 'couldn't generate' empty state with a retry CTA when failed and empty", () => { + const html = render( + {}} + generationFailed + />, + ); + // The rendered apostrophe is HTML-escaped; match the unambiguous tail. + expect(html).toContain("generate the briefing"); + expect(html).toMatch(/data-slot="daily-briefing-empty-cta"/); + expect(html).toContain("Retry"); + }); }); diff --git a/src/components/insights/daily-briefing.tsx b/src/components/insights/daily-briefing.tsx index 91b6142bf..311b761de 100644 --- a/src/components/insights/daily-briefing.tsx +++ b/src/components/insights/daily-briefing.tsx @@ -119,6 +119,14 @@ interface DailyBriefingProps { * the staleness reads as intentional, not as a live report. */ noProviderStale?: boolean; + /** + * v1.25 — the most recent generation attempt failed (provider timeout / + * error). The briefing keeps its last good text, so this never blanks a + * shown card; it only adds an honest "couldn't refresh" hint to the footer + * of a held briefing, and — when there is no briefing to show — swaps the + * generic empty state for a "couldn't generate" one whose CTA retries. + */ + generationFailed?: boolean; /** * Optional slot for a meta control mounted in the card header — the * comparison toggle migrates here from the hero in commit 5. @@ -349,6 +357,7 @@ export function DailyBriefing({ regenerating = false, noProvider = false, noProviderStale = false, + generationFailed = false, metaSlot, memory = null, }: DailyBriefingProps) { @@ -468,7 +477,7 @@ export function DailyBriefing({ )} - {(updatedAt || noProviderStale) && ( + {(updatedAt || noProviderStale || generationFailed) && (
{updatedAt && (

)} + {/* v1.25 — the held briefing is shown, but the last refresh + attempt failed (provider timeout / error). State it plainly + and offer a retry so the staleness reads as held, not + current. Suppressed when no provider is configured (that + hint owns the footer) — a retry would be futile there. */} + {generationFailed && !noProviderStale && ( +

+ + {t("insights.dailyBriefing.refreshFailedHint")} + + {onRegenerate && ( + + )} +

+ )}
)} {/* v1.21.0 (C4 H2) — hand off to the Coach for the whole @@ -542,8 +577,20 @@ export function DailyBriefing({ data-slot="daily-briefing-empty" variant="plain" icon={} - title={t("insights.dailyBriefing.emptyTitle")} - description={t("insights.dailyBriefing.emptyDescription")} + // v1.25 — when the last attempt FAILED and there is no last good + // text to fall back on, say so honestly ("couldn't generate") + // rather than the generic "no briefing yet". The CTA below is the + // same explicit regenerate — it retries the failed generation. + title={ + generationFailed + ? t("insights.dailyBriefing.failedTitle") + : t("insights.dailyBriefing.emptyTitle") + } + description={ + generationFailed + ? t("insights.dailyBriefing.failedDescription") + : t("insights.dailyBriefing.emptyDescription") + } action={ onRegenerate ? ( // v1.4.28 BK-M2 — switch the empty-state CTA from @@ -564,7 +611,9 @@ export function DailyBriefing({ {regenerating ? t("insights.heroRegenerating") - : t("insights.dailyBriefing.emptyAction")} + : generationFailed + ? t("insights.dailyBriefing.retryAction") + : t("insights.dailyBriefing.emptyAction")} ) : null diff --git a/src/components/insights/use-insights-advisor.ts b/src/components/insights/use-insights-advisor.ts index 53deb92cd..38594d6f5 100644 --- a/src/components/insights/use-insights-advisor.ts +++ b/src/components/insights/use-insights-advisor.ts @@ -71,6 +71,15 @@ export interface InsightAdvisorPayload { * predates the field — treated as "provider present" (no hint). */ hasProvider?: boolean; + /** + * v1.25 — true when the most recent generation attempt failed (a failure + * marker newer than the last successful generation). The briefing keeps its + * last good text on failure, so this is the honest signal that a shown + * briefing is held, or — with no last good text — that the empty state + * should read "couldn't generate" with a retry rather than the generic one. + * Absent on a pre-field cached payload → treated as "not failed" (no hint). + */ + generationFailed?: boolean; } /** @@ -279,6 +288,13 @@ export interface UseInsightsAdvisorResult { * or an unsettled query. */ hasProvider: boolean; + /** + * v1.25 — true when the GET reported the last generation attempt failed. + * Pairs a shown-but-held briefing with a discreet "couldn't refresh" hint, + * and swaps the generic empty state for a "couldn't generate — retry" one + * when there is no last good text. Defaults false (no hint) when absent. + */ + generationFailed: boolean; } /** @@ -364,5 +380,8 @@ export function useInsightsAdvisorQuery( // assume a provider is present so a transient unknown never flashes a // false "no provider" hint. hasProvider: query.data?.payload?.hasProvider ?? true, + // Absent (pre-field cached payload or unsettled query) → not failed, so a + // transient unknown never flashes a false "couldn't refresh" hint. + generationFailed: query.data?.payload?.generationFailed ?? false, }; }