From 552510c6545c6feb04aa9eed0a2203df90df0ed9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sat, 24 Oct 2026 09:23:47 +0200 Subject: [PATCH] fix(insights): keep polling for the briefing while the warm can land The briefing read polls while the warm it enqueued runs, and it stopped after ten polls (225 s) whatever the response timeout. A briefing that landed later never reached the open page, which kept the empty card, or the previous attempt's "timed out" hint, on screen until a reload. The read's poll ceiling now follows the published response timeout over the generation's sequential calls, never less per call than the briefing's 180 s default, plus one interval for the queue to pick the warm up. A settling regenerate and the overview body keep their ten polls. A test walks the poll schedule against the warm's own bound for every setting, and the client-abort guard holds the read to the derived ceiling. Refs #1274 --- src/__tests__/ai-client-abort-guard.test.ts | 28 ++++++++++++++- .../client-poll-inventory-guard.test.ts | 3 +- .../insights/use-insights-advisor.ts | 36 ++++++++++++++++--- .../__tests__/insight-pregenerate.test.ts | 30 ++++++++++++++++ 4 files changed, 91 insertions(+), 6 deletions(-) diff --git a/src/__tests__/ai-client-abort-guard.test.ts b/src/__tests__/ai-client-abort-guard.test.ts index 5a577e22e..715ed673c 100644 --- a/src/__tests__/ai-client-abort-guard.test.ts +++ b/src/__tests__/ai-client-abort-guard.test.ts @@ -15,8 +15,17 @@ * `use-insights-advisor-force.test.ts` proves the behaviour for the * regenerate with fake timers. * + * #1274 — the same holds for a browser that waits on a model by polling: the + * briefing read polls while the warm it enqueued runs, and a fixed ten polls + * (225 s) gave up on a warm the setting allowed far longer, so a briefing that + * landed later never reached the open page. The read's poll ceiling comes + * from `advisorRevalidatePollMaxAttempts(responseTimeoutMs)`; + * `insight-pregenerate.test.ts` proves it outlasts the warm's own bound for + * every setting. + * * Mutation check: put `FORCE_ADVISOR_TIMEOUT_MS = 45_000` back into the - * advisor, or drop the `signal` from one about-me save, and this goes red. + * advisor, drop the `signal` from one about-me save, or drop the ceiling + * argument from the advisor read's poll, and this goes red. */ import { readFileSync } from "node:fs"; import { join } from "node:path"; @@ -57,6 +66,23 @@ describe("browser aborts for model-backed requests", () => { ); }); + it("the briefing read polls for the warm as long as the setting allows", () => { + const source = stripComments( + readFileSync( + join(SRC, "components/insights/use-insights-advisor.ts"), + "utf8", + ), + ); + const polls = [...source.matchAll(/\bnextAdvisorPollInterval\s*\(/g)] + .map((m) => callText(source, (m.index ?? 0) + m[0].length - 1)) + // The definition, not a call. + .filter((text) => !/:\s*boolean\s*\|\s*undefined/.test(text)); + expect(polls).toHaveLength(1); + expect(polls[0]).toMatch( + /advisorRevalidatePollMaxAttempts\(\s*responseTimeoutMs\s*\)/, + ); + }); + it("every about-me save carries the setting-derived signal", () => { const sites: string[] = []; const missing: string[] = []; diff --git a/src/__tests__/client-poll-inventory-guard.test.ts b/src/__tests__/client-poll-inventory-guard.test.ts index 6fa158b6a..8e905819b 100644 --- a/src/__tests__/client-poll-inventory-guard.test.ts +++ b/src/__tests__/client-poll-inventory-guard.test.ts @@ -132,7 +132,8 @@ const INVENTORY: Readonly> = { "components/insights/use-insights-advisor.ts": { sites: 1, shape: "bounded", - cadence: "25 s, nextAdvisorPollInterval, 10 attempts (advisor)", + cadence: + "25 s, nextAdvisorPollInterval, attempts from the response timeout so the warm can land (advisor); 10 while settling", }, "components/insights/insights-layout-shell.tsx": { sites: 1, diff --git a/src/components/insights/use-insights-advisor.ts b/src/components/insights/use-insights-advisor.ts index 10fa7bc60..1ab2c1b77 100644 --- a/src/components/insights/use-insights-advisor.ts +++ b/src/components/insights/use-insights-advisor.ts @@ -18,7 +18,7 @@ import type { import { queryKeys, refetchInactiveDailyReads } from "@/lib/query-keys"; import { apiFetchRaw } from "@/lib/api/api-fetch"; import { useAiCapability, useAiProviderState } from "@/hooks/use-ai-capability"; -import { AI_BUDGETS } from "@/lib/ai/ai-budgets"; +import { AI_BUDGETS, COMPREHENSIVE_MODEL_CALLS } from "@/lib/ai/ai-budgets"; import { clientAbortMs } from "@/lib/ai/effective-timeout"; import type { BriefingFailureClass } from "@/lib/insights/briefing-failure-marker"; @@ -147,15 +147,41 @@ export function forceAdvisorTimeoutMs( * `revalidating: true` (stale briefing served, out-of-band warm in * flight). The query's 1 h `staleTime` plus the app-default * `refetchOnWindowFocus: false` means a stale-served briefing would - * otherwise never refresh in-session. 25 s comfortably covers the warm - * job's 45 s budget within two polls; the attempt ceiling stops a + * otherwise never refresh in-session. The attempt ceiling stops a * persistently failing generation from polling an open page forever. * Same bounded-poll shape as `nextStatusPollInterval` * (`src/hooks/use-insight-status.ts`). + * + * `ADVISOR_REVALIDATE_POLL_MAX_ATTEMPTS` is the ceiling for a settling + * regenerate and for callers whose poll waits on no model; the briefing read + * itself waits as long as the warm may run (`advisorRevalidatePollMaxAttempts`). */ export const ADVISOR_REVALIDATE_POLL_MS = 25_000; export const ADVISOR_REVALIDATE_POLL_MAX_ATTEMPTS = 10; +/** + * The attempt ceiling for the briefing read while the warm is in flight. + * + * #1274 — a fixed ten attempts stopped watching after 225 s, while the warm + * the read enqueued may run for the person's response timeout on each of the + * generation's sequential calls (`COMPREHENSIVE_MODEL_CALLS`). A local model + * needing 300 s per answer delivered its briefing to a page that had stopped + * looking, which kept the empty or the previous failure state on screen. The + * window is the warm's own bound read off the published setting, never less + * per call than the briefing's 180 s default (`clientAbortMs`), plus the first + * read and one interval for the queue to pick the warm up. + */ +export function advisorRevalidatePollMaxAttempts( + responseTimeoutMs: number | null | undefined, +): number { + const warmMs = clientAbortMs( + responseTimeoutMs, + COMPREHENSIVE_MODEL_CALLS, + AI_BUDGETS.comprehensive.timeoutMs, + ); + return Math.ceil(warmMs / ADVISOR_REVALIDATE_POLL_MS) + 2; +} + /** * Refs #786 — the settling state a timed-out force regenerate enters. * @@ -237,6 +263,7 @@ export function nextAdvisorPollInterval( revalidating: boolean | undefined, dataUpdateCount: number, settling?: Pick | null, + maxAttempts: number = ADVISOR_REVALIDATE_POLL_MAX_ATTEMPTS, ): number | false { if (settling) { if ( @@ -248,7 +275,7 @@ export function nextAdvisorPollInterval( return ADVISOR_REVALIDATE_POLL_MS; } if (!revalidating) return false; - if (dataUpdateCount >= ADVISOR_REVALIDATE_POLL_MAX_ATTEMPTS) return false; + if (dataUpdateCount >= maxAttempts) return false; return ADVISOR_REVALIDATE_POLL_MS; } @@ -487,6 +514,7 @@ export function useInsightsAdvisorQuery( query.state.data?.payload?.revalidating, pollAttemptCount(query.state), settling, + advisorRevalidatePollMaxAttempts(responseTimeoutMs), ), }); diff --git a/src/lib/jobs/__tests__/insight-pregenerate.test.ts b/src/lib/jobs/__tests__/insight-pregenerate.test.ts index bbdb13376..80e182903 100644 --- a/src/lib/jobs/__tests__/insight-pregenerate.test.ts +++ b/src/lib/jobs/__tests__/insight-pregenerate.test.ts @@ -139,6 +139,11 @@ import { loadedHandler, } from "../registry/__tests__/registry-helpers"; import { AI_BUDGETS, COMPREHENSIVE_MODEL_CALLS } from "@/lib/ai/ai-budgets"; +import { + ADVISOR_REVALIDATE_POLL_MS, + advisorRevalidatePollMaxAttempts, + nextAdvisorPollInterval, +} from "@/components/insights/use-insights-advisor"; function makePrisma( users: Array<{ @@ -1473,6 +1478,31 @@ describe("comprehensiveWarmBudgetMs — the warm waits for every call of one gen // is the structural backstop against a wedged call regardless. expect(comprehensiveWarmBudgetMs(100_000)).toBe(ceiling); }); + + it("the Insights page polls for the warm at least as long as the warm may run", () => { + // The page reads the published `responseTimeoutMs`: the setting in ms, or + // the 60 s provider default when nothing is set. + const cases: Array<[number | null, number]> = [ + [null, 60_000], + [10, 10_000], + [60, 60_000], + [180, 180_000], + [300, 300_000], + [600, 600_000], + ]; + for (const [seconds, published] of cases) { + const attempts = advisorRevalidatePollMaxAttempts(published); + let count = 1; // the read that enqueued the warm + let lastPollMs = 0; + while (nextAdvisorPollInterval(true, count, null, attempts) !== false) { + lastPollMs += ADVISOR_REVALIDATE_POLL_MS; + count += 1; + } + expect(lastPollMs, `setting ${seconds ?? "unset"}`).toBeGreaterThan( + comprehensiveWarmBudgetMs(seconds), + ); + } + }); }); describe("runInsightPregenerate — warm budget honours the response timeout (v1.25.3)", () => {