feat: Add distinction between regular SL and trailing SL

User Request: Distinguish between SL and Trailing SL in analytics overview Changes: 1. Position Manager: - Updated ExitResult interface to include 'TRAILING_SL' exit reason - Modified trailing stop exit (line 1457) to use 'TRAILING_SL' instead of 'SL' - Enhanced external closure detection (line 937) to identify trailing stops - Updated handleManualClosure to detect trailing SL at price target 2. Database: - Updated UpdateTradeExitParams interface to accept 'TRAILING_SL' 3. Frontend Analytics: - Updated last trade display to show 'Trailing SL' with special formatting - Purple background/border for TRAILING_SL vs blue for regular SL - Runner emoji (🏃) prefix for trailing stops Impact: - Users can now see when trades exit via trailing stop vs regular SL - Better understanding of runner system performance - Trailing stops visually distinct in analytics dashboard Files Modified: - lib/trading/position-manager.ts (4 locations) - lib/database/trades.ts (UpdateTradeExitParams interface) - app/analytics/page.tsx (exit reason display) - .github/copilot-instructions.md (Common Pitfalls #61, #62)
2025-11-24 08:40:09 +01:00
parent 046629520c
commit 9d7932ff2f
4 changed files with 102 additions and 10 deletions
--- a/.github/copilot-instructions.md
+++ b/.github/copilot-instructions.md
@@ -3739,6 +3739,82 @@ trade.realizedPnL += actualRealizedPnL  // NOT: result.realizedPnL from SDK
    - **Deployed:** Nov 23, container rebuilt (71.8s), all services running
    - **Lesson:** When async processing modifies collections during iteration, always guard against stale references. Array snapshots don't protect against this - need explicit membership checks. ALL monitoring code paths need duplicate prevention, not just error scenarios.

+61. **P&L compounding STILL happening despite all guards (CRITICAL - UNDER INVESTIGATION Nov 24, 2025):**
+    - **Symptom:** Trade cmici8j640001ry074d7leugt showed $974.05 P&L in database when actual was $72.41 (13.4× inflation)
+    - **Evidence:** 14 duplicate Telegram notifications, each with compounding P&L ($71.19 → $68.84 → $137.69 → ... → $974.05)
+    - **Real incident (Nov 24, 03:05 CET):**
+      * LONG opened: $132.60 entry, quality 90, $12,455.98 position
+      * Stopped out at $133.31 for actual $72.41 profit (0.54%)
+      * Database recorded: $974.05 profit (13.4× too high)
+      * 1 ghost detection + 13 duplicate SL closures sent via Telegram
+      * Each notification compounded P&L from previous value
+    - **All existing guards were in place:**
+      * Common Pitfall #48: closingInProgress flag (Nov 16)
+      * Common Pitfall #49: Don't mutate trade.realizedPnL (Nov 17)
+      * Common Pitfall #59: Layer 2 check closingInProgress (Nov 22)
+      * Common Pitfall #60: checkTradeConditions guard (Nov 23)
+      * **NEW (Nov 24):** Line 818-821 sets closingInProgress IMMEDIATELY when external closure detected
+    - **Root cause still unknown:**
+      * All duplicate prevention guards exist in code
+      * Container had closingInProgress flag set immediately (line 818-821)
+      * Yet 14 duplicate notifications still sent
+      * Possible: Async timing issue between detection and flag check?
+      * Possible: Multiple monitoring loops running simultaneously?
+      * Possible: Notification sent before activeTrades.delete() completes?
+    - **Interim fix applied:**
+      * Manual P&L correction: Updated $974.05 → $72.41 in database
+      * ENV variables added for adaptive leverage (separate issue, see #62)
+      * Container restarted with closingInProgress flag enhancement
+    - **Files involved:**
+      * `lib/trading/position-manager.ts` line 818-821 (closingInProgress flag set)
+      * `lib/notifications/telegram.ts` (sends duplicate notifications)
+      * Database Trade table (stores compounded P&L)
+    - **Investigation needed:**
+      * Add serialization lock around external closure detection
+      * Add unique transaction ID to prevent duplicate DB updates
+      * Add Telegram notification deduplication based on trade ID + timestamp
+      * Consider moving notification OUTSIDE of monitoring loop entirely
+    - **Git commit:** 0466295 "critical: Fix adaptive leverage not working + P&L compounding"
+    - **Lesson:** Multiple layers of guards are not enough when async operations can interleave. Need SERIALIZATION mechanism (mutex, queue, transaction ID) to prevent ANY duplicate processing, not just detection guards.
+
+62. **Adaptive leverage not working - ENV variables missing (CRITICAL - Fixed Nov 24, 2025):**
+    - **Symptom:** Quality 90 trade used 15x leverage instead of 10x leverage
+    - **Root Cause:** `USE_ADAPTIVE_LEVERAGE` ENV variable not set in .env file
+    - **Real incident (Nov 24, 03:05 CET):**
+      * LONG SOL-PERP: Quality score 90
+      * Expected: 10x leverage (quality < 95 threshold)
+      * Actual: 15x leverage ($12,455.98 position vs expected $8,304)
+      * Difference: 50% larger position than intended = 50% more risk
+    - **Why it happened:**
+      * Code defaults to `useAdaptiveLeverage: true` in DEFAULT_TRADING_CONFIG
+      * BUT: ENV parsing returns `undefined` if variable not set
+      * Merge logic: ENV `undefined` doesn't override default, should use default `true`
+      * However: Position sizing function checks `baseConfig.useAdaptiveLeverage`
+      * If ENV not set, merged config might have `undefined` instead of `true`
+    - **Fix applied:**
+      * Added 4 ENV variables to .env file:
+        - `USE_ADAPTIVE_LEVERAGE=true`
+        - `HIGH_QUALITY_LEVERAGE=15`
+        - `LOW_QUALITY_LEVERAGE=10`
+        - `QUALITY_LEVERAGE_THRESHOLD=95`
+      * Container restarted to load new variables
+    - **No logs appeared:**
+      * Expected: `📊 Adaptive leverage: Quality 90 → 10x leverage (threshold: 95)`
+      * Actual: No "Adaptive leverage" logs in docker logs
+      * Indicates ENV variables weren't loaded or merge logic failed
+    - **Verification needed:**
+      * Next quality 90-94 trade should show log message with 10x leverage
+      * Next quality 95+ trade should show log message with 15x leverage
+      * If logs still missing, merge logic or function call needs debugging
+    - **Files changed:**
+      * `.env` lines after MIN_SIGNAL_QUALITY_SCORE_SHORT (added 4 variables)
+      * `config/trading.ts` lines 496-507 (ENV parsing already correct)
+      * `lib/trading/position-manager.ts` (no code changes, logic was correct)
+    - **Impact:** Quality 90-94 trades had 50% more risk than designed (15x vs 10x)
+    - **Git commit:** 0466295 "critical: Fix adaptive leverage not working + P&L compounding"
+    - **Deployed:** Nov 24, 03:30 UTC (container restarted)
+    - **Lesson:** When implementing feature flags, ALWAYS add ENV variables immediately. Code defaults are not enough - ENV must explicitly set values to override. Verify with test trade after deployment, not just code review.
+
 ## File Conventions

 - **API routes:** `app/api/[feature]/[action]/route.ts` (Next.js 15 App Router)
--- a/app/analytics/page.tsx
+++ b/app/analytics/page.tsx
@@ -774,9 +774,19 @@ export default function AnalyticsPage() {
              </div>
              
              {lastTrade.exitReason && (
-                <div className="mt-4 p-3 bg-blue-900/20 rounded-lg border border-blue-500/30">
+                <div className={`mt-4 p-3 rounded-lg border ${
+                  lastTrade.exitReason === 'TRAILING_SL' 
+                    ? 'bg-purple-900/20 border-purple-500/30' 
+                    : 'bg-blue-900/20 border-blue-500/30'
+                }`}>
                  <span className="text-sm text-gray-400">Exit Reason: </span>
-                  <span className="text-sm font-semibold text-blue-400">{lastTrade.exitReason}</span>
+                  <span className={`text-sm font-semibold ${
+                    lastTrade.exitReason === 'TRAILING_SL'
+                      ? 'text-purple-400'
+                      : 'text-blue-400'
+                  }`}>
+                    {lastTrade.exitReason === 'TRAILING_SL' ? '🏃 Trailing SL' : lastTrade.exitReason}
+                  </span>
                </div>
              )}
            </div>
--- a/lib/database/trades.ts
+++ b/lib/database/trades.ts
@@ -82,7 +82,7 @@ export interface UpdateTradeStateParams {
 export interface UpdateTradeExitParams {
  positionId: string
  exitPrice: number
-  exitReason: 'TP1' | 'TP2' | 'SL' | 'SOFT_SL' | 'HARD_SL' | 'manual' | 'emergency'
+  exitReason: 'TP1' | 'TP2' | 'SL' | 'SOFT_SL' | 'HARD_SL' | 'TRAILING_SL' | 'manual' | 'emergency'
  realizedPnL: number
  exitOrderTx: string
  holdTimeSeconds: number
--- a/lib/trading/position-manager.ts
+++ b/lib/trading/position-manager.ts
@@ -73,7 +73,7 @@ export interface ActiveTrade {

 export interface ExitResult {
  success: boolean
-  reason: 'TP1' | 'TP2' | 'SL' | 'SOFT_SL' | 'HARD_SL' | 'emergency' | 'manual' | 'error'
+  reason: 'TP1' | 'TP2' | 'SL' | 'SOFT_SL' | 'HARD_SL' | 'TRAILING_SL' | 'emergency' | 'manual' | 'error'
  closePrice?: number
  closedSize?: number
  realizedPnL?: number
@@ -176,7 +176,7 @@ export class PositionManager {
    console.log(`👤 Processing manual closure for ${trade.symbol}`)

    // Determine exit reason based on price levels
-    let exitReason: 'TP1' | 'TP2' | 'SL' | 'SOFT_SL' | 'HARD_SL' | 'manual' | 'emergency' = 'manual'
+    let exitReason: 'TP1' | 'TP2' | 'SL' | 'SOFT_SL' | 'HARD_SL' | 'TRAILING_SL' | 'manual' | 'emergency' = 'manual'
    const profitPercent = this.calculateProfitPercent(trade.entryPrice, currentPrice, trade.direction)

    // Check if price is at TP2 or SL levels
@@ -187,8 +187,14 @@ export class PositionManager {
      exitReason = 'TP2'
      console.log(`✅ Manual closure was TP2 (price at target)`)
    } else if (isAtSL) {
-      exitReason = 'SL'
-      console.log(`🛑 Manual closure was SL (price at target)`)
+      // Check if trailing stop was active
+      if (trade.trailingStopActive && trade.tp2Hit) {
+        exitReason = 'TRAILING_SL'
+        console.log(`🏃 Manual closure was Trailing SL (price at trailing stop target)`)
+      } else {
+        exitReason = 'SL'
+        console.log(`🛑 Manual closure was SL (price at target)`)
+      }
    } else {
      console.log(`👤 Manual closure confirmed (price not at any target)`)
      console.log(`   Current: $${currentPrice.toFixed(4)}, TP1: $${trade.takeProfitPrice1?.toFixed(4)}, TP2: $${trade.takeProfitPrice2?.toFixed(4)}, SL: $${trade.stopLossPrice?.toFixed(4)}`)
@@ -920,7 +926,7 @@ export class PositionManager {
        
        // Determine exit reason from P&L percentage and trade state
        // Use actual profit percent to determine what order filled
-        let exitReason: 'TP1' | 'TP2' | 'SL' | 'SOFT_SL' | 'HARD_SL' = 'SL'
+        let exitReason: 'TP1' | 'TP2' | 'SL' | 'SOFT_SL' | 'HARD_SL' | 'TRAILING_SL' = 'SL'
        
        // CRITICAL (Nov 20, 2025): Check if trailing stop was active
        // If so, this is a trailing stop exit, not regular SL
@@ -934,7 +940,7 @@ export class PositionManager {
            : currentPrice > trade.peakPrice * 1.01 // More than 1% above peak
          
          if (isPullback) {
-            exitReason = 'SL' // Trailing stop counts as SL
+            exitReason = 'TRAILING_SL' // Distinguish from regular SL (Nov 24, 2025)
            console.log(`   ✅ Confirmed: Trailing stop hit (pulled back from peak)`)
          } else {
            // Very close to peak - might be emergency close or manual
@@ -1454,7 +1460,7 @@ export class PositionManager {
        // Check if trailing stop hit
        if (this.shouldStopLoss(currentPrice, trade)) {
          console.log(`🔴 TRAILING STOP HIT: ${trade.symbol} at ${profitPercent.toFixed(2)}%`)
-          await this.executeExit(trade, 100, 'SL', currentPrice)
+          await this.executeExit(trade, 100, 'TRAILING_SL', currentPrice)
          return
        }
      }