Files
trading_bot_v4/docs/bugs/CRITICAL_TP1_FALSE_DETECTION_BUG.md
mindesbunister 4c36fa2bc3 docs: Major documentation reorganization + ENV variable reference
**Documentation Structure:**
- Created docs/ subdirectory organization (analysis/, architecture/, bugs/,
  cluster/, deployments/, roadmaps/, setup/, archived/)
- Moved 68 root markdown files to appropriate categories
- Root directory now clean (only README.md remains)
- Total: 83 markdown files now organized by purpose

**New Content:**
- Added comprehensive Environment Variable Reference to copilot-instructions.md
- 100+ ENV variables documented with types, defaults, purpose, notes
- Organized by category: Required (Drift/RPC/Pyth), Trading Config (quality/
  leverage/sizing), ATR System, Runner System, Risk Limits, Notifications, etc.
- Includes usage examples (correct vs wrong patterns)

**File Distribution:**
- docs/analysis/ - Performance analyses, blocked signals, profit projections
- docs/architecture/ - Adaptive leverage, ATR trailing, indicator tracking
- docs/bugs/ - CRITICAL_*.md, FIXES_*.md bug reports (7 files)
- docs/cluster/ - EPYC setup, distributed computing docs (3 files)
- docs/deployments/ - *_COMPLETE.md, DEPLOYMENT_*.md status (12 files)
- docs/roadmaps/ - All *ROADMAP*.md strategic planning files (7 files)
- docs/setup/ - TradingView guides, signal quality, n8n setup (8 files)
- docs/archived/2025_pre_nov/ - Obsolete verification checklist (1 file)

**Key Improvements:**
- ENV variable reference: Single source of truth for all configuration
- Common Pitfalls #68-71: Already complete, verified during audit
- Better findability: Category-based navigation vs 68 files in root
- Preserves history: All files git mv (rename), not copy/delete
- Zero broken functionality: Only documentation moved, no code changes

**Verification:**
- 83 markdown files now in docs/ subdirectories
- Root directory cleaned: 68 files → 0 files (except README.md)
- Git history preserved for all moved files
- Container running: trading-bot-v4 (no restart needed)

**Next Steps:**
- Create README.md files in each docs subdirectory
- Add navigation index
- Update main README.md with new structure
- Consolidate duplicate deployment docs
- Archive truly obsolete files (old SQL backups)

See: docs/analysis/CLEANUP_PLAN.md for complete reorganization strategy
2025-12-04 08:29:59 +01:00

9.3 KiB

CRITICAL: TP1 False Detection Bug - Financial Loss Risk

Date: Nov 30, 2025 Severity: 🔴 CRITICAL - Causes premature order cancellation, lost profit potential Status: 🔧 FIXING NOW

Real Incident (Nov 30, 2025, 19:37-21:22 UTC)

Trade ID: cmim4ggkr00canv07pgve2to9 Symbol: SOL-PERP SHORT Entry: $137.76 Position: $89.10 TP1 Target: $137.07 (0.5%, 75% close) Actual Exit: $136.84 via external closure (on-chain TP1 order filled) P&L: $0.23 (should have been higher with proper order management)

The Bug Chain

1. False TP1 Detection (Position Manager lines 1002-1087)

Root Cause: Position Manager detects size mismatch and IMMEDIATELY sets trade.tp1Hit = true WITHOUT verifying price reached TP1 target.

Vulnerable Code:

// Line 1002-1087 in lib/trading/position-manager.ts
if (positionSizeUSD < trade.currentSize * 0.95) { // 5% tolerance
  console.log(`⚠️ Position size mismatch: expected $${trade.currentSize.toFixed(2)}, got $${positionSizeUSD.toFixed(2)}`)
  
  // ... various checks for signal flip, phantom trade, etc. ...
  
  // BUG: Sets tp1Hit = true based on SIZE MISMATCH, not PRICE TARGET
  trade.currentSize = positionSizeUSD
  trade.tp1Hit = true  // ← CRITICAL BUG: No price verification!
  await this.saveTradeState(trade)
}

