Skip to content

fix(tools): reject fuzzy edits with drifted content #983

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
PierrunoYT wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(tools): reject fuzzy edits with drifted content #983

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions internal/tools/edit_file.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,10 +102,10 @@ func (tool editFileTool) RunWithOptions(ctx context.Context, args map[string]any
}

// Fuzzy fallback: when the exact string (and its CRLF translation) is absent,
// run a cascade of tolerant matchers (trimmed lines, block anchors, collapsed
// whitespace, indentation drift, escape normalization) to locate the span the
// model intended. Only a span that occurs literally in the file is accepted,
// so the replacement applied below is still exact.
// run a cascade of tolerant matchers (trimmed lines, collapsed whitespace,
// indentation drift, escape normalization) to locate the span the model
// intended. These transformations must preserve non-whitespace content; a
// merely similar interior is not safe to replace.
if occurrences == 0 {
findOld, findNew := oldString, newString
if strings.Contains(content, "\r\n") && !strings.Contains(findOld, "\r\n") {
Expand Down
198 changes: 6 additions & 192 deletions internal/tools/edit_replacers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,16 @@ package tools

// Fuzzy fallback matching for edit_file. When the model's old_string fails to
// match byte-for-byte (drifted indentation, collapsed whitespace, escaped
// characters, a slightly-misremembered middle line), these replacers propose
// candidate spans that plausibly correspond to what the model intended. Only a
// candidate that occurs literally in the file is accepted, so the replacement
// itself is always exact even when the match was tolerant.
// characters), these replacers propose candidate spans that differ only by the
// normalization each strategy explicitly handles. They never infer a match
// from similar but different content.
//
// Strategy cascade ported from opencode's edit tool, whose replacers were in
// turn distilled from Cline's diff-apply evals and gemini-cli's edit corrector.

import (
"errors"
"regexp"
"sort"
"strings"
)

Expand All @@ -26,9 +24,6 @@ var (
errEditFuzzyAmbiguous = errors.New("fuzzy match for old_string is ambiguous")
)

// Minimum average middle-line similarity for block-anchor matches.
const editAnchorSimilarityThreshold = 0.65

// fuzzyEditMatch runs the replacer cascade and returns the exact span of
// content to replace. When replaceAll is false the span must be unique in
// content; an ambiguous candidate is skipped in favor of later candidates and
Expand All @@ -37,12 +32,10 @@ const editAnchorSimilarityThreshold = 0.65
func fuzzyEditMatch(content, find string, replaceAll bool) (string, error) {
replacers := []editReplacer{
lineTrimmedReplacer,
blockAnchorReplacer,
whitespaceNormalizedReplacer,
indentationFlexibleReplacer,
escapeNormalizedReplacer,
trimmedBoundaryReplacer,
contextAwareReplacer,
}
found := false
for _, replacer := range replacers {
Expand Down Expand Up @@ -89,9 +82,8 @@ func fuzzyEditMatch(content, find string, replaceAll bool) (string, error) {
return "", errEditFuzzyAmbiguous
}

// isDisproportionateEditMatch guards against anchor-style replacers matching a
// span far larger than the text the model asked to replace (e.g. first/last
// line anchors bridging hundreds of unrelated lines).
// isDisproportionateEditMatch guards against a fuzzy replacer matching a span
// far larger than the text the model asked to replace.
func isDisproportionateEditMatch(search, find string) bool {
findLines := strings.Count(find, "\n") + 1
searchLines := strings.Count(search, "\n") + 1
Expand Down Expand Up @@ -148,106 +140,6 @@ func lineTrimmedReplacer(content, find string) []string {
return candidates
}

// blockAnchorReplacer anchors on the first and last lines (trimmed) and
// accepts the block when the middle lines average >= the similarity threshold
// (Levenshtein-based), tolerating a slightly misremembered interior.
func blockAnchorReplacer(content, find string) []string {
findLines := splitFindLines(find)
if len(findLines) < 3 {
return nil
}
contentLines := strings.Split(content, "\n")
firstAnchor := strings.TrimSpace(findLines[0])
lastAnchor := strings.TrimSpace(findLines[len(findLines)-1])
searchBlockSize := len(findLines)
maxLineDelta := searchBlockSize / 4
if maxLineDelta < 1 {
maxLineDelta = 1
}

type span struct{ start, end int }
var candidates []span
for i := 0; i < len(contentLines); i++ {
if strings.TrimSpace(contentLines[i]) != firstAnchor {
continue
}
// Only the first occurrence of the last anchor after this start counts,
// mirroring the reference implementation: a farther-away closing line is
// assumed to close a different block.
for j := i + 2; j < len(contentLines); j++ {
if strings.TrimSpace(contentLines[j]) != lastAnchor {
continue
}
actualBlockSize := j - i + 1
delta := actualBlockSize - searchBlockSize
if delta < 0 {
delta = -delta
}
if delta <= maxLineDelta {
candidates = append(candidates, span{start: i, end: j})
}
break
}
}
if len(candidates) == 0 {
return nil
}

middleSimilarity := func(candidate span) float64 {
actualBlockSize := candidate.end - candidate.start + 1
linesToCheck := searchBlockSize - 2
if actualBlockSize-2 < linesToCheck {
linesToCheck = actualBlockSize - 2
}
if linesToCheck <= 0 {
return 1.0
}
similarity := 0.0
for j := 1; j < searchBlockSize-1 && j < actualBlockSize-1; j++ {
contentLine := strings.TrimSpace(contentLines[candidate.start+j])
findLine := strings.TrimSpace(findLines[j])
maxLen := len(contentLine)
if len(findLine) > maxLen {
maxLen = len(findLine)
}
if maxLen == 0 {
continue
}
distance := levenshtein(contentLine, findLine)
similarity += 1 - float64(distance)/float64(maxLen)
}
return similarity / float64(linesToCheck)
}

// Return EVERY candidate that clears the similarity threshold, best first.
// Picking only the best would hide competing blocks from fuzzyEditMatch's
// distinct-candidate ambiguity check and could silently edit the wrong
// block; with all qualifying spans surfaced, one clear winner still
// resolves (single candidate) while two plausible blocks are rejected as
// ambiguous. replaceAll consumers take the first (most similar) span.
type scoredSpan struct {
span span
similarity float64
}
var qualifying []scoredSpan
for _, candidate := range candidates {
if s := middleSimilarity(candidate); s >= editAnchorSimilarityThreshold {
qualifying = append(qualifying, scoredSpan{span: candidate, similarity: s})
}
}
if len(qualifying) == 0 {
return nil
}
sort.SliceStable(qualifying, func(i, j int) bool {
return qualifying[i].similarity > qualifying[j].similarity
})
spans := make([]string, 0, len(qualifying))
for _, scored := range qualifying {
spans = append(spans, strings.Join(contentLines[scored.span.start:scored.span.end+1], "\n"))
}
return spans
}

var editWhitespaceRun = regexp.MustCompile(`\s+`)

func normalizeEditWhitespace(text string) string {
Expand Down Expand Up @@ -407,49 +299,6 @@ func trimmedBoundaryReplacer(content, find string) []string {
return candidates
}

// contextAwareReplacer anchors on the first and last lines and accepts an
// equal-length block when at least half of its non-empty middle lines match
// after trimming — a cheaper, stricter cousin of blockAnchorReplacer.
func contextAwareReplacer(content, find string) []string {
findLines := splitFindLines(find)
if len(findLines) < 3 {
return nil
}
contentLines := strings.Split(content, "\n")
firstAnchor := strings.TrimSpace(findLines[0])
lastAnchor := strings.TrimSpace(findLines[len(findLines)-1])
var candidates []string
for i := 0; i < len(contentLines); i++ {
if strings.TrimSpace(contentLines[i]) != firstAnchor {
continue
}
for j := i + 2; j < len(contentLines); j++ {
if strings.TrimSpace(contentLines[j]) != lastAnchor {
continue
}
if j-i+1 == len(findLines) {
matching, nonEmpty := 0, 0
for k := 1; k < len(findLines)-1; k++ {
blockLine := strings.TrimSpace(contentLines[i+k])
findLine := strings.TrimSpace(findLines[k])
if blockLine == "" && findLine == "" {
continue
}
nonEmpty++
if blockLine == findLine {
matching++
}
}
if nonEmpty == 0 || float64(matching)/float64(nonEmpty) >= 0.5 {
candidates = append(candidates, strings.Join(contentLines[i:j+1], "\n"))
}
}
break
}
}
return candidates
}

// adaptReplacementToSpan re-shapes the model's replacement to the span a
// tolerant matcher resolved. When old_string only matched after normalization,
// new_string was written at old_string's (wrong) shape, so applying it raw
Expand All @@ -458,8 +307,7 @@ func contextAwareReplacer(content, find string) []string {
// 1. Uniform re-indent: when every span line equals delta + the corresponding
// find line's indentation (the line-trimmed / indentation-flexible shapes),
// the same delta is prepended to every non-blank replacement line. Any
// line that breaks the uniform-delta relationship disables the shift —
// block-anchor matches with a drifted interior are left untouched.
// line that breaks the uniform-delta relationship disables the shift.
// 2. Trailing CR: a span from a CRLF file ends mid-line at "\r" (candidates
// are built by joining lines split on "\n"); the replacement gets the same
// trailing "\r" so the file's CRLF pairs stay intact.
Expand Down Expand Up @@ -518,37 +366,3 @@ func uniformIndentDelta(span, find string) (string, bool) {
}
return delta, haveDelta
}

// levenshtein computes edit distance with a two-row rolling matrix.
func levenshtein(a, b string) int {
if a == "" {
return len(b)
}
if b == "" {
return len(a)
}
previous := make([]int, len(b)+1)
current := make([]int, len(b)+1)
for j := range previous {
previous[j] = j
}
for i := 1; i <= len(a); i++ {
current[0] = i
for j := 1; j <= len(b); j++ {
cost := 1
if a[i-1] == b[j-1] {
cost = 0
}
minimum := previous[j] + 1
if current[j-1]+1 < minimum {
minimum = current[j-1] + 1
}
if previous[j-1]+cost < minimum {
minimum = previous[j-1] + cost
}
current[j] = minimum
}
previous, current = current, previous
}
return previous[len(b)]
}
68 changes: 16 additions & 52 deletions internal/tools/edit_replacers_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,33 +78,18 @@ func TestEditFuzzyWhitespaceNormalizedSingleLine(t *testing.T) {
}
}

func TestEditFuzzyBlockAnchorToleratesMiddleDrift(t *testing.T) {
// First and last lines anchor the block; one interior line differs slightly
// (comment text drifted). Levenshtein similarity keeps it above 0.65.
initial := strings.Join([]string{
"func handler(w http.ResponseWriter, r *http.Request) {",
"\t// write the response body to the client",
"\tw.WriteHeader(http.StatusOK)",
"\tfmt.Fprint(w, \"done\")",
"}",
"",
}, "\n")
find := strings.Join([]string{
"func handler(w http.ResponseWriter, r *http.Request) {",
"\t// write the response body to client",
"\tw.WriteHeader(http.StatusOK)",
"\tfmt.Fprint(w, \"done\")",
"}",
}, "\n")
func TestEditFuzzyRejectsInteriorDrift(t *testing.T) {
initial := "func a() {\n\tx := 1\n\ty := 2\n\tz := 3\n\treturn\n}\n"
find := "func a() {\n\tx := 1\n\ty := 99\n\tz := 3\n\treturn\n}"
result, after := runEdit(t, t.TempDir(), initial, map[string]any{
"old_string": find,
"new_string": "func handler(w http.ResponseWriter, r *http.Request) {\n\tw.WriteHeader(http.StatusNoContent)\n}",
"new_string": "func a() {}",
})
if result.Status != StatusOK {
t.Fatalf("expected ok, got %q", result.Output)
if result.Status != StatusError || !strings.Contains(result.Output, "Could not find the exact string") {
t.Fatalf("expected exact-match error, got %q", result.Output)
}
if !strings.Contains(after, "StatusNoContent") || strings.Contains(after, "StatusOK") {
t.Fatalf("unexpected content: %q", after)
if after != initial {
t.Fatalf("file must be unchanged, got %q", after)
}
}

Expand Down Expand Up @@ -168,8 +153,8 @@ func TestUniformIndentDelta(t *testing.T) {
}

func TestAdaptReplacementLeavesNonUniformSpansAlone(t *testing.T) {
// Block-anchor style match with a drifted interior: no uniform delta, so
// the replacement must pass through untouched.
// A non-uniform span has no safe indentation delta, so the replacement must
// pass through untouched.
span := "\tfunc h() {\n\t\t// drifted comment\n\t}"
find := "func h() {\n// different comment\n}"
if got := adaptReplacementToSpan(span, find, "replacement()"); got != "replacement()" {
Expand Down Expand Up @@ -283,8 +268,7 @@ func TestEditFuzzyCRLFFile(t *testing.T) {
}

func TestIsDisproportionateEditMatch(t *testing.T) {
// A candidate span that dwarfs old_string must be refused: anchors bridging
// unrelated code would otherwise delete it all.
// A candidate span that dwarfs old_string must be refused.
big := strings.Repeat("line\n", 10)
if !isDisproportionateEditMatch(big, "a\nb\nc") {
t.Fatal("10-line span for 3-line find must be disproportionate")
Expand All @@ -300,24 +284,6 @@ func TestIsDisproportionateEditMatch(t *testing.T) {
}
}

func TestLevenshtein(t *testing.T) {
cases := []struct {
a, b string
want int
}{
{"", "", 0},
{"abc", "", 3},
{"", "abc", 3},
{"kitten", "sitting", 3},
{"same", "same", 0},
}
for _, c := range cases {
if got := levenshtein(c.a, c.b); got != c.want {
t.Fatalf("levenshtein(%q,%q) = %d, want %d", c.a, c.b, got, c.want)
}
}
}

func TestEditFuzzyDistinctCandidatesAreAmbiguous(t *testing.T) {
// Two same-content blocks at DIFFERENT indentation: each resolved span is
// distinct and occurs exactly once, so the old literal-uniqueness check
Expand All @@ -336,11 +302,9 @@ func TestEditFuzzyDistinctCandidatesAreAmbiguous(t *testing.T) {
}
}

func TestEditFuzzyBlockAnchorTwoPlausibleBlocksAmbiguous(t *testing.T) {
// Two blocks share the same first/last anchor lines and BOTH interiors sit
// above the similarity threshold. Picking the "best" one would silently
// edit a block the model may not have meant; both must surface so the
// cascade reports ambiguity and the file stays untouched.
func TestEditFuzzyDoesNotChooseBetweenDriftedBlocks(t *testing.T) {
// Shared first and last lines do not make either drifted interior safe to
// replace. Neither block may be selected.
initial := strings.Join([]string{
"func setup(cfg Config) {",
"\tvalue := compute(alpha, beta, gamma)",
Expand All @@ -360,8 +324,8 @@ func TestEditFuzzyBlockAnchorTwoPlausibleBlocksAmbiguous(t *testing.T) {
"old_string": find,
"new_string": "func setup(cfg Config) {}",
})
if result.Status != StatusError || !strings.Contains(result.Output, "multiple locations") {
t.Fatalf("two plausible anchor blocks must be ambiguous, got %q", result.Output)
if result.Status != StatusError || !strings.Contains(result.Output, "Could not find the exact string") {
t.Fatalf("expected exact-match error, got %q", result.Output)
}
if after != initial {
t.Fatalf("file must be unchanged, got %q", after)
Expand Down
Loading