Add Stricter limits to Summary & Peer Card (#400)
* fix: Add bounds to gemini client * fix: Prevent empty summaries from being saved to DB (HONCHO-M7) Raise LLMError on blocked Gemini responses (SAFETY, RECITATION, etc.) so retry/backup-provider logic triggers. Treat empty LLM responses in the summarizer as fallback instead of persisting empty strings. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat: Summary Eval via Locomo * fix: Code Rabbit Comments * fix: Code Rabbit Comments --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
5a42f9b3cd
commit
10ef7b96a8
|
|
@ -61,7 +61,9 @@ class BaseSpecialist(ABC):
|
|||
|
||||
name: str = "base"
|
||||
# Subclasses can override to customize the peer card update instruction
|
||||
peer_card_update_instruction: str = "Update this with `update_peer_card` if needed."
|
||||
peer_card_update_instruction: str = (
|
||||
"Only update this with durable profile facts via `update_peer_card`."
|
||||
)
|
||||
|
||||
@abstractmethod
|
||||
def get_tools(self, *, peer_card_enabled: bool = True) -> list[dict[str, Any]]:
|
||||
|
|
@ -108,6 +110,7 @@ class BaseSpecialist(ABC):
|
|||
{facts}
|
||||
|
||||
{self.peer_card_update_instruction}
|
||||
If you update it, send the full deduplicated list and remove stale entries.
|
||||
|
||||
"""
|
||||
|
||||
|
|
@ -289,7 +292,7 @@ class DeductionSpecialist(BaseSpecialist):
|
|||
"""
|
||||
|
||||
name: str = "deduction"
|
||||
peer_card_update_instruction: str = "Update this with `update_peer_card` if you discover new biographical information."
|
||||
peer_card_update_instruction: str = "Update this with `update_peer_card` only for stable biographical/profile facts."
|
||||
|
||||
def get_tools(self, *, peer_card_enabled: bool = True) -> list[dict[str, Any]]:
|
||||
if peer_card_enabled:
|
||||
|
|
@ -324,13 +327,16 @@ The peer card is a summary of stable biographical facts. You MUST update it when
|
|||
- Standing instructions ("call me X", "don't mention Y")
|
||||
- Core preferences and traits
|
||||
|
||||
Never add temporary event summaries, one-off conclusions, reasoning traces, or contradiction notes.
|
||||
|
||||
Format entries as:
|
||||
- Plain facts: "Name: Alice", "Works at Google", "Lives in NYC"
|
||||
- `INSTRUCTION: ...` for standing instructions
|
||||
- `PREFERENCE: ...` for preferences
|
||||
- `TRAIT: ...` for personality traits
|
||||
|
||||
Call `update_peer_card` with the complete updated list when you have new biographical info."""
|
||||
Call `update_peer_card` with the complete updated list when you have new biographical info.
|
||||
Keep it concise (max 40 entries), deduplicated, and current."""
|
||||
|
||||
return f"""You are a deductive reasoning agent analyzing observations about {observed}.
|
||||
|
||||
|
|
@ -429,9 +435,7 @@ class InductionSpecialist(BaseSpecialist):
|
|||
"""
|
||||
|
||||
name: str = "induction"
|
||||
peer_card_update_instruction: str = (
|
||||
"Update this with `update_peer_card` if you identify new patterns or traits."
|
||||
)
|
||||
peer_card_update_instruction: str = "Only add highly stable profile traits/preferences; do not copy transient conclusions."
|
||||
|
||||
def get_tools(self, *, peer_card_enabled: bool = True) -> list[dict[str, Any]]:
|
||||
if peer_card_enabled:
|
||||
|
|
@ -460,12 +464,14 @@ class InductionSpecialist(BaseSpecialist):
|
|||
|
||||
## PEER CARD (REQUIRED)
|
||||
|
||||
After identifying patterns, update the peer card with high-confidence traits and tendencies:
|
||||
After identifying patterns, only update the peer card for durable profile-level traits/preferences:
|
||||
- `TRAIT: Analytical thinker`
|
||||
- `TRAIT: Tends to reschedule when stressed`
|
||||
- `PREFERENCE: Prefers detailed explanations`
|
||||
|
||||
Call `update_peer_card` with the complete list when you identify new patterns."""
|
||||
Do NOT add temporary patterns, episode-specific conclusions, or reasoning summaries.
|
||||
Call `update_peer_card` with the complete deduplicated list only when a durable profile update is warranted.
|
||||
Keep it concise (max 40 entries)."""
|
||||
|
||||
return f"""You are an inductive reasoning agent identifying patterns about {observed}.
|
||||
|
||||
|
|
|
|||
|
|
@ -27,6 +27,9 @@ from src.utils.types import get_current_iteration
|
|||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
# Hard cap to prevent unbounded peer card growth from repeated agent updates.
|
||||
MAX_PEER_CARD_FACTS = 40
|
||||
|
||||
|
||||
def _safe_int(value: Any, default: int) -> int:
|
||||
"""Coerce a tool input value to int, returning default on failure.
|
||||
|
|
@ -247,13 +250,20 @@ TOOLS: dict[str, dict[str, Any]] = {
|
|||
},
|
||||
"update_peer_card": {
|
||||
"name": "update_peer_card",
|
||||
"description": "Update the peer card with facts about the observed peer. The peer card is a summary of key information about the peer.",
|
||||
"description": (
|
||||
"Update the peer card with durable profile facts about the observed peer. "
|
||||
+ "Only include stable biographical facts, standing instructions, and long-lived preferences/traits. "
|
||||
+ "Do not include one-off conclusions, temporary events, or duplicate entries."
|
||||
),
|
||||
"input_schema": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"content": {
|
||||
"type": "array",
|
||||
"description": "List of facts about the peer",
|
||||
"description": (
|
||||
"Complete deduplicated peer card list (max 40 entries). "
|
||||
+ "Each entry should be a concise standalone profile fact."
|
||||
),
|
||||
"items": {"type": "string"},
|
||||
},
|
||||
},
|
||||
|
|
@ -1106,12 +1116,58 @@ async def _handle_update_peer_card(ctx: ToolContext, tool_input: dict[str, Any])
|
|||
"Peer card creation is disabled for this workspace/session configuration."
|
||||
)
|
||||
|
||||
peer_card_content = tool_input["content"]
|
||||
raw_peer_card_content = tool_input.get("content")
|
||||
|
||||
# Guard against None or empty content — keep the existing peer card.
|
||||
if raw_peer_card_content is None:
|
||||
logger.warning(
|
||||
"Peer card update called with None content for %s, keeping existing card",
|
||||
ctx.workspace_name,
|
||||
)
|
||||
return "Peer card content was empty, no update performed."
|
||||
|
||||
# Normalize and deduplicate to keep peer cards bounded and stable.
|
||||
normalized_peer_card: list[str] = []
|
||||
seen: set[str] = set()
|
||||
items = (
|
||||
cast(list[str], raw_peer_card_content)
|
||||
if isinstance(raw_peer_card_content, list)
|
||||
else [str(raw_peer_card_content)]
|
||||
)
|
||||
for item in items:
|
||||
line = str(item).strip()
|
||||
if not line:
|
||||
continue
|
||||
|
||||
# Case-insensitive dedupe with whitespace normalization.
|
||||
normalized_key = " ".join(line.lower().split())
|
||||
if normalized_key in seen:
|
||||
continue
|
||||
seen.add(normalized_key)
|
||||
normalized_peer_card.append(line)
|
||||
|
||||
# Don't clear the peer card if all content normalized to empty.
|
||||
if not normalized_peer_card:
|
||||
logger.warning(
|
||||
"Peer card update normalized to empty for %s, keeping existing card",
|
||||
ctx.workspace_name,
|
||||
)
|
||||
return "Peer card content was empty after normalization, no update performed."
|
||||
|
||||
if len(normalized_peer_card) > MAX_PEER_CARD_FACTS:
|
||||
logger.warning(
|
||||
"Peer card update exceeded max facts (%s), truncating from %s to %s",
|
||||
MAX_PEER_CARD_FACTS,
|
||||
len(normalized_peer_card),
|
||||
MAX_PEER_CARD_FACTS,
|
||||
)
|
||||
normalized_peer_card = normalized_peer_card[:MAX_PEER_CARD_FACTS]
|
||||
|
||||
async with ctx.db_lock:
|
||||
await crud.set_peer_card(
|
||||
ctx.db,
|
||||
workspace_name=ctx.workspace_name,
|
||||
peer_card=peer_card_content,
|
||||
peer_card=normalized_peer_card,
|
||||
observer=ctx.observer,
|
||||
observed=ctx.observed,
|
||||
)
|
||||
|
|
@ -1122,15 +1178,6 @@ async def _handle_update_peer_card(ctx: ToolContext, tool_input: dict[str, Any])
|
|||
# Emit telemetry event if context is available
|
||||
if ctx.run_id and ctx.agent_type and ctx.parent_category:
|
||||
# Count facts in peer card (content is a list of strings per tool schema)
|
||||
facts_count: int
|
||||
if isinstance(peer_card_content, list):
|
||||
content_list = cast(list[str], peer_card_content)
|
||||
facts_count = len([line for line in content_list if line.strip()])
|
||||
else:
|
||||
# Fallback for string (defensive)
|
||||
facts_count = len(
|
||||
[line for line in str(peer_card_content).split("\n") if line.strip()]
|
||||
)
|
||||
emit(
|
||||
AgentToolPeerCardUpdatedEvent(
|
||||
run_id=ctx.run_id,
|
||||
|
|
@ -1140,7 +1187,7 @@ async def _handle_update_peer_card(ctx: ToolContext, tool_input: dict[str, Any])
|
|||
workspace_name=ctx.workspace_name,
|
||||
observer=ctx.observer,
|
||||
observed=ctx.observed,
|
||||
facts_count=facts_count,
|
||||
facts_count=len(normalized_peer_card),
|
||||
)
|
||||
)
|
||||
|
||||
|
|
|
|||
|
|
@ -23,6 +23,7 @@ from sentry_sdk.ai.monitoring import ai_track
|
|||
from tenacity import retry, stop_after_attempt, wait_exponential
|
||||
|
||||
from src.config import LLMComponentSettings, settings
|
||||
from src.exceptions import LLMError
|
||||
from src.telemetry.logging import conditional_observe
|
||||
from src.telemetry.reasoning_traces import log_reasoning_trace
|
||||
from src.utils.json_parser import validate_and_repair_json
|
||||
|
|
@ -32,6 +33,16 @@ from src.utils.types import SupportedProviders, set_current_iteration
|
|||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
# Gemini finish reasons that indicate the response was blocked by safety or policy
|
||||
# filters. When these occur, the response typically has no usable text content and
|
||||
# retrying with a backup provider is appropriate.
|
||||
GEMINI_BLOCKED_FINISH_REASONS = {
|
||||
"SAFETY",
|
||||
"RECITATION",
|
||||
"PROHIBITED_CONTENT",
|
||||
"BLOCKLIST",
|
||||
}
|
||||
|
||||
|
||||
@dataclass
|
||||
class IterationData:
|
||||
|
|
@ -2062,6 +2073,9 @@ async def honcho_llm_call_inner(
|
|||
# Build config for Gemini
|
||||
gemini_config: dict[str, Any] = {}
|
||||
|
||||
# Gemini uses max_output_tokens, not max_tokens.
|
||||
gemini_config["max_output_tokens"] = params["max_tokens"]
|
||||
|
||||
if temperature is not None:
|
||||
gemini_config["temperature"] = temperature
|
||||
|
||||
|
|
@ -2199,6 +2213,19 @@ async def honcho_llm_call_inner(
|
|||
else "stop"
|
||||
)
|
||||
|
||||
# Raise on blocked responses so retry/backup-provider logic kicks in
|
||||
if (
|
||||
not text_content
|
||||
and not gemini_tool_calls
|
||||
and finish_reason in GEMINI_BLOCKED_FINISH_REASONS
|
||||
):
|
||||
raise LLMError(
|
||||
f"Gemini response blocked (finish_reason={finish_reason})",
|
||||
provider="google",
|
||||
model=model,
|
||||
finish_reason=finish_reason,
|
||||
)
|
||||
|
||||
return HonchoLLMCallResponse(
|
||||
content=text_content,
|
||||
input_tokens=input_token_count,
|
||||
|
|
@ -2234,6 +2261,18 @@ async def honcho_llm_call_inner(
|
|||
else "stop"
|
||||
)
|
||||
|
||||
# Raise on blocked responses before checking parsed content
|
||||
if (
|
||||
not gemini_response.parsed
|
||||
and finish_reason in GEMINI_BLOCKED_FINISH_REASONS
|
||||
):
|
||||
raise LLMError(
|
||||
f"Gemini response blocked (finish_reason={finish_reason})",
|
||||
provider="google",
|
||||
model=model,
|
||||
finish_reason=finish_reason,
|
||||
)
|
||||
|
||||
# Validate that parsed content matches the response model
|
||||
if not isinstance(gemini_response.parsed, response_model):
|
||||
raise ValueError(
|
||||
|
|
@ -2449,23 +2488,25 @@ async def handle_streaming_response(
|
|||
|
||||
case genai.Client():
|
||||
prompt_text = params["messages"][0]["content"] if params["messages"] else ""
|
||||
stream_config: GenerateContentConfigDict = {
|
||||
"max_output_tokens": cast(int, params["max_tokens"]),
|
||||
}
|
||||
|
||||
if response_model is not None:
|
||||
stream_config["response_mime_type"] = "application/json"
|
||||
stream_config["response_schema"] = response_model
|
||||
response_stream = await client.aio.models.generate_content_stream(
|
||||
model=params["model"],
|
||||
contents=prompt_text,
|
||||
config={
|
||||
"response_mime_type": "application/json",
|
||||
"response_schema": response_model,
|
||||
},
|
||||
config=stream_config,
|
||||
)
|
||||
else:
|
||||
if json_mode:
|
||||
stream_config["response_mime_type"] = "application/json"
|
||||
response_stream = await client.aio.models.generate_content_stream(
|
||||
model=params["model"],
|
||||
contents=prompt_text,
|
||||
config={
|
||||
"response_mime_type": "application/json" if json_mode else None,
|
||||
},
|
||||
config=stream_config,
|
||||
)
|
||||
|
||||
final_chunk = None
|
||||
|
|
@ -2474,6 +2515,9 @@ async def handle_streaming_response(
|
|||
yield HonchoLLMCallStreamChunk(content=chunk.text)
|
||||
final_chunk = chunk
|
||||
|
||||
# NOTE: Blocked-response check is intentionally omitted for streaming.
|
||||
# Exceptions mid-iteration in an async generator won't be caught by
|
||||
# the tenacity retry wrapper in honcho_llm_call.
|
||||
finish_reason = "stop" # Default fallback
|
||||
gemini_output_tokens: int | None = None
|
||||
if (
|
||||
|
|
|
|||
|
|
@ -119,7 +119,7 @@ Return only the summary without any explanation or meta-commentary.
|
|||
{formatted_messages}
|
||||
</conversation>
|
||||
|
||||
Produce as thorough a summary as possible in {output_words} words or less.
|
||||
Hard limit: {output_words} words maximum. If needed, drop lower-priority detail to stay within the limit.
|
||||
""")
|
||||
|
||||
|
||||
|
|
@ -153,7 +153,7 @@ Return only the summary without any explanation or meta-commentary.
|
|||
{formatted_messages}
|
||||
</conversation>
|
||||
|
||||
Produce as thorough a summary as possible in {output_words} words or less.
|
||||
Hard limit: {output_words} words maximum. If needed, drop lower-priority detail to stay within the limit.
|
||||
""")
|
||||
|
||||
|
||||
|
|
@ -565,8 +565,18 @@ async def _create_summary(
|
|||
# Detect potential issues with the summary
|
||||
if not summary_text.strip():
|
||||
logger.error(
|
||||
"Generated summary is empty! This may indicate a token limit issue."
|
||||
"Generated summary is empty (finish_reasons=%s). Falling back to basic summary.",
|
||||
response.finish_reasons,
|
||||
)
|
||||
is_fallback = True
|
||||
summary_text = (
|
||||
f"Conversation with {message_count} messages about {last_message_content_preview}..."
|
||||
if message_count > 0
|
||||
else ""
|
||||
)
|
||||
summary_tokens = estimate_tokens(summary_text) if summary_text else 0
|
||||
llm_input_tokens = 0
|
||||
llm_output_tokens = 0
|
||||
except Exception:
|
||||
logger.exception("Error generating summary!")
|
||||
# Fallback to a basic summary in case of error
|
||||
|
|
@ -575,7 +585,7 @@ async def _create_summary(
|
|||
if message_count > 0
|
||||
else ""
|
||||
)
|
||||
summary_tokens = 50
|
||||
summary_tokens = 0
|
||||
is_fallback = True
|
||||
|
||||
return (
|
||||
|
|
|
|||
File diff suppressed because it is too large
Load Diff
|
|
@ -0,0 +1,577 @@
|
|||
"""
|
||||
Honcho LoCoMo Summary Evaluation Runner
|
||||
|
||||
Evaluates Honcho's summary-making process by using only session summaries
|
||||
as context for a base model to answer LoCoMo questions.
|
||||
|
||||
This isolates the quality of Honcho's summarization: instead of using the
|
||||
full dialectic agent or raw conversation context, we retrieve the generated
|
||||
summaries and feed them to a base LLM. The resulting scores measure how much
|
||||
information the summaries retain.
|
||||
|
||||
## Comparison points
|
||||
|
||||
- `locomo_baseline.py`: Raw conversation as context (upper bound for the model)
|
||||
- `locomo_summary.py` (this file): Honcho summaries only as context
|
||||
- `locomo.py`: Full Honcho memory system (dialectic agent)
|
||||
|
||||
## To use
|
||||
|
||||
0. Set up env:
|
||||
```
|
||||
uv sync
|
||||
source .venv/bin/activate
|
||||
```
|
||||
|
||||
1. Run the test harness:
|
||||
```
|
||||
python -m tests.bench.harness
|
||||
```
|
||||
|
||||
2. Run this file with the LoCoMo dataset:
|
||||
```
|
||||
python -m tests.bench.locomo_summary --data-file tests/bench/locomo_data/locomo10.json
|
||||
```
|
||||
|
||||
Optional arguments:
|
||||
```
|
||||
--timeout: Timeout for deriver queue to empty in seconds (default: 10 minutes)
|
||||
--base-api-port: Base port for Honcho API instances (default: 8000)
|
||||
--pool-size: Number of Honcho instances in the pool (default: 1)
|
||||
--batch-size: Number of conversations to run concurrently in each batch (default: 1)
|
||||
--json-output: Path to write JSON summary results for analytics
|
||||
--cleanup-workspace: Delete workspace after executing each conversation (default: False)
|
||||
--sample-id: Run only the conversation with this sample_id (skips all others)
|
||||
--test-count: Number of conversations to run (default: all)
|
||||
--question-count: Number of questions per conversation to run (default: all)
|
||||
--model: Model to use for answering questions given summary context (default: anthropic/claude-haiku-4.5)
|
||||
```
|
||||
|
||||
## Other notes
|
||||
- Summaries are enabled (overriding the base runner default) so the deriver generates them
|
||||
- Uses OpenRouter API for the base model (configured via LLM_OPENAI_COMPATIBLE_API_KEY)
|
||||
- Evaluation uses the same GPT-4o-mini judge as the standard LoCoMo runner
|
||||
"""
|
||||
|
||||
import argparse
|
||||
import os
|
||||
import time
|
||||
from datetime import datetime
|
||||
from pathlib import Path
|
||||
from typing import Any
|
||||
|
||||
from dotenv import load_dotenv
|
||||
from honcho.api_types import (
|
||||
MessageCreateParams,
|
||||
SessionConfiguration,
|
||||
SummaryConfiguration,
|
||||
)
|
||||
from honcho.session import SessionPeerConfig
|
||||
from openai import AsyncOpenAI
|
||||
|
||||
from src.config import settings
|
||||
|
||||
from .locomo import format_message_with_image
|
||||
from .locomo_common import (
|
||||
CATEGORY_NAMES,
|
||||
ConversationResult,
|
||||
QuestionResult,
|
||||
calculate_category_scores,
|
||||
calculate_tokens,
|
||||
extract_sessions,
|
||||
filter_questions,
|
||||
generate_json_summary,
|
||||
get_evidence_context,
|
||||
judge_response,
|
||||
load_locomo_data,
|
||||
parse_locomo_date,
|
||||
print_summary,
|
||||
)
|
||||
from .runner_common import (
|
||||
BaseRunner,
|
||||
ItemContext,
|
||||
RunnerConfig,
|
||||
add_common_arguments,
|
||||
create_openai_client,
|
||||
validate_common_arguments,
|
||||
)
|
||||
|
||||
# Load .env from bench directory
|
||||
bench_dir = Path(__file__).parent
|
||||
load_dotenv(bench_dir / ".env")
|
||||
|
||||
# Default model for answering questions given summary context
|
||||
DEFAULT_MODEL = "anthropic/claude-haiku-4.5"
|
||||
|
||||
|
||||
class LoCoMoSummaryRunner(BaseRunner[ConversationResult]):
|
||||
"""
|
||||
Evaluates Honcho summary quality by using only summaries as context
|
||||
for a base model to answer LoCoMo questions.
|
||||
"""
|
||||
|
||||
def __init__(
|
||||
self,
|
||||
config: RunnerConfig,
|
||||
data_file: Path,
|
||||
sample_id: str | None = None,
|
||||
test_count: int | None = None,
|
||||
question_count: int | None = None,
|
||||
model: str = DEFAULT_MODEL,
|
||||
):
|
||||
"""
|
||||
Initialize the LoCoMo summary evaluation runner.
|
||||
|
||||
Args:
|
||||
config: Common runner configuration
|
||||
data_file: Path to the LoCoMo JSON file
|
||||
sample_id: Optional sample_id to run only that conversation
|
||||
test_count: Optional number of conversations to run
|
||||
question_count: Optional limit on questions per conversation
|
||||
model: Model to use for answering questions given summary context
|
||||
"""
|
||||
self.data_file: Path = data_file
|
||||
self.sample_id_filter: str | None = sample_id
|
||||
self.test_count: int | None = test_count
|
||||
self.question_count: int | None = question_count
|
||||
self.model: str = model
|
||||
|
||||
# Initialize base class
|
||||
super().__init__(config)
|
||||
|
||||
# Initialize OpenRouter client for the model under test
|
||||
openrouter_base_url = os.getenv(
|
||||
"LLM_OPENAI_COMPATIBLE_BASE_URL", "https://openrouter.ai/api/v1"
|
||||
)
|
||||
self.openrouter_client: AsyncOpenAI = create_openai_client(
|
||||
base_url=openrouter_base_url,
|
||||
env_key_name="LLM_OPENAI_COMPATIBLE_API_KEY",
|
||||
)
|
||||
|
||||
# Initialize OpenAI client for judging responses
|
||||
self.openai_client: AsyncOpenAI = create_openai_client()
|
||||
|
||||
def get_metrics_prefix(self) -> str:
|
||||
return "locomo_summary"
|
||||
|
||||
def _get_session_configuration(self) -> SessionConfiguration:
|
||||
"""Enable summaries so the deriver generates them during processing."""
|
||||
return SessionConfiguration(summary=SummaryConfiguration(enabled=True))
|
||||
|
||||
def load_items(self) -> list[Any]:
|
||||
"""Load conversations from the data file."""
|
||||
conversations = load_locomo_data(self.data_file)
|
||||
|
||||
# Filter by sample_id if specified
|
||||
if self.sample_id_filter is not None:
|
||||
conversations = [
|
||||
c for c in conversations if c.get("sample_id") == self.sample_id_filter
|
||||
]
|
||||
if not conversations:
|
||||
print(
|
||||
f"Error: No conversation found with sample_id '{self.sample_id_filter}'"
|
||||
)
|
||||
return []
|
||||
print(f"Filtering to sample_id '{self.sample_id_filter}'")
|
||||
|
||||
# Limit by test_count
|
||||
if self.test_count is not None and self.test_count > 0:
|
||||
conversations = conversations[: self.test_count]
|
||||
print(f"Limiting to {len(conversations)} conversations")
|
||||
|
||||
return conversations
|
||||
|
||||
def get_workspace_id(self, item: Any) -> str:
|
||||
"""Return workspace ID for a conversation."""
|
||||
sample_id = item.get("sample_id", "unknown")
|
||||
return f"locomo_summary_{sample_id}"
|
||||
|
||||
def get_session_id(self, item: Any, workspace_id: str) -> str:
|
||||
"""Return session ID for a conversation."""
|
||||
return f"{workspace_id}_session"
|
||||
|
||||
async def setup_peers(self, ctx: ItemContext, item: Any) -> None:
|
||||
"""Create peers using speaker names as IDs."""
|
||||
conversation = item.get("conversation", {})
|
||||
speaker_a = conversation.get("speaker_a", "User")
|
||||
speaker_b = conversation.get("speaker_b", "Assistant")
|
||||
|
||||
ctx.peers["speaker_a"] = await ctx.honcho_client.aio.peer(id=speaker_a)
|
||||
ctx.peers["speaker_b"] = await ctx.honcho_client.aio.peer(id=speaker_b)
|
||||
ctx.peers["_speaker_a_name"] = speaker_a
|
||||
ctx.peers["_speaker_b_name"] = speaker_b
|
||||
|
||||
async def setup_session(self, ctx: ItemContext, item: Any) -> None:
|
||||
"""Create and configure session - observe BOTH peers."""
|
||||
peer_a = ctx.peers["speaker_a"]
|
||||
peer_b = ctx.peers["speaker_b"]
|
||||
|
||||
ctx.session = await ctx.honcho_client.aio.session(
|
||||
id=ctx.session_id, configuration=self._get_session_configuration()
|
||||
)
|
||||
|
||||
# Observe both peers since questions ask about both speakers
|
||||
await ctx.session.aio.add_peers(
|
||||
[
|
||||
(peer_a, SessionPeerConfig(observe_me=True, observe_others=False)),
|
||||
(peer_b, SessionPeerConfig(observe_me=True, observe_others=False)),
|
||||
]
|
||||
)
|
||||
|
||||
async def ingest_messages(self, ctx: ItemContext, item: Any) -> int:
|
||||
"""Ingest conversation messages into the session."""
|
||||
conversation = item.get("conversation", {})
|
||||
speaker_a = ctx.peers["_speaker_a_name"]
|
||||
speaker_b = ctx.peers["_speaker_b_name"]
|
||||
peer_a = ctx.peers["speaker_a"]
|
||||
peer_b = ctx.peers["speaker_b"]
|
||||
|
||||
# Extract and ingest all sessions
|
||||
sessions = extract_sessions(conversation)
|
||||
|
||||
messages: list[MessageCreateParams] = []
|
||||
total_tokens = 0
|
||||
|
||||
for date_str, session_messages in sessions:
|
||||
session_date = parse_locomo_date(date_str) if date_str else None
|
||||
|
||||
for msg in session_messages:
|
||||
speaker = msg.get("speaker", "")
|
||||
content, metadata = format_message_with_image(msg)
|
||||
total_tokens += calculate_tokens(content)
|
||||
|
||||
# Map speaker to peer by name
|
||||
if speaker == speaker_a:
|
||||
messages.append(
|
||||
peer_a.message(
|
||||
content, metadata=metadata, created_at=session_date
|
||||
)
|
||||
)
|
||||
elif speaker == speaker_b:
|
||||
messages.append(
|
||||
peer_b.message(
|
||||
content, metadata=metadata, created_at=session_date
|
||||
)
|
||||
)
|
||||
|
||||
# Store counts for results
|
||||
ctx.peers["_total_tokens"] = total_tokens
|
||||
ctx.peers["_total_sessions"] = len(sessions)
|
||||
ctx.peers["_total_turns"] = len(messages)
|
||||
|
||||
# Add messages in batches of 100
|
||||
for i in range(0, len(messages), 100):
|
||||
batch = messages[i : i + 100]
|
||||
await ctx.session.aio.add_messages(batch)
|
||||
|
||||
return len(messages)
|
||||
|
||||
def get_dream_observers(self, item: Any) -> list[str]:
|
||||
"""Return both speaker names - LoCoMo triggers dreams for both."""
|
||||
conversation = item.get("conversation", {})
|
||||
speaker_a = conversation.get("speaker_a", "User")
|
||||
speaker_b = conversation.get("speaker_b", "Assistant")
|
||||
return [speaker_a, speaker_b]
|
||||
|
||||
async def _retrieve_summary_context(self, ctx: ItemContext) -> str:
|
||||
"""
|
||||
Retrieve summaries from the session and format them as context.
|
||||
|
||||
Collects and concatenates both long and short summaries (long first,
|
||||
then short), including token counts in headings.
|
||||
|
||||
Returns:
|
||||
Formatted summary context string, or a note if no summaries available.
|
||||
"""
|
||||
workspace_id = ctx.workspace_id
|
||||
|
||||
summaries = await ctx.session.aio.summaries()
|
||||
|
||||
parts: list[str] = []
|
||||
|
||||
if summaries.long_summary:
|
||||
parts.append(
|
||||
f"[Long Summary ({summaries.long_summary.token_count} tokens)]"
|
||||
)
|
||||
parts.append(summaries.long_summary.content)
|
||||
print(
|
||||
f" [{workspace_id}] Retrieved long summary: {summaries.long_summary.token_count} tokens"
|
||||
)
|
||||
|
||||
if summaries.short_summary:
|
||||
parts.append(
|
||||
f"\n[Short Summary ({summaries.short_summary.token_count} tokens)]"
|
||||
)
|
||||
parts.append(summaries.short_summary.content)
|
||||
print(
|
||||
f" [{workspace_id}] Retrieved short summary: {summaries.short_summary.token_count} tokens"
|
||||
)
|
||||
|
||||
if not parts:
|
||||
print(f" [{workspace_id}] WARNING: No summaries available!")
|
||||
return "(No summaries were generated for this conversation.)"
|
||||
|
||||
return "\n".join(parts)
|
||||
|
||||
async def execute_questions(
|
||||
self, ctx: ItemContext, item: Any
|
||||
) -> ConversationResult:
|
||||
"""Execute all questions using only summary context."""
|
||||
start_time = time.time()
|
||||
sample_id = item.get("sample_id", "unknown")
|
||||
conversation = item.get("conversation", {})
|
||||
qa_list = item.get("qa", [])
|
||||
workspace_id = ctx.workspace_id
|
||||
|
||||
speaker_a = ctx.peers["_speaker_a_name"]
|
||||
speaker_b = ctx.peers["_speaker_b_name"]
|
||||
|
||||
result: ConversationResult = {
|
||||
"sample_id": sample_id,
|
||||
"speaker_a": speaker_a,
|
||||
"speaker_b": speaker_b,
|
||||
"total_sessions": ctx.peers.get("_total_sessions", 0),
|
||||
"total_turns": ctx.peers.get("_total_turns", 0),
|
||||
"total_tokens": ctx.peers.get("_total_tokens", 0),
|
||||
"question_results": [],
|
||||
"category_scores": {},
|
||||
"overall_score": 0.0,
|
||||
"error": None,
|
||||
"start_time": start_time,
|
||||
"end_time": 0.0,
|
||||
"duration_seconds": 0.0,
|
||||
}
|
||||
|
||||
# Retrieve summaries as context
|
||||
print(f"[{workspace_id}] Retrieving summaries...")
|
||||
summary_context = await self._retrieve_summary_context(ctx)
|
||||
|
||||
# Build system prompt with summary context
|
||||
system_prompt = (
|
||||
f"You are a helpful assistant with memory of past conversations "
|
||||
f"between {speaker_a} and {speaker_b}.\n\n"
|
||||
f"Below are summaries of their past conversations. Use these summaries "
|
||||
f"to answer the user's question as accurately as possible.\n\n"
|
||||
f"=== CONVERSATION SUMMARIES ===\n"
|
||||
f"{summary_context}\n"
|
||||
f"=== END SUMMARIES ==="
|
||||
)
|
||||
|
||||
# Filter questions
|
||||
filtered_qa = filter_questions(
|
||||
qa_list,
|
||||
exclude_adversarial=True,
|
||||
test_count=self.question_count,
|
||||
)
|
||||
|
||||
print(f"[{workspace_id}] Executing {len(filtered_qa)} questions...")
|
||||
|
||||
# Execute questions
|
||||
for q_idx, qa in enumerate(filtered_qa):
|
||||
question = qa.get("question", "")
|
||||
expected_answer = qa.get("answer", "")
|
||||
category = qa.get("category", 0)
|
||||
evidence = qa.get("evidence", [])
|
||||
category_name = CATEGORY_NAMES.get(category, f"category_{category}")
|
||||
|
||||
print(f" Q{q_idx + 1} [{category_name}]: {question[:80]}...")
|
||||
|
||||
try:
|
||||
# Ask the base model with summary context
|
||||
response = await self.openrouter_client.chat.completions.create(
|
||||
model=self.model,
|
||||
max_tokens=settings.DIALECTIC.MAX_OUTPUT_TOKENS,
|
||||
messages=[
|
||||
{"role": "system", "content": system_prompt},
|
||||
{"role": "user", "content": question},
|
||||
],
|
||||
)
|
||||
|
||||
if not response.choices or not response.choices[0].message.content:
|
||||
actual_response = ""
|
||||
else:
|
||||
actual_response = response.choices[0].message.content
|
||||
|
||||
# Get evidence context for the judge
|
||||
evidence_context = get_evidence_context(conversation, evidence)
|
||||
|
||||
# Judge the response
|
||||
judgment = await judge_response(
|
||||
self.openai_client,
|
||||
question,
|
||||
str(expected_answer),
|
||||
actual_response,
|
||||
evidence_context=evidence_context,
|
||||
)
|
||||
|
||||
passed = judgment.get("passed", False)
|
||||
|
||||
question_result = QuestionResult(
|
||||
question_id=q_idx,
|
||||
question=question,
|
||||
expected_answer=str(expected_answer),
|
||||
actual_response=actual_response,
|
||||
category=category,
|
||||
category_name=category_name,
|
||||
evidence=evidence,
|
||||
judgment=judgment,
|
||||
passed=passed,
|
||||
)
|
||||
|
||||
result["question_results"].append(question_result)
|
||||
|
||||
status = "PASS" if passed else "FAIL"
|
||||
print(f" [{status}]")
|
||||
if not passed:
|
||||
print(f" Expected: {expected_answer}")
|
||||
print(f" Got: {actual_response[:200]}...")
|
||||
|
||||
except Exception as e:
|
||||
self.logger.exception(f"Error executing question {q_idx}: {e}")
|
||||
question_result = QuestionResult(
|
||||
question_id=q_idx,
|
||||
question=question,
|
||||
expected_answer=str(expected_answer),
|
||||
actual_response=f"ERROR: {e}",
|
||||
category=category,
|
||||
category_name=category_name,
|
||||
evidence=evidence,
|
||||
judgment={"passed": False, "reasoning": str(e)},
|
||||
passed=False,
|
||||
)
|
||||
result["question_results"].append(question_result)
|
||||
|
||||
# Calculate category scores
|
||||
result["category_scores"] = calculate_category_scores(
|
||||
result["question_results"]
|
||||
)
|
||||
|
||||
# Calculate overall score (pass rate)
|
||||
if result["question_results"]:
|
||||
passed_count = sum(1 for qr in result["question_results"] if qr["passed"])
|
||||
result["overall_score"] = passed_count / len(result["question_results"])
|
||||
|
||||
result["end_time"] = time.time()
|
||||
result["duration_seconds"] = result["end_time"] - result["start_time"]
|
||||
|
||||
print(f"\nOverall Score: {result['overall_score']:.3f}")
|
||||
|
||||
return result
|
||||
|
||||
def print_summary(
|
||||
self, results: list[ConversationResult], total_duration: float
|
||||
) -> None:
|
||||
"""Print summary using the common function."""
|
||||
print_summary(results, total_duration)
|
||||
|
||||
def generate_output(
|
||||
self, results: list[ConversationResult], total_duration: float
|
||||
) -> None:
|
||||
"""Generate JSON output file."""
|
||||
if self.config.json_output:
|
||||
output_file = self.config.json_output
|
||||
else:
|
||||
output_file = (
|
||||
bench_dir
|
||||
/ f"eval_results/locomo_summary_results_{datetime.now().strftime('%Y%m%d_%H%M%S')}.json"
|
||||
)
|
||||
|
||||
generate_json_summary(
|
||||
results,
|
||||
total_duration,
|
||||
output_file,
|
||||
metadata_extra={
|
||||
"data_file": str(self.data_file),
|
||||
"runner_type": "summary_context",
|
||||
"model": self.model,
|
||||
"base_api_port": self.config.base_api_port,
|
||||
"pool_size": self.config.pool_size,
|
||||
"timeout_seconds": self.config.timeout_seconds,
|
||||
"deriver_settings": settings.DERIVER.model_dump(),
|
||||
"summary_settings": settings.SUMMARY.model_dump(),
|
||||
"dream_settings": settings.DREAM.model_dump(),
|
||||
},
|
||||
)
|
||||
|
||||
|
||||
def main() -> int:
|
||||
"""Main entry point for the LoCoMo summary evaluation runner."""
|
||||
parser = argparse.ArgumentParser(
|
||||
description="Evaluate Honcho summary quality using LoCoMo questions",
|
||||
formatter_class=argparse.RawDescriptionHelpFormatter,
|
||||
epilog="""
|
||||
Examples:
|
||||
%(prog)s --data-file tests/bench/locomo_data/locomo10.json
|
||||
%(prog)s --data-file locomo10.json --pool-size 4
|
||||
%(prog)s --data-file locomo10.json --sample-id "conv-26"
|
||||
%(prog)s --data-file locomo10.json --model anthropic/claude-sonnet-4.5
|
||||
%(prog)s --data-file locomo10.json --test-count 5 --question-count 20
|
||||
""",
|
||||
)
|
||||
|
||||
parser.add_argument(
|
||||
"--data-file",
|
||||
type=Path,
|
||||
required=True,
|
||||
help="Path to LoCoMo JSON file (required)",
|
||||
)
|
||||
|
||||
# Add common arguments shared across all runners
|
||||
add_common_arguments(parser)
|
||||
|
||||
# Summary-eval-specific arguments
|
||||
parser.add_argument(
|
||||
"--sample-id",
|
||||
type=str,
|
||||
help="Run only the conversation with this sample_id (skips all others)",
|
||||
)
|
||||
|
||||
parser.add_argument(
|
||||
"--test-count",
|
||||
type=int,
|
||||
help="Number of conversations to run from the data file (default: all)",
|
||||
)
|
||||
|
||||
parser.add_argument(
|
||||
"--question-count",
|
||||
type=int,
|
||||
help="Number of questions per conversation to run (default: all)",
|
||||
)
|
||||
|
||||
parser.add_argument(
|
||||
"--model",
|
||||
type=str,
|
||||
default=DEFAULT_MODEL,
|
||||
help=f"Model to use for answering questions given summary context (default: {DEFAULT_MODEL})",
|
||||
)
|
||||
|
||||
args = parser.parse_args()
|
||||
|
||||
# Validate common arguments
|
||||
error = validate_common_arguments(args)
|
||||
if error:
|
||||
print(error)
|
||||
return 1
|
||||
|
||||
# Validate locomo-specific arguments
|
||||
if not args.data_file.exists():
|
||||
print(f"Error: Data file {args.data_file} does not exist")
|
||||
return 1
|
||||
|
||||
# Create config and runner
|
||||
config = RunnerConfig.from_args(args, default_timeout=600)
|
||||
|
||||
runner = LoCoMoSummaryRunner(
|
||||
config=config,
|
||||
data_file=args.data_file,
|
||||
sample_id=args.sample_id,
|
||||
test_count=args.test_count,
|
||||
question_count=args.question_count,
|
||||
model=args.model,
|
||||
)
|
||||
|
||||
return runner.run_and_summarize()
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
exit(main())
|
||||
|
|
@ -13,6 +13,7 @@ from sqlalchemy.ext.asyncio import AsyncSession
|
|||
from src import crud, models, schemas
|
||||
from src.config import settings
|
||||
from src.utils.agent_tools import (
|
||||
MAX_PEER_CARD_FACTS,
|
||||
ObservationsCreatedResult,
|
||||
ToolContext,
|
||||
_handle_create_observations, # pyright: ignore[reportPrivateUsage]
|
||||
|
|
@ -788,6 +789,88 @@ class TestUpdatePeerCard:
|
|||
assert peer_card is not None
|
||||
assert "Name: John" in peer_card
|
||||
|
||||
async def test_deduplicates_and_caps_peer_card(
|
||||
self,
|
||||
db_session: AsyncSession,
|
||||
tool_test_data: Any,
|
||||
make_tool_context: Callable[..., ToolContext],
|
||||
):
|
||||
"""Normalizes peer card updates to avoid unbounded growth."""
|
||||
workspace, peer1, peer2, _, _, _ = tool_test_data
|
||||
ctx = make_tool_context()
|
||||
|
||||
oversized = ["Name: John", " Name: John ", "", " "]
|
||||
oversized.extend([f"Fact {i}" for i in range(MAX_PEER_CARD_FACTS + 5)])
|
||||
|
||||
await _handle_update_peer_card(ctx, {"content": oversized})
|
||||
|
||||
peer_card = await crud.get_peer_card(
|
||||
db_session,
|
||||
workspace_name=workspace.name,
|
||||
observer=peer1.name,
|
||||
observed=peer2.name,
|
||||
)
|
||||
assert peer_card is not None
|
||||
assert len(peer_card) == MAX_PEER_CARD_FACTS
|
||||
assert all(line.strip() for line in peer_card)
|
||||
assert peer_card.count("Name: John") == 1
|
||||
|
||||
async def test_none_content_preserves_existing_card(
|
||||
self,
|
||||
db_session: AsyncSession,
|
||||
tool_test_data: Any,
|
||||
make_tool_context: Callable[..., ToolContext],
|
||||
):
|
||||
"""None content should not overwrite the existing peer card."""
|
||||
workspace, peer1, peer2, _, _, _ = tool_test_data
|
||||
ctx = make_tool_context()
|
||||
|
||||
# First, create a valid peer card
|
||||
await _handle_update_peer_card(
|
||||
ctx, {"content": ["Name: Alice", "Location: NYC"]}
|
||||
)
|
||||
|
||||
# Now attempt to update with None — should be a no-op
|
||||
result = await _handle_update_peer_card(ctx, {"content": None})
|
||||
assert "empty" in result.lower()
|
||||
|
||||
# Verify original card is preserved
|
||||
peer_card = await crud.get_peer_card(
|
||||
db_session,
|
||||
workspace_name=workspace.name,
|
||||
observer=peer1.name,
|
||||
observed=peer2.name,
|
||||
)
|
||||
assert peer_card is not None
|
||||
assert "Name: Alice" in peer_card
|
||||
|
||||
async def test_empty_list_preserves_existing_card(
|
||||
self,
|
||||
db_session: AsyncSession,
|
||||
tool_test_data: Any,
|
||||
make_tool_context: Callable[..., ToolContext],
|
||||
):
|
||||
"""Empty list should not clear the existing peer card."""
|
||||
workspace, peer1, peer2, _, _, _ = tool_test_data
|
||||
ctx = make_tool_context()
|
||||
|
||||
# First, create a valid peer card
|
||||
await _handle_update_peer_card(ctx, {"content": ["Name: Bob", "Age: 30"]})
|
||||
|
||||
# Now attempt to update with empty list — should be a no-op
|
||||
result = await _handle_update_peer_card(ctx, {"content": []})
|
||||
assert "empty" in result.lower()
|
||||
|
||||
# Verify original card is preserved
|
||||
peer_card = await crud.get_peer_card(
|
||||
db_session,
|
||||
workspace_name=workspace.name,
|
||||
observer=peer1.name,
|
||||
observed=peer2.name,
|
||||
)
|
||||
assert peer_card is not None
|
||||
assert "Name: Bob" in peer_card
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
class TestGetPeerCard:
|
||||
|
|
|
|||
|
|
@ -26,6 +26,7 @@ from openai.types.completion_usage import CompletionUsage
|
|||
from pydantic import BaseModel, Field
|
||||
|
||||
from src.config import settings
|
||||
from src.exceptions import LLMError
|
||||
from src.utils.clients import (
|
||||
CLIENTS,
|
||||
HonchoLLMCallResponse,
|
||||
|
|
@ -566,6 +567,11 @@ class TestGoogleClient:
|
|||
assert response.output_tokens == 5
|
||||
assert response.finish_reasons == ["STOP"]
|
||||
|
||||
# Verify max output token cap is passed through
|
||||
mock_aio.models.generate_content.assert_called_once()
|
||||
call_args = mock_aio.models.generate_content.call_args
|
||||
assert call_args.kwargs["config"]["max_output_tokens"] == 100
|
||||
|
||||
async def test_google_json_mode(self):
|
||||
"""Test Google/Gemini with JSON mode"""
|
||||
from google import genai
|
||||
|
|
@ -606,9 +612,9 @@ class TestGoogleClient:
|
|||
# Verify JSON mode was set in config
|
||||
mock_aio.models.generate_content.assert_called_once()
|
||||
call_args = mock_aio.models.generate_content.call_args
|
||||
assert (
|
||||
call_args.kwargs["config"]["response_mime_type"] == "application/json"
|
||||
)
|
||||
config = call_args.kwargs["config"]
|
||||
assert config["response_mime_type"] == "application/json"
|
||||
assert config["max_output_tokens"] == 100
|
||||
|
||||
async def test_google_response_model(self):
|
||||
"""Test Google/Gemini with structured output"""
|
||||
|
|
@ -651,6 +657,7 @@ class TestGoogleClient:
|
|||
config = call_args.kwargs["config"]
|
||||
assert config["response_mime_type"] == "application/json"
|
||||
assert config["response_schema"] == SampleTestModel
|
||||
assert config["max_output_tokens"] == 100
|
||||
|
||||
async def test_google_streaming(self):
|
||||
"""Test Google/Gemini streaming response"""
|
||||
|
|
@ -705,6 +712,11 @@ class TestGoogleClient:
|
|||
assert chunks[2].is_done is True
|
||||
assert chunks[2].finish_reasons == ["STOP"]
|
||||
|
||||
# Verify streaming call includes max output token cap
|
||||
mock_aio.models.generate_content_stream.assert_called_once()
|
||||
call_args = mock_aio.models.generate_content_stream.call_args
|
||||
assert call_args.kwargs["config"]["max_output_tokens"] == 100
|
||||
|
||||
async def test_google_no_candidates_fallback(self):
|
||||
"""Test Google/Gemini fallback when no candidates"""
|
||||
from google import genai
|
||||
|
|
@ -732,6 +744,133 @@ class TestGoogleClient:
|
|||
assert response.output_tokens == 0 # Fallback value
|
||||
assert response.finish_reasons == ["stop"] # Default fallback
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"finish_reason", ["SAFETY", "RECITATION", "PROHIBITED_CONTENT", "BLOCKLIST"]
|
||||
)
|
||||
async def test_google_blocked_response_raises_error(self, finish_reason: str):
|
||||
"""Test that blocked Gemini responses raise LLMError for retry/failover."""
|
||||
from google import genai
|
||||
|
||||
mock_client = Mock(spec=genai.Client)
|
||||
mock_response = Mock()
|
||||
# Blocked responses typically have candidates with no content
|
||||
mock_finish_reason = Mock()
|
||||
mock_finish_reason.name = finish_reason
|
||||
mock_candidate = Mock()
|
||||
mock_candidate.content = None
|
||||
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 = 0
|
||||
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}),
|
||||
pytest.raises(LLMError, match=f"finish_reason={finish_reason}"),
|
||||
):
|
||||
await honcho_llm_call_inner(
|
||||
provider="google",
|
||||
model="gemini-2.5-flash",
|
||||
prompt="Summarize this",
|
||||
max_tokens=1000,
|
||||
)
|
||||
|
||||
async def test_google_max_tokens_empty_does_not_raise(self):
|
||||
"""Test that MAX_TOKENS with empty content returns normally (not a blocked response)."""
|
||||
from google import genai
|
||||
|
||||
mock_client = Mock(spec=genai.Client)
|
||||
mock_response = Mock()
|
||||
mock_finish_reason = Mock()
|
||||
mock_finish_reason.name = "MAX_TOKENS"
|
||||
mock_candidate = Mock()
|
||||
mock_candidate.content = None
|
||||
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 = 0
|
||||
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}):
|
||||
response = await honcho_llm_call_inner(
|
||||
provider="google",
|
||||
model="gemini-2.5-flash",
|
||||
prompt="Hello",
|
||||
max_tokens=100,
|
||||
)
|
||||
# MAX_TOKENS is not a blocked reason — returns empty content without raising
|
||||
assert response.content == ""
|
||||
assert response.finish_reasons == ["MAX_TOKENS"]
|
||||
|
||||
async def test_google_blocked_response_model_raises_error(self):
|
||||
"""Test that blocked responses in the response_model path raise LLMError."""
|
||||
from google import genai
|
||||
|
||||
mock_client = Mock(spec=genai.Client)
|
||||
mock_response = Mock()
|
||||
mock_response.parsed = None
|
||||
mock_finish_reason = Mock()
|
||||
mock_finish_reason.name = "SAFETY"
|
||||
mock_response.candidates = [Mock(finish_reason=mock_finish_reason)]
|
||||
mock_usage_metadata = Mock()
|
||||
mock_usage_metadata.prompt_token_count = 10
|
||||
mock_usage_metadata.candidates_token_count = 0
|
||||
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}),
|
||||
pytest.raises(LLMError, match="finish_reason=SAFETY"),
|
||||
):
|
||||
await honcho_llm_call_inner(
|
||||
provider="google",
|
||||
model="gemini-2.5-flash",
|
||||
prompt="Generate a person",
|
||||
max_tokens=100,
|
||||
response_model=SampleTestModel,
|
||||
)
|
||||
|
||||
async def test_google_blocked_finish_reason_with_valid_parsed_does_not_raise(self):
|
||||
"""Blocked finish_reason should not raise if parsed content is valid."""
|
||||
from google import genai
|
||||
|
||||
mock_client = Mock(spec=genai.Client)
|
||||
mock_response = Mock()
|
||||
mock_response.parsed = SampleTestModel(name="Alice", age=30, active=True)
|
||||
mock_finish_reason = Mock()
|
||||
mock_finish_reason.name = "SAFETY"
|
||||
mock_response.candidates = [Mock(finish_reason=mock_finish_reason)]
|
||||
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}):
|
||||
response = await honcho_llm_call_inner(
|
||||
provider="google",
|
||||
model="gemini-2.5-flash",
|
||||
prompt="Generate a person",
|
||||
max_tokens=100,
|
||||
response_model=SampleTestModel,
|
||||
)
|
||||
|
||||
assert isinstance(response.content, SampleTestModel)
|
||||
assert response.content.name == "Alice"
|
||||
assert response.finish_reasons == ["SAFETY"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
class TestGroqClient:
|
||||
|
|
|
|||
|
|
@ -0,0 +1,219 @@
|
|||
"""
|
||||
Tests for src/utils/summarizer.py
|
||||
|
||||
Covers the _create_summary function's handling of empty, blocked, and
|
||||
normal LLM responses, ensuring fallback logic prevents empty summaries
|
||||
from being persisted.
|
||||
"""
|
||||
|
||||
from unittest.mock import AsyncMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
from src.utils.clients import HonchoLLMCallResponse
|
||||
from src.utils.summarizer import (
|
||||
Summary,
|
||||
SummaryType,
|
||||
_create_summary, # pyright: ignore[reportPrivateUsage]
|
||||
)
|
||||
|
||||
# Common test arguments for _create_summary
|
||||
_FORMATTED_MESSAGES = "user: hello\nassistant: hi there"
|
||||
_INPUT_TOKENS = 100
|
||||
_MESSAGE_PUBLIC_ID = "msg_abc123"
|
||||
_LAST_MESSAGE_ID = 42
|
||||
_LAST_MESSAGE_CONTENT_PREVIEW = "hello there how are you"
|
||||
_MESSAGE_COUNT = 5
|
||||
|
||||
|
||||
async def _call_create_summary(
|
||||
summary_type: SummaryType,
|
||||
*,
|
||||
message_count: int = _MESSAGE_COUNT,
|
||||
input_tokens: int = _INPUT_TOKENS,
|
||||
) -> tuple[Summary, bool, int, int]:
|
||||
"""Helper to call _create_summary with standard test arguments."""
|
||||
return await _create_summary(
|
||||
formatted_messages=_FORMATTED_MESSAGES,
|
||||
previous_summary_text=None,
|
||||
summary_type=summary_type,
|
||||
input_tokens=input_tokens,
|
||||
message_public_id=_MESSAGE_PUBLIC_ID,
|
||||
last_message_id=_LAST_MESSAGE_ID,
|
||||
last_message_content_preview=_LAST_MESSAGE_CONTENT_PREVIEW,
|
||||
message_count=message_count,
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
class TestCreateSummary:
|
||||
"""Tests for the _create_summary function."""
|
||||
|
||||
async def test_normal_response_succeeds(self):
|
||||
"""Normal LLM response with content is preserved as-is."""
|
||||
mock_response = HonchoLLMCallResponse(
|
||||
content="User greeted the assistant and asked about the weather.",
|
||||
input_tokens=100,
|
||||
output_tokens=15,
|
||||
finish_reasons=["STOP"],
|
||||
)
|
||||
|
||||
with patch(
|
||||
"src.utils.summarizer.create_short_summary",
|
||||
new_callable=AsyncMock,
|
||||
return_value=mock_response,
|
||||
):
|
||||
(
|
||||
summary,
|
||||
is_fallback,
|
||||
input_tokens,
|
||||
output_tokens,
|
||||
) = await _call_create_summary(SummaryType.SHORT)
|
||||
|
||||
assert is_fallback is False
|
||||
assert (
|
||||
summary["content"]
|
||||
== "User greeted the assistant and asked about the weather."
|
||||
)
|
||||
assert input_tokens == 100
|
||||
assert output_tokens == 15
|
||||
|
||||
async def test_empty_response_uses_fallback(self):
|
||||
"""Empty LLM response triggers fallback text instead of saving empty string."""
|
||||
mock_response = HonchoLLMCallResponse(
|
||||
content="",
|
||||
input_tokens=100,
|
||||
output_tokens=0,
|
||||
finish_reasons=["SAFETY"],
|
||||
)
|
||||
|
||||
with patch(
|
||||
"src.utils.summarizer.create_short_summary",
|
||||
new_callable=AsyncMock,
|
||||
return_value=mock_response,
|
||||
):
|
||||
(
|
||||
summary,
|
||||
is_fallback,
|
||||
input_tokens,
|
||||
output_tokens,
|
||||
) = await _call_create_summary(SummaryType.SHORT)
|
||||
|
||||
assert is_fallback is True
|
||||
assert "Conversation with 5 messages" in summary["content"]
|
||||
assert summary["content"] != ""
|
||||
assert input_tokens == 0
|
||||
assert output_tokens == 0
|
||||
|
||||
async def test_whitespace_response_uses_fallback(self):
|
||||
"""Whitespace-only LLM response is treated as empty."""
|
||||
mock_response = HonchoLLMCallResponse(
|
||||
content=" \n \t ",
|
||||
input_tokens=100,
|
||||
output_tokens=3,
|
||||
finish_reasons=["STOP"],
|
||||
)
|
||||
|
||||
with patch(
|
||||
"src.utils.summarizer.create_short_summary",
|
||||
new_callable=AsyncMock,
|
||||
return_value=mock_response,
|
||||
):
|
||||
(
|
||||
summary,
|
||||
is_fallback,
|
||||
input_tokens,
|
||||
output_tokens,
|
||||
) = await _call_create_summary(SummaryType.SHORT)
|
||||
|
||||
assert is_fallback is True
|
||||
assert "Conversation with 5 messages" in summary["content"]
|
||||
assert input_tokens == 0
|
||||
assert output_tokens == 0
|
||||
|
||||
async def test_exception_uses_fallback(self):
|
||||
"""LLM exception triggers the existing fallback path."""
|
||||
with patch(
|
||||
"src.utils.summarizer.create_short_summary",
|
||||
new_callable=AsyncMock,
|
||||
side_effect=RuntimeError("API timeout"),
|
||||
):
|
||||
(
|
||||
summary,
|
||||
is_fallback,
|
||||
input_tokens,
|
||||
output_tokens,
|
||||
) = await _call_create_summary(SummaryType.SHORT)
|
||||
|
||||
assert is_fallback is True
|
||||
assert "Conversation with 5 messages" in summary["content"]
|
||||
assert input_tokens == 0
|
||||
assert output_tokens == 0
|
||||
|
||||
async def test_long_type_routes_to_long_summary(self):
|
||||
"""SummaryType.LONG calls create_long_summary, not create_short_summary."""
|
||||
mock_response = HonchoLLMCallResponse(
|
||||
content="A comprehensive summary of the conversation.",
|
||||
input_tokens=100,
|
||||
output_tokens=10,
|
||||
finish_reasons=["STOP"],
|
||||
)
|
||||
|
||||
with (
|
||||
patch(
|
||||
"src.utils.summarizer.create_long_summary",
|
||||
new_callable=AsyncMock,
|
||||
return_value=mock_response,
|
||||
) as mock_long,
|
||||
patch(
|
||||
"src.utils.summarizer.create_short_summary",
|
||||
new_callable=AsyncMock,
|
||||
) as mock_short,
|
||||
):
|
||||
summary, is_fallback, _, _ = await _call_create_summary(SummaryType.LONG)
|
||||
|
||||
assert is_fallback is False
|
||||
assert summary["content"] == "A comprehensive summary of the conversation."
|
||||
mock_long.assert_called_once()
|
||||
mock_short.assert_not_called()
|
||||
|
||||
async def test_non_stop_finish_with_content_keeps_content(self):
|
||||
"""Non-STOP finish reason with actual content is preserved (not a false positive)."""
|
||||
mock_response = HonchoLLMCallResponse(
|
||||
content="User discussed their project deadlines and asked for help prioritizing",
|
||||
input_tokens=100,
|
||||
output_tokens=12,
|
||||
finish_reasons=["MAX_TOKENS"],
|
||||
)
|
||||
|
||||
with patch(
|
||||
"src.utils.summarizer.create_short_summary",
|
||||
new_callable=AsyncMock,
|
||||
return_value=mock_response,
|
||||
):
|
||||
summary, is_fallback, _, _ = await _call_create_summary(SummaryType.SHORT)
|
||||
|
||||
assert is_fallback is False
|
||||
assert "project deadlines" in summary["content"]
|
||||
|
||||
async def test_zero_message_count_empty_fallback(self):
|
||||
"""Empty response with zero messages produces empty fallback text."""
|
||||
mock_response = HonchoLLMCallResponse(
|
||||
content="",
|
||||
input_tokens=0,
|
||||
output_tokens=0,
|
||||
finish_reasons=["SAFETY"],
|
||||
)
|
||||
|
||||
with patch(
|
||||
"src.utils.summarizer.create_short_summary",
|
||||
new_callable=AsyncMock,
|
||||
return_value=mock_response,
|
||||
):
|
||||
summary, is_fallback, _, _ = await _call_create_summary(
|
||||
SummaryType.SHORT, message_count=0, input_tokens=0
|
||||
)
|
||||
|
||||
assert is_fallback is True
|
||||
assert summary["content"] == ""
|
||||
assert summary["token_count"] == 0
|
||||
Loading…
Reference in New Issue