Why This Happens:

  • On-chain TP1 order fills when price crosses $137.07
  • Drift position size reduces from $89.10 → ~$22 (75% closed)
  • Position Manager monitoring loop detects size mismatch
  • ASSUMES any size reduction = TP1 hit (WRONG!)
  • Could be partial fill due to slippage, could be external closure, could be anything

2. Premature Order Cancellation (Lines 1228-1284)

After setting trade.tp1Hit = true, Position Manager executes:

// Line 1228-1257: Cancel ALL orders on the symbol
console.log('🗑️ Cancelling old stop loss orders...')
const { cancelAllOrders, placeExitOrders } = await import('../drift/orders')
const cancelResult = await cancelAllOrders(trade.symbol)

// BUG: This cancels the on-chain TP1 order that's about to fill!

Consequence: On-chain TP1 limit order ($137.07) gets cancelled by Position Manager BEFORE it can fill at optimal price.

3. Wrong SL Replacement (Lines 1258-1284)

After cancelling orders, Position Manager places NEW orders:

// Place ONLY new SL orders at breakeven/profit level
const exitOrdersResult = await placeExitOrders({
  symbol: trade.symbol,
  positionSizeUSD: trade.currentSize,  // Runner size (~$22)
  stopLossPrice: newStopLossPrice,     // Breakeven or profit
  tp1SizePercent: 0,   // No TP1 order
  tp2SizePercent: 0,   // No TP2 order
  direction: trade.direction,
})

Consequence: System places NEW stop loss for runner position, but original TP1 order is GONE.

4. Container Restart Chaos

User reported: "we had development going on which meant restarting the bot several times"

What Happens on Restart:

  1. Container restarts
  2. Startup validation runs (lib/startup/init-position-manager.ts)
  3. Queries database for open trades
  4. Finds trade with tp1Hit = false (database not updated yet)
  5. Queries Drift position, sees reduced size
  6. Restores position tracking with WRONG assumptions
  7. Places ANOTHER set of orders (TP1 + SL)

Result: Ghost orders accumulate with each restart.

Impact Assessment

Financial Loss:

  • TP1 order cancelled prematurely
  • Price continues moving favorably
  • Position exits via ghost order or manual intervention
  • Profit left on table: Difference between optimal TP1 exit vs actual exit

System Integrity:

  • Multiple redundant orders on exchange
  • Confusion about which orders belong to which trade
  • Database state doesn't match reality
  • Ghost orders can trigger unintended fills

User Experience:

  • Only received 1 Telegram notification (TP1 partial close at $136.84)
  • No runner exit notification
  • /status command not working (separate Telegram bot issue)
  • User had to manually monitor position

The Correct Logic

TP1 should be detected via TWO conditions (AND, not OR):

  1. Position size reduced by ~75%
  2. Price crossed TP1 target ($137.07)

Current logic: Only checks (1) → FALSE POSITIVES Fixed logic: Must verify BOTH (1) AND (2) → TRUE POSITIVES ONLY

Telegram Bot /status Issue

Separate bug discovered: Telegram bot showing errors:

telegram.error.Conflict: Conflict: terminated by other getUpdates request; 
make sure that only one bot instance is running

Root Cause: Multiple Telegram bot instances running, conflict on getUpdates polling.

Container: telegram-trade-bot (up 4 days) Status: Network errors + Conflict errors in logs Impact: /status command not responding

Fixes Required

Fix #1: Add Price Verification to TP1 Detection

File: lib/trading/position-manager.ts Lines: 1002-1087 (size mismatch detection block)

BEFORE (BROKEN):

if (positionSizeUSD < trade.currentSize * 0.95) {
  // ... checks ...
  trade.currentSize = positionSizeUSD
  trade.tp1Hit = true  // ← BUG: No price check!
  await this.saveTradeState(trade)
}

AFTER (FIXED):

