fix(coach): skeleton + save toast on prefs sheet

Render a skeleton shell while the persisted Coach prefs are loading
so the form never paints DEFAULT_COACH_PREFS as a ghost between mount
and the query resolving. Emit a sonner toast on save success before
closing the sheet so the action is visibly acknowledged.

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

src/components/insights/coach-panel/__tests__/coach-settings-sheet.test.tsx

@@ -0,0 +1,107 @@
+import { describe, it, expect, vi, beforeEach } from "vitest";
+import { renderToStaticMarkup } from "react-dom/server";
+import { toast } from "sonner";
+
+import { I18nProvider } from "@/lib/i18n/context";
+import { DEFAULT_COACH_PREFS } from "@/lib/validations/coach-prefs";
+
+// v1.4.23 W6 (Design-H3 + H4) — verify the settings sheet renders a
+// skeleton placeholder while the persisted prefs are loading (no
+// `DEFAULT_COACH_PREFS` ghost form) and emits a sonner toast on save
+// success.
+//
+// The shadcn `<Sheet>` wraps Radix Dialog which uses Portals — those
+// don't materialise under `renderToStaticMarkup`. We mock the sheet
+// primitives down to plain wrappers so the form/skeleton body is
+// reachable in the static markup.
+
+const queryState: { data: unknown } = { data: undefined };
+const mutationCallbacks: { onSuccess: ((env: unknown) => void) | null } = {
+  onSuccess: null,
+};
+
+vi.mock("@tanstack/react-query", () => ({
+  useQuery: () => ({ data: queryState.data }),
+  useMutation: (opts: { onSuccess?: (env: unknown) => void }) => {
+    mutationCallbacks.onSuccess = opts.onSuccess ?? null;
+    return {
+      mutate: (next: unknown) => {
+        mutationCallbacks.onSuccess?.({ data: next });
+      },
+      isPending: false,
+    };
+  },
+  useQueryClient: () => ({
+    setQueryData: vi.fn(),
+  }),
+}));
+
+vi.mock("sonner", () => ({
+  toast: { success: vi.fn() },
+}));
+
+vi.mock("@/components/ui/sheet", () => ({
+  Sheet: ({ children, open }: { children: React.ReactNode; open: boolean }) =>
+    open ? <div data-slot="mock-sheet">{children}</div> : null,
+  SheetContent: ({ children }: { children: React.ReactNode }) => (
+    <div data-slot="mock-sheet-content">{children}</div>
+  ),
+  SheetHeader: ({ children }: { children: React.ReactNode }) => (
+    <div>{children}</div>
+  ),
+  SheetTitle: ({ children }: { children: React.ReactNode }) => (
+    <h2>{children}</h2>
+  ),
+  SheetDescription: ({ children }: { children: React.ReactNode }) => (
+    <p>{children}</p>
+  ),
+}));
+
+import { CoachSettingsSheet } from "../coach-settings-sheet";
+
+function render(node: React.ReactNode) {
+  return renderToStaticMarkup(
+    <I18nProvider initialLocale="en">{node}</I18nProvider>,
+  );
+}
+
+beforeEach(() => {
+  queryState.data = undefined;
+  mutationCallbacks.onSuccess = null;
+  vi.mocked(toast.success).mockClear();
+});
+
+describe("<CoachSettingsSheet>", () => {
+  it("renders a skeleton shell while persisted prefs are loading (Design-H3)", () => {
+    queryState.data = undefined;
+    const html = render(
+      <CoachSettingsSheet open={true} onOpenChange={() => {}} />,
+    );
+    expect(html).toContain('data-slot="coach-prefs-skeleton"');
+    // Form body markers must be absent during the skeleton state so
+    // the user never sees the DEFAULT_COACH_PREFS ghost form.
+    expect(html).not.toContain('data-slot="coach-prefs-tone"');
+    expect(html).not.toContain('data-slot="coach-prefs-evidence"');
+  });
+
+  it("renders the form once persisted prefs land", () => {
+    queryState.data = DEFAULT_COACH_PREFS;
+    const html = render(
+      <CoachSettingsSheet open={true} onOpenChange={() => {}} />,
+    );
+    expect(html).not.toContain('data-slot="coach-prefs-skeleton"');
+    expect(html).toContain('data-slot="coach-prefs-tone"');
+    expect(html).toContain('data-slot="coach-prefs-evidence"');
+  });
+
+  it("fires a sonner toast on save success (Design-H4)", () => {
+    queryState.data = DEFAULT_COACH_PREFS;
+    render(<CoachSettingsSheet open={true} onOpenChange={() => {}} />);
+    // Static markup doesn't dispatch click events; trigger the captured
+    // mutation success path directly. Verifies the wired-up onSuccess
+    // emits the toast with the localised "Saved." string.
+    mutationCallbacks.onSuccess?.({ data: DEFAULT_COACH_PREFS });
+    expect(vi.mocked(toast.success)).toHaveBeenCalledTimes(1);
+    expect(vi.mocked(toast.success)).toHaveBeenCalledWith("Saved.");
+  });
+});

src/components/insights/coach-panel/coach-settings-sheet.tsx

