fix: revert back to our working diff method (#82)

* fix: diff mode

* test: update test

* test: update test

* chore: update no plan diff template

src/ai/parsers/diff-parser.ts

@@ -7,7 +7,8 @@ export function parseDiffFiles(response: string): AIFileInfo[] {
   const fileRegex =
     /<file>[\s\S]*?<file_path>(.*?)<\/file_path>[\s\S]*?<file_status>(.*?)<\/file_status>[\s\S]*?<file_content language="(.*?)">([\s\S]*?)<\/file_content>(?:[\s\S]*?<explanation>([\s\S]*?)<\/explanation>)?[\s\S]*?<\/file>/gs;
   let match: RegExpExecArray | null;
-  // biome-ignore lint/suspicious/noAssignInExpressions: avoid potential infinite loop
+
+  // biome-ignore lint/suspicious/noAssignInExpressions: <explanation>
   while ((match = fileRegex.exec(response)) !== null) {
     const [, path, status, language, content, explanation] = match;
     const fileInfo: AIFileInfo = {
@@ -19,14 +20,8 @@ export function parseDiffFiles(response: string): AIFileInfo[] {
 
     if (status.trim() === 'modified') {
       try {
-        // First, try parsing the content as-is
-        let parsedDiff = parsePatch(content);
-
-        // If parsing fails or results in empty hunks, try with normalized content
-        if (parsedDiff.length === 0 || parsedDiff[0].hunks.length === 0) {
-          const normalizedContent = content.trim().replace(/\r\n/g, '\n');
-          parsedDiff = parsePatch(normalizedContent);
-        }
+        const normalizedContent = content.trim().replace(/\r\n/g, '\n');
+        const parsedDiff = parsePatch(normalizedContent);
 
         if (parsedDiff.length > 0 && parsedDiff[0].hunks.length > 0) {
           const diff = parsedDiff[0];
@@ -57,7 +52,8 @@ export function parseDiffFiles(response: string): AIFileInfo[] {
   const deletedFileRegex =
     /<file>[\s\S]*?<file_path>(.*?)<\/file_path>[\s\S]*?<file_status>deleted<\/file_status>[\s\S]*?<\/file>/g;
   let deletedMatch: RegExpExecArray | null;
-  // biome-ignore lint/suspicious/noAssignInExpressions: Avoid potential infinite loop
+
+  // biome-ignore lint/suspicious/noAssignInExpressions: <explanation>
   while ((deletedMatch = deletedFileRegex.exec(response)) !== null) {
     const [, path] = deletedMatch;
     files.push({

src/git/apply-changes.ts

@@ -1,6 +1,6 @@
 import path from 'node:path';
 import chalk from 'chalk';
-import { applyPatch, createPatch } from 'diff';
+import { applyPatch } from 'diff';
 import fs from 'fs-extra';
 import type { AIFileInfo, ApplyChangesOptions } from '../types';
 
@@ -30,7 +30,6 @@ async function applyFileChange(
   dryRun: boolean,
 ): Promise<void> {
   const fullPath = path.join(basePath, file.path);
-
   try {
     switch (file.status) {
       case 'new':
@@ -70,31 +69,8 @@ async function applyFileChange(
         } else {
           if (file.diff) {
             const currentContent = await fs.readFile(fullPath, 'utf-8');
-
-            // Generate the new content based on the diff
-            const newContent = file.diff.hunks.reduce((acc, hunk) => {
-              const lines = acc.split('\n');
-              const newLines = hunk.lines
-                .filter((line) => !line.startsWith('-'))
-                .map((line) => (line.startsWith('+') ? line.slice(1) : line));
-              lines.splice(hunk.newStart - 1, hunk.oldLines, ...newLines);
-              return lines.join('\n');
-            }, currentContent);
-
-            // Create the patch
-            const patchString = createPatch(
-              file.path,
-              currentContent,
-              newContent,
-              file.diff.oldFileName || file.path,
-              file.diff.newFileName || file.path,
-              { context: 3 },
-            );
-
-            // Apply the patch
-            const updatedContent = applyPatch(currentContent, patchString);
-
-            if (typeof updatedContent === 'boolean') {
+            const updatedContent = applyPatch(currentContent, file.diff);
+            if (updatedContent === false) {
               throw new Error(
                 `Failed to apply patch to file: ${file.path}\nA common cause is that the file was not sent to the LLM and it hallucinated the content. Try running the task again (task --redo) and selecting the problematic file.`,
               );

src/templates/codegen-diff-no-plan-prompt.hbs

@@ -75,7 +75,6 @@ FORMAT: Instructions for how to format your response.
 
   <potential_issues>
     __LIST_OF_POTENTIAL_ISSUES_OR_TRADE_OFFS__
-    Include any constraints (e.g., performance, scalability, maintainability) you've considered and explain why the trade-offs you've made are appropriate for this task.
   </potential_issues>
 
   Then, for each file:
@@ -86,7 +85,7 @@ FORMAT: Instructions for how to format your response.
       __FILE_CONTENT_OR_DIFF__
     </file_content>
     <explanation>
-      __EXPLANATION__
+      __EXPLANATION__ (if necessary)
     </explanation>
   </file>
 
@@ -103,15 +102,22 @@ FORMAT: Instructions for how to format your response.
   etc
 
   FILE_CONTENT_OR_DIFF:
-  - For new files: Provide the complete file content, including all necessary imports, function definitions, and exports.
-  - For modified files: Provide a unified diff format. Use '---' for removed lines and '+++' for added lines.
+  - For new files: Provide the complete file content, including all necessary imports, function definitions, and
+  exports.
+  - For modified files: Provide a unified diff format. This MUST include:
+    1. File header lines (starting with "---" and "+++")
+    2. Hunk headers (starting with "@@")
+    3. Context lines (starting with a space)
+    4. Removed lines (starting with "-")
+    5. Added lines (starting with "+")
   - For deleted files: Leave this section empty.
 
   Ensure proper indentation and follow the project's coding standards.
 
-  STATUS: Use 'new' for newly created files, 'modified' for existing files that are being updated, and 'deleted' for files that are being deleted.
+  STATUS: Use 'new' for newly created files, 'modified' for existing files that are being updated, and 'deleted' for
+  files that are being deleted.
 
-  EXPLANATION: Provide a brief explanation for your implementation choices, including any significant design decisions, alternatives considered, and reasons for your final decision. Address any non-obvious implementations or optimizations.
+  EXPLANATION: Provide a brief explanation for any significant design decisions or non-obvious implementations.
 
   When creating diffs for modified files:
   - Always include file header lines with the correct file paths.
@@ -142,7 +148,8 @@ FORMAT: Instructions for how to format your response.
      description?: string;
   </file_content>
 
-  Before modifying a file, carefully review its entire content. Ensure that your changes, especially new imports, are placed in the correct location and don't duplicate existing code.
+  Before modifying a file, carefully review its entire content. Ensure that your changes, especially new imports, are
+  placed in the correct location and don't duplicate existing code.
 
   When generating diffs:
   1. Start with the original file content.
@@ -157,7 +164,7 @@ FORMAT: Instructions for how to format your response.
   added/removed lines.
   - Check that there are sufficient unchanged context lines around modifications.
 
-    Example for a new file:
+  Example for a new file:
   <file>
     <file_path>src/components/IssueList.tsx</file_path>
     <file_status>new</file_status>
@@ -216,22 +223,25 @@ FORMAT: Instructions for how to format your response.
 
 
   Ensure that:
-  - You have thoroughly analyzed the task and planned your implementation strategy.
+  - You have thoroughly analyzed the task description and have planned your implementation strategy.
   - Everything specified in the task description and instructions is implemented.
   - All new files contain the full code.
-  - All modified files have accurate and clear diffs.
+  - All modified files have accurate and clear diffs in the correct unified diff format.
+  - Diff formatting across all modified files is consistent and includes all required elements (file headers, hunk headers, context lines).
   - The content includes all necessary imports, function definitions, and exports.
-  - The code is clean, maintainable, efficient, and considers performance implications.
+  - The code is clean, maintainable, and efficient.
   - The code is properly formatted and follows the project's coding standards.
   - Necessary comments for clarity are included if needed.
   - Any conceptual or high-level descriptions are translated into actual, executable code.
   - You've considered and handled potential edge cases.
   - Your changes are consistent with the existing codebase.
   - You haven't introduced any potential bugs or performance issues.
   - Your code is easy to understand and maintain.
-  - You complete all necessary work to fully implement the task.
+  - You complete all necessary work.
 
-  Note: The accuracy of the diff format is crucial for successful patch application. Even small errors in formatting can cause the entire patch to fail. Pay extra attention to the correctness of your diff output.
+  Note: The accuracy of the diff format is crucial for successful patch application. Even small errors in formatting can
+  cause the entire patch to fail. Pay extra attention to the correctness of your diff output, ensuring all required
+  elements (file headers, hunk headers, context lines) are present and correctly formatted.
 
 </format>
 

tests/unit/apply-changes.test.ts

@@ -1,5 +1,5 @@
 import path from 'node:path';
-import { applyPatch, createPatch } from 'diff';
+import { applyPatch } from 'diff';
 import fs from 'fs-extra';
 import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
 import { applyChanges } from '../../src/git/apply-changes';
@@ -13,7 +13,6 @@ vi.mock('diff', async () => {
   return {
     ...actual,
     applyPatch: vi.fn(),
-    createPatch: vi.fn(),
   };
 });
 
@@ -212,6 +211,7 @@ describe('applyChanges', () => {
   });
 
   it('should apply diffs for modified files when provided', async () => {
+    const mockBasePath = '/mock/base/path';
     const mockParsedResponse = {
       fileList: ['modified-file.js'],
       files: [
@@ -246,35 +246,29 @@ describe('applyChanges', () => {
     const oldContent = 'console.log("Old content");';
     const newContent = 'console.log("New content");';
 
-    // biome-ignore lint/suspicious/noExplicitAny: explicit any is fine here
+    // biome-ignore lint/suspicious/noExplicitAny: explicit any is fine here, we're mocking fs.readFile
     vi.mocked(fs.readFile).mockResolvedValue(oldContent as any);
-    vi.mocked(createPatch).mockReturnValue('mocked patch string');
     vi.mocked(applyPatch).mockReturnValue(newContent);
 
     await applyChanges({
       basePath: mockBasePath,
-      parsedResponse: {
-        ...mockParsedResponse,
-        files: [
-          {
-            ...mockParsedResponse.files[0],
-            status: 'modified' as const,
-          },
-        ],
-      },
+      parsedResponse: mockParsedResponse as AIParsedResponse,
       dryRun: false,
     });
 
     expect(fs.readFile).toHaveBeenCalledTimes(1);
-    expect(createPatch).toHaveBeenCalledWith(
-      'modified-file.js',
+    expect(fs.readFile).toHaveBeenCalledWith(
+      expect.stringContaining('modified-file.js'),
+      'utf-8',
+    );
+
+    expect(applyPatch).toHaveBeenCalledTimes(1);
+    expect(applyPatch).toHaveBeenCalledWith(
       oldContent,
-      newContent,
-      'modified-file.js',
-      'modified-file.js',
-      { context: 3 },
+      mockParsedResponse.files[0].diff,
     );
-    expect(applyPatch).toHaveBeenCalledWith(oldContent, 'mocked patch string');
+
+    expect(fs.writeFile).toHaveBeenCalledTimes(1);
     expect(fs.writeFile).toHaveBeenCalledWith(
       expect.stringContaining('modified-file.js'),
       newContent,