mirror of
https://github.com/vegu-ai/talemate.git
synced 2026-08-29 10:08:58 +02:00
* fix(google): stop 'Setup incomplete' sentinel leaking into model name (#40) * docs(google): collapse duplicated model-heal comment per review
This commit is contained in:
@@ -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."
|
||||
|
||||
@@ -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:
|
||||
|
||||
148
tests/test_client_google.py
Normal file
148
tests/test_client_google.py
Normal file
@@ -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")
|
||||
Reference in New Issue
Block a user