-
Notifications
You must be signed in to change notification settings - Fork 209
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
fix: migrate to chat history manager #2059
Conversation
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.
Looks good but e2e tests appear to have broken.
await this.saveTranscriptToChatHistory() | ||
this.cancelCompletion() | ||
const history = chatHistory.getChat(chatID) | ||
if (!history || chatID === this.sessionID) { |
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.
Just curious when chatID == this.sessionID
would happen.
await localStorage.removeChatHistory() | ||
} | ||
|
||
public async saveInput(input: string): Promise<UserLocalHistory> { |
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.
Worth a brief comment that this saves human input for the "up" history list, and same for getInput
. Because there's a few methods running around with "save" this or that.
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.
updated name to save/getHumanInputHistory
and added comment
CLOSE #1968
This PR focused on simplify chat history handling by moving the old chat history handling in MessageProvider to the new Chat History Manager.
Removed redundant chat history code and use shared chatHistory module instead. This simplifies the MessageProvider by removing duplicated chat history logic.
This also fixes issue described in the attached issue as previously, we would save chat history as group instead of individual chat, so a new group can overwritten the old one, unable to keep history across panels in sync.
Test plan
Follow instructions in attached issue