//! Portfolio state tracking for DQN training
//!
//! This module provides portfolio state management for the DQN agent, tracking:
//! - Portfolio value (cash + unrealized P&L)
//! - Position size (signed: +Long, -Short, 0 for flat)
//! - Bid-ask spread (for transaction costs)
//!
//! The PortfolioTracker is used by DQNTrainer to provide portfolio features
//! to the reward function, enabling P&L-based reward calculations.

use crate::common::action::FactoredAction;
use crate::trading_action::TradingAction;
use tracing::{debug, warn};

/// Trade action enum with quantities for TradeExecutor
///
/// This is different from TradingAction which doesn't carry quantities.
/// TradeExecutor uses this to specify exact quantities for risk management.
#[derive(Debug, Clone, Copy, PartialEq)]
pub enum TradeAction {
    /// Buy with specified quantity
    Buy(f64),
    /// Sell with specified quantity
    Sell(f64),
    /// Hold (no action)
    Hold,
}

/// Portfolio state tracker for DQN training
///
/// Tracks cash, position size, and spread to provide portfolio features
/// for reward calculations. The tracker maintains state across training
/// steps within an epoch, and can be reset at epoch boundaries.
#[derive(Debug, Clone)]
pub struct PortfolioTracker {
    /// Current cash balance
    cash: f32,
    /// Current position size (positive = long, negative = short, 0 = flat)
    position_size: f32,
    /// Entry price for current position
    position_entry_price: f32,
    /// Initial capital (for reset)
    initial_capital: f32,
    /// Average bid-ask spread (estimated from historical data)
    avg_spread: f32,
    /// Last observed price (for parameter-less total_value() calls)
    last_price: f32,
    /// Cash reserve requirement as percentage of portfolio value (0-100)
    cash_reserve_percent: f32,
    /// Cumulative transaction costs
    cumulative_transaction_costs: f32,
    /// Peak portfolio value (high-water mark for drawdown calculation)
    peak_value: f32,
}

impl PortfolioTracker {
    /// Create a new portfolio tracker
    ///
    /// # Arguments
    ///
    /// * `initial_capital` - Starting cash balance (e.g., 10,000.0)
    /// * `avg_spread` - Average bid-ask spread as a fraction (e.g., 0.0001 = 1 basis point)
    /// * `cash_reserve_percent` - Cash reserve requirement as percentage of portfolio value (0-100, default: 0)
    ///
    /// # Example
    ///
    /// ```
    /// use ml::dqn::portfolio_tracker::PortfolioTracker;
    ///
    /// let tracker = PortfolioTracker::new(10_000.0, 0.0001, 0.0);
    /// ```
    pub fn new(initial_capital: f32, avg_spread: f32, cash_reserve_percent: f64) -> Self {
        Self {
            cash: initial_capital,
            position_size: 0.0,
            position_entry_price: 0.0,
            initial_capital,
            avg_spread,
            last_price: 0.0,
            cash_reserve_percent: cash_reserve_percent as f32,
            cumulative_transaction_costs: 0.0,
            peak_value: initial_capital,
        }
    }

    /// Create a new portfolio tracker with default spread
    ///
    /// This is a convenience constructor for backward compatibility with TradeExecutor.
    /// Uses a default spread of 0.0001 (1 basis point).
    ///
    /// # Arguments
    ///
    /// * `initial_capital` - Starting cash balance (e.g., 10,000.0)
    ///
    /// # Example
    ///
    /// ```
    /// use ml::dqn::portfolio_tracker::PortfolioTracker;
    ///
    /// let tracker = PortfolioTracker::with_default_spread(10_000.0);
    /// ```
    pub fn with_default_spread(initial_capital: f32) -> Self {
        Self::new(initial_capital, 0.0001, 0.0)
    }

    /// Get portfolio features for TradingState
    ///
    /// Returns a 3-element array:
    /// - `[0]`: Portfolio value (cash + unrealized P&L)
    /// - `[1]`: Position size (signed: +Long, -Short, 0 for flat)
    /// - `[2]`: Bid-ask spread (for transaction costs)
    ///
    /// # Arguments
    ///
    /// * `current_price` - Current market price for calculating unrealized P&L
    ///
    /// # Example
    ///
    /// ```
    /// use ml::dqn::portfolio_tracker::PortfolioTracker;
    ///
    /// let tracker = PortfolioTracker::new(10_000.0, 0.0001, 0.0);
    /// let features = tracker.get_portfolio_features(100.0);
    /// assert_eq!(features[0], 10_000.0); // Portfolio value = cash (no position)
    /// assert_eq!(features[1], 0.0);      // No position
    /// assert_eq!(features[2], 0.0001);   // Spread
    /// ```
    /// Returns raw (unnormalized) portfolio features for testing and internal use
    /// [portfolio_value, position_size, spread]
    pub fn get_raw_portfolio_features(&self, current_price: f32) -> [f32; 3] {
        let portfolio_value = self.get_portfolio_value(current_price);
        [
            portfolio_value,    // [0] Raw portfolio value
            self.position_size, // [1] Raw position size
            self.avg_spread,    // [2] Spread
        ]
    }

