fix(deepagents): edit_file reports initial-content write, not '0 occurrence(s)', on empty files - #635
Open
Andrew Kondratev (andruhon) wants to merge 1 commit into
Conversation
…)', on empty files When edit_file runs on an empty file with an empty old_string, performStringReplacement takes the intentional init special case (added in the fix for langchain-ai#161) and returns [newString, 0]: the content IS written, but the tool reported `Successfully replaced 0 occurrence(s)`. That reads like a no-op and misleads LLM agents into thinking the write failed / retrying. Branch the success message on occurrences === 0 — the only non-error path that reaches it, since a genuine no-match returns 'String not found' before this point — to report `Successfully wrote initial content to '<path>'`. Adds deterministic tests for both the init-path message and the normal replacement-count message. Co-Authored-By: Claude Opus 4.8 <[email protected]>
|
|
Andrew Kondratev (@andruhon) is attempting to deploy a commit to the LangChain Team on Vercel. A member of the Team first needs to authorize it. |
deepagents-acp
deepagents
@langchain/sandbox-standard-tests
@langchain/daytona
@langchain/deno
@langchain/modal
@langchain/node-vfs
@langchain/quickjs
commit: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to #161. That issue was the
occurrences = -1bug when both the file content andold_stringare empty; the fix added an intentional "set initial content" special case inperformStringReplacementthat returns[newString, 0]— the content is written, and occurrences is0.The success message for that path was never updated, so
edit_fileon an empty file now reports:The content was written correctly, but the message describes a no-op replacement. Because
edit_fileresults are consumed by an LLM, an agent that writes initial content into an empty file reads "replaced 0 occurrence(s)" and reasonably concludes the write failed — retrying, switching tools, or reporting it as a bug. (We hit exactly this during manual testing: an agent flagged "edit_file succeeded but reported 0 occurrences despite writing content" as a suspected harness anomaly.)Change
Branch the
edit_filesuccess message onresult.occurrences === 0:This is safe because a genuine zero-match on a non-empty file returns
Error: String not found in filebefore the success message is built (and an emptyold_stringon a non-empty file returnsError: oldString cannot be empty). So the only non-error path that reaches the message withoccurrences === 0is the empty-file init case — where content really was written.Tests
Added two deterministic tests in
fs.permissions.test.ts(mock-backend harness, no live model):occurrences: 0→ message contains "initial content" and not "0 occurrence(s)"occurrences: 1→ message still reports "replaced 1 occurrence(s)"vitest run src/middleware/fs.permissions.test.ts→ 31 passed, no type errors.Notes
[email protected](the shippedSuccessfully replaced …template and thecontent === "" && oldString === ""special case are both present in the published bundle) and on currentmain.