Fix remaining prefix cache review issues
This commit is contained in:
parent
4ceec9f778
commit
56924eb956
|
|
@ -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)
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Reference in New Issue