diff --git a/src/app/api/insights/generate/route.ts b/src/app/api/insights/generate/route.ts index 731779898..849e7ad85 100644 --- a/src/app/api/insights/generate/route.ts +++ b/src/app/api/insights/generate/route.ts @@ -57,6 +57,7 @@ import { singleUserTurn, type InsightResult, } from "@/lib/ai/types"; +import { AI_BUDGETS } from "@/lib/ai/ai-budgets"; import { resolveProvider, resolveProviderChain } from "@/lib/ai/provider"; import { AllProvidersFailedError, @@ -700,6 +701,10 @@ export const POST = apiHandler((request: NextRequest) => user: userPrompt, temperature: 0.3, maxTokens: 1500, + // 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, // 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", @@ -836,6 +841,8 @@ export const POST = apiHandler((request: NextRequest) => user: `${userPrompt}\n\n${buildBriefingGroundingCorrection(ungrounded)}`, temperature: 0.3, maxTokens: 1500, + // v1.21.5 — wider upstream budget; see the first generation call. + timeoutMs: AI_BUDGETS.comprehensive.timeoutMs, // Parsed with `JSON.parse` below — opt OpenAI / Codex into JSON mode. responseFormat: "json", }), diff --git a/src/lib/ai/__tests__/codex-timeout.test.ts b/src/lib/ai/__tests__/codex-timeout.test.ts new file mode 100644 index 000000000..b0c68750b --- /dev/null +++ b/src/lib/ai/__tests__/codex-timeout.test.ts @@ -0,0 +1,76 @@ +/** + * v1.21.5 — the Codex client honours the caller's per-request timeout + * override (`CompletionParams.timeoutMs`), falling back to the shared 60 s + * default when unset. The comprehensive briefing is the one caller that + * overrides it; the 60 s default was aborting its reasoning-heavy generation + * mid-stream on large accounts, leaving the briefing blank. + */ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +const safeFetch = vi.fn(); +vi.mock("@/lib/safe-fetch", () => ({ + safeFetch: (...a: unknown[]) => safeFetch(...a), +})); + +import { CodexClient } from "../codex-client"; +import { singleUserTurn } from "../types"; + +function sseResponse(): Response { + const encoder = new TextEncoder(); + const events = [ + `event: response.output_item.done\ndata: ${JSON.stringify({ + type: "response.output_item.done", + item: { + type: "message", + role: "assistant", + content: [{ type: "output_text", text: '{"summary":"ok"}' }], + }, + })}\n\n`, + `event: response.completed\ndata: ${JSON.stringify({ + type: "response.completed", + response: { id: "r1", usage: { total_tokens: 10 } }, + })}\n\n`, + ]; + const stream = new ReadableStream({ + start(controller) { + for (const ev of events) controller.enqueue(encoder.encode(ev)); + controller.close(); + }, + }); + return new Response(stream, { + status: 200, + headers: { "content-type": "text/event-stream" }, + }); +} + +function makeClient() { + return new CodexClient({ + accessToken: "t", + accountId: "acct", + onTokenRefresh: vi + .fn() + .mockResolvedValue({ accessToken: "t", accountId: "acct" }), + slugChain: ["gpt-5.5"], + }); +} + +beforeEach(() => { + vi.clearAllMocks(); + safeFetch.mockResolvedValue(sseResponse()); +}); + +describe("CodexClient timeout override", () => { + it("forwards an explicit timeoutMs to safeFetch", async () => { + await makeClient().generateCompletion( + singleUserTurn({ system: "s", user: "u", timeoutMs: 120_000 }), + ); + expect(safeFetch.mock.calls[0][2].timeoutMs).toBe(120_000); + }); + + it("falls back to the 60 s default when no override is set", async () => { + await makeClient().generateCompletion( + singleUserTurn({ system: "s", user: "u" }), + ); + expect(safeFetch.mock.calls[0][2].timeoutMs).toBe(60_000); + }); +}); diff --git a/src/lib/ai/ai-budgets.ts b/src/lib/ai/ai-budgets.ts index c1b4538fe..6716ac5ee 100644 --- a/src/lib/ai/ai-budgets.ts +++ b/src/lib/ai/ai-budgets.ts @@ -24,6 +24,15 @@ export interface AiBudget { * Optional for surfaces whose caller pins its own token const (self-context). */ maxTokens?: number; + /** + * v1.21.5 — optional per-surface upstream timeout (ms) threaded onto the + * provider call via `CompletionParams.timeoutMs`. Omitted → the client's + * shared 60 s default holds. Only the comprehensive briefing sets it: its + * reasoning-heavy generation over the full feature snapshot legitimately + * runs past 60 s on large accounts, and the default abort clipped it + * mid-stream — leaving the briefing permanently blank. + */ + timeoutMs?: number; } /** @@ -43,8 +52,17 @@ export const AI_BUDGETS = { * Comprehensive insight + dailyBriefing — the largest structured payload * (summary + recommendations + dailyBriefing 80-200 words + signals + * keyFindings + trendAnnotations + storyboardAnnotations). 1500 tokens. + * + * v1.21.5 — `timeoutMs` raised to 120 s for THIS surface only. The 60 s + * client default aborted the reasoning-heavy single-turn generation + * mid-stream on large accounts (observed: a 60.7 s "operation aborted due + * to timeout" hop, no upstream status), which left the daily briefing and + * the insights trend narrative that share its cached block blank. The + * generation runs off the request hot path (nightly warm + the on-demand + * background warm), and the explicit regenerate is rate-limited, so the + * wider ceiling is bounded. */ - comprehensive: { temperature: 0.3, maxTokens: 1500 }, + comprehensive: { temperature: 0.3, maxTokens: 1500, timeoutMs: 120_000 }, /** * Per-metric status assessment cards — output is a single diff --git a/src/lib/ai/anthropic-client.ts b/src/lib/ai/anthropic-client.ts index 349db83d2..f2bea274d 100644 --- a/src/lib/ai/anthropic-client.ts +++ b/src/lib/ai/anthropic-client.ts @@ -261,7 +261,12 @@ export class AnthropicClient implements AIProvider { // check so a private/metadata address is rejected (SSRF/rebinding). // v1.20.1 — compose the caller's cancel signal (Coach SSE disconnect) so // a mid-generation abort tears the upstream call down early. - { timeoutMs: 60_000, requirePublicHost: true, signal: params.signal }, + // v1.21.5 — honour the caller's per-request timeout override; default 60 s. + { + timeoutMs: params.timeoutMs ?? 60_000, + requirePublicHost: true, + signal: params.signal, + }, ); if (!res.ok) { diff --git a/src/lib/ai/codex-client.ts b/src/lib/ai/codex-client.ts index 42b31c3c6..9a2232e2b 100644 --- a/src/lib/ai/codex-client.ts +++ b/src/lib/ai/codex-client.ts @@ -509,7 +509,9 @@ export class CodexClient implements AIProvider { }, // v1.20.1 — compose the caller's cancel signal (Coach SSE disconnect) so // a mid-generation abort tears the upstream call down early. - { timeoutMs: 60_000, signal: params.signal }, + // v1.21.5 — honour the caller's per-request timeout override (the + // comprehensive briefing needs >60 s); default unchanged at 60 s. + { timeoutMs: params.timeoutMs ?? 60_000, signal: params.signal }, ); } diff --git a/src/lib/ai/local-client.ts b/src/lib/ai/local-client.ts index eaf30ceff..784e53337 100644 --- a/src/lib/ai/local-client.ts +++ b/src/lib/ai/local-client.ts @@ -128,7 +128,8 @@ export class LocalOpenAICompatibleClient implements AIProvider { // v1.20.1 — compose the caller's cancel signal (Coach SSE disconnect) so // a mid-generation abort tears the upstream call down early. { - timeoutMs: 60_000, + // v1.21.5 — honour the caller's per-request timeout override; default 60 s. + timeoutMs: params.timeoutMs ?? 60_000, requirePublicHost: !allowPrivate, signal: params.signal, }, diff --git a/src/lib/ai/openai-client.ts b/src/lib/ai/openai-client.ts index 26b291296..04355ab25 100644 --- a/src/lib/ai/openai-client.ts +++ b/src/lib/ai/openai-client.ts @@ -92,7 +92,12 @@ export class OpenAIClient implements AIProvider { // address is rejected, closing the SSRF/rebinding surface. // v1.20.1 — compose the caller's cancel signal (Coach SSE disconnect) so // a mid-generation abort tears the upstream call down early. - { timeoutMs: 60_000, requirePublicHost: true, signal: params.signal }, + // v1.21.5 — honour the caller's per-request timeout override; default 60 s. + { + timeoutMs: params.timeoutMs ?? 60_000, + requirePublicHost: true, + signal: params.signal, + }, ); if (!res.ok) { diff --git a/src/lib/ai/types.ts b/src/lib/ai/types.ts index 318becc18..0837fbd01 100644 --- a/src/lib/ai/types.ts +++ b/src/lib/ai/types.ts @@ -280,6 +280,18 @@ export interface CompletionParams { * `safeFetch`. Omitted → behaviour is unchanged (timeout-only). */ signal?: AbortSignal; + /** + * v1.21.5 — per-request upstream timeout, in milliseconds. Each client + * passes this straight to its `safeFetch` call, falling back to the shared + * 60 s default when unset — so every existing caller (Coach, status cards) + * is byte-identical. The comprehensive briefing surface is the one caller + * that overrides it: its reasoning-heavy single-turn generation over the + * full feature snapshot ran past the 60 s default on large accounts and was + * aborted mid-stream ("operation aborted due to timeout"), which left the + * briefing — and the insights trend narrative that reads the same cached + * block — permanently blank. Sized in `AI_BUDGETS.comprehensive.timeoutMs`. + */ + timeoutMs?: number; } export interface CompletionResult { @@ -324,6 +336,8 @@ export function singleUserTurn(p: { toolChoice?: "auto" | "none"; /** v1.20.1 — caller-owned cancel signal threaded to the client fetch. */ signal?: AbortSignal; + /** v1.21.5 — per-request upstream timeout override (ms); see CompletionParams. */ + timeoutMs?: number; }): CompletionParams { const images = p.images ?? []; const documents = p.documents ?? []; @@ -361,6 +375,7 @@ export function singleUserTurn(p: { tools: p.tools, toolChoice: p.toolChoice, signal: p.signal, + timeoutMs: p.timeoutMs, }; } diff --git a/src/lib/insights/__tests__/comprehensive-warm-timeout.test.ts b/src/lib/insights/__tests__/comprehensive-warm-timeout.test.ts new file mode 100644 index 000000000..e90976081 --- /dev/null +++ b/src/lib/insights/__tests__/comprehensive-warm-timeout.test.ts @@ -0,0 +1,148 @@ +/** + * v1.21.5 — comprehensive briefing warm-path robustness. + * + * Two guarantees, both regressions that left the daily briefing (and the + * insights trend narrative that reads the same cached block) permanently + * blank for a large account on a reasoning provider: + * + * 1. The generation call carries the wider per-surface upstream timeout + * (`AI_BUDGETS.comprehensive.timeoutMs`), so a reasoning-heavy single + * turn is no longer aborted at the client's 60 s default mid-stream. + * 2. A provider failure does NOT silently persist an empty block: no cache + * row is written, and the failure is surfaced as a queryable wide-event + * annotation rather than swallowed. + */ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +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 annotate = vi.fn(); + +vi.mock("@/lib/db", () => ({ + prisma: { + user: { + findUnique: (...a: unknown[]) => findUnique(...a), + update: (...a: unknown[]) => userUpdate(...a), + }, + auditLog: { deleteMany: vi.fn() }, + }, +})); +vi.mock("@/lib/ai/provider", () => ({ + resolveProviderChain: (...a: unknown[]) => resolveProviderChain(...a), + resolveProvider: (...a: unknown[]) => resolveProvider(...a), +})); +vi.mock("@/lib/ai/provider-runner", () => { + // Defined inside the factory: the mock is hoisted above the module body, so + // a top-level class reference would hit the temporal dead zone. + class AllProvidersFailedError extends Error { + constructor() { + super("All providers failed"); + this.name = "AllProvidersFailedError"; + } + } + return { + AllProvidersFailedError, + 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/logging/context", () => ({ + annotate: (...a: unknown[]) => annotate(...a), +})); + +import { generateComprehensiveInsight } from "../comprehensive-generate"; +import { AllProvidersFailedError } from "@/lib/ai/provider-runner"; +import { AI_BUDGETS } from "@/lib/ai/ai-budgets"; + +const FEATURES = { weight: { count: 12, latest: 81.4, mean30: 82.1 } }; +const VALID = JSON.stringify({ dailyBriefing: { paragraph: "ok" } }); + +beforeEach(() => { + vi.clearAllMocks(); + resolveProviderChain.mockResolvedValue([ + { providerType: "codex", instance: {} }, + ]); + resolveProvider.mockResolvedValue({ type: "none" }); + extractFeatures.mockResolvedValue(FEATURES); + userUpdate.mockResolvedValue({}); + findUnique.mockResolvedValue({ + insightsPrivacyMode: "aggregated", + insightsCachedAt: null, + insightsCachedText: null, + insightsExcludeMetrics: [], + insightsSnapshotHash: null, + insightsBriefingRerollDate: null, + }); +}); + +describe("comprehensive warm-path timeout + failure observability", () => { + it("threads the wider comprehensive timeout onto the provider call", async () => { + // The budget must carry an override above the 60 s client default. + expect(AI_BUDGETS.comprehensive.timeoutMs).toBeGreaterThan(60_000); + + runRawCompletionWithFallback.mockResolvedValue({ + result: { content: VALID, tokensUsed: 10, model: "m" }, + workingProvider: { providerType: "codex" }, + fallbackHops: [], + }); + + const outcome = await generateComprehensiveInsight("u1", { locale: "de" }); + + expect(outcome).toEqual({ status: "generated", providerType: "codex" }); + const params = runRawCompletionWithFallback.mock.calls[0][0].params; + expect(params.timeoutMs).toBe(AI_BUDGETS.comprehensive.timeoutMs); + }); + + it("does not silently persist an empty block when every provider fails", async () => { + // The mocked class ignores the argument; `[]` satisfies the real type. + runRawCompletionWithFallback.mockRejectedValue( + new AllProvidersFailedError([]), + ); + + const outcome = await generateComprehensiveInsight("u1", { locale: "de" }); + + expect(outcome).toEqual({ + status: "failed", + reason: "all-providers-failed", + }); + // No cache write — the briefing block is never persisted empty, so the + // next warm / visit retries instead of serving a permanent blank. + expect(userUpdate).not.toHaveBeenCalled(); + // The failure is queryable, not swallowed. + const failAnnotation = annotate.mock.calls.find( + (c) => + (c[0] as { action?: { name?: string } }).action?.name === + "insights.generate.comprehensive_failed", + ); + expect(failAnnotation).toBeDefined(); + expect( + (failAnnotation![0] as { meta?: { reason?: string } }).meta?.reason, + ).toBe("all-providers-failed"); + }); +}); diff --git a/src/lib/insights/comprehensive-generate.ts b/src/lib/insights/comprehensive-generate.ts index ab052a27e..8c683c5d7 100644 --- a/src/lib/insights/comprehensive-generate.ts +++ b/src/lib/insights/comprehensive-generate.ts @@ -220,6 +220,9 @@ async function rerollBriefingParagraph(args: { // exactly what we are varying. Higher temperature for prose variety. temperature: BRIEFING_REROLL_TEMPERATURE, 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, responseFormat: "json", }), }); @@ -957,6 +960,9 @@ export async function generateComprehensiveInsight( user: userPrompt, temperature: AI_BUDGETS.comprehensive.temperature, 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.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. @@ -966,10 +972,30 @@ export async function generateComprehensiveInsight( result = fallback.result; workingProviderType = fallback.workingProvider.providerType; } catch (e) { - if (e instanceof AllProvidersFailedError) { - return { status: "failed", reason: "all-providers-failed" }; - } - return { status: "failed", reason: "provider-error" }; + // v1.21.5 — make the failed generation queryable. This path used to return + // a `failed` outcome with NO annotation, so a provider that consistently + // failed (e.g. a codex briefing aborted by the 60 s timeout on a large + // account) left the cached block empty AND emitted nothing the operator + // could grep — the briefing and the insights trend narrative that share + // the block stayed blank with no signal. No cache row is written, so the + // next warm / visit retries; the wider `timeoutMs` above is what lets that + // retry actually land. + const reason = + e instanceof AllProvidersFailedError + ? "all-providers-failed" + : "provider-error"; + const err = e as { httpStatus?: number; message?: string }; + annotate({ + action: { name: "insights.generate.comprehensive_failed" }, + meta: { + locale, + reason, + provider_status: + typeof err.httpStatus === "number" ? err.httpStatus : null, + provider_message: err.message?.slice(0, 240) ?? null, + }, + }); + return { status: "failed", reason }; } // Anthropic + local have no native JSON mode, so a ```json-fenced or @@ -999,6 +1025,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, responseFormat: "json", }), }); @@ -1038,6 +1066,8 @@ export async function generateComprehensiveInsight( user: `${userPrompt}\n\n${buildBriefingGroundingCorrection(ungrounded)}`, 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, responseFormat: "json", }), }); diff --git a/src/lib/jobs/__tests__/insight-pregenerate.test.ts b/src/lib/jobs/__tests__/insight-pregenerate.test.ts index fd60e55a7..1dc34196b 100644 --- a/src/lib/jobs/__tests__/insight-pregenerate.test.ts +++ b/src/lib/jobs/__tests__/insight-pregenerate.test.ts @@ -296,7 +296,9 @@ describe("runInsightPregenerate — outcome tally", () => { observedSignal = opts.signal; setTimeout( () => resolve({ status: "generated", providerType: "x" }), - 120_000, + // v1.21.5 — above COMPREHENSIVE_WARM_TIMEOUT_MS (130 s) so the + // bounded budget fires before this "slow" generation settles. + 200_000, ); }), ); @@ -312,7 +314,7 @@ describe("runInsightPregenerate — outcome tally", () => { statusGenerators, warmGenericMetrics, }); - await vi.advanceTimersByTimeAsync(60_000); + await vi.advanceTimersByTimeAsync(140_000); const result = await promise; // The timeout fired, aborted the still-running generation (so its @@ -787,7 +789,9 @@ describe("forceWarmUser — on-demand single-user warm (v1.8.7.1)", () => { // advance past it so the warm continues. setTimeout( () => resolve({ status: "generated", providerType: "x" }), - 120_000, + // v1.21.5 — above COMPREHENSIVE_WARM_TIMEOUT_MS (130 s) so the + // bounded budget fires before this "slow" generation settles. + 200_000, ); }), ); @@ -804,7 +808,7 @@ describe("forceWarmUser — on-demand single-user warm (v1.8.7.1)", () => { warmGenericMetrics, }); // Advance past the comprehensive budget so withTimeout fires. - await vi.advanceTimersByTimeAsync(60_000); + await vi.advanceTimersByTimeAsync(140_000); const result = await promise; for (const g of statusGenerators) { @@ -902,7 +906,9 @@ describe("forceWarmUser — on-demand single-user warm (v1.8.7.1)", () => { observedSignal = opts.signal; setTimeout( () => resolve({ status: "generated", providerType: "x" }), - 120_000, + // v1.21.5 — above COMPREHENSIVE_WARM_TIMEOUT_MS (130 s) so the + // bounded budget fires before this "slow" generation settles. + 200_000, ); }), ); @@ -918,7 +924,7 @@ describe("forceWarmUser — on-demand single-user warm (v1.8.7.1)", () => { statusGenerators, warmGenericMetrics, }); - await vi.advanceTimersByTimeAsync(60_000); + await vi.advanceTimersByTimeAsync(140_000); const result = await promise; // The timeout fired and aborted the still-running generation. diff --git a/src/lib/jobs/insight-pregenerate.ts b/src/lib/jobs/insight-pregenerate.ts index bb878ef4d..ed91ac1d3 100644 --- a/src/lib/jobs/insight-pregenerate.ts +++ b/src/lib/jobs/insight-pregenerate.ts @@ -156,8 +156,16 @@ const WARM_PASS_CONCURRENCY = 3; * loop the cap also bounds head-of-line blocking: without it one stalled * provider pinned the whole batch behind a single candidate. Set a hair * below the queue's own retry horizon so a single tick stays bounded. + * + * v1.21.5 — raised from 45 s. The old cap sat BELOW the provider call's own + * timeout (`AI_BUDGETS.comprehensive.timeoutMs`, now 120 s), so on a large + * account the warm bound — not the provider — aborted the generation, and the + * briefing never landed. The cap must sit just above the provider timeout so + * the provider's own abort governs a genuine completion while this stays the + * backstop against a wedged call. The per-user budget gate + the WARM_PASS + * concurrency cap still bound total nightly cost. */ -const COMPREHENSIVE_WARM_TIMEOUT_MS = 45_000; +const COMPREHENSIVE_WARM_TIMEOUT_MS = 130_000; /** Per-user result of one forced full warm. */ export interface ForceWarmResult { @@ -540,6 +548,21 @@ export async function runInsightPregenerate( ); if (bounded.timedOut || bounded.errored || bounded.value === null) { result.failed++; + // v1.21.5 — make the warm failure queryable. The nightly loop used to + // bump `failed` with no annotation, so a candidate whose comprehensive + // generation timed out (e.g. the briefing clipped at the old 45 s + // bound) left the cached block empty AND emitted nothing greppable. + annotate({ + action: { name: "insights.pregenerate.comprehensive_failed" }, + meta: { + locale, + cause: bounded.timedOut + ? "timeout" + : bounded.errored + ? "error" + : "null", + }, + }); } else { outcome = bounded.value; switch (outcome.status) {