    /// Check if the action would cause a position reversal
    ///
    /// A reversal occurs when the position changes sign (Long→Short or Short→Long)
    ///
    /// # Arguments
    ///
    /// * `target_position` - The target position size after executing the action
    ///
    /// # Returns
    ///
    /// true if this is a reversal (sign change), false otherwise
    fn is_reversal(&self, target_position: f32) -> bool {
        (self.position_size > 0.0 && target_position < 0.0) ||
        (self.position_size < 0.0 && target_position > 0.0)
    }

    /// Returns normalized portfolio features for ML reward calculation
    /// [normalized_value, normalized_position, spread]
    pub fn get_portfolio_features(&self, current_price: f32) -> [f32; 3] {
        let portfolio_value = self.get_portfolio_value(current_price);

        // Normalize portfolio value by initial capital, centered at 0.0
        // Example: 10,100 / 10,000 - 1.0 = 0.01 (1% gain)
        let normalized_value = portfolio_value / self.initial_capital - 1.0;

        // Normalize position size: Assume max position = initial_capital / current_price
        // Example: For $10,000 capital at $100/contract: max = 100 contracts
        // Position of 10 contracts = 10/100 = 0.1 (10% exposure)
        let max_position = if current_price > 0.0 {
            self.initial_capital / current_price
        } else {
            1.0 // Fallback to avoid division by zero
        };
        let normalized_position = self.position_size / max_position;

        [
            normalized_value,    // [0] Normalized portfolio value (1.0 = initial capital)
            normalized_position, // [1] Normalized position size (1.0 = max exposure)
            self.avg_spread,     // [2] Spread (already small, doesn't need normalization)
        ]
    }

    /// Execute factored trading action and update portfolio state
    ///
    /// # Arguments
    ///
    /// * `action` - The factored trading action to execute (exposure: -1.0 to +1.0)
    /// * `price` - Current market price
    /// * `max_position` - Maximum position size (e.g., 100.0 contracts)
    ///
    /// # Action Behavior
    ///
    /// Uses the action's target exposure (-1.0 to +1.0) to set the portfolio position:
    /// - **Short100**: Sets position to -1.0 * max_position (full short)
    /// - **Short50**: Sets position to -0.5 * max_position
    /// - **Flat**: Sets position to 0.0 (closes position)
    /// - **Long50**: Sets position to +0.5 * max_position
    /// - **Long100**: Sets position to +1.0 * max_position (full long)
    ///
    /// # Example
    ///
    /// ```
    /// use ml::dqn::portfolio_tracker::PortfolioTracker;
    /// use ml::dqn::action_space::{FactoredAction, ExposureLevel, OrderType, Urgency};
    ///
    /// let mut tracker = PortfolioTracker::new(10_000.0, 0.0001, 0.0);
    /// let action = FactoredAction::new(ExposureLevel::Long100, OrderType::Market, Urgency::Normal);
    /// tracker.execute_action(action, 100.0, 100.0);
    /// assert_eq!(tracker.position_size, 100.0); // Full long position
    /// ```
    pub fn execute_action_with_kelly(&mut self, action: FactoredAction, price: f32, max_position: f32, kelly_fraction: f64) {
        self.execute_action_internal(action, price, max_position, Some(kelly_fraction as f32));
    }

    pub fn execute_action(&mut self, action: FactoredAction, price: f32, max_position: f32) {
        self.execute_action_internal(action, price, max_position, None);
    }

