Merge pull request #103551 from NousResearch/fix/tool-output-caps
feat(file_tools): write_file flags a whole-file rewrite that mostly re-sends what is on disk (661 such writes, ≈$155, in one run)
This commit is contained in:
34
tests/tools/test_write_file_rewrite_hint.py
Normal file
34
tests/tools/test_write_file_rewrite_hint.py
Normal file
@@ -0,0 +1,34 @@
|
||||
"""write_file tells the caller when it just re-sent a large file that was already on disk.
|
||||
|
||||
In one 1,393-agent run 661 read->whole-file-rewrites of >20k-char files cost ~25M output chars (~$155)
|
||||
while `patch` (1.3k chars/call) succeeded 99.7% of the time; the tool result is the only place to say so.
|
||||
"""
|
||||
import json
|
||||
|
||||
from tools.file_tools import write_file_tool
|
||||
|
||||
|
||||
def _big(n_lines=800):
|
||||
return "\n".join(f"line {i}: " + "x" * 40 for i in range(n_lines)) + "\n" # ~40k chars
|
||||
|
||||
|
||||
def test_rewrite_of_a_large_file_with_few_changes_gets_a_patch_hint(tmp_path):
|
||||
f = tmp_path / "mod.py"
|
||||
old = _big()
|
||||
f.write_text(old, encoding="utf-8")
|
||||
new = old.replace("line 400:", "line 400 (edited):").replace("line 401:", "line 401 (edited):")
|
||||
r = json.loads(write_file_tool(str(f), new, task_id="t"))
|
||||
assert r.get("error") is None and f.read_text(encoding="utf-8") == new # the write still happens
|
||||
assert "use patch" in r["hint"] and "798 of 800 lines were already on disk" in r["hint"]
|
||||
|
||||
|
||||
def test_new_files_small_files_and_real_rewrites_get_no_hint(tmp_path):
|
||||
new_file = tmp_path / "new.py"
|
||||
assert "hint" not in json.loads(write_file_tool(str(new_file), _big(), task_id="t"))
|
||||
small = tmp_path / "small.py"
|
||||
small.write_text("a\nb\n", encoding="utf-8")
|
||||
assert "hint" not in json.loads(write_file_tool(str(small), "a\nc\n", task_id="t"))
|
||||
big = tmp_path / "big.py"
|
||||
big.write_text(_big(), encoding="utf-8")
|
||||
rewritten = "\n".join(f"other {i}: " + "y" * 40 for i in range(800)) + "\n"
|
||||
assert "hint" not in json.loads(write_file_tool(str(big), rewritten, task_id="t"))
|
||||
@@ -710,6 +710,52 @@ def _note_edited(task_id: str, paths: list[str], path_to_resolved: dict, session
|
||||
file_state.note_write(task_id, path_to_resolved[p])
|
||||
|
||||
|
||||
# Whole-file rewrite hint: an overwrite of an existing file this large whose new content keeps at least
|
||||
# this fraction of the old lines is a patch written the expensive way. In one 1,393-agent run 661 such
|
||||
# rewrites of >20k-char files cost ~25M output chars (~$155) where `patch` averaged 1.3k chars/call.
|
||||
_REWRITE_HINT_MIN_CHARS = 20_000
|
||||
_REWRITE_HINT_MIN_UNCHANGED = 0.80
|
||||
|
||||
|
||||
# Above this the line diff is skipped: SequenceMatcher on pathological repeated-line files is
|
||||
# quadratic (a 460 KB same-line file took ~22 s under the write lock).
|
||||
_REWRITE_HINT_MAX_CHARS = 400_000
|
||||
|
||||
|
||||
def _whole_file_rewrite_hint(task_id: str, resolved: str | None, new_content: str) -> str | None:
|
||||
"""Return a hint when ``new_content`` mostly re-sends what is already at ``resolved``.
|
||||
|
||||
Reads the OLD content through the task's own file ops (``read_file_raw``, the sandbox/remote
|
||||
backend the write targets), never the host path: on a remote backend the host file is a
|
||||
different file, and a host FIFO at that path would block the write lock forever. Bounded size
|
||||
and a line multiset comparison (linear) instead of a sequence diff (quadratic on repeated lines)."""
|
||||
if not resolved or not (_REWRITE_HINT_MIN_CHARS <= len(new_content) <= _REWRITE_HINT_MAX_CHARS):
|
||||
return None
|
||||
try:
|
||||
result = _get_file_ops(task_id).read_file_raw(resolved)
|
||||
old = getattr(result, "content", None)
|
||||
if getattr(result, "error", None) or not isinstance(old, str):
|
||||
return None
|
||||
except Exception:
|
||||
return None
|
||||
if not (_REWRITE_HINT_MIN_CHARS <= len(old) <= _REWRITE_HINT_MAX_CHARS):
|
||||
return None
|
||||
old_lines, new_lines = old.splitlines(), new_content.splitlines()
|
||||
if not old_lines:
|
||||
return None
|
||||
from collections import Counter
|
||||
unchanged = sum((Counter(old_lines) & Counter(new_lines)).values())
|
||||
ratio = unchanged / max(len(old_lines), len(new_lines))
|
||||
if ratio < _REWRITE_HINT_MIN_UNCHANGED:
|
||||
return None
|
||||
changed = max(len(old_lines), len(new_lines)) - unchanged
|
||||
return (
|
||||
f"{unchanged:,} of {len(new_lines):,} lines were already on disk ({ratio:.0%} unchanged); ~{changed:,} "
|
||||
f"line(s) actually changed. Re-sending a {len(new_content):,}-char file costs output tokens for every "
|
||||
"unchanged line; for edits like this use patch (old_string/new_string), which sends only the changed region."
|
||||
)
|
||||
|
||||
|
||||
def write_file_tool(path: str, content: str, task_id: str = "default",
|
||||
cross_profile: bool = False,
|
||||
session_id: str | None = None) -> str:
|
||||
@@ -742,9 +788,12 @@ def write_file_tool(path: str, content: str, task_id: str = "default",
|
||||
# subagents; different paths stay fully parallel.
|
||||
_lock.enter_context(file_state.lock_path(_resolved))
|
||||
warnings = _edit_warnings([path], path_to_resolved, task_id)
|
||||
rewrite_hint = _whole_file_rewrite_hint(task_id, _resolved, content)
|
||||
result_dict = _get_file_ops(task_id).write_file(_resolved or path, content).to_dict()
|
||||
if warnings:
|
||||
result_dict["_warning"] = warnings[0]
|
||||
if rewrite_hint and not result_dict.get("error"):
|
||||
result_dict["hint"] = rewrite_hint
|
||||
if _resolved:
|
||||
# Always report the ABSOLUTE path written so a wrong-cwd mismatch
|
||||
# is visible in the response instead of silently landing elsewhere.
|
||||
|
||||
Reference in New Issue
Block a user