Refactor apiConversationHistory and claudeMessages to be local to ClaudeDev instance to avoid polluting storage unnecessarily

This commit is contained in:
Saoud Rizwan
2024-08-02 14:59:33 -04:00
parent c994f64c6f
commit 89c242d966
2 changed files with 63 additions and 63 deletions

View File

@@ -13,7 +13,7 @@ import { listFiles, parseSourceCodeForDefinitionsTopLevel } from "./parse-source
import { ClaudeDevProvider } from "./providers/ClaudeDevProvider" import { ClaudeDevProvider } from "./providers/ClaudeDevProvider"
import { ClaudeRequestResult } from "./shared/ClaudeRequestResult" import { ClaudeRequestResult } from "./shared/ClaudeRequestResult"
import { DEFAULT_MAX_REQUESTS_PER_TASK } from "./shared/Constants" import { DEFAULT_MAX_REQUESTS_PER_TASK } from "./shared/Constants"
import { ClaudeAsk, ClaudeSay, ClaudeSayTool } from "./shared/ExtensionMessage" import { ClaudeAsk, ClaudeMessage, ClaudeSay, ClaudeSayTool } from "./shared/ExtensionMessage"
import { Tool, ToolName } from "./shared/Tool" import { Tool, ToolName } from "./shared/Tool"
import { ClaudeAskResponse } from "./shared/WebviewMessage" import { ClaudeAskResponse } from "./shared/WebviewMessage"
@@ -227,6 +227,8 @@ export class ClaudeDev {
private client: Anthropic private client: Anthropic
private maxRequestsPerTask: number private maxRequestsPerTask: number
private requestCount = 0 private requestCount = 0
apiConversationHistory: Anthropic.MessageParam[] = []
claudeMessages: ClaudeMessage[] = []
private askResponse?: ClaudeAskResponse private askResponse?: ClaudeAskResponse
private askResponseText?: string private askResponseText?: string
private lastMessageTs?: number private lastMessageTs?: number
@@ -263,7 +265,7 @@ export class ClaudeDev {
this.askResponseText = undefined this.askResponseText = undefined
const askTs = Date.now() const askTs = Date.now()
this.lastMessageTs = askTs this.lastMessageTs = askTs
await this.providerRef.deref()?.addClaudeMessage({ ts: askTs, type: "ask", ask: type, text: question }) this.claudeMessages.push({ ts: askTs, type: "ask", ask: type, text: question })
await this.providerRef.deref()?.postStateToWebview() await this.providerRef.deref()?.postStateToWebview()
await pWaitFor(() => this.askResponse !== undefined || this.lastMessageTs !== askTs, { interval: 100 }) await pWaitFor(() => this.askResponse !== undefined || this.lastMessageTs !== askTs, { interval: 100 })
if (this.lastMessageTs !== askTs) { if (this.lastMessageTs !== askTs) {
@@ -281,14 +283,15 @@ export class ClaudeDev {
} }
const sayTs = Date.now() const sayTs = Date.now()
this.lastMessageTs = sayTs this.lastMessageTs = sayTs
await this.providerRef.deref()?.addClaudeMessage({ ts: sayTs, type: "say", say: type, text: text }) this.claudeMessages.push({ ts: sayTs, type: "say", say: type, text: text })
await this.providerRef.deref()?.postStateToWebview() await this.providerRef.deref()?.postStateToWebview()
} }
private async startTask(task: string): Promise<void> { private async startTask(task: string): Promise<void> {
// conversationHistory (for API) and claudeMessages (for webview) need to be in sync // conversationHistory (for API) and claudeMessages (for webview) need to be in sync
// if the extension process were killed, then on restart the claudeMessages might not be empty, so we need to set it to [] when we create a new ClaudeDev client (otherwise webview would show stale messages from previous session) // if the extension process were killed, then on restart the claudeMessages might not be empty, so we need to set it to [] when we create a new ClaudeDev client (otherwise webview would show stale messages from previous session)
await this.providerRef.deref()?.setClaudeMessages(undefined) this.claudeMessages = []
this.apiConversationHistory = []
await this.providerRef.deref()?.postStateToWebview() await this.providerRef.deref()?.postStateToWebview()
// This first message kicks off a task, it is not included in every subsequent message. // This first message kicks off a task, it is not included in every subsequent message.
@@ -693,7 +696,7 @@ export class ClaudeDev {
// beta max tokens // beta max tokens
max_tokens: 8192, max_tokens: 8192,
system: SYSTEM_PROMPT(), system: SYSTEM_PROMPT(),
messages: (await this.providerRef.deref()?.getApiConversationHistory()) || [], messages: this.apiConversationHistory,
tools: tools, tools: tools,
tool_choice: { type: "auto" }, tool_choice: { type: "auto" },
}, },
@@ -729,7 +732,7 @@ export class ClaudeDev {
throw new Error("ClaudeDev instance aborted") throw new Error("ClaudeDev instance aborted")
} }
await this.providerRef.deref()?.addMessageToApiConversationHistory({ role: "user", content: userContent }) this.apiConversationHistory.push({ role: "user", content: userContent })
if (this.requestCount >= this.maxRequestsPerTask) { if (this.requestCount >= this.maxRequestsPerTask) {
const { response } = await this.ask( const { response } = await this.ask(
"request_limit_reached", "request_limit_reached",
@@ -739,7 +742,7 @@ export class ClaudeDev {
if (response === "yesButtonTapped") { if (response === "yesButtonTapped") {
this.requestCount = 0 this.requestCount = 0
} else { } else {
await this.providerRef.deref()?.addMessageToApiConversationHistory({ this.apiConversationHistory.push({
role: "assistant", role: "assistant",
content: [ content: [
{ {
@@ -823,13 +826,11 @@ export class ClaudeDev {
} }
if (assistantResponses.length > 0) { if (assistantResponses.length > 0) {
await this.providerRef this.apiConversationHistory.push({ role: "assistant", content: assistantResponses })
.deref()
?.addMessageToApiConversationHistory({ role: "assistant", content: assistantResponses })
} else { } else {
// this should never happen! it there's no assistant_responses, that means we got no text or tool_use content blocks from API which we should assume is an error // this should never happen! it there's no assistant_responses, that means we got no text or tool_use content blocks from API which we should assume is an error
this.say("error", "Unexpected Error: No assistant messages were found in the API response") this.say("error", "Unexpected Error: No assistant messages were found in the API response")
await this.providerRef.deref()?.addMessageToApiConversationHistory({ this.apiConversationHistory.push({
role: "assistant", role: "assistant",
content: [{ type: "text", text: "Failure: I did not have a response to provide." }], content: [{ type: "text", text: "Failure: I did not have a response to provide." }],
}) })
@@ -859,10 +860,8 @@ export class ClaudeDev {
if (toolResults.length > 0) { if (toolResults.length > 0) {
if (didEndLoop) { if (didEndLoop) {
await this.providerRef this.apiConversationHistory.push({ role: "user", content: toolResults })
.deref() this.apiConversationHistory.push({
?.addMessageToApiConversationHistory({ role: "user", content: toolResults })
await this.providerRef.deref()?.addMessageToApiConversationHistory({
role: "assistant", role: "assistant",
content: [ content: [
{ {

View File

@@ -19,7 +19,6 @@ export class ClaudeDevProvider implements vscode.WebviewViewProvider {
public static readonly tabPanelId = "claude-dev.TabPanelProvider" public static readonly tabPanelId = "claude-dev.TabPanelProvider"
private disposables: vscode.Disposable[] = [] private disposables: vscode.Disposable[] = []
private view?: vscode.WebviewView | vscode.WebviewPanel private view?: vscode.WebviewView | vscode.WebviewPanel
private providerInstanceIdentifier = Date.now()
private claudeDev?: ClaudeDev private claudeDev?: ClaudeDev
private latestAnnouncementId = "jul-29-2024" // update to some unique identifier when we add a new announcement private latestAnnouncementId = "jul-29-2024" // update to some unique identifier when we add a new announcement
@@ -307,7 +306,7 @@ export class ClaudeDevProvider implements vscode.WebviewViewProvider {
const fileName = `claude_dev_task_${month}-${day}-${year}_${hours}-${minutes}-${ampm}.md` const fileName = `claude_dev_task_${month}-${day}-${year}_${hours}-${minutes}-${ampm}.md`
// Generate markdown // Generate markdown
const conversationHistory = await this.getApiConversationHistory() const conversationHistory = this.claudeDev?.apiConversationHistory || []
const markdownContent = conversationHistory const markdownContent = conversationHistory
.map((message) => { .map((message) => {
const role = message.role === "user" ? "**User:**" : "**Assistant:**" const role = message.role === "user" ? "**User:**" : "**Assistant:**"
@@ -370,10 +369,9 @@ export class ClaudeDevProvider implements vscode.WebviewViewProvider {
} }
async postStateToWebview() { async postStateToWebview() {
const [apiKey, maxRequestsPerTask, claudeMessages, lastShownAnnouncementId] = await Promise.all([ const [apiKey, maxRequestsPerTask, lastShownAnnouncementId] = await Promise.all([
this.getSecret("apiKey") as Promise<string | undefined>, this.getSecret("apiKey") as Promise<string | undefined>,
this.getGlobalState("maxRequestsPerTask") as Promise<number | undefined>, this.getGlobalState("maxRequestsPerTask") as Promise<number | undefined>,
this.getClaudeMessages(),
this.getGlobalState("lastShownAnnouncementId") as Promise<string | undefined>, this.getGlobalState("lastShownAnnouncementId") as Promise<string | undefined>,
]) ])
this.postMessageToWebview({ this.postMessageToWebview({
@@ -382,7 +380,7 @@ export class ClaudeDevProvider implements vscode.WebviewViewProvider {
apiKey, apiKey,
maxRequestsPerTask, maxRequestsPerTask,
themeName: vscode.workspace.getConfiguration("workbench").get<string>("colorTheme"), themeName: vscode.workspace.getConfiguration("workbench").get<string>("colorTheme"),
claudeMessages, claudeMessages: this.claudeDev?.claudeMessages || [],
shouldShowAnnouncement: lastShownAnnouncementId !== this.latestAnnouncementId, shouldShowAnnouncement: lastShownAnnouncementId !== this.latestAnnouncementId,
}, },
}) })
@@ -393,14 +391,14 @@ export class ClaudeDevProvider implements vscode.WebviewViewProvider {
this.claudeDev.abort = true // will stop any agentically running promises this.claudeDev.abort = true // will stop any agentically running promises
this.claudeDev = undefined // removes reference to it, so once promises end it will be garbage collected this.claudeDev = undefined // removes reference to it, so once promises end it will be garbage collected
} }
await this.setApiConversationHistory(undefined) // this.setApiConversationHistory(undefined)
await this.setClaudeMessages(undefined) // this.setClaudeMessages(undefined)
} }
// Caching mechanism to keep track of webview messages + API conversation history per provider instance // Caching mechanism to keep track of webview messages + API conversation history per provider instance
/* /*
Now that we use retainContextWhenHidden, we don't have to store a cache of claude messages in the user's state, but we do to reduce memory footprint in long conversations. Now that we use retainContextWhenHidden, we don't have to store a cache of claude messages in the user's state, but we could to reduce memory footprint in long conversations.
- We have to be careful of what state is shared between ClaudeDevProvider instances since there could be multiple instances of the extension running at once. For example when we cached claude messages using the same key, two instances of the extension could end up using the same key and overwriting each other's messages. - We have to be careful of what state is shared between ClaudeDevProvider instances since there could be multiple instances of the extension running at once. For example when we cached claude messages using the same key, two instances of the extension could end up using the same key and overwriting each other's messages.
- Some state does need to be shared between the instances, i.e. the API key--however there doesn't seem to be a good way to notfy the other instances that the API key has changed. - Some state does need to be shared between the instances, i.e. the API key--however there doesn't seem to be a good way to notfy the other instances that the API key has changed.
@@ -411,32 +409,36 @@ export class ClaudeDevProvider implements vscode.WebviewViewProvider {
However in the future when we implement task history, we'll need to use a unique identifier for each task. As well as manage a data structure that keeps track of task history with their associated identifiers and the task message itself, to present in a 'Task History' view. However in the future when we implement task history, we'll need to use a unique identifier for each task. As well as manage a data structure that keeps track of task history with their associated identifiers and the task message itself, to present in a 'Task History' view.
Task history is a significant undertaking as it would require refactoring how we wait for ask responses--it would need to be a hidden claudeMessage, so that user's can resume tasks that ended with an ask. Task history is a significant undertaking as it would require refactoring how we wait for ask responses--it would need to be a hidden claudeMessage, so that user's can resume tasks that ended with an ask.
*/ */
// private providerInstanceIdentifier = Date.now()
// getClaudeMessagesStateKey() {
// return `claudeMessages-${this.providerInstanceIdentifier}`
// }
getClaudeMessagesStateKey() { // getApiConversationHistoryStateKey() {
return `claudeMessages-${this.providerInstanceIdentifier}` // return `apiConversationHistory-${this.providerInstanceIdentifier}`
} // }
getApiConversationHistoryStateKey() {
return `apiConversationHistory-${this.providerInstanceIdentifier}`
}
// claude messages to present in the webview // claude messages to present in the webview
async getClaudeMessages(): Promise<ClaudeMessage[]> { // getClaudeMessages(): ClaudeMessage[] {
const messages = (await this.getGlobalState(this.getClaudeMessagesStateKey())) as ClaudeMessage[] // // const messages = (await this.getGlobalState(this.getClaudeMessagesStateKey())) as ClaudeMessage[]
return messages || [] // // return messages || []
} // return this.claudeMessages
// }
async setClaudeMessages(messages: ClaudeMessage[] | undefined) { // setClaudeMessages(messages: ClaudeMessage[] | undefined) {
await this.updateGlobalState(this.getClaudeMessagesStateKey(), messages) // // await this.updateGlobalState(this.getClaudeMessagesStateKey(), messages)
} // this.claudeMessages = messages || []
// }
async addClaudeMessage(message: ClaudeMessage): Promise<ClaudeMessage[]> { // addClaudeMessage(message: ClaudeMessage): ClaudeMessage[] {
const messages = await this.getClaudeMessages() // // const messages = await this.getClaudeMessages()
messages.push(message) // // messages.push(message)
await this.setClaudeMessages(messages) // // await this.setClaudeMessages(messages)
return messages // // return messages
} // this.claudeMessages.push(message)
// return this.claudeMessages
// }
// conversation history to send in API requests // conversation history to send in API requests
@@ -445,29 +447,28 @@ export class ClaudeDevProvider implements vscode.WebviewViewProvider {
VSCode docs about state: "The value must be JSON-stringifyable ... value — A value. MUST not contain cyclic references." VSCode docs about state: "The value must be JSON-stringifyable ... value — A value. MUST not contain cyclic references."
For now we'll store the conversation history in memory, and if we need to store in state directly we'd need to do a manual conversion to ensure proper json stringification. For now we'll store the conversation history in memory, and if we need to store in state directly we'd need to do a manual conversion to ensure proper json stringification.
*/ */
private apiConversationHistory: Anthropic.MessageParam[] = []
async getApiConversationHistory(): Promise<Anthropic.MessageParam[]> { // getApiConversationHistory(): Anthropic.MessageParam[] {
// const history = (await this.getGlobalState( // // const history = (await this.getGlobalState(
// this.getApiConversationHistoryStateKey() // // this.getApiConversationHistoryStateKey()
// )) as Anthropic.MessageParam[] // // )) as Anthropic.MessageParam[]
// return history || [] // // return history || []
return this.apiConversationHistory // return this.apiConversationHistory
} // }
async setApiConversationHistory(history: Anthropic.MessageParam[] | undefined) { // setApiConversationHistory(history: Anthropic.MessageParam[] | undefined) {
// await this.updateGlobalState(this.getApiConversationHistoryStateKey(), history) // // await this.updateGlobalState(this.getApiConversationHistoryStateKey(), history)
this.apiConversationHistory = history || [] // this.apiConversationHistory = history || []
} // }
async addMessageToApiConversationHistory(message: Anthropic.MessageParam): Promise<Anthropic.MessageParam[]> { // addMessageToApiConversationHistory(message: Anthropic.MessageParam): Anthropic.MessageParam[] {
// const history = await this.getApiConversationHistory() // // const history = await this.getApiConversationHistory()
// history.push(message) // // history.push(message)
// await this.setApiConversationHistory(history) // // await this.setApiConversationHistory(history)
// return history // // return history
this.apiConversationHistory.push(message) // this.apiConversationHistory.push(message)
return this.apiConversationHistory // return this.apiConversationHistory
} // }
/* /*
Storage Storage