    /// Internal method that handles both Kelly-scaled and non-scaled execution
    ///
    /// This is the core execution logic that applies Kelly scaling to position sizes.
    /// When kelly_fraction is provided, the target exposure is scaled by the Kelly fraction
    /// before calculating the target position size.
    ///
    /// # Arguments
    ///
    /// * `action` - The factored trading action to execute
    /// * `price` - Current market price
    /// * `max_position` - Maximum position size (e.g., 100.0 contracts)
    /// * `kelly_fraction` - Optional Kelly scaling factor (0.0 to 1.5 typical range)
    ///
    /// # Kelly Scaling Example
    ///
    /// - Without Kelly: Long100 with max_position=10.0 → target_position = 10.0
    /// - With Kelly=0.5: Long100 with max_position=10.0 → target_position = 5.0 (50% sizing)
    /// - With Kelly=1.5: Long100 with max_position=10.0 → target_position = 15.0 (leverage)
    fn execute_action_internal(&mut self, action: FactoredAction, price: f32, max_position: f32, kelly_fraction: Option<f32>) {
        // Update high-water mark before any trade logic
        self.update_peak_value(price);

        // Drawdown circuit breaker: prevent new positions when drawdown exceeds 20%
        if self.peak_value > 0.0 {
            let current_value = self.get_portfolio_value(price);
            let drawdown = 1.0 - (current_value / self.peak_value);
            if drawdown > 0.20 {
                if self.position_size.abs() < f32::EPSILON {
                    // Already flat, refuse any new trades
                    debug!(
                        "Drawdown circuit breaker: {:.1}% drawdown, refusing new position. Peak: ${:.2}, Current: ${:.2}",
                        drawdown * 100.0, self.peak_value, current_value
                    );
                    return;
                }
                // Have an open position: force close to flat
                debug!(
                    "Drawdown circuit breaker: {:.1}% drawdown, force-closing position {:.2}. Peak: ${:.2}, Current: ${:.2}",
                    drawdown * 100.0, self.position_size, self.peak_value, current_value
                );
                self.cash += self.position_size * price;
                self.position_size = 0.0;
                self.position_entry_price = 0.0;
                return;
            }
        }

        // Get target exposure from action (-1.0 to +1.0)
        let target_exposure = action.target_exposure() as f32;

        // Calculate target position size
        let base_target_position = target_exposure * max_position;

        // Apply Kelly scaling if provided (Kelly Criterion position sizing)
        let target_position = if let Some(kelly) = kelly_fraction {
            base_target_position * kelly
        } else {
            base_target_position
        };

        // P2-C: Partial Reversal Support
        // When reversing position (Long→Short or Short→Long), split into two phases:
        // - Phase 1: Close current position (always affordable, generates cash)
        // - Phase 2: Open opposite position (may be partial if cash insufficient)
        if self.is_reversal(target_position) {
            // Calculate Phase 1: Close current position
            let tx_cost_rate = action.transaction_cost() as f32;

            // Phase 1: Close current position
            let phase1_delta = -self.position_size; // Delta to reach flat (0.0)
            let phase1_cost = phase1_delta.abs() * price;

            // Check if we can afford Phase 1 (safety check)
            if self.cash < 0.0 {
                debug!(
                    "P2-C: Reversal REJECTED - negative cash. Cash: ${:.2}, Position: {:.2}",
                    self.cash, self.position_size
                );
                return;
            }

            // BUG #42 FIX: Capture portfolio value BEFORE Phase 1 close for reserve calculation
            // The reserve should be based on the original portfolio value, not the inflated cash
            // after closing the position. This ensures consistent reserve requirements.
            let portfolio_value_before_phase1 = self.get_portfolio_value(price);

            // Execute Phase 1: Close current position
            // When closing:
            // - Long position: Sell generates cash (cash += position * price)
            // - Short position: Buy to cover costs cash (cash -= |position| * price)
            let phase1_cash_change = if self.position_size > 0.0 {
                // Closing long: Sell position, receive cash
                self.position_size * price
            } else {
                // Closing short: Buy to cover, pay cash
                self.position_size * price // position_size is negative, so this is negative
            };

            // Apply Phase 1 transaction cost (scaled by urgency)
            let urgency_mult = action.urgency_weight() as f32;
            let phase1_tx_cost = phase1_cost * tx_cost_rate * urgency_mult;
            self.cumulative_transaction_costs += phase1_tx_cost;

            self.cash += phase1_cash_change - phase1_tx_cost;
            let old_position = self.position_size;
            self.position_size = 0.0;
            self.position_entry_price = 0.0;

            debug!(
                "P2-C: Phase 1 complete - Closed position {:.2} → 0.0, Cash: ${:.2} → ${:.2}",
                old_position, self.cash - phase1_cash_change, self.cash
            );

            // Calculate Phase 2: Open opposite position
            let phase2_target = target_position; // Full target position

            // Calculate affordable Phase 2 position respecting cash reserve
            // BUG #42 FIX: Use portfolio value from BEFORE Phase 1 close
            // This prevents the reserve from being calculated on inflated cash
            let reserve_required = if self.cash_reserve_percent > 0.0 {
                portfolio_value_before_phase1 * (self.cash_reserve_percent / 100.0)
            } else {
                0.0
            };

            let affordable_cash = (self.cash - reserve_required).max(0.0);

            // Phase 2 also incurs transaction costs
            let max_affordable_with_costs = if price > 0.0 {
                affordable_cash / (price * (1.0 + tx_cost_rate))
            } else {
                0.0
            }.floor();

            // Determine actual Phase 2 position (may be partial)
            let actual_phase2_contracts = max_affordable_with_costs.min(phase2_target.abs());

            if actual_phase2_contracts <= 0.0 {
                debug!(
                    "P2-C: Phase 2 SKIPPED - insufficient cash. Cash: ${:.2}, Reserve: ${:.2}, Target: {:.2}",
                    self.cash, reserve_required, phase2_target
                );
                return; // Stay at flat position (0.0)
            }

            // Execute Phase 2: Open new position (partial if needed)
            let actual_phase2_position = if phase2_target > 0.0 {
                actual_phase2_contracts
            } else {
                -actual_phase2_contracts
            };

            // Apply Phase 2 transaction cost
            let phase2_cost = actual_phase2_position.abs() * price;
            let phase2_tx_cost = phase2_cost * tx_cost_rate * urgency_mult;
            self.cumulative_transaction_costs += phase2_tx_cost;

            self.cash -= actual_phase2_position * price + phase2_tx_cost;
            self.position_size = actual_phase2_position;
            self.position_entry_price = price;

            debug!(
                "P2-C: Phase 2 complete - Opened position {:.2} (target: {:.2}), Cash: ${:.2}",
                actual_phase2_position, phase2_target, self.cash
            );

            return; // Reversal handled
        }

        // Calculate position change
        // Non-reversal path: apply transaction costs (scaled by urgency)
        let tx_cost_rate = action.transaction_cost() as f32;
        let urgency_mult = action.urgency_weight() as f32;
        let mut target_position = target_position;
        let mut position_delta = target_position - self.position_size;

        // Calculate transaction cost for this trade
        if position_delta.abs() > 0.0 {
            let trade_value = position_delta.abs() * price;
            let tx_cost = trade_value * tx_cost_rate * urgency_mult;
            self.cumulative_transaction_costs += tx_cost;
            
            // Update cash accounting for transaction costs
            // For buys: cash -= (position_delta * price + tx_cost)
            // For sells: cash += (|position_delta| * price - tx_cost)
            if position_delta > 0.0 {
                // Buying: pay both trade cost and transaction cost
                self.cash -= tx_cost;
            } else {
                // Selling: receive trade proceeds minus transaction cost
                self.cash -= tx_cost;
            }
        }

        // P2-B: Cash Reserve Requirement Check
        // Only apply to BUY trades (position_delta > 0) as sells add cash
        if self.cash_reserve_percent > 0.0 && position_delta > 0.0 {
            // Calculate trade cost
            let trade_cost = position_delta * price;

            // Calculate current portfolio value and required reserve
            let portfolio_value = self.get_portfolio_value(price);
            let reserve_required = portfolio_value * (self.cash_reserve_percent / 100.0);

            // Check if trade would violate reserve
            let cash_after_trade = self.cash - trade_cost;

            if cash_after_trade < reserve_required {
                // Calculate affordable position (respecting reserve)
                let affordable_cash = (self.cash - reserve_required).max(0.0);
                let affordable_contracts = (affordable_cash / price).floor();

                if affordable_contracts <= 0.0 {
                    // Cannot afford any contracts - reject trade
                    warn!(
                        "P2-B: Trade REJECTED - cash reserve violated. Cash: ${:.2}, Reserve: ${:.2} ({:.1}%), Trade cost: ${:.2}",
                        self.cash, reserve_required, self.cash_reserve_percent, trade_cost
                    );
                    return;
                }

                // Reduce position to affordable amount
                warn!(
                    "P2-B: Trade REDUCED - cash reserve enforced. Requested: {:.1}, Reduced to: {:.1}",
                    target_position, self.position_size + affordable_contracts
                );
                target_position = self.position_size + affordable_contracts;
                position_delta = target_position - self.position_size;
            }
        }

        // Update entry price if opening/increasing position
        if target_position.abs() > self.position_size.abs() {
            self.position_entry_price = price;
        } else if target_position == 0.0 {
            self.position_entry_price = 0.0;
        } else {
            // Decreasing position but not closing - keep entry price
        }

        // Update cash (negative delta = buying, positive = selling)
        self.cash -= position_delta * price;

        // Update position
        self.position_size = target_position;
    }

