fix: Address audit findings in orchestrators package

CRITICAL fixes:
- HierarchicalOrchestrator now accepts config parameter and uses max_iterations
- Added timeout protection (DEFAULT_TIMEOUT_SECONDS = 300.0)

HIGH priority fixes:
- Added OrchestratorProtocol to base.py for type safety
- Factory returns OrchestratorProtocol instead of Any

MEDIUM priority fixes:
- Added deprecation warnings for MagenticOrchestrator class alias
- Added deprecation warning for get_magentic_orchestrator() function
- Improved error handling with separate ImportError/Exception blocks
- Added null check before calling analyzer.analyze() in simple.py

Type safety improvements:
- simple.py uses proper StatisticalAnalyzer/EmbeddingService types via TYPE_CHECKING
- All public API functions have proper return type annotations
- Sorted __all__ alphabetically per ruff RUF022

All 147 tests pass, linting and mypy clean.

src/orchestrators/__init__.py

@@ -17,7 +17,7 @@ Usage:
     orchestrator = create_orchestrator(mode="advanced", api_key="sk-...")
 
 Protocols:
-    from src.orchestrators import SearchHandlerProtocol, JudgeHandlerProtocol
+    from src.orchestrators import SearchHandlerProtocol, JudgeHandlerProtocol, OrchestratorProtocol
 
 Design Patterns Applied:
 - Factory Pattern: create_orchestrator() creates appropriate orchestrator
@@ -25,8 +25,17 @@ Design Patterns Applied:
 - Facade Pattern: This __init__.py provides a clean public API
 """
 
+from __future__ import annotations
+
+import warnings
+from typing import TYPE_CHECKING
+
 # Protocols (Interface Segregation Principle)
-from src.orchestrators.base import JudgeHandlerProtocol, SearchHandlerProtocol
+from src.orchestrators.base import (
+    JudgeHandlerProtocol,
+    OrchestratorProtocol,
+    SearchHandlerProtocol,
+)
 
 # Factory (creational pattern)
 from src.orchestrators.factory import create_orchestrator
@@ -34,35 +43,66 @@ from src.orchestrators.factory import create_orchestrator
 # Orchestrators (Strategy Pattern implementations)
 from src.orchestrators.simple import Orchestrator
 
+if TYPE_CHECKING:
+    from src.orchestrators.advanced import AdvancedOrchestrator
+    from src.orchestrators.hierarchical import HierarchicalOrchestrator
+
 # Lazy imports for optional dependencies
 # These are not imported at module level to avoid breaking simple mode
 # when agent-framework-core is not installed
 
 
-def get_advanced_orchestrator() -> type:
-    """Get the AdvancedOrchestrator class (requires agent-framework-core)."""
+def get_advanced_orchestrator() -> type[AdvancedOrchestrator]:
+    """Get the AdvancedOrchestrator class (requires agent-framework-core).
+
+    Returns:
+        The AdvancedOrchestrator class
+
+    Raises:
+        ImportError: If agent-framework-core is not installed
+    """
     from src.orchestrators.advanced import AdvancedOrchestrator
 
     return AdvancedOrchestrator
 
 
-def get_hierarchical_orchestrator() -> type:
-    """Get the HierarchicalOrchestrator class (requires agent-framework-core)."""
+def get_hierarchical_orchestrator() -> type[HierarchicalOrchestrator]:
+    """Get the HierarchicalOrchestrator class (requires agent-framework-core).
+
+    Returns:
+        The HierarchicalOrchestrator class
+
+    Raises:
+        ImportError: If agent-framework-core is not installed
+    """
     from src.orchestrators.hierarchical import HierarchicalOrchestrator
 
     return HierarchicalOrchestrator
 
 
-# Backwards compatibility aliases
-# TODO: Remove after migration period
-def get_magentic_orchestrator() -> type:
-    """Deprecated: Use get_advanced_orchestrator() instead."""
+def get_magentic_orchestrator() -> type[AdvancedOrchestrator]:
+    """Get the AdvancedOrchestrator class.
+
+    .. deprecated:: 0.1.0
+        Use :func:`get_advanced_orchestrator` instead.
+        The name 'magentic' was confusing with the 'magentic' PyPI package.
+
+    Returns:
+        The AdvancedOrchestrator class
+    """
+    warnings.warn(
+        "get_magentic_orchestrator() is deprecated, use get_advanced_orchestrator() instead. "
+        "The name 'magentic' was confusing with the 'magentic' PyPI package.",
+        DeprecationWarning,
+        stacklevel=2,
+    )
     return get_advanced_orchestrator()
 
 
 __all__ = [
     "JudgeHandlerProtocol",
     "Orchestrator",
+    "OrchestratorProtocol",
     "SearchHandlerProtocol",
     "create_orchestrator",
     "get_advanced_orchestrator",

src/orchestrators/advanced.py

@@ -279,6 +279,26 @@ The final output should be a structured research report."""
         # taking care to avoid infinite recursion if str() calls .text
         return str(message)
 
