Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Empty file added src/ordo/core/__init__.py
Empty file.
331 changes: 331 additions & 0 deletions src/ordo/core/orchestrator.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,331 @@
"""
Request Orchestrator - Central coordination layer for multi-broker operations.
"""
import asyncio
import time
from typing import List, Dict, Any, Callable, Type
from ordo.models.api.unified import BrokerResult, UnifiedResponse
from ordo.models.api.portfolio import Portfolio
from ordo.models.api.order import OrderResponse
from ordo.adapters.base import IBrokerAdapter
from ordo.security.session import SessionManager


class SessionCheckResult:
"""Result of session validation check."""

def __init__(self, status: str, message: str = ""):
self.status = status # "valid", "expired", "refresh_failed", "unsupported"
self.message = message

Comment on lines +14 to +20

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Remove duplicate/unreferenced SessionCheckResult

This local class isn’t used and likely duplicates the canonical type in ordo.security.session. Keep a single source of truth.

-class SessionCheckResult:
-    """Result of session validation check."""
-
-    def __init__(self, status: str, message: str = ""):
-        self.status = status  # "valid", "expired", "refresh_failed", "unsupported"
-        self.message = message
+# Session validation result type should be defined in ordo.security.session.
+# Import and use that canonical type there instead of redefining locally.

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In src/ordo/core/orchestrator.py around lines 14 to 20, remove the local
SessionCheckResult class (it's duplicate/unreferenced) and instead import and
use the canonical type from ordo.security.session; delete the class definition,
add the appropriate import (e.g., from ordo.security.session import
SessionCheckResult) at the top, and update any local references to use the
imported symbol.


class RequestOrchestrator:
"""
Central coordination layer for multi-broker operations.

Handles concurrent fan-out to multiple brokers, result collation,
error handling, and timeout management.

Attributes:
session_manager: SessionManager for session validation
adapter_registry: Dictionary mapping broker_id to adapter class
per_broker_timeout: Timeout in seconds for individual broker operations
global_timeout: Timeout in seconds for entire orchestration
"""

def __init__(
self,
session_manager: SessionManager,
adapter_registry: Dict[str, Type[IBrokerAdapter]],
per_broker_timeout: int = 8,
global_timeout: int = 12,
):
"""
Initialize RequestOrchestrator.

Args:
session_manager: SessionManager instance for session validation
adapter_registry: Dict mapping broker_id to adapter class
per_broker_timeout: Timeout for individual broker operations (default: 8s)
global_timeout: Timeout for entire orchestration (default: 12s)
"""
self.session_manager = session_manager
self.adapter_registry = adapter_registry
self.per_broker_timeout = per_broker_timeout
self.global_timeout = global_timeout

Comment on lines +36 to +56

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Implement and enforce global_timeout; cancel pending tasks and map to TIMEOUT

global_timeout is defined but never used. Wrap orchestration in wait_for, cancel pending tasks on timeout, and convert global timeouts to BrokerResult with code TIMEOUT.

Apply these diffs.

Get portfolio: create tasks and enforce global timeout

@@
-        # Create tasks for concurrent execution
-        tasks = []
-        for broker_id in broker_ids:
+        # Create tasks for concurrent execution
+        tasks_by_broker = {}
+        for broker_id in broker_ids:
             # Create operation function for this broker
             async def operation(bid=broker_id):
                 adapter_class = self.adapter_registry[bid]
                 adapter = adapter_class()
                 session_data = context.get("session_data", {}).get(bid, {})
                 return await adapter.get_portfolio(session_data)
 
-            # Add to tasks list
-            task = self._execute_for_broker(broker_id, operation, context)
-            tasks.append(task)
+            # Add to tasks map
+            tasks_by_broker[broker_id] = asyncio.create_task(
+                self._execute_for_broker(broker_id, operation, context)
+            )
@@
-        # Execute all tasks concurrently using asyncio.gather
-        # return_exceptions=True ensures one failure doesn't stop others
-        results = await asyncio.gather(*tasks, return_exceptions=True)
+        # Execute all tasks concurrently with a global timeout
+        try:
+            results = await asyncio.wait_for(
+                asyncio.gather(
+                    *tasks_by_broker.values(), return_exceptions=True
+                ),
+                timeout=self.global_timeout,
+            )
+        except asyncio.TimeoutError:
+            # Cancel pending tasks and synthesize timeout results
+            for t in tasks_by_broker.values():
+                if not t.done():
+                    t.cancel()
+            results = [
+                (t.result() if t.done() else asyncio.TimeoutError())
+                for t in tasks_by_broker.values()
+            ]
@@
-        for i, result in enumerate(results):
+        for i, result in enumerate(results):
             if isinstance(result, BrokerResult):
                 broker_results.append(result)
             elif isinstance(result, Exception):
-                # Handle exceptions that weren't caught by _execute_for_broker
-                broker_results.append(
-                    BrokerResult(
-                        broker_id=broker_ids[i],
-                        status="failed",
-                        code="INTERNAL_ERROR",
-                        message=str(result),
-                        payload=None,
-                        latency_ms=0,
-                    )
-                )
+                # Map global timeout separately
+                if isinstance(result, asyncio.TimeoutError):
+                    broker_results.append(
+                        BrokerResult(
+                            broker_id=broker_ids[i],
+                            status="failed",
+                            code="TIMEOUT",
+                            message=f"Global timeout after {self.global_timeout}s",
+                            payload=None,
+                            latency_ms=0,
+                        )
+                    )
+                else:
+                    broker_results.append(
+                        BrokerResult(
+                            broker_id=broker_ids[i],
+                            status="failed",
+                            code="INTERNAL_ERROR",
+                            message=str(result),
+                            payload=None,
+                            latency_ms=0,
+                        )
+                    )

Place order: create tasks and enforce global timeout

@@
-        # Create tasks for concurrent execution
-        tasks = []
-        for broker_id in broker_ids:
+        # Create tasks for concurrent execution
+        tasks_by_broker = {}
+        for broker_id in broker_ids:
             # Create operation function for this broker
             async def operation(bid=broker_id):
                 adapter_class = self.adapter_registry[bid]
                 adapter = adapter_class()
                 session_data = context.get("session_data", {}).get(bid, {})
-                # Note: Actual order placement method needs to be added to IBrokerAdapter
-                # For now, we'll use a placeholder
-                return OrderResponse(order_id=f"{bid}_order_123", status="pending")
+                return await adapter.place_order(order_data, session_data)  # async
@@
-            # Add to tasks list
-            task = self._execute_for_broker(broker_id, operation, context)
-            tasks.append(task)
+            # Add to tasks map
+            tasks_by_broker[broker_id] = asyncio.create_task(
+                self._execute_for_broker(broker_id, operation, context)
+            )
@@
-        # Execute all tasks concurrently
-        results = await asyncio.gather(*tasks, return_exceptions=True)
+        # Execute all tasks concurrently with a global timeout
+        try:
+            results = await asyncio.wait_for(
+                asyncio.gather(
+                    *tasks_by_broker.values(), return_exceptions=True
+                ),
+                timeout=self.global_timeout,
+            )
+        except asyncio.TimeoutError:
+            for t in tasks_by_broker.values():
+                if not t.done():
+                    t.cancel()
+            results = [
+                (t.result() if t.done() else asyncio.TimeoutError())
+                for t in tasks_by_broker.values()
+            ]
@@
-        for i, result in enumerate(results):
+        for i, result in enumerate(results):
             if isinstance(result, BrokerResult):
                 broker_results.append(result)
             elif isinstance(result, Exception):
-                broker_results.append(
-                    BrokerResult(
-                        broker_id=broker_ids[i],
-                        status="failed",
-                        code="INTERNAL_ERROR",
-                        message=str(result),
-                        payload=None,
-                        latency_ms=0,
-                    )
-                )
+                if isinstance(result, asyncio.TimeoutError):
+                    broker_results.append(
+                        BrokerResult(
+                            broker_id=broker_ids[i],
+                            status="failed",
+                            code="TIMEOUT",
+                            message=f"Global timeout after {self.global_timeout}s",
+                            payload=None,
+                            latency_ms=0,
+                        )
+                    )
+                else:
+                    broker_results.append(
+                        BrokerResult(
+                            broker_id=broker_ids[i],
+                            status="failed",
+                            code="INTERNAL_ERROR",
+                            message=str(result),
+                            payload=None,
+                            latency_ms=0,
+                        )
+                    )

Also applies to: 195-208, 209-213, 213-229, 272-287, 288-290, 292-306

🤖 Prompt for AI Agents
In src/ordo/core/orchestrator.py around lines 36-56 (and similarly for ranges
195-208, 209-213, 213-229, 272-287, 288-290, 292-306), global_timeout is defined
but unused; wrap the orchestration logic that gathers per-broker tasks in an
asyncio.wait_for using self.global_timeout, catch asyncio.TimeoutError, cancel
any still-pending tasks (call task.cancel() and await them safely), and for each
broker whose task was cancelled or whose wait_for timed out, produce a
BrokerResult with code TIMEOUT and an appropriate error message; ensure
successful and failed tasks still return/propagate their normal BrokerResult
values and that cancelled tasks are mapped into the results list before
returning from the orchestration methods (get_portfolio/place_order).

def _compute_overall_status(self, results: List[BrokerResult]) -> str:
"""
Compute overall status from individual broker results.

Logic:
- All success → "success"
- All failed → "failure"
- Mix of success and failed → "partial_success"

Args:
results: List of BrokerResult objects

Returns:
Overall status: "success", "partial_success", or "failure"
"""
if not results:
return "failure"

success_count = sum(1 for r in results if r.status == "success")
failed_count = sum(1 for r in results if r.status == "failed")

if success_count == len(results):
return "success"
elif failed_count == len(results):
return "failure"
else:
return "partial_success"

async def _execute_for_broker(
self,
broker_id: str,
operation: Callable,
context: Dict[str, Any],
) -> BrokerResult:
"""
Execute operation for a single broker with timeout and error handling.

Args:
broker_id: Broker identifier
operation: Async callable to execute
context: Request context with session data

Returns:
BrokerResult with operation outcome
"""
start_time = time.time()

try:
# Check if broker_id is valid
if broker_id not in self.adapter_registry:
return BrokerResult(
broker_id=broker_id,
status="failed",
code="INVALID_BROKER",
message=f"Unknown broker: {broker_id}",
payload=None,
latency_ms=int((time.time() - start_time) * 1000),
)

# Validate session before making call
session_result = await self.session_manager.ensure_valid_session(
broker_id, context.get("account_id", "")
)

if session_result.status != "valid":
return BrokerResult(
broker_id=broker_id,
status="failed",
code="INVALID_SESSION",
message=f"Session validation failed: {session_result.status}",
payload={"session_status": session_result.status},
latency_ms=int((time.time() - start_time) * 1000),
)

# Execute operation with timeout
result = await asyncio.wait_for(
operation(), timeout=self.per_broker_timeout
)

latency_ms = int((time.time() - start_time) * 1000)

return BrokerResult(
broker_id=broker_id,
status="success",
code="SUCCESS",
message="Operation completed successfully",
payload=result.model_dump() if hasattr(result, "model_dump") else result,
latency_ms=latency_ms,
)

except asyncio.TimeoutError:
return BrokerResult(
broker_id=broker_id,
status="failed",
code="TIMEOUT",
message=f"Operation timed out after {self.per_broker_timeout}s",
payload=None,
latency_ms=int((time.time() - start_time) * 1000),
)

except Exception as e:
# Handle any unexpected exceptions
error_type = type(e).__name__

# Map known exceptions to standardized codes
if "SessionExpired" in error_type:
code = "SESSION_EXPIRED"
elif "InsufficientFunds" in error_type:
code = "INSUFFICIENT_FUNDS"
elif "NetworkError" in error_type or "ConnectionError" in error_type:
code = "NETWORK_ERROR"
else:
code = "INTERNAL_ERROR"

return BrokerResult(
broker_id=broker_id,
status="failed",
code=code,
message=str(e),
payload={"error_type": error_type},
latency_ms=int((time.time() - start_time) * 1000),
)

async def get_portfolio(
self, broker_ids: List[str], context: Dict[str, Any]
) -> UnifiedResponse:
"""
Retrieve portfolio data from multiple brokers concurrently.

Args:
broker_ids: List of target broker identifiers
context: Request context with authentication and session data

Returns:
UnifiedResponse with aggregated portfolio results
"""
start_time = time.time()

# Create tasks for concurrent execution
tasks = []
for broker_id in broker_ids:
# Create operation function for this broker
async def operation(bid=broker_id):
adapter_class = self.adapter_registry[bid]
adapter = adapter_class()
session_data = context.get("session_data", {}).get(bid, {})
return await adapter.get_portfolio(session_data)

# Add to tasks list
task = self._execute_for_broker(broker_id, operation, context)
tasks.append(task)

# Execute all tasks concurrently using asyncio.gather
# return_exceptions=True ensures one failure doesn't stop others
results = await asyncio.gather(*tasks, return_exceptions=True)

# Convert any exceptions to BrokerResult
broker_results = []
for i, result in enumerate(results):
if isinstance(result, BrokerResult):
broker_results.append(result)
elif isinstance(result, Exception):
# Handle exceptions that weren't caught by _execute_for_broker
broker_results.append(
BrokerResult(
broker_id=broker_ids[i],
status="failed",
code="INTERNAL_ERROR",
message=str(result),
payload=None,
latency_ms=0,
)
)

# Compute overall status
overall_status = self._compute_overall_status(broker_results)

# Calculate total elapsed time
elapsed_ms = int((time.time() - start_time) * 1000)

# Collect errors if any
errors = []
for result in broker_results:
if result.status == "failed":
errors.append(
{
"broker_id": result.broker_id,
"code": result.code,
"message": result.message,
}
)

return UnifiedResponse(
overall_status=overall_status,
results=broker_results,
elapsed_ms=elapsed_ms,
errors=errors if errors else None,
)
Comment on lines +231 to +254

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Reduce duplication in result collation

The error aggregation and UnifiedResponse construction are duplicated across both methods. Extract a small helper to compute errors and build UnifiedResponse.

Also applies to: 314-331

🤖 Prompt for AI Agents
In src/ordo/core/orchestrator.py around lines 231-254 and 314-331, the logic
that collates broker_results into errors, computes elapsed_ms and constructs the
UnifiedResponse is duplicated; extract a small helper method (private on the
orchestrator class or a module-level function) that accepts broker_results,
overall_status and start_time (or elapsed_ms) and returns the UnifiedResponse:
inside the helper compute elapsed_ms, build the errors list by iterating
broker_results and collecting dicts for results with status == "failed" (set
errors to None if empty), and construct/return UnifiedResponse with
overall_status, results, elapsed_ms and errors; then replace the duplicated
blocks at both locations with a single call to this helper, ensuring
imports/typing are adjusted as needed.


async def place_order(
self, order_data: Dict[str, Any], broker_ids: List[str], context: Dict[str, Any]
) -> UnifiedResponse:
"""
Place order across multiple brokers concurrently.

Args:
order_data: Order details to be placed
broker_ids: List of target broker identifiers
context: Request context with authentication and session data

Returns:
UnifiedResponse with aggregated order placement results
"""
start_time = time.time()

# Create tasks for concurrent execution
tasks = []
for broker_id in broker_ids:
# Create operation function for this broker
async def operation(bid=broker_id):
adapter_class = self.adapter_registry[bid]
adapter = adapter_class()
session_data = context.get("session_data", {}).get(bid, {})
# Note: Actual order placement method needs to be added to IBrokerAdapter
# For now, we'll use a placeholder
return OrderResponse(order_id=f"{bid}_order_123", status="pending")

Comment on lines +276 to +283

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Order placement is a placeholder; not calling adapters

This silently returns a fake order response and never hits broker APIs.

Apply this diff to call the adapter (and fail fast if unsupported):

-            async def operation(bid=broker_id):
+            async def operation(bid=broker_id):
                 adapter_class = self.adapter_registry[bid]
                 adapter = adapter_class()
                 session_data = context.get("session_data", {}).get(bid, {})
-                # Note: Actual order placement method needs to be added to IBrokerAdapter
-                # For now, we'll use a placeholder
-                return OrderResponse(order_id=f"{bid}_order_123", status="pending")
+                if not hasattr(adapter, "place_order"):
+                    raise NotImplementedError(f"Adapter {bid} does not support order placement")
+                return await adapter.place_order(order_data, session_data)

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In src/ordo/core/orchestrator.py around lines 276 to 283, the async operation
currently returns a hard-coded OrderResponse and never calls the broker adapter;
replace the placeholder with a real adapter invocation: instantiate the adapter,
retrieve session_data and order details from context, check that the adapter
implements an async place_order (or place_order) method and if not raise
NotImplementedError, then call and await adapter.place_order(session_data,
order_payload) (or await adapter.place_order_async(...)) and return its
OrderResponse; ensure any adapter exceptions are not swallowed (let them
propagate or re-raise) so failures fail fast.

# Add to tasks list
task = self._execute_for_broker(broker_id, operation, context)
tasks.append(task)

# Execute all tasks concurrently
results = await asyncio.gather(*tasks, return_exceptions=True)

# Convert any exceptions to BrokerResult
broker_results = []
for i, result in enumerate(results):
if isinstance(result, BrokerResult):
broker_results.append(result)
elif isinstance(result, Exception):
broker_results.append(
BrokerResult(
broker_id=broker_ids[i],
status="failed",
code="INTERNAL_ERROR",
message=str(result),
payload=None,
latency_ms=0,
)
)

# Compute overall status
overall_status = self._compute_overall_status(broker_results)

# Calculate total elapsed time
elapsed_ms = int((time.time() - start_time) * 1000)

# Collect errors if any
errors = []
for result in broker_results:
if result.status == "failed":
errors.append(
{
"broker_id": result.broker_id,
"code": result.code,
"message": result.message,
}
)

return UnifiedResponse(
overall_status=overall_status,
results=broker_results,
elapsed_ms=elapsed_ms,
errors=errors if errors else None,
)
Loading