fix(frontend): keep unsaved memory edits when a memory is added (#261)

Editing a memory row and then adding a new memory reverted the edit, with no
warning and nothing to undo it.

The draft list is re-seeded from the server whenever the server's answer
changes. That effect exists for a reason -- #241 added it so a row the server
normalized away (trimmed to nothing, a case-insensitive duplicate) stops
sitting on screen looking saved -- but it re-seeded wholesale. Adding a memory
also changes the server's answer, so every other row the user had edited and
not yet saved was silently overwritten with the server's copy.

Its own comment claimed the opposite ("leaves in-progress edits alone between
saves"); that only held while nothing else changed the list.

The effect now reconciles instead of replacing. Each row remembers the server
value it was seeded from, and a server list that has changed carries every
surviving row's in-progress value across by matching on that origin, building
a fresh row only for genuinely new entries. Matching on the origin rather than
the current value is the point: an edited row no longer equals its server
value, which is exactly the case being preserved.

Both behaviours are now pinned, including one test that exercises them
together -- a normalized-away row is dropped in the same update that an edit
to a surviving row is preserved -- so the fix cannot be satisfied by simply
preferring the draft over the server or the reverse.

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

Author: dovvnloading

frontend/src/features/settings/MemoryPanel.test.tsx

@@ -1,5 +1,6 @@
 import { render, screen } from "@testing-library/react";
 import userEvent from "@testing-library/user-event";
+import { useState } from "react";
 import { describe, expect, it, vi } from "vitest";
 import { MemoryPanel } from "./MemoryPanel";
 
@@ -24,6 +25,39 @@ describe("MemoryPanel", () => {
     expect(input).toHaveFocus();
   });
 
+  it("keeps unsaved edits to other rows when a memory is added", async () => {
+    // Adding a memory changes the server list, which re-seeded the whole
+    // draft. Every other row the user had edited but not yet saved silently
+    // reverted to the server's copy.
+    const user = userEvent.setup();
+    const onReplace = vi.fn<(memos: string[]) => Promise<void>>().mockResolvedValue();
+
+    function Host() {
+      const [memos, setMemos] = useState(["First", "Second"]);
+      return (
+        <MemoryPanel
+          memos={memos}
+          busy={false}
+          onAdd={async (memo: string) => { setMemos((current) => [...current, memo]); }}
+          onReplace={onReplace}
+          onClear={vi.fn<() => Promise<void>>().mockResolvedValue()}
+        />
+      );
+    }
+    render(<Host />);
+
+    await user.clear(screen.getByRole("textbox", { name: "Memory 2" }));
+    await user.type(screen.getByRole("textbox", { name: "Memory 2" }), "Edited");
+
+    await user.type(screen.getByRole("textbox", { name: "New memory" }), "Third");
+    await user.click(screen.getByRole("button", { name: "Add memory" }));
+    await screen.findByRole("textbox", { name: "Memory 3" });
+
+    expect(screen.getByRole("textbox", { name: "Memory 2" })).toHaveValue("Edited");
+    await user.click(screen.getByRole("button", { name: "Save changes" }));
+    expect(onReplace).toHaveBeenCalledWith(["First", "Edited", "Third"]);
+  });
+
   it("preserves edited rows when removing a neighboring row", async () => {
     const user = userEvent.setup();
     const onReplace = vi.fn<(memos: string[]) => Promise<void>>().mockResolvedValue();
@@ -161,4 +195,38 @@ describe("MemoryPanel server reconciliation", () => {
     });
     expect(screen.getByDisplayValue("kept")).toBeInTheDocument();
   });
+
+  it("drops a normalized-away row without discarding an edit to a surviving one", async () => {
+    // Both halves at once: reconciliation must not be satisfied by simply
+    // preferring the draft (which would keep the rejected row on screen) or
+    // by simply preferring the server (the original defect).
+    const user = userEvent.setup();
+    const { waitFor } = await import("@testing-library/react");
+
+    function Harness() {
+      const [memos, setMemos] = useState<string[]>(["kept", "duplicate"]);
+      return (
+        <>
+          <button onClick={() => setMemos(["kept"])}>server responded</button>
+          <MemoryPanel
+            memos={memos}
+            busy={false}
+            onAdd={vi.fn<(memo: string) => Promise<void>>().mockResolvedValue()}
+            onReplace={vi.fn<(memos: string[]) => Promise<void>>().mockResolvedValue()}
+            onClear={vi.fn<() => Promise<void>>().mockResolvedValue()}
+          />
+        </>
+      );
+    }
+    render(<Harness />);
+
+    await user.clear(screen.getByRole("textbox", { name: "Memory 1" }));
+    await user.type(screen.getByRole("textbox", { name: "Memory 1" }), "kept and edited");
+    await user.click(screen.getByRole("button", { name: "server responded" }));
+
+    await waitFor(() => {
+      expect(screen.queryByDisplayValue("duplicate")).not.toBeInTheDocument();
+    });
+    expect(screen.getByRole("textbox", { name: "Memory 1" })).toHaveValue("kept and edited");
+  });
 });

frontend/src/features/settings/MemoryPanel.tsx

@@ -9,23 +9,53 @@ type Props = {
   onClear: () => Promise<void>;
 };
 
+type DraftRow = {
+  id: number;
+  value: string;
+  // The server value this row was last seeded from. An edited row no longer
+  // equals it, which is exactly why reconciliation cannot match on `value`.
+  origin: string;
+};
+
 export function MemoryPanel({ memos, busy, onAdd, onReplace, onClear }: Props) {
   const [memo, setMemo] = useState("");
-  const [draft, setDraft] = useState(() => memos.map((value, id) => ({ id, value })));
+  const [draft, setDraft] = useState<DraftRow[]>(
+    () => memos.map((value, id) => ({ id, value, origin: value })),
+  );
 
   // `memos` is the authoritative list the server returned. The draft was
   // seeded from it once and never re-derived, so an entry the server
   // normalized away -- trimmed to nothing, or a case-insensitive duplicate --
-  // stayed on screen looking saved. Re-seed whenever the server's answer
-  // actually changes, which leaves in-progress edits alone between saves.
+  // stayed on screen looking saved.
+  //
+  // Re-seeding wholesale fixed that and broke something else: adding a memory
+  // also changes the server's answer, so every *other* row the user had edited
+  // and not yet saved silently reverted. Reconcile instead -- carry each
+  // surviving row's in-progress value across by matching on the server value
+  // it came from, and build a fresh row only for genuinely new entries.
   const lastServerMemos = useRef(memos);
   useEffect(() => {
     const previous = lastServerMemos.current;
     const changed =
       previous.length !== memos.length || previous.some((value, index) => value !== memos[index]);
     if (!changed) return;
     lastServerMemos.current = memos;
-    setDraft(memos.map((value, id) => ({ id, value })));
+    setDraft((current) => {
+      const byOrigin = new Map<string, DraftRow[]>();
+      for (const row of current) {
+        const bucket = byOrigin.get(row.origin);
+        if (bucket) bucket.push(row);
+        else byOrigin.set(row.origin, [row]);
+      }
+      let nextId = current.reduce((highest, row) => Math.max(highest, row.id), -1) + 1;
+      return memos.map((value) => {
+        // shift(), so duplicate server values claim distinct rows.
+        const existing = byOrigin.get(value)?.shift();
+        return existing
+          ? { ...existing, origin: value }
+          : { id: nextId++, value, origin: value };
+      });
+    });
   }, [memos]);
 
   const handleSubmit = (event: FormEvent<HTMLFormElement>) => {
@@ -36,7 +66,7 @@ export function MemoryPanel({ memos, busy, onAdd, onReplace, onClear }: Props) {
       setDraft((current) => {
         if (current.some((item) => item.value.toLocaleLowerCase() === value.toLocaleLowerCase())) return current;
         const id = current.reduce((highest, item) => Math.max(highest, item.id), -1) + 1;
-        return [...current, { id, value }];
+        return [...current, { id, value, origin: value }];
       });
       setMemo("");
     }).catch(() => undefined);