fix(fs_write): guide model to read-before-write and prevent sed fallback#3724
Open
sandikodev wants to merge 1 commit intoaws:mainfrom
Open
fix(fs_write): guide model to read-before-write and prevent sed fallback#3724sandikodev wants to merge 1 commit intoaws:mainfrom
sandikodev wants to merge 1 commit intoaws:mainfrom
Conversation
Two related issues caused cascading failures during AI-assisted editing: 1. The fs_write tool description did not explicitly require the model to read the file before calling str_replace. The model would guess or reconstruct old_str from memory, causing 'no occurrences found' errors. 2. When str_replace failed, the model would fall back to shell commands like 'sed -i' which have no diff preview, no validation, and can silently corrupt files or affect unintended lines. Fix both by: - Adding explicit read-before-write instruction to the tool description: 'ALWAYS use fs_read to read the file content BEFORE calling str_replace' - Adding explicit anti-fallback instruction: 'If str_replace fails, do NOT fall back to shell commands like sed. Instead, re-read the file with fs_read and retry.' - Improving the 'no occurrences found' error message to include the same guidance so the model receives it at the point of failure - Improving the 'multiple occurrences' error message to suggest adding more context to make old_str unique
Author
|
Superseded by #3725. This PR added read-before-write guidance to the tool description and improved the error message. PR #3725 includes all of that plus the actual fuzzy matching implementation, making the guidance accurate (the tool now tolerates minor indentation differences). The two PRs should be reviewed together or #3724 closed in favour of #3725. |
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.
Problem
Two related issues caused cascading failures during AI-assisted file editing:
Issue 1: Model guesses
old_strwithout reading the file firstThe
fs_writetool description did not explicitly require the model to read the file before callingstr_replace. The model would reconstructold_strfrom memory or context, which fails when:This produces the error:
no occurrences of "..." were foundIssue 2: Model falls back to
sed -iafterstr_replacefailureWhen
str_replacefails, the model would fall back to shell commands likesed -ias a workaround. This is dangerous because:This compounds the original error into a potentially destructive cascade.
Fix
1. Tool description — explicit read-before-write instruction:
2. Tool description — explicit anti-fallback instruction:
3. Error message improvement — guidance at point of failure:
Before:
After:
4. Multiple occurrences error — actionable hint:
Before:
After:
Testing
cargo test -p chat_cliBy submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.