Grayscale/nanobot
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(tools): enforce edit_file line hints

Browse Source
This commit is contained in:
chengyongru
2026-07-11 01:14:50 +08:00
committed by Xubin Ren
parent 052fdc132d
commit 7675364eae
3 changed files with 48 additions and 406 deletions
Download Patch File Download Diff File Expand all files Collapse all files
nanobot/agent/tools/filesystem.py
+26 -234
View File
@@ -626,82 +626,8 @@ def _match_end_line(match: _MatchSpan) -> int:
return match.line + comparable.count("\n")
def _line_distance_to_match(match: _MatchSpan, line: int) -> int:
end_line = _match_end_line(match)
if match.line <= line <= end_line:
return 0
if line < match.line:
return match.line - line
return line - end_line
def _format_match_locations(matches: list[_MatchSpan], *, max_items: int = 5) -> str:
locations: list[str] = []
for match in matches[:max_items]:
end_line = _match_end_line(match)
if end_line == match.line:
locations.append(f"line {match.line}")
else:
locations.append(f"lines {match.line}-{end_line}")
if len(matches) > max_items:
locations.append("...")
return ", ".join(locations)
def _line_guard_error(
*,
guard_name: str,
guard_line: int,
matches: list[_MatchSpan],
max_distance: int,
) -> str:
nearest = min(matches, key=lambda match: _line_distance_to_match(match, guard_line))
distance = _line_distance_to_match(nearest, guard_line)
return ToolResult.error(
f"Error: {guard_name} {guard_line} does not match the old_text location. "
f"old_text appears at {_format_match_locations(matches)}. "
f"Nearest distance is {distance} line(s); allowed distance is {max_distance}. "
"Re-read the intended region and copy old_text from the target line, "
"or adjust the line guard."
)
def _start_line_guard_error(
*,
guard_line: int,
matches: list[_MatchSpan],
) -> str:
nearest = min(matches, key=lambda match: abs(match.line - guard_line))
distance = abs(nearest.line - guard_line)
return ToolResult.error(
f"Error: target_start_line {guard_line} does not match the old_text start. "
f"old_text appears at {_format_match_locations(matches)}. "
f"Nearest start-line distance is {distance} line(s). "
"Re-read the intended region and copy old_text starting at the target block line."
)
def _matches_for_line_guards(
matches: list[_MatchSpan],
*,
target_line: int | None,
target_line_window: int,
target_start_line: int | None,
) -> list[_MatchSpan]:
candidates = matches
if target_line is not None:
candidates = [
match
for match in candidates
if _line_distance_to_match(match, target_line) <= target_line_window
]
if target_start_line is not None:
candidates = [
match
for match in candidates
if match.line == target_start_line
]
return candidates
def _match_covers_line(match: _MatchSpan, line: int) -> bool:
return match.line <= line <= _match_end_line(match)
def _find_exact_matches(content: str, old_text: str) -> list[_MatchSpan]:
@@ -878,44 +804,8 @@ def _find_match(content: str, old_text: str) -> tuple[str | None, int]:
line_hint=IntegerSchema(
1,
description=(
"Optional 1-based line hint copied from read_file. Used to choose "
"the nearest match and reject matches outside line_hint_window."
),
minimum=1,
nullable=True,
),
line_hint_window=IntegerSchema(
5,
description=(
"Maximum line distance allowed between line_hint and the selected "
"old_text match (default 5)."
),
minimum=0,
nullable=True,
),
target_line=IntegerSchema(
1,
description=(
"Optional 1-based line from read_file that the selected old_text "
"match must cover. Prefer this when editing a specific numbered line."
),
minimum=1,
nullable=True,
),
target_line_window=IntegerSchema(
0,
description=(
"Maximum line distance allowed around target_line (default 0, "
"meaning old_text must cover target_line)."
),
minimum=0,
nullable=True,
),
target_start_line=IntegerSchema(
1,
description=(
"Optional 1-based line where the selected old_text match must start. "
"Use this for block edits that must begin at a specific numbered line."
"Optional exact 1-based target line copied from read_file. "
"The selected old_text match must cover this line."
),
minimum=1,
nullable=True,
@@ -935,7 +825,6 @@ class EditFileTool(_FsTool):
_MAX_EDIT_FILE_SIZE = 1024 * 1024 * 1024 # 1 GiB
_MARKDOWN_EXTS = frozenset({".md", ".mdx", ".markdown"})
_DEFAULT_LINE_HINT_WINDOW = 5
@property
def name(self) -> str:
@@ -949,9 +838,8 @@ class EditFileTool(_FsTool):
"with old_text copied from read_file. For multi-file, structural, "
"or generated code edits, prefer apply_patch. If old_text matches "
"multiple times, provide more context or set occurrence, line_hint, "
"target_line, target_start_line, replace_all, and expected_replacements. "
"Prefer target_line when editing from numbered read_file output. Use "
"target_start_line when a block edit must start at a specific line. "
"replace_all, and expected_replacements. When editing from numbered "
"read_file output, set line_hint to the exact target line. "
"Shows closest-match diagnostics on failure."
)
@@ -964,10 +852,7 @@ class EditFileTool(_FsTool):
self, path: str | None = None, old_text: str | None = None,
new_text: str | None = None,
replace_all: bool = False, occurrence: int | None = None,
line_hint: int | None = None, line_hint_window: int | None = None,
target_line: int | None = None, target_line_window: int | None = None,
target_start_line: int | None = None,
expected_replacements: int | None = None, **kwargs: Any,
line_hint: int | None = None, expected_replacements: int | None = None, **kwargs: Any,
) -> str:
try:
if not path:
@@ -980,22 +865,6 @@ class EditFileTool(_FsTool):
return ToolResult.error("Error: occurrence must be >= 1.")
if line_hint is not None and line_hint < 1:
return ToolResult.error("Error: line_hint must be >= 1.")
if line_hint_window is not None and line_hint_window < 0:
return ToolResult.error("Error: line_hint_window must be >= 0.")
if target_line is not None and target_line < 1:
return ToolResult.error("Error: target_line must be >= 1.")
if target_line_window is not None and target_line_window < 0:
return ToolResult.error("Error: target_line_window must be >= 0.")
if target_start_line is not None and target_start_line < 1:
return ToolResult.error("Error: target_start_line must be >= 1.")
if line_hint_window is not None and line_hint is None:
return ToolResult.error("Error: line_hint_window requires line_hint.")
if target_line_window is not None and target_line is None:
return ToolResult.error("Error: target_line_window requires target_line.")
if line_hint is not None and target_line is not None:
return ToolResult.error("Error: line_hint cannot be used with target_line.")
if line_hint is not None and target_start_line is not None:
return ToolResult.error("Error: line_hint cannot be used with target_start_line.")
if expected_replacements is not None and expected_replacements < 1:
return ToolResult.error("Error: expected_replacements must be >= 1.")
@@ -1044,65 +913,14 @@ class EditFileTool(_FsTool):
return ToolResult.error("Error: occurrence cannot be used with replace_all=true.")
if replace_all and line_hint is not None:
return ToolResult.error("Error: line_hint cannot be used with replace_all=true.")
if replace_all and target_line is not None:
return ToolResult.error("Error: target_line cannot be used with replace_all=true.")
if replace_all and target_start_line is not None:
return ToolResult.error("Error: target_start_line cannot be used with replace_all=true.")
if occurrence is not None and line_hint is not None:
return ToolResult.error("Error: line_hint cannot be used with occurrence.")
if occurrence is not None and target_line is not None:
return ToolResult.error("Error: target_line cannot be used with occurrence.")
if occurrence is not None and target_start_line is not None:
return ToolResult.error("Error: target_start_line cannot be used with occurrence.")
if count > 1 and not replace_all:
if occurrence is not None:
if occurrence > count:
if occurrence is not None and occurrence > count:
return ToolResult.error(
f"Error: occurrence {occurrence} is out of range; "
f"old_text appears {count} times."
f"old_text appears {count} time(s)."
)
elif line_hint is not None:
nearest = min(matches, key=lambda match: _line_distance_to_match(match, line_hint))
distance = _line_distance_to_match(nearest, line_hint)
if sum(
1 for match in matches
if _line_distance_to_match(match, line_hint) == distance
) > 1:
return ToolResult.error(
f"Error: line_hint {line_hint} is ambiguous; "
f"old_text appears {count} times."
)
elif target_line is not None or target_start_line is not None:
target_window = target_line_window if target_line_window is not None else 0
candidates = _matches_for_line_guards(
matches,
target_line=target_line,
target_line_window=target_window,
target_start_line=target_start_line,
)
if not candidates:
if target_start_line is not None:
start_candidates = [
match for match in matches if match.line == target_start_line
]
if not start_candidates:
return _start_line_guard_error(
guard_line=target_start_line,
matches=matches,
)
return _line_guard_error(
guard_name="target_line",
guard_line=target_line or 1,
matches=matches,
max_distance=target_window,
)
if len(candidates) > 1:
return ToolResult.error(
f"Error: target_line {target_line} is ambiguous; "
f"old_text appears {count} times at "
f"{_format_match_locations(candidates)}."
)
else:
if count > 1 and not replace_all and occurrence is None and line_hint is None:
line_numbers = [match.line for match in matches]
preview = ", ".join(f"line {n}" for n in line_numbers[:3])
if len(line_numbers) > 3:
@@ -1113,11 +931,6 @@ class EditFileTool(_FsTool):
"Provide more context, set occurrence to choose one match, "
"or set replace_all=true."
)
elif occurrence is not None and occurrence > count:
return ToolResult.error(
f"Error: occurrence {occurrence} is out of range; "
f"old_text appears {count} time."
)
norm_new = new_text.replace("\r\n", "\n")
@@ -1127,48 +940,27 @@ class EditFileTool(_FsTool):
if replace_all:
selected = matches
elif target_line is not None or target_start_line is not None:
target_window = target_line_window if target_line_window is not None else 0
candidates = _matches_for_line_guards(
matches,
target_line=target_line,
target_line_window=target_window,
target_start_line=target_start_line,
)
elif occurrence is not None:
selected = [matches[occurrence - 1]]
elif line_hint is not None:
candidates = [match for match in matches if _match_covers_line(match, line_hint)]
if not candidates:
if target_start_line is not None:
start_candidates = [
match for match in matches if match.line == target_start_line
]
if not start_candidates:
return _start_line_guard_error(
guard_line=target_start_line,
matches=matches,
locations = ", ".join(f"line {match.line}" for match in matches[:3])
if len(matches) > 3:
locations += ", ..."
return ToolResult.error(
f"Error: line_hint {line_hint} does not match the old_text location. "
f"old_text appears at {locations}. Re-read the intended region and "
"copy old_text that covers the target line."
)
return _line_guard_error(
guard_name="target_line",
guard_line=target_line or 1,
matches=matches,
max_distance=target_window,
if len(candidates) > 1:
return ToolResult.error(
f"Error: line_hint {line_hint} is ambiguous; "
f"old_text appears {len(candidates)} times on that line."
)
selected = candidates
elif line_hint is not None:
nearest = min(matches, key=lambda match: _line_distance_to_match(match, line_hint))
window = (
self._DEFAULT_LINE_HINT_WINDOW
if line_hint_window is None
else line_hint_window
)
if _line_distance_to_match(nearest, line_hint) > window:
return _line_guard_error(
guard_name="line_hint",
guard_line=line_hint,
matches=matches,
max_distance=window,
)
selected = [nearest]
else:
selected = [matches[occurrence - 1 if occurrence else 0]]
selected = [matches[0]]
if expected_replacements is not None and len(selected) != expected_replacements:
return ToolResult.error(
f"Error: expected {expected_replacements} replacements but "
nanobot/templates/agent/tool_contract.md
+1 -1
View File
@@ -26,7 +26,7 @@ Tool signatures are provided automatically via function calling. This section do
- For code or config changes, the default loop is: locate (`find_files`/`grep`), inspect (`read_file`), edit (`apply_patch`), then verify (`exec` or re-read).
- Use `apply_patch` as the default code editing tool, especially for multi-file changes, structural edits, generated code, moves, adds, or deletes.
- Use `apply_patch dry_run=true` when the patch is uncertain and you want validation plus a change summary before writing.
- Use `edit_file` only for small exact replacements in one file, with `old_text` copied from `read_file`; when editing a specific numbered line, pass `target_line`; when a block must start at a specific line, also pass `target_start_line`; add `occurrence`, `line_hint`, or `expected_replacements` when ambiguity matters.
- Use `edit_file` only for small exact replacements in one file, with `old_text` copied from `read_file`; when editing a specific numbered line, pass that exact line as `line_hint`; add `occurrence` or `expected_replacements` when ambiguity matters.
- Use `write_file` for new files or intentional full-file rewrites, not routine partial edits.
- If `apply_patch` or `edit_file` fails, re-read with `force=true`, narrow the context, and try a smaller patch rather than switching to shell `sed` or `echo`.
tests/tools/test_file_edit_coding_enhancements.py
+10 -160
View File
@@ -70,7 +70,7 @@ def test_edit_file_expected_replacements_allows_replace_all_when_count_matches(t
assert target.read_text() == "changed\nchanged\n"
def test_edit_file_can_select_nearest_line_hint(tmp_path):
def test_edit_file_line_hint_selects_matching_occurrence(tmp_path):
target = tmp_path / "duplicate.txt"
target.write_text("one\nsame\ntwo\nsame\n")
tool = EditFileTool(workspace=tmp_path)
@@ -86,7 +86,7 @@ def test_edit_file_can_select_nearest_line_hint(tmp_path):
assert target.read_text() == "one\nsame\ntwo\nchanged\n"
def test_edit_file_rejects_unique_match_far_from_line_hint(tmp_path):
def test_edit_file_rejects_unique_match_outside_line_hint(tmp_path):
target = tmp_path / "wrong-line.txt"
target.write_text("one\nsame\ntwo\nother\n")
tool = EditFileTool(workspace=tmp_path)
@@ -95,48 +95,15 @@ def test_edit_file_rejects_unique_match_far_from_line_hint(tmp_path):
path=str(target),
old_text="same",
new_text="changed",
line_hint=20,
line_hint=4,
))
assert "line_hint 20 does not match the old_text location" in result
assert "line_hint 4 does not match the old_text location" in result
assert "old_text appears at line 2" in result
assert target.read_text() == "one\nsame\ntwo\nother\n"
def test_edit_file_target_line_selects_matching_occurrence(tmp_path):
target = tmp_path / "duplicate.txt"
target.write_text("one\nsame\ntwo\nsame\n")
tool = EditFileTool(workspace=tmp_path)
result = asyncio.run(tool.execute(
path=str(target),
old_text="same",
new_text="changed",
target_line=4,
))
assert "Successfully edited" in result
assert target.read_text() == "one\nsame\ntwo\nchanged\n"
def test_edit_file_target_line_rejects_unique_match_on_wrong_line(tmp_path):
target = tmp_path / "wrong-line.txt"
target.write_text("one\nsame\ntwo\nother\n")
tool = EditFileTool(workspace=tmp_path)
result = asyncio.run(tool.execute(
path=str(target),
old_text="same",
new_text="changed",
target_line=4,
))
assert "target_line 4 does not match the old_text location" in result
assert "old_text appears at line 2" in result
assert target.read_text() == "one\nsame\ntwo\nother\n"
def test_edit_file_target_line_can_cover_multiline_match(tmp_path):
def test_edit_file_line_hint_can_cover_multiline_match(tmp_path):
target = tmp_path / "block.txt"
target.write_text("before\nstart\nmiddle\nend\nafter\n")
tool = EditFileTool(workspace=tmp_path)
@@ -145,64 +112,13 @@ def test_edit_file_target_line_can_cover_multiline_match(tmp_path):
path=str(target),
old_text="start\nmiddle\nend",
new_text="start\nchanged\nend",
target_line=3,
line_hint=3,
))
assert "Successfully edited" in result
assert target.read_text() == "before\nstart\nchanged\nend\nafter\n"
def test_edit_file_target_start_line_rejects_context_that_starts_too_early(tmp_path):
target = tmp_path / "block.txt"
target.write_text("before\nstart\nmiddle\nend\nafter\n")
tool = EditFileTool(workspace=tmp_path)
result = asyncio.run(tool.execute(
path=str(target),
old_text="before\nstart\nmiddle",
new_text="before\nchanged\nmiddle",
target_line=2,
target_start_line=2,
))
assert "target_start_line 2 does not match the old_text start" in result
assert target.read_text() == "before\nstart\nmiddle\nend\nafter\n"
def test_edit_file_target_start_line_allows_exact_block_start(tmp_path):
target = tmp_path / "block.txt"
target.write_text("before\nstart\nmiddle\nend\nafter\n")
tool = EditFileTool(workspace=tmp_path)
result = asyncio.run(tool.execute(
path=str(target),
old_text="start\nmiddle\nend",
new_text="start\nchanged\nend",
target_line=3,
target_start_line=2,
))
assert "Successfully edited" in result
assert target.read_text() == "before\nstart\nchanged\nend\nafter\n"
def test_edit_file_target_line_error_wins_when_start_line_matches(tmp_path):
target = tmp_path / "block.txt"
target.write_text("before\nstart\nmiddle\nend\nafter\n")
tool = EditFileTool(workspace=tmp_path)
result = asyncio.run(tool.execute(
path=str(target),
old_text="start\nmiddle",
new_text="changed\nmiddle",
target_line=4,
target_start_line=2,
))
assert "target_line 4 does not match the old_text location" in result
assert target.read_text() == "before\nstart\nmiddle\nend\nafter\n"
def test_edit_file_can_edit_ipynb_as_json(tmp_path):
target = tmp_path / "analysis.ipynb"
target.write_text('{"cells": []}')
@@ -236,18 +152,18 @@ def test_edit_file_multiple_match_hint_mentions_occurrence(tmp_path):
def test_edit_file_rejects_ambiguous_line_hint(tmp_path):
target = tmp_path / "duplicate.txt"
target.write_text("same\nmiddle\nsame\n")
target.write_text("same same\n")
tool = EditFileTool(workspace=tmp_path)
result = asyncio.run(tool.execute(
path=str(target),
old_text="same",
new_text="changed",
line_hint=2,
line_hint=1,
))
assert "line_hint 2 is ambiguous" in result
assert target.read_text() == "same\nmiddle\nsame\n"
assert "line_hint 1 is ambiguous" in result
assert target.read_text() == "same same\n"
def test_edit_file_rejects_occurrence_with_replace_all(tmp_path):
@@ -301,40 +217,6 @@ def test_edit_file_rejects_line_hint_with_occurrence(tmp_path):
assert target.read_text() == "same\nsame\n"
def test_edit_file_rejects_target_line_with_occurrence(tmp_path):
target = tmp_path / "duplicate.txt"
target.write_text("same\nsame\n")
tool = EditFileTool(workspace=tmp_path)
result = asyncio.run(tool.execute(
path=str(target),
old_text="same",
new_text="changed",
occurrence=1,
target_line=1,
))
assert "target_line cannot be used with occurrence" in result
assert target.read_text() == "same\nsame\n"
def test_edit_file_rejects_target_start_line_with_occurrence(tmp_path):
target = tmp_path / "duplicate.txt"
target.write_text("same\nsame\n")
tool = EditFileTool(workspace=tmp_path)
result = asyncio.run(tool.execute(
path=str(target),
old_text="same",
new_text="changed",
occurrence=1,
target_start_line=1,
))
assert "target_start_line cannot be used with occurrence" in result
assert target.read_text() == "same\nsame\n"
def test_edit_file_rejects_zero_occurrence(tmp_path):
target = tmp_path / "duplicate.txt"
target.write_text("same\n")
@@ -365,35 +247,3 @@ def test_edit_file_rejects_zero_line_hint(tmp_path):
assert "line_hint must be >= 1" in result
assert target.read_text() == "same\n"
def test_edit_file_rejects_zero_target_line(tmp_path):
target = tmp_path / "duplicate.txt"
target.write_text("same\n")
tool = EditFileTool(workspace=tmp_path)
result = asyncio.run(tool.execute(
path=str(target),
old_text="same",
new_text="changed",
target_line=0,
))
assert "target_line must be >= 1" in result
assert target.read_text() == "same\n"
def test_edit_file_rejects_zero_target_start_line(tmp_path):
target = tmp_path / "duplicate.txt"
target.write_text("same\n")
tool = EditFileTool(workspace=tmp_path)
result = asyncio.run(tool.execute(
path=str(target),
old_text="same",
new_text="changed",
target_start_line=0,
))
assert "target_start_line must be >= 1" in result
assert target.read_text() == "same\n"