@@ -1,8 +1,9 @@
 "use client";
 
 import { useState } from "react";
-import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query";
+import { useMutation, useQueryClient } from "@tanstack/react-query";
 import { Loader2 } from "lucide-react";
+import { toast } from "sonner";
 
 import {
   Sheet,
@@ -21,7 +22,9 @@ import {
   SelectTrigger,
   SelectValue,
 } from "@/components/ui/select";
+import { Skeleton } from "@/components/ui/skeleton";
 import { useTranslations } from "@/lib/i18n/context";
+import { useCoachPrefs } from "@/hooks/use-coach-prefs";
 import {
   DEFAULT_COACH_PREFS,
   type CoachExcludeMetric,
@@ -43,6 +46,13 @@ import {
  * The four controls match the v1.4.23 H4 spec exactly: tone (3-state),
  * verbosity (3-state), excludeMetrics (multi-toggle), and
  * showEvidenceByDefault (single toggle).
+ *
+ * v1.4.23 W6 design-pass:
+ * - Skeleton placeholder while the prefs are loading (Design-H3) so
+ *   the user never sees the `DEFAULT_COACH_PREFS` ghost form snap to
+ *   their saved values mid-paint.
+ * - On save success the sheet emits a sonner toast (Design-H4) before
+ *   closing so the action is visibly acknowledged.
  */
 export interface CoachSettingsSheetProps {
   open: boolean;
@@ -68,16 +78,7 @@ export function CoachSettingsSheet({
   const { t } = useTranslations();
   const queryClient = useQueryClient();
 
-  const { data: persisted } = useQuery({
+  const { data: persisted } = useCoachPrefs({ enabled: open });
-    queryKey: ["coach-prefs"],
-    queryFn: async () => {
-      const res = await fetch("/api/auth/me/coach-prefs");
-      if (!res.ok) throw new Error("coach-prefs.fetch_failed");
-      const env = (await res.json()) as { data: CoachPrefs };
-      return env.data;
-    },
-    enabled: open,
-  });
 
   // Local working copy edited by the form. Re-syncs to the persisted
   // shape on every fetch via the same in-render setState pattern as
@@ -105,6 +106,7 @@ export function CoachSettingsSheet({
     },
     onSuccess: (envelope) => {
       queryClient.setQueryData(["coach-prefs"], envelope.data);
+      toast.success(t("insights.coach.settingsSaved"));
       onOpenChange(false);
     },
   });
@@ -118,6 +120,11 @@ export function CoachSettingsSheet({
     }));
   }
 
+  // Render a skeleton shell while the persisted prefs are loading so
+  // we never render the form against `DEFAULT_COACH_PREFS` and snap
+  // the controls when the fetch resolves a moment later.
+  const isLoading = open && !persisted;
+
   return (
     <Sheet open={open} onOpenChange={onOpenChange}>
       <SheetContent
@@ -134,6 +141,33 @@ export function CoachSettingsSheet({
           </SheetDescription>
         </SheetHeader>
 
+        {isLoading ? (
+          <div
+            data-slot="coach-prefs-skeleton"
+            className="flex min-h-0 flex-1 flex-col gap-5 overflow-y-auto p-4"
+          >
+            <div className="flex flex-col gap-2">
+              <Skeleton className="h-3 w-20" />
+              <Skeleton className="h-9 w-full" />
+            </div>
+            <div className="flex flex-col gap-2">
+              <Skeleton className="h-3 w-24" />
+              <Skeleton className="h-9 w-full" />
+            </div>
+            <div className="flex flex-col gap-2">
+              <Skeleton className="h-3 w-28" />
+              <Skeleton className="h-3 w-3/4" />
+              <Skeleton className="h-44 w-full" />
+            </div>
+            <div className="flex items-start justify-between gap-3">
+              <div className="flex flex-1 flex-col gap-1">
+                <Skeleton className="h-3 w-32" />
+                <Skeleton className="h-3 w-2/3" />
+              </div>
+              <Skeleton className="h-5 w-9 rounded-full" />
+            </div>
+          </div>
+        ) : (
           <div className="flex min-h-0 flex-1 flex-col gap-5 overflow-y-auto p-4">
             {/* Tone */}
             <div className="flex flex-col gap-2">
@@ -233,7 +267,9 @@ export function CoachSettingsSheet({
                         id={id}
                         data-slot={`coach-prefs-exclude-${metric}`}
                         checked={checked}
-                      onCheckedChange={(next) => toggleExclude(metric, next)}
+                        onCheckedChange={(next) =>
+                          toggleExclude(metric, next)
+                        }
                       />
                     </li>
                   );
@@ -267,6 +303,7 @@ export function CoachSettingsSheet({
               />
             </div>
           </div>
+        )}
 
         <div className="border-border/70 flex items-center justify-end gap-2 border-t p-4">
           <Button
@@ -282,7 +319,7 @@ export function CoachSettingsSheet({
             type="button"
             size="sm"
             onClick={() => save.mutate(draft)}
-            disabled={save.isPending}
+            disabled={save.isPending || isLoading}
             data-slot="coach-prefs-save"
           >
             {save.isPending ? (