Browse Source

Refactor util functions out of claude dev

Saoud Rizwan 1 year ago
parent
commit
ab327e3bbf

+ 32 - 75
src/core/ClaudeDev.ts

@@ -27,7 +27,7 @@ import { getApiMetrics } from "../shared/getApiMetrics"
 import { HistoryItem } from "../shared/HistoryItem"
 import { HistoryItem } from "../shared/HistoryItem"
 import { ToolName } from "../shared/Tool"
 import { ToolName } from "../shared/Tool"
 import { ClaudeAskResponse } from "../shared/WebviewMessage"
 import { ClaudeAskResponse } from "../shared/WebviewMessage"
-import { arePathsEqual } from "../utils/path"
+import { arePathsEqual, getReadablePath } from "../utils/path"
 import {
 import {
 	AssistantMessageContent,
 	AssistantMessageContent,
 	TextContent,
 	TextContent,
@@ -39,7 +39,7 @@ import {
 } from "./AssistantMessage"
 } from "./AssistantMessage"
 import { parseMentions } from "./mentions"
 import { parseMentions } from "./mentions"
 import { formatResponse } from "./prompts/responses"
 import { formatResponse } from "./prompts/responses"
-import { SYSTEM_PROMPT } from "./prompts/system"
+import { addCustomInstructions, SYSTEM_PROMPT } from "./prompts/system"
 import { truncateHalfConversation } from "./sliding-window"
 import { truncateHalfConversation } from "./sliding-window"
 import { ClaudeDevProvider, GlobalFileNames } from "./webview/ClaudeDevProvider"
 import { ClaudeDevProvider, GlobalFileNames } from "./webview/ClaudeDevProvider"
 import { calculateApiCost } from "../utils/cost"
 import { calculateApiCost } from "../utils/cost"
@@ -55,12 +55,12 @@ type UserContent = Array<
 
 
 export class ClaudeDev {
 export class ClaudeDev {
 	readonly taskId: string
 	readonly taskId: string
-	private api: ApiHandler
+	api: ApiHandler
 	private terminalManager: TerminalManager
 	private terminalManager: TerminalManager
 	private urlContentFetcher: UrlContentFetcher
 	private urlContentFetcher: UrlContentFetcher
 	private didEditFile: boolean = false
 	private didEditFile: boolean = false
-	private customInstructions?: string
-	private alwaysAllowReadOnly: boolean
+	customInstructions?: string
+	alwaysAllowReadOnly: boolean
 	apiConversationHistory: Anthropic.MessageParam[] = []
 	apiConversationHistory: Anthropic.MessageParam[] = []
 	claudeMessages: ClaudeMessage[] = []
 	claudeMessages: ClaudeMessage[] = []
 	private askResponse?: ClaudeAskResponse
 	private askResponse?: ClaudeAskResponse
@@ -98,25 +98,7 @@ export class ClaudeDev {
 		}
 		}
 	}
 	}
 
 
-	updateApi(apiConfiguration: ApiConfiguration) {
-		this.api = buildApiHandler(apiConfiguration)
-	}
-
-	updateCustomInstructions(customInstructions: string | undefined) {
-		this.customInstructions = customInstructions
-	}
-
-	updateAlwaysAllowReadOnly(alwaysAllowReadOnly: boolean | undefined) {
-		this.alwaysAllowReadOnly = alwaysAllowReadOnly ?? false
-	}
-
-	async handleWebviewAskResponse(askResponse: ClaudeAskResponse, text?: string, images?: string[]) {
-		this.askResponse = askResponse
-		this.askResponseText = text
-		this.askResponseImages = images
-	}
-
-	// storing task to disk for history
+	// Storing task to disk for history
 
 
 	private async ensureTaskDirectoryExists(): Promise<string> {
 	private async ensureTaskDirectoryExists(): Promise<string> {
 		const globalStoragePath = this.providerRef.deref()?.context.globalStorageUri.fsPath
 		const globalStoragePath = this.providerRef.deref()?.context.globalStorageUri.fsPath
@@ -205,6 +187,8 @@ export class ClaudeDev {
 		}
 		}
 	}
 	}
 
 
