Change part of a file without rewriting it
file_write replaces a file entirely, so a model wanting to change one line either rewrote the whole thing from memory -- silently dropping everything it did not happen to recall -- or shelled out to sed. file_edit takes a unified diff instead, and services/agent/patch.py applies it. Four behaviours carry that module, and each exists because of how models actually write patches rather than how the format is specified. Fuzzy offset, exact content. A hunk header is a hint: models count from a truncated read or from the file as it was three edits ago and get the numbers wrong, and get the context lines right. So the hinted position is tried, then the file is scanned outward for an exact match of the context block. One match wins; more than one refuses, because guessing between two identical blocks is the one failure that silently corrupts a file. Line endings are normalised in and restored out, or every hunk on a CRLF file fails on context that looks identical in the error message. A blank context line that lost its leading space is read as blank, because trailing whitespace is stripped by half the things a model's output passes through. And nothing is written unless every hunk applies: a half-applied file is worse than a refused one, and the model cannot tell the difference without reading it again. It refuses a file this reply has not read, in those words. A patch written from memory either fails on context -- the good case -- or matches something it did not mean. AgentContext.read_paths records what was read; it lives there because runners never see a Generation and a read path is a fact about the machine, and it is shared with the approved copy because as_approved is dataclasses.replace, which copies field references. It resets each reply, and that is right rather than a limitation: tool_calls_json is never replayed, so on the next turn the model does not have the contents either. Writes and edits both render a git-style diff in the transcript now, escaped like everything else there and bounded at write time -- a generated file's diff can be larger than the file, and it sits on the row forever. That costs file_write one extra SFTP round trip to read the old contents, on the hottest agent operation, and it is a conscious trade: it is the difference between seeing what an agent did and having to go and look. It earns its keep twice, because that read also counts as having read the file. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Jaroslav Beneš
@@ -206,6 +206,193 @@ async def test_files_are_written_and_read_back(db, user_id, machine, tmp_path):
|
||||
assert "note.txt" in listed.content
|
||||
|
||||
|
||||
# --- Changing part of a file -------------------------------------------------------
|
||||
def _context(db, user_id, machine, **kwargs):
|
||||
chat, _profile = _setup(db, user_id, machine, mode=policy.MODE_AUTO, **kwargs)
|
||||
user = db.get(User, user_id)
|
||||
resolved = tools_service.resolve_tools(db, chat, user)
|
||||
return tools_service.context_for(db, user, chat, tools=resolved)
|
||||
|
||||
|
||||
async def test_editing_a_file_that_was_not_read_is_refused(db, user_id, machine, tmp_path):
|
||||
"""Both halves matter. The wording is what the model acts on; that nothing
|
||||
was written is the actual guarantee."""
|
||||
(tmp_path / "project" / "note.txt").write_text("alpha\nbeta\n")
|
||||
context = _context(db, user_id, machine)
|
||||
|
||||
outcome = await tools_service.run_tool(
|
||||
context,
|
||||
"file_edit",
|
||||
_json.dumps({"path": "note.txt", "patch": "@@ -1,2 +1,2 @@\n alpha\n-beta\n+BETA\n"}),
|
||||
)
|
||||
|
||||
assert outcome.content.startswith("Read the file first!")
|
||||
assert outcome.event["status"] == "error"
|
||||
assert (tmp_path / "project" / "note.txt").read_text() == "alpha\nbeta\n"
|
||||
|
||||
|
||||
async def test_reading_then_editing_writes_the_new_text(db, user_id, machine, tmp_path):
|
||||
(tmp_path / "project" / "note.txt").write_text("alpha\nbeta\ngamma\n")
|
||||
context = _context(db, user_id, machine)
|
||||
|
||||
await tools_service.run_tool(context, "file_read", '{"path": "note.txt"}')
|
||||
outcome = await tools_service.run_tool(
|
||||
context,
|
||||
"file_edit",
|
||||
_json.dumps(
|
||||
{"path": "note.txt", "patch": "@@ -1,3 +1,3 @@\n alpha\n-beta\n+BETA\n gamma\n"}
|
||||
),
|
||||
)
|
||||
|
||||
assert outcome.event["status"] == "ok"
|
||||
assert (tmp_path / "project" / "note.txt").read_text() == "alpha\nBETA\ngamma\n"
|
||||
|
||||
|
||||
async def test_a_relative_and_an_absolute_path_are_the_same_file(db, user_id, machine, tmp_path):
|
||||
"""`./note.txt` read and `note.txt` edited has to count as having read it,
|
||||
or the check refuses the very thing it was meant to permit."""
|
||||
target = tmp_path / "project" / "note.txt"
|
||||
target.write_text("alpha\nbeta\n")
|
||||
context = _context(db, user_id, machine)
|
||||
|
||||
await tools_service.run_tool(context, "file_read", '{"path": "./note.txt"}')
|
||||
outcome = await tools_service.run_tool(
|
||||
context,
|
||||
"file_edit",
|
||||
_json.dumps({"path": str(target), "patch": "@@ -1,2 +1,2 @@\n alpha\n-beta\n+BETA\n"}),
|
||||
)
|
||||
|
||||
assert outcome.event["status"] == "ok", outcome.content
|
||||
|
||||
|
||||
async def test_a_failed_hunk_names_it_and_writes_nothing(db, user_id, machine, tmp_path):
|
||||
(tmp_path / "project" / "note.txt").write_text("alpha\nbeta\n")
|
||||
context = _context(db, user_id, machine)
|
||||
|
||||
await tools_service.run_tool(context, "file_read", '{"path": "note.txt"}')
|
||||
outcome = await tools_service.run_tool(
|
||||
context,
|
||||
"file_edit",
|
||||
_json.dumps({"path": "note.txt", "patch": "@@ -1,2 +1,2 @@\n nope\n-wrong\n+x\n"}),
|
||||
)
|
||||
|
||||
assert outcome.event["status"] == "error"
|
||||
assert "Hunk 1" in outcome.content
|
||||
assert (tmp_path / "project" / "note.txt").read_text() == "alpha\nbeta\n"
|
||||
|
||||
|
||||
async def test_a_write_counts_as_having_read_it(db, user_id, machine, tmp_path):
|
||||
"""`_run_write` reads the old content for its diff anyway, so write-then-edit
|
||||
works in one reply without a second round trip."""
|
||||
context = _context(db, user_id, machine)
|
||||
|
||||
await tools_service.run_tool(
|
||||
context, "file_write", '{"path": "new.txt", "content": "one\\ntwo\\n"}'
|
||||
)
|
||||
outcome = await tools_service.run_tool(
|
||||
context,
|
||||
"file_edit",
|
||||
_json.dumps({"path": "new.txt", "patch": "@@ -1,2 +1,2 @@\n one\n-two\n+TWO\n"}),
|
||||
)
|
||||
|
||||
assert outcome.event["status"] == "ok", outcome.content
|
||||
assert (tmp_path / "project" / "new.txt").read_text() == "one\nTWO\n"
|
||||
|
||||
|
||||
async def test_the_read_set_survives_an_approval(db, user_id, machine, tmp_path):
|
||||
"""`as_approved` is `dataclasses.replace`, which copies field *references*,
|
||||
so the set is shared with the per-call copy a runner actually gets. That is
|
||||
wanted, and it is not obvious enough to leave unpinned."""
|
||||
from dataclasses import replace
|
||||
|
||||
(tmp_path / "project" / "note.txt").write_text("alpha\nbeta\n")
|
||||
context = _context(db, user_id, machine)
|
||||
approved = replace(context, agent=context.agent.as_approved())
|
||||
|
||||
await tools_service.run_tool(approved, "file_read", '{"path": "note.txt"}')
|
||||
|
||||
assert context.agent.read_paths, "the read done under approval is not visible"
|
||||
|
||||
|
||||
async def test_an_edit_that_changes_nothing_says_so(db, user_id, machine, tmp_path):
|
||||
(tmp_path / "project" / "note.txt").write_text("alpha\nbeta\n")
|
||||
context = _context(db, user_id, machine)
|
||||
|
||||
await tools_service.run_tool(context, "file_read", '{"path": "note.txt"}')
|
||||
outcome = await tools_service.run_tool(
|
||||
context,
|
||||
"file_edit",
|
||||
_json.dumps({"path": "note.txt", "patch": "@@ -1,2 +1,2 @@\n alpha\n-beta\n+beta\n"}),
|
||||
)
|
||||
|
||||
assert outcome.event["status"] == "ok"
|
||||
assert "changes nothing" in outcome.content
|
||||
|
||||
|
||||
# --- The diff on the event ------------------------------------------------------------
|
||||
async def test_an_edit_carries_a_diff(db, user_id, machine, tmp_path):
|
||||
(tmp_path / "project" / "note.txt").write_text("alpha\nbeta\ngamma\n")
|
||||
context = _context(db, user_id, machine)
|
||||
|
||||
await tools_service.run_tool(context, "file_read", '{"path": "note.txt"}')
|
||||
outcome = await tools_service.run_tool(
|
||||
context,
|
||||
"file_edit",
|
||||
_json.dumps(
|
||||
{"path": "note.txt", "patch": "@@ -1,3 +1,3 @@\n alpha\n-beta\n+BETA\n gamma\n"}
|
||||
),
|
||||
)
|
||||
|
||||
diff = outcome.event["diff"]
|
||||
assert "-beta" in diff
|
||||
assert "+BETA" in diff
|
||||
|
||||
|
||||
async def test_writing_a_new_file_shows_it_as_all_additions(db, user_id, machine):
|
||||
"""Which is what git does, and the right display."""
|
||||
context = _context(db, user_id, machine)
|
||||
|
||||
outcome = await tools_service.run_tool(
|
||||
context, "file_write", '{"path": "fresh.txt", "content": "one\\ntwo\\n"}'
|
||||
)
|
||||
|
||||
body = [
|
||||
line
|
||||
for line in outcome.event["diff"].split("\n")
|
||||
if line and not line.startswith(("@@", "+++", "---"))
|
||||
]
|
||||
assert body and all(line.startswith("+") for line in body), body
|
||||
|
||||
|
||||
async def test_overwriting_a_file_shows_what_changed(db, user_id, machine, tmp_path):
|
||||
(tmp_path / "project" / "note.txt").write_text("alpha\nbeta\n")
|
||||
context = _context(db, user_id, machine)
|
||||
|
||||
outcome = await tools_service.run_tool(
|
||||
context, "file_write", '{"path": "note.txt", "content": "alpha\\nBETA\\n"}'
|
||||
)
|
||||
|
||||
assert "-beta" in outcome.event["diff"]
|
||||
assert "+BETA" in outcome.event["diff"]
|
||||
|
||||
|
||||
async def test_a_file_too_big_to_read_is_written_without_a_diff(db, user_id, machine, tmp_path):
|
||||
"""A truncated original would invent deletions of the tail, which is worse
|
||||
than showing no diff at all."""
|
||||
from lembas.services import settings_store as store
|
||||
|
||||
store.update(db, {"max_output_bytes": 1024}, key=store.AGENTS)
|
||||
(tmp_path / "project" / "big.txt").write_text("x" * 4000)
|
||||
context = _context(db, user_id, machine)
|
||||
|
||||
outcome = await tools_service.run_tool(
|
||||
context, "file_write", '{"path": "big.txt", "content": "small"}'
|
||||
)
|
||||
|
||||
assert outcome.event["status"] == "ok"
|
||||
assert "diff" not in outcome.event
|
||||
|
||||
|
||||
async def test_writing_a_file_drops_the_project_listing(db, user_id, machine):
|
||||
"""Otherwise the model is shown a five-minute-old tree that it knows is
|
||||
wrong, and concludes the file it has just created does not exist.
|
||||
|
||||
Reference in New Issue
Block a user