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({
-        conversationId: workingConversationId,
-        snapshot: snapshot.provenance,
-        code: "coach.provider.none",
-      });
+      return streamProviderError({ 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({
-        conversationId: workingConversationId,
-        snapshot: snapshot.provenance,
-        code: "coach.provider.unavailable",
-      });
+      return streamProviderError({ code: "coach.provider.unavailable" });
     }
     throw err;
   }
 
   const replyText = (result.content ?? "").trim();
   if (!replyText) {
-    return streamProviderError({
-      conversationId: workingConversationId,
-      snapshot: snapshot.provenance,
-      code: "coach.provider.empty",
-    });
+    return streamProviderError({ 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: {
-  conversationId: string;
-  snapshot: ReturnType<typeof Object.assign>;
-  code: string;
-}): Response {
+function streamProviderError(args: { 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 wrap is applied only when the user is creating a new
-  // conversation (the spec). For follow-up turns inside an existing
-  // thread, retries SHOULD hit the route again — every turn is a real
-  // server-side action, not a duplicate-prone POST.
-  let cloneForCheck: NextRequest | undefined = undefined;
-  let conversationId: string | undefined = undefined;
-  try {
-    cloneForCheck = request.clone() as NextRequest;
-    const body = await cloneForCheck.json();
-    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;
+// Idempotency is intentionally NOT applied to this SSE-streaming route.
+// `withIdempotency()` caches the response body via `cloned.text()` and
+// replays it through `NextResponse.json(JSON.parse(...))` — that path
+// turns an SSE wire format (`data: …\n\n` frames) into a `null` body
+// because the cached text isn't JSON. The PWA never sets
+// `Idempotency-Key` here so the bug is invisible today, but the iOS
+// client does. Dedup still holds: a duplicate first-turn POST creates
+// a second conversation row (cheap), and follow-up turns are gated by
+// the conversationId existence check + 20-turn cap.
+export const POST = apiHandler(handleChatRequest);
 
 /**
  * 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(() => {