fix(edit): show best matching region on mismatch instead of raw file head#86168
fix(edit): show best matching region on mismatch instead of raw file head#86168imwyvern wants to merge 1 commit into
Conversation
…head When edit oldText doesn't match exactly, the error hint now locates and displays the most similar region in the file (with line numbers) instead of dumping the first 800 characters. This helps the model quickly spot which lines diverged from its expectation. Falls back to the current full-file-head behavior when no region scores above zero shared lines. Combines the intent of openclaw#56857 and openclaw#42265.
|
Thanks for the context here. I swept through the related work, and this is now duplicate or superseded. Close as superseded: #88531 is the current open replacement for the same closest-match edit hint, while this branch targets the removed Canonical path: Live GitHub search for the central closest-edit-match problem found this PR and the newer open replacement #88531; older related attempts such as #42265 are closed. So I’m closing this here and keeping the remaining discussion on #88531 and #42265. Review detailsBest possible solution: Close this stale branch and continue the closest-match edit hint work in #88531, which targets the current shared edit implementation. Do we have a high-confidence way to reproduce the issue? Yes. Source inspection of current main shows exact-match failures still append a bounded Is this the best way to solve the issue? No. This stale branch is not the best way now because it edits removed Security review: Security review cleared: The diff only changes edit-tool error hint logic and a focused test, with no dependency, workflow, secret, package, or code-execution surface change. AGENTS.md: found and applied where relevant. What I checked:
Likely related people:
Codex review notes: model internal, reasoning high; reviewed against 44e6caff5401. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0c4d4f5ccc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (snippet.length > maxLen) { | ||
| snippet = `${snippet.slice(0, maxLen)}\n... (truncated)`; | ||
| } | ||
| return { snippet, startLine: ctxBefore + 1 }; |
There was a problem hiding this comment.
The line label in the new hint is off by up to two lines because startLine is derived from the context window (ctxBefore + 1) instead of the matched window start. When a match is found away from the file top, the message Best matching region (lines N+) points above the true match and can misdirect follow-up edits to the wrong section. Keep the context lines in the snippet, but report the match anchor (bestStart + 1).
Useful? React with 👍 / 👎.
|
|
||
| let bestScore = 0; | ||
| let bestStart = 0; | ||
|
|
||
| for (let i = 0; i <= contentLines.length - windowSize; i++) { |
There was a problem hiding this comment.
The sliding-window loop never executes when oldText is longer than the current file (contentLines.length - windowSize < 0), so bestScore stays zero and the code falls back to dumping the file head even when the file shares a meaningful leading prefix with oldText. That drops the new “best matching region” behavior in a common mismatch case (extra lines in oldText), making recovery guidance much less useful.
Useful? React with 👍 / 👎.
|
ClawSweeper PR egg 🎁 Pass real behavior proof to wake the egg and unlock a hatchable treat. Where did the egg go?
|
|
Thanks for the PR. Before this can move forward, please add live proof from the affected surface, not just unit tests, mocked tests, or source inspection. A useful proof update should include:
Please redact secrets, tokens, phone numbers, and private message content from logs or screenshots. |
|
Thanks @imwyvern for continuing the closest-match edit hint work here. I am closing this as superseded by #88531 because both PRs track the same edit This branch still targets the older I am keeping the canonical path open at #88531 so validation and follow-up stay in one place. If this branch has a distinct reproduction path that is not covered there, please reply and we can reopen or split it back out. |
Problem
When
editoldText doesn't match the file contents exactly, the current error hint shows the first 800 characters of the file regardless of where the closest match might be. For large files this is rarely helpful — the model sees the file header and has to guess where its oldText diverged.Solution
Replace the raw file-head dump with a best-matching region finder that:
oldText) across the fileoldTextWhen no region scores above zero (totally unrelated content), the behavior falls back to the existing full-file-head hint.
Before:
After:
Changes
src/agents/pi-tools.host-edit.ts— AddfindBestMatchRegion()+countSharedLines(); updateappendMismatchHint()to prefer region-based hintssrc/agents/pi-tools.read.host-edit-recovery.test.ts— Add test for best-match region behaviorTests
All 18 existing + new host-edit-recovery tests pass.
Supersedes #56857 and #42265 (combined approach, fresh implementation on current main).