Skip to content

Clarify search replace line matching#3708

Merged
wwwillchen merged 2 commits into
dyad-sh:mainfrom
wwwillchen:clarify-search-replace-line-matching
Jun 29, 2026
Merged

Clarify search replace line matching#3708
wwwillchen merged 2 commits into
dyad-sh:mainfrom
wwwillchen:clarify-search-replace-line-matching

Conversation

@wwwillchen

@wwwillchen wwwillchen commented Jun 29, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Clarifies that local-agent search_replace matching is line-based, not partial substring matching within a line.
  • Updates both prompt guidance and tool schema descriptions so models include full lines when editing part of a line.
  • Refreshes request/prompt snapshots that serialize the updated guidance.

Note

Low Risk
Prompt and tool-description only; no change to search/replace execution logic.

Overview
Documents that local-agent search_replace matches whole lines, not substrings within a line, and that partial-line edits must use full original and replacement lines.

Updates the tool definition in search_replace.ts (description and old_string schema text), adds matching guidance to file_editing_tool_selection in local_agent_prompt.ts, and refreshes prompt unit snapshots plus E2E request snapshots so serialized tool/prompt payloads stay in sync. rules/local-agent-tools.md now reminds contributors to grep extensionless baselines under e2e-tests/snapshots/ when tool copy changes.

Reviewed by Cursor Bugbot for commit 53041ea. Bugbot is set up for automated code reviews on this repo. Configure here.

Review in cubic

@cursor

cursor Bot commented Jun 29, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_15a47acd-099f-441b-84f8-d58f8e8eaa3d)

@wwwillchen

Copy link
Copy Markdown
Collaborator Author

@BugBot run

@cursor

cursor Bot commented Jun 29, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_7fba3602-73c8-482c-bed5-d1139638c261)

@wwwillchen wwwillchen merged commit b54e4b1 into dyad-sh:main Jun 29, 2026
11 of 13 checks passed

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the search_replace tool description and system prompts to explicitly clarify that matching is line-based and requires full-line matches rather than partial fragments. It also updates corresponding E2E test snapshots, prompt test snapshots, and the local agent tools documentation to reflect these changes and guide future snapshot updates. I have no feedback to provide as there are no review comments.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR clarifies how the local-agent search_replace tool matches text by documenting that matching is line-based (whole-line matching), and updates prompt guidance plus snapshot baselines so serialized tool/prompt payloads stay consistent across tests.

Changes:

  • Updated local-agent prompt guidance to explicitly instruct that partial-line edits must include the full original and replacement lines.
  • Updated search_replace tool schema/description text to reflect line-based matching and discourage partial-line fragments.
  • Refreshed prompt unit snapshots and E2E request snapshots (including extensionless baselines) to match the updated copy.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/prompts/local_agent_prompt.ts Adds explicit guidance that search_replace matches whole lines and requires full-line old/new strings for partial-line edits.
src/prompts/snapshots/local_agent_prompt.test.ts.snap Updates prompt snapshots to reflect the new guidance text.
src/pro/main/ipc/handlers/local_agent/tools/search_replace.ts Updates tool schema and description text to document line-based matching and full-line requirements.
rules/local-agent-tools.md Adds guidance to also grep extensionless E2E snapshot baselines when tool copy changes.
e2e-tests/snapshots/local_agent_explore_code.spec.ts_disabled Refreshes serialized tool description/schema snapshot for search_replace (extensionless baseline).
e2e-tests/snapshots/local_agent_basic.spec.ts_local-agent---dump-request-1.txt Refreshes request snapshot with updated search_replace description/schema text.
e2e-tests/snapshots/local_agent_auto.spec.ts_local-agent---auto-model-1.txt Refreshes request snapshot to include updated prompt/tool copy.
e2e-tests/snapshots/local_agent_advanced.spec.ts_local-agent---mention-apps-1.txt Refreshes request snapshot with updated search_replace description/schema text.
e2e-tests/snapshots/local_agent_advanced.spec.ts_local-agent---mcp-tool-search-1.txt Refreshes request snapshot with updated search_replace description/schema text.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@dyad-assistant dyad-assistant Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude review: 1 inline finding(s).

