diff --git a/openspace/llm_factory.py b/openspace/llm_factory.py new file mode 100644 index 00000000..010687a3 --- /dev/null +++ b/openspace/llm_factory.py @@ -0,0 +1,67 @@ +"""LLMFactory — centralized LLM client creation from config. + +Extracted from OpenSpace.initialize() in Epic 4.5. Owns: + • Main LLM client creation with full config + • Optional tool retrieval LLM creation (shares credentials via llm_kwargs) +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING, Optional + +from openspace.llm import LLMClient +from openspace.utils.logging import Logger + +if TYPE_CHECKING: + from openspace.tool_layer import OpenSpaceConfig + +logger = Logger.get_logger(__name__) + + +class LLMFactory: + """Creates LLM clients from OpenSpaceConfig.""" + + def __init__(self, *, config: OpenSpaceConfig) -> None: + self._config = config + self._llm_client: Optional[LLMClient] = None + self._tool_retrieval_llm: Optional[LLMClient] = None + + @property + def llm_client(self) -> Optional[LLMClient]: + return self._llm_client + + @property + def tool_retrieval_llm(self) -> Optional[LLMClient]: + return self._tool_retrieval_llm + + def create_main(self) -> LLMClient: + """Create the primary LLM client from config.""" + self._llm_client = LLMClient( + model=self._config.llm_model, + enable_thinking=self._config.llm_enable_thinking, + rate_limit_delay=self._config.llm_rate_limit_delay, + max_retries=self._config.llm_max_retries, + timeout=self._config.llm_timeout, + **self._config.llm_kwargs, + ) + return self._llm_client + + def create_tool_retrieval(self) -> Optional[LLMClient]: + """Create optional tool retrieval LLM. Returns None if not configured. + + Inherits llm_kwargs (api_key, api_base, etc.) so credentials + from the host agent are shared across all internal LLM clients. + + Note: intentionally omits ``enable_thinking`` and ``rate_limit_delay`` + since tool retrieval is a simple selection task, not a reasoning task. + """ + if not self._config.tool_retrieval_model: + return None + + self._tool_retrieval_llm = LLMClient( + model=self._config.tool_retrieval_model, + timeout=self._config.llm_timeout, + max_retries=self._config.llm_max_retries, + **self._config.llm_kwargs, + ) + return self._tool_retrieval_llm diff --git a/openspace/tool_layer.py b/openspace/tool_layer.py index e6ec96c4..06b2d966 100644 --- a/openspace/tool_layer.py +++ b/openspace/tool_layer.py @@ -13,6 +13,7 @@ from openspace.grounding.core.grounding_client import GroundingClient from openspace.execution_engine import ExecutionEngine from openspace.llm import LLMClient +from openspace.llm_factory import LLMFactory from openspace.recording_service import RecordingService from openspace.skill_engine import ExecutionAnalyzer, SkillRegistry, SkillStore from openspace.skill_engine.evolver import SkillEvolver @@ -125,6 +126,7 @@ def __init__( self._container = container or AppContainer() self._llm_client: Optional[LLMClient] = None + self._llm_factory: Optional[LLMFactory] = None self._grounding_client: Optional[GroundingClient] = None self._grounding_config = None # GroundingConfig reference for skill settings self._grounding_agent: Optional[GroundingAgent] = None @@ -205,14 +207,8 @@ async def initialize(self) -> None: logger.info("Initializing OpenSpace...") try: - self._llm_client = LLMClient( - model=self.config.llm_model, - enable_thinking=self.config.llm_enable_thinking, - rate_limit_delay=self.config.llm_rate_limit_delay, - max_retries=self.config.llm_max_retries, - timeout=self.config.llm_timeout, - **self.config.llm_kwargs, - ) + self._llm_factory = LLMFactory(config=self.config) + self._llm_client = self._llm_factory.create_main() logger.info(f"✓ LLM Client: {self.config.llm_model}") # Load grounding config @@ -300,16 +296,8 @@ async def initialize(self) -> None: logger.info(f"✓ Recording enabled: {len(self._recording_manager.backends or [])} backends") # Create separate LLM client for tool retrieval if configured - # Inherits llm_kwargs (api_key, api_base, etc.) so credentials - # from the host agent are shared across all internal LLM clients. - tool_retrieval_llm = None - if self.config.tool_retrieval_model: - tool_retrieval_llm = LLMClient( - model=self.config.tool_retrieval_model, - timeout=self.config.llm_timeout, - max_retries=self.config.llm_max_retries, - **self.config.llm_kwargs, - ) + tool_retrieval_llm = self._llm_factory.create_tool_retrieval() + if tool_retrieval_llm: logger.info(f"✓ Tool retrieval LLM: {self.config.tool_retrieval_model}") self._grounding_agent = GroundingAgent( diff --git a/tests/test_llm_factory.py b/tests/test_llm_factory.py new file mode 100644 index 00000000..74f6738d --- /dev/null +++ b/tests/test_llm_factory.py @@ -0,0 +1,217 @@ +"""Tests for LLMFactory — extracted LLM client creation from OpenSpace.""" + +from __future__ import annotations + +import pytest +from unittest.mock import MagicMock, patch, call + +try: + from openspace.tool_layer import OpenSpace, OpenSpaceConfig + from openspace.llm_factory import LLMFactory + + _HAS_TOOL_LAYER = True +except Exception: + _HAS_TOOL_LAYER = False + +pytestmark = pytest.mark.skipif(not _HAS_TOOL_LAYER, reason="tool_layer deps unavailable") + + +# --------------------------------------------------------------------------- +# Fixtures +# --------------------------------------------------------------------------- + +@pytest.fixture +def config(): + return OpenSpaceConfig( + llm_model="openrouter/anthropic/claude-sonnet-4.5", + llm_enable_thinking=True, + llm_timeout=60.0, + llm_max_retries=5, + llm_rate_limit_delay=0.5, + llm_kwargs={"api_key": "sk-test"}, + tool_retrieval_model="openrouter/openai/gpt-4o", + ) + + +@pytest.fixture +def minimal_config(): + return OpenSpaceConfig() + + +# --------------------------------------------------------------------------- +# LLMFactory.__init__ +# --------------------------------------------------------------------------- + +class TestLLMFactoryInit: + + def test_initial_state(self, config): + factory = LLMFactory(config=config) + assert factory.llm_client is None + assert factory.tool_retrieval_llm is None + + def test_stores_config(self, config): + factory = LLMFactory(config=config) + assert factory._config is config + + +# --------------------------------------------------------------------------- +# LLMFactory.create_main() +# --------------------------------------------------------------------------- + +class TestCreateMain: + + def test_creates_llm_client(self, config): + with patch("openspace.llm_factory.LLMClient") as MockLLM: + mock_client = MagicMock() + MockLLM.return_value = mock_client + + factory = LLMFactory(config=config) + result = factory.create_main() + + assert result is mock_client + assert factory.llm_client is mock_client + + def test_passes_all_config_fields(self, config): + with patch("openspace.llm_factory.LLMClient") as MockLLM: + MockLLM.return_value = MagicMock() + + factory = LLMFactory(config=config) + factory.create_main() + + MockLLM.assert_called_once_with( + model="openrouter/anthropic/claude-sonnet-4.5", + enable_thinking=True, + rate_limit_delay=0.5, + max_retries=5, + timeout=60.0, + api_key="sk-test", + ) + + def test_uses_defaults_when_minimal_config(self, minimal_config): + with patch("openspace.llm_factory.LLMClient") as MockLLM: + MockLLM.return_value = MagicMock() + + factory = LLMFactory(config=minimal_config) + factory.create_main() + + MockLLM.assert_called_once_with( + model="openrouter/anthropic/claude-sonnet-4.5", + enable_thinking=False, + rate_limit_delay=0.0, + max_retries=3, + timeout=120.0, + ) + + def test_create_main_twice_replaces_client(self, config): + with patch("openspace.llm_factory.LLMClient") as MockLLM: + first = MagicMock() + second = MagicMock() + MockLLM.side_effect = [first, second] + + factory = LLMFactory(config=config) + factory.create_main() + assert factory.llm_client is first + factory.create_main() + assert factory.llm_client is second + + +# --------------------------------------------------------------------------- +# LLMFactory.create_tool_retrieval() +# --------------------------------------------------------------------------- + +class TestCreateToolRetrieval: + + def test_creates_when_model_configured(self, config): + with patch("openspace.llm_factory.LLMClient") as MockLLM: + mock_client = MagicMock() + MockLLM.return_value = mock_client + + factory = LLMFactory(config=config) + result = factory.create_tool_retrieval() + + assert result is mock_client + assert factory.tool_retrieval_llm is mock_client + + def test_returns_none_when_no_model(self, minimal_config): + factory = LLMFactory(config=minimal_config) + result = factory.create_tool_retrieval() + assert result is None + assert factory.tool_retrieval_llm is None + + def test_passes_correct_config(self, config): + with patch("openspace.llm_factory.LLMClient") as MockLLM: + MockLLM.return_value = MagicMock() + + factory = LLMFactory(config=config) + factory.create_tool_retrieval() + + MockLLM.assert_called_once_with( + model="openrouter/openai/gpt-4o", + timeout=60.0, + max_retries=5, + api_key="sk-test", + ) + + def test_inherits_llm_kwargs(self, config): + """Tool retrieval LLM inherits credentials from llm_kwargs.""" + config.llm_kwargs = {"api_key": "sk-shared", "api_base": "https://custom"} + with patch("openspace.llm_factory.LLMClient") as MockLLM: + MockLLM.return_value = MagicMock() + + factory = LLMFactory(config=config) + factory.create_tool_retrieval() + + kwargs = MockLLM.call_args.kwargs + assert kwargs["api_key"] == "sk-shared" + assert kwargs["api_base"] == "https://custom" + + def test_returns_none_when_empty_string_model(self, config): + """Empty string model treated as unconfigured.""" + config.tool_retrieval_model = "" + factory = LLMFactory(config=config) + result = factory.create_tool_retrieval() + assert result is None + assert factory.tool_retrieval_llm is None + + def test_create_tool_retrieval_twice_replaces(self, config): + """Second call replaces the tool retrieval client.""" + with patch("openspace.llm_factory.LLMClient") as MockLLM: + first, second = MagicMock(), MagicMock() + MockLLM.side_effect = [first, second] + + factory = LLMFactory(config=config) + factory.create_tool_retrieval() + assert factory.tool_retrieval_llm is first + factory.create_tool_retrieval() + assert factory.tool_retrieval_llm is second + + +# --------------------------------------------------------------------------- +# Error handling +# --------------------------------------------------------------------------- + +class TestErrorHandling: + + def test_create_main_exception_propagates(self, config): + """LLMClient constructor failure propagates, client stays None.""" + with patch("openspace.llm_factory.LLMClient", side_effect=RuntimeError("boom")): + factory = LLMFactory(config=config) + with pytest.raises(RuntimeError, match="boom"): + factory.create_main() + assert factory.llm_client is None + + +# --------------------------------------------------------------------------- +# OpenSpace backward compatibility +# --------------------------------------------------------------------------- + +class TestOpenSpaceDelegation: + + def test_openspace_has_llm_factory_attr(self): + os_instance = OpenSpace() + assert hasattr(os_instance, "_llm_factory") + + def test_llm_client_still_accessible(self): + os_instance = OpenSpace() + assert hasattr(os_instance, "_llm_client") + assert os_instance._llm_client is None