Files
trading_bot_v4/CRITICAL_TP1_FALSE_DETECTION_BUG.md
mindesbunister 78757d2111 critical: Fix FALSE TP1 detection - add price verification (Pitfall #63)
CRITICAL BUG FIXED (Nov 30, 2025):
Position Manager was setting tp1Hit=true based ONLY on size mismatch,
without verifying price actually reached TP1 target. This caused:
- Premature order cancellation (on-chain TP1 removed before fill)
- Lost profit potential (optimal exits missed)
- Ghost orders after container restarts

ROOT CAUSE (line 1086 in position-manager.ts):
  trade.tp1Hit = true  // Set without checking this.shouldTakeProfit1()

FIX IMPLEMENTED:
- Added price verification: this.shouldTakeProfit1(currentPrice, trade)
- Only set tp1Hit when BOTH conditions met:
  1. Size reduced by 5%+ (positionSizeUSD < trade.currentSize * 0.95)
  2. Price crossed TP1 target (this.shouldTakeProfit1 returns true)
- Verbose logging for debugging (shows price vs target, size ratio)
- Fallback: Update tracked size but don't trigger TP1 logic

REAL INCIDENT:
- Trade cmim4ggkr00canv07pgve2to9 (SHORT SOL-PERP Nov 30)
- TP1 target: $137.07, actual exit: $136.84
- False detection triggered premature order cancellation
- Position closed successfully but system integrity compromised

FILES CHANGED:
- lib/trading/position-manager.ts (lines 1082-1111)
- CRITICAL_TP1_FALSE_DETECTION_BUG.md (comprehensive incident report)

TESTING REQUIRED:
- Monitor next trade with TP1 for correct detection
- Verify logs show TP1 VERIFIED or TP1 price NOT reached
- Confirm no premature order cancellation

ALSO FIXED:
- Restarted telegram-trade-bot to fix /status command conflict

See: Common Pitfall #63 in copilot-instructions.md (to be added)
2025-11-30 23:08:34 +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