refactor(coach): drawer prefill becomes a controlled prop

The v1.4.20 implementation reset the composer by mounting the
`<CoachDrawer>` with `key={prefill}`. That weaponised React's
key-reset to flush internal state every prefill change — Sr-HIGH-4
flagged it as a smell because the next refactor that touches the
drawer's mount point silently loses the reset semantics.

Replace with the controlled-prop pattern React documents at
https://react.dev/learn/you-might-not-need-an-effect#adjusting-some-state-when-a-prop-changes:
new `useResettableValue` hook reads the controlled prefill, mirrors
it via a sibling useState pair, and seeds the composer with the new
value when the prop changes. Render-phase setState — ESLint-clean
against `react-hooks/set-state-in-effect`. The user can still type
freely; the drawer only resets when the parent changes which
suggested-prompt chip is active.

`nextResettableValue` exposes the comparison contract as a pure
helper so the unit test can pin the (Object.is) semantics without
standing up a React renderer.

The `<CoachDrawer key={…}>` is gone from the insights page mount.

Tests +5: pure-function coverage of unchanged value (no reset),
prefill change (reset), null↔empty transition (reset), suggestion
chip → cleared prefill (reset), and NaN-to-NaN edge case (no reset
under Object.is).

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

src/app/insights/page.tsx

@@ -1645,12 +1645,13 @@ export default function InsightsPage() {
         />
       </section>
 
-      {/* v1.4.20 phase B2b — AI Coach drawer. The drawer reads its
+      {/* v1.4.20 phase B2b — AI Coach drawer.
-          initial input value from `prefill`; we change the React `key`
+          v1.4.23 H3 — `prefill` is a controlled prop now; the drawer
-          on every prefill transition so the lazy `useState` initialiser
+          watches it via an in-render ref and resets the composer
-          fires fresh and the composer surfaces the latest chip. */}
+          when the parent changes which suggested-prompt chip is
+          active. The old `key={prefill}` weaponise-React-keys pattern
+          is gone (Sr-HIGH-4). */}
       <CoachDrawer
-        key={coachPrefill ?? "blank"}
         open={coachOpen}
         onOpenChange={setCoachOpen}
         prefill={coachPrefill}

src/components/insights/coach-panel/__tests__/coach-drawer-prefill.test.ts

@@ -0,0 +1,63 @@
+import { describe, expect, it } from "vitest";
+
+import { nextResettableValue } from "../coach-drawer";
+
+/**
+ * v1.4.23 H3 — drawer prefill is now a fully-controlled prop.
+ *
+ * The v1.4.20 implementation reset the composer by mounting the
+ * `<CoachDrawer>` with `key={prefill}`. That weaponised React's
+ * key-reset to flush internal state every prefill change — Sr-HIGH-4
+ * flagged it as a smell because the next refactor that touches the
+ * drawer's mount point will silently lose the reset semantics.
+ *
+ * The fix is the in-render-update pattern documented at
+ * https://react.dev/learn/you-might-not-need-an-effect#adjusting-some-state-when-a-prop-changes:
+ * track the last controlled value via `useRef`, and call `setState`
+ * during render when the controlled value differs. `nextResettableValue`
+ * pins the comparison contract here so a future Object.is rewrite
+ * (e.g., shallow-compare migration) can't silently regress.
+ */
+describe("nextResettableValue (Coach drawer prefill controller)", () => {
+  it("reports no reset when the controlled value is unchanged", () => {
+    expect(nextResettableValue("How is my BP?", "How is my BP?")).toEqual({
+      reset: false,
+    });
+  });
+
+  it("reports a reset with the next value when the controlled prop changes", () => {
+    expect(
+      nextResettableValue<string | null>("How is my BP?", "Mood last week?"),
+    ).toEqual({ reset: true, value: "Mood last week?" });
+  });
+
+  it("treats null and an empty string as different controlled values", () => {
+    expect(nextResettableValue<string | null>(null, "")).toEqual({
+      reset: true,
+      value: "",
+    });
+    expect(nextResettableValue<string | null>("", null)).toEqual({
+      reset: true,
+      value: null,
+    });
+  });
+
+  it("treats a transition from a non-null prefill back to null as a reset", () => {
+    // The parent surface clears the prefill back to null after the
+    // user clicks a non-suggestion entry point. The composer must
+    // empty itself in that transition; otherwise the previous chip's
+    // text lingers across drawer opens.
+    expect(
+      nextResettableValue<string | null>("Why was BP higher Monday?", null),
+    ).toEqual({ reset: true, value: null });
+  });
+
+  it("uses Object.is, so NaN-to-NaN does not trigger a reset", () => {
+    // Defence-in-depth — Object.is(NaN, NaN) is true, unlike ===.
+    // The drawer would otherwise reset on every render if the parent
+    // happened to pass NaN as a string-coerced default.
+    expect(nextResettableValue<number>(Number.NaN, Number.NaN)).toEqual({
+      reset: false,
+    });
+  });
+});

