-
Notifications
You must be signed in to change notification settings - Fork 1.4k
fix: restore desktop chat context after reload #672
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,6 +20,11 @@ import { | |
| } from "./hooks/useDashboardChatTransport"; | ||
| import { useI18n } from "../../components/useI18n"; | ||
| import { buildChatTranscript } from "./transcriptUtils"; | ||
| import { dbItemsToChatMessages, type DbHistoryItem } from "./sessionHistory"; | ||
| import { | ||
| deleteChatRunTranscript, | ||
| saveChatRunTranscript, | ||
| } from "../Layout/chatRunPersistence"; | ||
| import { ConfigHealthBanner } from "../../components/ConfigHealthBanner"; | ||
| import type { Attachment } from "../../../../shared/attachments"; | ||
| import type { ActiveTurn, ChatMessage, UsageState } from "./types"; | ||
|
|
@@ -265,6 +270,31 @@ function Chat({ | |
| activeTurnRef, | ||
| }); | ||
|
|
||
| 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]); | ||
|
|
||
| useEffect(() => { | ||
| saveChatRunTranscript(runId, messages); | ||
| }, [runId, messages]); | ||
|
Comment on lines
+294
to
+296
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
|
||
| // No parent-driven reset effects: each run is its own <Chat key={runId}> | ||
| // instance. A new chat is a fresh mount, and switching sessions just flips | ||
| // which mounted instance is shown — local state (session id, context folder, | ||
|
|
@@ -368,14 +398,17 @@ function Chat({ | |
| void window.hermesAPI.clearStagedAttachments(idToDelete); | ||
| } | ||
| setMessages([]); | ||
| deleteChatRunTranscript(runId); | ||
| setHermesSessionId(null); | ||
| setContextFolder(null); | ||
| activeTurnRef.current = null; | ||
| setUsage(null); | ||
| setToolProgress(null); | ||
| queueRef.current = []; | ||
| setQueuedMessages([]); | ||
| }, [isLoading, runId, hermesSessionId, setMessages]); | ||
| reportedTitleRef.current = false; | ||
| onTitleChange?.(runId, ""); | ||
| }, [isLoading, runId, hermesSessionId, setMessages, onTitleChange]); | ||
|
|
||
| const localCommands = useLocalCommands({ | ||
| profile, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,72 @@ | ||
| import { beforeEach, describe, expect, it } from "vitest"; | ||
| import type { ChatRun } from "./chatRuns"; | ||
| import { | ||
| deleteChatRunTranscript, | ||
| loadChatRunTranscript, | ||
| persistChatRunsState, | ||
| restoreChatRunsState, | ||
| saveChatRunTranscript, | ||
| } from "./chatRunPersistence"; | ||
| import type { ChatMessage } from "../Chat/types"; | ||
|
|
||
| const run: ChatRun = { | ||
| runId: "run-saved", | ||
| profile: "research-agent", | ||
| sessionId: "session-saved", | ||
| loading: true, | ||
| title: "Long investigation", | ||
| }; | ||
|
|
||
| const transcript: ChatMessage[] = [ | ||
| { | ||
| id: "user-1", | ||
| role: "user", | ||
| content: "trace the context flow", | ||
| }, | ||
| { | ||
| id: "agent-1", | ||
| role: "agent", | ||
| content: "partial analysis that streamed before reload", | ||
| pending: true, | ||
| }, | ||
| ]; | ||
|
|
||
| describe("chat run persistence", () => { | ||
| beforeEach(() => { | ||
| window.localStorage.clear(); | ||
| }); | ||
|
|
||
| it("restores open runs with their visible transcript snapshots", () => { | ||
| saveChatRunTranscript(run.runId, transcript); | ||
| persistChatRunsState([run], run.runId); | ||
|
|
||
| const restored = restoreChatRunsState(); | ||
|
|
||
| expect(restored?.activeRunId).toBe(run.runId); | ||
| expect(restored?.activeProfile).toBe(run.profile); | ||
| expect(restored?.runs).toHaveLength(1); | ||
| expect(restored?.runs[0]).toMatchObject({ | ||
| runId: run.runId, | ||
| profile: run.profile, | ||
| sessionId: run.sessionId, | ||
| loading: false, | ||
| title: run.title, | ||
| }); | ||
| expect(restored?.runs[0].seed?.[0]).toMatchObject(transcript[0]); | ||
| expect(restored?.runs[0].seed?.[1]).toMatchObject({ | ||
| id: "agent-1", | ||
| role: "agent", | ||
| content: "partial analysis that streamed before reload", | ||
| pending: false, | ||
| }); | ||
| }); | ||
|
|
||
| it("deletes transcript snapshots for discarded runs", () => { | ||
| saveChatRunTranscript(run.runId, transcript); | ||
| expect(loadChatRunTranscript(run.runId)).toHaveLength(2); | ||
|
|
||
| deleteChatRunTranscript(run.runId); | ||
|
|
||
| expect(loadChatRunTranscript(run.runId)).toEqual([]); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
messages.lengthchangehydratedInitialSessionRef.current = trueis set synchronously before thegetSessionMessagespromise is awaited. If the user sends a message before the DB call resolves (changingmessages.lengthfrom 0 to 1), the effect cleanup firescancelled = 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 sameinitialSessionIdcan retry.