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