From f6e85fa1333c5f2ca58ff84c3dac98dc5c0c9be5 Mon Sep 17 00:00:00 2001 From: Daniel Riccio Date: Tue, 14 Jan 2025 17:57:09 -0500 Subject: [PATCH] feat: introduce experimental diff strategy toggle and enhance diff handling - Added support for an experimental diff strategy in the Cline class, allowing users to opt for a new unified diff approach. - Updated the getDiffStrategy function to accommodate the experimental strategy, adjusting the fuzzy match threshold accordingly. - Integrated experimentalDiffStrategy into the global state management, enabling persistence across sessions. - Enhanced the ClineProvider and related components to handle the new experimental strategy, including UI updates for user settings. - Improved task history management to include the experimentalDiffStrategy setting, ensuring consistency in task execution. - Updated relevant interfaces and types to reflect the new experimentalDiffStrategy property. --- src/core/Cline.ts | 25 ++++++----- src/core/diff/DiffStrategy.ts | 12 ++++-- src/core/webview/ClineProvider.ts | 41 +++++++++++++++---- src/shared/ExtensionMessage.ts | 1 + src/shared/HistoryItem.ts | 1 + src/shared/WebviewMessage.ts | 1 + .../src/components/settings/SettingsView.tsx | 26 +++++++++++- .../src/context/ExtensionStateContext.tsx | 9 +++- 8 files changed, 89 insertions(+), 27 deletions(-) diff --git a/src/core/Cline.ts b/src/core/Cline.ts index 8e25679..e76b1fb 100644 --- a/src/core/Cline.ts +++ b/src/core/Cline.ts @@ -51,6 +51,7 @@ import { detectCodeOmission } from "../integrations/editor/detect-omission" import { BrowserSession } from "../services/browser/BrowserSession" import { OpenRouterHandler } from "../api/providers/openrouter" import { McpHub } from "../services/mcp/McpHub" +import crypto from "crypto" const cwd = vscode.workspace.workspaceFolders?.map((folder) => folder.uri.fsPath).at(0) ?? path.join(os.homedir(), "Desktop") // may or may not exist but fs checking existence would immediately ask for permission which would be bad UX, need to come up with a better solution @@ -105,26 +106,30 @@ export class Cline { task?: string | undefined, images?: string[] | undefined, historyItem?: HistoryItem | undefined, + experimentalDiffStrategy?: boolean, ) { - this.providerRef = new WeakRef(provider) + this.taskId = crypto.randomUUID() this.api = buildApiHandler(apiConfiguration) this.terminalManager = new TerminalManager() this.urlContentFetcher = new UrlContentFetcher(provider.context) this.browserSession = new BrowserSession(provider.context) - this.diffViewProvider = new DiffViewProvider(cwd) this.customInstructions = customInstructions this.diffEnabled = enableDiff ?? false - if (this.diffEnabled && this.api.getModel().id) { - this.diffStrategy = getDiffStrategy(this.api.getModel().id, fuzzyMatchThreshold ?? 1.0) - } + + // Prioritize experimentalDiffStrategy from history item if available + const effectiveExperimentalDiffStrategy = historyItem?.experimentalDiffStrategy ?? experimentalDiffStrategy + this.diffStrategy = getDiffStrategy(this.api.getModel().id, fuzzyMatchThreshold, effectiveExperimentalDiffStrategy) + this.diffViewProvider = new DiffViewProvider(cwd) + this.providerRef = new WeakRef(provider) + if (historyItem) { this.taskId = historyItem.id - this.resumeTaskFromHistory() - } else if (task || images) { - this.taskId = Date.now().toString() + } + + if (task || images) { this.startTask(task, images) - } else { - throw new Error("Either historyItem or task/images must be provided") + } else if (historyItem) { + this.resumeTaskFromHistory() } } diff --git a/src/core/diff/DiffStrategy.ts b/src/core/diff/DiffStrategy.ts index 5d71ed6..2a05417 100644 --- a/src/core/diff/DiffStrategy.ts +++ b/src/core/diff/DiffStrategy.ts @@ -7,10 +7,14 @@ import { NewUnifiedDiffStrategy } from './strategies/new-unified' * @param model The name of the model being used (e.g., 'gpt-4', 'claude-3-opus') * @returns The appropriate diff strategy for the model */ -export function getDiffStrategy(model: string, fuzzyMatchThreshold?: number): DiffStrategy { - // For now, return SearchReplaceDiffStrategy for all models - // This architecture allows for future optimizations based on model capabilities - return new NewUnifiedDiffStrategy() +export function getDiffStrategy(model: string, fuzzyMatchThreshold?: number, experimentalDiffStrategy?: boolean): DiffStrategy { + if (experimentalDiffStrategy) { + // Use the fuzzyMatchThreshold with a minimum of 0.8 (80%) + const threshold = Math.max(fuzzyMatchThreshold ?? 1.0, 0.8) + return new NewUnifiedDiffStrategy(threshold) + } + // Default to the stable SearchReplaceDiffStrategy + return new SearchReplaceDiffStrategy() } export type { DiffStrategy } diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 579a12e..67b9208 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -85,6 +85,7 @@ type GlobalStateKey = | "mcpEnabled" | "alwaysApproveResubmit" | "requestDelaySeconds" + | "experimentalDiffStrategy" export const GlobalFileNames = { apiConversationHistory: "api_conversation_history.json", uiMessages: "ui_messages.json", @@ -233,7 +234,8 @@ export class ClineProvider implements vscode.WebviewViewProvider { apiConfiguration, customInstructions, diffEnabled, - fuzzyMatchThreshold + fuzzyMatchThreshold, + experimentalDiffStrategy } = await this.getState() this.cline = new Cline( @@ -243,7 +245,9 @@ export class ClineProvider implements vscode.WebviewViewProvider { diffEnabled, fuzzyMatchThreshold, task, - images + images, + undefined, + experimentalDiffStrategy ) } @@ -253,7 +257,8 @@ export class ClineProvider implements vscode.WebviewViewProvider { apiConfiguration, customInstructions, diffEnabled, - fuzzyMatchThreshold + fuzzyMatchThreshold, + experimentalDiffStrategy } = await this.getState() this.cline = new Cline( @@ -264,7 +269,8 @@ export class ClineProvider implements vscode.WebviewViewProvider { fuzzyMatchThreshold, undefined, undefined, - historyItem + historyItem, + experimentalDiffStrategy ) } @@ -805,6 +811,10 @@ export class ClineProvider implements vscode.WebviewViewProvider { } break } + case "experimentalDiffStrategy": + await this.updateGlobalState("experimentalDiffStrategy", message.bool ?? false) + await this.postStateToWebview() + break } }, null, @@ -1155,7 +1165,7 @@ export class ClineProvider implements vscode.WebviewViewProvider { uiMessagesFilePath: string apiConversationHistory: Anthropic.MessageParam[] }> { - const history = ((await this.getGlobalState("taskHistory")) as HistoryItem[] | undefined) || [] + const history = (await this.getGlobalState("taskHistory") as HistoryItem[] | undefined) || [] const historyItem = history.find((item) => item.id === id) if (historyItem) { const taskDirPath = path.join(this.context.globalStorageUri.fsPath, "tasks", id) @@ -1220,7 +1230,7 @@ export class ClineProvider implements vscode.WebviewViewProvider { async deleteTaskFromState(id: string) { // Remove the task from history - const taskHistory = ((await this.getGlobalState("taskHistory")) as HistoryItem[]) || [] + const taskHistory = (await this.getGlobalState("taskHistory") as HistoryItem[]) || [] const updatedTaskHistory = taskHistory.filter((task) => task.id !== id) await this.updateGlobalState("taskHistory", updatedTaskHistory) @@ -1256,6 +1266,7 @@ export class ClineProvider implements vscode.WebviewViewProvider { mcpEnabled, alwaysApproveResubmit, requestDelaySeconds, + experimentalDiffStrategy, } = await this.getState() const allowedCommands = vscode.workspace @@ -1290,6 +1301,7 @@ export class ClineProvider implements vscode.WebviewViewProvider { mcpEnabled: mcpEnabled ?? true, alwaysApproveResubmit: alwaysApproveResubmit ?? false, requestDelaySeconds: requestDelaySeconds ?? 5, + experimentalDiffStrategy: experimentalDiffStrategy ?? false, } } @@ -1397,6 +1409,7 @@ export class ClineProvider implements vscode.WebviewViewProvider { mcpEnabled, alwaysApproveResubmit, requestDelaySeconds, + experimentalDiffStrategy, ] = await Promise.all([ this.getGlobalState("apiProvider") as Promise, this.getGlobalState("apiModelId") as Promise, @@ -1449,6 +1462,7 @@ export class ClineProvider implements vscode.WebviewViewProvider { this.getGlobalState("mcpEnabled") as Promise, this.getGlobalState("alwaysApproveResubmit") as Promise, this.getGlobalState("requestDelaySeconds") as Promise, + this.getGlobalState("experimentalDiffStrategy") as Promise, ]) let apiProvider: ApiProvider @@ -1545,16 +1559,25 @@ export class ClineProvider implements vscode.WebviewViewProvider { mcpEnabled: mcpEnabled ?? true, alwaysApproveResubmit: alwaysApproveResubmit ?? false, requestDelaySeconds: requestDelaySeconds ?? 5, + experimentalDiffStrategy: experimentalDiffStrategy ?? false, } } async updateTaskHistory(item: HistoryItem): Promise { - const history = ((await this.getGlobalState("taskHistory")) as HistoryItem[]) || [] + const history = (await this.getGlobalState("taskHistory") as HistoryItem[] | undefined) || [] const existingItemIndex = history.findIndex((h) => h.id === item.id) + + // Ensure experimentalDiffStrategy is included from current settings if not already set + const { experimentalDiffStrategy } = await this.getState() ?? {} + const updatedItem = { + ...item, + experimentalDiffStrategy: item.experimentalDiffStrategy ?? experimentalDiffStrategy + } + if (existingItemIndex !== -1) { - history[existingItemIndex] = item + history[existingItemIndex] = updatedItem } else { - history.push(item) + history.push(updatedItem) } await this.updateGlobalState("taskHistory", history) return history diff --git a/src/shared/ExtensionMessage.ts b/src/shared/ExtensionMessage.ts index 6b877a0..b911e01 100644 --- a/src/shared/ExtensionMessage.ts +++ b/src/shared/ExtensionMessage.ts @@ -70,6 +70,7 @@ export interface ExtensionState { writeDelayMs: number terminalOutputLineLimit?: number mcpEnabled: boolean + experimentalDiffStrategy?: boolean } export interface ClineMessage { diff --git a/src/shared/HistoryItem.ts b/src/shared/HistoryItem.ts index d4539f6..4127b88 100644 --- a/src/shared/HistoryItem.ts +++ b/src/shared/HistoryItem.ts @@ -7,4 +7,5 @@ export type HistoryItem = { cacheWrites?: number cacheReads?: number totalCost: number + experimentalDiffStrategy?: boolean } diff --git a/src/shared/WebviewMessage.ts b/src/shared/WebviewMessage.ts index 0ca7cb3..caeefe8 100644 --- a/src/shared/WebviewMessage.ts +++ b/src/shared/WebviewMessage.ts @@ -54,6 +54,7 @@ export interface WebviewMessage { | "searchCommits" | "alwaysApproveResubmit" | "requestDelaySeconds" + | "experimentalDiffStrategy" text?: string disabled?: boolean askResponse?: ClineAskResponse diff --git a/webview-ui/src/components/settings/SettingsView.tsx b/webview-ui/src/components/settings/SettingsView.tsx index 956b76b..0049443 100644 --- a/webview-ui/src/components/settings/SettingsView.tsx +++ b/webview-ui/src/components/settings/SettingsView.tsx @@ -55,6 +55,8 @@ const SettingsView = ({ onDone }: SettingsViewProps) => { setAlwaysApproveResubmit, requestDelaySeconds, setRequestDelaySeconds, + experimentalDiffStrategy, + setExperimentalDiffStrategy, } = useExtensionState() const [apiErrorMessage, setApiErrorMessage] = useState(undefined) const [modelIdErrorMessage, setModelIdErrorMessage] = useState(undefined) @@ -89,6 +91,7 @@ const SettingsView = ({ onDone }: SettingsViewProps) => { vscode.postMessage({ type: "mcpEnabled", bool: mcpEnabled }) vscode.postMessage({ type: "alwaysApproveResubmit", bool: alwaysApproveResubmit }) vscode.postMessage({ type: "requestDelaySeconds", value: requestDelaySeconds }) + vscode.postMessage({ type: "experimentalDiffStrategy", bool: experimentalDiffStrategy }) onDone() } } @@ -252,7 +255,13 @@ const SettingsView = ({ onDone }: SettingsViewProps) => {
- setDiffEnabled(e.target.checked)}> + { + setDiffEnabled(e.target.checked) + if (!e.target.checked) { + // Reset experimental strategy when diffs are disabled + setExperimentalDiffStrategy(false) + } + }}> Enable editing through diffs

{ {diffEnabled && (

+
+ ⚠️ + setExperimentalDiffStrategy(e.target.checked)}> + Use experimental unified diff strategy + +
+

+ Enable the experimental unified diff strategy. This strategy might reduce the number of retries caused by model errors but may cause unexpected behavior or incorrect edits. + Only enable if you understand the risks and are willing to carefully review all changes. +

+
Match precision { {Math.round((fuzzyMatchThreshold || 1) * 100)}%
-

+

This slider controls how precisely code sections must match when applying diffs. Lower values allow more flexible matching but increase the risk of incorrect replacements. Use values below 100% with extreme caution.

diff --git a/webview-ui/src/context/ExtensionStateContext.tsx b/webview-ui/src/context/ExtensionStateContext.tsx index 131364b..eef9b07 100644 --- a/webview-ui/src/context/ExtensionStateContext.tsx +++ b/webview-ui/src/context/ExtensionStateContext.tsx @@ -50,6 +50,8 @@ export interface ExtensionStateContextType extends ExtensionState { setAlwaysApproveResubmit: (value: boolean) => void requestDelaySeconds: number setRequestDelaySeconds: (value: number) => void + experimentalDiffStrategy: boolean + setExperimentalDiffStrategy: (value: boolean) => void } export const ExtensionStateContext = createContext(undefined) @@ -72,7 +74,8 @@ export const ExtensionStateContextProvider: React.FC<{ children: React.ReactNode terminalOutputLineLimit: 500, mcpEnabled: true, alwaysApproveResubmit: false, - requestDelaySeconds: 5 + requestDelaySeconds: 0, + experimentalDiffStrategy: false, }) const [didHydrateState, setDidHydrateState] = useState(false) const [showWelcome, setShowWelcome] = useState(false) @@ -208,7 +211,9 @@ export const ExtensionStateContextProvider: React.FC<{ children: React.ReactNode setTerminalOutputLineLimit: (value) => setState((prevState) => ({ ...prevState, terminalOutputLineLimit: value })), setMcpEnabled: (value) => setState((prevState) => ({ ...prevState, mcpEnabled: value })), setAlwaysApproveResubmit: (value) => setState((prevState) => ({ ...prevState, alwaysApproveResubmit: value })), - setRequestDelaySeconds: (value) => setState((prevState) => ({ ...prevState, requestDelaySeconds: value })) + setRequestDelaySeconds: (value) => setState((prevState) => ({ ...prevState, requestDelaySeconds: value })), + experimentalDiffStrategy: state.experimentalDiffStrategy ?? false, + setExperimentalDiffStrategy: (value) => setState((prevState) => ({ ...prevState, experimentalDiffStrategy: value })) } return {children}