Refactor MarketMonitor to enhance external trade handling and orphaned position management. Improved logic for linking bot orders to trades, recording fills, and determining action types based on trade context. Updated entry price estimation for orphaned positions with enhanced side detection and fallback mechanisms. Enhanced logging for better traceability of trade processing and notifications.

src/monitoring/market_monitor.py

@@ -625,102 +625,90 @@ class MarketMonitor:
                         
                         else:
                             # Handle as regular external trade
-                            # Check if this corresponds to a bot order by exchange_order_id
-                            linked_order_db_id = None
+                            
+                            # Part 1: Link to bot order if applicable. This logic updates the bot's own order records.
+                            # It's independent of whether the fill results in a new entry in the 'trades' table here.
+                            linked_order_db_id_for_trade_record = None # Will be passed to stats.record_trade if fill is new
                             if exchange_order_id_from_fill:
                                 order_in_db = stats.get_order_by_exchange_id(exchange_order_id_from_fill)
                                 if order_in_db:
-                                    linked_order_db_id = order_in_db.get('id')
-                                    logger.info(f"🔗 Linked external fill {trade_id} to bot order DB ID {linked_order_db_id} (Exchange OID: {exchange_order_id_from_fill})")
+                                    linked_order_db_id_for_trade_record = order_in_db.get('id')
+                                    logger.info(f"🔗 External fill {trade_id} corresponds to bot order DB ID {linked_order_db_id_for_trade_record} (Exchange OID: {exchange_order_id_from_fill}).")
                                     
                                     # Update order status to filled if it was open
                                     current_status = order_in_db.get('status', '')
                                     if current_status in ['open', 'partially_filled', 'pending_submission']:
-                                        # Determine if this is a partial or full fill
                                         order_amount_requested = float(order_in_db.get('amount_requested', 0))
-                                        if abs(amount - order_amount_requested) < 0.000001:  # Allow small floating point differences
+                                        # This needs to be robust for partial fills accumulating
+                                        # For simplicity, assume 'amount' is the current fill amount.
+                                        # A more complete solution would check order_in_db.get('filled_amount', 0) + amount vs order_amount_requested
+                                        if abs(amount - order_amount_requested) < 0.000001:  # Approximation for full fill
                                             new_status_after_fill = 'filled'
                                         else:
-                                            new_status_after_fill = 'partially_filled'
+                                            # This could be refined by checking cumulative fills against requested amount
+                                            new_status_after_fill = 'partially_filled' 
                                         
                                         stats.update_order_status(
-                                            order_db_id=linked_order_db_id, 
+                                            order_db_id=linked_order_db_id_for_trade_record, 
                                             new_status=new_status_after_fill
                                         )
-                                        logger.info(f"📊 Updated bot order {linked_order_db_id} status: {current_status} → {new_status_after_fill}")
+                                        logger.info(f"📊 Updated bot order {linked_order_db_id_for_trade_record} status: {current_status} → {new_status_after_fill}")
                                         
-                                        # Check if this order is now fully filled and has pending stop losses to activate
                                         if new_status_after_fill == 'filled':
                                             await self._activate_pending_stop_losses(order_in_db, stats)
-                                
-                                # 🧹 PHASE 3: Record trade simply, use active_trades for tracking
+                            
+                            # Part 2: Record this fill in the Trades table IF it's new.
+                            # Assumes stats.get_trade_by_exchange_fill_id returns a truthy value if exists, None/falsy otherwise.
+                            if hasattr(stats, 'get_trade_by_exchange_fill_id') and stats.get_trade_by_exchange_fill_id(trade_id):
+                                logger.warning(f"Trade record for external fill {trade_id} (Bot Order OID: {exchange_order_id_from_fill or 'N/A'}) already exists in DB. Skipping re-recording and its specific processing.")
+                            else:
+                                # This fill is new to the trades table. Record it.
                                 stats.record_trade(
                                     full_symbol, side, amount, price, 
                                     exchange_fill_id=trade_id, trade_type="external",
                                     timestamp=timestamp_dt.isoformat(),
-                                    linked_order_table_id_to_link=linked_order_db_id
+                                    linked_order_table_id_to_link=linked_order_db_id_for_trade_record 
                                 )
                                 
