chore(release): v1.28.30 — the daily briefing stops silently skipping days

The recurring no-briefing day had a chain of causes, closed end to end:

Every failure path in the nightly pregenerate + the comprehensive generator
now annotates a queryable cause (several classes — features-error,
payload-too-large, aborted, generator-failed — previously bumped a counter
with zero trace). A failed nightly warm enqueues one bounded retry ~45
minutes later (singleton per user, existing budgets apply), so a transient
provider hiccup heals before morning. A cached payload that carries NO
briefing no longer satisfies the 24h POST short-circuit nor the
unchanged-content hash gate — a once-stripped briefing could previously
block its own regeneration indefinitely. The card surfaces the omission
reason persistently (grounding strip, generation failure, not attempted)
with a forcing retry.

Documentation corrected against code truth: Withings credentials are
per-user (the documented env path never existed; the dead example entries
and manifest rows are gone, and WITHINGS_WEBHOOK_SECRET is now actually
whitelisted in compose — stock deploys could never deliver it); the classic
Fitbit portal no longer accepts new registrations; the backup-restore
example no longer mints a fresh encryption key and uses pnpm dlx tsx; the
TLS guide describes the real client model (two CA-level SPKI pins,
any-match, leaf renewals need no build); scaling/admin references fixed;
the operator-shared subscription provider is documented.
This commit is contained in:
Marc-André Bombeck committed 2026-07-12 11:08:59 +02:00
1 parent 14a517387e
commit a4feabec81
26 files changed
+1190 -148

No files matched your search

@@ -405,11 +405,16 @@ describe("POST /api/insights/generate — cache write (v1.16.8)", () => {
it("does NOT touch the cache when serving from the 24h DB cache", async () => {
// Cached path: route returns early without touching the LLM or the
// cache write.
// cache write. v1.28.30 — the short-circuit requires the cached
// payload to CARRY a briefing (a briefingless cache is stale for a
// briefing-expecting caller), so the fixture carries one.
vi.mocked(prisma.user.findUnique).mockResolvedValueOnce({
insightsPrivacyMode: "aggregated",
insightsCachedAt: new Date(),
insightsCachedText: JSON.stringify({ changed: "still fresh" }),
insightsCachedText: JSON.stringify({
changed: "still fresh",
dailyBriefing: { paragraph: "ok", keyFindings: [] },
}),
locale: "en",
} as never);
@@ -445,7 +450,10 @@ describe("POST /api/insights/generate — cache write (v1.16.8)", () => {
vi.mocked(prisma.user.findUnique).mockResolvedValueOnce({
insightsPrivacyMode: "aggregated",
insightsCachedAt: new Date(),
insightsCachedText: JSON.stringify({ changed: "still fresh" }),
insightsCachedText: JSON.stringify({
changed: "still fresh",
dailyBriefing: { paragraph: "ok", keyFindings: [] },
}),
locale: "en",
} as never);
@@ -467,6 +475,99 @@ describe("POST /api/insights/generate — cache write (v1.16.8)", () => {
});
});
// v1.28.30 — the "no briefing today" chain: a failed nightly warm left a
// fresh-but-briefingless cache inside the 24 h window, and the POST's
// unconditional short-circuit served it to every regenerate attempt
// (`cached: true` three times in prod, zero generations, a full day
// without a briefing). The short-circuit now only holds when the cached
// payload carries a briefing OR the account has no usable provider.
describe("POST /api/insights/generate — briefingless fresh cache (v1.28.30)", () => {
function mockBriefinglessFreshCache() {
vi.mocked(prisma.user.findUnique).mockResolvedValueOnce({
insightsPrivacyMode: "aggregated",
insightsCachedAt: new Date(Date.now() - 6 * 60 * 60 * 1000),
insightsCachedText: JSON.stringify({
changed: "fresh but no briefing",
dailyBriefing: null,
}),
insightsExcludeMetrics: [],
locale: "en",
} as never);
}
it("POST without force + fresh cache WITH briefing → served cached", async () => {
vi.mocked(prisma.user.findUnique).mockResolvedValueOnce({
insightsPrivacyMode: "aggregated",
insightsCachedAt: new Date(Date.now() - 6 * 60 * 60 * 1000),
insightsCachedText: JSON.stringify({
dailyBriefing: { paragraph: "today", keyFindings: [] },
}),
locale: "en",
} as never);
const res = await POST(jsonRequest({}) as never);
expect(res.status).toBe(200);
const body = (await res.json()) as { data: { cached: boolean } };
expect(body.data.cached).toBe(true);
expect(prisma.user.update).not.toHaveBeenCalled();
});
it("POST without force + fresh cache WITHOUT briefing + provider → regenerates", async () => {
makeWorkingProvider();
mockBriefinglessFreshCache();
const res = await POST(jsonRequest({}) as never);
expect(res.status).toBe(200);
const body = (await res.json()) as { data: { cached: boolean } };
// The stale-for-briefing cache was bypassed: a real generation ran and
// wrote a fresh cache row.
expect(body.data.cached).toBe(false);
expect(prisma.user.update).toHaveBeenCalledTimes(1);
});
it("POST without force + fresh cache WITHOUT briefing + NO provider → serves cached (regeneration is futile)", async () => {
vi.mocked(hasUsableStatusProvider).mockResolvedValueOnce(false);
mockBriefinglessFreshCache();
const res = await POST(jsonRequest({}) as never);
expect(res.status).toBe(200);
const body = (await res.json()) as { data: { cached: boolean } };
expect(body.data.cached).toBe(true);
expect(prisma.user.update).not.toHaveBeenCalled();
});
it("degrades to the cached payload instead of 429 when the briefingless fall-through is rate-limited", async () => {
makeWorkingProvider();
mockBriefinglessFreshCache();
vi.mocked(checkRateLimit).mockResolvedValue({
allowed: false,
remaining: 0,
resetAt: Date.now() + 3600_000,
});
const res = await POST(jsonRequest({}) as never);
// A POST-as-read caller never used to see a 429 on a fresh cache;
// exhausting the quota falls back to the old cached-serve behaviour.
expect(res.status).toBe(200);
const body = (await res.json()) as { data: { cached: boolean } };
expect(body.data.cached).toBe(true);
expect(prisma.user.update).not.toHaveBeenCalled();
});
it("explicit force keeps the honest 429 when rate-limited", async () => {
makeWorkingProvider();
mockBriefinglessFreshCache();
vi.mocked(checkRateLimit).mockResolvedValue({
allowed: false,
remaining: 0,
resetAt: Date.now() + 3600_000,
});
const res = await POST(jsonRequest({ force: true }) as never);
expect(res.status).toBe(429);
});
});
// v1.4.36 QA H1 — when both the raw-mode extraction AND the aggregated
// retry cross the 5 MB ceiling, the route used to throw uncaught → 500.
// It now wraps the retry, downgrades further via the exclude filter,
@@ -619,6 +720,66 @@ describe("GET /api/insights/generate — read-only advisor read", () => {
expect(body.data.revalidating).toBe(false);
});
// v1.28.30 — a briefing the grounding gate withheld used to be visible
// only on the transient POST response; every later read showed a generic
// "no briefing yet". The marker written after the cache write now
// surfaces the omission on the read path, WITHOUT reading as a failure.
it("surfaces briefingOmittedReason from a briefing-ungrounded marker (not as a failure)", async () => {
const cachedAt = new Date(Date.now() - 60 * 60 * 1000);
vi.mocked(prisma.user.findUnique).mockResolvedValueOnce({
insightsCachedAt: cachedAt,
insightsCachedText: JSON.stringify({ dailyBriefing: null }),
locale: "en",
} as never);
// Marker newer than the last successful generation (written right
// after the cache write on the strip path).
vi.mocked(prisma.auditLog.findFirst).mockResolvedValueOnce({
createdAt: new Date(cachedAt.getTime() + 1000),
details: JSON.stringify({ reason: "briefing-ungrounded" }),
} as never);
const res = await (GET as unknown as (req: Request) => Promise<Response>)(
new Request("http://localhost/api/insights/generate"),
);
expect(res.status).toBe(200);
const body = (await res.json()) as {
data: {
briefingOmittedReason: string | null;
generationFailed: boolean;
generationFailureClass: string | null;
};
};
expect(body.data.briefingOmittedReason).toBe("ungrounded");
// The generation SUCCEEDED (the briefing was withheld) — the card must
// render "withheld", not "couldn't generate".
expect(body.data.generationFailed).toBe(false);
expect(body.data.generationFailureClass).toBeNull();
});
it("still reports generationFailed for a genuine failure marker", async () => {
const cachedAt = new Date(Date.now() - 60 * 60 * 1000);
vi.mocked(prisma.user.findUnique).mockResolvedValueOnce({
insightsCachedAt: cachedAt,
insightsCachedText: JSON.stringify({
dailyBriefing: { paragraph: "held", keyFindings: [] },
}),
locale: "en",
} as never);
vi.mocked(prisma.auditLog.findFirst).mockResolvedValueOnce({
createdAt: new Date(cachedAt.getTime() + 1000),
details: JSON.stringify({ reason: "provider-error", httpStatus: 503 }),
} as never);
const res = await (GET as unknown as (req: Request) => Promise<Response>)(
new Request("http://localhost/api/insights/generate"),
);
const body = (await res.json()) as {
data: { briefingOmittedReason: string | null; generationFailed: boolean };
};
expect(body.data.generationFailed).toBe(true);
expect(body.data.briefingOmittedReason).toBeNull();
});
it("reports hasProvider: true on a normal cached read", async () => {
vi.mocked(prisma.user.findUnique).mockResolvedValueOnce({
insightsCachedAt: new Date(),
+93 -10
View File
@@ -81,6 +81,7 @@ import {
buildComparisonSnapshotForUser,
enqueueStatusRefillForUser,
} from "@/lib/insights/comprehensive-generate";
import { cachedPayloadCarriesBriefing } from "@/lib/insights/briefing-payload";
export const dynamic = "force-dynamic";
@@ -168,10 +169,21 @@ export const GET = apiHandler(async (request: NextRequest) => {
userId,
since: cachedAt,
});
const generationFailed = briefingFailure !== null;
// v1.28.30 — a `briefing-ungrounded` marker is NOT a failure: the
// generation succeeded but the grounding gate withheld the briefing
// (marker written after the cache write, so it survives the `since`
// filter). Surface it as the persistent omission signal — the card's
// "briefing withheld" state used to exist only on the transient POST
// response, so a nightly strip read as a silent generic empty card.
const briefingOmittedReason: "ungrounded" | null =
briefingFailure?.reason === "briefing-ungrounded" ? "ungrounded" : null;
const generationFailed =
briefingFailure !== null && briefingOmittedReason === null;
// v1.25.3 — the failure class lets the empty state point its hint at the
// right lever (raise the response timeout vs re-check the provider).
const generationFailureClass = briefingFailure?.failureClass ?? null;
const generationFailureClass = generationFailed
? (briefingFailure?.failureClass ?? null)
: 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
@@ -222,6 +234,9 @@ export const GET = apiHandler(async (request: NextRequest) => {
// v1.25.3 — coarse failure class for the empty-state hint (null when
// the last attempt succeeded).
generationFailureClass,
// v1.28.30 — persistent grounding-omission signal (see above), so a
// nightly-stripped briefing renders "withheld", not "no briefing yet".
briefingOmittedReason,
});
} catch {
// Invalid cache row — fall through to the empty payload below. The
@@ -246,6 +261,8 @@ export const GET = apiHandler(async (request: NextRequest) => {
generationFailed,
// v1.25.3 — coarse failure class for the empty-state hint.
generationFailureClass,
// v1.28.30 — see the cached branch above.
briefingOmittedReason,
});
});
@@ -301,6 +318,24 @@ export const POST = apiHandler((request: NextRequest) =>
if (jsonError) return jsonError;
const forceRefresh = body.force === true;
// v1.28.30 — a fresh cache WITHOUT a briefing must not satisfy a
// briefing-expecting caller: after a failed nightly warm the cached
// payload can sit inside the 24 h window all day, and the old
// unconditional short-circuit served it to every POST (three
// regenerate attempts, three `cached: true` responses, zero
// generations). The short-circuit now holds only when the cached
// payload actually carries a briefing OR the account has no usable
// provider (regenerating would be futile — serve what exists). The
// read-only GET is untouched: it never generates and its out-of-band
// revalidation behaviour stays as-is. When the briefingless fall-
// through is rate-limited below, the cached payload is served after
// all so a POST-as-read client degrades to the old behaviour instead
// of a 429.
let briefinglessCacheFallback: {
cached: unknown;
legacyPayload: boolean;
cachedAt: Date;
} | null = null;
if (
!forceRefresh &&
dbUser?.insightsCachedAt &&
@@ -314,15 +349,33 @@ export const POST = apiHandler((request: NextRequest) =>
// We don't auto-regenerate — that would burn rate-limit tokens
// on a cache-hit silently. User-initiated only.
const legacyPayload = isLegacyInsightPayload(cached);
annotate({
action: { name: "insights.generate" },
meta: { cached: true, legacyPayload },
});
return apiSuccess({
insights: cached,
cached: true,
cachedAt: dbUser.insightsCachedAt,
const carriesBriefing = cachedPayloadCarriesBriefing(
dbUser.insightsCachedText,
);
if (carriesBriefing || !(await hasUsableStatusProvider(userId))) {
annotate({
action: { name: "insights.generate" },
meta: {
cached: true,
legacyPayload,
briefingless: !carriesBriefing,
},
});
return apiSuccess({
insights: cached,
cached: true,
cachedAt: dbUser.insightsCachedAt,
legacyPayload,
});
}
briefinglessCacheFallback = {
cached,
legacyPayload,
cachedAt: dbUser.insightsCachedAt,
};
annotate({
action: { name: "insights.generate.briefingless_cache_bypassed" },
meta: { locale },
});
} catch {
// Invalid cache, regenerate
@@ -346,6 +399,22 @@ export const POST = apiHandler((request: NextRequest) =>
60 * 60 * 1000,
);
if (!rl.allowed) {
// v1.28.30 — a POST-as-read caller whose fresh-but-briefingless cache
// fell through to regeneration degrades to the cached payload when
// the hourly quota is exhausted, instead of surfacing a 429 it never
// used to see. Explicit `force` keeps the honest 429.
if (briefinglessCacheFallback !== null) {
annotate({
action: { name: "insights.generate" },
meta: { cached: true, rate_limited_fallback: true },
});
return apiSuccess({
insights: briefinglessCacheFallback.cached,
cached: true,
cachedAt: briefinglessCacheFallback.cachedAt,
legacyPayload: briefinglessCacheFallback.legacyPayload,
});
}
return apiError(`Maximum ${limit} insight generations per hour.`, 429);
}
@@ -855,6 +924,20 @@ export const POST = apiHandler((request: NextRequest) =>
},
});
// v1.28.30 — persist the grounding omission (marker AFTER the cache
// write so it is newer than `insightsCachedAt`). The transient
// `briefingOmittedReason` on this response only reached the caller that
// forced the regenerate; every later read showed a generic "no briefing
// yet". The dated marker lets the GET surface the same honest
// "briefing withheld" state until the next successful generation.
if (briefingOmittedReason === "ungrounded") {
void recordBriefingFailure({
userId,
reason: "briefing-ungrounded",
locale,
});
}
// v1.16.8 — no blanket per-status eviction here any more (nuking ~45
// cache rows per manual regenerate deleted every hash baseline and was
// the source of the post-regenerate card-regeneration storm). Instead,
@@ -0,0 +1,76 @@
/**
* v1.28.30 — wire-contract for the explicit regenerate.
*
* House rule: generation = nightly cron + explicit button. The button's
* mutation therefore MUST issue `POST { force: true }` — without `force`
* the route's 24 h short-circuit re-serves the cached payload and the
* button reads as doing nothing (one half of the recurring
* "no briefing today" chain). The read path stays a plain GET.
*/
import { describe, it, expect, vi, beforeEach } from "vitest";
const apiFetchRaw = vi.fn();
vi.mock("@/lib/api/api-fetch", () => ({
apiFetchRaw: (...a: unknown[]) => apiFetchRaw(...a),
}));
import { fetchAdvisor } from "../use-insights-advisor";
function jsonResponse(data: unknown): Response {
return {
ok: true,
status: 200,
json: async () => ({ data }),
} as unknown as Response;
}
beforeEach(() => {
vi.clearAllMocks();
apiFetchRaw.mockResolvedValue(
jsonResponse({ insights: { dailyBriefing: null }, cached: false }),
);
});
describe("fetchAdvisor — explicit regenerate forces a generation", () => {
it("POSTs force: true on the regenerate path", async () => {
const result = await fetchAdvisor({ force: true });
expect(apiFetchRaw).toHaveBeenCalledTimes(1);
const [url, init] = apiFetchRaw.mock.calls[0] as [
string,
{ method: string; body?: string },
];
expect(url).toBe("/api/insights/generate");
expect(init.method).toBe("POST");
expect(JSON.parse(init.body ?? "{}")).toEqual({ force: true });
expect(result.outcome).toBe("fresh");
});
it("uses a read-only GET (no body, no force) on the read path", async () => {
await fetchAdvisor();
expect(apiFetchRaw).toHaveBeenCalledTimes(1);
const [url, init] = apiFetchRaw.mock.calls[0] as [
string,
{ method: string; body?: string },
];
expect(url).toBe("/api/insights/generate");
expect(init.method).toBe("GET");
expect(init.body).toBeUndefined();
});
it("lifts briefingOmittedReason through the payload for the card", async () => {
apiFetchRaw.mockResolvedValue(
jsonResponse({
insights: { dailyBriefing: null },
cached: true,
briefingOmittedReason: "ungrounded",
}),
);
const result = await fetchAdvisor();
expect(result.payload?.briefingOmittedReason).toBe("ungrounded");
expect(result.payload?.dailyBriefing).toBeNull();
});
});
@@ -92,8 +92,12 @@ export interface InsightAdvisorPayload {
* v1.28.28 (#470) — set by the POST when the number-grounding gate stripped
* the freshly generated briefing (after its one corrective retry). Without
* it the 200 carried a silently-null briefing and the card's "no briefing
* yet" made the regenerate button read as doing nothing. Only ever present
* on a force-regenerate response; absent on the read path.
* yet" made the regenerate button read as doing nothing.
*
* v1.28.30 — also carried by the read-only GET, backed by a dated
* server-side marker: a briefing the nightly warm stripped now renders
* the same honest "withheld" state on every read until the next
* successful generation, not only on the regenerate response.
*/
briefingOmittedReason?: "ungrounded" | null;
}
@@ -169,7 +173,14 @@ interface AdvisorFetchResult {
outcome: AdvisorFetchOutcome;
}
async function fetchAdvisor(
/**
* Exported as a test seam (like `nextAdvisorPollInterval`): the explicit
* regenerate MUST post `force: true` — generation happens on the nightly
* cron and on explicit intent only, and a force-less button silently
* re-reads the 24 h cache (the "button does nothing" half of the
* no-briefing-today chain). The wire-contract test pins it.
*/
export async function fetchAdvisor(
options: { force?: boolean } = {},
): Promise<AdvisorFetchResult> {
const controller = new AbortController();
@@ -409,9 +420,10 @@ export function useInsightsAdvisorQuery(
generationFailed: query.data?.payload?.generationFailed ?? false,
// Absent → no specific lever hint; the generic failed-description holds.
generationFailureClass: query.data?.payload?.generationFailureClass ?? null,
// v1.28.28 (#470) — only a force-regenerate response carries the field
// (setQueryData writes that response into this cache); the read path's
// GET never does, so the hint clears on the next successful read.
// v1.28.28 (#470) — a force-regenerate response carries the field via
// setQueryData; since v1.28.30 the GET carries it too (marker-backed),
// so the "withheld" state persists across reads and clears on the next
// successful generation.
briefingOmittedReason: query.data?.payload?.briefingOmittedReason ?? null,
};
}
@@ -246,6 +246,49 @@ describe("generateComprehensiveInsight — content-hash gate (v1.16.8)", () => {
expect(runRawCompletionWithFallback).toHaveBeenCalledTimes(1);
});
it("regenerates when the hash matches but the cached payload carries NO briefing (v1.28.30)", async () => {
// A grounding-stripped (or model-omitted) briefing left a briefingless
// payload WITH a stored hash: on unchanged data every warm re-stamped
// the timestamp and the briefing never came back. A briefingless cache
// must not satisfy the unchanged gate.
makeByokChain();
findUnique.mockResolvedValue({
insightsPrivacyMode: "aggregated",
insightsCachedAt: new Date(Date.now() - 26 * 60 * 60 * 1000),
insightsCachedText: JSON.stringify({
dailyBriefing: null,
recommendations: [],
}),
insightsExcludeMetrics: [],
insightsSnapshotHash: FEATURES_HASH,
});
runRawCompletionWithFallback.mockResolvedValue({
result: {
content: JSON.stringify({ dailyBriefing: { p: "recovered" } }),
tokensUsed: 10,
providerType: "openai",
model: "m",
},
workingProvider: { providerType: "openai" },
fallbackHops: [],
});
const outcome = await generateComprehensiveInsight("u1", {
locale: "de",
force: true,
});
expect(outcome).toEqual({ status: "generated", providerType: "openai" });
expect(runRawCompletionWithFallback).toHaveBeenCalledTimes(1);
// A real cache write happened (not a timestamp-only refresh).
const write = userUpdate.mock.calls.find(
(c) =>
(c[0] as { data: Record<string, unknown> }).data.insightsCachedText !==
undefined,
);
expect(write).toBeTruthy();
});
it("does not treat a matching hash as unchanged when there is no cached text to serve", async () => {
makeByokChain();
findUnique.mockResolvedValue({
+26
View File
@@ -0,0 +1,26 @@
/**
* v1.28.30 — does a cached comprehensive payload actually carry a daily
* briefing?
*
* Shared by the generator's content-hash gate and the POST route's 24 h
* short-circuit: a fresh-but-briefingless cache must not satisfy a
* briefing-expecting caller for the rest of the day (the "one silent
* nightly failure + 24 h cache = a full day without a briefing" chain).
* Unparseable text counts as briefingless — regeneration repairs it.
*
* Lives in its own leaf module (no provider-tree imports) so the route
* can consume it without pulling the whole generation pipeline into its
* bundle, and tests that mock `comprehensive-generate` keep the real
* implementation.
*/
export function cachedPayloadCarriesBriefing(
cachedText: string | null | undefined,
): boolean {
if (!cachedText) return false;
try {
const parsed = JSON.parse(cachedText) as Record<string, unknown>;
return parsed.dailyBriefing != null;
} catch {
return false;
}
}
+113 -7
View File
@@ -86,6 +86,7 @@ import { annotate } from "@/lib/logging/context";
import { AI_BUDGETS, resolveInsightsMaxTokens } from "@/lib/ai/ai-budgets";
import { resolveEffectiveTimeoutMs } from "@/lib/ai/effective-timeout";
import { recordBriefingFailure } from "@/lib/insights/briefing-failure-marker";
import { cachedPayloadCarriesBriefing } from "@/lib/insights/briefing-payload";
// Scope invalidation + refill moved to a sibling module; re-exported so
// every existing call site keeps importing from here.
@@ -406,6 +407,35 @@ export async function buildComparisonSnapshotForUser(
return { baseline, metrics };
}
/**
* v1.28.30 — typed `failed` return for the pre-provider stages (feature
* extraction / oversize payload). These paths used to return
* `{ status: "failed" }` with NO annotation and NO failure marker: the
* nightly cron counted them into its `failed` tally silently, and the
* read path could not tell the user the last attempt failed. One helper
* so every failure return is greppable under the same action name and
* leaves a dated marker for the briefing card's honest empty state.
*/
function failBeforeProvider(
userId: string,
locale: "de" | "en",
reason: "payload-too-large" | "features-error" | "aborted",
err?: unknown,
): GenerateOutcome {
const message =
err instanceof Error
? err.message.slice(0, 240)
: err != null
? String(err).slice(0, 240)
: null;
annotate({
action: { name: "insights.generate.comprehensive_failed" },
meta: { locale, reason, provider_status: null, provider_message: message },
});
void recordBriefingFailure({ userId, reason, locale });
return { status: "failed", reason };
}
interface GenerateOptions {
/** Resolved UI locale for the prompt + cache row. */
locale: "de" | "en";
@@ -544,15 +574,20 @@ export async function generateComprehensiveInsight(
aggregated,
MAX_DOWNGRADE_TOKENS,
);
} catch {
return { status: "failed", reason: "payload-too-large" };
} catch (finalErr) {
return failBeforeProvider(
userId,
locale,
"payload-too-large",
finalErr,
);
}
} else {
return { status: "failed", reason: "features-error" };
return failBeforeProvider(userId, locale, "features-error", retryErr);
}
}
} else {
return { status: "failed", reason: "features-error" };
return failBeforeProvider(userId, locale, "features-error", err);
}
}
@@ -651,9 +686,30 @@ export async function generateComprehensiveInsight(
referenceMetrics,
);
// v1.28.30 — the unchanged short-circuit only holds when the cached
// payload actually carries a briefing. A grounding-stripped (or model-
// omitted) briefing left a briefingless payload WITH a stored hash;
// on unchanged data every subsequent warm re-stamped the timestamp and
// the briefing never came back — a permanent silent hole. Treating the
// briefingless cache as "not unchanged" re-runs the full generation
// (bounded by the nightly budget bucket + forced-warm caps).
const cachedCarriesBriefing = cachedPayloadCarriesBriefing(
dbUser?.insightsCachedText,
);
if (
dbUser?.insightsCachedText &&
dbUser.insightsSnapshotHash === snapshotHash
dbUser.insightsSnapshotHash === snapshotHash &&
!cachedCarriesBriefing
) {
annotate({
action: { name: "insights.generate.briefingless_cache_regenerate" },
meta: { locale },
});
}
if (
dbUser?.insightsCachedText &&
dbUser.insightsSnapshotHash === snapshotHash &&
cachedCarriesBriefing
) {
// v1.18.7 (MEDIUM-3) — findings are byte-stable, but re-roll the daily-
// briefing PARAGRAPH once per calendar day so the prose reads fresh
@@ -811,12 +867,41 @@ export async function generateComprehensiveInsight(
});
result = retry.result;
workingProviderType = retry.workingProvider.providerType;
} catch {
} catch (retryErr) {
// v1.28.30 — annotate alongside the marker so an invalid-JSON night
// is greppable under the same action as every other failure class.
annotate({
action: { name: "insights.generate.comprehensive_failed" },
meta: {
locale,
reason: "invalid-json",
provider_status: null,
provider_message:
retryErr instanceof Error
? retryErr.message.slice(0, 240)
: "retry-transport-failed",
},
});
void recordBriefingFailure({ userId, reason: "invalid-json", locale });
return { status: "failed", reason: "invalid-json" };
}
insights = parseComprehensiveResult(result.content);
if (insights === null) {
annotate({
action: { name: "insights.generate.comprehensive_failed" },
meta: {
locale,
reason: "invalid-json",
provider_status: null,
// v1.28.28 truncation-aware: a `length` finish means the token
// ceiling cut the JSON mid-string — a budget problem, not a
// model-quality one.
provider_message:
result.finishReason === "length"
? "response-truncated-at-token-ceiling"
: "unparseable-after-retry",
},
});
void recordBriefingFailure({ userId, reason: "invalid-json", locale });
return { status: "failed", reason: "invalid-json" };
}
@@ -827,6 +912,11 @@ export async function generateComprehensiveInsight(
// enforces the same cross-check: every number the briefing restates must
// trace to a `features.signalsOfDay` figure. One corrective retry, then strip
// the briefing rather than persist a fabricated number.
// v1.28.30 — true when the grounding gate stripped the briefing HARD (no
// previous-cached fallback available). Persisted as a dated marker after
// the cache write so the read path can render the honest "briefing
// withheld" state instead of a silent generic empty card.
let briefingStrippedHard = false;
{
const signals = features.signalsOfDay ?? null;
let retryTransportFailed = false;
@@ -895,6 +985,7 @@ export async function generateComprehensiveInsight(
}
}
(insights as Record<string, unknown>).dailyBriefing = fallbackBriefing;
briefingStrippedHard = fallbackBriefing == null;
annotate({
action: { name: "insights.generate.briefing_grounding_stripped" },
meta: {
@@ -913,7 +1004,10 @@ export async function generateComprehensiveInsight(
// stamp a timestamp/text the caller no longer expects. The provider call
// already cost what it cost; what we must NOT do now is touch the cache.
if (signal?.aborted) {
return { status: "failed", reason: "aborted" };
// v1.28.30 — annotated + marked like every other failure: an abandoned
// generation is a real "no fresh briefing tonight" outcome, and the read
// path needs the marker to render an honest state instead of silence.
return failBeforeProvider(userId, locale, "aborted");
}
await prisma.user.update({
@@ -931,5 +1025,17 @@ export async function generateComprehensiveInsight(
// every warm had to regenerate ~45 cards across both locales.
invalidateUserInsights(userId);
// v1.28.30 — the generation SUCCEEDED but the grounding gate withheld the
// briefing. Record the marker AFTER the cache write so it is newer than
// `insightsCachedAt` and the read path surfaces "briefing withheld"
// instead of a silent generic empty card until the next generation.
if (briefingStrippedHard) {
void recordBriefingFailure({
userId,
reason: "briefing-ungrounded",
locale,
});
}
return { status: "generated", providerType: workingProviderType };
}
@@ -0,0 +1,67 @@
/**
* v1.28.30 — bounded intra-day retry for a failed nightly comprehensive
* warm. Pins the enqueue contract: delayed start (~45 min), per-user
* singleton key distinct from the on-demand `force:` key, small retry
* policy, and a clean no-op (no throw) when no boss instance exists.
*/
import { describe, it, expect, vi, beforeEach } from "vitest";
const send = vi.fn();
let boss: { send: typeof send } | null = null;
vi.mock("@/lib/jobs/boss-instance", () => ({
getGlobalBoss: () => boss,
}));
import {
enqueuePregenerateFailureRetry,
PREGENERATE_RETRY_DELAY_SECONDS,
INSIGHT_PREGENERATE_QUEUE,
} from "../insight-pregenerate-shared";
beforeEach(() => {
vi.clearAllMocks();
boss = { send };
});
describe("enqueuePregenerateFailureRetry", () => {
it("enqueues one delayed forced single-user warm with a per-user retry singleton", async () => {
await enqueuePregenerateFailureRetry({ userId: "u1", locale: "de" });
expect(send).toHaveBeenCalledTimes(1);
const [queue, payload, options] = send.mock.calls[0] as [
string,
Record<string, unknown>,
Record<string, unknown>,
];
expect(queue).toBe(INSIGHT_PREGENERATE_QUEUE);
// Same forced single-user payload the on-demand warm uses, so the
// worker routes it through `forceWarmUser` (freshness re-check,
// failure backoff, daily cap all apply at execution time).
expect(payload).toEqual({ userId: "u1", force: true, locale: "de" });
// Delayed so a transient nightly hiccup has time to clear; distinct
// singleton key so a page-open force warm cannot swallow the retry.
expect(options.startAfter).toBe(PREGENERATE_RETRY_DELAY_SECONDS);
expect(options.singletonKey).toBe("retry:u1");
});
it("keeps the retry window inside the same morning (30-60 min)", () => {
expect(PREGENERATE_RETRY_DELAY_SECONDS).toBeGreaterThanOrEqual(30 * 60);
expect(PREGENERATE_RETRY_DELAY_SECONDS).toBeLessThanOrEqual(60 * 60);
});
it("no-ops without throwing when no boss instance is available", async () => {
boss = null;
await expect(
enqueuePregenerateFailureRetry({ userId: "u1", locale: "en" }),
).resolves.toBeUndefined();
expect(send).not.toHaveBeenCalled();
});
it("swallows a send failure (best-effort; the nightly tick stays the catch-net)", async () => {
send.mockRejectedValueOnce(new Error("boss unavailable"));
await expect(
enqueuePregenerateFailureRetry({ userId: "u1", locale: "de" }),
).resolves.toBeUndefined();
});
});
@@ -19,10 +19,23 @@ import path from "node:path";
const checkRateLimit = vi.fn();
const getAssistantFlags = vi.fn();
const annotateSpy = vi.fn();
vi.mock("@/lib/rate-limit", () => ({
checkRateLimit: (...a: unknown[]) => checkRateLimit(...a),
}));
// v1.28.30 — spy on annotate so the no-silent-failure contract is pinned:
// every failure increment must emit a queryable action. Spread the actual
// module so `getEvent` (used by the feature-cache scope) stays real.
vi.mock("@/lib/logging/context", async () => {
const actual = await vi.importActual<typeof import("@/lib/logging/context")>(
"@/lib/logging/context",
);
return {
...actual,
annotate: (...a: unknown[]) => annotateSpy(...a),
};
});
vi.mock("@/lib/feature-flags", () => ({
getAssistantFlags: (...a: unknown[]) => getAssistantFlags(...a),
}));
@@ -342,6 +355,170 @@ describe("runInsightPregenerate — outcome tally", () => {
});
});
// v1.28.30 — the recurring "no briefing today" chain started with a nightly
// failure that was invisible: a generator resolving `{ status: "failed" }`
// bumped the tally through a switch case with NO annotation (only the
// bounded timeout/error path was annotated), and the failed user was not
// re-attempted until the next night. These tests pin both halves of the
// fix: every failure path annotates `insights.pregenerate.comprehensive_failed`
// with a stage + cause, and every failure enqueues exactly one bounded
// intra-day retry for that user.
describe("runInsightPregenerate — failure visibility + intra-day retry (v1.28.30)", () => {
function failureAnnotations() {
return annotateSpy.mock.calls
.map(
(call) =>
call[0] as {
action?: { name?: string };
meta?: Record<string, unknown>;
},
)
.filter(
(a) => a.action?.name === "insights.pregenerate.comprehensive_failed",
);
}
it("annotates a generator-returned `failed` outcome (the previously silent switch case) and enqueues one retry", async () => {
const { prisma } = makePrisma([{ id: "u1", locale: "de" }]);
const generate = vi
.fn()
.mockResolvedValue({ status: "failed", reason: "invalid-json" });
const statusGenerators = Array.from({ length: 7 }, () =>
vi.fn().mockResolvedValue({ hasProvider: true, cached: true }),
);
const warmGenericMetrics = vi.fn().mockResolvedValue(0);
const enqueueRetry = vi.fn().mockResolvedValue(undefined);
const result = await runInsightPregenerate(prisma as never, {
generate,
statusGenerators,
warmGenericMetrics,
enqueueRetry,
});
expect(result.failed).toBe(1);
const failures = failureAnnotations();
expect(failures).toHaveLength(1);
expect(failures[0].meta).toMatchObject({
locale: "de",
stage: "nightly.generator",
cause: "generator:invalid-json",
});
expect(enqueueRetry).toHaveBeenCalledTimes(1);
expect(enqueueRetry).toHaveBeenCalledWith({ userId: "u1", locale: "de" });
});
it("annotates a thrown generator error with its message and enqueues one retry", async () => {
const { prisma } = makePrisma([{ id: "u1", locale: "en" }]);
const generate = vi
.fn()
.mockRejectedValue(new Error("ECONNRESET upstream"));
const statusGenerators = Array.from({ length: 7 }, () =>
vi.fn().mockResolvedValue({ hasProvider: true, cached: true }),
);
const warmGenericMetrics = vi.fn().mockResolvedValue(0);
const enqueueRetry = vi.fn().mockResolvedValue(undefined);
const result = await runInsightPregenerate(prisma as never, {
generate,
statusGenerators,
warmGenericMetrics,
enqueueRetry,
});
expect(result.failed).toBe(1);
const failures = failureAnnotations();
expect(failures).toHaveLength(1);
expect(failures[0].meta).toMatchObject({
locale: "en",
stage: "nightly.bound",
cause: "error",
message: "ECONNRESET upstream",
});
expect(enqueueRetry).toHaveBeenCalledWith({ userId: "u1", locale: "en" });
});
it("annotates a bounded-budget timeout and enqueues one retry", async () => {
const { prisma } = makePrisma([{ id: "u1", locale: "de" }]);
const generate = vi.fn().mockImplementation(
() =>
new Promise((resolve) => {
setTimeout(
() => resolve({ status: "generated", providerType: "x" }),
260_000,
);
}),
);
const statusGenerators = Array.from({ length: 7 }, () =>
vi.fn().mockResolvedValue({ hasProvider: true, cached: true }),
);
const warmGenericMetrics = vi.fn().mockResolvedValue(0);
const enqueueRetry = vi.fn().mockResolvedValue(undefined);
vi.useFakeTimers();
try {
const promise = runInsightPregenerate(prisma as never, {
generate,
statusGenerators,
warmGenericMetrics,
enqueueRetry,
});
await vi.advanceTimersByTimeAsync(230_000);
const result = await promise;
expect(result.failed).toBe(1);
const failures = failureAnnotations();
expect(failures).toHaveLength(1);
expect(failures[0].meta).toMatchObject({
stage: "nightly.bound",
cause: "timeout",
});
expect(enqueueRetry).toHaveBeenCalledTimes(1);
} finally {
vi.useRealTimers();
}
});
it("does NOT enqueue a retry for generated / cached / unchanged / skipped / budget-blocked users", async () => {
const { prisma } = makePrisma([
{ id: "a", locale: "de" },
{ id: "b", locale: "de" },
{ id: "c", locale: "de" },
{ id: "d", locale: "de" },
{ id: "e", locale: "de" },
]);
// e is budget-blocked; a-d run the generator.
checkRateLimit
.mockResolvedValueOnce({ allowed: true })
.mockResolvedValueOnce({ allowed: true })
.mockResolvedValueOnce({ allowed: true })
.mockResolvedValueOnce({ allowed: true })
.mockResolvedValueOnce({ allowed: false });
const generate = vi
.fn()
.mockResolvedValueOnce({ status: "generated", providerType: "x" })
.mockResolvedValueOnce({ status: "cached" })
.mockResolvedValueOnce({ status: "unchanged" })
.mockResolvedValueOnce({ status: "skipped", reason: "no-provider" });
const statusGenerators = Array.from({ length: 7 }, () =>
vi.fn().mockResolvedValue({ hasProvider: true, cached: true }),
);
const warmGenericMetrics = vi.fn().mockResolvedValue(0);
const enqueueRetry = vi.fn().mockResolvedValue(undefined);
const result = await runInsightPregenerate(prisma as never, {
generate,
statusGenerators,
warmGenericMetrics,
enqueueRetry,
});
expect(result.failed).toBe(0);
expect(enqueueRetry).not.toHaveBeenCalled();
expect(failureAnnotations()).toHaveLength(0);
});
});
describe("runInsightPregenerate — per-metric warm pass", () => {
function warmGen(result: { hasProvider: boolean; cached: boolean }) {
return vi.fn().mockResolvedValue(result);
@@ -92,3 +92,74 @@ export async function enqueueForceWarm(payload: {
// cold until the next poll / nightly cron warms them.
}
}
/**
* Delay before the single bounded retry of a failed nightly comprehensive
* warm. 45 minutes: far enough out that a transient 02:30–04:30 provider
* brown-out has usually cleared, early enough that the retry lands before
* the user's morning visit.
*/
export const PREGENERATE_RETRY_DELAY_SECONDS = 45 * 60;
/**
* Bounded intra-day retry for a user whose NIGHTLY comprehensive warm
* failed. Without this, a single 04:30 provider hiccup left the user's
* briefing stale until the next night: the nightly tick is the only
* scheduled machinery, the GET path's warm enqueue only fires once the
* cache crosses the 24 h freshness line (often mid-evening), and the POST
* cache short-circuit served the stale payload all day.
*
* Enqueues ONE delayed forced single-user warm (the same `forceWarmUser`
* pipeline the on-demand route uses), so every existing bound still
* applies at execution time: the job-start freshness re-check, the
* failure backoff, and the per-user daily forced-warm cap. `singletonKey`
* collapses repeated failures within the window into one queued retry.
* Best-effort like `enqueueForceWarm`: no boss instance → annotated no-op,
* the nightly tick remains the catch-net.
*/
export async function enqueuePregenerateFailureRetry(payload: {
userId: string;
locale: "de" | "en";
}): Promise<void> {
const boss = getGlobalBoss();
if (!boss) {
annotate({
action: { name: "insights.pregenerate.retry.no_boss" },
meta: { locale: payload.locale },
});
return;
}
try {
await boss.send(
INSIGHT_PREGENERATE_QUEUE,
{
userId: payload.userId,
force: true,
locale: payload.locale,
} satisfies InsightPregeneratePayload,
{
startAfter: PREGENERATE_RETRY_DELAY_SECONDS,
// Distinct from the on-demand `force:` key so a user's page-open
// warm can never swallow the scheduled retry (and vice versa);
// one retry per user per window.
singletonKey: `retry:${payload.userId}`,
singletonSeconds: 3600,
// A worker crash mid-warm re-runs the job; a warm that completes
// with a failed comprehensive does NOT throw, so this cannot loop.
retryLimit: 2,
retryDelay: 300,
retryBackoff: true,
},
);
annotate({
action: { name: "insights.pregenerate.retry.enqueued" },
meta: {
locale: payload.locale,
delay_seconds: PREGENERATE_RETRY_DELAY_SECONDS,
},
});
} catch {
// Best-effort: a failed enqueue leaves the nightly tick as the
// catch-net, exactly the pre-retry behaviour.
}
}
+129 -6
View File
@@ -72,6 +72,7 @@ import { resolveEffectiveTimeoutMs } from "@/lib/ai/effective-timeout";
import {
INSIGHT_PREGENERATE_QUEUE,
enqueueForceWarm,
enqueuePregenerateFailureRetry,
type InsightPregeneratePayload,
} from "@/lib/jobs/insight-pregenerate-shared";
@@ -226,6 +227,17 @@ export function comprehensiveWarmBudgetMs(
);
}
/**
* Short, redaction-safe error description for the failure annotations
* below. The central egress redaction (`redactSecrets` in the
* WideEventBuilder) is the safety net; keeping the excerpt short bounds
* the event size and avoids echoing whole provider bodies.
*/
function describeError(err: unknown): string {
const message = err instanceof Error ? err.message : String(err ?? "unknown");
return message.slice(0, 240);
}
/** Per-user result of one forced full warm. */
export interface ForceWarmResult {
/**
@@ -321,9 +333,19 @@ async function warmPerStatusCaches(
try {
const outcome = await generate(userId, { locale, force: false });
return outcome.hasProvider && !outcome.cached ? 1 : 0;
} catch {
} catch (err) {
// A single card's generation failing must not abort the rest of
// the warm pass for this user, nor the cron's user loop.
// the warm pass for this user, nor the cron's user loop — but it
// must not be invisible either: annotate so a failing card is
// diagnosable from one grep instead of a silent zero.
annotate({
action: { name: "insights.pregenerate.warm_failed" },
meta: {
stage: "status-card",
locale,
message: describeError(err),
},
});
return 0;
}
}),
@@ -360,8 +382,17 @@ async function warmStatusBatch(
force: false,
});
warmed += result.batched + result.fellBack;
} catch {
// The batch must never throw the cron's user loop off course.
} catch (err) {
// The batch must never throw the cron's user loop off course — but
// a swallowed batch failure left ~7 cards cold with zero signal.
annotate({
action: { name: "insights.pregenerate.warm_failed" },
meta: {
stage: "status-batch",
locale,
message: describeError(err),
},
});
}
}
return warmed;
@@ -400,7 +431,13 @@ async function warmGenericMetricCaches(
orderBy: { type: "asc" },
});
typesWithData = new Set(rows.map((r) => r.type));
} catch {
} catch (err) {
// Best-effort discovery — but a silently-failing grouped read used to
// zero out the whole generic warm with no trace.
annotate({
action: { name: "insights.pregenerate.warm_failed" },
meta: { stage: "metric-discovery", message: describeError(err) },
});
return 0;
}
@@ -434,7 +471,16 @@ async function warmGenericMetricCaches(
outcome.insufficient !== true
? 1
: 0;
} catch {
} catch (err) {
annotate({
action: { name: "insights.pregenerate.warm_failed" },
meta: {
stage: "metric-card",
metric,
locale,
message: describeError(err),
},
});
return 0;
}
}),
@@ -522,11 +568,22 @@ export async function runInsightPregenerate(
userId: string,
locales: ReadonlyArray<"de" | "en">,
) => Promise<number>;
/**
* v1.28.30 — injected for the test. Defaults to the real delayed
* single-user retry enqueue; fired once per candidate whose
* comprehensive warm failed so a transient nightly provider hiccup
* heals within the day instead of waiting for the next tick.
*/
enqueueRetry?: (payload: {
userId: string;
locale: "de" | "en";
}) => Promise<void>;
} = {},
): Promise<PregenerateRunResult> {
const now = options.now ?? new Date();
const cap = options.cap ?? PREGENERATE_BATCH_CAP;
const generate = options.generate ?? generateComprehensiveInsight;
const enqueueRetry = options.enqueueRetry ?? enqueuePregenerateFailureRetry;
// v1.18.7 (HIGH-1) — when the caller injects its own `statusGenerators`
// (the unit tests do), keep the per-card warm so those assertions hold;
// production injects nothing, so the seven warm calls collapse into ONE
@@ -608,12 +665,19 @@ export async function runInsightPregenerate(
const warmBudgetMs = comprehensiveWarmBudgetMs(
candidate.aiResponseTimeoutSeconds,
);
// Capture the rejection so the failure annotation can carry the
// actual error text — `withTimeout` folds a rejection into a bare
// `errored: true` envelope, which made the cause undiagnosable.
let thrown: unknown = null;
const bounded = await withTimeout(
() =>
generate(candidate.id, {
locale,
force: true,
signal: controller.signal,
}).catch((err: unknown) => {
thrown = err;
throw err;
}),
warmBudgetMs,
null,
@@ -625,17 +689,26 @@ export async function runInsightPregenerate(
// 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.
// v1.28.30 — carry the stage + error text so ONE grep on
// `comprehensive_failed` names the failing catch and its cause.
annotate({
action: { name: "insights.pregenerate.comprehensive_failed" },
meta: {
locale,
stage: "nightly.bound",
cause: bounded.timedOut
? "timeout"
: bounded.errored
? "error"
: "null",
budget_ms: warmBudgetMs,
message: thrown === null ? null : describeError(thrown),
},
});
// v1.28.30 — bounded intra-day retry: one delayed forced warm for
// just this user, so a transient 04:30 provider hiccup heals
// before the morning visit instead of waiting for the next night.
await enqueueRetry({ userId: candidate.id, locale });
} else {
outcome = bounded.value;
switch (outcome.status) {
@@ -653,6 +726,24 @@ export async function runInsightPregenerate(
break;
case "failed":
result.failed++;
// v1.28.30 — THE previously-silent failure path. A generator
// that resolves with `{ status: "failed" }` (features error,
// oversize payload, invalid JSON after retry, abort) bumped
// the tally with no annotation at all — the nightly summary
// showed `failed: 1` and nothing else was greppable. Reuse
// the `comprehensive_failed` shape with the generator's own
// reason as the cause.
annotate({
action: { name: "insights.pregenerate.comprehensive_failed" },
meta: {
locale,
stage: "nightly.generator",
cause: `generator:${outcome.reason}`,
budget_ms: warmBudgetMs,
message: null,
},
});
await enqueueRetry({ userId: candidate.id, locale });
break;
}
}
@@ -896,12 +987,17 @@ export async function forceWarmUser(
const warmBudgetMs = comprehensiveWarmBudgetMs(
freshness?.aiResponseTimeoutSeconds,
);
// v1.28.30 — capture the rejection for the failure annotation.
let thrown: unknown = null;
const comprehensive = await withTimeout(
() =>
generate(userId, {
locale,
force: true,
signal: controller.signal,
}).catch((err: unknown) => {
thrown = err;
throw err;
}),
warmBudgetMs,
null,
@@ -914,6 +1010,33 @@ export async function forceWarmUser(
} else {
result.comprehensive = comprehensive.value.status;
}
// v1.28.30 — same queryable failure shape as the nightly loop, so
// one grep on `comprehensive_failed` covers both entry points.
if (
result.comprehensive === "failed" ||
result.comprehensive === "timeout"
) {
annotate({
action: { name: "insights.pregenerate.comprehensive_failed" },
meta: {
locale,
stage: "force",
cause: comprehensive.timedOut
? "timeout"
: comprehensive.errored
? "error"
: comprehensive.value === null
? "null"
: `generator:${
comprehensive.value.status === "failed"
? comprehensive.value.reason
: comprehensive.value.status
}`,
budget_ms: warmBudgetMs,
message: thrown === null ? null : describeError(thrown),
},
});
}
// Stamp / clear the failure marker for the backoff gate above. A
// `skipped` outcome (no provider / no consent) is not a failure —