+    def _get_event_type_for_agent(self, agent_name: str) -> str:
+        """Map agent name to appropriate event type.
+
+        Args:
+            agent_name: The agent ID from the workflow event
+
+        Returns:
+            Event type string matching AgentEvent.type Literal
+        """
+        agent_lower = agent_name.lower()
+        if "search" in agent_lower:
+            return "search_complete"
+        if "judge" in agent_lower:
+            return "judge_complete"
+        if "hypothes" in agent_lower:
+            return "hypothesizing"
+        if "report" in agent_lower:
+            return "synthesizing"
+        return "judging"  # Default for unknown agents
+
     def _process_event(self, event: Any, iteration: int) -> AgentEvent | None:
         """Process workflow event into AgentEvent."""
         if isinstance(event, MagenticOrchestratorMessageEvent):
@@ -293,17 +313,9 @@ The final output should be a structured research report."""
         elif isinstance(event, MagenticAgentMessageEvent):
             agent_name = event.agent_id or "unknown"
             text = self._extract_text(event.message)
+            event_type = self._get_event_type_for_agent(agent_name)
 
-            event_type = "judging"
-            if "search" in agent_name.lower():
-                event_type = "search_complete"
-            elif "judge" in agent_name.lower():
-                event_type = "judge_complete"
-            elif "hypothes" in agent_name.lower():
-                event_type = "hypothesizing"
-            elif "report" in agent_name.lower():
-                event_type = "synthesizing"
-
+            # All returned types are valid AgentEvent.type literals
             return AgentEvent(
                 type=event_type,  # type: ignore[arg-type]
                 message=f"{agent_name}: {text[:200]}...",
@@ -339,6 +351,28 @@ The final output should be a structured research report."""
         return None
 
 
-# Backwards compatibility alias
-# TODO: Remove after all imports are updated
-MagenticOrchestrator = AdvancedOrchestrator
+def _create_deprecated_alias() -> type["AdvancedOrchestrator"]:
+    """Create a deprecated alias that warns on use."""
+    import warnings
+
+    class MagenticOrchestrator(AdvancedOrchestrator):
+        """Deprecated alias for AdvancedOrchestrator.
+
+        .. deprecated:: 0.1.0
+            Use :class:`AdvancedOrchestrator` instead.
+        """
+
+        def __init__(self, *args: Any, **kwargs: Any) -> None:
+            warnings.warn(
+                "MagenticOrchestrator is deprecated, use AdvancedOrchestrator instead. "
+                "The name 'magentic' was confusing with the 'magentic' PyPI package.",
+                DeprecationWarning,
+                stacklevel=2,
+            )
+            super().__init__(*args, **kwargs)
+
+    return MagenticOrchestrator
+
+
+# Backwards compatibility alias with deprecation warning
+MagenticOrchestrator = _create_deprecated_alias()

src/orchestrators/base.py

@@ -5,9 +5,10 @@ following the Interface Segregation Principle (ISP) and
 Dependency Inversion Principle (DIP).
 """
 
-from typing import Protocol
+from collections.abc import AsyncGenerator
+from typing import Protocol, runtime_checkable
 
-from src.utils.models import Evidence, JudgeAssessment, SearchResult
+from src.utils.models import AgentEvent, Evidence, JudgeAssessment, SearchResult
 
 
 class SearchHandlerProtocol(Protocol):
@@ -50,3 +51,23 @@ class JudgeHandlerProtocol(Protocol):
             JudgeAssessment with sufficiency determination and next steps
         """
         ...