-                                # Derive action type from trade context for notifications
-                                if linked_order_db_id:
-                                    # Bot order - determine action from order context
-                                    order_side = order_in_db.get('side', side).lower()
-                                    if order_side == 'buy':
-                                        action_type = 'long_opened'
-                                    elif order_side == 'sell':
-                                        action_type = 'short_opened'
-                                    else:
-                                        action_type = 'position_opened'
+                                # Derive action type from trade context for notifications (Only if newly recorded)
+                                current_action_type = "unknown_action" # Default
+                                if linked_order_db_id_for_trade_record:
+                                    # If linked to a bot order, the action is relative to that order
+                                    order_data_for_action = stats.get_order_by_db_id(linked_order_db_id_for_trade_record) # Potentially re-fetch
+                                    order_side_for_action = order_data_for_action.get('side', side).lower() if order_data_for_action else side.lower()
+                                    
+                                    # Simplified action type; more complex logic might be needed for "modified" vs "opened"
+                                    if order_side_for_action == 'buy': current_action_type = 'long_entry_fill' 
+                                    elif order_side_for_action == 'sell': current_action_type = 'short_entry_fill'
+                                    else: current_action_type = 'order_fill'
+                                    logger.debug(f"📊 Bot order (DB ID {linked_order_db_id_for_trade_record}) associated with new trade record for fill {trade_id}.")
+
                                 else:
-                                    # External trade - determine from current active trades
-                                    existing_active_trade = stats.get_active_trade_by_symbol(full_symbol, status='active')
-                                    if existing_active_trade:
-                                        # Has active position - this is likely a closure
-                                        existing_side = existing_active_trade.get('side')
-                                        if existing_side == 'buy' and side.lower() == 'sell':
-                                            action_type = 'long_closed'
-                                        elif existing_side == 'sell' and side.lower() == 'buy':
-                                            action_type = 'short_closed'
-                                        else:
-                                            action_type = 'position_modified'
+                                    # Not linked to a specific bot order, truly external or untracked by an open bot order
+                                    existing_active_trade_lifecycle = stats.get_active_trade_by_symbol(full_symbol, status='active') # Check trade lifecycle
+                                    if existing_active_trade_lifecycle:
+                                        existing_trade_side = existing_active_trade_lifecycle.get('side') # 'buy' for long, 'sell' for short
+                                        if existing_trade_side == 'buy' and side.lower() == 'sell': current_action_type = 'external_long_close'
+                                        elif existing_trade_side == 'sell' and side.lower() == 'buy': current_action_type = 'external_short_close'
+                                        else: current_action_type = 'external_position_modify' # e.g. adding to existing
                                     else:
-                                        # No active position - this opens a new one
-                                        if side.lower() == 'buy':
-                                            action_type = 'long_opened'
-                                        else:
-                                            action_type = 'short_opened'
-                                
-                                # 🧹 PHASE 3: Update active trades based on action
-                                if linked_order_db_id:
-                                    # Bot order - update linked active trade
-                                    order_data = stats.get_order_by_db_id(linked_order_db_id)
-                                    if order_data:
-                                        exchange_order_id = order_data.get('exchange_order_id')
-                                        logger.debug(f"📊 Bot order fill detected for exchange order {exchange_order_id} (handled by trade lifecycle system)")
-                            
-                            elif action_type in ['long_opened', 'short_opened']:
-                                # External trade that opened a position - handled by auto-sync in positions command
-                                logger.debug(f"📊 External position open detected: {side.upper()} {full_symbol} @ ${price:.2f} (handled by auto-sync)")
-                            
-                            elif action_type in ['long_closed', 'short_closed']:
-                                # External closure - handled by auto-sync in positions command  
-                                logger.debug(f"📊 External position close detected: {side.upper()} {full_symbol} @ ${price:.2f} (handled by auto-sync)")
+                                        if side.lower() == 'buy': current_action_type = 'external_long_open'
+                                        else: current_action_type = 'external_short_open'
                                 
-                                # Track symbol for potential stop loss activation
-                                symbols_with_fills.add(token)
-                                
-                                # Send notification for external trade
-                                if self.notification_manager:
-                                    await self.notification_manager.send_external_trade_notification(
-                                        full_symbol, side, amount, price, action_type, timestamp_dt.isoformat()
-                                    )
+                                # Send notification for external trade if it was a closure and not linked to a bot order fill
+                                if current_action_type in ['external_long_close', 'external_short_close']:
+                                    symbols_with_fills.add(token) # For safety net SL activation
+                                    if self.notification_manager:
+                                        await self.notification_manager.send_external_trade_notification(
+                                            full_symbol, side, amount, price, current_action_type, timestamp_dt.isoformat()
+                                        )
                                 
-                                logger.info(f"📋 Processed external trade: {side} {amount} {full_symbol} @ ${price:.2f} ({action_type}) using timestamp {timestamp_dt.isoformat()}")
+                                logger.info(f"📋 Recorded new trade from external fill {trade_id}: {side.upper()} {amount} {full_symbol} @ ${price:.2f} (Action: {current_action_type}, Linked Bot Order DB ID: {linked_order_db_id_for_trade_record or 'N/A'})")
                         
