Fix 10 bugs from QA audit: drawdown, PDT, Sharpe, reviewer alignment

src/financial_agent/broker/alpaca_client.py

@@ -140,7 +140,7 @@ def get_todays_filled_sides(self) -> dict[str, set[str]]:
             for o in raw_orders:
                 if str(o.status) == "filled":
                     sym = o.symbol
-                    side = str(o.side)
+                    side = o.side.value if hasattr(o.side, "value") else str(o.side)
                     if sym not in result:
                         result[sym] = set()
                     result[sym].add(side)

src/financial_agent/performance/benchmarking.py

@@ -113,9 +113,11 @@ def _mean(values: list[float]) -> float:
 
     @staticmethod
     def _std(values: list[float]) -> float:
-        """Compute population standard deviation without numpy."""
+        """Compute sample standard deviation without numpy."""
+        if len(values) < 2:
+            return 0.0
         m = sum(values) / len(values)
-        variance = sum((x - m) ** 2 for x in values) / len(values)
+        variance = sum((x - m) ** 2 for x in values) / (len(values) - 1)
         return math.sqrt(variance)
 
     def sharpe_ratio(

src/financial_agent/performance_main.py

@@ -56,7 +56,7 @@ def main() -> None:
     portfolio = broker.get_portfolio_snapshot()
 
     # Calculate metrics
-    daily_returns = equity_tracker.daily_returns(30)
+    daily_returns = equity_tracker.daily_returns(90)
     sharpe = perf_tracker.sharpe_ratio(daily_returns)
     sortino = perf_tracker.sortino_ratio(daily_returns)
     win_rate = perf_tracker.win_rate()
@@ -69,8 +69,8 @@ def main() -> None:
 
     # Build report
     metrics_rows = [
-        f"| Sharpe Ratio (30d) | {sharpe:.2f} |" if sharpe is not None else "",
-        f"| Sortino Ratio (30d) | {sortino:.2f} |" if sortino is not None else "",
+        f"| Sharpe Ratio (90d) | {sharpe:.2f} |" if sharpe is not None else "",
+        f"| Sortino Ratio (90d) | {sortino:.2f} |" if sortino is not None else "",
         f"| Win Rate | {win_rate:.1%} |" if win_rate is not None else "",
         f"| Profit Factor | {profit_factor:.2f} |" if profit_factor is not None else "",
         f"| Avg Win | ${avg_win:,.2f} |" if avg_win is not None else "",

src/financial_agent/persistence/equity_tracker.py

@@ -74,6 +74,11 @@ def _load(self) -> None:
             )
             self._peak_equity = 0.0
 
+        # Recover peak from history if peak file was missing/corrupt
+        if not self._peak_equity and self._history:
+            self._peak_equity = max(r.equity for r in self._history)
+            log.info("peak_recovered_from_history", peak=self._peak_equity)
+
     def _save(self) -> None:
         """Persist history (capped) and peak to disk."""
         try:
@@ -135,7 +140,8 @@ def current_drawdown(self, equity: float) -> float:
         """
         if self._peak_equity == 0:
             return 0.0
-        return (self._peak_equity - equity) / self._peak_equity
+        dd = (self._peak_equity - equity) / self._peak_equity
+        return max(dd, 0.0)
 
     def daily_returns(self, days: int = 30) -> list[float]:
         """Return the last *days* daily return percentages."""

src/financial_agent/persistence/thesis_store.py

@@ -152,19 +152,40 @@ def is_on_cooldown(self, symbol: str, cooldown_hours: int) -> bool:
             return False
         try:
             sell_time = datetime.fromisoformat(sell_time_str)
+            if sell_time.tzinfo is None:
+                sell_time = sell_time.replace(tzinfo=UTC)
             elapsed_hours = (datetime.now(tz=UTC) - sell_time).total_seconds() / 3600
             return elapsed_hours < cooldown_hours
         except (ValueError, TypeError):
             return False
 
     def _load_cooldowns(self) -> None:
-        """Load sell cooldown timestamps from disk."""
+        """Load sell cooldown timestamps from disk and prune expired entries."""
         try:
             if self._cooldown_path.exists():
                 self._cooldowns = json.loads(self._cooldown_path.read_text(encoding="utf-8"))
+                self._prune_expired_cooldowns()
         except Exception:
             self._cooldowns = {}
 
+    def _prune_expired_cooldowns(self, max_age_hours: int = 72) -> None:
+        """Remove cooldown entries older than max_age_hours."""
+        now = datetime.now(tz=UTC)
+        expired = []
+        for symbol, sell_time_str in self._cooldowns.items():
+            try:
+                sell_time = datetime.fromisoformat(sell_time_str)
+                if sell_time.tzinfo is None:
+                    sell_time = sell_time.replace(tzinfo=UTC)
+                if (now - sell_time).total_seconds() / 3600 >= max_age_hours:
+                    expired.append(symbol)
+            except (ValueError, TypeError):
+                expired.append(symbol)
+        for symbol in expired:
+            del self._cooldowns[symbol]
+        if expired:
+            self._save_cooldowns()
+
     def _save_cooldowns(self) -> None:
         """Write sell cooldown timestamps to disk."""
         try:

src/financial_agent/review/reviewer.py

@@ -15,22 +15,28 @@
 log = structlog.get_logger()
 
 REVIEW_SYSTEM_PROMPT = """\
-You are an expert portfolio manager and quantitative analyst reviewing a trading bot's \
-performance. Your job is to analyze the portfolio's current state, identify problems, and \
-suggest concrete improvements to the trading strategy and code.
+You are an expert portfolio manager reviewing an AGGRESSIVE MOMENTUM trading bot on a small \
+account (~$1,000). This bot's goal is FAST GROWTH through concentrated, high-conviction trades. \
+Capital sitting idle is capital losing to opportunity cost. Diversification is NOT a goal — \
+concentration in the strongest setups IS the goal.
 
 Focus on:
-1. **Performance issues** — positions with large unrealized losses, poor risk/reward.
-2. **Concentration risk** — over-allocation to single positions or correlated assets.
-3. **Strategy gaps** — missed opportunities, incorrect position sizing, parameter tuning.
-4. **Code suggestions** — concrete changes to config values, strategy logic, or watchlist \
-   that would improve results.
+1. **Capital deployment** — is too much cash sitting idle? Are positions sized big enough? \
+   The bot should have 3-5 concentrated positions, not 10 tiny ones.
+2. **Momentum alignment** — are positions riding winners and cutting losers? Is the bot \
+   holding names with broken setups instead of redeploying into strength?
+3. **Speed to alpha** — is the bot acting quickly enough on screener alerts and market moves? \
+   Are stop-losses and take-profits set at levels that match momentum trading (not buy-and-hold)?
+4. **Config tuning** — concrete parameter changes that would improve results for a small \
+   account momentum strategy.
 
 Rules:
+- This is a MOMENTUM strategy. Concentration is EXPECTED and DESIRED. Do NOT suggest \
+  diversifying — suggest concentrating into the BEST setups.
 - Be specific and actionable. Reference exact symbols, percentages, and config parameters.
 - Prioritize suggestions by expected impact (high/medium/low).
 - Limit to 3-5 suggestions to keep issues focused.
-- Each suggestion should map to a concrete code or config change.
+- If cash > 30% and VIX < 35, the #1 suggestion should ALWAYS be about deploying capital.
 - Consider both stock and crypto positions separately.
 
 Respond ONLY with valid JSON matching this schema:

src/financial_agent/review_main.py

@@ -327,13 +327,7 @@ def main() -> None:
         category = suggestion.get("category", "")
         body_text = suggestion.get("body", "")
 
-        # Skip if an open issue already covers this category
-        if category and category in existing_categories:
-            log.info("issue_skipped_duplicate_category", title=title, category=category)
-            skipped += 1
-            continue
-
-        # Skip if a similar title already exists (prevents repeated themes across categories)
+        # Skip if a similar title already exists (fuzzy match on keywords)
         if _is_duplicate_title(title, existing_titles):
             log.info("issue_skipped_duplicate_title", title=title)
             skipped += 1

src/financial_agent/risk/drawdown.py

@@ -28,7 +28,7 @@ class DrawdownAction(StrEnum):
 # Populate after class is defined to avoid forward reference issues.
 _SIZE_MULTIPLIERS = {
     DrawdownAction.NORMAL: 1.0,
-    DrawdownAction.REDUCE_SIZE: 0.75,
+    DrawdownAction.REDUCE_SIZE: 0.50,
     DrawdownAction.BUYS_ONLY_BLOCKED: 0.0,
     DrawdownAction.DERISK: 0.0,
     DrawdownAction.HALT: 0.0,
@@ -119,7 +119,7 @@ def size_multiplier(self, current_equity: float) -> float:
         """Return a position-sizing multiplier for the current drawdown state.
 
         * NORMAL -> 1.0
-        * REDUCE_SIZE -> 0.5
+        * REDUCE_SIZE -> 0.50
         * BUYS_ONLY_BLOCKED / DERISK / HALT -> 0.0
         """
         action = self.get_action(current_equity)

src/financial_agent/watchlist_main.py

@@ -209,6 +209,17 @@ def main() -> None:
         log.warning("empty_watchlist", message="AI returned empty watchlists. Skipping update.")
         return
 
+    # Enforce: held positions must always stay on the watchlist
+    for sym in held_stocks:
+        if sym not in new_stocks:
+            log.info("watchlist_held_position_preserved", symbol=sym)
+            new_stocks.append(sym)
+    for sym in held_crypto:
+        normalized = sym if "/" in sym else (sym[:-3] + "/USD" if sym.endswith("USD") else sym)
+        if normalized not in new_crypto:
+            log.info("watchlist_held_crypto_preserved", symbol=normalized)
+            new_crypto.append(normalized)
+
     # Get current watchlists for comparison
     old_stocks = [s.strip() for s in config.trading.watchlist.split(",")]
     old_crypto = [s.strip() for s in config.trading.crypto_watchlist.split(",")]

tests/unit/test_drawdown.py

@@ -56,10 +56,10 @@ def test_normal_returns_1(self):
         mult = breaker.size_multiplier(100_000.0)
         assert mult == 1.0
 
-    def test_reduce_size_returns_075(self):
+    def test_reduce_size_returns_050(self):
         breaker = DrawdownCircuitBreaker(peak_equity=100_000.0)
         mult = breaker.size_multiplier(85_000.0)
-        assert mult == 0.75
+        assert mult == 0.50
 
     def test_buys_blocked_returns_zero(self):
         breaker = DrawdownCircuitBreaker(peak_equity=100_000.0)

tests/unit/test_enhanced_engine.py

@@ -152,7 +152,7 @@ def test_halt_blocks_all_trading(self):
         assert len(orders) == 0
 
     def test_reduced_sizing_during_moderate_drawdown(self):
-        """15% drawdown should reduce buy sizes to 75%."""
+        """15% drawdown should reduce buy sizes to 50%."""
         breaker = DrawdownCircuitBreaker(peak_equity=100_000.0)
         engine = StrategyEngine(
             config=_make_config(),
@@ -165,9 +165,9 @@ def test_reduced_sizing_during_moderate_drawdown(self):
         orders = engine.generate_orders(signals, portfolio, technicals)
         assert len(orders) == 1
         # Max position = 85000 * 0.10 = 8500
-        # Target = 8500 * 0.8 * 0.75 (size_multiplier) = 5100
-        # qty = 5100 / 100 = 51
-        assert orders[0].qty == 51.0
+        # Target = 8500 * 0.8 * 0.50 (size_multiplier) = 3400
+        # qty = 3400 / 100 = 34
+        assert orders[0].qty == 34.0
 
 
 class TestEarningsBufferIntegration:

tests/unit/test_equity_tracker.py

@@ -72,13 +72,13 @@ def test_below_peak_calculates_correctly(self):
         dd = tracker.current_drawdown(90_000.0)
         assert abs(dd - 0.10) < 1e-9
 
-    def test_above_peak_returns_negative(self):
+    def test_above_peak_returns_zero(self):
         data_dir = tempfile.mkdtemp()
         tracker = EquityTracker(data_dir=data_dir)
         tracker.record(100_000.0, cash=20_000.0, positions_count=5)
-        # Formula: (peak - equity) / peak = (100k - 110k) / 100k = -0.1
+        # Above peak = no drawdown, clamped to 0
         dd = tracker.current_drawdown(110_000.0)
-        assert dd < 0
+        assert dd == 0.0
 
 
 class TestDailyReturns: