mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-13 14:32:03 +00:00
coach: close an aborted turn with a persisted cancelled marker
Navigating away mid-reply aborts the SSE fetch; the provider call tears down, but the conversation was left with the user message dangling and no trace of the interruption. Persist an empty assistant row tagged providerType "cancelled" (the refusal/nudge sentinel channel) when the request signal is aborted, refund stays intact, and the abort is annotated as insights.coach.cancelled instead of a provider failure or a GlitchTip defect. Cancelled markers are filtered out of the provider transcript on later turns. Refs #781
This commit is contained in:
1 parent
12e5817c16
commit
9a631c8f8b
4 files changed
+300
-6
No files matched your search
@@ -28647,7 +28647,9 @@ components:
|
||||
anyOf:
|
||||
- type: string
|
||||
- type: "null"
|
||||
description: Provider that produced the reply (e.g. anthropic, openai, local, refusal); null for user turns.
|
||||
description: Provider that produced the reply (e.g. anthropic, openai, local, refusal). The sentinel `cancelled` marks
|
||||
an empty assistant row closing a turn the client aborted mid-generation (navigation away); clients should
|
||||
render it as an interrupted turn with a retry affordance. Null for user turns.
|
||||
promptVersion:
|
||||
anyOf:
|
||||
- type: string
|
||||
|
||||
@@ -0,0 +1,246 @@
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
|
||||
/**
|
||||
* #781 — a client that navigates away mid-generation aborts the SSE fetch.
|
||||
* The route must close the turn honestly: persist an EMPTY assistant marker
|
||||
* with `providerType: "cancelled"` so the conversation shows the interrupted
|
||||
* turn (and the client can offer a retry), instead of leaving the user
|
||||
* message dangling unanswered. The reservation is refunded in full, and the
|
||||
* abort is never misreported as a provider failure.
|
||||
*
|
||||
* Route-level on purpose: the abort branch lives in `route.ts`'s catch, a
|
||||
* place no pure-module test can see (the #591 lesson).
|
||||
*/
|
||||
|
||||
const SNAPSHOT_JSON = '{"bp":{"aggregate":{"mean":128}}}';
|
||||
|
||||
vi.mock("@/lib/api-handler", () => ({
|
||||
apiHandler: <T extends (...args: unknown[]) => unknown>(fn: T) => fn,
|
||||
requireAuth: vi.fn(async () => ({ user: { id: "u1", locale: "en" } })),
|
||||
HttpError: class HttpError extends Error {
|
||||
status: number;
|
||||
constructor(status: number, message: string) {
|
||||
super(message);
|
||||
this.status = status;
|
||||
}
|
||||
},
|
||||
}));
|
||||
vi.mock("@/lib/modules/gate", () => ({
|
||||
requireModuleEnabled: vi.fn(async () => ({ enabled: true })),
|
||||
isModuleEnabled: vi.fn(async () => true),
|
||||
}));
|
||||
vi.mock("@/lib/feature-flags", () => ({
|
||||
requireAssistantSurface: vi.fn(async () => undefined),
|
||||
}));
|
||||
vi.mock("@/lib/api-response", () => ({
|
||||
apiError: (error: string, status: number) => ({ data: null, error, status }),
|
||||
apiSuccess: (data: unknown) => ({ data, error: null, status: 200 }),
|
||||
}));
|
||||
const { annotate } = vi.hoisted(() => ({ annotate: vi.fn() }));
|
||||
vi.mock("@/lib/logging/context", () => ({
|
||||
annotate,
|
||||
getEvent: vi.fn(() => null),
|
||||
}));
|
||||
vi.mock("@/lib/auth/audit", () => ({ auditLog: vi.fn() }));
|
||||
vi.mock("@/lib/db", () => ({
|
||||
prisma: {
|
||||
user: { findUnique: vi.fn(async () => ({ coachPrefsJson: null })) },
|
||||
coachConversation: { findFirst: vi.fn(async () => ({ id: "c1" })) },
|
||||
workout: { findFirst: vi.fn(async () => null) },
|
||||
},
|
||||
}));
|
||||
vi.mock("@/lib/rate-limit", () => ({
|
||||
checkRateLimit: vi.fn(async () => ({ allowed: true })),
|
||||
}));
|
||||
vi.mock("@/lib/i18n/server-locale", () => ({
|
||||
resolveServerLocale: vi.fn(async () => "en"),
|
||||
}));
|
||||
|
||||
const { runStreamingRawCompletionWithFallback } = vi.hoisted(() => ({
|
||||
runStreamingRawCompletionWithFallback: vi.fn(),
|
||||
}));
|
||||
vi.mock("@/lib/ai/provider-runner", () => ({
|
||||
AllProvidersFailedError: class extends Error {},
|
||||
runStreamingRawCompletionWithFallback,
|
||||
}));
|
||||
|
||||
const { resolveProviderChain } = vi.hoisted(() => ({
|
||||
resolveProviderChain: vi.fn(),
|
||||
}));
|
||||
vi.mock("@/lib/ai/provider", () => ({
|
||||
resolveProviderChain,
|
||||
resolveProvider: vi.fn(),
|
||||
}));
|
||||
vi.mock("@/lib/ai/consent-guard", () => ({
|
||||
assertConsentForChain: vi.fn(async () => undefined),
|
||||
}));
|
||||
vi.mock("@/lib/ai/prompts/insight-generator", () => ({ PROMPT_VERSION: "x" }));
|
||||
vi.mock("@/lib/ai/ai-budgets", () => ({
|
||||
AI_BUDGETS: { coach: { maxTokens: 1500, temperature: 0.4 } },
|
||||
}));
|
||||
|
||||
vi.mock("@/lib/ai/coach/persistence", () => ({
|
||||
appendMessage: vi.fn(async () => ({ id: "m1" })),
|
||||
createConversation: vi.fn(async () => ({ id: "c1" })),
|
||||
fetchConversationWithMessages: vi.fn(),
|
||||
listConversations: vi.fn(),
|
||||
}));
|
||||
vi.mock("@/lib/ai/coach/coach-memory-shared", () => ({
|
||||
enqueueCoachMemoryRefresh: vi.fn(),
|
||||
}));
|
||||
vi.mock("@/lib/ai/coach/facts", () => ({
|
||||
storeDeterministicFacts: vi.fn(async () => undefined),
|
||||
}));
|
||||
|
||||
const { reserveBudget, reconcileSpend } = vi.hoisted(() => ({
|
||||
reserveBudget: vi.fn(async () => ({ allowed: true, reserved: 3000 })),
|
||||
reconcileSpend: vi.fn(async () => undefined),
|
||||
}));
|
||||
vi.mock("@/lib/ai/coach/budget", () => ({
|
||||
buildDateKey: vi.fn(() => "2026-08-13"),
|
||||
reserveBudget,
|
||||
reconcileSpend,
|
||||
resolveDailyCap: vi.fn(() => 2_000_000),
|
||||
}));
|
||||
|
||||
vi.mock("@/lib/ai/coach/refusal", () => ({
|
||||
detectRefusal: vi.fn(() => ({ refuse: false })),
|
||||
}));
|
||||
vi.mock("@/lib/ai/coach/system-prompt", () => ({
|
||||
getCoachSystemPrompt: vi.fn(() => "SYSTEM"),
|
||||
}));
|
||||
vi.mock("@/lib/ai/coach/about-me", () => ({
|
||||
getSelfContextTextForUser: vi.fn(async () => null),
|
||||
}));
|
||||
vi.mock("@/lib/ai/coach/snapshot", () => ({
|
||||
buildCoachSnapshot: vi.fn(async () => ({
|
||||
snapshotJson: SNAPSHOT_JSON,
|
||||
sections: { bloodPressure: { aggregate: { mean: 128 } } },
|
||||
provenance: { windows: ["last30days"], metrics: ["bp"] },
|
||||
referenceGrounding: "REFERENCE RANGES",
|
||||
})),
|
||||
}));
|
||||
vi.mock("@/lib/workouts/hr-series", () => ({
|
||||
buildWorkoutHrSeries: vi.fn(async () => null),
|
||||
}));
|
||||
vi.mock("@/lib/workouts/zones", () => ({
|
||||
computeZones: vi.fn(() => null),
|
||||
hrMaxFromAge: vi.fn(() => 185),
|
||||
parseWhoopZoneDurations: vi.fn(() => null),
|
||||
}));
|
||||
vi.mock("@/lib/workouts/sport-context", () => ({
|
||||
buildSportContext: vi.fn(async () => null),
|
||||
}));
|
||||
|
||||
vi.mock("@/lib/ai/coach/tools", () => ({
|
||||
COACH_TOOL_DEFS: [{ name: "get_metric_series" }],
|
||||
MAX_ROUNDS: 3,
|
||||
buildCoachDataInventory: vi.fn(async () => ({
|
||||
entries: [],
|
||||
restMode: false,
|
||||
cycleEnabled: false,
|
||||
window: "last30days",
|
||||
probeScope: { sources: ["bp"], window: "last30days" },
|
||||
})),
|
||||
renderDataInventory: vi.fn(() => "DATA INVENTORY"),
|
||||
renderFocusHint: vi.fn(() => ""),
|
||||
buildToolModeAddendum: vi.fn(() => "TOOL ADDENDUM"),
|
||||
runCoachToolLoop: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock("@/lib/ai/coach/keyvalues", () => ({
|
||||
parseKeyValuesSentinel: vi.fn(),
|
||||
}));
|
||||
vi.mock("@/lib/ai/coach/suggest-reminder", () => ({
|
||||
parseSuggestReminder: vi.fn(),
|
||||
}));
|
||||
vi.mock("@/lib/ai/coach/suggest-gate", () => ({ gateSuggestion: vi.fn() }));
|
||||
vi.mock("@/lib/validations/coach-prefs", () => ({
|
||||
parseCoachPrefs: vi.fn(() => ({ defaultWindow: undefined })),
|
||||
DEFAULT_REMINDER_SUGGESTION_PREFS: {},
|
||||
}));
|
||||
|
||||
const { appendMessage } = await import("@/lib/ai/coach/persistence");
|
||||
|
||||
const sse = vi.hoisted(() => ({ done: Promise.resolve() as Promise<unknown> }));
|
||||
vi.mock("@/lib/sse/create-stream", () => ({
|
||||
createSseStream: (
|
||||
producer: (c: {
|
||||
signal: { aborted: boolean };
|
||||
enqueue: () => void;
|
||||
}) => void | Promise<void>,
|
||||
) => {
|
||||
sse.done = Promise.resolve(
|
||||
producer({ signal: { aborted: true }, enqueue: () => {} }),
|
||||
);
|
||||
return new ReadableStream();
|
||||
},
|
||||
}));
|
||||
|
||||
import { POST } from "../route";
|
||||
|
||||
const post = POST as unknown as (req: Request) => Promise<Response>;
|
||||
|
||||
function abortedChatReq(body: Record<string, unknown>): Request {
|
||||
const controller = new AbortController();
|
||||
const req = new Request("http://localhost/api/insights/chat", {
|
||||
method: "POST",
|
||||
headers: { "Content-Type": "application/json" },
|
||||
body: JSON.stringify(body),
|
||||
signal: controller.signal,
|
||||
});
|
||||
// The body is read synchronously by the handler before the provider call;
|
||||
// aborting here models the client tearing the connection down while the
|
||||
// provider is generating (the runner rejects below).
|
||||
controller.abort();
|
||||
return req;
|
||||
}
|
||||
|
||||
function abortError(): Error {
|
||||
const err = new Error("This operation was aborted");
|
||||
err.name = "AbortError";
|
||||
return err;
|
||||
}
|
||||
|
||||
describe("coach chat — client abort mid-generation (#781)", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
reserveBudget.mockResolvedValue({ allowed: true, reserved: 3000 });
|
||||
// No-tools chain so the turn runs the streaming runner (which rejects
|
||||
// with the abort the torn-down connection produces).
|
||||
resolveProviderChain.mockResolvedValue([
|
||||
{ providerType: "local", instance: { supportsTools: false } },
|
||||
]);
|
||||
runStreamingRawCompletionWithFallback.mockRejectedValue(abortError());
|
||||
});
|
||||
|
||||
it("persists an empty assistant marker with providerType 'cancelled'", async () => {
|
||||
const res = await post(abortedChatReq({ message: "How is my BP?" }));
|
||||
expect(res.status).toBe(200);
|
||||
await sse.done;
|
||||
|
||||
const calls = (appendMessage as ReturnType<typeof vi.fn>).mock.calls;
|
||||
// The user turn is persisted first, then the cancelled marker.
|
||||
const roles = calls.map((c) => (c[0] as { role: string }).role);
|
||||
expect(roles).toEqual(["user", "assistant"]);
|
||||
const marker = calls[1][0] as {
|
||||
role: string;
|
||||
content: string;
|
||||
providerType: string | null;
|
||||
};
|
||||
expect(marker.providerType).toBe("cancelled");
|
||||
expect(marker.content).toBe("");
|
||||
});
|
||||
|
||||
it("refunds the full reservation and annotates the abort as cancelled, not a provider failure", async () => {
|
||||
await post(abortedChatReq({ message: "How is my BP?" }));
|
||||
await sse.done;
|
||||
|
||||
expect(reconcileSpend).toHaveBeenCalledWith("u1", 3000, 0, "2026-08-13");
|
||||
const actions = annotate.mock.calls
|
||||
.map((c) => (c[0] as { action?: { name?: string } }).action?.name)
|
||||
.filter(Boolean);
|
||||
expect(actions).toContain("insights.coach.cancelled");
|
||||
expect(actions).not.toContain("insights.coach.providerFailed");
|
||||
});
|
||||
});
|
||||
@@ -325,10 +325,16 @@ async function handleChatRequest(request: NextRequest): Promise<Response> {
|
||||
}
|
||||
workingConversationId = existing.id;
|
||||
priorSummary = existing.summary ?? null;
|
||||
priorTurns = existing.messages.map((m) => ({
|
||||
role: m.role,
|
||||
content: m.content,
|
||||
}));
|
||||
// #781 — cancelled-turn markers (`providerType: "cancelled"`, empty body)
|
||||
// are UI rows, not conversation content: keep them out of the provider
|
||||
// transcript so an interrupted turn adds no empty assistant line to the
|
||||
// prompt. They stay in the DTO the client renders.
|
||||
priorTurns = existing.messages
|
||||
.filter((m) => m.providerType !== "cancelled")
|
||||
.map((m) => ({
|
||||
role: m.role,
|
||||
content: m.content,
|
||||
}));
|
||||
// Ledger cross-turn sources (D3-safe): user-authored numbers + the
|
||||
// persisted tool figures of prior turns. Assistant PROSE is never read.
|
||||
priorUserMessages = existing.messages
|
||||
@@ -761,6 +767,35 @@ async function handleChatRequest(request: NextRequest): Promise<Response> {
|
||||
await reconcileSpend(userId, reservation.reserved, 0, reqDateKey).catch(
|
||||
() => {},
|
||||
);
|
||||
// #781 — the client walked away mid-generation. The request's abort
|
||||
// signal is threaded into every provider call, so the teardown surfaces
|
||||
// here as an abort-shaped failure with `request.signal` already flipped.
|
||||
// Close the turn honestly instead of letting the user message dangle
|
||||
// unanswered: persist an EMPTY assistant marker tagged
|
||||
// `providerType: "cancelled"` (the same channel "refusal" and "nudge"
|
||||
// already use for non-provider rows) so the thread shows the turn as
|
||||
// interrupted and offers a retry on reload. The content is empty by
|
||||
// construction — nothing guarded was produced, and anything the client
|
||||
// ever SAW was persisted before the first token frame left, so no
|
||||
// unguarded partial text is ever written. The reservation was refunded
|
||||
// above; a retry pays exactly what a fresh turn pays.
|
||||
if (request.signal.aborted) {
|
||||
await appendMessage({
|
||||
conversationId: workingConversationId,
|
||||
role: "assistant",
|
||||
content: "",
|
||||
providerType: "cancelled",
|
||||
promptVersion: PROMPT_VERSION,
|
||||
}).catch(() => {
|
||||
// Marker persistence is best-effort — a failure leaves the
|
||||
// dangling user turn, which is exactly the pre-#781 state.
|
||||
});
|
||||
annotate({
|
||||
action: { name: "insights.coach.cancelled" },
|
||||
meta: { conversationId: workingConversationId },
|
||||
});
|
||||
return { ok: false, code: "coach.cancelled" };
|
||||
}
|
||||
if (err instanceof AllProvidersFailedError) {
|
||||
annotate({
|
||||
action: { name: "insights.coach.providerFailed" },
|
||||
@@ -1241,6 +1276,17 @@ async function handleChatRequest(request: NextRequest): Promise<Response> {
|
||||
outcome = await produceReply();
|
||||
} catch (err) {
|
||||
clearInterval(heartbeat);
|
||||
// #781 — a client disconnect can also surface as an abort-shaped error
|
||||
// on a path `produceReply`'s own catch does not own (e.g. a teardown
|
||||
// rejection out of persistence). That is not a defect and nobody is
|
||||
// listening: skip the GlitchTip forward and the error frame.
|
||||
if (request.signal.aborted) {
|
||||
annotate({
|
||||
action: { name: "insights.coach.cancelled" },
|
||||
meta: { conversationId: workingConversationId, lateAbort: true },
|
||||
});
|
||||
return;
|
||||
}
|
||||
// The stream is already open, so we cannot 500: a tagged provider error
|
||||
// surfaces a graceful `coach.provider.*` frame; anything else degrades to
|
||||
// a generic unavailable frame. A genuine defect is still annotated.
|
||||
|
||||
@@ -540,7 +540,7 @@ const coachMessageSchema = z
|
||||
.string()
|
||||
.nullable()
|
||||
.describe(
|
||||
"Provider that produced the reply (e.g. anthropic, openai, local, refusal); null for user turns.",
|
||||
"Provider that produced the reply (e.g. anthropic, openai, local, refusal). The sentinel `cancelled` marks an empty assistant row closing a turn the client aborted mid-generation (navigation away); clients should render it as an interrupted turn with a retry affordance. Null for user turns.",
|
||||
),
|
||||
promptVersion: z
|
||||
.string()
|
||||
|
||||
Reference in new issue
Block a user