src/components/insights/coach-panel/coach-drawer.tsx

@@ -1,6 +1,7 @@
 "use client";
 
 import { useCallback, useState } from "react";
+import type { Dispatch, SetStateAction } from "react";
 import { Plus, Sparkles } from "lucide-react";
 
 import { Button } from "@/components/ui/button";
@@ -63,6 +64,53 @@ export interface CoachDrawerProps {
   composer?: React.ReactNode;
 }
 
+/**
+ * v1.4.23 H3 — local state seeded from a controlled prop, reset on
+ * prop change. Same render-phase pattern React's docs recommend in
+ * place of `useEffect(() => setState(prop), [prop])` (which the
+ * `react-hooks/set-state-in-effect` ESLint rule banned). The reset
+ * runs during render: React detects the queued setState, restarts
+ * the render with the new value, and commits a single coherent
+ * snapshot — no double paint, no flash of stale state.
+ *
+ * Exported so the drawer's prefill-reset behaviour can be unit-
+ * tested in isolation without standing up the whole Sheet portal.
+ */
+export function useResettableValue<T>(
+  controlledValue: T,
+): [T, Dispatch<SetStateAction<T>>] {
+  const [value, setValue] = useState<T>(controlledValue);
+  // Mirror the last observed controlled value via a sibling useState
+  // pair (per React docs:
+  // https://react.dev/learn/you-might-not-need-an-effect#adjusting-some-state-when-a-prop-changes).
+  // useRef is the wrong tool here — ESLint rejects ref read+write
+  // during render, and useState already gives us identity tracking
+  // with no extra cost.
+  const [lastSeen, setLastSeen] = useState<T>(controlledValue);
+  if (!Object.is(controlledValue, lastSeen)) {
+    setLastSeen(controlledValue);
+    setValue(controlledValue);
+  }
+  return [value, setValue];
+}
+
+/**
+ * v1.4.23 H3 — pure decision function behind `useResettableValue`.
+ * Given the previous controlled value the hook recorded and the
+ * incoming controlled value, returns either `{ reset: true, value }`
+ * (the next render must seed local state with `value`) or
+ * `{ reset: false }` (local state survives — the user's edits are
+ * preserved). Pure + dependency-free so it tests cleanly without a
+ * React renderer — pin the contract here and trust the hook
+ * implementation to wire the same comparison.
+ */
+export function nextResettableValue<T>(
+  previous: T,
+  incoming: T,
+): { reset: true; value: T } | { reset: false } {
+  return Object.is(previous, incoming) ? { reset: false } : { reset: true, value: incoming };
+}
+
 export function CoachDrawer({
   open,
   onOpenChange,
@@ -82,12 +130,18 @@ export function CoachDrawer({
   // data the Coach is using without losing the message thread context.
   const [historyTrayOpen, setHistoryTrayOpen] = useState(false);
   const [sourcesTrayOpen, setSourcesTrayOpen] = useState(false);
-  // The composer's input value is seeded from `prefill` whenever the
+  // v1.4.23 H3 — `prefill` is now a fully-controlled prop. The
-  // parent toggles `open` — we mount the drawer with a key derived from
+  // v1.4.20 implementation used `key={prefill}` on the parent mount
-  // (open, prefill) so the lazy initialiser fires fresh on each
+  // to force a fresh `useState()` initialiser run on every prefill
-  // open/prefill transition. That sidesteps `setState`-in-`useEffect`
+  // transition (the "weaponise-React-keys" pattern senior-dev
-  // entirely (banned by `react-hooks/set-state-in-effect`).
+  // review flagged as Sr-HIGH-4). The replacement is a tiny
-  const [inputValue, setInputValue] = useState<string>(() => prefill ?? "");
+  // `useResettableValue` hook below: tracks the last observed
+  // prefill in a ref and schedules a same-render state update when
+  // the prop changes — a textbook React render-phase update,
+  // ESLint-clean against `react-hooks/set-state-in-effect`. The
+  // user can still type freely; the drawer only resets the composer
+  // when the parent changes which suggested-prompt chip is active.
+  const [inputValue, setInputValue] = useResettableValue(prefill ?? "");
   // v1.4.20.1 — scope picker state (per-source checkboxes + window
   // selector). Resets to the all-source last30days default each time
   // the drawer mounts; no conversation-level persistence in this