mirror of
https://github.com/mark3labs/kit.git
synced 2026-06-13 19:20:06 +00:00
refactor(core): remove redundant single-edit mode from edit tool
- Remove top-level old_text/new_text params from edit tool schema - Make edits array the sole interface; single edits pass 1-item array - Simplify normalizeEditInput, removing dual-mode branching logic - Update UI renderer to only read from edits array - Remove old_text/new_text from bodyKeys in message summarizer - Update web session HTML to iterate edits array - Convert all single-edit tests to use Edits array - Replace mixed-mode test with empty-array validation test
This commit is contained in:
+6
-42
@@ -21,12 +21,9 @@ type Edit struct {
|
||||
}
|
||||
|
||||
// editArgs holds the arguments for the edit tool.
|
||||
// Supports both single-edit mode (old_text/new_text) and multi-edit mode (edits array).
|
||||
type editArgs struct {
|
||||
Path string `json:"path"`
|
||||
OldText string `json:"old_text"` // Single-edit mode
|
||||
NewText string `json:"new_text"` // Single-edit mode
|
||||
Edits []Edit `json:"edits"` // Multi-edit mode
|
||||
Path string `json:"path"`
|
||||
Edits []Edit `json:"edits"`
|
||||
}
|
||||
|
||||
// replacement represents a normalized edit ready for processing.
|
||||
@@ -52,20 +49,12 @@ func NewEditTool(opts ...ToolOption) fantasy.AgentTool {
|
||||
return &coreTool{
|
||||
info: fantasy.ToolInfo{
|
||||
Name: "edit",
|
||||
Description: "Edit a file by replacing exact text. Supports single edit via old_text/new_text, or multiple edits via the edits array. All edits in the array are matched against the original file content (non-incremental) and must be non-overlapping.",
|
||||
Description: "Edit a file by replacing exact text. All edits in the array are matched against the original file content (non-incremental) and must be non-overlapping.",
|
||||
Parameters: map[string]any{
|
||||
"path": map[string]any{
|
||||
"type": "string",
|
||||
"description": "Path to the file to edit (relative or absolute)",
|
||||
},
|
||||
"old_text": map[string]any{
|
||||
"type": "string",
|
||||
"description": "Exact text to find and replace (single-edit mode). Must not be used with 'edits' array.",
|
||||
},
|
||||
"new_text": map[string]any{
|
||||
"type": "string",
|
||||
"description": "New text to replace the old text with (single-edit mode). Must not be used with 'edits' array.",
|
||||
},
|
||||
"edits": map[string]any{
|
||||
"type": "array",
|
||||
"description": "Array of edits for multi-region replacement. Each edit must have unique, non-overlapping old_text. All matches are against the original file content.",
|
||||
@@ -85,7 +74,7 @@ func NewEditTool(opts ...ToolOption) fantasy.AgentTool {
|
||||
},
|
||||
},
|
||||
},
|
||||
Required: []string{"path"},
|
||||
Required: []string{"path", "edits"},
|
||||
},
|
||||
handler: func(ctx context.Context, call fantasy.ToolCall) (fantasy.ToolResponse, error) {
|
||||
return executeEdit(ctx, call, cfg.WorkDir)
|
||||
@@ -163,36 +152,11 @@ func executeEdit(ctx context.Context, call fantasy.ToolCall, workDir string) (fa
|
||||
}
|
||||
|
||||
// normalizeEditInput validates and normalizes the edit input.
|
||||
// Returns error if both single-edit and multi-edit modes are used.
|
||||
func normalizeEditInput(args editArgs) ([]replacement, error) {
|
||||
singleMode := args.OldText != "" || args.NewText != ""
|
||||
multiMode := len(args.Edits) > 0
|
||||
|
||||
if singleMode && multiMode {
|
||||
return nil, fmt.Errorf("cannot use old_text/new_text together with edits array")
|
||||
if len(args.Edits) == 0 {
|
||||
return nil, fmt.Errorf("edits array is required and must not be empty")
|
||||
}
|
||||
|
||||
if !singleMode && !multiMode {
|
||||
return nil, fmt.Errorf("must provide either old_text/new_text or edits array")
|
||||
}
|
||||
|
||||
if singleMode {
|
||||
if args.OldText == "" {
|
||||
return nil, fmt.Errorf("old_text is required when using single-edit mode")
|
||||
}
|
||||
if args.NewText == "" {
|
||||
return nil, fmt.Errorf("new_text is required when using single-edit mode")
|
||||
}
|
||||
return []replacement{{
|
||||
oldText: strings.ReplaceAll(args.OldText, "\r\n", "\n"),
|
||||
newText: strings.ReplaceAll(args.NewText, "\r\n", "\n"),
|
||||
originalOld: args.OldText,
|
||||
originalNew: args.NewText,
|
||||
index: 0,
|
||||
}}, nil
|
||||
}
|
||||
|
||||
// Multi-edit mode
|
||||
var reps []replacement
|
||||
for i, edit := range args.Edits {
|
||||
if edit.OldText == "" {
|
||||
|
||||
+62
-44
@@ -389,9 +389,11 @@ func TestExecuteEdit_ExactMatch(t *testing.T) {
|
||||
writeFileOrFail(t, path, original)
|
||||
|
||||
input, _ := json.Marshal(editArgs{
|
||||
Path: path,
|
||||
OldText: "fmt.Println(\"hello\")",
|
||||
NewText: "fmt.Println(\"world\")",
|
||||
Path: path,
|
||||
Edits: []Edit{{
|
||||
OldText: "fmt.Println(\"hello\")",
|
||||
NewText: "fmt.Println(\"world\")",
|
||||
}},
|
||||
})
|
||||
|
||||
resp, err := executeEdit(t.Context(), fantasy.ToolCall{Input: string(input)}, dir)
|
||||
@@ -426,9 +428,11 @@ func TestExecuteEdit_ExactMatch_DoesNotCorruptRest(t *testing.T) {
|
||||
target := lines[49]
|
||||
replacement := "REPLACED_LINE_50"
|
||||
input, _ := json.Marshal(editArgs{
|
||||
Path: path,
|
||||
OldText: target,
|
||||
NewText: replacement,
|
||||
Path: path,
|
||||
Edits: []Edit{{
|
||||
OldText: target,
|
||||
NewText: replacement,
|
||||
}},
|
||||
})
|
||||
|
||||
resp, err := executeEdit(t.Context(), fantasy.ToolCall{Input: string(input)}, dir)
|
||||
@@ -470,9 +474,11 @@ func TestExecuteEdit_FuzzyMatch_TrailingWhitespace(t *testing.T) {
|
||||
|
||||
// Search without trailing whitespace (common LLM behavior)
|
||||
input, _ := json.Marshal(editArgs{
|
||||
Path: path,
|
||||
OldText: "func foo() {\n\treturn 1\n}",
|
||||
NewText: "func foo() {\n\treturn 2\n}",
|
||||
Path: path,
|
||||
Edits: []Edit{{
|
||||
OldText: "func foo() {\n\treturn 1\n}",
|
||||
NewText: "func foo() {\n\treturn 2\n}",
|
||||
}},
|
||||
})
|
||||
|
||||
resp, err := executeEdit(t.Context(), fantasy.ToolCall{Input: string(input)}, dir)
|
||||
@@ -519,9 +525,11 @@ func TestExecuteEdit_FuzzyMatch_DoesNotCorruptRest(t *testing.T) {
|
||||
search := strings.Repeat("x", 10) + "\n" + strings.Repeat("x", 10)
|
||||
// But this matches lines 1-2, 2-3, etc. — should fail due to ambiguity.
|
||||
input, _ := json.Marshal(editArgs{
|
||||
Path: path,
|
||||
OldText: search,
|
||||
NewText: "REPLACED",
|
||||
Path: path,
|
||||
Edits: []Edit{{
|
||||
OldText: search,
|
||||
NewText: "REPLACED",
|
||||
}},
|
||||
})
|
||||
|
||||
resp, err := executeEdit(t.Context(), fantasy.ToolCall{Input: string(input)}, dir)
|
||||
@@ -546,9 +554,11 @@ func TestExecuteEdit_MultipleMatches_Fails(t *testing.T) {
|
||||
writeFileOrFail(t, path, "hello\nworld\nhello\n")
|
||||
|
||||
input, _ := json.Marshal(editArgs{
|
||||
Path: path,
|
||||
OldText: "hello",
|
||||
NewText: "goodbye",
|
||||
Path: path,
|
||||
Edits: []Edit{{
|
||||
OldText: "hello",
|
||||
NewText: "goodbye",
|
||||
}},
|
||||
})
|
||||
|
||||
resp, err := executeEdit(t.Context(), fantasy.ToolCall{Input: string(input)}, dir)
|
||||
@@ -575,9 +585,11 @@ func TestExecuteEdit_NoMatch_Fails(t *testing.T) {
|
||||
writeFileOrFail(t, path, "hello world\n")
|
||||
|
||||
input, _ := json.Marshal(editArgs{
|
||||
Path: path,
|
||||
OldText: "nonexistent text",
|
||||
NewText: "replacement",
|
||||
Path: path,
|
||||
Edits: []Edit{{
|
||||
OldText: "nonexistent text",
|
||||
NewText: "replacement",
|
||||
}},
|
||||
})
|
||||
|
||||
resp, err := executeEdit(t.Context(), fantasy.ToolCall{Input: string(input)}, dir)
|
||||
@@ -601,9 +613,11 @@ func TestExecuteEdit_CRLFNormalization(t *testing.T) {
|
||||
writeFileOrFail(t, path, "line1\r\nline2\r\nline3\r\n")
|
||||
|
||||
input, _ := json.Marshal(editArgs{
|
||||
Path: path,
|
||||
OldText: "line2",
|
||||
NewText: "LINE2",
|
||||
Path: path,
|
||||
Edits: []Edit{{
|
||||
OldText: "line2",
|
||||
NewText: "LINE2",
|
||||
}},
|
||||
})
|
||||
|
||||
resp, err := executeEdit(t.Context(), fantasy.ToolCall{Input: string(input)}, dir)
|
||||
@@ -622,8 +636,10 @@ func TestExecuteEdit_CRLFNormalization(t *testing.T) {
|
||||
|
||||
func TestExecuteEdit_MissingPath(t *testing.T) {
|
||||
input, _ := json.Marshal(editArgs{
|
||||
OldText: "x",
|
||||
NewText: "y",
|
||||
Edits: []Edit{{
|
||||
OldText: "x",
|
||||
NewText: "y",
|
||||
}},
|
||||
})
|
||||
resp, err := executeEdit(t.Context(), fantasy.ToolCall{Input: string(input)}, "")
|
||||
if err != nil {
|
||||
@@ -636,9 +652,11 @@ func TestExecuteEdit_MissingPath(t *testing.T) {
|
||||
|
||||
func TestExecuteEdit_NonexistentFile(t *testing.T) {
|
||||
input, _ := json.Marshal(editArgs{
|
||||
Path: "/tmp/nonexistent_edit_test_file_12345.go",
|
||||
OldText: "x",
|
||||
NewText: "y",
|
||||
Path: "/tmp/nonexistent_edit_test_file_12345.go",
|
||||
Edits: []Edit{{
|
||||
OldText: "x",
|
||||
NewText: "y",
|
||||
}},
|
||||
})
|
||||
resp, err := executeEdit(t.Context(), fantasy.ToolCall{Input: string(input)}, "")
|
||||
if err != nil {
|
||||
@@ -661,9 +679,11 @@ func TestExecuteEdit_DiffContainsHunkHeader(t *testing.T) {
|
||||
writeFileOrFail(t, path, strings.Join(lines, "\n")+"\n")
|
||||
|
||||
input, _ := json.Marshal(editArgs{
|
||||
Path: path,
|
||||
OldText: "line_10_content",
|
||||
NewText: "REPLACED",
|
||||
Path: path,
|
||||
Edits: []Edit{{
|
||||
OldText: "line_10_content",
|
||||
NewText: "REPLACED",
|
||||
}},
|
||||
})
|
||||
|
||||
resp, err := executeEdit(t.Context(), fantasy.ToolCall{Input: string(input)}, dir)
|
||||
@@ -684,9 +704,11 @@ func TestExecuteEdit_MetadataContainsFileDiffs(t *testing.T) {
|
||||
writeFileOrFail(t, path, "old content\n")
|
||||
|
||||
input, _ := json.Marshal(editArgs{
|
||||
Path: path,
|
||||
OldText: "old content",
|
||||
NewText: "new content",
|
||||
Path: path,
|
||||
Edits: []Edit{{
|
||||
OldText: "old content",
|
||||
NewText: "new content",
|
||||
}},
|
||||
})
|
||||
|
||||
resp, err := executeEdit(t.Context(), fantasy.ToolCall{Input: string(input)}, dir)
|
||||
@@ -905,18 +927,14 @@ func TestExecuteEdit_MultiEdit_EmptyArray(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestExecuteEdit_MultiEdit_MixedWithSingleMode(t *testing.T) {
|
||||
func TestExecuteEdit_EmptyEditsArray_Fails(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
path := filepath.Join(dir, "mixed.txt")
|
||||
path := filepath.Join(dir, "empty.txt")
|
||||
writeFileOrFail(t, path, "hello\n")
|
||||
|
||||
input, _ := json.Marshal(map[string]any{
|
||||
"path": path,
|
||||
"old_text": "hello",
|
||||
"new_text": "HELLO",
|
||||
"edits": []Edit{
|
||||
{OldText: "hello", NewText: "HI"},
|
||||
},
|
||||
input, _ := json.Marshal(editArgs{
|
||||
Path: path,
|
||||
Edits: []Edit{},
|
||||
})
|
||||
|
||||
resp, err := executeEdit(t.Context(), fantasy.ToolCall{Input: string(input)}, dir)
|
||||
@@ -924,10 +942,10 @@ func TestExecuteEdit_MultiEdit_MixedWithSingleMode(t *testing.T) {
|
||||
t.Fatalf("executeEdit error: %v", err)
|
||||
}
|
||||
if !resp.IsError {
|
||||
t.Error("expected error when mixing single and multi-edit modes")
|
||||
t.Error("expected error for empty edits array")
|
||||
}
|
||||
if !strings.Contains(resp.Content, "cannot use") {
|
||||
t.Errorf("expected 'cannot use' in error, got: %s", resp.Content)
|
||||
if !strings.Contains(resp.Content, "required") {
|
||||
t.Errorf("expected 'required' in error, got: %s", resp.Content)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -88,13 +88,9 @@ func formatToolParams(toolArgs string, maxWidth int) string {
|
||||
}
|
||||
|
||||
bodyKeys := map[string]bool{
|
||||
"content": true,
|
||||
"old_text": true,
|
||||
"new_text": true,
|
||||
"oldText": true,
|
||||
"newText": true,
|
||||
"edits": true,
|
||||
"todos": true,
|
||||
"content": true,
|
||||
"edits": true,
|
||||
"todos": true,
|
||||
}
|
||||
var remaining []string
|
||||
for key, val := range params {
|
||||
|
||||
@@ -79,8 +79,7 @@ func renderToolBody(toolName, toolArgs, toolResult string, width int) string {
|
||||
// Edit tool — side-by-side diff
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
// renderEditBody renders a side-by-side diff from old_text/new_text in toolArgs.
|
||||
// Supports both single-edit mode and multi-edit mode (edits array).
|
||||
// renderEditBody renders a side-by-side diff from the edits array in toolArgs.
|
||||
func renderEditBody(toolArgs, toolResult string, width int) string {
|
||||
var args map[string]any
|
||||
if err := json.Unmarshal([]byte(toolArgs), &args); err != nil {
|
||||
@@ -90,35 +89,28 @@ func renderEditBody(toolArgs, toolResult string, width int) string {
|
||||
// Try to extract the starting line number from the unified diff in the result
|
||||
startLine := extractDiffStartLine(toolResult)
|
||||
|
||||
// Check for multi-edit mode (edits array)
|
||||
if editsArr, ok := args["edits"].([]any); ok && len(editsArr) > 0 {
|
||||
var results []string
|
||||
for _, edit := range editsArr {
|
||||
if e, ok := edit.(map[string]any); ok {
|
||||
oldText, _ := e["old_text"].(string)
|
||||
newText, _ := e["new_text"].(string)
|
||||
if oldText != "" || newText != "" {
|
||||
diff := renderDiffBlock(oldText, newText, startLine, width)
|
||||
if diff != "" {
|
||||
results = append(results, diff)
|
||||
}
|
||||
editsArr, ok := args["edits"].([]any)
|
||||
if !ok || len(editsArr) == 0 {
|
||||
return ""
|
||||
}
|
||||
|
||||
var results []string
|
||||
for _, edit := range editsArr {
|
||||
if e, ok := edit.(map[string]any); ok {
|
||||
oldText, _ := e["old_text"].(string)
|
||||
newText, _ := e["new_text"].(string)
|
||||
if oldText != "" || newText != "" {
|
||||
diff := renderDiffBlock(oldText, newText, startLine, width)
|
||||
if diff != "" {
|
||||
results = append(results, diff)
|
||||
}
|
||||
}
|
||||
}
|
||||
if len(results) > 0 {
|
||||
return strings.Join(results, "\n")
|
||||
}
|
||||
return ""
|
||||
}
|
||||
|
||||
// Single-edit mode (legacy)
|
||||
oldText, _ := args["old_text"].(string)
|
||||
newText, _ := args["new_text"].(string)
|
||||
if oldText == "" && newText == "" {
|
||||
return ""
|
||||
if len(results) > 0 {
|
||||
return strings.Join(results, "\n")
|
||||
}
|
||||
|
||||
return renderDiffBlock(oldText, newText, startLine, width)
|
||||
return ""
|
||||
}
|
||||
|
||||
// extractDiffStartLine parses the first @@ hunk header from a unified diff
|
||||
|
||||
@@ -1968,35 +1968,42 @@ a:hover { text-decoration: underline; }
|
||||
|
||||
function renderEditBody(input, result) {
|
||||
let html = '';
|
||||
const oldText = input.old_text || '';
|
||||
const newText = input.new_text || '';
|
||||
const edits = input.edits || [];
|
||||
|
||||
html += '<div class="tool-input">';
|
||||
html += '<div class="tool-input-label">Changes</div>';
|
||||
html += '<div class="diff-display">';
|
||||
|
||||
// Render old text lines as removed
|
||||
if (oldText) {
|
||||
const oldLines = oldText.split('\n');
|
||||
for (const line of oldLines) {
|
||||
html += `<div class="diff-line removed">- ${escapeHtml(line)}</div>`;
|
||||
for (const edit of edits) {
|
||||
const oldText = edit.old_text || '';
|
||||
const newText = edit.new_text || '';
|
||||
|
||||
html += '<div class="diff-display">';
|
||||
|
||||
// Render old text lines as removed
|
||||
if (oldText) {
|
||||
const oldLines = oldText.split('\n');
|
||||
for (const line of oldLines) {
|
||||
html += `<div class="diff-line removed">- ${escapeHtml(line)}</div>`;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Separator
|
||||
if (oldText && newText) {
|
||||
html += '<div class="diff-separator">───</div>';
|
||||
}
|
||||
|
||||
// Render new text lines as added
|
||||
if (newText) {
|
||||
const newLines = newText.split('\n');
|
||||
for (const line of newLines) {
|
||||
html += `<div class="diff-line added">+ ${escapeHtml(line)}</div>`;
|
||||
// Separator
|
||||
if (oldText && newText) {
|
||||
html += '<div class="diff-separator">───</div>';
|
||||
}
|
||||
|
||||
// Render new text lines as added
|
||||
if (newText) {
|
||||
const newLines = newText.split('\n');
|
||||
for (const line of newLines) {
|
||||
html += `<div class="diff-line added">+ ${escapeHtml(line)}</div>`;
|
||||
}
|
||||
}
|
||||
|
||||
html += '</div>';
|
||||
}
|
||||
|
||||
html += '</div></div>';
|
||||
html += '</div>';
|
||||
|
||||
if (result && result.is_error) {
|
||||
html += renderToolOutput(result.content, true);
|
||||
|
||||
Reference in New Issue
Block a user