chore(tools/edit): various fixes (#1921)

Christian Rocha created 2 weeks ago

Change summary

internal/agent/tools/edit.go | 15 +++------------
internal/history/file.go     |  7 +++++++
2 files changed, 10 insertions(+), 12 deletions(-)

Detailed changes

internal/agent/tools/edit.go

@@ -216,12 +216,10 @@ func deleteContent(edit editContext, filePath, oldString string, replaceAll bool
 	oldContent, isCrlf := fsext.ToUnixLineEndings(string(content))
 
 	var newContent string
-	var deletionCount int
 
 	if replaceAll {
 		newContent = strings.ReplaceAll(oldContent, oldString, "")
-		deletionCount = strings.Count(oldContent, oldString)
-		if deletionCount == 0 {
+		if newContent == oldContent {
 			return oldStringNotFoundErr, nil
 		}
 	} else {
@@ -236,13 +234,12 @@ func deleteContent(edit editContext, filePath, oldString string, replaceAll bool
 		}
 
 		newContent = oldContent[:index] + oldContent[index+len(oldString):]
-		deletionCount = 1
 	}
 
 	sessionID := GetSessionFromContext(edit.ctx)
 
 	if sessionID == "" {
-		return fantasy.ToolResponse{}, fmt.Errorf("session ID is required for creating a new file")
+		return fantasy.ToolResponse{}, fmt.Errorf("session ID is required for deleting content")
 	}
 
 	_, additions, removals := diff.GenerateDiff(
@@ -299,7 +296,7 @@ func deleteContent(edit editContext, filePath, oldString string, replaceAll bool
 		}
 	}
 	// Store the new version
-	_, err = edit.files.CreateVersion(edit.ctx, sessionID, filePath, "")
+	_, err = edit.files.CreateVersion(edit.ctx, sessionID, filePath, newContent)
 	if err != nil {
 		slog.Error("Error creating file history version", "error", err)
 	}
@@ -352,14 +349,9 @@ func replaceContent(edit editContext, filePath, oldString, newString string, rep
 	oldContent, isCrlf := fsext.ToUnixLineEndings(string(content))
 
 	var newContent string
-	var replacementCount int
 
 	if replaceAll {
 		newContent = strings.ReplaceAll(oldContent, oldString, newString)
-		replacementCount = strings.Count(oldContent, oldString)
-		if replacementCount == 0 {
-			return oldStringNotFoundErr, nil
-		}
 	} else {
 		index := strings.Index(oldContent, oldString)
 		if index == -1 {
@@ -372,7 +364,6 @@ func replaceContent(edit editContext, filePath, oldString, newString string, rep
 		}
 
 		newContent = oldContent[:index] + newString + oldContent[index+len(oldString):]
-		replacementCount = 1
 	}
 
 	if oldContent == newContent {

internal/history/file.go

@@ -25,10 +25,14 @@ type File struct {
 	UpdatedAt int64
 }
 
+// Service manages file versions and history for sessions.
 type Service interface {
 	pubsub.Subscriber[File]
 	Create(ctx context.Context, sessionID, path, content string) (File, error)
+
+	// CreateVersion creates a new version of a file.
 	CreateVersion(ctx context.Context, sessionID, path, content string) (File, error)
+
 	Get(ctx context.Context, id string) (File, error)
 	GetByPathAndSession(ctx context.Context, path, sessionID string) (File, error)
 	ListBySession(ctx context.Context, sessionID string) ([]File, error)
@@ -55,6 +59,9 @@ func (s *service) Create(ctx context.Context, sessionID, path, content string) (
 	return s.createWithVersion(ctx, sessionID, path, content, InitialVersion)
 }
 
+// CreateVersion creates a new version of a file with auto-incremented version
+// number. If no previous versions exist for the path, it creates the initial
+// version. The provided content is stored as the new version.
 func (s *service) CreateVersion(ctx context.Context, sessionID, path, content string) (File, error) {
 	// Get the latest version for this path
 	files, err := s.q.ListFilesByPath(ctx, path)