The argument that landed in the wrong call

Four bugs fixed in one afternoon, in the tools that write your code, and every one of them was a place where a tool took something on trust: a position, a write, a verdict, a line number. What each one did, how it was found, and what it checks now.

Here is a diff a refactoring tool produced against this repository:

-            if gate.as_ref().is_some_and(|g| g.applied) {
+            if gate.as_ref().is_some_and(|g| g.applied, 2) {

It was asked to add a parameter to a function called render and pass 2 at every call. It found the call, appended the argument - and the call it appended it to was is_some_and, four lines above the one it meant. The type check caught it, which is the only reason this is a post and not an incident.

It was the last of four bugs fixed that afternoon. All four were in the tools that write code, and all four had the same shape: the tool was told something, and believed it.

A position it was told

The tools that rewrite call sites ask the analyzer where the calls are, then edit the text at those positions. The analyzer said the render call was on line 1949:

$ prod-code refs crates/prod-code-mcp/src/signature.rs 74 12
  • …/crates/prod-code-mcp/src/tools.rs:1949:35
$ grep -n 'change.render(6000)' crates/prod-code-mcp/src/tools.rs
1953:            let mut text = change.render(6000);

Four lines off. The cause took longer to find than the symptom. All day, formatting had been done the way this project says it should be - cargo fmt --all run on the build node through code_exec, so nothing compiles on the laptop - and a command’s changed files come back to the checkout and are recorded as synced. Recorded as synced means no later sync announces them to the analyzer. And nothing else told the analyzer on the node that the files had changed under it. It went on answering from the text it had before the formatter reflowed four lines of that file.

It is silent because of a courtesy: the file a query is about is always opened fresh by the client, so it is always right. Only the other files come from the analyzer’s own copy - which is exactly where call sites are.

Two fixes, because it was two failures. The gateway now tells the analyzer about every file a command changed, the way a sync does (#75); the live test that reproduces it prepends three lines to a file through code_exec and asks where a reference in it is - store.rs:6 before the fix, store.rs:9 after. And the tools no longer act on a position without checking that the name is there:

not given the argument (1 reference(s) …):
  src/home.rs:3:13 (the analyzer places `render` here, but the file says otherwise)

A stale position does damage exactly when it, plus the length of the name being looked for, lands on an opening parenthesis - the tool looks for ( right after where the name should end. In the opening diff the analyzer said column 35 of a line where is_some_and starts at column 30; 35 plus the six letters of render is 41, which is is_some_and’s (. Any call can be hit that way, of any length. The test for the check reproduces it with a double(40) placed so the stale position lands on its parenthesis, and with the check removed it fails with the bug in its most convincing form:

the call the stale position pointed at is untouched: …
    let w = double(40, 80);

A write it assumed would finish

Every write tool ends by writing several files. They were written one after another, and the first failure returned:

File exists (os error 17)

with the files before it already rewritten and nothing saying which. A refusal before writing is fine; a finished write is fine. A half-written multi-file refactor is the one outcome that cannot be recovered from by reading the report, because the report is an OS error.

Every path an edit touches is now snapshotted before the first byte moves, and any failure puts each one back (#70) - the bytes it had, or no file where there was none - and the error says what happened rather than what the operating system said: the edit failed partway and was undone, and how many files were put back.

A verdict it repeated

“The analyzer accepts the result: 0 errors” is the line every write tool ends with, and the post on bundling parameters showed what it does not cover: an unresolved type, a module path that goes nowhere. The fix there was local - that tool adds its own imports. The fix here is general. Every write tool now takes verify: "compile", which puts the proposed files into a shadow of the workspace and runs cargo check there, against the warm build, before anything is written (#63):

the analyzer accepts the result: 0 errors

the compiler accepts the result too: `cargo check` in a shadow of the workspace, 2291 ms

It proved its own point while it was being written. The analyzer, asked about the file that implements the gate, said:

crates/prod-code-mcp/src/tools.rs: 0 error(s), 0 warning(s)

and the compiler said cannot find type PathBuf in this scope. The check is not a compiler, and now a caller can ask for the compiler.

Four things a write tool had taken on trust, each paired with what it checks now. A position from the analyzer, now checked for the name before anything is edited. A multi-file write, now snapshotted and put back whole if any file fails. The analyzer's verdict, now optionally confirmed by the compiler in a shadow of the workspace. A declaration's line and column, now found by its own text — Four trusted inputs, four checks. None of them is expensive; each is the difference between a refusal and damage.

A line number it remembered

The fourth one here was older than today. Changing a function’s signature rewrites its call sites first, then its declaration - and the structural rewrite renders every call it changes on one line. When a call above the declaration had been written across four lines, everything below it moved up, and the declaration was looked for again at its old line and column:

Error: the declaration moved while its call sites were rewritten

A message that names no function and suggests nothing is indistinguishable from a crash to the agent reading it. The declaration had not moved in any sense that matters: it is a declaration, not a call, and the rewrite never touches it. So it is now found by its own text (#58), and the change that failed - reordering execute_lsp_query, 178 lines in 6 files - goes through.

What it costs, and what it does not fix

The checks are cheap: a comparison of a few bytes at each position, one read per file before writing, one text search for a declaration. verify: "compile" is not - it is a build, 2.3 to 2.6 seconds on this repository - which is why it is asked for rather than always on.

And one thing turned up that is not fixed. A dry run that changes the items of a file much of the crate imports - moving a function into it, say - makes the analyzer re-resolve names for the crate twice, once for the overlay and once when it is closed. The next query pays for both:

refs, settled                                0.10 s
move snake_case --to lang.rs, dry run        ~24-44 s
refs, right after                           20.6 s

A comment in the same file costs nothing; an item costs twenty seconds. That is #73, and it is open.

The common thread is not carelessness. Each of these trusted something that is almost always true - the analyzer’s positions are almost always current, writes almost always finish, the analyzer almost always sees what the compiler sees, a declaration is almost always where it was. Almost always is exactly the rate at which a tool run unattended, many times an hour, turns into a tool that eventually writes an argument into the wrong call.