Skip to content

Commit a95ee44

Browse files
ctecte
committed
Clean up the way we compute the current diff strategy
1 parent 3d0fc3c commit a95ee44

File tree

11 files changed

+102
-94
lines changed

11 files changed

+102
-94
lines changed

‎src/core/Cline.ts‎

Lines changed: 9 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -230,11 +230,8 @@ export class Cline extends EventEmitter<ClineEvents> {
230230
telemetryService.captureTaskCreated(this.taskId)
231231
}
232232

233-
// Initialize diffStrategy based on current state
234-
this.updateDiffStrategy(
235-
Experiments.isEnabled(experiments ?? {}, EXPERIMENT_IDS.DIFF_STRATEGY),
236-
Experiments.isEnabled(experiments ?? {}, EXPERIMENT_IDS.MULTI_SEARCH_AND_REPLACE),
237-
)
233+
// Initialize diffStrategy based on current state.
234+
this.updateDiffStrategy(experiments ?? {})
238235

239236
onCreated?.(this)
240237

@@ -269,25 +266,13 @@ export class Cline extends EventEmitter<ClineEvents> {
269266
return getWorkspacePath(path.join(os.homedir(), "Desktop"))
270267
}
271268

272-
// Add method to update diffStrategy
273-
async updateDiffStrategy(experimentalDiffStrategy?: boolean, multiSearchReplaceDiffStrategy?: boolean) {
274-
// If not provided, get from current state
275-
if (experimentalDiffStrategy === undefined || multiSearchReplaceDiffStrategy === undefined) {
276-
const { experiments: stateExperimental } = (await this.providerRef.deref()?.getState()) ?? {}
277-
if (experimentalDiffStrategy === undefined) {
278-
experimentalDiffStrategy = stateExperimental?.[EXPERIMENT_IDS.DIFF_STRATEGY] ?? false
279-
}
280-
if (multiSearchReplaceDiffStrategy === undefined) {
281-
multiSearchReplaceDiffStrategy = stateExperimental?.[EXPERIMENT_IDS.MULTI_SEARCH_AND_REPLACE] ?? false
282-
}
283-
}
284-
285-
this.diffStrategy = getDiffStrategy(
286-
this.api.getModel().id,
287-
this.fuzzyMatchThreshold,
288-
experimentalDiffStrategy,
289-
multiSearchReplaceDiffStrategy,
290-
)
269+
// Add method to update diffStrategy.
270+
async updateDiffStrategy(experiments: Partial<Record<ExperimentId, boolean>>) {
271+
this.diffStrategy = getDiffStrategy({
272+
model: this.api.getModel().id,
273+
experiments,
274+
fuzzyMatchThreshold: this.fuzzyMatchThreshold,
275+
})
291276
}
292277

293278
// Storing task to disk for history

‎src/core/__tests__/Cline.test.ts‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -304,7 +304,12 @@ describe("Cline", () => {
304304

305305
expect(cline.diffEnabled).toBe(true)
306306
expect(cline.diffStrategy).toBeDefined()
307-
expect(getDiffStrategySpy).toHaveBeenCalledWith("claude-3-5-sonnet-20241022", 0.9, false, false)
307+
308+
expect(getDiffStrategySpy).toHaveBeenCalledWith({
309+
model: "claude-3-5-sonnet-20241022",
310+
experiments: {},
311+
fuzzyMatchThreshold: 0.9,
312+
})
308313
})
309314

310315
it("should pass default threshold to diff strategy when not provided", async () => {
@@ -321,10 +326,14 @@ describe("Cline", () => {
321326

322327
expect(cline.diffEnabled).toBe(true)
323328
expect(cline.diffStrategy).toBeDefined()
324-
expect(getDiffStrategySpy).toHaveBeenCalledWith("claude-3-5-sonnet-20241022", 1.0, false, false)
329+
expect(getDiffStrategySpy).toHaveBeenCalledWith({
330+
model: "claude-3-5-sonnet-20241022",
331+
experiments: {},
332+
fuzzyMatchThreshold: 1.0,
333+
})
325334
})
326335

327-
it("should require either task or historyItem", () => {
336+
it("should require either task or historyItem", async () => {
328337
expect(() => {
329338
new Cline({ provider: mockProvider, apiConfiguration: mockApiConfig })
330339
}).toThrow("Either historyItem or task/images must be provided")

‎src/core/config/CustomModesManager.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ export class CustomModesManager {
1919
private readonly context: vscode.ExtensionContext,
2020
private readonly onUpdate: () => Promise<void>,
2121
) {
22+
// TODO: We really shouldn't have async methods in the constructor.
2223
this.watchCustomModesFiles()
2324
}
2425

‎src/core/diff/DiffStrategy.ts‎

Lines changed: 16 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,29 +1,28 @@
11
import type { DiffStrategy } from "./types"
2-
import { UnifiedDiffStrategy } from "./strategies/unified"
32
import { SearchReplaceDiffStrategy } from "./strategies/search-replace"
43
import { NewUnifiedDiffStrategy } from "./strategies/new-unified"
54
import { MultiSearchReplaceDiffStrategy } from "./strategies/multi-search-replace"
5+
import { EXPERIMENT_IDS, ExperimentId } from "../../shared/experiments"
6+
7+
export type { DiffStrategy }
8+
69
/**
710
* Get the appropriate diff strategy for the given model
811
* @param model The name of the model being used (e.g., 'gpt-4', 'claude-3-opus')
912
* @returns The appropriate diff strategy for the model
1013
*/
11-
export function getDiffStrategy(
12-
model: string,
13-
fuzzyMatchThreshold?: number,
14-
experimentalDiffStrategy: boolean = false,
15-
multiSearchReplaceDiffStrategy: boolean = false,
16-
): DiffStrategy {
17-
if (experimentalDiffStrategy) {
18-
return new NewUnifiedDiffStrategy(fuzzyMatchThreshold)
19-
}
2014

21-
if (multiSearchReplaceDiffStrategy) {
22-
return new MultiSearchReplaceDiffStrategy(fuzzyMatchThreshold)
23-
} else {
24-
return new SearchReplaceDiffStrategy(fuzzyMatchThreshold)
25-
}
15+
export type DiffStrategyName = "unified" | "multi-search-and-replace" | "search-and-replace"
16+
17+
type GetDiffStrategyOptions = {
18+
model: string
19+
experiments: Partial<Record<ExperimentId, boolean>>
20+
fuzzyMatchThreshold?: number
2621
}
2722

28-
export type { DiffStrategy }
29-
export { UnifiedDiffStrategy, SearchReplaceDiffStrategy }
23+
export const getDiffStrategy = ({ fuzzyMatchThreshold, experiments }: GetDiffStrategyOptions): DiffStrategy =>
24+
experiments[EXPERIMENT_IDS.DIFF_STRATEGY_UNIFIED]
25+
? new NewUnifiedDiffStrategy(fuzzyMatchThreshold)
26+
: experiments[EXPERIMENT_IDS.DIFF_STRATEGY_MULTI_SEARCH_AND_REPLACE]
27+
? new MultiSearchReplaceDiffStrategy(fuzzyMatchThreshold)
28+
: new SearchReplaceDiffStrategy(fuzzyMatchThreshold)

‎src/core/prompts/__tests__/system.test.ts‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ describe("SYSTEM_PROMPT", () => {
171171
beforeEach(() => {
172172
// Reset experiments before each test to ensure they're disabled by default
173173
experiments = {
174-
[EXPERIMENT_IDS.SEARCH_AND_REPLACE]: false,
174+
[EXPERIMENT_IDS.DIFF_STRATEGY_SEARCH_AND_REPLACE]: false,
175175
[EXPERIMENT_IDS.INSERT_BLOCK]: false,
176176
}
177177
})
@@ -482,7 +482,7 @@ describe("SYSTEM_PROMPT", () => {
482482
it("should disable experimental tools by default", async () => {
483483
// Set experiments to explicitly disable experimental tools
484484
const experimentsConfig = {
485-
[EXPERIMENT_IDS.SEARCH_AND_REPLACE]: false,
485+
[EXPERIMENT_IDS.DIFF_STRATEGY_SEARCH_AND_REPLACE]: false,
486486
[EXPERIMENT_IDS.INSERT_BLOCK]: false,
487487
}
488488

@@ -516,7 +516,7 @@ describe("SYSTEM_PROMPT", () => {
516516
it("should enable experimental tools when explicitly enabled", async () => {
517517
// Set experiments for testing experimental features
518518
const experimentsEnabled = {
519-
[EXPERIMENT_IDS.SEARCH_AND_REPLACE]: true,
519+
[EXPERIMENT_IDS.DIFF_STRATEGY_SEARCH_AND_REPLACE]: true,
520520
[EXPERIMENT_IDS.INSERT_BLOCK]: true,
521521
}
522522

@@ -552,7 +552,7 @@ describe("SYSTEM_PROMPT", () => {
552552
it("should selectively enable experimental tools", async () => {
553553
// Set experiments for testing selective enabling
554554
const experimentsSelective = {
555-
[EXPERIMENT_IDS.SEARCH_AND_REPLACE]: true,
555+
[EXPERIMENT_IDS.DIFF_STRATEGY_SEARCH_AND_REPLACE]: true,
556556
[EXPERIMENT_IDS.INSERT_BLOCK]: false,
557557
}
558558

@@ -587,7 +587,7 @@ describe("SYSTEM_PROMPT", () => {
587587

588588
it("should list all available editing tools in base instruction", async () => {
589589
const experiments = {
590-
[EXPERIMENT_IDS.SEARCH_AND_REPLACE]: true,
590+
[EXPERIMENT_IDS.DIFF_STRATEGY_SEARCH_AND_REPLACE]: true,
591591
[EXPERIMENT_IDS.INSERT_BLOCK]: true,
592592
}
593593

@@ -615,7 +615,7 @@ describe("SYSTEM_PROMPT", () => {
615615
})
616616
it("should provide detailed instructions for each enabled tool", async () => {
617617
const experiments = {
618-
[EXPERIMENT_IDS.SEARCH_AND_REPLACE]: true,
618+
[EXPERIMENT_IDS.DIFF_STRATEGY_SEARCH_AND_REPLACE]: true,
619619
[EXPERIMENT_IDS.INSERT_BLOCK]: true,
620620
}
621621

‎src/core/webview/ClineProvider.ts‎

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ export class ClineProvider extends EventEmitter<ClineProviderEvents> implements
102102
protected mcpHub?: McpHub // Change from private to protected
103103
private latestAnnouncementId = "mar-20-2025-3-10" // update to some unique identifier when we add a new announcement
104104
private settingsImportedAt?: number
105-
private contextProxy: ContextProxy
105+
public readonly contextProxy: ContextProxy
106106
public readonly providerSettingsManager: ProviderSettingsManager
107107
public readonly customModesManager: CustomModesManager
108108

@@ -1590,6 +1590,7 @@ export class ClineProvider extends EventEmitter<ClineProviderEvents> implements
15901590
t("common:confirmation.just_this_message"),
15911591
t("common:confirmation.this_and_subsequent"),
15921592
)
1593+
15931594
if (
15941595
(answer === t("common:confirmation.just_this_message") ||
15951596
answer === t("common:confirmation.this_and_subsequent")) &&
@@ -1598,9 +1599,11 @@ export class ClineProvider extends EventEmitter<ClineProviderEvents> implements
15981599
message.value
15991600
) {
16001601
const timeCutoff = message.value - 1000 // 1 second buffer before the message to delete
1602+
16011603
const messageIndex = this.getCurrentCline()!.clineMessages.findIndex(
16021604
(msg) => msg.ts && msg.ts >= timeCutoff,
16031605
)
1606+
16041607
const apiConversationHistoryIndex =
16051608
this.getCurrentCline()?.apiConversationHistory.findIndex(
16061609
(msg) => msg.ts && msg.ts >= timeCutoff,
@@ -1621,6 +1624,7 @@ export class ClineProvider extends EventEmitter<ClineProviderEvents> implements
16211624
const nextUserMessageIndex = this.getCurrentCline()!.clineMessages.findIndex(
16221625
(msg) => msg === nextUserMessage,
16231626
)
1627+
16241628
// Keep messages before current message and after next user message
16251629
await this.getCurrentCline()!.overwriteClineMessages([
16261630
...this.getCurrentCline()!.clineMessages.slice(0, messageIndex),
@@ -2032,12 +2036,11 @@ export class ClineProvider extends EventEmitter<ClineProviderEvents> implements
20322036

20332037
await this.updateGlobalState("experiments", updatedExperiments)
20342038

2035-
// Update diffStrategy in current Cline instance if it exists
2036-
if (message.values[EXPERIMENT_IDS.DIFF_STRATEGY] !== undefined && this.getCurrentCline()) {
2037-
await this.getCurrentCline()!.updateDiffStrategy(
2038-
Experiments.isEnabled(updatedExperiments, EXPERIMENT_IDS.DIFF_STRATEGY),
2039-
Experiments.isEnabled(updatedExperiments, EXPERIMENT_IDS.MULTI_SEARCH_AND_REPLACE),
2040-
)
2039+
const currentCline = this.getCurrentCline()
2040+
2041+
// Update diffStrategy in current Cline instance if it exists.
2042+
if (message.values[EXPERIMENT_IDS.DIFF_STRATEGY_UNIFIED] !== undefined && currentCline) {
2043+
await currentCline.updateDiffStrategy(updatedExperiments)
20412044
}
20422045

20432046
await this.postStateToWebview()
@@ -2135,13 +2138,13 @@ export class ClineProvider extends EventEmitter<ClineProviderEvents> implements
21352138
language,
21362139
} = await this.getState()
21372140

2138-
// Create diffStrategy based on current model and settings
2139-
const diffStrategy = getDiffStrategy(
2140-
apiConfiguration.apiModelId || apiConfiguration.openRouterModelId || "",
2141+
// Create diffStrategy based on current model and settings.
2142+
const diffStrategy = getDiffStrategy({
2143+
model: apiConfiguration.apiModelId || apiConfiguration.openRouterModelId || "",
2144+
experiments,
21412145
fuzzyMatchThreshold,
2142-
Experiments.isEnabled(experiments, EXPERIMENT_IDS.DIFF_STRATEGY),
2143-
Experiments.isEnabled(experiments, EXPERIMENT_IDS.MULTI_SEARCH_AND_REPLACE),
2144-
)
2146+
})
2147+
21452148
const cwd = this.cwd
21462149

21472150
const mode = message.mode ?? defaultModeSlug
@@ -2197,6 +2200,7 @@ export class ClineProvider extends EventEmitter<ClineProviderEvents> implements
21972200
public async handleModeSwitch(newMode: Mode) {
21982201
// Capture mode switch telemetry event
21992202
const currentTaskId = this.getCurrentCline()?.taskId
2203+
22002204
if (currentTaskId) {
22012205
telemetryService.captureModeSwitch(currentTaskId, newMode)
22022206
}
@@ -2213,8 +2217,10 @@ export class ClineProvider extends EventEmitter<ClineProviderEvents> implements
22132217
// If this mode has a saved config, use it
22142218
if (savedConfigId) {
22152219
const config = listApiConfig?.find((c) => c.id === savedConfigId)
2220+
22162221
if (config?.name) {
22172222
const apiConfig = await this.providerSettingsManager.loadConfig(config.name)
2223+
22182224
await Promise.all([
22192225
this.updateGlobalState("currentApiConfigName", config.name),
22202226
this.updateApiConfiguration(apiConfig),
@@ -2226,6 +2232,7 @@ export class ClineProvider extends EventEmitter<ClineProviderEvents> implements
22262232

22272233
if (currentApiConfigName) {
22282234
const config = listApiConfig?.find((c) => c.name === currentApiConfigName)
2235+
22292236
if (config?.id) {
22302237
await this.providerSettingsManager.setModeConfig(newMode, config.id)
22312238
}

‎src/exports/roo-code.d.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -251,11 +251,11 @@ type GlobalSettings = {
251251
fuzzyMatchThreshold?: number | undefined
252252
experiments?:
253253
| {
254-
experimentalDiffStrategy: boolean
255254
search_and_replace: boolean
255+
experimentalDiffStrategy: boolean
256+
multi_search_and_replace: boolean
256257
insert_content: boolean
257258
powerSteering: boolean
258-
multi_search_and_replace: boolean
259259
}
260260
| undefined
261261
language?:

‎src/exports/types.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -254,11 +254,11 @@ type GlobalSettings = {
254254
fuzzyMatchThreshold?: number | undefined
255255
experiments?:
256256
| {
257-
experimentalDiffStrategy: boolean
258257
search_and_replace: boolean
258+
experimentalDiffStrategy: boolean
259+
multi_search_and_replace: boolean
259260
insert_content: boolean
260261
powerSteering: boolean
261-
multi_search_and_replace: boolean
262262
}
263263
| undefined
264264
language?:

‎src/schemas/index.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -213,11 +213,11 @@ export type CustomSupportPrompts = z.infer<typeof customSupportPromptsSchema>
213213
*/
214214

215215
export const experimentIds = [
216-
"experimentalDiffStrategy",
217216
"search_and_replace",
217+
"experimentalDiffStrategy",
218+
"multi_search_and_replace",
218219
"insert_content",
219220
"powerSteering",
220-
"multi_search_and_replace",
221221
] as const
222222

223223
export const experimentIdsSchema = z.enum(experimentIds)
@@ -229,11 +229,11 @@ export type ExperimentId = z.infer<typeof experimentIdsSchema>
229229
*/
230230

231231
const experimentsSchema = z.object({
232-
experimentalDiffStrategy: z.boolean(),
233232
search_and_replace: z.boolean(),
233+
experimentalDiffStrategy: z.boolean(),
234+
multi_search_and_replace: z.boolean(),
234235
insert_content: z.boolean(),
235236
powerSteering: z.boolean(),
236-
multi_search_and_replace: z.boolean(),
237237
})
238238

239239
export type Experiments = z.infer<typeof experimentsSchema>

‎src/shared/experiments.ts‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,11 @@ import { AssertEqual, Equals, Keys, Values } from "../utils/type-fu"
44
export type { ExperimentId }
55

66
export const EXPERIMENT_IDS = {
7-
DIFF_STRATEGY: "experimentalDiffStrategy",
8-
SEARCH_AND_REPLACE: "search_and_replace",
7+
DIFF_STRATEGY_SEARCH_AND_REPLACE: "search_and_replace",
8+
DIFF_STRATEGY_UNIFIED: "experimentalDiffStrategy",
9+
DIFF_STRATEGY_MULTI_SEARCH_AND_REPLACE: "multi_search_and_replace",
910
INSERT_BLOCK: "insert_content",
1011
POWER_STEERING: "powerSteering",
11-
MULTI_SEARCH_AND_REPLACE: "multi_search_and_replace",
1212
} as const satisfies Record<string, ExperimentId>
1313

1414
type _AssertExperimentIds = AssertEqual<Equals<ExperimentId, Values<typeof EXPERIMENT_IDS>>>
@@ -20,11 +20,11 @@ interface ExperimentConfig {
2020
}
2121

2222
export const experimentConfigsMap: Record<ExperimentKey, ExperimentConfig> = {
23-
DIFF_STRATEGY: { enabled: false },
24-
SEARCH_AND_REPLACE: { enabled: false },
23+
DIFF_STRATEGY_SEARCH_AND_REPLACE: { enabled: false },
24+
DIFF_STRATEGY_UNIFIED: { enabled: false },
25+
DIFF_STRATEGY_MULTI_SEARCH_AND_REPLACE: { enabled: false },
2526
INSERT_BLOCK: { enabled: false },
2627
POWER_STEERING: { enabled: false },
27-
MULTI_SEARCH_AND_REPLACE: { enabled: false },
2828
}
2929

3030
export const experimentDefault = Object.fromEntries(

0 commit comments

Comments
 (0)