Skip to content

Suggest closest edit match when oldText is missing#88531

Closed
imwyvern wants to merge 1 commit into
openclaw:mainfrom
imwyvern:feat/edit-fuzzy-match-hint
Closed

Suggest closest edit match when oldText is missing#88531
imwyvern wants to merge 1 commit into
openclaw:mainfrom
imwyvern:feat/edit-fuzzy-match-hint

Conversation

@imwyvern

@imwyvern imwyvern commented May 31, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

  • Add line-numbered closest-match hints when the edit tool cannot find an oldText block.
  • Reuse the edit diff failure path so execution and preview errors share the same hint wording.
  • Keep hint generation bounded for large files, oversized oldText blocks, and expensive character comparisons.

Verification

  • pnpm build passed.
  • node scripts/run-vitest.mjs src/agents/sessions/tools/edit.test.ts passed: 1 file, 9 tests.
  • git diff --check passed.
  • .agents/skills/autoreview/scripts/autoreview --mode branch --base upstream/main passed with no accepted/actionable findings.
  • node scripts/check-changed.mjs --base upstream/main did not complete locally because this checkout has no usable crabbox binary; the local remote-child fallback reached core test typecheck and failed in unchanged upstream file ui/src/ui/control-ui-vite-config.node.test.ts.

Real behavior proof

Behavior addressed: The edit tool now includes a closest matching line hint when oldText is near existing content but not exact.
Real environment tested: Local OpenClaw checkout on feat/edit-fuzzy-match-hint at ca0f52d0818c6913a66a4b4cfd16d6c816c254c9, Node 22.22.2, invoking the production createEditTool against a real temporary file.
Exact steps or command run after this patch: Ran pnpm exec tsx -e ... to create a temp file containing const value = "actual";, call createEditTool(dir).execute(...) with missing oldText const value = "actuel";, and print the thrown edit-tool error.
Evidence after fix: Console output from that real edit-tool invocation:

REAL_EDIT_TOOL_ERROR_START
Could not find the exact text in /tmp/openclaw-edit-proof-Zl7UYp/sample.ts. The old text must match exactly including all whitespace and newlines.
Closest match at line 2 (96% similar): "const value = \"actual\";".
Current file contents:
alpha
const value = "actual";
omega

REAL_EDIT_TOOL_ERROR_END

Observed result after fix: The failed edit reported the closest match at line 2 with similarity and included the real current file contents; no replacement was applied.
What was not tested: Full Vitest suite and remote Testbox proof were not run from this local checkout; broad GitHub CI is running on the pushed PR head.

@openclaw-barnacle openclaw-barnacle Bot added agents Agent runtime and tooling size: S triage: mock-only-proof Candidate: PR proof only shows tests, mocks, snapshots, lint, typecheck, or CI. labels May 31, 2026
@clawsweeper

clawsweeper Bot commented May 31, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 24, 2026, 10:21 PM ET / 02:21 UTC.

Summary
This PR adds bounded line-numbered closest-match hints to shared edit-tool oldText mismatch errors and focused edit-tool tests.

PR surface: Source +145, Tests +105. Total +250 across 2 files.

Reproducibility: not applicable. as a feature/ergonomics PR rather than a broken existing behavior report. Source inspection shows current main and v2026.6.10 lack closest-match hints on oldText mismatch errors, while the PR body demonstrates the intended production createEditTool path on an older head.

Review metrics: none identified.

Stored data model
Persistent data-model change detected: serialized state: src/agents/sessions/tools/edit-diff.ts, serialized state: src/agents/sessions/tools/edit.test.ts, unknown-data-model-change: src/agents/sessions/tools/edit-diff.ts, unknown-data-model-change: src/agents/sessions/tools/edit.test.ts. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: canonical
Canonical: #88531
Summary: This PR is the current open closest-match hint candidate; older attempts are closed or superseded, while the open P0 fuzzy-edit issue and PR touch the same helper for a distinct data-preservation bug.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🐚 platinum hermit
Result: blocked until stronger real behavior proof is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

Proof guidance:

  • [P1] Needs stronger real behavior proof before merge: The PR body includes useful terminal proof, but it was captured on an older head; rerun the real createEditTool oldText-mismatch invocation on ee672ab, redact private paths/IPs/API keys/phone numbers/endpoints, and update the PR body to trigger review or ask a maintainer for @clawsweeper re-review.

Risk before merge

  • [P1] The supplied terminal proof is useful but was captured on an older head; current-head proof is still needed before this contributor PR clears the real-behavior gate.
  • [P1] The open P0 fuzzy-edit preservation PR touches the same shared helper, so maintainers should coordinate merge order or a combined resolution to avoid delaying or complicating the data-preservation fix.

Maintainer options:

  1. Refresh Proof And Coordinate Helper Order (recommended)
    Rerun the real createEditTool mismatch proof on the current head, then merge only after maintainers choose the order or combined resolution with the open fuzzy-edit preservation PR.
  2. Accept Later Conflict Work
    Maintainers can merge this first if they are prepared to rebase or manually reconcile the adjacent P0 edit fix afterward.

Next step before merge

  • [P1] The remaining blockers are contributor proof refresh and maintainer coordination with the adjacent P0 edit PR, not a narrow automated code repair.

Security
Cleared: The diff changes edit-tool error formatting and colocated tests only, with no dependency, workflow, secret, auth, package, permission, or new code-execution surface.

Review details

