Add Spike Bot trading strategy

[guzus]

Implements momentum trading strategy that exploits price spikes on Polymarket.

Features

• Real-time spike detection algorithm
• Configurable thresholds and risk parameters
• Position management with profit targets and stop losses
• Example script with CLI interface
• Comprehensive documentation

Closes #34

Generated with Claude Code

[claude]

Pull Request Review: Spike Bot Trading Strategy

I've completed a thorough review of PR #35. Overall, this is a well-structured implementation with good documentation. Below are my findings organized by category:

---

Code Quality & Best Practices

Strengths:

• Clean separation of concerns with strategy inheriting from base Strategy class
• Good use of type hints throughout the codebase
• Well-documented with comprehensive docstrings
• Example script follows good CLI patterns with argparse
• Proper error handling in critical sections

Issues:

1. CLAUDE.md Violation - Documentation Created (spike_bot.py:1)

   • Per CLAUDE.md rule add logo #2: "DO NOT create a new document"
   • The 338-line wiki/strategies/spike_bot.md file violates this guideline
   • Recommendation: Remove or consolidate documentation into existing files

2. PnL Calculation Bug (spike_bot.py:226)

   pnl_pct = (current_price - entry_price) / entry_price

   • This assumes all positions are BUY positions
   • SELL positions (from upward spikes) will have inverted P&L
   • When you SELL at 0.65 and price drops to 0.63, you're profitable, but this shows negative P&L
   • Fix needed: Track position side and adjust calculation accordingly

3. Incomplete Position Exit Logic (spike_bot.py:216-257)

   • Only handles positions > 0, doesn't track or exit short positions
   • Short positions (from selling into upward spikes) are not managed
   • Risk: Unmanaged short exposure could accumulate

4. Position Tracking Inconsistency (spike_bot.py:115-120)

   if position > 0:
       self.manage_position(...)
   else:
       self.detect_and_trade_spike(...)

   • Assumes position tracking reflects actual market state
   • If a SELL order creates a short, position may not reflect this
   • Recommendation: Verify how the base Strategy class tracks positions for sells

---

Potential Bugs

5. Race Condition in Spike Detection (spike_bot.py:180-181)

   if self.spike_detected.get(outcome, False):
       return

   • Single boolean flag doesn't prevent multiple concurrent spike detections
   • If price spikes again before position exits, second spike is ignored
   • Recommendation: Consider time-based cooldown or better state management

6. Integer Truncation Loss (spike_bot.py:244, 290)

   exit_size = float(int(position))

   • Silently discards fractional positions
   • Could leave small positions unmanaged over time
   • Recommendation: Log when truncation occurs or handle fractional positions

7. Missing Price Validation (spike_bot.py:189, 197)

   • round_price() called on best_bid/best_ask without validating they're valid prices
   • If best_bid is None or <= 0, this could fail
   • Note: The earlier check at line 184 should prevent this, but defensive programming would help

---

Performance Considerations

8. Inefficient Price History Updates (spike_bot.py:125-130)

   • Calls get_current_price() twice per outcome per tick
   • Once in update_price_history(), again in on_tick()
   • Optimization: Cache the price from update_price_history

9. No Rate Limiting

   • 1-second polling interval with WebSocket/REST calls could hit API limits
   • No backoff or retry logic for failed API calls
   • Recommendation: Add rate limiting awareness

---

Security Concerns

10. Private Key Handling (examples/spike_bot.py:73-78)

    • Good: Private key read from environment variable
    • Good: Clear error message if not set
    • Note: CLAUDE.md rule fix: polymarket impl #4 says "DO NOT place many variables in .env file. Place them in the code instead."
    • However, for security-sensitive data like private keys, environment variables are the correct approach
    • This is an acceptable exception to the rule

11. No Input Validation (examples/spike_bot.py)

    • User-provided parameters (spike_threshold, profit_target, etc.) not validated
    • Could accept negative or nonsensical values
    • Recommendation: Add parameter validation (e.g., 0 < spike_threshold < 1)

---

Test Coverage

12. No Tests
    • No unit tests for SpikeBot class
    • No integration tests for spike detection logic
    • No tests for edge cases (empty orderbook, API failures, etc.)
    • Critical: Per CLAUDE.md rule Create debugging frontend for trading strategies #5 "Run and Debug yourself PROACTIVELY"
    • Recommendation: Add test coverage before merging

---

Architecture & Design

Strengths:

• Good inheritance from base Strategy class
• Proper use of deque for bounded price history
• Clean state management with dictionaries

Concerns:

13. Hardcoded Constants (spike_bot.py:157, 160)

    • Magic number 10 for recent price window hardcoded
    • Should be configurable or documented why 10 seconds
    • Recommendation: Make it a parameter or add comment explaining the choice

14. Tight Coupling to Polymarket

    • While docs say it's for Polymarket, the code doesn't actually enforce this
    • Base Strategy is exchange-agnostic, but spike bot assumes certain market dynamics
    • Note: Not necessarily a problem, but worth documenting

---

Documentation Quality

Strengths:

• Excellent README integration
• Comprehensive wiki documentation
• Good inline comments
• Clear usage examples

Issues:
15. Contradicts CLAUDE.md Guidelines
- The 338-line markdown file violates the "DO NOT create a new document" rule
- Action Required: Decide if this is essential or can be removed/consolidated

---

Recommendations Summary

Must Fix Before Merge:

1. Fix P&L calculation for SELL positions (spike_bot.py:226)
2. Add test coverage (at least basic unit tests)
3. Resolve CLAUDE.md documentation conflict - remove wiki file or update guidelines
4. Add input parameter validation

Should Fix:
5. Handle short position tracking and exit logic
6. Add time-based cooldown for spike detection
7. Cache price calculations to avoid redundant calls
8. Log integer truncation in position sizes

Nice to Have:
9. Add rate limiting awareness
10. Make magic numbers (like 10-second window) configurable
11. Add defensive checks for edge cases

---

Verdict

This is a solid implementation of a momentum trading strategy with good structure and documentation. The core logic is sound, but there are several bugs that could lead to incorrect P&L calculations and unmanaged positions. The lack of tests is concerning given the financial nature of the code.

Recommendation: Request changes to address the critical bugs (P&L calculation, position tracking) and add basic test coverage before merging.

The documentation is excellent but violates project guidelines - this needs to be resolved with the team.

---

Code Quality: 7/10
Test Coverage: 2/10 (no tests)
Documentation: 9/10
Security: 7/10
Overall: 6.5/10 - Good foundation, needs bug fixes and tests