+                        # Update last processed time and counter if no exception occurred for this fill
                         external_trades_processed += 1
-                        
-                        # Update last processed time
-                        self._last_processed_trade_time = timestamp_dt
+                        if self._last_processed_trade_time is None or timestamp_dt > self._last_processed_trade_time:
+                           self._last_processed_trade_time = timestamp_dt
                         
                 except Exception as e:
                     logger.error(f"Error processing fill {fill}: {e}")
@@ -998,12 +986,24 @@ class MarketMonitor:
                     parent_order = stats.get_order_by_bot_ref_id(parent_bot_ref_id)
                     if parent_order:
                         parent_status = parent_order.get('status', '').lower()
+                        parent_order_id = parent_order.get('id')
                         
                         # Cancel if parent order failed, was cancelled, or disappeared
-                        if parent_status in ['failed_submission', 'failed_submission_no_data', 'cancelled_manually', 
-                                           'cancelled_externally', 'disappeared_from_exchange']:
+                        if parent_status in ['failed_submission', 'failed_submission_no_data', 'cancelled_manually', 'disappeared_from_exchange']:
                             should_cancel = True
                             cancel_reason = f"parent order {parent_status}"
+                        elif parent_status == 'cancelled_externally':
+                            # If parent was 'cancelled_externally', check if it was actually filled
+                            # This requires a method in TradingStats like 'is_order_considered_filled'
+                            # that checks for fills associated with this parent_order_id.
+                            # For now, we assume such a method exists or can be added.
+                            if hasattr(stats, 'is_order_considered_filled') and await stats.is_order_considered_filled(parent_order_id):
+                                logger.info(f"Parent order {parent_order_id} ({parent_bot_ref_id}) was 'cancelled_externally' but found to be filled. SL {order_db_id} will not be orphaned.")
+                                should_cancel = False # Do not cancel if filled
+                            else:
+                                logger.info(f"Parent order {parent_order_id} ({parent_bot_ref_id}) was 'cancelled_externally' and not found to be filled. SL {order_db_id} will be orphaned.")
+                                should_cancel = True
+                                cancel_reason = f"parent order {parent_status} and not filled"
                         elif parent_status == 'filled':
                             # Parent order filled but no position - position might have been closed externally
                             if symbol not in position_symbols:
@@ -1024,7 +1024,7 @@ class MarketMonitor:
                     # Cancel this orphaned stop loss
                     success = stats.update_order_status(
                         order_db_id=order_db_id,
-                        new_status='cancelled_orphaned_no_position'
+                        new_status='cancelled_orphaned' # Generic orphaned status
                     )
                     
                     if success:
@@ -1544,26 +1544,89 @@ class MarketMonitor:
 
             for exchange_pos in exchange_positions:
                 symbol = exchange_pos.get('symbol')
-                contracts = float(exchange_pos.get('contracts', 0))
+                # Get contracts (size), ensure it's absolute for initial check
+                contracts_abs = abs(float(exchange_pos.get('contracts', 0))) 
                 
-                if symbol and abs(contracts) > 0:
+                if symbol and contracts_abs > 0:
                     # Check if we have a trade lifecycle record for this position
                     existing_trade = stats.get_trade_by_symbol_and_status(symbol, 'position_opened')
                     
                     if not existing_trade:
                         # 🚨 ORPHANED POSITION: Auto-create trade lifecycle record
