refactor(edit): replace coarse whitespace normalization with layered matching pipeline by Betterlol · Pull Request #1256 · moonbitlang/openseek

Betterlol · 2026-09-03T10:26:15Z

Summary

Replace the old coarse-grained whitespace normalization matching (tab/space/CRLF/LF all lumped together) with a precise layered matching pipeline, inspired by Claude Code's FileEditTool.

Matching Pipeline (3 layers)

CRLF normalization — \r\n → \n at the very start, with an offset map to map match positions back to original content. CRLF is never part of "fuzzy" matching; it's a transparent normalization.
Exact match — on CRLF-normalized content.
Quote normalization — curly/smart quotes (\u2018/\u2019/\u201C/\u201D) → straight quotes, as a separate fallback layer. Models cannot output curly quotes, so this bridges the gap.

Removed

normalize_whitespace — collapsed tab/space/CR/LF into a single space
find_whitespace_normalized_candidates — coarse fuzzy matching based on above
is_whitespace_equivalent — generic "whitespace-only-differences" check

Error Message Structure

Hints now appear before the file context, with code blocks and section separators:

— the file uses CRLF line endings; old_string uses LF
— the file contains curly/smart quotes

─── file context ───────────────────────────────

1 | alpha
2 | beta


─── first difference ───────────────────────────
at character 1: expected `d`, got `b`
— CRLF vs LF line-ending mismatch

─── hint ───────────────────────────────────────
check for CRLF vs LF line endings, tab vs space...

Not Yet Implemented

preserve_quote_style: new_string's straight quotes are not automatically converted back to curly quotes on write

…matching pipeline - CRLF normalization: normalize \\r\\n to \\n at the start for matching, build offset map to map match positions back to original content - Quote normalization: curly/smart quotes -> straight quotes as a separate fallback layer (models cannot output curly quotes) - Exact match on CRLF-normalized content, then quote-normalized match, then error with specific hints (CRLF, curly quotes) - Remove normalize_whitespace / find_whitespace_normalized_candidates / is_whitespace_equivalent (coarse tab/space/CRLF lumped together) - Update old_string_context with targeted diagnostics per mismatch type instead of generic 'whitespace-only-differences' - preview_replace_all also uses CRLF-normalized content for matching

…ode blocks - File-level hints (CRLF, curly quotes) now appear BEFORE file context - File context wrapped in a code block (`) for visual separation - Sections delimited by em-dash separators (─── file context ───) - old_string_context now accepts optional file_hints parameter - Tests updated to match new section headers

…remove obsolete fuzzy tests

@matching

…norm in multi_edit - Create agent_tool/internal/matching/ with normalize_content_for_matching, normalize_quotes, find_quote_normalized_match, has_crlf, has_curly_quotes - edit.mbt: remove duplicate definitions, use @matching.* - multi_edit.mbt: validate_edit now normalizes CRLF->LF before matching, falls back to quote normalization, and maps offsets back to original content via offset_map - Fix Windows path separator tests in both edit_test and multi_edit_test

Copilot

🟡 Changes recommended

There are confirmed correctness issues in CRLF normalization/matching and in user-facing diagnostics formatting that can lead to failed matches or misleading output.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR refactors the edit/multi-edit matching logic by replacing the previous coarse whitespace-normalized fuzzy matching with a layered matching pipeline (CRLF normalization → exact match → quote-normalized fallback), and updates diagnostics/tests accordingly.

Changes:

Introduces agent_tool/internal/matching with CRLF normalization + quote normalization helpers and integrates it into edit and multi_edit.
Reworks edit “old_string not found” diagnostics to be structured (section headers + code blocks) and adds targeted mismatch hints.
Updates tests to reflect new output format and relaxes some assertions to avoid brittle full-path comparisons.

File summaries

File	Description
agent_tool/multi_edit/multi_edit.mbt	Switches multi-edit validation to CRLF-normalized matching with quote-normalized fallback and offset remapping.
agent_tool/multi_edit/multi_edit_test.mbt	Updates assertions to be less brittle about full output formatting/paths.
agent_tool/multi_edit/moon.pkg	Adds dependency on the new internal matching package.
agent_tool/internal/matching/matching.mbt	New matching utilities: CRLF normalization with offset map + quote normalization + detectors.
agent_tool/internal/matching/moon.pkg	New package config (warnings).
agent_tool/internal/matching/pkg.generated.mbti	Generated public interface for the new matching package.
agent_tool/edit/moon.pkg	Adds dependency on the new internal matching package.
agent_tool/edit/edit.mbt	Integrates matching pipeline, updates replacement span mapping, and rewrites “old_string not found” diagnostics.
agent_tool/edit/edit_test.mbt	Updates expected diagnostics strings and adds platform gating for some tests.

Review details

Suppressed comments (1)

agent_tool/edit/edit.mbt:991

The curly-quote mismatch hint currently renders the "curly quotes" examples as straight quotes ("/"), which makes the diagnostic ambiguous/misleading. Show the actual curly quote characters (via \u escapes) so users can visually distinguish them.

          (actual_char == "\u{201C}" || actual_char == "\u{201D}") {
          result.write_string(
            "\n— curly vs straight double-quote mismatch: the file uses curly quotes (`\"`/`\"`), but models can only output straight quotes (`\"`); re-read the file to get the exact text",
          )

