feat: added extra verification for search_and_replace when more than one match is found; added test case for searchAndReplaceTool (fixes #4744)

[pwilkin]

Related GitHub Issue

Closes: #4744

Description

Added extra condition in searchAndReplaceTool.ts to return an error and extra instructions when the search part of search_and_replace has multiple matches.

Added a test case for searchAndReplaceTool.ts to test this and a few other cases.

Test Procedure

Run searchAndReplaceTool.test.ts

Type of Change

• 🐛 Bug Fix: Non-breaking change that fixes an issue.
• ✨ New Feature: Non-breaking change that adds functionality.
• 💥 Breaking Change: Fix or feature that would cause existing functionality to not work as expected.
• ♻️ Refactor: Code change that neither fixes a bug nor adds a feature.
• 💅 Style: Changes that do not affect the meaning of the code (white-space, formatting, etc.).
• 📚 Documentation: Updates to documentation files.
• ⚙️ Build/CI: Changes to the build process or CI configuration.
• 🧹 Chore: Other changes that don't modify src or test files.

Pre-Submission Checklist

• Issue Linked: This PR is linked to an approved GitHub Issue (see "Related GitHub Issue" above).
• Scope: My changes are focused on the linked issue (one major feature/fix per PR).
• Self-Review: I have performed a thorough self-review of my code.
• Code Quality:
  • My code adheres to the project's style guidelines.
  • There are no new linting errors or warnings (npm run lint).
  • All debug code (e.g., console.log) has been removed.
• Testing:
  • New and/or updated tests have been added to cover my changes.
  • All tests pass locally (npm test).
  • The application builds successfully with my changes.
• Branch Hygiene: My branch is up-to-date (rebased) with the main branch.
• Documentation Impact: I have considered if my changes require documentation updates (see "Documentation Updates" section below).
• Changeset: A changeset has been created using npm run changeset if this PR includes user-facing changes or dependency updates.
• Contribution Guidelines: I have read and agree to the Contributor Guidelines.

Screenshots / Videos

N/A

Documentation Updates

Does this PR necessitate updates to user-facing documentation?

• No documentation updates are required.
• Yes, documentation updates are required. (Please describe what needs to be updated or link to a PR in the docs repository).

Get in Touch

On Discord as Ilintar

---

Important

Enhances searchAndReplaceTool.ts to handle multiple matches with error prompts and adds test cases for various scenarios.

• Behavior:
  • Adds condition in searchAndReplaceTool.ts to return an error when multiple matches are found without line range, prompting for more specific search criteria.
  • Supports successful replacement with line range or refined search criteria.
• Tests:
  • Adds test cases in searchAndReplaceTool.test.ts for multiple matches, regex patterns, and case-insensitive search.
• Misc:
  • No changes to existing functionality; only enhancements and additional test coverage.

^{This description was created by for 8e0a1145f34dd2de05a6947d653f4bccd717dab1. You can customize this summary. It will automatically update as commits are pushed.}

[pwilkin]

I think this catches all the cases, made the error message properly variable to account for the line range case.

[daniel-lxs]

Thank you @pwilkin for your contribution and the effort you put into this PR!

After reviewing the changes and the original issue #4744, I believe there might be a misunderstanding. The issue describes a problem with the apply_diff tool (which uses the <<<<<<< SEARCH / >>>>>>> REPLACE format), but this PR modifies the search_and_replace tool.

The search_and_replace tool is actually designed to replace all matches when using the global flag, which is its intended behavior. It already has optional start_line and end_line parameters that allow the model to limit the scope of replacements when needed.

The original issue appears to be about the apply_diff tool incorrectly matching a } character in a JSON documentation string when trying to add code at the end of a file. That's a different tool with different matching behavior and it will hopefully be fixed by #4777. I got confused as well initially when reviewing this!

That said, I appreciate the thorough implementation and comprehensive test suite you've added. The code quality is good and follows the project's patterns well. I would be happy to see you contribute in the future!

[pwilkin]

Oh damn, you are right :) I actually fell into this trap because of the naming conventions ;)