feat(diff): add inline diff editor for file edits and enhance symbol diffing logic

Author: AndyLiner13

src/client-pipe.ts

@@ -879,6 +879,24 @@ export function fileHighlightReadRange(
   });
 }
 
+/**
+ * Open an inline diff editor showing old vs new content after an edit.
+ * Old content was pre-captured by the extension's handleFileApplyEdit.
+ * Fire-and-forget — does not block the tool response.
+ */
+export function fileShowEditDiff(
+  filePath: string,
+  editStartLine: number,
+): void {
+  sendClientRequest(
+    'file.showEditDiff',
+    {filePath, editStartLine},
+    10_000,
+  ).catch(() => {
+    // Best-effort — don't let diff viewer failures affect tool responses
+  });
+}
+
 /**
  * Apply a text replacement (range → new content) and save.
  */

src/tools/file/safety-layer.ts

@@ -7,14 +7,16 @@
 import {
   fileGetSymbols,
   fileApplyEdit,
+  fileReadContent,
   fileGetDiagnostics,
   fileExecuteRename,
   fileFindReferences,
   fileGetCodeActions,
   fileApplyCodeAction,
+  fileShowEditDiff,
   type NativeDocumentSymbol,
 } from '../../client-pipe.js';
-import {diffSymbols, extractNewName} from './symbol-diff.js';
+import {diffSymbols, type EditInfo} from './symbol-diff.js';
 import type {
   DetectedIntent,
   PropagatedChange,
@@ -27,11 +29,63 @@ const DIAGNOSTIC_SETTLE_DELAY_MS = 800;
 const MAX_AUTO_FIX_ATTEMPTS = 5;
 
 /**
- * Execute a file edit with the full safety layer:
+ * Spelling-related Code Action titles that can corrupt semantics after deletions.
+ * These "fix" missing references by renaming to similarly-named symbols,
+ * which changes program behavior rather than fixing an actual typo.
+ */
+const HARMFUL_FIX_PATTERNS = [
+  /change spelling/i,
+  /did you mean/i,
+];
+
+/**
+ * Check whether a Code Action is potentially harmful given the edit context.
+ * Spelling corrections after deliberate deletions can silently corrupt semantics —
+ * e.g. renaming `level15()` to `level5()` after the user deleted `level15`.
+ */
+function isHarmfulAutoFix(title: string, hasDeleteIntents: boolean): boolean {
+  if (!hasDeleteIntents) return false;
+  return HARMFUL_FIX_PATTERNS.some(pattern => pattern.test(title));
+}
+
+/**
+ * Find a symbol by qualified name (e.g. `ParentName.childName`).
+ * For top-level symbols, the name is matched directly.
+ * For child symbols, splits on `.` and walks the hierarchy.
+ */
+function findSymbolByQualifiedName(
+  symbols: NativeDocumentSymbol[],
+  qualifiedName: string,
+): NativeDocumentSymbol | undefined {
+  const parts = qualifiedName.split('.');
+  if (parts.length === 1) {
+    return symbols.find(s => s.name === qualifiedName);
+  }
+  const parent = symbols.find(s => s.name === parts[0]);
+  if (!parent?.children) return undefined;
+  const childName = parts.slice(1).join('.');
+  return findSymbolByQualifiedName(parent.children, childName);
+}
+
+/**
+ * Normalize file paths for comparison (forward slashes, lowercase on Windows).
+ */
+function normalizePath(p: string): string {
+  return p.replace(/\\/g, '/').toLowerCase();
+}
+
+/**
+ * Execute a file edit with the full safety layer.
  *
- * Phase 1: Virtual pre-check — compare old vs new DocumentSymbols to detect intents
- * Phase 2: Atomic apply — apply the edit + propagate renames
- * Phase 3: Auto-fix — apply Code Actions for new errors
+ * All symbol detection uses VS Code's DocumentSymbol provider — zero regex.
+ *
+ * Flow:
+ *   Phase 0: Snapshot old symbols + old content
+ *   Phase 1: Tentatively apply the edit
+ *   Phase 2: Semantic intent detection via diffSymbols (DocumentSymbol diff)
+ *   Phase 3: If deletes with external refs → revert and block
+ *   Phase 4: If renames → revert, execute VS Code rename provider, re-apply body
+ *   Phase 5: Auto-fix via Code Actions, final diagnostics
  */
 export async function executeEditWithSafetyLayer(
   filePath: string,
@@ -43,25 +97,25 @@ export async function executeEditWithSafetyLayer(
   const propagated: PropagatedChange[] = [];
   const autoFixed: AutoFix[] = [];
 
-  // ── Phase 1: Pre-check — Get symbols before edit ────────────────
+  // ── Phase 0: Snapshot ───────────────────────────────────────────
   let oldSymbols: NativeDocumentSymbol[] = [];
   try {
     const beforeResult = await fileGetSymbols(filePath);
     oldSymbols = beforeResult.symbols;
   } catch {
-    // No symbol provider available — proceed without intent detection
+    // No symbol provider — proceed without safety checks
   }
 
-  // Get pre-existing diagnostics to compare after edit
-  let preExistingErrors = 0;
+  // Read old content of the edit range for potential revert
+  let oldContent: string | undefined;
   try {
-    const preDiags = await fileGetDiagnostics(filePath);
-    preExistingErrors = preDiags.diagnostics.filter(d => d.severity.toLowerCase() === 'error').length;
+    const contentResult = await fileReadContent(filePath, startLine, endLine);
+    oldContent = contentResult.content;
   } catch {
-    // Best-effort
+    // Best-effort — revert won't be possible
   }
 
-  // ── Phase 2: Apply the edit ─────────────────────────────────────
+  // ── Phase 1: Tentatively apply the edit ─────────────────────────
   try {
     await fileApplyEdit(filePath, startLine, endLine, newContent);
   } catch (err) {
@@ -77,38 +131,100 @@ export async function executeEditWithSafetyLayer(
     };
   }
 
-  // ── Phase 1b: Post-apply intent detection ───────────────────────
-  // Get symbols after apply for comparison
-  let newSymbols: NativeDocumentSymbol[] = [];
+  // ── Phase 2: Semantic intent detection ──────────────────────────
+  // Get new symbols via VS Code's DocumentSymbol provider — the only source of truth.
+  let allIntents: DetectedIntent[] = [];
+
   if (oldSymbols.length > 0) {
     try {
       const afterResult = await fileGetSymbols(filePath);
-      newSymbols = afterResult.symbols;
-      const intents = diffSymbols(oldSymbols, newSymbols);
-      detectedIntents.push(...intents);
+      const newLineCount = newContent.split('\n').length;
+      const oldLineCount = endLine - startLine + 1;
+      const editInfoForDiff: EditInfo = {
+        newContentEndLine: startLine + newLineCount - 1,
+        linesDelta: newLineCount - oldLineCount,
+      };
+
+      allIntents = diffSymbols(oldSymbols, afterResult.symbols, editInfoForDiff);
     } catch {
       // Proceed without intent detection
     }
   }
 
-  // ── Phase 2b: Rename propagation ───────────────────────────────
-  const renames = detectedIntents.filter(i => i.type === 'rename');
-  for (const renameIntent of renames) {
-    const newName = extractNewName(renameIntent);
-    if (!newName) continue;
+  const renames = allIntents.filter(i => i.type === 'rename' && i.newName);
+  const deletes = allIntents.filter(i => i.type === 'delete');
 
+  // ── Phase 3: Revert + delete protection + rename propagation ────
+  if ((renames.length > 0 || deletes.length > 0) && oldContent !== undefined) {
+    // Revert to original state so we can check refs / execute renames
+    const tentativeEndLine = startLine + newContent.split('\n').length - 1;
     try {
-      // Find a reference to the OLD name in another file to trigger rename
-      const refs = await fileFindReferences(filePath, 0, 0);
-      // Search for a reference in a DIFFERENT file to propagate cross-file
-      const externalRef = refs.references.find(r => !r.file.endsWith(filePath.replace(/\\/g, '/')));
+      await fileApplyEdit(filePath, startLine, tentativeEndLine, oldContent);
+    } catch {
+      // Can't revert — skip protection and propagation, edit stays applied
+      detectedIntents.push(...allIntents);
+      return finalize(filePath, detectedIntents, propagated, autoFixed, startLine);
+    }
+
+    // Check deletes for external references
+    for (const del of deletes) {
+      const oldSym = oldSymbols.find(s => s.name === del.symbol);
+      if (!oldSym) continue;
+
+      try {
+        const refs = await fileFindReferences(
+          filePath,
+          oldSym.selectionRange.startLine,
+          oldSym.selectionRange.startChar,
+        );
+        const normalizedFilePath = normalizePath(filePath);
+        const externalRefs = refs.references.filter(r => {
+          const refPath = normalizePath(r.file);
+          return (
+            refPath !== normalizedFilePath &&
+            !refPath.endsWith(normalizedFilePath) &&
+            !normalizedFilePath.endsWith(refPath)
+          );
+        });
+
+        if (externalRefs.length > 0) {
+          const refFiles = [...new Set(externalRefs.map(r => r.file))];
+          return {
+            success: false,
+            file: filePath,
+            detectedIntents: [{type: 'delete', symbol: del.symbol}],
+            propagated: [],
+            autoFixed: [],
+            remainingErrors: [],
+            summary:
+              `Blocked: Cannot delete '${del.symbol}' — it has ${externalRefs.length} ` +
+              `reference(s) in ${refFiles.length} other file(s): ` +
+              `${refFiles.slice(0, 5).join(', ')}` +
+              `${refFiles.length > 5 ? ` and ${refFiles.length - 5} more` : ''}. ` +
+              `Resolve or remove these references first.`,
+          };
+        }
+      } catch {
+        // Can't check references — allow
+      }
+    }
 
-      if (externalRef) {
+    // Execute VS Code rename provider for each rename.
+    // File is in ORIGINAL state — old names exist, so the provider can resolve all refs.
+    for (const renameIntent of renames) {
+      if (!renameIntent.newName) continue;
+
+      // Find the old symbol — supports both top-level and child renames.
+      // Child renames use `ParentName.childName` notation.
+      const oldSym = findSymbolByQualifiedName(oldSymbols, renameIntent.symbol);
+      if (!oldSym) continue;
+
+      try {
         const renameResult = await fileExecuteRename(
-          externalRef.file,
-          externalRef.line - 1,
-          externalRef.character,
-          newName,
+          filePath,
+          oldSym.selectionRange.startLine,
+          oldSym.selectionRange.startChar,
+          renameIntent.newName,
         );
         if (renameResult.success) {
           propagated.push({
@@ -117,15 +233,84 @@ export async function executeEditWithSafetyLayer(
             totalEdits: renameResult.totalEdits,
           });
         }
+      } catch {
+        // Rename provider failed — body edit will still be applied
       }
-    } catch {
-      // Rename propagation is best-effort
+
+      detectedIntents.push(renameIntent);
+    }
+
+    // After renames, re-resolve the edit range (rename may have changed positions)
+    let editStartLine = startLine;
+    let editEndLine = endLine;
+
+    if (propagated.length > 0) {
+      try {
+        const refreshed = await fileGetSymbols(filePath);
+        for (const renameIntent of renames) {
+          if (!renameIntent.newName) continue;
+          const renamedSym = refreshed.symbols.find(s => s.name === renameIntent.newName);
+          if (renamedSym) {
+            editStartLine = renamedSym.range.startLine;
+            editEndLine = renamedSym.range.endLine;
+          }
+        }
+        // Update oldSymbols so post-hoc diff sees the renamed state
+        oldSymbols = refreshed.symbols;
+      } catch {
+        // Fall back to original range
+      }
+    }
+
+    // Re-apply the body edit (newContent already contains the new name + new body)
+    try {
+      await fileApplyEdit(filePath, editStartLine, editEndLine, newContent);
+    } catch (err) {
+      const msg = err instanceof Error ? err.message : String(err);
+      return {
+        success: false,
+        file: filePath,
+        detectedIntents,
+        propagated,
+        autoFixed: [],
+        remainingErrors: [],
+        summary: `Edit failed on re-apply after rename: ${msg}`,
+      };
+    }
+  }
+
+  // Add non-rename/non-delete intents (body_change, add)
+  for (const intent of allIntents) {
+    if (intent.type !== 'rename' && intent.type !== 'delete') {
+      detectedIntents.push(intent);
     }
   }
+  // Add delete intents that weren't blocked (no external refs)
+  for (const del of deletes) {
+    detectedIntents.push(del);
+  }
+
+  return finalize(filePath, detectedIntents, propagated, autoFixed, startLine);
+}
 
-  // ── Phase 3: Auto-fix via Code Actions ─────────────────────────
+/**
+ * Post-edit finalization: show diff, run auto-fix, collect diagnostics.
+ */
+async function finalize(
+  filePath: string,
+  detectedIntents: DetectedIntent[],
+  propagated: PropagatedChange[],
+  autoFixed: AutoFix[],
+  editStartLine: number,
+): Promise<FileEditResult> {
+  // Show inline diff editor (fire-and-forget)
+  fileShowEditDiff(filePath, editStartLine);
+
+  // ── Auto-fix via Code Actions ──────────────────────────────────
   await delay(DIAGNOSTIC_SETTLE_DELAY_MS);
 
+  const hasDeletes = detectedIntents.some(i => i.type === 'delete');
+
   try {
     const postDiags = await fileGetDiagnostics(filePath);
     const newErrors = postDiags.diagnostics.filter(d => d.severity.toLowerCase() === 'error');
@@ -137,7 +322,7 @@ export async function executeEditWithSafetyLayer(
       try {
         const actions = await fileGetCodeActions(filePath, error.line - 1, error.endLine - 1);
         const preferred = actions.actions.find(a => a.isPreferred);
-        if (preferred) {
+        if (preferred && !isHarmfulAutoFix(preferred.title, hasDeletes)) {
           const applyResult = await fileApplyCodeAction(
             filePath,
             error.line - 1,
@@ -174,7 +359,6 @@ export async function executeEditWithSafetyLayer(
     // Best-effort
   }
 
-  // ── Build summary ──────────────────────────────────────────────
   const summary = buildSummary(detectedIntents, propagated, autoFixed, remainingErrors);
 
   return {