Skip to content

Commit eb2cd82

Browse files
djexrobertpiosik
andauthored
Fixed issue where a final search block may have a trailing new line causing patch to fail (#278)
* added debug function and fixed issue with trailing empty new line in last search block. * Refactor the diff patch processor to use structured logging and throw errors --------- Co-authored-by: Robert Piosik <robertpiosik@gmail.com>
1 parent ca43ff5 commit eb2cd82

2 files changed

Lines changed: 105 additions & 37 deletions

File tree

packages/vscode/src/commands/apply-chat-response-command/utils/diff-patch-processor.ts

Lines changed: 95 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import * as vscode from 'vscode'
22
import * as path from 'path'
33
import * as fs from 'fs'
4+
import { Logger } from '@/helpers/logger'
45

56
class SearchBlock {
67
public search_lines: string[]
@@ -25,10 +26,10 @@ class SearchBlock {
2526
}
2627
}
2728

28-
export async function process_diff_patch(
29+
export const process_diff_patch = async (
2930
file_path: string,
3031
diff_path_patch: string
31-
): Promise<boolean> {
32+
): Promise<void> => {
3233
if (!fs.existsSync(file_path)) {
3334
// Check if file exists, if not then create it and any directories
3435
try {
@@ -42,11 +43,16 @@ export async function process_diff_patch(
4243
// Delay to prevent file system issues when creating a lot of files quickly
4344
await new Promise((f) => setTimeout(f, 500))
4445

45-
console.log(`File created successfully at: ${file_path}`)
46+
Logger.log({
47+
function_name: 'process_diff_patch',
48+
message: `File created successfully at: ${file_path}`
49+
})
4650
} catch (error: any) {
47-
console.error(`Error creating file: ${error.message}`)
48-
49-
return false
51+
Logger.error({
52+
function_name: 'process_diff_patch',
53+
message: `Error creating file: ${error.message}`
54+
})
55+
throw new Error(`Failed to create file: ${error.message}`)
5056
}
5157
}
5258

@@ -64,25 +70,27 @@ export async function process_diff_patch(
6470
result = apply_diff_patch(file_content, diff_patch_content)
6571
}
6672

67-
if (result == 'error') {
68-
return false
69-
}
70-
7173
// Save result to file
7274
try {
7375
await vscode.workspace.fs.writeFile(
7476
vscode.Uri.file(file_path),
7577
Buffer.from(result, 'utf8')
7678
)
77-
console.log('File saved successfully')
78-
return true // Return true after successful write
79+
Logger.log({
80+
function_name: 'process_diff_patch',
81+
message: 'File saved successfully'
82+
})
7983
} catch (error) {
80-
console.error('Error saving file:', error)
81-
return false // Return false if write fails
84+
Logger.error({
85+
function_name: 'process_diff_patch',
86+
message: 'Error saving file',
87+
data: error
88+
})
89+
throw new Error('Failed to save file after applying diff patch')
8290
}
8391
}
8492

85-
function create_new_file_from_patch(diff_patch: string): string {
93+
const create_new_file_from_patch = (diff_patch: string): string => {
8694
let new_file_content = ''
8795

8896
const patch_normalized = diff_patch.replace(/\r\n/g, '\n')
@@ -109,7 +117,10 @@ function create_new_file_from_patch(diff_patch: string): string {
109117
return new_file_content
110118
}
111119

112-
function apply_diff_patch(original_code: string, diff_patch: string): string {
120+
const apply_diff_patch = (
121+
original_code: string,
122+
diff_patch: string
123+
): string => {
113124
try {
114125
const original_code_normalized = original_code.replace(/\r\n/g, '\n') // Remove windows line endings
115126
const original_code_lines = original_code_normalized.split(/^/m) // Split by new line
@@ -293,6 +304,14 @@ function apply_diff_patch(original_code: string, diff_patch: string): string {
293304

294305
// Reached end of patch. Add the final search block to the searchReplaceBlocks
295306
if (search_chunks.length > 0) {
307+
// When there are only + lines in the last search block and corresponding no replace lines remove a trailing ~nnn if exists
308+
// A extra trailing new line is sometimes added by the AI model
309+
if (search_chunks[search_chunks.length - 1] == '~nnn') {
310+
// If the last search chunk ends in a ~nnn (blank new line), remove it and it's corresponding replace chunk new line
311+
search_chunks.pop()
312+
replace_chunks.pop()
313+
}
314+
296315
// Add the final search block to the searchReplaceBlocks
297316
// This is crucial for diffs that only have deletions
298317
search_replace_blocks.push(
@@ -325,7 +344,7 @@ function apply_diff_patch(original_code: string, diff_patch: string): string {
325344
)
326345
const chunk_string = chunk.map((line) => line.value).join('')
327346

328-
//console.log('Chunk string: ' + chunk_string);
347+
// console.log('Chunk string: ' + chunk_string);
329348

330349
// Check if the chunk matches the search string
331350
if (chunk_string == search_string) {
@@ -348,11 +367,19 @@ function apply_diff_patch(original_code: string, diff_patch: string): string {
348367

349368
// If not found, set the search_block_start_index to -1
350369
if (!found) {
351-
console.log('Search block not found: ' + search_string)
352-
search_replace_block.search_block_start_index = -1 // Not found
353-
354-
return 'error'
355-
//throw new Error('Search block not found: ' + search_string);
370+
const error_message = `Search block not found: ${search_string}`
371+
Logger.error({
372+
function_name: 'apply_diff_patch',
373+
message: error_message
374+
})
375+
Logger.error({
376+
function_name: 'apply_diff_patch',
377+
message: `search_replace_block: ${JSON.stringify(
378+
search_replace_block
379+
)}`
380+
})
381+
382+
throw new Error(`Search block not found: ${search_string}`)
356383
}
357384
}
358385

@@ -375,11 +402,12 @@ function apply_diff_patch(original_code: string, diff_patch: string): string {
375402

376403
if (start_index < 0 || start_index > result_lines.length) {
377404
// start_index can be == result_lines.length for appending
378-
console.error(
379-
`Invalid start index ${start_index} for block application. Max index: ${
405+
Logger.error({
406+
function_name: 'apply_diff_patch',
407+
message: `Invalid start index ${start_index} for block application. Max index: ${
380408
result_lines.length - 1
381409
}`
382-
)
410+
})
383411
continue
384412
}
385413

@@ -389,7 +417,7 @@ function apply_diff_patch(original_code: string, diff_patch: string): string {
389417
result_lines.length - start_index
390418
)
391419

392-
//console.log(`Applying block at index ${start_index}: removing ${actual_search_count} lines, inserting ${replacement_content.length} lines.`);
420+
// console.log(`Applying block at index ${start_index}: removing ${actual_search_count} lines, inserting ${replacement_content.length} lines.`);
393421
// Apply the replacement
394422
result_lines.splice(
395423
start_index,
@@ -398,24 +426,58 @@ function apply_diff_patch(original_code: string, diff_patch: string): string {
398426
)
399427
}
400428

429+
// print_debug(result_lines, original_code_lines, original_code_lines_normalized, patch_lines_normalized, search_replace_blocks);
430+
401431
return result_lines.join('')
402432
} catch (error) {
403-
console.error('Error during diff processing:', error)
404-
405-
return 'error'
406-
//throw new Error('Error during diff processing: ' + error);
433+
Logger.error({
434+
function_name: 'apply_diff_patch',
435+
message: 'Error during diff processing',
436+
data: error
437+
})
438+
throw error
407439
}
408440
}
409441

410-
function count_trailing_new_lines(text: string): number {
442+
// function print_debug(
443+
// original_code_lines: string[],
444+
// original_code_lines_normalized: any[],
445+
// patch_lines_normalized: string[],
446+
// search_replace_blocks: SearchBlock[]
447+
// ) {
448+
// // Output the original code lines
449+
// console.log('\n=== Original Code Lines ===')
450+
// console.log(JSON.stringify(original_code_lines, null, 2))
451+
452+
// // Output the search_replace_blocks in JSON format
453+
// console.log('=== Search and Replace Blocks ===')
454+
// console.log(JSON.stringify(search_replace_blocks, null, 2))
455+
456+
// let output_code_lines = ''
457+
// for (let i = 0; i < original_code_lines_normalized.length; i++) {
458+
// output_code_lines +=
459+
// original_code_lines_normalized[i].key +
460+
// ' ' +
461+
// original_code_lines_normalized[i].value +
462+
// '\n'
463+
// }
464+
465+
// // Output the normalized lines
466+
// console.log('=== Normalized Original Code Lines ===')
467+
// console.log(output_code_lines)
468+
469+
// console.log('=== Normalized Patch Lines ===')
470+
// console.log(patch_lines_normalized.join('\n'))
471+
// }
472+
473+
const count_trailing_new_lines = (text: string): number => {
411474
let count = 0
412475
for (let i = text.length - 1; i >= 0; i--) {
413-
if (text[i] === '\n') {
476+
if (text[i] == '\n') {
414477
count++
415478
} else {
416479
break
417480
}
418481
}
419-
420482
return count
421483
}

packages/vscode/src/commands/apply-chat-response-command/utils/patch-handler.ts

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -228,11 +228,17 @@ export async function apply_git_patch(
228228
throw new Error('File path is null')
229229
}
230230

231-
if ((await process_diff_patch(file_path_safe, temp_file)) == false) {
232-
throw new Error('Failed to apply diff patch for all methods')
231+
try {
232+
await process_diff_patch(file_path_safe, temp_file)
233+
used_fallback = true
234+
} catch (error: any) {
235+
Logger.error({
236+
function_name: 'apply_git_patch',
237+
message: 'Custom diff processor failed',
238+
data: error
239+
})
240+
throw new Error(`Failed to apply diff patch: ${error.message}`)
233241
}
234-
235-
used_fallback = true
236242
}
237243

238244
Logger.log({

0 commit comments

Comments
 (0)