From dadde112128343f59431e567969ce28488b32691 Mon Sep 17 00:00:00 2001 From: veguAI <152010387+vegu-ai-tools@users.noreply.github.com> Date: Mon, 8 Jun 2026 02:17:36 +0300 Subject: [PATCH] fix(google): stop 'Setup incomplete' sentinel leaking into model name (#40) (#41) * fix(google): stop 'Setup incomplete' sentinel leaking into model name (#40) * docs(google): collapse duplicated model-heal comment per review --- CHANGELOG.yaml | 1 + src/talemate/client/google.py | 30 +++++-- tests/test_client_google.py | 148 ++++++++++++++++++++++++++++++++++ 3 files changed, 173 insertions(+), 6 deletions(-) create mode 100644 tests/test_client_google.py diff --git a/CHANGELOG.yaml b/CHANGELOG.yaml index 6a209c39..c5d638dc 100644 --- a/CHANGELOG.yaml +++ b/CHANGELOG.yaml @@ -41,6 +41,7 @@ changes: - "World State Snapshot Cadence: The snapshot now refreshes per individual character turn rather than per full scene round. The default interval was raised from 5 to 10 to keep the effective cadence comparable." fixes: + - "Google Client: Fixed the 'Setup incomplete' status leaking into the client's model name. Removing then re-adding the API key could persist 'Setup incomplete' as the model, leaving the client looking configured while every request failed with HTTP 400. The incomplete-setup state is now reported as an error message without overwriting the model, and a client that already has the bad value persisted is healed to report 'No model loaded' instead of sending the invalid model to Google." - "Anthropic / OpenRouter Clients: Fixed the 'Optimize for Prompt Caching' toggle not actually enabling caching on Anthropic — the required `cache_control` parameter was never sent. It is now sent when the toggle is on, and the OpenRouter client does the same for `anthropic/*` models. Note: enabling caching for Anthropic via OpenRouter forces routing to direct Anthropic." - "Game Loop Event: Fixed an internal scene-loop event being constructed with the wrong scene reference, surfaced by the pydantic migration." - "Conversation Agent: Stopped injecting `#` into the LLM stop-sequence list on every conversation turn." diff --git a/src/talemate/client/google.py b/src/talemate/client/google.py index a73240bd..193c5a79 100644 --- a/src/talemate/client/google.py +++ b/src/talemate/client/google.py @@ -53,6 +53,12 @@ ALWAYS_REASONING_MODELS = [ DEFAULT_MODEL = "gemini-3.5-flash" +# Older versions communicated an incomplete setup by overwriting the client's +# model name with this sentinel. That value could leak back into the persisted +# `model` field, causing valid requests to be sent to Google with an invalid +# model. It is kept here so existing broken configs can be detected and healed. +SETUP_INCOMPLETE_MODEL = "Setup incomplete" + class Defaults(EndpointOverride, CommonDefaults, pydantic.BaseModel): max_token_length: int = 16384 @@ -108,6 +114,14 @@ class GoogleClient( self.google_project_id = None super().__init__(**kwargs) + @property + def model(self) -> str | None: + model = self.client_config.model + # Heal a leaked SETUP_INCOMPLETE_MODEL sentinel by reporting no model selected. + if model == SETUP_INCOMPLETE_MODEL: + return None + return model + @property def disable_safety_settings(self): return self.client_config.disable_safety_settings @@ -259,15 +273,15 @@ class GoogleClient( def emit_status(self, processing: bool = None): error_action = None + error_message: str | None = None if processing is not None: self.processing = processing if self.ready: status = "busy" if self.processing else "idle" - model_name = self.model_name else: status = "error" - model_name = "Setup incomplete" + error_message = "Setup incomplete" error_action = ErrorAction( title="Setup Google API credentials", action_name="openAppConfig", @@ -280,7 +294,7 @@ class GoogleClient( if not self.model_name: status = "error" - model_name = "No model loaded" + error_message = "No model loaded" self.current_status = status data = { @@ -288,14 +302,15 @@ class GoogleClient( "error_action": error_action.model_dump() if error_action else None, "meta": self.Meta().model_dump(), "enabled": self.enabled, + "error_message": error_message, } data.update(self._common_status_data()) self.populate_extra_fields(data) - if self.using == "VertexAI": - details = f"{model_name} (VertexAI)" + if self.model_name and self.using == "VertexAI": + details = f"{self.model_name} (VertexAI)" else: - details = model_name + details = self.model_name emit( "client_status", @@ -356,6 +371,9 @@ class GoogleClient( if not self.ready: raise Exception("Google setup incomplete") + if not self.model_name: + raise Exception("Google client has no model selected") + client = self.make_client() if self.can_be_coerced: diff --git a/tests/test_client_google.py b/tests/test_client_google.py new file mode 100644 index 00000000..dc6a12d9 --- /dev/null +++ b/tests/test_client_google.py @@ -0,0 +1,148 @@ +"""Unit tests for talemate.client.google.GoogleClient. + +Focused on the setup-incomplete status handling for issue #40: + +- The "Setup incomplete" sentinel must not leak into the emitted model name. + The frontend persists the emitted model name back into the client `model` + field, so leaking the sentinel corrupts the config and causes every Google + request to fail with HTTP 400. +- A config that already has the leaked sentinel persisted in its `model` field + must be healed to "no model selected" rather than sent to the API verbatim. +""" + +from __future__ import annotations + +import pytest + +import talemate.config.state as config_state +from talemate.client import google as google_module +from talemate.client.google import ( + ClientConfig, + GoogleClient, + SETUP_INCOMPLETE_MODEL, +) + + +@pytest.fixture +def cfg_isolation(): + """Snapshot/restore the config sections these tests mutate.""" + saved_clients = dict(config_state.CONFIG.clients) + saved_api_key = config_state.CONFIG.google.api_key + saved_creds = config_state.CONFIG.google.gcloud_credentials_path + saved_location = config_state.CONFIG.google.gcloud_location + yield + config_state.CONFIG.clients.clear() + config_state.CONFIG.clients.update(saved_clients) + config_state.CONFIG.google.api_key = saved_api_key + config_state.CONFIG.google.gcloud_credentials_path = saved_creds + config_state.CONFIG.google.gcloud_location = saved_location + + +@pytest.fixture +def no_google_credentials(cfg_isolation): + """Ensure the Google service is treated as not configured.""" + config_state.CONFIG.google.api_key = None + config_state.CONFIG.google.gcloud_credentials_path = None + config_state.CONFIG.google.gcloud_location = None + + +@pytest.fixture +def capture_emit(monkeypatch): + """Capture every emit() call made by the google client module.""" + calls = [] + + def _fake_emit(*args, **kwargs): + calls.append({"args": args, "kwargs": kwargs}) + + monkeypatch.setattr(google_module, "emit", _fake_emit) + return calls + + +def _register(name: str, **kwargs) -> ClientConfig: + cfg = ClientConfig(type="google", name=name, **kwargs) + config_state.CONFIG.clients[name] = cfg + return cfg + + +class TestModelSentinelHealing: + def test_persisted_sentinel_model_resolves_to_none(self, no_google_credentials): + _register("g_healed", model=SETUP_INCOMPLETE_MODEL) + client = GoogleClient(name="g_healed") + # The leaked sentinel must not be treated as a real model. + assert client.model is None + assert client.model_name is None + + def test_real_model_passes_through(self, no_google_credentials): + _register("g_real", model="gemini-3.5-flash") + client = GoogleClient(name="g_real") + assert client.model == "gemini-3.5-flash" + assert client.model_name == "gemini-3.5-flash" + + +class TestEmitStatus: + def test_not_ready_keeps_real_model_name(self, no_google_credentials, capture_emit): + _register("g_not_ready", model="gemini-3.5-flash") + client = GoogleClient(name="g_not_ready") + + client.emit_status() + + assert client.current_status == "error" + emitted = capture_emit[-1]["kwargs"] + # The model name shown to (and persisted by) the frontend must stay the + # real model, never the "Setup incomplete" sentinel. + assert emitted["details"] == "gemini-3.5-flash" + assert emitted["details"] != SETUP_INCOMPLETE_MODEL + # The incomplete-setup state is communicated via error_message instead. + assert emitted["data"]["error_message"] == "Setup incomplete" + assert emitted["data"]["error_action"] is not None + + def test_ready_with_model_is_idle(self, cfg_isolation, capture_emit): + config_state.CONFIG.google.api_key = "test-key" + config_state.CONFIG.google.gcloud_credentials_path = None + config_state.CONFIG.google.gcloud_location = None + _register("g_ready", model="gemini-3.5-flash") + client = GoogleClient(name="g_ready") + + client.emit_status() + + assert client.current_status == "idle" + emitted = capture_emit[-1]["kwargs"] + assert emitted["details"] == "gemini-3.5-flash" + assert emitted["data"]["error_message"] is None + assert emitted["data"]["error_action"] is None + + def test_healed_sentinel_reports_no_model_loaded(self, cfg_isolation, capture_emit): + # Key is set (so `ready` is True) but the persisted model is the leaked + # sentinel. Instead of shipping it to Google, the client must surface a + # clean "No model loaded" state. + config_state.CONFIG.google.api_key = "test-key" + config_state.CONFIG.google.gcloud_credentials_path = None + config_state.CONFIG.google.gcloud_location = None + _register("g_healed_ready", model=SETUP_INCOMPLETE_MODEL) + client = GoogleClient(name="g_healed_ready") + + client.emit_status() + + assert client.current_status == "error" + emitted = capture_emit[-1]["kwargs"] + assert emitted["data"]["error_message"] == "No model loaded" + assert emitted["details"] != SETUP_INCOMPLETE_MODEL + + +class TestGenerateGuards: + @pytest.mark.asyncio + async def test_generate_raises_when_not_ready(self, no_google_credentials): + _register("g_gen_not_ready", model="gemini-3.5-flash") + client = GoogleClient(name="g_gen_not_ready") + with pytest.raises(Exception, match="setup incomplete"): + await client.generate("prompt", {}, "conversation") + + @pytest.mark.asyncio + async def test_generate_raises_when_no_model(self, cfg_isolation): + config_state.CONFIG.google.api_key = "test-key" + config_state.CONFIG.google.gcloud_credentials_path = None + config_state.CONFIG.google.gcloud_location = None + _register("g_gen_no_model", model=SETUP_INCOMPLETE_MODEL) + client = GoogleClient(name="g_gen_no_model") + with pytest.raises(Exception, match="no model selected"): + await client.generate("prompt", {}, "conversation")