if (positionSizeUSD < trade.currentSize * 0.95) {
  console.log(`⚠️ Position size mismatch: expected $${trade.currentSize.toFixed(2)}, got $${positionSizeUSD.toFixed(2)}`)
  
  // CRITICAL FIX (Nov 30, 2025): Verify PRICE reached TP1 before setting flag
  // Size mismatch alone is NOT enough - could be partial fill, slippage, external action
  const tp1PriceReached = this.shouldTakeProfit1(currentPrice, trade)
  
  if (tp1PriceReached) {
    console.log(`✅ TP1 PRICE VERIFIED: Size mismatch + price target reached`)
    // Update current size to match reality (already in USD)
    trade.currentSize = positionSizeUSD
    trade.tp1Hit = true
    await this.saveTradeState(trade)
    
    // Trigger TP1 exit processing (move SL to breakeven, cancel old orders, etc.)
    console.log(`🎉 TP1 HIT: ${trade.symbol} via on-chain order (size reduction detected)`)
    // The normal TP1 processing will happen on next monitoring loop iteration
  } else {
    console.log(`⚠️ Size reduced but TP1 price NOT reached yet`)
    console.log(`   Current price: ${currentPrice.toFixed(4)}, TP1 target: ${trade.tp1Price.toFixed(4)}`)
    console.log(`   Keeping TP1 flag false - likely partial fill or external action`)
    
    // Update tracked size but DON'T trigger TP1 logic
    trade.currentSize = positionSizeUSD
    await this.saveTradeState(trade)
  }
  
  // ... rest of checks (signal flip, phantom, etc.) ...
}

Fix #2: Prevent Order Cancellation When TP1 Hit Externally

Problem: When TP1 fills on-chain, Position Manager shouldn't cancel orders immediately.

Solution: Add check before cancelling orders:

// In TP1 processing block (lines 1228-1257)
// BEFORE cancelling orders, check if TP1 was detected via size reduction
if (trade.tp1Hit && !trade.tp1Filled) {
  console.log(`⚠️ TP1 detected via size reduction but not filled by Position Manager`)
  console.log(`   This means on-chain TP1 order already filled`)
  console.log(`   Skipping order cancellation - no need to cancel already-filled orders`)
  
  // Update database to reflect on-chain fill
  trade.tp1Filled = true
  await this.saveTradeState(trade)
  return // Don't cancel/replace orders
}

Fix #3: Restart Telegram Bot

Action: Restart telegram-trade-bot container to fix /status command

docker restart telegram-trade-bot

Fix #4: Add Better Logging

Add to Position Manager monitoring:

console.log(`📊 Position check: Drift=$${positionSizeUSD.toFixed(2)} Tracked=$${trade.currentSize.toFixed(2)} ` +
            `TP1Hit=${trade.tp1Hit} TP1Price=$${trade.tp1Price.toFixed(4)} CurrentPrice=$${currentPrice.toFixed(4)}`)

Verification Steps

  1. Check database for recent trade details
  2. Review Docker logs for TP1 detection sequence
  3. Identify premature order cancellation
  4. Understand restart behavior
  5. 🔧 Implement Fix #1 (price verification)
  6. 🔧 Implement Fix #2 (prevent premature cancellation)
  7. 🔧 Restart Telegram bot
  8. 🔧 Docker rebuild and restart
  9. Execute test trade to verify fix
  10. Monitor logs for correct TP1 detection
  11. Verify orders remain active until proper fill

Prevention for Future

Add to Common Pitfalls section:

  • #63: TP1 False Detection via Size Mismatch (Nov 30, 2025)
    • Symptom: System cancels TP1 orders prematurely
    • Root cause: Size reduction assumed to mean TP1 hit, no price verification
    • Fix: ALWAYS verify BOTH size reduction AND price target reached
    • Impact: Lost profit potential from premature exits
    • Detection: Log shows "TP1 hit: true" but price never reached TP1 target
  • Telegram bot /status not working (separate fix needed)
  • Container restarts during active trades cause order duplication
  • Need better coordination between on-chain orders and Position Manager software monitoring

User Communication

Immediate actions:

  1. Explain the bug chain clearly
  2. Show evidence from logs and database
  3. Outline the fix strategy
  4. Estimate deployment timeline
  5. Test thoroughly before declaring "fixed"

Follow-up:

  1. Document in copilot-instructions.md
  2. Add to Common Pitfalls with full incident details
  3. Update Position Manager verification procedures
  4. Consider adding "order already filled" detection