+
+
+@runtime_checkable
+class OrchestratorProtocol(Protocol):
+    """Protocol for orchestrators.
+
+    All orchestrators (Simple, Advanced, Hierarchical) implement this interface,
+    allowing them to be used interchangeably by the factory and UI.
+    """
+
+    def run(self, query: str) -> AsyncGenerator[AgentEvent, None]:
+        """Run the orchestrator workflow.
+
+        Args:
+            query: User's research question
+
+        Yields:
+            AgentEvent objects for real-time UI updates
+        """
+        ...

src/orchestrators/factory.py

@@ -9,22 +9,35 @@ Design Principles:
 - Single Responsibility: Only handles orchestrator creation logic
 """
 
-from typing import Any, Literal
+from typing import TYPE_CHECKING, Literal
 
 import structlog
 
-from src.orchestrators.base import JudgeHandlerProtocol, SearchHandlerProtocol
+from src.orchestrators.base import (
+    JudgeHandlerProtocol,
+    OrchestratorProtocol,
+    SearchHandlerProtocol,
+)
 from src.orchestrators.simple import Orchestrator
 from src.utils.config import settings
 from src.utils.models import OrchestratorConfig
 
+if TYPE_CHECKING:
+    from src.orchestrators.advanced import AdvancedOrchestrator
+
 logger = structlog.get_logger()
 
 
-def _get_advanced_orchestrator_class() -> Any:
+def _get_advanced_orchestrator_class() -> type["AdvancedOrchestrator"]:
     """Import AdvancedOrchestrator lazily to avoid hard dependency.
 
     This allows the simple mode to work without agent-framework-core installed.
+
+    Returns:
+        The AdvancedOrchestrator class
+
+    Raises:
+        ValueError: If agent-framework-core is not installed
     """
     try:
         from src.orchestrators.advanced import AdvancedOrchestrator
@@ -33,7 +46,9 @@ def _get_advanced_orchestrator_class() -> Any:
     except ImportError as e:
         logger.error("Failed to import AdvancedOrchestrator", error=str(e))
         raise ValueError(
-            "Advanced mode requires agent-framework-core. Please install it or use mode='simple'."
+            "Advanced mode requires agent-framework-core. "
+            "Install with: pip install agent-framework-core. "
+            "Or use mode='simple' instead."
         ) from e
 
 
@@ -43,7 +58,7 @@ def create_orchestrator(
     config: OrchestratorConfig | None = None,
     mode: Literal["simple", "magentic", "advanced", "hierarchical"] | None = None,
     api_key: str | None = None,
-) -> Any:
+) -> OrchestratorProtocol:
     """
     Create an orchestrator instance.
 
