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({
-    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,
-  });
+  const { data: persisted } = useCoachPrefs({ 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,139 +141,169 @@ export function CoachSettingsSheet({
           </SheetDescription>
         </SheetHeader>
 
-        <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">
-            <Label
-              htmlFor="coach-prefs-tone"
-              className="text-xs font-medium"
-            >
-              {t("insights.coach.settingsToneLabel")}
-            </Label>
-            <Select
-              value={draft.tone}
-              onValueChange={(value) =>
-                setDraft((prev) => ({ ...prev, tone: value as CoachTone }))
-              }
-            >
-              <SelectTrigger
-                id="coach-prefs-tone"
-                data-slot="coach-prefs-tone"
-              >
-                <SelectValue />
-              </SelectTrigger>
-              <SelectContent>
-                <SelectItem value="warm">
-                  {t("insights.coach.settingsToneWarm")}
-                </SelectItem>
-                <SelectItem value="neutral">
-                  {t("insights.coach.settingsToneNeutral")}
-                </SelectItem>
-                <SelectItem value="concise">
-                  {t("insights.coach.settingsToneConcise")}
-                </SelectItem>
-              </SelectContent>
-            </Select>
+        {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>
-
-          {/* Verbosity */}
-          <div className="flex flex-col gap-2">
-            <Label
-              htmlFor="coach-prefs-verbosity"
-              className="text-xs font-medium"
-            >
-              {t("insights.coach.settingsVerbosityLabel")}
-            </Label>
-            <Select
-              value={draft.verbosity}
-              onValueChange={(value) =>
-                setDraft((prev) => ({
-                  ...prev,
-                  verbosity: value as CoachVerbosity,
-                }))
-              }
-            >
-              <SelectTrigger
-                id="coach-prefs-verbosity"
-                data-slot="coach-prefs-verbosity"
-              >
-                <SelectValue />
-              </SelectTrigger>
-              <SelectContent>
-                <SelectItem value="brief">
-                  {t("insights.coach.settingsVerbosityBrief")}
-                </SelectItem>
-                <SelectItem value="default">
-                  {t("insights.coach.settingsVerbosityDefault")}
-                </SelectItem>
-                <SelectItem value="detailed">
-                  {t("insights.coach.settingsVerbosityDetailed")}
-                </SelectItem>
-              </SelectContent>
-            </Select>
-          </div>
-
-          {/* Exclude metrics */}
-          <div className="flex flex-col gap-2">
-            <Label className="text-xs font-medium">
-              {t("insights.coach.settingsExcludeLabel")}
-            </Label>
-            <p className="text-muted-foreground text-[11px] leading-relaxed">
-              {t("insights.coach.settingsExcludeHint")}
-            </p>
-            <ul
-              data-slot="coach-prefs-exclude-list"
-              className="border-border/60 divide-border/50 mt-1 flex flex-col divide-y rounded-md border"
-            >
-              {EXCLUDE_OPTIONS.map((metric) => {
-                const id = `coach-prefs-exclude-${metric}`;
-                const checked = draft.excludeMetrics.includes(metric);
-                return (
-                  <li
-                    key={metric}
-                    className="flex items-center justify-between gap-3 px-3 py-2"
-                  >
-                    <Label htmlFor={id} className="cursor-pointer text-xs">
-                      {t(`insights.coach.metric.${metric}`)}
-                    </Label>
-                    <Switch
-                      id={id}
-                      data-slot={`coach-prefs-exclude-${metric}`}
-                      checked={checked}
-                      onCheckedChange={(next) => toggleExclude(metric, next)}
-                    />
-                  </li>
-                );
-              })}
-            </ul>
-          </div>
-
-          {/* Show evidence by default */}
-          <div className="flex items-start justify-between gap-3">
-            <div className="flex flex-col gap-1">
+        ) : (
+          <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">
               <Label
-                htmlFor="coach-prefs-evidence"
-                className="cursor-pointer text-xs font-medium"
+                htmlFor="coach-prefs-tone"
+                className="text-xs font-medium"
               >
-                {t("insights.coach.settingsEvidenceLabel")}
+                {t("insights.coach.settingsToneLabel")}
+              </Label>
+              <Select
+                value={draft.tone}
+                onValueChange={(value) =>
+                  setDraft((prev) => ({ ...prev, tone: value as CoachTone }))
+                }
+              >
+                <SelectTrigger
+                  id="coach-prefs-tone"
+                  data-slot="coach-prefs-tone"
+                >
+                  <SelectValue />
+                </SelectTrigger>
+                <SelectContent>
+                  <SelectItem value="warm">
+                    {t("insights.coach.settingsToneWarm")}
+                  </SelectItem>
+                  <SelectItem value="neutral">
+                    {t("insights.coach.settingsToneNeutral")}
+                  </SelectItem>
+                  <SelectItem value="concise">
+                    {t("insights.coach.settingsToneConcise")}
+                  </SelectItem>
+                </SelectContent>
+              </Select>
+            </div>
+
+            {/* Verbosity */}
+            <div className="flex flex-col gap-2">
+              <Label
+                htmlFor="coach-prefs-verbosity"
+                className="text-xs font-medium"
+              >
+                {t("insights.coach.settingsVerbosityLabel")}
+              </Label>
+              <Select
+                value={draft.verbosity}
+                onValueChange={(value) =>
+                  setDraft((prev) => ({
+                    ...prev,
+                    verbosity: value as CoachVerbosity,
+                  }))
+                }
+              >
+                <SelectTrigger
+                  id="coach-prefs-verbosity"
+                  data-slot="coach-prefs-verbosity"
+                >
+                  <SelectValue />
+                </SelectTrigger>
+                <SelectContent>
+                  <SelectItem value="brief">
+                    {t("insights.coach.settingsVerbosityBrief")}
+                  </SelectItem>
+                  <SelectItem value="default">
+                    {t("insights.coach.settingsVerbosityDefault")}
+                  </SelectItem>
+                  <SelectItem value="detailed">
+                    {t("insights.coach.settingsVerbosityDetailed")}
+                  </SelectItem>
+                </SelectContent>
+              </Select>
+            </div>
+
+            {/* Exclude metrics */}
+            <div className="flex flex-col gap-2">
+              <Label className="text-xs font-medium">
+                {t("insights.coach.settingsExcludeLabel")}
               </Label>
               <p className="text-muted-foreground text-[11px] leading-relaxed">
-                {t("insights.coach.settingsEvidenceHint")}
+                {t("insights.coach.settingsExcludeHint")}
               </p>
+              <ul
+                data-slot="coach-prefs-exclude-list"
+                className="border-border/60 divide-border/50 mt-1 flex flex-col divide-y rounded-md border"
+              >
+                {EXCLUDE_OPTIONS.map((metric) => {
+                  const id = `coach-prefs-exclude-${metric}`;
+                  const checked = draft.excludeMetrics.includes(metric);
+                  return (
+                    <li
+                      key={metric}
+                      className="flex items-center justify-between gap-3 px-3 py-2"
+                    >
+                      <Label htmlFor={id} className="cursor-pointer text-xs">
+                        {t(`insights.coach.metric.${metric}`)}
+                      </Label>
+                      <Switch
+                        id={id}
+                        data-slot={`coach-prefs-exclude-${metric}`}
+                        checked={checked}
+                        onCheckedChange={(next) =>
+                          toggleExclude(metric, next)
+                        }
+                      />
+                    </li>
+                  );
+                })}
+              </ul>
+            </div>
+
+            {/* Show evidence by default */}
+            <div className="flex items-start justify-between gap-3">
+              <div className="flex flex-col gap-1">
+                <Label
+                  htmlFor="coach-prefs-evidence"
+                  className="cursor-pointer text-xs font-medium"
+                >
+                  {t("insights.coach.settingsEvidenceLabel")}
+                </Label>
+                <p className="text-muted-foreground text-[11px] leading-relaxed">
+                  {t("insights.coach.settingsEvidenceHint")}
+                </p>
+              </div>
+              <Switch
+                id="coach-prefs-evidence"
+                data-slot="coach-prefs-evidence"
+                checked={draft.showEvidenceByDefault}
+                onCheckedChange={(next) =>
+                  setDraft((prev) => ({
+                    ...prev,
+                    showEvidenceByDefault: next,
+                  }))
+                }
+              />
             </div>
-            <Switch
-              id="coach-prefs-evidence"
-              data-slot="coach-prefs-evidence"
-              checked={draft.showEvidenceByDefault}
-              onCheckedChange={(next) =>
-                setDraft((prev) => ({
-                  ...prev,
-                  showEvidenceByDefault: next,
-                }))
-              }
-            />
           </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 ? (