Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
32 changes: 32 additions & 0 deletions tests/tools/test_file_operations.py
Original file line number Diff line number Diff line change
Expand Up @@ -552,6 +552,38 @@ def side_effect(command, stdin_data=None, **kwargs):
assert result.success is True
assert state["content"] == "hi world\n", f"File not actually updated: {state['content']!r}"

def test_patch_replace_identical_old_new_reports_noop(self, mock_env):
"""Identical old/new replacement should be a successful no-op, not a fuzzy-match failure."""
state = {"content": "hello world\n"}
writes = []

def side_effect(command, stdin_data=None, **kwargs):
if command.startswith("cat >"):
writes.append(stdin_data)
state["content"] = stdin_data or ""
return {"output": "", "returncode": 0}
if command.startswith("cat "):
return {"output": state["content"], "returncode": 0}
if command.startswith("mkdir "):
return {"output": "", "returncode": 0}
if command.startswith("wc -c"):
return {"output": str(len(state["content"].encode())), "returncode": 0}
return {"output": "", "returncode": 0}

mock_env.execute.side_effect = side_effect
ops = ShellFileOperations(mock_env)
result = ops.patch_replace("/tmp/test/a.py", "hello", "hello")

assert result.success is True
assert result.noop is True
assert result.error is None
assert result.diff == ""
assert result.files_modified == []
assert result.message is not None
assert "old_string and new_string are identical" in result.message
assert writes == []
assert state["content"] == "hello world\n"

def test_patch_replace_fails_when_verify_read_errors(self, mock_env):
"""If the verify-read step itself fails (exit code != 0), return an error."""
call_count = {"cat": 0}
Expand Down
36 changes: 36 additions & 0 deletions tests/tools/test_patch_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,42 @@ def write_file(self, path, content):
' return result + 1'
)

def test_pure_context_update_reports_noop_without_writing(self):
"""A V4A update hunk with no effective change should be a clear no-op."""
patch = """\
*** Begin Patch
*** Update File: sample.py
@@ def run @@
def run():
return 1
*** End Patch"""
operations, err = parse_v4a_patch(patch)
assert err is None

class FakeFileOps:
written = False

def read_file_raw(self, path):
return SimpleNamespace(
content="def run():\n return 1\n",
error=None,
)

def write_file(self, path, content):
self.written = True
return SimpleNamespace(error=None)

file_ops = FakeFileOps()
result = apply_v4a_operations(operations, file_ops)

assert result.success is True
assert result.noop is True
assert result.message is not None
assert "No files modified" in result.message
assert result.files_modified == []
assert result.diff == ""
assert file_ops.written is False


class TestAdditionOnlyHunks:
"""Regression tests for #3081 — addition-only hunks were silently dropped."""
Expand Down
19 changes: 18 additions & 1 deletion tools/file_operations.py
Original file line number Diff line number Diff line change
Expand Up @@ -146,9 +146,11 @@ class PatchResult:
# See :class:`WriteResult.lsp_diagnostics`.
lsp_diagnostics: Optional[str] = None
error: Optional[str] = None
noop: bool = False
message: Optional[str] = None

def to_dict(self) -> dict:
result = {"success": self.success}
result: Dict[str, Any] = {"success": self.success}
if self.diff:
result["diff"] = self.diff
if self.files_modified:
Expand All @@ -163,6 +165,10 @@ def to_dict(self) -> dict:
result["lsp_diagnostics"] = self.lsp_diagnostics
if self.error:
result["error"] = self.error
if self.noop:
result["noop"] = True
if self.message:
result["message"] = self.message
return result


Expand Down Expand Up @@ -1014,6 +1020,17 @@ def patch_replace(self, path: str, old_string: str, new_string: str,
return PatchResult(error=f"Failed to read file: {path}")

content = read_result.stdout

if old_string == new_string:
return PatchResult(
success=True,
noop=True,
message=(
"No files modified: old_string and new_string are identical. "
"If you expected a change, re-read the file and provide a "
"replacement that differs from the matched text."
),
)

# Import and use fuzzy matching
from tools.fuzzy_match import fuzzy_find_and_replace
Expand Down
26 changes: 24 additions & 2 deletions tools/patch_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,14 @@ def _validate_operations(
replace_lines = [l.content for l in hunk.lines if l.prefix in {' ', '+'}]
replacement = '\n'.join(replace_lines)

if search_pattern == replacement:
# Pure-context/no-op hunk. Treat as valid without calling
# fuzzy_find_and_replace(), which deliberately reports
# identical old/new strings as a no-op. This keeps V4A
# patch validation from surfacing harmless no-op hunks as
# alarming validation failures.
continue

new_simulated, count, _strategy, match_error = fuzzy_find_and_replace(
simulated, search_pattern, replacement, replace_all=False
)
Expand Down Expand Up @@ -394,8 +402,9 @@ def apply_v4a_operations(operations: List[PatchOperation],
elif op.operation == OperationType.UPDATE:
result = _apply_update(op, file_ops)
if result[0]:
files_modified.append(op.file_path)
all_diffs.append(result[1])
if result[1]:
files_modified.append(op.file_path)
all_diffs.append(result[1])
else:
errors.append(f"Failed to update {op.file_path}: {result[1]}")

Expand Down Expand Up @@ -423,13 +432,19 @@ def apply_v4a_operations(operations: List[PatchOperation],
+ "\n".join(f" • {e}" for e in errors),
)

changed = bool(files_modified or files_created or files_deleted)
return PatchResult(
success=True,
diff=combined_diff,
files_modified=files_modified,
files_created=files_created,
files_deleted=files_deleted,
lint=lint_results if lint_results else None,
noop=not changed,
message=(
"No files modified: patch contained no effective changes. "
"If you expected a change, re-read the file and regenerate the patch."
) if not changed else None,
)


Expand Down Expand Up @@ -519,6 +534,10 @@ def _apply_update(op: PatchOperation, file_ops: Any) -> Tuple[bool, str]:
search_pattern = '\n'.join(search_lines)
replacement = '\n'.join(replace_lines)

if search_pattern == replacement:
# Pure-context/no-op hunk. Nothing to apply.
continue

new_content, count, _strategy, error = fuzzy_find_and_replace(
new_content, search_pattern, replacement, replace_all=False
)
Expand Down Expand Up @@ -575,6 +594,9 @@ def _apply_update(op: PatchOperation, file_ops: Any) -> Tuple[bool, str]:
else:
new_content = new_content.rstrip('\n') + '\n' + insert_text + '\n'

if new_content == current_content:
return True, ""

# Write new content
write_result = file_ops.write_file(op.file_path, new_content)
if write_result.error:
Expand Down