    /// Execute legacy trading action (backward compatibility)
    ///
    /// # Arguments
    ///
    /// * `action` - The trading action to execute (Buy, Sell, Hold)
    /// * `price` - Current market price
    /// * `position_units` - Number of units to trade (e.g., 1.0 for 1 contract)
    ///
    /// # Action Behavior
    ///
    /// - **Buy**: Opens long position if flat, or closes short position
    /// - **Sell**: Opens short position if flat, or closes long position
    /// - **Hold**: No change to portfolio state
    ///
    /// # Example
    ///
    /// ```
    /// use ml::dqn::portfolio_tracker::PortfolioTracker;
    /// use ml::dqn::agent::TradingAction;
    ///
    /// let mut tracker = PortfolioTracker::new(10_000.0, 0.0001, 0.0);
    /// tracker.execute_legacy_action(TradingAction::Buy, 100.0, 10.0);
    /// assert_eq!(tracker.position_size, 10.0);
    /// assert_eq!(tracker.cash, 9_000.0); // 10_000 - (10 * 100)
    /// ```
    pub fn execute_legacy_action(
        &mut self,
        action: TradingAction,
        price: f32,
        position_units: f32,
    ) {
        match action {
            TradingAction::Buy => {
                if self.position_size == 0.0 {
                    // Open long position
                    self.position_size = position_units;
                    self.position_entry_price = price;
                    self.cash -= position_units * price;
                } else if self.position_size < 0.0 {
                    // Close short position (buy to cover)
                    self.cash += self.position_size * price; // position_size is negative, so this subtracts
                    self.position_size = 0.0;
                    self.position_entry_price = 0.0;
                } else {
                    // Already long, do nothing (no position sizing changes)
                }
            },
            TradingAction::Sell => {
                if self.position_size == 0.0 {
                    // Open short position
                    self.position_size = -position_units;
                    self.position_entry_price = price;
                    self.cash += position_units * price;
                } else if self.position_size > 0.0 {
                    // Close long position
                    self.cash += self.position_size * price; // Sell to close
                    self.position_size = 0.0;
                    self.position_entry_price = 0.0;
                } else {
                    // Already short, do nothing (no position sizing changes)
                }
            },
            TradingAction::Hold => {
                // No action - portfolio state unchanged
            },
        }
    }