-                        entry_price = float(exchange_pos.get('entryPrice', 0))
-                        position_side = 'long' if contracts > 0 else 'short'
-                        order_side = 'buy' if contracts > 0 else 'sell'
+                        entry_price = float(exchange_pos.get('entryPrice', 0)) # CCXT usually has entryPrice
+                        if not entry_price or entry_price <= 0: # Fallback for safety
+                            entry_price = float(exchange_pos.get('entryPx', 0))
+
+
+                        position_side = ''
+                        order_side = ''
+
+                        # Determine side using CCXT 'side' field first
+                        ccxt_side = exchange_pos.get('side', '').lower()
+                        if ccxt_side == 'long':
+                            position_side = 'long'
+                            order_side = 'buy'
+                        elif ccxt_side == 'short':
+                            position_side = 'short'
+                            order_side = 'sell'
+                        
+                        # If CCXT 'side' is not present or ambiguous, try raw exchange data (e.g., Hyperliquid 'szi')
+                        if not position_side:
+                            raw_info = exchange_pos.get('info', {})
+                            if isinstance(raw_info, dict): # Ensure raw_info is a dict
+                                # Hyperliquid specific: position object within info
+                                raw_pos_data = raw_info.get('position', {}) 
+                                if isinstance(raw_pos_data, dict):
+                                    szi_str = raw_pos_data.get('szi') # sZi from Hyperliquid indicates signed size
+                                    if szi_str is not None:
+                                        try:
+                                            szi_val = float(szi_str)
+                                            if szi_val > 0:
+                                                position_side = 'long'
+                                                order_side = 'buy'
+                                            elif szi_val < 0:
+                                                position_side = 'short'
+                                                order_side = 'sell'
+                                            # If szi_val is 0, it's not an open position, abs(contracts) > 0 check should prevent this
+                                        except ValueError:
+                                            logger.warning(f"AUTO-SYNC: Could not parse 'szi' value '{szi_str}' for {symbol}")
+
+                        # Final fallback if side is still undetermined (less ideal)
+                        if not position_side:
+                            # This uses the 'contracts' field which might be ambiguous (always positive)
+                            # Only use if 'side' and 'szi' failed.
+                            contracts_from_exchange = float(exchange_pos.get('contracts', 0)) # Re-fetch, might be signed from some exchanges
+                            if contracts_from_exchange > 0: # Assuming positive for long, negative for short if not specified otherwise
+                                position_side = 'long'
+                                order_side = 'buy'
+                            elif contracts_from_exchange < 0:
+                                position_side = 'short'
+                                order_side = 'sell'
+                            else: # contracts is 0
+                                logger.warning(f"AUTO-SYNC: Position size is 0 for {symbol} after side checks, skipping sync.")
+                                continue
+                        
+                        if not position_side:
+                            logger.error(f"AUTO-SYNC: CRITICAL - Could not determine position side for {symbol}. Data: {exchange_pos}. Skipping sync.")
+                            continue
+
                         token = symbol.split('/')[0] if '/' in symbol else symbol
                         
-                        # ✅ Use exchange data - no need to estimate!
+                        # Ensure actual_contracts_size reflects the true size from exchange_pos.get('contracts')
+                        actual_contracts_size = abs(float(exchange_pos.get('contracts', 0)))
+
+                        logger_message_detail = f"{symbol} {position_side.upper()} {actual_contracts_size}"
                         if entry_price > 0:
-                            logger.info(f"🔄 AUTO-SYNC: Orphaned position detected - {symbol} {position_side} {abs(contracts)} @ ${entry_price} (exchange data)")
+                            logger.info(f"🔄 AUTO-SYNC: Orphaned position detected - {logger_message_detail} @ ${entry_price:.4f} (exchange data)")
                         else:
                             # Fallback only if exchange truly doesn't provide entry price
-                            entry_price = await self._estimate_entry_price_for_orphaned_position(symbol, contracts)
-                            logger.warning(f"🔄 AUTO-SYNC: Orphaned position detected - {symbol} {position_side} {abs(contracts)} @ ${entry_price} (estimated)")
+                            estimated_entry_price = await self._estimate_entry_price_for_orphaned_position(symbol, actual_contracts_size, position_side)
+                            if estimated_entry_price > 0:
+                                entry_price = estimated_entry_price
+                                logger.warning(f"🔄 AUTO-SYNC: Orphaned position detected - {logger_message_detail} @ ${entry_price:.4f} (estimated)")
+                            else:
+                                logger.error(f"AUTO-SYNC: Could not determine entry price for {logger_message_detail}. Skipping sync.")
+                                continue
                         
                         # Get additional exchange data for notification
                         unrealized_pnl = float(exchange_pos.get('unrealizedPnl', 0))
@@ -1584,7 +1647,7 @@ class MarketMonitor:
                             success = stats.update_trade_position_opened(
                                 lifecycle_id, 
                                 entry_price, 
-                                abs(contracts),
+                                actual_contracts_size, # Use the determined actual_contracts_size
                                 f"external_fill_{int(datetime.now().timestamp())}"
                             )
                             
@@ -1598,7 +1661,7 @@ class MarketMonitor:
                                     f"🔄 <b>Position Auto-Synced</b>\n\n"
                                     f"Token: {token}\n"
                                     f"Direction: {position_side.upper()}\n"
-                                    f"Size: {abs(contracts):.6f} {token}\n"
+                                    f"Size: {actual_contracts_size:.6f} {token}\n"
                                     f"Entry Price: ${entry_price:,.4f}\n"
                                     f"Position Value: ${position_value:,.2f}\n"
                                     f"{pnl_emoji} P&L: ${unrealized_pnl:,.2f}\n"
