From 11dd48a685c64118453a1b0909d5c114bbcc0d84 Mon Sep 17 00:00:00 2001 From: Shawn <5414767+playcations@users.noreply.github.com> Date: Tue, 2 Sep 2025 21:22:41 -0400 Subject: [PATCH] Simplify FileChangeManager architecture with proper accept and reject logic for Files Change Overview MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit • Replace dual Set (acceptedFiles/rejectedFiles) with single Map (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 --- src/core/checkpoints/index.ts | 19 +- .../file-changes/FileChangeManager.ts | 149 +++-- .../__tests__/FileChangeManager.test.ts | 594 +++++++++++++++++- src/services/file-changes/updateAfterEdit.ts | 25 +- 4 files changed, 733 insertions(+), 54 deletions(-) diff --git a/src/core/checkpoints/index.ts b/src/core/checkpoints/index.ts index ed33307d7e..d1700d8f37 100644 --- a/src/core/checkpoints/index.ts +++ b/src/core/checkpoints/index.ts @@ -284,8 +284,19 @@ async function checkGitInstallation( log(`[Task#checkpointCreated] Found ${fileChanges.length} cumulative file changes`) - // Update FileChangeManager with the new files so view diff can find them - checkpointFileChangeManager.setFiles(fileChanges) + // Apply per-file baselines to show only incremental changes for accepted files + const updatedChanges = await checkpointFileChangeManager.applyPerFileBaselines( + fileChanges, + service, + toHash, + ) + + log( + `[Task#checkpointCreated] Applied per-file baselines, ${updatedChanges.length} changes after filtering`, + ) + + // Update FileChangeManager with the per-file baseline changes + checkpointFileChangeManager.setFiles(updatedChanges) // DON'T clear accepted/rejected state here - preserve user's accept/reject decisions // The state should only be cleared on baseline changes (checkpoint restore) or task restart @@ -479,8 +490,8 @@ export async function checkpointRestore( ) // Clear accept/reject state - checkpoint restore is time travel, start with clean slate - if (typeof fileChangeManager.clearAcceptedRejectedState === "function") { - fileChangeManager.clearAcceptedRejectedState() + if (typeof fileChangeManager.clearFileStates === "function") { + fileChangeManager.clearFileStates() provider?.log(`[checkpointRestore] Cleared accept/reject state for fresh start`) } diff --git a/src/services/file-changes/FileChangeManager.ts b/src/services/file-changes/FileChangeManager.ts index 6079599d54..e43f4f95d0 100644 --- a/src/services/file-changes/FileChangeManager.ts +++ b/src/services/file-changes/FileChangeManager.ts @@ -1,4 +1,4 @@ -import { FileChange, FileChangeset } from "@roo-code/types" +import { FileChange, FileChangeset, FileChangeType } from "@roo-code/types" import type { FileContextTracker } from "../../core/context-tracking/FileContextTracker" /** @@ -7,30 +7,21 @@ import type { FileContextTracker } from "../../core/context-tracking/FileContext */ export class FileChangeManager { private changeset: FileChangeset - private acceptedFiles: Set - private rejectedFiles: Set + private acceptedBaselines: Map // uri -> accepted baseline checkpoint constructor(baseCheckpoint: string) { this.changeset = { baseCheckpoint, files: [], } - this.acceptedFiles = new Set() - this.rejectedFiles = new Set() + this.acceptedBaselines = new Map() } /** - * Get current changeset with accepted/rejected files filtered out + * Get current changeset - visibility determined by actual diffs */ public getChanges(): FileChangeset { - const filteredFiles = this.changeset.files.filter( - (file) => !this.acceptedFiles.has(file.uri) && !this.rejectedFiles.has(file.uri), - ) - - return { - ...this.changeset, - files: filteredFiles, - } + return this.changeset } /** @@ -47,13 +38,10 @@ export class FileChangeManager { .map((entry) => entry.path), ) - // Filter changeset to only include LLM-modified files - const filteredFiles = this.changeset.files.filter( - (file) => - llmModifiedFiles.has(file.uri) && - !this.acceptedFiles.has(file.uri) && - !this.rejectedFiles.has(file.uri), - ) + // Filter changeset to only include LLM-modified files that haven't been accepted + const filteredFiles = this.changeset.files.filter((file) => { + return llmModifiedFiles.has(file.uri) && !this.acceptedBaselines.has(file.uri) // Not accepted (no baseline set) + }) return { ...this.changeset, @@ -72,16 +60,20 @@ export class FileChangeManager { * Accept a specific file change */ public async acceptChange(uri: string): Promise { - this.acceptedFiles.add(uri) - this.rejectedFiles.delete(uri) + const file = this.getFileChange(uri) + if (file) { + // Set baseline - file will disappear from FCO naturally (no diff from baseline) + this.acceptedBaselines.set(uri, file.toCheckpoint) + } } /** * Reject a specific file change */ public async rejectChange(uri: string): Promise { - this.rejectedFiles.add(uri) - this.acceptedFiles.delete(uri) + // Remove the file from current changeset - it will be reverted by FCOMessageHandler + // If file is edited again after reversion, it will reappear via updateFCOAfterEdit + this.changeset.files = this.changeset.files.filter((file) => file.uri !== uri) } /** @@ -89,19 +81,18 @@ export class FileChangeManager { */ public async acceptAll(): Promise { this.changeset.files.forEach((file) => { - this.acceptedFiles.add(file.uri) + // Set baseline for each file + this.acceptedBaselines.set(file.uri, file.toCheckpoint) }) - this.rejectedFiles.clear() } /** * Reject all file changes */ public async rejectAll(): Promise { - this.changeset.files.forEach((file) => { - this.rejectedFiles.add(file.uri) - }) - this.acceptedFiles.clear() + // Clear all files from current changeset - they will be reverted by FCOMessageHandler + // If files are edited again after reversion, they will reappear via updateFCOAfterEdit + this.changeset.files = [] } /** @@ -121,10 +112,9 @@ export class FileChangeManager { // The actual diff calculation should be handled by the checkpoint service this.changeset.files = [] - // Clear accepted/rejected state - baseline change means we're starting fresh + // Clear accepted baselines - baseline change means we're starting fresh // This happens during checkpoint restore (time travel) where we want a clean slate - this.acceptedFiles.clear() - this.rejectedFiles.clear() + this.acceptedBaselines.clear() } /** @@ -136,11 +126,91 @@ export class FileChangeManager { } /** - * Clear accepted/rejected state (called when new checkpoint created) + * Clear accepted baselines (called when new checkpoint created) */ - public clearAcceptedRejectedState(): void { - this.acceptedFiles.clear() - this.rejectedFiles.clear() + public clearFileStates(): void { + this.acceptedBaselines.clear() + } + + /** + * Apply per-file baselines to a changeset for incremental diff calculation + * For files that have been accepted, calculate diff from their acceptance point instead of global baseline + */ + public async applyPerFileBaselines( + baseChanges: FileChange[], + checkpointService: any, + currentCheckpoint: string, + ): Promise { + const updatedChanges: FileChange[] = [] + + for (const change of baseChanges) { + // Get accepted baseline for this file (null = use global baseline) + const acceptedBaseline = this.acceptedBaselines.get(change.uri) + + if (acceptedBaseline) { + // This file was accepted before - calculate incremental diff from acceptance point + try { + const incrementalChanges = await checkpointService.getDiff({ + from: acceptedBaseline, + to: currentCheckpoint, + }) + + // Find this specific file in the incremental diff + const incrementalChange = incrementalChanges?.find((c: any) => c.paths.relative === change.uri) + + if (incrementalChange) { + // Convert to FileChange with per-file baseline + const type = ( + incrementalChange.paths.newFile + ? "create" + : incrementalChange.paths.deletedFile + ? "delete" + : "edit" + ) as FileChangeType + + let linesAdded = 0 + let linesRemoved = 0 + + if (type === "create") { + linesAdded = incrementalChange.content.after + ? incrementalChange.content.after.split("\n").length + : 0 + linesRemoved = 0 + } else if (type === "delete") { + linesAdded = 0 + linesRemoved = incrementalChange.content.before + ? incrementalChange.content.before.split("\n").length + : 0 + } else { + const lineDifferences = FileChangeManager.calculateLineDifferences( + incrementalChange.content.before || "", + incrementalChange.content.after || "", + ) + linesAdded = lineDifferences.linesAdded + linesRemoved = lineDifferences.linesRemoved + } + + updatedChanges.push({ + uri: change.uri, + type, + fromCheckpoint: acceptedBaseline, // Use per-file baseline + toCheckpoint: currentCheckpoint, + linesAdded, + linesRemoved, + }) + } + // If no incremental change found, file hasn't changed since acceptance - don't include it + } catch (error) { + // If we can't calculate incremental diff, fall back to original change + updatedChanges.push(change) + } + } else { + // File was never accepted - use original change + updatedChanges.push(change) + } + } + + return updatedChanges } /** @@ -188,8 +258,7 @@ export class FileChangeManager { */ public dispose(): void { this.changeset.files = [] - this.acceptedFiles.clear() - this.rejectedFiles.clear() + this.acceptedBaselines.clear() } } diff --git a/src/services/file-changes/__tests__/FileChangeManager.test.ts b/src/services/file-changes/__tests__/FileChangeManager.test.ts index 314a44a568..34393e9f53 100644 --- a/src/services/file-changes/__tests__/FileChangeManager.test.ts +++ b/src/services/file-changes/__tests__/FileChangeManager.test.ts @@ -3,7 +3,7 @@ import { describe, beforeEach, afterEach, it, expect, vi } from "vitest" import { FileChangeManager } from "../FileChangeManager" -import { FileChange } from "@roo-code/types" +import { FileChange, FileChangeType } from "@roo-code/types" import type { FileContextTracker } from "../../../core/context-tracking/FileContextTracker" import type { TaskMetadata } from "../../../core/context-tracking/FileContextTrackerTypes" @@ -36,7 +36,7 @@ describe("FileChangeManager (Simplified)", () => { expect(changes.files).toEqual([]) }) - it("should filter out accepted files", () => { + it("should filter out rejected files", () => { // Setup some files const testFiles: FileChange[] = [ { @@ -59,8 +59,8 @@ describe("FileChangeManager (Simplified)", () => { fileChangeManager.setFiles(testFiles) - // Accept one file - fileChangeManager.acceptChange("file1.txt") + // Reject one file + fileChangeManager.rejectChange("file1.txt") const changes = fileChangeManager.getChanges() expect(changes.files).toHaveLength(1) @@ -122,7 +122,7 @@ describe("FileChangeManager (Simplified)", () => { }) describe("acceptChange", () => { - it("should mark file as accepted", async () => { + it("should mark file as accepted and store checkpoint", async () => { const testFile: FileChange = { uri: "test.txt", type: "edit", @@ -136,8 +136,13 @@ describe("FileChangeManager (Simplified)", () => { await fileChangeManager.acceptChange("test.txt") + // Accepted files are not filtered out by getChanges anymore const changes = fileChangeManager.getChanges() - expect(changes.files).toHaveLength(0) // File filtered out + expect(changes.files).toHaveLength(1) + + // Check that the accepted baseline was stored correctly + const acceptedBaseline = fileChangeManager["acceptedBaselines"].get("test.txt") + expect(acceptedBaseline).toBe("current") }) it("should remove from rejected if previously rejected", async () => { @@ -154,10 +159,19 @@ describe("FileChangeManager (Simplified)", () => { // First reject, then accept await fileChangeManager.rejectChange("test.txt") + // File should be hidden when rejected + let rejectedChanges = fileChangeManager.getChanges() + expect(rejectedChanges.files).toHaveLength(0) + await fileChangeManager.acceptChange("test.txt") + // File should reappear when accepted (no longer filtered as rejected) const changes = fileChangeManager.getChanges() - expect(changes.files).toHaveLength(0) // File filtered out as accepted + expect(changes.files).toHaveLength(1) + + // Should have correct accepted baseline + const acceptedBaseline = fileChangeManager["acceptedBaselines"].get("test.txt") + expect(acceptedBaseline).toBe("current") }) }) @@ -206,8 +220,15 @@ describe("FileChangeManager (Simplified)", () => { await fileChangeManager.acceptAll() + // Accepted files are not filtered out by getChanges anymore const changes = fileChangeManager.getChanges() - expect(changes.files).toHaveLength(0) // All files filtered out + expect(changes.files).toHaveLength(2) // All files still present + + // Check that both files have their baselines stored correctly + const baseline1 = fileChangeManager["acceptedBaselines"].get("file1.txt") + const baseline2 = fileChangeManager["acceptedBaselines"].get("file2.txt") + expect(baseline1).toBe("current") + expect(baseline2).toBe("current") }) }) @@ -530,4 +551,561 @@ describe("FileChangeManager (Simplified)", () => { expect(llmOnlyChanges.files).toHaveLength(0) }) }) + + describe("Per-File Baseline Behavior", () => { + let mockCheckpointService: any + + beforeEach(() => { + mockCheckpointService = { + getDiff: vi.fn(), + } + }) + + describe("applyPerFileBaselines", () => { + it("should show only incremental changes for accepted files", async () => { + const initialChange: FileChange = { + uri: "test.txt", + type: "edit", + fromCheckpoint: "baseline", + toCheckpoint: "checkpoint1", + linesAdded: 5, + linesRemoved: 2, + } + + // Set initial file and accept it + fileChangeManager.setFiles([initialChange]) + await fileChangeManager.acceptChange("test.txt") + + // Mock incremental diff from acceptance point to new checkpoint + mockCheckpointService.getDiff.mockResolvedValue([ + { + paths: { relative: "test.txt", newFile: false, deletedFile: false }, + content: { before: "line1\nline2", after: "line1\nline2\nline3" }, + }, + ]) + + const baseChanges: FileChange[] = [ + { + uri: "test.txt", + type: "edit", + fromCheckpoint: "baseline", // This would be cumulative + toCheckpoint: "checkpoint2", + linesAdded: 10, // Cumulative + linesRemoved: 3, // Cumulative + }, + ] + + const result = await fileChangeManager.applyPerFileBaselines( + baseChanges, + mockCheckpointService, + "checkpoint2", + ) + + expect(result).toHaveLength(1) + expect(result[0]).toEqual({ + uri: "test.txt", + type: "edit", + fromCheckpoint: "checkpoint1", // Per-file baseline + toCheckpoint: "checkpoint2", + linesAdded: 1, // Only incremental changes + linesRemoved: 0, + }) + + expect(mockCheckpointService.getDiff).toHaveBeenCalledWith({ + from: "checkpoint1", + to: "checkpoint2", + }) + }) + + it("should not show accepted files that haven't changed", async () => { + const initialChange: FileChange = { + uri: "test.txt", + type: "edit", + fromCheckpoint: "baseline", + toCheckpoint: "checkpoint1", + linesAdded: 5, + linesRemoved: 2, + } + + // Set initial file and accept it + fileChangeManager.setFiles([initialChange]) + await fileChangeManager.acceptChange("test.txt") + + // Mock no incremental changes + mockCheckpointService.getDiff.mockResolvedValue([]) + + const baseChanges: FileChange[] = [ + { + uri: "test.txt", + type: "edit", + fromCheckpoint: "baseline", + toCheckpoint: "checkpoint2", + linesAdded: 5, // Same as before - no new changes + linesRemoved: 2, + }, + ] + + const result = await fileChangeManager.applyPerFileBaselines( + baseChanges, + mockCheckpointService, + "checkpoint2", + ) + + // File with no incremental changes shouldn't appear + expect(result).toHaveLength(0) + }) + + it("should use original changes for never-accepted files", async () => { + const baseChanges: FileChange[] = [ + { + uri: "new-file.txt", + type: "create", + fromCheckpoint: "baseline", + toCheckpoint: "checkpoint1", + linesAdded: 10, + linesRemoved: 0, + }, + ] + + const result = await fileChangeManager.applyPerFileBaselines( + baseChanges, + mockCheckpointService, + "checkpoint1", + ) + + // Never-accepted file should use original change + expect(result).toHaveLength(1) + expect(result[0]).toEqual(baseChanges[0]) + + // Should not call getDiff for never-accepted files + expect(mockCheckpointService.getDiff).not.toHaveBeenCalled() + }) + + it("should handle mixed scenario with accepted and new files", async () => { + // Set up an accepted file + const acceptedFile: FileChange = { + uri: "accepted.txt", + type: "edit", + fromCheckpoint: "baseline", + toCheckpoint: "checkpoint1", + linesAdded: 3, + linesRemoved: 1, + } + fileChangeManager.setFiles([acceptedFile]) + await fileChangeManager.acceptChange("accepted.txt") + + // Mock incremental changes for accepted file + mockCheckpointService.getDiff.mockResolvedValue([ + { + paths: { relative: "accepted.txt", newFile: false, deletedFile: false }, + content: { before: "old content", after: "old content\nnew line" }, + }, + ]) + + const baseChanges: FileChange[] = [ + { + uri: "accepted.txt", + type: "edit", + fromCheckpoint: "baseline", + toCheckpoint: "checkpoint2", + linesAdded: 5, // Cumulative + linesRemoved: 2, + }, + { + uri: "new-file.txt", + type: "create", + fromCheckpoint: "baseline", + toCheckpoint: "checkpoint2", + linesAdded: 10, + linesRemoved: 0, + }, + ] + + const result = await fileChangeManager.applyPerFileBaselines( + baseChanges, + mockCheckpointService, + "checkpoint2", + ) + + expect(result).toHaveLength(2) + + // Accepted file should show incremental changes + const acceptedFileResult = result.find((f) => f.uri === "accepted.txt") + expect(acceptedFileResult).toEqual({ + uri: "accepted.txt", + type: "edit", + fromCheckpoint: "checkpoint1", // Per-file baseline + toCheckpoint: "checkpoint2", + linesAdded: 1, // Only incremental + linesRemoved: 0, + }) + + // New file should use original change + const newFileResult = result.find((f) => f.uri === "new-file.txt") + expect(newFileResult).toEqual(baseChanges[1]) + }) + + it("should fall back to original change if incremental diff fails", async () => { + const initialChange: FileChange = { + uri: "test.txt", + type: "edit", + fromCheckpoint: "baseline", + toCheckpoint: "checkpoint1", + linesAdded: 5, + linesRemoved: 2, + } + + fileChangeManager.setFiles([initialChange]) + await fileChangeManager.acceptChange("test.txt") + + // Mock getDiff to throw an error + mockCheckpointService.getDiff.mockRejectedValue(new Error("Checkpoint not found")) + + const baseChanges: FileChange[] = [ + { + uri: "test.txt", + type: "edit", + fromCheckpoint: "baseline", + toCheckpoint: "checkpoint2", + linesAdded: 8, + linesRemoved: 3, + }, + ] + + const result = await fileChangeManager.applyPerFileBaselines( + baseChanges, + mockCheckpointService, + "checkpoint2", + ) + + // Should fall back to original change + expect(result).toHaveLength(1) + expect(result[0]).toEqual(baseChanges[0]) + }) + + it("should handle multiple accept cycles on same file", async () => { + // First change and acceptance + const firstChange: FileChange = { + uri: "test.txt", + type: "edit", + fromCheckpoint: "baseline", + toCheckpoint: "checkpoint1", + linesAdded: 3, + linesRemoved: 1, + } + fileChangeManager.setFiles([firstChange]) + await fileChangeManager.acceptChange("test.txt") + + // Second change and acceptance + const secondChange: FileChange = { + uri: "test.txt", + type: "edit", + fromCheckpoint: "checkpoint1", + toCheckpoint: "checkpoint2", + linesAdded: 2, + linesRemoved: 0, + } + fileChangeManager.setFiles([secondChange]) + await fileChangeManager.acceptChange("test.txt") + + // Third change - should calculate from checkpoint2 + mockCheckpointService.getDiff.mockResolvedValue([ + { + paths: { relative: "test.txt", newFile: false, deletedFile: false }, + content: { before: "content v2", after: "content v3" }, + }, + ]) + + const baseChanges: FileChange[] = [ + { + uri: "test.txt", + type: "edit", + fromCheckpoint: "baseline", // Cumulative from original baseline + toCheckpoint: "checkpoint3", + linesAdded: 10, // Cumulative + linesRemoved: 4, + }, + ] + + const result = await fileChangeManager.applyPerFileBaselines( + baseChanges, + mockCheckpointService, + "checkpoint3", + ) + + expect(result).toHaveLength(1) + expect(result[0]).toEqual({ + uri: "test.txt", + type: "edit", + fromCheckpoint: "checkpoint2", // Latest acceptance point + toCheckpoint: "checkpoint3", + linesAdded: 1, // Only changes since last acceptance + linesRemoved: 1, + }) + + expect(mockCheckpointService.getDiff).toHaveBeenCalledWith({ + from: "checkpoint2", + to: "checkpoint3", + }) + }) + }) + }) + + describe("Rejected Files Behavior", () => { + let mockCheckpointService: any + + beforeEach(() => { + mockCheckpointService = { + getDiff: vi.fn(), + } + }) + + it("should show rejected file again when edited after rejection", async () => { + const initialChange: FileChange = { + uri: "test.txt", + type: "edit", + fromCheckpoint: "baseline", + toCheckpoint: "checkpoint1", + linesAdded: 5, + linesRemoved: 2, + } + + // Set initial file and reject it + fileChangeManager.setFiles([initialChange]) + await fileChangeManager.rejectChange("test.txt") + + // File should be hidden after rejection + let changes = fileChangeManager.getChanges() + expect(changes.files).toHaveLength(0) + + // File is edited again with new changes + const newChange: FileChange = { + uri: "test.txt", + type: "edit", + fromCheckpoint: "baseline", + toCheckpoint: "checkpoint2", // Different checkpoint = file changed + linesAdded: 8, + linesRemoved: 3, + } + + const result = await fileChangeManager.applyPerFileBaselines( + [newChange], + mockCheckpointService, + "checkpoint2", + ) + + // Should reappear with cumulative changes from global baseline + expect(result).toHaveLength(1) + expect(result[0]).toEqual({ + uri: "test.txt", + type: "edit", + fromCheckpoint: "baseline", // Global baseline + toCheckpoint: "checkpoint2", + linesAdded: 8, + linesRemoved: 3, + }) + }) + + it("should preserve accepted baseline through rejection", async () => { + // First accept a file + const acceptedChange: FileChange = { + uri: "test.txt", + type: "edit", + fromCheckpoint: "baseline", + toCheckpoint: "checkpoint1", + linesAdded: 3, + linesRemoved: 1, + } + fileChangeManager.setFiles([acceptedChange]) + await fileChangeManager.acceptChange("test.txt") + + // Then reject the same file (simulating new changes that user rejects) + const rejectedChange: FileChange = { + uri: "test.txt", + type: "edit", + fromCheckpoint: "checkpoint1", + toCheckpoint: "checkpoint2", + linesAdded: 2, + linesRemoved: 0, + } + fileChangeManager.setFiles([rejectedChange]) + await fileChangeManager.rejectChange("test.txt") + + // File should be hidden after rejection + let changes = fileChangeManager.getChanges() + expect(changes.files).toHaveLength(0) + + // File is edited again after rejection + mockCheckpointService.getDiff.mockResolvedValue([ + { + paths: { relative: "test.txt", newFile: false, deletedFile: false }, + content: { before: "accepted content", after: "accepted content\nnew line" }, + }, + ]) + + const newChange: FileChange = { + uri: "test.txt", + type: "edit", + fromCheckpoint: "baseline", + toCheckpoint: "checkpoint3", + linesAdded: 10, // Cumulative from baseline + linesRemoved: 4, + } + + const result = await fileChangeManager.applyPerFileBaselines( + [newChange], + mockCheckpointService, + "checkpoint3", + ) + + // Should show incremental changes from accepted baseline, not global baseline + expect(result).toHaveLength(1) + expect(result[0]).toEqual({ + uri: "test.txt", + type: "edit", + fromCheckpoint: "checkpoint1", // Preserved accepted baseline + toCheckpoint: "checkpoint3", + linesAdded: 1, // Only incremental since acceptance + linesRemoved: 0, + }) + + expect(mockCheckpointService.getDiff).toHaveBeenCalledWith({ + from: "checkpoint1", // Uses accepted baseline + to: "checkpoint3", + }) + }) + + it("should keep rejected file hidden if no changes since rejection", async () => { + const initialChange: FileChange = { + uri: "test.txt", + type: "edit", + fromCheckpoint: "baseline", + toCheckpoint: "checkpoint1", + linesAdded: 5, + linesRemoved: 2, + } + + fileChangeManager.setFiles([initialChange]) + await fileChangeManager.rejectChange("test.txt") + + // Same change (no new edits since rejection) + const sameChange: FileChange = { + uri: "test.txt", + type: "edit", + fromCheckpoint: "baseline", + toCheckpoint: "checkpoint1", // Same checkpoint = no changes + linesAdded: 5, + linesRemoved: 2, + } + + const result = await fileChangeManager.applyPerFileBaselines( + [sameChange], + mockCheckpointService, + "checkpoint1", + ) + + // Should remain hidden (not in results) + expect(result).toHaveLength(0) + }) + + it("should handle rejectAll properly", async () => { + const testFiles: FileChange[] = [ + { + uri: "file1.txt", + type: "edit", + fromCheckpoint: "baseline", + toCheckpoint: "checkpoint1", + linesAdded: 3, + linesRemoved: 1, + }, + { + uri: "file2.txt", + type: "create", + fromCheckpoint: "baseline", + toCheckpoint: "checkpoint1", + linesAdded: 10, + linesRemoved: 0, + }, + ] + + fileChangeManager.setFiles(testFiles) + await fileChangeManager.rejectAll() + + // All files should be hidden + let changes = fileChangeManager.getChanges() + expect(changes.files).toHaveLength(0) + + // Edit one file + const newChanges: FileChange[] = [ + { + uri: "file1.txt", + type: "edit", + fromCheckpoint: "baseline", + toCheckpoint: "checkpoint2", // Changed + linesAdded: 5, + linesRemoved: 2, + }, + { + uri: "file2.txt", + type: "create", + fromCheckpoint: "baseline", + toCheckpoint: "checkpoint1", // Same - no changes + linesAdded: 10, + linesRemoved: 0, + }, + ] + + const result = await fileChangeManager.applyPerFileBaselines( + newChanges, + mockCheckpointService, + "checkpoint2", + ) + + // Only the changed file should reappear + expect(result).toHaveLength(1) + expect(result[0].uri).toBe("file1.txt") + }) + + it("should handle accept then reject then accept again", async () => { + // First acceptance + const firstChange: FileChange = { + uri: "test.txt", + type: "edit", + fromCheckpoint: "baseline", + toCheckpoint: "checkpoint1", + linesAdded: 3, + linesRemoved: 1, + } + fileChangeManager.setFiles([firstChange]) + await fileChangeManager.acceptChange("test.txt") + + // Rejection (but baseline should be preserved) + const rejectedChange: FileChange = { + uri: "test.txt", + type: "edit", + fromCheckpoint: "checkpoint1", + toCheckpoint: "checkpoint2", + linesAdded: 2, + linesRemoved: 0, + } + fileChangeManager.setFiles([rejectedChange]) + await fileChangeManager.rejectChange("test.txt") + + // Accept again after new edits + const newChange: FileChange = { + uri: "test.txt", + type: "edit", + fromCheckpoint: "checkpoint1", // Should still use original accepted baseline + toCheckpoint: "checkpoint3", + linesAdded: 4, + linesRemoved: 1, + } + fileChangeManager.setFiles([newChange]) + await fileChangeManager.acceptChange("test.txt") + + // The accepted baseline should be updated + const acceptedBaseline = fileChangeManager["acceptedBaselines"].get("test.txt") + expect(acceptedBaseline).toBe("checkpoint3") + }) + }) }) diff --git a/src/services/file-changes/updateAfterEdit.ts b/src/services/file-changes/updateAfterEdit.ts index 168c49afd8..20ac39c8d1 100644 --- a/src/services/file-changes/updateAfterEdit.ts +++ b/src/services/file-changes/updateAfterEdit.ts @@ -72,8 +72,29 @@ export async function updateFCOAfterEdit(task: Task): Promise { } }) - // Update FileChangeManager with the new files - fileChangeManager.setFiles(fileChanges) + // Apply per-file baselines to show only incremental changes for accepted files + const updatedChanges = await fileChangeManager.applyPerFileBaselines( + fileChanges, + checkpointService, + "HEAD", // Current working directory state + ) + + // Get existing files and merge with new changes (maintaining existing files) + const existingFiles = fileChangeManager.getChanges().files + const updatedFiles = [...existingFiles] + + // Update or add new files with per-file baseline changes + updatedChanges.forEach((newChange) => { + const existingIndex = updatedFiles.findIndex((existing) => existing.uri === newChange.uri) + if (existingIndex >= 0) { + updatedFiles[existingIndex] = newChange // Update existing + } else { + updatedFiles.push(newChange) // Add new + } + }) + + // Update FileChangeManager with merged files + fileChangeManager.setFiles(updatedFiles) // Get LLM-only changes for the webview (filters out accepted/rejected files) const filteredChangeset = await fileChangeManager.getLLMOnlyChanges(task.taskId, task.fileContextTracker)