opensource/coder
3
0
Fork 0
mirror of https://github.com/coder/coder.git synced 2026-10-30 18:12:29 +08:00
Code Issues Packages Projects Releases Wiki Activity

fix: use old_text/new_text vocabulary across edit_files (#28626)

Browse Source 
The schema has advertised old_text/new_text since #25658, but agent
error strings and hints still said "search string", directing the
model toward field names it is not sending.

Fixes CODAGT-523
This commit is contained in:
Mathias Fredriksson
2026-08-26 18:35:27 +03:00
committed by GitHub
parent c51ffc7c88
commit 6093705476
5 changed files with 94 additions and 62 deletions
Download Patch File Download Diff File Expand all files Collapse all files
agent/agentfiles/files.go
+41 -27
View File
@@ -1081,19 +1081,43 @@ func buildReplacementLines(matched, searchLines []string, replace, forcedEnding
return b.String()
}
// fuzzyReplace attempts to find `search` inside `content` and replace it
// with `replace`. It uses a cascading match strategy inspired by
// openai/codex's apply_patch:
// errOldTextAmbiguous is returned when the old_text matches more
// than one location and replace_all is false. Shared by the
// byte-level pass and the line-level passes so both spell the
// remediation identically.
const errOldTextAmbiguous = "old_text matches %d occurrences " +
"(expected exactly 1). Include more surrounding " +
"context to make the match unique, or set " +
"replace_all to true"
// errOldTextNotFoundPrefix is the shared lead of every not-found
// message; the per-pass tails below complete it. Rewording the lead
// touches this one constant.
const errOldTextNotFoundPrefix = "old_text not found in file. Verify that old_text " +
"matches the file content exactly, including whitespace"
// errOldTextNotFound is the leading sentence returned when all match
// passes miss; diagnostic hints are appended after it.
const errOldTextNotFound = errOldTextNotFoundPrefix + " and indentation"
// errOldTextNotFoundLineEndings is the ending-rewrite variant, used
// when the caller signaled intent to change line endings and the
// exact pass missed.
const errOldTextNotFoundLineEndings = errOldTextNotFoundPrefix + ", indentation, and line endings"
// fuzzyReplace attempts to find the edit's old_text inside content
// and replace it with new_text. It uses a cascading match strategy
// inspired by openai/codex's apply_patch:
//
// 1. Exact substring match (byte-for-byte).
// 2. Line-by-line match ignoring trailing whitespace on each line.
// 3. Line-by-line match ignoring all leading/trailing whitespace
// (indentation-tolerant).
//
// When edit.ReplaceAll is false (the default), the search string must
// match exactly one location. If multiple matches are found, an error
// is returned asking the caller to include more context or set
// replace_all.
// When edit.ReplaceAll is false (the default), the old_text must
// match exactly one location. If multiple matches are found, an
// error is returned asking the caller to include more context or
// set replace_all.
//
// When a fuzzy match is found (passes 2 or 3), buildReplacementLines
// emits the spliced output by per-position substitution at
@@ -1112,7 +1136,7 @@ func fuzzyReplace(content string, edit workspacesdk.FileEdit) (string, error) {
// replace_all=true can't silently inject the replacement between
// every byte.
if search == "" {
return "", xerrors.New("search string must not be empty; include the " +
return "", xerrors.New("old_text must not be empty; include the " +
"text you want to match")
}
@@ -1159,10 +1183,7 @@ func fuzzyReplace(content string, edit workspacesdk.FileEdit) (string, error) {
}
count := strings.Count(content, search)
if count > 1 {
return "", xerrors.Errorf("search string matches %d occurrences "+
"(expected exactly 1). Include more surrounding "+
"context to make the match unique, or set "+
"replace_all to true", count)
return "", xerrors.Errorf(errOldTextAmbiguous, count)
}
// Exactly one match.
return strings.Replace(content, search, pass1Replace, 1), nil
@@ -1171,9 +1192,7 @@ func fuzzyReplace(content string, edit workspacesdk.FileEdit) (string, error) {
if callerEndingIntent {
// Intent signaled but pass 1 missed; reject rather than let
// pass 2's CRLF/LF interchange bridge a mismatched search.
return "", xerrors.New("search string not found in file. Verify the search " +
"string matches the file content exactly, including whitespace, " +
"indentation, and line endings")
return "", xerrors.New(errOldTextNotFoundLineEndings)
}
trimRight := func(a, b string) bool {
@@ -1201,9 +1220,7 @@ func fuzzyReplace(content string, edit workspacesdk.FileEdit) (string, error) {
return result, err
}
msg := "search string not found in file. Verify the search " +
"string matches the file content exactly, including whitespace " +
"and indentation"
msg := errOldTextNotFound
// miscount takes precedence: a near-match means the search is the
// model's typo'd new text, not a swapped field. Emitting both can
// trick an agent into following the inversion hint and corrupting
@@ -1222,8 +1239,8 @@ func fuzzyReplace(content string, edit workspacesdk.FileEdit) (string, error) {
// truncation with " and N more".
const maxHintLines = 5
// inversionHint detects the case where the caller swapped `search`
// and `replace`: search did not match but replace appears in the file.
// inversionHint detects the case where the caller swapped `old_text`
// and `new_text`: old_text did not match but new_text appears in the file.
func inversionHint(
content string,
contentLines []string,
@@ -1246,8 +1263,8 @@ func inversionHint(
return ""
}
return fmt.Sprintf(
"Did you swap %q and %q? Your replace string appears at line %s",
"search", "replace", formatLineList(lines),
"Did you swap %q and %q? Your new_text string appears at line %s",
"old_text", "new_text", formatLineList(lines),
)
}
@@ -1367,7 +1384,7 @@ func miscountHint(contentLines, searchLines []string) string {
// formatMiscount renders one miscount candidate group.
func formatMiscount(sCount int, r rune, cands []candidate) string {
var b strings.Builder
_, _ = fmt.Fprintf(&b, "Your search has %d %q (U+%04X); the file has ", sCount, string(r), r)
_, _ = fmt.Fprintf(&b, "Your old_text has %d %q (U+%04X); the file has ", sCount, string(r), r)
shown := min(len(cands), maxHintLines)
for i := 0; i < shown; i++ {
if i > 0 {
@@ -1505,10 +1522,7 @@ func fuzzyReplaceLines(
if !replaceAll {
if count := countLineMatches(contentLines, searchLines, eq); count > 1 {
return "", true, xerrors.Errorf("search string matches %d occurrences "+
"(expected exactly 1). Include more surrounding "+
"context to make the match unique, or set "+
"replace_all to true", count)
return "", true, xerrors.Errorf(errOldTextAmbiguous, count)
}
var b strings.Builder
for _, l := range contentLines[:start] {
agent/agentfiles/files_test.go
+48 -32
View File
@@ -851,7 +851,7 @@ func TestEditFiles(t *testing.T) {
},
},
errCode: http.StatusBadRequest,
errors: []string{"search string not found in file"},
errors: []string{"old_text not found in file"},
// File should remain unchanged.
expected: map[string]string{filepath.Join(tmpdir, "no-match"): "original content"},
},
@@ -1025,7 +1025,7 @@ func TestEditFiles(t *testing.T) {
},
},
errCode: http.StatusBadRequest,
errors: []string{"search string not found"},
errors: []string{"old_text not found"},
expected: map[string]string{
filepath.Join(tmpdir, "atomic-a"): "aaa",
filepath.Join(tmpdir, "atomic-b"): "bbb",
@@ -2376,7 +2376,7 @@ func TestEditFiles_WhitespaceAndLineEndings(t *testing.T) {
content: "above\n\n\nbelow\n",
search: "above\nbelow",
replace: "above\nbelow",
errSub: "search string not found",
errSub: "old_text not found",
},
// Search matches blank lines exactly; replacement
@@ -2522,7 +2522,7 @@ func TestFuzzyReplace_Rejects(t *testing.T) {
name: "EmptySearch_Rejects",
content: "hello\n",
edits: []edit{{search: "", replace: "X"}},
errSub: "search string must not be empty",
errSub: "old_text must not be empty",
},
// Empty search with replace_all=true: historically
// injected the replacement between every byte, silently
@@ -2531,7 +2531,7 @@ func TestFuzzyReplace_Rejects(t *testing.T) {
name: "EmptySearch_ReplaceAll_Rejects",
content: "hello\n",
edits: []edit{{search: "", replace: "X", replaceAll: true}},
errSub: "search string must not be empty",
errSub: "old_text must not be empty",
},
// Ambiguous single-replace: 3 distinct matches, caller
// did not ask for replace_all.
@@ -2547,7 +2547,7 @@ func TestFuzzyReplace_Rejects(t *testing.T) {
name: "NotFound_Rejects",
content: "hello\nworld\n",
edits: []edit{{search: "nonexistent\n", replace: "X\n"}},
errSub: "search string not found",
errSub: "old_text not found",
},
// Content mismatch that trimming cannot recover: search
// has different letters, not just different whitespace.
@@ -2555,7 +2555,7 @@ func TestFuzzyReplace_Rejects(t *testing.T) {
name: "ContentMismatch_Rejects",
content: "hello\n",
edits: []edit{{search: "Hello\n", replace: "HELLO\n"}},
errSub: "search string not found",
errSub: "old_text not found",
},
// Blank lines in the file that the search omits: the
// fuzzy window cannot align against the blank lines, so
@@ -2564,7 +2564,7 @@ func TestFuzzyReplace_Rejects(t *testing.T) {
name: "BlankLineMismatch_Rejects",
content: "above\n\n\nbelow\n",
edits: []edit{{search: "above\nbelow\n", replace: "above\nbelow\n"}},
errSub: "search string not found",
errSub: "old_text not found",
},
// Search/replace disagreement signals intent to rewrite
// endings; search must byte-match the file's. LF search
@@ -2574,7 +2574,7 @@ func TestFuzzyReplace_Rejects(t *testing.T) {
name: "CallerIntent_SearchDoesNotMatchFileEnding_Rejects",
content: "x\r\ny\r\nz\r\n",
edits: []edit{{search: "x\ny\n", replace: "X\r\nY\r\n"}},
errSub: "search string not found",
errSub: "old_text not found",
},
}
@@ -3223,8 +3223,8 @@ func TestFuzzyReplace_Expansion_PreservesFileIndent(t *testing.T) {
// returns when all three passes miss. It must remain the leading
// sentence even when diagnostic hints are appended, so existing log
// scrapers continue to match.
const baseFuzzyNotFoundMessage = "search string not found in file. " +
"Verify the search string matches the file content exactly, " +
const baseFuzzyNotFoundMessage = "old_text not found in file. " +
"Verify that old_text matches the file content exactly, " +
"including whitespace and indentation"
// TestFuzzyReplace_Hints exercises the post-fail diagnostic hints:
@@ -3260,9 +3260,25 @@ func TestFuzzyReplace_Hints(t *testing.T) {
},
wantSubs: []string{
baseFuzzyNotFoundMessage,
`Did you swap "search" and "replace"? Your replace string appears at line 3`,
`Did you swap "old_text" and "new_text"? Your new_text string appears at line 3`,
},
},
{
// CODAGT-523 falsifier: the hint must name the current
// schema fields, not the deprecated search/replace.
name: "Inversion_HintUsesNewSchemaNames",
content: "package main\n" +
"\n" +
"func adder(a int, b int) int { return a + b }\n" +
"\n" +
"// trailing comment\n",
edit: edit{
search: "func adder(a, b int) int {\n\treturn a + b\n}\n",
replace: "func adder(a int, b int) int { return a + b }\n",
},
wantSubs: []string{`"old_text"`, `"new_text"`},
notWantSubs: []string{`"search"`, `"replace"`},
},
{
name: "Inversion_ThreeAnchors_AllListed",
content: "a\n" +
@@ -3278,7 +3294,7 @@ func TestFuzzyReplace_Hints(t *testing.T) {
},
wantSubs: []string{
baseFuzzyNotFoundMessage,
`Did you swap "search" and "replace"? Your replace string appears at line 2, 4, 6`,
`Did you swap "old_text" and "new_text"? Your new_text string appears at line 2, 4, 6`,
},
notWantSubs: []string{"more"},
},
@@ -3297,7 +3313,7 @@ func TestFuzzyReplace_Hints(t *testing.T) {
},
wantSubs: []string{
baseFuzzyNotFoundMessage,
`Did you swap "search" and "replace"? Your replace string appears at line 1, 2, 3, 4, 5 and 2 more`,
`Did you swap "old_text" and "new_text"? Your new_text string appears at line 1, 2, 3, 4, 5 and 2 more`,
},
},
{
@@ -3313,7 +3329,7 @@ func TestFuzzyReplace_Hints(t *testing.T) {
},
wantSubs: []string{
baseFuzzyNotFoundMessage,
`Did you swap "search" and "replace"? Your replace string appears at line 2, 3, 4, 5, 6 and 2 more`,
`Did you swap "old_text" and "new_text"? Your new_text string appears at line 2, 3, 4, 5, 6 and 2 more`,
},
},
{
@@ -3327,7 +3343,7 @@ func TestFuzzyReplace_Hints(t *testing.T) {
},
wantSubs: []string{
baseFuzzyNotFoundMessage,
"Your search has 32 \"\u2500\" (U+2500); the file has 37 at line 2",
"Your old_text has 32 \"\u2500\" (U+2500); the file has 37 at line 2",
},
},
{
@@ -3341,7 +3357,7 @@ func TestFuzzyReplace_Hints(t *testing.T) {
},
wantSubs: []string{
baseFuzzyNotFoundMessage,
`Your search has 5 "=" (U+003D); the file has 7 at line 2`,
`Your old_text has 5 "=" (U+003D); the file has 7 at line 2`,
},
},
{
@@ -3354,7 +3370,7 @@ func TestFuzzyReplace_Hints(t *testing.T) {
},
wantSubs: []string{
baseFuzzyNotFoundMessage,
`Your search has 5 "=" (U+003D); the file has 7 at line 1, 3 at line 2`,
`Your old_text has 5 "=" (U+003D); the file has 7 at line 1, 3 at line 2`,
},
notWantSubs: []string{"more"},
},
@@ -3372,7 +3388,7 @@ func TestFuzzyReplace_Hints(t *testing.T) {
},
wantSubs: []string{
baseFuzzyNotFoundMessage,
`Your search has 5 "=" (U+003D); the file has 2 at line 1, 3 at line 2, 6 at line 3, 7 at line 4, 8 at line 5 and 1 more`,
`Your old_text has 5 "=" (U+003D); the file has 2 at line 1, 3 at line 2, 6 at line 3, 7 at line 4, 8 at line 5 and 1 more`,
},
},
{
@@ -3385,7 +3401,7 @@ func TestFuzzyReplace_Hints(t *testing.T) {
replace: "unused\n",
},
wantSubs: []string{baseFuzzyNotFoundMessage},
notWantSubs: []string{"Your search has", "the file has"},
notWantSubs: []string{"Your old_text has", "the file has"},
},
{
name: "Miscount_Unrelated_NoHint",
@@ -3395,7 +3411,7 @@ func TestFuzzyReplace_Hints(t *testing.T) {
replace: "unused\n",
},
wantSubs: []string{baseFuzzyNotFoundMessage},
notWantSubs: []string{"Your search has", "the file has"},
notWantSubs: []string{"Your old_text has", "the file has"},
},
{
name: "Miscount_SuppressesInversion_WhenBothCouldFire",
@@ -3415,7 +3431,7 @@ func TestFuzzyReplace_Hints(t *testing.T) {
},
wantSubs: []string{
baseFuzzyNotFoundMessage,
"Your search has 6 \"\u2500\" (U+2500); the file has 8 at line 2",
"Your old_text has 6 \"\u2500\" (U+2500); the file has 8 at line 2",
},
notWantSubs: []string{"swap", "appears at line"},
},
@@ -3430,7 +3446,7 @@ func TestFuzzyReplace_Hints(t *testing.T) {
},
wantSubs: []string{
baseFuzzyNotFoundMessage,
`Did you swap "search" and "replace"? Your replace string appears at line 2`,
`Did you swap "old_text" and "new_text"? Your new_text string appears at line 2`,
},
// Line 2 must appear once, not 2, 2, 2.
notWantSubs: []string{"line 2, 2", "more"},
@@ -3449,7 +3465,7 @@ func TestFuzzyReplace_Hints(t *testing.T) {
},
wantSubs: []string{
baseFuzzyNotFoundMessage,
`Did you swap "search" and "replace"? Your replace string appears at line 2`,
`Did you swap "old_text" and "new_text"? Your new_text string appears at line 2`,
},
},
{
@@ -3467,7 +3483,7 @@ func TestFuzzyReplace_Hints(t *testing.T) {
},
wantSubs: []string{
baseFuzzyNotFoundMessage,
`Did you swap "search" and "replace"? Your replace string appears at line 2`,
`Did you swap "old_text" and "new_text"? Your new_text string appears at line 2`,
},
},
{
@@ -3480,7 +3496,7 @@ func TestFuzzyReplace_Hints(t *testing.T) {
replace: "unused\n",
},
wantSubs: []string{baseFuzzyNotFoundMessage},
notWantSubs: []string{"Your search has", "the file has"},
notWantSubs: []string{"Your old_text has", "the file has"},
},
{
name: "Miscount_TotalHintsCapped",
@@ -3500,9 +3516,9 @@ func TestFuzzyReplace_Hints(t *testing.T) {
},
wantSubs: []string{
baseFuzzyNotFoundMessage,
`Your search has 4 "=" (U+003D)`,
`Your search has 4 "+" (U+002B)`,
`Your search has 4 "#" (U+0023)`,
`Your old_text has 4 "=" (U+003D)`,
`Your old_text has 4 "+" (U+002B)`,
`Your old_text has 4 "#" (U+0023)`,
"and 1 more",
},
// The fourth hint (`@`) is suppressed by the cap.
@@ -3526,7 +3542,7 @@ func TestFuzzyReplace_Hints(t *testing.T) {
},
wantSubs: []string{
baseFuzzyNotFoundMessage,
`Did you swap "search" and "replace"? Your replace string appears at line 1, 3`,
`Did you swap "old_text" and "new_text"? Your new_text string appears at line 1, 3`,
},
notWantSubs: []string{"more"},
},
@@ -3542,7 +3558,7 @@ func TestFuzzyReplace_Hints(t *testing.T) {
},
wantSubs: []string{
baseFuzzyNotFoundMessage,
`Your search has 0 "b" (U+0062); the file has 2 at line 1`,
`Your old_text has 0 "b" (U+0062); the file has 2 at line 1`,
},
},
{
@@ -3555,7 +3571,7 @@ func TestFuzzyReplace_Hints(t *testing.T) {
replace: "new\n",
},
wantSubs: []string{baseFuzzyNotFoundMessage},
notWantSubs: []string{"swap", "Your search has", "appears at line"},
notWantSubs: []string{"swap", "Your old_text has", "appears at line"},
},
}
docs/ai-coder/agents/architecture.md
+1 -1
View File
@@ -141,7 +141,7 @@ They traverse the same Tailnet tunnel used by web terminals and IDE connections.
|------------------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| `read_file` | Reads file contents with line-number pagination. |
| `write_file` | Writes content to a file. |
| `edit_files` | Performs atomic search-and-replace edits across one or more files. |
| `edit_files` | Performs atomic, per-file edits across one or more files, replacing `old_text` with `new_text`. |
| `execute` | Runs a shell command, waiting for completion up to a timeout. |
| `process_output` | Retrieves output from a tracked process. |
| `process_list` | Lists all tracked processes in the workspace. |
docs/ai-coder/agents/index.md
+1 -1
View File
@@ -244,7 +244,7 @@ tasks:
| `ask_user_question` | Ask the user structured clarification questions during plan mode |
| `read_file` | Read file contents from the workspace |
| `write_file` | Write a file to the workspace |
| `edit_files` | Perform search-and-replace edits across files |
| `edit_files` | Replace `old_text` with `new_text` across files |
| `execute` | Run shell commands in the workspace |
| `process_output` | Retrieve output from a background process |
| `process_list` | List all tracked processes in the workspace |
site/src/pages/AgentsPage/components/ChatElements/tools/utils.ts
+3 -1
View File
@@ -13,7 +13,9 @@ export interface EditFilesFileEntry {
// Validates that the edit has at least the shape of an object with
// string-typed text fields. Accepts both current field names
// (old_text/new_text) and deprecated names (search/replace).
// (old_text/new_text) and old names (search/replace): tool-call args
// persisted from older chats use the old names and are immutable, so
// this branch must stay to keep their diffs rendering.
const normalizeEdit = (
e: unknown,
): { search: string; replace: string } | null => {