Refactor orphaned trade handling in SimplePositionTracker to improve clarity and efficiency. Simplify cancellation logic by directly checking for open orders and enhancing logging for orphaned trades. Update AggregationManager to streamline trade data migration and improve error handling for missing trade data.

src/monitoring/simple_position_tracker.py

@@ -422,70 +422,38 @@ class SimplePositionTracker:
             }
             
             for trade in pending_trades:
-                try:
-                    lifecycle_id = trade['trade_lifecycle_id']
-                    symbol = trade['symbol']
-                    entry_order_id = trade.get('entry_order_id')
-                    
-                    # Check if this trade should be cancelled
-                    should_cancel = False
-                    cancel_reason = ""
-                    
-                    # Case 1: Entry order ID exists but order is no longer on exchange
-                    if entry_order_id and entry_order_id not in exchange_order_ids:
-                        # Check if a position was opened (even if order disappeared)
-                        if symbol not in exchange_position_symbols:
-                            should_cancel = True
-                            cancel_reason = "entry_order_cancelled_no_position"
-                            logger.debug(f"🗑️ Pending trade {lifecycle_id[:8]} for {symbol}: entry order {entry_order_id} no longer exists and no position opened")
-                    
-                    # Case 2: No entry order ID but no position exists (shouldn't happen but safety check)
-                    elif not entry_order_id and symbol not in exchange_position_symbols:
-                        should_cancel = True
-                        cancel_reason = "no_entry_order_no_position"
-                        logger.debug(f"🗑️ Pending trade {lifecycle_id[:8]} for {symbol}: no entry order ID and no position")
-                    
-                    # Case 3: Check if trade is very old (safety net for other edge cases)
-                    else:
-                        from datetime import datetime, timezone, timedelta
-                        created_at_str = trade.get('timestamp')
-                        if created_at_str:
-                            try:
-                                created_at = datetime.fromisoformat(created_at_str.replace('Z', '+00:00'))
-                                if datetime.now(timezone.utc) - created_at > timedelta(hours=1):
-                                    # Very old pending trade, likely orphaned
-                                    # Only cancel if no position AND no open orders
-                                    if symbol not in exchange_position_symbols and symbol not in symbols_with_open_orders:
-                                        should_cancel = True
-                                        cancel_reason = "old_pending_trade_no_position"
-                                        logger.debug(f"🗑️ Pending trade {lifecycle_id[:8]} for {symbol}: very old ({created_at}) with no position and no open orders")
-                                    else:
-                                        logger.debug(f"⏳ Keeping old pending trade {lifecycle_id[:8]} for {symbol}: has position or open orders")
-                            except (ValueError, TypeError) as e:
-                                logger.warning(f"Could not parse timestamp for pending trade {lifecycle_id}: {e}")
-                    
-                    # Cancel the orphaned trade
-                    if should_cancel:
-                        success = stats.update_trade_cancelled(lifecycle_id, reason=cancel_reason)
-                        if success:
-                            logger.info(f"🗑️ Cancelled orphaned pending trade: {symbol} (Lifecycle: {lifecycle_id[:8]}) - {cancel_reason}")
-                            
-                            # Send a notification about the cancelled trade
-                            await self._send_trade_cancelled_notification(symbol, cancel_reason, trade)
-                            
-                            # Migrate cancelled trade to aggregated stats
-                            stats.migrate_trade_to_aggregated_stats(lifecycle_id)
-                        else:
-                            logger.error(f"❌ Failed to cancel orphaned pending trade: {lifecycle_id}")
-                            
-                except Exception as e:
-                    logger.error(f"❌ Error processing pending trade {trade.get('trade_lifecycle_id', 'unknown')}: {e}")
+                lifecycle_id = trade['trade_lifecycle_id']
+                entry_order_id = trade.get('entry_order_id')
+                symbol = trade['symbol']
+
+                # If the order is still open on the exchange, it's not orphaned.
+                if entry_order_id and entry_order_id in exchange_order_ids:
+                    logger.info(f"Trade {lifecycle_id} for {symbol} is pending but its order {entry_order_id} is still open. Skipping.")
+                    continue
+
+                # If no order is linked, or the order is not on the exchange,
+                # we assume it was cancelled or failed.
+                logger.warning(f"Orphaned pending trade detected for {symbol} (Lifecycle: {lifecycle_id}). Cancelling.")
+
+                # Mark the trade as cancelled (this is a sync function)
+                cancelled = stats.update_trade_cancelled(lifecycle_id, "Orphaned pending trade")
+                
+                if cancelled:
+                    # Migrate the cancelled trade to aggregated stats
+                    stats.migrate_trade_to_aggregated_stats(lifecycle_id)
                     
