Skip to content

Commit 8e65a4d

Browse files
committed
feat: add granular checkboxes for re-read after edit settings
- Added reReadAfterEditGranular experiment with individual toggles for each edit operation type - Updated all file editing tools to check their specific granular setting - Created new ReReadAfterEditGranularSettings UI component with master toggle and individual checkboxes - Maintained backward compatibility with legacy reReadAfterEdit setting that overrides granular settings - Added comprehensive tests for the new granular settings functionality - Added translations for the new UI elements This allows users to selectively enable automatic review for specific edit types like: - Apply Diff (search and replace blocks) - Multi-file Apply Diff - Write to File (complete file rewrites) - Insert Content (add lines to files) - Search and Replace (find and replace text) Addresses Issue #8927 - implements the suggestion to have checkboxes for selecting review based on edit type
1 parent f86af76 commit 8e65a4d

File tree

14 files changed

+347
-45
lines changed

14 files changed

+347
-45
lines changed

packages/types/src/experiment.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ export const experimentIds = [
1313
"imageGeneration",
1414
"runSlashCommand",
1515
"reReadAfterEdit",
16+
"reReadAfterEditGranular",
1617
] as const
1718

1819
export const experimentIdsSchema = z.enum(experimentIds)
@@ -23,13 +24,25 @@ export type ExperimentId = z.infer<typeof experimentIdsSchema>
2324
* Experiments
2425
*/
2526

27+
// Schema for granular re-read after edit settings
28+
export const reReadAfterEditGranularSchema = z.object({
29+
applyDiff: z.boolean().optional(),
30+
multiApplyDiff: z.boolean().optional(),
31+
writeToFile: z.boolean().optional(),
32+
insertContent: z.boolean().optional(),
33+
searchAndReplace: z.boolean().optional(),
34+
})
35+
36+
export type ReReadAfterEditGranular = z.infer<typeof reReadAfterEditGranularSchema>
37+
2638
export const experimentsSchema = z.object({
2739
powerSteering: z.boolean().optional(),
2840
multiFileApplyDiff: z.boolean().optional(),
2941
preventFocusDisruption: z.boolean().optional(),
3042
imageGeneration: z.boolean().optional(),
3143
runSlashCommand: z.boolean().optional(),
3244
reReadAfterEdit: z.boolean().optional(),
45+
reReadAfterEditGranular: reReadAfterEditGranularSchema.optional(),
3346
})
3447

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