@@ -54,32 +69,33 @@ def create_orchestrator(
     Args:
         search_handler: The search handler (required for simple mode)
         judge_handler: The judge handler (required for simple mode)
-        config: Optional configuration
+        config: Optional configuration (max_iterations, timeouts, etc.)
         mode: "simple", "magentic", "advanced", "hierarchical" or None (auto-detect)
               Note: "magentic" is an alias for "advanced" (kept for backwards compatibility)
         api_key: Optional API key for advanced mode (OpenAI)
 
     Returns:
-        Orchestrator instance
+        Orchestrator instance implementing OrchestratorProtocol
 
     Raises:
         ValueError: If required handlers are missing for simple mode
         ValueError: If advanced mode is requested but dependencies are missing
     """
+    effective_config = config or OrchestratorConfig()
     effective_mode = _determine_mode(mode, api_key)
     logger.info("Creating orchestrator", mode=effective_mode)
 
     if effective_mode == "advanced":
         orchestrator_cls = _get_advanced_orchestrator_class()
         return orchestrator_cls(
-            max_rounds=config.max_iterations if config else 10,
+            max_rounds=effective_config.max_iterations,
             api_key=api_key,
         )
 
     if effective_mode == "hierarchical":
         from src.orchestrators.hierarchical import HierarchicalOrchestrator
 
-        return HierarchicalOrchestrator()
+        return HierarchicalOrchestrator(config=effective_config)
 
     # Simple mode requires handlers
     if search_handler is None or judge_handler is None:
@@ -88,7 +104,7 @@ def create_orchestrator(
     return Orchestrator(
         search_handler=search_handler,
         judge_handler=judge_handler,
-        config=config,
+        config=effective_config,
     )
 
 

src/orchestrators/hierarchical.py

@@ -21,10 +21,13 @@ from src.agents.magentic_agents import create_search_agent
 from src.middleware.sub_iteration import SubIterationMiddleware, SubIterationTeam
 from src.services.embeddings import get_embedding_service
 from src.state import init_magentic_state
-from src.utils.models import AgentEvent
+from src.utils.models import AgentEvent, OrchestratorConfig
 
 logger = structlog.get_logger()
 
+# Default timeout for hierarchical orchestrator (5 minutes)
+DEFAULT_TIMEOUT_SECONDS = 300.0
+
 
 class ResearchTeam(SubIterationTeam):
     """Adapts ChatAgent to SubIterationTeam protocol.
@@ -60,13 +63,27 @@ class HierarchicalOrchestrator:
     - Sub-iteration middleware for fine-grained control
     - LLM-based judge for sub-iteration decisions
     - Event-driven architecture for UI updates
+    - Configurable iterations and timeout
     """
 
-    def __init__(self) -> None:
-        """Initialize the hierarchical orchestrator."""
+    def __init__(
+        self,
+        config: OrchestratorConfig | None = None,
+        timeout_seconds: float = DEFAULT_TIMEOUT_SECONDS,
+    ) -> None:
+        """Initialize the hierarchical orchestrator.
+
+        Args:
+            config: Optional configuration (uses defaults if not provided)
+            timeout_seconds: Maximum workflow duration (default: 5 minutes)
+        """
+        self.config = config or OrchestratorConfig()
+        self._timeout_seconds = timeout_seconds
         self.team = ResearchTeam()
         self.judge = LLMSubIterationJudge()
-        self.middleware = SubIterationMiddleware(self.team, self.judge, max_iterations=5)
+        self.middleware = SubIterationMiddleware(
+            self.team, self.judge, max_iterations=self.config.max_iterations
+        )
 
     async def run(self, query: str) -> AsyncGenerator[AgentEvent, None]:
         """Run the hierarchical workflow.
@@ -82,10 +99,14 @@ class HierarchicalOrchestrator:
         try:
             service = get_embedding_service()
             init_magentic_state(service)
+        except ImportError:
+            logger.info("Embedding service not available (dependencies missing)")
+            init_magentic_state()
         except Exception as e:
             logger.warning(
-                "Embedding service initialization failed, using default state",
+                "Embedding service initialization failed",
                 error=str(e),
+                error_type=type(e).__name__,
             )
             init_magentic_state()
 
@@ -96,38 +117,52 @@ class HierarchicalOrchestrator:
         async def event_callback(event: AgentEvent) -> None:
             await queue.put(event)
 
-        task_future = asyncio.create_task(self.middleware.run(query, event_callback))
-
-        while not task_future.done():
-            get_event = asyncio.create_task(queue.get())
-            done, _ = await asyncio.wait(
-                {task_future, get_event}, return_when=asyncio.FIRST_COMPLETED
-            )
-
-            if get_event in done:
-                event = get_event.result()
-                if event:
-                    yield event
-            else:
-                get_event.cancel()
-
-        # Process remaining events
-        while not queue.empty():
-            ev = queue.get_nowait()
-            if ev:
-                yield ev
-
         try:
-            result, assessment = await task_future
-
-            assessment_text = assessment.reasoning if assessment else "None"
+            async with asyncio.timeout(self._timeout_seconds):
+                task_future = asyncio.create_task(self.middleware.run(query, event_callback))
+
+                while not task_future.done():
+                    get_event = asyncio.create_task(queue.get())
+                    done, _ = await asyncio.wait(
+                        {task_future, get_event}, return_when=asyncio.FIRST_COMPLETED
+                    )
+
+                    if get_event in done:
+                        event = get_event.result()
+                        if event:
+                            yield event
+                    else:
+                        get_event.cancel()
+
+                # Process remaining events
+                while not queue.empty():
+                    ev = queue.get_nowait()
+                    if ev:
+                        yield ev
+
+                result, assessment = await task_future
+
+                assessment_text = assessment.reasoning if assessment else "None"
+                yield AgentEvent(
+                    type="complete",
+                    message=(
+                        f"Research complete.\n\nResult:\n{result}\n\nAssessment:\n{assessment_text}"
+                    ),
+                    data={"assessment": assessment.model_dump() if assessment else None},
+                )
+
+        except TimeoutError:
+            logger.warning("Hierarchical workflow timed out", query=query)
             yield AgentEvent(
                 type="complete",
-                message=(
-                    f"Research complete.\n\nResult:\n{result}\n\nAssessment:\n{assessment_text}"
-                ),
-                data={"assessment": assessment.model_dump() if assessment else None},
+                message="Research timed out. Results may be incomplete.",
+                data={"reason": "timeout"},
             )
+
         except Exception as e:
-            logger.error("Orchestrator failed", error=str(e))
+            logger.error(
+                "Orchestrator failed",
+                error=str(e),
+                error_type=type(e).__name__,
+            )
             yield AgentEvent(type="error", message=f"Orchestrator failed: {e}")

src/orchestrators/simple.py

@@ -8,9 +8,11 @@ Design Pattern: Template Method - defines the skeleton of the search-judge loop
 while allowing handlers to implement specific behaviors.
 """
 
+from __future__ import annotations
+
 import asyncio
 from collections.abc import AsyncGenerator
-from typing import Any
+from typing import TYPE_CHECKING, Any
 
 import structlog
 
@@ -24,6 +26,10 @@ from src.utils.models import (
     SearchResult,
 )
 
+if TYPE_CHECKING:
+    from src.services.embeddings import EmbeddingService
+    from src.services.statistical_analyzer import StatisticalAnalyzer
+
 logger = structlog.get_logger()
 
 
@@ -61,26 +67,36 @@ class Orchestrator:
         self._enable_analysis = enable_analysis and settings.modal_available
         self._enable_embeddings = enable_embeddings
 
-        # Lazy-load services
-        self._analyzer: Any = None
-        self._embeddings: Any = None
+        # Lazy-load services (typed for IDE support)
+        self._analyzer: StatisticalAnalyzer | None = None
+        self._embeddings: EmbeddingService | None = None
 
-    def _get_analyzer(self) -> Any:
+    def _get_analyzer(self) -> StatisticalAnalyzer | None:
         """Lazy initialization of StatisticalAnalyzer.
 
         Note: This imports from src.services, NOT src.agents,
         so it works without the magentic optional dependency.
+
+        Returns:
+            StatisticalAnalyzer instance, or None if Modal is unavailable
         """
         if self._analyzer is None:
-            from src.services.statistical_analyzer import get_statistical_analyzer
+            try:
+                from src.services.statistical_analyzer import get_statistical_analyzer
 
-            self._analyzer = get_statistical_analyzer()
+                self._analyzer = get_statistical_analyzer()
+            except ImportError:
+                logger.info("StatisticalAnalyzer not available (Modal dependencies missing)")
+                self._enable_analysis = False
         return self._analyzer
 
-    def _get_embeddings(self) -> Any:
+    def _get_embeddings(self) -> EmbeddingService | None:
         """Lazy initialization of EmbeddingService.
 
         Uses local sentence-transformers - NO API key required.
+
+        Returns:
+            EmbeddingService instance, or None if unavailable
         """
         if self._embeddings is None and self._enable_embeddings:
             try:
@@ -88,8 +104,15 @@ class Orchestrator:
 
                 self._embeddings = get_embedding_service()
                 logger.info("Embedding service enabled for semantic ranking")
+            except ImportError:
+                logger.info("Embedding service not available (dependencies missing)")
+                self._enable_embeddings = False
             except Exception as e:
-                logger.warning("Embeddings unavailable, using basic ranking", error=str(e))
+                logger.warning(
+                    "Embedding service initialization failed",
+                    error=str(e),
+                    error_type=type(e).__name__,
+                )
                 self._enable_embeddings = False
         return self._embeddings
 
@@ -128,6 +151,9 @@ class Orchestrator:
 
         try:
             analyzer = self._get_analyzer()
+            if analyzer is None:
+                logger.info("StatisticalAnalyzer not available, skipping analysis phase")
+                return
 
             # Run Modal analysis (no agent_framework needed!)
             analysis_result = await analyzer.analyze(