mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-14 06:40:50 +00:00
fix(insights): wait as long as the response timeout allows in the browser
The regenerate button aborted its inline POST after a fixed 45 s, so on a slow local model every regenerate was cut while the server still had minutes left on the person's setting. It now uses clientAbortMs over the route's sequential model calls, never less per call than the briefing's own 180 s default, because the published value cannot say whether the person set it. A save of the about-me self-context derives the clarifying questions inline and ran under the 15 s request default, which reported a failed save while the server went on to commit it. All six save sites now take their abort from the same setting. Every other browser request that waits on a model either has no client timeout or only queues the work. Refs #1267
This commit is contained in:
1 parent
221960af88
commit
6d250d092d
17 files changed
+289
-27
No files matched your search
@@ -7,7 +7,7 @@
|
||||
* 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";
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
|
||||
|
||||
const apiFetchRaw = vi.fn();
|
||||
|
||||
@@ -15,7 +15,7 @@ vi.mock("@/lib/api/api-fetch", () => ({
|
||||
apiFetchRaw: (...a: unknown[]) => apiFetchRaw(...a),
|
||||
}));
|
||||
|
||||
import { fetchAdvisor } from "../use-insights-advisor";
|
||||
import { fetchAdvisor, forceAdvisorTimeoutMs } from "../use-insights-advisor";
|
||||
|
||||
function jsonResponse(data: unknown): Response {
|
||||
return {
|
||||
@@ -74,3 +74,56 @@ describe("fetchAdvisor — explicit regenerate forces a generation", () => {
|
||||
expect(result.payload?.dailyBriefing).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe("fetchAdvisor — the regenerate waits as long as the response timeout allows (#1267)", () => {
|
||||
afterEach(() => {
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
/** A POST that only ends when its signal aborts, like a slow model. */
|
||||
function hangUntilAborted() {
|
||||
apiFetchRaw.mockImplementation(
|
||||
(_url: string, init: { signal: AbortSignal }) =>
|
||||
new Promise((_resolve, reject) => {
|
||||
init.signal.addEventListener("abort", () =>
|
||||
reject(new DOMException("aborted", "AbortError")),
|
||||
);
|
||||
}),
|
||||
);
|
||||
}
|
||||
|
||||
it("is still waiting at 45 s and at 10 min with a 600 s setting", async () => {
|
||||
vi.useFakeTimers();
|
||||
hangUntilAborted();
|
||||
let settled: unknown = null;
|
||||
void fetchAdvisor({ force: true, responseTimeoutMs: 600_000 }).then(
|
||||
(r) => (settled = r),
|
||||
);
|
||||
await vi.advanceTimersByTimeAsync(45_001);
|
||||
expect(settled).toBeNull();
|
||||
await vi.advanceTimersByTimeAsync(10 * 60_000);
|
||||
expect(settled).toBeNull();
|
||||
await vi.advanceTimersByTimeAsync(forceAdvisorTimeoutMs(600_000));
|
||||
expect(settled).toEqual({ payload: null, outcome: "timeout" });
|
||||
});
|
||||
|
||||
it("never waits less than the briefing's own 180 s per call", () => {
|
||||
// 60 s is what the payload publishes when nothing is set; the route still
|
||||
// gives each briefing call 180 s.
|
||||
expect(forceAdvisorTimeoutMs(60_000)).toBeGreaterThanOrEqual(2 * 180_000);
|
||||
expect(forceAdvisorTimeoutMs(undefined)).toBe(
|
||||
forceAdvisorTimeoutMs(60_000),
|
||||
);
|
||||
});
|
||||
|
||||
it("keeps the read path at its fast 8 s bound", async () => {
|
||||
vi.useFakeTimers();
|
||||
hangUntilAborted();
|
||||
let settled: unknown = null;
|
||||
void fetchAdvisor({ responseTimeoutMs: 600_000 }).then(
|
||||
(r) => (settled = r),
|
||||
);
|
||||
await vi.advanceTimersByTimeAsync(8_001);
|
||||
expect(settled).toEqual({ payload: null, outcome: "timeout" });
|
||||
});
|
||||
});
|
||||
@@ -713,7 +713,7 @@ function InsightsTabStripImpl({
|
||||
//
|
||||
// Refs #786 — the edge watches the COMBINED busy state (`regenerating ||
|
||||
// regenerateSettling`): a timed-out regenerate flows straight into
|
||||
// settling, so the spinner never falls at 45 s and NO error toast fires
|
||||
// settling, so the spinner never falls at the client abort and NO error toast fires
|
||||
// while the server may still deliver. The edge fires once settling
|
||||
// resolves, with the proven outcome — `"fresh"` (success) or
|
||||
// `"settle-failed"` (its own honest error copy). The mapping is the pure
|
||||
|
||||
@@ -17,7 +17,9 @@ import type {
|
||||
} from "@/lib/ai/schema";
|
||||
import { queryKeys, refetchInactiveDailyReads } from "@/lib/query-keys";
|
||||
import { apiFetchRaw } from "@/lib/api/api-fetch";
|
||||
import { useAiCapability } from "@/hooks/use-ai-capability";
|
||||
import { useAiCapability, useAiProviderState } from "@/hooks/use-ai-capability";
|
||||
import { AI_BUDGETS } from "@/lib/ai/ai-budgets";
|
||||
import { clientAbortMs } from "@/lib/ai/effective-timeout";
|
||||
import type { BriefingFailureClass } from "@/lib/insights/briefing-failure-marker";
|
||||
|
||||
/**
|
||||
@@ -116,16 +118,29 @@ 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 a generous 45 s client budget 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.
|
||||
* generation that routinely runs longer than the 8 s read budget; bounding it
|
||||
* at 8 s discarded a slow-but-successful generation.
|
||||
*
|
||||
* #1267 — the budget follows the person's response-timeout setting instead of
|
||||
* a constant 45 s, which cut every regenerate on a slow local model while the
|
||||
* server still had minutes left. The route makes up to two sequential model
|
||||
* calls (the generation and its grounding retry), and a reasoning model that
|
||||
* answered nothing is asked once more inside the first, so three calls at the
|
||||
* per-call timeout, at least the briefing's own 180 s default each. Never
|
||||
* shorter than the server: the abort is only for a request nobody answers.
|
||||
*/
|
||||
const FORCE_ADVISOR_TIMEOUT_MS = 45_000;
|
||||
const FORCE_ADVISOR_MODEL_CALLS = 3;
|
||||
|
||||
/** The client budget for a forced regenerate (exported for the test). */
|
||||
export function forceAdvisorTimeoutMs(
|
||||
responseTimeoutMs: number | null | undefined,
|
||||
): number {
|
||||
return clientAbortMs(
|
||||
responseTimeoutMs,
|
||||
FORCE_ADVISOR_MODEL_CALLS,
|
||||
AI_BUDGETS.comprehensive.timeoutMs,
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* v1.16.7 — poll cadence + ceiling while the server reports
|
||||
@@ -146,7 +161,7 @@ export const ADVISOR_REVALIDATE_POLL_MAX_ATTEMPTS = 10;
|
||||
*
|
||||
* Async-truth contract, client end (the server end is documented at the head
|
||||
* of `src/app/api/insights/generate/route.ts`): the force POST generates
|
||||
* INLINE and keeps running after the client aborts at 45 s — the server
|
||||
* INLINE and keeps running after the client aborts — the server
|
||||
* usually writes the fresh cache moments later. A page that knows work is
|
||||
* running must keep saying so until it can prove an outcome, so the timeout
|
||||
* is NOT surfaced as an error. Instead the hook remembers the baseline it was
|
||||
@@ -276,12 +291,18 @@ interface AdvisorFetchResult {
|
||||
* no-briefing-today chain). The wire-contract test pins it.
|
||||
*/
|
||||
export async function fetchAdvisor(
|
||||
options: { force?: boolean } = {},
|
||||
options: {
|
||||
force?: boolean;
|
||||
/** `ai.provider.responseTimeoutMs` from the account payload. */
|
||||
responseTimeoutMs?: number | null;
|
||||
} = {},
|
||||
): Promise<AdvisorFetchResult> {
|
||||
const controller = new AbortController();
|
||||
const timeoutHandle = setTimeout(
|
||||
() => controller.abort(),
|
||||
options.force ? FORCE_ADVISOR_TIMEOUT_MS : ADVISOR_TIMEOUT_MS,
|
||||
options.force
|
||||
? forceAdvisorTimeoutMs(options.responseTimeoutMs)
|
||||
: ADVISOR_TIMEOUT_MS,
|
||||
);
|
||||
let res: Response;
|
||||
try {
|
||||
@@ -437,6 +458,7 @@ export function useInsightsAdvisorQuery(
|
||||
enabled: boolean,
|
||||
): UseInsightsAdvisorResult {
|
||||
const briefing = useAiCapability("briefing");
|
||||
const { responseTimeoutMs } = useAiProviderState();
|
||||
const queryClient = useQueryClient();
|
||||
// Refs #786 — settling state for a timed-out force regenerate (see the
|
||||
// contract comment above `AdvisorSettleState`). `settleOutcome` carries the
|
||||
@@ -500,7 +522,7 @@ export function useInsightsAdvisorQuery(
|
||||
}
|
||||
|
||||
const mutation = useMutation({
|
||||
mutationFn: () => fetchAdvisor({ force: true }),
|
||||
mutationFn: () => fetchAdvisor({ force: true, responseTimeoutMs }),
|
||||
onSuccess: (result) => {
|
||||
if (result.payload) {
|
||||
// A genuinely fresh generation landed — write it into the shared
|
||||
@@ -509,7 +531,7 @@ export function useInsightsAdvisorQuery(
|
||||
} 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
|
||||
// client gave up, so re-read the GET to converge — but the
|
||||
// honest toast is gated on `regenerateOutcome` below, not on this
|
||||
// invalidate.
|
||||
//
|
||||
|
||||
@@ -15,6 +15,7 @@ import {
|
||||
} from "@/lib/profile/height-unit-display";
|
||||
import { useTranslations } from "@/lib/i18n/context";
|
||||
import { apiGet, apiPut } from "@/lib/api/api-fetch";
|
||||
import { useAboutMeSaveTimeoutMs } from "@/hooks/use-ai-capability";
|
||||
import { localizedApiError } from "@/lib/api/localized-error";
|
||||
import {
|
||||
AnamnesisCard,
|
||||
@@ -153,6 +154,7 @@ export function BaselineForm({
|
||||
allergies: "",
|
||||
});
|
||||
const [baseAboutMe, setBaseAboutMe] = useState("");
|
||||
const saveTimeoutMs = useAboutMeSaveTimeoutMs();
|
||||
|
||||
useEffect(() => {
|
||||
let active = true;
|
||||
@@ -229,7 +231,9 @@ export function BaselineForm({
|
||||
// collapsed card never round-trips).
|
||||
const aboutMeBody = buildAnamnesisAboutMeBody(baseAboutMe, anamnesis);
|
||||
if (aboutMeBody) {
|
||||
await apiPut("/api/coach/about-me", aboutMeBody);
|
||||
await apiPut("/api/coach/about-me", aboutMeBody, {
|
||||
signal: AbortSignal.timeout(saveTimeoutMs),
|
||||
});
|
||||
}
|
||||
}
|
||||
await onConfirmed();
|
||||
|
||||
@@ -28,6 +28,13 @@ vi.mock("@tanstack/react-query", () => ({
|
||||
useMutation: () => ({ mutate: vi.fn(), isPending: false }),
|
||||
}));
|
||||
|
||||
// #1267 — the about-me save's abort reads the account payload; the
|
||||
// react-query mock above has no client to read it from.
|
||||
vi.mock("@/hooks/use-ai-capability", async (importOriginal) => ({
|
||||
...(await importOriginal<typeof import("@/hooks/use-ai-capability")>()),
|
||||
useAboutMeSaveTimeoutMs: () => 90_000,
|
||||
}));
|
||||
|
||||
vi.mock("sonner", () => ({
|
||||
toast: { error: vi.fn(), success: vi.fn() },
|
||||
}));
|
||||
|
||||
@@ -32,6 +32,14 @@ vi.mock("@tanstack/react-query", () => ({
|
||||
};
|
||||
},
|
||||
}));
|
||||
|
||||
// #1267 — the about-me save's abort reads the account payload; the
|
||||
// react-query mock above has no client to read it from.
|
||||
vi.mock("@/hooks/use-ai-capability", async (importOriginal) => ({
|
||||
...(await importOriginal<typeof import("@/hooks/use-ai-capability")>()),
|
||||
useAboutMeSaveTimeoutMs: () => 90_000,
|
||||
}));
|
||||
|
||||
vi.mock("@/lib/api/api-fetch", () => ({
|
||||
apiGet: vi.fn(),
|
||||
apiPut: vi.fn(),
|
||||
|
||||
@@ -38,7 +38,10 @@ import { QueryErrorRow } from "@/components/ui/query-error-row";
|
||||
import { SettingsInfoTile } from "@/components/settings/_info-tile";
|
||||
import { Label } from "@/components/ui/label";
|
||||
import { ConfirmButton } from "@/components/ui/confirm-button";
|
||||
import { useAiCapability } from "@/hooks/use-ai-capability";
|
||||
import {
|
||||
useAboutMeSaveTimeoutMs,
|
||||
useAiCapability,
|
||||
} from "@/hooks/use-ai-capability";
|
||||
import { useTranslations } from "@/lib/i18n/context";
|
||||
import { queryKeys } from "@/lib/query-keys";
|
||||
import { cn } from "@/lib/utils";
|
||||
@@ -82,6 +85,7 @@ export function AboutMeNoteManager() {
|
||||
// there is a note to clear.
|
||||
const hasNote = savedNote.length > 0;
|
||||
|
||||
const saveTimeoutMs = useAboutMeSaveTimeoutMs();
|
||||
const save = useMutation({
|
||||
mutationKey: queryKeys.coachAboutMe(),
|
||||
// Send only the note. `conditions` / `coachFocus` / `allergies` are
|
||||
@@ -93,6 +97,7 @@ export function AboutMeNoteManager() {
|
||||
return apiPut<AboutMeData>(
|
||||
"/api/coach/about-me",
|
||||
withBaseToken(input, query.data?.updatedAt ?? undefined),
|
||||
{ signal: AbortSignal.timeout(saveTimeoutMs) },
|
||||
);
|
||||
},
|
||||
onSuccess: (next) => {
|
||||
|
||||
@@ -11,6 +11,7 @@ import { toastWrittenOutcome } from "@/components/outcome/outcome-toast";
|
||||
import { QueryErrorCard } from "@/components/ui/query-error-card";
|
||||
import { Switch } from "@/components/ui/switch";
|
||||
import { apiGet, apiPut } from "@/lib/api/api-fetch";
|
||||
import { useAboutMeSaveTimeoutMs } from "@/hooks/use-ai-capability";
|
||||
import {
|
||||
isConflict,
|
||||
readUpdatedAtToken,
|
||||
@@ -72,6 +73,7 @@ export function AiProfileInclusionManager() {
|
||||
(section) => draft.includes(section) !== saved.includes(section),
|
||||
);
|
||||
|
||||
const saveTimeoutMs = useAboutMeSaveTimeoutMs();
|
||||
const save = useMutation({
|
||||
mutationKey: queryKeys.coachAboutMe(),
|
||||
mutationFn: (sections: HealthProfileAiSection[]) =>
|
||||
@@ -84,6 +86,7 @@ export function AiProfileInclusionManager() {
|
||||
readUpdatedAtToken(queryClient, queryKeys.coachAboutMe()) ??
|
||||
undefined,
|
||||
),
|
||||
{ signal: AbortSignal.timeout(saveTimeoutMs) },
|
||||
),
|
||||
onSuccess: () => {
|
||||
setDraft(null);
|
||||
|
||||
@@ -36,6 +36,7 @@ import { useTranslations } from "@/lib/i18n/context";
|
||||
import { queryKeys } from "@/lib/query-keys";
|
||||
import { cn } from "@/lib/utils";
|
||||
import { apiGet, apiPut } from "@/lib/api/api-fetch";
|
||||
import { useAboutMeSaveTimeoutMs } from "@/hooks/use-ai-capability";
|
||||
import { withBaseToken, isConflict } from "@/lib/api/optimistic-token";
|
||||
|
||||
const FALLBACK_FIELD_MAX_CHARS = 500;
|
||||
@@ -75,6 +76,7 @@ export function AllergyFreeTextNote({
|
||||
const value = draft ?? savedLine;
|
||||
const dirty = value !== savedLine;
|
||||
|
||||
const saveTimeoutMs = useAboutMeSaveTimeoutMs();
|
||||
const save = useMutation({
|
||||
mutationKey: queryKeys.coachAboutMe(),
|
||||
// Send only the allergies line — omitted fields are preserved server-side
|
||||
@@ -84,6 +86,7 @@ export function AllergyFreeTextNote({
|
||||
return apiPut<AboutMeData>(
|
||||
"/api/coach/about-me",
|
||||
withBaseToken(input, query.data?.updatedAt ?? undefined),
|
||||
{ signal: AbortSignal.timeout(saveTimeoutMs) },
|
||||
);
|
||||
},
|
||||
onSuccess: () => {
|
||||
|
||||
@@ -35,6 +35,7 @@ import { useTranslations } from "@/lib/i18n/context";
|
||||
import { queryKeys } from "@/lib/query-keys";
|
||||
import { cn } from "@/lib/utils";
|
||||
import { apiGet, apiPut } from "@/lib/api/api-fetch";
|
||||
import { useAboutMeSaveTimeoutMs } from "@/hooks/use-ai-capability";
|
||||
|
||||
const FALLBACK_FIELD_MAX_CHARS = 500;
|
||||
|
||||
@@ -76,13 +77,18 @@ export function ConditionsManager() {
|
||||
value.conditions !== saved.conditions ||
|
||||
value.coachFocus !== saved.coachFocus;
|
||||
|
||||
const saveTimeoutMs = useAboutMeSaveTimeoutMs();
|
||||
const save = useMutation({
|
||||
mutationKey: queryKeys.coachAboutMe(),
|
||||
mutationFn: async (input: { conditions: string; coachFocus: string }) =>
|
||||
apiPut<AboutMeData>("/api/coach/about-me", {
|
||||
conditions: input.conditions,
|
||||
coachFocus: input.coachFocus,
|
||||
}),
|
||||
apiPut<AboutMeData>(
|
||||
"/api/coach/about-me",
|
||||
{
|
||||
conditions: input.conditions,
|
||||
coachFocus: input.coachFocus,
|
||||
},
|
||||
{ signal: AbortSignal.timeout(saveTimeoutMs) },
|
||||
),
|
||||
onSuccess: () => {
|
||||
toast.success(t("records.conditions.savedToast"));
|
||||
setDrafts({ conditions: null, coachFocus: null });
|
||||
|
||||
@@ -26,6 +26,7 @@ import { Input } from "@/components/ui/input";
|
||||
import { Label } from "@/components/ui/label";
|
||||
import { ResponsiveSheet } from "@/components/ui/responsive-sheet";
|
||||
import { apiGet, apiPost, apiPut } from "@/lib/api/api-fetch";
|
||||
import { useAboutMeSaveTimeoutMs } from "@/hooks/use-ai-capability";
|
||||
import { withBaseToken } from "@/lib/api/optimistic-token";
|
||||
import { showErrorToast } from "@/lib/errors/error-toast";
|
||||
import { useTranslations } from "@/lib/i18n/context";
|
||||
@@ -90,12 +91,14 @@ export function FreeTextTakeOver({
|
||||
showErrorToast(t("records.life.takeOver.saveError"), { cause: error }),
|
||||
});
|
||||
|
||||
const saveTimeoutMs = useAboutMeSaveTimeoutMs();
|
||||
const clear = useMutation({
|
||||
mutationKey: queryKeys.coachAboutMe(),
|
||||
mutationFn: () =>
|
||||
apiPut(
|
||||
"/api/coach/about-me",
|
||||
withBaseToken({ [field]: "" }, aboutMe.data?.updatedAt ?? undefined),
|
||||
{ signal: AbortSignal.timeout(saveTimeoutMs) },
|
||||
),
|
||||
onSuccess: () => {
|
||||
toastWrittenOutcome("success", t("records.life.takeOver.clearedToast"));
|
||||
|
||||
@@ -17,6 +17,8 @@ vi.mock("@/hooks/use-ai-capability", () => ({
|
||||
}),
|
||||
// v1.41 — the thinking depth reads the resolved block; none here.
|
||||
useCoachReasoning: () => null,
|
||||
// #1267 — the about-me save's abort, from the response-timeout setting.
|
||||
useAboutMeSaveTimeoutMs: () => 90_000,
|
||||
}));
|
||||
vi.mock("next/navigation", () => ({
|
||||
useRouter: () => ({ push: vi.fn(), replace: vi.fn() }),
|
||||
|
||||
@@ -33,6 +33,8 @@ vi.mock("@/hooks/use-ai-capability", () => ({
|
||||
reason: null,
|
||||
onDeviceAllowed: true,
|
||||
}),
|
||||
// #1267 — the about-me save's abort, from the response-timeout setting.
|
||||
useAboutMeSaveTimeoutMs: () => 90_000,
|
||||
}));
|
||||
|
||||
vi.mock("next/navigation", () => ({
|
||||
|
||||
Reference in new issue
Block a user