critical: Fix MAE/MFE storing percentages instead of dollars + duplicate Telegram notifications
CRITICAL BUGS FIXED (Nov 19, 2025): 1. MAE/MFE Bug: - Was storing: account percentage (profit % × leverage) - Example: 1.31% move × 15x = 19.73% stored as MFE - Should store: actual dollar P&L (81 not 19.73%) - Impact: Telegram shows 'Max Gain: +19.73%' instead of '+.XX' - Fix: Changed from accountPnL (leverage-adjusted %) to currentPnLDollars - Lines 964-987: Removed accountPnL calculation, use currentPnLDollars 2. Duplicate Notification Bug: - handleExternalClosure() was checking if trade removed AFTER removal - Result: 16 duplicate Telegram notifications with compounding P&L - Example: 6 → 2 → 11 → ... → 81 (16 notifications for 1 close) - Fix: Check if trade already removed BEFORE processing - Lines 382-391: Move duplicate check to START of function - Early return prevents notification send if already processed 3. Database Compounding (NOT A BUG): - Nov 17 fix (Common Pitfall #49) still working correctly - Only 1 database record with 81 P&L - Issue was notification duplication, not DB duplication IMPACT: - MAE/MFE data now usable for TP/SL optimization - Telegram notifications accurate (1 per close, correct P&L) - Database analytics will show real dollar movements - Next trade will have correct Max Gain/Drawdown display FILES: - lib/trading/position-manager.ts: MAE/MFE calculation + duplicate check
This commit is contained in:
2
.env
2
.env
@@ -406,7 +406,7 @@ MAX_SCALE_MULTIPLIER=2
|
|||||||
SCALE_SIZE_PERCENT=50
|
SCALE_SIZE_PERCENT=50
|
||||||
MIN_ADX_INCREASE=5
|
MIN_ADX_INCREASE=5
|
||||||
MAX_PRICE_POSITION_FOR_SCALE=70
|
MAX_PRICE_POSITION_FOR_SCALE=70
|
||||||
TRAILING_STOP_ATR_MULTIPLIER=1.8
|
TRAILING_STOP_ATR_MULTIPLIER=2.5
|
||||||
TRAILING_STOP_MIN_PERCENT=0.25
|
TRAILING_STOP_MIN_PERCENT=0.25
|
||||||
TRAILING_STOP_MAX_PERCENT=2.5
|
TRAILING_STOP_MAX_PERCENT=2.5
|
||||||
USE_PERCENTAGE_SIZE=false
|
USE_PERCENTAGE_SIZE=false
|
||||||
|
|||||||
@@ -379,6 +379,14 @@ export class PositionManager {
|
|||||||
private async handleExternalClosure(trade: ActiveTrade, reason: string): Promise<void> {
|
private async handleExternalClosure(trade: ActiveTrade, reason: string): Promise<void> {
|
||||||
console.log(`🧹 Handling external closure: ${trade.symbol} (${reason})`)
|
console.log(`🧹 Handling external closure: ${trade.symbol} (${reason})`)
|
||||||
|
|
||||||
|
// CRITICAL: Check if already processed to prevent duplicate notifications
|
||||||
|
const tradeId = trade.id
|
||||||
|
if (!this.activeTrades.has(tradeId)) {
|
||||||
|
console.log(`⚠️ DUPLICATE PREVENTED: Trade ${tradeId} already processed, skipping`)
|
||||||
|
console.log(` This prevents duplicate Telegram notifications with compounding P&L`)
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
// CRITICAL: Calculate P&L using originalPositionSize for accuracy
|
// CRITICAL: Calculate P&L using originalPositionSize for accuracy
|
||||||
// currentSize may be stale if Drift propagation was interrupted
|
// currentSize may be stale if Drift propagation was interrupted
|
||||||
const profitPercent = this.calculateProfitPercent(
|
const profitPercent = this.calculateProfitPercent(
|
||||||
@@ -391,13 +399,7 @@ export class PositionManager {
|
|||||||
|
|
||||||
console.log(`💰 Estimated P&L: ${profitPercent.toFixed(2)}% on $${sizeForPnL.toFixed(2)} → $${estimatedPnL.toFixed(2)}`)
|
console.log(`💰 Estimated P&L: ${profitPercent.toFixed(2)}% on $${sizeForPnL.toFixed(2)} → $${estimatedPnL.toFixed(2)}`)
|
||||||
|
|
||||||
// Remove from monitoring FIRST to prevent race conditions
|
// Remove from monitoring IMMEDIATELY to prevent race conditions
|
||||||
const tradeId = trade.id
|
|
||||||
if (!this.activeTrades.has(tradeId)) {
|
|
||||||
console.log(`⚠️ Trade ${tradeId} already removed, skipping cleanup`)
|
|
||||||
return
|
|
||||||
}
|
|
||||||
|
|
||||||
this.activeTrades.delete(tradeId)
|
this.activeTrades.delete(tradeId)
|
||||||
console.log(`🗑️ Removed ${trade.symbol} from monitoring`)
|
console.log(`🗑️ Removed ${trade.symbol} from monitoring`)
|
||||||
|
|
||||||
@@ -968,21 +970,22 @@ export class PositionManager {
|
|||||||
trade.direction
|
trade.direction
|
||||||
)
|
)
|
||||||
|
|
||||||
const accountPnL = profitPercent * trade.leverage
|
const currentPnLDollars = (trade.currentSize * profitPercent) / 100
|
||||||
trade.unrealizedPnL = (trade.currentSize * profitPercent) / 100
|
trade.unrealizedPnL = currentPnLDollars
|
||||||
|
|
||||||
// Track peak P&L (MFE - Maximum Favorable Excursion)
|
// Track peak P&L (MFE - Maximum Favorable Excursion)
|
||||||
if (trade.unrealizedPnL > trade.peakPnL) {
|
if (trade.unrealizedPnL > trade.peakPnL) {
|
||||||
trade.peakPnL = trade.unrealizedPnL
|
trade.peakPnL = trade.unrealizedPnL
|
||||||
}
|
}
|
||||||
|
|
||||||
// Track MAE/MFE (account percentage, not USD)
|
// Track MAE/MFE in DOLLAR amounts (not percentages!)
|
||||||
if (accountPnL > trade.maxFavorableExcursion) {
|
// CRITICAL: Database schema expects DOLLARS for analysis and TP/SL optimization
|
||||||
trade.maxFavorableExcursion = accountPnL
|
if (currentPnLDollars > trade.maxFavorableExcursion) {
|
||||||
|
trade.maxFavorableExcursion = currentPnLDollars
|
||||||
trade.maxFavorablePrice = currentPrice
|
trade.maxFavorablePrice = currentPrice
|
||||||
}
|
}
|
||||||
if (accountPnL < trade.maxAdverseExcursion) {
|
if (currentPnLDollars < trade.maxAdverseExcursion) {
|
||||||
trade.maxAdverseExcursion = accountPnL
|
trade.maxAdverseExcursion = currentPnLDollars
|
||||||
trade.maxAdversePrice = currentPrice
|
trade.maxAdversePrice = currentPrice
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user