From ce7c4a31a1e13b585f382134a8edf466128213b5 Mon Sep 17 00:00:00 2001 From: perf3ct Date: Mon, 2 Jun 2025 21:43:36 +0000 Subject: [PATCH] refactor(llm): enhance configuration handling to avoid default assumptions and improve error handling --- .../src/services/llm/ai_service_manager.ts | 8 +- .../llm/config/configuration_helpers.ts | 59 +++++++++- .../llm/config/configuration_manager.ts | 51 ++++---- .../interfaces/configuration_interfaces.ts | 4 +- .../pipeline/stages/model_selection_stage.ts | 111 ++++++++++-------- 5 files changed, 150 insertions(+), 83 deletions(-) diff --git a/apps/server/src/services/llm/ai_service_manager.ts b/apps/server/src/services/llm/ai_service_manager.ts index b8ec8325e..cee457ebf 100644 --- a/apps/server/src/services/llm/ai_service_manager.ts +++ b/apps/server/src/services/llm/ai_service_manager.ts @@ -532,7 +532,13 @@ export class AIServiceManager implements IAIServiceManager { */ async getPreferredProviderAsync(): Promise { try { - return await getPreferredProvider(); + const preferredProvider = await getPreferredProvider(); + if (preferredProvider === null) { + // No providers configured, fallback to first available + log.info('No providers configured in precedence, using first available provider'); + return this.providerOrder[0]; + } + return preferredProvider; } catch (error) { log.error(`Error getting preferred provider: ${error}`); return this.providerOrder[0]; diff --git a/apps/server/src/services/llm/config/configuration_helpers.ts b/apps/server/src/services/llm/config/configuration_helpers.ts index b4a9d5432..88d2cf1da 100644 --- a/apps/server/src/services/llm/config/configuration_helpers.ts +++ b/apps/server/src/services/llm/config/configuration_helpers.ts @@ -23,8 +23,11 @@ export async function getProviderPrecedence(): Promise { /** * Get the default/preferred AI provider */ -export async function getPreferredProvider(): Promise { +export async function getPreferredProvider(): Promise { const config = await configurationManager.getProviderPrecedence(); + if (config.providers.length === 0) { + return null; // No providers configured + } return config.defaultProvider || config.providers[0]; } @@ -39,8 +42,11 @@ export async function getEmbeddingProviderPrecedence(): Promise { /** * Get the default embedding provider */ -export async function getPreferredEmbeddingProvider(): Promise { +export async function getPreferredEmbeddingProvider(): Promise { const config = await configurationManager.getEmbeddingProviderPrecedence(); + if (config.providers.length === 0) { + return null; // No providers configured + } return config.defaultProvider || config.providers[0]; } @@ -61,9 +67,9 @@ export function createModelConfig(modelString: string, defaultProvider?: Provide /** * Get the default model for a specific provider */ -export async function getDefaultModelForProvider(provider: ProviderType): Promise { +export async function getDefaultModelForProvider(provider: ProviderType): Promise { const config = await configurationManager.getAIConfig(); - return config.defaultModels[provider]; + return config.defaultModels[provider]; // This can now be undefined } /** @@ -106,13 +112,17 @@ export async function isProviderConfigured(provider: ProviderType): Promise { const providers = await getProviderPrecedence(); + if (providers.length === 0) { + return null; // No providers configured + } + for (const provider of providers) { if (await isProviderConfigured(provider)) { return provider; } } - return null; + return null; // No providers are properly configured } /** @@ -128,3 +138,42 @@ export async function validateConfiguration() { export function clearConfigurationCache(): void { configurationManager.clearCache(); } + +/** + * Get a model configuration with validation that no defaults are assumed + */ +export async function getValidModelConfig(provider: ProviderType): Promise<{ model: string; provider: ProviderType } | null> { + const defaultModel = await getDefaultModelForProvider(provider); + + if (!defaultModel) { + // No default model configured for this provider + return null; + } + + const isConfigured = await isProviderConfigured(provider); + if (!isConfigured) { + // Provider is not properly configured + return null; + } + + return { + model: defaultModel, + provider + }; +} + +/** + * Get the first valid model configuration from the provider precedence list + */ +export async function getFirstValidModelConfig(): Promise<{ model: string; provider: ProviderType } | null> { + const providers = await getProviderPrecedence(); + + for (const provider of providers) { + const config = await getValidModelConfig(provider); + if (config) { + return config; + } + } + + return null; // No valid model configuration found +} diff --git a/apps/server/src/services/llm/config/configuration_manager.ts b/apps/server/src/services/llm/config/configuration_manager.ts index 621e5d3f4..5bc9611b8 100644 --- a/apps/server/src/services/llm/config/configuration_manager.ts +++ b/apps/server/src/services/llm/config/configuration_manager.ts @@ -75,13 +75,14 @@ export class ConfigurationManager { return { providers: providers as ProviderType[], - defaultProvider: providers[0] as ProviderType + defaultProvider: providers.length > 0 ? providers[0] as ProviderType : undefined }; } catch (error) { log.error(`Error parsing provider precedence: ${error}`); + // Only return known providers if they exist, don't assume defaults return { - providers: ['openai', 'anthropic', 'ollama'], - defaultProvider: 'openai' + providers: [], + defaultProvider: undefined }; } } @@ -96,13 +97,14 @@ export class ConfigurationManager { return { providers: providers as EmbeddingProviderType[], - defaultProvider: providers[0] as EmbeddingProviderType + defaultProvider: providers.length > 0 ? providers[0] as EmbeddingProviderType : undefined }; } catch (error) { log.error(`Error parsing embedding provider precedence: ${error}`); + // Don't assume defaults, return empty configuration return { - providers: ['openai', 'ollama'], - defaultProvider: 'openai' + providers: [], + defaultProvider: undefined }; } } @@ -167,9 +169,9 @@ export class ConfigurationManager { } /** - * Get default models for each provider + * Get default models for each provider - ONLY from user configuration */ - public async getDefaultModels(): Promise> { + public async getDefaultModels(): Promise> { try { const [openaiModel, anthropicModel, ollamaModel] = await Promise.all([ options.getOption('openaiDefaultModel'), @@ -178,16 +180,17 @@ export class ConfigurationManager { ]); return { - openai: openaiModel || 'gpt-3.5-turbo', - anthropic: anthropicModel || 'claude-3-sonnet-20240229', - ollama: ollamaModel || 'llama2' + openai: openaiModel || undefined, + anthropic: anthropicModel || undefined, + ollama: ollamaModel || undefined }; } catch (error) { log.error(`Error loading default models: ${error}`); + // Return undefined for all providers if we can't load config return { - openai: 'gpt-3.5-turbo', - anthropic: 'claude-3-sonnet-20240229', - ollama: 'llama2' + openai: undefined, + anthropic: undefined, + ollama: undefined }; } } @@ -322,7 +325,8 @@ export class ConfigurationManager { private parseProviderList(precedenceOption: string | null): string[] { if (!precedenceOption) { - return ['openai', 'anthropic', 'ollama']; + // Don't assume any defaults - return empty array + return []; } try { @@ -344,7 +348,8 @@ export class ConfigurationManager { } catch (error) { log.error(`Error parsing provider list "${precedenceOption}": ${error}`); - return ['openai', 'anthropic', 'ollama']; + // Don't assume defaults on parse error + return []; } } @@ -352,17 +357,17 @@ export class ConfigurationManager { return { enabled: false, providerPrecedence: { - providers: ['openai', 'anthropic', 'ollama'], - defaultProvider: 'openai' + providers: [], + defaultProvider: undefined }, embeddingProviderPrecedence: { - providers: ['openai', 'ollama'], - defaultProvider: 'openai' + providers: [], + defaultProvider: undefined }, defaultModels: { - openai: 'gpt-3.5-turbo', - anthropic: 'claude-3-sonnet-20240229', - ollama: 'llama2' + openai: undefined, + anthropic: undefined, + ollama: undefined }, providerSettings: {} }; diff --git a/apps/server/src/services/llm/interfaces/configuration_interfaces.ts b/apps/server/src/services/llm/interfaces/configuration_interfaces.ts index 080f373fe..5a03dc4f1 100644 --- a/apps/server/src/services/llm/interfaces/configuration_interfaces.ts +++ b/apps/server/src/services/llm/interfaces/configuration_interfaces.ts @@ -48,7 +48,7 @@ export interface AIConfig { enabled: boolean; providerPrecedence: ProviderPrecedenceConfig; embeddingProviderPrecedence: EmbeddingProviderPrecedenceConfig; - defaultModels: Record; + defaultModels: Record; providerSettings: ProviderSettings; } @@ -105,4 +105,4 @@ export interface ConfigValidationResult { isValid: boolean; errors: string[]; warnings: string[]; -} \ No newline at end of file +} diff --git a/apps/server/src/services/llm/pipeline/stages/model_selection_stage.ts b/apps/server/src/services/llm/pipeline/stages/model_selection_stage.ts index 427c63653..fdecc216e 100644 --- a/apps/server/src/services/llm/pipeline/stages/model_selection_stage.ts +++ b/apps/server/src/services/llm/pipeline/stages/model_selection_stage.ts @@ -100,61 +100,65 @@ export class ModelSelectionStage extends BasePipelineStage query.toLowerCase().includes(term)); - const isLongQuery = query.length > 100; - const hasMultipleQuestions = (query.match(/\?/g) || []).length > 1; - - if ((hasComplexTerms && isLongQuery) || hasMultipleQuestions) { - queryComplexity = 'high'; - } else if (hasComplexTerms || isLongQuery) { - queryComplexity = 'medium'; + if (!preferredProvider) { + throw new Error('No AI providers are configured. Please check your AI settings.'); } + + const modelName = await getDefaultModelForProvider(preferredProvider); + + if (!modelName) { + throw new Error(`No default model configured for provider ${preferredProvider}. Please set a default model in your AI settings.`); + } + + log.info(`Selected provider: ${preferredProvider}, model: ${modelName}`); + + // Determine query complexity + let queryComplexity = 'low'; + if (query) { + // Simple heuristic: longer queries or those with complex terms indicate higher complexity + const complexityIndicators = [ + 'explain', 'analyze', 'compare', 'evaluate', 'synthesize', + 'summarize', 'elaborate', 'investigate', 'research', 'debate' + ]; + + const hasComplexTerms = complexityIndicators.some(term => query.toLowerCase().includes(term)); + const isLongQuery = query.length > 100; + const hasMultipleQuestions = (query.match(/\?/g) || []).length > 1; + + if ((hasComplexTerms && isLongQuery) || hasMultipleQuestions) { + queryComplexity = 'high'; + } else if (hasComplexTerms || isLongQuery) { + queryComplexity = 'medium'; + } + } + + // Check content length if provided + if (contentLength && contentLength > SEARCH_CONSTANTS.CONTEXT.CONTENT_LENGTH.MEDIUM_THRESHOLD) { + // For large content, favor more powerful models + queryComplexity = contentLength > SEARCH_CONSTANTS.CONTEXT.CONTENT_LENGTH.HIGH_THRESHOLD ? 'high' : 'medium'; + } + + // Set the model and add provider metadata + updatedOptions.model = modelName; + this.addProviderMetadata(updatedOptions, preferredProvider as ServiceProviders, modelName); + + log.info(`Selected model: ${modelName} from provider: ${preferredProvider} for query complexity: ${queryComplexity}`); + log.info(`[ModelSelectionStage] Final options: ${JSON.stringify({ + model: updatedOptions.model, + stream: updatedOptions.stream, + provider: preferredProvider, + enableTools: updatedOptions.enableTools + })}`); + + return { options: updatedOptions }; + } catch (error) { + log.error(`Error determining default model: ${error}`); + throw new Error(`Failed to determine AI model configuration: ${error}`); } - - // Check content length if provided - if (contentLength && contentLength > SEARCH_CONSTANTS.CONTEXT.CONTENT_LENGTH.MEDIUM_THRESHOLD) { - // For large content, favor more powerful models - queryComplexity = contentLength > SEARCH_CONSTANTS.CONTEXT.CONTENT_LENGTH.HIGH_THRESHOLD ? 'high' : 'medium'; - } - - // Set the model and add provider metadata - updatedOptions.model = defaultModelName; - this.addProviderMetadata(updatedOptions, defaultProvider as ServiceProviders, defaultModelName); - - log.info(`Selected model: ${defaultModelName} from provider: ${defaultProvider} for query complexity: ${queryComplexity}`); - log.info(`[ModelSelectionStage] Final options: ${JSON.stringify({ - model: updatedOptions.model, - stream: updatedOptions.stream, - provider: defaultProvider, - enableTools: updatedOptions.enableTools - })}`); - - return { options: updatedOptions }; } /** @@ -225,6 +229,10 @@ export class ModelSelectionStage extends BasePipelineStage