fix(tools): reject ambiguous edits instead of silently editing the first match - #3395
fix(tools): reject ambiguous edits instead of silently editing the first match#3395kovtcharov-amd wants to merge 2 commits into
Conversation
… the repo Nine design and reference docs were sitting untracked in the working copy, so the decisions they record were invisible to everyone else — including the skill-bound task execution design that the async-task and multi-slot-broker work is meant to build against. Alongside them sat a 45MB mailbox corpus, agent-run captures, and internal analysis, none of which were gitignored. `git add -A` would have committed a mailbox to a public repo. The ignore block that already quarantines private working reports now covers those classes too, and its pointer to where that material lives is corrected — the path it named has not existed for some time.
…rst match When the text an edit replaced appeared more than once in a file, the edit tools replaced the *first* occurrence and reported success. The agent believed it had changed the region it named; the user reviewed a diff that touched a different one. It is the failure mode least likely to be caught in review, because the diff is a real edit of the right shape in the wrong place. Three sites had the defect independently — a membership test proving *at least* one match, followed by `.replace(old, new, 1)` picking the first, with nothing in between establishing uniqueness: - file_io_tools.edit_file - file_io_tools.edit_python_file - file_tools.edit_file All three now route through one module, gaia.agents.tools.file_edit, so they cannot drift apart. old_content must match exactly one location; several is an error naming the count and the line of each. Two smaller problems in the same path: A not-found error said only "not found", so the agent spent a round trip re-reading the file and often guessed again. Every rejection now carries the current content around the region the caller was aiming at, anchored on the closest line when the text does not match at all. There was no staleness check, so an edit could land on a version the agent had never seen. FileStateTracker — ported from the C++ tracker in cpp/include/gaia/file_tools.h — records a content hash on read and write and rejects an edit against a file that changed since. The rejection hands the content back and re-anchors, so the corrected retry proceeds rather than livelocking against a superseded hash. The C++ tree converges on the same contract. Its file_edit replaced *all* occurrences — a third semantic, and a worse one: it corrupts every region the model did not name rather than just one. It now requires a unique match and carries current content on rejection too, so an agent behaves identically on either tree. No caller relies on first-match semantics: these tools are model-invoked, and a grep for edit_file(/edit_python_file( across src/, hub/ and tests/ returns no call sites, only registration and grant lists.
|
Closing as a duplicate of #3396 — my fault for racing this branch rather than waiting for it. The implementation here was the better one and #3396 now carries all of it: the shared The reason to keep #3396 instead of this one is scope, not content. This branch was cut from a local commit that has not landed on main, so its diff against main also carries about 2,700 lines of unrelated plan and reference docs. #3396 is the same change with only the files it needs. Nothing from this branch is lost. |
Request changesThis fixes a genuinely nasty failure: an edit whose target text appeared twice silently changed the first one and reported success. Now a non-unique match is an error that says how many places matched and where, nothing is written, and a rejected edit hands back the file's current text so the retry doesn't cost a re-read. The implementation is shared across all three Python edit tools and the C++ tool, with a test table that runs against every one of them — it's careful work. The one thing holding it up is proof rather than code. This changes the description and parameter text of a tool the flagship agent is handed, and it changes what the model gets back when an edit fails. That is exactly the class of change the project requires an agent eval run for, because the new behaviour only pays off if the model actually recovers from an ambiguity error instead of looping on it — no unit test can tell you that. The test plan lists unit and C++ suites only, and the automated evidence stage on this run errored out, so nothing exercised the real surface. Two smaller things worth fixing while you're here, both on the C++ side: the stale-edit message still tells the model to go re-read the file, which is the round trip this PR set out to remove and now contradicts the content it ships alongside; and the ambiguity error lists every matching line with no cap, so on a file with many matches it crowds out the excerpt the model actually needs. Real-world evidenceThe evidence harness failed to run on this PR — What's missing and would settle it: a 🔍 Technical details🟡 ImportantNo agent eval for an LLM-affecting change. 🟢 MinorC++ stale message contradicts the payload it now ships ( C++ ambiguity error has no match cap, and its excerpt budget outruns the tool-result budget (
The tracker is process-wide, not per-agent/session. Four new Strengths
|
Closes #3377.
When the text an edit was replacing appeared more than once in a file, GAIA edited the first occurrence and reported success. The agent believed it had changed the thing it named, and the user reviewed a diff that touched a different region of the file. It is the failure mode least likely to be caught in review, because the diff is a real edit, of the right shape, in the wrong place. Now a non-unique match is an error that names how many places matched and where, and nothing is written. Two related costs go away with it: a failed edit hands back the file's current content instead of a bare "not found", so the retry no longer burns a round trip re-reading; and an edit issued against a file that changed since the agent read it is rejected rather than clobbering the newer version.
Threads
file_io_tools.edit_file,file_io_tools.edit_python_fileandfile_tools.edit_fileeach had the defect independently. They now sharegaia.agents.tools.file_edit, and the test table runs against all three so they cannot drift apart again.file_editreplaced all occurrences — worse than first-match, since it corrupts every region the model did not name. It now requires a unique match and carries current content on rejection, so an agent behaves the same on either tree.file_writestays strict, because it names noold_stringand a blind retry would clobber.No caller relies on first-match semantics. These tools are model-invoked; grepping
edit_file(/edit_python_file(acrosssrc/,hub/andtests/returns no call sites, only registration and grant lists.Test plan
python -m pytest tests/unit/agents/test_file_edit_semantics.py -q— 66 pass. Parametrized over all three tools: ambiguity errors and writes nothing, not-found carries verbatim content, stale rejection carries content and the retry then succeeds, unique match still replaces.apply_unique_replacementback toif old not in c: error; else c.replace(old, new, 1)and re-run — 42 of the 63 then-existing tests fail.python -m pytest tests/unit/ -q— no regressions. Against cleanmainas a baseline: 616 failed / 488 errors / 9621 passed → 614 / 488 / 9686. Pre-existing failures are environmental (no network in the sandbox; an editable install resolvinggaia_agent_emailto another worktree causes 24 collection errors, excluded from both runs).python -m pyteston every test touching the changed modules (test_file_tools,test_file_write_guardrails,test_builder_agent,test_tool_grants,test_confirmation_required_tools,test_chat_agent_integration,test_skill_binary_grants,test_starter_skills,test_mcp_tool_risk_classification) — 900 pass, 0 fail.cmake --build <dir> --target tests_mock && tests_mock.exe— 1068 pass, 0 fail.--gtest_filter='FileTools*'covers the new ambiguity, recovery, and excerpt-anchoring cases.black,isort,flake8,pylintclean on the changed files (util/lint.py --allcannot fetch its tools viauvxin this sandbox, so each was run from the local env with the same project config).