    /// Calculate current portfolio value (cash + unrealized P&L)
    ///
    /// # Arguments
    ///
    /// * `current_price` - Current market price
    ///
    /// # Returns
    ///
    /// Total portfolio value including unrealized P&L from open positions
    fn get_portfolio_value(&self, current_price: f32) -> f32 {
        // Portfolio value = cash + position_value_at_current_price
        // This naturally handles both long and short positions:
        // - Long: cash decreases when buying, position value increases with price
        // - Short: cash increases when selling, position value is negative (liability)
        self.cash + (self.position_size * current_price)
    }

    /// Update peak_value high-water mark (call from mutable methods only)
    fn update_peak_value(&mut self, current_price: f32) {
        let value = self.get_portfolio_value(current_price);
        if value > self.peak_value {
            self.peak_value = value;
        }
    }

    /// Reset portfolio to initial state (for new episode/epoch)
    ///
    /// This resets:
    /// - Cash to initial capital
    /// - Position size to 0 (flat)
    /// - Entry price to 0
    ///
    /// # Example
    ///
    /// ```
    /// use ml::dqn::portfolio_tracker::PortfolioTracker;
    /// use ml::dqn::agent::TradingAction;
    ///
    /// let mut tracker = PortfolioTracker::new(10_000.0, 0.0001, 0.0);
    /// tracker.execute_action(TradingAction::Buy, 100.0, 10.0);
    /// tracker.reset();
    /// assert_eq!(tracker.cash, 10_000.0);
    /// assert_eq!(tracker.position_size, 0.0);
    /// ```
    pub fn reset(&mut self) {
        self.cash = self.initial_capital;
        self.position_size = 0.0;
        self.position_entry_price = 0.0;
        self.last_price = 0.0;
        self.cumulative_transaction_costs = 0.0;
        self.peak_value = self.initial_capital;
    }

