-
Notifications
You must be signed in to change notification settings - Fork 751
fix(chat): fix for chat history for multuple tabs and user tool ignore #6906
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
Changes from 1 commit
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 |
|---|---|---|
|
|
@@ -87,11 +87,11 @@ import { | |
| defaultContextLengths, | ||
| } from '../../constants' | ||
| import { ChatSession } from '../../clients/chat/v0/chat' | ||
| import { ChatHistoryManager } from '../../storages/chatHistory' | ||
| import { amazonQTabSuffix } from '../../../shared/constants' | ||
| import { OutputKind } from '../../tools/toolShared' | ||
| import { ToolUtils, Tool, ToolType } from '../../tools/toolUtils' | ||
| import { ChatStream } from '../../tools/chatStream' | ||
| import { ChatHistoryStorage } from '../../storages/chatHistoryStorage' | ||
| import { FsWrite, FsWriteParams } from '../../tools/fsWrite' | ||
| import { tempDirPath } from '../../../shared/filesystemUtilities' | ||
|
|
||
|
|
@@ -155,7 +155,7 @@ export class ChatController { | |
| private readonly userIntentRecognizer: UserIntentRecognizer | ||
| private readonly telemetryHelper: CWCTelemetryHelper | ||
| private userPromptsWatcher: vscode.FileSystemWatcher | undefined | ||
| private readonly chatHistoryManager: ChatHistoryManager | ||
| private readonly chatHistoryStorage: ChatHistoryStorage | ||
|
|
||
| public constructor( | ||
| private readonly chatControllerMessageListeners: ChatControllerMessageListeners, | ||
|
|
@@ -173,7 +173,7 @@ export class ChatController { | |
| this.editorContentController = new EditorContentController() | ||
| this.promptGenerator = new PromptsGenerator() | ||
| this.userIntentRecognizer = new UserIntentRecognizer() | ||
| this.chatHistoryManager = new ChatHistoryManager() | ||
| this.chatHistoryStorage = new ChatHistoryStorage() | ||
|
|
||
| onDidChangeAmazonQVisibility((visible) => { | ||
| if (visible) { | ||
|
|
@@ -424,7 +424,7 @@ export class ChatController { | |
|
|
||
| private async processTabCloseMessage(message: TabClosedMessage) { | ||
| this.sessionStorage.deleteSession(message.tabID) | ||
| this.chatHistoryManager.clear() | ||
| this.chatHistoryStorage.deleteHistory(message.tabID) | ||
| this.triggerEventsStorage.removeTabEvents(message.tabID) | ||
| // this.telemetryHelper.recordCloseChat(message.tabID) | ||
| } | ||
|
|
@@ -710,7 +710,7 @@ export class ChatController { | |
| customization: getSelectedCustomization(), | ||
| toolResults: toolResults, | ||
| origin: Origin.IDE, | ||
| chatHistory: this.chatHistoryManager.getHistory(), | ||
| chatHistory: this.chatHistoryStorage.getHistory(tabID).getHistory(), | ||
| context: session.context ?? [], | ||
| relevantTextDocuments: [], | ||
| additionalContents: [], | ||
|
|
@@ -890,7 +890,6 @@ export class ChatController { | |
| getLogger().error(`error: ${errorMessage} tabID: ${tabID} requestID: ${requestID}`) | ||
|
|
||
| this.sessionStorage.deleteSession(tabID) | ||
| this.chatHistoryManager.clear() | ||
| } | ||
|
|
||
| private async processContextMenuCommand(command: EditorContextCommand) { | ||
|
|
@@ -964,7 +963,6 @@ export class ChatController { | |
| codeQuery: context?.focusAreaContext?.names, | ||
| userIntent: this.userIntentRecognizer.getFromContextMenuCommand(command), | ||
| customization: getSelectedCustomization(), | ||
| chatHistory: this.chatHistoryManager.getHistory(), | ||
| additionalContents: [], | ||
| relevantTextDocuments: [], | ||
| documentReferences: [], | ||
|
|
@@ -1012,7 +1010,7 @@ export class ChatController { | |
| switch (message.command) { | ||
| case 'clear': | ||
| this.sessionStorage.deleteSession(message.tabID) | ||
| this.chatHistoryManager.clear() | ||
| this.chatHistoryStorage.getHistory(message.tabID).clear() | ||
| this.triggerEventsStorage.removeTabEvents(message.tabID) | ||
| recordTelemetryChatRunCommand('clear') | ||
| return | ||
|
|
@@ -1051,7 +1049,7 @@ export class ChatController { | |
| codeQuery: lastTriggerEvent.context?.focusAreaContext?.names, | ||
| userIntent: message.userIntent, | ||
| customization: getSelectedCustomization(), | ||
| chatHistory: this.chatHistoryManager.getHistory(), | ||
| chatHistory: this.chatHistoryStorage.getHistory(message.tabID).getHistory(), | ||
| contextLengths: { | ||
| ...defaultContextLengths, | ||
| }, | ||
|
|
@@ -1100,7 +1098,7 @@ export class ChatController { | |
| codeQuery: context?.focusAreaContext?.names, | ||
| userIntent: this.userIntentRecognizer.getFromPromptChatMessage(message), | ||
| customization: getSelectedCustomization(), | ||
| chatHistory: this.chatHistoryManager.getHistory(), | ||
| chatHistory: this.chatHistoryStorage.getHistory(message.tabID).getHistory(), | ||
| origin: Origin.IDE, | ||
| context: message.context ?? [], | ||
| relevantTextDocuments: [], | ||
|
|
@@ -1327,16 +1325,28 @@ export class ChatController { | |
|
|
||
| triggerPayload.contextLengths.userInputContextLength = triggerPayload.message.length | ||
| triggerPayload.contextLengths.focusFileContextLength = triggerPayload.fileText.length | ||
| const request = triggerPayloadToChatRequest(triggerPayload) | ||
| if ( | ||
| this.chatHistoryManager.getConversationId() !== undefined && | ||
| this.chatHistoryManager.getConversationId() !== '' | ||
| ) { | ||
| request.conversationState.conversationId = this.chatHistoryManager.getConversationId() | ||
| } else { | ||
| this.chatHistoryManager.setConversationId(randomUUID()) | ||
| request.conversationState.conversationId = this.chatHistoryManager.getConversationId() | ||
|
|
||
| const chatHistory = this.chatHistoryStorage.getHistory(tabID) | ||
| const newUserMessage = { | ||
| userInputMessage: { | ||
| content: triggerPayload.message, | ||
| userIntent: triggerPayload.userIntent, | ||
| ...(triggerPayload.origin && { origin: triggerPayload.origin }), | ||
| userInputMessageContext: { | ||
| tools: tools, | ||
| ...(triggerPayload.toolResults && { toolResults: triggerPayload.toolResults }), | ||
| }, | ||
| }, | ||
| } | ||
| const fixedHistoryMessage = chatHistory.fixHistory(newUserMessage) | ||
| if (fixedHistoryMessage.userInputMessage?.userInputMessageContext) { | ||
| triggerPayload.toolResults = fixedHistoryMessage.userInputMessage.userInputMessageContext.toolResults | ||
| } | ||
|
Comment on lines
+1342
to
1344
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. why can't we do this inside
Contributor
Author
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 can but i'm not sending triggerPayload to fixHistory. fixHistory could be used for other operations, so i set it up to take in only ChatMessage |
||
| const request = triggerPayloadToChatRequest(triggerPayload) | ||
| const conversationId = chatHistory.getConversationId() || randomUUID() | ||
| chatHistory.setConversationId(conversationId) | ||
| request.conversationState.conversationId = conversationId | ||
|
|
||
| triggerPayload.documentReferences = this.mergeRelevantTextDocuments(triggerPayload.relevantTextDocuments) | ||
|
|
||
| // Update context transparency after it's truncated dynamically to show users only the context sent. | ||
|
|
@@ -1386,32 +1396,14 @@ export class ChatController { | |
| } | ||
| this.telemetryHelper.recordEnterFocusConversation(triggerEvent.tabID) | ||
| this.telemetryHelper.recordStartConversation(triggerEvent, triggerPayload) | ||
|
|
||
| this.chatHistoryManager.appendUserMessage({ | ||
| userInputMessage: { | ||
| content: triggerPayload.message, | ||
| userIntent: triggerPayload.userIntent, | ||
| ...(triggerPayload.origin && { origin: triggerPayload.origin }), | ||
| userInputMessageContext: { | ||
| tools: tools, | ||
| ...(triggerPayload.toolResults && { toolResults: triggerPayload.toolResults }), | ||
| }, | ||
| }, | ||
| }) | ||
| chatHistory.appendUserMessage(fixedHistoryMessage) | ||
|
|
||
| getLogger().info( | ||
| `response to tab: ${tabID} conversationID: ${session.sessionIdentifier} requestID: ${ | ||
| response.$metadata.requestId | ||
| } metadata: ${inspect(response.$metadata, { depth: 12 })}` | ||
| ) | ||
| await this.messenger.sendAIResponse( | ||
| response, | ||
| session, | ||
| tabID, | ||
| triggerID, | ||
| triggerPayload, | ||
| this.chatHistoryManager | ||
| ) | ||
| await this.messenger.sendAIResponse(response, session, tabID, triggerID, triggerPayload, chatHistory) | ||
| } catch (e: any) { | ||
| this.telemetryHelper.recordMessageResponseError(triggerPayload, tabID, getHttpStatusCode(e) ?? 0) | ||
| // clears session, record telemetry before this call | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| /*! | ||
| * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| import { ChatHistoryManager } from './chatHistory' | ||
|
|
||
| /** | ||
| * ChatHistoryStorage manages ChatHistoryManager instances for multiple tabs. | ||
| * Each tab has its own ChatHistoryManager to maintain separate chat histories. | ||
| */ | ||
| export class ChatHistoryStorage { | ||
| private histories: Map<string, ChatHistoryManager> = new Map() | ||
|
|
||
| /** | ||
| * Gets the ChatHistoryManager for a specific tab. | ||
| * If no history exists for the tab, creates a new one. | ||
| * | ||
| * @param tabId The ID of the tab | ||
| * @returns The ChatHistoryManager for the specified tab | ||
| */ | ||
| public getHistory(tabId: string): ChatHistoryManager { | ||
| const historyFromStorage = this.histories.get(tabId) | ||
| if (historyFromStorage !== undefined) { | ||
| return historyFromStorage | ||
| } | ||
|
|
||
| // Create a new ChatHistoryManager with the tabId | ||
| const newHistory = new ChatHistoryManager(tabId) | ||
| this.histories.set(tabId, newHistory) | ||
|
|
||
| return newHistory | ||
| } | ||
|
|
||
| /** | ||
| * Deletes the ChatHistoryManager for a specific tab. | ||
| * | ||
| * @param tabId The ID of the tab | ||
| */ | ||
| public deleteHistory(tabId: string) { | ||
| this.histories.delete(tabId) | ||
| } | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.