diff options
Diffstat (limited to 'packages/core/src/utils')
| -rw-r--r-- | packages/core/src/utils/editCorrector.test.ts | 63 | ||||
| -rw-r--r-- | packages/core/src/utils/editCorrector.test.ts.backup | 503 | ||||
| -rw-r--r-- | packages/core/src/utils/editCorrector.ts | 38 | ||||
| -rw-r--r-- | packages/core/src/utils/positionBasedEditProcessor.ts | 239 |
4 files changed, 8 insertions, 835 deletions
diff --git a/packages/core/src/utils/editCorrector.test.ts b/packages/core/src/utils/editCorrector.test.ts index 53872253..7d6f5a53 100644 --- a/packages/core/src/utils/editCorrector.test.ts +++ b/packages/core/src/utils/editCorrector.test.ts @@ -214,9 +214,6 @@ describe('editCorrector', () => { const currentContent = 'This is a test string to find me.'; const originalParams = { file_path: '/test/file.txt', - edits: [ - { old_string: 'find me', new_string: 'replace with \\"this\\"' }, - ], old_string: 'find me', new_string: 'replace with \\"this\\"', }; @@ -238,7 +235,6 @@ describe('editCorrector', () => { const currentContent = 'This is a test string to find me.'; const originalParams = { file_path: '/test/file.txt', - edits: [{ old_string: 'find me', new_string: 'replace with this' }], old_string: 'find me', new_string: 'replace with this', }; @@ -257,9 +253,6 @@ describe('editCorrector', () => { const currentContent = 'This is a test string to find\\me.'; const originalParams = { file_path: '/test/file.txt', - edits: [ - { old_string: 'find\\me', new_string: 'replace with \\"this\\"' }, - ], old_string: 'find\\me', new_string: 'replace with \\"this\\"', }; @@ -281,7 +274,6 @@ describe('editCorrector', () => { const currentContent = 'This is a test string to find\\me.'; const originalParams = { file_path: '/test/file.txt', - edits: [{ old_string: 'find\\me', new_string: 'replace with this' }], old_string: 'find\\me', new_string: 'replace with this', }; @@ -303,12 +295,6 @@ describe('editCorrector', () => { const currentContent = 'This is a test string to find "me".'; const originalParams = { file_path: '/test/file.txt', - edits: [ - { - old_string: 'find \\"me\\"', - new_string: 'replace with \\"this\\"', - }, - ], old_string: 'find \\"me\\"', new_string: 'replace with \\"this\\"', }; @@ -328,9 +314,6 @@ describe('editCorrector', () => { const currentContent = 'This is a test string to find "me".'; const originalParams = { file_path: '/test/file.txt', - edits: [ - { old_string: 'find \\"me\\"', new_string: 'replace with this' }, - ], old_string: 'find \\"me\\"', new_string: 'replace with this', }; @@ -349,9 +332,6 @@ describe('editCorrector', () => { const currentContent = 'This is a test string to find \\me.'; const originalParams = { file_path: '/test/file.txt', - edits: [ - { old_string: 'find \\\\me', new_string: 'replace with foobar' }, - ], old_string: 'find \\\\me', new_string: 'replace with foobar', }; @@ -376,12 +356,6 @@ describe('editCorrector', () => { const currentContent = 'This is a test string to corrected find me.'; const originalParams = { file_path: '/test/file.txt', - edits: [ - { - old_string: 'find me', - new_string: 'replace with \\\\"this\\\\"', - }, - ], old_string: 'find me', new_string: 'replace with \\\\"this\\\\"', }; @@ -402,12 +376,6 @@ describe('editCorrector', () => { const currentContent = 'This is a test string to corrected find me.'; const originalParams = { file_path: '/test/file.txt', - edits: [ - { - old_string: 'find\\me', - new_string: 'replace with \\\\"this\\\\"', - }, - ], old_string: 'find\\me', new_string: 'replace with \\\\"this\\\\"', }; @@ -430,9 +398,6 @@ describe('editCorrector', () => { const currentContent = 'This is a test string to be corrected.'; const originalParams = { file_path: '/test/file.txt', - edits: [ - { old_string: 'fiiind me', new_string: 'replace with "this"' }, - ], old_string: 'fiiind me', new_string: 'replace with "this"', }; @@ -453,12 +418,6 @@ describe('editCorrector', () => { const currentContent = 'This is a test string to corrected find me.'; const originalParams = { file_path: '/test/file.txt', - edits: [ - { - old_string: 'find me', - new_string: 'replace with \\\\"this\\\\"', - }, - ], old_string: 'find me', new_string: 'replace with \\\\"this\\\\"', }; @@ -483,9 +442,6 @@ describe('editCorrector', () => { const currentContent = 'This content has nothing to find.'; const originalParams = { file_path: '/test/file.txt', - edits: [ - { old_string: 'nonexistent string', new_string: 'some new string' }, - ], old_string: 'nonexistent string', new_string: 'some new string', }; @@ -497,11 +453,7 @@ describe('editCorrector', () => { abortSignal, ); expect(mockGenerateJson).toHaveBeenCalledTimes(1); - expect(result.params).toEqual({ - file_path: originalParams.file_path, - old_string: originalParams.old_string, - new_string: originalParams.new_string, - }); + expect(result.params).toEqual(originalParams); expect(result.occurrences).toBe(0); }); it('Test 4.2: unescapedOldStringAttempt results in >1 occurrences -> returns original params, count occurrences', async () => { @@ -509,7 +461,6 @@ describe('editCorrector', () => { 'This content has find "me" and also find "me" again.'; const originalParams = { file_path: '/test/file.txt', - edits: [{ old_string: 'find "me"', new_string: 'some new string' }], old_string: 'find "me"', new_string: 'some new string', }; @@ -520,11 +471,7 @@ describe('editCorrector', () => { abortSignal, ); expect(mockGenerateJson).toHaveBeenCalledTimes(0); - expect(result.params).toEqual({ - file_path: originalParams.file_path, - old_string: originalParams.old_string, - new_string: originalParams.new_string, - }); + expect(result.params).toEqual(originalParams); expect(result.occurrences).toBe(2); }); }); @@ -534,12 +481,6 @@ describe('editCorrector', () => { const currentContent = 'const x = "a\\nbc\\\\"def\\\\"'; const originalParams = { file_path: '/test/file.txt', - edits: [ - { - old_string: 'const x = \\\\"a\\\\nbc\\\\\\\\"def\\\\\\\\"', - new_string: 'const y = \\\\"new\\\\nval\\\\\\\\"content\\\\\\\\"', - }, - ], old_string: 'const x = \\\\"a\\\\nbc\\\\\\\\"def\\\\\\\\"', new_string: 'const y = \\\\"new\\\\nval\\\\\\\\"content\\\\\\\\"', }; diff --git a/packages/core/src/utils/editCorrector.test.ts.backup b/packages/core/src/utils/editCorrector.test.ts.backup deleted file mode 100644 index 7d6f5a53..00000000 --- a/packages/core/src/utils/editCorrector.test.ts.backup +++ /dev/null @@ -1,503 +0,0 @@ -/** - * @license - * Copyright 2025 Google LLC - * SPDX-License-Identifier: Apache-2.0 - */ - -/* eslint-disable @typescript-eslint/no-explicit-any */ -import { vi, describe, it, expect, beforeEach, type Mocked } from 'vitest'; - -// MOCKS -let callCount = 0; -const mockResponses: any[] = []; - -let mockGenerateJson: any; -let mockStartChat: any; -let mockSendMessageStream: any; - -vi.mock('../core/client.js', () => ({ - GeminiClient: vi.fn().mockImplementation(function ( - this: any, - _config: Config, - ) { - this.generateJson = (...params: any[]) => mockGenerateJson(...params); // Corrected: use mockGenerateJson - this.startChat = (...params: any[]) => mockStartChat(...params); // Corrected: use mockStartChat - this.sendMessageStream = (...params: any[]) => - mockSendMessageStream(...params); // Corrected: use mockSendMessageStream - return this; - }), -})); -// END MOCKS - -import { - countOccurrences, - ensureCorrectEdit, - unescapeStringForGeminiBug, - resetEditCorrectorCaches_TEST_ONLY, -} from './editCorrector.js'; -import { GeminiClient } from '../core/client.js'; -import type { Config } from '../config/config.js'; -import { ToolRegistry } from '../tools/tool-registry.js'; - -vi.mock('../tools/tool-registry.js'); - -describe('editCorrector', () => { - describe('countOccurrences', () => { - it('should return 0 for empty string', () => { - expect(countOccurrences('', 'a')).toBe(0); - }); - it('should return 0 for empty substring', () => { - expect(countOccurrences('abc', '')).toBe(0); - }); - it('should return 0 if substring is not found', () => { - expect(countOccurrences('abc', 'd')).toBe(0); - }); - it('should return 1 if substring is found once', () => { - expect(countOccurrences('abc', 'b')).toBe(1); - }); - it('should return correct count for multiple occurrences', () => { - expect(countOccurrences('ababa', 'a')).toBe(3); - expect(countOccurrences('ababab', 'ab')).toBe(3); - }); - it('should count non-overlapping occurrences', () => { - expect(countOccurrences('aaaaa', 'aa')).toBe(2); - expect(countOccurrences('ababab', 'aba')).toBe(1); - }); - it('should correctly count occurrences when substring is longer', () => { - expect(countOccurrences('abc', 'abcdef')).toBe(0); - }); - it('should be case sensitive', () => { - expect(countOccurrences('abcABC', 'a')).toBe(1); - expect(countOccurrences('abcABC', 'A')).toBe(1); - }); - }); - - describe('unescapeStringForGeminiBug', () => { - it('should unescape common sequences', () => { - expect(unescapeStringForGeminiBug('\\n')).toBe('\n'); - expect(unescapeStringForGeminiBug('\\t')).toBe('\t'); - expect(unescapeStringForGeminiBug("\\'")).toBe("'"); - expect(unescapeStringForGeminiBug('\\"')).toBe('"'); - expect(unescapeStringForGeminiBug('\\`')).toBe('`'); - }); - it('should handle multiple escaped sequences', () => { - expect(unescapeStringForGeminiBug('Hello\\nWorld\\tTest')).toBe( - 'Hello\nWorld\tTest', - ); - }); - it('should not alter already correct sequences', () => { - expect(unescapeStringForGeminiBug('\n')).toBe('\n'); - expect(unescapeStringForGeminiBug('Correct string')).toBe( - 'Correct string', - ); - }); - it('should handle mixed correct and incorrect sequences', () => { - expect(unescapeStringForGeminiBug('\\nCorrect\t\\`')).toBe( - '\nCorrect\t`', - ); - }); - it('should handle backslash followed by actual newline character', () => { - expect(unescapeStringForGeminiBug('\\\n')).toBe('\n'); - expect(unescapeStringForGeminiBug('First line\\\nSecond line')).toBe( - 'First line\nSecond line', - ); - }); - it('should handle multiple backslashes before an escapable character', () => { - expect(unescapeStringForGeminiBug('\\\\n')).toBe('\n'); - expect(unescapeStringForGeminiBug('\\\\\\t')).toBe('\t'); - expect(unescapeStringForGeminiBug('\\\\\\\\`')).toBe('`'); - }); - it('should return empty string for empty input', () => { - expect(unescapeStringForGeminiBug('')).toBe(''); - }); - it('should not alter strings with no targeted escape sequences', () => { - expect(unescapeStringForGeminiBug('abc def')).toBe('abc def'); - expect(unescapeStringForGeminiBug('C:\\Folder\\File')).toBe( - 'C:\\Folder\\File', - ); - }); - it('should correctly process strings with some targeted escapes', () => { - expect(unescapeStringForGeminiBug('C:\\Users\\name')).toBe( - 'C:\\Users\name', - ); - }); - it('should handle complex cases with mixed slashes and characters', () => { - expect( - unescapeStringForGeminiBug('\\\\\\\nLine1\\\nLine2\\tTab\\\\`Tick\\"'), - ).toBe('\nLine1\nLine2\tTab`Tick"'); - }); - }); - - describe('ensureCorrectEdit', () => { - let mockGeminiClientInstance: Mocked<GeminiClient>; - let mockToolRegistry: Mocked<ToolRegistry>; - let mockConfigInstance: Config; - const abortSignal = new AbortController().signal; - - beforeEach(() => { - mockToolRegistry = new ToolRegistry({} as Config) as Mocked<ToolRegistry>; - const configParams = { - apiKey: 'test-api-key', - model: 'test-model', - sandbox: false as boolean | string, - targetDir: '/test', - debugMode: false, - question: undefined as string | undefined, - fullContext: false, - coreTools: undefined as string[] | undefined, - toolDiscoveryCommand: undefined as string | undefined, - toolCallCommand: undefined as string | undefined, - mcpServerCommand: undefined as string | undefined, - mcpServers: undefined as Record<string, any> | undefined, - userAgent: 'test-agent', - userMemory: '', - geminiMdFileCount: 0, - alwaysSkipModificationConfirmation: false, - }; - mockConfigInstance = { - ...configParams, - getApiKey: vi.fn(() => configParams.apiKey), - getModel: vi.fn(() => configParams.model), - getSandbox: vi.fn(() => configParams.sandbox), - getTargetDir: vi.fn(() => configParams.targetDir), - getToolRegistry: vi.fn(() => mockToolRegistry), - getDebugMode: vi.fn(() => configParams.debugMode), - getQuestion: vi.fn(() => configParams.question), - getFullContext: vi.fn(() => configParams.fullContext), - getCoreTools: vi.fn(() => configParams.coreTools), - getToolDiscoveryCommand: vi.fn(() => configParams.toolDiscoveryCommand), - getToolCallCommand: vi.fn(() => configParams.toolCallCommand), - getMcpServerCommand: vi.fn(() => configParams.mcpServerCommand), - getMcpServers: vi.fn(() => configParams.mcpServers), - getUserAgent: vi.fn(() => configParams.userAgent), - getUserMemory: vi.fn(() => configParams.userMemory), - setUserMemory: vi.fn((mem: string) => { - configParams.userMemory = mem; - }), - getGeminiMdFileCount: vi.fn(() => configParams.geminiMdFileCount), - setGeminiMdFileCount: vi.fn((count: number) => { - configParams.geminiMdFileCount = count; - }), - getAlwaysSkipModificationConfirmation: vi.fn( - () => configParams.alwaysSkipModificationConfirmation, - ), - setAlwaysSkipModificationConfirmation: vi.fn((skip: boolean) => { - configParams.alwaysSkipModificationConfirmation = skip; - }), - } as unknown as Config; - - callCount = 0; - mockResponses.length = 0; - mockGenerateJson = vi - .fn() - .mockImplementation((_contents, _schema, signal) => { - // Check if the signal is aborted. If so, throw an error or return a specific response. - if (signal && signal.aborted) { - return Promise.reject(new Error('Aborted')); // Or some other specific error/response - } - const response = mockResponses[callCount]; - callCount++; - if (response === undefined) return Promise.resolve({}); - return Promise.resolve(response); - }); - mockStartChat = vi.fn(); - mockSendMessageStream = vi.fn(); - - mockGeminiClientInstance = new GeminiClient( - mockConfigInstance, - ) as Mocked<GeminiClient>; - resetEditCorrectorCaches_TEST_ONLY(); - }); - - describe('Scenario Group 1: originalParams.old_string matches currentContent directly', () => { - it('Test 1.1: old_string (no literal \\), new_string (escaped by Gemini) -> new_string unescaped', async () => { - const currentContent = 'This is a test string to find me.'; - const originalParams = { - file_path: '/test/file.txt', - old_string: 'find me', - new_string: 'replace with \\"this\\"', - }; - mockResponses.push({ - corrected_new_string_escaping: 'replace with "this"', - }); - const result = await ensureCorrectEdit( - currentContent, - originalParams, - mockGeminiClientInstance, - abortSignal, - ); - expect(mockGenerateJson).toHaveBeenCalledTimes(1); - expect(result.params.new_string).toBe('replace with "this"'); - expect(result.params.old_string).toBe('find me'); - expect(result.occurrences).toBe(1); - }); - it('Test 1.2: old_string (no literal \\), new_string (correctly formatted) -> new_string unchanged', async () => { - const currentContent = 'This is a test string to find me.'; - const originalParams = { - file_path: '/test/file.txt', - old_string: 'find me', - new_string: 'replace with this', - }; - const result = await ensureCorrectEdit( - currentContent, - originalParams, - mockGeminiClientInstance, - abortSignal, - ); - expect(mockGenerateJson).toHaveBeenCalledTimes(0); - expect(result.params.new_string).toBe('replace with this'); - expect(result.params.old_string).toBe('find me'); - expect(result.occurrences).toBe(1); - }); - it('Test 1.3: old_string (with literal \\), new_string (escaped by Gemini) -> new_string unchanged (still escaped)', async () => { - const currentContent = 'This is a test string to find\\me.'; - const originalParams = { - file_path: '/test/file.txt', - old_string: 'find\\me', - new_string: 'replace with \\"this\\"', - }; - mockResponses.push({ - corrected_new_string_escaping: 'replace with "this"', - }); - const result = await ensureCorrectEdit( - currentContent, - originalParams, - mockGeminiClientInstance, - abortSignal, - ); - expect(mockGenerateJson).toHaveBeenCalledTimes(1); - expect(result.params.new_string).toBe('replace with "this"'); - expect(result.params.old_string).toBe('find\\me'); - expect(result.occurrences).toBe(1); - }); - it('Test 1.4: old_string (with literal \\), new_string (correctly formatted) -> new_string unchanged', async () => { - const currentContent = 'This is a test string to find\\me.'; - const originalParams = { - file_path: '/test/file.txt', - old_string: 'find\\me', - new_string: 'replace with this', - }; - const result = await ensureCorrectEdit( - currentContent, - originalParams, - mockGeminiClientInstance, - abortSignal, - ); - expect(mockGenerateJson).toHaveBeenCalledTimes(0); - expect(result.params.new_string).toBe('replace with this'); - expect(result.params.old_string).toBe('find\\me'); - expect(result.occurrences).toBe(1); - }); - }); - - describe('Scenario Group 2: originalParams.old_string does NOT match, but unescapeStringForGeminiBug(originalParams.old_string) DOES match', () => { - it('Test 2.1: old_string (over-escaped, no intended literal \\), new_string (escaped by Gemini) -> new_string unescaped', async () => { - const currentContent = 'This is a test string to find "me".'; - const originalParams = { - file_path: '/test/file.txt', - old_string: 'find \\"me\\"', - new_string: 'replace with \\"this\\"', - }; - mockResponses.push({ corrected_new_string: 'replace with "this"' }); - const result = await ensureCorrectEdit( - currentContent, - originalParams, - mockGeminiClientInstance, - abortSignal, - ); - expect(mockGenerateJson).toHaveBeenCalledTimes(1); - expect(result.params.new_string).toBe('replace with "this"'); - expect(result.params.old_string).toBe('find "me"'); - expect(result.occurrences).toBe(1); - }); - it('Test 2.2: old_string (over-escaped, no intended literal \\), new_string (correctly formatted) -> new_string unescaped (harmlessly)', async () => { - const currentContent = 'This is a test string to find "me".'; - const originalParams = { - file_path: '/test/file.txt', - old_string: 'find \\"me\\"', - new_string: 'replace with this', - }; - const result = await ensureCorrectEdit( - currentContent, - originalParams, - mockGeminiClientInstance, - abortSignal, - ); - expect(mockGenerateJson).toHaveBeenCalledTimes(0); - expect(result.params.new_string).toBe('replace with this'); - expect(result.params.old_string).toBe('find "me"'); - expect(result.occurrences).toBe(1); - }); - it('Test 2.3: old_string (over-escaped, with intended literal \\), new_string (simple) -> new_string corrected', async () => { - const currentContent = 'This is a test string to find \\me.'; - const originalParams = { - file_path: '/test/file.txt', - old_string: 'find \\\\me', - new_string: 'replace with foobar', - }; - mockResponses.push({ - corrected_target_snippet: 'find \\me', - }); - const result = await ensureCorrectEdit( - currentContent, - originalParams, - mockGeminiClientInstance, - abortSignal, - ); - expect(mockGenerateJson).toHaveBeenCalledTimes(1); - expect(result.params.new_string).toBe('replace with foobar'); - expect(result.params.old_string).toBe('find \\me'); - expect(result.occurrences).toBe(1); - }); - }); - - describe('Scenario Group 3: LLM Correction Path', () => { - it('Test 3.1: old_string (no literal \\), new_string (escaped by Gemini), LLM re-escapes new_string -> final new_string is double unescaped', async () => { - const currentContent = 'This is a test string to corrected find me.'; - const originalParams = { - file_path: '/test/file.txt', - old_string: 'find me', - new_string: 'replace with \\\\"this\\\\"', - }; - const llmNewString = 'LLM says replace with "that"'; - mockResponses.push({ corrected_new_string_escaping: llmNewString }); - const result = await ensureCorrectEdit( - currentContent, - originalParams, - mockGeminiClientInstance, - abortSignal, - ); - expect(mockGenerateJson).toHaveBeenCalledTimes(1); - expect(result.params.new_string).toBe(llmNewString); - expect(result.params.old_string).toBe('find me'); - expect(result.occurrences).toBe(1); - }); - it('Test 3.2: old_string (with literal \\), new_string (escaped by Gemini), LLM re-escapes new_string -> final new_string is unescaped once', async () => { - const currentContent = 'This is a test string to corrected find me.'; - const originalParams = { - file_path: '/test/file.txt', - old_string: 'find\\me', - new_string: 'replace with \\\\"this\\\\"', - }; - const llmCorrectedOldString = 'corrected find me'; - const llmNewString = 'LLM says replace with "that"'; - mockResponses.push({ corrected_target_snippet: llmCorrectedOldString }); - mockResponses.push({ corrected_new_string: llmNewString }); - const result = await ensureCorrectEdit( - currentContent, - originalParams, - mockGeminiClientInstance, - abortSignal, - ); - expect(mockGenerateJson).toHaveBeenCalledTimes(2); - expect(result.params.new_string).toBe(llmNewString); - expect(result.params.old_string).toBe(llmCorrectedOldString); - expect(result.occurrences).toBe(1); - }); - it('Test 3.3: old_string needs LLM, new_string is fine -> old_string corrected, new_string original', async () => { - const currentContent = 'This is a test string to be corrected.'; - const originalParams = { - file_path: '/test/file.txt', - old_string: 'fiiind me', - new_string: 'replace with "this"', - }; - const llmCorrectedOldString = 'to be corrected'; - mockResponses.push({ corrected_target_snippet: llmCorrectedOldString }); - const result = await ensureCorrectEdit( - currentContent, - originalParams, - mockGeminiClientInstance, - abortSignal, - ); - expect(mockGenerateJson).toHaveBeenCalledTimes(1); - expect(result.params.new_string).toBe('replace with "this"'); - expect(result.params.old_string).toBe(llmCorrectedOldString); - expect(result.occurrences).toBe(1); - }); - it('Test 3.4: LLM correction path, correctNewString returns the originalNewString it was passed (which was unescaped) -> final new_string is unescaped', async () => { - const currentContent = 'This is a test string to corrected find me.'; - const originalParams = { - file_path: '/test/file.txt', - old_string: 'find me', - new_string: 'replace with \\\\"this\\\\"', - }; - const newStringForLLMAndReturnedByLLM = 'replace with "this"'; - mockResponses.push({ - corrected_new_string_escaping: newStringForLLMAndReturnedByLLM, - }); - const result = await ensureCorrectEdit( - currentContent, - originalParams, - mockGeminiClientInstance, - abortSignal, - ); - expect(mockGenerateJson).toHaveBeenCalledTimes(1); - expect(result.params.new_string).toBe(newStringForLLMAndReturnedByLLM); - expect(result.occurrences).toBe(1); - }); - }); - - describe('Scenario Group 4: No Match Found / Multiple Matches', () => { - it('Test 4.1: No version of old_string (original, unescaped, LLM-corrected) matches -> returns original params, 0 occurrences', async () => { - const currentContent = 'This content has nothing to find.'; - const originalParams = { - file_path: '/test/file.txt', - old_string: 'nonexistent string', - new_string: 'some new string', - }; - mockResponses.push({ corrected_target_snippet: 'still nonexistent' }); - const result = await ensureCorrectEdit( - currentContent, - originalParams, - mockGeminiClientInstance, - abortSignal, - ); - expect(mockGenerateJson).toHaveBeenCalledTimes(1); - expect(result.params).toEqual(originalParams); - expect(result.occurrences).toBe(0); - }); - it('Test 4.2: unescapedOldStringAttempt results in >1 occurrences -> returns original params, count occurrences', async () => { - const currentContent = - 'This content has find "me" and also find "me" again.'; - const originalParams = { - file_path: '/test/file.txt', - old_string: 'find "me"', - new_string: 'some new string', - }; - const result = await ensureCorrectEdit( - currentContent, - originalParams, - mockGeminiClientInstance, - abortSignal, - ); - expect(mockGenerateJson).toHaveBeenCalledTimes(0); - expect(result.params).toEqual(originalParams); - expect(result.occurrences).toBe(2); - }); - }); - - describe('Scenario Group 5: Specific unescapeStringForGeminiBug checks (integrated into ensureCorrectEdit)', () => { - it('Test 5.1: old_string needs LLM to become currentContent, new_string also needs correction', async () => { - const currentContent = 'const x = "a\\nbc\\\\"def\\\\"'; - const originalParams = { - file_path: '/test/file.txt', - old_string: 'const x = \\\\"a\\\\nbc\\\\\\\\"def\\\\\\\\"', - new_string: 'const y = \\\\"new\\\\nval\\\\\\\\"content\\\\\\\\"', - }; - const expectedFinalNewString = 'const y = "new\\nval\\\\"content\\\\"'; - mockResponses.push({ corrected_target_snippet: currentContent }); - mockResponses.push({ corrected_new_string: expectedFinalNewString }); - const result = await ensureCorrectEdit( - currentContent, - originalParams, - mockGeminiClientInstance, - abortSignal, - ); - expect(mockGenerateJson).toHaveBeenCalledTimes(2); - expect(result.params.old_string).toBe(currentContent); - expect(result.params.new_string).toBe(expectedFinalNewString); - expect(result.occurrences).toBe(1); - }); - }); - }); -}); diff --git a/packages/core/src/utils/editCorrector.ts b/packages/core/src/utils/editCorrector.ts index c1d7be77..67d23cde 100644 --- a/packages/core/src/utils/editCorrector.ts +++ b/packages/core/src/utils/editCorrector.ts @@ -61,16 +61,10 @@ export interface CorrectedEditResult { */ export async function ensureCorrectEdit( currentContent: string, - originalParams: EditToolParams & { old_string: string; new_string: string }, + originalParams: EditToolParams, // This is the EditToolParams from edit.ts, without \'corrected\' client: GeminiClient, abortSignal: AbortSignal, ): Promise<CorrectedEditResult> { - // Ensure we have required old_string and new_string - if (!originalParams.old_string || !originalParams.new_string) { - throw new Error( - 'old_string and new_string are required for edit correction', - ); - } const cacheKey = `${currentContent}---${originalParams.old_string}---${originalParams.new_string}`; const cachedResult = editCorrectionCache.get(cacheKey); if (cachedResult) { @@ -102,11 +96,7 @@ export async function ensureCorrectEdit( // If user expects multiple replacements, return as-is if (occurrences === expectedReplacements) { const result: CorrectedEditResult = { - params: { - file_path: originalParams.file_path, - old_string: originalParams.old_string!, - new_string: originalParams.new_string!, - }, + params: { ...originalParams }, occurrences, }; editCorrectionCache.set(cacheKey, result); @@ -116,11 +106,7 @@ export async function ensureCorrectEdit( // If user expects 1 but found multiple, try to correct (existing behavior) if (expectedReplacements === 1) { const result: CorrectedEditResult = { - params: { - file_path: originalParams.file_path, - old_string: originalParams.old_string!, - new_string: originalParams.new_string!, - }, + params: { ...originalParams }, occurrences, }; editCorrectionCache.set(cacheKey, result); @@ -129,11 +115,7 @@ export async function ensureCorrectEdit( // If occurrences don't match expected, return as-is (will fail validation later) const result: CorrectedEditResult = { - params: { - file_path: originalParams.file_path, - old_string: finalOldString, - new_string: finalNewString, - }, + params: { ...originalParams }, occurrences, }; editCorrectionCache.set(cacheKey, result); @@ -187,11 +169,7 @@ export async function ensureCorrectEdit( } else { // LLM correction also failed for old_string const result: CorrectedEditResult = { - params: { - file_path: originalParams.file_path, - old_string: finalOldString, - new_string: finalNewString, - }, + params: { ...originalParams }, occurrences: 0, // Explicitly 0 as LLM failed }; editCorrectionCache.set(cacheKey, result); @@ -200,11 +178,7 @@ export async function ensureCorrectEdit( } else { // Unescaping old_string resulted in > 1 occurrences const result: CorrectedEditResult = { - params: { - file_path: originalParams.file_path, - old_string: finalOldString, - new_string: finalNewString, - }, + params: { ...originalParams }, occurrences, // This will be > 1 }; editCorrectionCache.set(cacheKey, result); diff --git a/packages/core/src/utils/positionBasedEditProcessor.ts b/packages/core/src/utils/positionBasedEditProcessor.ts deleted file mode 100644 index 977d6859..00000000 --- a/packages/core/src/utils/positionBasedEditProcessor.ts +++ /dev/null @@ -1,239 +0,0 @@ -/** - * @license - * Copyright 2025 Google LLC - * SPDX-License-Identifier: Apache-2.0 - */ - -/** - * Single edit operation with position information - */ -export interface PositionedEdit { - /** - * Original edit operation - */ - original: EditOperation; - - /** - * Index in the original edits array - */ - index: number; - - /** - * Start position in the file content - */ - startPos: number; - - /** - * End position in the file content - */ - endPos: number; - - /** - * The replacement text - */ - newString: string; -} - -/** - * Result of failed edit with reason - */ -export interface FailedEdit { - index: number; - edit: EditOperation; - reason: 'not_found' | 'multiple_matches' | 'empty_old_string'; -} - -/** - * Result of position-based edit processing - */ -export interface PositionBasedEditResult { - finalContent: string; - appliedEdits: PositionedEdit[]; - failedEdits: FailedEdit[]; -} - -/** - * Edit operation interface (matches existing EditOperation) - */ -export interface EditOperation { - old_string: string; - new_string: string; -} - -/** - * Efficient position-based edit processor that minimizes memory usage - * and applies edits in optimal order to prevent position conflicts. - */ -export class PositionBasedEditProcessor { - /** - * Process edits with minimal memory usage and optimal ordering - */ - processEdits( - content: string, - edits: EditOperation[], - ): PositionBasedEditResult { - // Phase 1: Find positions for all edits - const { validEdits, failedEdits } = this.analyzeEdits(content, edits); - - // Phase 2: Sort by position (highest first) to prevent position drift - const sortedEdits = validEdits.sort((a, b) => b.startPos - a.startPos); - - // Phase 3: Build final content in single pass - const finalContent = this.buildFinalContent(content, sortedEdits); - - return { - finalContent, - appliedEdits: sortedEdits, - failedEdits, - }; - } - - /** - * Analyze all edits and categorize into valid/failed - */ - private analyzeEdits( - content: string, - edits: EditOperation[], - ): { - validEdits: PositionedEdit[]; - failedEdits: FailedEdit[]; - } { - const validEdits: PositionedEdit[] = []; - const failedEdits: FailedEdit[] = []; - - for (let i = 0; i < edits.length; i++) { - const edit = edits[i]; - - // Handle empty old_string (file creation case) - if (edit.old_string === '') { - failedEdits.push({ - index: i, - edit, - reason: 'empty_old_string', - }); - continue; - } - - // Find all positions of old_string - const positions = this.findAllPositions(content, edit.old_string); - - if (positions.length === 0) { - failedEdits.push({ - index: i, - edit, - reason: 'not_found', - }); - } else if (positions.length > 1) { - failedEdits.push({ - index: i, - edit, - reason: 'multiple_matches', - }); - } else { - // Exactly one match - valid edit - const startPos = positions[0]; - validEdits.push({ - original: edit, - index: i, - startPos, - endPos: startPos + edit.old_string.length, - newString: edit.new_string, - }); - } - } - - return { validEdits, failedEdits }; - } - - /** - * Find all positions where searchString occurs in content - */ - private findAllPositions(content: string, searchString: string): number[] { - const positions: number[] = []; - let index = content.indexOf(searchString); - - while (index !== -1) { - positions.push(index); - index = content.indexOf(searchString, index + 1); - } - - return positions; - } - - /** - * Build final content in single pass with minimal memory allocation - */ - private buildFinalContent(original: string, edits: PositionedEdit[]): string { - if (edits.length === 0) { - return original; - } - - // Check for overlapping edits (should not happen with our validation, but safety check) - if (this.hasOverlappingEdits(edits)) { - throw new Error('Overlapping edits detected - this should not happen'); - } - - // Build segments array working backwards through the string - const segments: string[] = []; - let currentPos = original.length; - - // Process edits from end to beginning (edits are already sorted by startPos desc) - for (const edit of edits) { - // Add text after this edit position - if (currentPos > edit.endPos) { - segments.unshift(original.slice(edit.endPos, currentPos)); - } - - // Add the replacement text - segments.unshift(edit.newString); - - // Update current position - currentPos = edit.startPos; - } - - // Add remaining text from beginning - if (currentPos > 0) { - segments.unshift(original.slice(0, currentPos)); - } - - // Single join operation creates final string - return segments.join(''); - } - - /** - * Check if any edits overlap (safety validation) - */ - private hasOverlappingEdits(edits: PositionedEdit[]): boolean { - for (let i = 0; i < edits.length - 1; i++) { - for (let j = i + 1; j < edits.length; j++) { - const edit1 = edits[i]; - const edit2 = edits[j]; - - // Check if ranges overlap - if ( - !(edit1.endPos <= edit2.startPos || edit2.endPos <= edit1.startPos) - ) { - return true; - } - } - } - - return false; - } - - /** - * Get human-readable error message for failed edit reason - */ - static getErrorMessage(reason: FailedEdit['reason']): string { - switch (reason) { - case 'not_found': - return 'Old string not found in current content'; - case 'multiple_matches': - return 'Old string found multiple times - please be more specific'; - case 'empty_old_string': - return 'Cannot use empty old_string on existing file'; - default: - return 'Unknown error'; - } - } -} |