From 7833686b7bf63f26afcd3babbcb42af560a84a0e Mon Sep 17 00:00:00 2001
From: mindesbunister <github_service@egonetix.de>
Date: Wed, 19 Nov 2025 08:41:10 +0100
Subject: [PATCH] critical: Fix P&L compounding in external closure detection

Root cause: trade.realizedPnL was reading from in-memory ActiveTrade object
which could have stale/mutated values from previous detection cycles.

Bug sequence:
1. External closure detected, calculates P&L including previouslyRealized
2. Updates database with totalRealizedPnL
3. Same closure detected again (due to race condition or rate limits)
4. Reads previouslyRealized from same in-memory object (now has accumulated value)
5. Adds MORE P&L to it, compounds 2x, 5x, 10x

Real impact: 9 expected P&L became 81 (10x inflation)

Fix: Remove previouslyRealized from calculation entirely for external closures.
External closures calculate ONLY the current position P&L, not cumulative.
Database will have correct cumulative value if TP1 was processed separately.

Lines changed: lib/trading/position-manager.ts:785-803
- Removed: const previouslyRealized = trade.realizedPnL
- Removed: previouslyRealized + runnerRealized
- Now: totalRealizedPnL = runnerRealized (ONLY this closure's P&L)

Tested: Build completed successfully, container deployed and monitoring positions
---
 lib/trading/position-manager.ts | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/lib/trading/position-manager.ts b/lib/trading/position-manager.ts
index 47e7863..4c2f815 100644
--- a/lib/trading/position-manager.ts
+++ b/lib/trading/position-manager.ts
@@ -782,10 +782,10 @@ export class PositionManager {
         // CRITICAL: Use trade state flags, not current price (on-chain orders filled in the past!)
         let exitReason: 'TP1' | 'TP2' | 'SL' | 'SOFT_SL' | 'HARD_SL' = 'SL'
         
-        // Include any previously realized profit (e.g., from TP1 partial close)
-        // CRITICAL: Get from original database value, not mutated in-memory value
-        const previouslyRealized = trade.realizedPnL
-        
+        // CRITICAL: Calculate P&L for THIS close only, do NOT add previouslyRealized
+        // Previous bug: Added trade.realizedPnL which could be from prior detection cycles
+        // This caused 10x P&L inflation when same trade detected multiple times
+        // FIX: Calculate ONLY the runner P&L for this specific closure
         let runnerRealized = 0
         let runnerProfitPercent = 0
         if (!wasPhantom) {
@@ -797,10 +797,13 @@ export class PositionManager {
           runnerRealized = (sizeForPnL * runnerProfitPercent) / 100
         }
         
-        const totalRealizedPnL = previouslyRealized + runnerRealized
-        // DON'T mutate trade.realizedPnL here - causes compounding on re-detection!
-        // trade.realizedPnL = totalRealizedPnL  ← REMOVED: Causes duplicate P&L bug
-        console.log(`   Realized P&L calculation → Previous: $${previouslyRealized.toFixed(2)} | Runner: $${runnerRealized.toFixed(2)} (Δ${runnerProfitPercent.toFixed(2)}%) on $${sizeForPnL.toFixed(2)} | Total: $${totalRealizedPnL.toFixed(2)}`)
+        // For external closures, we DON'T know if TP1 already hit, so calculate full position P&L
+        // Database will have correct previouslyRealized if TP1 was hit
+        // For external closures, we DON'T know if TP1 already hit, so calculate full position P&L
+        // Database will have correct previouslyRealized if TP1 was hit
+        const totalRealizedPnL = runnerRealized
+        console.log(`   External closure P&L → ${runnerProfitPercent.toFixed(2)}% on $${sizeForPnL.toFixed(2)} = $${totalRealizedPnL.toFixed(2)}`)
+
         
         // Determine exit reason from trade state and P&L
         if (trade.tp2Hit) {