    /// Reset drawdown tracking without resetting capital.
    ///
    /// Sets the peak_value (high-water mark) to the current portfolio value
    /// so the circuit breaker only triggers on within-epoch drawdown, not
    /// cross-epoch drawdown. Capital and position state are preserved.
    pub fn reset_drawdown_tracking(&mut self) {
        let current_value = self.total_value(self.last_price);
        self.peak_value = if current_value > 0.0 { current_value } else { self.initial_capital };
    }

    /// Get initial capital
    pub fn initial_capital(&self) -> f32 {
        self.initial_capital
    }

    /// Get average spread
    pub fn spread(&self) -> f32 {
        self.avg_spread
    }

    /// Set position directly for backtest portfolio state synchronization.
    ///
    /// Used between evaluation chunks to sync the tracker with the
    /// EvaluationEngine's current position without executing a trade.
    pub fn set_position_direct(
        &mut self,
        position_size: f32,
        entry_price: f32,
        current_price: f32,
    ) {
        self.position_size = position_size;
        self.position_entry_price = entry_price;
        self.last_price = current_price;
        // Reconstruct cash: initial_capital - position cost
        self.cash = self.initial_capital - (position_size * entry_price).abs();
    }

    // ========== Public Accessors for TradeExecutor Integration ==========

    /// Get current cash balance
    ///
    /// # Returns
    ///
    /// Current cash balance (may be negative if leveraged)
    pub fn cash_balance(&self) -> f32 {
        self.cash
    }

    /// Get current position size
    ///
    /// # Returns
    ///
    /// Current position size (positive = long, negative = short, 0 = flat)
    pub fn current_position(&self) -> f32 {
        self.position_size
    }

    /// Get total portfolio value (cash + unrealized P&L)
    ///
    /// This requires the current market price to calculate unrealized P&L.
    /// For flat positions, this is equivalent to cash balance.
    ///
    /// # Arguments
    ///
    /// * `current_price` - Current market price
    ///
    /// # Returns
    ///
    /// Total portfolio value including unrealized P&L
    pub fn total_value(&self, current_price: f32) -> f32 {
        self.get_portfolio_value(current_price)
    }

    /// Get average entry price for current position
    ///
    /// # Returns
    ///
    /// Average entry price (0.0 if no position)
    pub fn average_entry_price(&self) -> f32 {
        self.position_entry_price
    }

    /// Get realized P&L
    ///
    /// Realized P&L is the profit/loss from closed positions.
    /// This is calculated as: (current cash - initial capital)
    ///
    /// # Returns
    ///
    /// Realized profit/loss
    pub fn realized_pnl(&self) -> f32 {
        self.cash - self.initial_capital
    }

    /// Get unrealized P&L
    ///
    /// Unrealized P&L is the profit/loss from open positions at current market price.
    ///
    /// # Arguments
    ///
    /// * `current_price` - Current market price
    ///
    /// # Returns
    ///
    /// Unrealized profit/loss (0.0 if no position)
    pub fn unrealized_pnl(&self, current_price: f32) -> f32 {
        // Unrealized P&L = current_portfolio_value - initial_capital
        // This is the profit/loss from open positions
        self.get_portfolio_value(current_price) - self.initial_capital
    }

    /// Get cumulative transaction costs
    ///
    /// # Returns
    ///
    /// Total transaction costs incurred across all trades
    pub fn transaction_costs(&self) -> f32 {
        self.cumulative_transaction_costs
    }

    // ========== Parameter-less Overloads (Use Last Price) ==========

    /// Get total portfolio value using last observed price
    ///
    /// This is a convenience method for TradeExecutor which doesn't track current price.
    /// Uses the last price from execute_trade() calls.
    ///
    /// # Returns
    ///
    /// Total portfolio value at last observed price
    ///
    /// # Panics
    ///
    /// May return incorrect values if called before any execute_trade() call.
    /// For accurate values, prefer `total_value(current_price)`.
    pub fn total_value_cached(&self) -> f32 {
        self.total_value(self.last_price)
    }

