fix: migrate to chat history manager (#2059)

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
sourcegraph · Dec 4, 2023 · 8fb7c5a · 8fb7c5a
1 parent 7dce4fe
commit 8fb7c5a
Show file tree

Hide file tree

Showing 11 changed files with 89 additions and 86 deletions.
diff --git a/vscode/CHANGELOG.md b/vscode/CHANGELOG.md
@@ -12,6 +12,7 @@ This is a log of all notable changes to Cody for VS Code. [Unreleased] changes a
 
 - Chat: Display OS specific keybinding in chat welcome message. [pull/2051](https://github.com/sourcegraph/cody/pull/2051)
 - Embeddings indexes can be generated and stored locally in repositories with a default fetch URL that is not already indexed by sourcegraph.com through the Enhanced Context selector. [pull/2069](https://github.com/sourcegraph/cody/pull/2069)
+- Support chat input history on "up" and "down" arrow keys again. [pull/2059](https://github.com/sourcegraph/cody/pull/2059)
 
 ### Changed
 

diff --git a/vscode/src/chat/MessageProvider.ts b/vscode/src/chat/MessageProvider.ts
@@ -10,12 +10,7 @@ import { newInteraction } from '@sourcegraph/cody-shared/src/chat/prompts/utils'
 import { Recipe, RecipeID, RecipeType } from '@sourcegraph/cody-shared/src/chat/recipes/recipe'
 import { Transcript } from '@sourcegraph/cody-shared/src/chat/transcript'
 import { Interaction } from '@sourcegraph/cody-shared/src/chat/transcript/interaction'
-import {
-    ChatEventSource,
-    ChatHistory,
-    ChatMessage,
-    UserLocalHistory,
-} from '@sourcegraph/cody-shared/src/chat/transcript/messages'
+import { ChatEventSource, ChatMessage, UserLocalHistory } from '@sourcegraph/cody-shared/src/chat/transcript/messages'
 import { Typewriter } from '@sourcegraph/cody-shared/src/chat/typewriter'
 import { reformatBotMessageForChat, reformatBotMessageForEdit } from '@sourcegraph/cody-shared/src/chat/viewHelpers'
 import { annotateAttribution, Guardrails } from '@sourcegraph/cody-shared/src/guardrails'
@@ -35,6 +30,7 @@ import { telemetryService } from '../services/telemetry'
 import { telemetryRecorder } from '../services/telemetry-v2'
 import { TestSupport } from '../test-support'
 
+import { chatHistory } from './chat-view/ChatHistoryManager'
 import { ContextProvider } from './ContextProvider'
 import { countGeneratedCode } from './utils'
 
@@ -86,10 +82,6 @@ export abstract class MessageProvider extends MessageHandler implements vscode.D
     public sessionID = new Date(Date.now()).toUTCString()
     public currentRequestID: string | undefined = undefined
 
-    // input and chat history are shared across all MessageProvider instances
-    protected static inputHistory: string[] = []
-    protected static chatHistory: ChatHistory = {}
-
     private isMessageInProgress = false
     private cancelCompletionCallback: (() => void) | null = null
 
@@ -129,15 +121,13 @@ export abstract class MessageProvider extends MessageHandler implements vscode.D
     }
 
     protected async init(chatID?: string): Promise<void> {
-        this.loadChatHistory()
+        if (chatID) {
+            await this.restoreSession(chatID)
+        }
         this.sendTranscript()
         this.sendHistory()
         await this.contextProvider.init()
         await this.sendCodyCommands()
-
-        if (chatID) {
-            await this.restoreSession(chatID)
-        }
     }
 
     private get isDotComUser(): boolean {
@@ -147,8 +137,8 @@ export abstract class MessageProvider extends MessageHandler implements vscode.D
 
     public async clearAndRestartSession(): Promise<void> {
         await this.saveTranscriptToChatHistory()
-        this.createNewChatID()
         this.cancelCompletion()
+        this.createNewChatID()
         this.isMessageInProgress = false
         this.transcript.reset()
         this.handleSuggestions([])
@@ -159,9 +149,7 @@ export abstract class MessageProvider extends MessageHandler implements vscode.D
     }
 
     public async clearHistory(): Promise<void> {
-        MessageProvider.chatHistory = {}
-        MessageProvider.inputHistory = []
-        await localStorage.removeChatHistory()
+        await chatHistory.clear()
         // Reset the current transcript
         this.transcript = new Transcript()
         await this.clearAndRestartSession()
@@ -174,10 +162,14 @@ export abstract class MessageProvider extends MessageHandler implements vscode.D
      * Restores a session from a chatID
      */
     public async restoreSession(chatID: string): Promise<void> {
+        const history = chatHistory.getChat(chatID)
+        if (!history || chatID === this.sessionID) {
+            return
+        }
         await this.saveTranscriptToChatHistory()
         this.cancelCompletion()
         this.createNewChatID(chatID)
-        this.transcript = Transcript.fromJSON(MessageProvider.chatHistory[chatID])
+        this.transcript = Transcript.fromJSON(history)
         this.chatModel = this.transcript.chatModel
         await this.transcript.toJSON()
         this.sendTranscript()
@@ -734,7 +726,6 @@ export abstract class MessageProvider extends MessageHandler implements vscode.D
         if (this.transcript.isEmpty) {
             return
         }
-        MessageProvider.chatHistory[this.sessionID] = await this.transcript.toJSON()
         await this.saveChatHistory()
         this.sendHistory()
     }
@@ -743,44 +734,29 @@ export abstract class MessageProvider extends MessageHandler implements vscode.D
      * Save chat history
      */
     private async saveChatHistory(): Promise<void> {
-        const userHistory = {
-            chat: MessageProvider.chatHistory,
-            input: MessageProvider.inputHistory,
-        }
-        await localStorage.setChatHistory(userHistory)
+        const json = await this.transcript.toJSON()
+        await chatHistory.saveChat(json)
     }
 
     /**
      * Delete history from current chat history and local storage
      */
     protected async deleteHistory(chatID: string): Promise<void> {
-        delete MessageProvider.chatHistory[chatID]
-        await localStorage.deleteChatHistory(chatID)
+        await chatHistory.deleteChat(chatID)
         this.sendHistory()
         telemetryService.log('CodyVSCodeExtension:deleteChatHistoryButton:clicked', undefined, { hasV2Event: true })
         telemetryRecorder.recordEvent('cody.deleteChatHistoryButton', 'clicked')
     }
 
-    /**
-     * Loads chat history from local storage
-     */
-    private loadChatHistory(): void {
-        const localHistory = localStorage.getChatHistory()
-        if (localHistory) {
-            MessageProvider.chatHistory = localHistory?.chat
-            MessageProvider.inputHistory = localHistory.input
-        }
-    }
-
     /**
      * Export chat history to file system
      */
     public async exportHistory(): Promise<void> {
         telemetryService.log('CodyVSCodeExtension:exportChatHistoryButton:clicked', undefined, { hasV2Event: true })
         telemetryRecorder.recordEvent('cody.exportChatHistoryButton', 'clicked')
-        const historyJson = MessageProvider.chatHistory
+        const historyJson = localStorage.getChatHistory()?.chat
         const exportPath = await vscode.window.showSaveDialog({ filters: { 'Chat History': ['json'] } })
-        if (!exportPath) {
+        if (!exportPath || !historyJson) {
             return
         }
         try {
@@ -801,10 +777,10 @@ export abstract class MessageProvider extends MessageHandler implements vscode.D
      * Send history to view
      */
     private sendHistory(): void {
-        this.handleHistory({
-            chat: MessageProvider.chatHistory,
-            input: MessageProvider.inputHistory,
-        })
+        const userHistory = chatHistory.localHistory
+        if (userHistory) {
+            this.handleHistory(userHistory)
+        }
     }
 
     /**

diff --git a/vscode/src/chat/chat-view/ChatHistoryManager.ts b/vscode/src/chat/chat-view/ChatHistoryManager.ts
@@ -4,25 +4,45 @@ import { UserLocalHistory } from '@sourcegraph/cody-shared/src/chat/transcript/m
 import { localStorage } from '../../services/LocalStorageProvider'
 
 export class ChatHistoryManager {
-    public getChat(sessionID: string): TranscriptJSON | null {
-        const chatHistory = localStorage.getChatHistory()
-        if (!chatHistory) {
-            return null
-        }
+    public get localHistory(): UserLocalHistory | null {
+        return localStorage.getChatHistory()
+    }
 
-        return chatHistory.chat[sessionID]
+    public getChat(sessionID: string): TranscriptJSON | null {
+        const chatHistory = this.localHistory
+        return chatHistory?.chat ? chatHistory.chat[sessionID] : null
     }
 
     public async saveChat(chat: TranscriptJSON): Promise<UserLocalHistory> {
-        let history = localStorage.getChatHistory()
-        if (!history) {
-            history = {
-                chat: {},
-                input: [],
-            }
-        }
+        const history = localStorage.getChatHistory()
         history.chat[chat.id] = chat
         await localStorage.setChatHistory(history)
         return history
     }
+
+    public async deleteChat(chatID: string): Promise<void> {
+        await localStorage.deleteChatHistory(chatID)
+    }
+
+    // HumanInputHistory is the history list when user presses "up" in the chat input box
+    public async saveHumanInputHistory(input: string): Promise<UserLocalHistory> {
+        const history = localStorage.getChatHistory()
+        history.input.push(input)
+        await localStorage.setChatHistory(history)
+        return history
+    }
+    public getHumanInputHistory(): string[] {
+        const history = localStorage.getChatHistory()
+        if (!history) {
+            return []
+        }
+        return history.input
+    }
+
+    // Remove chat history and input history
+    public async clear(): Promise<void> {
+        await localStorage.removeChatHistory()
+    }
 }
+
+export const chatHistory = new ChatHistoryManager()
diff --git a/vscode/src/chat/chat-view/ChatPanelProvider.ts b/vscode/src/chat/chat-view/ChatPanelProvider.ts
@@ -22,6 +22,7 @@ import { MessageErrorType, MessageProvider, MessageProviderOptions } from '../Me
 import { ConfigurationSubsetForWebview, ExtensionMessage, LocalEnv, WebviewMessage } from '../protocol'
 
 import { getChatPanelTitle } from './chat-helpers'
+import { chatHistory } from './ChatHistoryManager'
 import { addWebviewViewHTML, CodyChatPanelViewType } from './ChatManager'
 
 export interface ChatViewProviderWebview extends Omit<vscode.Webview, 'postMessage'> {
@@ -141,7 +142,7 @@ export class ChatPanelProvider extends MessageProvider {
     ): Promise<void> {
         logDebug('ChatPanelProvider:onHumanMessageSubmitted', 'chat', { verbose: { text, submitType } })
 
-        MessageProvider.inputHistory.push(text)
+        await chatHistory.saveHumanInputHistory(text)
 
         if (submitType === 'suggestion') {
             const args = { requestID: this.currentRequestID }
@@ -248,7 +249,7 @@ export class ChatPanelProvider extends MessageProvider {
             type: 'history',
             messages: userHistory,
         })
-        void this.treeView.updateTree(createCodyChatTreeItems(userHistory))
+        void this.treeView.updateTree(createCodyChatTreeItems())
     }
 
     /**

diff --git a/vscode/src/chat/chat-view/ChatPanelsManager.ts b/vscode/src/chat/chat-view/ChatPanelsManager.ts
@@ -11,7 +11,6 @@ import { featureFlagProvider } from '@sourcegraph/cody-shared/src/experimentatio
 import { View } from '../../../webviews/NavBar'
 import { LocalEmbeddingsController } from '../../local-context/local-embeddings'
 import { logDebug } from '../../log'
-import { localStorage } from '../../services/LocalStorageProvider'
 import { createCodyChatTreeItems } from '../../services/treeViewItems'
 import { TreeViewProvider } from '../../services/TreeViewProvider'
 import { AuthStatus } from '../protocol'
@@ -103,8 +102,6 @@ export class ChatPanelsManager implements vscode.Disposable {
                 provider.setConfiguration?.(options.contextProvider.config)
             })
         })
-
-        this.updateTreeViewHistory()
     }
 
     public async syncAuthStatus(authStatus: AuthStatus): Promise<void> {
@@ -114,6 +111,7 @@ export class ChatPanelsManager implements vscode.Disposable {
         }
 
         await vscode.commands.executeCommand('setContext', CodyChatPanelViewType, authStatus.isLoggedIn)
+        await this.updateTreeViewHistory()
     }
 
     public async getChatPanel(): Promise<IChatPanelProvider> {
@@ -235,24 +233,16 @@ export class ChatPanelsManager implements vscode.Disposable {
         await chatProvider.executeCustomCommand(title, type)
     }
 
-    private updateTreeViewHistory(): void {
-        const localHistory = localStorage.getChatHistory()
-        if (localHistory) {
-            void this.treeViewProvider.updateTree(
-                createCodyChatTreeItems({
-                    chat: localHistory?.chat,
-                    input: localHistory.input,
-                })
-            )
-        }
+    private async updateTreeViewHistory(): Promise<void> {
+        await this.treeViewProvider.updateTree(createCodyChatTreeItems())
     }
 
     public async clearHistory(chatID?: string): Promise<void> {
         if (chatID) {
             this.disposeProvider(chatID)
 
             await this.activePanelProvider?.clearChatHistory(chatID)
-            this.updateTreeViewHistory()
+            await this.updateTreeViewHistory()
             return
         }
 
@@ -316,7 +306,7 @@ export class ChatPanelsManager implements vscode.Disposable {
             provider.dispose()
         })
         this.panelProvidersMap.clear()
-        this.updateTreeViewHistory()
+        void this.updateTreeViewHistory()
     }
 
     public dispose(): void {

diff --git a/vscode/src/chat/chat-view/SidebarChatProvider.ts b/vscode/src/chat/chat-view/SidebarChatProvider.ts
@@ -29,6 +29,7 @@ import {
     WebviewMessage,
 } from '../protocol'
 
+import { chatHistory } from './ChatHistoryManager'
 import { addWebviewViewHTML } from './ChatManager'
 
 export interface SidebarChatWebview extends Omit<vscode.Webview, 'postMessage'> {
@@ -237,7 +238,7 @@ export class SidebarChatProvider extends MessageProvider implements vscode.Webvi
             verbose: { text, submitType, addEnhancedContext },
         })
 
-        MessageProvider.inputHistory.push(text)
+        await chatHistory.saveHumanInputHistory(text)
 
         if (submitType === 'suggestion') {
             const args = { requestID: this.currentRequestID }

diff --git a/vscode/src/chat/chat-view/SimpleChatPanelProvider.ts b/vscode/src/chat/chat-view/SimpleChatPanelProvider.ts
@@ -355,7 +355,7 @@ export class SimpleChatPanelProvider implements vscode.Disposable, IChatPanelPro
             type: 'history',
             messages: allHistory,
         })
-        await this.treeView.updateTree(createCodyChatTreeItems(allHistory))
+        await this.treeView.updateTree(createCodyChatTreeItems())
     }
 
     public async clearAndRestartSession(): Promise<void> {
@@ -405,7 +405,7 @@ export class SimpleChatPanelProvider implements vscode.Disposable, IChatPanelPro
             const args = { requestID }
             telemetryService.log('CodyVSCodeExtension:chatPredictions:used', args, { hasV2Event: true })
         }
-
+        await this.history.saveHumanInputHistory(text)
         this.chatModel.addHumanMessage({ text })
         // trigger the context progress indicator
         void this.postViewTranscript({ speaker: 'assistant' })

diff --git a/vscode/src/services/LocalStorageProvider.ts b/vscode/src/services/LocalStorageProvider.ts
@@ -67,9 +67,9 @@ export class LocalStorage {
         await this.storage.update(this.CODY_ENDPOINT_HISTORY, [...historySet])
     }
 
-    public getChatHistory(): UserLocalHistory | null {
+    public getChatHistory(): UserLocalHistory {
         const history = this.storage.get<UserLocalHistory | null>(this.KEY_LOCAL_HISTORY, null)
-        return history
+        return history || { chat: {}, input: [] }
     }
 
     public async setChatHistory(history: UserLocalHistory): Promise<void> {

diff --git a/vscode/src/services/treeViewItems.ts b/vscode/src/services/treeViewItems.ts
@@ -1,10 +1,10 @@
-import { UserLocalHistory } from '@sourcegraph/cody-shared/src/chat/transcript/messages'
 import { FeatureFlag } from '@sourcegraph/cody-shared/src/experimentation/FeatureFlagProvider'
 
 import { getChatPanelTitle } from '../chat/chat-view/chat-helpers'
 import { CODY_DOC_URL, CODY_FEEDBACK_URL, DISCORD_URL } from '../chat/protocol'
 
 import { envInit } from './LocalAppDetector'
+import { localStorage } from './LocalStorageProvider'
 
 export type CodyTreeItemType = 'command' | 'support' | 'search' | 'chat'
 
@@ -37,9 +37,13 @@ export function getCodyTreeItems(type: CodyTreeItemType): CodySidebarTreeItem[]
 }
 
 // functon to create chat tree items from user chat history
-export function createCodyChatTreeItems(userHistory: UserLocalHistory): CodySidebarTreeItem[] {
+export function createCodyChatTreeItems(): CodySidebarTreeItem[] {
+    const userHistory = localStorage.getChatHistory()?.chat
+    if (!userHistory) {
+        return []
+    }
     const chatTreeItems: CodySidebarTreeItem[] = []
-    const chatHistoryEntries = [...Object.entries(userHistory.chat)]
+    const chatHistoryEntries = [...Object.entries(userHistory)]
     chatHistoryEntries.forEach(([id, entry]) => {
         const lastHumanMessage = entry?.interactions?.findLast(interaction => interaction?.humanMessage)
         if (lastHumanMessage?.humanMessage.displayText && lastHumanMessage?.humanMessage.text) {