+                    # Send a notification
+                    await self._send_trade_cancelled_notification(
+                        symbol, "Orphaned, presumed cancelled before fill.", trade
+                    )
+                else:
+                    logger.error(f"Failed to cancel orphaned trade lifecycle {lifecycle_id}")
+
         except Exception as e:
-            logger.error(f"❌ Error handling orphaned pending trades: {e}")
+            logger.error(f"❌ Error handling orphaned pending trades: {e}", exc_info=True)
     
     async def _send_trade_cancelled_notification(self, symbol: str, cancel_reason: str, trade: Dict[str, Any]):
-        """Send notification for cancelled trade."""
+        """Send notification for a cancelled trade."""
         try:
             if not self.notification_manager:
                 return

src/stats/aggregation_manager.py

@@ -22,13 +22,17 @@ class AggregationManager:
         """Initialize with database manager."""
         self.db = db_manager
 
-    def migrate_trade_to_aggregated_stats(self, trade_lifecycle_id: str):
-        """Migrate a completed/cancelled trade's stats to aggregate tables and delete the original trade."""
-        trade_data = self.db._fetchone_query("SELECT * FROM trades WHERE trade_lifecycle_id = ?", (trade_lifecycle_id,))
+    def aggregate_trade_data(self, trade_data: Dict[str, Any]):
+        """Migrate a completed/cancelled trade's stats to aggregate tables."""
         if not trade_data:
-            logger.error(f"Cannot migrate trade {trade_lifecycle_id}: Not found.")
+            logger.error("Cannot migrate trade: No trade data provided.")
             return
 
+        trade_lifecycle_id = trade_data.get('trade_lifecycle_id')
+        if not trade_lifecycle_id:
+            logger.error(f"Cannot migrate trade: trade_lifecycle_id missing from trade data.")
+            return
+        
         status = trade_data.get('status')
         symbol = trade_data.get('symbol')
         token = symbol.split('/')[0] if symbol and '/' in symbol else symbol
@@ -45,9 +49,9 @@ class AggregationManager:
                 elif status == 'cancelled':
                     self._migrate_cancelled_position(trade_data, token, now_iso)
                 
-                # Delete the original trade from the 'trades' table
-                self.db._execute_query("DELETE FROM trades WHERE trade_lifecycle_id = ?", (trade_lifecycle_id,))
-                logger.info(f"Deleted trade lifecycle {trade_lifecycle_id} from trades table after aggregation.")
+                # Deletion is now handled by the caller in TradingStats if desired
+                # self.db._execute_query("DELETE FROM trades WHERE trade_lifecycle_id = ?", (trade_lifecycle_id,))
+                # logger.info(f"Deleted trade lifecycle {trade_lifecycle_id} from trades table after aggregation.")
 
         except sqlite3.Error as e:
             logger.error(f"Database error migrating trade {trade_lifecycle_id} to aggregate stats: {e}", exc_info=True)

trading_bot.py

@@ -14,7 +14,7 @@ from datetime import datetime
 from pathlib import Path
 
 # Bot version
-BOT_VERSION = "2.4.244"
+BOT_VERSION = "2.4.245"
 
 # Add src directory to Python path
 sys.path.insert(0, str(Path(__file__).parent / "src"))