fix: guide edit calls to use target file content

commit: 4463630d7459838337a5f1a71e0d9142a113cdf0
author: patriceckhart · 3mo ago · 2026-07-11 07:12 UTC
parent: 93967bc
signature: unsigned

3 files · +40 -104

M packages/agent/systemprompt.go +1 -1


@@ -86,4 +86,4 @@ const defaultIdentity = `You are an expert coding assistant operating inside zot
86	86
87	87		Your output renders in a TUI that understands markdown for prose and plain text for tool output. Use markdown freely, keep answers concise, and let tool calls speak for themselves rather than narrating them in prose before you invoke them. Act first, then summarise what you did.
88	88
89		-	When changing file contents, prefer the edit tool for in-place changes and the write tool for creating or fully replacing files. Do not use bash with cat/echo/sed/tee redirections to mutate files; those changes render as opaque shell output while edit renders as a readable diff.`
	89	+	For focused changes to an existing file, inspect its current contents and use edit with verbatim oldText taken from that same file. Include only enough context to make each match unambiguous. Use write when creating a file or replacing it wholesale. Do not mutate files through bash redirections or commands such as cat, echo, sed, or tee, because those changes appear as opaque shell output instead of a readable edit diff.`


@@ -86,4 +86,4 @@ const defaultIdentity = `You are an expert coding assistant operating inside zot
86			86
87		Your output renders in a TUI that understands markdown for prose and plain text for tool output. Use markdown freely, keep answers concise, and let tool calls speak for themselves rather than narrating them in prose before you invoke them. Act first, then summarise what you did.	87		Your output renders in a TUI that understands markdown for prose and plain text for tool output. Use markdown freely, keep answers concise, and let tool calls speak for themselves rather than narrating them in prose before you invoke them. Act first, then summarise what you did.
88			88
89	-	When changing file contents, prefer the edit tool for in-place changes and the write tool for creating or fully replacing files. Do not use bash with cat/echo/sed/tee redirections to mutate files; those changes render as opaque shell output while edit renders as a readable diff.`	89	+	For focused changes to an existing file, inspect its current contents and use edit with verbatim oldText taken from that same file. Include only enough context to make each match unambiguous. Use write when creating a file or replacing it wholesale. Do not mutate files through bash redirections or commands such as cat, echo, sed, or tee, because those changes appear as opaque shell output instead of a readable edit diff.`

M packages/agent/tools/edit.go +16 -27


@@ -28,11 +28,11 @@ type editArgs struct {
28	28		Edits []editOp `json:"edits"`
29	29		}
30	30
31		-	const editSchema = `{"type":"object","properties":{"path":{"type":"string"},"edits":{"type":"array","items":{"type":"object","properties":{"oldText":{"type":"string"},"newText":{"type":"string"}},"required":["oldText","newText"]}}},"required":["path","edits"]}`
	31	+	const editSchema = `{"type":"object","properties":{"path":{"type":"string","description":"File to modify, given as an absolute path or one relative to the working directory."},"edits":{"type":"array","description":"A set of non-overlapping substitutions, all located in the file content as it existed before this call.","items":{"type":"object","properties":{"oldText":{"type":"string","description":"A verbatim excerpt from the file being modified. Include enough surrounding text to identify exactly one location, without intersecting another edit."},"newText":{"type":"string","description":"Content that will replace the matched excerpt."}},"required":["oldText","newText"]}}},"required":["path","edits"]}`
32	32
33	33		func (t *EditTool) Name() string { return "edit" }
34	34		func (t *EditTool) Description() string {
35		-	return "Edit a file via exact-match replacements. Each oldText must be unique in the file. NEVER pass text from a different file as oldText. Prefer small unique blocks; for large or highly divergent rewrites, use write instead."
	35	+	return "Apply exact substitutions to an existing file. Inspect that file before editing and take every oldText directly from its current contents. Use short excerpts that identify one location; choose write when replacing most or all of a file."
36	36		}
37	37		func (t *EditTool) Schema() json.RawMessage { return json.RawMessage(editSchema) }
38	38

