mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-09-05 08:10:14 +00:00
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
This commit is contained in:
Shawn
Hannes Rudolph
parent
21661d7484
commit
11dd48a685
4 changed files with 733 additions and 54 deletions
|
|
@ -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`)
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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<string>
|
||||
private rejectedFiles: Set<string>
|
||||
private acceptedBaselines: Map<string, string> // 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<void> {
|
||||
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<void> {
|
||||
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<void> {
|
||||
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<void> {
|
||||
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<FileChange[]> {
|
||||
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()
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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")
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -72,8 +72,29 @@ export async function updateFCOAfterEdit(task: Task): Promise<void> {
|
|||
}
|
||||
})
|
||||
|
||||
// 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)
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue