feat: autosave chat to localStorage with demo reset#13
Conversation
Roadmap Phase 2, item #12 (autosave half). - Messages and config persist to localStorage on every change and are restored on load, so refreshes no longer lose work. Everything still stays in the browser. - Saved configs merge over defaults so sessions saved before newer config fields existed load with sensible values; corrupt or unexpected payloads fall back to the demo chat. - Autosave failures (e.g. storage quota with several images) warn and never break the editor. - New 'Reset chat to demo conversation' button (with confirm) clears the saved state. - Test setup clears localStorage between tests; new tests cover persistence across remount, reset, and corrupt-data fallback. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SN53MtH3hvbHr7syhwu54q
There was a problem hiding this comment.
Code Review
This pull request introduces an autosave feature that persists the chat state (messages and configuration) to localStorage and restores it on application load, along with a reset button to restore the demo conversation. The feedback highlights critical performance and reliability improvements: debouncing the autosave to prevent UI lag during rapid state updates, avoiding storage pollution when resetting to the demo state, validating the sender field of loaded messages to prevent UI crashes, and updating the tests to support debounced timers.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| useEffect(() => { | ||
| try { | ||
| localStorage.setItem(STORAGE_KEY, JSON.stringify({ messages, config })) | ||
| } catch (err) { | ||
| console.warn("Autosave failed (storage quota?):", err) | ||
| } | ||
| }, [messages, config]) | ||
|
|
||
| const handleReset = useCallback(() => { | ||
| if (!confirm("Reset the chat to the demo conversation?")) return | ||
| try { | ||
| localStorage.removeItem(STORAGE_KEY) | ||
| } catch { | ||
| // Ignore storage failures; state reset below still applies. | ||
| } | ||
| setMessages(INITIAL_MESSAGES) | ||
| setConfig(INITIAL_CONFIG) | ||
| }, []) |
There was a problem hiding this comment.
Performance & Storage Optimization
- Performance Bottleneck (High Severity): Typing in fields like "Contact Name" or dragging the battery slider triggers state updates on every keystroke/tick. Without debouncing, the app performs expensive
JSON.stringifyandlocalStorage.setItemoperations (which can be several megabytes if images are attached) on every single frame, causing severe UI jank and lag. Debouncing the write by 1 second resolves this. - Forward-Compatibility / Storage Pollution (Medium Severity): When resetting the chat,
localStorage.removeItemis called, but then theuseEffectimmediately runs because the state changed toINITIAL_MESSAGESandINITIAL_CONFIG, saving the default state back intolocalStorage. This pollutes the user's storage and prevents them from receiving future updates to the demo conversation (since their browser will always load the old hardcoded demo state fromlocalStorage). Checking if the state matches the initial references and clearing the storage instead of saving solves this beautifully.
useEffect(() => {
if (messages === INITIAL_MESSAGES && config === INITIAL_CONFIG) {
try {
localStorage.removeItem(STORAGE_KEY)
} catch (err) {
console.warn("Failed to clear storage:", err)
}
return
}
const timer = setTimeout(() => {
try {
localStorage.setItem(STORAGE_KEY, JSON.stringify({ messages, config }))
} catch (err) {
console.warn("Autosave failed (storage quota?):", err)
}
}, 1000)
return () => clearTimeout(timer)
}, [messages, config])
const handleReset = useCallback(() => {
if (!confirm("Reset the chat to the demo conversation?")) return
setMessages(INITIAL_MESSAGES)
setConfig(INITIAL_CONFIG)
}, [])
| messages: saved.messages.filter( | ||
| (m) => m && typeof m.id === "string" && typeof m.text === "string", | ||
| ), |
There was a problem hiding this comment.
Robust State Validation (Medium Severity)
When loading saved messages from localStorage, we only validate that id and text are strings. However, sender is a required field of the Message type and determines critical rendering logic (e.g., bubble styles and layout). If sender is missing or invalid, it could lead to UI rendering issues or crashes. We should validate that sender is one of the expected values ("me" | "them" | "system").
messages: saved.messages.filter(
(m) =>
m &&
typeof m.id === "string" &&
typeof m.text === "string" &&
(m.sender === "me" || m.sender === "them" || m.sender === "system"),
),
| it("restores the chat after unmounting and remounting", () => { | ||
| const first = render(<App />) | ||
| fireEvent.change(screen.getByPlaceholderText("Type your message..."), { | ||
| target: { value: "Persist me" }, | ||
| }) | ||
| fireEvent.click(screen.getByRole("button", { name: "Add Message" })) | ||
| expect(screen.getAllByText("Persist me").length).toBeGreaterThan(0) | ||
| first.unmount() | ||
|
|
||
| render(<App />) | ||
| expect(screen.getAllByText("Persist me").length).toBeGreaterThan(0) | ||
| }) |
There was a problem hiding this comment.
Test Compatibility with Debounced Autosave
Since we introduced a 1000ms debounce to the autosave useEffect in App.tsx to prevent performance issues, the unmount/remount test needs to use fake timers to fast-forward the debounce delay. Otherwise, the unmount will happen before the state is written to localStorage, causing the test to fail.
it("restores the chat after unmounting and remounting", () => {
vi.useFakeTimers()
const first = render(<App />)
fireEvent.change(screen.getByPlaceholderText("Type your message..."), {
target: { value: "Persist me" },
})
fireEvent.click(screen.getByRole("button", { name: "Add Message" }))
expect(screen.getAllByText("Persist me").length).toBeGreaterThan(0)
vi.advanceTimersByTime(1000)
first.unmount()
vi.useRealTimers()
render(<App />)
expect(screen.getAllByText("Persist me").length).toBeGreaterThan(0)
})
Address review feedback on #13: - Debounce localStorage writes by 1s — serializing chats with embedded images on every keystroke (contact name, battery slider) caused needless stringify/setItem churn. - Pristine state (first visit or post-reset) now clears storage instead of saving the demo, so future demo updates aren't shadowed by a saved copy of the old one; reset relies on this instead of removing the key itself. - Validate the sender field when restoring saved messages. - Persistence test flushes the debounce with fake timers. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SN53MtH3hvbHr7syhwu54q
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Summary
Implements the autosave half of Roadmap Phase 2, item #12. Refreshing or closing the tab no longer loses the chat — the biggest papercut for anyone building a longer conversation.
What changed
localStorageon every change and restore on load. Still 100% in-browser; the privacy promise holds.localStoragebetween tests, since state would otherwise leak across the suite.Verification
pnpm test— 38/38 pass (3 new: persistence across unmount/remount, reset restores demo, corrupt-data fallback)pnpm lint— cleanpnpm build— succeedsNotes
.txtimport, shareable URL state, bulk script entry) are natural follow-ups once the stack lands.🤖 Generated with Claude Code
Generated by Claude Code