Refactor config loading and env handling

- Extract environment helpers (_get_required_env, _get_optional_env, _get_choice_env)
  and path/config helpers (_load_env_from_locations, _try_load_dotenv_from_path,
  _get_validated_path, _get_azure_config, _get_ollama_config).
- Standardize .env lookup (priority: CWD/.env, XDG config dir, fallback to load_dotenv).
- Centralize loading and validation of DIARY_PATH and PLANNER_PATH with clearer error
  messages and optional validation toggle for tests.
- Consolidate LLM provider handling and provider-specific config population (azure vs
  ollama), and normalize defaults.
- Use helper for BRAIN_LOG_LEVEL retrieval.

Author: madebygps

brain_core/config.py

@@ -1,12 +1,199 @@
 """Configuration management for diary system."""
 
 import os
+from collections.abc import Callable
 from functools import lru_cache
 from pathlib import Path
 
 from dotenv import load_dotenv
 
 
+def _get_required_env(key: str, error_context: str = "") -> str:
+    """Get required environment variable or raise clear error.
+
+    Args:
+        key: Environment variable name
+        error_context: Additional context for error message
+
+    Returns:
+        Value of environment variable
+
+    Raises:
+        ValueError: If environment variable is not set
+    """
+    value = os.getenv(key)
+    if not value:
+        error_msg = f"{key} must be set in .env"
+        if error_context:
+            error_msg += f" {error_context}"
+        raise ValueError(error_msg)
+    return value
+
+
+def _get_optional_env(key: str, default: str) -> str:
+    """Get optional environment variable with default.
+
+    Args:
+        key: Environment variable name
+        default: Default value if not set
+
+    Returns:
+        Value of environment variable or default
+    """
+    return os.getenv(key, default)
+
+
+def _get_choice_env(
+    key: str,
+    allowed_choices: list[str],
+    default: str,
+    transform: Callable[[str], str] = str.lower,
+) -> str:
+    """Get environment variable and validate against allowed choices.
+
+    Args:
+        key: Environment variable name
+        allowed_choices: List of valid values
+        default: Default value if not set
+        transform: Optional transformation function (e.g., str.lower)
+
+    Returns:
+        Validated and transformed value
+
+    Raises:
+        ValueError: If value is not in allowed choices
+    """
+    value = transform(os.getenv(key, default))
+    if value not in allowed_choices:
+        choices_str = "', '".join(allowed_choices)
+        raise ValueError(f"{key} must be one of ['{choices_str}'], got: {value}")
+    return value
+
+
+def _load_env_from_locations(env_file: Path | None = None) -> bool:
+    """Load environment variables from .env file in standard locations.
+
+    Args:
+        env_file: Optional explicit path to .env file
+
+    Returns:
+        True if .env file was found and loaded, False otherwise
+    """
+    if env_file:
+        load_dotenv(env_file)
+        return True
+
+    # Search for .env in standard locations (in priority order)
+    env_locations = [
+        Path.cwd() / ".env",  # Current directory (highest priority)
+        Path.home() / ".config" / "brain" / ".env",  # XDG config dir
+    ]
+
+    for env_path in env_locations:
+        if _try_load_dotenv_from_path(env_path):
+            return True
+
+    # Try default load_dotenv() which searches up the directory tree
+    load_dotenv()
+    return False  # Unknown if found, but we tried
+
+
+def _try_load_dotenv_from_path(env_path: Path) -> bool:
+    """Attempt to load .env from a specific path.
+
+    Args:
+        env_path: Path to .env file
+
+    Returns:
+        True if file was loaded successfully, False otherwise
+    """
+    try:
+        resolved_path = env_path.resolve(strict=False)
+        # Security: Only load if file is readable and is a regular file
+        if resolved_path.exists() and resolved_path.is_file():
+            load_dotenv(resolved_path)
+            return True
+    except (OSError, RuntimeError):
+        # Skip paths that can't be resolved (permission issues, etc.)
+        pass
+    return False
+
+
+def _get_validated_path(key: str, validate: bool = True) -> Path:
+    """Get path from environment and optionally validate it exists.
+
+    Args:
+        key: Environment variable name
+        validate: If True, validate that path exists
+
+    Returns:
+        Path object
+
+    Raises:
+        ValueError: If path is not set or doesn't exist (when validate=True)
+    """
+    path_str = os.getenv(key)
+    if not path_str:
+        if key == "DIARY_PATH":
+            raise ValueError(
+                "DIARY_PATH must be set in .env file.\n"
+                "Create .env file in one of these locations:\n"
+                f"  - {Path.home() / '.config' / 'brain' / '.env'} (recommended)\n"
+                f"  - {Path.home() / '.brain' / '.env'}\n"
+                f"  - {Path.cwd() / '.env'}\n"
+                "See SETUP_CHECKLIST.md for configuration guide."
+            )
+        raise ValueError(f"{key} must be set in .env")
+
+    path = Path(path_str)
+    if validate and not path.exists():
+        raise ValueError(f"{key} does not exist: {path}")
+    return path
+
+
+def _get_azure_config() -> dict[str, str]:
+    """Get Azure OpenAI configuration from environment.
+
+    Returns:
+        Dictionary with Azure config keys
+
+    Raises:
+        ValueError: If required Azure credentials are missing
+    """
+    api_key = _get_required_env(
+        "AZURE_OPENAI_API_KEY",
+        "when LLM_PROVIDER=azure",
+    )
+    endpoint = _get_required_env(
+        "AZURE_OPENAI_ENDPOINT",
+        "when LLM_PROVIDER=azure",
+    )
+    deployment = _get_optional_env("AZURE_OPENAI_DEPLOYMENT", "gpt-4o")
+    api_version = _get_optional_env("AZURE_OPENAI_API_VERSION", "2024-02-15-preview")
+
+    return {
+        "api_key": api_key,
+        "endpoint": endpoint,
+        "deployment": deployment,
+        "api_version": api_version,
+    }
+
+
+def _get_ollama_config() -> dict[str, str]:
+    """Get Ollama configuration from environment.
+
+    Returns:
+        Dictionary with Ollama config keys
+    """
+    base_url = _get_optional_env("OLLAMA_BASE_URL", "http://localhost:11434")
+    model = _get_optional_env("OLLAMA_MODEL", "llama3.1")
+
+    return {
+        "base_url": base_url,
+        "model": model,
+    }
+
+
 class Config:
     """Configuration loaded from .env file.
 
@@ -38,98 +225,45 @@ def __init__(self, env_file: Path | None = None, validate_paths: bool = True):
             env_file: Optional path to .env file. If None, searches standard locations.
             validate_paths: If True, validates that paths exist. Set to False for testing.
         """