    /// Get unrealized P&L using last observed price
    ///
    /// This is a convenience method for TradeExecutor which doesn't track current price.
    /// Uses the last price from execute_trade() calls.
    ///
    /// # Returns
    ///
    /// Unrealized P&L at last observed price
    ///
    /// # Panics
    ///
    /// May return incorrect values if called before any execute_trade() call.
    /// For accurate values, prefer `unrealized_pnl(current_price)`.
    pub fn unrealized_pnl_cached(&self) -> f32 {
        self.unrealized_pnl(self.last_price)
    }

    /// Execute trade (backward compatibility wrapper for execute_legacy_action)
    ///
    /// This is a wrapper method that accepts TradeAction with quantities and f64 types,
    /// converting to the internal f32 representation for compatibility with TradeExecutor.
    ///
    /// # Arguments
    ///
    /// * `action` - Trading action (Buy/Sell/Hold with quantity)
    /// * `price` - Current market price
    pub fn execute_trade(&mut self, action: TradeAction, price: f64) {
        let price_f32 = price as f32;
        self.last_price = price_f32; // Track last price for parameter-less methods

        // Convert TradeAction with quantities to basic TradingAction
        let (trading_action, quantity) = match action {
            TradeAction::Buy(qty) => (TradingAction::Buy, qty as f32),
            TradeAction::Sell(qty) => (TradingAction::Sell, qty as f32),
            TradeAction::Hold => (TradingAction::Hold, 0.0),
        };
        self.execute_legacy_action(trading_action, price_f32, quantity);
    }
}

#[cfg(test)]
mod tests {
    use super::*;

    #[test]
    fn test_portfolio_tracker_initial_state() {
        let tracker = PortfolioTracker::new(10_000.0, 0.0001, 0.0);
        let features = tracker.get_raw_portfolio_features(100.0);

        assert_eq!(features[0], 10_000.0); // Portfolio value = cash
        assert_eq!(features[1], 0.0); // No position
        assert_eq!(features[2], 0.0001); // Spread
    }

    #[test]
    fn test_portfolio_tracker_buy_action() {
        let mut tracker = PortfolioTracker::new(10_000.0, 0.0001, 0.0);
        tracker.execute_legacy_action(TradingAction::Buy, 100.0, 10.0);

        assert_eq!(tracker.position_size, 10.0);
        assert_eq!(tracker.position_entry_price, 100.0);
        assert_eq!(tracker.cash, 9_000.0); // 10_000 - (10 * 100)
    }

    #[test]
    fn test_portfolio_tracker_sell_action() {
        let mut tracker = PortfolioTracker::new(10_000.0, 0.0001, 0.0);
        tracker.execute_legacy_action(TradingAction::Sell, 100.0, 10.0);

        assert_eq!(tracker.position_size, -10.0);
        assert_eq!(tracker.position_entry_price, 100.0);
        assert_eq!(tracker.cash, 11_000.0); // 10_000 + (10 * 100)
    }

    #[test]
    fn test_portfolio_tracker_pnl_calculation_long() {
        let mut tracker = PortfolioTracker::new(10_000.0, 0.0001, 0.0);
        tracker.execute_legacy_action(TradingAction::Buy, 100.0, 10.0);

        // Price rises to 110
        let features = tracker.get_raw_portfolio_features(110.0);
        // Portfolio value = cash + position_value = 9000 + (10 * 110) = 10100
        let expected_value = 10_100.0;
        assert_eq!(features[0], expected_value);
    }

    #[test]
    fn test_portfolio_tracker_pnl_calculation_short() {
        let mut tracker = PortfolioTracker::new(10_000.0, 0.0001, 0.0);
        tracker.execute_legacy_action(TradingAction::Sell, 100.0, 10.0);

        // Price falls to 90 (profitable for short)
        let features = tracker.get_raw_portfolio_features(90.0);
        // Portfolio value = cash + position_value = 11000 + (-10 * 90) = 11000 - 900 = 10100
        // Unrealized P&L = 10100 - 10000 = +100 (profit from price drop)
        let expected_value = 10_100.0;
        assert_eq!(features[0], expected_value);
    }

    #[test]
    fn test_portfolio_tracker_close_long_position() {
        let mut tracker = PortfolioTracker::new(10_000.0, 0.0001, 0.0);
        tracker.execute_legacy_action(TradingAction::Buy, 100.0, 10.0);
        tracker.execute_legacy_action(TradingAction::Sell, 110.0, 10.0);

        // Position closed with profit
        assert_eq!(tracker.position_size, 0.0);
        assert_eq!(tracker.cash, 10_100.0); // 9000 + 1000 (return capital) + 1100 (sale) = 11100, or equivalently: 10000 (initial) + 100 (profit)
    }

