fix(coach): repair SSE idempotency and error-frame handling

- Drop the withIdempotency wrap on POST /api/insights/chat. The wrapper
  caches response bodies via cloned.text() then replays through
  NextResponse.json(JSON.parse(...)) — turning the SSE wire format into
  a null body on retry. The PWA never sends Idempotency-Key so the bug
  is hidden today; the iOS client would walk straight into it. Dedup
  still holds via the conversationId existence check + 20-turn cap.

- Read the SSE response body even on !response.ok when the content type
  is text/event-stream so the structured error frame the route emits
  surfaces its errorCode to the bubble. Without this the stream was
  read only on 2xx and the rich error channel was dead code.

- Stabilise the useSendCoachMessage opts reference via useRef so an
  inline-literal { onDone } from the drawer no longer re-memoises send
  on every render.

- Drop the unused snapshot parameter from streamProviderError (the
  ReturnType<typeof Object.assign> alias widened to any). Switch the
  error response to status 200 so the streaming client reads the SSE
  body and parses the error frame instead of branching on HTTP status.

Fixes Code-HIGH-1, Code-HIGH-2, Code-HIGH-3, Code-HIGH-4, Security-M-1,
Senior-HIGH-1, Simplify-S-01.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

src/app/api/insights/chat/route.ts

@@ -30,7 +30,6 @@ import { NextResponse, type NextRequest } from "next/server";
 
 import { apiHandler, requireAuth, HttpError } from "@/lib/api-handler";
 import { apiSuccess } from "@/lib/api-response";
-import { withIdempotency } from "@/lib/idempotency";
 import { annotate } from "@/lib/logging/context";
 import { prisma } from "@/lib/db";
 
@@ -227,11 +226,7 @@ Reply now as the assistant, in ${locale === "de" ? "German" : "English"}.`;
       annotate({
         action: { name: "insights.coach.noProvider" },
       });
-      return streamProviderError({
+      return streamProviderError({ code: "coach.provider.none" });
-        conversationId: workingConversationId,
-        snapshot: snapshot.provenance,
-        code: "coach.provider.none",
-      });
     }
     chain.push({ providerType: "admin-openai", instance: legacy });
   }
@@ -260,22 +255,14 @@ Reply now as the assistant, in ${locale === "de" ? "German" : "English"}.`;
           firstStatus: err.attempts[0]?.httpStatus ?? null,
         },
       });
-      return streamProviderError({
+      return streamProviderError({ code: "coach.provider.unavailable" });
-        conversationId: workingConversationId,
-        snapshot: snapshot.provenance,
-        code: "coach.provider.unavailable",
-      });
     }
     throw err;
   }
 
   const replyText = (result.content ?? "").trim();
   if (!replyText) {
-    return streamProviderError({
+    return streamProviderError({ code: "coach.provider.empty" });
-      conversationId: workingConversationId,
-      snapshot: snapshot.provenance,
-      code: "coach.provider.empty",
-    });
   }
 
   // Persist the assistant message BEFORE we begin streaming; if the
@@ -403,11 +390,7 @@ async function streamRefusal(args: {
   return new Response(stream, { status: 200, headers: SSE_HEADERS });
 }
 
-function streamProviderError(args: {
+function streamProviderError(args: { code: string }): Response {
-  conversationId: string;
-  snapshot: ReturnType<typeof Object.assign>;
-  code: string;
-}): Response {
   const stream = new ReadableStream<Uint8Array>({
     start(controller) {
       try {
@@ -423,33 +406,22 @@ function streamProviderError(args: {
       }
     },
   });
-  return new Response(stream, { status: 503, headers: SSE_HEADERS });
+  // Status 200 so the streaming client reads the SSE body and parses
+  // the structured `error` frame (HTTP-status branches drop the
+  // structured code on the floor).
+  return new Response(stream, { status: 200, headers: SSE_HEADERS });
 }
 
-const handler = apiHandler(async (request: NextRequest) => {
+// Idempotency is intentionally NOT applied to this SSE-streaming route.
-  // Idempotency wrap is applied only when the user is creating a new
+// `withIdempotency()` caches the response body via `cloned.text()` and
-  // conversation (the spec). For follow-up turns inside an existing
+// replays it through `NextResponse.json(JSON.parse(...))` — that path
-  // thread, retries SHOULD hit the route again — every turn is a real
+// turns an SSE wire format (`data: …\n\n` frames) into a `null` body
-  // server-side action, not a duplicate-prone POST.
+// because the cached text isn't JSON. The PWA never sets
-  let cloneForCheck: NextRequest | undefined = undefined;
+// `Idempotency-Key` here so the bug is invisible today, but the iOS
-  let conversationId: string | undefined = undefined;
+// client does. Dedup still holds: a duplicate first-turn POST creates
-  try {
+// a second conversation row (cheap), and follow-up turns are gated by
-    cloneForCheck = request.clone() as NextRequest;
+// the conversationId existence check + 20-turn cap.
-    const body = await cloneForCheck.json();
+export const POST = apiHandler(handleChatRequest);
-    if (typeof body?.conversationId === "string") {
-      conversationId = body.conversationId;
-    }
-  } catch {
-    // fall through; validation will surface the issue inside the
-    // handler.
-  }
-  if (conversationId) {
-    return handleChatRequest(request);
-  }
-  return withIdempotency(handleChatRequest)(request);
-});
-
-export const POST = handler;
 
 /**
  * GET /api/insights/chat?cursor=<id>&limit=<n>

src/components/insights/coach-panel/use-coach.ts

@@ -1,6 +1,6 @@
 "use client";
 
-import { useCallback, useRef, useState } from "react";
+import { useCallback, useEffect, useRef, useState } from "react";
 import {
   useMutation,
   useQuery,
@@ -243,6 +243,14 @@ export function useSendCoachMessage(opts: UseSendCoachMessageOptions = {}) {
     useState<CoachStreamingMessage>(EMPTY_STREAMING);
   const abortRef = useRef<AbortController | null>(null);
 
+  // Stash `opts` in a ref so callers can pass an inline-literal object
+  // (`useSendCoachMessage({ onDone: … })`) without re-memoising `send`
+  // on every render. The hook owns the latest-callback-wins contract.
+  const optsRef = useRef(opts);
+  useEffect(() => {
+    optsRef.current = opts;
+  }, [opts]);
+
   const reset = useCallback(() => {
     setStreaming(EMPTY_STREAMING);
   }, []);
@@ -290,7 +298,27 @@ export function useSendCoachMessage(opts: UseSendCoachMessageOptions = {}) {
         return;
       }
 
-      if (!response.ok || !response.body) {
+      if (!response.body) {
+        setStreaming({
+          content: "",
+          metricSource: null,
+          inProgress: false,
+          messageId: null,
+          errorCode: `coach.http.${response.status}`,
+        });
+        return;
+      }
+
+      // Even on `!response.ok`, the route may have emitted a structured
+      // `error` frame in an SSE body (e.g. 4xx/5xx + text/event-stream).
+      // Falling back to `coach.http.<status>` would discard the richer
+      // `errorCode`. Read the stream and let the parser route the frame
+      // through the same path as the success case.
+      const contentType = response.headers.get("Content-Type") ?? "";
+      const isEventStream = contentType
+        .toLowerCase()
+        .startsWith("text/event-stream");
+      if (!response.ok && !isEventStream) {
         setStreaming({
           content: "",
           metricSource: null,
@@ -375,10 +403,10 @@ export function useSendCoachMessage(opts: UseSendCoachMessageOptions = {}) {
           queryKey: QUERY_KEYS.one(resolvedConversationId),
         });
         queryClient.invalidateQueries({ queryKey: QUERY_KEYS.list() });
-        opts.onDone?.(resolvedConversationId);
+        optsRef.current.onDone?.(resolvedConversationId);
       }
     },
-    [opts, queryClient],
+    [queryClient],
   );
 
   const cancel = useCallback(() => {