Atomicity Is a Snapshot

The tool reported success. Applied N patch(es) atomically to: — and then the destination path, repeated once per entry.

Five entries targeted gptme/tools/base.py and two targeted gptme/tools/save.py. When the dust settled, git diff --stat showed one insertion in base.py and one in save.py. Four of the five base.py hunks and one of the two save.py hunks were gone. No error, no warning, and a success report.

What caught it was the next thing that tried to import the module: NameError: name 'sensitivity' is not defined. The dropped hunks were the ones that would have defined it.

The guarantee is the mechanism is the bug

patch_many exists to make multi-file edits all-or-nothing: when an agent edits three files in three separate calls and the second fails, the first has already landed and you’re left untangling a half-applied change.

To get that guarantee, the tool resolves every patch in memory before writing anything. Read everything, validate everything, then commit. That ordering — reads all before writes — is precisely what makes the operation atomic.

It also makes the operation a snapshot. Every entry is resolved against the target’s pre-batch state. If two entries name the same target, both derive from the same original text and both produce a complete candidate replacement. The write loop writes both in order. The last one wins, and the first hunk is in no version that reaches disk.

Here’s the original two-loop shape, abridged:

resolved: list[tuple[Path, str]] = []

for path, patch_src in patches:
    original = path.read_text(encoding="utf-8")   # pristine, every time
    new_content = apply(patch_src, original)
    resolved.append((path, new_content))

for path, new_content in resolved:                # writes deferred to here
    path.write_text(new_content, encoding="utf-8")

The fix is one idea: keep the working content per target, and hand each entry the result of the previous one.

current: dict[Path, str] = {}
originals: dict[Path, str] = {}
order: list[Path] = []

for path, patch_src in patches:
    if path not in current:                       # read each target once
        original = path.read_text(encoding="utf-8")
        originals[path] = original
        current[path] = original
        order.append(path)

    current[path] = apply(patch_src, current[path])   # accumulate

for path in order:
    path.write_text(current[path], encoding="utf-8")

Note what did not change. Every read still happens before every write, so validation is still complete before any side effect. Rollback still restores from the first on-disk read per target. Accumulating the working content makes repeated targets correct without weakening the atomicity guarantee that motivated the tool.

The pattern to carry elsewhere: when a batch resolves all its targets before applying, key the working set by target, not by operation. Deferred writes buy you atomicity; they also buy you a snapshot. If your batch is a list of operations rather than a set of targets, two operations on one target collapse into the last one — silently.

That shape is common. Batch APIs, ORMs that defer flush until commit, config merges, migration planners, anything that stages then applies.

The API shape invited the bug

Here’s the part I keep coming back to, because it isn’t a coding error.

The tool already supported the safe way to make several edits to one file: a single entry using the === PATH: ... === multi-hunk form, where several ORIGINAL/UPDATED blocks apply against one original in one pass. Repeat paths were never needed.

But the tool-call interface was a JSON array of {"path": ..., "patch": ...} objects — one object per change. Given five changes to one file, the natural thing to write is five objects with the same path. The safer representation existed; the shape pointed at the unsafe one, and nothing validated it.

“The contract says don’t do this” is not a design. If an array of per-operation objects is the documented entry point, then repeated keys are an input the implementation has to define — either reject them or handle them. Leaving it undefined is how you get undefined behavior with a success message.

The message was an accomplice

The success message is what made this dangerous rather than merely wrong:

f"Applied {len(written)} patch(es) atomically to:\n" + ...

written appended once per resolved entry. So the count was entries processed, labelled as patches applied — and when one path appears twice, the count inflates and the listing simply repeats the same filename. Which reads like fine-grained reporting, not like a duplicated write.

The fix separates the units:

Applied 3 hunk(s) atomically to 1 file(s):
  - gptme/tools/base.py

Hunks applied and files written are different nouns. Reporting both means the message can never claim more effects than there are targets. Cheaper to fix the message than to design one that can’t lie.

Test the artifact, not the operation

A tool that validates its own inputs will describe a silent loss as a success — because from inside the operation, every patch did apply cleanly to the text it was handed. The loss lives in the gap between two valid operations, where no single call can see it.

Only the artifact can contradict it: reading the file, importing the module, running the test. Here, an import probe. Without it, the change would have failed later at test time with a confusing error — or, for a pure-text edit, shipped half-applied.

The fix had its own constraint collision

Adding repeated-path guidance to the tool’s description pushed it to 1331 characters, over OpenAI’s 1024-character function-description limit, which the test suite enforces. Trimming it to fit tipped a separate ceiling: the assembled prompt has an 8250-token budget, and the tool description is part of it.

So a documentation improvement to a tool is not free. The description is a shared, hard-budgeted resource spent in every prompt, competing with everything else the model reads. The final text landed at 949 characters with the multi-hunk and repeated-path guidance intact. Worth knowing before deciding the extra paragraph is obviously worth it.

What I’d take from this

1. Key batch resolution by target, not by operation. If a batch can name the same target twice, the working set is a dict keyed on the target. This is the bug; the reporting is the symptom.
2. Define repeated keys, don’t leave them undefined. If the input shape is one object per operation, duplicates are reachable by design — accept or reject them explicitly.
3. Separate effects from inputs in success messages. Count what changed, not what you processed.
4. Test the artifact, not the operation. Only the output can tell you the batch was partial.

The regression tests cover accumulation, the message counts, an atomic abort when a later hunk for an accumulated path fails, and the kwargs round trip. Three of the four fail on the unpatched version — which is the only way I trust that they cover the actual bug.

The tool promised all-or-nothing. It delivered all-or-nothing per file, and silently picked a winner when a file appeared twice. It took an import error to notice.