From 581709df80769ae5174a1fab66bd3aa9d2967c15 Mon Sep 17 00:00:00 2001 From: Allen Hutchison Date: Thu, 22 May 2025 10:57:06 -0700 Subject: [PATCH] Refactor: Streamline memoryUtils and update slash commands (#478) --- packages/cli/src/config/memoryUtils.test.ts | 92 +++++++++++++ packages/cli/src/config/memoryUtils.ts | 130 +----------------- .../ui/hooks/slashCommandProcessor.test.ts | 12 +- .../cli/src/ui/hooks/slashCommandProcessor.ts | 2 +- 4 files changed, 101 insertions(+), 135 deletions(-) create mode 100644 packages/cli/src/config/memoryUtils.test.ts diff --git a/packages/cli/src/config/memoryUtils.test.ts b/packages/cli/src/config/memoryUtils.test.ts new file mode 100644 index 00000000..3ed51e74 --- /dev/null +++ b/packages/cli/src/config/memoryUtils.test.ts @@ -0,0 +1,92 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { + describe, + it, + expect, + vi, + beforeEach, + // afterEach, // Removed as it's not used + type Mocked, + type Mock, +} from 'vitest'; +import * as path from 'path'; +import { homedir } from 'os'; +import * as fs from 'fs/promises'; +import { getGlobalMemoryFilePath, addMemoryEntry } from './memoryUtils.js'; +import { SETTINGS_DIRECTORY_NAME } from './settings.js'; +import { + MemoryTool, + GEMINI_MD_FILENAME, + // MEMORY_SECTION_HEADER, // Removed as it's not used + // getErrorMessage, // Removed as it's not used +} from '@gemini-code/server'; + +// Mock the entire fs/promises module +vi.mock('fs/promises'); +// Mock MemoryTool static method +vi.mock('@gemini-code/server', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + MemoryTool: { + ...actual.MemoryTool, + performAddMemoryEntry: vi.fn(), + }, + }; +}); + +describe('memoryUtils', () => { + beforeEach(() => { + // Reset mocks before each test + vi.resetAllMocks(); + }); + + describe('getGlobalMemoryFilePath', () => { + it('should return the correct global memory file path', () => { + const expectedPath = path.join( + homedir(), + SETTINGS_DIRECTORY_NAME, + GEMINI_MD_FILENAME, + ); + expect(getGlobalMemoryFilePath()).toBe(expectedPath); + }); + }); + + describe('addMemoryEntry', () => { + const mockFs = fs as Mocked; // Type cast for mocked fs + const mockPerformAddMemoryEntry = MemoryTool.performAddMemoryEntry as Mock; + + it('should call MemoryTool.performAddMemoryEntry with correct parameters', async () => { + const testText = 'Remember this important fact.'; + const expectedFilePath = getGlobalMemoryFilePath(); + + await addMemoryEntry(testText); + + expect(mockPerformAddMemoryEntry).toHaveBeenCalledOnce(); + expect(mockPerformAddMemoryEntry).toHaveBeenCalledWith( + testText, + expectedFilePath, + { + readFile: mockFs.readFile, + writeFile: mockFs.writeFile, + mkdir: mockFs.mkdir, + }, + ); + }); + + it('should propagate errors from MemoryTool.performAddMemoryEntry', async () => { + const testText = 'This will fail.'; + const expectedError = new Error('Failed to add memory entry'); + mockPerformAddMemoryEntry.mockRejectedValueOnce(expectedError); + + await expect(addMemoryEntry(testText)).rejects.toThrow(expectedError); + }); + }); + + // More tests will be added here +}); diff --git a/packages/cli/src/config/memoryUtils.ts b/packages/cli/src/config/memoryUtils.ts index d44b5ebf..63a7734f 100644 --- a/packages/cli/src/config/memoryUtils.ts +++ b/packages/cli/src/config/memoryUtils.ts @@ -9,10 +9,10 @@ import * as path from 'path'; import { homedir } from 'os'; import { SETTINGS_DIRECTORY_NAME } from './settings.js'; import { - getErrorMessage, + // getErrorMessage, // Removed as it's not used MemoryTool, GEMINI_MD_FILENAME, - MEMORY_SECTION_HEADER, + // MEMORY_SECTION_HEADER, // Removed as it's not used } from '@gemini-code/server'; /** @@ -35,129 +35,3 @@ export async function addMemoryEntry(text: string): Promise { mkdir: fs.mkdir, }); } - -/** - * Deletes the last added memory entry from the "Gemini Added Memories" section. - */ -export async function deleteLastMemoryEntry(): Promise { - const filePath = getGlobalMemoryFilePath(); - try { - let content = await fs.readFile(filePath, 'utf-8'); - const headerIndex = content.indexOf(MEMORY_SECTION_HEADER); - - if (headerIndex === -1) return false; // Section not found - - const startOfSectionContent = headerIndex + MEMORY_SECTION_HEADER.length; - let endOfSectionIndex = content.indexOf('\n## ', startOfSectionContent); - if (endOfSectionIndex === -1) { - endOfSectionIndex = content.length; - } - - const sectionPart = content.substring( - startOfSectionContent, - endOfSectionIndex, - ); - const lines = sectionPart.split(/\r?\n/).map((line) => line.trimEnd()); - - let lastBulletLineIndex = -1; - for (let i = lines.length - 1; i >= 0; i--) { - if (lines[i].trim().startsWith('- ')) { - lastBulletLineIndex = i; - break; - } - } - - if (lastBulletLineIndex === -1) return false; // No bullets found in section - - lines.splice(lastBulletLineIndex, 1); - - const newSectionPart = lines - .filter((line) => line.trim().length > 0) - .join('\n'); - - const beforeHeader = content.substring(0, headerIndex); - const afterSection = content.substring(endOfSectionIndex); - - if (newSectionPart.trim().length === 0) { - // If section is now empty (no bullets), remove header too or leave it clean - // For simplicity, let's leave the header but ensure it has a newline after if content follows - content = `${beforeHeader}${MEMORY_SECTION_HEADER}\n${afterSection}` - .replace(/\n{3,}/g, '\n\n') - .trimEnd(); - if (content.length > 0) content += '\n'; - } else { - content = - `${beforeHeader}${MEMORY_SECTION_HEADER}\n${newSectionPart}\n${afterSection}` - .replace(/\n{3,}/g, '\n\n') - .trimEnd(); - if (content.length > 0) content += '\n'; - } - - await fs.writeFile(filePath, content, 'utf-8'); - return true; - } catch (error) { - if (error instanceof Error && 'code' in error && error.code === 'ENOENT') { - return false; - } - console.error(`Error deleting last memory entry from ${filePath}:`, error); - throw new Error( - `Failed to delete last memory entry: ${getErrorMessage(error)}`, - ); - } -} - -/** - * Deletes all added memory entries (the entire "Gemini Added Memories" section). - */ -export async function deleteAllAddedMemoryEntries(): Promise { - const filePath = getGlobalMemoryFilePath(); - try { - let content = await fs.readFile(filePath, 'utf-8'); - const headerIndex = content.indexOf(MEMORY_SECTION_HEADER); - - if (headerIndex === -1) return 0; // Section not found - - let endOfSectionIndex = content.indexOf( - '\n## ', - headerIndex + MEMORY_SECTION_HEADER.length, - ); - if (endOfSectionIndex === -1) { - endOfSectionIndex = content.length; // Section goes to EOF - } - - const sectionContent = content.substring(headerIndex, endOfSectionIndex); - const bulletCount = (sectionContent.match(/\n- /g) || []).length; - - if (bulletCount === 0 && !sectionContent.includes('- ')) { - // No bullets found - // If we only remove if bullets exist, or remove header if no bullets. - // For now, if header exists but no bullets, consider 0 deleted if we only count bullets. - // If the goal is to remove the section if it exists, this logic changes. - // Let's assume we only care about bulleted items for the count. - } - - // Remove the section including the header - const beforeHeader = content.substring(0, headerIndex); - const afterSection = content.substring(endOfSectionIndex); - - content = ( - beforeHeader.trimEnd() + - (afterSection.length > 0 ? '\n' + afterSection.trimStart() : '') - ).trim(); - if (content.length > 0) content += '\n'; - - await fs.writeFile(filePath, content, 'utf-8'); - return bulletCount; // This counts '\n- ' occurrences, might need refinement for exact bullet count - } catch (error) { - if (error instanceof Error && 'code' in error && error.code === 'ENOENT') { - return 0; - } - console.error( - `Error deleting all added memory entries from ${filePath}:`, - error, - ); - throw new Error( - `Failed to delete all added memory entries: ${getErrorMessage(error)}`, - ); - } -} diff --git a/packages/cli/src/ui/hooks/slashCommandProcessor.test.ts b/packages/cli/src/ui/hooks/slashCommandProcessor.test.ts index beee1418..1d9eec53 100644 --- a/packages/cli/src/ui/hooks/slashCommandProcessor.test.ts +++ b/packages/cli/src/ui/hooks/slashCommandProcessor.test.ts @@ -77,13 +77,13 @@ describe('useSlashCommandProcessor', () => { performAddMemoryEntrySpy.mockReset(); (open as Mock).mockClear(); - vi.spyOn(memoryUtils, 'deleteLastMemoryEntry').mockImplementation(vi.fn()); - vi.spyOn(memoryUtils, 'deleteAllAddedMemoryEntries').mockImplementation( - vi.fn(), - ); + // vi.spyOn(memoryUtils, 'deleteLastMemoryEntry').mockImplementation(vi.fn()); + // vi.spyOn(memoryUtils, 'deleteAllAddedMemoryEntries').mockImplementation( + // vi.fn(), + // ); - vi.mocked(memoryUtils.deleteLastMemoryEntry).mockClear(); - vi.mocked(memoryUtils.deleteAllAddedMemoryEntries).mockClear(); + // vi.mocked(memoryUtils.deleteLastMemoryEntry).mockClear(); + // vi.mocked(memoryUtils.deleteAllAddedMemoryEntries).mockClear(); mockProcessExit.mockClear(); (ShowMemoryCommandModule.createShowMemoryAction as Mock).mockClear(); diff --git a/packages/cli/src/ui/hooks/slashCommandProcessor.ts b/packages/cli/src/ui/hooks/slashCommandProcessor.ts index 858897f0..1f1c0444 100644 --- a/packages/cli/src/ui/hooks/slashCommandProcessor.ts +++ b/packages/cli/src/ui/hooks/slashCommandProcessor.ts @@ -113,7 +113,7 @@ export const useSlashCommandProcessor = ( { name: 'memory', description: - 'Manage memory. Usage: /memory [text for add]', + 'Manage memory. Usage: /memory [text for add]', action: (mainCommand, subCommand, args) => { switch (subCommand) { case 'show':