    #[test]
    fn test_portfolio_tracker_close_short_position() {
        let mut tracker = PortfolioTracker::new(10_000.0, 0.0001, 0.0);
        tracker.execute_legacy_action(TradingAction::Sell, 100.0, 10.0);
        tracker.execute_legacy_action(TradingAction::Buy, 90.0, 10.0);

        // Position closed with profit
        assert_eq!(tracker.position_size, 0.0);
        assert_eq!(tracker.cash, 10_100.0); // 11000 (after short) - 900 (buyback) = 10100, or: 10000 (initial) + 100 (profit)
    }

    #[test]
    fn test_portfolio_tracker_reset() {
        let mut tracker = PortfolioTracker::new(10_000.0, 0.0001, 0.0);
        tracker.execute_legacy_action(TradingAction::Buy, 100.0, 10.0);
        tracker.reset();

        assert_eq!(tracker.cash, 10_000.0);
        assert_eq!(tracker.position_size, 0.0);
        assert_eq!(tracker.position_entry_price, 0.0);
    }

    #[test]
    fn test_portfolio_tracker_hold_action() {
        let mut tracker = PortfolioTracker::new(10_000.0, 0.0001, 0.0);
        let initial_cash = tracker.cash;
        let initial_position = tracker.position_size;

        tracker.execute_legacy_action(TradingAction::Hold, 100.0, 10.0);

        // No change after hold
        assert_eq!(tracker.cash, initial_cash);
        assert_eq!(tracker.position_size, initial_position);
    }

    #[test]
    fn test_drawdown_circuit_breaker() {
        use crate::common::action::{ExposureLevel, OrderType, Urgency};

        let mut tracker = PortfolioTracker::new(100_000.0, 0.0, 0.0);

        // Simulate loss to trigger 25% drawdown
        // Start with $100K, lose to $75K
        tracker.cash = 75_000.0;
        tracker.position_size = 0.0;
        // peak_value should be 100K from initialization

        let action = FactoredAction::new(ExposureLevel::Long100, OrderType::Market, Urgency::Normal);
        tracker.execute_action(action, 100.0, 4.0);

        // Should refuse to open position when drawdown > 20%
        assert!(
            tracker.current_position().abs() < f32::EPSILON,
            "Circuit breaker should prevent new positions at >20% drawdown, got {}",
            tracker.current_position()
        );
    }

    #[test]
    fn test_drawdown_circuit_breaker_force_close() {
        use crate::common::action::{ExposureLevel, OrderType, Urgency};

        let mut tracker = PortfolioTracker::new(100_000.0, 0.0, 0.0);

        // Open a long position first (at no drawdown)
        let action = FactoredAction::new(ExposureLevel::Long100, OrderType::Market, Urgency::Normal);
        tracker.execute_action(action, 100.0, 4.0);
        assert!((tracker.current_position() - 4.0).abs() < f32::EPSILON);

        // Simulate catastrophic loss: manually set cash to create >20% drawdown
        // With position=4.0 at price=100, position_value=400. Total = cash + 400.
        // Peak was 100K. Need total < 80K. So cash < 79_600.
        tracker.cash = 50_000.0;
        // Total value = 50_000 + 4*100 = 50_400. Drawdown = 1 - 50400/100000 = 49.6%

        // Any action should force-close the position
        let action2 = FactoredAction::new(ExposureLevel::Long100, OrderType::Market, Urgency::Normal);
        tracker.execute_action(action2, 100.0, 4.0);

        // Should have force-closed to flat
        assert!(
            tracker.current_position().abs() < f32::EPSILON,
            "Circuit breaker should force-close position at >20% drawdown, got {}",
            tracker.current_position()
        );
    }

    #[test]
    fn test_drawdown_below_threshold_allows_trading() {
        use crate::common::action::{ExposureLevel, OrderType, Urgency};

        let mut tracker = PortfolioTracker::new(100_000.0, 0.0, 0.0);

        // 15% drawdown (below 20% threshold)
        tracker.cash = 85_000.0;
        tracker.position_size = 0.0;

        let action = FactoredAction::new(ExposureLevel::Long100, OrderType::Market, Urgency::Normal);
        tracker.execute_action(action, 100.0, 4.0);

        // Should allow the trade (drawdown < 20%)
        assert!(
            (tracker.current_position() - 4.0).abs() < f32::EPSILON,
            "Should allow trading at 15% drawdown, got position {}",
            tracker.current_position()
        );
    }
}
