fix(tools): make the empty/whitespace old_string rejection actionable
Port from cline/cline#13970: models that send patch calls with an empty old_string got back 'old_string cannot be empty' — an error that names the problem but not the recovery, so the next call was byte-identical and the run burned turns until loop detection killed it (upstream repro: Kimi K3 looping on old_text: null). The rejection now states the recovery: set old_string to the exact text the replacement should replace, read the file first if unsure, use write_file for new files/full rewrites, and do not re-send the call unchanged. The whitespace-only rejection gets the same treatment. No behavior change for valid calls.
This commit is contained in:
@@ -21,9 +21,13 @@ class TestExactMatch:
|
||||
assert new == content # untouched
|
||||
|
||||
def test_empty_old_string_rejected(self):
|
||||
"""The rejection must carry a recovery path — a bare "cannot be empty"
|
||||
leaves models re-sending the identical call until the loop detector
|
||||
kills the run (cline/cline#13970)."""
|
||||
new, count, _, err = fuzzy_find_and_replace("abc", "", "x")
|
||||
assert count == 0
|
||||
assert err is not None
|
||||
assert "read the file" in err and "write_file" in err
|
||||
|
||||
def test_identical_strings(self):
|
||||
new, count, _, err = fuzzy_find_and_replace("abc", "abc", "abc")
|
||||
|
||||
@@ -342,11 +342,21 @@ def fuzzy_find_and_replace(content: str, old_string: str, new_string: str,
|
||||
``(content, 0, None, error)``.
|
||||
"""
|
||||
if not old_string:
|
||||
return content, 0, None, "old_string cannot be empty"
|
||||
# Actionable recovery text: a terse "cannot be empty" leaves the model
|
||||
# re-sending the identical call until the loop detector kills the run
|
||||
# (upstream report: cline/cline#13970 — Kimi K3 looped on old_text: null).
|
||||
return content, 0, None, (
|
||||
"old_string is empty — nothing to match. Set old_string to the exact "
|
||||
"existing text the replacement should replace (read the file first if "
|
||||
"unsure). To create a new file or fully rewrite one, use write_file "
|
||||
"instead. Do not re-send this call unchanged.")
|
||||
if not old_string.strip():
|
||||
# Whitespace-only anchors match trivially and mass-replace or
|
||||
# ambiguity-error; never meaningful.
|
||||
return content, 0, None, "old_string is only whitespace — provide non-blank text to match"
|
||||
return content, 0, None, (
|
||||
"old_string is only whitespace — provide non-blank text to match. Set it "
|
||||
"to the exact existing text the replacement should replace (read the file "
|
||||
"first if unsure). Do not re-send this call unchanged.")
|
||||
if old_string == new_string:
|
||||
return content, 0, None, IDENTICAL_STRINGS_ERROR
|
||||
|
||||
|
||||
Reference in New Issue
Block a user