-
Notifications
You must be signed in to change notification settings - Fork 3.6k
chore: remove chat history from redux persistent storage #7944
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
18a4fbb
4932cb2
e78456c
a6daeb8
ce9ced8
93cf238
a8f1d30
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 |
---|---|---|
|
@@ -4,7 +4,6 @@ import { | |
ThunkDispatch, | ||
UnknownAction, | ||
} from "@reduxjs/toolkit"; | ||
import { createLogger } from "redux-logger"; | ||
import { | ||
createMigrate, | ||
MigrationManifest, | ||
|
@@ -35,9 +34,7 @@ const rootReducer = combineReducers({ | |
|
||
const saveSubsetFilters = [ | ||
createFilter("session", [ | ||
"history", | ||
"id", | ||
"lastSessionId", | ||
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. We still need to persist lastSessionId |
||
"title", | ||
|
||
// Persist edit mode in case closes in middle | ||
|
@@ -73,7 +70,6 @@ const migrations: MigrationManifest = { | |
defaultModelTitle: oldState?.state?.defaultModelTitle ?? undefined, | ||
}, | ||
session: { | ||
history: oldState?.state?.history ?? [], | ||
id: oldState?.state?.sessionId ?? "", | ||
}, | ||
tabs: { | ||
|
@@ -108,13 +104,6 @@ const persistedReducer = persistReducer<ReturnType<typeof rootReducer>>( | |
export function setupStore(options: { ideMessenger?: IIdeMessenger }) { | ||
const ideMessenger = options.ideMessenger ?? new IdeMessenger(); | ||
|
||
const logger = createLogger({ | ||
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. This can be used if you uncomment the |
||
// Customize logger options if needed | ||
collapsed: true, // Collapse console groups by default | ||
timestamp: false, // Remove timestamps from log | ||
diff: true, // Show diff between states | ||
}); | ||
|
||
return configureStore({ | ||
// persistedReducer causes type errors with async thunks | ||
reducer: persistedReducer as unknown as typeof rootReducer, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
import { createAsyncThunk, unwrapResult } from "@reduxjs/toolkit"; | ||
import { ChatMessage, Session, BaseSessionMetadata } from "core"; | ||
import { BaseSessionMetadata, ChatMessage, Session } from "core"; | ||
import { RemoteSessionMetadata } from "core/control-plane/client"; | ||
import { NEW_SESSION_TITLE } from "core/util/constants"; | ||
import { renderChatMessage } from "core/util/messageContent"; | ||
|
@@ -67,11 +67,7 @@ export const deleteSession = createAsyncThunk<void, string, ThunkApiType>( | |
dispatch(deleteSessionMetadata(id)); // optimistic | ||
const state = getState(); | ||
if (id === state.session.id) { | ||
await dispatch( | ||
loadLastSession({ | ||
saveCurrentSession: false, | ||
}), | ||
); | ||
await dispatch(loadLastSession()); | ||
} | ||
const result = await extra.ideMessenger.request("history/delete", { id }); | ||
if (result.status === "error") { | ||
|
@@ -146,20 +142,17 @@ export const loadRemoteSession = createAsyncThunk< | |
}, | ||
); | ||
|
||
export const loadLastSession = createAsyncThunk< | ||
void, | ||
{ | ||
saveCurrentSession: boolean; | ||
}, | ||
ThunkApiType | ||
>( | ||
export const loadLastSession = createAsyncThunk<void, void, ThunkApiType>( | ||
"session/loadLast", | ||
async ({ saveCurrentSession }, { extra, dispatch, getState }) => { | ||
const state = getState(); | ||
|
||
if (state.session.id && saveCurrentSession) { | ||
async (_, { extra, dispatch, getState }) => { | ||
let lastSessionId = getState().session.lastSessionId; | ||
|
||
const lastSessionResult = await extra.ideMessenger.request("history/list", { | ||
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. I would just used persisted lastSessionId still rather than listing history. If there's noLast sessionId we start a new session 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. That being said might just comment it out since we will want some version of this soon for per-workspace sessions! |
||
limit: 1, | ||
}); | ||
if (lastSessionResult.status === "success") { | ||
lastSessionId = lastSessionResult.content.at(0)?.sessionId; | ||
} | ||
const lastSessionId = getState().session.lastSessionId; | ||
|
||
if (!lastSessionId) { | ||
dispatch(newSession()); | ||
|
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.
Since this is pretty critical loading could we add some retry logic and error handling? Maybe retry with exponential backoff or a few times or similar