diff --git a/scripts/compare_prefix_cache.py b/scripts/compare_prefix_cache.py index fb72c83a..1c075113 100644 --- a/scripts/compare_prefix_cache.py +++ b/scripts/compare_prefix_cache.py @@ -216,6 +216,18 @@ def parse_provider_spec(raw: str) -> ProviderSpec: ) label, provider_model = raw.split("=", 1) provider, model = provider_model.split(":", 1) + if not label: + raise ValueError( + f"Invalid provider spec {raw!r}. Label must not be empty." + ) + if not provider: + raise ValueError( + f"Invalid provider spec {raw!r}. Provider must not be empty." + ) + if not model: + raise ValueError( + f"Invalid provider spec {raw!r}. Model must not be empty." + ) return ProviderSpec(label=label, provider=provider, model=model) diff --git a/src/deriver/prompts.py b/src/deriver/prompts.py index b3b0ffcd..4d453066 100644 --- a/src/deriver/prompts.py +++ b/src/deriver/prompts.py @@ -20,6 +20,7 @@ Analyze messages from {peer_id} to extract **explicit atomic facts** about them. [EXPLICIT] DEFINITION: Facts about {peer_id} that can be derived directly from their messages. - Transform statements into one or multiple conclusions - Each conclusion must be self-contained with enough context + - Do not infer unstated background facts or implications - Use absolute dates/times when possible (e.g. "June 26, 2025" not "yesterday") RULES: @@ -29,9 +30,9 @@ RULES: - Contextualize each observation sufficiently (e.g. "Ann is nervous about the job interview at the pharmacy" not just "Ann is nervous") EXAMPLES: -- EXPLICIT: "I just had my 25th birthday last Saturday" → "{peer_id} is 25 years old", "{peer_id}'s birthday is June 21st" -- EXPLICIT: "I took my dog for a walk in NYC" → "{peer_id} has a dog", "{peer_id} lives in NYC" -- EXPLICIT: "{peer_id} attended college" + general knowledge → "{peer_id} completed high school or equivalent" +- EXPLICIT: "I just had my 25th birthday last Saturday" → "{peer_id} is 25 years old", "{peer_id} celebrated their birthday last Saturday" +- EXPLICIT: "I took my dog for a walk in NYC" → "{peer_id} has a dog", "{peer_id} said they took their dog for a walk in NYC" +- EXPLICIT: "{peer_id} attended college" → "{peer_id} said they attended college" """ ) @@ -78,5 +79,5 @@ def estimate_minimal_deriver_prompt_tokens() -> int: ] ) return estimate_tokens(prompt) - except Exception: + except ValueError: return 300 diff --git a/src/utils/clients.py b/src/utils/clients.py index 80d4609b..31ae3013 100644 --- a/src/utils/clients.py +++ b/src/utils/clients.py @@ -137,6 +137,12 @@ def _build_gemini_contents_from_messages( if role == "system": if isinstance(msg.get("content"), str): system_instruction_parts.append(msg["content"]) + elif isinstance(msg.get("content"), list): + for block in cast(list[dict[str, Any]], msg["content"]): + if block.get("type") == "text" and isinstance( + block.get("text"), str + ): + system_instruction_parts.append(block["text"]) continue if role == "assistant": @@ -1535,6 +1541,7 @@ async def honcho_llm_call( True, # type: ignore[arg-type] converted_tools, tool_choice, + messages, ) else: return await honcho_llm_call_inner( @@ -1552,6 +1559,7 @@ async def honcho_llm_call( False, # type: ignore[arg-type] converted_tools, tool_choice, + messages, ) decorated = _call_with_provider_selection diff --git a/tests/utils/test_clients.py b/tests/utils/test_clients.py index 565a6100..1efe5223 100644 --- a/tests/utils/test_clients.py +++ b/tests/utils/test_clients.py @@ -101,6 +101,22 @@ class TestLLMCallResponse: assert cache_creation == 0 assert cache_read == 321 + def test_extract_gemini_cache_tokens_none_metadata(self): + """Missing Gemini usage metadata should return zero cache tokens.""" + cache_creation, cache_read = extract_gemini_cache_tokens(None) + + assert cache_creation == 0 + assert cache_read == 0 + + def test_extract_gemini_cache_tokens_missing_attribute(self): + """Gemini usage metadata without the cached token field should return zeroes.""" + usage_metadata = Mock(spec=[]) + + cache_creation, cache_read = extract_gemini_cache_tokens(usage_metadata) + + assert cache_creation == 0 + assert cache_read == 0 + @pytest.mark.asyncio class TestAnthropicClient: @@ -836,6 +852,59 @@ class TestGoogleClient: assert config["response_schema"] == PromptRepresentation assert config["response_mime_type"] == "application/json" + async def test_google_preserves_list_form_system_messages(self): + """Gemini should preserve text blocks from list-form system content.""" + from google import genai + + mock_client = Mock(spec=genai.Client) + mock_response = Mock() + mock_part = Mock() + mock_part.text = "ok" + mock_part.function_call = None + mock_content = Mock() + mock_content.parts = [mock_part] + mock_finish_reason = Mock() + mock_finish_reason.name = "STOP" + mock_candidate = Mock() + mock_candidate.content = mock_content + mock_candidate.finish_reason = mock_finish_reason + mock_response.candidates = [mock_candidate] + mock_usage_metadata = Mock() + mock_usage_metadata.prompt_token_count = 10 + mock_usage_metadata.candidates_token_count = 5 + mock_response.usage_metadata = mock_usage_metadata + mock_aio = Mock() + mock_aio.models.generate_content = AsyncMock(return_value=mock_response) + mock_client.aio = mock_aio + + with patch.dict(CLIENTS, {"google": mock_client}): + await honcho_llm_call_inner( + provider="google", + model="gemini-1.5-pro", + prompt="ignored", + max_tokens=100, + messages=[ + { + "role": "system", + "content": [ + {"type": "text", "text": "stable instructions"}, + { + "type": "text", + "text": "rolling session context", + "cache_control": {"type": "ephemeral"}, + }, + ], + }, + {"role": "user", "content": "Hello"}, + ], + ) + + call_args = mock_aio.models.generate_content.call_args + assert call_args is not None + assert call_args.kwargs["config"]["system_instruction"] == ( + "stable instructions\n\nrolling session context" + ) + async def test_google_streaming(self): """Test Google/Gemini streaming response""" from google import genai