fix: restore desktop chat context after reload#672
Conversation
Greptile SummaryThis PR adds localStorage-backed persistence of active desktop chat runs (run metadata, profile, session id, title) and per-run transcript snapshots, restoring them on app reload instead of always starting from a blank default chat. It also threads a configurable per-request timeout through
Confidence Score: 4/5Safe to merge; the new persistence layer is well-guarded with try/catch and graceful fallbacks, and no new blocking issues were introduced. The three issues noted in the previous review round (hydration guard set before the async await, transcript saves on every streaming chunk, and orphaned transcript keys beyond the 12-run cap) are still present and unaddressed, which is the main reason for caution. src/renderer/src/screens/Chat/Chat.tsx (hydration guard and streaming-chunk save rate) and src/renderer/src/screens/Layout/chatRunPersistence.ts (orphaned transcript cleanup, tool-call running-status clamping) Important Files Changed
|
| const hydratedInitialSessionRef = useRef(false); | ||
| useEffect(() => { | ||
| if (hydratedInitialSessionRef.current) return; | ||
| if (!initialSessionId || messages.length > 0) return; | ||
| hydratedInitialSessionRef.current = true; | ||
| let cancelled = false; | ||
| window.hermesAPI | ||
| .getSessionMessages(initialSessionId) | ||
| .then((items) => { | ||
| if (cancelled) return; | ||
| const restored = dbItemsToChatMessages(items as DbHistoryItem[]); | ||
| if (restored.length > 0) setMessages(restored); | ||
| }) | ||
| .catch(() => { | ||
| /* best-effort restore; sending can still resume by session id */ | ||
| }); | ||
| return () => { | ||
| cancelled = true; | ||
| }; | ||
| }, [initialSessionId, messages.length]); |
There was a problem hiding this comment.
Hydration ref set before async, then cancelled on
messages.length change
hydratedInitialSessionRef.current = true is set synchronously before the getSessionMessages promise is awaited. If the user sends a message before the DB call resolves (changing messages.length from 0 to 1), the effect cleanup fires cancelled = true, abandoning the in-flight fetch. On the next invocation the ref guard causes an immediate early return, so session history is silently never restored. For a restored run with a session id but an empty local snapshot this leaves the chat visually history-less. Since the intent is a one-shot initialisation, consider tracking the promise itself or moving the guard set to after the fetch completes so a re-trigger with the same initialSessionId can retry.
| useEffect(() => { | ||
| saveChatRunTranscript(runId, messages); | ||
| }, [runId, messages]); |
There was a problem hiding this comment.
Transcript saved on every streaming chunk
saveChatRunTranscript is called synchronously inside useEffect with messages as a dependency, meaning it fires on every single state update — including each streaming token during generation. This serializes and writes the entire message array to localStorage (potentially several KB) on every chunk, which during a long response could be hundreds of writes per second. Only the final or last-known state matters for crash recovery, so the intermediate-chunk saves are wasted work. Consider debouncing or writing only when streaming has paused (e.g. when isLoading turns false).
| JSON.stringify(messages), | ||
| ); | ||
| } catch { | ||
| // localStorage can be unavailable or full. The canonical Hermes session DB | ||
| // still remains the primary source for completed turns; this snapshot is a | ||
| // best-effort crash/reload safety net for renderer-visible state. | ||
| } | ||
| } | ||
|
|
||
| export function deleteChatRunTranscript(runId: string): void { | ||
| if (!runId) return; | ||
| try { | ||
| window.localStorage.removeItem(transcriptStorageKey(runId)); | ||
| } catch { | ||
| /* ignore */ |
There was a problem hiding this comment.
Transcript keys orphaned when run list exceeds
MAX_RESTORED_RUNS
persistChatRunsState saves only the last 12 runs via runs.slice(-MAX_RESTORED_RUNS) but never removes the localStorage transcript entries for the runs that were dropped from the slice. When a user accumulates more than 12 open runs and then reloads, restoreChatRunsState only sees the kept 12 and never calls deleteChatRunTranscript for the others, so their keys (hermes.chat.transcript.<id>) accumulate in localStorage indefinitely. Consider iterating over runs.slice(0, -MAX_RESTORED_RUNS) and calling deleteChatRunTranscript for each before persisting.
Summary
Test Plan