Houmeres/LLeMbas
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 3 Wiki Activity

Change part of a file without rewriting it

Browse Source 
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š
2026-08-03 11:05:36 +02:00
parent a58e48fce5
commit 3345df5b38
8 changed files with 970 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/lembas/services/agent/tools.py
+157 -9
View File
@@ -19,9 +19,10 @@ from __future__ import annotations
import json
import logging
import posixpath
from typing import Any
from lembas.services.agent import index, policy
from lembas.services.agent import index, patch, policy
from lembas.services.agent.base import ExecError, ExecRequest
from lembas.services.agent.session import AgentContext
from lembas.services.tools import (
@@ -171,6 +172,37 @@ def _timeout(raw: Any, agent: AgentContext) -> float:
# --- Files ---------------------------------------------------------------------
def _path_key(agent: AgentContext, path: str) -> str:
"""One name for one file, so `./a.py` and `a.py` are the same file.
Relative paths are resolved against the project directory, which is what the
executor does with them, so the two cannot disagree about what was read.
"""
if not posixpath.isabs(path) and agent.project_dir:
path = posixpath.join(agent.project_dir, path)
return posixpath.normpath(path)
async def _current(agent: AgentContext, path: str) -> tuple[str, bool]:
"""What is in the file now, and whether it is safe to diff against.
Best effort, and one extra SFTP round trip on every write -- see the note in
`_run_write`. A file that cannot be read and a file that does not exist are
the same thing over SFTP without a second trip for a stat, and both are
shown as a new file, which is what git does and is honest enough here.
Not diffable when the read came back at the ceiling: `read_file` truncates
and says so in the text rather than in a flag, so a file at `max_output` is
assumed truncated. Diffing a truncated original invents deletions of the
tail, which is worse than showing no diff at all.
"""
try:
text = await agent.executor().read_file(path, max_bytes=agent.max_output)
except ExecError:
return "", True
return text, len(text) < agent.max_output
async def _run_read(context: ToolContext, args: dict[str, Any]) -> ToolOutcome:
agent = _agent(context)
path = str(args.get("path") or "").strip()
@@ -187,6 +219,10 @@ async def _run_read(context: ToolContext, args: dict[str, Any]) -> ToolOutcome:
exc.message, _event("file_read", agent, path, status="error", error=exc.message)
)
# What makes `file_edit` possible: a patch may only be applied to something
# this reply has actually looked at.
agent.read_paths.add(_path_key(agent, path))
return ToolOutcome(
text or "(the file is empty)",
_event("file_read", agent, path, status="ok", text=text[:MAX_EVENT_CHARS]),
@@ -206,6 +242,13 @@ async def _run_write(context: ToolContext, args: dict[str, Any]) -> ToolOutcome:
if not isinstance(content, str):
content = "" if content is None else json.dumps(content, ensure_ascii=False)
# One extra SFTP round trip per write, on the hottest agent operation, and a
# conscious trade. It buys the transcript a real diff instead of "1284
# bytes" -- which is the difference between being able to see what an agent
# did and having to go and look -- and it counts as having read the file, so
# a write followed by an edit works in one reply.
before, diffable = await _current(agent, path)
try:
written = await agent.executor().write_file(path, content)
except ExecError as exc:
@@ -213,6 +256,8 @@ async def _run_write(context: ToolContext, args: dict[str, Any]) -> ToolOutcome:
exc.message, _event("file_write", agent, path, status="error", error=exc.message)
)
agent.read_paths.add(_path_key(agent, path))
# The tree just changed, and this process is what changed it. The listing's
# TTL is for drift nobody can see coming; leaving five more minutes of a
# listing known to be wrong makes a model conclude the file it has just
@@ -220,10 +265,80 @@ async def _run_write(context: ToolContext, args: dict[str, Any]) -> ToolOutcome:
if agent.profile_id:
index.forget_dir(agent.profile_id, agent.project_dir)
return ToolOutcome(
f"Wrote {written} bytes to {path}.",
_event("file_write", agent, path, status="ok", text=f"{written} bytes"),
)
event = _event("file_write", agent, path, status="ok", text=f"{written} bytes")
if diffable and before != content:
event["diff"] = patch.render(before, content, path, max_lines=MAX_DIFF_LINES)
return ToolOutcome(f"Wrote {written} bytes to {path}.", event)
async def _run_edit(context: ToolContext, args: dict[str, Any]) -> ToolOutcome:
"""Change part of a file by applying a unified diff.
The read-first requirement is the whole point. A patch written from memory
against a file the model has not looked at either fails on context -- the
good case -- or matches something it did not mean, and `file_write`'s
failure mode is worse still: it silently drops everything the model did not
happen to recall. Making the read compulsory turns "lost half the file" into
"was told to read it first".
"""
agent = _agent(context)
path = str(args.get("path") or "").strip()
if agent is None or not path:
return _no_connection_or_path("file_edit", agent, path)
if reason := _permitted(agent, "file_edit", RISK_WRITE):
return _refused("file_edit", agent, path, reason)
if _path_key(agent, path) not in agent.read_paths:
return ToolOutcome(
f"Read the file first! Nothing was written. Call file_read on {path} "
f"in this reply, then send a patch that matches what came back.",
_event("file_edit", agent, path, status="error", error="Not read yet."),
)
raw = args.get("patch")
if not isinstance(raw, str) or not raw.strip():
return ToolOutcome(
"No patch was given. Send a unified diff: one or more "
"`@@ -old,count +new,count @@` hunks.",
_event("file_edit", agent, path, status="error", error="No patch."),
)
before, diffable = await _current(agent, path)
try:
after = patch.apply(before, patch.parse(raw))
except patch.PatchError as exc:
# Returned, never raised: `run_tool`'s blanket catch would keep the
# model going but lose the detail, and the detail is what it retries
# from.
return ToolOutcome(
exc.message,
_event("file_edit", agent, path, status="error", error=exc.message[:200]),
)
if after == before:
return ToolOutcome(
f"That patch changes nothing in {path}. It is already as you want it.",
_event("file_edit", agent, path, status="ok", text="no change"),
)
try:
written = await agent.executor().write_file(path, after)
except ExecError as exc:
return ToolOutcome(
exc.message, _event("file_edit", agent, path, status="error", error=exc.message)
)
# Deliberately NOT index.forget_dir: an edit does not change the listing,
# because the file was already there. Forgetting it would cost the next
# reply either a wait on `INDEX_WAIT` or a turn with no listing at all, and
# buy nothing.
event = _event("file_edit", agent, path, status="ok", text=f"{written} bytes")
if diffable:
event["diff"] = patch.render(before, after, path, max_lines=MAX_DIFF_LINES)
return ToolOutcome(f"Updated {path} ({written} bytes).", event)
async def _run_list(context: ToolContext, args: dict[str, Any]) -> ToolOutcome:
@@ -363,9 +478,12 @@ def tool_defs(context: AgentContext | None = None) -> list[ToolDef]:
name="file_write",
family=FAMILY_AGENT,
description=(
"Write a text file, replacing it entirely if it already exists. "
"A relative path is taken from the project directory. Read a file "
"before rewriting it unless you are certain what is in it."
"Create a text file, or replace an existing one entirely. A "
"relative path is taken from the project directory. Use this for a "
"new file, or when you are rewriting the whole thing. To change "
"part of a file that already exists, use file_edit instead: it is "
"cheaper, and it cannot silently lose the parts you did not mean "
"to touch."
),
parameters={
"type": "object",
@@ -378,6 +496,36 @@ def tool_defs(context: AgentContext | None = None) -> list[ToolDef]:
run=_run_write,
risk=RISK_WRITE,
),
ToolDef(
name="file_edit",
family=FAMILY_AGENT,
description=(
"Change part of a text file by applying a unified diff. You must "
"have read the file with file_read in this same reply first, or "
"this is refused — a patch written from memory is how a change "
"quietly becomes a rewrite.\n"
"\n"
"Send an ordinary patch: one or more `@@ -old,count +new,count @@` "
"hunks, each with about three unchanged lines of context on either "
"side of the change, ' ' for context, '-' to remove and '+' to add. "
"The line numbers may be approximate — the context lines must be "
"exact. Nothing is written unless every hunk applies, and you are "
"told which one failed and what the file has there instead."
),
parameters={
"type": "object",
"properties": {
"path": {**_STRING, "description": "The file to change."},
"patch": {
**_STRING,
"description": "The unified diff to apply.",
},
},
"required": ["path", "patch"],
},
run=_run_edit,
risk=RISK_WRITE,
),
ToolDef(
name="file_list",
family=FAMILY_AGENT,
@@ -426,4 +574,4 @@ def tool_defs(context: AgentContext | None = None) -> list[ToolDef]:
return defs
__all__ = ["FAMILY_AGENT", "MAX_EVENT_CHARS", "tool_defs"]
__all__ = ["FAMILY_AGENT", "MAX_DIFF_LINES", "MAX_EVENT_CHARS", "tool_defs"]