From fdbb474e689226a101ffdf7c23d057a1a7fb6358 Mon Sep 17 00:00:00 2001 From: mindesbunister Date: Tue, 4 Nov 2025 11:18:57 +0100 Subject: [PATCH] fix(n8n): CRITICAL - Add quality score validation to old workflow path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PROBLEM: - Trades with quality score 35 and 45 were executed (threshold: 60) - Position opened without risk management after signal flips - "Parse Signal" node didn't extract ATR/ADX/RSI/volumeRatio/pricePosition - "Check Risk" node only sent symbol+direction, skipped quality validation - "Execute Trade" node didn't forward metrics to backend ROOT CAUSE: n8n workflow had TWO paths: 1. NEW: Parse Signal Enhanced → Check Risk1 → Execute Trade1 (working) 2. OLD: Parse Signal → Check Risk → Execute Trade (broken) Old path bypassed quality check because check-risk endpoint saw hasContextMetrics=false and allowed trade without validation. FIX: 1. Changed "Parse Signal" from 'set' to 'code' node with metric extraction 2. Updated "Check Risk" to send atr/adx/rsi/volumeRatio/pricePosition 3. Updated "Execute Trade" to forward all metrics to backend IMPACT: - All trades now validated against quality score threshold (60) - Low-quality signals properly blocked before execution - Prevents positions opening without proper risk management Evidence from database showed 3 trades in 2 hours with scores <60: - 10:00:31 SOL LONG - Score 35 (phantom detected) - 09:55:30 SOL SHORT - Score 35 (executed) - 09:35:14 SOL LONG - Score 45 (executed) All three should have been blocked. Fix prevents future bypasses. --- docs/history/N8N_QUALITY_SCORE_BUG_FIX.md | 236 ++++++++++++++++++++++ workflows/trading/Money_Machine.json | 30 +-- 2 files changed, 241 insertions(+), 25 deletions(-) create mode 100644 docs/history/N8N_QUALITY_SCORE_BUG_FIX.md diff --git a/docs/history/N8N_QUALITY_SCORE_BUG_FIX.md b/docs/history/N8N_QUALITY_SCORE_BUG_FIX.md new file mode 100644 index 0000000..96bbf01 --- /dev/null +++ b/docs/history/N8N_QUALITY_SCORE_BUG_FIX.md @@ -0,0 +1,236 @@ +# n8n Workflow Quality Score Bug Fix + +**Date:** November 4, 2025 +**Severity:** CRITICAL +**Impact:** Trades with quality scores below threshold (60) were being executed + +## Problem Description + +User reported a SOL-PERP LONG position opened without risk management (no TP/SL orders) after multiple signal flips. The position had a quality score of 35/100, which should have blocked execution (threshold: 60). + +### What Went Wrong + +The n8n workflow "Money Machine" had **TWO execution paths**: + +1. **NEW PATH (working correctly):** + - `Parse Signal Enhanced` → `Check Risk1` → `Execute Trade1` + - ✅ Sends ALL metrics (ATR, ADX, RSI, volumeRatio, pricePosition) + +2. **OLD PATH (broken):** + - `Parse Signal` → `Check Risk` → `Execute Trade` + - ❌ Only sent `symbol` and `direction` + - ❌ Quality score check was SKIPPED + +### Evidence from Database + +```sql +SELECT "entryTime", symbol, direction, status, "signalQualityScore" +FROM "Trade" +WHERE symbol = 'SOL-PERP' + AND "entryTime" > NOW() - INTERVAL '2 hours' +ORDER BY "entryTime" DESC; +``` + +Results showed TWO trades with low quality scores executed: +- **10:00:31** - LONG (phantom) - Score: 35 ❌ +- **09:55:30** - SHORT (executed) - Score: 35 ❌ +- **09:35:14** - LONG (executed) - Score: 45 ❌ + +All three should have been blocked (threshold 60). + +### Root Cause + +The "Check Risk" node in n8n was configured with: + +```json +"jsonBody": "={\n \"symbol\": \"{{ $json.symbol }}\",\n \"direction\": \"{{ $json.direction }}\"\n}" +``` + +Missing: `atr`, `adx`, `rsi`, `volumeRatio`, `pricePosition` + +When `/api/trading/check-risk` received no metrics, it checked `hasContextMetrics = false` and **allowed the trade to pass** without quality validation. + +## The Fix + +### 1. Updated "Parse Signal" Node + +Changed from simple `set` node to `code` node with full metric extraction: + +```javascript +// Parse new context metrics from enhanced format: +// "ETH buy 15 | ATR:1.85 | ADX:28.3 | RSI:62.5 | VOL:1.45 | POS:75.3" +const atrMatch = body.match(/ATR:([\d.]+)/); +const atr = atrMatch ? parseFloat(atrMatch[1]) : 0; + +const adxMatch = body.match(/ADX:([\d.]+)/); +const adx = adxMatch ? parseFloat(adxMatch[1]) : 0; + +// ... etc for RSI, volumeRatio, pricePosition +``` + +### 2. Updated "Check Risk" Node + +Added all metrics to request body: + +```json +"jsonBody": "={\n \"symbol\": \"{{ $json.symbol }}\",\n \"direction\": \"{{ $json.direction }}\",\n \"atr\": {{ $json.atr || 0 }},\n \"adx\": {{ $json.adx || 0 }},\n \"rsi\": {{ $json.rsi || 0 }},\n \"volumeRatio\": {{ $json.volumeRatio || 0 }},\n \"pricePosition\": {{ $json.pricePosition || 0 }}\n}" +``` + +### 3. Updated "Execute Trade" Node + +Added metrics to execution request: + +```json +"jsonBody": "={\n \"symbol\": \"{{ $('Parse Signal').item.json.symbol }}\",\n \"direction\": \"{{ $('Parse Signal').item.json.direction }}\",\n \"timeframe\": \"{{ $('Parse Signal').item.json.timeframe }}\",\n \"signalStrength\": \"strong\",\n \"atr\": {{ $('Parse Signal').item.json.atr }},\n \"adx\": {{ $('Parse Signal').item.json.adx }},\n \"rsi\": {{ $('Parse Signal').item.json.rsi }},\n \"volumeRatio\": {{ $('Parse Signal').item.json.volumeRatio }},\n \"pricePosition\": {{ $('Parse Signal').item.json.pricePosition }}\n}" +``` + +## How Quality Check Works + +From `/app/api/trading/check-risk/route.ts`: + +```typescript +// Line 263-276 +const hasContextMetrics = body.atr !== undefined && body.atr > 0 + +if (hasContextMetrics) { + const qualityScore = scoreSignalQuality({ + atr: body.atr || 0, + adx: body.adx || 0, + rsi: body.rsi || 0, + volumeRatio: body.volumeRatio || 0, + pricePosition: body.pricePosition || 0, + direction: body.direction, + minScore: 60 // Hardcoded threshold + }) + + if (!qualityScore.passed) { + return NextResponse.json({ + allowed: false, + reason: 'Signal quality too low', + details: `Score: ${qualityScore.score}/100 - ${qualityScore.reasons.join(', ')}` + }) + } +} +``` + +**Before fix:** `hasContextMetrics = false` → quality check SKIPPED +**After fix:** `hasContextMetrics = true` → quality check ENFORCED + +## Impact on Position Management Issue + +The user's main complaint was: +> "Position opened WITHOUT any risk management whatsoever" + +This was actually TWO separate issues: + +1. **Quality score bypass** (this fix) - Trade shouldn't have opened at all +2. **Phantom position** (already fixed) - Position opened but was tiny ($1.41 instead of $2,100) + +The phantom detection worked correctly: +``` +🚨 PHANTOM TRADE DETECTED - Not adding to Position Manager + Expected: $2100.00 + Actual: $1.41 +``` + +So the position WAS NOT added to Position Manager. But it shouldn't have been attempted in the first place due to low quality score. + +## Testing Instructions + +### 1. Import Updated Workflow + +In n8n: +1. Open "Money Machine" workflow +2. File → Import from file → Select `/home/icke/traderv4/workflows/trading/Money_Machine.json` +3. Activate workflow + +### 2. Send Test Signal with Low Quality + +```bash +curl -X POST https://n8n.your-domain.com/webhook/tradingview-bot-v4 \ + -H "Content-Type: application/json" \ + -d "SOL buy 5 | ATR:0.52 | ADX:21.5 | RSI:59.7 | VOL:0.9 | POS:96.4" +``` + +Expected result: +```json +{ + "allowed": false, + "reason": "Signal quality too low", + "details": "Score: 35/100 - ATR healthy (0.52%), Moderate trend (ADX 21.5), RSI supports long (59.7), Price near top of range (96%) - risky long", + "qualityScore": 35, + "qualityReasons": [...] +} +``` + +Telegram should show: +``` +⚠️ TRADE BLOCKED + +SOL buy 5 | ATR:0.52 | ADX:21.5 | RSI:59.7 | VOL:0.9 | POS:96.4 + +🛑 Reason: Signal quality too low +📋 Details: Score: 35/100 - ... +``` + +### 3. Verify Database + +```sql +-- Should see NO new trades with quality score < 60 +SELECT COUNT(*) FROM "Trade" +WHERE "signalQualityScore" < 60 + AND "entryTime" > NOW() - INTERVAL '1 hour'; +``` + +Expected: 0 rows + +## Prevention for Future + +### Code Review Checklist + +When modifying n8n workflows: +- [ ] Ensure ALL execution paths send same parameters +- [ ] Test with low-quality signals (score < 60) +- [ ] Verify Telegram shows "TRADE BLOCKED" message +- [ ] Check database for trades with low scores + +### Monitoring Queries + +Run daily to catch quality score bypasses: + +```sql +-- Trades that should have been blocked +SELECT + "entryTime", + symbol, + direction, + "signalQualityScore", + status, + "realizedPnL" +FROM "Trade" +WHERE "signalQualityScore" < 60 + AND "entryTime" > NOW() - INTERVAL '24 hours' +ORDER BY "entryTime" DESC; +``` + +If ANY results appear, quality check is being bypassed. + +## Files Modified + +1. `/workflows/trading/Money_Machine.json` + - Changed "Parse Signal" from `set` to `code` node + - Added metric extraction regex + - Updated "Check Risk" to send all metrics + - Updated "Execute Trade" to send all metrics + +## Related Issues + +- [PHANTOM_TRADE_DETECTION.md](./PHANTOM_TRADE_DETECTION.md) - Oracle price mismatch issue +- [SIGNAL_QUALITY_SETUP_GUIDE.md](../../SIGNAL_QUALITY_SETUP_GUIDE.md) - Quality scoring system +- [DUPLICATE_POSITION_FIX.md](./DUPLICATE_POSITION_FIX.md) - Signal flip coordination + +## Conclusion + +This was a **critical bug** that allowed low-quality trades to bypass validation and execute without proper risk management. The fix ensures that ALL execution paths in the n8n workflow properly validate signal quality before execution. + +**Key takeaway:** Always verify that all workflow paths send identical parameters to API endpoints. Split paths (old vs new) can create gaps in validation logic. diff --git a/workflows/trading/Money_Machine.json b/workflows/trading/Money_Machine.json index c935388..92c78b8 100644 --- a/workflows/trading/Money_Machine.json +++ b/workflows/trading/Money_Machine.json @@ -19,32 +19,12 @@ }, { "parameters": { - "fields": { - "values": [ - { - "name": "rawMessage", - "stringValue": "={{ $json.body }}" - }, - { - "name": "symbol", - "stringValue": "={{ $json.body.match(/\\bSOL\\b/i) ? 'SOL-PERP' : ($json.body.match(/\\bBTC\\b/i) ? 'BTC-PERP' : ($json.body.match(/\\bETH\\b/i) ? 'ETH-PERP' : 'SOL-PERP')) }}" - }, - { - "name": "direction", - "stringValue": "={{ $json.body.match(/\\b(sell|short)\\b/i) ? 'short' : 'long' }}" - }, - { - "name": "timeframe", - "stringValue": "={{ $json.body.match(/\\.P\\s+(\\d+)/)?.[1] || '15' }}" - } - ] - }, - "options": {} + "jsCode": "// Get the body - it might be a string or nested in an object\nlet body = $json.body || $json.query?.body || JSON.stringify($json);\n\n// If body is an object, stringify it\nif (typeof body === 'object') {\n body = JSON.stringify(body);\n}\n\n// Parse basic signal (existing logic)\nconst symbolMatch = body.match(/\\b(SOL|BTC|ETH)/i);\nconst symbol = symbolMatch ? symbolMatch[1].toUpperCase() + '-PERP' : 'SOL-PERP';\n\nconst direction = body.match(/\\b(sell|short)\\b/i) ? 'short' : 'long';\n\n// Updated regex to match new format: \"ETH buy 15\" (no .P)\nconst timeframeMatch = body.match(/\\b(buy|sell)\\s+(\\d+|D|W|M)\\b/i);\nconst timeframe = timeframeMatch ? timeframeMatch[2] : '5';\n\n// Parse new context metrics from enhanced format:\n// \"ETH buy 15 | ATR:1.85 | ADX:28.3 | RSI:62.5 | VOL:1.45 | POS:75.3\"\nconst atrMatch = body.match(/ATR:([\\d.]+)/);\nconst atr = atrMatch ? parseFloat(atrMatch[1]) : 0;\n\nconst adxMatch = body.match(/ADX:([\\d.]+)/);\nconst adx = adxMatch ? parseFloat(adxMatch[1]) : 0;\n\nconst rsiMatch = body.match(/RSI:([\\d.]+)/);\nconst rsi = rsiMatch ? parseFloat(rsiMatch[1]) : 0;\n\nconst volumeMatch = body.match(/VOL:([\\d.]+)/);\nconst volumeRatio = volumeMatch ? parseFloat(volumeMatch[1]) : 0;\n\nconst pricePositionMatch = body.match(/POS:([\\d.]+)/);\nconst pricePosition = pricePositionMatch ? parseFloat(pricePositionMatch[1]) : 0;\n\nreturn {\n rawMessage: body,\n symbol,\n direction,\n timeframe,\n // New context fields\n atr,\n adx,\n rsi,\n volumeRatio,\n pricePosition\n};" }, "id": "97d5b0ad-d078-411f-8f34-c9a81d18d921", "name": "Parse Signal", - "type": "n8n-nodes-base.set", - "typeVersion": 3.2, + "type": "n8n-nodes-base.code", + "typeVersion": 2, "position": [ -760, 580 @@ -91,7 +71,7 @@ }, "sendBody": true, "specifyBody": "json", - "jsonBody": "={\n \"symbol\": \"{{ $json.symbol }}\",\n \"direction\": \"{{ $json.direction }}\"\n}", + "jsonBody": "={\n \"symbol\": \"{{ $json.symbol }}\",\n \"direction\": \"{{ $json.direction }}\",\n \"atr\": {{ $json.atr || 0 }},\n \"adx\": {{ $json.adx || 0 }},\n \"rsi\": {{ $json.rsi || 0 }},\n \"volumeRatio\": {{ $json.volumeRatio || 0 }},\n \"pricePosition\": {{ $json.pricePosition || 0 }}\n}", "options": {} }, "id": "c1165de4-2095-4f5f-b9b1-18e76fd8c47b", @@ -150,7 +130,7 @@ }, "sendBody": true, "specifyBody": "json", - "jsonBody": "={\n \"symbol\": \"{{ $('Parse Signal').item.json.symbol }}\",\n \"direction\": \"{{ $('Parse Signal').item.json.direction }}\",\n \"timeframe\": \"{{ $('Parse Signal').item.json.timeframe }}\",\n \"signalStrength\": \"strong\"\n}", + "jsonBody": "={\n \"symbol\": \"{{ $('Parse Signal').item.json.symbol }}\",\n \"direction\": \"{{ $('Parse Signal').item.json.direction }}\",\n \"timeframe\": \"{{ $('Parse Signal').item.json.timeframe }}\",\n \"signalStrength\": \"strong\",\n \"atr\": {{ $('Parse Signal').item.json.atr }},\n \"adx\": {{ $('Parse Signal').item.json.adx }},\n \"rsi\": {{ $('Parse Signal').item.json.rsi }},\n \"volumeRatio\": {{ $('Parse Signal').item.json.volumeRatio }},\n \"pricePosition\": {{ $('Parse Signal').item.json.pricePosition }}\n}", "options": { "timeout": 120000 }