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)
|
||||
|
||||
Reference in New Issue
Block a user