From e0131e020222ee5d9f841e009fededc2a5ebcdfe Mon Sep 17 00:00:00 2001 From: jkbennitt Date: Tue, 9 Jun 2026 21:27:52 -0400 Subject: [PATCH 01/12] fix(types): resolve all 32 mypy strict errors - Type RateLimitError.__init__ explicitly (no more LSP-violating **kwargs) - Annotate _get_client() and client_kwargs in both cloud providers - Add return type to AnthropicProvider._format_messages - Parametrize ProviderRegistry._providers as type[BaseProvider] - Type CentralPost async queue as asyncio.Queue[Message] and lifecycle callbacks as Callable[[str], None] - Narrow event bus reference before emit in EventEmitterMixin - HelixGeometry/HelixConfig turns: int -> float (fractional turns are geometrically valid; aligns with YAML loader and documented 2.0 default) - Fix keyword-loop variable shadowing in ContentAnalyzer - Annotate CLI _TEMPLATES, collaboration tuples, SQLite count() cast - Add types-PyYAML to dev deps; ignore_missing_imports for optional provider SDKs (anthropic/openai/tiktoken) Co-authored-by: Caleb Gross <209704970+CalebisGross@users.noreply.github.com> Co-Authored-By: Claude Fable 5 --- pyproject.toml | 6 ++++++ src/felix_agent_sdk/cli/init_command.py | 3 ++- .../communication/central_post.py | 6 +++--- src/felix_agent_sdk/communication/registry.py | 4 ++-- src/felix_agent_sdk/core/helix.py | 8 ++++---- src/felix_agent_sdk/events/mixins.py | 5 +++-- src/felix_agent_sdk/memory/backends/sqlite.py | 2 +- src/felix_agent_sdk/providers/anthropic.py | 16 +++++++++------- src/felix_agent_sdk/providers/errors.py | 10 ++++++++-- src/felix_agent_sdk/providers/openai_provider.py | 2 +- src/felix_agent_sdk/providers/registry.py | 15 ++++++++++----- src/felix_agent_sdk/spawning/content_analyzer.py | 4 ++-- src/felix_agent_sdk/spawning/spawner.py | 2 +- 13 files changed, 52 insertions(+), 31 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 1853342..b5c922b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -48,6 +48,7 @@ dev = [ "pytest-cov>=4.0", "ruff>=0.1", "mypy>=1.5", + "types-PyYAML>=6.0", "mkdocs-material>=9.0", ] @@ -73,3 +74,8 @@ line-length = 100 [tool.mypy] python_version = "3.10" strict = true + +[[tool.mypy.overrides]] +# Optional provider SDKs — not installed in the base dev environment. +module = ["anthropic", "anthropic.*", "openai", "openai.*", "tiktoken", "tiktoken.*"] +ignore_missing_imports = true diff --git a/src/felix_agent_sdk/cli/init_command.py b/src/felix_agent_sdk/cli/init_command.py index 936c80a..ebd095b 100644 --- a/src/felix_agent_sdk/cli/init_command.py +++ b/src/felix_agent_sdk/cli/init_command.py @@ -4,8 +4,9 @@ import sys from pathlib import Path +from typing import Any, Dict -_TEMPLATES = { +_TEMPLATES: Dict[str, Dict[str, Any]] = { "research": { "team": [ {"type": "research"}, diff --git a/src/felix_agent_sdk/communication/central_post.py b/src/felix_agent_sdk/communication/central_post.py index 9b44199..6f7fd3d 100644 --- a/src/felix_agent_sdk/communication/central_post.py +++ b/src/felix_agent_sdk/communication/central_post.py @@ -95,10 +95,10 @@ def __init__( self._total_messages_processed: int = 0 # Async message queue (lazy — created on first async use) - self._async_queue: Optional[asyncio.Queue] = None # type: ignore[type-arg] + self._async_queue: Optional[asyncio.Queue[Message]] = None # Lifecycle callbacks: event -> list of callables - self._lifecycle_callbacks: Dict[AgentLifecycleEvent, List[Callable]] = { + self._lifecycle_callbacks: Dict[AgentLifecycleEvent, List[Callable[[str], None]]] = { event: [] for event in AgentLifecycleEvent } @@ -264,7 +264,7 @@ def process_next_message(self) -> Optional[Message]: # Async message queue # ------------------------------------------------------------------ - def _ensure_async_queue(self) -> asyncio.Queue: # type: ignore[type-arg] + def _ensure_async_queue(self) -> asyncio.Queue[Message]: """Lazily create the async queue on first use.""" if self._async_queue is None: self._async_queue = asyncio.Queue() diff --git a/src/felix_agent_sdk/communication/registry.py b/src/felix_agent_sdk/communication/registry.py index d9b8889..4971819 100644 --- a/src/felix_agent_sdk/communication/registry.py +++ b/src/felix_agent_sdk/communication/registry.py @@ -8,7 +8,7 @@ import logging import time -from typing import Any, Dict, List, Optional +from typing import Any, Dict, List, Optional, Tuple from felix_agent_sdk.core.helix import ANALYSIS_END, EXPLORATION_END @@ -36,7 +36,7 @@ def __init__(self) -> None: # agent_id -> performance metrics dict self._performance: Dict[str, Dict[str, Any]] = {} # agent_id -> list of (influenced_agent_id, timestamp) tuples - self._collaborations: Dict[str, List[tuple]] = {} + self._collaborations: Dict[str, List[Tuple[str, float]]] = {} # rolling confidence window for trend calculation self._confidence_history: List[float] = [] diff --git a/src/felix_agent_sdk/core/helix.py b/src/felix_agent_sdk/core/helix.py index aa18541..901c1a5 100644 --- a/src/felix_agent_sdk/core/helix.py +++ b/src/felix_agent_sdk/core/helix.py @@ -37,7 +37,7 @@ def __init__( top_radius: float, bottom_radius: float, height: float, - turns: int, + turns: float, ) -> None: """Initialize helix with geometric parameters. @@ -45,7 +45,7 @@ def __init__( top_radius: Radius at the top of the helix (t=0, z=height). bottom_radius: Radius at the bottom of the helix (t=1, z=0). height: Total vertical height of the helix. - turns: Number of complete rotations from top to bottom. + turns: Number of rotations from top to bottom (fractional turns allowed). Raises: ValueError: If parameters are invalid. @@ -62,7 +62,7 @@ def _validate_parameters( top_radius: float, bottom_radius: float, height: float, - turns: int, + turns: float, ) -> None: """Validate helix parameters for mathematical consistency.""" if top_radius <= bottom_radius: @@ -223,7 +223,7 @@ class HelixConfig: top_radius: float bottom_radius: float height: float - turns: int + turns: float def __post_init__(self) -> None: """Validate config parameters at construction time.""" diff --git a/src/felix_agent_sdk/events/mixins.py b/src/felix_agent_sdk/events/mixins.py index cc9c275..ada40ca 100644 --- a/src/felix_agent_sdk/events/mixins.py +++ b/src/felix_agent_sdk/events/mixins.py @@ -40,14 +40,15 @@ def emit_event( data: Arbitrary payload dict. source: Override the default source identifier. """ - if getattr(self, "_event_bus", None) is None: + bus = getattr(self, "_event_bus", None) + if bus is None: return event = FelixEvent( event_type=event_type, source=source or self._default_event_source(), data=data or {}, ) - self._event_bus.emit(event) + bus.emit(event) def _default_event_source(self) -> str: """Return a default source string. Override for richer identification.""" diff --git a/src/felix_agent_sdk/memory/backends/sqlite.py b/src/felix_agent_sdk/memory/backends/sqlite.py index 2365012..36901f4 100644 --- a/src/felix_agent_sdk/memory/backends/sqlite.py +++ b/src/felix_agent_sdk/memory/backends/sqlite.py @@ -183,7 +183,7 @@ def count(self, table: str, filters: Optional[dict[str, Any]] = None) -> int: if where_parts: sql += " WHERE " + " AND ".join(where_parts) - return self._conn.execute(sql, params).fetchone()[0] + return int(self._conn.execute(sql, params).fetchone()[0]) def search_text( self, diff --git a/src/felix_agent_sdk/providers/anthropic.py b/src/felix_agent_sdk/providers/anthropic.py index fbf2738..0d9e395 100644 --- a/src/felix_agent_sdk/providers/anthropic.py +++ b/src/felix_agent_sdk/providers/anthropic.py @@ -6,7 +6,7 @@ from __future__ import annotations import os -from typing import Any, Dict, Iterator, List, Optional, Sequence +from typing import Any, Dict, Iterator, List, Optional, Sequence, Tuple from .base import BaseProvider from .errors import ( @@ -58,7 +58,7 @@ def __init__( ) super().__init__(config) - def _get_client(self): + def _get_client(self) -> Any: """Lazy-initialize the Anthropic client.""" if self._client is None: try: @@ -68,7 +68,7 @@ def _get_client(self): "Anthropic provider requires the 'anthropic' package. " "Install with: pip install felix-agent-sdk[anthropic]" ) - client_kwargs = {} + client_kwargs: Dict[str, Any] = {} if self.config.api_key: client_kwargs["api_key"] = self.config.api_key if self.config.base_url: @@ -82,14 +82,16 @@ def _supports_sampling_params(self) -> bool: """Whether the configured model accepts temperature/top_p/top_k.""" return not self.config.model.startswith(self._SAMPLING_UNSUPPORTED_PREFIXES) - def _format_messages(self, messages: Sequence[ChatMessage]): + def _format_messages( + self, messages: Sequence[ChatMessage] + ) -> Tuple[Optional[str], List[Dict[str, str]]]: """Convert ChatMessages to Anthropic's format. Anthropic uses a separate 'system' parameter rather than a system message in the messages list, so we extract it here. """ - system_content = None - api_messages = [] + system_content: Optional[str] = None + api_messages: List[Dict[str, str]] = [] for msg in messages: if msg.role == MessageRole.SYSTEM: system_content = msg.content @@ -199,7 +201,7 @@ def count_tokens(self, messages: Sequence[ChatMessage]) -> int: model=self.config.model, messages=api_messages, ) - return response.input_tokens + return int(response.input_tokens) except Exception: # Fallback: rough approximation total_chars = sum(len(m.content) for m in messages) diff --git a/src/felix_agent_sdk/providers/errors.py b/src/felix_agent_sdk/providers/errors.py index e339584..49f2d10 100644 --- a/src/felix_agent_sdk/providers/errors.py +++ b/src/felix_agent_sdk/providers/errors.py @@ -27,9 +27,15 @@ class RateLimitError(ProviderError): retry_after: Suggested wait time in seconds before retrying. """ - def __init__(self, message: str, retry_after: Optional[float] = None, **kwargs): # type: ignore[override] + def __init__( + self, + message: str, + retry_after: Optional[float] = None, + provider: str = "", + status_code: Optional[int] = None, + ): self.retry_after = retry_after - super().__init__(message, **kwargs) + super().__init__(message, provider=provider, status_code=status_code) class ModelNotFoundError(ProviderError): diff --git a/src/felix_agent_sdk/providers/openai_provider.py b/src/felix_agent_sdk/providers/openai_provider.py index 47225e5..752ae18 100644 --- a/src/felix_agent_sdk/providers/openai_provider.py +++ b/src/felix_agent_sdk/providers/openai_provider.py @@ -53,7 +53,7 @@ def __init__( ) super().__init__(config) - def _get_client(self): + def _get_client(self) -> Any: """Lazy-initialize the OpenAI client.""" if self._client is None: try: diff --git a/src/felix_agent_sdk/providers/registry.py b/src/felix_agent_sdk/providers/registry.py index 5775e30..4b5359c 100644 --- a/src/felix_agent_sdk/providers/registry.py +++ b/src/felix_agent_sdk/providers/registry.py @@ -31,11 +31,16 @@ class ProviderRegistry: LOCAL_BASE_URL: Local server URL """ - _providers: Dict[str, type] = {} + _providers: Dict[str, type[BaseProvider]] = {} _detection_order: List[str] = [] @classmethod - def register(cls, name: str, provider_class: type, detection_priority: int = 100) -> None: + def register( + cls, + name: str, + provider_class: type[BaseProvider], + detection_priority: int = 100, + ) -> None: """Register a provider class. Args: @@ -54,7 +59,7 @@ def register(cls, name: str, provider_class: type, detection_priority: int = 100 logger.debug(f"Registered provider: {name}") @classmethod - def get(cls, name: str) -> type: + def get(cls, name: str) -> type[BaseProvider]: """Get a provider class by name. Raises: @@ -117,8 +122,8 @@ def auto_detect(cls) -> BaseProvider: # Fall back to local if "local" in cls._providers: logger.info("No cloud API keys found, falling back to local provider") - model_name = os.getenv("FELIX_MODEL", "local-model") - return cls._providers["local"](model=model_name) + local_kwargs: Dict[str, Any] = {"model": os.getenv("FELIX_MODEL", "local-model")} + return cls._providers["local"](**local_kwargs) raise ProviderError( "No provider could be auto-detected. Set FELIX_PROVIDER or " diff --git a/src/felix_agent_sdk/spawning/content_analyzer.py b/src/felix_agent_sdk/spawning/content_analyzer.py index 1c0154a..8afcc3f 100644 --- a/src/felix_agent_sdk/spawning/content_analyzer.py +++ b/src/felix_agent_sdk/spawning/content_analyzer.py @@ -70,8 +70,8 @@ def analyze_coverage(self, results: List[Dict[str, Any]]) -> CoverageReport: # A topic is "sparse" if it appears in fewer than 2 results topic_counts: Dict[str, int] = {} for kw_set in per_result_keywords: - for kw in kw_set: - topic_counts[kw] = topic_counts.get(kw, 0) + 1 + for keyword in kw_set: + topic_counts[keyword] = topic_counts.get(keyword, 0) + 1 sparse = {kw for kw, count in topic_counts.items() if count < 2} covered = all_keywords - sparse diff --git a/src/felix_agent_sdk/spawning/spawner.py b/src/felix_agent_sdk/spawning/spawner.py index 39ea507..ba7066f 100644 --- a/src/felix_agent_sdk/spawning/spawner.py +++ b/src/felix_agent_sdk/spawning/spawner.py @@ -118,7 +118,7 @@ def check_and_spawn( spawn_time=spawn_time, ) if getattr(self, "_event_bus", None) is not None: - agent.set_event_bus(self._event_bus) # type: ignore[arg-type] + agent.set_event_bus(self._event_bus) self._spoke_mgr.create_spoke(agent.agent_id, agent=agent) self._total_spawned += 1 From b294628838a442b89b23f7693b715fc06c4d174d Mon Sep 17 00:00:00 2001 From: jkbennitt Date: Tue, 9 Jun 2026 21:29:39 -0400 Subject: [PATCH 02/12] fix(spawning): propagate agent_type into position_info for gap analysis MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DynamicSpawner._results_to_dicts read position_info['agent_type'], but Agent.get_position_info() never set that key, so ContentAnalyzer always received empty agent types and the type-based gap analysis (e.g. 'spawn an analysis agent when analysis coverage is thin') could never trigger. LLMAgent now overrides get_position_info() to include its agent_type. Adds regression tests on the real agent path — existing tests passed because fixtures hand-built position_info with the key already present. Co-authored-by: Caleb Gross <209704970+CalebisGross@users.noreply.github.com> Co-Authored-By: Claude Fable 5 --- src/felix_agent_sdk/agents/llm_agent.py | 14 ++++++++++++++ tests/unit/test_dynamic_spawner.py | 15 +++++++++++++++ tests/unit/test_llm_agent.py | 12 ++++++++++++ 3 files changed, 41 insertions(+) diff --git a/src/felix_agent_sdk/agents/llm_agent.py b/src/felix_agent_sdk/agents/llm_agent.py index f7b811d..9ca66a5 100644 --- a/src/felix_agent_sdk/agents/llm_agent.py +++ b/src/felix_agent_sdk/agents/llm_agent.py @@ -127,6 +127,20 @@ def __init__( # Feedback calibration (Phase 5+ integration point) self._confidence_calibration_offset: float = 0.0 + # ------------------------------------------------------------------ + # Position info + # ------------------------------------------------------------------ + + def get_position_info(self) -> Dict[str, Any]: + """Position dictionary extended with this agent's type. + + Downstream consumers (DynamicSpawner gap analysis, registry metadata) + rely on ``agent_type`` being present alongside the positional fields. + """ + info = super().get_position_info() + info["agent_type"] = self.agent_type + return info + # ------------------------------------------------------------------ # Temperature # ------------------------------------------------------------------ diff --git a/tests/unit/test_dynamic_spawner.py b/tests/unit/test_dynamic_spawner.py index 6198593..18b738f 100644 --- a/tests/unit/test_dynamic_spawner.py +++ b/tests/unit/test_dynamic_spawner.py @@ -102,6 +102,21 @@ def test_emits_events(self): assert "spawn.triggered" in types assert "spawn.completed" in types + def test_results_to_dicts_sees_real_agent_types(self): + """Regression: position_info from a real LLMAgent must carry + agent_type through to the ContentAnalyzer input dicts.""" + from felix_agent_sdk.agents.llm_agent import LLMAgent, LLMTask + from felix_agent_sdk.core.helix import HelixGeometry + from felix_agent_sdk.spawning.spawner import DynamicSpawner + + helix = HelixGeometry(top_radius=3.0, bottom_radius=0.5, height=8.0, turns=2) + agent = LLMAgent("a1", _mock_provider(), helix, agent_type="analysis") + agent.spawn(0.0) + result = agent.process_task(LLMTask(task_id="t1", description="analyse")) + + dicts = DynamicSpawner._results_to_dicts([result]) + assert dicts[0]["agent_type"] == "analysis" + def test_spawn_completed_has_agent_id(self): bus = EventBus() bus.enable_history() diff --git a/tests/unit/test_llm_agent.py b/tests/unit/test_llm_agent.py index 010d34a..ed4db13 100644 --- a/tests/unit/test_llm_agent.py +++ b/tests/unit/test_llm_agent.py @@ -325,6 +325,18 @@ def test_result_includes_position_info(self, agent, task): assert "progress" in result.position_info assert "phase" in result.position_info + def test_result_position_info_includes_agent_type(self, agent, task): + # Regression: DynamicSpawner gap analysis reads agent_type from + # position_info; it must be populated by real agents, not just + # hand-built test fixtures. + agent.spawn(0.0) + result = agent.process_task(task) + assert result.position_info["agent_type"] == "research" + + def test_get_position_info_includes_agent_type(self, agent): + agent.spawn(0.0) + assert agent.get_position_info()["agent_type"] == "research" + # ------------------------------------------------------------------------- # Helical checkpoints From c633f39a4c5fc3365b59fcf514c686c000c17ece Mon Sep 17 00:00:00 2001 From: jkbennitt Date: Tue, 9 Jun 2026 21:33:32 -0400 Subject: [PATCH 03/12] fix(communication): bound message history, eager async queue, consistent capacity errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - _processed_messages is now a deque bounded by a new message_history_limit constructor param (default 1000). Previously every processed message was retained until shutdown — unbounded memory growth in long-running workflows. - Async queue is constructed eagerly in __init__ (safe on Python 3.10+, asyncio.Queue no longer binds a loop at construction), removing the lazy-init double-creation race. - register_agent and register_agent_id now both raise HubCapacityError at capacity instead of one returning None and the other raising RuntimeError. HubCapacityError subclasses RuntimeError for backward compatibility; exported from felix_agent_sdk and .communication. - register_agent no longer silently swaps an empty-string agent_id for id(agent) — only a missing/None agent_id falls back. Co-authored-by: Caleb Gross <209704970+CalebisGross@users.noreply.github.com> Co-Authored-By: Claude Fable 5 --- src/felix_agent_sdk/__init__.py | 9 ++- src/felix_agent_sdk/communication/__init__.py | 7 +- .../communication/central_post.py | 70 +++++++++++-------- src/felix_agent_sdk/communication/spoke.py | 22 +++--- tests/unit/test_central_post.py | 26 +++++-- 5 files changed, 85 insertions(+), 49 deletions(-) diff --git a/src/felix_agent_sdk/__init__.py b/src/felix_agent_sdk/__init__.py index 117cf52..393ccbc 100644 --- a/src/felix_agent_sdk/__init__.py +++ b/src/felix_agent_sdk/__init__.py @@ -30,7 +30,13 @@ OpenAIProvider, auto_detect_provider, ) -from felix_agent_sdk.communication import CentralPost, Message, MessageType, Spoke +from felix_agent_sdk.communication import ( + CentralPost, + HubCapacityError, + Message, + MessageType, + Spoke, +) from felix_agent_sdk.events import EventBus, EventType, FelixEvent from felix_agent_sdk.memory import ContextCompressor, KnowledgeStore, TaskMemory from felix_agent_sdk.spawning import ConfidenceMonitor, DynamicSpawner @@ -60,6 +66,7 @@ "AgentFactory", # Communication "CentralPost", + "HubCapacityError", "Message", "MessageType", "Spoke", diff --git a/src/felix_agent_sdk/communication/__init__.py b/src/felix_agent_sdk/communication/__init__.py index 1c3846c..926fc44 100644 --- a/src/felix_agent_sdk/communication/__init__.py +++ b/src/felix_agent_sdk/communication/__init__.py @@ -3,7 +3,11 @@ All agent communication routes through CentralPost at O(N) complexity. """ -from felix_agent_sdk.communication.central_post import AgentLifecycleEvent, CentralPost +from felix_agent_sdk.communication.central_post import ( + AgentLifecycleEvent, + CentralPost, + HubCapacityError, +) from felix_agent_sdk.communication.messages import Message, MessageType from felix_agent_sdk.communication.registry import AgentRegistry from felix_agent_sdk.communication.spoke import Spoke, SpokeConnection, SpokeManager @@ -11,6 +15,7 @@ __all__ = [ "CentralPost", "AgentLifecycleEvent", + "HubCapacityError", "Message", "MessageType", "AgentRegistry", diff --git a/src/felix_agent_sdk/communication/central_post.py b/src/felix_agent_sdk/communication/central_post.py index 6f7fd3d..8498bb6 100644 --- a/src/felix_agent_sdk/communication/central_post.py +++ b/src/felix_agent_sdk/communication/central_post.py @@ -10,9 +10,10 @@ import asyncio import logging import time +from collections import deque from enum import Enum from queue import Empty, Queue -from typing import Any, Callable, Dict, List, Optional, TYPE_CHECKING +from typing import Any, Callable, Deque, Dict, List, Optional, TYPE_CHECKING from felix_agent_sdk.communication.messages import Message, MessageType from felix_agent_sdk.communication.registry import AgentRegistry @@ -31,6 +32,14 @@ # --------------------------------------------------------------------------- +class HubCapacityError(RuntimeError): + """Raised when the hub is at ``max_agents`` capacity and a new agent registers. + + Subclasses :class:`RuntimeError` for backward compatibility with callers + that caught ``RuntimeError`` from ``register_agent_id`` in earlier versions. + """ + + class AgentLifecycleEvent(Enum): """Events emitted during an agent's lifecycle.""" @@ -68,6 +77,10 @@ class CentralPost: max_agents: Maximum number of agents that may be registered simultaneously. enable_metrics: Reserved flag for future metrics collection. provider: Optional provider reference (slot — not used by message handling yet). + event_bus: Optional event bus for lifecycle event bridging. + message_history_limit: Maximum number of processed messages retained + for :meth:`get_recent_messages`. Older messages are discarded + (the ``total_messages_processed`` counter is unaffected). """ def __init__( @@ -76,6 +89,7 @@ def __init__( enable_metrics: bool = False, provider: Optional[BaseProvider] = None, event_bus: Optional[EventBus] = None, + message_history_limit: int = 1000, ) -> None: self._max_agents = max_agents self._enable_metrics = enable_metrics @@ -91,11 +105,13 @@ def __init__( # Sync message queue self._message_queue: Queue[Message] = Queue() - self._processed_messages: List[Message] = [] + # Bounded history — prevents unbounded memory growth in long runs. + self._processed_messages: Deque[Message] = deque(maxlen=message_history_limit) self._total_messages_processed: int = 0 - # Async message queue (lazy — created on first async use) - self._async_queue: Optional[asyncio.Queue[Message]] = None + # Async message queue. Safe to construct eagerly on Python >= 3.10: + # asyncio.Queue no longer binds an event loop at construction time. + self._async_queue: asyncio.Queue[Message] = asyncio.Queue() # Lifecycle callbacks: event -> list of callables self._lifecycle_callbacks: Dict[AgentLifecycleEvent, List[Callable[[str], None]]] = { @@ -135,7 +151,7 @@ def total_messages_processed(self) -> int: def register_agent( self, agent: Agent, metadata: Optional[Dict[str, Any]] = None - ) -> Optional[str]: + ) -> str: """Register an agent object and extract metadata from its attributes. Args: @@ -143,21 +159,21 @@ def register_agent( metadata: Extra metadata to merge (takes precedence over auto-extracted). Returns: - The ``agent_id`` string on success, or ``None`` if the hub is at capacity. + The ``agent_id`` string. + + Raises: + HubCapacityError: If the hub is at capacity. """ - agent_id: str = getattr(agent, "agent_id", None) or str(id(agent)) + raw_id = getattr(agent, "agent_id", None) + agent_id: str = str(raw_id) if raw_id is not None else str(id(agent)) if ( len(self._registered_agents) >= self._max_agents and agent_id not in self._registered_agents ): - logger.warning( - "CentralPost at capacity (%d/%d) — cannot register %s", - len(self._registered_agents), - self._max_agents, - agent_id, + raise HubCapacityError( + f"CentralPost at capacity ({self._max_agents} agents) — cannot register {agent_id!r}" ) - return None auto_meta: Dict[str, Any] = {} for attr in ("agent_type", "spawn_time", "confidence", "state"): @@ -184,13 +200,13 @@ def register_agent_id(self, agent_id: str, metadata: Optional[Dict[str, Any]] = The ``agent_id`` string. Raises: - RuntimeError: If the hub is at capacity. + HubCapacityError: If the hub is at capacity. """ if ( len(self._registered_agents) >= self._max_agents and agent_id not in self._registered_agents ): - raise RuntimeError( + raise HubCapacityError( f"CentralPost at capacity ({self._max_agents} agents) — cannot register {agent_id!r}" ) @@ -264,19 +280,13 @@ def process_next_message(self) -> Optional[Message]: # Async message queue # ------------------------------------------------------------------ - def _ensure_async_queue(self) -> asyncio.Queue[Message]: - """Lazily create the async queue on first use.""" - if self._async_queue is None: - self._async_queue = asyncio.Queue() - return self._async_queue - async def queue_message_async(self, message: Message) -> None: """Enqueue a message for asynchronous processing. Args: message: The Message to enqueue. """ - await self._ensure_async_queue().put(message) + await self._async_queue.put(message) logger.debug( "Async queued message %s [%s] from %s", message.message_id, @@ -290,9 +300,8 @@ async def process_next_message_async(self) -> Optional[Message]: Returns: The processed Message, or None if the queue was empty. """ - q = self._ensure_async_queue() try: - message = q.get_nowait() + message = self._async_queue.get_nowait() except asyncio.QueueEmpty: return None @@ -479,7 +488,7 @@ def get_recent_messages( Returns: List of Message objects, most recent last. """ - messages = self._processed_messages + messages: List[Message] = list(self._processed_messages) if message_type is not None: messages = [m for m in messages if m.message_type == message_type] return messages[-count:] @@ -609,12 +618,11 @@ async def shutdown_async(self) -> None: """Shutdown the hub asynchronously, draining the async queue.""" self._is_active = False - if self._async_queue is not None: - while not self._async_queue.empty(): - try: - self._async_queue.get_nowait() - except asyncio.QueueEmpty: - break + while not self._async_queue.empty(): + try: + self._async_queue.get_nowait() + except asyncio.QueueEmpty: + break self._registered_agents.clear() self._connection_times.clear() diff --git a/src/felix_agent_sdk/communication/spoke.py b/src/felix_agent_sdk/communication/spoke.py index d5717b9..1d3ca43 100644 --- a/src/felix_agent_sdk/communication/spoke.py +++ b/src/felix_agent_sdk/communication/spoke.py @@ -16,6 +16,7 @@ from dataclasses import dataclass, field from typing import Any, Callable, Dict, List, Optional, TYPE_CHECKING +from felix_agent_sdk.communication.central_post import HubCapacityError from felix_agent_sdk.communication.messages import Message, MessageType if TYPE_CHECKING: @@ -140,18 +141,15 @@ def connect(self, metadata: Optional[Dict[str, Any]] = None) -> bool: if self._is_connected: return True - if self._agent is not None: - result = self._hub.register_agent(self._agent, metadata) - else: - try: - result = self._hub.register_agent_id(self._agent_id, metadata) - except RuntimeError: - logger.warning( - "Spoke.connect: hub at capacity, could not register %s", self._agent_id - ) - return False - - if result is None: + try: + if self._agent is not None: + self._hub.register_agent(self._agent, metadata) + else: + self._hub.register_agent_id(self._agent_id, metadata) + except HubCapacityError: + logger.warning( + "Spoke.connect: hub at capacity, could not register %s", self._agent_id + ) return False self._is_connected = True diff --git a/tests/unit/test_central_post.py b/tests/unit/test_central_post.py index 6a3caf2..111c0a7 100644 --- a/tests/unit/test_central_post.py +++ b/tests/unit/test_central_post.py @@ -9,6 +9,7 @@ from felix_agent_sdk.communication.central_post import ( AgentLifecycleEvent, CentralPost, + HubCapacityError, ) from felix_agent_sdk.communication.messages import Message, MessageType from felix_agent_sdk.communication.registry import AgentRegistry @@ -101,15 +102,19 @@ def test_register_with_extra_metadata(self, central_post): central_post.register_agent(agent, metadata={"priority": "high"}) assert central_post.is_agent_registered("agent-001") - def test_register_at_capacity_returns_none(self, central_post): + def test_register_at_capacity_raises(self, central_post): # Fill to capacity (max_agents=10) for i in range(10): agent = _make_mock_agent(f"agent-{i:03d}") central_post.register_agent(agent) - # 11th registration should fail + # 11th registration should fail consistently with register_agent_id overflow_agent = _make_mock_agent("overflow") - result = central_post.register_agent(overflow_agent) - assert result is None + with pytest.raises(HubCapacityError, match="at capacity"): + central_post.register_agent(overflow_agent) + + def test_capacity_error_is_runtime_error(self, central_post): + """Backward compat: pre-0.3.0 callers caught RuntimeError.""" + assert issubclass(HubCapacityError, RuntimeError) def test_re_register_same_agent_succeeds(self, central_post): """Re-registering an already registered agent updates, doesn't fail.""" @@ -252,6 +257,19 @@ def test_get_recent_messages_filtered_by_type(self, central_post): assert len(status_msgs) == 1 assert status_msgs[0].message_type == MessageType.STATUS_UPDATE + def test_message_history_is_bounded(self): + hub = CentralPost(max_agents=5, message_history_limit=3) + for i in range(5): + hub.queue_message(_make_message(sender_id=f"agent-{i}")) + hub.process_next_message() + recent = hub.get_recent_messages(count=10) + assert len(recent) == 3 + # Oldest messages evicted, most recent retained + assert [m.sender_id for m in recent] == ["agent-2", "agent-3", "agent-4"] + # Counter is not affected by eviction + assert hub.total_messages_processed == 5 + hub.shutdown() + def test_status_update_handler_updates_registry(self, central_post): central_post.register_agent_id("agent-1") msg = _make_message( From fa0a5be1cff7ca628c736b541e4ece42d176d088 Mon Sep 17 00:00:00 2001 From: jkbennitt Date: Tue, 9 Jun 2026 21:37:26 -0400 Subject: [PATCH 04/12] fix(memory): harden SQLite backend and soft-delete semantics - Validate all interpolated SQL identifiers (table names, schema columns, filter fields) against a strict identifier pattern; order_by is also checked against the table's known columns. Closes the ORDER BY injection vector flagged in review. - Share one connection across threads safely: check_same_thread=False plus an RLock serialising all database access. - Close the connection if PRAGMA setup fails during __init__ (no leak). - Add context manager protocol (__enter__/__exit__) to SQLiteBackend. - KnowledgeStore.get_relationships now excludes relationships involving soft-deleted entries, matching entry-query soft-delete semantics. Co-authored-by: Caleb Gross <209704970+CalebisGross@users.noreply.github.com> Co-Authored-By: Claude Fable 5 --- src/felix_agent_sdk/memory/backends/sqlite.py | 131 +++++++++++++----- src/felix_agent_sdk/memory/knowledge_store.py | 29 +++- tests/unit/test_backends.py | 55 ++++++++ tests/unit/test_knowledge_store.py | 16 +++ 4 files changed, 193 insertions(+), 38 deletions(-) diff --git a/src/felix_agent_sdk/memory/backends/sqlite.py b/src/felix_agent_sdk/memory/backends/sqlite.py index 36901f4..d20bce8 100644 --- a/src/felix_agent_sdk/memory/backends/sqlite.py +++ b/src/felix_agent_sdk/memory/backends/sqlite.py @@ -9,7 +9,9 @@ import json import logging +import re import sqlite3 +import threading from typing import Any, Optional from felix_agent_sdk.memory.backends.base import BaseBackend @@ -24,10 +26,30 @@ "$lte": "<=", } +# Column/table names are interpolated into SQL and must be plain identifiers. +_IDENTIFIER_RE = re.compile(r"^[A-Za-z_][A-Za-z0-9_]*$") + + +def _validate_identifier(name: str, context: str) -> None: + """Reject anything that is not a bare SQL identifier. + + Raises: + ValueError: If *name* contains characters outside [A-Za-z0-9_] + or does not start with a letter/underscore. + """ + if not _IDENTIFIER_RE.match(name): + raise ValueError(f"Invalid SQL identifier for {context}: {name!r}") + class SQLiteBackend(BaseBackend): """SQLite-backed storage with FTS5 support. + A single connection is shared across threads; all access is serialised + through an internal lock, so the backend is safe for multi-threaded use + (though writes from many threads will contend on that lock). + + Can be used as a context manager to guarantee the connection is closed. + Args: db_path: Path to the database file, or ``":memory:"`` for an in-memory database (the default). @@ -35,22 +57,41 @@ class SQLiteBackend(BaseBackend): def __init__(self, db_path: str = ":memory:") -> None: self._db_path = db_path - self._conn = sqlite3.connect(db_path) - self._conn.execute("PRAGMA journal_mode=WAL") - self._conn.execute("PRAGMA foreign_keys=ON") + self._lock = threading.RLock() + self._conn = sqlite3.connect(db_path, check_same_thread=False) + try: + self._conn.execute("PRAGMA journal_mode=WAL") + self._conn.execute("PRAGMA foreign_keys=ON") + except Exception: + self._conn.close() + raise self._fts_tables: set[str] = set() self._fts_text_cols: dict[str, list[str]] = {} self._initialized_tables: set[str] = set() self._table_columns: dict[str, set[str]] = {} + def __enter__(self) -> SQLiteBackend: + return self + + def __exit__(self, *exc_info: Any) -> None: + self.close() + # ------------------------------------------------------------------ # Schema # ------------------------------------------------------------------ def initialize(self, table: str, schema: dict[str, str]) -> None: + with self._lock: + self._initialize_locked(table, schema) + + def _initialize_locked(self, table: str, schema: dict[str, str]) -> None: if table in self._initialized_tables: return + _validate_identifier(table, "table name") + for col_name in schema: + _validate_identifier(col_name, "column name") + # Build column definitions from schema hints col_defs = ["_id TEXT PRIMARY KEY"] for col_name, col_type in schema.items(): @@ -105,41 +146,47 @@ def store(self, table: str, record_id: str, data: dict[str, Any]) -> None: valid_cols = self._table_columns.get(table) if valid_cols: data_copy = {k: v for k, v in data_copy.items() if k in valid_cols} + else: + for col_name in data_copy: + _validate_identifier(col_name, "column name") cols = list(data_copy.keys()) placeholders = ", ".join(["?"] * len(cols)) col_names = ", ".join(cols) - self._conn.execute( - f"INSERT OR REPLACE INTO [{table}] ({col_names}) VALUES ({placeholders})", - [data_copy[c] for c in cols], - ) + with self._lock: + self._conn.execute( + f"INSERT OR REPLACE INTO [{table}] ({col_names}) VALUES ({placeholders})", + [data_copy[c] for c in cols], + ) - # Sync FTS - if table in self._fts_tables: - self._sync_fts(table, record_id, data_copy) + # Sync FTS + if table in self._fts_tables: + self._sync_fts(table, record_id, data_copy) - self._conn.commit() + self._conn.commit() def get(self, table: str, record_id: str) -> Optional[dict[str, Any]]: - cur = self._conn.execute(f"SELECT * FROM [{table}] WHERE _id = ?", (record_id,)) - row = cur.fetchone() - if row is None: - return None - return self._row_to_dict(cur.description, row) + with self._lock: + cur = self._conn.execute(f"SELECT * FROM [{table}] WHERE _id = ?", (record_id,)) + row = cur.fetchone() + if row is None: + return None + return self._row_to_dict(cur.description, row) def delete(self, table: str, record_id: str) -> bool: - # Delete FTS entry first - if table in self._fts_tables: - fts_name = f"{table}_fts" - self._conn.execute( - f"DELETE FROM [{fts_name}] WHERE record_id = ?", - (record_id,), - ) + with self._lock: + # Delete FTS entry first + if table in self._fts_tables: + fts_name = f"{table}_fts" + self._conn.execute( + f"DELETE FROM [{fts_name}] WHERE record_id = ?", + (record_id,), + ) - cur = self._conn.execute(f"DELETE FROM [{table}] WHERE _id = ?", (record_id,)) - self._conn.commit() - return cur.rowcount > 0 + cur = self._conn.execute(f"DELETE FROM [{table}] WHERE _id = ?", (record_id,)) + self._conn.commit() + return cur.rowcount > 0 # ------------------------------------------------------------------ # Query @@ -162,8 +209,14 @@ def query( sql += " WHERE " + " AND ".join(where_parts) if order_by: + _validate_identifier(order_by, "order_by") + known_cols = self._table_columns.get(table) + if known_cols and order_by not in known_cols: + raise ValueError( + f"Unknown order_by column {order_by!r} for table {table!r}" + ) direction = "ASC" if ascending else "DESC" - sql += f" ORDER BY {order_by} {direction}" + sql += f" ORDER BY [{order_by}] {direction}" if limit is not None: sql += " LIMIT ?" @@ -172,8 +225,9 @@ def query( sql += " OFFSET ?" params.append(offset) - cur = self._conn.execute(sql, params) - return [self._row_to_dict(cur.description, row) for row in cur.fetchall()] + with self._lock: + cur = self._conn.execute(sql, params) + return [self._row_to_dict(cur.description, row) for row in cur.fetchall()] def count(self, table: str, filters: Optional[dict[str, Any]] = None) -> int: sql = f"SELECT COUNT(*) FROM [{table}]" @@ -183,7 +237,8 @@ def count(self, table: str, filters: Optional[dict[str, Any]] = None) -> int: if where_parts: sql += " WHERE " + " AND ".join(where_parts) - return int(self._conn.execute(sql, params).fetchone()[0]) + with self._lock: + return int(self._conn.execute(sql, params).fetchone()[0]) def search_text( self, @@ -201,7 +256,8 @@ def search_text( # ------------------------------------------------------------------ def close(self) -> None: - self._conn.close() + with self._lock: + self._conn.close() # ------------------------------------------------------------------ # Internal helpers @@ -226,6 +282,7 @@ def _build_where( parts: list[str] = [] for field, value in filters.items(): + _validate_identifier(field, "filter field") if isinstance(value, dict): # Operator filter: {"$gt": 5} for op, op_val in value.items(): @@ -274,8 +331,9 @@ def _fts_search(self, table: str, query: str, limit: int) -> list[dict[str, Any] f"WHERE [{fts_name}] MATCH ? " f"ORDER BY rank LIMIT ?" ) - cur = self._conn.execute(sql, (query, limit)) - return [self._row_to_dict(cur.description, row) for row in cur.fetchall()] + with self._lock: + cur = self._conn.execute(sql, (query, limit)) + return [self._row_to_dict(cur.description, row) for row in cur.fetchall()] def _like_search( self, @@ -292,8 +350,9 @@ def _like_search( conditions = [f"{f} LIKE ?" for f in safe_fields] sql = f"SELECT * FROM [{table}] WHERE " + " OR ".join(conditions) + " LIMIT ?" params = [f"%{query}%"] * len(safe_fields) + [limit] - cur = self._conn.execute(sql, params) - return [self._row_to_dict(cur.description, row) for row in cur.fetchall()] + with self._lock: + cur = self._conn.execute(sql, params) + return [self._row_to_dict(cur.description, row) for row in cur.fetchall()] @staticmethod def _row_to_dict(description: Any, row: tuple[Any, ...]) -> dict[str, Any]: @@ -307,7 +366,7 @@ def _row_to_dict(description: Any, row: tuple[Any, ...]) -> dict[str, Any]: parsed = json.loads(value) if isinstance(parsed, (dict, list)): value = parsed - except (json.JSONDecodeError, ValueError): + except ValueError: pass result[col_name] = value return result diff --git a/src/felix_agent_sdk/memory/knowledge_store.py b/src/felix_agent_sdk/memory/knowledge_store.py index dbf2cde..a72d277 100644 --- a/src/felix_agent_sdk/memory/knowledge_store.py +++ b/src/felix_agent_sdk/memory/knowledge_store.py @@ -426,10 +426,35 @@ def add_relationship( return True def get_relationships(self, knowledge_id: str) -> list[dict[str, Any]]: - """Get all relationships for an entry (bidirectional).""" + """Get all relationships for an entry (bidirectional). + + Relationships involving soft-deleted entries are excluded, matching + the soft-delete semantics of entry queries. Returns an empty list + if the entry itself is deleted or does not exist. + """ + if not self._is_active_entry(knowledge_id): + return [] + outgoing = self._backend.query(_REL_TABLE, filters={"source_id": knowledge_id}) incoming = self._backend.query(_REL_TABLE, filters={"target_id": knowledge_id}) - return outgoing + incoming + + results: list[dict[str, Any]] = [] + for rel in outgoing + incoming: + other_id = ( + rel.get("target_id") + if rel.get("source_id") == knowledge_id + else rel.get("source_id") + ) + if other_id and self._is_active_entry(str(other_id)): + results.append(rel) + return results + + def _is_active_entry(self, knowledge_id: str) -> bool: + """True if the entry exists and is not soft-deleted.""" + data = self._backend.get(_TABLE, knowledge_id) + if data is None: + return False + return not KnowledgeEntry.from_dict(data).is_deleted # ------------------------------------------------------------------ # Success rate diff --git a/tests/unit/test_backends.py b/tests/unit/test_backends.py index 5a81c03..c068feb 100644 --- a/tests/unit/test_backends.py +++ b/tests/unit/test_backends.py @@ -47,6 +47,61 @@ def test_idempotent_init(self, memory_backend): memory_backend.initialize("t", _SCHEMA) # should not raise assert memory_backend.count("t") == 0 + def test_context_manager_closes_connection(self): + with SQLiteBackend() as backend: + backend.initialize("t", _SCHEMA) + backend.store("t", "1", {"name": "x", "value": 1.0, "tags": ""}) + import sqlite3 + + with pytest.raises(sqlite3.ProgrammingError): + backend.get("t", "1") + + +class TestSQLiteBackendHardening: + def test_order_by_rejects_injection(self, memory_backend): + memory_backend.initialize("t", _SCHEMA) + with pytest.raises(ValueError, match="identifier"): + memory_backend.query("t", order_by="name; DROP TABLE t--") + + def test_order_by_rejects_unknown_column(self, memory_backend): + memory_backend.initialize("t", _SCHEMA) + with pytest.raises(ValueError, match="Unknown order_by"): + memory_backend.query("t", order_by="not_a_column") + + def test_filter_field_rejects_injection(self, memory_backend): + memory_backend.initialize("t", _SCHEMA) + with pytest.raises(ValueError, match="identifier"): + memory_backend.query("t", filters={"name = '' OR 1=1 --": "x"}) + + def test_table_name_rejects_injection(self, memory_backend): + with pytest.raises(ValueError, match="identifier"): + memory_backend.initialize("t]; DROP TABLE t--", _SCHEMA) + + def test_multithreaded_access(self, memory_backend): + import threading + + memory_backend.initialize("t", _SCHEMA) + errors: list[Exception] = [] + + def writer(start: int) -> None: + try: + for i in range(start, start + 25): + memory_backend.store( + "t", f"r{i}", {"name": f"n{i}", "value": float(i), "tags": ""} + ) + memory_backend.get("t", f"r{i}") + except Exception as e: # pragma: no cover - failure path + errors.append(e) + + threads = [threading.Thread(target=writer, args=(n * 25,)) for n in range(4)] + for t in threads: + t.start() + for t in threads: + t.join() + + assert errors == [] + assert memory_backend.count("t") == 100 + class TestSQLiteBackendCRUD: def test_store_and_get(self, memory_backend): diff --git a/tests/unit/test_knowledge_store.py b/tests/unit/test_knowledge_store.py index f592516..8340d6f 100644 --- a/tests/unit/test_knowledge_store.py +++ b/tests/unit/test_knowledge_store.py @@ -257,6 +257,22 @@ def test_bidirectional_lookup(self, knowledge_store): rels = knowledge_store.get_relationships(kid2) assert len(rels) >= 1 + def test_relationships_exclude_soft_deleted_entries(self, knowledge_store): + kid1 = knowledge_store.add_entry( + KnowledgeType.AGENT_INSIGHT, {"x": 1}, + ConfidenceLevel.HIGH, "a", "d", + ) + kid2 = knowledge_store.add_entry( + KnowledgeType.AGENT_INSIGHT, {"x": 2}, + ConfidenceLevel.HIGH, "b", "d", + ) + knowledge_store.add_relationship(kid1, kid2) + + knowledge_store.delete_entry(kid2) + assert knowledge_store.get_relationships(kid1) == [] + # The deleted entry itself reports no relationships + assert knowledge_store.get_relationships(kid2) == [] + # ------------------------------------------------------------------ # Success rate / cleanup / summary From 59f692bb55599ed146d690e2f529a188e952792c Mon Sep 17 00:00:00 2001 From: jkbennitt Date: Tue, 9 Jun 2026 21:50:53 -0400 Subject: [PATCH 05/12] feat(providers): async support, typed error translation, key masking - AnthropicProvider and OpenAIProvider (and LocalProvider via inheritance) now implement acomplete()/astream() using the vendor SDKs' async clients. Base-class defaults still raise for custom sync-only providers. - Error translation now reads the HTTP status_code carried by vendor SDK exceptions and populates RateLimitError.retry_after from the Retry-After header (extract_status_code/extract_retry_after helpers in providers.errors). Message-based fallbacks unchanged. - ProviderConfig.__repr__ masks the API key so configs are safe to log. - OpenAI stream() stops consuming after the final usage chunk. - ProviderRegistry: detection priorities are stored per provider and honored by auto_detect (the old sort reset existing priorities to 100 on every register, and auto-detect ignored the order entirely). - Deduplicate request building into _build_create_kwargs/_to_completion_result shared by sync and async paths; document that validate() issues a real (tiny) completion. Co-Authored-By: Claude Fable 5 --- src/felix_agent_sdk/providers/anthropic.py | 214 ++++++++++----- src/felix_agent_sdk/providers/base.py | 11 +- src/felix_agent_sdk/providers/errors.py | 32 +++ .../providers/openai_provider.py | 245 ++++++++++++------ src/felix_agent_sdk/providers/registry.py | 35 +-- src/felix_agent_sdk/providers/types.py | 10 + tests/unit/test_anthropic_provider.py | 125 +++++++++ tests/unit/test_base_provider.py | 20 ++ tests/unit/test_errors.py | 42 +++ tests/unit/test_openai_provider.py | 156 +++++++++++ tests/unit/test_registry.py | 31 ++- 11 files changed, 753 insertions(+), 168 deletions(-) diff --git a/src/felix_agent_sdk/providers/anthropic.py b/src/felix_agent_sdk/providers/anthropic.py index 0d9e395..3c84297 100644 --- a/src/felix_agent_sdk/providers/anthropic.py +++ b/src/felix_agent_sdk/providers/anthropic.py @@ -6,7 +6,7 @@ from __future__ import annotations import os -from typing import Any, Dict, Iterator, List, Optional, Sequence, Tuple +from typing import Any, AsyncIterator, Dict, Iterator, List, Optional, Sequence, Tuple from .base import BaseProvider from .errors import ( @@ -15,6 +15,8 @@ ModelNotFoundError, ProviderError, RateLimitError, + extract_retry_after, + extract_status_code, ) from .types import ChatMessage, CompletionResult, MessageRole, ProviderConfig, StreamChunk @@ -58,26 +60,40 @@ def __init__( ) super().__init__(config) + def _import_sdk(self) -> Any: + try: + import anthropic + except ImportError: + raise ImportError( + "Anthropic provider requires the 'anthropic' package. " + "Install with: pip install felix-agent-sdk[anthropic]" + ) + return anthropic + + def _client_kwargs(self) -> Dict[str, Any]: + client_kwargs: Dict[str, Any] = {} + if self.config.api_key: + client_kwargs["api_key"] = self.config.api_key + if self.config.base_url: + client_kwargs["base_url"] = self.config.base_url + client_kwargs["max_retries"] = self.config.max_retries + client_kwargs["timeout"] = self.config.timeout + return client_kwargs + def _get_client(self) -> Any: """Lazy-initialize the Anthropic client.""" if self._client is None: - try: - import anthropic - except ImportError: - raise ImportError( - "Anthropic provider requires the 'anthropic' package. " - "Install with: pip install felix-agent-sdk[anthropic]" - ) - client_kwargs: Dict[str, Any] = {} - if self.config.api_key: - client_kwargs["api_key"] = self.config.api_key - if self.config.base_url: - client_kwargs["base_url"] = self.config.base_url - client_kwargs["max_retries"] = self.config.max_retries - client_kwargs["timeout"] = self.config.timeout - self._client = anthropic.Anthropic(**client_kwargs) + anthropic = self._import_sdk() + self._client = anthropic.Anthropic(**self._client_kwargs()) return self._client + def _get_async_client(self) -> Any: + """Lazy-initialize the async Anthropic client.""" + if self._async_client is None: + anthropic = self._import_sdk() + self._async_client = anthropic.AsyncAnthropic(**self._client_kwargs()) + return self._async_client + def _supports_sampling_params(self) -> bool: """Whether the configured model accepts temperature/top_p/top_k.""" return not self.config.model.startswith(self._SAMPLING_UNSUPPORTED_PREFIXES) @@ -102,18 +118,16 @@ def _format_messages( }) return system_content, api_messages - def complete( + def _build_create_kwargs( self, messages: Sequence[ChatMessage], - *, - temperature: Optional[float] = None, - max_tokens: Optional[int] = None, - stop_sequences: Optional[List[str]] = None, - **kwargs: Any, - ) -> CompletionResult: - client = self._get_client() + temperature: Optional[float], + max_tokens: Optional[int], + stop_sequences: Optional[List[str]], + extra: Dict[str, Any], + ) -> Dict[str, Any]: + """Assemble the keyword arguments for a messages.create/stream call.""" system_content, api_messages = self._format_messages(messages) - create_kwargs: Dict[str, Any] = { "model": self.config.model, "messages": api_messages, @@ -125,24 +139,67 @@ def complete( create_kwargs["system"] = system_content if stop_sequences: create_kwargs["stop_sequences"] = stop_sequences - create_kwargs.update(kwargs) + create_kwargs.update(extra) + return create_kwargs + + @staticmethod + def _to_completion_result(response: Any) -> CompletionResult: + content = "".join( + block.text for block in response.content if hasattr(block, "text") + ) + return CompletionResult( + content=content, + model=response.model, + usage=AnthropicProvider._usage_dict(response), + finish_reason=response.stop_reason or "stop", + raw_response=response, + ) + + @staticmethod + def _usage_dict(response: Any) -> Dict[str, int]: + return { + "prompt_tokens": response.usage.input_tokens, + "completion_tokens": response.usage.output_tokens, + "total_tokens": response.usage.input_tokens + response.usage.output_tokens, + } + + def complete( + self, + messages: Sequence[ChatMessage], + *, + temperature: Optional[float] = None, + max_tokens: Optional[int] = None, + stop_sequences: Optional[List[str]] = None, + **kwargs: Any, + ) -> CompletionResult: + client = self._get_client() + create_kwargs = self._build_create_kwargs( + messages, temperature, max_tokens, stop_sequences, kwargs + ) try: response = client.messages.create(**create_kwargs) - content = "".join( - block.text for block in response.content if hasattr(block, "text") - ) - return CompletionResult( - content=content, - model=response.model, - usage={ - "prompt_tokens": response.usage.input_tokens, - "completion_tokens": response.usage.output_tokens, - "total_tokens": response.usage.input_tokens + response.usage.output_tokens, - }, - finish_reason=response.stop_reason or "stop", - raw_response=response, - ) + return self._to_completion_result(response) + except Exception as e: + raise self._translate_error(e) + + async def acomplete( + self, + messages: Sequence[ChatMessage], + *, + temperature: Optional[float] = None, + max_tokens: Optional[int] = None, + stop_sequences: Optional[List[str]] = None, + **kwargs: Any, + ) -> CompletionResult: + client = self._get_async_client() + create_kwargs = self._build_create_kwargs( + messages, temperature, max_tokens, stop_sequences, kwargs + ) + + try: + response = await client.messages.create(**create_kwargs) + return self._to_completion_result(response) except Exception as e: raise self._translate_error(e) @@ -156,20 +213,9 @@ def stream( **kwargs: Any, ) -> Iterator[StreamChunk]: client = self._get_client() - system_content, api_messages = self._format_messages(messages) - - create_kwargs: Dict[str, Any] = { - "model": self.config.model, - "messages": api_messages, - "max_tokens": self._resolve_max_tokens(max_tokens), - } - if self._supports_sampling_params(): - create_kwargs["temperature"] = self._resolve_temperature(temperature) - if system_content: - create_kwargs["system"] = system_content - if stop_sequences: - create_kwargs["stop_sequences"] = stop_sequences - create_kwargs.update(kwargs) + create_kwargs = self._build_create_kwargs( + messages, temperature, max_tokens, stop_sequences, kwargs + ) try: with client.messages.stream(**create_kwargs) as stream: @@ -177,15 +223,31 @@ def stream( yield StreamChunk(text=text) # Final chunk with usage final = stream.get_final_message() - yield StreamChunk( - text="", - is_final=True, - usage={ - "prompt_tokens": final.usage.input_tokens, - "completion_tokens": final.usage.output_tokens, - "total_tokens": final.usage.input_tokens + final.usage.output_tokens, - }, - ) + yield StreamChunk(text="", is_final=True, usage=self._usage_dict(final)) + except Exception as e: + raise self._translate_error(e) + + async def astream( + self, + messages: Sequence[ChatMessage], + *, + temperature: Optional[float] = None, + max_tokens: Optional[int] = None, + stop_sequences: Optional[List[str]] = None, + **kwargs: Any, + ) -> AsyncIterator[StreamChunk]: + client = self._get_async_client() + create_kwargs = self._build_create_kwargs( + messages, temperature, max_tokens, stop_sequences, kwargs + ) + + try: + async with client.messages.stream(**create_kwargs) as stream: + async for text in stream.text_stream: + yield StreamChunk(text=text) + # Final chunk with usage + final = await stream.get_final_message() + yield StreamChunk(text="", is_final=True, usage=self._usage_dict(final)) except Exception as e: raise self._translate_error(e) @@ -208,17 +270,27 @@ def count_tokens(self, messages: Sequence[ChatMessage]) -> int: return total_chars // 4 def _translate_error(self, error: Exception) -> ProviderError: - """Translate Anthropic-specific exceptions to Felix provider errors.""" + """Translate Anthropic-specific exceptions to Felix provider errors. + + Prefers the HTTP status code carried by the SDK's typed exceptions; + falls back to message/type-name matching for plain exceptions. + """ error_str = str(error) error_type = type(error).__name__ lowered = error_str.lower() + status_code = extract_status_code(error) - if "authentication" in lowered or "api_key" in lowered: - return AuthenticationError(error_str, provider="anthropic") - if "rate_limit" in error_type.lower() or "429" in error_str: - return RateLimitError(error_str, provider="anthropic") + if status_code in (401, 403) or "authentication" in lowered or "api_key" in lowered: + return AuthenticationError(error_str, provider="anthropic", status_code=status_code) + if status_code == 429 or "rate_limit" in error_type.lower() or "429" in error_str: + return RateLimitError( + error_str, + retry_after=extract_retry_after(error), + provider="anthropic", + status_code=status_code, + ) if "context" in lowered or "too long" in lowered: - return ContextLengthError(error_str, provider="anthropic") + return ContextLengthError(error_str, provider="anthropic", status_code=status_code) # Bare "model" in the message is not enough: param-rejection 400s # ("temperature is not supported on this model") must stay generic. if "notfound" in error_type.lower().replace("_", "") or ( @@ -228,8 +300,8 @@ def _translate_error(self, error: Exception) -> ProviderError: for phrase in ("not exist", "not found", "not available", "unknown") ) ): - return ModelNotFoundError(error_str, provider="anthropic") - return ProviderError(error_str, provider="anthropic") + return ModelNotFoundError(error_str, provider="anthropic", status_code=status_code) + return ProviderError(error_str, provider="anthropic", status_code=status_code) __all__ = ["AnthropicProvider"] diff --git a/src/felix_agent_sdk/providers/base.py b/src/felix_agent_sdk/providers/base.py index 3c64f40..3488470 100644 --- a/src/felix_agent_sdk/providers/base.py +++ b/src/felix_agent_sdk/providers/base.py @@ -24,11 +24,16 @@ class BaseProvider(ABC): - acomplete(): Async completion - astream(): Async streaming completion - validate(): Test connectivity and authentication + + The built-in Anthropic, OpenAI, and Local providers implement the async + interface; the base-class defaults raise NotImplementedError so custom + sync-only providers remain valid. """ def __init__(self, config: ProviderConfig): self.config = config self._client: Optional[Any] = None # Lazy-initialized provider client + self._async_client: Optional[Any] = None # Lazy-initialized async client @property def model(self) -> str: @@ -151,7 +156,11 @@ def validate(self) -> bool: """Test that the provider is correctly configured and reachable. Returns True if a test request succeeds, False otherwise. - Implementations should make a lightweight API call (e.g., list models). + + Note: the default implementation issues a real (tiny, max 5 tokens) + completion request, which consumes API credits. Providers with a + cheaper health check (e.g. a model-list endpoint) should override + this — see :class:`LocalProvider`. """ try: result = self.complete( diff --git a/src/felix_agent_sdk/providers/errors.py b/src/felix_agent_sdk/providers/errors.py index 49f2d10..f4cdd20 100644 --- a/src/felix_agent_sdk/providers/errors.py +++ b/src/felix_agent_sdk/providers/errors.py @@ -50,10 +50,42 @@ class ContextLengthError(ProviderError): pass +def extract_status_code(error: Exception) -> Optional[int]: + """Pull an HTTP status code off a vendor SDK exception, if present. + + Both the ``anthropic`` and ``openai`` SDKs expose ``status_code`` on + their ``APIStatusError`` hierarchy. + """ + code = getattr(error, "status_code", None) + return code if isinstance(code, int) else None + + +def extract_retry_after(error: Exception) -> Optional[float]: + """Pull a Retry-After value (seconds) off a vendor SDK exception. + + Vendor SDK status errors carry the httpx response; the header is + optional and may be a date string, in which case None is returned. + """ + response = getattr(error, "response", None) + headers = getattr(response, "headers", None) + if headers is None: + return None + try: + value = headers.get("retry-after") + except (AttributeError, TypeError): + return None + try: + return float(value) if value is not None else None + except (TypeError, ValueError): + return None + + __all__ = [ "ProviderError", "AuthenticationError", "RateLimitError", "ModelNotFoundError", "ContextLengthError", + "extract_status_code", + "extract_retry_after", ] diff --git a/src/felix_agent_sdk/providers/openai_provider.py b/src/felix_agent_sdk/providers/openai_provider.py index 752ae18..f88d2c7 100644 --- a/src/felix_agent_sdk/providers/openai_provider.py +++ b/src/felix_agent_sdk/providers/openai_provider.py @@ -9,7 +9,7 @@ from __future__ import annotations import os -from typing import Any, Dict, Iterator, List, Optional, Sequence +from typing import Any, AsyncIterator, Dict, Iterator, List, Optional, Sequence from .base import BaseProvider from .errors import ( @@ -18,6 +18,8 @@ ModelNotFoundError, ProviderError, RateLimitError, + extract_retry_after, + extract_status_code, ) from .types import ChatMessage, CompletionResult, ProviderConfig, StreamChunk @@ -53,67 +55,139 @@ def __init__( ) super().__init__(config) + def _import_sdk(self) -> Any: + try: + import openai + except ImportError: + raise ImportError( + "OpenAI provider requires the 'openai' package. " + "Install with: pip install felix-agent-sdk[openai]" + ) + return openai + + def _client_kwargs(self) -> Dict[str, Any]: + client_kwargs: Dict[str, Any] = {} + if self.config.api_key: + client_kwargs["api_key"] = self.config.api_key + if self.config.base_url: + client_kwargs["base_url"] = self.config.base_url + client_kwargs["max_retries"] = self.config.max_retries + client_kwargs["timeout"] = self.config.timeout + return client_kwargs + def _get_client(self) -> Any: """Lazy-initialize the OpenAI client.""" if self._client is None: - try: - import openai - except ImportError: - raise ImportError( - "OpenAI provider requires the 'openai' package. " - "Install with: pip install felix-agent-sdk[openai]" - ) - client_kwargs: Dict[str, Any] = {} - if self.config.api_key: - client_kwargs["api_key"] = self.config.api_key - if self.config.base_url: - client_kwargs["base_url"] = self.config.base_url - client_kwargs["max_retries"] = self.config.max_retries - client_kwargs["timeout"] = self.config.timeout - self._client = openai.OpenAI(**client_kwargs) + openai = self._import_sdk() + self._client = openai.OpenAI(**self._client_kwargs()) return self._client + def _get_async_client(self) -> Any: + """Lazy-initialize the async OpenAI client.""" + if self._async_client is None: + openai = self._import_sdk() + self._async_client = openai.AsyncOpenAI(**self._client_kwargs()) + return self._async_client + def _format_messages(self, messages: Sequence[ChatMessage]) -> List[Dict[str, str]]: """Convert ChatMessages to OpenAI's format (system stays inline).""" return [{"role": msg.role.value, "content": msg.content} for msg in messages] - def complete( + def _build_create_kwargs( self, messages: Sequence[ChatMessage], - *, - temperature: Optional[float] = None, - max_tokens: Optional[int] = None, - stop_sequences: Optional[List[str]] = None, - **kwargs: Any, - ) -> CompletionResult: - client = self._get_client() - api_messages = self._format_messages(messages) - + temperature: Optional[float], + max_tokens: Optional[int], + stop_sequences: Optional[List[str]], + extra: Dict[str, Any], + stream: bool = False, + ) -> Dict[str, Any]: + """Assemble the keyword arguments for a chat.completions.create call.""" create_kwargs: Dict[str, Any] = { "model": self.config.model, - "messages": api_messages, + "messages": self._format_messages(messages), "temperature": self._resolve_temperature(temperature), "max_tokens": self._resolve_max_tokens(max_tokens), } + if stream: + create_kwargs["stream"] = True + create_kwargs["stream_options"] = {"include_usage": True} if stop_sequences: create_kwargs["stop"] = stop_sequences - create_kwargs.update(kwargs) + create_kwargs.update(extra) + return create_kwargs - try: - response = client.chat.completions.create(**create_kwargs) - choice = response.choices[0] - usage = response.usage - return CompletionResult( - content=choice.message.content or "", - model=response.model, + @staticmethod + def _to_completion_result(response: Any) -> CompletionResult: + choice = response.choices[0] + usage = response.usage + return CompletionResult( + content=choice.message.content or "", + model=response.model, + usage={ + "prompt_tokens": usage.prompt_tokens if usage else 0, + "completion_tokens": usage.completion_tokens if usage else 0, + "total_tokens": usage.total_tokens if usage else 0, + }, + finish_reason=choice.finish_reason or "stop", + raw_response=response, + ) + + @staticmethod + def _chunk_to_stream_chunks(chunk: Any) -> Iterator[StreamChunk]: + """Translate one raw OpenAI stream chunk into StreamChunks.""" + if chunk.choices and chunk.choices[0].delta.content: + yield StreamChunk(text=chunk.choices[0].delta.content) + + # Final chunk with usage (sent last when include_usage is set) + if chunk.usage: + yield StreamChunk( + text="", + is_final=True, usage={ - "prompt_tokens": usage.prompt_tokens if usage else 0, - "completion_tokens": usage.completion_tokens if usage else 0, - "total_tokens": usage.total_tokens if usage else 0, + "prompt_tokens": chunk.usage.prompt_tokens, + "completion_tokens": chunk.usage.completion_tokens, + "total_tokens": chunk.usage.total_tokens, }, - finish_reason=choice.finish_reason or "stop", - raw_response=response, ) + + def complete( + self, + messages: Sequence[ChatMessage], + *, + temperature: Optional[float] = None, + max_tokens: Optional[int] = None, + stop_sequences: Optional[List[str]] = None, + **kwargs: Any, + ) -> CompletionResult: + client = self._get_client() + create_kwargs = self._build_create_kwargs( + messages, temperature, max_tokens, stop_sequences, kwargs + ) + + try: + response = client.chat.completions.create(**create_kwargs) + return self._to_completion_result(response) + except Exception as e: + raise self._translate_error(e) + + async def acomplete( + self, + messages: Sequence[ChatMessage], + *, + temperature: Optional[float] = None, + max_tokens: Optional[int] = None, + stop_sequences: Optional[List[str]] = None, + **kwargs: Any, + ) -> CompletionResult: + client = self._get_async_client() + create_kwargs = self._build_create_kwargs( + messages, temperature, max_tokens, stop_sequences, kwargs + ) + + try: + response = await client.chat.completions.create(**create_kwargs) + return self._to_completion_result(response) except Exception as e: raise self._translate_error(e) @@ -127,37 +201,45 @@ def stream( **kwargs: Any, ) -> Iterator[StreamChunk]: client = self._get_client() - api_messages = self._format_messages(messages) - - create_kwargs: Dict[str, Any] = { - "model": self.config.model, - "messages": api_messages, - "temperature": self._resolve_temperature(temperature), - "max_tokens": self._resolve_max_tokens(max_tokens), - "stream": True, - "stream_options": {"include_usage": True}, - } - if stop_sequences: - create_kwargs["stop"] = stop_sequences - create_kwargs.update(kwargs) + create_kwargs = self._build_create_kwargs( + messages, temperature, max_tokens, stop_sequences, kwargs, stream=True + ) try: response = client.chat.completions.create(**create_kwargs) for chunk in response: - if chunk.choices and chunk.choices[0].delta.content: - yield StreamChunk(text=chunk.choices[0].delta.content) - - # Final chunk with usage - if chunk.usage: - yield StreamChunk( - text="", - is_final=True, - usage={ - "prompt_tokens": chunk.usage.prompt_tokens, - "completion_tokens": chunk.usage.completion_tokens, - "total_tokens": chunk.usage.total_tokens, - }, - ) + done = False + for stream_chunk in self._chunk_to_stream_chunks(chunk): + done = done or stream_chunk.is_final + yield stream_chunk + if done: + break + except Exception as e: + raise self._translate_error(e) + + async def astream( + self, + messages: Sequence[ChatMessage], + *, + temperature: Optional[float] = None, + max_tokens: Optional[int] = None, + stop_sequences: Optional[List[str]] = None, + **kwargs: Any, + ) -> AsyncIterator[StreamChunk]: + client = self._get_async_client() + create_kwargs = self._build_create_kwargs( + messages, temperature, max_tokens, stop_sequences, kwargs, stream=True + ) + + try: + response = await client.chat.completions.create(**create_kwargs) + async for chunk in response: + done = False + for stream_chunk in self._chunk_to_stream_chunks(chunk): + done = done or stream_chunk.is_final + yield stream_chunk + if done: + break except Exception as e: raise self._translate_error(e) @@ -182,19 +264,30 @@ def count_tokens(self, messages: Sequence[ChatMessage]) -> int: return total_chars // 4 def _translate_error(self, error: Exception) -> ProviderError: - """Translate OpenAI-specific exceptions to Felix provider errors.""" + """Translate OpenAI-specific exceptions to Felix provider errors. + + Prefers the HTTP status code carried by the SDK's typed exceptions; + falls back to message/type-name matching for plain exceptions. + """ error_str = str(error) error_type = type(error).__name__ + lowered = error_str.lower() + status_code = extract_status_code(error) - if "authentication" in error_str.lower() or "api_key" in error_str.lower(): - return AuthenticationError(error_str, provider="openai") - if "rate_limit" in error_type.lower() or "429" in error_str: - return RateLimitError(error_str, provider="openai") + if status_code in (401, 403) or "authentication" in lowered or "api_key" in lowered: + return AuthenticationError(error_str, provider="openai", status_code=status_code) + if status_code == 429 or "rate_limit" in error_type.lower() or "429" in error_str: + return RateLimitError( + error_str, + retry_after=extract_retry_after(error), + provider="openai", + status_code=status_code, + ) if "not_found" in error_type.lower(): - return ModelNotFoundError(error_str, provider="openai") - if "context_length" in error_str.lower() or "maximum context" in error_str.lower(): - return ContextLengthError(error_str, provider="openai") - return ProviderError(error_str, provider="openai") + return ModelNotFoundError(error_str, provider="openai", status_code=status_code) + if "context_length" in lowered or "maximum context" in lowered: + return ContextLengthError(error_str, provider="openai", status_code=status_code) + return ProviderError(error_str, provider="openai", status_code=status_code) __all__ = ["OpenAIProvider"] diff --git a/src/felix_agent_sdk/providers/registry.py b/src/felix_agent_sdk/providers/registry.py index 4b5359c..8aa6861 100644 --- a/src/felix_agent_sdk/providers/registry.py +++ b/src/felix_agent_sdk/providers/registry.py @@ -32,7 +32,15 @@ class ProviderRegistry: """ _providers: Dict[str, type[BaseProvider]] = {} - _detection_order: List[str] = [] + _detection_priorities: Dict[str, int] = {} + + # Environment variables consulted during auto-detection, by provider name. + _DETECTION_ENV_VARS: Dict[str, str] = { + "anthropic": "ANTHROPIC_API_KEY", + "openai": "OPENAI_API_KEY", + "bedrock": "AWS_ACCESS_KEY_ID", + "vertex": "GOOGLE_APPLICATION_CREDENTIALS", + } @classmethod def register( @@ -51,13 +59,14 @@ def register( if not issubclass(provider_class, BaseProvider): raise TypeError(f"{provider_class} must inherit from BaseProvider") cls._providers[name] = provider_class - # Maintain sorted detection order - cls._detection_order.append(name) - cls._detection_order.sort( - key=lambda n: detection_priority if n == name else 100 - ) + cls._detection_priorities[name] = detection_priority logger.debug(f"Registered provider: {name}") + @classmethod + def _detection_order(cls) -> List[str]: + """Registered provider names sorted by detection priority (low first).""" + return sorted(cls._providers, key=lambda n: cls._detection_priorities.get(n, 100)) + @classmethod def get(cls, name: str) -> type[BaseProvider]: """Get a provider class by name. @@ -101,16 +110,10 @@ def auto_detect(cls) -> BaseProvider: kwargs: Dict[str, Any] = {"model": model} if model else {} return provider_class(**kwargs) - # Auto-detect from available keys - detection_map = { - "anthropic": "ANTHROPIC_API_KEY", - "openai": "OPENAI_API_KEY", - "bedrock": "AWS_ACCESS_KEY_ID", - "vertex": "GOOGLE_APPLICATION_CREDENTIALS", - } - - for provider_name, env_var in detection_map.items(): - if os.getenv(env_var) and provider_name in cls._providers: + # Auto-detect from available keys, honoring registration priority + for provider_name in cls._detection_order(): + env_var = cls._DETECTION_ENV_VARS.get(provider_name) + if env_var and os.getenv(env_var): model = os.getenv("FELIX_MODEL") provider_class = cls._providers[provider_name] kwargs = {"model": model} if model else {} diff --git a/src/felix_agent_sdk/providers/types.py b/src/felix_agent_sdk/providers/types.py index d7f61b5..b6cd574 100644 --- a/src/felix_agent_sdk/providers/types.py +++ b/src/felix_agent_sdk/providers/types.py @@ -98,6 +98,16 @@ class ProviderConfig: default_max_tokens: int = 1024 extra: Dict[str, Any] = field(default_factory=dict) + def __repr__(self) -> str: + """Repr with the API key masked so configs are safe to log.""" + api_key = "***" if self.api_key else None + return ( + f"{self.__class__.__name__}(model={self.model!r}, api_key={api_key!r}, " + f"base_url={self.base_url!r}, max_retries={self.max_retries}, " + f"timeout={self.timeout}, default_temperature={self.default_temperature}, " + f"default_max_tokens={self.default_max_tokens}, extra={self.extra!r})" + ) + __all__ = [ "MessageRole", diff --git a/tests/unit/test_anthropic_provider.py b/tests/unit/test_anthropic_provider.py index 8325843..850153b 100644 --- a/tests/unit/test_anthropic_provider.py +++ b/tests/unit/test_anthropic_provider.py @@ -521,3 +521,128 @@ def test_stream_translates_exceptions(self, user_message): p._client.messages.stream.side_effect = Exception("Error 429: rate limit exceeded") with pytest.raises(RateLimitError): list(p.stream([user_message])) + + +# --------------------------------------------------------------------------- +# Status-code / retry-after aware error translation +# --------------------------------------------------------------------------- + + +def _status_error(message, status_code, retry_after=None): + """Mimic a vendor SDK APIStatusError carrying status_code and response.""" + err = Exception(message) + err.status_code = status_code + headers = {} + if retry_after is not None: + headers["retry-after"] = retry_after + response = MagicMock() + response.headers = headers + err.response = response + return err + + +class TestAnthropicTypedErrorTranslation: + def test_status_401_maps_to_authentication(self): + p = _make_provider() + result = p._translate_error(_status_error("nope", 401)) + assert isinstance(result, AuthenticationError) + assert result.status_code == 401 + + def test_status_429_maps_to_rate_limit_with_retry_after(self): + p = _make_provider() + result = p._translate_error(_status_error("slow down", 429, retry_after="1.5")) + assert isinstance(result, RateLimitError) + assert result.status_code == 429 + assert result.retry_after == 1.5 + + def test_generic_error_carries_status_code(self): + p = _make_provider() + result = p._translate_error(_status_error("boom", 500)) + assert isinstance(result, ProviderError) + assert result.status_code == 500 + + +# --------------------------------------------------------------------------- +# Async interface +# --------------------------------------------------------------------------- + + +class _AsyncTextStream: + def __init__(self, texts): + self._texts = list(texts) + + def __aiter__(self): + return self + + async def __anext__(self): + if not self._texts: + raise StopAsyncIteration + return self._texts.pop(0) + + +class _MockAsyncMessageStream: + """Mimics anthropic's AsyncMessageStream context manager.""" + + def __init__(self, texts, final_message): + self.text_stream = _AsyncTextStream(texts) + self._final = final_message + + async def __aenter__(self): + return self + + async def __aexit__(self, *exc_info): + return False + + async def get_final_message(self): + return self._final + + +class TestAnthropicAsync: + @pytest.mark.asyncio + async def test_acomplete_returns_completion_result(self, user_message): + from unittest.mock import AsyncMock + + p = _make_provider() + p._async_client = MagicMock() + p._async_client.messages.create = AsyncMock(return_value=_mock_response()) + + result = await p.acomplete([user_message]) + assert isinstance(result, CompletionResult) + assert result.content == "Hello!" + assert result.usage["total_tokens"] == 15 + + @pytest.mark.asyncio + async def test_astream_yields_chunks_with_final_usage(self, user_message): + p = _make_provider() + p._async_client = MagicMock() + p._async_client.messages.stream.return_value = _MockAsyncMessageStream( + ["Hello", " world"], _mock_response(input_tokens=12, output_tokens=6) + ) + + out = [chunk async for chunk in p.astream([user_message])] + texts = [c.text for c in out if not c.is_final] + assert texts == ["Hello", " world"] + assert out[-1].is_final is True + assert out[-1].usage["total_tokens"] == 18 + + @pytest.mark.asyncio + async def test_acomplete_translates_errors(self, user_message): + from unittest.mock import AsyncMock + + p = _make_provider() + p._async_client = MagicMock() + p._async_client.messages.create = AsyncMock( + side_effect=Exception("authentication failed") + ) + with pytest.raises(AuthenticationError): + await p.acomplete([user_message]) + + def test_async_client_created_once(self): + mock_mod = MagicMock() + mock_mod.AsyncAnthropic.return_value = MagicMock() + with patch.dict("sys.modules", {"anthropic": mock_mod}): + p = AnthropicProvider(api_key="k") + c1 = p._get_async_client() + c2 = p._get_async_client() + assert c1 is c2 + mock_mod.AsyncAnthropic.assert_called_once() diff --git a/tests/unit/test_base_provider.py b/tests/unit/test_base_provider.py index 93f4e8a..e9f1189 100644 --- a/tests/unit/test_base_provider.py +++ b/tests/unit/test_base_provider.py @@ -218,3 +218,23 @@ class TestLazyClient: def test_client_initially_none(self): p = StubProvider() assert p._client is None + + +# --------------------------------------------------------------------------- +# ProviderConfig repr masking +# --------------------------------------------------------------------------- + + +class TestProviderConfigRepr: + def test_api_key_masked_in_repr(self): + config = ProviderConfig(model="m", api_key="sk-super-secret") + assert "sk-super-secret" not in repr(config) + assert "***" in repr(config) + + def test_none_api_key_shown_as_none(self): + config = ProviderConfig(model="m") + assert "api_key=None" in repr(config) + + def test_repr_still_shows_model(self): + config = ProviderConfig(model="gpt-4o", api_key="secret") + assert "gpt-4o" in repr(config) diff --git a/tests/unit/test_errors.py b/tests/unit/test_errors.py index 74becbb..106aa95 100644 --- a/tests/unit/test_errors.py +++ b/tests/unit/test_errors.py @@ -187,3 +187,45 @@ def test_catch_generic_catches_all(self): pass # expected else: pytest.fail(f"{cls.__name__} not caught by ProviderError handler") + + +# --------------------------------------------------------------------------- +# Vendor exception helpers +# --------------------------------------------------------------------------- + + +class TestExtractHelpers: + def test_extract_status_code(self): + from felix_agent_sdk.providers.errors import extract_status_code + + err = Exception("boom") + err.status_code = 429 + assert extract_status_code(err) == 429 + + def test_extract_status_code_missing(self): + from felix_agent_sdk.providers.errors import extract_status_code + + assert extract_status_code(Exception("boom")) is None + + def test_extract_status_code_non_int(self): + from felix_agent_sdk.providers.errors import extract_status_code + + err = Exception("boom") + err.status_code = "429" + assert extract_status_code(err) is None + + def test_extract_retry_after(self): + from unittest.mock import MagicMock + + from felix_agent_sdk.providers.errors import extract_retry_after + + err = Exception("boom") + response = MagicMock() + response.headers = {"retry-after": "3"} + err.response = response + assert extract_retry_after(err) == 3.0 + + def test_extract_retry_after_missing_response(self): + from felix_agent_sdk.providers.errors import extract_retry_after + + assert extract_retry_after(Exception("boom")) is None diff --git a/tests/unit/test_openai_provider.py b/tests/unit/test_openai_provider.py index 2bdb47c..064fa01 100644 --- a/tests/unit/test_openai_provider.py +++ b/tests/unit/test_openai_provider.py @@ -464,3 +464,159 @@ def test_stream_translates_exceptions(self, user_message): p._client.chat.completions.create.side_effect = Exception("429 rate limit") with pytest.raises(RateLimitError): list(p.stream([user_message])) + + +# --------------------------------------------------------------------------- +# Status-code / retry-after aware error translation +# --------------------------------------------------------------------------- + + +def _status_error(message, status_code, retry_after=None): + """Mimic a vendor SDK APIStatusError carrying status_code and response.""" + err = Exception(message) + err.status_code = status_code + headers = {} + if retry_after is not None: + headers["retry-after"] = retry_after + response = MagicMock() + response.headers = headers + err.response = response + return err + + +class TestOpenAITypedErrorTranslation: + def test_status_401_maps_to_authentication(self): + p = _make_provider() + result = p._translate_error(_status_error("nope", 401)) + assert isinstance(result, AuthenticationError) + assert result.status_code == 401 + + def test_status_429_maps_to_rate_limit_with_retry_after(self): + p = _make_provider() + result = p._translate_error(_status_error("slow down", 429, retry_after="2.5")) + assert isinstance(result, RateLimitError) + assert result.status_code == 429 + assert result.retry_after == 2.5 + + def test_retry_after_absent_is_none(self): + p = _make_provider() + result = p._translate_error(_status_error("slow down", 429)) + assert isinstance(result, RateLimitError) + assert result.retry_after is None + + def test_non_numeric_retry_after_is_none(self): + p = _make_provider() + result = p._translate_error( + _status_error("slow down", 429, retry_after="Wed, 21 Oct 2026 07:28:00 GMT") + ) + assert isinstance(result, RateLimitError) + assert result.retry_after is None + + def test_generic_error_carries_status_code(self): + p = _make_provider() + result = p._translate_error(_status_error("boom", 500)) + assert isinstance(result, ProviderError) + assert result.status_code == 500 + + +# --------------------------------------------------------------------------- +# Streaming stops after the final usage chunk +# --------------------------------------------------------------------------- + + +class TestOpenAIStreamTermination: + def test_stream_stops_after_final_chunk(self, user_message): + p = _make_provider() + chunks = _mock_stream_chunks(["a", "b"]) + + # A chunk after the usage chunk must never be consumed + poisoned = MagicMock() + poisoned.choices = [] + poisoned.usage = None + + consumed = [] + + def chunk_iter(): + for c in chunks: + consumed.append(c) + yield c + consumed.append(poisoned) + yield poisoned + + p._client.chat.completions.create.return_value = chunk_iter() + out = list(p.stream([user_message])) + + assert out[-1].is_final is True + assert poisoned not in consumed + + +# --------------------------------------------------------------------------- +# Async interface +# --------------------------------------------------------------------------- + + +class _AsyncChunkStream: + """Async-iterable wrapper over a list of mock chunks.""" + + def __init__(self, chunks): + self._chunks = list(chunks) + + def __aiter__(self): + return self + + async def __anext__(self): + if not self._chunks: + raise StopAsyncIteration + return self._chunks.pop(0) + + +class TestOpenAIAsync: + @pytest.mark.asyncio + async def test_acomplete_returns_completion_result(self, user_message): + from unittest.mock import AsyncMock + + p = _make_provider() + p._async_client = MagicMock() + p._async_client.chat.completions.create = AsyncMock(return_value=_mock_response()) + + result = await p.acomplete([user_message]) + assert isinstance(result, CompletionResult) + assert result.content == "Hello!" + + @pytest.mark.asyncio + async def test_astream_yields_chunks_with_final_usage(self, user_message): + from unittest.mock import AsyncMock + + p = _make_provider() + p._async_client = MagicMock() + p._async_client.chat.completions.create = AsyncMock( + return_value=_AsyncChunkStream(_mock_stream_chunks(["Hello", " world"])) + ) + + out = [chunk async for chunk in p.astream([user_message])] + texts = [c.text for c in out if not c.is_final] + assert texts == ["Hello", " world"] + assert out[-1].is_final is True + assert out[-1].usage["total_tokens"] == 15 + + @pytest.mark.asyncio + async def test_acomplete_translates_errors(self, user_message): + from unittest.mock import AsyncMock + + p = _make_provider() + p._async_client = MagicMock() + p._async_client.chat.completions.create = AsyncMock( + side_effect=Exception("429 rate limit") + ) + with pytest.raises(RateLimitError): + await p.acomplete([user_message]) + + def test_async_client_created_once(self): + mock_mod = MagicMock() + mock_mod.AsyncOpenAI.return_value = MagicMock() + with patch.dict("sys.modules", {"openai": mock_mod}): + p = OpenAIProvider(api_key="k") + c1 = p._get_async_client() + c2 = p._get_async_client() + assert c1 is c2 + mock_mod.AsyncOpenAI.assert_called_once() diff --git a/tests/unit/test_registry.py b/tests/unit/test_registry.py index 1cd4af5..2ccac10 100644 --- a/tests/unit/test_registry.py +++ b/tests/unit/test_registry.py @@ -45,10 +45,10 @@ class _NotAProvider: def _clean_registry(): """Save and restore registry state around each test.""" original_providers = ProviderRegistry._providers.copy() - original_order = ProviderRegistry._detection_order.copy() + original_priorities = ProviderRegistry._detection_priorities.copy() yield ProviderRegistry._providers = original_providers - ProviderRegistry._detection_order = original_order + ProviderRegistry._detection_priorities = original_priorities # --------------------------------------------------------------------------- @@ -73,7 +73,7 @@ def test_register_overwrites_existing(self): def test_register_adds_to_detection_order(self): ProviderRegistry.register("mytest", _TestProvider) - assert "mytest" in ProviderRegistry._detection_order + assert "mytest" in ProviderRegistry._detection_order() # --------------------------------------------------------------------------- @@ -193,7 +193,7 @@ def test_no_provider_found_raises(self): """When local is not registered and no keys found, should raise.""" # Remove all providers ProviderRegistry._providers.clear() - ProviderRegistry._detection_order.clear() + ProviderRegistry._detection_priorities.clear() with patch.dict("os.environ", {}, clear=True): with pytest.raises(ProviderError, match="No provider could be auto-detected"): ProviderRegistry.auto_detect() @@ -210,3 +210,26 @@ def test_delegates_to_registry(self): provider = auto_detect_provider() from felix_agent_sdk.providers.local import LocalProvider assert isinstance(provider, LocalProvider) + + +# --------------------------------------------------------------------------- +# Detection priority ordering +# --------------------------------------------------------------------------- + + +class TestDetectionPriority: + def test_detection_order_sorted_by_priority(self): + ProviderRegistry.register("zeta", _TestProvider, detection_priority=5) + ProviderRegistry.register("alpha", _TestProvider, detection_priority=50) + order = ProviderRegistry._detection_order() + assert order.index("zeta") < order.index("alpha") + # Built-in anthropic (priority 10) sits between them + assert order.index("zeta") < order.index("anthropic") < order.index("alpha") + + def test_priorities_survive_later_registrations(self): + """Regression: the old sort reset every existing priority to 100.""" + ProviderRegistry.register("first", _TestProvider, detection_priority=1) + ProviderRegistry.register("second", _TestProvider, detection_priority=99) + ProviderRegistry.register("third", _TestProvider, detection_priority=50) + order = ProviderRegistry._detection_order() + assert order.index("first") < order.index("third") < order.index("second") From 8668cb1f1b8762589417f80d19ceef6e0c63866c Mon Sep 17 00:00:00 2001 From: jkbennitt Date: Tue, 9 Jun 2026 21:52:47 -0400 Subject: [PATCH 06/12] fix(cli,workflows): surface provider init failures, public set_progress - felix run now prints the underlying cause when a provider cannot be created (missing dependency, bad key) instead of swallowing it. - Agent gains a public set_progress() with range/state validation; Synthesizer uses it instead of poking Agent._progress. - Drop unused enumerate index in ContextBuilder scoring loop. Co-authored-by: Caleb Gross <209704970+CalebisGross@users.noreply.github.com> Co-Authored-By: Claude Fable 5 --- src/felix_agent_sdk/agents/base.py | 16 ++++++++++++++ src/felix_agent_sdk/cli/run_command.py | 14 +++++++++---- .../workflows/context_builder.py | 2 +- src/felix_agent_sdk/workflows/synthesizer.py | 2 +- tests/unit/test_agent_base.py | 21 +++++++++++++++++++ 5 files changed, 49 insertions(+), 6 deletions(-) diff --git a/src/felix_agent_sdk/agents/base.py b/src/felix_agent_sdk/agents/base.py index 700eb12..a7288b6 100644 --- a/src/felix_agent_sdk/agents/base.py +++ b/src/felix_agent_sdk/agents/base.py @@ -172,6 +172,22 @@ def update_position(self, current_time: float) -> None: if self._progress >= 1.0: self._state = AgentState.COMPLETED + def set_progress(self, progress: float) -> None: + """Place the agent directly at *progress* along the helix. + + Bypasses time-based progression — used by orchestrators that need an + agent at a specific phase (e.g. a synthesis agent at t=1.0). + + Raises: + ValueError: If *progress* is outside [0, 1] or the agent has + not been spawned. + """ + if not (0.0 <= progress <= 1.0): + raise ValueError("progress must be between 0 and 1") + if self._state == AgentState.WAITING: + raise ValueError("Cannot set progress of unspawned agent") + self._progress = progress + def get_position(self, current_time: float) -> Optional[Tuple[float, float, float]]: """Return (x, y, z) on the helix, or ``None`` if not yet spawned.""" if self._state == AgentState.WAITING: diff --git a/src/felix_agent_sdk/cli/run_command.py b/src/felix_agent_sdk/cli/run_command.py index 5d770e0..4c578a2 100644 --- a/src/felix_agent_sdk/cli/run_command.py +++ b/src/felix_agent_sdk/cli/run_command.py @@ -97,13 +97,19 @@ def _on_round_complete(event): # type: ignore[no-untyped-def] def _resolve_provider(name: str, model: str) -> BaseProvider | None: - """Try to create a provider by name. Returns None on failure.""" + """Try to create a provider by name. + + Returns None on failure, after printing the underlying cause to stderr + so users see *why* (missing dependency, bad API key, …) rather than a + bare "could not create provider". + """ if not name or name == "auto": try: from felix_agent_sdk.providers import auto_detect_provider return auto_detect_provider() - except Exception: + except Exception as e: + print(f"Provider auto-detection failed: {e}", file=sys.stderr) return None try: @@ -119,6 +125,6 @@ def _resolve_provider(name: str, model: str) -> BaseProvider | None: from felix_agent_sdk.providers import LocalProvider return LocalProvider(model=model or "default") - except Exception: - pass + except Exception as e: + print(f"Provider '{name}' could not be created: {e}", file=sys.stderr) return None diff --git a/src/felix_agent_sdk/workflows/context_builder.py b/src/felix_agent_sdk/workflows/context_builder.py index c2dd54f..5bf4465 100644 --- a/src/felix_agent_sdk/workflows/context_builder.py +++ b/src/felix_agent_sdk/workflows/context_builder.py @@ -176,7 +176,7 @@ def _score_contributions(self) -> list[tuple[Contribution, float]]: now = time.time() scored: list[tuple[Contribution, float]] = [] - for i, contrib in enumerate(self._contributions): + for contrib in self._contributions: # Recency: more recent = higher (0.0 – 0.5) age = now - contrib.timestamp recency = max(0.0, 0.5 - age * 0.01) diff --git a/src/felix_agent_sdk/workflows/synthesizer.py b/src/felix_agent_sdk/workflows/synthesizer.py index f98732c..a804b1c 100644 --- a/src/felix_agent_sdk/workflows/synthesizer.py +++ b/src/felix_agent_sdk/workflows/synthesizer.py @@ -117,7 +117,7 @@ def _compressed_merge( ) # Move to synthesis phase synth_agent.spawn(current_time=0.0) - synth_agent._progress = 1.0 # noqa: SLF001 — place at synthesis end + synth_agent.set_progress(1.0) # place at synthesis end task = LLMTask( task_id="synthesis", diff --git a/tests/unit/test_agent_base.py b/tests/unit/test_agent_base.py index 1dc40f1..c86f06a 100644 --- a/tests/unit/test_agent_base.py +++ b/tests/unit/test_agent_base.py @@ -262,3 +262,24 @@ def test_repr_contains_id(self, agent): def test_repr_contains_state(self, agent): assert "waiting" in repr(agent) + + +# --------------------------------------------------------------------------- +# set_progress +# --------------------------------------------------------------------------- + + +class TestSetProgress: + def test_set_progress_directly(self, agent): + agent.spawn(0.2) + agent.set_progress(1.0) + assert agent.progress == 1.0 + + def test_set_progress_validates_range(self, agent): + agent.spawn(0.2) + with pytest.raises(ValueError, match="between 0 and 1"): + agent.set_progress(1.5) + + def test_set_progress_requires_spawn(self, agent): + with pytest.raises(ValueError, match="unspawned"): + agent.set_progress(0.5) From 15d71925550eb399ad330a299bb40094fa1bdd9f Mon Sep 17 00:00:00 2001 From: jkbennitt Date: Tue, 9 Jun 2026 21:54:01 -0400 Subject: [PATCH 07/12] ci: type-check gate, Windows + Python 3.13 matrix, coverage, release version check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - CI now runs mypy src/ (strict config was previously unenforced — 32 errors had accumulated) and reports coverage via pytest-cov. - Test matrix expands to windows-latest (two Windows-only encoding bugs shipped in 0.2.x that Ubuntu-only CI could not catch) and Python 3.13. - Publish workflow verifies the release tag matches _version.py before building, and runs the same mypy gate. - pyproject: 3.13 classifier, explicit asyncio_mode=strict, fix author email domain (appsprout.dev). Co-Authored-By: Claude Fable 5 --- .github/workflows/ci.yml | 11 ++++++++--- .github/workflows/publish.yml | 13 +++++++++++++ pyproject.toml | 6 +++++- 3 files changed, 26 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 98a5027..9c29052 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -8,10 +8,12 @@ on: jobs: test: - runs-on: ubuntu-latest + runs-on: ${{ matrix.os }} strategy: + fail-fast: false matrix: - python-version: ["3.10", "3.11", "3.12"] + os: [ubuntu-latest, windows-latest] + python-version: ["3.10", "3.11", "3.12", "3.13"] steps: - uses: actions/checkout@v4 @@ -30,8 +32,11 @@ jobs: - name: Lint run: ruff check src/ + - name: Type check + run: mypy src/ + - name: Test - run: pytest tests/ -v --tb=short + run: pytest tests/ -v --tb=short --cov=felix_agent_sdk --cov-report=term-missing build: name: Verify package builds diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index e8525ff..be084a5 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -26,9 +26,22 @@ jobs: python -m pip install --upgrade pip pip install -e ".[dev]" + - name: Verify release tag matches package version + run: | + PKG_VERSION=$(python -c "from felix_agent_sdk import __version__; print(__version__)") + TAG_VERSION="${GITHUB_REF_NAME#v}" + if [ "$PKG_VERSION" != "$TAG_VERSION" ]; then + echo "::error::Release tag v$TAG_VERSION does not match package version $PKG_VERSION (_version.py)." + exit 1 + fi + echo "Version check OK: $PKG_VERSION" + - name: Lint run: ruff check src/ + - name: Type check + run: mypy src/ + - name: Test run: pytest tests/ -v --tb=short diff --git a/pyproject.toml b/pyproject.toml index b5c922b..dc38b4c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -10,7 +10,7 @@ readme = "README.md" license = "MIT" requires-python = ">=3.10" authors = [ - { name = "AppSprout", email = "team@appsprout.com" }, + { name = "AppSprout", email = "team@appsprout.dev" }, ] keywords = [ "ai", "agents", "multi-agent", "orchestration", "llm", @@ -24,6 +24,7 @@ classifiers = [ "Programming Language :: Python :: 3.10", "Programming Language :: Python :: 3.11", "Programming Language :: Python :: 3.12", + "Programming Language :: Python :: 3.13", "Topic :: Scientific/Engineering :: Artificial Intelligence", ] @@ -67,6 +68,9 @@ path = "src/felix_agent_sdk/_version.py" [tool.hatch.build.targets.wheel] packages = ["src/felix_agent_sdk"] +[tool.pytest.ini_options] +asyncio_mode = "strict" + [tool.ruff] target-version = "py310" line-length = 100 From 05be819c98915eee9e1750faacc6d92c8dd4f252 Mon Sep 17 00:00:00 2001 From: jkbennitt Date: Tue, 9 Jun 2026 21:56:16 -0400 Subject: [PATCH 08/12] docs: refresh model references, roadmap, and phase status - Update claude-sonnet-4-5 -> claude-sonnet-4-6 across README, examples, docstrings, tests, and the AnthropicProvider/CLI default model. - README roadmap: mark Phase 3 (Hardening & Async, v0.3.0) current and add tool use / structured output as an explicit Phase 4 roadmap item. - CLAUDE.md: porting phases 1-7 are complete; current work is post-port hardening and release engineering. Co-Authored-By: Claude Fable 5 --- CLAUDE.md | 17 ++++++++++------- README.md | 12 +++++++----- examples/02_research_workflow.py | 2 +- src/felix_agent_sdk/__init__.py | 2 +- src/felix_agent_sdk/cli/run_command.py | 2 +- src/felix_agent_sdk/providers/__init__.py | 2 +- src/felix_agent_sdk/providers/anthropic.py | 2 +- src/felix_agent_sdk/providers/registry.py | 4 ++-- src/felix_agent_sdk/providers/types.py | 2 +- tests/conftest.py | 4 ++-- tests/unit/test_anthropic_provider.py | 8 ++++---- tests/unit/test_cli.py | 4 ++-- 12 files changed, 33 insertions(+), 28 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 8c63e2c..6c78200 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -105,10 +105,13 @@ The original Felix source is at `https://github.com/CalebisGross/felix`. Key fil ## Phased Implementation -- **Phase 1** (current): Package skeleton + provider layer -- **Phase 2** `feat/core-geometry`: Port HelixGeometry, add HelixConfig/HelixPosition -- **Phase 3** `feat/providers`: Tests for provider layer -- **Phase 4** `feat/agents`: Port agent classes, wire to BaseProvider -- **Phase 5** `feat/communication`: Port CentralPost, Spoke, messages -- **Phase 6** `feat/memory`: Port KnowledgeStore, TaskMemory, ContextCompressor -- **Phase 7** `feat/workflows`: Port workflow runner + templates +All porting phases below are **complete** (shipped across v0.1.0–v0.2.x). Current +work is post-port hardening, async support, and release engineering (v0.3.x). + +- **Phase 1** ✅: Package skeleton + provider layer +- **Phase 2** ✅ `feat/core-geometry`: Port HelixGeometry, add HelixConfig/HelixPosition +- **Phase 3** ✅ `feat/providers`: Tests for provider layer +- **Phase 4** ✅ `feat/agents`: Port agent classes, wire to BaseProvider +- **Phase 5** ✅ `feat/communication`: Port CentralPost, Spoke, messages +- **Phase 6** ✅ `feat/memory`: Port KnowledgeStore, TaskMemory, ContextCompressor +- **Phase 7** ✅ `feat/workflows`: Port workflow runner + templates diff --git a/README.md b/README.md index 821e95f..0334719 100644 --- a/README.md +++ b/README.md @@ -131,7 +131,7 @@ Felix provides three specialized agent types, each designed for a phase of the c from felix_agent_sdk import AgentFactory from felix_agent_sdk.providers import AnthropicProvider -provider = AnthropicProvider(model="claude-sonnet-4-5") +provider = AnthropicProvider(model="claude-sonnet-4-6") factory = AgentFactory(provider) team = factory.create_specialized_team("moderate") @@ -252,7 +252,7 @@ Felix supports multiple LLM providers through a clean abstraction layer: from felix_agent_sdk.providers import AnthropicProvider, OpenAIProvider, LocalProvider # Anthropic Claude -provider = AnthropicProvider(model="claude-sonnet-4-5", api_key="sk-...") +provider = AnthropicProvider(model="claude-sonnet-4-6", api_key="sk-...") # OpenAI provider = OpenAIProvider(model="gpt-4o", api_key="sk-...") @@ -269,7 +269,7 @@ provider = LocalProvider( ```bash export FELIX_PROVIDER=anthropic export ANTHROPIC_API_KEY=sk-ant-... -export FELIX_MODEL=claude-sonnet-4-5 +export FELIX_MODEL=claude-sonnet-4-6 ``` ```python @@ -284,9 +284,11 @@ provider = auto_detect_provider() # Reads from environment **Phase 1 — Core SDK (v0.1.0):** Pip-installable package with provider abstraction, core primitives, and documentation. -**Phase 2 — Developer Experience (v0.2.0, Current):** Event system, structured logging, streaming, dynamic spawning, CLI tooling (`felix init`, `felix run`), expanded examples. +**Phase 2 — Developer Experience (v0.2.0):** Event system, structured logging, streaming, dynamic spawning, CLI tooling (`felix init`, `felix run`), expanded examples. -**Phase 3 — Community & Ecosystem:** MCP server integration, vector database connectors, observability adapters, community contribution framework. +**Phase 3 — Hardening & Async (v0.3.0, Current):** Async provider interface (`acomplete`/`astream`), typed provider errors with retry-after extraction, strict type-checking gate, Windows + Python 3.13 CI, SQLite hardening, bounded message history. + +**Phase 4 — Community & Ecosystem:** Tool use / structured output support in the provider layer, MCP server integration, vector database connectors, observability adapters, community contribution framework. --- diff --git a/examples/02_research_workflow.py b/examples/02_research_workflow.py index 66dbad3..4801e36 100644 --- a/examples/02_research_workflow.py +++ b/examples/02_research_workflow.py @@ -34,7 +34,7 @@ def get_provider(provider_name: str): try: from felix_agent_sdk.providers import AnthropicProvider - return AnthropicProvider(model="claude-sonnet-4-5") + return AnthropicProvider(model="claude-sonnet-4-6") except Exception as e: print(f"Could not create Anthropic provider: {e}") return None diff --git a/src/felix_agent_sdk/__init__.py b/src/felix_agent_sdk/__init__.py index 393ccbc..2326c11 100644 --- a/src/felix_agent_sdk/__init__.py +++ b/src/felix_agent_sdk/__init__.py @@ -6,7 +6,7 @@ Quickstart: from felix_agent_sdk.providers import AnthropicProvider - provider = AnthropicProvider(model="claude-sonnet-4-5") + provider = AnthropicProvider(model="claude-sonnet-4-6") Full documentation: https://github.com/AppSprout-dev/felix-agent-sdk """ diff --git a/src/felix_agent_sdk/cli/run_command.py b/src/felix_agent_sdk/cli/run_command.py index 4c578a2..6b99db1 100644 --- a/src/felix_agent_sdk/cli/run_command.py +++ b/src/felix_agent_sdk/cli/run_command.py @@ -120,7 +120,7 @@ def _resolve_provider(name: str, model: str) -> BaseProvider | None: elif name == "anthropic": from felix_agent_sdk.providers import AnthropicProvider - return AnthropicProvider(model=model or "claude-sonnet-4-5") + return AnthropicProvider(model=model or "claude-sonnet-4-6") elif name == "local": from felix_agent_sdk.providers import LocalProvider diff --git a/src/felix_agent_sdk/providers/__init__.py b/src/felix_agent_sdk/providers/__init__.py index 10ba57a..668bf7b 100644 --- a/src/felix_agent_sdk/providers/__init__.py +++ b/src/felix_agent_sdk/providers/__init__.py @@ -7,7 +7,7 @@ from felix_agent_sdk.providers import AnthropicProvider from felix_agent_sdk.providers import auto_detect_provider - provider = AnthropicProvider(model="claude-sonnet-4-5") + provider = AnthropicProvider(model="claude-sonnet-4-6") # or provider = auto_detect_provider() """ diff --git a/src/felix_agent_sdk/providers/anthropic.py b/src/felix_agent_sdk/providers/anthropic.py index 3c84297..fc7d037 100644 --- a/src/felix_agent_sdk/providers/anthropic.py +++ b/src/felix_agent_sdk/providers/anthropic.py @@ -47,7 +47,7 @@ class AnthropicProvider(BaseProvider): def __init__( self, - model: str = "claude-sonnet-4-5", + model: str = "claude-sonnet-4-6", api_key: Optional[str] = None, base_url: Optional[str] = None, **kwargs: Any, diff --git a/src/felix_agent_sdk/providers/registry.py b/src/felix_agent_sdk/providers/registry.py index 8aa6861..5f6d284 100644 --- a/src/felix_agent_sdk/providers/registry.py +++ b/src/felix_agent_sdk/providers/registry.py @@ -145,12 +145,12 @@ def auto_detect_provider() -> BaseProvider: Example: export ANTHROPIC_API_KEY=sk-ant-... - export FELIX_MODEL=claude-sonnet-4-5 + export FELIX_MODEL=claude-sonnet-4-6 >>> from felix_agent_sdk.providers import auto_detect_provider >>> provider = auto_detect_provider() >>> print(provider) - AnthropicProvider(model='claude-sonnet-4-5') + AnthropicProvider(model='claude-sonnet-4-6') """ return ProviderRegistry.auto_detect() diff --git a/src/felix_agent_sdk/providers/types.py b/src/felix_agent_sdk/providers/types.py index b6cd574..458ebaa 100644 --- a/src/felix_agent_sdk/providers/types.py +++ b/src/felix_agent_sdk/providers/types.py @@ -79,7 +79,7 @@ class ProviderConfig: """Configuration for a provider instance. Attributes: - model: Model identifier (e.g., "claude-sonnet-4-5", "gpt-4o"). + model: Model identifier (e.g., "claude-sonnet-4-6", "gpt-4o"). api_key: API key for authentication. If None, reads from environment. base_url: Override the default API endpoint (useful for proxies, local servers). max_retries: Number of retries on transient failures. diff --git a/tests/conftest.py b/tests/conftest.py index 4de71c3..938e057 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -62,7 +62,7 @@ def default_config(): @pytest.fixture def anthropic_config(): - return ProviderConfig(model="claude-sonnet-4-5", api_key="sk-ant-test") + return ProviderConfig(model="claude-sonnet-4-6", api_key="sk-ant-test") @pytest.fixture @@ -99,7 +99,7 @@ def _make(content="Hello!", input_tokens=10, output_tokens=5, stop_reason="end_t response = MagicMock() response.content = [block] - response.model = "claude-sonnet-4-5" + response.model = "claude-sonnet-4-6" response.usage = usage response.stop_reason = stop_reason return response diff --git a/tests/unit/test_anthropic_provider.py b/tests/unit/test_anthropic_provider.py index 850153b..3384fd7 100644 --- a/tests/unit/test_anthropic_provider.py +++ b/tests/unit/test_anthropic_provider.py @@ -26,7 +26,7 @@ # --------------------------------------------------------------------------- -def _make_provider(api_key="sk-ant-test", model="claude-sonnet-4-5", **kwargs): +def _make_provider(api_key="sk-ant-test", model="claude-sonnet-4-6", **kwargs): """Create an AnthropicProvider with a pre-injected mock client.""" p = AnthropicProvider(model=model, api_key=api_key, **kwargs) p._client = MagicMock() @@ -34,7 +34,7 @@ def _make_provider(api_key="sk-ant-test", model="claude-sonnet-4-5", **kwargs): def _mock_response(content="Hello!", input_tokens=10, output_tokens=5, - stop_reason="end_turn", model="claude-sonnet-4-5"): + stop_reason="end_turn", model="claude-sonnet-4-6"): block = MagicMock() block.text = content block.type = "text" @@ -60,7 +60,7 @@ def _mock_response(content="Hello!", input_tokens=10, output_tokens=5, class TestAnthropicProviderInit: def test_default_model(self): p = AnthropicProvider(api_key="k") - assert p.model == "claude-sonnet-4-5" + assert p.model == "claude-sonnet-4-6" def test_custom_model(self): p = AnthropicProvider(model="claude-opus-4-5", api_key="k") @@ -186,7 +186,7 @@ def test_returns_completion_result(self, conversation): assert isinstance(result, CompletionResult) assert result.content == "Hello!" - assert result.model == "claude-sonnet-4-5" + assert result.model == "claude-sonnet-4-6" def test_usage_populated(self, conversation): p = _make_provider() diff --git a/tests/unit/test_cli.py b/tests/unit/test_cli.py index fb88f02..61ffabd 100644 --- a/tests/unit/test_cli.py +++ b/tests/unit/test_cli.py @@ -162,11 +162,11 @@ def test_provider_info(self, tmp_path): path = self._write_yaml(tmp_path, { "task": "Test", "provider": "anthropic", - "model": "claude-sonnet-4-5", + "model": "claude-sonnet-4-6", }) _, _, info = load_workflow_yaml(path) assert info["provider"] == "anthropic" - assert info["model"] == "claude-sonnet-4-5" + assert info["model"] == "claude-sonnet-4-6" def test_missing_task_raises(self, tmp_path): path = self._write_yaml(tmp_path, {"helix": "default"}) From 8e6a0b6843a8f3685f151c4b833f2c94ed8998ab Mon Sep 17 00:00:00 2001 From: jkbennitt Date: Tue, 9 Jun 2026 21:57:05 -0400 Subject: [PATCH 09/12] release: bump version to 0.3.0 Co-Authored-By: Claude Fable 5 --- CHANGELOG.md | 55 +++++++++++++++++++++++++++++++++ src/felix_agent_sdk/_version.py | 2 +- 2 files changed, 56 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0e1396d..bf5fe42 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,61 @@ All notable changes to Felix Agent SDK will be documented in this file. +## [0.3.0] — 2026-06-09 + +Phase 3: Hardening & Async. Driven by a full-repo code review. + +### Added +- **Async provider interface**: `AnthropicProvider`, `OpenAIProvider`, and + `LocalProvider` now implement `acomplete()` and `astream()` using the + vendor SDKs' async clients. +- `Agent.set_progress()` — public API to place an agent at a specific helix + position (replaces orchestrators poking `_progress`). +- `CentralPost(message_history_limit=...)` — processed-message history is + now bounded (default 1000); previously it grew without limit. +- `HubCapacityError` (subclass of `RuntimeError`), exported from + `felix_agent_sdk` and `felix_agent_sdk.communication`. +- `extract_status_code()` / `extract_retry_after()` helpers in + `providers.errors`; `RateLimitError.retry_after` is now populated from + the Retry-After header when present. +- `SQLiteBackend` supports the context manager protocol and is now safe + for multi-threaded use (shared connection + internal lock). + +### Changed +- **Breaking**: `CentralPost.register_agent()` raises `HubCapacityError` at + capacity instead of returning `None`, matching `register_agent_id()` + (which now raises the same subclass — `except RuntimeError` still works). +- `HelixGeometry`/`HelixConfig` `turns` is typed `float` (fractional turns + are geometrically valid and the YAML loader already produced floats). +- `LLMAgent.get_position_info()` now includes `agent_type`. +- `ProviderConfig.__repr__` masks the API key. +- Provider error translation prefers HTTP status codes from vendor SDK + exceptions over message string matching; all translated errors carry + `status_code` when available. +- `ProviderRegistry` detection priorities are stored per provider and + honored during `auto_detect()` (previously priorities were reset on + re-registration and ignored by detection). +- Default Anthropic model updated to `claude-sonnet-4-6`. +- `felix run` now prints the underlying cause when provider creation fails. + +### Fixed +- Dynamic spawning gap analysis received an empty `agent_type` for every + result, so type-based spawn recommendations could never trigger. +- `SQLiteBackend.query()` interpolated `order_by` into SQL unvalidated; + identifiers are now strictly validated and checked against known columns. +- `KnowledgeStore.get_relationships()` no longer returns relationships + involving soft-deleted entries. +- OpenAI `stream()` stops consuming after the final usage chunk. +- CentralPost async queue is created eagerly (removes a lazy-init race). +- 32 mypy strict errors across 13 files; `mypy src/` now gates CI. + +### Infrastructure +- CI: Windows + Ubuntu matrix, Python 3.10–3.13, mypy gate, coverage report. +- Publish workflow verifies the release tag matches `_version.py`. +- `types-PyYAML` added to dev dependencies; explicit `asyncio_mode = "strict"`. + +--- + ## [0.2.2] — 2026-06-09 ### Fixed diff --git a/src/felix_agent_sdk/_version.py b/src/felix_agent_sdk/_version.py index b5fdc75..493f741 100644 --- a/src/felix_agent_sdk/_version.py +++ b/src/felix_agent_sdk/_version.py @@ -1 +1 @@ -__version__ = "0.2.2" +__version__ = "0.3.0" From 124458cfc027e51cf31871f003bb349ae1ba430e Mon Sep 17 00:00:00 2001 From: jkbennitt Date: Tue, 9 Jun 2026 23:13:17 -0400 Subject: [PATCH 10/12] fix: address adversarial review findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Confirmed findings from a 24-agent multi-lens review of the branch diff, each independently verified before fixing: - Agent.set_progress(1.0) now transitions to COMPLETED (mirroring update_position), so the placement survives subsequent get_position calls instead of being recomputed from elapsed time; completed/failed agents can no longer be repositioned into inconsistent states. - OpenAI/Local streaming: a usage payload alone no longer terminates the stream — terminal only when usage arrives with empty choices or a finish_reason. Prevents silent truncation on OpenAI-compatible servers that attach running usage to content chunks (vLLM continuous_usage_stats). Streams are also closed on early break. - KnowledgeStore.add_relationship() validates both entries exist and are active instead of storing relationships get_relationships() can never return. - SQLiteBackend order_by validation falls back to on-disk columns (PRAGMA table_info, cached) so schema evolution and out-of-band tables are handled consistently. - CentralPost: message_history_limit accepts None for unbounded (pre-0.3.0) history; stale 'lazy-initialised' docstring corrected. - extract_status_code/extract_retry_after re-exported from felix_agent_sdk.providers like every other public error symbol. - publish.yml: version-mismatch message reports the actual tag name; mypy gate removed from the release path (CI enforces it per-PR; an unpinned mypy minor release should not block a tagged release). - Test gaps closed: CentralPost async queue API (previously zero coverage), astream early-termination, continuous-usage-stats truncation regression, CLI provider-failure stderr surfacing, set_progress state semantics. Co-authored-by: Caleb Gross <209704970+CalebisGross@users.noreply.github.com> Co-Authored-By: Claude Fable 5 --- .github/workflows/publish.yml | 6 +- CHANGELOG.md | 12 ++- src/felix_agent_sdk/agents/base.py | 15 ++- .../communication/central_post.py | 5 +- src/felix_agent_sdk/memory/backends/sqlite.py | 19 +++- src/felix_agent_sdk/memory/knowledge_store.py | 10 +- src/felix_agent_sdk/providers/__init__.py | 4 + .../providers/openai_provider.py | 59 +++++++---- tests/unit/test_agent_base.py | 21 ++++ tests/unit/test_backends.py | 22 +++++ tests/unit/test_central_post.py | 59 +++++++++++ tests/unit/test_cli.py | 26 +++++ tests/unit/test_knowledge_store.py | 22 +++++ tests/unit/test_openai_provider.py | 99 +++++++++++++++++++ 14 files changed, 348 insertions(+), 31 deletions(-) diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index be084a5..cf54344 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -31,7 +31,7 @@ jobs: PKG_VERSION=$(python -c "from felix_agent_sdk import __version__; print(__version__)") TAG_VERSION="${GITHUB_REF_NAME#v}" if [ "$PKG_VERSION" != "$TAG_VERSION" ]; then - echo "::error::Release tag v$TAG_VERSION does not match package version $PKG_VERSION (_version.py)." + echo "::error::Release tag $GITHUB_REF_NAME does not match package version $PKG_VERSION (_version.py)." exit 1 fi echo "Version check OK: $PKG_VERSION" @@ -39,8 +39,8 @@ jobs: - name: Lint run: ruff check src/ - - name: Type check - run: mypy src/ + # No mypy gate here: CI enforces it on every PR, and an unpinned + # mypy minor release should not be able to block a tagged release. - name: Test run: pytest tests/ -v --tb=short diff --git a/CHANGELOG.md b/CHANGELOG.md index bf5fe42..2ba5b14 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,9 +11,9 @@ Phase 3: Hardening & Async. Driven by a full-repo code review. `LocalProvider` now implement `acomplete()` and `astream()` using the vendor SDKs' async clients. - `Agent.set_progress()` — public API to place an agent at a specific helix - position (replaces orchestrators poking `_progress`). -- `CentralPost(message_history_limit=...)` — processed-message history is - now bounded (default 1000); previously it grew without limit. + position (replaces orchestrators poking `_progress`). Placing at `t=1.0` + transitions the agent to `COMPLETED`, making the placement stable across + subsequent position updates. - `HubCapacityError` (subclass of `RuntimeError`), exported from `felix_agent_sdk` and `felix_agent_sdk.communication`. - `extract_status_code()` / `extract_retry_after()` helpers in @@ -26,6 +26,12 @@ Phase 3: Hardening & Async. Driven by a full-repo code review. - **Breaking**: `CentralPost.register_agent()` raises `HubCapacityError` at capacity instead of returning `None`, matching `register_agent_id()` (which now raises the same subclass — `except RuntimeError` still works). +- **Behavior**: `CentralPost` processed-message history is now bounded + (`message_history_limit=1000` by default); previously it grew without + limit. Pass `message_history_limit=None` to restore unbounded history. +- `KnowledgeStore.add_relationship()` returns `False` instead of storing a + relationship when either entry is missing or soft-deleted (such + relationships were invisible to `get_relationships()` anyway). - `HelixGeometry`/`HelixConfig` `turns` is typed `float` (fractional turns are geometrically valid and the YAML loader already produced floats). - `LLMAgent.get_position_info()` now includes `agent_type`. diff --git a/src/felix_agent_sdk/agents/base.py b/src/felix_agent_sdk/agents/base.py index a7288b6..34eba53 100644 --- a/src/felix_agent_sdk/agents/base.py +++ b/src/felix_agent_sdk/agents/base.py @@ -178,15 +178,26 @@ def set_progress(self, progress: float) -> None: Bypasses time-based progression — used by orchestrators that need an agent at a specific phase (e.g. a synthesis agent at t=1.0). + Mirrors :meth:`update_position` semantics: placing the agent at + ``progress >= 1.0`` transitions it to ``COMPLETED``, which also makes + the placement stable (completed agents are exempt from time-based + recomputation). Placements below 1.0 are a starting point only — a + later :meth:`update_position`/:meth:`get_position` call recomputes + progress from elapsed time. + Raises: - ValueError: If *progress* is outside [0, 1] or the agent has - not been spawned. + ValueError: If *progress* is outside [0, 1], or the agent has + not been spawned, or has already completed/failed. """ if not (0.0 <= progress <= 1.0): raise ValueError("progress must be between 0 and 1") if self._state == AgentState.WAITING: raise ValueError("Cannot set progress of unspawned agent") + if self._state in (AgentState.COMPLETED, AgentState.FAILED): + raise ValueError(f"Cannot set progress of {self._state.value} agent") self._progress = progress + if progress >= 1.0: + self._state = AgentState.COMPLETED def get_position(self, current_time: float) -> Optional[Tuple[float, float, float]]: """Return (x, y, z) on the helix, or ``None`` if not yet spawned.""" diff --git a/src/felix_agent_sdk/communication/central_post.py b/src/felix_agent_sdk/communication/central_post.py index 8498bb6..a77ac50 100644 --- a/src/felix_agent_sdk/communication/central_post.py +++ b/src/felix_agent_sdk/communication/central_post.py @@ -69,7 +69,7 @@ class CentralPost: Architecture: - Sync ``Queue`` for single-threaded / test usage. - - Async ``asyncio.Queue`` (lazy-initialised) for async runtimes. + - Async ``asyncio.Queue`` (created eagerly at construction) for async runtimes. - ``AgentRegistry`` tracks helix positions, phases, and confidence. - Lifecycle callbacks notify callers when agents spawn/complete/fail. @@ -81,6 +81,7 @@ class CentralPost: message_history_limit: Maximum number of processed messages retained for :meth:`get_recent_messages`. Older messages are discarded (the ``total_messages_processed`` counter is unaffected). + Pass ``None`` for unbounded history (pre-0.3.0 behavior). """ def __init__( @@ -89,7 +90,7 @@ def __init__( enable_metrics: bool = False, provider: Optional[BaseProvider] = None, event_bus: Optional[EventBus] = None, - message_history_limit: int = 1000, + message_history_limit: Optional[int] = 1000, ) -> None: self._max_agents = max_agents self._enable_metrics = enable_metrics diff --git a/src/felix_agent_sdk/memory/backends/sqlite.py b/src/felix_agent_sdk/memory/backends/sqlite.py index d20bce8..c131b3b 100644 --- a/src/felix_agent_sdk/memory/backends/sqlite.py +++ b/src/felix_agent_sdk/memory/backends/sqlite.py @@ -69,6 +69,7 @@ def __init__(self, db_path: str = ":memory:") -> None: self._fts_text_cols: dict[str, list[str]] = {} self._initialized_tables: set[str] = set() self._table_columns: dict[str, set[str]] = {} + self._disk_columns: dict[str, set[str]] = {} def __enter__(self) -> SQLiteBackend: return self @@ -128,6 +129,7 @@ def _initialize_locked(self, table: str, schema: dict[str, str]) -> None: self._conn.commit() self._initialized_tables.add(table) self._table_columns[table] = {"_id"} | set(schema.keys()) + self._disk_columns.pop(table, None) # ------------------------------------------------------------------ # CRUD @@ -210,7 +212,7 @@ def query( if order_by: _validate_identifier(order_by, "order_by") - known_cols = self._table_columns.get(table) + known_cols = self._all_columns(table) if known_cols and order_by not in known_cols: raise ValueError( f"Unknown order_by column {order_by!r} for table {table!r}" @@ -263,6 +265,21 @@ def close(self) -> None: # Internal helpers # ------------------------------------------------------------------ + def _all_columns(self, table: str) -> set[str]: + """All known columns for *table*: session schema plus on-disk columns. + + The on-disk lookup (PRAGMA table_info) covers tables created + out-of-band or with a wider schema than this session declared. + Results are cached per table. + """ + cached = self._disk_columns.get(table) + if cached is None: + with self._lock: + cur = self._conn.execute(f"PRAGMA table_info([{table}])") + cached = {row[1] for row in cur.fetchall()} + self._disk_columns[table] = cached + return self._table_columns.get(table, set()) | cached + @staticmethod def _map_type(hint: str) -> str: hint_lower = hint.lower() diff --git a/src/felix_agent_sdk/memory/knowledge_store.py b/src/felix_agent_sdk/memory/knowledge_store.py index a72d277..1898103 100644 --- a/src/felix_agent_sdk/memory/knowledge_store.py +++ b/src/felix_agent_sdk/memory/knowledge_store.py @@ -399,7 +399,15 @@ def add_relationship( relationship_type: str = "related", confidence: float = 0.7, ) -> bool: - """Create a relationship between two entries.""" + """Create a relationship between two entries. + + Returns False (without storing anything) if either entry does not + exist or is soft-deleted — otherwise the relationship would be + stored but permanently invisible to :meth:`get_relationships`. + """ + if not self._is_active_entry(source_id) or not self._is_active_entry(target_id): + return False + rel_id = hashlib.sha256( f"{source_id}:{target_id}:{relationship_type}".encode() ).hexdigest()[:16] diff --git a/src/felix_agent_sdk/providers/__init__.py b/src/felix_agent_sdk/providers/__init__.py index 668bf7b..68a105a 100644 --- a/src/felix_agent_sdk/providers/__init__.py +++ b/src/felix_agent_sdk/providers/__init__.py @@ -25,6 +25,8 @@ ModelNotFoundError, ProviderError, RateLimitError, + extract_retry_after, + extract_status_code, ) from felix_agent_sdk.providers.base import BaseProvider from felix_agent_sdk.providers.anthropic import AnthropicProvider @@ -53,4 +55,6 @@ "RateLimitError", "ModelNotFoundError", "ContextLengthError", + "extract_status_code", + "extract_retry_after", ] diff --git a/src/felix_agent_sdk/providers/openai_provider.py b/src/felix_agent_sdk/providers/openai_provider.py index f88d2c7..abaf7fd 100644 --- a/src/felix_agent_sdk/providers/openai_provider.py +++ b/src/felix_agent_sdk/providers/openai_provider.py @@ -135,12 +135,19 @@ def _to_completion_result(response: Any) -> CompletionResult: @staticmethod def _chunk_to_stream_chunks(chunk: Any) -> Iterator[StreamChunk]: - """Translate one raw OpenAI stream chunk into StreamChunks.""" - if chunk.choices and chunk.choices[0].delta.content: - yield StreamChunk(text=chunk.choices[0].delta.content) + """Translate one raw OpenAI stream chunk into StreamChunks. - # Final chunk with usage (sent last when include_usage is set) - if chunk.usage: + A usage payload alone is not treated as terminal: stock OpenAI sends + usage on a dedicated choices-empty chunk, but some OpenAI-compatible + servers (e.g. vLLM with continuous_usage_stats) attach running usage + to content chunks mid-stream. A chunk is final only when it carries + usage AND is either choices-empty or carries a finish_reason. + """ + choice = chunk.choices[0] if chunk.choices else None + if choice is not None and choice.delta.content: + yield StreamChunk(text=choice.delta.content) + + if chunk.usage and (choice is None or choice.finish_reason): yield StreamChunk( text="", is_final=True, @@ -207,13 +214,19 @@ def stream( try: response = client.chat.completions.create(**create_kwargs) - for chunk in response: - done = False - for stream_chunk in self._chunk_to_stream_chunks(chunk): - done = done or stream_chunk.is_final - yield stream_chunk - if done: - break + try: + for chunk in response: + done = False + for stream_chunk in self._chunk_to_stream_chunks(chunk): + done = done or stream_chunk.is_final + yield stream_chunk + if done: + break + finally: + # Release the HTTP response if we broke out early + close = getattr(response, "close", None) + if callable(close): + close() except Exception as e: raise self._translate_error(e) @@ -233,13 +246,21 @@ async def astream( try: response = await client.chat.completions.create(**create_kwargs) - async for chunk in response: - done = False - for stream_chunk in self._chunk_to_stream_chunks(chunk): - done = done or stream_chunk.is_final - yield stream_chunk - if done: - break + try: + async for chunk in response: + done = False + for stream_chunk in self._chunk_to_stream_chunks(chunk): + done = done or stream_chunk.is_final + yield stream_chunk + if done: + break + finally: + # Release the HTTP response if we broke out early + close = getattr(response, "close", None) + if callable(close): + maybe_coro = close() + if hasattr(maybe_coro, "__await__"): + await maybe_coro except Exception as e: raise self._translate_error(e) diff --git a/tests/unit/test_agent_base.py b/tests/unit/test_agent_base.py index c86f06a..465ea61 100644 --- a/tests/unit/test_agent_base.py +++ b/tests/unit/test_agent_base.py @@ -271,8 +271,23 @@ def test_repr_contains_state(self, agent): class TestSetProgress: def test_set_progress_directly(self, agent): + agent.spawn(0.2) + agent.set_progress(0.5) + assert agent.progress == 0.5 + + def test_set_progress_one_completes_agent(self, agent): + """Placing at t=1.0 mirrors update_position: agent completes.""" + agent.spawn(0.2) + agent.set_progress(1.0) + assert agent.progress == 1.0 + assert agent.state == AgentState.COMPLETED + + def test_set_progress_one_is_stable_across_position_queries(self, agent): + """Regression: time-based recomputation must not clobber a t=1.0 + placement on the next get_position call.""" agent.spawn(0.2) agent.set_progress(1.0) + agent.get_position(0.3) assert agent.progress == 1.0 def test_set_progress_validates_range(self, agent): @@ -283,3 +298,9 @@ def test_set_progress_validates_range(self, agent): def test_set_progress_requires_spawn(self, agent): with pytest.raises(ValueError, match="unspawned"): agent.set_progress(0.5) + + def test_set_progress_rejects_completed_agent(self, agent): + agent.spawn(0.2) + agent.set_progress(1.0) + with pytest.raises(ValueError, match="completed"): + agent.set_progress(0.2) diff --git a/tests/unit/test_backends.py b/tests/unit/test_backends.py index c068feb..3a948bc 100644 --- a/tests/unit/test_backends.py +++ b/tests/unit/test_backends.py @@ -229,3 +229,25 @@ def test_close_then_reopen(self, tmp_path): b2.initialize("t", {"x": "TEXT"}) assert b2.get("t", "1")["x"] == "v" b2.close() + + +class TestOrderByOnDiskColumns: + def test_order_by_allows_columns_outside_session_schema(self, tmp_path): + """Schema evolution: a column present on disk but absent from this + session's (narrower) initialize() schema is a valid order_by.""" + db = str(tmp_path / "evolve.db") + with SQLiteBackend(db_path=db) as b1: + b1.initialize("t", {"name": "TEXT", "extra_col": "TEXT"}) + b1.store("t", "1", {"name": "a", "extra_col": "x"}) + + with SQLiteBackend(db_path=db) as b2: + b2.initialize("t", {"name": "TEXT"}) + rows = b2.query("t", order_by="extra_col") + assert len(rows) == 1 + + def test_order_by_unknown_column_still_rejected(self, tmp_path): + db = str(tmp_path / "evolve2.db") + with SQLiteBackend(db_path=db) as b: + b.initialize("t", {"name": "TEXT"}) + with pytest.raises(ValueError, match="Unknown order_by"): + b.query("t", order_by="never_existed") diff --git a/tests/unit/test_central_post.py b/tests/unit/test_central_post.py index 111c0a7..257d373 100644 --- a/tests/unit/test_central_post.py +++ b/tests/unit/test_central_post.py @@ -467,3 +467,62 @@ def test_shutdown_clears_processed_messages(self): hub.process_next_message() hub.shutdown() assert hub.get_recent_messages() == [] + + +# ------------------------------------------------------------------------- +# Async message queue +# ------------------------------------------------------------------------- + + +class TestAsyncMessageQueue: + @pytest.mark.asyncio + async def test_queue_and_process_async(self): + hub = CentralPost(max_agents=5) + msg = _make_message() + await hub.queue_message_async(msg) + processed = await hub.process_next_message_async() + assert processed is not None + assert processed.message_id == msg.message_id + assert hub.total_messages_processed == 1 + await hub.shutdown_async() + + @pytest.mark.asyncio + async def test_process_empty_async_queue_returns_none(self): + hub = CentralPost(max_agents=5) + assert await hub.process_next_message_async() is None + await hub.shutdown_async() + + @pytest.mark.asyncio + async def test_async_messages_recorded_in_history(self): + hub = CentralPost(max_agents=5) + await hub.queue_message_async(_make_message(sender_id="async-agent")) + await hub.process_next_message_async() + recent = hub.get_recent_messages() + assert len(recent) == 1 + assert recent[0].sender_id == "async-agent" + await hub.shutdown_async() + + @pytest.mark.asyncio + async def test_shutdown_async_drains_queue(self): + hub = CentralPost(max_agents=5) + await hub.queue_message_async(_make_message()) + await hub.queue_message_async(_make_message()) + await hub.shutdown_async() + assert hub.is_active is False + assert await hub.process_next_message_async() is None + + def test_queue_constructed_without_running_loop(self): + """Eager asyncio.Queue construction must not require an event loop.""" + hub = CentralPost(max_agents=5) + assert hub._async_queue is not None + hub.shutdown() + + +class TestUnboundedHistory: + def test_none_limit_keeps_all_messages(self): + hub = CentralPost(max_agents=5, message_history_limit=None) + for i in range(5): + hub.queue_message(_make_message(sender_id=f"agent-{i}")) + hub.process_next_message() + assert len(hub.get_recent_messages(count=100)) == 5 + hub.shutdown() diff --git a/tests/unit/test_cli.py b/tests/unit/test_cli.py index 61ffabd..fe24eb0 100644 --- a/tests/unit/test_cli.py +++ b/tests/unit/test_cli.py @@ -204,3 +204,29 @@ def test_scaffolded_yaml_is_loadable(self, tmp_path): assert task is not None assert len(config.team_composition) > 0 assert info["provider"] == "openai" + + +class TestResolveProviderErrorSurfacing: + def test_provider_failure_cause_printed_to_stderr(self, monkeypatch, capsys): + """Regression: provider creation failures must surface their cause, + not be silently swallowed.""" + import felix_agent_sdk.providers as providers_pkg + from felix_agent_sdk.cli.run_command import _resolve_provider + + def boom(**kwargs): + raise RuntimeError("no api key configured") + + monkeypatch.setattr(providers_pkg, "OpenAIProvider", boom) + assert _resolve_provider("openai", "") is None + assert "no api key configured" in capsys.readouterr().err + + def test_auto_detect_failure_cause_printed_to_stderr(self, monkeypatch, capsys): + import felix_agent_sdk.providers as providers_pkg + from felix_agent_sdk.cli.run_command import _resolve_provider + + def boom(): + raise RuntimeError("detection exploded") + + monkeypatch.setattr(providers_pkg, "auto_detect_provider", boom) + assert _resolve_provider("auto", "") is None + assert "detection exploded" in capsys.readouterr().err diff --git a/tests/unit/test_knowledge_store.py b/tests/unit/test_knowledge_store.py index 8340d6f..e4af74e 100644 --- a/tests/unit/test_knowledge_store.py +++ b/tests/unit/test_knowledge_store.py @@ -311,3 +311,25 @@ def test_summary(self, knowledge_store): def test_semantic_search_not_implemented(self, knowledge_store): with pytest.raises(NotImplementedError): knowledge_store.semantic_search([0.1, 0.2, 0.3]) + + +class TestRelationshipValidation: + def test_add_relationship_rejects_unknown_target(self, knowledge_store): + kid = knowledge_store.add_entry( + KnowledgeType.AGENT_INSIGHT, {"x": 1}, + ConfidenceLevel.HIGH, "a", "d", + ) + assert knowledge_store.add_relationship(kid, "nonexistent-id") is False + assert knowledge_store.get_relationships(kid) == [] + + def test_add_relationship_rejects_deleted_entry(self, knowledge_store): + kid1 = knowledge_store.add_entry( + KnowledgeType.AGENT_INSIGHT, {"x": 1}, + ConfidenceLevel.HIGH, "a", "d", + ) + kid2 = knowledge_store.add_entry( + KnowledgeType.AGENT_INSIGHT, {"x": 2}, + ConfidenceLevel.HIGH, "b", "d", + ) + knowledge_store.delete_entry(kid2) + assert knowledge_store.add_relationship(kid1, kid2) is False diff --git a/tests/unit/test_openai_provider.py b/tests/unit/test_openai_provider.py index 064fa01..cfad004 100644 --- a/tests/unit/test_openai_provider.py +++ b/tests/unit/test_openai_provider.py @@ -620,3 +620,102 @@ def test_async_client_created_once(self): c2 = p._get_async_client() assert c1 is c2 mock_mod.AsyncOpenAI.assert_called_once() + + +# --------------------------------------------------------------------------- +# Continuous usage stats (vLLM-style) must not truncate the stream +# --------------------------------------------------------------------------- + + +def _content_chunk(text, usage=None, finish_reason=None): + """A content chunk with explicit usage/finish_reason (no MagicMock auto-attrs).""" + delta = MagicMock() + delta.content = text + choice = MagicMock() + choice.delta = delta + choice.finish_reason = finish_reason + chunk = MagicMock() + chunk.choices = [choice] + chunk.usage = usage + return chunk + + +def _usage(prompt=10, completion=5): + usage = MagicMock() + usage.prompt_tokens = prompt + usage.completion_tokens = completion + usage.total_tokens = prompt + completion + return usage + + +class TestOpenAIContinuousUsageStats: + def test_mid_stream_usage_does_not_truncate(self, user_message): + """Regression: servers like vLLM (continuous_usage_stats) attach + running usage to every content chunk; the stream must not stop at + the first one.""" + p = _make_provider() + chunks = [ + _content_chunk("The ", usage=_usage(10, 1)), + _content_chunk("quick ", usage=_usage(10, 2)), + _content_chunk("fox", usage=_usage(10, 3), finish_reason="stop"), + ] + p._client.chat.completions.create.return_value = iter(chunks) + + out = list(p.stream([user_message])) + texts = [c.text for c in out if not c.is_final] + assert texts == ["The ", "quick ", "fox"] + # finish_reason chunk carries usage -> terminal + assert out[-1].is_final is True + assert out[-1].usage["completion_tokens"] == 3 + + def test_usage_only_final_chunk_still_terminal(self, user_message): + """Stock OpenAI shape: dedicated choices-empty usage chunk.""" + p = _make_provider() + final = MagicMock() + final.choices = [] + final.usage = _usage() + p._client.chat.completions.create.return_value = iter( + [_content_chunk("hi"), final] + ) + out = list(p.stream([user_message])) + assert out[-1].is_final is True + + +class TestOpenAIAstreamTermination: + @pytest.mark.asyncio + async def test_astream_stops_after_final_chunk(self, user_message): + """Async twin of the poisoned-chunk guard: chunks after the terminal + usage chunk must never be consumed.""" + from unittest.mock import AsyncMock + + consumed = [] + final = MagicMock() + final.choices = [] + final.usage = _usage() + poisoned = MagicMock() + poisoned.choices = [] + poisoned.usage = None + + class _TrackingAsyncStream: + def __init__(self, chunks): + self._chunks = list(chunks) + + def __aiter__(self): + return self + + async def __anext__(self): + if not self._chunks: + raise StopAsyncIteration + chunk = self._chunks.pop(0) + consumed.append(chunk) + return chunk + + p = _make_provider() + p._async_client = MagicMock() + p._async_client.chat.completions.create = AsyncMock( + return_value=_TrackingAsyncStream([_content_chunk("a"), final, poisoned]) + ) + + out = [c async for c in p.astream([user_message])] + assert out[-1].is_final is True + assert poisoned not in consumed From 8e89f93566ed1bbe63e0f70b93ef33fc03b6d1b2 Mon Sep 17 00:00:00 2001 From: jkbennitt Date: Tue, 9 Jun 2026 23:24:27 -0400 Subject: [PATCH 11/12] fix: Windows timer-resolution test failures on Python < 3.13 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit First run of the new Windows CI matrix failed on 3.10-3.12 (3.13 passed): time.monotonic() and time.time() tick at ~15.6ms on Windows before Python 3.13 switched to QueryPerformanceCounter, so sub-millisecond mocked provider calls measured 0.0 elapsed and a delete + cleanup landing in the same tick did not register as older than the cutoff. - Measure durations with time.perf_counter() (high resolution on all platforms/versions) in LLMAgent and the workflow runner. Event/stream timestamps keep time.monotonic() — they only need ordering. - test_cleanup_old_entries backdates the tombstone instead of relying on two distinct timer ticks. Co-authored-by: Caleb Gross <209704970+CalebisGross@users.noreply.github.com> Co-Authored-By: Claude Fable 5 --- src/felix_agent_sdk/agents/llm_agent.py | 8 ++++---- src/felix_agent_sdk/workflows/runner.py | 4 ++-- tests/unit/test_knowledge_store.py | 6 ++++++ 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/src/felix_agent_sdk/agents/llm_agent.py b/src/felix_agent_sdk/agents/llm_agent.py index 9ca66a5..d5b57a4 100644 --- a/src/felix_agent_sdk/agents/llm_agent.py +++ b/src/felix_agent_sdk/agents/llm_agent.py @@ -333,7 +333,7 @@ def process_task(self, task: LLMTask) -> LLMResult: This is the primary entry point for running an agent on a task. """ - start = time.monotonic() + start = time.perf_counter() self.emit_event( EventType.TASK_STARTED, @@ -350,7 +350,7 @@ def process_task(self, task: LLMTask) -> LLMResult: confidence = self.calculate_confidence(completion.content) self.record_confidence(confidence) - elapsed = time.monotonic() - start + elapsed = time.perf_counter() - start self.total_tokens_used += completion.total_tokens self.total_processing_time += elapsed @@ -400,7 +400,7 @@ def process_task_streaming( :class:`LLMResult` identical to what ``process_task`` would return for the same content. """ - start = time.monotonic() + start = time.perf_counter() self.emit_event( EventType.TASK_STARTED, @@ -431,7 +431,7 @@ def process_task_streaming( confidence = self.calculate_confidence(completion.content) self.record_confidence(confidence) - elapsed = time.monotonic() - start + elapsed = time.perf_counter() - start self.total_tokens_used += completion.total_tokens self.total_processing_time += elapsed diff --git a/src/felix_agent_sdk/workflows/runner.py b/src/felix_agent_sdk/workflows/runner.py index 18e5412..304bca2 100644 --- a/src/felix_agent_sdk/workflows/runner.py +++ b/src/felix_agent_sdk/workflows/runner.py @@ -76,7 +76,7 @@ def run(self, task_description: str, context: str = "") -> WorkflowResult: Returns: :class:`WorkflowResult` with synthesised output and metadata. """ - start = time.monotonic() + start = time.perf_counter() # --- Setup --- hub = CentralPost(max_agents=self._config.max_agents, event_bus=self._event_bus) @@ -173,7 +173,7 @@ def run(self, task_description: str, context: str = "") -> WorkflowResult: if all_results: final_confidence = sum(r.confidence for r in all_results) / len(all_results) - elapsed = time.monotonic() - start + elapsed = time.perf_counter() - start total_tokens = sum(r.token_budget_used for r in all_results) result = WorkflowResult( diff --git a/tests/unit/test_knowledge_store.py b/tests/unit/test_knowledge_store.py index e4af74e..83cc773 100644 --- a/tests/unit/test_knowledge_store.py +++ b/tests/unit/test_knowledge_store.py @@ -295,6 +295,12 @@ def test_cleanup_old_entries(self, knowledge_store): ConfidenceLevel.LOW, "a", "d", ) knowledge_store.delete_entry(kid) + # Backdate the tombstone so the strict updated_at < cutoff comparison + # holds regardless of platform timer resolution (Windows time.time() + # ticks at ~15.6ms before Python 3.13). + raw = knowledge_store._backend.get("knowledge_entries", kid) + raw["updated_at"] = raw["updated_at"] - 60.0 + knowledge_store._backend.store("knowledge_entries", kid, raw) # With max_age_days=0 it should purge immediately removed = knowledge_store.cleanup_old_entries(max_age_days=0) assert removed >= 1 From 123eeb3630ea7b7169191c0ad2c9aec2b9ef155f Mon Sep 17 00:00:00 2001 From: jkbennitt Date: Wed, 10 Jun 2026 03:55:40 -0400 Subject: [PATCH 12/12] fix: address Caleb's PR #33 review findings Re-review (RLE-consumer adversarial pass) found 7 should-fixes: 1. Async Fable sampling gate untested -> add parametrized acomplete/astream tests asserting temperature is omitted for claude-fable/opus-4.7+ models (the invariant RLE's Fable runs depend on). 2. stream()/astream() close() ran inside the translated try, so a teardown error after a successful stream surfaced as ProviderError. close() is now best-effort (_safe_close/_safe_aclose, debug-logged) outside the translate-error region. 3. SpokeManager.create_spoke(auto_connect=True) silently returned a disconnected spoke at capacity -> now raises HubCapacityError and does not retain the spoke. 4. SQL identifier validation: switch to re.fullmatch (closes the trailing -newline bypass that ^...$ allowed) and explicitly reject ']' so it cannot break out of [..] bracket quoting. 5. CI: add Python 3.14 to the matrix (RLE requires >=3.14) and a dedicated typecheck job that installs .[all,dev] so mypy checks the provider layer against real vendor types instead of the ignore-missing-imports Any. 6. _translate_error: map HTTP 404 -> ModelNotFoundError, and match the SDK's NotFoundError class name (the old 'not_found' substring never matched); applied to both OpenAI and Anthropic. 7. LocalProvider async (acomplete/astream) was inherited but untested -> add coverage. Also: order_by validation now falls back to on-disk columns (PRAGMA) so schema evolution still sorts; changelog documents the no-action behavior notes (403->AuthenticationError, empty-string agent_id, no-usage streams emit no is_final, extract_retry_after numeric-only). 911 passed (+20); mypy clean with vendor SDKs installed; ruff clean. Co-authored-by: Caleb Gross <209704970+CalebisGross@users.noreply.github.com> Co-Authored-By: Claude Fable 5 --- .github/workflows/ci.yml | 29 +++++- CHANGELOG.md | 38 ++++++- pyproject.toml | 1 + src/felix_agent_sdk/communication/spoke.py | 10 +- src/felix_agent_sdk/memory/backends/sqlite.py | 16 ++- src/felix_agent_sdk/providers/anthropic.py | 14 ++- .../providers/openai_provider.py | 91 +++++++++++------ tests/unit/test_anthropic_provider.py | 70 +++++++++++++ tests/unit/test_backends.py | 17 ++++ tests/unit/test_local_provider.py | 98 +++++++++++++++++++ tests/unit/test_openai_provider.py | 52 ++++++++++ tests/unit/test_spoke.py | 25 +++++ 12 files changed, 411 insertions(+), 50 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9c29052..07d7ec7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -13,7 +13,7 @@ jobs: fail-fast: false matrix: os: [ubuntu-latest, windows-latest] - python-version: ["3.10", "3.11", "3.12", "3.13"] + python-version: ["3.10", "3.11", "3.12", "3.13", "3.14"] steps: - uses: actions/checkout@v4 @@ -22,6 +22,7 @@ jobs: uses: actions/setup-python@v5 with: python-version: ${{ matrix.python-version }} + allow-prereleases: true cache: 'pip' - name: Install dependencies @@ -32,12 +33,32 @@ jobs: - name: Lint run: ruff check src/ - - name: Type check - run: mypy src/ - - name: Test run: pytest tests/ -v --tb=short --cov=felix_agent_sdk --cov-report=term-missing + typecheck: + name: Type check (mypy, vendor SDKs installed) + runs-on: ubuntu-latest + + steps: + - uses: actions/checkout@v4 + + - name: Set up Python + uses: actions/setup-python@v5 + with: + python-version: "3.12" + cache: "pip" + + # Install the optional provider SDKs so mypy checks the provider layer + # against their real types rather than the ignore-missing-imports `Any`. + - name: Install dependencies + run: | + python -m pip install --upgrade pip + pip install -e ".[all,dev]" + + - name: Type check + run: mypy src/ + build: name: Verify package builds runs-on: ubuntu-latest diff --git a/CHANGELOG.md b/CHANGELOG.md index 2ba5b14..080f801 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,9 +2,10 @@ All notable changes to Felix Agent SDK will be documented in this file. -## [0.3.0] — 2026-06-09 +## [0.3.0] — 2026-06-10 -Phase 3: Hardening & Async. Driven by a full-repo code review. +Phase 3: Hardening & Async. Driven by a full-repo code review and a follow-up +adversarial re-review from the RLE-consumer perspective. ### Added - **Async provider interface**: `AnthropicProvider`, `OpenAIProvider`, and @@ -44,23 +45,50 @@ Phase 3: Hardening & Async. Driven by a full-repo code review. re-registration and ignored by detection). - Default Anthropic model updated to `claude-sonnet-4-6`. - `felix run` now prints the underlying cause when provider creation fails. +- `SpokeManager.create_spoke(auto_connect=True)` raises `HubCapacityError` + when the hub is at capacity instead of silently returning a disconnected + spoke (the spoke is not retained on failure). +- SQL identifier validation uses `re.fullmatch` (closing a trailing-newline + bypass) and explicitly rejects `]` so it cannot break out of `[..]` + bracket quoting. ### Fixed - Dynamic spawning gap analysis received an empty `agent_type` for every result, so type-based spawn recommendations could never trigger. - `SQLiteBackend.query()` interpolated `order_by` into SQL unvalidated; - identifiers are now strictly validated and checked against known columns. + identifiers are now strictly validated and checked against known columns + (session schema plus on-disk columns, so schema evolution still sorts). - `KnowledgeStore.get_relationships()` no longer returns relationships involving soft-deleted entries. -- OpenAI `stream()` stops consuming after the final usage chunk. +- OpenAI/Local `stream()`/`astream()` stop consuming only at a true terminal + chunk (usage with empty choices or a finish_reason), so servers that emit + running usage on content chunks (e.g. vLLM `continuous_usage_stats`) are no + longer silently truncated. A `close()` failure during stream teardown is + swallowed (debug-logged) instead of masking a successful stream as a + `ProviderError`. +- Provider error translation maps HTTP 404 to `ModelNotFoundError`, and the + SDK exception class name (`NotFoundError`) is now matched correctly (the + previous `"not_found"` substring check could never match). - CentralPost async queue is created eagerly (removes a lazy-init race). - 32 mypy strict errors across 13 files; `mypy src/` now gates CI. ### Infrastructure -- CI: Windows + Ubuntu matrix, Python 3.10–3.13, mypy gate, coverage report. +- CI: Windows + Ubuntu matrix, Python 3.10–3.14, coverage report. A dedicated + type-check job installs the provider SDKs (`.[all,dev]`) so mypy checks the + provider layer against real vendor types rather than `Any`. - Publish workflow verifies the release tag matches `_version.py`. - `types-PyYAML` added to dev dependencies; explicit `asyncio_mode = "strict"`. +### Notes (behavior worth knowing) +- Provider HTTP 403 now maps to `AuthenticationError` — a per-model permission + denial aborts rather than being retried as a generic error. +- `CentralPost.register_agent` no longer substitutes `id(agent)` for an + empty-string `agent_id`; an empty id registers verbatim as `""`. +- A stream that completes without the server ever sending usage emits no + `is_final` event (there is no terminal usage chunk to mark). +- `extract_retry_after` reads the numeric `Retry-After` header form only; + `retry-after-ms` and HTTP-date forms are not yet parsed. + --- ## [0.2.2] — 2026-06-09 diff --git a/pyproject.toml b/pyproject.toml index dc38b4c..60eae9a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -25,6 +25,7 @@ classifiers = [ "Programming Language :: Python :: 3.11", "Programming Language :: Python :: 3.12", "Programming Language :: Python :: 3.13", + "Programming Language :: Python :: 3.14", "Topic :: Scientific/Engineering :: Artificial Intelligence", ] diff --git a/src/felix_agent_sdk/communication/spoke.py b/src/felix_agent_sdk/communication/spoke.py index 1d3ca43..aa436fd 100644 --- a/src/felix_agent_sdk/communication/spoke.py +++ b/src/felix_agent_sdk/communication/spoke.py @@ -356,10 +356,16 @@ def create_spoke( Returns: The newly created (and optionally connected) Spoke. + + Raises: + HubCapacityError: If ``auto_connect`` is requested but the hub is + at capacity. The spoke is not registered in that case. """ spoke = Spoke(agent_id=agent_id, hub=self._hub, agent=agent) - if auto_connect: - spoke.connect(metadata) + if auto_connect and not spoke.connect(metadata): + raise HubCapacityError( + f"Cannot create spoke for {agent_id!r}: hub is at capacity" + ) self._spokes[agent_id] = spoke return spoke diff --git a/src/felix_agent_sdk/memory/backends/sqlite.py b/src/felix_agent_sdk/memory/backends/sqlite.py index c131b3b..cdc998d 100644 --- a/src/felix_agent_sdk/memory/backends/sqlite.py +++ b/src/felix_agent_sdk/memory/backends/sqlite.py @@ -26,18 +26,24 @@ "$lte": "<=", } -# Column/table names are interpolated into SQL and must be plain identifiers. -_IDENTIFIER_RE = re.compile(r"^[A-Za-z_][A-Za-z0-9_]*$") +# Column/table names are interpolated into SQL (inside [..] quoting) and must +# be plain identifiers. No anchors: fullmatch requires the *entire* string to +# match, which rejects the trailing-newline that ``$`` would otherwise allow. +_IDENTIFIER_RE = re.compile(r"[A-Za-z_][A-Za-z0-9_]*") def _validate_identifier(name: str, context: str) -> None: """Reject anything that is not a bare SQL identifier. + Identifiers are interpolated into ``[name]`` bracket quoting, so a literal + ``]`` is rejected explicitly (it would close the quoting and allow + injection). ``re.fullmatch`` rejects embedded/trailing newlines. + Raises: - ValueError: If *name* contains characters outside [A-Za-z0-9_] - or does not start with a letter/underscore. + ValueError: If *name* is not a bare identifier (letters, digits, + underscores; not starting with a digit) or contains ``]``. """ - if not _IDENTIFIER_RE.match(name): + if "]" in name or not _IDENTIFIER_RE.fullmatch(name): raise ValueError(f"Invalid SQL identifier for {context}: {name!r}") diff --git a/src/felix_agent_sdk/providers/anthropic.py b/src/felix_agent_sdk/providers/anthropic.py index fc7d037..d41766e 100644 --- a/src/felix_agent_sdk/providers/anthropic.py +++ b/src/felix_agent_sdk/providers/anthropic.py @@ -293,11 +293,15 @@ def _translate_error(self, error: Exception) -> ProviderError: return ContextLengthError(error_str, provider="anthropic", status_code=status_code) # Bare "model" in the message is not enough: param-rejection 400s # ("temperature is not supported on this model") must stay generic. - if "notfound" in error_type.lower().replace("_", "") or ( - "model" in lowered - and any( - phrase in lowered - for phrase in ("not exist", "not found", "not available", "unknown") + if ( + status_code == 404 + or "notfound" in error_type.lower().replace("_", "") + or ( + "model" in lowered + and any( + phrase in lowered + for phrase in ("not exist", "not found", "not available", "unknown") + ) ) ): return ModelNotFoundError(error_str, provider="anthropic", status_code=status_code) diff --git a/src/felix_agent_sdk/providers/openai_provider.py b/src/felix_agent_sdk/providers/openai_provider.py index abaf7fd..3467054 100644 --- a/src/felix_agent_sdk/providers/openai_provider.py +++ b/src/felix_agent_sdk/providers/openai_provider.py @@ -8,6 +8,7 @@ from __future__ import annotations +import logging import os from typing import Any, AsyncIterator, Dict, Iterator, List, Optional, Sequence @@ -23,6 +24,8 @@ ) from .types import ChatMessage, CompletionResult, ProviderConfig, StreamChunk +logger = logging.getLogger("felix.providers") + class OpenAIProvider(BaseProvider): """Provider for OpenAI models and OpenAI-compatible APIs. @@ -198,6 +201,34 @@ async def acomplete( except Exception as e: raise self._translate_error(e) + @staticmethod + def _safe_close(response: Any) -> None: + """Best-effort release of a stream's HTTP response. + + Teardown errors must never surface: a close() failure after the + stream has already yielded its content would otherwise be caught by + the surrounding translate-error handler and turn a fully successful + stream into a ProviderError. + """ + close = getattr(response, "close", None) + if callable(close): + try: + close() + except Exception: + logger.debug("Error closing OpenAI stream response", exc_info=True) + + @staticmethod + async def _safe_aclose(response: Any) -> None: + """Async counterpart of :meth:`_safe_close`.""" + close = getattr(response, "close", None) + if callable(close): + try: + result = close() + if hasattr(result, "__await__"): + await result + except Exception: + logger.debug("Error closing OpenAI async stream response", exc_info=True) + def stream( self, messages: Sequence[ChatMessage], @@ -214,22 +245,24 @@ def stream( try: response = client.chat.completions.create(**create_kwargs) - try: - for chunk in response: - done = False - for stream_chunk in self._chunk_to_stream_chunks(chunk): - done = done or stream_chunk.is_final - yield stream_chunk - if done: - break - finally: - # Release the HTTP response if we broke out early - close = getattr(response, "close", None) - if callable(close): - close() except Exception as e: raise self._translate_error(e) + try: + for chunk in response: + done = False + for stream_chunk in self._chunk_to_stream_chunks(chunk): + done = done or stream_chunk.is_final + yield stream_chunk + if done: + break + except Exception as e: + raise self._translate_error(e) + finally: + # Release the HTTP response if we broke out early. Best-effort: + # a teardown error must not mask a successful stream. + self._safe_close(response) + async def astream( self, messages: Sequence[ChatMessage], @@ -246,24 +279,22 @@ async def astream( try: response = await client.chat.completions.create(**create_kwargs) - try: - async for chunk in response: - done = False - for stream_chunk in self._chunk_to_stream_chunks(chunk): - done = done or stream_chunk.is_final - yield stream_chunk - if done: - break - finally: - # Release the HTTP response if we broke out early - close = getattr(response, "close", None) - if callable(close): - maybe_coro = close() - if hasattr(maybe_coro, "__await__"): - await maybe_coro except Exception as e: raise self._translate_error(e) + try: + async for chunk in response: + done = False + for stream_chunk in self._chunk_to_stream_chunks(chunk): + done = done or stream_chunk.is_final + yield stream_chunk + if done: + break + except Exception as e: + raise self._translate_error(e) + finally: + await self._safe_aclose(response) + def count_tokens(self, messages: Sequence[ChatMessage]) -> int: """Estimate tokens using tiktoken when available. @@ -304,7 +335,9 @@ def _translate_error(self, error: Exception) -> ProviderError: provider="openai", status_code=status_code, ) - if "not_found" in error_type.lower(): + # The SDK's class is ``NotFoundError`` -> ``notfounderror`` after + # stripping underscores; a plain ``"not_found"`` substring never matches. + if status_code == 404 or "notfound" in error_type.lower().replace("_", ""): return ModelNotFoundError(error_str, provider="openai", status_code=status_code) if "context_length" in lowered or "maximum context" in lowered: return ContextLengthError(error_str, provider="openai", status_code=status_code) diff --git a/tests/unit/test_anthropic_provider.py b/tests/unit/test_anthropic_provider.py index 3384fd7..ae47f90 100644 --- a/tests/unit/test_anthropic_provider.py +++ b/tests/unit/test_anthropic_provider.py @@ -646,3 +646,73 @@ def test_async_client_created_once(self): c2 = p._get_async_client() assert c1 is c2 mock_mod.AsyncAnthropic.assert_called_once() + + +class TestAnthropicAsyncSamplingGate: + """The Fable/Opus-4.7+ sampling gate must hold on the async paths too — + RLE's Fable runs depend on temperature never reaching the API.""" + + @pytest.mark.asyncio + @pytest.mark.parametrize( + "model", + ["claude-fable-5", "claude-opus-4-7", "claude-opus-4-8"], + ) + async def test_acomplete_omits_temperature_for_no_sampling_models( + self, user_message, model + ): + from unittest.mock import AsyncMock + + p = _make_provider(model=model) + p._async_client = MagicMock() + p._async_client.messages.create = AsyncMock(return_value=_mock_response()) + + await p.acomplete([user_message], temperature=0.5) + call_kwargs = p._async_client.messages.create.call_args[1] + assert "temperature" not in call_kwargs + + @pytest.mark.asyncio + @pytest.mark.parametrize( + "model", + ["claude-fable-5", "claude-opus-4-7", "claude-opus-4-8"], + ) + async def test_astream_omits_temperature_for_no_sampling_models( + self, user_message, model + ): + p = _make_provider(model=model) + p._async_client = MagicMock() + p._async_client.messages.stream.return_value = _MockAsyncMessageStream( + [], _mock_response() + ) + + [chunk async for chunk in p.astream([user_message], temperature=0.5)] + call_kwargs = p._async_client.messages.stream.call_args[1] + assert "temperature" not in call_kwargs + + @pytest.mark.asyncio + async def test_acomplete_keeps_temperature_for_sampling_models(self, user_message): + from unittest.mock import AsyncMock + + p = _make_provider(model="claude-sonnet-4-6") + p._async_client = MagicMock() + p._async_client.messages.create = AsyncMock(return_value=_mock_response()) + + await p.acomplete([user_message], temperature=0.5) + call_kwargs = p._async_client.messages.create.call_args[1] + assert call_kwargs["temperature"] == 0.5 + + +class TestAnthropicModelNotFoundTranslation: + def test_status_404_maps_to_model_not_found(self): + p = _make_provider() + result = p._translate_error(_status_error("missing", 404)) + assert isinstance(result, ModelNotFoundError) + assert result.status_code == 404 + + def test_sdk_notfounderror_class_name_maps(self): + p = _make_provider() + + class NotFoundError(Exception): + pass + + result = p._translate_error(NotFoundError("the model is gone")) + assert isinstance(result, ModelNotFoundError) diff --git a/tests/unit/test_backends.py b/tests/unit/test_backends.py index 3a948bc..e20771b 100644 --- a/tests/unit/test_backends.py +++ b/tests/unit/test_backends.py @@ -251,3 +251,20 @@ def test_order_by_unknown_column_still_rejected(self, tmp_path): b.initialize("t", {"name": "TEXT"}) with pytest.raises(ValueError, match="Unknown order_by"): b.query("t", order_by="never_existed") + + +class TestIdentifierValidationEdgeCases: + def test_trailing_newline_rejected(self, memory_backend): + """re.fullmatch closes the ``$``-allows-trailing-newline bypass.""" + memory_backend.initialize("t", _SCHEMA) + with pytest.raises(ValueError, match="identifier"): + memory_backend.query("t", order_by="name\n") + + def test_bracket_rejected_in_table_name(self, memory_backend): + with pytest.raises(ValueError, match="identifier"): + memory_backend.initialize("foo]bar", _SCHEMA) + + def test_bracket_rejected_in_order_by(self, memory_backend): + memory_backend.initialize("t", _SCHEMA) + with pytest.raises(ValueError, match="identifier"): + memory_backend.query("t", order_by="name]") diff --git a/tests/unit/test_local_provider.py b/tests/unit/test_local_provider.py index c33e05f..4bbcef8 100644 --- a/tests/unit/test_local_provider.py +++ b/tests/unit/test_local_provider.py @@ -4,6 +4,7 @@ from unittest.mock import MagicMock, patch +import pytest from felix_agent_sdk.providers.local import LocalProvider from felix_agent_sdk.providers.openai_provider import OpenAIProvider @@ -186,3 +187,100 @@ def side_effect(name, *args, **kwargs): # Just verify char heuristic works directly result = p.count_tokens(messages) assert result == 200 // 4 + + +# --------------------------------------------------------------------------- +# Async interface (inherited from OpenAIProvider, exercised on LocalProvider) +# --------------------------------------------------------------------------- + + +def _mock_chat_response(content="hi", prompt=5, completion=3): + message = MagicMock() + message.content = content + choice = MagicMock() + choice.message = message + choice.finish_reason = "stop" + usage = MagicMock() + usage.prompt_tokens = prompt + usage.completion_tokens = completion + usage.total_tokens = prompt + completion + response = MagicMock() + response.choices = [choice] + response.model = "local-model" + response.usage = usage + return response + + +class _AsyncChunkStream: + def __init__(self, chunks): + self._chunks = list(chunks) + + def __aiter__(self): + return self + + async def __anext__(self): + if not self._chunks: + raise StopAsyncIteration + return self._chunks.pop(0) + + +def _content_chunk(text, usage=None, finish_reason=None): + delta = MagicMock() + delta.content = text + choice = MagicMock() + choice.delta = delta + choice.finish_reason = finish_reason + chunk = MagicMock() + chunk.choices = [choice] + chunk.usage = usage + return chunk + + +def _usage(prompt=5, completion=3): + u = MagicMock() + u.prompt_tokens = prompt + u.completion_tokens = completion + u.total_tokens = prompt + completion + return u + + +class TestLocalProviderAsync: + @pytest.mark.asyncio + async def test_acomplete(self): + from unittest.mock import AsyncMock + + p = LocalProvider() + p._async_client = MagicMock() + p._async_client.chat.completions.create = AsyncMock( + return_value=_mock_chat_response("local async") + ) + msgs = [ChatMessage(role=MessageRole.USER, content="hi")] + result = await p.acomplete(msgs) + assert result.content == "local async" + + @pytest.mark.asyncio + async def test_astream(self): + from unittest.mock import AsyncMock + + p = LocalProvider() + p._async_client = MagicMock() + final = MagicMock() + final.choices = [] + final.usage = _usage() + p._async_client.chat.completions.create = AsyncMock( + return_value=_AsyncChunkStream([_content_chunk("tok"), final]) + ) + msgs = [ChatMessage(role=MessageRole.USER, content="hi")] + out = [c async for c in p.astream(msgs)] + assert [c.text for c in out if not c.is_final] == ["tok"] + assert out[-1].is_final is True + + def test_async_client_uses_async_openai(self): + mock_mod = MagicMock() + mock_mod.AsyncOpenAI.return_value = MagicMock() + with patch.dict("sys.modules", {"openai": mock_mod}): + p = LocalProvider() + p._get_async_client() + mock_mod.AsyncOpenAI.assert_called_once() + # local default base_url is forwarded to the async client + assert mock_mod.AsyncOpenAI.call_args[1]["base_url"] == p.config.base_url diff --git a/tests/unit/test_openai_provider.py b/tests/unit/test_openai_provider.py index cfad004..6752f47 100644 --- a/tests/unit/test_openai_provider.py +++ b/tests/unit/test_openai_provider.py @@ -719,3 +719,55 @@ async def __anext__(self): out = [c async for c in p.astream([user_message])] assert out[-1].is_final is True assert poisoned not in consumed + + +class TestOpenAIModelNotFoundTranslation: + def test_status_404_maps_to_model_not_found(self): + p = _make_provider() + result = p._translate_error(_status_error("missing", 404)) + assert isinstance(result, ModelNotFoundError) + assert result.status_code == 404 + + def test_sdk_notfounderror_class_name_maps(self): + """Regression: 'not_found' substring never matched the SDK's + NotFoundError class name.""" + p = _make_provider() + + class NotFoundError(Exception): + pass + + result = p._translate_error(NotFoundError("model gone")) + assert isinstance(result, ModelNotFoundError) + + +class _ClosableStream: + """Iterable stream whose close() raises, mimicking a teardown failure.""" + + def __init__(self, chunks, raise_on_close=True): + self._chunks = list(chunks) + self._raise_on_close = raise_on_close + self.closed = False + + def __iter__(self): + return iter(self._chunks) + + def close(self): + self.closed = True + if self._raise_on_close: + raise RuntimeError("connection reset during teardown") + + +class TestOpenAIStreamCloseErrors: + def test_close_failure_does_not_mask_successful_stream(self, user_message): + """A close() raising after the final chunk must not turn a complete + stream into a ProviderError.""" + p = _make_provider() + final = MagicMock() + final.choices = [] + final.usage = _usage() + stream = _ClosableStream([_content_chunk("hi"), final]) + p._client.chat.completions.create.return_value = stream + + out = list(p.stream([user_message])) # must not raise + assert out[-1].is_final is True + assert stream.closed is True diff --git a/tests/unit/test_spoke.py b/tests/unit/test_spoke.py index 277f06f..12b6477 100644 --- a/tests/unit/test_spoke.py +++ b/tests/unit/test_spoke.py @@ -4,6 +4,7 @@ from unittest.mock import MagicMock +import pytest from felix_agent_sdk.communication.central_post import CentralPost from felix_agent_sdk.communication.messages import Message, MessageType @@ -474,3 +475,27 @@ def test_repr(self, central_post, spoke_manager): r = repr(spoke_manager) assert "spokes=1" in r assert "active=1" in r + + +# ------------------------------------------------------------------------- +# SpokeManager — capacity propagation +# ------------------------------------------------------------------------- + + +class TestSpokeManagerCapacity: + def test_create_spoke_raises_at_capacity(self, central_post, spoke_manager): + from felix_agent_sdk.communication.central_post import HubCapacityError + + # Fill the hub (fixture max_agents=10) + for i in range(10): + spoke_manager.create_spoke(f"agent-{i}") + with pytest.raises(HubCapacityError): + spoke_manager.create_spoke("overflow") + # The failed spoke must not be retained + assert spoke_manager.get_spoke("overflow") is None + + def test_no_auto_connect_never_raises_at_capacity(self, central_post, spoke_manager): + for i in range(10): + spoke_manager.create_spoke(f"agent-{i}") + spoke = spoke_manager.create_spoke("overflow", auto_connect=False) + assert spoke.is_connected is False