@@ -68,43 +68,32 @@ func (t *EditTool) Execute(ctx context.Context, raw json.RawMessage, progress fu
68	68		body := string(bytes.ReplaceAll(orig, []byte("\r\n"), []byte("\n")))
69	69
70	70		// Validate all edits first (against original content, not sequentially).
71		-	// oldText == newText is a no-op (destination already has the desired
72		-	// text) — skip those rather than erroring so equalize/port flows can
73		-	// continue when a block is already in sync.
74		-	type span struct {
75		-	start, end int
76		-	replacement string
77		-	}
78		-	spans := make([]span, 0, len(a.Edits))
79		-	applied := 0
80	71		for i, e := range a.Edits {
81	72		if e.OldText == "" {
82	73		return core.ToolResult{}, fmt.Errorf("edit %d: oldText must not be empty", i+1)
83	74		}
	75	+	if e.OldText == e.NewText {
	76	+	return core.ToolResult{}, fmt.Errorf("edit %d: oldText equals newText", i+1)
	77	+	}
84	78		count := strings.Count(body, e.OldText)
85	79		if count == 0 {
86		-	return core.ToolResult{}, fmt.Errorf("edit %d: oldText not found in %s", i+1, a.Path)
	80	+	return core.ToolResult{}, fmt.Errorf("edit %d: oldText not found in %s; inspect that file again and use a verbatim excerpt with matching spaces and line breaks", i+1, a.Path)
87	81		}
88	82		if count > 1 {
89	83		return core.ToolResult{}, fmt.Errorf("edit %d: oldText matches %d times (must be unique) in %s", i+1, count, a.Path)
90	84		}
91		-	if e.OldText == e.NewText {
92		-	continue
93		-	}
94		-	idx := strings.Index(body, e.OldText)
95		-	spans = append(spans, span{start: idx, end: idx + len(e.OldText), replacement: e.NewText})
96		-	applied++
97		-	}
98		-
99		-	if applied == 0 {
100		-	// Every edit was already a no-op; file is unchanged.
101		-	return core.ToolResult{
102		-	Content: []provider.Content{provider.TextBlock{Text: "already up to date\n"}},
103		-	Details: map[string]any{"path": path, "edits": 0, "diff": ""},
104		-	}, nil
105	85		}
106	86
107	87		// Apply atomically (sorted by position so offsets stay valid as we splice).
	88	+	type span struct {
	89	+	start, end int
	90	+	replacement string
	91	+	}
	92	+	spans := make([]span, 0, len(a.Edits))
	93	+	for _, e := range a.Edits {
	94	+	idx := strings.Index(body, e.OldText)
	95	+	spans = append(spans, span{start: idx, end: idx + len(e.OldText), replacement: e.NewText})
	96	+	}
108	97		// Check for overlaps.
109	98		for i := 0; i < len(spans); i++ {
110	99		for j := i + 1; j < len(spans); j++ {

@@ -148,7 +137,7 @@ func (t *EditTool) Execute(ctx context.Context, raw json.RawMessage, progress fu
148	137		// want it.
149	138		return core.ToolResult{
150	139		Content: []provider.Content{provider.TextBlock{Text: diff}},
151		-	Details: map[string]any{"path": path, "edits": applied, "diff": diff},
	140	+	Details: map[string]any{"path": path, "edits": len(a.Edits), "diff": diff},
152	141		}, nil
153	142		}
154	143

 	Edits []editOp `json:"edits"`
 }
-const editSchema = `{"type":"object","properties":{"path":{"type":"string"},"edits":{"type":"array","items":{"type":"object","properties":{"oldText":{"type":"string"},"newText":{"type":"string"}},"required":["oldText","newText"]}}},"required":["path","edits"]}`
+const editSchema = `{"type":"object","properties":{"path":{"type":"string","description":"File to modify, given as an absolute path or one relative to the working directory."},"edits":{"type":"array","description":"A set of non-overlapping substitutions, all located in the file content as it existed before this call.","items":{"type":"object","properties":{"oldText":{"type":"string","description":"A verbatim excerpt from the file being modified. Include enough surrounding text to identify exactly one location, without intersecting another edit."},"newText":{"type":"string","description":"Content that will replace the matched excerpt."}},"required":["oldText","newText"]}}},"required":["path","edits"]}`
 func (t *EditTool) Name() string { return "edit" }
 func (t *EditTool) Description() string {
-	return "Edit a file via exact-match replacements. Each oldText must be unique in the file. NEVER pass text from a different file as oldText. Prefer small unique blocks; for large or highly divergent rewrites, use write instead."
+	return "Apply exact substitutions to an existing file. Inspect that file before editing and take every oldText directly from its current contents. Use short excerpts that identify one location; choose write when replacing most or all of a file."
 }
 func (t *EditTool) Schema() json.RawMessage { return json.RawMessage(editSchema) }
 	body := string(bytes.ReplaceAll(orig, []byte("\r\n"), []byte("\n")))
 	// Validate all edits first (against original content, not sequentially).
-	// oldText == newText is a no-op (destination already has the desired
-	// text) — skip those rather than erroring so equalize/port flows can
-	// continue when a block is already in sync.
-	type span struct {
-		start, end  int
-		replacement string
-	}
-	spans := make([]span, 0, len(a.Edits))
-	applied := 0
 	for i, e := range a.Edits {
 		if e.OldText == "" {
 			return core.ToolResult{}, fmt.Errorf("edit %d: oldText must not be empty", i+1)
 		}
+		if e.OldText == e.NewText {
+			return core.ToolResult{}, fmt.Errorf("edit %d: oldText equals newText", i+1)
+		}
 		count := strings.Count(body, e.OldText)
 		if count == 0 {
-			return core.ToolResult{}, fmt.Errorf("edit %d: oldText not found in %s", i+1, a.Path)
+			return core.ToolResult{}, fmt.Errorf("edit %d: oldText not found in %s; inspect that file again and use a verbatim excerpt with matching spaces and line breaks", i+1, a.Path)
 		}
 		if count > 1 {
 			return core.ToolResult{}, fmt.Errorf("edit %d: oldText matches %d times (must be unique) in %s", i+1, count, a.Path)
 		}
-		if e.OldText == e.NewText {
-			continue
-		}
-		idx := strings.Index(body, e.OldText)
-		spans = append(spans, span{start: idx, end: idx + len(e.OldText), replacement: e.NewText})
-		applied++
-	}
-	if applied == 0 {
-		// Every edit was already a no-op; file is unchanged.
-		return core.ToolResult{
-			Content: []provider.Content{provider.TextBlock{Text: "already up to date\n"}},
-			Details: map[string]any{"path": path, "edits": 0, "diff": ""},
-		}, nil
 	}
 	// Apply atomically (sorted by position so offsets stay valid as we splice).
+	type span struct {
+		start, end  int
+		replacement string
+	}
+	spans := make([]span, 0, len(a.Edits))
+	for _, e := range a.Edits {
+		idx := strings.Index(body, e.OldText)
+		spans = append(spans, span{start: idx, end: idx + len(e.OldText), replacement: e.NewText})
+	}
 	// Check for overlaps.
 	for i := 0; i < len(spans); i++ {
 		for j := i + 1; j < len(spans); j++ {
 	// want it.
 	return core.ToolResult{
 		Content: []provider.Content{provider.TextBlock{Text: diff}},
-		Details: map[string]any{"path": path, "edits": applied, "diff": diff},
+		Details: map[string]any{"path": path, "edits": len(a.Edits), "diff": diff},
 	}, nil
 }

M packages/agent/tools/tools_test.go +23 -76


@@ -158,98 +158,45 @@ func TestEditAmbiguous(t *testing.T) {
158	158		}
159	159		}
160	160
161		-	// TestEditNoOpWhenAlreadyInSync: oldText == newText means the destination
162		-	// already has the desired block. That is success, not an error — equalize
163		-	// flows must keep going.
164		-	func TestEditNoOpWhenAlreadyInSync(t *testing.T) {
165		-	dir := t.TempDir()
166		-	p := filepath.Join(dir, "a.txt")
167		-	orig := "hello world\n"
168		-	if err := os.WriteFile(p, []byte(orig), 0o644); err != nil {
169		-	t.Fatal(err)
170		-	}
171		-	tool := &EditTool{CWD: dir}
172		-	res, err := tool.Execute(context.Background(), mustJSON(t, map[string]any{
173		-	"path": "a.txt",
174		-	"edits": []map[string]any{{"oldText": "hello world", "newText": "hello world"}},
175		-	}), nil)
176		-	if err != nil {
177		-	t.Fatalf("no-op edit should succeed: %v", err)
178		-	}
179		-	b, _ := os.ReadFile(p)
180		-	if string(b) != orig {
181		-	t.Fatalf("file changed on no-op: got %q", b)
182		-	}
183		-	got := res.Content[0].(provider.TextBlock).Text
184		-	if !strings.Contains(got, "already up to date") {
185		-	t.Fatalf("want already-up-to-date result, got %q", got)
	161	+	func TestEditGuidance(t *testing.T) {
	162	+	desc := (&EditTool{}).Description()
	163	+	for _, want := range []string{"Inspect that file", "directly from its current contents", "short excerpts", "write"} {
	164	+	if !strings.Contains(desc, want) {
	165	+	t.Errorf("description missing %q: %q", want, desc)
	166	+	}
186	167		}
187		-	}
188	168
189		-	func TestEditSkipsNoOpAmongRealEdits(t *testing.T) {
190		-	dir := t.TempDir()
191		-	p := filepath.Join(dir, "a.txt")
192		-	if err := os.WriteFile(p, []byte("a\nb\nc\n"), 0o644); err != nil {
	169	+	var schema map[string]any
	170	+	if err := json.Unmarshal((&EditTool{}).Schema(), &schema); err != nil {
193	171		t.Fatal(err)
194	172		}
195		-	tool := &EditTool{CWD: dir}
196		-	_, err := tool.Execute(context.Background(), mustJSON(t, map[string]any{
197		-	"path": "a.txt",
198		-	"edits": []map[string]any{
199		-	{"oldText": "a", "newText": "a"}, // already in sync
200		-	{"oldText": "c", "newText": "C"},
201		-	},
202		-	}), nil)
203		-	if err != nil {
204		-	t.Fatal(err)
205		-	}
206		-	b, _ := os.ReadFile(p)
207		-	if string(b) != "a\nb\nC\n" {
208		-	t.Fatalf("got %q", b)
	173	+	properties := schema["properties"].(map[string]any)
	174	+	edits := properties["edits"].(map[string]any)
	175	+	items := edits["items"].(map[string]any)
	176	+	editProperties := items["properties"].(map[string]any)
	177	+	oldText := editProperties["oldText"].(map[string]any)
	178	+	if got, _ := oldText["description"].(string); !strings.Contains(got, "file being modified") {
	179	+	t.Fatalf("oldText schema description missing target-file guidance: %q", got)
209	180		}
210	181		}
211	182
212		-	// TestEditOldTextNotFoundGuidesRecovery covers the equalize-divergent-files
213		-	// failure mode: oldText taken from a different file than path. The error
214		-	// must tell the model to re-read the destination and edit in blocks (or write).
215		-	func TestEditOldTextNotFoundGuidesRecovery(t *testing.T) {
	183	+	func TestEditNotFoundGuidesRecovery(t *testing.T) {
216	184		dir := t.TempDir()
217		-	p := filepath.Join(dir, "desktop.ts")
218		-	if err := os.WriteFile(p, []byte("export const timer: ReturnType<typeof setTimeout> \| null = null\n"), 0o644); err != nil {
	185	+	p := filepath.Join(dir, "destination.txt")
	186	+	if err := os.WriteFile(p, []byte("destination content\n"), 0o644); err != nil {
219	187		t.Fatal(err)
220	188		}
221	189		tool := &EditTool{CWD: dir}
222		-	// Content shaped like a sibling "browser" file — not present in desktop.ts.
223	190		_, err := tool.Execute(context.Background(), mustJSON(t, map[string]any{
224		-	"path": "desktop.ts",
225		-	"edits": []map[string]any{{
226		-	"oldText": "export const timer: number \| null = null;",
227		-	"newText": "export const timer: ReturnType<typeof setTimeout> \| null = null",
228		-	}},
	191	+	"path": "destination.txt",
	192	+	"edits": []map[string]any{{"oldText": "content from another file", "newText": "replacement"}},
229	193		}), nil)
230	194		if err == nil {
231	195		t.Fatal("want oldText not found error")
232	196		}
233		-	msg := err.Error()
234		-	for _, want := range []string{
235		-	"oldText not found",
236		-	"desktop.ts",
237		-	} {
238		-	if !strings.Contains(msg, want) {
239		-	t.Errorf("error missing %q; got %q", want, msg)
240		-	}
241		-	}
242		-	}
243		-
244		-	func TestEditDescriptionGuidesReadBeforeEdit(t *testing.T) {
245		-	desc := (&EditTool{}).Description()
246		-	for _, want := range []string{
247		-	"different file",
248		-	"write",
249		-	"small unique blocks",
250		-	} {
251		-	if !strings.Contains(desc, want) {
252		-	t.Errorf("Description missing %q; got %q", want, desc)
	197	+	for _, want := range []string{"oldText not found", "destination.txt", "inspect that file again", "matching spaces and line breaks"} {
	198	+	if !strings.Contains(err.Error(), want) {
	199	+	t.Errorf("error missing %q: %q", want, err)
253	200		}
254	201		}
255	202		}


@@ -158,98 +158,45 @@ func TestEditAmbiguous(t *testing.T) {
158		}	158		}
159		}	159		}
160			160
161	-	// TestEditNoOpWhenAlreadyInSync: oldText == newText means the destination	161	+	func TestEditGuidance(t *testing.T) {
162	-	// already has the desired block. That is success, not an error — equalize	162	+	desc := (&EditTool{}).Description()
163	-	// flows must keep going.	163	+	for _, want := range []string{"Inspect that file", "directly from its current contents", "short excerpts", "write"} {
164	-	func TestEditNoOpWhenAlreadyInSync(t *testing.T) {	164	+	if !strings.Contains(desc, want) {
165	-	dir := t.TempDir()	165	+	t.Errorf("description missing %q: %q", want, desc)
166	-	p := filepath.Join(dir, "a.txt")	166	+	}
167	-	orig := "hello world\n"
168	-	if err := os.WriteFile(p, []byte(orig), 0o644); err != nil {
169	-	t.Fatal(err)
170	-	}
171	-	tool := &EditTool{CWD: dir}
172	-	res, err := tool.Execute(context.Background(), mustJSON(t, map[string]any{
173	-	"path": "a.txt",
174	-	"edits": []map[string]any{{"oldText": "hello world", "newText": "hello world"}},
175	-	}), nil)
176	-	if err != nil {
177	-	t.Fatalf("no-op edit should succeed: %v", err)
178	-	}
179	-	b, _ := os.ReadFile(p)
180	-	if string(b) != orig {
181	-	t.Fatalf("file changed on no-op: got %q", b)
182	-	}
183	-	got := res.Content[0].(provider.TextBlock).Text
184	-	if !strings.Contains(got, "already up to date") {
185	-	t.Fatalf("want already-up-to-date result, got %q", got)
186		}	167		}
187	-	}
188			168
189	-	func TestEditSkipsNoOpAmongRealEdits(t *testing.T) {	169	+	var schema map[string]any
190	-	dir := t.TempDir()	170	+	if err := json.Unmarshal((&EditTool{}).Schema(), &schema); err != nil {
191	-	p := filepath.Join(dir, "a.txt")
192	-	if err := os.WriteFile(p, []byte("a\nb\nc\n"), 0o644); err != nil {
193		t.Fatal(err)	171		t.Fatal(err)
194		}	172		}
195	-	tool := &EditTool{CWD: dir}	173	+	properties := schema["properties"].(map[string]any)
196	-	_, err := tool.Execute(context.Background(), mustJSON(t, map[string]any{	174	+	edits := properties["edits"].(map[string]any)
197	-	"path": "a.txt",	175	+	items := edits["items"].(map[string]any)
198	-	"edits": []map[string]any{	176	+	editProperties := items["properties"].(map[string]any)
199	-	{"oldText": "a", "newText": "a"}, // already in sync	177	+	oldText := editProperties["oldText"].(map[string]any)
200	-	{"oldText": "c", "newText": "C"},	178	+	if got, _ := oldText["description"].(string); !strings.Contains(got, "file being modified") {
201	-	},	179	+	t.Fatalf("oldText schema description missing target-file guidance: %q", got)
202	-	}), nil)
203	-	if err != nil {
204	-	t.Fatal(err)
205	-	}
206	-	b, _ := os.ReadFile(p)
207	-	if string(b) != "a\nb\nC\n" {
208	-	t.Fatalf("got %q", b)
209		}	180		}
210		}	181		}
211			182
212	-	// TestEditOldTextNotFoundGuidesRecovery covers the equalize-divergent-files	183	+	func TestEditNotFoundGuidesRecovery(t *testing.T) {
213	-	// failure mode: oldText taken from a different file than path. The error
214	-	// must tell the model to re-read the destination and edit in blocks (or write).
215	-	func TestEditOldTextNotFoundGuidesRecovery(t *testing.T) {
216		dir := t.TempDir()	184		dir := t.TempDir()
217	-	p := filepath.Join(dir, "desktop.ts")	185	+	p := filepath.Join(dir, "destination.txt")
218	-	if err := os.WriteFile(p, []byte("export const timer: ReturnType<typeof setTimeout> \| null = null\n"), 0o644); err != nil {	186	+	if err := os.WriteFile(p, []byte("destination content\n"), 0o644); err != nil {
219		t.Fatal(err)	187		t.Fatal(err)
220		}	188		}
221		tool := &EditTool{CWD: dir}	189		tool := &EditTool{CWD: dir}
222	-	// Content shaped like a sibling "browser" file — not present in desktop.ts.
223		_, err := tool.Execute(context.Background(), mustJSON(t, map[string]any{	190		_, err := tool.Execute(context.Background(), mustJSON(t, map[string]any{
224	-	"path": "desktop.ts",	191	+	"path": "destination.txt",
225	-	"edits": []map[string]any{{	192	+	"edits": []map[string]any{{"oldText": "content from another file", "newText": "replacement"}},
226	-	"oldText": "export const timer: number \| null = null;",
227	-	"newText": "export const timer: ReturnType<typeof setTimeout> \| null = null",
228	-	}},
229		}), nil)	193		}), nil)
230		if err == nil {	194		if err == nil {
231		t.Fatal("want oldText not found error")	195		t.Fatal("want oldText not found error")
232		}	196		}
233	-	msg := err.Error()	197	+	for _, want := range []string{"oldText not found", "destination.txt", "inspect that file again", "matching spaces and line breaks"} {
234	-	for _, want := range []string{	198	+	if !strings.Contains(err.Error(), want) {
235	-	"oldText not found",	199	+	t.Errorf("error missing %q: %q", want, err)
236	-	"desktop.ts",
237	-	} {
238	-	if !strings.Contains(msg, want) {
239	-	t.Errorf("error missing %q; got %q", want, msg)
240	-	}
241	-	}
242	-	}
243	-
244	-	func TestEditDescriptionGuidesReadBeforeEdit(t *testing.T) {
245	-	desc := (&EditTool{}).Description()
246	-	for _, want := range []string{
247	-	"different file",
248	-	"write",
249	-	"small unique blocks",
250	-	} {
251	-	if !strings.Contains(desc, want) {
252	-	t.Errorf("Description missing %q; got %q", want, desc)
253		}	200		}
254		}	201		}
255		}	202		}