Skip to content

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

Merged
mikejamescalvert merged 1 commit intomainfrom
fix/qa-quick-wins-strategy-alignment
Mar 27, 2026
Merged

Fix 10 bugs from QA audit: drawdown, PDT, Sharpe, reviewer alignment#115
mikejamescalvert merged 1 commit intomainfrom
fix/qa-quick-wins-strategy-alignment

Conversation

@mikejamescalvert
Copy link
Copy Markdown
Owner

Summary

Deep dive QA audit across all 38 source files found ~80 issues. This PR fixes the 10 highest-impact ones (quick wins + strategy alignment).

Bug Fixes (8)

  • Drawdown returns negative valuesEquityTracker.current_drawdown() went negative when equity exceeded peak, confusing the circuit breaker. Now clamped to 0.
  • Peak equity lost on file corruption — If peak_equity.json was missing/corrupt, peak reset to 0 forever. Now recovers from equity history.
  • REDUCE_SIZE multiplier was 0.75, docstring said 0.50 — Changed to 0.50 (more conservative during drawdown, which is correct for a small account)
  • Sharpe ratio inflated ~5% — Was using population std (÷N) instead of sample std (÷N-1)
  • Performance report used 30-day window — Statistically meaningless. Changed to 90 days.
  • PDT protection silently brokenstr(o.side) might produce "OrderSide.BUY" not "buy". Now uses .value for reliable enum extraction.
  • Cooldown timezone bug — Naive vs aware datetime comparison could raise TypeError, silently disabling anti-churn protection.
  • Cooldown dict grows forever — Added pruning of expired entries (>72h) on load.

Strategy Alignment (3)

  • Portfolio reviewer gave defensive advice — Prompt emphasized "diversification" and "concentration risk" for a momentum strategy that WANTS concentration. Rewritten to focus on capital deployment, momentum alignment, and speed to alpha.
  • Review issue dedup too aggressive — Category-based dedup blocked ALL new suggestions if any issue in that category existed (e.g., one "risk" issue = no more risk suggestions ever). Removed. Title-based fuzzy matching still prevents true duplicates.
  • Watchlist reviewer could drop held positions — AI was only asked to keep them (prompt instruction). Now enforced in code — held positions are always merged back into the watchlist after AI review.

Test plan

  • All 270 tests pass
  • Ruff lint clean
  • mypy strict clean (38 source files)
  • Updated 3 tests to match new behavior (drawdown clamp, multiplier change)
  • Verify next portfolio review creates actionable momentum-aligned suggestions
  • Verify PDT protection works correctly (check logs for order_blocked_pdt)

🤖 Generated with Claude Code

… alignment

Quick wins:
- Fix EquityTracker.current_drawdown() returning negative values (clamp to 0)
- Recover peak equity from history when peak file is corrupted
- Fix DrawdownCircuitBreaker REDUCE_SIZE multiplier (0.75→0.50, matches docstring)
- Fix Sharpe/Sortino using population std instead of sample std
- Change performance report lookback from 30→90 days for statistical significance
- Fix PDT protection: use enum .value instead of str() for order side comparison
- Add timezone safety to cooldown check (naive→aware datetime fix)
- Prune expired cooldowns on load (prevents unbounded growth)

Strategy alignment:
- Rewrite portfolio reviewer prompt for aggressive momentum (was giving
  defensive advice like "diversify" to a concentrated momentum strategy)
- Remove category-based issue dedup (was blocking all new suggestions
  when any issue in same category existed) — keep title-based only
- Enforce held positions stay on watchlist (code-level guarantee, not
  just AI prompt instruction)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@mikejamescalvert mikejamescalvert merged commit d9f8f37 into main Mar 27, 2026
3 checks passed
@mikejamescalvert mikejamescalvert deleted the fix/qa-quick-wins-strategy-alignment branch March 27, 2026 11:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant