diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 04f2144..16fcef3 100644 --- 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) diff --git a/app/analytics/page.tsx b/app/analytics/page.tsx index 379b04a..2e8b132 100644 --- a/app/analytics/page.tsx +++ b/app/analytics/page.tsx @@ -774,9 +774,19 @@ export default function AnalyticsPage() { {lastTrade.exitReason && ( -