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.
This commit is contained in:
74
.github/copilot-instructions.md
vendored
74
.github/copilot-instructions.md
vendored
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user