src/core/tools/applyDiffTool.ts

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -238,11 +238,8 @@ export async function applyDiffToolLegacy(
238238
? "\n<notice>Making multiple related changes in a single apply_diff is more efficient. If other changes are needed in this file, please include them as additional SEARCH/REPLACE blocks.</notice>"
239239
: ""
240240

241-
// Check if RE_READ_AFTER_EDIT experiment is enabled
242-
const isReReadAfterEditEnabled = experiments.isEnabled(
243-
state?.experiments ?? {},
244-
EXPERIMENT_IDS.RE_READ_AFTER_EDIT,
245-
)
241+
// Check if RE_READ_AFTER_EDIT experiment is enabled for applyDiff
242+
const isReReadAfterEditEnabled = experiments.isReReadAfterEditEnabled(state?.experiments ?? {}, "applyDiff")
246243

247244
const reReadSuggestion = isReReadAfterEditEnabled
248245
? `\n\n<review_suggestion>The file has been edited. Consider using the read_file tool to review the changes and ensure they are correct and complete.</review_suggestion>`

src/core/tools/insertContentTool.ts

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -184,11 +184,8 @@ export async function insertContentTool(
184184
// Get the formatted response message
185185
const message = await cline.diffViewProvider.pushToolWriteResult(cline, cline.cwd, !fileExists)
186186

187-
// Check if RE_READ_AFTER_EDIT experiment is enabled
188-
const isReReadAfterEditEnabled = experiments.isEnabled(
189-
state?.experiments ?? {},
190-
EXPERIMENT_IDS.RE_READ_AFTER_EDIT,
191-
)
187+
// Check if RE_READ_AFTER_EDIT experiment is enabled for insertContent
188+
const isReReadAfterEditEnabled = experiments.isReReadAfterEditEnabled(state?.experiments ?? {}, "insertContent")
192189

193190
const reReadSuggestion = isReReadAfterEditEnabled
194191
? `\n\n<review_suggestion>Content has been inserted into the file. Consider using the read_file tool to review the changes and ensure they are correct and complete.</review_suggestion>`

src/core/tools/multiApplyDiffTool.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -676,12 +676,12 @@ ${errorDetails ? `\nTechnical details:\n${errorDetails}\n` : ""}
676676
? "\n<notice>Making multiple related changes in a single apply_diff is more efficient. If other changes are needed in this file, please include them as additional SEARCH/REPLACE blocks.</notice>"
677677
: ""
678678

679-
// Check if RE_READ_AFTER_EDIT experiment is enabled
679+
// Check if RE_READ_AFTER_EDIT experiment is enabled for multiApplyDiff
680680
const provider = cline.providerRef.deref()
681681
const state = await provider?.getState()
682-
const isReReadAfterEditEnabled = experiments.isEnabled(
682+
const isReReadAfterEditEnabled = experiments.isReReadAfterEditEnabled(
683683
state?.experiments ?? {},
684-
EXPERIMENT_IDS.RE_READ_AFTER_EDIT,
684+
"multiApplyDiff",
685685
)
686686

687687
// Count how many files were successfully edited

src/core/tools/searchAndReplaceTool.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -258,10 +258,10 @@ export async function searchAndReplaceTool(
258258
false, // Always false for search_and_replace
259259
)
260260

261-
// Check if RE_READ_AFTER_EDIT experiment is enabled
262-
const isReReadAfterEditEnabled = experiments.isEnabled(
261+
// Check if RE_READ_AFTER_EDIT experiment is enabled for searchAndReplace
262+
const isReReadAfterEditEnabled = experiments.isReReadAfterEditEnabled(
263263
state?.experiments ?? {},
264-
EXPERIMENT_IDS.RE_READ_AFTER_EDIT,
264+
"searchAndReplace",
265265
)
266266

267267
const reReadSuggestion = isReReadAfterEditEnabled

src/core/tools/writeToFileTool.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -304,10 +304,10 @@ export async function writeToFileTool(
304304
// Get the formatted response message
305305
const message = await cline.diffViewProvider.pushToolWriteResult(cline, cline.cwd, !fileExists)
306306

307-
// Check if RE_READ_AFTER_EDIT experiment is enabled
308-
const isReReadAfterEditEnabled = experiments.isEnabled(
307+
// Check if RE_READ_AFTER_EDIT experiment is enabled for writeToFile
308+
const isReReadAfterEditEnabled = experiments.isReReadAfterEditEnabled(
309309
state?.experiments ?? {},
310-
EXPERIMENT_IDS.RE_READ_AFTER_EDIT,
310+
"writeToFile",
311311
)
312312

313313
const reReadSuggestion = isReReadAfterEditEnabled
Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
import { describe, it, expect } from "vitest"
2+
import type { Experiments, ReReadAfterEditGranular } from "@roo-code/types"
3+
import { experiments } from "../experiments"
4+
5+
describe("granular re-read after edit experiment", () => {
6+
describe("isReReadAfterEditEnabled", () => {
7+
it("should return true when legacy reReadAfterEdit is enabled", () => {
8+
const config: Experiments = {
9+
reReadAfterEdit: true,
10+
}
11+
12+
expect(experiments.isReReadAfterEditEnabled(config, "applyDiff")).toBe(true)
13+
expect(experiments.isReReadAfterEditEnabled(config, "multiApplyDiff")).toBe(true)
14+
expect(experiments.isReReadAfterEditEnabled(config, "writeToFile")).toBe(true)
15+
expect(experiments.isReReadAfterEditEnabled(config, "insertContent")).toBe(true)
16+
expect(experiments.isReReadAfterEditEnabled(config, "searchAndReplace")).toBe(true)
17+
})
18+
19+
it("should return false when legacy reReadAfterEdit is disabled and no granular settings", () => {
20+
const config: Experiments = {
21+
reReadAfterEdit: false,
22+
}
23+
24+
expect(experiments.isReReadAfterEditEnabled(config, "applyDiff")).toBe(false)
25+
expect(experiments.isReReadAfterEditEnabled(config, "multiApplyDiff")).toBe(false)
26+
expect(experiments.isReReadAfterEditEnabled(config, "writeToFile")).toBe(false)
27+
expect(experiments.isReReadAfterEditEnabled(config, "insertContent")).toBe(false)
28+
expect(experiments.isReReadAfterEditEnabled(config, "searchAndReplace")).toBe(false)
29+
})
30+
31+
it("should return true for specific edit types when granular settings are enabled", () => {
32+
const config: Experiments = {
33+
reReadAfterEdit: false,
34+
reReadAfterEditGranular: {
35+
applyDiff: true,
36+
multiApplyDiff: false,
37+
writeToFile: true,
38+
insertContent: false,
39+
searchAndReplace: true,
40+
},
41+
}
42+
43+
expect(experiments.isReReadAfterEditEnabled(config, "applyDiff")).toBe(true)
44+
expect(experiments.isReReadAfterEditEnabled(config, "multiApplyDiff")).toBe(false)
45+
expect(experiments.isReReadAfterEditEnabled(config, "writeToFile")).toBe(true)
46+
expect(experiments.isReReadAfterEditEnabled(config, "insertContent")).toBe(false)
47+
expect(experiments.isReReadAfterEditEnabled(config, "searchAndReplace")).toBe(true)
48+
})
49+
50+
it("should prioritize legacy setting over granular when both are present", () => {
51+
const config: Experiments = {
52+
reReadAfterEdit: true, // Legacy enabled
53+
reReadAfterEditGranular: {
54+
applyDiff: false, // Granular disabled
55+
multiApplyDiff: false,
56+
writeToFile: false,
57+
insertContent: false,
58+
searchAndReplace: false,
59+
},
60+
}
61+
62+
// Legacy setting should override granular settings
63+
expect(experiments.isReReadAfterEditEnabled(config, "applyDiff")).toBe(true)
64+
expect(experiments.isReReadAfterEditEnabled(config, "multiApplyDiff")).toBe(true)
65+
expect(experiments.isReReadAfterEditEnabled(config, "writeToFile")).toBe(true)
66+
expect(experiments.isReReadAfterEditEnabled(config, "insertContent")).toBe(true)
67+
expect(experiments.isReReadAfterEditEnabled(config, "searchAndReplace")).toBe(true)
68+
})
69+
70+
it("should handle partial granular settings", () => {
71+
const config: Experiments = {
72+
reReadAfterEdit: false,
73+
reReadAfterEditGranular: {
74+
applyDiff: true,
75+
// Other fields undefined
76+
} as ReReadAfterEditGranular,
77+
}
78+
79+
expect(experiments.isReReadAfterEditEnabled(config, "applyDiff")).toBe(true)
80+
expect(experiments.isReReadAfterEditEnabled(config, "multiApplyDiff")).toBe(false)
81+
expect(experiments.isReReadAfterEditEnabled(config, "writeToFile")).toBe(false)
82+
expect(experiments.isReReadAfterEditEnabled(config, "insertContent")).toBe(false)
83+
expect(experiments.isReReadAfterEditEnabled(config, "searchAndReplace")).toBe(false)
84+
})
85+
86+
it("should return false when no experiments config is provided", () => {
87+
const config: Experiments = {}
88+
89+
expect(experiments.isReReadAfterEditEnabled(config, "applyDiff")).toBe(false)
90+
expect(experiments.isReReadAfterEditEnabled(config, "multiApplyDiff")).toBe(false)
91+
expect(experiments.isReReadAfterEditEnabled(config, "writeToFile")).toBe(false)
92+
expect(experiments.isReReadAfterEditEnabled(config, "insertContent")).toBe(false)
93+
expect(experiments.isReReadAfterEditEnabled(config, "searchAndReplace")).toBe(false)
94+
})
95+
})
96+
})

src/shared/__tests__/experiments.spec.ts

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
// npx vitest run src/shared/__tests__/experiments.spec.ts
22

3-
import type { ExperimentId } from "@roo-code/types"
3+
import type { ExperimentId, Experiments as ExperimentsType } from "@roo-code/types"
44

55
import { EXPERIMENT_IDS, experimentConfigsMap, experiments as Experiments } from "../experiments"
66

@@ -25,39 +25,51 @@ describe("experiments", () => {
2525

2626
describe("isEnabled", () => {
2727
it("returns false when POWER_STEERING experiment is not enabled", () => {
28-
const experiments: Record<ExperimentId, boolean> = {
28+
const experiments: ExperimentsType = {
2929
powerSteering: false,
3030
multiFileApplyDiff: false,
3131
preventFocusDisruption: false,
3232
imageGeneration: false,
3333
runSlashCommand: false,
3434
reReadAfterEdit: false,
35+
reReadAfterEditGranular: undefined,
3536
}
3637
expect(Experiments.isEnabled(experiments, EXPERIMENT_IDS.POWER_STEERING)).toBe(false)
3738
})
3839

3940
it("returns true when experiment POWER_STEERING is enabled", () => {
40-
const experiments: Record<ExperimentId, boolean> = {
41+
const experiments: ExperimentsType = {
4142
powerSteering: true,
4243
multiFileApplyDiff: false,
4344
preventFocusDisruption: false,
4445
imageGeneration: false,
4546
runSlashCommand: false,
4647
reReadAfterEdit: false,
48+
reReadAfterEditGranular: undefined,
4749
}
4850
expect(Experiments.isEnabled(experiments, EXPERIMENT_IDS.POWER_STEERING)).toBe(true)
4951
})
5052

5153
it("returns false when experiment is not present", () => {
52-
const experiments: Record<ExperimentId, boolean> = {
54+
const experiments: ExperimentsType = {
5355
powerSteering: false,
5456
multiFileApplyDiff: false,
5557
preventFocusDisruption: false,
5658
imageGeneration: false,
5759
runSlashCommand: false,
5860
reReadAfterEdit: false,
61+
reReadAfterEditGranular: undefined,
5962
}
6063
expect(Experiments.isEnabled(experiments, EXPERIMENT_IDS.POWER_STEERING)).toBe(false)
6164
})
6265
})
66+
67+
describe("RE_READ_AFTER_EDIT_GRANULAR", () => {
68+
it("is configured correctly", () => {
69+
expect(EXPERIMENT_IDS.RE_READ_AFTER_EDIT_GRANULAR).toBe("reReadAfterEditGranular")
70+
expect(experimentConfigsMap.RE_READ_AFTER_EDIT_GRANULAR).toMatchObject({
71+
enabled: false,
72+
})
73+
})
74+
})
6375
})

src/shared/experiments.ts

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,12 @@
1-
import type { AssertEqual, Equals, Keys, Values, ExperimentId, Experiments } from "@roo-code/types"
1+
import type {
2+
AssertEqual,
3+
Equals,
4+
Keys,
5+
Values,
6+
ExperimentId,
7+
Experiments,
8+
ReReadAfterEditGranular,
9+
} from "@roo-code/types"
210

311
export const EXPERIMENT_IDS = {
412
MULTI_FILE_APPLY_DIFF: "multiFileApplyDiff",
@@ -7,14 +15,15 @@ export const EXPERIMENT_IDS = {
715
IMAGE_GENERATION: "imageGeneration",
816
RUN_SLASH_COMMAND: "runSlashCommand",
917
RE_READ_AFTER_EDIT: "reReadAfterEdit",
18+
RE_READ_AFTER_EDIT_GRANULAR: "reReadAfterEditGranular",
1019
} as const satisfies Record<string, ExperimentId>
1120

1221
type _AssertExperimentIds = AssertEqual<Equals<ExperimentId, Values<typeof EXPERIMENT_IDS>>>
1322

1423
type ExperimentKey = Keys<typeof EXPERIMENT_IDS>
1524

1625
interface ExperimentConfig {
17-
enabled: boolean
26+
enabled: boolean | ReReadAfterEditGranular
1827
}
1928

2029
export const experimentConfigsMap: Record<ExperimentKey, ExperimentConfig> = {
@@ -24,16 +33,39 @@ export const experimentConfigsMap: Record<ExperimentKey, ExperimentConfig> = {
2433
IMAGE_GENERATION: { enabled: false },
2534
RUN_SLASH_COMMAND: { enabled: false },
2635
RE_READ_AFTER_EDIT: { enabled: false },
36+
RE_READ_AFTER_EDIT_GRANULAR: {
37+
enabled: {
38+
applyDiff: false,
39+
multiApplyDiff: false,
40+
writeToFile: false,
41+
insertContent: false,
42+
searchAndReplace: false,
43+
},
44+
},
2745
}
2846

2947
export const experimentDefault = Object.fromEntries(
3048
Object.entries(experimentConfigsMap).map(([_, config]) => [
3149
EXPERIMENT_IDS[_ as keyof typeof EXPERIMENT_IDS] as ExperimentId,
3250
config.enabled,
3351
]),
34-
) as Record<ExperimentId, boolean>
52+
) as Experiments
3553

3654
export const experiments = {
3755
get: (id: ExperimentKey): ExperimentConfig | undefined => experimentConfigsMap[id],
3856
isEnabled: (experimentsConfig: Experiments, id: ExperimentId) => experimentsConfig[id] ?? experimentDefault[id],
57+
isReReadAfterEditEnabled: (experimentsConfig: Experiments, editType: keyof ReReadAfterEditGranular): boolean => {
58+
// If the legacy RE_READ_AFTER_EDIT is enabled, it applies to all edit types
59+
if (experimentsConfig.reReadAfterEdit) {
60+
return true
61+
}
62+
63+
// Check if granular settings are enabled and if the specific edit type is enabled
64+
const granularSettings = experimentsConfig.reReadAfterEditGranular
65+
if (granularSettings && granularSettings[editType]) {
66+
return true
67+
}
68+
69+
return false
70+
},
3971
} as const

0 commit comments

Comments
 (0)