docs: Add Common Pitfall #49 - P&L exponential compounding bug

Documents the critical P&L compounding bug fixed in commit 6156c0f where
trade.realizedPnL mutation during external closure detection caused 15-20x
inflation of actual profit/loss values.

Includes:
- Root cause: Mutating trade.realizedPnL in monitoring loop
- Real incident: $6 actual profit → $92.46 in database
- Bug mechanism with code examples
- Why previous fixes (Common Pitfalls #27, #48) didn't prevent this
- Fix: Don't mutate shared state during calculations
- Related to other P&L compounding variants for cross-reference

.github/copilot-instructions.md

@@ -2258,6 +2258,63 @@ trade.realizedPnL += actualRealizedPnL  // NOT: result.realizedPnL from SDK
     - **Files changed:** `lib/trading/position-manager.ts` (interface + logic)
     - **Lesson:** When introducing wait periods in financial systems, always add flags to prevent duplicate state updates during the wait
 
+49. **P&L exponential compounding in external closure detection (CRITICAL - Fixed Nov 17, 2025):**
+    - **Symptom:** Database P&L shows 15-20× actual value (e.g., $92.46 when Drift shows $6.00)
+    - **Root Cause:** `trade.realizedPnL` was being mutated during each external closure detection cycle
+    - **Real incident (Nov 17, 13:54 CET):**
+      * SOL-PERP SHORT closed by on-chain orders: 1.54 SOL at -1.95% + 2.3 SOL at -0.57%
+      * Actual P&L from Drift: ~$6.00 profit
+      * Database recorded: $92.46 profit (15.4× too high)
+      * Rate limiting caused 15+ detection cycles before trade removal
+      * Each cycle compounded: $6 → $12 → $24 → $48 → $96
+    - **Bug mechanism (line 799 in position-manager.ts):**
+    ```typescript
+    // BROKEN CODE:
+    const previouslyRealized = trade.realizedPnL  // Gets from mutated in-memory object
+    const totalRealizedPnL = previouslyRealized + runnerRealized
+    trade.realizedPnL = totalRealizedPnL  // ← BUG: Mutates in-memory trade object
+    
+    // Next monitoring cycle (2 seconds later):
+    const previouslyRealized = trade.realizedPnL  // ← Gets ACCUMULATED value from previous cycle
+    const totalRealizedPnL = previouslyRealized + runnerRealized  // ← Adds it AGAIN
+    trade.realizedPnL = totalRealizedPnL  // ← Compounds further
+    // Repeats 15-20 times before activeTrades.delete() removes trade
+    ```
+    - **Why Common Pitfall #48 didn't prevent this:**
+      * `closingInProgress` flag only applies when Position Manager initiates the close
+      * External closures (on-chain TP/SL orders) don't set this flag
+      * External closure detection runs in monitoring loop WITHOUT closingInProgress protection
+      * Rate limiting delays cause monitoring loop to detect closure multiple times
+    - **Fix:**
+    ```typescript
+    // CORRECT CODE (line 798):
+    const previouslyRealized = trade.realizedPnL  // Get original value from DB
+    const totalRealizedPnL = previouslyRealized + runnerRealized
+    // DON'T mutate trade.realizedPnL here - causes compounding on re-detection!
+    // trade.realizedPnL = totalRealizedPnL  ← REMOVED
+    console.log(`   Realized P&L calculation → Previous: $${previouslyRealized.toFixed(2)} | Runner: $${runnerRealized.toFixed(2)} ... | Total: $${totalRealizedPnL.toFixed(2)}`)
+    
+    // Later in same function (line 850):
+    await updateTradeExit({
+      realizedPnL: totalRealizedPnL,  // Use local variable for DB update
+      // ... other fields
+    })
+    ```
+    - **Impact:** Every external closure (on-chain TP/SL fills) affected, especially with rate limiting
+    - **Database correction:** Manual UPDATE required for trades with inflated P&L
+    - **Verification:** Check that updateTradeExit uses `totalRealizedPnL` (local variable) not `trade.realizedPnL` (mutated field)
+    - **Why activeTrades.delete() before DB update didn't help:**
+      * That fix (Common Pitfall #27) prevents duplicates AFTER database update completes
+      * But external closure detection calculates P&L BEFORE calling activeTrades.delete()
+      * If rate limits delay the detection→delete cycle, monitoring loop runs detection multiple times
+      * Each time, it mutates trade.realizedPnL before checking if trade already removed
+    - **Git commit:** 6156c0f "critical: Fix P&L compounding bug in external closure detection"
+    - **Related bugs:**
+      * Common Pitfall #27: Duplicate external closure updates (fixed by delete before DB update)
+      * Common Pitfall #48: P&L compounding during close verification (fixed by closingInProgress flag)
+      * This bug (#49): P&L compounding in external closure detection (fixed by not mutating trade.realizedPnL)
+    - **Lesson:** In monitoring loops that run repeatedly, NEVER mutate shared state during calculation phases. Calculate locally, update shared state ONCE at the end. Immutability prevents compounding bugs in retry/race scenarios.
+
 46. **100% position sizing causes InsufficientCollateral (Fixed Nov 16, 2025):**
     - **Symptom:** Bot configured for 100% position size gets InsufficientCollateral errors, but Drift UI can open same size position
     - **Root Cause:** Drift's margin calculation includes fees, slippage buffers, and rounding - exact 100% leaves no room