Skip to content

Commit 11dd48a

Browse files
playcationshannesrudolph
authored andcommitted
Simplify FileChangeManager architecture with proper accept and reject logic for Files Change Overview
• Replace dual Set<string> (acceptedFiles/rejectedFiles) with single Map<string, string> (acceptedBaselines) for cleaner state management • Remove complex hiding/unhiding logic in applyPerFileBaselines() • Rejected files are simply removed from changeset and reappear naturally when edited again via update FCOAfterEdit • Accepted files get per-file baselines to show only incremental changes • Self-correcting system: file visibility determined by diffs, not flags
1 parent 21661d7 commit 11dd48a

File tree

4 files changed

+733
-54
lines changed

4 files changed

+733
-54
lines changed

src/core/checkpoints/index.ts

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -284,8 +284,19 @@ async function checkGitInstallation(
284284

285285
log(`[Task#checkpointCreated] Found ${fileChanges.length} cumulative file changes`)
286286

287-
// Update FileChangeManager with the new files so view diff can find them
288-
checkpointFileChangeManager.setFiles(fileChanges)
287+
// Apply per-file baselines to show only incremental changes for accepted files
288+
const updatedChanges = await checkpointFileChangeManager.applyPerFileBaselines(
289+
fileChanges,
290+
service,
291+
toHash,
292+
)
293+
294+
log(
295+
`[Task#checkpointCreated] Applied per-file baselines, ${updatedChanges.length} changes after filtering`,
296+
)
297+
298+
// Update FileChangeManager with the per-file baseline changes
299+
checkpointFileChangeManager.setFiles(updatedChanges)
289300

290301
// DON'T clear accepted/rejected state here - preserve user's accept/reject decisions
291302
// The state should only be cleared on baseline changes (checkpoint restore) or task restart
@@ -479,8 +490,8 @@ export async function checkpointRestore(
479490
)
480491

481492
// Clear accept/reject state - checkpoint restore is time travel, start with clean slate
482-
if (typeof fileChangeManager.clearAcceptedRejectedState === "function") {
483-
fileChangeManager.clearAcceptedRejectedState()
493+
if (typeof fileChangeManager.clearFileStates === "function") {
494+
fileChangeManager.clearFileStates()
484495
provider?.log(`[checkpointRestore] Cleared accept/reject state for fresh start`)
485496
}
486497

src/services/file-changes/FileChangeManager.ts

Lines changed: 109 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { FileChange, FileChangeset } from "@roo-code/types"
1+
import { FileChange, FileChangeset, FileChangeType } from "@roo-code/types"
22
import type { FileContextTracker } from "../../core/context-tracking/FileContextTracker"
33

44
/**
@@ -7,30 +7,21 @@ import type { FileContextTracker } from "../../core/context-tracking/FileContext
77
*/
88
export class FileChangeManager {
99
private changeset: FileChangeset
10-
private acceptedFiles: Set<string>
11-
private rejectedFiles: Set<string>
10+
private acceptedBaselines: Map<string, string> // uri -> accepted baseline checkpoint
1211

1312
constructor(baseCheckpoint: string) {
1413
this.changeset = {
1514
baseCheckpoint,
1615
files: [],
1716
}
18-
this.acceptedFiles = new Set()
19-
this.rejectedFiles = new Set()
17+
this.acceptedBaselines = new Map()
2018
}
2119

2220
/**
23-
* Get current changeset with accepted/rejected files filtered out
21+
* Get current changeset - visibility determined by actual diffs
2422
*/
2523
public getChanges(): FileChangeset {
26-
const filteredFiles = this.changeset.files.filter(
27-
(file) => !this.acceptedFiles.has(file.uri) && !this.rejectedFiles.has(file.uri),
28-
)
29-
30-
return {
31-
...this.changeset,
32-
files: filteredFiles,
33-
}
24+
return this.changeset
3425
}
3526

3627
/**
@@ -47,13 +38,10 @@ export class FileChangeManager {
4738
.map((entry) => entry.path),
4839
)
4940

50-
// Filter changeset to only include LLM-modified files
51-
const filteredFiles = this.changeset.files.filter(
52-
(file) =>
53-
llmModifiedFiles.has(file.uri) &&
54-
!this.acceptedFiles.has(file.uri) &&
55-
!this.rejectedFiles.has(file.uri),
56-
)
41+
// Filter changeset to only include LLM-modified files that haven't been accepted
42+
const filteredFiles = this.changeset.files.filter((file) => {
43+
return llmModifiedFiles.has(file.uri) && !this.acceptedBaselines.has(file.uri) // Not accepted (no baseline set)
44+
})
5745

5846
return {
5947
...this.changeset,
@@ -72,36 +60,39 @@ export class FileChangeManager {
7260
* Accept a specific file change
7361
*/
7462
public async acceptChange(uri: string): Promise<void> {
75-
this.acceptedFiles.add(uri)
76-
this.rejectedFiles.delete(uri)
63+
const file = this.getFileChange(uri)
64+
if (file) {
65+
// Set baseline - file will disappear from FCO naturally (no diff from baseline)
66+
this.acceptedBaselines.set(uri, file.toCheckpoint)
67+
}
7768
}
7869

7970
/**
8071
* Reject a specific file change
8172
*/
8273
public async rejectChange(uri: string): Promise<void> {
83-
this.rejectedFiles.add(uri)
84-
this.acceptedFiles.delete(uri)
74+
// Remove the file from current changeset - it will be reverted by FCOMessageHandler
75+
// If file is edited again after reversion, it will reappear via updateFCOAfterEdit
76+
this.changeset.files = this.changeset.files.filter((file) => file.uri !== uri)
8577
}
8678

8779
/**
8880
* Accept all file changes
8981
*/
9082
public async acceptAll(): Promise<void> {
9183
this.changeset.files.forEach((file) => {
92-
this.acceptedFiles.add(file.uri)
84+
// Set baseline for each file
85+
this.acceptedBaselines.set(file.uri, file.toCheckpoint)
9386
})
94-
this.rejectedFiles.clear()
9587
}
9688

9789
/**
9890
* Reject all file changes
9991
*/
10092
public async rejectAll(): Promise<void> {
101-
this.changeset.files.forEach((file) => {
102-
this.rejectedFiles.add(file.uri)
103-
})
104-
this.acceptedFiles.clear()
93+
// Clear all files from current changeset - they will be reverted by FCOMessageHandler
94+
// If files are edited again after reversion, they will reappear via updateFCOAfterEdit
95+
this.changeset.files = []
10596
}
10697

10798
/**
@@ -121,10 +112,9 @@ export class FileChangeManager {
121112
// The actual diff calculation should be handled by the checkpoint service
122113
this.changeset.files = []
123114

124-
// Clear accepted/rejected state - baseline change means we're starting fresh
115+
// Clear accepted baselines - baseline change means we're starting fresh
125116
// This happens during checkpoint restore (time travel) where we want a clean slate
126-
this.acceptedFiles.clear()
127-
this.rejectedFiles.clear()
117+
this.acceptedBaselines.clear()
128118
}
129119

130120
/**
@@ -136,11 +126,91 @@ export class FileChangeManager {
136126
}
137127

138128
/**
139-
* Clear accepted/rejected state (called when new checkpoint created)
129+
* Clear accepted baselines (called when new checkpoint created)
130+
*/
131+
public clearFileStates(): void {
132+
this.acceptedBaselines.clear()
133+
}
134+
135+
/**
136+
* Apply per-file baselines to a changeset for incremental diff calculation
137+
* For files that have been accepted, calculate diff from their acceptance point instead of global baseline
140138
*/
141-
public clearAcceptedRejectedState(): void {
142-
this.acceptedFiles.clear()
143-
this.rejectedFiles.clear()
139+
public async applyPerFileBaselines(
140+
baseChanges: FileChange[],
141+
checkpointService: any,
142+
currentCheckpoint: string,
143+
): Promise<FileChange[]> {
144+
const updatedChanges: FileChange[] = []
145+
146+
for (const change of baseChanges) {
147+
// Get accepted baseline for this file (null = use global baseline)
148+
const acceptedBaseline = this.acceptedBaselines.get(change.uri)
149+
150+
if (acceptedBaseline) {
151+
// This file was accepted before - calculate incremental diff from acceptance point
152+
try {
153+
const incrementalChanges = await checkpointService.getDiff({
154+
from: acceptedBaseline,
155+
to: currentCheckpoint,
156+
})
157+
158+
// Find this specific file in the incremental diff
159+
const incrementalChange = incrementalChanges?.find((c: any) => c.paths.relative === change.uri)
160+
161+
if (incrementalChange) {
162+
// Convert to FileChange with per-file baseline
163+
const type = (
164+
incrementalChange.paths.newFile
165+
? "create"
166+
: incrementalChange.paths.deletedFile
167+
? "delete"
168+
: "edit"
169+
) as FileChangeType
170+
171+
let linesAdded = 0
172+
let linesRemoved = 0
173+
174+
if (type === "create") {
175+
linesAdded = incrementalChange.content.after
176+
? incrementalChange.content.after.split("\n").length
177+
: 0
178+
linesRemoved = 0
179+
} else if (type === "delete") {
180+
linesAdded = 0
181+
linesRemoved = incrementalChange.content.before
182+
? incrementalChange.content.before.split("\n").length
183+
: 0
184+
} else {
185+
const lineDifferences = FileChangeManager.calculateLineDifferences(
186+
incrementalChange.content.before || "",
187+
incrementalChange.content.after || "",
188+
)
189+
linesAdded = lineDifferences.linesAdded
190+
linesRemoved = lineDifferences.linesRemoved
191+
}
192+
193+
updatedChanges.push({
194+
uri: change.uri,
195+
type,
196+
fromCheckpoint: acceptedBaseline, // Use per-file baseline
197+
toCheckpoint: currentCheckpoint,
198+
linesAdded,
199+
linesRemoved,
200+
})
201+
}
202+
// If no incremental change found, file hasn't changed since acceptance - don't include it
203+
} catch (error) {
204+
// If we can't calculate incremental diff, fall back to original change
205+
updatedChanges.push(change)
206+
}
207+
} else {
208+
// File was never accepted - use original change
209+
updatedChanges.push(change)
210+
}
211+
}
212+
213+
return updatedChanges
144214
}
145215

146216
/**
@@ -188,8 +258,7 @@ export class FileChangeManager {
188258
*/
189259
public dispose(): void {
190260
this.changeset.files = []
191-
this.acceptedFiles.clear()
192-
this.rejectedFiles.clear()
261+
this.acceptedBaselines.clear()
193262
}
194263
}
195264

0 commit comments

Comments
 (0)