From 4bc7c6ae1f66a44736bbcf8a13f202ee6e6e5903 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Mon, 8 Jun 2026 18:54:56 +0200 Subject: [PATCH] fix(insights): keep slow daily-briefing regenerations and tell the truth MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tapping the top-right regenerate button POSTs an inline ~1500-token warm generation that routinely runs past the 8 s read budget. The force branch shared that 8 s AbortController, so a slow-but-successful generation was discarded: the abort returned null, the mutation only invalidated (re- reading the OLD cache), and the falling-edge toast claimed success while the text never changed — even though the server had often written the fresh briefing. Give the force branch its own 45 s budget (matching the out-of-band warm job) so a slow success is kept, not dropped. Tag every fetch outcome (fresh / empty / timeout / no-provider) and surface the last regenerate outcome through the tab strip, which now toasts "refreshed" only on a genuinely fresh result, shows a "still working, try again" hint on a timeout, and stays silent when no provider is configured. The read path keeps its fast 8 s cap. --- .../insights/insights-layout-shell.tsx | 1 + .../insights/insights-tab-strip.tsx | 31 +++++-- .../insights/use-insights-advisor.ts | 89 +++++++++++++++---- 3 files changed, 97 insertions(+), 24 deletions(-) diff --git a/src/components/insights/insights-layout-shell.tsx b/src/components/insights/insights-layout-shell.tsx index 942b33a13..145b5e5d6 100644 --- a/src/components/insights/insights-layout-shell.tsx +++ b/src/components/insights/insights-layout-shell.tsx @@ -141,6 +141,7 @@ export function InsightsLayoutShell({ children }: { children: ReactNode }) { void; /** Spinner state — disables the button and swaps the icon. */ regenerating?: boolean; + /** + * v1.15.18 — the outcome of the last settled regenerate. The falling-edge + * toast fires "refreshed" ONLY when this is `"fresh"`; a slow generation the + * client gave up on (`"timeout"`) shows a "still working" hint instead of a + * misleading success, and a missing provider (`"no-provider"`) stays silent + * (the surface already shows the connect-AI empty state). Absent (legacy + * mounts / no regenerate) falls back to the prior unconditional success. + */ + regenerateOutcome?: "fresh" | "empty" | "timeout" | "no-provider" | null; /** * v1.4.27 F19 — analytics + event-driven availability inputs the * gating helper reads. When omitted the strip falls back to its @@ -440,6 +449,7 @@ function buildTabs( function InsightsTabStripImpl({ onRegenerate, regenerating = false, + regenerateOutcome, availability, visibleTileIds, tileOrder, @@ -463,16 +473,27 @@ function InsightsTabStripImpl({ [availability, visibleTileIds, tileOrder], ); - // Fire success toast on the falling edge of `regenerating`. Same - // rising-edge ref guard as the W3 implementation so the toast fires - // exactly once per regenerate cycle. + // Fire a toast on the falling edge of `regenerating`. Same rising-edge ref + // guard as the W3 implementation so the toast fires exactly once per + // regenerate cycle. v1.15.18 — the toast is now HONEST: only a `"fresh"` + // outcome reads "refreshed". A `"timeout"` (a slow generation the client + // gave up on while the server may still be writing) shows a "still working, + // try again" hint rather than claiming success; a `"no-provider"` stays + // silent (the surface already shows the connect-AI empty state). The latest + // outcome is read through a ref so the effect doesn't re-fire when only the + // outcome (not the spinner edge) changed. const lastRegeneratingRef = useRef(regenerating); useEffect(() => { if (lastRegeneratingRef.current && !regenerating) { - toast.success(t("insights.regenerateSuccess")); + if (regenerateOutcome === "timeout") { + toast.error(t("insights.regenerateError")); + } else if (regenerateOutcome !== "no-provider") { + // `"fresh"`, `"empty"`, or legacy `undefined`/null → success. + toast.success(t("insights.regenerateSuccess")); + } } lastRegeneratingRef.current = regenerating; - }, [regenerating, t]); + }, [regenerating, regenerateOutcome, t]); const regenerateLabel = t("insights.regenerateAnalysis"); const customizeLabel = t("insights.customize"); diff --git a/src/components/insights/use-insights-advisor.ts b/src/components/insights/use-insights-advisor.ts index aad5d43ec..ef159ec94 100644 --- a/src/components/insights/use-insights-advisor.ts +++ b/src/components/insights/use-insights-advisor.ts @@ -51,25 +51,53 @@ export interface InsightAdvisorPayload { } /** - * v1.4.31 — bound the advisor POST with an 8-second - * `AbortController` so a cache-miss path (server still waiting on - * the provider chain) does not pin the mother-page main thread for - * the LLM's full completion tail. The strip stays interactive in - * the DOM, but every re-render of a parent during a long pending - * fetch competes with WebKit's gesture-recognition timeout — - * dropping the worst case from 30 s to 8 s eliminates the - * mobile-tap-block window per + * v1.4.31 — bound the advisor READ (GET) with an 8-second + * `AbortController` so a cache-miss path (server still warming out of + * band) does not pin the mother-page main thread. The strip stays + * interactive in the DOM; dropping the worst case from 30 s to 8 s + * eliminates the mobile-tap-block window per * `.planning/research/v15-insights-blocking-bug.md` fix 1. */ const ADVISOR_TIMEOUT_MS = 8_000; +/** + * v1.15.18 — the user-initiated regenerate (`force`) POSTs an INLINE + * generation: a ~1500-token warm completion that routinely runs longer + * than the 8 s read budget. Bounding the force branch at 8 s silently + * discarded a slow-but-successful generation — the abort returned a null + * payload, the mutation only invalidated (re-reading the OLD cache), yet + * the server had often already written the fresh briefing. Give the force + * branch the same 45 s budget the out-of-band warm job uses + * (`FORCE_WARM_COMPREHENSIVE_TIMEOUT_MS`) so a slow success is kept, not + * dropped. The READ path stays fast at 8 s — only the explicit user tap + * may wait. Per `.planning/v1.15.18-daily-briefing-audit.md` Fix 1b. + */ +const FORCE_ADVISOR_TIMEOUT_MS = 45_000; + +/** + * v1.15.18 — the outcome of a force regenerate, so the UI can be HONEST: + * only a `fresh` outcome should toast "refreshed". A `timeout` (slow gen + * the client gave up on) and a `no-provider` (422) are distinct failure + * modes the audit's Fix 2 + Fix 4 call out. + */ +export type AdvisorFetchOutcome = + | "fresh" + | "empty" + | "timeout" + | "no-provider"; + +interface AdvisorFetchResult { + payload: InsightAdvisorPayload | null; + outcome: AdvisorFetchOutcome; +} + async function fetchAdvisor( options: { force?: boolean } = {}, -): Promise { +): Promise { const controller = new AbortController(); const timeoutHandle = setTimeout( () => controller.abort(), - ADVISOR_TIMEOUT_MS, + options.force ? FORCE_ADVISOR_TIMEOUT_MS : ADVISOR_TIMEOUT_MS, ); let res: Response; try { @@ -92,7 +120,8 @@ async function fetchAdvisor( if (err instanceof DOMException && err.name === "AbortError") { // Graceful empty payload — the UI surfaces the empty / regen // CTA exactly as it does for the 422 / 429 / 503 paths below. - return null; + // `timeout` lets the regenerate path avoid claiming success. + return { payload: null, outcome: "timeout" }; } throw err; } finally { @@ -101,9 +130,14 @@ async function fetchAdvisor( if (!res.ok) { // 422 (no provider configured) and 429 (rate-limited) are expected // surfaces — return null so the consuming UI shows the empty / error - // state without the query slipping into an `isError` retry loop. - if (res.status === 422 || res.status === 429 || res.status === 503) { - return null; + // state without the query slipping into an `isError` retry loop. 422 + // surfaces as `no-provider` so the regenerate path can distinguish a + // missing provider from a slow generation (audit Fix 4). + if (res.status === 422) { + return { payload: null, outcome: "no-provider" }; + } + if (res.status === 429 || res.status === 503) { + return { payload: null, outcome: "empty" }; } throw new Error(`HTTP ${res.status}`); } @@ -139,7 +173,7 @@ async function fetchAdvisor( } else { payload.trendAnnotations = null; } - return payload; + return { payload, outcome: "fresh" }; } export interface UseInsightsAdvisorResult { @@ -150,6 +184,13 @@ export interface UseInsightsAdvisorResult { regenerate: () => void; isRegenerating: boolean; regenerateError: Error | null; + /** + * v1.15.18 — the outcome of the LAST settled regenerate. The tab strip + * fires the "refreshed" toast only when this is `"fresh"`, so a slow gen + * the client gave up on (`"timeout"`) or a missing provider + * (`"no-provider"`) never reads as "done". + */ + regenerateOutcome: AdvisorFetchOutcome | null; } /** @@ -162,7 +203,9 @@ export function useInsightsAdvisorQuery( const queryClient = useQueryClient(); const query = useQuery({ queryKey: queryKeys.insightsAdvisor(), - queryFn: () => fetchAdvisor(), + // Map the tagged fetch result back onto the payload the cache stores — + // the read path only cares about the payload, not the outcome tag. + queryFn: async () => (await fetchAdvisor()).payload, enabled, // 24h cache window matches the server-side `insightsCachedAt` TTL. staleTime: 60 * 60 * 1000, @@ -171,10 +214,17 @@ export function useInsightsAdvisorQuery( const mutation = useMutation({ mutationFn: () => fetchAdvisor({ force: true }), - onSuccess: (next) => { - if (next) { - queryClient.setQueryData(queryKeys.insightsAdvisor(), next); + onSuccess: ({ payload }) => { + if (payload) { + // A genuinely fresh generation landed — write it into the shared + // cache so the hero subtitle + briefing card repaint immediately. + queryClient.setQueryData(queryKeys.insightsAdvisor(), payload); } else { + // No fresh payload (timeout / no-provider / transient). The server's + // inline POST may STILL have written a fresh briefing after the + // client gave up at 45 s, so re-read the GET to converge — but the + // honest toast is gated on `regenerateOutcome` below, not on this + // invalidate. queryClient.invalidateQueries({ queryKey: queryKeys.insightsAdvisor(), }); @@ -202,5 +252,6 @@ export function useInsightsAdvisorQuery( regenerate, isRegenerating: mutation.isPending, regenerateError: (mutation.error as Error | null) ?? null, + regenerateOutcome: mutation.data?.outcome ?? null, }; }