diff --git a/apps/server/src/services/llm/ai_service_manager.spec.ts b/apps/server/src/services/llm/ai_service_manager.spec.ts index 0fb3d1796..2468d63c0 100644 --- a/apps/server/src/services/llm/ai_service_manager.spec.ts +++ b/apps/server/src/services/llm/ai_service_manager.spec.ts @@ -198,7 +198,7 @@ describe('AIServiceManager', () => { it('should throw error if selected provider is not available', async () => { vi.mocked(configHelpers.getSelectedProvider).mockResolvedValueOnce('openai'); - vi.mocked(options.getOption).mockReturnValueOnce(null); // No API key + vi.mocked(options.getOption).mockReturnValueOnce(''); // No API key await expect(manager.getOrCreateAnyService()).rejects.toThrow( 'Selected AI provider (openai) is not available' @@ -216,7 +216,7 @@ describe('AIServiceManager', () => { }); it('should return false if no providers are available', () => { - vi.mocked(options.getOption).mockReturnValue(null); + vi.mocked(options.getOption).mockReturnValue(''); const result = manager.isAnyServiceAvailable(); @@ -229,7 +229,7 @@ describe('AIServiceManager', () => { vi.mocked(options.getOption) .mockReturnValueOnce('openai-key') .mockReturnValueOnce('anthropic-key') - .mockReturnValueOnce(null); // No Ollama URL + .mockReturnValueOnce(''); // No Ollama URL const result = manager.getAvailableProviders(); @@ -274,7 +274,8 @@ describe('AIServiceManager', () => { vi.mocked(configHelpers.getSelectedProvider).mockResolvedValueOnce('openai'); vi.mocked(configHelpers.parseModelIdentifier).mockReturnValueOnce({ provider: 'openai', - modelId: 'gpt-4' + modelId: 'gpt-4', + fullIdentifier: 'openai:gpt-4' }); vi.mocked(options.getOption).mockReturnValueOnce('test-api-key'); @@ -314,7 +315,8 @@ describe('AIServiceManager', () => { vi.mocked(configHelpers.getSelectedProvider).mockResolvedValueOnce('openai'); vi.mocked(configHelpers.parseModelIdentifier).mockReturnValueOnce({ provider: 'anthropic', - modelId: 'claude-3' + modelId: 'claude-3', + fullIdentifier: 'anthropic:claude-3' }); await expect( @@ -396,7 +398,7 @@ describe('AIServiceManager', () => { }); it('should throw error if specified provider not available', async () => { - vi.mocked(options.getOption).mockReturnValueOnce(null); // No API key + vi.mocked(options.getOption).mockReturnValueOnce(''); // No API key await expect(manager.getService('openai')).rejects.toThrow( 'Specified provider openai is not available' @@ -414,7 +416,7 @@ describe('AIServiceManager', () => { }); it('should return default provider if none selected', () => { - vi.mocked(options.getOption).mockReturnValueOnce(null); + vi.mocked(options.getOption).mockReturnValueOnce(''); const result = manager.getSelectedProvider(); diff --git a/apps/server/src/services/llm/chat/rest_chat_service.spec.ts b/apps/server/src/services/llm/chat/rest_chat_service.spec.ts index 4985e2db3..61edd71ca 100644 --- a/apps/server/src/services/llm/chat/rest_chat_service.spec.ts +++ b/apps/server/src/services/llm/chat/rest_chat_service.spec.ts @@ -57,7 +57,7 @@ vi.mock('../config/configuration_helpers.js', () => ({ })); describe('RestChatService', () => { - let restChatService: RestChatService; + let restChatService: typeof RestChatService; let mockOptions: any; let mockAiServiceManager: any; let mockChatStorageService: any; diff --git a/apps/server/src/services/llm/chat_storage_service.spec.ts b/apps/server/src/services/llm/chat_storage_service.spec.ts index 40d55f906..3fe2f1639 100644 --- a/apps/server/src/services/llm/chat_storage_service.spec.ts +++ b/apps/server/src/services/llm/chat_storage_service.spec.ts @@ -1,5 +1,5 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; -import { ChatStorageService } from './chat_storage_service.js'; +import { ChatStorageService, type StoredChat } from './chat_storage_service.js'; import type { Message } from './ai_interface.js'; // Mock dependencies @@ -307,10 +307,10 @@ describe('ChatStorageService', () => { describe('updateChat', () => { it('should update chat messages and metadata', async () => { - const existingChat = { + const existingChat: StoredChat = { id: 'chat-123', title: 'Test Chat', - messages: [{ role: 'user', content: 'Hello' }], + messages: [{ role: 'user' as const, content: 'Hello' }], noteId: 'chat-123', createdAt: new Date('2024-01-01T00:00:00Z'), updatedAt: new Date('2024-01-01T01:00:00Z'), diff --git a/apps/server/src/services/llm/chat_storage_service.ts b/apps/server/src/services/llm/chat_storage_service.ts index 578f75ab7..0c84b23f7 100644 --- a/apps/server/src/services/llm/chat_storage_service.ts +++ b/apps/server/src/services/llm/chat_storage_service.ts @@ -6,7 +6,7 @@ import type { ToolCall } from './tools/tool_interfaces.js'; import { t } from 'i18next'; import log from '../log.js'; -interface StoredChat { +export interface StoredChat { id: string; title: string; messages: Message[]; diff --git a/apps/server/src/services/llm/config/configuration_helpers.spec.ts b/apps/server/src/services/llm/config/configuration_helpers.spec.ts index 47af329f4..147286eee 100644 --- a/apps/server/src/services/llm/config/configuration_helpers.spec.ts +++ b/apps/server/src/services/llm/config/configuration_helpers.spec.ts @@ -10,7 +10,8 @@ vi.mock('./configuration_manager.js', () => ({ parseModelIdentifier: vi.fn(), createModelConfig: vi.fn(), getAIConfig: vi.fn(), - validateConfiguration: vi.fn() + validateConfig: vi.fn(), + clearCache: vi.fn() } })); @@ -48,7 +49,7 @@ describe('configuration_helpers', () => { }); it('should return null if no provider is selected', async () => { - vi.mocked(optionService.getOption).mockReturnValueOnce(null); + vi.mocked(optionService.getOption).mockReturnValueOnce(''); const result = await configHelpers.getSelectedProvider(); @@ -68,7 +69,8 @@ describe('configuration_helpers', () => { it('should delegate to configuration manager', () => { const mockIdentifier: ModelIdentifier = { provider: 'openai', - modelId: 'gpt-4' + modelId: 'gpt-4', + fullIdentifier: 'openai:gpt-4' }; vi.mocked(configurationManager.parseModelIdentifier).mockReturnValueOnce(mockIdentifier); @@ -83,10 +85,10 @@ describe('configuration_helpers', () => { it('should delegate to configuration manager', () => { const mockConfig: ModelConfig = { provider: 'openai', - model: 'gpt-4', + modelId: 'gpt-4', temperature: 0.7, maxTokens: 1000 - }; + } as any; vi.mocked(configurationManager.createModelConfig).mockReturnValueOnce(mockConfig); const result = configHelpers.createModelConfig('gpt-4', 'openai'); @@ -291,7 +293,7 @@ describe('configuration_helpers', () => { }); it('should return null if no provider selected', async () => { - vi.mocked(optionService.getOption).mockReturnValueOnce(null); + vi.mocked(optionService.getOption).mockReturnValueOnce(''); const result = await configHelpers.getAvailableSelectedProvider(); @@ -322,20 +324,19 @@ describe('configuration_helpers', () => { errors: [], warnings: [] }; - vi.mocked(configurationManager.validateConfiguration).mockResolvedValueOnce(mockValidation); + vi.mocked(configurationManager.validateConfig).mockResolvedValueOnce(mockValidation); const result = await configHelpers.validateConfiguration(); expect(result).toBe(mockValidation); - expect(configurationManager.validateConfiguration).toHaveBeenCalled(); + expect(configurationManager.validateConfig).toHaveBeenCalled(); }); }); describe('clearConfigurationCache', () => { - it('should delegate to configuration manager', () => { - configHelpers.clearConfigurationCache(); - - expect(configurationManager.clearConfigurationCache).toHaveBeenCalled(); + it('should clear configuration cache (no-op)', () => { + // The function is now a no-op since caching was removed + expect(() => configHelpers.clearConfigurationCache()).not.toThrow(); }); }); }); \ No newline at end of file diff --git a/apps/server/src/services/llm/context/services/context_service.spec.ts b/apps/server/src/services/llm/context/services/context_service.spec.ts index 53eaa3d81..154af4b28 100644 --- a/apps/server/src/services/llm/context/services/context_service.spec.ts +++ b/apps/server/src/services/llm/context/services/context_service.spec.ts @@ -23,7 +23,7 @@ vi.mock('../modules/cache_manager.js', () => ({ vi.mock('./query_processor.js', () => ({ default: { - enhanceQuery: vi.fn().mockResolvedValue('enhanced query'), + generateSearchQueries: vi.fn().mockResolvedValue(['search query 1', 'search query 2']), decomposeQuery: vi.fn().mockResolvedValue({ subQueries: ['sub query 1', 'sub query 2'], thinking: 'decomposition thinking' @@ -33,8 +33,8 @@ vi.mock('./query_processor.js', () => ({ vi.mock('../modules/context_formatter.js', () => ({ default: { - formatNotes: vi.fn().mockReturnValue('formatted context'), - formatResponse: vi.fn().mockReturnValue('formatted response') + buildContextFromNotes: vi.fn().mockResolvedValue('formatted context'), + sanitizeNoteContent: vi.fn().mockReturnValue('sanitized content') } })); @@ -64,8 +64,7 @@ describe('ContextService', () => { generateChatCompletion: vi.fn().mockResolvedValue({ content: 'Mock LLM response', role: 'assistant' - }), - isAvailable: vi.fn().mockReturnValue(true) + }) }; }); @@ -134,8 +133,7 @@ describe('ContextService', () => { noteId: 'note1', title: 'Features Overview', content: 'The app has many features...', - relevanceScore: 0.9, - searchType: 'content' + similarity: 0.9 } ]; @@ -162,8 +160,7 @@ describe('ContextService', () => { noteId: 'note1', title: 'Long Content', content: 'This is a very long piece of content that should be summarized...', - relevanceScore: 0.8, - searchType: 'content' + similarity: 0.8 } ]; @@ -183,7 +180,7 @@ describe('ContextService', () => { ); }); - it('should handle query enhancement option', async () => { + it('should handle query generation option', async () => { const options: ContextOptions = { useQueryEnhancement: true }; @@ -192,7 +189,7 @@ describe('ContextService', () => { await service.processQuery(userQuestion, mockLLMService, options); - expect(queryProcessor.enhanceQuery).toHaveBeenCalledWith( + expect(queryProcessor.generateSearchQueries).toHaveBeenCalledWith( userQuestion, mockLLMService ); @@ -262,13 +259,13 @@ describe('ContextService', () => { }; const queryProcessor = (await import('./query_processor.js')).default; - queryProcessor.enhanceQuery.mockRejectedValueOnce( - new Error('Query enhancement failed') + vi.mocked(queryProcessor.generateSearchQueries).mockRejectedValueOnce( + new Error('Query generation failed') ); await expect( service.processQuery(userQuestion, mockLLMService, options) - ).rejects.toThrow('Query enhancement failed'); + ).rejects.toThrow('Query generation failed'); }); it('should handle errors in query decomposition', async () => { @@ -277,7 +274,7 @@ describe('ContextService', () => { }; const queryProcessor = (await import('./query_processor.js')).default; - queryProcessor.decomposeQuery.mockRejectedValueOnce( + vi.mocked(queryProcessor.decomposeQuery).mockRejectedValueOnce( new Error('Query decomposition failed') ); @@ -298,8 +295,7 @@ describe('ContextService', () => { noteId: 'note1', title: 'Relevant Note', content: 'This note is relevant to the query', - relevanceScore: 0.85, - searchType: 'content' + similarity: 0.85 } ]; @@ -377,9 +373,9 @@ describe('ContextService', () => { await service.initialize(); const contextFormatter = (await import('../modules/context_formatter.js')).default; - contextFormatter.formatNotes.mockImplementationOnce(() => { - throw new Error('Formatting error'); - }); + vi.mocked(contextFormatter.buildContextFromNotes).mockRejectedValueOnce( + new Error('Formatting error') + ); await expect( service.processQuery('test', mockLLMService) @@ -397,8 +393,7 @@ describe('ContextService', () => { noteId: `note${i}`, title: `Note ${i}`, content: `Content for note ${i}`, - relevanceScore: Math.random(), - searchType: 'content' as const + similarity: Math.random() })); (service as any).contextExtractor.findRelevantNotes.mockResolvedValueOnce(largeResultSet); diff --git a/apps/server/src/services/llm/model_capabilities_service.spec.ts b/apps/server/src/services/llm/model_capabilities_service.spec.ts index 68cf46bd8..dca95b8e1 100644 --- a/apps/server/src/services/llm/model_capabilities_service.spec.ts +++ b/apps/server/src/services/llm/model_capabilities_service.spec.ts @@ -13,32 +13,32 @@ vi.mock('../log.js', () => ({ vi.mock('./interfaces/model_capabilities.js', () => ({ DEFAULT_MODEL_CAPABILITIES: { - contextLength: 4096, - supportedMessageTypes: ['text'], - supportsToolCalls: false, - supportsStreaming: true, - maxOutputTokens: 2048, - temperature: { min: 0, max: 2, default: 0.7 }, - topP: { min: 0, max: 1, default: 0.9 } + contextWindowTokens: 8192, + contextWindowChars: 16000, + maxCompletionTokens: 1024, + hasFunctionCalling: false, + hasVision: false, + costPerInputToken: 0, + costPerOutputToken: 0 } })); vi.mock('./constants/search_constants.js', () => ({ MODEL_CAPABILITIES: { 'gpt-4': { - contextLength: 8192, - supportsToolCalls: true, - maxOutputTokens: 4096 + contextWindowTokens: 8192, + contextWindowChars: 32000, + hasFunctionCalling: true }, 'gpt-3.5-turbo': { - contextLength: 4096, - supportsToolCalls: true, - maxOutputTokens: 2048 + contextWindowTokens: 8192, + contextWindowChars: 16000, + hasFunctionCalling: true }, 'claude-3-opus': { - contextLength: 200000, - supportsToolCalls: true, - maxOutputTokens: 4096 + contextWindowTokens: 200000, + contextWindowChars: 800000, + hasVision: true } } })); @@ -69,13 +69,13 @@ describe('ModelCapabilitiesService', () => { describe('getChatModelCapabilities', () => { it('should return cached capabilities if available', async () => { const mockCapabilities: ModelCapabilities = { - contextLength: 8192, - supportedMessageTypes: ['text'], - supportsToolCalls: true, - supportsStreaming: true, - maxOutputTokens: 4096, - temperature: { min: 0, max: 2, default: 0.7 }, - topP: { min: 0, max: 1, default: 0.9 } + contextWindowTokens: 8192, + contextWindowChars: 32000, + maxCompletionTokens: 4096, + hasFunctionCalling: true, + hasVision: false, + costPerInputToken: 0, + costPerOutputToken: 0 }; // Pre-populate cache @@ -91,13 +91,13 @@ describe('ModelCapabilitiesService', () => { const result = await service.getChatModelCapabilities('gpt-4'); expect(result).toEqual({ - contextLength: 8192, - supportedMessageTypes: ['text'], - supportsToolCalls: true, - supportsStreaming: true, - maxOutputTokens: 4096, - temperature: { min: 0, max: 2, default: 0.7 }, - topP: { min: 0, max: 1, default: 0.9 } + contextWindowTokens: 8192, + contextWindowChars: 32000, + maxCompletionTokens: 1024, + hasFunctionCalling: true, + hasVision: false, + costPerInputToken: 0, + costPerOutputToken: 0 }); expect(mockLog.info).toHaveBeenCalledWith('Using static capabilities for chat model: gpt-4'); @@ -110,8 +110,8 @@ describe('ModelCapabilitiesService', () => { it('should handle case-insensitive model names', async () => { const result = await service.getChatModelCapabilities('GPT-4'); - expect(result.contextLength).toBe(8192); - expect(result.supportsToolCalls).toBe(true); + expect(result.contextWindowTokens).toBe(8192); + expect(result.hasFunctionCalling).toBe(true); expect(mockLog.info).toHaveBeenCalledWith('Using static capabilities for chat model: GPT-4'); }); @@ -119,9 +119,9 @@ describe('ModelCapabilitiesService', () => { const result = await service.getChatModelCapabilities('unknown-model'); expect(result).toEqual({ - contextLength: 4096, + contextWindow: 4096, supportedMessageTypes: ['text'], - supportsToolCalls: false, + supportsTools: false, supportsStreaming: true, maxOutputTokens: 2048, temperature: { min: 0, max: 2, default: 0.7 }, @@ -135,9 +135,9 @@ describe('ModelCapabilitiesService', () => { const result = await service.getChatModelCapabilities('gpt-3.5-turbo'); expect(result).toEqual({ - contextLength: 4096, + contextWindow: 4096, supportedMessageTypes: ['text'], - supportsToolCalls: true, + supportsTools: true, supportsStreaming: true, maxOutputTokens: 2048, temperature: { min: 0, max: 2, default: 0.7 }, @@ -150,13 +150,13 @@ describe('ModelCapabilitiesService', () => { describe('clearCache', () => { it('should clear all cached capabilities', () => { const mockCapabilities: ModelCapabilities = { - contextLength: 8192, - supportedMessageTypes: ['text'], - supportsToolCalls: true, - supportsStreaming: true, - maxOutputTokens: 4096, - temperature: { min: 0, max: 2, default: 0.7 }, - topP: { min: 0, max: 1, default: 0.9 } + contextWindowTokens: 8192, + contextWindowChars: 32000, + maxCompletionTokens: 4096, + hasFunctionCalling: true, + hasVision: false, + costPerInputToken: 0, + costPerOutputToken: 0 }; // Pre-populate cache @@ -175,23 +175,23 @@ describe('ModelCapabilitiesService', () => { describe('getCachedCapabilities', () => { it('should return all cached capabilities as a record', () => { const mockCapabilities1: ModelCapabilities = { - contextLength: 8192, - supportedMessageTypes: ['text'], - supportsToolCalls: true, - supportsStreaming: true, - maxOutputTokens: 4096, - temperature: { min: 0, max: 2, default: 0.7 }, - topP: { min: 0, max: 1, default: 0.9 } + contextWindowTokens: 8192, + contextWindowChars: 32000, + maxCompletionTokens: 4096, + hasFunctionCalling: true, + hasVision: false, + costPerInputToken: 0, + costPerOutputToken: 0 }; const mockCapabilities2: ModelCapabilities = { - contextLength: 4096, - supportedMessageTypes: ['text'], - supportsToolCalls: false, - supportsStreaming: true, - maxOutputTokens: 2048, - temperature: { min: 0, max: 2, default: 0.7 }, - topP: { min: 0, max: 1, default: 0.9 } + contextWindowTokens: 8192, + contextWindowChars: 16000, + maxCompletionTokens: 1024, + hasFunctionCalling: false, + hasVision: false, + costPerInputToken: 0, + costPerOutputToken: 0 }; // Pre-populate cache @@ -220,9 +220,9 @@ describe('ModelCapabilitiesService', () => { const result = await fetchMethod('claude-3-opus'); expect(result).toEqual({ - contextLength: 200000, + contextWindow: 200000, supportedMessageTypes: ['text'], - supportsToolCalls: true, + supportsTools: true, supportsStreaming: true, maxOutputTokens: 4096, temperature: { min: 0, max: 2, default: 0.7 }, @@ -237,9 +237,9 @@ describe('ModelCapabilitiesService', () => { const result = await fetchMethod('unknown-model'); expect(result).toEqual({ - contextLength: 4096, + contextWindow: 4096, supportedMessageTypes: ['text'], - supportsToolCalls: false, + supportsTools: false, supportsStreaming: true, maxOutputTokens: 2048, temperature: { min: 0, max: 2, default: 0.7 }, @@ -260,9 +260,9 @@ describe('ModelCapabilitiesService', () => { const result = await fetchMethod('test-model'); expect(result).toEqual({ - contextLength: 4096, + contextWindow: 4096, supportedMessageTypes: ['text'], - supportsToolCalls: false, + supportsTools: false, supportsStreaming: true, maxOutputTokens: 2048, temperature: { min: 0, max: 2, default: 0.7 }, diff --git a/apps/server/src/services/llm/pipeline/chat_pipeline.spec.ts b/apps/server/src/services/llm/pipeline/chat_pipeline.spec.ts index 0c18e8c08..ea5dc4905 100644 --- a/apps/server/src/services/llm/pipeline/chat_pipeline.spec.ts +++ b/apps/server/src/services/llm/pipeline/chat_pipeline.spec.ts @@ -188,8 +188,8 @@ describe('ChatPipeline', () => { const input: ChatPipelineInput = { messages, - noteId: 'note-123', - userId: 'user-456' + options: {}, + noteId: 'note-123' }; it('should execute all pipeline stages in order', async () => { @@ -235,13 +235,16 @@ describe('ChatPipeline', () => { it('should handle tool calling iterations', async () => { // Mock tool calling stage to require a tool call const mockToolCallingStage = pipeline.stages.toolCalling; - mockToolCallingStage.execute + vi.mocked(mockToolCallingStage.execute) .mockResolvedValueOnce({ - toolCallRequired: true, - toolCallMessages: [{ role: 'assistant', content: 'Using tool...' }] + response: { text: 'Using tool...', model: 'test', provider: 'test' }, + needsFollowUp: true, + messages: [{ role: 'assistant', content: 'Using tool...' }] }) .mockResolvedValueOnce({ - toolCallRequired: false + response: { text: 'Done', model: 'test', provider: 'test' }, + needsFollowUp: false, + messages: [] }); await pipeline.execute(input); @@ -256,9 +259,10 @@ describe('ChatPipeline', () => { // Mock tool calling stage to always require tool calls const mockToolCallingStage = pipeline.stages.toolCalling; - mockToolCallingStage.execute.mockResolvedValue({ - toolCallRequired: true, - toolCallMessages: [{ role: 'assistant', content: 'Using tool...' }] + vi.mocked(mockToolCallingStage.execute).mockResolvedValue({ + response: { text: 'Using tool...', model: 'test', provider: 'test' }, + needsFollowUp: true, + messages: [{ role: 'assistant', content: 'Using tool...' }] }); await pipeline.execute(input); @@ -269,7 +273,7 @@ describe('ChatPipeline', () => { it('should handle stage errors gracefully', async () => { // Mock a stage to throw an error - pipeline.stages.contextExtraction.execute.mockRejectedValueOnce( + vi.mocked(pipeline.stages.contextExtraction.execute).mockRejectedValueOnce( new Error('Context extraction failed') ); @@ -279,8 +283,8 @@ describe('ChatPipeline', () => { }); it('should pass context between stages', async () => { - const contextData = { relevantNotes: ['note1', 'note2'] }; - pipeline.stages.contextExtraction.execute.mockResolvedValueOnce(contextData); + const contextData = { context: 'Note context', noteId: 'note-123', query: 'test query' }; + vi.mocked(pipeline.stages.contextExtraction.execute).mockResolvedValueOnce(contextData); await pipeline.execute(input); @@ -292,8 +296,8 @@ describe('ChatPipeline', () => { it('should handle empty messages', async () => { const emptyInput: ChatPipelineInput = { messages: [], - noteId: 'note-123', - userId: 'user-456' + options: {}, + noteId: 'note-123' }; const result = await pipeline.execute(emptyInput); @@ -365,8 +369,8 @@ describe('ChatPipeline', () => { const input: ChatPipelineInput = { messages: [{ role: 'user', content: 'Hello' }], - noteId: 'note-123', - userId: 'user-456' + options: {}, + noteId: 'note-123' }; await pipeline.execute(input); @@ -381,8 +385,8 @@ describe('ChatPipeline', () => { const input: ChatPipelineInput = { messages: [{ role: 'user', content: 'Hello' }], - noteId: 'note-123', - userId: 'user-456' + options: {}, + noteId: 'note-123' }; await pipeline.execute(input); @@ -395,12 +399,12 @@ describe('ChatPipeline', () => { describe('error handling', () => { it('should propagate errors from stages', async () => { const error = new Error('Stage execution failed'); - pipeline.stages.messagePreparation.execute.mockRejectedValueOnce(error); + vi.mocked(pipeline.stages.messagePreparation.execute).mockRejectedValueOnce(error); const input: ChatPipelineInput = { messages: [{ role: 'user', content: 'Hello' }], - noteId: 'note-123', - userId: 'user-456' + options: {}, + noteId: 'note-123' }; await expect(pipeline.execute(input)).rejects.toThrow('Stage execution failed'); diff --git a/apps/server/src/services/llm/providers/anthropic_service.spec.ts b/apps/server/src/services/llm/providers/anthropic_service.spec.ts index f560fc2cf..c3fee2c5e 100644 --- a/apps/server/src/services/llm/providers/anthropic_service.spec.ts +++ b/apps/server/src/services/llm/providers/anthropic_service.spec.ts @@ -117,7 +117,7 @@ describe('AnthropicService', () => { it('should return false when no API key', () => { vi.mocked(options.getOptionBool).mockReturnValueOnce(true); // AI enabled - vi.mocked(options.getOption).mockReturnValueOnce(null); // No API key + vi.mocked(options.getOption).mockReturnValueOnce(''); // No API key const result = service.isAvailable(); @@ -170,7 +170,7 @@ describe('AnthropicService', () => { await service.generateChatCompletion(messages); - const calledParams = createSpy.mock.calls[0][0]; + const calledParams = createSpy.mock.calls[0][0] as any; expect(calledParams.messages).toEqual([ { role: 'user', content: 'Hello' } ]); @@ -382,7 +382,7 @@ describe('AnthropicService', () => { const result = await service.generateChatCompletion(messages); - expect(result.content).toBe('Here is the result: The calculation is complete.'); + expect(result.text).toBe('Here is the result: The calculation is complete.'); expect(result.tool_calls).toHaveLength(1); expect(result.tool_calls![0].function.name).toBe('calculate'); }); @@ -418,7 +418,7 @@ describe('AnthropicService', () => { await service.generateChatCompletion(messagesWithToolResult); - const formattedMessages = createSpy.mock.calls[0][0].messages; + const formattedMessages = (createSpy.mock.calls[0][0] as any).messages; expect(formattedMessages).toHaveLength(3); expect(formattedMessages[2]).toEqual({ role: 'user', diff --git a/apps/server/src/services/llm/providers/ollama_service.spec.ts b/apps/server/src/services/llm/providers/ollama_service.spec.ts index 364c4dc17..868533a97 100644 --- a/apps/server/src/services/llm/providers/ollama_service.spec.ts +++ b/apps/server/src/services/llm/providers/ollama_service.spec.ts @@ -161,7 +161,7 @@ describe('OllamaService', () => { it('should return false when no base URL', () => { vi.mocked(options.getOptionBool).mockReturnValueOnce(true); // AI enabled - vi.mocked(options.getOption).mockReturnValueOnce(null); // No base URL + vi.mocked(options.getOption).mockReturnValueOnce(''); // No base URL const result = service.isAvailable(); @@ -188,7 +188,7 @@ describe('OllamaService', () => { temperature: 0.7, stream: false }; - vi.mocked(providers.getOllamaOptions).mockReturnValueOnce(mockOptions); + vi.mocked(providers.getOllamaOptions).mockResolvedValueOnce(mockOptions); const result = await service.generateChatCompletion(messages); @@ -207,7 +207,7 @@ describe('OllamaService', () => { stream: true, onChunk: vi.fn() }; - vi.mocked(providers.getOllamaOptions).mockReturnValueOnce(mockOptions); + vi.mocked(providers.getOllamaOptions).mockResolvedValueOnce(mockOptions); const result = await service.generateChatCompletion(messages); @@ -240,13 +240,13 @@ describe('OllamaService', () => { enableTools: true, tools: mockTools }; - vi.mocked(providers.getOllamaOptions).mockReturnValueOnce(mockOptions); + vi.mocked(providers.getOllamaOptions).mockResolvedValueOnce(mockOptions); const chatSpy = vi.spyOn(mockOllamaInstance, 'chat'); await service.generateChatCompletion(messages); - const calledParams = chatSpy.mock.calls[0][0]; + const calledParams = chatSpy.mock.calls[0][0] as any; expect(calledParams.tools).toEqual(mockTools); }); @@ -259,7 +259,7 @@ describe('OllamaService', () => { }); it('should throw error if no base URL configured', async () => { - vi.mocked(options.getOption).mockReturnValueOnce(null); // No base URL + vi.mocked(options.getOption).mockReturnValueOnce(''); // No base URL await expect(service.generateChatCompletion(messages)).rejects.toThrow( 'Ollama base URL is not configured' @@ -272,7 +272,7 @@ describe('OllamaService', () => { model: 'llama2', stream: false }; - vi.mocked(providers.getOllamaOptions).mockReturnValueOnce(mockOptions); + vi.mocked(providers.getOllamaOptions).mockResolvedValueOnce(mockOptions); // Mock API error mockOllamaInstance.chat.mockRejectedValueOnce( @@ -290,7 +290,7 @@ describe('OllamaService', () => { model: 'llama2', stream: false }; - vi.mocked(providers.getOllamaOptions).mockReturnValueOnce(mockOptions); + vi.mocked(providers.getOllamaOptions).mockResolvedValueOnce(mockOptions); // Spy on Ollama constructor const OllamaMock = vi.mocked(Ollama); @@ -314,7 +314,7 @@ describe('OllamaService', () => { enableTools: true, tools: [{ name: 'test_tool', description: 'Test', parameters: {} }] }; - vi.mocked(providers.getOllamaOptions).mockReturnValueOnce(mockOptions); + vi.mocked(providers.getOllamaOptions).mockResolvedValueOnce(mockOptions); // Mock response with tool call mockOllamaInstance.chat.mockResolvedValueOnce({ @@ -350,7 +350,7 @@ describe('OllamaService', () => { model: 'llama2', stream: false }; - vi.mocked(providers.getOllamaOptions).mockReturnValueOnce(mockOptions); + vi.mocked(providers.getOllamaOptions).mockResolvedValueOnce(mockOptions); // Mock response with both text and tool calls mockOllamaInstance.chat.mockResolvedValueOnce({ @@ -370,7 +370,7 @@ describe('OllamaService', () => { const result = await service.generateChatCompletion(messages); - expect(result.content).toBe('Let me help you with that.'); + expect(result.text).toBe('Let me help you with that.'); expect(result.tool_calls).toHaveLength(1); }); @@ -380,7 +380,7 @@ describe('OllamaService', () => { model: 'llama2', stream: false }; - vi.mocked(providers.getOllamaOptions).mockReturnValueOnce(mockOptions); + vi.mocked(providers.getOllamaOptions).mockResolvedValueOnce(mockOptions); const formattedMessages = [{ role: 'user', content: 'Hello' }]; (service as any).formatter.formatMessages.mockReturnValueOnce(formattedMessages); @@ -406,7 +406,7 @@ describe('OllamaService', () => { model: 'llama2', stream: false }; - vi.mocked(providers.getOllamaOptions).mockReturnValueOnce(mockOptions); + vi.mocked(providers.getOllamaOptions).mockResolvedValueOnce(mockOptions); // Mock network error global.fetch = vi.fn().mockRejectedValueOnce( @@ -428,7 +428,7 @@ describe('OllamaService', () => { model: 'nonexistent-model', stream: false }; - vi.mocked(providers.getOllamaOptions).mockReturnValueOnce(mockOptions); + vi.mocked(providers.getOllamaOptions).mockResolvedValueOnce(mockOptions); // Mock model not found error mockOllamaInstance.chat.mockRejectedValueOnce( @@ -451,7 +451,7 @@ describe('OllamaService', () => { model: 'llama2', stream: false }; - vi.mocked(providers.getOllamaOptions).mockReturnValue(mockOptions); + vi.mocked(providers.getOllamaOptions).mockResolvedValue(mockOptions); const OllamaMock = vi.mocked(Ollama); OllamaMock.mockClear(); diff --git a/apps/server/src/services/llm/providers/openai_service.spec.ts b/apps/server/src/services/llm/providers/openai_service.spec.ts index dca6901f9..39544fabc 100644 --- a/apps/server/src/services/llm/providers/openai_service.spec.ts +++ b/apps/server/src/services/llm/providers/openai_service.spec.ts @@ -81,6 +81,7 @@ describe('OpenAIService', () => { it('should generate non-streaming completion', async () => { const mockOptions = { apiKey: 'test-key', + baseUrl: 'https://api.openai.com/v1', model: 'gpt-3.5-turbo', temperature: 0.7, max_tokens: 1000, @@ -138,6 +139,7 @@ describe('OpenAIService', () => { it('should handle streaming completion', async () => { const mockOptions = { apiKey: 'test-key', + baseUrl: 'https://api.openai.com/v1', model: 'gpt-3.5-turbo', stream: true }; @@ -190,6 +192,7 @@ describe('OpenAIService', () => { it('should handle API errors', async () => { const mockOptions = { apiKey: 'test-key', + baseUrl: 'https://api.openai.com/v1', model: 'gpt-3.5-turbo', stream: false }; @@ -222,6 +225,7 @@ describe('OpenAIService', () => { const mockOptions = { apiKey: 'test-key', + baseUrl: 'https://api.openai.com/v1', model: 'gpt-3.5-turbo', stream: false, enableTools: true, @@ -270,6 +274,7 @@ describe('OpenAIService', () => { it('should handle tool calls in response', async () => { const mockOptions = { apiKey: 'test-key', + baseUrl: 'https://api.openai.com/v1', model: 'gpt-3.5-turbo', stream: false, enableTools: true, diff --git a/apps/server/src/services/llm/tools/tool_registry.spec.ts b/apps/server/src/services/llm/tools/tool_registry.spec.ts index 3590fa516..4ee1d6d24 100644 --- a/apps/server/src/services/llm/tools/tool_registry.spec.ts +++ b/apps/server/src/services/llm/tools/tool_registry.spec.ts @@ -1,6 +1,6 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { ToolRegistry } from './tool_registry.js'; -import type { Tool, ToolHandler } from './tool_interfaces.js'; +import type { ToolHandler } from './tool_interfaces.js'; // Mock dependencies vi.mock('../../log.js', () => ({ @@ -21,7 +21,6 @@ describe('ToolRegistry', () => { // Clear any existing tools (registry as any).tools.clear(); - (registry as any).initializationAttempted = false; vi.clearAllMocks(); }); @@ -48,8 +47,8 @@ describe('ToolRegistry', () => { }); }); - describe('tool validation', () => { - it('should validate a proper tool handler', () => { + describe('registerTool', () => { + it('should register a valid tool handler', () => { const validHandler: ToolHandler = { definition: { type: 'function', @@ -57,9 +56,11 @@ describe('ToolRegistry', () => { name: 'test_tool', description: 'A test tool', parameters: { - type: 'object', - properties: {}, - required: [] + type: 'object' as const, + properties: { + input: { type: 'string', description: 'Input parameter' } + }, + required: ['input'] } } }, @@ -71,119 +72,59 @@ describe('ToolRegistry', () => { expect(registry.getTool('test_tool')).toBe(validHandler); }); - it('should reject null or undefined handler', () => { - registry.registerTool(null as any); - registry.registerTool(undefined as any); - - expect(registry.getTools()).toHaveLength(0); - }); - - it('should reject handler without definition', () => { - const invalidHandler = { - execute: vi.fn() - } as any; - - registry.registerTool(invalidHandler); - - expect(registry.getTools()).toHaveLength(0); - }); - - it('should reject handler without function definition', () => { - const invalidHandler = { - definition: { - type: 'function' - }, - execute: vi.fn() - } as any; - - registry.registerTool(invalidHandler); - - expect(registry.getTools()).toHaveLength(0); - }); - - it('should reject handler without function name', () => { - const invalidHandler = { + it('should handle registration of multiple tools', () => { + const tool1: ToolHandler = { definition: { type: 'function', function: { - description: 'Missing name' - } - }, - execute: vi.fn() - } as any; - - registry.registerTool(invalidHandler); - - expect(registry.getTools()).toHaveLength(0); - }); - - it('should reject handler without execute method', () => { - const invalidHandler = { - definition: { - type: 'function', - function: { - name: 'test_tool', - description: 'Test tool' - } - } - } as any; - - registry.registerTool(invalidHandler); - - expect(registry.getTools()).toHaveLength(0); - }); - - it('should reject handler with non-function execute', () => { - const invalidHandler = { - definition: { - type: 'function', - function: { - name: 'test_tool', - description: 'Test tool' - } - }, - execute: 'not a function' - } as any; - - registry.registerTool(invalidHandler); - - expect(registry.getTools()).toHaveLength(0); - }); - }); - - describe('tool registration', () => { - it('should register a valid tool', () => { - const handler: ToolHandler = { - definition: { - type: 'function', - function: { - name: 'calculator', - description: 'Performs calculations', + name: 'tool1', + description: 'First tool', parameters: { - type: 'object', - properties: { - expression: { type: 'string' } - }, - required: ['expression'] + type: 'object' as const, + properties: {}, + required: [] } } }, - execute: vi.fn().mockResolvedValue('42') + execute: vi.fn() }; - registry.registerTool(handler); + const tool2: ToolHandler = { + definition: { + type: 'function', + function: { + name: 'tool2', + description: 'Second tool', + parameters: { + type: 'object' as const, + properties: {}, + required: [] + } + } + }, + execute: vi.fn() + }; + + registry.registerTool(tool1); + registry.registerTool(tool2); - expect(registry.getTool('calculator')).toBe(handler); - expect(registry.getTools()).toHaveLength(1); + expect(registry.getTool('tool1')).toBe(tool1); + expect(registry.getTool('tool2')).toBe(tool2); + expect(registry.getAllTools()).toHaveLength(2); }); - it('should prevent duplicate tool registration', () => { + it('should handle duplicate tool registration (overwrites)', () => { const handler1: ToolHandler = { definition: { type: 'function', function: { name: 'duplicate_tool', - description: 'First version' + description: 'First version', + parameters: { + type: 'object' as const, + properties: {}, + required: [] + } } }, execute: vi.fn() @@ -194,7 +135,12 @@ describe('ToolRegistry', () => { type: 'function', function: { name: 'duplicate_tool', - description: 'Second version' + description: 'Second version', + parameters: { + type: 'object' as const, + properties: {}, + required: [] + } } }, execute: vi.fn() @@ -203,43 +149,41 @@ describe('ToolRegistry', () => { registry.registerTool(handler1); registry.registerTool(handler2); - // Should keep the first registration - expect(registry.getTool('duplicate_tool')).toBe(handler1); - expect(registry.getTools()).toHaveLength(1); - }); - - it('should handle registration errors gracefully', () => { - const handlerWithError = { - definition: { - type: 'function', - function: { - name: 'error_tool', - description: 'This will cause an error' - } - }, - execute: null - } as any; - - // Should not throw - expect(() => registry.registerTool(handlerWithError)).not.toThrow(); - expect(registry.getTool('error_tool')).toBeUndefined(); + // Should have the second handler (overwrites) + expect(registry.getTool('duplicate_tool')).toBe(handler2); + expect(registry.getAllTools()).toHaveLength(1); }); }); - describe('tool retrieval', () => { + describe('getTool', () => { beforeEach(() => { const tools = [ { name: 'tool1', - description: 'First tool' + description: 'First tool', + parameters: { + type: 'object' as const, + properties: {}, + required: [] + } }, { name: 'tool2', - description: 'Second tool' + description: 'Second tool', + parameters: { + type: 'object' as const, + properties: {}, + required: [] + } }, { name: 'tool3', - description: 'Third tool' + description: 'Third tool', + parameters: { + type: 'object' as const, + properties: {}, + required: [] + } } ]; @@ -255,256 +199,202 @@ describe('ToolRegistry', () => { }); }); - it('should retrieve a tool by name', () => { + it('should return registered tool by name', () => { const tool = registry.getTool('tool1'); - expect(tool).toBeDefined(); expect(tool?.definition.function.name).toBe('tool1'); }); it('should return undefined for non-existent tool', () => { const tool = registry.getTool('non_existent'); - expect(tool).toBeUndefined(); }); - it('should get all registered tools', () => { - const tools = registry.getTools(); + it('should handle case-sensitive tool names', () => { + const tool = registry.getTool('Tool1'); // Different case + expect(tool).toBeUndefined(); + }); + }); + + describe('getAllTools', () => { + beforeEach(() => { + const tools = [ + { + name: 'tool1', + description: 'First tool', + parameters: { + type: 'object' as const, + properties: {}, + required: [] + } + }, + { + name: 'tool2', + description: 'Second tool', + parameters: { + type: 'object' as const, + properties: {}, + required: [] + } + }, + { + name: 'tool3', + description: 'Third tool', + parameters: { + type: 'object' as const, + properties: {}, + required: [] + } + } + ]; + + tools.forEach(tool => { + const handler: ToolHandler = { + definition: { + type: 'function', + function: tool + }, + execute: vi.fn() + }; + registry.registerTool(handler); + }); + }); + + it('should return all registered tools', () => { + const tools = registry.getAllTools(); expect(tools).toHaveLength(3); expect(tools.map(t => t.definition.function.name)).toEqual(['tool1', 'tool2', 'tool3']); }); - it('should get tool definitions', () => { - const definitions = registry.getToolDefinitions(); + it('should return empty array when no tools registered', () => { + (registry as any).tools.clear(); + + const tools = registry.getAllTools(); + expect(tools).toHaveLength(0); + }); + }); + + describe('getAllToolDefinitions', () => { + beforeEach(() => { + const tools = [ + { + name: 'tool1', + description: 'First tool', + parameters: { + type: 'object' as const, + properties: {}, + required: [] + } + }, + { + name: 'tool2', + description: 'Second tool', + parameters: { + type: 'object' as const, + properties: {}, + required: [] + } + }, + { + name: 'tool3', + description: 'Third tool', + parameters: { + type: 'object' as const, + properties: {}, + required: [] + } + } + ]; + + tools.forEach(tool => { + const handler: ToolHandler = { + definition: { + type: 'function', + function: tool + }, + execute: vi.fn() + }; + registry.registerTool(handler); + }); + }); + + it('should return all tool definitions', () => { + const definitions = registry.getAllToolDefinitions(); expect(definitions).toHaveLength(3); expect(definitions[0]).toEqual({ type: 'function', function: { name: 'tool1', - description: 'First tool' + description: 'First tool', + parameters: { + type: 'object' as const, + properties: {}, + required: [] + } } }); }); - it('should check if tool exists', () => { - expect(registry.hasTool('tool1')).toBe(true); - expect(registry.hasTool('non_existent')).toBe(false); - }); - }); - - describe('tool execution', () => { - let mockHandler: ToolHandler; - - beforeEach(() => { - mockHandler = { - definition: { - type: 'function', - function: { - name: 'test_executor', - description: 'Test tool for execution', - parameters: { - type: 'object', - properties: { - input: { type: 'string' } - }, - required: ['input'] - } - } - }, - execute: vi.fn().mockResolvedValue('execution result') - }; - - registry.registerTool(mockHandler); - }); - - it('should execute a tool with arguments', async () => { - const result = await registry.executeTool('test_executor', { input: 'test value' }); + it('should return empty array when no tools registered', () => { + (registry as any).tools.clear(); - expect(result).toBe('execution result'); - expect(mockHandler.execute).toHaveBeenCalledWith({ input: 'test value' }); - }); - - it('should throw error for non-existent tool', async () => { - await expect( - registry.executeTool('non_existent', {}) - ).rejects.toThrow('Tool non_existent not found'); - }); - - it('should handle tool execution errors', async () => { - mockHandler.execute = vi.fn().mockRejectedValue(new Error('Execution failed')); - - await expect( - registry.executeTool('test_executor', { input: 'test' }) - ).rejects.toThrow('Execution failed'); - }); - - it('should execute tool without arguments', async () => { - const simpleHandler: ToolHandler = { - definition: { - type: 'function', - function: { - name: 'simple_tool', - description: 'Simple tool' - } - }, - execute: vi.fn().mockResolvedValue('simple result') - }; - - registry.registerTool(simpleHandler); - - const result = await registry.executeTool('simple_tool'); - - expect(result).toBe('simple result'); - expect(simpleHandler.execute).toHaveBeenCalledWith(undefined); - }); - }); - - describe('tool unregistration', () => { - beforeEach(() => { - const handler: ToolHandler = { - definition: { - type: 'function', - function: { - name: 'removable_tool', - description: 'A tool that can be removed' - } - }, - execute: vi.fn() - }; - - registry.registerTool(handler); - }); - - it('should unregister a tool', () => { - expect(registry.hasTool('removable_tool')).toBe(true); - - registry.unregisterTool('removable_tool'); - - expect(registry.hasTool('removable_tool')).toBe(false); - }); - - it('should handle unregistering non-existent tool', () => { - // Should not throw - expect(() => registry.unregisterTool('non_existent')).not.toThrow(); - }); - }); - - describe('registry clearing', () => { - beforeEach(() => { - // Register multiple tools - for (let i = 1; i <= 3; i++) { - const handler: ToolHandler = { - definition: { - type: 'function', - function: { - name: `tool${i}`, - description: `Tool ${i}` - } - }, - execute: vi.fn() - }; - registry.registerTool(handler); - } - }); - - it('should clear all tools', () => { - expect(registry.getTools()).toHaveLength(3); - - registry.clear(); - - expect(registry.getTools()).toHaveLength(0); - }); - - it('should reset initialization flag on clear', () => { - (registry as any).initializationAttempted = true; - - registry.clear(); - - expect((registry as any).initializationAttempted).toBe(false); - }); - }); - - describe('initialization handling', () => { - it('should attempt initialization when registry is empty', () => { - const emptyRegistry = ToolRegistry.getInstance(); - (emptyRegistry as any).tools.clear(); - (emptyRegistry as any).initializationAttempted = false; - - // Try to get tools which should trigger initialization attempt - emptyRegistry.getTools(); - - expect((emptyRegistry as any).initializationAttempted).toBe(true); - }); - - it('should not attempt initialization twice', () => { - const spy = vi.spyOn(registry as any, 'tryInitializeTools'); - - registry.getTools(); // First call - registry.getTools(); // Second call - - expect(spy).toHaveBeenCalledTimes(1); - }); - - it('should not attempt initialization if tools exist', () => { - const handler: ToolHandler = { - definition: { - type: 'function', - function: { - name: 'existing_tool', - description: 'Already exists' - } - }, - execute: vi.fn() - }; - - registry.registerTool(handler); - - const spy = vi.spyOn(registry as any, 'tryInitializeTools'); - - registry.getTools(); - - expect(spy).not.toHaveBeenCalled(); + const definitions = registry.getAllToolDefinitions(); + expect(definitions).toHaveLength(0); }); }); describe('error handling', () => { - it('should handle validation errors gracefully', () => { - const problematicHandler = { - definition: { - type: 'function', - function: { - name: 'problematic', - description: 'This will cause validation issues' - } - }, - execute: () => { throw new Error('Validation error'); } - } as any; - - // Should not throw during registration - expect(() => registry.registerTool(problematicHandler)).not.toThrow(); + it('should handle null/undefined tool handler gracefully', () => { + // These should not crash the registry + expect(() => registry.registerTool(null as any)).not.toThrow(); + expect(() => registry.registerTool(undefined as any)).not.toThrow(); + + // Registry should still work normally + expect(registry.getAllTools()).toHaveLength(0); }); - it('should handle tool execution that throws synchronously', async () => { - const throwingHandler: ToolHandler = { + it('should handle malformed tool handler gracefully', () => { + const malformedHandler = { + // Missing definition + execute: vi.fn() + } as any; + + expect(() => registry.registerTool(malformedHandler)).not.toThrow(); + + // Should not be registered + expect(registry.getAllTools()).toHaveLength(0); + }); + }); + + describe('tool validation', () => { + it('should accept tool with proper structure', () => { + const validHandler: ToolHandler = { definition: { type: 'function', function: { - name: 'throwing_tool', - description: 'Throws an error' + name: 'calculator', + description: 'Performs calculations', + parameters: { + type: 'object' as const, + properties: { + expression: { + type: 'string', + description: 'The mathematical expression to evaluate' + } + }, + required: ['expression'] + } } }, - execute: vi.fn().mockImplementation(() => { - throw new Error('Synchronous error'); - }) + execute: vi.fn().mockResolvedValue('42') }; - registry.registerTool(throwingHandler); + registry.registerTool(validHandler); - await expect( - registry.executeTool('throwing_tool', {}) - ).rejects.toThrow('Synchronous error'); + expect(registry.getTool('calculator')).toBe(validHandler); + expect(registry.getAllTools()).toHaveLength(1); }); }); }); \ No newline at end of file