Files reviewed: 9/9 changed files
Comments generated: 6
Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Betterlol · 2026-09-08T09:43:48Z

+  let orig_end = if norm_global + input.old_string.length() <
+    offset_map.length() {
+    offset_map[norm_global + input.old_string.length()]
+  } else {
+    content.length()
+  }


Real Problem: Fixing Done.

Betterlol · 2026-09-08T09:45:15Z

+  let (norm_content, offset_map) = @matching.normalize_content_for_matching(
    content,
-    region.prefix.length() + match_start,
  )
-  let new_body = "\{region.body.unsafe_substring(start=0, end=match_start)}\{input.new_string}\{region.body.unsafe_substring(start=match_end, end=region.body.length())}"
-  let new_content = "\{region.prefix}\{new_body}\{region.suffix}"
+  let region = select_edit_region(norm_content, input)
+  let (match_start, is_quote_match) = match region.body.find(input.old_string) {


Real Problem: Fixing Done.

Betterlol · 2026-09-08T09:58:14Z

+    } else {
+      match content[i].to_char() {
+        Some(ch) => {
+          result.write_char(ch)
+          i += 1
+        }
+        _ => break
+      }
+    }


+  let (norm_content, offset_map) = @matching.normalize_content_for_matching(
+    content,
+  )
+  let region = select_edit_region(norm_content, item.start_line, item.end_line)
+  let match_offset = match region.body.find(item.old_string) {


+  let result = StringBuilder()
+  // File-level hints first, before the context
+  if file_hints.length() > 0 {
+    result.write_string(file_hints)
+    result.write_string("\n\n")
+  }


+          result.write_string(
+            "\n— curly vs straight single-quote mismatch: the file uses curly quotes (`'`/`'`), but models can only output straight quotes (`'`); re-read the file to get the exact text",
+          )


…dit and multi_edit When old_string contains CRLF, its un-normalized length is larger than the normalized length. Using the raw length to index offset_map would overshoot, potentially deleting too much content. Normalize old_string first and use its normalized length.

…dit and multi_edit Previously only file content was normalized (CRLF->LF), but old_string was searched as-is. An old_string containing CRLF would never match the LF-only body. Now old_string is normalized through the same CRLF->LF layer before both exact and quote-normalized matching.

…hints These hints mislead the model into thinking CRLF/curly quotes are the cause of a mismatch, when the real issue is often a different old_string.

…aracters to_char() returns None for surrogate halves, which caused the loop to break and silently truncate content containing non-BMP characters. Use unsafe_substring to write code units directly as strings instead.

…scapes - Add \n\n before old_string_context output so sections render on separate lines from the error header - Show curly quote characters as \u{2018}/\u{2019} in mismatch diagnostic so they are visually distinguishable from straight quotes

Copilot

🟡 Changes recommended

The new CRLF-normalized matching can cause edits to introduce mixed line endings by inserting LF-only new_string into CRLF files, which is a behavioral risk that should be addressed before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (2)

Previously missed (2) — in code that hasn't changed since the last review.

agent_tool/edit/edit.mbt:832

EditRegion.suffix is no longer referenced in this file, and the new #warnings("-unused_field") suppression appears to exist solely to silence that. Consider removing the unused suffix field (and the warning suppression) to keep the region type minimal and avoid masking genuine unused-field warnings on this definition.

///|
#warnings("-unused_field")
priv struct EditRegion {
  prefix : String
  body : String
  suffix : String
}

agent_tool/edit/edit.mbt:893

The doc comment says this diagnostic prints “file-level hints first”, but the implementation writes the file context block first and emits specific mismatch hints later under “first difference” / “hint”. Update the comment to match the actual output order (or reorder the output to match the comment).

Files reviewed: 9/9 changed files
Comments generated: 2
Review effort level: Lite

+  let match_line = line_number_at_offset(content, orig_global)
+  let new_content = "\{content.unsafe_substring(start=0, end=orig_global)}\{input.new_string}\{content.unsafe_substring(start=orig_end, end=content.length())}"
+  let summary = if is_quote_match {


  Valid({
    index,
-    start,
-    end: start + item.old_string.length(),
+    start: orig_start,
+    end: orig_end,
    new_string: item.new_string,


Betterlol force-pushed the jiacheng/edit branch 2 times, most recently from 954e886 to 6a47b0f Compare September 3, 2026 10:40

Betterlol marked this pull request as ready for review September 3, 2026 10:40

Betterlol force-pushed the jiacheng/edit branch 3 times, most recently from 0c6f743 to d7ee161 Compare September 5, 2026 07:21

Betterlol requested a review from tonyfettes September 5, 2026 07:38

Betterlol force-pushed the jiacheng/edit branch 2 times, most recently from b6901d3 to 4ec5a68 Compare September 8, 2026 08:50

Betterlol added 7 commits September 8, 2026 16:50

chore: ignore 'suffix' unused warnning

1e0116e

fix(edit): restore missing 'old_string not found' prefix, fix tests, …

2e7254e

…remove obsolete fuzzy tests

fix(test): fix native test

168ba07

fix: remove duplicate codes, add #cfg #warning

6a8561e

Betterlol force-pushed the jiacheng/edit branch from 4ec5a68 to 6a8561e Compare September 8, 2026 08:51

Betterlol requested a lite review from Copilot September 8, 2026 08:54

Copilot started reviewing on behalf of Betterlol September 8, 2026 08:55 View session

Copilot AI reviewed Sep 8, 2026

View reviewed changes

Betterlol added 6 commits September 8, 2026 17:48

remove has_crlf/has_curly_quotes from matching module and edit error …

b29eab4

…hints These hints mislead the model into thinking CRLF/curly quotes are the cause of a mismatch, when the real issue is often a different old_string.

chore: run moon info

b5f7266

Betterlol requested a lite review from Copilot September 8, 2026 10:04

Copilot started reviewing on behalf of Betterlol September 8, 2026 10:05 View session

Copilot AI reviewed Sep 8, 2026

View reviewed changes

Navigation Menu

Uh oh!

refactor(edit): replace coarse whitespace normalization with layered matching pipeline - #1256

refactor(edit): replace coarse whitespace normalization with layered matching pipeline#1256
Betterlol wants to merge 13 commits into
moonbitlang:mainfrom
Betterlol:jiacheng/edit

Betterlol commented Sep 3, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Betterlol Sep 8, 2026

Uh oh!

Betterlol Sep 8, 2026

Uh oh!

Betterlol Sep 8, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

Betterlol commented Sep 3, 2026

Summary

Matching Pipeline (3 layers)

Removed

Error Message Structure

Not Yet Implemented

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (1)

Uh oh!

Betterlol Sep 8, 2026

Choose a reason for hiding this comment

Uh oh!

Betterlol Sep 8, 2026

Choose a reason for hiding this comment

Uh oh!

Betterlol Sep 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (2)

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants