fix(tools): count StrReplaceFile replacements against running content - #2554
fix(tools): count StrReplaceFile replacements against running content#2554ayaangazali wants to merge 1 commit into
Conversation
The success message summed occurrences of each edit's old string in the original file, but edits apply sequentially so a later edit operates on the output of earlier ones. When edits interact the reported total was wrong (e.g. two replace_all edits on "foo bar foo" reported 3 instead of 5). Count each replacement as it actually happens by returning the count from _apply_edit, which also removes the now-redundant second pass over the edits.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
|
Status update on this one. What it fixes: Worth noting #2526 was opened after this PR by someone who hit the same thing independently, so it is not purely theoretical. Their repro ( Branch is current with main (main has not moved since 4a550ef, nothing to rebase onto) and still merges clean. Same note as my other PRs: the checks here show as Glad to change the approach if you would rather solve it a different way. |
Related Issue
No open issue for this one, I hit it while reading
StrReplaceFile. It is a small self-contained correctness fix (well under the 100 LOC discussion threshold), happy to close if you would rather have an issue first.Description
The success message for
StrReplaceFilecounted replacements by summingoriginal_content.count(edit.old)for each edit. But edits apply sequentially, so a later edit runs against the output of the earlier ones, not the original file. When edits interact, the reported total is wrong.Example: file
foo bar foo, tworeplace_alleditsfoo -> "foo bar"thenbar -> "baz":foo -> "foo bar"replaces 2, givingfoo bar bar foo barbar -> "baz"then replaces 3, givingfoo baz baz foo bazThe file edit itself was always correct; only the reported count was off. Fix counts each replacement as it actually happens by having
_apply_editreturn the count alongside the new content, which also lets the redundant second pass over the edits go away.Single-edit cases are unchanged (a
replace_alledit still reports its occurrence count, a single edit reports 1).Checklist
test_str_replace_file.py: a single-edit count check and the interacting-edits case above, which reported 3 before)make gen-changelogto update the changelog. (hand-edited CHANGELOG.md in the same style, I do not have the Kimi API setup the skill needs)make gen-docsto update the user documentation. (no user docs describe this message, nothing to regenerate)quick note: freshman here, found this reading the edit tool and figured a wrong count could quietly mislead. kept the change tiny and consulted claude code on the counting semantics to be sure str.count lines up with str.replace. if you would rather not touch it, no worries at all, still learning what is worth a PR :)