From 4f579d5c6662c0d8b5394b3c77217436f7438619 Mon Sep 17 00:00:00 2001 From: Vineeth Voruganti <13438633+VVoruganti@users.noreply.github.com> Date: Thu, 21 May 2026 13:25:34 -0400 Subject: [PATCH] fix: reframe peer card prompts as stable identity markers (#686) * fix: reframe peer card prompts as stable identity markers * fix: remove strict parameter validation for thinking on anthropic and openai transports * fix(dreamer): Add backwards compatability instructions for peer card prompt --- src/dreamer/specialists.py | 118 ++++++++------ src/llm/backends/anthropic.py | 12 +- src/llm/backends/openai.py | 23 +-- src/utils/agent_tools.py | 124 +++++++++++++-- tests/dreamer/test_model_config_usage.py | 57 ++++++- tests/llm/test_backends/test_anthropic.py | 37 +++-- tests/llm/test_backends/test_openai.py | 84 ++++++++-- tests/utils/test_agent_tools.py | 185 ++++++++++++++++++++-- 8 files changed, 531 insertions(+), 109 deletions(-) diff --git a/src/dreamer/specialists.py b/src/dreamer/specialists.py index 4d3ecc97..af67b989 100644 --- a/src/dreamer/specialists.py +++ b/src/dreamer/specialists.py @@ -75,9 +75,12 @@ class BaseSpecialist(ABC): """Base class for agentic specialists.""" name: str = "base" + # Whether this specialist is allowed to write to the peer card. Defaults to True; + # specialists that should never touch the card (e.g., induction) override to False. + can_update_peer_card: bool = True # Subclasses can override to customize the peer card update instruction peer_card_update_instruction: str = ( - "Only update this with durable profile facts via `update_peer_card`." + "Only update this with durable identity markers via `update_peer_card`." ) @abstractmethod @@ -195,8 +198,10 @@ If you update it, send the full deduplicated list and remove stale entries. db, workspace_name, schemas.PeerCreate(name=observed) ) - # Determine if peer card tools should be included - peer_card_enabled = ( + # Determine if peer card tools should be included. Specialists that + # cannot write to the peer card (e.g., induction) skip the fetch and + # the prompt section entirely. + peer_card_enabled = self.can_update_peer_card and ( configuration is None or configuration.peer_card.create ) @@ -434,7 +439,7 @@ class DeductionSpecialist(BaseSpecialist): """ name: str = "deduction" - peer_card_update_instruction: str = "Update this with `update_peer_card` only for stable biographical/profile facts." + peer_card_update_instruction: str = "Update this with `update_peer_card` only for stable identity markers. See the PEER CARD section in the system prompt for the allowed entry kinds and rules." def get_tools(self, *, peer_card_enabled: bool = True) -> list[dict[str, Any]]: if peer_card_enabled: @@ -462,26 +467,61 @@ class DeductionSpecialist(BaseSpecialist): ) -> str: peer_card_section = "" if peer_card_enabled: - peer_card_section = """ + peer_card_section = f""" ## PEER CARD (REQUIRED) -The peer card is a summary of stable biographical facts. You MUST update it when you learn: -- Name, age, location, occupation -- Family members and relationships -- Standing instructions ("call me X", "don't mention Y") -- Core preferences and traits +The peer card is {observed}'s identity store: stable identity markers that distinguish this entity from others and persist across interactions. Behavior, tendencies, transient state, and episodic facts belong in observations, not on the peer card. -Never add temporary event summaries, one-off conclusions, reasoning traces, or contradiction notes. +A peer can be anything with identity that changes over time — a human, an agent, a codebase, a team, an organization. Do not assume {observed} is human. Do not require any field; empty is the correct output when evidence is absent. -Format entries as: -- Plain facts: "Name: Alice", "Works at Google", "Lives in NYC" -- `INSTRUCTION: ...` for standing instructions -- `PREFERENCE: ...` for preferences -- `TRAIT: ...` for personality traits +### Allowed entry kinds -Call `update_peer_card` with the complete updated list when you have new biographical info. -Keep it concise (max 40 entries), deduplicated, and current.""" +Each entry must start with one of these four prefixes (exact case, followed by a space): + +- `IDENTITY: ...` — canonical name, kind, aliases, IDs + - `IDENTITY: Name: Alice` + - `IDENTITY: Kind: Python monorepo` + - `IDENTITY: Version: 4.2` + - `IDENTITY: Aliases: alice@example.com` +- `ATTRIBUTE: ...` — stable durable property of the entity (including explicitly stated standing preferences) + - `ATTRIBUTE: Location: NYC` + - `ATTRIBUTE: Language: Python` + - `ATTRIBUTE: Prefers tea` + - `ATTRIBUTE: Charter: ship Honcho infrastructure` +- `RELATIONSHIP: ...` — durable link to another entity + - `RELATIONSHIP: Spouse: Bob` + - `RELATIONSHIP: Maintainer: vineeth` + - `RELATIONSHIP: Members: vineeth, rajat` +- `INSTRUCTION: ...` — standing rule of engagement that {observed} has explicitly stated (do/don't for the observer). Only when explicit; never inferred from behavior. + - `INSTRUCTION: Call me Vee` + - `INSTRUCTION: Never push to main without review` + +### Rules + +1. **Stable.** If the value plausibly changes within six months absent a deliberate announcement, it does not belong on the card. Prefer leaving the card empty over filling it with volatile content. +2. **Subject is {observed}.** Every entry must be a fact about {observed}, not about another participant in the session. Never write facts about co-occurring peers into the card, no matter how frequently they appear in the messages. +3. **Evidence-grounded.** Only write what {observed} has explicitly stated, or what another participant has explicitly stated about {observed} with {observed}'s assent. No "general knowledge" inferences (`"co-founder"` does not imply an age; mentioning a colleague does not imply a family relationship). +4. **Type-agnostic.** {observed} may not be human. Do not require name/age/location/family/occupation fields. +5. **No behavioral content.** TRAITs, behavioral tendencies, patterns, and inferred preferences belong in observations, not on the peer card. Do not write `TRAIT:` entries or behavioral `PREFERENCE:` entries — they will be rejected. +6. **No evidence bundles.** Each entry is one concise fact. No `e.g.` clauses, no parenthetical example lists, no semicolon-separated value dumps. + +### Migrating an existing peer card + +The CURRENT PEER CARD shown in the user message may contain entries from an older format that do not start with an allowed prefix (e.g. `Name: Alice`, `Lives in NYC`, `TRAIT: Analytical`, `PREFERENCE: Detailed explanations`). When you call `update_peer_card`, you are responsible for re-emitting the entries you want to keep — entries you omit are dropped, and entries without an allowed prefix are silently rejected. + +For each legacy entry: + +- If it is still a valid identity marker, re-emit it under the correct prefix and keep the original content where reasonable. Examples: + - `Name: Alice` → `IDENTITY: Name: Alice` + - `Lives in NYC` → `ATTRIBUTE: Location: NYC` + - `Works at Google` → `ATTRIBUTE: Employer: Google` + - `INSTRUCTION: Call me Vee` → keep as is (already correctly prefixed) +- Drop entries that violate the rules above: behavioral `TRAIT:` lines, inferred behavioral `PREFERENCE:` lines, one-off events, transient state, evidence bundles. Do not re-prefix them — they are not identity markers. + +When in doubt about a specific legacy entry, prefer migrating it (so valid info isn't lost) over dropping it. Splitting one dense legacy entry into multiple correctly-prefixed entries is fine and encouraged (e.g. a semicolon-separated `Tech Stack:` dump can become several `ATTRIBUTE:` lines, one per durable tool/platform). + +Call `update_peer_card` with the complete deduplicated list when there is a durable identity update to record, or when the existing card needs migration. Entries that do not start with one of the four allowed prefixes will be rejected. Keep concise (max 40 entries).""" return f"""You are a deductive reasoning agent analyzing observations about {observed}. @@ -578,20 +618,19 @@ class InductionSpecialist(BaseSpecialist): 1. Explores observations to understand what's there 2. Identifies patterns and generalizations across multiple observations 3. Creates new inductive observations with source linkage - 4. Updates peer card with high-confidence traits and tendencies + + Does not write to the peer card — the peer card stores stable identity markers, + which is deduction's responsibility. Inductive patterns and tendencies stay as + observations. """ name: str = "induction" - peer_card_update_instruction: str = "Only add highly stable profile traits/preferences; do not copy transient conclusions." + # Induction never writes to the peer card; behavioral patterns are observations. + can_update_peer_card: bool = False def get_tools(self, *, peer_card_enabled: bool = True) -> list[dict[str, Any]]: - if peer_card_enabled: - return INDUCTION_SPECIALIST_TOOLS - return [ - t - for t in INDUCTION_SPECIALIST_TOOLS - if t["name"] not in PEER_CARD_TOOL_NAMES - ] + _ = peer_card_enabled + return INDUCTION_SPECIALIST_TOOLS def get_model_config(self) -> ConfiguredModelSettings: return _require_specialist_model_config( @@ -608,21 +647,7 @@ class InductionSpecialist(BaseSpecialist): def build_system_prompt( self, observed: str, *, peer_card_enabled: bool = True ) -> str: - peer_card_section = "" - if peer_card_enabled: - peer_card_section = """ - -## PEER CARD (REQUIRED) - -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` - -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).""" - + _ = peer_card_enabled return f"""You are an inductive reasoning agent identifying patterns about {observed}. ## YOUR JOB @@ -658,7 +683,6 @@ Create inductive observations when you see patterns: ### Temporal Patterns - "Career goals have remained consistent" - "Living situation changes frequently" -{peer_card_section} ## CREATING OBSERVATIONS @@ -690,11 +714,13 @@ Use `create_observations_inductive`. hints: list[str] | None, peer_card: list[str] | None = None, ) -> str: - peer_card_context = self._build_peer_card_context(peer_card) + # Induction does not consume peer card context — it produces inductive + # observations, not identity-marker updates. + _ = peer_card if hints: hints_str = "\n".join(f"- {q}" for q in hints[:5]) - return f"""{peer_card_context}Explore and find patterns. These areas may be worth investigating: + return f"""Explore and find patterns. These areas may be worth investigating: {hints_str} @@ -702,7 +728,7 @@ But follow the evidence - if you find patterns elsewhere, pursue those. Start with `get_recent_observations`.""" - return f"""{peer_card_context}Explore the observation space and identify patterns. + return """Explore the observation space and identify patterns. Remember: patterns need 2+ sources. Look for tendencies, preferences, and behavioral regularities. diff --git a/src/llm/backends/anthropic.py b/src/llm/backends/anthropic.py index cdf775be..17138583 100644 --- a/src/llm/backends/anthropic.py +++ b/src/llm/backends/anthropic.py @@ -34,11 +34,7 @@ class AnthropicBackend: max_output_tokens: int | None = None, extra_params: dict[str, Any] | None = None, ) -> CompletionResult: - del max_output_tokens - if thinking_effort is not None: - raise ValueError( - "Anthropic backend does not support thinking_effort; use thinking_budget_tokens instead" - ) + del max_output_tokens, thinking_effort request_messages, system_messages = self._extract_system(messages) params: dict[str, Any] = { @@ -123,11 +119,7 @@ class AnthropicBackend: extra_params: dict[str, Any] | None = None, ) -> AsyncIterator[StreamChunk]: is_json_mode = self._json_mode(extra_params) - del max_output_tokens - if thinking_effort is not None: - raise ValueError( - "Anthropic backend does not support thinking_effort; use thinking_budget_tokens instead" - ) + del max_output_tokens, thinking_effort request_messages, system_messages = self._extract_system(messages) params: dict[str, Any] = { diff --git a/src/llm/backends/openai.py b/src/llm/backends/openai.py index 1e01e78a..b2d82d91 100644 --- a/src/llm/backends/openai.py +++ b/src/llm/backends/openai.py @@ -128,11 +128,6 @@ class OpenAIBackend: max_output_tokens: int | None = None, extra_params: dict[str, Any] | None = None, ) -> CompletionResult: - if thinking_budget_tokens is not None: - raise ValidationException( - "OpenAI backend does not support thinking_budget_tokens; use thinking_effort instead" - ) - params = self._build_params( model=model, messages=messages, @@ -142,6 +137,7 @@ class OpenAIBackend: tools=tools, tool_choice=tool_choice, thinking_effort=thinking_effort, + thinking_budget_tokens=thinking_budget_tokens, extra_params=extra_params, ) @@ -221,11 +217,6 @@ class OpenAIBackend: max_output_tokens: int | None = None, extra_params: dict[str, Any] | None = None, ) -> AsyncIterator[StreamChunk]: - if thinking_budget_tokens is not None: - raise ValidationException( - "OpenAI backend does not support thinking_budget_tokens; use thinking_effort instead" - ) - params = self._build_params( model=model, messages=messages, @@ -235,6 +226,7 @@ class OpenAIBackend: tools=tools, tool_choice=tool_choice, thinking_effort=thinking_effort, + thinking_budget_tokens=thinking_budget_tokens, extra_params=extra_params, ) params["stream"] = True @@ -284,6 +276,7 @@ class OpenAIBackend: tools: list[dict[str, Any]] | None, tool_choice: str | dict[str, Any] | None, thinking_effort: str | None, + thinking_budget_tokens: int | None, extra_params: dict[str, Any] | None, ) -> dict[str, Any]: params: dict[str, Any] = { @@ -304,6 +297,16 @@ class OpenAIBackend: if thinking_effort: params["reasoning_effort"] = thinking_effort + # Token-budget style thinking is not part of the native OpenAI API, but + # OpenAI-compatible proxies (OpenRouter, etc.) accept a `reasoning` object + # on the request body. Pass through via extra_body so it reaches those + # backends; operators on providers that need a different shape (vLLM, + # Fireworks, ...) can override via ModelConfig.provider_params. + if thinking_budget_tokens is not None and thinking_budget_tokens > 0: + params.setdefault("extra_body", {}).setdefault("reasoning", {})[ + "max_tokens" + ] = thinking_budget_tokens + if stop: params["stop"] = stop if tools: diff --git a/src/utils/agent_tools.py b/src/utils/agent_tools.py index 8cdfd6cb..de5b2b09 100644 --- a/src/utils/agent_tools.py +++ b/src/utils/agent_tools.py @@ -33,6 +33,36 @@ logger = logging.getLogger(__name__) # Hard cap to prevent unbounded peer card growth from repeated agent updates. MAX_PEER_CARD_FACTS = 40 +# Identity-marker prefixes allowed on the peer card. Anything else is rejected +# structurally — see `_validate_peer_card_entry`. +PEER_CARD_ALLOWED_PREFIXES: tuple[str, ...] = ( + "IDENTITY:", + "ATTRIBUTE:", + "RELATIONSHIP:", + "INSTRUCTION:", +) + +# Per-entry character cap to block evidence-bundle dumps and runaway lines. +MAX_PEER_CARD_ENTRY_LENGTH = 200 + + +def _validate_peer_card_entry(line: str) -> bool: + """Structural validation for a single peer card entry. + + Returns True when the line starts with one of the allowed prefixes followed + by a space, has a non-empty body after the prefix, and fits within the per- + entry length cap. Subject-substance correctness (is this actually about the + observed peer?) is left to the prompt — this is form-only. + """ + if not line or len(line) > MAX_PEER_CARD_ENTRY_LENGTH: + return False + for prefix in PEER_CARD_ALLOWED_PREFIXES: + prefix_with_space = f"{prefix} " + if line.startswith(prefix_with_space): + body = line[len(prefix_with_space) :].strip() + return bool(body) + return False + def _normalized_observation_input( obs: schemas.ObservationInput, @@ -460,9 +490,16 @@ TOOLS: dict[str, dict[str, Any]] = { "update_peer_card": { "name": "update_peer_card", "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." + "Update the peer card with stable identity markers about the observed peer. " + "An identity marker distinguishes the peer from others of its kind and persists across interactions. " + "The peer may be any entity with identity that changes over time (human, agent, codebase, team, organization) — do not assume the peer is human. " + "Each entry must start with one of four prefixes: `IDENTITY:` (canonical name, kind, aliases, IDs), " + "`ATTRIBUTE:` (stable durable property, including explicitly stated standing preferences), " + "`RELATIONSHIP:` (durable link to another entity), or " + "`INSTRUCTION:` (standing rule of engagement the peer has explicitly stated). " + "Do not write `TRAIT:` or behavioral `PREFERENCE:` entries, one-off observations, transient state, " + "inferred facts not directly supported by evidence, evidence bundles / `e.g.` clauses, or entries about co-occurring peers. " + "Entries without an allowed prefix or that exceed the per-entry length cap are rejected." ), "input_schema": { "type": "object", @@ -471,7 +508,9 @@ TOOLS: dict[str, dict[str, Any]] = { "type": "array", "description": ( "Complete deduplicated peer card list (max 40 entries). " - + "Each entry should be a concise standalone profile fact." + "Each entry must start with one of the allowed prefixes " + "(`IDENTITY: `, `ATTRIBUTE: `, `RELATIONSHIP: `, `INSTRUCTION: `) " + "followed by one concise identity marker. Entries without an allowed prefix are rejected." ), "items": {"type": "string"}, }, @@ -796,7 +835,7 @@ DEDUCTION_SPECIALIST_TOOLS: list[dict[str, Any]] = [ # Tools for the induction specialist (dreamer phase 2) # Creates inductive observations from explicit and deductive observations # Includes message access for context and self-directed exploration -# Note: get_peer_card is not included - peer card is injected into the prompt directly +# Induction does not write to the peer card — that is deduction's responsibility. INDUCTION_SPECIALIST_TOOLS: list[dict[str, Any]] = [ # Discovery tools TOOLS["get_recent_observations"], @@ -804,7 +843,6 @@ INDUCTION_SPECIALIST_TOOLS: list[dict[str, Any]] = [ TOOLS["search_messages"], # Action tools TOOLS["create_observations_inductive"], - TOOLS["update_peer_card"], ] @@ -1425,9 +1463,16 @@ async def _handle_update_peer_card( ) return "Peer card content was empty, no update performed." - # Normalize and deduplicate to keep peer cards bounded and stable. + # Normalize, validate structure, and deduplicate to keep peer cards bounded + # and on-spec. normalized_peer_card: list[str] = [] seen: set[str] = set() + rejected_count = 0 + # Keep a small sample of rejected entries to surface back to the model so it + # can self-correct on a retry. Capped to avoid bloating the tool response. + rejected_samples: list[str] = [] + _REJECTED_SAMPLE_CAP = 3 + _REJECTED_SAMPLE_LINE_LIMIT = 120 items = ( cast(list[str], raw_peer_card_content) if isinstance(raw_peer_card_content, list) @@ -1438,6 +1483,16 @@ async def _handle_update_peer_card( if not line: continue + if not _validate_peer_card_entry(line): + rejected_count += 1 + if len(rejected_samples) < _REJECTED_SAMPLE_CAP: + rejected_samples.append(line[:_REJECTED_SAMPLE_LINE_LIMIT]) + logger.info( + "Rejecting peer card entry (no allowed prefix, empty body, or over length cap): %r", + line[:80], + ) + continue + # Case-insensitive dedupe with whitespace normalization. normalized_key = " ".join(line.lower().split()) if normalized_key in seen: @@ -1445,12 +1500,44 @@ async def _handle_update_peer_card( seen.add(normalized_key) normalized_peer_card.append(line) - # Don't clear the peer card if all content normalized to empty. + if rejected_count: + logger.info( + "Peer card update for %s/%s/%s rejected %d structurally invalid entries", + ctx.workspace_name, + ctx.observer, + ctx.observed, + rejected_count, + ) + + def _format_rejection_feedback(scope: str) -> str: + """Build a self-correction hint for the model. `scope` is grammar glue: + either "all" (every entry rejected) or e.g. "3 of 12" (partial).""" + samples_block = "" + if rejected_samples: + sample_lines = "\n".join(f" - {s!r}" for s in rejected_samples) + extra = ( + f" (+{rejected_count - len(rejected_samples)} more)" + if rejected_count > len(rejected_samples) + else "" + ) + samples_block = f" Examples of rejected entries{extra}:\n{sample_lines}" + return ( + f"Rejected {scope} entries for failing structural validation. " + "Each entry must start with one of `IDENTITY: `, `ATTRIBUTE: `, " + "`RELATIONSHIP: `, or `INSTRUCTION: ` and stay under the per-entry " + f"length cap.{samples_block}" + ) + + # Don't clear the peer card if all content normalized to empty or every + # entry was structurally invalid. if not normalized_peer_card: logger.warning( - "Peer card update normalized to empty for %s, keeping existing card", + "Peer card update normalized to empty for %s (rejected=%d), keeping existing card", ctx.workspace_name, + rejected_count, ) + if rejected_count: + return _format_rejection_feedback(f"all {rejected_count}") return "Peer card content was empty after normalization, no update performed." if len(normalized_peer_card) > MAX_PEER_CARD_FACTS: @@ -1494,9 +1581,24 @@ async def _handle_update_peer_card( # can set its `peer_card_updated` flag without name-counting. from src.utils.types import ToolResult + success_content = ( + f"Updated peer card for {ctx.observed} by {ctx.observer} " + f"with {len(normalized_peer_card)} entries." + ) + if rejected_count: + # Partial reject: surface the rejection so the model can re-emit the + # dropped entries (with correct prefixes) on a retry instead of + # silently losing them. + accepted = len(normalized_peer_card) + total = accepted + rejected_count + success_content = f"{success_content} {_format_rejection_feedback(f'{rejected_count} of {total}')}" return ToolResult( - content=f"Updated peer card for {ctx.observed} by {ctx.observer}", - metadata={"peer_card_updated": True, "facts_count": len(normalized_peer_card)}, + content=success_content, + metadata={ + "peer_card_updated": True, + "facts_count": len(normalized_peer_card), + "rejected_count": rejected_count, + }, ) diff --git a/tests/dreamer/test_model_config_usage.py b/tests/dreamer/test_model_config_usage.py index 91d1d141..90892632 100644 --- a/tests/dreamer/test_model_config_usage.py +++ b/tests/dreamer/test_model_config_usage.py @@ -3,10 +3,65 @@ from unittest.mock import AsyncMock, patch import pytest from src.config import settings -from src.dreamer.specialists import DeductionSpecialist +from src.dreamer.specialists import DeductionSpecialist, InductionSpecialist from src.llm import HonchoLLMCallResponse +def test_deduction_prompt_uses_identity_markers_framing() -> None: + """Deduction prompt must frame the peer card as an identity store with the + entity-agnostic prefix taxonomy, not as a human bio sheet.""" + prompt = DeductionSpecialist().build_system_prompt("alice", peer_card_enabled=True) + + assert "identity store" in prompt + assert "stable identity markers" in prompt + for prefix in ("IDENTITY:", "ATTRIBUTE:", "RELATIONSHIP:", "INSTRUCTION:"): + assert prefix in prompt + # Cross-entity examples confirm the prompt is not biased toward humans. + assert "codebase" in prompt + assert "team" in prompt + # Behavioral content must be explicitly excluded. + assert "TRAIT:" in prompt + # The old human-shaped REQUIRED enumeration must be gone. + assert "Family members and relationships" not in prompt + assert "Core preferences and traits" not in prompt + + +def test_deduction_prompt_omits_peer_card_when_disabled() -> None: + prompt = DeductionSpecialist().build_system_prompt("alice", peer_card_enabled=False) + assert "PEER CARD" not in prompt + assert "IDENTITY:" not in prompt + + +def test_induction_prompt_has_no_peer_card_section() -> None: + """Induction no longer writes to the peer card; its prompt must not reference it.""" + prompt = InductionSpecialist().build_system_prompt("alice", peer_card_enabled=True) + assert "PEER CARD" not in prompt + assert "update_peer_card" not in prompt + + +def test_induction_specialist_cannot_update_peer_card() -> None: + """Induction must have can_update_peer_card=False and no update_peer_card tool.""" + specialist = InductionSpecialist() + assert specialist.can_update_peer_card is False + + tool_names = {t["name"] for t in specialist.get_tools()} + assert "update_peer_card" not in tool_names + # Sanity: induction still has the discovery and create tools it actually needs. + assert "create_observations_inductive" in tool_names + assert "search_memory" in tool_names + + +def test_deduction_specialist_can_update_peer_card() -> None: + specialist = DeductionSpecialist() + assert specialist.can_update_peer_card is True + + tool_names = {t["name"] for t in specialist.get_tools(peer_card_enabled=True)} + assert "update_peer_card" in tool_names + + disabled_names = {t["name"] for t in specialist.get_tools(peer_card_enabled=False)} + assert "update_peer_card" not in disabled_names + + @pytest.mark.asyncio async def test_deduction_specialist_uses_nested_model_config( monkeypatch: pytest.MonkeyPatch, diff --git a/tests/llm/test_backends/test_anthropic.py b/tests/llm/test_backends/test_anthropic.py index c8f2bbdf..52de0fa2 100644 --- a/tests/llm/test_backends/test_anthropic.py +++ b/tests/llm/test_backends/test_anthropic.py @@ -123,13 +123,32 @@ async def test_anthropic_backend_skips_assistant_prefill_for_claude_4_models() - @pytest.mark.asyncio -async def test_anthropic_backend_rejects_thinking_effort() -> None: - backend = AnthropicBackend(Mock()) - - with pytest.raises(ValueError, match="does not support thinking_effort"): - await backend.complete( - model="claude-haiku-4-5", - messages=[{"role": "user", "content": "Hello"}], - max_tokens=100, - thinking_effort="high", +async def test_anthropic_backend_ignores_thinking_effort() -> None: + client = Mock() + client.messages.create = AsyncMock( + return_value=SimpleNamespace( + content=[TextBlock(type="text", text="ok")], + usage=SimpleNamespace( + input_tokens=10, + output_tokens=5, + cache_creation_input_tokens=0, + cache_read_input_tokens=0, + ), + stop_reason="end_turn", ) + ) + + backend = AnthropicBackend(client) + await backend.complete( + model="claude-haiku-4-5", + messages=[{"role": "user", "content": "Hello"}], + max_tokens=100, + thinking_effort="high", + ) + + await_args = client.messages.create.await_args + if await_args is None: + raise AssertionError("Expected Anthropic client call") + call = await_args.kwargs + assert "thinking" not in call + assert "reasoning_effort" not in call diff --git a/tests/llm/test_backends/test_openai.py b/tests/llm/test_backends/test_openai.py index 81838202..695b12cd 100644 --- a/tests/llm/test_backends/test_openai.py +++ b/tests/llm/test_backends/test_openai.py @@ -3,7 +3,6 @@ from unittest.mock import AsyncMock, Mock import pytest -from src.exceptions import ValidationException from src.llm.backends.openai import OpenAIBackend @@ -153,18 +152,79 @@ async def test_openai_backend_does_not_treat_proxy_models_with_gpt5_substring_as @pytest.mark.asyncio -async def test_openai_backend_rejects_thinking_budget_tokens() -> None: - backend = OpenAIBackend(Mock()) - - with pytest.raises( - ValidationException, match="does not support thinking_budget_tokens" - ): - await backend.complete( - model="gpt-5-mini", - messages=[{"role": "user", "content": "Hello"}], - max_tokens=100, - thinking_budget_tokens=256, +async def test_openai_backend_passes_thinking_budget_via_extra_body() -> None: + client = Mock() + client.chat.completions.create = AsyncMock( + return_value=SimpleNamespace( + choices=[ + SimpleNamespace( + finish_reason="stop", + message=SimpleNamespace( + content="ok", + tool_calls=[], + reasoning_details=[], + ), + ) + ], + usage=SimpleNamespace( + prompt_tokens=10, + completion_tokens=5, + prompt_tokens_details=None, + ), ) + ) + + backend = OpenAIBackend(client) + await backend.complete( + model="x-ai/grok-4.1-fast", + messages=[{"role": "user", "content": "Hello"}], + max_tokens=100, + thinking_budget_tokens=256, + ) + + await_args = client.chat.completions.create.await_args + if await_args is None: + raise AssertionError("Expected OpenAI create call") + call = await_args.kwargs + assert call["extra_body"] == {"reasoning": {"max_tokens": 256}} + + +@pytest.mark.asyncio +async def test_openai_backend_skips_extra_body_when_thinking_budget_zero() -> None: + client = Mock() + client.chat.completions.create = AsyncMock( + return_value=SimpleNamespace( + choices=[ + SimpleNamespace( + finish_reason="stop", + message=SimpleNamespace( + content="ok", + tool_calls=[], + reasoning_details=[], + ), + ) + ], + usage=SimpleNamespace( + prompt_tokens=10, + completion_tokens=5, + prompt_tokens_details=None, + ), + ) + ) + + backend = OpenAIBackend(client) + await backend.complete( + model="x-ai/grok-4.1-fast", + messages=[{"role": "user", "content": "Hello"}], + max_tokens=100, + thinking_budget_tokens=0, + ) + + await_args = client.chat.completions.create.await_args + if await_args is None: + raise AssertionError("Expected OpenAI create call") + call = await_args.kwargs + assert "extra_body" not in call @pytest.mark.asyncio diff --git a/tests/utils/test_agent_tools.py b/tests/utils/test_agent_tools.py index b300417d..8ddab1bc 100644 --- a/tests/utils/test_agent_tools.py +++ b/tests/utils/test_agent_tools.py @@ -14,7 +14,9 @@ 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_ENTRY_LENGTH, MAX_PEER_CARD_FACTS, + PEER_CARD_ALLOWED_PREFIXES, ObservationsCreatedResult, ToolContext, _handle_create_observations, # pyright: ignore[reportPrivateUsage] @@ -32,6 +34,7 @@ from src.utils.agent_tools import ( _handle_search_messages, # pyright: ignore[reportPrivateUsage] _handle_search_messages_temporal, # pyright: ignore[reportPrivateUsage] _handle_update_peer_card, # pyright: ignore[reportPrivateUsage] + _validate_peer_card_entry, # pyright: ignore[reportPrivateUsage] create_observations, create_tool_executor, extract_preferences, @@ -415,7 +418,7 @@ class TestCreateObservations: result = await create_observations( observations=[ - schemas.ObservationInput(content=" ", level="explicit"), + schemas.ObservationInput(content=" ", level="explicit"), schemas.ObservationInput( content=" trimmed observation ", level="explicit" ), @@ -1023,7 +1026,14 @@ class TestUpdatePeerCard: ctx = make_tool_context() result = await _handle_update_peer_card( - ctx, {"content": ["Name: John", "Location: NYC", "Occupation: Engineer"]} + ctx, + { + "content": [ + "IDENTITY: Name: John", + "ATTRIBUTE: Location: NYC", + "ATTRIBUTE: Occupation: Engineer", + ] + }, ) assert "Updated peer card" in result @@ -1038,7 +1048,7 @@ class TestUpdatePeerCard: observed=peer2.name, ) assert peer_card is not None - assert "Name: John" in peer_card + assert "IDENTITY: Name: John" in peer_card async def test_deduplicates_and_caps_peer_card( self, @@ -1050,8 +1060,15 @@ class TestUpdatePeerCard: 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)]) + oversized = [ + "IDENTITY: Name: John", + " IDENTITY: Name: John ", + "", + " ", + ] + oversized.extend( + [f"IDENTITY: Aliases: alias-{i}" for i in range(MAX_PEER_CARD_FACTS + 5)] + ) await _handle_update_peer_card(ctx, {"content": oversized}) @@ -1066,7 +1083,7 @@ class TestUpdatePeerCard: 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 + assert peer_card.count("IDENTITY: Name: John") == 1 async def test_none_content_preserves_existing_card( self, @@ -1080,7 +1097,8 @@ class TestUpdatePeerCard: # First, create a valid peer card await _handle_update_peer_card( - ctx, {"content": ["Name: Alice", "Location: NYC"]} + ctx, + {"content": ["IDENTITY: Name: Alice", "ATTRIBUTE: Location: NYC"]}, ) # Now attempt to update with None — should be a no-op @@ -1097,7 +1115,7 @@ class TestUpdatePeerCard: observed=peer2.name, ) assert peer_card is not None - assert "Name: Alice" in peer_card + assert "IDENTITY: Name: Alice" in peer_card async def test_empty_list_preserves_existing_card( self, @@ -1110,7 +1128,10 @@ class TestUpdatePeerCard: ctx = make_tool_context() # First, create a valid peer card - await _handle_update_peer_card(ctx, {"content": ["Name: Bob", "Age: 30"]}) + await _handle_update_peer_card( + ctx, + {"content": ["IDENTITY: Name: Bob", "ATTRIBUTE: Age: 30"]}, + ) # Now attempt to update with empty list — should be a no-op result = await _handle_update_peer_card(ctx, {"content": []}) @@ -1126,7 +1147,151 @@ class TestUpdatePeerCard: observed=peer2.name, ) assert peer_card is not None - assert "Name: Bob" in peer_card + assert "IDENTITY: Name: Bob" in peer_card + + async def test_rejects_entries_without_allowed_prefix( + self, + db_session: AsyncSession, + tool_test_data: Any, + make_tool_context: Callable[..., ToolContext], + ): + """Entries without an allowed prefix are dropped; valid entries pass through.""" + from src.utils.types import ToolResult + + workspace, peer1, peer2, _, _, _ = tool_test_data + ctx = make_tool_context() + + result = await _handle_update_peer_card( + ctx, + { + "content": [ + "IDENTITY: Name: Carol", + "Age: 39+", # rejected: no prefix + "Daughter: Keyan", # rejected: no prefix + "TRAIT: Methodical", # rejected: TRAIT not allowed + "PREFERENCE: Tea", # rejected: bare PREFERENCE not allowed + "ATTRIBUTE: Location: Germantown, TN", + ] + }, + ) + + # Partial-reject success path must surface the rejection in the tool + # response so the model can re-emit the dropped entries (with correct + # prefixes) on a retry instead of silently losing them. + assert isinstance(result, ToolResult) + content_lower = str(result).lower() + assert "updated peer card" in content_lower + assert "rejected 4 of 6" in content_lower + # At least one rejected sample should appear so the model knows what + # to fix. + assert "age: 39+" in content_lower or "trait: methodical" in content_lower + assert result.metadata is not None + assert result.metadata["peer_card_updated"] is True + assert result.metadata["facts_count"] == 2 + assert result.metadata["rejected_count"] == 4 + + await db_session.refresh(peer1) + 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 peer_card == [ + "IDENTITY: Name: Carol", + "ATTRIBUTE: Location: Germantown, TN", + ] + + async def test_all_entries_rejected_preserves_existing_card( + self, + db_session: AsyncSession, + tool_test_data: Any, + make_tool_context: Callable[..., ToolContext], + ): + """When every entry fails validation, the existing card is preserved.""" + workspace, peer1, peer2, _, _, _ = tool_test_data + ctx = make_tool_context() + + await _handle_update_peer_card(ctx, {"content": ["IDENTITY: Name: Dana"]}) + + result = await _handle_update_peer_card( + ctx, + { + "content": [ + "TRAIT: Detail-oriented", + "PREFERENCE: Coffee", + "Random unprefixed line", + ] + }, + ) + assert "rejected" in str(result).lower() + + await db_session.refresh(peer1) + peer_card = await crud.get_peer_card( + db_session, + workspace_name=workspace.name, + observer=peer1.name, + observed=peer2.name, + ) + assert peer_card == ["IDENTITY: Name: Dana"] + + +class TestPeerCardEntryValidator: + """Unit tests for the pure structural validator.""" + + @pytest.mark.parametrize( + "entry", + [ + "IDENTITY: Name: Alice", + "ATTRIBUTE: Location: NYC", + "ATTRIBUTE: Prefers tea", + "RELATIONSHIP: Spouse: Bob", + "RELATIONSHIP: Maintainer: vineeth", + "INSTRUCTION: Call me Vee", + "INSTRUCTION: Never push to main without review", + ], + ) + def test_accepts_well_formed_entries(self, entry: str): + assert _validate_peer_card_entry(entry) is True + + @pytest.mark.parametrize( + "entry", + [ + "", + " ", + "Name: Alice", # missing prefix + "Age: 39+", # missing prefix + "Daughter: Keyan", # missing prefix + "TRAIT: Methodical", # disallowed kind + "PREFERENCE: Tea", # disallowed kind + "identity: name: alice", # wrong case + "IDENTITY:Name: Alice", # missing space after colon + "IDENTITY: ", # empty body + "IDENTITY: ", # whitespace-only body + ], + ) + def test_rejects_malformed_entries(self, entry: str): + assert _validate_peer_card_entry(entry) is False + + def test_rejects_over_length_cap(self): + long_value = "x" * (MAX_PEER_CARD_ENTRY_LENGTH + 1) + assert _validate_peer_card_entry(f"IDENTITY: Name: {long_value}") is False + + def test_accepts_at_length_cap(self): + # Build an entry exactly at the cap. + prefix = "IDENTITY: " + body = "x" * (MAX_PEER_CARD_ENTRY_LENGTH - len(prefix)) + assert _validate_peer_card_entry(prefix + body) is True + + def test_allowed_prefixes_constant_is_complete(self): + # Guard against silent drift between the prompt and the validator. + assert PEER_CARD_ALLOWED_PREFIXES == ( + "IDENTITY:", + "ATTRIBUTE:", + "RELATIONSHIP:", + "INSTRUCTION:", + ) @pytest.mark.asyncio