.string()
.describe(
"The text to replace (must be unique within the file, and must match the file contents exactly, including all whitespace and indentation)",
"The text block to replace. Matching is line-based: each line in old_string must match a whole line in the file, not just a substring within a line. To edit part of a line, include the entire original line in old_string and the entire edited line in new_string. The block must be unique within the file.",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 MEDIUM

old_string field description drops exact-match whitespace guidance

The old old_string schema description explicitly told the model the text "must match the file contents exactly, including all whitespace and indentation." The new description focuses on line-based semantics and drops this exact-match emphasis. While the guidance still exists in the tool's main description body (the UNIQUENESS section still says "Include all whitespace, indentation, and surrounding code exactly as it appears in the file"), the per-field description is the most prominent place for per-parameter guidance. Models sometimes weight per-field descriptions more heavily, so losing the exact-match emphasis here could reduce initial attempt accuracy for whitespace fidelity.

💡 Suggestion: Consider restoring the exact-match phrasing alongside the new line-based guidance: "The text block to replace. Matching is line-based: each line in old_string must match a whole line in the file exactly (including whitespace and indentation), not just a substring within a line. To edit part of a line, include the entire original line in old_string and the entire edited line in new_string. The block must be unique within the file."

@dyad-assistant

Copy link
Copy Markdown
Contributor

🔍 Dyadbot Code Review Summary

Verdict: ✅ YES - Ready to merge
Recommendation: ready

This PR correctly clarifies that the search_replace tool uses line-based matching. The claim is verified by search_replace_processor.ts -- findMatchPositions splits file content into lines and compares line-by-line, so partial-line inputs genuinely won't match. The changes are consistent across all three surfaces (tool definition, system prompt, snapshots), and no snapshot files were missed.

Issues Summary

Severity File Issue
🟡 MEDIUM src/pro/main/ipc/handlers/local_agent/tools/search_replace.ts:33 old_string field description drops exact-match whitespace guidance

MEDIUM: old_string field description drops exact-match whitespace guidance (src/pro/main/ipc/handlers/local_agent/tools/search_replace.ts:33)

The old old_string schema description explicitly told the model the text "must match the file contents exactly, including all whitespace and indentation." The new description focuses on line-based semantics and drops this exact-match emphasis. While the guidance still exists in the tool's main description body (the UNIQUENESS section still says "Include all whitespace, indentation, and surrounding code exactly as it appears in the file"), the per-field description is prominent for model behavior. Consider restoring it alongside the new line-based guidance, e.g.: "...each line in old_string must match a whole line in the file exactly (including whitespace and indentation), not just a substring within a line."

🟢 Low Priority Notes (3 items)
  • Slight wording inconsistency across surfaces - Tool description says "old_string must match whole file lines, not a partial fragment", the Pro prompt says "the target text must match whole file lines, not only a partial fragment", and the Basic prompt omits the negative clause entirely. All are semantically equivalent but create three phrasings to maintain. (src/pro/main/ipc/handlers/local_agent/tools/search_replace.ts, src/prompts/local_agent_prompt.ts)

  • New rule names a specific file that may become stale - The added rule in rules/local-agent-tools.md calls out local_agent_explore_code.spec.ts_disabled by name. If this file is renamed or removed, the example goes stale, though the broader guidance ("search all e2e-tests/snapshots/ baselines") remains valuable. (rules/local-agent-tools.md)

  • No enforcement change needed - The documentation says old_string "must match whole file lines," but no runtime validation is needed because the line-by-line matching architecture already prevents partial-line matches from succeeding. The tool returns a clear error if the model sends a partial line. This is purely an observation that the docs accurately describe existing behavior.


Generated by Dyadbot persona-based code review

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.

2 participants