refactor(core): Use BaseLlmClient for utility LLM calls in edit corrector (#8443)

This commit is contained in:
Abhi
2025-09-15 20:46:41 -04:00
committed by GitHub
parent d5d150449d
commit 1634d5fcca
6 changed files with 120 additions and 112 deletions
+8 -1
View File
@@ -58,6 +58,7 @@ describe('EditTool', () => {
let rootDir: string;
let mockConfig: Config;
let geminiClient: any;
let baseLlmClient: any;
beforeEach(() => {
vi.restoreAllMocks();
@@ -69,8 +70,13 @@ describe('EditTool', () => {
generateJson: mockGenerateJson, // mockGenerateJson is already defined and hoisted
};
baseLlmClient = {
generateJson: vi.fn(),
};
mockConfig = {
getGeminiClient: vi.fn().mockReturnValue(geminiClient),
getBaseLlmClient: vi.fn().mockReturnValue(baseLlmClient),
getTargetDir: () => rootDir,
getApprovalMode: vi.fn(),
setApprovalMode: vi.fn(),
@@ -424,11 +430,12 @@ describe('EditTool', () => {
// Set a specific mock for this test case
let mockCalled = false;
mockEnsureCorrectEdit.mockImplementationOnce(
async (_, content, p, client) => {
async (_, content, p, client, baseClient) => {
mockCalled = true;
expect(content).toBe(originalContent);
expect(p).toBe(params);
expect(client).toBe(geminiClient);
expect(baseClient).toBe(baseLlmClient);
return {
params: {
file_path: filePath,
+1
View File
@@ -164,6 +164,7 @@ class EditToolInvocation implements ToolInvocation<EditToolParams, ToolResult> {
currentContent,
params,
this.config.getGeminiClient(),
this.config.getBaseLlmClient(),
abortSignal,
);
finalOldString = correctedEdit.params.old_string;
+21 -7
View File
@@ -26,6 +26,7 @@ import path from 'node:path';
import fs from 'node:fs';
import os from 'node:os';
import { GeminiClient } from '../core/client.js';
import type { BaseLlmClient } from '../core/baseLlmClient.js';
import type { CorrectedEditResult } from '../utils/editCorrector.js';
import {
ensureCorrectEdit,
@@ -47,6 +48,7 @@ vi.mock('../ide/ide-client.js', () => ({
},
}));
let mockGeminiClientInstance: Mocked<GeminiClient>;
let mockBaseLlmClientInstance: Mocked<BaseLlmClient>;
const mockEnsureCorrectEdit = vi.fn<typeof ensureCorrectEdit>();
const mockEnsureCorrectFileContent = vi.fn<typeof ensureCorrectFileContent>();
const mockIdeClient = {
@@ -70,6 +72,7 @@ const mockConfigInternal = {
getApprovalMode: vi.fn(() => ApprovalMode.DEFAULT),
setApprovalMode: vi.fn(),
getGeminiClient: vi.fn(), // Initialize as a plain mock function
getBaseLlmClient: vi.fn(), // Initialize as a plain mock function
getFileSystemService: () => fsService,
getIdeMode: vi.fn(() => false),
getWorkspaceContext: () => createMockWorkspaceContext(rootDir),
@@ -123,15 +126,23 @@ describe('WriteFileTool', () => {
) as Mocked<GeminiClient>;
vi.mocked(GeminiClient).mockImplementation(() => mockGeminiClientInstance);
// Setup BaseLlmClient mock
mockBaseLlmClientInstance = {
generateJson: vi.fn(),
} as unknown as Mocked<BaseLlmClient>;
vi.mocked(ensureCorrectEdit).mockImplementation(mockEnsureCorrectEdit);
vi.mocked(ensureCorrectFileContent).mockImplementation(
mockEnsureCorrectFileContent,
);
// Now that mockGeminiClientInstance is initialized, set the mock implementation for getGeminiClient
// Now that mock instances are initialized, set the mock implementations for config getters
mockConfigInternal.getGeminiClient.mockReturnValue(
mockGeminiClientInstance,
);
mockConfigInternal.getBaseLlmClient.mockReturnValue(
mockBaseLlmClientInstance,
);
tool = new WriteFileTool(mockConfig);
@@ -148,7 +159,8 @@ describe('WriteFileTool', () => {
_currentContent: string,
params: EditToolParams,
_client: GeminiClient,
signal?: AbortSignal, // Make AbortSignal optional to match usage
_baseClient: BaseLlmClient,
signal?: AbortSignal,
): Promise<CorrectedEditResult> => {
if (signal?.aborted) {
return Promise.reject(new Error('Aborted'));
@@ -162,10 +174,9 @@ describe('WriteFileTool', () => {
mockEnsureCorrectFileContent.mockImplementation(
async (
content: string,
_client: GeminiClient,
_baseClient: BaseLlmClient,
signal?: AbortSignal,
): Promise<string> => {
// Make AbortSignal optional
if (signal?.aborted) {
return Promise.reject(new Error('Aborted'));
}
@@ -263,7 +274,7 @@ describe('WriteFileTool', () => {
expect(mockEnsureCorrectFileContent).toHaveBeenCalledWith(
proposedContent,
mockGeminiClientInstance,
mockBaseLlmClientInstance,
abortSignal,
);
expect(mockEnsureCorrectEdit).not.toHaveBeenCalled();
@@ -307,6 +318,7 @@ describe('WriteFileTool', () => {
file_path: filePath,
},
mockGeminiClientInstance,
mockBaseLlmClientInstance,
abortSignal,
);
expect(mockEnsureCorrectFileContent).not.toHaveBeenCalled();
@@ -383,7 +395,7 @@ describe('WriteFileTool', () => {
expect(mockEnsureCorrectFileContent).toHaveBeenCalledWith(
proposedContent,
mockGeminiClientInstance,
mockBaseLlmClientInstance,
abortSignal,
);
expect(confirmation).toEqual(
@@ -433,6 +445,7 @@ describe('WriteFileTool', () => {
file_path: filePath,
},
mockGeminiClientInstance,
mockBaseLlmClientInstance,
abortSignal,
);
expect(confirmation).toEqual(
@@ -599,7 +612,7 @@ describe('WriteFileTool', () => {
expect(mockEnsureCorrectFileContent).toHaveBeenCalledWith(
proposedContent,
mockGeminiClientInstance,
mockBaseLlmClientInstance,
abortSignal,
);
expect(result.llmContent).toMatch(
@@ -663,6 +676,7 @@ describe('WriteFileTool', () => {
file_path: filePath,
},
mockGeminiClientInstance,
mockBaseLlmClientInstance,
abortSignal,
);
expect(result.llmContent).toMatch(/Successfully overwrote file/);
+2 -1
View File
@@ -121,6 +121,7 @@ export async function getCorrectedFileContent(
file_path: filePath,
},
config.getGeminiClient(),
config.getBaseLlmClient(),
abortSignal,
);
correctedContent = correctedParams.new_string;
@@ -128,7 +129,7 @@ export async function getCorrectedFileContent(
// This implies new file (ENOENT)
correctedContent = await ensureCorrectFileContent(
proposedContent,
config.getGeminiClient(),
config.getBaseLlmClient(),
abortSignal,
);
}