Skip to content

Commit 2303f67

Browse files
ctecte
authored
Clean up the way we compute the current diff strategy (#2049)
* Clean up the way we compute the current diff strategy * Add changeset
1 parent 87b0647 commit 2303f67

File tree

12 files changed

+106
-93
lines changed

12 files changed

+106
-93
lines changed

‎.changeset/orange-items-remember.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"roo-cline": patch
3+
---
4+
5+
Consolidate logic that computes the current diff strategy

‎src/core/Cline.ts‎

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

230-
// Initialize diffStrategy based on current state
231-
this.updateDiffStrategy(
232-
Experiments.isEnabled(experiments ?? {}, EXPERIMENT_IDS.DIFF_STRATEGY),
233-
Experiments.isEnabled(experiments ?? {}, EXPERIMENT_IDS.MULTI_SEARCH_AND_REPLACE),
234-
)
230+
// Initialize diffStrategy based on current state.
231+
this.updateDiffStrategy(experiments ?? {})
235232

236233
onCreated?.(this)
237234

@@ -266,25 +263,13 @@ export class Cline extends EventEmitter<ClineEvents> {
266263
return getWorkspacePath(path.join(os.homedir(), "Desktop"))
267264
}
268265

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

290275
// Storing task to disk for history

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

Lines changed: 11 additions & 2 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,7 +326,11 @@ 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

327336
it("should require either task or historyItem", () => {

‎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

@@ -1539,6 +1539,7 @@ export class ClineProvider extends EventEmitter<ClineProviderEvents> implements
15391539
t("common:confirmation.just_this_message"),
15401540
t("common:confirmation.this_and_subsequent"),
15411541
)
1542+
15421543
if (
15431544
(answer === t("common:confirmation.just_this_message") ||
15441545
answer === t("common:confirmation.this_and_subsequent")) &&
@@ -1547,9 +1548,11 @@ export class ClineProvider extends EventEmitter<ClineProviderEvents> implements
15471548
message.value
15481549
) {
15491550
const timeCutoff = message.value - 1000 // 1 second buffer before the message to delete
1551+
15501552
const messageIndex = this.getCurrentCline()!.clineMessages.findIndex(
15511553
(msg) => msg.ts && msg.ts >= timeCutoff,
15521554
)
1555+
15531556
const apiConversationHistoryIndex =
15541557
this.getCurrentCline()?.apiConversationHistory.findIndex(
15551558
(msg) => msg.ts && msg.ts >= timeCutoff,
@@ -1570,6 +1573,7 @@ export class ClineProvider extends EventEmitter<ClineProviderEvents> implements
15701573
const nextUserMessageIndex = this.getCurrentCline()!.clineMessages.findIndex(
15711574
(msg) => msg === nextUserMessage,
15721575
)
1576+
15731577
// Keep messages before current message and after next user message
15741578
await this.getCurrentCline()!.overwriteClineMessages([
15751579
...this.getCurrentCline()!.clineMessages.slice(0, messageIndex),
@@ -1981,12 +1985,11 @@ export class ClineProvider extends EventEmitter<ClineProviderEvents> implements
19811985

19821986
await this.updateGlobalState("experiments", updatedExperiments)
19831987

1984-
// Update diffStrategy in current Cline instance if it exists
1985-
if (message.values[EXPERIMENT_IDS.DIFF_STRATEGY] !== undefined && this.getCurrentCline()) {
1986-
await this.getCurrentCline()!.updateDiffStrategy(
1987-
Experiments.isEnabled(updatedExperiments, EXPERIMENT_IDS.DIFF_STRATEGY),
1988-
Experiments.isEnabled(updatedExperiments, EXPERIMENT_IDS.MULTI_SEARCH_AND_REPLACE),
1989-
)
1988+
const currentCline = this.getCurrentCline()
1989+
1990+
// Update diffStrategy in current Cline instance if it exists.
1991+
if (message.values[EXPERIMENT_IDS.DIFF_STRATEGY_UNIFIED] !== undefined && currentCline) {
1992+
await currentCline.updateDiffStrategy(updatedExperiments)
19901993
}
19911994

19921995
await this.postStateToWebview()
@@ -2084,13 +2087,13 @@ export class ClineProvider extends EventEmitter<ClineProviderEvents> implements
20842087
language,
20852088
} = await this.getState()
20862089

2087-
// Create diffStrategy based on current model and settings
2088-
const diffStrategy = getDiffStrategy(
2089-
apiConfiguration.apiModelId || apiConfiguration.openRouterModelId || "",
2090+
// Create diffStrategy based on current model and settings.
2091+
const diffStrategy = getDiffStrategy({
2092+
model: apiConfiguration.apiModelId || apiConfiguration.openRouterModelId || "",
2093+
experiments,
20902094
fuzzyMatchThreshold,
2091-
Experiments.isEnabled(experiments, EXPERIMENT_IDS.DIFF_STRATEGY),
2092-
Experiments.isEnabled(experiments, EXPERIMENT_IDS.MULTI_SEARCH_AND_REPLACE),
2093-
)
2095+
})
2096+
20942097
const cwd = this.cwd
20952098

20962099
const mode = message.mode ?? defaultModeSlug
@@ -2146,6 +2149,7 @@ export class ClineProvider extends EventEmitter<ClineProviderEvents> implements
21462149
public async handleModeSwitch(newMode: Mode) {
21472150
// Capture mode switch telemetry event
21482151
const currentTaskId = this.getCurrentCline()?.taskId
2152+
21492153
if (currentTaskId) {
21502154
telemetryService.captureModeSwitch(currentTaskId, newMode)
21512155
}
@@ -2162,8 +2166,10 @@ export class ClineProvider extends EventEmitter<ClineProviderEvents> implements
21622166
// If this mode has a saved config, use it
21632167
if (savedConfigId) {
21642168
const config = listApiConfig?.find((c) => c.id === savedConfigId)
2169+
21652170
if (config?.name) {
21662171
const apiConfig = await this.providerSettingsManager.loadConfig(config.name)
2172+
21672173
await Promise.all([
21682174
this.updateGlobalState("currentApiConfigName", config.name),
21692175
this.updateApiConfiguration(apiConfig),
@@ -2175,6 +2181,7 @@ export class ClineProvider extends EventEmitter<ClineProviderEvents> implements
21752181

21762182
if (currentApiConfigName) {
21772183
const config = listApiConfig?.find((c) => c.name === currentApiConfigName)
2184+
21782185
if (config?.id) {
21792186
await this.providerSettingsManager.setModeConfig(newMode, config.id)
21802187
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -252,11 +252,11 @@ type GlobalSettings = {
252252
fuzzyMatchThreshold?: number | undefined
253253
experiments?:
254254
| {
255-
experimentalDiffStrategy: boolean
256255
search_and_replace: boolean
256+
experimentalDiffStrategy: boolean
257+
multi_search_and_replace: boolean
257258
insert_content: boolean
258259
powerSteering: boolean
259-
multi_search_and_replace: boolean
260260
}
261261
| undefined
262262
language?:

‎src/exports/types.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -255,11 +255,11 @@ type GlobalSettings = {
255255
fuzzyMatchThreshold?: number | undefined
256256
experiments?:
257257
| {
258-
experimentalDiffStrategy: boolean
259258
search_and_replace: boolean
259+
experimentalDiffStrategy: boolean
260+
multi_search_and_replace: boolean
260261
insert_content: boolean
261262
powerSteering: boolean
262-
multi_search_and_replace: boolean
263263
}
264264
| undefined
265265
language?:

‎src/schemas/index.ts‎

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

277277
export const experimentIds = [
278-
"experimentalDiffStrategy",
279278
"search_and_replace",
279+
"experimentalDiffStrategy",
280+
"multi_search_and_replace",
280281
"insert_content",
281282
"powerSteering",
282-
"multi_search_and_replace",
283283
] as const
284284

285285
export const experimentIdsSchema = z.enum(experimentIds)
@@ -291,11 +291,11 @@ export type ExperimentId = z.infer<typeof experimentIdsSchema>
291291
*/
292292

293293
const experimentsSchema = z.object({
294-
experimentalDiffStrategy: z.boolean(),
295294
search_and_replace: z.boolean(),
295+
experimentalDiffStrategy: z.boolean(),
296+
multi_search_and_replace: z.boolean(),
296297
insert_content: z.boolean(),
297298
powerSteering: z.boolean(),
298-
multi_search_and_replace: z.boolean(),
299299
})
300300

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

0 commit comments

Comments
 (0)