Skip to content

fix: add fsReplace tool to batch edits #1533

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

jguoamz
Copy link
Contributor

@jguoamz jguoamz commented Jun 5, 2025

Problem

  • LLM does not batch small edits on the same file together and instead calls fsWrite multiple times which adds latency
  • LLM sometimes does not respect the the fact that parent directory must exist before file creation
  • Our existing replace logic does not handle special characters (non UTF-8 encodings) which causes replacement failures

Solution

  • Break fsWrite into fsWrite and fsReplace tool where fsReplace accepts an array of diffs to allow LLM to batch edits on the same file together
  • Having a simplified fsWrite tool now leads to better performance as LLM can better understand the toolSpec and reduces the chance of it trying to create a new file in a directory that does not exist yet
  • Updated logic to handle special characters in different encodings

Summary of the new tools

fsWrite
Purpose: Creates new files or adds to existing ones
Commands: create (new/overwrite) and append (add to end)
Key point: Parent directories must exist beforehand
Best for: New files, complete rewrites, or adding to file ends

fsReplace
Purpose: Makes targeted changes within existing files
Method: Uses oldStr/newStr pairs for precise replacements
Multiple changes: Supports batch edits via diffs array
Key point: Each oldStr must match exactly and be unique in the file
Best for: Surgical edits to specific parts of files

License

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@jguoamz jguoamz closed this Jun 5, 2025
@jguoamz jguoamz reopened this Jun 5, 2025
@jguoamz jguoamz closed this Jun 5, 2025
@jguoamz jguoamz reopened this Jun 5, 2025
@jguoamz jguoamz marked this pull request as ready for review June 5, 2025 20:25
@jguoamz jguoamz requested a review from a team as a code owner June 5, 2025 20:25
@jguoamz jguoamz closed this Jun 5, 2025
@jguoamz jguoamz reopened this Jun 5, 2025
@leigaol
Copy link
Contributor

leigaol commented Jun 6, 2025

can you do a global search in the agenticChatController and search for where 'fsWrite' is used and consider adding 'fsReplace' when necessary?

For example, at

#getTools(session: ChatSessionService) {
        const tools = this.#features.agent.getTools({ format: 'bedrock' })

        // it's disabled so filter out the write tools
        if (!session.pairProgrammingMode) {
            return tools.filter(tool => !['fsWrite', 'executeBash'].includes(tool.toolSpecification?.name || ''))
        }
        return tools
    }
``

@jguoamz
Copy link
Contributor Author

jguoamz commented Jun 6, 2025

can you do a global search in the agenticChatController and search for where 'fsWrite' is used and consider adding 'fsReplace' when necessary?

For example, at

#getTools(session: ChatSessionService) {
        const tools = this.#features.agent.getTools({ format: 'bedrock' })

        // it's disabled so filter out the write tools
        if (!session.pairProgrammingMode) {
            return tools.filter(tool => !['fsWrite', 'executeBash'].includes(tool.toolSpecification?.name || ''))
        }
        return tools
    }
``

I already did, it's updated

@jguoamz jguoamz force-pushed the fsReplace branch 4 times, most recently from eb20916 to f632a24 Compare June 11, 2025 02:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants