Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions packages/types/src/experiment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ export const experimentIds = [
"preventFocusDisruption",
"imageGeneration",
"runSlashCommand",
"reReadAfterEdit",
] as const

export const experimentIdsSchema = z.enum(experimentIds)
Expand All @@ -28,6 +29,7 @@ export const experimentsSchema = z.object({
preventFocusDisruption: z.boolean().optional(),
imageGeneration: z.boolean().optional(),
runSlashCommand: z.boolean().optional(),
reReadAfterEdit: z.boolean().optional(),
})

export type Experiments = z.infer<typeof experimentsSchema>
Expand Down
14 changes: 12 additions & 2 deletions src/core/tools/applyDiffTool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -238,10 +238,20 @@ export async function applyDiffToolLegacy(
? "\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>"
: ""

// Check if RE_READ_AFTER_EDIT experiment is enabled
const isReReadAfterEditEnabled = experiments.isEnabled(
state?.experiments ?? {},
EXPERIMENT_IDS.RE_READ_AFTER_EDIT,
)

const reReadSuggestion = isReReadAfterEditEnabled
? `\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>`
: ""

if (partFailHint) {
pushToolResult(partFailHint + message + singleBlockNotice)
pushToolResult(partFailHint + message + singleBlockNotice + reReadSuggestion)
} else {
pushToolResult(message + singleBlockNotice)
pushToolResult(message + singleBlockNotice + reReadSuggestion)
}

await cline.diffViewProvider.reset()
Expand Down
12 changes: 11 additions & 1 deletion src/core/tools/insertContentTool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,17 @@ export async function insertContentTool(
// Get the formatted response message
const message = await cline.diffViewProvider.pushToolWriteResult(cline, cline.cwd, !fileExists)

pushToolResult(message)
// Check if RE_READ_AFTER_EDIT experiment is enabled
const isReReadAfterEditEnabled = experiments.isEnabled(
state?.experiments ?? {},
EXPERIMENT_IDS.RE_READ_AFTER_EDIT,
)

const reReadSuggestion = isReReadAfterEditEnabled
? `\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>`
: ""

pushToolResult(message + reReadSuggestion)

await cline.diffViewProvider.reset()

Expand Down
17 changes: 16 additions & 1 deletion src/core/tools/multiApplyDiffTool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -676,8 +676,23 @@ ${errorDetails ? `\nTechnical details:\n${errorDetails}\n` : ""}
? "\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>"
: ""

// Check if RE_READ_AFTER_EDIT experiment is enabled
const provider = cline.providerRef.deref()
const state = await provider?.getState()
const isReReadAfterEditEnabled = experiments.isEnabled(
state?.experiments ?? {},
EXPERIMENT_IDS.RE_READ_AFTER_EDIT,
)

// Count how many files were successfully edited
const editedFiles = operationResults.filter((op) => op.status === "approved").length
const reReadSuggestion =
isReReadAfterEditEnabled && editedFiles > 0
? `\n\n<review_suggestion>${editedFiles === 1 ? "The file has" : `${editedFiles} files have`} been edited. Consider using the read_file tool to review the changes and ensure they are correct and complete.</review_suggestion>`
: ""
Comment on lines +687 to +692
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The review suggestion count may be inaccurate. When a file is approved but all its diffs fail to apply (successCount === 0 at line 494), the code continues without saving changes (line 514), but the file remains with status "approved". Later at line 688, the code counts all "approved" files for the review suggestion, incorrectly including files that were approved but never actually edited. This could result in suggestions like "2 files have been edited" when only 1 file was successfully modified.


// Push the final result combining all operation results
pushToolResult(results.join("\n\n") + singleBlockNotice)
pushToolResult(results.join("\n\n") + singleBlockNotice + reReadSuggestion)
cline.processQueuedMessages()
return
} catch (error) {
Expand Down
12 changes: 11 additions & 1 deletion src/core/tools/searchAndReplaceTool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,17 @@ export async function searchAndReplaceTool(
false, // Always false for search_and_replace
)

pushToolResult(message)
// Check if RE_READ_AFTER_EDIT experiment is enabled
const isReReadAfterEditEnabled = experiments.isEnabled(
state?.experiments ?? {},
EXPERIMENT_IDS.RE_READ_AFTER_EDIT,
)

const reReadSuggestion = isReReadAfterEditEnabled
? `\n\n<review_suggestion>The file has been modified via search and replace. Consider using the read_file tool to review the changes and ensure they are correct and complete.</review_suggestion>`
: ""

pushToolResult(message + reReadSuggestion)

// Record successful tool usage and cleanup
cline.recordToolUsage("search_and_replace")
Expand Down
12 changes: 11 additions & 1 deletion src/core/tools/writeToFileTool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,17 @@ export async function writeToFileTool(
// Get the formatted response message
const message = await cline.diffViewProvider.pushToolWriteResult(cline, cline.cwd, !fileExists)

pushToolResult(message)
// Check if RE_READ_AFTER_EDIT experiment is enabled
const isReReadAfterEditEnabled = experiments.isEnabled(
state?.experiments ?? {},
EXPERIMENT_IDS.RE_READ_AFTER_EDIT,
)

const reReadSuggestion = isReReadAfterEditEnabled
? `\n\n<review_suggestion>The file has been ${fileExists ? "edited" : "created"}. Consider using the read_file tool to review the ${fileExists ? "changes" : "content"} and ensure ${fileExists ? "they are" : "it is"} correct and complete.</review_suggestion>`
: ""

pushToolResult(message + reReadSuggestion)

await cline.diffViewProvider.reset()

Expand Down
23 changes: 23 additions & 0 deletions src/shared/__tests__/experiments-reReadAfterEdit.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { describe, it, expect } from "vitest"
import { EXPERIMENT_IDS, experiments, experimentDefault } from "../experiments"

describe("RE_READ_AFTER_EDIT experiment", () => {
it("should include RE_READ_AFTER_EDIT in EXPERIMENT_IDS", () => {
expect(EXPERIMENT_IDS.RE_READ_AFTER_EDIT).toBe("reReadAfterEdit")
})

it("should have RE_READ_AFTER_EDIT in default configuration", () => {
expect(experimentDefault.reReadAfterEdit).toBe(false)
})

it("should correctly check if RE_READ_AFTER_EDIT is enabled", () => {
const disabledConfig = { reReadAfterEdit: false }
expect(experiments.isEnabled(disabledConfig, EXPERIMENT_IDS.RE_READ_AFTER_EDIT)).toBe(false)

const enabledConfig = { reReadAfterEdit: true }
expect(experiments.isEnabled(enabledConfig, EXPERIMENT_IDS.RE_READ_AFTER_EDIT)).toBe(true)

const emptyConfig = {}
expect(experiments.isEnabled(emptyConfig, EXPERIMENT_IDS.RE_READ_AFTER_EDIT)).toBe(false)
})
})
3 changes: 3 additions & 0 deletions src/shared/__tests__/experiments.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ describe("experiments", () => {
preventFocusDisruption: false,
imageGeneration: false,
runSlashCommand: false,
reReadAfterEdit: false,
}
expect(Experiments.isEnabled(experiments, EXPERIMENT_IDS.POWER_STEERING)).toBe(false)
})
Expand All @@ -42,6 +43,7 @@ describe("experiments", () => {
preventFocusDisruption: false,
imageGeneration: false,
runSlashCommand: false,
reReadAfterEdit: false,
}
expect(Experiments.isEnabled(experiments, EXPERIMENT_IDS.POWER_STEERING)).toBe(true)
})
Expand All @@ -53,6 +55,7 @@ describe("experiments", () => {
preventFocusDisruption: false,
imageGeneration: false,
runSlashCommand: false,
reReadAfterEdit: false,
}
expect(Experiments.isEnabled(experiments, EXPERIMENT_IDS.POWER_STEERING)).toBe(false)
})
Expand Down
2 changes: 2 additions & 0 deletions src/shared/experiments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ export const EXPERIMENT_IDS = {
PREVENT_FOCUS_DISRUPTION: "preventFocusDisruption",
IMAGE_GENERATION: "imageGeneration",
RUN_SLASH_COMMAND: "runSlashCommand",
RE_READ_AFTER_EDIT: "reReadAfterEdit",
} as const satisfies Record<string, ExperimentId>

type _AssertExperimentIds = AssertEqual<Equals<ExperimentId, Values<typeof EXPERIMENT_IDS>>>
Expand All @@ -22,6 +23,7 @@ export const experimentConfigsMap: Record<ExperimentKey, ExperimentConfig> = {
PREVENT_FOCUS_DISRUPTION: { enabled: false },
IMAGE_GENERATION: { enabled: false },
RUN_SLASH_COMMAND: { enabled: false },
RE_READ_AFTER_EDIT: { enabled: false },
}

export const experimentDefault = Object.fromEntries(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,7 @@ describe("mergeExtensionState", () => {
newTaskRequireTodos: false,
imageGeneration: false,
runSlashCommand: false,
reReadAfterEdit: false,
} as Record<ExperimentId, boolean>,
checkpointTimeout: DEFAULT_CHECKPOINT_TIMEOUT_SECONDS + 5,
}
Expand All @@ -258,6 +259,7 @@ describe("mergeExtensionState", () => {
newTaskRequireTodos: false,
imageGeneration: false,
runSlashCommand: false,
reReadAfterEdit: false,
})
})
})
4 changes: 4 additions & 0 deletions webview-ui/src/i18n/locales/en/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -775,6 +775,10 @@
"RUN_SLASH_COMMAND": {
"name": "Enable model-initiated slash commands",
"description": "When enabled, Roo can run your slash commands to execute workflows."
},
"RE_READ_AFTER_EDIT": {
"name": "Suggest review after file edits",
"description": "When enabled, Roo will suggest reviewing changes after editing files. This helps catch errors that might be introduced during editing by prompting to review the changes."
}
},
"promptCaching": {
Expand Down
Loading