From d58c36d100f2eaf0a302c45a834fd9ec87391ed8 Mon Sep 17 00:00:00 2001 From: Benjamin McCormick Date: Tue, 11 Nov 2025 17:42:25 -0500 Subject: [PATCH] chore: code review --- .env.template | 32 +++++++-- config.toml.example | 25 ++++++- src/config.py | 85 +++++++----------------- src/utils/clients.py | 151 +++++++++++++++++++++++-------------------- 4 files changed, 155 insertions(+), 138 deletions(-) diff --git a/.env.template b/.env.template index a91ede16..5879b5ed 100644 --- a/.env.template +++ b/.env.template @@ -89,14 +89,18 @@ LLM_ANTHROPIC_API_KEY=your-anthropic-api-key-here # DERIVER_WORKING_REPRESENTATION_MAX_OBSERVATIONS=100 # DERIVER_REPRESENTATION_BATCH_MAX_TOKENS=4096 # DERIVER_MAX_INPUT_TOKENS=23000 +# DERIVER_BACKUP_PROVIDER= +# DERIVER_BACKUP_MODEL= # ============================================================================= # Peer Card Configuration # ============================================================================= -# ENABLED=true -# PROVIDER=openai -# MODEL=gpt-5-nano-2025-08-07 -# MAX_OUTPUT_TOKENS=4000 +# PEER_CARD_ENABLED=true +# PEER_CARD_PROVIDER=openai +# PEER_CARD_MODEL=gpt-5-nano-2025-08-07 +# PEER_CARD_MAX_OUTPUT_TOKENS=4000 +# PEER_CARD_BACKUP_PROVIDER= +# PEER_CARD_BACKUP_MODEL= # ============================================================================= # Dialectic Settings @@ -111,6 +115,8 @@ LLM_ANTHROPIC_API_KEY=your-anthropic-api-key-here # DIALECTIC_SEMANTIC_SEARCH_MAX_DISTANCE=0.85 # DIALECTIC_THINKING_BUDGET_TOKENS=1024 # DIALECTIC_CONTEXT_WINDOW_SIZE=100000 +# DIALECTIC_BACKUP_PROVIDER= +# DIALECTIC_BACKUP_MODEL= # ============================================================================= # Summary Settings @@ -119,10 +125,26 @@ LLM_ANTHROPIC_API_KEY=your-anthropic-api-key-here # SUMMARY_MESSAGES_PER_SHORT_SUMMARY=20 # SUMMARY_MESSAGES_PER_LONG_SUMMARY=60 # SUMMARY_PROVIDER=google -# SUMMARY_MODEL=gemini-1.5-flash-latest +# SUMMARY_MODEL=gemini-2.5-flash # SUMMARY_MAX_TOKENS_SHORT=1000 # SUMMARY_MAX_TOKENS_LONG=2000 # SUMMARY_THINKING_BUDGET_TOKENS=512 +# SUMMARY_BACKUP_PROVIDER= +# SUMMARY_BACKUP_MODEL= + +# ============================================================================= +# Dream Settings +# ============================================================================= +# DREAM_ENABLED=true +# DREAM_DOCUMENT_THRESHOLD=50 +# DREAM_IDLE_TIMEOUT_MINUTES=60 +# DREAM_MIN_HOURS_BETWEEN_DREAMS=8 +# DREAM_ENABLED_TYPES=["consolidate"] +# DREAM_PROVIDER=openai +# DREAM_MODEL=gpt-4o-mini-2024-07-18 +# DREAM_MAX_TOKENS=2000 +# DREAM_BACKUP_PROVIDER= +# DREAM_BACKUP_MODEL= # ============================================================================= # Webhook Settings diff --git a/config.toml.example b/config.toml.example index 50f618ba..dddd8690 100644 --- a/config.toml.example +++ b/config.toml.example @@ -63,9 +63,11 @@ WORKERS = 1 POLLING_SLEEP_INTERVAL_SECONDS = 1.0 STALE_SESSION_TIMEOUT_MINUTES = 5 PROVIDER = "google" -MODEL = "gemini-2.0-flash-lite" +MODEL = "gemini-2.5-flash-lite" MAX_OUTPUT_TOKENS = 2500 THINKING_BUDGET_TOKENS = 1024 # only applied when using Anthropic +# BACKUP_PROVIDER = "google" +# BACKUP_MODEL = "gemini-2.5-flash-lite" WORKING_REPRESENTATION_MAX_OBSERVATIONS = 100 REPRESENTATION_BATCH_MAX_TOKENS = 4096 @@ -77,6 +79,8 @@ ENABLED = true PROVIDER = "openai" MODEL = "gpt-5-nano-2025-08-07" MAX_OUTPUT_TOKENS = 4000 +# BACKUP_PROVIDER = "google" +# BACKUP_MODEL = "gemini-2.5-flash-lite" # Dialectic settings [dialectic] @@ -90,6 +94,8 @@ SEMANTIC_SEARCH_TOP_K = 10 SEMANTIC_SEARCH_MAX_DISTANCE = 0.85 THINKING_BUDGET_TOKENS = 1024 CONTEXT_WINDOW_SIZE = 100000 +# BACKUP_PROVIDER = "google" +# BACKUP_MODEL = "gemini-2.5-flash" # Summary settings [summary] @@ -97,10 +103,25 @@ ENABLED = true MESSAGES_PER_SHORT_SUMMARY = 20 MESSAGES_PER_LONG_SUMMARY = 60 PROVIDER = "google" -MODEL = "gemini-1.5-flash-latest" +MODEL = "gemini-2.5-flash" MAX_TOKENS_SHORT = 1000 MAX_TOKENS_LONG = 2000 THINKING_BUDGET_TOKENS = 512 +# BACKUP_PROVIDER = "google" +# BACKUP_MODEL = "gemini-2.5-flash" + +# Dream settings +[dream] +ENABLED = true +DOCUMENT_THRESHOLD = 50 +IDLE_TIMEOUT_MINUTES = 60 +MIN_HOURS_BETWEEN_DREAMS = 8 +ENABLED_TYPES = ["consolidate"] +PROVIDER = "openai" +MODEL = "gpt-4o-mini-2024-07-18" +MAX_OUTPUT_TOKENS = 2000 +# BACKUP_PROVIDER = "google" +# BACKUP_MODEL = "gemini-2.5-flash" # Webhook settings [webhook] diff --git a/src/config.py b/src/config.py index 681c9225..a0219789 100644 --- a/src/config.py +++ b/src/config.py @@ -128,6 +128,26 @@ class HonchoSettings(BaseSettings): ) +class BackupLLMSettingsMixin: + """Mixin class for settings that support backup LLM provider configuration. + + Provides backup provider and model fields along with validation to ensure + both fields are set together or both are None. + """ + + BACKUP_PROVIDER: SupportedProviders | None = None + BACKUP_MODEL: str | None = None + + @model_validator(mode="after") + def _validate_backup_configuration(self): + """Ensure both backup fields are set together or both are None.""" + if (self.BACKUP_PROVIDER is None) != (self.BACKUP_MODEL is None): + raise ValueError( + "BACKUP_PROVIDER and BACKUP_MODEL must both be set or both be None" + ) + return self + + class DBSettings(HonchoSettings): model_config = SettingsConfigDict(env_prefix="DB_", extra="ignore") # pyright: ignore @@ -191,7 +211,7 @@ class LLMSettings(HonchoSettings): DEFAULT_MAX_TOKENS: Annotated[int, Field(default=1000, gt=0, le=100_000)] = 2500 -class DeriverSettings(HonchoSettings): +class DeriverSettings(BackupLLMSettingsMixin, HonchoSettings): model_config = SettingsConfigDict(env_prefix="DERIVER_", extra="ignore") # pyright: ignore WORKERS: Annotated[int, Field(default=1, gt=0, le=100)] = 1 @@ -207,8 +227,6 @@ class DeriverSettings(HonchoSettings): PROVIDER: SupportedProviders = "google" MODEL: str = "gemini-2.5-flash-lite" - BACKUP_PROVIDER: SupportedProviders | None = None - BACKUP_MODEL: str | None = None MAX_OUTPUT_TOKENS: Annotated[int, Field(default=10_000, gt=0, le=100_000)] = 10_000 # Thinking budget tokens are only applied when using Anthropic as provider @@ -230,15 +248,6 @@ class DeriverSettings(HonchoSettings): MAX_INPUT_TOKENS: Annotated[int, Field(default=23000, gt=0, le=23000)] = 23000 - @model_validator(mode="after") - def validate_backup_configuration(self): - """Ensure both backup fields are set together or both are None.""" - if (self.BACKUP_PROVIDER is None) != (self.BACKUP_MODEL is None): - raise ValueError( - "BACKUP_PROVIDER and BACKUP_MODEL must both be set or both be None" - ) - return self - @model_validator(mode="after") def validate_batch_tokens_vs_context_limit(self): if self.REPRESENTATION_BATCH_MAX_TOKENS > self.MAX_INPUT_TOKENS: @@ -248,35 +257,22 @@ class DeriverSettings(HonchoSettings): return self -class PeerCardSettings(HonchoSettings): +class PeerCardSettings(BackupLLMSettingsMixin, HonchoSettings): model_config = SettingsConfigDict(env_prefix="PEER_CARD_", extra="ignore") # pyright: ignore ENABLED: bool = True PROVIDER: SupportedProviders = "openai" MODEL: str = "gpt-5-nano-2025-08-07" - BACKUP_PROVIDER: SupportedProviders | None = None - BACKUP_MODEL: str | None = None # Note: peer cards should be very short, but GPT-5 models need output tokens for thinking which cannot be turned off... MAX_OUTPUT_TOKENS: Annotated[int, Field(default=4000, gt=1000, le=10_000)] = 4000 - @model_validator(mode="after") - def validate_backup_configuration(self): - """Ensure both backup fields are set together or both are None.""" - if (self.BACKUP_PROVIDER is None) != (self.BACKUP_MODEL is None): - raise ValueError( - "BACKUP_PROVIDER and BACKUP_MODEL must both be set or both be None" - ) - return self - -class DialecticSettings(HonchoSettings): +class DialecticSettings(BackupLLMSettingsMixin, HonchoSettings): model_config = SettingsConfigDict(env_prefix="DIALECTIC_", extra="ignore") # pyright: ignore PROVIDER: SupportedProviders = "anthropic" MODEL: str = "claude-sonnet-4-20250514" - BACKUP_PROVIDER: SupportedProviders | None = None - BACKUP_MODEL: str | None = None PERFORM_QUERY_GENERATION: bool = False QUERY_GENERATION_PROVIDER: SupportedProviders = "groq" @@ -295,17 +291,8 @@ class DialecticSettings(HonchoSettings): int, Field(default=100_000, gt=10_000, le=200_000) ] = 100_000 - @model_validator(mode="after") - def validate_backup_configuration(self): - """Ensure both backup fields are set together or both are None.""" - if (self.BACKUP_PROVIDER is None) != (self.BACKUP_MODEL is None): - raise ValueError( - "BACKUP_PROVIDER and BACKUP_MODEL must both be set or both be None" - ) - return self - -class SummarySettings(HonchoSettings): +class SummarySettings(BackupLLMSettingsMixin, HonchoSettings): model_config = SettingsConfigDict(env_prefix="SUMMARY_", extra="ignore") # pyright: ignore ENABLED: bool = True @@ -315,22 +302,11 @@ class SummarySettings(HonchoSettings): PROVIDER: SupportedProviders = "openai" MODEL: str = "gpt-4o-mini-2024-07-18" - BACKUP_PROVIDER: SupportedProviders | None = None - BACKUP_MODEL: str | None = None MAX_TOKENS_SHORT: Annotated[int, Field(default=1000, gt=0, le=10_000)] = 1000 MAX_TOKENS_LONG: Annotated[int, Field(default=4000, gt=0, le=20_000)] = 4000 THINKING_BUDGET_TOKENS: Annotated[int, Field(default=512, gt=0, le=2000)] = 512 - @model_validator(mode="after") - def validate_backup_configuration(self): - """Ensure both backup fields are set together or both are None.""" - if (self.BACKUP_PROVIDER is None) != (self.BACKUP_MODEL is None): - raise ValueError( - "BACKUP_PROVIDER and BACKUP_MODEL must both be set or both be None" - ) - return self - class WebhookSettings(HonchoSettings): model_config = SettingsConfigDict(env_prefix="WEBHOOK_", extra="ignore") # pyright: ignore @@ -346,7 +322,7 @@ class MetricsSettings(HonchoSettings): NAMESPACE: str = "honcho" -class DreamSettings(HonchoSettings): +class DreamSettings(BackupLLMSettingsMixin, HonchoSettings): model_config = SettingsConfigDict(env_prefix="DREAM_", extra="ignore") # pyright: ignore ENABLED: bool = True @@ -358,19 +334,8 @@ class DreamSettings(HonchoSettings): # LLM settings for dream processing PROVIDER: SupportedProviders = "openai" MODEL: str = "gpt-4o-mini-2024-07-18" - BACKUP_PROVIDER: SupportedProviders | None = None - BACKUP_MODEL: str | None = None MAX_OUTPUT_TOKENS: Annotated[int, Field(default=2000, gt=0, le=10_000)] = 2000 - @model_validator(mode="after") - def validate_backup_configuration(self): - """Ensure both backup fields are set together or both are None.""" - if (self.BACKUP_PROVIDER is None) != (self.BACKUP_MODEL is None): - raise ValueError( - "BACKUP_PROVIDER and BACKUP_MODEL must both be set or both be None" - ) - return self - class AppSettings(HonchoSettings): # No env_prefix for app-level settings diff --git a/src/utils/clients.py b/src/utils/clients.py index 64588f2b..8913d2e9 100644 --- a/src/utils/clients.py +++ b/src/utils/clients.py @@ -208,53 +208,92 @@ async def honcho_llm_call( Raises: ValueError: If provider is not configured """ - # Extract provider/model from settings, with backup support - attempt = _current_attempt.get() + # Set attempt counter to 1 for first call (tenacity uses 1-indexed attempts) + _current_attempt.set(1) - # Use backup if: (1) we're retrying, (2) backup is configured, (3) backup client exists - use_backup = ( - attempt > 0 - and llm_settings.BACKUP_PROVIDER is not None - and llm_settings.BACKUP_MODEL is not None - and llm_settings.BACKUP_PROVIDER in CLIENTS - ) - - if ( - use_backup - and llm_settings.BACKUP_PROVIDER is not None - and llm_settings.BACKUP_MODEL is not None + async def _call_with_provider_selection() -> ( + HonchoLLMCallResponse[Any] | AsyncIterator[HonchoLLMCallStreamChunk] ): - provider = llm_settings.BACKUP_PROVIDER - model = llm_settings.BACKUP_MODEL - logger.info( - f"Retry attempt {attempt}: switching from " - + f"{llm_settings.PROVIDER}/{llm_settings.MODEL} to " - + f"backup {provider}/{model}" - ) + """ + Inner function that selects provider/model based on current attempt. + This function is retried, so provider selection happens on each attempt. + """ + attempt = _current_attempt.get() - # Filter out incompatible parameters when using backup - if provider != "anthropic" and thinking_budget_tokens: - logger.warning( - f"thinking_budget_tokens not supported by {provider}, ignoring" + # Use backup on final retry attempt (when attempt == retry_attempts) + if ( + attempt == retry_attempts + and llm_settings.BACKUP_PROVIDER is not None + and llm_settings.BACKUP_MODEL is not None + and llm_settings.BACKUP_PROVIDER in CLIENTS + ): + provider: SupportedProviders = llm_settings.BACKUP_PROVIDER + model: str = llm_settings.BACKUP_MODEL + logger.info( + f"Final retry attempt {attempt}: switching from " + + f"{llm_settings.PROVIDER}/{llm_settings.MODEL} to " + + f"backup {provider}/{model}" ) - thinking_budget_tokens = None - if model and "gpt-5" not in model and (reasoning_effort or verbosity): - logger.warning( - "reasoning_effort/verbosity only supported by GPT-5 models, ignoring" + # Filter out incompatible parameters when using backup + thinking_budget = thinking_budget_tokens + gpt5_reasoning_effort = reasoning_effort + gpt5_verbosity = verbosity + + if provider != "anthropic" and thinking_budget: + logger.warning( + f"thinking_budget_tokens not supported by {provider}, ignoring" + ) + thinking_budget = None + + if "gpt-5" not in model and (gpt5_reasoning_effort or gpt5_verbosity): + logger.warning( + "reasoning_effort/verbosity only supported by GPT-5 models, ignoring" + ) + gpt5_reasoning_effort = None + gpt5_verbosity = None + else: + provider = llm_settings.PROVIDER + model = llm_settings.MODEL + thinking_budget = thinking_budget_tokens + gpt5_reasoning_effort = reasoning_effort + gpt5_verbosity = verbosity + + # Validate client exists + client = CLIENTS.get(provider) + if not client: + raise ValueError(f"Missing client for {provider}") + + if stream: + return await honcho_llm_call_inner( + provider, + model, + prompt, + max_tokens, + response_model, + json_mode, + stop_seqs, + gpt5_reasoning_effort, + gpt5_verbosity, + thinking_budget, + True, # type: ignore[arg-type] + ) + else: + return await honcho_llm_call_inner( + provider, + model, + prompt, + max_tokens, + response_model, + json_mode, + stop_seqs, + gpt5_reasoning_effort, + gpt5_verbosity, + thinking_budget, + False, # type: ignore[arg-type] ) - reasoning_effort = None - verbosity = None - else: - provider = llm_settings.PROVIDER - model = llm_settings.MODEL - # Validate client exists - client = CLIENTS.get(provider) - if not client: - raise ValueError(f"Missing client for {provider}") - - decorated = honcho_llm_call_inner + decorated = _call_with_provider_selection # apply tracking if track_name: @@ -279,37 +318,7 @@ async def honcho_llm_call( before_sleep=before_retry_callback, )(decorated) - # Reset attempt counter at start - _current_attempt.set(0) - - if stream: - return await decorated( - provider, - model, - prompt, - max_tokens, - response_model, - json_mode, - stop_seqs, - reasoning_effort, - verbosity, - thinking_budget_tokens, - True, - ) - else: - return await decorated( - provider, - model, - prompt, - max_tokens, - response_model, - json_mode, - stop_seqs, - reasoning_effort, - verbosity, - thinking_budget_tokens, - False, - ) + return await decorated() @overload