Best possible solution:

Land the shared edit-diff hint implementation after current-head terminal proof is refreshed and maintainers choose the merge order or combined resolution with the open fuzzy-edit preservation fix.

Do we have a high-confidence way to reproduce the issue?

Not applicable as a feature/ergonomics PR rather than a broken existing behavior report. Source inspection shows current main and v2026.6.10 lack closest-match hints on oldText mismatch errors, while the PR body demonstrates the intended production createEditTool path on an older head.

Is this the best way to solve the issue?

Yes, with proof refresh and same-helper coordination. Centralizing the hint in applyEditsToNormalizedContent is the maintainable path because execution and preview already share that helper; a caller-only wrapper would duplicate behavior and risk drift.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 1a04b8eb9853.

Label changes

Label justifications:

  • P3: This is a focused edit-tool ergonomics improvement with limited blast radius, not an urgent runtime regression.
  • merge-risk: 🚨 other: The PR touches the same shared edit-diff helper as an open P0 data-preservation fix, so merge order can create maintainer-visible conflict risk.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The PR body includes useful terminal proof, but it was captured on an older head; rerun the real createEditTool oldText-mismatch invocation on ee672ab, redact private paths/IPs/API keys/phone numbers/endpoints, and update the PR body to trigger review or ask a maintainer for @clawsweeper re-review.
Evidence reviewed

PR surface:

Source +145, Tests +105. Total +250 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 149 4 +145
Tests 1 105 0 +105
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 254 4 +250

What I checked:

Likely related people:

  • vincentkoc: GitHub path history shows recent maintenance on the current session edit-diff helper, including trimming session tool internals. (role: recent area contributor; confidence: high; commits: 0455028a3cd2; files: src/agents/sessions/tools/edit-diff.ts)
  • steipete: The current session edit-tool files were carried into the OpenClaw agent runtime during the internalization refactor, with later session tool documentation touches. (role: runtime refactor provenance; confidence: high; commits: bb46b79d3c14, 600a57e60f1e; files: src/agents/sessions/tools/edit-diff.ts, src/agents/sessions/tools/edit.ts)
  • mbelinky: Earlier host-edit recovery hardening shaped the edit mismatch and recovery behavior that was later folded into the current sessions/tools implementation. (role: adjacent edit recovery contributor; confidence: medium; commits: 922f4e66ea1a; files: src/agents/pi-tools.host-edit.ts, src/agents/pi-tools.read.host-edit-recovery.test.ts)
  • stainlu: Recent host edit/write path-resolution work touched the prior edit recovery surface and remains relevant context for edit-target behavior. (role: adjacent path-resolution contributor; confidence: medium; commits: ecfaf64526a7; files: src/agents/pi-tools.host-edit.ts, src/agents/pi-tools.read.host-edit-recovery.test.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. labels May 31, 2026
@imwyvern
imwyvern force-pushed the feat/edit-fuzzy-match-hint branch from 4c48120 to ca0f52d Compare May 31, 2026 15:53
@openclaw-barnacle openclaw-barnacle Bot added size: M proof: supplied External PR includes structured after-fix real behavior proof. and removed size: S triage: mock-only-proof Candidate: PR proof only shows tests, mocks, snapshots, lint, typecheck, or CI. labels May 31, 2026
@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels May 31, 2026
@imwyvern
imwyvern force-pushed the feat/edit-fuzzy-match-hint branch from ca0f52d to 40e86f8 Compare May 31, 2026 17:11
@openclaw-barnacle openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 31, 2026
@imwyvern
imwyvern force-pushed the feat/edit-fuzzy-match-hint branch from 40e86f8 to f22e921 Compare May 31, 2026 17:12
@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels May 31, 2026
@clawsweeper clawsweeper Bot mentioned this pull request Jun 15, 2026
Closed
@imwyvern
imwyvern force-pushed the feat/edit-fuzzy-match-hint branch from f22e921 to ee672ab Compare June 17, 2026 21:36
This was referenced Jun 19, 2026
Closed
Closed
@clawsweeper clawsweeper Bot removed the rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. label Jun 21, 2026
@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. labels Jun 21, 2026
This was referenced Jun 25, 2026
Closed
Closed
Merged
Closed
Closed
@clawsweeper

clawsweeper Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

@imwyvern thanks for the PR. ClawSweeper is still waiting on real behavior proof before this can move forward.

Useful proof can be a screenshot, short video, terminal output, copied live output, linked artifact, or redacted logs that show the changed behavior after the fix. Please redact private tokens, phone numbers, private endpoints, customer data, and anything else sensitive.

Once proof is added to the PR body or a comment, ClawSweeper or a maintainer can re-check it.

@vincentkoc

vincentkoc commented Jul 6, 2026

Copy link
Copy Markdown
Member

Thanks @imwyvern. The canonical fix for #97032 landed through #97038 as 4204a948457cafc601c1f27a1965fcfb011c2131.

We selected #97038 because it keeps the diagnostic in the shared edit-diff owner path, bounds both scan cost and output, and covers typo, indentation, escaping, cap, and surrogate-safe truncation behavior with focused tests. This PR targets the same behavior, so I’m closing it as superseded. Your parallel implementation helped validate the need and solution space.

@vincentkoc vincentkoc closed this Jul 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

agents Agent runtime and tooling merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. proof: supplied External PR includes structured after-fix real behavior proof. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. size: M status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@imwyvern @vincentkoc