docs: Document runner SL gap as Common Pitfall #27

Added comprehensive documentation of the runner stop loss protection gap:
- Root cause analysis (SL check only before TP1)
- Bug sequence and impact details
- Code fix with examples
- Compounding factors (small runner + no on-chain orders)
- Lesson learned for future risk management code
This commit is contained in:
mindesbunister
2025-11-15 22:12:04 +01:00
parent 59bc267206
commit 9cd3a015f5

View File

@@ -1183,6 +1183,43 @@ trade.realizedPnL += actualRealizedPnL // NOT: result.realizedPnL from SDK
- **v6:** HalfTrend + BarColor strategy (Nov 12+)
- Used for performance comparison between strategies
27. **Runner stop loss gap - NO protection between TP1 and TP2 (CRITICAL - Fixed Nov 15, 2025):**
- **Symptom:** Runner position remained open despite price moving far past stop loss level
- **Root Cause:** Position Manager only checked stop loss BEFORE TP1 (line 877: `if (!trade.tp1Hit && this.shouldStopLoss(...)`), creating a protection gap
- **Bug sequence:**
1. SHORT opened, TP1 hit at 70% close (runner = 30% remaining)
2. Runner had stop loss at profit-lock level (+0.5%)
3. Price moved past stop loss → NO CHECK RAN (tp1Hit = true, so SL check skipped)
4. Runner exposed to unlimited loss for hours during TP1→TP2 window
5. Made worse by runner below Drift minimum size ($12.79 < $15) = no on-chain orders either
- **Impact:** Hours of unprotected runner exposure = potential unlimited loss on 25-30% remaining position
- **Code analysis:**
```typescript
// Line 877: Stop loss checked ONLY before TP1
if (!trade.tp1Hit && this.shouldStopLoss(currentPrice, trade)) {
console.log(`🔴 STOP LOSS: ${trade.symbol}`)
await this.executeExit(trade, 100, 'SL', currentPrice)
}
// Lines 881-895: TP1 and TP2 processing - NO STOP LOSS CHECK
// BUG: Runner between TP1-TP2 had ZERO stop loss protection!
```
- **Fix:** Added explicit runner stop loss check at line ~881:
```typescript
// 2b. CRITICAL: Runner stop loss (AFTER TP1, BEFORE TP2)
// This protects the runner position after TP1 closes main position
if (trade.tp1Hit && !trade.tp2Hit && this.shouldStopLoss(currentPrice, trade)) {
console.log(`🔴 RUNNER STOP LOSS: ${trade.symbol} at ${profitPercent.toFixed(2)}% (profit lock triggered)`)
await this.executeExit(trade, 100, 'SL', currentPrice)
return
}
```
- **Why undetected:** Runner system relatively new (Nov 11), most trades hit TP2 quickly without price reversals
- **Compounded by:** Drift minimum size check ($15 for SOL) prevented on-chain SL orders for small runners
- **Log warning:** `⚠️ SL size below market min, skipping on-chain SL` indicates runner has NO on-chain protection
- **Lesson:** Every conditional branch in risk management MUST have explicit stop loss checks - never assume "it'll get caught somewhere"
27. **External closure duplicate updates bug (CRITICAL - Fixed Nov 12, 2025):**
- **Symptom:** Trades showing 7-8x larger losses than actual ($58 loss when Drift shows $7 loss)
- **Root Cause:** Position Manager monitoring loop re-processes external closures multiple times before trade removed from activeTrades Map
@@ -1212,7 +1249,7 @@ trade.realizedPnL += actualRealizedPnL // NOT: result.realizedPnL from SDK
- **Root cause:** Async timing issue - `removeTrade()` is async but monitoring loop continues synchronously
- **Evidence:** Logs showed 8 consecutive "External closure recorded" messages with increasing P&L
- **Line:** `lib/trading/position-manager.ts` line 493 (external closure detection block)
- Must update `CreateTradeParams` interface when adding new database fields (see pitfall #21)
- Must update `CreateTradeParams` interface when adding new database fields (see pitfall #23)
- Analytics endpoint `/api/analytics/version-comparison` compares v5 vs v6 performance
28. **Signal quality threshold adjustment (Nov 12, 2025):**