**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
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:
- Container restarts
- Startup validation runs (lib/startup/init-position-manager.ts)
- Queries database for open trades
- Finds trade with
tp1Hit = false(database not updated yet) - Queries Drift position, sees reduced size
- Restores position tracking with WRONG assumptions
- 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
/statuscommand 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):
- ✅ Position size reduced by ~75%
- ✅ 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
- ✅ Check database for recent trade details
- ✅ Review Docker logs for TP1 detection sequence
- ✅ Identify premature order cancellation
- ✅ Understand restart behavior
- 🔧 Implement Fix #1 (price verification)
- 🔧 Implement Fix #2 (prevent premature cancellation)
- 🔧 Restart Telegram bot
- 🔧 Docker rebuild and restart
- ✅ Execute test trade to verify fix
- ✅ Monitor logs for correct TP1 detection
- ✅ 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
Related Issues
- 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:
- Explain the bug chain clearly
- Show evidence from logs and database
- Outline the fix strategy
- Estimate deployment timeline
- Test thoroughly before declaring "fixed"
Follow-up:
- Document in copilot-instructions.md
- Add to Common Pitfalls with full incident details
- Update Position Manager verification procedures
- Consider adding "order already filled" detection