diff --git a/artemis/agents/flash/summarizer.py b/artemis/agents/flash/summarizer.py index 56c5e098..d8b6a1a4 100644 --- a/artemis/agents/flash/summarizer.py +++ b/artemis/agents/flash/summarizer.py @@ -36,7 +36,7 @@ from artemis.context import ArtemisContext from artemis.memory.step_memory import JobKey, StepMemoryService -from artemis.services.llm import RobustChatModelWrapper, get_google_llm, get_llm +from artemis.services.llm import RobustChatModelWrapper, get_llm, get_node_model from artemis.services.token_meter import record_llm_usage from artemis.utils.logger import get_logger from artemis.utils.task_tree import format_actions_clean @@ -166,11 +166,15 @@ def __init__( self._model_name = target_model try: if model_name: - self._llm = get_google_llm(model_name=target_model, temperature=0.0) + self._llm = get_node_model( + ctx, "summarizer", model_name=target_model, is_utils=True, temperature=0.0 + ) else: self._llm = get_llm(ctx, name="summarizer", is_utils=True) except Exception: - self._llm = get_google_llm(model_name=target_model, temperature=0.0) + self._llm = get_node_model( + ctx, "summarizer", model_name=target_model, is_utils=True, temperature=0.0 + ) try: configured = getattr(self._llm, "model", None) or getattr(self._llm, "model_name", None) if isinstance(configured, str) and configured: @@ -341,7 +345,7 @@ def _meter_lens_call(self, response: Any) -> None: """Meter one raw-model lens call as an ``llm_usage`` trace, best-effort. Gateway-wrapped models already meter at the wrapper exit; only the raw - ``get_google_llm`` bypass needs explicit metering here. Lens prompts + ``get_node_model`` bypass needs explicit metering here. Lens prompts are tiny and must not overwrite the session's ``last_prompt_tokens`` (the compaction thresholds' live context base), hence ``update_last_prompt=False``. diff --git a/artemis/memory/chunking.py b/artemis/memory/chunking.py index 4e564540..741e2045 100644 --- a/artemis/memory/chunking.py +++ b/artemis/memory/chunking.py @@ -323,17 +323,23 @@ def __init__( def _get_llm(self): if self._llm is None: - from artemis.services.llm import get_google_llm + from artemis.services.llm import get_node_model - self._llm = get_google_llm(model_name=self._model_name, temperature=0.0) + self._llm = get_node_model( + self._ctx, "summarizer", model_name=self._model_name, is_utils=True, temperature=0.0 + ) return self._llm def _get_fallback_llm(self): if self._fallback_llm is None and self._fallback_model_name: - from artemis.services.llm import get_google_llm - - self._fallback_llm = get_google_llm( - model_name=self._fallback_model_name, temperature=0.0 + from artemis.services.llm import get_node_model + + self._fallback_llm = get_node_model( + self._ctx, + "summarizer", + model_name=self._fallback_model_name, + is_utils=True, + temperature=0.0, ) return self._fallback_llm diff --git a/artemis/services/llm.py b/artemis/services/llm.py index 784e971f..57cfe2ff 100644 --- a/artemis/services/llm.py +++ b/artemis/services/llm.py @@ -906,6 +906,36 @@ async def invoke_llm_with_timeout_message[T]( await asyncio.gather(waiter_task, return_exceptions=True) +def get_node_model( + ctx: ArtemisContext | None, + name: str, + *, + model_name: str, + is_utils: bool = False, + temperature: float | None = None, +) -> BaseChatModel: + """Build a raw model for ``name``'s configured provider, overriding only the model. + + Callers that pin a specific lightweight model still have to honour the + provider the operator configured. ``get_google_llm`` hard-codes + ``ModelProvider.GOOGLE``, so pinning a model there sends an OpenAI-compatible + deployment to the Gemini client and fails on an empty ``GOOGLE_API_KEY``. + + Returns a raw model rather than a ``RobustChatModelWrapper``, matching what + ``get_google_llm`` returned, so callers keep metering their own calls. + + Without a context there is no configuration to read and no provider to + honour, so Google stays the default for that case alone. + """ + if ctx is None: + return get_google_llm(model_name=model_name, temperature=temperature) + endpoint = _resolve_endpoint(ctx, name, is_utils=is_utils) + update: dict[str, Any] = {"model_name": model_name} + if temperature is not None: + update["temperature"] = temperature + return ModelFactory.create_model(endpoint.model_copy(update=update)) + + # Backward compatible factory functions delegating to ModelFactory def get_google_llm( model_name: str = "gemini-3.8-flash", diff --git a/tests/unit/test_llm_node_model.py b/tests/unit/test_llm_node_model.py new file mode 100644 index 00000000..22152e2c --- /dev/null +++ b/tests/unit/test_llm_node_model.py @@ -0,0 +1,107 @@ +"""A pinned lightweight model must still use the operator's configured provider.""" + +from types import SimpleNamespace + +from artemis.llm.router import ModelEndpoint, ModelProvider +from artemis.services import llm as llm_service +from artemis.services.llm import get_node_model + + +def _capture(monkeypatch): + seen: list[ModelEndpoint] = [] + monkeypatch.setattr( + llm_service.ModelFactory, + "create_model", + staticmethod(lambda endpoint: seen.append(endpoint) or SimpleNamespace(endpoint=endpoint)), + ) + return seen + + +def _endpoint(provider: ModelProvider) -> ModelEndpoint: + return ModelEndpoint(provider=provider, model_name="configured-default", temperature=0.7) + + +def test_pinned_model_keeps_the_configured_openai_provider(monkeypatch): + """Issue #138: the Flash summarizer pinned a model and got a Gemini client. + + An OpenAI-compatible deployment then failed on an empty GOOGLE_API_KEY + before the first step ran, even though the provider was configured + correctly. + """ + seen = _capture(monkeypatch) + monkeypatch.setattr( + llm_service, "_resolve_endpoint", lambda *a, **k: _endpoint(ModelProvider.OPENAI) + ) + + get_node_model( + SimpleNamespace(), "summarizer", model_name="my-lite", is_utils=True, temperature=0.0 + ) + + assert len(seen) == 1 + assert seen[0].provider is ModelProvider.OPENAI + assert seen[0].model_name == "my-lite" + assert seen[0].temperature == 0.0 + + +def test_a_google_deployment_is_unaffected(monkeypatch): + seen = _capture(monkeypatch) + monkeypatch.setattr( + llm_service, "_resolve_endpoint", lambda *a, **k: _endpoint(ModelProvider.GOOGLE) + ) + + get_node_model(SimpleNamespace(), "summarizer", model_name="gemini-lite", is_utils=True) + + assert seen[0].provider is ModelProvider.GOOGLE + assert seen[0].model_name == "gemini-lite" + # Temperature is left alone when the caller does not pin one. + assert seen[0].temperature == 0.7 + + +def test_without_a_context_there_is_no_configured_provider_to_honour(monkeypatch): + """No context means no config to read, so Google stays the default there.""" + seen = _capture(monkeypatch) + + get_node_model(None, "summarizer", model_name="gemini-lite", temperature=0.0) + + assert seen[0].provider is ModelProvider.GOOGLE + assert seen[0].model_name == "gemini-lite" + + +def test_the_capsule_lens_builds_its_model_on_the_configured_provider(monkeypatch): + """The same bypass at its real call site, which is what the issue reports. + + StepCapsuleLens pins its own compression model, so before the fix it built + a GOOGLE endpoint no matter what the operator configured. This assertion, + not the import, is the one that fails on an unpatched tree. + """ + from artemis.memory.chunking import StepCapsuleLens + + seen = _capture(monkeypatch) + monkeypatch.setattr( + llm_service, "_resolve_endpoint", lambda *a, **k: _endpoint(ModelProvider.OPENAI) + ) + + lens = StepCapsuleLens(model_name="my-compressor", ctx=SimpleNamespace()) + lens._get_llm() + + assert seen[0].provider is ModelProvider.OPENAI, ( + "a pinned compression model must not force the Gemini client" + ) + assert seen[0].model_name == "my-compressor" + + +def test_the_capsule_lens_fallback_model_follows_the_same_provider(monkeypatch): + from artemis.memory.chunking import StepCapsuleLens + + seen = _capture(monkeypatch) + monkeypatch.setattr( + llm_service, "_resolve_endpoint", lambda *a, **k: _endpoint(ModelProvider.OPENAI) + ) + + lens = StepCapsuleLens( + model_name="my-compressor", fallback_model_name="my-backup", ctx=SimpleNamespace() + ) + lens._get_fallback_llm() + + assert seen[0].provider is ModelProvider.OPENAI + assert seen[0].model_name == "my-backup"