Add v7-momentum indicator (experimental, disabled)
- Created momentum scalper indicator for catching rapid price acceleration - ROC-based detection: 2.0% threshold over 5 bars - Volume confirmation: 2.0x spike (checks last 3 bars) - ADX filter: Requires 12+ minimum directional movement - Anti-chop filter: Blocks signals in dead markets - Debug table: Real-time metric display for troubleshooting Status: Functional but signal quality inferior to v6 HalfTrend Decision: Shelved for now, continue with proven v6 strategy File: docs/guides/MOMENTUM_INDICATOR_V1.pine (239 lines) Lessons learned: - Momentum indicators inherently noisy (40-50% WR expected) - Signals either too early (false breakouts) or too late (miss move) - Volume spike timing issue: Often lags price move by 1-2 bars - Better to optimize proven strategy than add complexity Related: Position Manager duplicate update bug fixed (awaiting verification)
This commit is contained in:
30
.github/copilot-instructions.md
vendored
30
.github/copilot-instructions.md
vendored
@@ -888,6 +888,36 @@ trade.realizedPnL += actualRealizedPnL // NOT: result.realizedPnL from SDK
|
||||
- **v5:** Buy/Sell Signal strategy (pre-Nov 12)
|
||||
- **v6:** HalfTrend + BarColor strategy (Nov 12+)
|
||||
- Used for performance comparison between strategies
|
||||
|
||||
26. **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
|
||||
- **Bug sequence:**
|
||||
1. Trade closed externally (on-chain SL order fills at -$7.98)
|
||||
2. Position Manager detects closure: `position === null`
|
||||
3. Calculates P&L and calls `updateTradeExit()` → -$7.50 in DB
|
||||
4. **BUT:** Trade still in `activeTrades` Map (removal happens after DB update)
|
||||
5. Next monitoring loop (2s later) detects closure AGAIN
|
||||
6. Accumulates P&L: `previouslyRealized (-$7.50) + runnerRealized (-$7.50) = -$15.00`
|
||||
7. Updates database AGAIN → -$15.00 in DB
|
||||
8. Repeats 8 times → final -$58.43 (8× the actual loss)
|
||||
- **Fix:** Remove trade from `activeTrades` Map BEFORE database update:
|
||||
```typescript
|
||||
// BEFORE (BROKEN):
|
||||
await updateTradeExit({ ... })
|
||||
await this.removeTrade(trade.id) // Too late! Loop already ran again
|
||||
|
||||
// AFTER (FIXED):
|
||||
this.activeTrades.delete(trade.id) // Remove FIRST
|
||||
await updateTradeExit({ ... }) // Then update DB
|
||||
if (this.activeTrades.size === 0) {
|
||||
this.stopMonitoring()
|
||||
}
|
||||
```
|
||||
- **Impact:** Without this fix, every external closure is recorded 5-8 times with compounding P&L
|
||||
- **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)
|
||||
- Analytics endpoint `/api/analytics/version-comparison` compares v5 vs v6 performance
|
||||
|
||||
|
||||
Reference in New Issue
Block a user