-        if env_file:
-            load_dotenv(env_file)
-        else:
-            # Search for .env in standard locations (in priority order)
-            env_locations = [
-                Path.cwd() / ".env",  # Current directory (highest priority)
-                Path.home() / ".config" / "brain" / ".env",  # XDG config dir
-            ]
-
-            env_found = False
-            for env_path in env_locations:
-                # Resolve symlinks and check if file exists
-                try:
-                    resolved_path = env_path.resolve(strict=False)
-                    # Security: Only load if file is readable and is a regular file
-                    if resolved_path.exists() and resolved_path.is_file():
-                        load_dotenv(resolved_path)
-                        env_found = True
-                        break
-                except (OSError, RuntimeError):
-                    # Skip paths that can't be resolved (permission issues, etc.)
-                    continue
-
-            if not env_found:
-                # Try default load_dotenv() which searches up the directory tree
-                load_dotenv()
-
-        # Required paths
-        diary_path = os.getenv("DIARY_PATH")
-        if not diary_path:
-            raise ValueError(
-                "DIARY_PATH must be set in .env file.\n"
-                "Create .env file in one of these locations:\n"
-                f"  - {Path.home() / '.config' / 'brain' / '.env'} (recommended)\n"
-                f"  - {Path.home() / '.brain' / '.env'}\n"
-                f"  - {Path.cwd() / '.env'}\n"
-                "See SETUP_CHECKLIST.md for configuration guide."
-            )
-
-        planner_path = os.getenv("PLANNER_PATH")
-        if not planner_path:
-            raise ValueError("PLANNER_PATH must be set in .env")
-
-        self.diary_path = Path(diary_path)
-        self.planner_path = Path(planner_path)
-
-        # Validate paths exist (can be disabled for testing)
-        if validate_paths:
-            if not self.diary_path.exists():
-                raise ValueError(f"DIARY_PATH does not exist: {self.diary_path}")
-            if not self.planner_path.exists():
-                raise ValueError(f"PLANNER_PATH does not exist: {self.planner_path}")
-
-        # LLM provider configuration
-        self.llm_provider = os.getenv("LLM_PROVIDER", "azure").lower()
-        if self.llm_provider not in ["azure", "ollama"]:
-            raise ValueError(f"LLM_PROVIDER must be 'azure' or 'ollama', got: {self.llm_provider}")
+        # Load environment variables from .env file
+        _load_env_from_locations(env_file)
+
+        # Load required paths
+        self.diary_path = _get_validated_path("DIARY_PATH", validate=validate_paths)
+        self.planner_path = _get_validated_path("PLANNER_PATH", validate=validate_paths)
+
+        # Load LLM provider configuration
+        self.llm_provider = _get_choice_env(
+            "LLM_PROVIDER",
+            allowed_choices=["azure", "ollama"],
+            default="azure",
+            transform=str.lower,
+        )
 