+	// Communicate with webview
+
 	// partial has three valid states true (partial message), false (completion of partial message), undefined (individual complete message)
 	// partial has three valid states true (partial message), false (completion of partial message), undefined (individual complete message)
 	async ask(
 	async ask(
 		type: ClaudeAsk,
 		type: ClaudeAsk,
@@ -291,6 +275,12 @@ export class ClaudeDev {
 		return result
 		return result
 	}
 	}
 
 
+	async handleWebviewAskResponse(askResponse: ClaudeAskResponse, text?: string, images?: string[]) {
+		this.askResponse = askResponse
+		this.askResponseText = text
+		this.askResponseImages = images
+	}
+
 	async say(type: ClaudeSay, text?: string, images?: string[], partial?: boolean): Promise<undefined> {
 	async say(type: ClaudeSay, text?: string, images?: string[], partial?: boolean): Promise<undefined> {
 		if (this.abort) {
 		if (this.abort) {
 			throw new Error("ClaudeDev instance aborted")
 			throw new Error("ClaudeDev instance aborted")
@@ -356,6 +346,8 @@ export class ClaudeDev {
 		return formatResponse.toolError(formatResponse.missingToolParameterError(paramName))
 		return formatResponse.toolError(formatResponse.missingToolParameterError(paramName))
 	}
 	}
 
 
+	// Task lifecycle
+
 	private async startTask(task?: string, images?: string[]): Promise<void> {
 	private async startTask(task?: string, images?: 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)
@@ -597,6 +589,8 @@ export class ClaudeDev {
 		this.urlContentFetcher.closeBrowser()
 		this.urlContentFetcher.closeBrowser()
 	}
 	}
 
 
+	// Tools
+
 	// return is [didUserRejectTool, ToolResponse]
 	// return is [didUserRejectTool, ToolResponse]
 	async writeToFile(relPath?: string, newContent?: string): Promise<[boolean, ToolResponse]> {
 	async writeToFile(relPath?: string, newContent?: string): Promise<[boolean, ToolResponse]> {
 		if (relPath === undefined) {
 		if (relPath === undefined) {
@@ -772,8 +766,8 @@ export class ClaudeDev {
 					"tool",
 					"tool",
 					JSON.stringify({
 					JSON.stringify({
 						tool: "editedExistingFile",
 						tool: "editedExistingFile",
-						path: this.getReadablePath(relPath),
-						diff: this.createPrettyPatch(relPath, originalContent, newContent),
+						path: getReadablePath(cwd, relPath),
+						diff: formatResponse.createPrettyPatch(relPath, originalContent, newContent),
 					} satisfies ClaudeSayTool)
 					} satisfies ClaudeSayTool)
 				)
 				)
 			} else {
 			} else {
@@ -781,7 +775,7 @@ export class ClaudeDev {
 					"tool",
 					"tool",
 					JSON.stringify({
 					JSON.stringify({
 						tool: "newFileCreated",
 						tool: "newFileCreated",
-						path: this.getReadablePath(relPath),
+						path: getReadablePath(cwd, relPath),
 						content: newContent,
 						content: newContent,
 					} satisfies ClaudeSayTool)
 					} satisfies ClaudeSayTool)
 				)
 				)
@@ -952,8 +946,8 @@ export class ClaudeDev {
 					"user_feedback_diff",
 					"user_feedback_diff",
 					JSON.stringify({
 					JSON.stringify({
 						tool: fileExists ? "editedExistingFile" : "newFileCreated",
 						tool: fileExists ? "editedExistingFile" : "newFileCreated",
-						path: this.getReadablePath(relPath),
-						diff: this.createPrettyPatch(relPath, normalizedNewContent, normalizedEditedContent),
+						path: getReadablePath(cwd, relPath),
+						diff: formatResponse.createPrettyPatch(relPath, normalizedNewContent, normalizedEditedContent),
 					} satisfies ClaudeSayTool)
 					} satisfies ClaudeSayTool)
 				)
 				)
 				return [
 				return [
@@ -973,13 +967,6 @@ export class ClaudeDev {
 		}
 		}
 	}
 	}
 
 
-	createPrettyPatch(filename = "file", oldStr: string, newStr: string) {
-		const patch = diff.createPatch(filename.toPosix(), oldStr, newStr)
-		const lines = patch.split("\n")
-		const prettyPatchLines = lines.slice(4)
-		return prettyPatchLines.join("\n")
-	}
-
 	async closeDiffViews() {
 	async closeDiffViews() {
 		const tabs = vscode.window.tabGroups.all
 		const tabs = vscode.window.tabGroups.all
 			.map((tg) => tg.tabs)
 			.map((tg) => tg.tabs)
@@ -997,28 +984,6 @@ export class ClaudeDev {
 		}
 		}
 	}
 	}
 
 
-	getReadablePath(relPath?: string): string {
-		relPath = relPath || ""
-		// path.resolve is flexible in that it will resolve relative paths like '../../' to the cwd and even ignore the cwd if the relPath is actually an absolute path
-		const absolutePath = path.resolve(cwd, relPath)
-		if (arePathsEqual(cwd, path.join(os.homedir(), "Desktop"))) {
-			// User opened vscode without a workspace, so cwd is the Desktop. Show the full absolute path to keep the user aware of where files are being created
-			return absolutePath.toPosix()
-		}
-		if (arePathsEqual(path.normalize(absolutePath), path.normalize(cwd))) {
-			return path.basename(absolutePath).toPosix()
-		} else {
-			// show the relative path to the cwd
-			const normalizedRelPath = path.relative(cwd, absolutePath)
-			if (absolutePath.includes(cwd)) {
-				return normalizedRelPath.toPosix()
-			} else {
-				// we are outside the cwd, so show the absolute path (useful for when claude passes in '../../' for example)
-				return absolutePath.toPosix()
-			}
-		}
-	}
-
 	async executeCommandTool(
 	async executeCommandTool(
 		command: string,
 		command: string,
 		returnEmptyStringOnSuccess: boolean = false
 		returnEmptyStringOnSuccess: boolean = false
@@ -1108,15 +1073,7 @@ export class ClaudeDev {
 			let systemPrompt = await SYSTEM_PROMPT(cwd, this.api.getModel().info.supportsImages)
 			let systemPrompt = await SYSTEM_PROMPT(cwd, this.api.getModel().info.supportsImages)
 			if (this.customInstructions && this.customInstructions.trim()) {
 			if (this.customInstructions && this.customInstructions.trim()) {
 				// altering the system prompt mid-task will break the prompt cache, but in the grand scheme this will not change often so it's better to not pollute user messages with it the way we have to with <potentially relevant details>
 				// altering the system prompt mid-task will break the prompt cache, but in the grand scheme this will not change often so it's better to not pollute user messages with it the way we have to with <potentially relevant details>
-				systemPrompt += `
-====
-
-USER'S CUSTOM INSTRUCTIONS
-
-The following additional instructions are provided by the user. They should be followed and given precedence in case of conflicts with previous instructions.
-
-${this.customInstructions.trim()}
-`
+				systemPrompt += addCustomInstructions(this.customInstructions)
 			}
 			}
 
 
 			// If the previous API request's total token usage is close to the context window, truncate the conversation history to free up space for the new request
 			// If the previous API request's total token usage is close to the context window, truncate the conversation history to free up space for the new request
@@ -1307,7 +1264,7 @@ ${this.customInstructions.trim()}
 						}
 						}
 						const sharedMessageProps: ClaudeSayTool = {
 						const sharedMessageProps: ClaudeSayTool = {
 							tool: fileExists ? "editedExistingFile" : "newFileCreated",
 							tool: fileExists ? "editedExistingFile" : "newFileCreated",
-							path: this.getReadablePath(relPath),
+							path: getReadablePath(cwd, relPath),
 						}
 						}
 						try {
 						try {
 							const absolutePath = path.resolve(cwd, relPath)
 							const absolutePath = path.resolve(cwd, relPath)
@@ -1516,7 +1473,7 @@ ${this.customInstructions.trim()}
 									...sharedMessageProps,
 									...sharedMessageProps,
 									content: fileExists ? undefined : newContent,
 									content: fileExists ? undefined : newContent,
 									diff: fileExists
 									diff: fileExists
-										? this.createPrettyPatch(relPath, originalContent, newContent)
+										? formatResponse.createPrettyPatch(relPath, originalContent, newContent)
 										: undefined,
 										: undefined,
 								} satisfies ClaudeSayTool)
 								} satisfies ClaudeSayTool)
 								const didApprove = await askApproval("tool", completeMessage)
 								const didApprove = await askApproval("tool", completeMessage)
@@ -1587,8 +1544,8 @@ ${this.customInstructions.trim()}
 										"user_feedback_diff",
 										"user_feedback_diff",
 										JSON.stringify({
 										JSON.stringify({
 											tool: fileExists ? "editedExistingFile" : "newFileCreated",
 											tool: fileExists ? "editedExistingFile" : "newFileCreated",
-											path: this.getReadablePath(relPath),
-											diff: this.createPrettyPatch(
+											path: getReadablePath(cwd, relPath),
+											diff: formatResponse.createPrettyPatch(
 												relPath,
 												relPath,
 												normalizedNewContent,
 												normalizedNewContent,
 												normalizedEditedContent
 												normalizedEditedContent
@@ -1627,7 +1584,7 @@ ${this.customInstructions.trim()}
 						const relPath: string | undefined = block.params.path
 						const relPath: string | undefined = block.params.path
 						const sharedMessageProps: ClaudeSayTool = {
 						const sharedMessageProps: ClaudeSayTool = {
 							tool: "readFile",
 							tool: "readFile",
-							path: this.getReadablePath(relPath),
+							path: getReadablePath(cwd, relPath),
 						}
 						}
 						try {
 						try {
 							if (block.partial) {
 							if (block.partial) {
@@ -1677,7 +1634,7 @@ ${this.customInstructions.trim()}
 						const recursive = recursiveRaw?.toLowerCase() === "true"
 						const recursive = recursiveRaw?.toLowerCase() === "true"
 						const sharedMessageProps: ClaudeSayTool = {
 						const sharedMessageProps: ClaudeSayTool = {
 							tool: !recursive ? "listFilesTopLevel" : "listFilesRecursive",
 							tool: !recursive ? "listFilesTopLevel" : "listFilesRecursive",
-							path: this.getReadablePath(relDirPath),
+							path: getReadablePath(cwd, relDirPath),
 						}
 						}
 						try {
 						try {
 							if (block.partial) {
 							if (block.partial) {
@@ -1725,7 +1682,7 @@ ${this.customInstructions.trim()}
 						const relDirPath: string | undefined = block.params.path
 						const relDirPath: string | undefined = block.params.path
 						const sharedMessageProps: ClaudeSayTool = {
 						const sharedMessageProps: ClaudeSayTool = {
 							tool: "listCodeDefinitionNames",
 							tool: "listCodeDefinitionNames",
-							path: this.getReadablePath(relDirPath),
+							path: getReadablePath(cwd, relDirPath),
 						}
 						}
 						try {
 						try {
 							if (block.partial) {
 							if (block.partial) {
@@ -1776,7 +1733,7 @@ ${this.customInstructions.trim()}
 						const filePattern: string | undefined = block.params.file_pattern
 						const filePattern: string | undefined = block.params.file_pattern
 						const sharedMessageProps: ClaudeSayTool = {
 						const sharedMessageProps: ClaudeSayTool = {
 							tool: "searchFiles",
 							tool: "searchFiles",
-							path: this.getReadablePath(relDirPath),
+							path: getReadablePath(cwd, relDirPath),
 							regex: regex || "",
 							regex: regex || "",
 							filePattern: filePattern || "",
 							filePattern: filePattern || "",
 						}
 						}

+ 9 - 0
src/core/prompts/responses.ts

@@ -1,5 +1,6 @@
 import { Anthropic } from "@anthropic-ai/sdk"
 import { Anthropic } from "@anthropic-ai/sdk"
 import * as path from "path"
 import * as path from "path"
+import * as diff from "diff"
 
 
 export const formatResponse = {
 export const formatResponse = {
 	toolDenied: () => `The user denied this operation.`,
 	toolDenied: () => `The user denied this operation.`,
@@ -74,6 +75,14 @@ export const formatResponse = {
 			return sorted.join("\n")
 			return sorted.join("\n")
 		}
 		}
 	},
 	},
+
+	createPrettyPatch: (filename = "file", oldStr: string, newStr: string) => {
+		// strings cannot be undefined or diff throws exception
+		const patch = diff.createPatch(filename.toPosix(), oldStr || "", newStr || "")
+		const lines = patch.split("\n")
+		const prettyPatchLines = lines.slice(4)
+		return prettyPatchLines.join("\n")
+	},
 }
 }
 
 
 // to avoid circular dependency
 // to avoid circular dependency

+ 12 - 2
src/core/prompts/system.ts

@@ -338,5 +338,15 @@ open http://localhost:3000
 </command>
 </command>
 </attempt_completion>
 </attempt_completion>
 
 
-By following this approach, you are able to make informed decisions at each step, using tools one at a time and ensuring each action is based on the previous step's result and logical progression of the task.
-`
+By following this approach, you are able to make informed decisions at each step, using tools one at a time and ensuring each action is based on the previous step's result and logical progression of the task.`
+
+export function addCustomInstructions(customInstructions: string): string {
+	return `
+====
+
+USER'S CUSTOM INSTRUCTIONS
+
+The following additional instructions are provided by the user. They should be followed and given precedence in case of conflicts with previous instructions.
+
+${customInstructions.trim()}`
+}

+ 13 - 4
src/core/webview/ClaudeDevProvider.ts

@@ -18,6 +18,7 @@ import { openFile, openImage } from "../../integrations/misc/open-file"
 import WorkspaceTracker from "../../integrations/workspace/WorkspaceTracker"
 import WorkspaceTracker from "../../integrations/workspace/WorkspaceTracker"
 import { openMention } from "../mentions"
 import { openMention } from "../mentions"
 import { fileExistsAtPath } from "../../utils/fs"
 import { fileExistsAtPath } from "../../utils/fs"
+import { buildApiHandler } from "../../api"
 
 
 /*
 /*
 https://github.com/microsoft/vscode-webview-ui-toolkit-samples/blob/main/default/weather-webview/src/providers/WeatherViewProvider.ts
 https://github.com/microsoft/vscode-webview-ui-toolkit-samples/blob/main/default/weather-webview/src/providers/WeatherViewProvider.ts
@@ -378,7 +379,9 @@ export class ClaudeDevProvider implements vscode.WebviewViewProvider {
 							await this.storeSecret("geminiApiKey", geminiApiKey)
 							await this.storeSecret("geminiApiKey", geminiApiKey)
 							await this.storeSecret("openAiNativeApiKey", openAiNativeApiKey)
 							await this.storeSecret("openAiNativeApiKey", openAiNativeApiKey)
 							await this.updateGlobalState("azureApiVersion", azureApiVersion)
 							await this.updateGlobalState("azureApiVersion", azureApiVersion)
-							this.claudeDev?.updateApi(message.apiConfiguration)
+							if (this.claudeDev) {
+								this.claudeDev.api = buildApiHandler(message.apiConfiguration)
+							}
 						}
 						}
 						await this.postStateToWebview()
 						await this.postStateToWebview()
 						break
 						break
@@ -387,7 +390,9 @@ export class ClaudeDevProvider implements vscode.WebviewViewProvider {
 						break
 						break
 					case "alwaysAllowReadOnly":
 					case "alwaysAllowReadOnly":
 						await this.updateGlobalState("alwaysAllowReadOnly", message.bool ?? undefined)
 						await this.updateGlobalState("alwaysAllowReadOnly", message.bool ?? undefined)
-						this.claudeDev?.updateAlwaysAllowReadOnly(message.bool ?? undefined)
+						if (this.claudeDev) {
+							this.claudeDev.alwaysAllowReadOnly = message.bool ?? false
+						}
 						await this.postStateToWebview()
 						await this.postStateToWebview()
 						break
 						break
 					case "askResponse":
 					case "askResponse":
@@ -449,7 +454,9 @@ export class ClaudeDevProvider implements vscode.WebviewViewProvider {
 	async updateCustomInstructions(instructions?: string) {
 	async updateCustomInstructions(instructions?: string) {
 		// User may be clearing the field
 		// User may be clearing the field
 		await this.updateGlobalState("customInstructions", instructions || undefined)
 		await this.updateGlobalState("customInstructions", instructions || undefined)
-		this.claudeDev?.updateCustomInstructions(instructions || undefined)
+		if (this.claudeDev) {
+			this.claudeDev.customInstructions = instructions || undefined
+		}
 		await this.postStateToWebview()
 		await this.postStateToWebview()
 	}
 	}
 
 
@@ -492,7 +499,9 @@ export class ClaudeDevProvider implements vscode.WebviewViewProvider {
 		await this.updateGlobalState("apiProvider", openrouter)
 		await this.updateGlobalState("apiProvider", openrouter)
 		await this.storeSecret("openRouterApiKey", apiKey)
 		await this.storeSecret("openRouterApiKey", apiKey)
 		await this.postStateToWebview()
 		await this.postStateToWebview()
-		this.claudeDev?.updateApi({ apiProvider: openrouter, openRouterApiKey: apiKey })
+		if (this.claudeDev) {
+			this.claudeDev.api = buildApiHandler({ apiProvider: openrouter, openRouterApiKey: apiKey })
+		}
 		// await this.postMessageToWebview({ type: "action", action: "settingsButtonTapped" }) // bad ux if user is on welcome
 		// await this.postMessageToWebview({ type: "action", action: "settingsButtonTapped" }) // bad ux if user is on welcome
 	}
 	}
 
 

+ 23 - 0
src/utils/path.ts

@@ -1,4 +1,5 @@
 import * as path from "path"
 import * as path from "path"
+import os from "os"
 
 
 /*
 /*
 The Node.js 'path' module resolves and normalizes paths differently depending on the platform:
 The Node.js 'path' module resolves and normalizes paths differently depending on the platform:
@@ -76,3 +77,25 @@ function normalizePath(p: string): string {
 	}
 	}
 	return normalized
 	return normalized
 }
 }
+
+export function getReadablePath(cwd: string, relPath?: string): string {
+	relPath = relPath || ""
+	// path.resolve is flexible in that it will resolve relative paths like '../../' to the cwd and even ignore the cwd if the relPath is actually an absolute path
+	const absolutePath = path.resolve(cwd, relPath)
+	if (arePathsEqual(cwd, path.join(os.homedir(), "Desktop"))) {
+		// User opened vscode without a workspace, so cwd is the Desktop. Show the full absolute path to keep the user aware of where files are being created
+		return absolutePath.toPosix()
+	}
+	if (arePathsEqual(path.normalize(absolutePath), path.normalize(cwd))) {
+		return path.basename(absolutePath).toPosix()
+	} else {
+		// show the relative path to the cwd
+		const normalizedRelPath = path.relative(cwd, absolutePath)
+		if (absolutePath.includes(cwd)) {
+			return normalizedRelPath.toPosix()
+		} else {
+			// we are outside the cwd, so show the absolute path (useful for when claude passes in '../../' for example)
+			return absolutePath.toPosix()
+		}
+	}
+}