From 96f5cfae770204f6186d24e53276ecefe9f16392 Mon Sep 17 00:00:00 2001
From: mindesbunister <github_service@egonetix.de>
Date: Wed, 19 Nov 2025 09:09:53 +0100
Subject: [PATCH] docs: Add Common Pitfall #51 - TP1 detection for fast
 on-chain fills

Documents the TP1 detection bug where on-chain limit orders fill faster
than Position Manager monitoring loop can detect. When both TP1 (75%)
and runner (25%) close before next monitoring cycle, PM doesn't know
TP1 filled first, marks entire closure as 'SL' instead of 'TP1'.

Fix uses profit percentage thresholds instead of state flags:
- >1.2%: TP2 range
- 0.3-1.2%: TP1 range
- <0.3%: SL range

Simpler and more reliable than tracking order fill state.
---
 .github/copilot-instructions.md | 74 +++++++++++++++++++++++++++++++++
 1 file changed, 74 insertions(+)

diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md
index f068399..acc2a2b 100644
--- a/.github/copilot-instructions.md
+++ b/.github/copilot-instructions.md
@@ -2349,6 +2349,80 @@ trade.realizedPnL += actualRealizedPnL  // NOT: result.realizedPnL from SDK
       * All future trades will track accurately
     - **Lesson:** In-memory state can accumulate stale values across detection cycles. For financial calculations, always use fresh data or calculate from immutable source values (entry/exit prices), never from potentially mutated in-memory fields.
 
+51. **TP1 detection fails when on-chain orders fill fast (CRITICAL - Fixed Nov 19, 2025):**
+    - **Symptom:** TP1 order fills on-chain (75% of position), but database records exitReason as "SL" instead of "TP1"
+    - **Root Cause:** Position Manager monitoring loop detects closure AFTER both TP1 and runner already closed on-chain, can't determine TP1 triggered first
+    - **Real incident (Nov 19, 07:40 UTC / 08:40 CET):**
+      * LONG opened: $140.1735 entry, $7788 position
+      * TP1 order placed: $140.8743 for 75% of position
+      * TP1 filled on-chain within seconds
+      * Runner (25%) also closed within 7 minutes
+      * Position Manager detected external closure with entire position gone
+      * `trade.tp1Hit = false` (never updated when TP1 filled)
+      * Treated as full position SL close instead of TP1 + runner
+      * Database: exitReason="SL", but actual profit $37.67 (0.48%, clearly TP1 range!)
+    - **Why it happens:**
+      * On-chain limit orders fill independently of Position Manager monitoring
+      * TP1 fills in <1 second when price crosses
+      * Runner closes via SL or trailing stop (also fast)
+      * Position Manager monitoring loop runs every 2 seconds
+      * By time PM checks, entire position gone - can't tell TP1 filled first
+      * tp1Hit flag only updates when PM directly closes position, not for external fills
+    - **Impact:** 
+      * Analytics shows wrong exit reason distribution (too many "SL", missing "TP1" wins)
+      * Can't accurately assess TP1 effectiveness
+      * Profit attribution incorrect (winners marked as losers)
+    - **Fix Attempts:**
+      1. Query Drift order history - FAILED (SDK doesn't expose simple API)
+      2. Infer from P&L magnitude ratio - FAILED (too complex, unreliable)
+      3. Simple percentage-based thresholds - DEPLOYED ✅
+    - **Final Fix (Nov 19, 08:08 UTC):**
+    ```typescript
+    // In lib/trading/position-manager.ts lines 760-835
+    
+    // Always use full position for P&L calculation
+    const sizeForPnL = trade.originalPositionSize
+    
+    // Calculate profit percentage
+    const runnerProfitPercent = this.calculateProfitPercent(
+      trade.entryPrice, 
+      currentPrice, 
+      trade.direction
+    )
+    
+    // Simple percentage-based exit reason
+    let exitReason: string
+    if (runnerProfitPercent > 0.3) {
+      if (runnerProfitPercent >= 1.2) {
+        exitReason = 'TP2'  // Large profit (>1.2%)
+      } else {
+        exitReason = 'TP1'  // Moderate profit (0.3-1.2%)
+      }
+    } else {
+      exitReason = 'SL'  // Negative or tiny profit (<0.3%)
+    }
+    ```
+    - **Threshold Rationale:**
+      * TP1 typically placed at ~0.86% (ATR × 2.0)
+      * Profit 0.3-1.2% strongly indicates TP1 filled
+      * Profit >1.2% indicates TP2 range
+      * Profit <0.3% is SL/breakeven territory
+    - **Removed Logic:**
+      * Complex tp1Hit flag inference
+      * Drift order history querying (doesn't exist)
+      * P&L ratio calculations (unreliable)
+    - **Benefits:**
+      * Simple, reliable, based on actual results
+      * Works regardless of how position closed
+      * No dependency on Drift SDK order APIs
+      * More accurate than state flag tracking for external fills
+    - **Verification Required:**
+      * Next trade that hits TP1 should show exitReason="TP1"
+      * Profit in 0.3-1.2% range should categorize correctly
+      * Analytics exit reason distribution should match reality
+    - **Git commit:** de57c96 "fix: Correct TP1 detection for on-chain order fills"
+    - **Lesson:** When orders fill externally (on-chain), state flags (tp1Hit) unreliable. Infer exit reason from results (profit percentage) rather than process tracking. Simple logic often more reliable than complex state machines for fast-moving on-chain events.
+
 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