diff --git a/src/__tests__/baselines/version-stamp-comments.json b/src/__tests__/baselines/version-stamp-comments.json index 2892e75da..deaf95332 100644 --- a/src/__tests__/baselines/version-stamp-comments.json +++ b/src/__tests__/baselines/version-stamp-comments.json @@ -1,7 +1,7 @@ { "$comment": "Comment lines that open with a release version, per file under src/. Maintained by src/__tests__/version-stamp-ratchet.test.ts.", "count": 2897, - "total": 9259, + "total": 9256, "entries": { "__tests__/account-payload-consumer-guard.test.ts": 2, "__tests__/acting-account-boundary-guard.test.ts": 7, @@ -2158,7 +2158,7 @@ "lib/insights/chart-tokens.ts": 24, "lib/insights/coach-launch-context.tsx": 10, "lib/insights/comprehensive-aggregator.ts": 27, - "lib/insights/comprehensive-generate.ts": 48, + "lib/insights/comprehensive-generate.ts": 45, "lib/insights/correlation-channel-series.ts": 16, "lib/insights/correlation-discovery.ts": 30, "lib/insights/correlation-series-builders.ts": 1, diff --git a/src/lib/insights/__tests__/briefing-provider-budget.test.ts b/src/lib/insights/__tests__/briefing-provider-budget.test.ts index bd9ff8143..cf19b4441 100644 --- a/src/lib/insights/__tests__/briefing-provider-budget.test.ts +++ b/src/lib/insights/__tests__/briefing-provider-budget.test.ts @@ -147,6 +147,30 @@ describe("runBriefingCompletion — ledger accounting", () => { expect(ledgerTotal).toBe(1234 + 2700); }); + it("reserves and sends nothing once the caller's bound fired (#1279)", async () => { + mockProviderReply(100); + const controller = new AbortController(); + controller.abort(); + + await expect( + runBriefingCompletion(completionArgs({ signal: controller.signal })), + ).rejects.toThrow(); + + expect(runRawCompletionWithFallback).not.toHaveBeenCalled(); + expect(ledgerTotal).toBe(0); + }); + + it("hands the bound's signal to the provider call", async () => { + mockProviderReply(100); + const controller = new AbortController(); + + await runBriefingCompletion(completionArgs({ signal: controller.signal })); + + expect(runRawCompletionWithFallback.mock.calls[0][0].params.signal).toBe( + controller.signal, + ); + }); + it("charges the reservation when the provider reports no token count", async () => { mockProviderReply(null); diff --git a/src/lib/insights/__tests__/comprehensive-warm-timeout.test.ts b/src/lib/insights/__tests__/comprehensive-warm-timeout.test.ts index e90362f4e..5468a5de7 100644 --- a/src/lib/insights/__tests__/comprehensive-warm-timeout.test.ts +++ b/src/lib/insights/__tests__/comprehensive-warm-timeout.test.ts @@ -155,6 +155,34 @@ describe("comprehensive warm-path timeout + failure observability", () => { expect(params.timeoutMs).toBe(AI_BUDGETS.comprehensive.timeoutMs); }); + it("sends nothing more once the caller's bound has given up (#1279)", async () => { + // A slow model: its reply (not valid JSON, so the generator would ask + // again) lands only after the warm's bound fired and aborted the run. + const controller = new AbortController(); + runRawCompletionWithFallback.mockImplementation(async () => { + controller.abort(); + return { + result: { content: "not json", tokensUsed: 10, model: "m" }, + workingProvider: { providerType: "codex" }, + fallbackHops: [], + tokensSpent: 0, + }; + }); + + await generateComprehensiveInsight("u1", { + locale: "de", + signal: controller.signal, + }); + + // The call in flight carried the bound's signal, so the client could tear + // its request down, and no retry left after the bound. + expect(runRawCompletionWithFallback).toHaveBeenCalledTimes(1); + expect(runRawCompletionWithFallback.mock.calls[0][0].params.signal).toBe( + controller.signal, + ); + expect(userUpdate).not.toHaveBeenCalled(); + }); + 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( diff --git a/src/lib/insights/briefing-provider.ts b/src/lib/insights/briefing-provider.ts index ea4e493ae..d4758b9a2 100644 --- a/src/lib/insights/briefing-provider.ts +++ b/src/lib/insights/briefing-provider.ts @@ -112,6 +112,15 @@ export interface RunBriefingCompletionArgs { * Absent for the scheduled generator. */ interactive?: boolean; + /** + * #1279 — the caller's outer bound (the nightly and forced warms, the + * morning refresh). It reaches every provider call, so a call the bound + * gave up on is torn down, and once it fired no further call is reserved + * or sent: a briefing that outlives its bound does not keep running + * detached. The on-demand route passes none on purpose: it finishes and + * writes its briefing after the browser stopped waiting (#786). + */ + signal?: AbortSignal; } /** @@ -131,6 +140,7 @@ export interface RunBriefingCompletionArgs { export async function runBriefingCompletion( args: RunBriefingCompletionArgs, ): Promise { + args.signal?.throwIfAborted(); const dateKey = args.dateKey ?? (await buildDateKey(args.userId)); // v1.41 — one resolution per call: the operator's switch and cap, the payer @@ -195,6 +205,7 @@ export async function runBriefingCompletion( maxTokens: args.maxTokens, seed: args.seed, timeoutMs: args.timeoutMs, + signal: args.signal, // Every briefing call parses its reply with `JSON.parse`, so opt the // chains with a native JSON mode into it. responseFormat: "json", diff --git a/src/lib/insights/comprehensive-generate.ts b/src/lib/insights/comprehensive-generate.ts index 8ae3feb59..a5fb5d5a9 100644 --- a/src/lib/insights/comprehensive-generate.ts +++ b/src/lib/insights/comprehensive-generate.ts @@ -244,6 +244,8 @@ async function rerollBriefingParagraph(args: { * slow-backend allowance the main generation does. */ effectiveTimeoutMs: number; + /** The caller's bound: a re-roll it gave up on sends nothing more. */ + signal?: AbortSignal; }): Promise<{ text: string; providerType: string } | null> { let cached: Record; try { @@ -277,6 +279,7 @@ async function rerollBriefingParagraph(args: { // generation is not clipped at the client's 60 s default mid-stream. // v1.25 — honours the per-user response-timeout setting (see caller). timeoutMs: args.effectiveTimeoutMs, + signal: args.signal, stage: "reroll", // v1.41 — the re-roll is a fresh pass at the paragraph, so it may think // like the generation it replaces; the operator's switch still wins. @@ -716,13 +719,10 @@ interface GenerateOptions { /** Skip the 24 h cache short-circuit and force a fresh generation. */ force?: boolean; /** - * v1.9.0 — abort the generation when the caller has given up on it. - * - * The on-demand `forceWarmUser` path bounds the comprehensive step with a - * timeout and then proceeds to warm the per-status caches itself. Without - * this signal a comprehensive that resolves AFTER the timeout would still - * write a cache row + timestamp the caller no longer expects. The signal - * lets the caller cut the generation off before the cache write. + * The caller's bound gave up on the generation (the warms bound it, then + * move on). Every provider call carries it, so the call in flight is torn + * down and no later one is sent (#1279), and a generation that still + * resolves writes no cache row the caller no longer expects. */ signal?: AbortSignal; } @@ -1141,6 +1141,7 @@ export async function generateComprehensiveInsight( features, comparison: groundingExtra, effectiveTimeoutMs, + signal, }); if (rerolled) { // The reroll rewrites content derived from the scope captured before @@ -1232,6 +1233,7 @@ export async function generateComprehensiveInsight( // generation is not clipped at the client's 60 s default mid-stream. // v1.25 — honours the per-user response-timeout setting. timeoutMs: effectiveTimeoutMs, + signal, stage: "generate", // v1.41 — the briefing is one of the few background jobs that may // reason (medium on the person's own provider, low on the operator's, @@ -1328,6 +1330,7 @@ export async function generateComprehensiveInsight( // v1.21.5 — wider upstream budget; see the first generation call. // v1.25 — honours the per-user response-timeout setting. timeoutMs: effectiveTimeoutMs, + signal, stage: "json-retry", }); result = retry.result; @@ -1421,6 +1424,7 @@ export async function generateComprehensiveInsight( // v1.21.5 — wider upstream budget; see the first generation call. // v1.25 — honours the per-user response-timeout setting. timeoutMs: effectiveTimeoutMs, + signal, stage: "grounding-retry", }); const retryInsights = parseComprehensiveResult(retry.result.content); @@ -1553,6 +1557,7 @@ export async function generateComprehensiveInsight( temperature: AI_BUDGETS.comprehensive.temperature, maxTokens: resolveInsightsMaxTokens(), timeoutMs: effectiveTimeoutMs, + signal, stage: "screen-retry", }); const retryInsights = parseComprehensiveResult(retry.result.content); @@ -1594,15 +1599,10 @@ export async function generateComprehensiveInsight( } } - // v1.9.0 — the caller abandoned this generation (its bounded timeout - // fired and it has moved on to warm the per-status caches itself). Bail - // out before the cache write so a late-resolving comprehensive cannot - // 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. + // The caller abandoned this generation (its bound fired). Bail out before + // the cache write, annotated and marked like every other failure: an + // abandoned generation is a real "no fresh briefing tonight" outcome. if (signal?.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"); }