-
Notifications
You must be signed in to change notification settings - Fork 749
feat(chat): Adding state Management to stop the response for new user input prompt #6934
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 all commits
9b5cef6
5ac03e7
c5456cd
e761f78
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 |
|---|---|---|
|
|
@@ -295,7 +295,7 @@ export class Messenger { | |
| } | ||
| return true | ||
| }, | ||
| { timeout: 60000, truthy: true } | ||
| { timeout: 600000, truthy: true } | ||
|
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. Is this an intentional increase?
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. yes....because fsWrite for long files is exceeding and failing the conversation from service chat history validations.
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. Just to add bit more context: the 60 second timeout is for the mynah chat, for agentic chat, we noticed the request could easily exceed 60 seconds if it's trying to create some new large files, hence making this change. (This is also causing the history to be invalid when we hit the 60s timeout) |
||
| ) | ||
| .catch((error: any) => { | ||
| let errorMessage = 'Error reading chat stream.' | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,14 +2,7 @@ | |
| * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
| import { | ||
| ChatMessage, | ||
| Tool, | ||
| ToolResult, | ||
| ToolResultStatus, | ||
| UserInputMessage, | ||
| UserInputMessageContext, | ||
| } from '@amzn/codewhisperer-streaming' | ||
| import { ChatMessage, Tool, ToolResult, ToolResultStatus, ToolUse } from '@amzn/codewhisperer-streaming' | ||
| import { randomUUID } from '../../shared/crypto' | ||
| import { getLogger } from '../../shared/logger/logger' | ||
| import { tools } from '../constants' | ||
|
|
@@ -105,168 +98,99 @@ export class ChatHistoryManager { | |
| * message is set without tool results, then the user message will have cancelled tool results. | ||
| */ | ||
| public fixHistory(newUserMessage: ChatMessage): ChatMessage { | ||
| // Trim the conversation history if it exceeds the maximum length | ||
| if (this.history.length > MaxConversationHistoryLength) { | ||
| // Find the second oldest user message without tool results | ||
| let indexToTrim: number | undefined | ||
| this.trimConversationHistory() | ||
| this.ensureLastMessageFromAssistant() | ||
| return this.handleToolUses(newUserMessage) | ||
| } | ||
|
|
||
| for (let i = 1; i < this.history.length; i++) { | ||
| const message = this.history[i] | ||
| if (message.userInputMessage) { | ||
| const userMessage = message.userInputMessage | ||
| const ctx = userMessage.userInputMessageContext | ||
| const hasNoToolResults = ctx && (!ctx.toolResults || ctx.toolResults.length === 0) | ||
| if (hasNoToolResults && userMessage.content !== '') { | ||
| indexToTrim = i | ||
| break | ||
| } | ||
| } | ||
| } | ||
| if (indexToTrim !== undefined) { | ||
| this.logger.debug(`Removing the first ${indexToTrim} elements in the history`) | ||
| this.history.splice(0, indexToTrim) | ||
| } else { | ||
| this.logger.debug('No valid starting user message found in the history, clearing') | ||
| this.history = [] | ||
| } | ||
| private trimConversationHistory(): void { | ||
| if (this.history.length <= MaxConversationHistoryLength) { | ||
| return | ||
| } | ||
|
|
||
| // Ensure the last message is from the assistant | ||
| if (this.history.length > 0 && this.history[this.history.length - 1].userInputMessage !== undefined) { | ||
| this.logger.debug('Last message in history is from the user, dropping') | ||
| this.history.pop() | ||
| const indexToTrim = this.findIndexToTrim() | ||
| if (indexToTrim !== undefined) { | ||
| this.logger.debug(`Removing the first ${indexToTrim} elements in the history`) | ||
| this.history.splice(0, indexToTrim) | ||
| } else { | ||
| this.logger.debug('No valid starting user message found in the history, clearing') | ||
| this.history = [] | ||
| } | ||
| } | ||
|
|
||
| // If the last message from the assistant contains tool uses, ensure the next user message contains tool results | ||
|
|
||
| const lastHistoryMessage = this.history[this.history.length - 1] | ||
|
|
||
| if ( | ||
| lastHistoryMessage && | ||
| (lastHistoryMessage.assistantResponseMessage || | ||
| lastHistoryMessage.assistantResponseMessage !== undefined) && | ||
| newUserMessage | ||
| ) { | ||
| const toolUses = lastHistoryMessage.assistantResponseMessage.toolUses | ||
|
|
||
| if (toolUses && toolUses.length > 0) { | ||
| if (newUserMessage.userInputMessage) { | ||
| if (newUserMessage.userInputMessage.userInputMessageContext) { | ||
| const ctx = newUserMessage.userInputMessage.userInputMessageContext | ||
|
|
||
| if (!ctx.toolResults || ctx.toolResults.length === 0) { | ||
| ctx.toolResults = toolUses.map((toolUse) => ({ | ||
| toolUseId: toolUse.toolUseId, | ||
| content: [ | ||
| { | ||
| type: 'Text', | ||
| text: 'Tool use was cancelled by the user', | ||
| }, | ||
| ], | ||
| status: ToolResultStatus.ERROR, | ||
| })) | ||
| } | ||
| } else { | ||
| const toolResults = toolUses.map((toolUse) => ({ | ||
| toolUseId: toolUse.toolUseId, | ||
| content: [ | ||
| { | ||
| type: 'Text', | ||
| text: 'Tool use was cancelled by the user', | ||
| }, | ||
| ], | ||
| status: ToolResultStatus.ERROR, | ||
| })) | ||
|
|
||
| newUserMessage.userInputMessage.userInputMessageContext = { | ||
| shellState: undefined, | ||
| envState: undefined, | ||
| toolResults: toolResults, | ||
| tools: this.tools.length === 0 ? undefined : [...this.tools], | ||
| } | ||
|
|
||
| return newUserMessage | ||
| } | ||
| } | ||
| private findIndexToTrim(): number | undefined { | ||
|
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 think we would probably change this to follow more of what the CLI will do using history compacting. This is ok for now.
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. Will look into the CLI's history compacting. I think they followed this sliding window trimming until a 10 days ago when i ported their history utilities. |
||
| for (let i = 1; i < this.history.length; i++) { | ||
| const message = this.history[i] | ||
| if (this.isValidUserMessageWithoutToolResults(message)) { | ||
| return i | ||
| } | ||
| } | ||
|
|
||
| // Always return the message to fix the TypeScript error | ||
| return newUserMessage | ||
| return undefined | ||
| } | ||
|
|
||
| /** | ||
| * Adds tool results to the conversation. | ||
| */ | ||
| addToolResults(toolResults: ToolResult[]): void { | ||
| const userInputMessageContext: UserInputMessageContext = { | ||
| shellState: undefined, | ||
| envState: undefined, | ||
| toolResults: toolResults, | ||
| tools: this.tools.length === 0 ? undefined : [...this.tools], | ||
| } | ||
|
|
||
| const msg: UserInputMessage = { | ||
| content: '', | ||
| userInputMessageContext: userInputMessageContext, | ||
| private isValidUserMessageWithoutToolResults(message: ChatMessage): boolean { | ||
| if (!message.userInputMessage) { | ||
| return false | ||
| } | ||
| const ctx = message.userInputMessage.userInputMessageContext | ||
| return Boolean( | ||
| ctx && (!ctx.toolResults || ctx.toolResults.length === 0) && message.userInputMessage.content !== '' | ||
| ) | ||
| } | ||
|
|
||
| if (this.lastUserMessage?.userInputMessage) { | ||
| this.lastUserMessage.userInputMessage = msg | ||
| private ensureLastMessageFromAssistant(): void { | ||
| if (this.history.length > 0 && this.history[this.history.length - 1].userInputMessage !== undefined) { | ||
| this.logger.debug('Last message in history is from the user, dropping') | ||
| this.history.pop() | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Checks if the latest message in history is an Assistant Message. | ||
| * If it is and doesn't have toolUse, it will be removed. | ||
| * If it has toolUse, an assistantResponse message with cancelled tool status will be added. | ||
| */ | ||
| public checkLatestAssistantMessage(): void { | ||
| if (this.history.length === 0) { | ||
| return | ||
| private handleToolUses(newUserMessage: ChatMessage): ChatMessage { | ||
| const lastHistoryMessage = this.history[this.history.length - 1] | ||
| if (!lastHistoryMessage || !lastHistoryMessage.assistantResponseMessage || !newUserMessage) { | ||
| return newUserMessage | ||
| } | ||
|
|
||
| const lastMessage = this.history[this.history.length - 1] | ||
|
|
||
| if (lastMessage.assistantResponseMessage) { | ||
| const toolUses = lastMessage.assistantResponseMessage.toolUses | ||
| const toolUses = lastHistoryMessage.assistantResponseMessage.toolUses | ||
| if (!toolUses || toolUses.length === 0) { | ||
| return newUserMessage | ||
| } | ||
|
|
||
| if (!toolUses || toolUses.length === 0) { | ||
| // If there are no tool uses, remove the assistant message | ||
| this.logger.debug('Removing assistant message without tool uses') | ||
| this.history.pop() | ||
| } else { | ||
| // If there are tool uses, add cancelled tool results | ||
| const toolResults = toolUses.map((toolUse) => ({ | ||
| toolUseId: toolUse.toolUseId, | ||
| content: [ | ||
| { | ||
| type: 'Text', | ||
| text: 'Tool use was cancelled by the user', | ||
| }, | ||
| ], | ||
| status: ToolResultStatus.ERROR, | ||
| })) | ||
| return this.addToolResultsToUserMessage(newUserMessage, toolUses) | ||
| } | ||
|
|
||
| // Create a new user message with cancelled tool results | ||
| const userInputMessageContext: UserInputMessageContext = { | ||
| shellState: undefined, | ||
| envState: undefined, | ||
| toolResults: toolResults, | ||
| tools: this.tools.length === 0 ? undefined : [...this.tools], | ||
| } | ||
| private addToolResultsToUserMessage(newUserMessage: ChatMessage, toolUses: ToolUse[]): ChatMessage { | ||
| if (!newUserMessage.userInputMessage) { | ||
| return newUserMessage | ||
| } | ||
|
|
||
| const userMessage: ChatMessage = { | ||
| userInputMessage: { | ||
| content: '', | ||
| userInputMessageContext: userInputMessageContext, | ||
| }, | ||
| } | ||
| const toolResults = this.createToolResults(toolUses) | ||
|
|
||
| this.history.push(this.formatChatHistoryMessage(userMessage)) | ||
| this.logger.debug('Added user message with cancelled tool results') | ||
| if (newUserMessage.userInputMessage.userInputMessageContext) { | ||
| newUserMessage.userInputMessage.userInputMessageContext.toolResults = toolResults | ||
| } else { | ||
| newUserMessage.userInputMessage.userInputMessageContext = { | ||
|
Comment on lines
+171
to
+172
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. when would this happen? (UserInputMessageContext is undefined) 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. Same question
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. As of now..haven't seen any case of UserInputMessageContext being undefined but covered the case for any potential errors to maintain history thread. |
||
| shellState: undefined, | ||
| envState: undefined, | ||
| toolResults: toolResults, | ||
| tools: this.tools.length === 0 ? undefined : [...this.tools], | ||
| } | ||
| } | ||
|
|
||
| return newUserMessage | ||
| } | ||
|
|
||
| private createToolResults(toolUses: ToolUse[]): ToolResult[] { | ||
| return toolUses.map((toolUse) => ({ | ||
| toolUseId: toolUse.toolUseId, | ||
| content: [ | ||
| { | ||
| type: 'Text', | ||
| text: 'Tool use was cancelled by the user', | ||
| }, | ||
| ], | ||
| status: ToolResultStatus.ERROR, | ||
| })) | ||
| } | ||
|
|
||
| private formatChatHistoryMessage(message: ChatMessage): ChatMessage { | ||
|
|
@@ -283,4 +207,18 @@ export class ChatHistoryManager { | |
| } | ||
| return message | ||
| } | ||
|
|
||
| public clearRecentHistory(): void { | ||
| if (this.history.length === 0) { | ||
| return | ||
| } | ||
|
|
||
| const lastHistoryMessage = this.history[this.history.length - 1] | ||
|
|
||
| if (lastHistoryMessage.userInputMessage?.userInputMessageContext) { | ||
| this.history.pop() | ||
| } else if (lastHistoryMessage.assistantResponseMessage) { | ||
| this.history.splice(-2) | ||
| } | ||
| } | ||
| } | ||
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.
Isn't this going to keep setting/resetting the flag within a single agentic loop?
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.
currently yes...i'll expand it to tool use next to stop tool execution.
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.
It is a bit limited right now