mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-08 12:32:43 +00:00
fix(insights): keep slow daily-briefing regenerations and tell the truth
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.
This commit is contained in:
1 parent
5509f8c15a
commit
4bc7c6ae1f
3 files changed
+97
-24
No files matched your search
@@ -141,6 +141,7 @@ export function InsightsLayoutShell({ children }: { children: ReactNode }) {
|
||||
<InsightsTabStrip
|
||||
onRegenerate={advisorEnabled ? advisor.regenerate : undefined}
|
||||
regenerating={advisor.isRegenerating}
|
||||
regenerateOutcome={advisor.regenerateOutcome}
|
||||
availability={availability}
|
||||
visibleTileIds={visibleTileIds}
|
||||
tileOrder={tileOrder}
|
||||
|
||||
@@ -66,6 +66,15 @@ export interface InsightsTabStripProps {
|
||||
onRegenerate?: () => 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<boolean>(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");
|
||||
|
||||
@@ -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<InsightAdvisorPayload | null> {
|
||||
): Promise<AdvisorFetchResult> {
|
||||
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,
|
||||
};
|
||||
}
|
||||
Reference in new issue
Block a user