From 3345df5b3819afc9d41dbbe470b00bc88ea4cc1e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaroslav=20Bene=C5=A1?= Date: Mon, 3 Aug 2026 11:05:36 +0200 Subject: [PATCH] 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) --- src/lembas/services/agent/patch.py | 286 ++++++++++++++++++ src/lembas/services/agent/session.py | 20 ++ src/lembas/services/agent/tools.py | 166 +++++++++- src/lembas/web/static/css/chat.css | 33 ++ .../web/templates/chat/_tool_activity.html | 35 +++ tests/test_agent_patch.py | 195 ++++++++++++ tests/test_agent_tools.py | 187 ++++++++++++ tests/test_tool_activity.py | 57 ++++ 8 files changed, 970 insertions(+), 9 deletions(-) create mode 100644 src/lembas/services/agent/patch.py create mode 100644 tests/test_agent_patch.py diff --git a/src/lembas/services/agent/patch.py b/src/lembas/services/agent/patch.py new file mode 100644 index 0000000..020a1c2 --- /dev/null +++ b/src/lembas/services/agent/patch.py @@ -0,0 +1,286 @@ +"""Applying a unified diff, and rendering one. + +`difflib` produces a unified diff and cannot apply one, so `render` uses it and +`apply` is written here. No new dependency: hard rule 1 is about the browser, +but a patch applier is fifty lines and pulling a package in for it would be +worse than the fifty lines. + +Four behaviours carry the whole module, and each of them exists because of how +models actually write patches rather than how the format is specified. + +**Fuzzy offset, exact content.** A hunk's `@@ -41,7 +41,8 @@` is a hint and +nothing more. Models get line numbers wrong constantly -- they count from a +truncated read, or from the file as it was three edits ago -- and get the +context lines right. So the hinted position is tried first and then the file is +scanned outward for an exact match of the context block. One match wins; more +than one refuses, because guessing which of two identical blocks was meant is +the one failure that silently corrupts a file. + +**Line endings are normalised in and restored out.** A CRLF file otherwise +fails on every single hunk, on context that looks identical in the error +message, which is unfixable from the model's side. + +**A blank context line may have lost its leading space.** Trailing whitespace +is stripped by half the things a model's output passes through, so `""` is read +as a blank context line rather than as a malformed one. + +**Nothing is written unless every hunk applies.** The new text is built whole in +memory and handed back; a half-applied file is worse than a refused one, and the +model cannot tell the difference without reading it again. +""" + +from __future__ import annotations + +import difflib +import re +from dataclasses import dataclass + +# A patch bigger than this is a rewrite wearing a diff's clothes, and +# `file_write` is the tool for that. +MAX_HUNKS = 60 + +# How far either side of the hinted line to look for the context block. Wide +# enough for a file that has grown a few hundred lines since the model read it, +# narrow enough that an accidental match is unlikely. +MAX_DRIFT = 200 + +_HEADER = re.compile(r"^@@\s*-(\d+)(?:,(\d+))?\s+\+(\d+)(?:,(\d+))?\s*@@") +_NO_NEWLINE = "\\ No newline at end of file" + + +class PatchError(Exception): + """A patch that did not apply, said precisely enough to retry from.""" + + def __init__(self, message: str, *, hunk: int = 0) -> None: + super().__init__(message) + self.message = message + self.hunk = hunk + + +@dataclass(frozen=True) +class Hunk: + old_start: int + old_count: int + new_start: int + new_count: int + # Each line still carrying its ' ', '+' or '-'. + lines: tuple[str, ...] + # A `\ No newline at end of file` marker followed a line this hunk *adds*, + # so the result is meant to end without one. Honoured only when the hunk + # actually reaches the end of the file -- git emits the marker for the old + # side too, and reading that as an instruction would strip a newline the + # patch never touched. + ends_without_newline: bool = False + + @property + def before(self) -> tuple[str, ...]: + """The lines this hunk expects to find, without their markers.""" + return tuple(line[1:] for line in self.lines if line[:1] in (" ", "-")) + + @property + def after(self) -> tuple[str, ...]: + return tuple(line[1:] for line in self.lines if line[:1] in (" ", "+")) + + +def parse(patch: str) -> list[Hunk]: + """Read a unified diff into hunks. + + File headers are tolerated and ignored -- `diff --git`, `index`, `---`, + `+++` -- because models emit them by habit and refusing would cost a round + trip to say so. The `@@` header is required: without one there is nothing to + anchor against, and the resulting error is at least mechanical to fix. + """ + hunks: list[Hunk] = [] + state: dict = {"header": None, "body": [], "bare": False} + + def flush() -> None: + if state["header"] is None: + return + hunks.append( + Hunk( + *state["header"], + lines=tuple(state["body"]), + ends_without_newline=state["bare"], + ) + ) + state["header"] = None + state["body"] = [] + state["bare"] = False + + body = (patch or "").replace("\r\n", "\n").replace("\r", "\n").split("\n") + # The patch's own final newline, not a blank context line. Without this every + # well-formed patch acquires one phantom line of context at the end and + # matches nothing -- which looks exactly like the model getting it wrong. + if body and body[-1] == "": + body.pop() + + for raw in body: + matched = _HEADER.match(raw) + if matched: + flush() + state["header"] = ( + int(matched.group(1)), + int(matched.group(2) or 1), + int(matched.group(3)), + int(matched.group(4) or 1), + ) + continue + + if state["header"] is None: + # Preamble. Anything before the first @@ is a file header we do not + # need: the path is a parameter, not something read out of the diff. + continue + + if raw.startswith(_NO_NEWLINE): + # It describes whichever side the line above belonged to. Only the + # new side is an instruction; the old side is a description of the + # file we are about to read for ourselves. + if state["body"] and state["body"][-1][:1] in ("+", " "): + state["bare"] = True + continue + if raw[:1] in ("+", "-", " "): + state["body"].append(raw) + elif raw == "": + # A blank line that lost its leading space. Common enough to be the + # normal case rather than an exceptional one. + state["body"].append(" ") + else: + # A stray line inside a hunk -- a second `diff --git`, a signature. + # Ends the hunk rather than corrupting it. + flush() + + flush() + + if not hunks: + raise PatchError( + "That patch has no hunks. A patch needs at least one " + "`@@ -old,count +new,count @@` header, followed by the lines to " + "change: ' ' for context, '-' to remove, '+' to add." + ) + if len(hunks) > MAX_HUNKS: + raise PatchError( + f"That patch has {len(hunks)} hunks, and {MAX_HUNKS} is the most " + f"that will be applied at once. Rewrite the file with file_write " + f"instead, or send the change in pieces." + ) + return hunks + + +def _find(lines: list[str], wanted: tuple[str, ...], hint: int, floor: int) -> int: + """Where `wanted` sits in `lines`, at or after `floor`. Raises if unclear.""" + if not wanted: + # A pure insertion has no context to match. The hint is all there is. + return max(floor, min(hint, len(lines))) + + span = len(wanted) + if hint >= floor and lines[hint : hint + span] == list(wanted): + return hint + + matches = [ + at + for at in range(max(floor, hint - MAX_DRIFT), min(len(lines) - span, hint + MAX_DRIFT) + 1) + if lines[at : at + span] == list(wanted) + ] + if len(matches) == 1: + return matches[0] + if len(matches) > 1: + raise PatchError( + f"Those context lines appear {len(matches)} times in the file, and " + f"the line numbers in the hunk header do not point at any of them, " + f"so there is no way to tell which was meant. Include more " + f"unchanged lines around the change." + ) + raise PatchError("") # Filled in by the caller, which knows the hunk number. + + +def apply(text: str, hunks: list[Hunk]) -> str: + """The file with every hunk applied, or a PatchError naming the first that + would not. + + Hunks are applied in order against a cursor, so one cannot match inside + territory an earlier one already consumed -- which is what a duplicated or + overlapping hunk would otherwise do, applying the same change twice. + """ + crlf = "\r\n" in text + lines = text.replace("\r\n", "\n").replace("\r", "\n").split("\n") + trailing = lines and lines[-1] == "" + if trailing: + lines.pop() + + out: list[str] = [] + cursor = 0 + reached_end = False + + for number, hunk in enumerate(hunks, start=1): + wanted = hunk.before + # A pure insertion names the line it goes *after*, not the line it + # replaces, so it is not off by one the way every other hunk is. + hint = hunk.old_start if hunk.old_count == 0 else max(hunk.old_start - 1, 0) + try: + at = _find(lines, wanted, hint, cursor) + except PatchError as exc: + raise _mismatch(number, hunk, lines, hint, exc.message) from None + + out.extend(lines[cursor:at]) + out.extend(hunk.after) + cursor = at + len(wanted) + reached_end = hunk.ends_without_newline and cursor >= len(lines) + + out.extend(lines[cursor:]) + + result = "\n".join(out) + if trailing and not reached_end: + result += "\n" + return result.replace("\n", "\r\n") if crlf else result + + +def _mismatch(number: int, hunk: Hunk, lines: list[str], hint: int, why: str) -> PatchError: + """The message the model retries from, so it has to say what is actually + there rather than only that something is wrong.""" + if why: + return PatchError( + f"Hunk {number} did not apply. {why} Nothing was written.", hunk=number + ) + + expected = next((line[1:] for line in hunk.lines if line[:1] in (" ", "-")), "") + found = lines[hint] if 0 <= hint < len(lines) else "(past the end of the file)" + return PatchError( + f"Hunk {number} did not apply. It expects line {hint + 1} to be\n" + f" {expected}\n" + f"but the file has\n" + f" {found}\n" + f"and those lines are nowhere else nearby either. Nothing was written. " + f"Read the file again and send a patch that matches it.", + hunk=number, + ) + + +def render(before: str, after: str, path: str, *, max_lines: int = 200) -> str: + """A unified diff of one change, for the transcript. + + Bounded here rather than at render time: this ends up in + `Message.tool_calls_json`, which is on the row forever and re-parsed on + every page load, and a generated file's diff can be larger than the file. + """ + # splitlines, not split("\n"): a file's own final newline would otherwise be + # an empty last element, which difflib renders as a stray context line at + # the bottom of every diff -- and as a spurious change whenever one side has + # it and the other does not. The trailing-newline difference is invisible + # here as a result, which is right for a display and irrelevant to the write. + lines = list( + difflib.unified_diff( + before.replace("\r\n", "\n").splitlines(), + after.replace("\r\n", "\n").splitlines(), + fromfile=f"a/{path}", + tofile=f"b/{path}", + lineterm="", + n=3, + ) + ) + if len(lines) > max_lines: + dropped = len(lines) - max_lines + lines = lines[:max_lines] + [f"… ({dropped} more lines)"] + return "\n".join(lines) + + +__all__ = ["MAX_DRIFT", "MAX_HUNKS", "Hunk", "PatchError", "apply", "parse", "render"] diff --git a/src/lembas/services/agent/session.py b/src/lembas/services/agent/session.py index be7fee4..14693f0 100644 --- a/src/lembas/services/agent/session.py +++ b/src/lembas/services/agent/session.py @@ -58,6 +58,26 @@ class AgentContext: # this they would refuse the very thing that was approved -- the mode says # "ask", and asking is exactly what happened. approved: bool = False + # Absolute paths this reply has read. `file_edit` refuses a file that is not + # in here, because a patch written from memory against a file the model has + # not looked at is how a rewrite silently loses somebody's work. + # + # Here rather than on `Generation` for two reasons. Runners never see a + # Generation -- they get a `ToolContext`, which is a session-free snapshot + # precisely so nothing in a tool holds live state -- and a read path is a + # fact about the machine, which is what this class is. + # + # It is **shared with the approved copy**: `as_approved` is + # `dataclasses.replace`, which copies field references, so a path read + # through an approved call is visible here. That is wanted and is not + # obvious, so there is a test for it. + # + # It resets each reply, and that is correct rather than a limitation. + # `Message.tool_calls_json` is deliberately never replayed as context, so on + # the next turn the model does not have the file's contents either -- + # requiring a re-read in the reply that edits is asking for something it + # needs anyway. + read_paths: set[str] = field(default_factory=set) def executor(self) -> Executor: return ssh_service.SshExecutor(self.spec, self.project_dir) diff --git a/src/lembas/services/agent/tools.py b/src/lembas/services/agent/tools.py index e4852dd..f187615 100644 --- a/src/lembas/services/agent/tools.py +++ b/src/lembas/services/agent/tools.py @@ -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"] diff --git a/src/lembas/web/static/css/chat.css b/src/lembas/web/static/css/chat.css index 717a061..6a3995d 100644 --- a/src/lembas/web/static/css/chat.css +++ b/src/lembas/web/static/css/chat.css @@ -400,6 +400,39 @@ overflow-y: auto; } +/* A diff, from a write or an update. + + The same visual language as .tool-result__text above -- both answer "what did + it do", and two languages would suggest a difference that is not there. The + colours are --success and --danger rather than anything new: --success is + deliberately a different hue from --leaf so an added line does not read as the + brand accent, and --danger is already what an error border uses, so a removed + line reads as removed rather than as broken. + + The padding is on the line, not on the block, so a highlighted row runs the + full width instead of stopping short of the rounded corner. */ +.diff { + margin: 0; + padding: var(--sp-2) 0; + border-radius: var(--radius-sm); + background: var(--bg-sunken); + font-family: var(--font-mono); + font-size: var(--text-xs); + line-height: var(--leading-relaxed); + max-height: 26em; + overflow: auto; +} +.diff__line { + display: block; + padding: 0 var(--sp-3); + white-space: pre-wrap; + overflow-wrap: anywhere; +} +.diff__line--add { background: var(--success-soft); color: var(--success); } +.diff__line--del { background: var(--danger-soft); color: var(--danger); } +.diff__line--meta { color: var(--ink-faint); } +.diff__line--ctx { color: var(--ink-muted); } + /* --- The model asking you something --------------------------------------- */ /* Attributed to the model on purpose. A card styled like the application is a card people answer with things they would not tell a chatbot. */ diff --git a/src/lembas/web/templates/chat/_tool_activity.html b/src/lembas/web/templates/chat/_tool_activity.html index e972cc2..378420a 100644 --- a/src/lembas/web/templates/chat/_tool_activity.html +++ b/src/lembas/web/templates/chat/_tool_activity.html @@ -80,6 +80,41 @@ 
{{ event.text }}
{% endif %} + {% if event.diff %} + {# + What a write or an update changed, in the shape everybody already reads a + change in. + + Every line is text off somebody's machine and is escaped exactly like the + rest of this file. The classification reads the first character and never + interprets the rest — a line is coloured, never parsed. + + Header lines are checked BEFORE the bare +/- ones, or `+++ b/x.py` renders + as an addition and `--- a/x.py` as a removal at the top of every diff. The + test is against the `a/` and `b/` prefixes rather than against three + dashes, because a removed line whose own text begins with `--` produces + exactly three dashes too. + + No whitespace between the spans: they are `display: block` inside a 
,
+      so a newline between two of them is a blank line on screen.
+    #}
+    

+      {%- for line in event.diff.split("\n") -%}
+        {%- if line.startswith('@@') or line.startswith('--- a/')
+               or line.startswith('+++ b/') or line.startswith('diff ') -%}
+          {%- set cls = 'meta' -%}
+        {%- elif line.startswith('+') -%}
+          {%- set cls = 'add' -%}
+        {%- elif line.startswith('-') -%}
+          {%- set cls = 'del' -%}
+        {%- else -%}
+          {%- set cls = 'ctx' -%}
+        {%- endif -%}
+        {{ line }}
+      {%- endfor -%}
+    

+    {% endif %}
+
     {% for result in event.results %}
     

       {% set scheme = (result.url or "").split(":")[0] | lower %}
diff --git a/tests/test_agent_patch.py b/tests/test_agent_patch.py
new file mode 100644
index 0000000..90c93ee
--- /dev/null
+++ b/tests/test_agent_patch.py
@@ -0,0 +1,195 @@
+"""Applying a unified diff.
+
+Pure unit tests, no server and no database: this is where the behaviour that
+makes `file_edit` usable by a real model lives, and every case here is one that
+a real model produces.
+"""
+
+from __future__ import annotations
+
+import pytest
+
+from lembas.services.agent import patch
+
+
+def _apply(text: str, diff: str) -> str:
+    return patch.apply(text, patch.parse(diff))
+
+
+FILE = "\n".join(f"line {n}" for n in range(1, 21)) + "\n"
+
+
+# --- The ordinary case ----------------------------------------------------------
+def test_a_hunk_at_the_line_it_says_applies():
+    result = _apply(
+        FILE,
+        "@@ -4,3 +4,3 @@\n line 3\n-line 4\n+LINE FOUR\n line 5\n",
+    )
+    assert "LINE FOUR" in result
+    assert "line 4\n" not in result
+    assert result.count("\n") == FILE.count("\n"), "no lines gained or lost"
+
+
+def test_headers_are_tolerated():
+    """Models emit them by habit. Refusing costs a round trip to say so."""
+    result = _apply(
+        FILE,
+        "diff --git a/x.py b/x.py\nindex 1234567..89abcde 100644\n"
+        "--- a/x.py\n+++ b/x.py\n@@ -4,3 +4,3 @@\n line 3\n-line 4\n+LINE FOUR\n line 5\n",
+    )
+    assert "LINE FOUR" in result
+
+
+def test_several_hunks_apply_in_order():
+    result = _apply(
+        FILE,
+        "@@ -2,3 +2,3 @@\n line 1\n-line 2\n+TWO\n line 3\n"
+        "@@ -15,3 +15,3 @@\n line 14\n-line 15\n+FIFTEEN\n line 16\n",
+    )
+    assert "TWO" in result and "FIFTEEN" in result
+
+
+def test_a_pure_insertion_needs_no_context():
+    """And names the line it goes *after*, so it is not off by one the way
+    every other hunk is."""
+    result = _apply("a\nb\n", "@@ -1,0 +2,1 @@\n+inserted\n")
+    assert result == "a\ninserted\nb\n"
+
+
+# --- Line numbers drift, context does not ---------------------------------------
+def test_a_hunk_whose_line_numbers_are_wrong_still_applies():
+    """The single highest-value behaviour here. Models count from a truncated
+    read or from the file as it was three edits ago and get the numbers wrong;
+    they get the context right."""
+    result = _apply(
+        FILE,
+        "@@ -1,3 +1,3 @@\n line 11\n-line 12\n+TWELVE\n line 13\n",
+    )
+    assert "TWELVE" in result
+    assert "line 12\n" not in result
+
+
+def test_a_hunk_that_matches_nowhere_is_refused_and_names_what_is_there():
+    with pytest.raises(patch.PatchError) as caught:
+        _apply(FILE, "@@ -4,3 +4,3 @@\n nothing\n-like this\n+new\n at all\n")
+
+    message = caught.value.message
+    assert "Hunk 1 did not apply" in message
+    assert "Nothing was written" in message
+    assert "Read the file again" in message
+
+
+def test_ambiguous_context_is_refused_rather_than_guessed_at():
+    """The one failure that silently corrupts a file. Two identical blocks and a
+    hint pointing at neither: there is no way to tell which was meant."""
+    text = "start\nsame\nsame\nsame\nmiddle\nsame\nsame\nsame\nend\n"
+    with pytest.raises(patch.PatchError) as caught:
+        _apply(text, "@@ -50,3 +50,3 @@\n same\n-same\n+CHANGED\n same\n")
+
+    assert "appear" in caught.value.message
+    assert "more unchanged lines" in caught.value.message.lower()
+
+
+def test_drift_beyond_the_ceiling_is_not_searched():
+    long = "\n".join(f"line {n}" for n in range(1, 1000)) + "\n"
+    with pytest.raises(patch.PatchError):
+        _apply(long, "@@ -1,3 +1,3 @@\n line 900\n-line 901\n+NINE\n line 902\n")
+
+
+def test_nothing_is_written_when_a_later_hunk_fails():
+    """Atomic. A half-applied file is worse than a refused one, and the model
+    cannot tell the difference without reading it again."""
+    with pytest.raises(patch.PatchError) as caught:
+        _apply(
+            FILE,
+            "@@ -2,3 +2,3 @@\n line 1\n-line 2\n+TWO\n line 3\n"
+            "@@ -15,3 +15,3 @@\n bogus\n-nope\n+x\n also bogus\n",
+        )
+    assert caught.value.hunk == 2
+
+
+def test_hunks_out_of_order_are_refused():
+    """Otherwise a duplicated hunk applies the same change twice."""
+    with pytest.raises(patch.PatchError):
+        _apply(
+            FILE,
+            "@@ -15,3 +15,3 @@\n line 14\n-line 15\n+FIFTEEN\n line 16\n"
+            "@@ -2,3 +2,3 @@\n line 1\n-line 2\n+TWO\n line 3\n",
+        )
+
+
+# --- The things that break on real files ------------------------------------------
+def test_a_crlf_file_round_trips_as_crlf():
+    """Without normalising in and restoring out, every hunk on a Windows file
+    fails on context that looks identical in the error message."""
+    text = "alpha\r\nbeta\r\ngamma\r\n"
+    result = _apply(text, "@@ -1,3 +1,3 @@\n alpha\n-beta\n+BETA\n gamma\n")
+
+    assert result == "alpha\r\nBETA\r\ngamma\r\n"
+    assert "\n\n" not in result.replace("\r\n", "\n\n").replace("\n\n", "\r\n")
+
+
+def test_a_blank_context_line_with_no_leading_space_applies():
+    """Trailing whitespace is stripped by half the things a model's output
+    passes through, so this is the normal case rather than a malformed one."""
+    text = "alpha\n\ngamma\n"
+    result = _apply(text, "@@ -1,3 +1,3 @@\n alpha\n\n-gamma\n+GAMMA\n")
+    assert result == "alpha\n\nGAMMA\n"
+
+
+def test_a_file_with_no_trailing_newline_keeps_none():
+    result = _apply("alpha\nbeta", "@@ -1,2 +1,2 @@\n alpha\n-beta\n+BETA\n")
+    assert result == "alpha\nBETA"
+
+
+def test_the_no_newline_marker_on_the_new_side_removes_the_trailing_newline():
+    result = _apply(
+        "alpha\nbeta\n",
+        "@@ -1,2 +1,2 @@\n alpha\n-beta\n+BETA\n\\ No newline at end of file\n",
+    )
+    assert result == "alpha\nBETA"
+
+
+def test_the_no_newline_marker_on_the_old_side_is_not_an_instruction():
+    """git emits it for the old side too. Reading that as an instruction would
+    strip a newline the patch never touched."""
+    result = _apply(
+        "alpha\nbeta\n",
+        "@@ -1,2 +1,2 @@\n alpha\n-beta\n\\ No newline at end of file\n+BETA\n",
+    )
+    assert result == "alpha\nBETA\n"
+
+
+# --- Refusing the unusable ---------------------------------------------------------
+def test_a_patch_with_no_hunks_says_what_one_looks_like():
+    with pytest.raises(patch.PatchError) as caught:
+        patch.parse("just change line four please")
+    assert "@@" in caught.value.message
+
+
+def test_too_many_hunks_is_refused_and_points_at_file_write():
+    diff = "".join(
+        f"@@ -{n},1 +{n},1 @@\n-line {n}\n+LINE {n}\n" for n in range(1, patch.MAX_HUNKS + 5)
+    )
+    with pytest.raises(patch.PatchError) as caught:
+        patch.parse(diff)
+    assert "file_write" in caught.value.message
+
+
+# --- Rendering -----------------------------------------------------------------------
+def test_render_produces_a_diff_of_the_change():
+    diff = patch.render("alpha\nbeta\n", "alpha\nBETA\n", "x.py")
+    assert "-beta" in diff
+    assert "+BETA" in diff
+    assert "a/x.py" in diff
+
+
+def test_render_is_bounded():
+    """It goes on the message row forever and is re-parsed on every page load,
+    and a generated file's diff can be larger than the file."""
+    before = "\n".join(str(n) for n in range(500))
+    after = "\n".join(f"x{n}" for n in range(500))
+    diff = patch.render(before, after, "big.txt", max_lines=20)
+
+    assert len(diff.split("\n")) <= 21
+    assert "more lines" in diff
diff --git a/tests/test_agent_tools.py b/tests/test_agent_tools.py
index 29ae54d..76f8592 100644
--- a/tests/test_agent_tools.py
+++ b/tests/test_agent_tools.py
@@ -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.
diff --git a/tests/test_tool_activity.py b/tests/test_tool_activity.py
index 5d3eb41..8e3a301 100644
--- a/tests/test_tool_activity.py
+++ b/tests/test_tool_activity.py
@@ -192,3 +192,60 @@ def test_an_unknown_tool_falls_back_to_its_name():
     assert tool_labels.label_for({"name": "mcp_thing"}) == "mcp_thing"
     assert tool_labels.icon_for({"name": "mcp_thing", "kind": "mcp"}) == "server"
     assert tool_labels.icon_for({"name": "whatever"}) == tool_labels.FALLBACK_ICON
+
+
+# --- Diffs -----------------------------------------------------------------------
+def test_a_diff_renders_added_and_removed_lines():
+    html = _render(
+        {
+            "name": "file_edit",
+            "kind": "agent",
+            "query": "src/app.py",
+            "status": "ok",
+            "results": [],
+            "diff": "--- a/src/app.py\n+++ b/src/app.py\n@@ -1,2 +1,2 @@\n alpha\n-beta\n+BETA",
+        }
+    )
+    assert 'diff__line--del">-beta' in html
+    assert 'diff__line--add">+BETA' in html
+    assert 'diff__line--ctx"> alpha' in html
+    assert 'diff__line--meta">@@ -1,2 +1,2 @@' in html
+
+
+def test_a_diff_header_is_not_an_addition():
+    """`+++ b/x` at the top of every diff would otherwise render green, and
+    `--- a/x` red, which reads as the file being replaced by itself."""
+    html = _render(
+        {
+            "name": "file_edit",
+            "results": [],
+            "diff": "--- a/x.py\n+++ b/x.py\n@@ -1 +1 @@\n-a\n+b",
+        }
+    )
+    assert 'diff__line--meta">--- a/x.py' in html
+    assert 'diff__line--meta">+++ b/x.py' in html
+
+
+def test_a_removed_line_of_dashes_is_still_a_removal():
+    """A removed line whose own text begins with `--` produces exactly three
+    dashes, which is why the header test is against the a/ and b/ prefixes."""
+    html = _render({"name": "file_edit", "results": [], "diff": "@@ -1 +1 @@\n--- a dashed line"})
+    assert 'diff__line--del">--- a dashed line' in html
+
+
+def test_a_diff_line_is_escaped():
+    """Hard rule 6. It is a file off somebody else's machine."""
+    html = _render(
+        {
+            "name": "file_edit",
+            "results": [],
+            "diff": "@@ -1 +1 @@\n+",
+        }
+    )
+    assert "