Address PR review comments for line number handling

- Normalize CRLF to LF in addLineNumberPrefixes and read_file line-range branch to avoid embedded \r characters in output
- Require at least 2 sequential line numbers for stripping (prevents false positives on single-line content)
- Add fallback to original content when line number stripping causes match failure (handles false positive edge cases)
- Only call unescapeMarkers for diff-format blocks, not 3-arg direct invocation path
- Clamp displayed line range in error messages to actual file length
- Update test expectations for new single-line behavior

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Author: wwwillchen

src/pro/main/ipc/handlers/local_agent/tools/read_file.ts

@@ -96,10 +96,12 @@ export const readFileTool: ToolDefinition<z.infer<typeof readFileSchema>> = {
       return addLineNumberPrefixes(content);
     }
 
-    const hasTrailingNewline = content.endsWith("\n");
-    const lines = (hasTrailingNewline ? content.slice(0, -1) : content).split(
-      "\n",
-    );
+    // Normalize CRLF to LF before line operations to avoid embedded \r characters
+    const normalized = content.replace(/\r\n/g, "\n");
+    const hasTrailingNewline = normalized.endsWith("\n");
+    const lines = (
+      hasTrailingNewline ? normalized.slice(0, -1) : normalized
+    ).split("\n");
     const startIdx = Math.max(0, (start ?? 1) - 1);
     const endIdx = Math.min(lines.length, end ?? lines.length);
     const result = lines.slice(startIdx, endIdx).join("\n");

src/pro/main/ipc/processors/line_number_utils.spec.ts

@@ -146,10 +146,12 @@ describe("line_number_utils", () => {
       expect(result.hasLineNumbers).toBe(false);
     });
 
-    it("strips line numbers from single line", () => {
+    it("does not strip line numbers from single line (too ambiguous)", () => {
+      // Single lines matching the pattern are too ambiguous to confidently strip
+      // (e.g., "42| some data" could be actual file content, not a line number prefix)
       const result = stripLineNumberPrefixes("1| hello");
-      expect(result.content).toBe("hello");
-      expect(result.hasLineNumbers).toBe(true);
+      expect(result.content).toBe("1| hello");
+      expect(result.hasLineNumbers).toBe(false);
     });
 
     it("strips line numbers from multiple lines", () => {

src/pro/main/ipc/processors/line_number_utils.ts

@@ -40,7 +40,8 @@ export function addLineNumberPrefixes(
     return "";
   }
 
-  const lines = content.split("\n");
+  // Normalize CRLF to LF for consistent output (line numbers are for display to LLM, not for writing back)
+  const lines = content.split(/\r?\n/);
   const totalLines = lines.length;
   // Calculate width based on the largest line number
   const maxLineNumber = startLineNumber + totalLines - 1;
@@ -71,7 +72,8 @@ export function stripLineNumberPrefixes(content: string): {
     return { content: "", hasLineNumbers: false };
   }
 
-  const lines = content.split("\n");
+  // Normalize CRLF to LF for consistent processing
+  const lines = content.split(/\r?\n/);
   const extractedNumbers: number[] = [];
 
   // Check if all non-empty lines have line number prefixes
@@ -90,14 +92,20 @@ export function stripLineNumberPrefixes(content: string): {
     return { content, hasLineNumbers: false };
   }
 
+  // Require at least 2 line numbers to validate sequentiality.
+  // Single lines matching the pattern are too ambiguous to confidently strip
+  // (e.g., "42| some data" could be actual file content, not a line number prefix).
+  // Also handles the all-empty-lines edge case where extractedNumbers would be empty.
+  if (extractedNumbers.length < 2) {
+    return { content, hasLineNumbers: false };
+  }
+
   // Verify that extracted line numbers are sequential (monotonically increasing by 1)
   // This dramatically reduces false positives on content that coincidentally matches
   // the line number pattern (e.g., "1| Alice", "2| Bob" as data content)
-  if (extractedNumbers.length > 1) {
-    for (let i = 1; i < extractedNumbers.length; i++) {
-      if (extractedNumbers[i] !== extractedNumbers[i - 1] + 1) {
-        return { content, hasLineNumbers: false };
-      }
+  for (let i = 1; i < extractedNumbers.length; i++) {
+    if (extractedNumbers[i] !== extractedNumbers[i - 1] + 1) {
+      return { content, hasLineNumbers: false };
     }
   }
 

src/pro/main/ipc/processors/search_replace_line_numbers_processor.ts

@@ -184,8 +184,13 @@ function generateNoMatchError(
   });
 
   lines.push("");
+  // Clamp the displayed end line to actual file length
+  const displayEnd = Math.min(
+    bestMatch.startIndex + searchLines.length,
+    resultLines.length,
+  );
   lines.push(
-    `BEST PARTIAL MATCH (${bestMatch.matchingLines}/${searchLines.length} lines matched at file lines ${bestMatch.startIndex + 1}-${bestMatch.startIndex + searchLines.length}):`,
+    `BEST PARTIAL MATCH (${bestMatch.matchingLines}/${searchLines.length} lines matched at file lines ${bestMatch.startIndex + 1}-${displayEnd}):`,
   );
 
   for (let j = 0; j < searchLines.length; j++) {
@@ -393,16 +398,30 @@ export function applySearchReplaceWithLineNumbers(
   let resultLines = originalContent.split(/\r?\n/);
   let appliedCount = 0;
 
+  // Track whether we're processing diff-format blocks (2-arg path) vs direct invocation (3-arg path)
+  const isDiffFormat = newContent === undefined;
+
   for (const block of blocks) {
     let { searchContent, replaceContent } = block;
 
-    searchContent = unescapeMarkers(searchContent);
-    replaceContent = unescapeMarkers(replaceContent);
+    // Only unescape markers for diff-format blocks (they were escaped by escapeSearchReplaceMarkers).
+    // In the 3-arg direct invocation path, content was never escaped, so skip unescaping
+    // to avoid corrupting user content that happens to contain backslash-prefixed markers.
+    if (isDiffFormat) {
+      searchContent = unescapeMarkers(searchContent);
+      replaceContent = unescapeMarkers(replaceContent);
+    }
+
+    // Save original content before stripping line numbers for fallback
+    const originalSearchContent = searchContent;
+    const originalReplaceContent = replaceContent;
+    let usedLineNumberStripping = false;
 
     // Strip line numbers from search content if present
     const strippedSearch = stripLineNumberPrefixes(searchContent);
     if (strippedSearch.hasLineNumbers) {
       searchContent = strippedSearch.content;
+      usedLineNumberStripping = true;
       logger.debug("Stripped line number prefixes from search content");
     }
 
@@ -447,6 +466,33 @@ export function applySearchReplaceWithLineNumbers(
       }
     }
 
+    // If still no match and we stripped line numbers, try again with original (unstripped) content.
+    // This handles false positives where the file actually contains the N| pattern as real content.
+    if (
+      matchResult.error &&
+      !matchResult.ambiguousPositions &&
+      usedLineNumberStripping
+    ) {
+      const originalSearchLines =
+        originalSearchContent === ""
+          ? []
+          : originalSearchContent.split(/\r?\n/);
+      const originalMatchResult = cascadingMatch(
+        resultLines,
+        originalSearchLines,
+      );
+      if (!originalMatchResult.error) {
+        matchResult = originalMatchResult;
+        searchLines = originalSearchLines;
+        replaceContent = originalReplaceContent;
+        replaceLines =
+          replaceContent === "" ? [] : replaceContent.split(/\r?\n/);
+        logger.debug(
+          "Matched after falling back to original (un-stripped) search content",
+        );
+      }
+    }
+
     if (matchResult.error) {
       // Log detailed diagnostic information for debugging
       logMatchFailure(resultLines, searchLines, appliedCount);