-        # Azure OpenAI configuration (required if using Azure)
+        # Load provider-specific configuration
         if self.llm_provider == "azure":
-            self.azure_api_key = os.getenv("AZURE_OPENAI_API_KEY")
-            if not self.azure_api_key:
-                raise ValueError("AZURE_OPENAI_API_KEY must be set in .env when LLM_PROVIDER=azure")
-
-            self.azure_endpoint = os.getenv("AZURE_OPENAI_ENDPOINT")
-            if not self.azure_endpoint:
-                raise ValueError(
-                    "AZURE_OPENAI_ENDPOINT must be set in .env when LLM_PROVIDER=azure"
-                )
-
-            self.azure_deployment = os.getenv("AZURE_OPENAI_DEPLOYMENT", "gpt-4o")
-            self.azure_api_version = os.getenv("AZURE_OPENAI_API_VERSION", "2024-02-15-preview")
+            azure_config = _get_azure_config()
+            self.azure_api_key = azure_config["api_key"]
+            self.azure_endpoint = azure_config["endpoint"]
+            self.azure_deployment = azure_config["deployment"]
+            self.azure_api_version = azure_config["api_version"]
+            # Set Ollama to None when using Azure
+            self.ollama_base_url = None
+            self.ollama_model = None
         else:
-            # Set defaults for Azure when using Ollama (for optional Azure features)
+            # Using Ollama
+            ollama_config = _get_ollama_config()
+            self.ollama_base_url = ollama_config["base_url"]
+            self.ollama_model = ollama_config["model"]
+            # Set Azure to None when using Ollama
             self.azure_api_key = None
             self.azure_endpoint = None
             self.azure_deployment = None
-            self.azure_api_version = None
+            self.azure_api_version = "2025-01-01-preview"  # Keep default for consistency
 
-        # Ollama configuration (required if using Ollama)
-        if self.llm_provider == "ollama":
-            self.ollama_base_url = os.getenv("OLLAMA_BASE_URL", "http://localhost:11434")
-            self.ollama_model = os.getenv("OLLAMA_MODEL", "llama3.1")
-        else:
-            self.ollama_base_url = None
-            self.ollama_model = None
-
-        # Cost tracking configuration (optional)
+        # Load optional configuration
         self.cost_db_path = os.getenv("BRAIN_COST_DB_PATH")  # Default handled in CostTracker
-
-        # Logging configuration (optional)
-        self.log_level = os.getenv("BRAIN_LOG_LEVEL", "INFO")
+        self.log_level = _get_optional_env("BRAIN_LOG_LEVEL", "INFO")
         self.log_file = os.getenv("BRAIN_LOG_FILE")  # Optional file logging