diff --git a/frontend/src/features/settings/MemoryPanel.test.tsx b/frontend/src/features/settings/MemoryPanel.test.tsx index 9f0839e..81b1474 100644 --- a/frontend/src/features/settings/MemoryPanel.test.tsx +++ b/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>().mockResolvedValue(); + + function Host() { + const [memos, setMemos] = useState(["First", "Second"]); + return ( + { setMemos((current) => [...current, memo]); }} + onReplace={onReplace} + onClear={vi.fn<() => Promise>().mockResolvedValue()} + /> + ); + } + render(); + + 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>().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(["kept", "duplicate"]); + return ( + <> + + Promise>().mockResolvedValue()} + onReplace={vi.fn<(memos: string[]) => Promise>().mockResolvedValue()} + onClear={vi.fn<() => Promise>().mockResolvedValue()} + /> + + ); + } + render(); + + 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"); + }); }); diff --git a/frontend/src/features/settings/MemoryPanel.tsx b/frontend/src/features/settings/MemoryPanel.tsx index 4316998..5aa7e06 100644 --- a/frontend/src/features/settings/MemoryPanel.tsx +++ b/frontend/src/features/settings/MemoryPanel.tsx @@ -9,15 +9,30 @@ type Props = { onClear: () => Promise; }; +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( + () => 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; @@ -25,7 +40,22 @@ export function MemoryPanel({ memos, busy, onAdd, onReplace, onClear }: Props) { 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(); + 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) => { @@ -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);