@@ -1629,50 +1692,57 @@ class MarketMonitor:
         except Exception as e:
             logger.error(f"❌ Error in auto-sync orphaned positions: {e}", exc_info=True)
 
-    async def _estimate_entry_price_for_orphaned_position(self, symbol: str, contracts: float) -> float:
+    async def _estimate_entry_price_for_orphaned_position(self, symbol: str, contracts: float, side: str) -> float:
         """Estimate entry price for an orphaned position by checking recent fills and market data."""
         try:
-            # Method 1: Check recent fills from the exchange
-            recent_fills = self.trading_engine.get_recent_fills()
+            # Method 1: Check recent fills from the exchange for the correct side
+            recent_fills = self.trading_engine.get_recent_fills(symbol=symbol, limit=20) # Fetch more fills for better chance
             if recent_fills:
-                # Look for recent fills for this symbol
-                symbol_fills = [fill for fill in recent_fills if fill.get('symbol') == symbol]
+                # Filter fills that match the position's side (e.g., if position is short, entry fill was a 'sell')
+                # The 'side' parameter here is the position side ('long' or 'short')
+                # Entry fill for a 'long' position is a 'buy'
+                # Entry fill for a 'short' position is a 'sell'
+                entry_fill_side = 'buy' if side == 'long' else 'sell'
+                
+                symbol_side_fills = [
+                    fill for fill in recent_fills 
+                    if fill.get('symbol') == symbol and fill.get('side') == entry_fill_side
+                ]
                 
-                if symbol_fills:
-                    # Get the most recent fill as entry price estimate
-                    latest_fill = symbol_fills[0]  # Assuming sorted by newest first
+                if symbol_side_fills:
+                    # Try to find a fill that approximately matches the quantity, or take the latest.
+                    # This logic can be refined, e.g., by looking for fills around the time the position might have opened.
+                    # For now, taking the latest matching side fill.
+                    latest_fill = symbol_side_fills[0] # Assuming CCXT sorts newest first, or needs sorting
                     fill_price = float(latest_fill.get('price', 0))
                     
                     if fill_price > 0:
-                        logger.info(f"💡 AUTO-SYNC: Found recent fill price for {symbol}: ${fill_price:.4f}")
+                        logger.info(f"💡 AUTO-SYNC: Estimated entry for {side} {symbol} via recent {entry_fill_side} fill: ${fill_price:.4f}")
                         return fill_price
             
-            # Method 2: Use current market price as fallback
+            # Method 2: Use current market price as fallback (less accurate for entry)
             market_data = self.trading_engine.get_market_data(symbol)
             if market_data and market_data.get('ticker'):
                 current_price = float(market_data['ticker'].get('last', 0))
-                
                 if current_price > 0:
-                    logger.warning(f"⚠️ AUTO-SYNC: Using current market price as entry estimate for {symbol}: ${current_price:.4f}")
+                    logger.warning(f"⚠️ AUTO-SYNC: Using current market price as entry estimate for {side} {symbol}: ${current_price:.4f}")
                     return current_price
             
-            # Method 3: Last resort - try bid/ask average
+            # Method 3: Last resort - try bid/ask average (even less accurate for entry)
             if market_data and market_data.get('ticker'):
                 bid = float(market_data['ticker'].get('bid', 0))
                 ask = float(market_data['ticker'].get('ask', 0))
-                
                 if bid > 0 and ask > 0:
                     avg_price = (bid + ask) / 2
-                    logger.warning(f"⚠️ AUTO-SYNC: Using bid/ask average as entry estimate for {symbol}: ${avg_price:.4f}")
+                    logger.warning(f"⚠️ AUTO-SYNC: Using bid/ask average as entry estimate for {side} {symbol}: ${avg_price:.4f}")
                     return avg_price
-            
-            # Method 4: Absolute fallback - return a small positive value to avoid 0
-            logger.error(f"❌ AUTO-SYNC: Could not estimate entry price for {symbol}, using fallback value of $1.00")
-            return 1.0
-            
+
+            logger.warning(f"AUTO-SYNC: Could not estimate entry price for {side} {symbol} through any method.")
+            return 0.0
+
         except Exception as e:
-            logger.error(f"❌ AUTO-SYNC: Error estimating entry price for {symbol}: {e}")
-            return 1.0  # Safe fallback
+            logger.error(f"❌ Error estimating entry price for orphaned position {symbol}: {e}", exc_info=True)
+            return 0.0
 
     async def _handle_orphaned_position(self, symbol, contracts):
         """Handle the orphaned position."""