WIP: 6+ weeks of uncommitted EA development and preset tuning
Confluence EA (v1.16 → v1.20): - Per-EA realized P&L tracking via history deals - Weekly drawdown protection - Warmup bars, pivot cache, state persistence - Point-scaled pivot thresholds, ranging ATR factor - Market filling mode helper per symbol Grid EA (v3.1 → v4.1): - Adaptive filters, adaptive entry, spread filter - Session filter, breakeven, correlation caps, range drift - Profit protection (stop-after-profit, cycle reports) - Edge cleanup v5.0 — close wrong-side positions outside grid - Master one-shot shutdown, grid state persistence Presets: - Fix GetOut=Y shutdown bug on 4 grid presets - Relax ADXMax 18→40, widen RSI 20/80 across grid presets - Standardize daily drawdown 3%→5%, add weekly 10% - Increase grid lots 0.01→0.03 - Normalize confluence ATR thresholds per pair - Add XAGUSD, EURCHF, EURGBP, AUDNZD presets Docs & DevOps: - April 23 audit files (preset mismatch, code review, checklist) - n8n workflow and validation infrastructure updates - AI agent analyses in notes/ Known issues carried forward: - Shared drawdown budget contamination (both EAs) - Confluence ranging-market threshold inversion - Older grid presets missing v4.1 safety controls
This commit is contained in:
@@ -0,0 +1,380 @@
|
||||
# OrdersEA Smart Grid v4.1 — Comprehensive Code Review
|
||||
|
||||
**Date:** 2026-04-23
|
||||
**File:** `OrdersEA_Smart_Grid.mq5`
|
||||
**Lines:** 1,423
|
||||
**Version history reviewed:** v3.0 (Mar 24) → v3.1 (Mar 30) → v3.2 (Apr 1) → v3.4 (Apr 17-18) → v4.0 (Apr 19) → v4.1 (Apr 23)
|
||||
**Context sources:** Vault notes Mar 22–Apr 23, live MT5 logs Apr 22-23, Git history
|
||||
|
||||
---
|
||||
|
||||
## Executive Summary
|
||||
|
||||
The EA has evolved from a 480-line MT4→MT5 conversion (Mar 24) to a 1,423-line multi-feature grid system (Apr 23). **v4.1 is functionally sound at its core** — pivot calculation, range filters, order placement, per-EA drawdown, and state persistence all work correctly. However, **the April 19 "big bang" feature addition (v4.0) introduced several bugs and design debt items** that need attention. The most critical — a backwards breakeven SL calculation — was fixed in v4.1.
|
||||
|
||||
| Category | Count | Severity |
|
||||
|----------|-------|----------|
|
||||
| Critical bugs (fixed + latent) | 2 | 🔴 High |
|
||||
| Logic/design issues | 5 | 🟡 Medium |
|
||||
| Performance concerns | 3 | 🟡 Medium |
|
||||
| Missing safeguards | 4 | 🟡 Medium |
|
||||
| Code quality / debt | 6 | 🟢 Low |
|
||||
|
||||
---
|
||||
|
||||
## Critical Issues
|
||||
|
||||
### 1. Breakeven SL Direction Bug — FIXED in v4.1 ✅
|
||||
|
||||
**Location:** `ApplyBreakeven()` line 791
|
||||
**History:** Introduced in v4.0 (Apr 19). Discovered Apr 23 when AUDNZD log flooded with 17,822 `err=10016` entries.
|
||||
|
||||
```cpp
|
||||
// v4.0 BROKEN:
|
||||
double newSL = (ptype == POSITION_TYPE_BUY) ? (open + buffer) : (open - buffer);
|
||||
|
||||
// v4.1 FIXED:
|
||||
double newSL = (ptype == POSITION_TYPE_BUY) ? (open - buffer) : (open + buffer);
|
||||
```
|
||||
|
||||
**Root cause:** For BUY, SL must be *below* entry. For SELL, SL must be *above* entry. The original code had both reversed.
|
||||
|
||||
**Why it mattered:** The invalid-stops retry loop ran every 5 seconds per symbol, producing 4.7MB log files and obscuring all other journal entries.
|
||||
|
||||
**Remaining concern:** Even with the fix, `ApplyBreakeven()` still has **no failure-tracking**. If a future broker rule makes a breakeven modification impossible (e.g., minimum freeze level), the same log-flood will recur. A `static map<ticket, lastFailureTime>` or simple `lastFailedTicket + timestamp` guard would prevent this.
|
||||
|
||||
---
|
||||
|
||||
### 2. Drawdown Reset Uses `ACCOUNT_EQUITY` — Shared-Risk Latent Bug
|
||||
|
||||
**Location:** `CheckDailyDrawdown()` line 340, 353
|
||||
**History:** Identified Mar 31. Partially fixed in v4.0 with per-EA P&L tracking, but the **rollover reset still uses account equity**.
|
||||
|
||||
```cpp
|
||||
// Line 340 — daily reset
|
||||
dailyStartEquity = AccountInfoDouble(ACCOUNT_EQUITY);
|
||||
// Line 353 — weekly reset
|
||||
weeklyStartEquity = AccountInfoDouble(ACCOUNT_EQUITY);
|
||||
```
|
||||
|
||||
**The problem:** At broker midnight, if Confluence EA has open floating P&L, that P&L is baked into `dailyStartEquity`. Grid EA's daily limit is then calculated against a number that includes Confluence's positions. If Confluence is +$500 and Grid's true limit is 5% of $100K = $5K, Grid now thinks its limit is 5% of $100.5K = $5,025 — only a $25 difference, but the asymmetry grows with larger floating P&L.
|
||||
|
||||
**More importantly:** If both EAs reset at the same midnight tick, they both capture the *same* account equity as their baseline. This is technically "fair" but means each EA's limit is a percentage of the *combined* account, not its own allocated capital.
|
||||
|
||||
**Fix:** Use `dailyStartEquity = AccountInfoDouble(ACCOUNT_BALANCE)` instead of equity. Balance excludes open P&L, giving a stable baseline. Or better: allocate a fixed dollar budget per EA and track it independently.
|
||||
|
||||
---
|
||||
|
||||
## Logic / Design Issues
|
||||
|
||||
### 3. Breakout Handler Closes Positions Without Checking Direction
|
||||
|
||||
**Location:** `OnTick()` lines 1276-1287
|
||||
|
||||
```cpp
|
||||
if(IsBreakout())
|
||||
{
|
||||
CancelAllOrders("Breakout");
|
||||
CloseAllPositions("Breakout"); // ← closes ALL positions
|
||||
...
|
||||
}
|
||||
```
|
||||
|
||||
When price breaks R2/S2, the EA closes **all** positions — even those that are deeply in profit on the breakout side. A more nuanced approach would close only the *losing* side (e.g., if breaking above R2, close SELL positions but let BUY positions run toward their TPs).
|
||||
|
||||
**Impact:** In strong breakouts, profitable legs are killed alongside losing ones, leaving money on the table.
|
||||
|
||||
---
|
||||
|
||||
### 4. `CalcLots()` Uses `initEquity` Set From `BaseEquity` Input — Stale Reference
|
||||
|
||||
**Location:** `CalcLots()` lines 683-711
|
||||
|
||||
```cpp
|
||||
// OnInit line 1132:
|
||||
initEquity = (double)BaseEquity; // BaseEquity default = 10000
|
||||
|
||||
// CalcLots line 685:
|
||||
double tmp = (AccountInfoDouble(ACCOUNT_EQUITY) - initEquity);
|
||||
```
|
||||
|
||||
If the account is $100,000 and `BaseEquity` is left at the default 10,000, `tmp` is always ~$90,000. The compound-lot formula then exponentiates from a huge base, producing unexpectedly large lot sizes.
|
||||
|
||||
**Evidence:** None of the `.set` files set `BaseEquity` to match actual account size. Most have `EquityFactorPercent=0` which bypasses this path (falls back to fixed `Lots`), but if a user enables equity-based sizing, the default is dangerous.
|
||||
|
||||
**Fix:** Default `BaseEquity` should be 0, and `CalcLots()` should auto-detect current balance if `BaseEquity == 0`.
|
||||
|
||||
---
|
||||
|
||||
### 5. Correlation Cap Scans All Positions on Every Bar — O(N²) per Symbol
|
||||
|
||||
**Location:** `CheckCorrelationCap()` → `CountSymbolsWithOpenPositions()` lines 476-525
|
||||
|
||||
`CountSymbolsWithOpenPositions()` iterates `PositionsTotal()` and does an inner linear search through a `string[]` array. Called twice per bar (long + short check) per symbol. With 11 symbols × ~20 positions each, this is ~220 iterations per bar, manageable but wasteful.
|
||||
|
||||
**Bigger issue:** The correlation cap counts *positions*, not *grid cycles*. A symbol with 1 filled limit order counts the same as a symbol with 10 filled orders. The intent (per the Apr 19 plan) was to cap *distinct symbols with active grid cycles*, which is what the code does — but the name and user expectation may differ.
|
||||
|
||||
---
|
||||
|
||||
### 6. Adaptive Filter Relaxation Uses `lastTradePlacedTime` — Resets on Every Fill
|
||||
|
||||
**Location:** `UpdateFilterRelaxation()` lines 530-538
|
||||
|
||||
```cpp
|
||||
if(secsIdle > (long)InpRelaxFilterAfterDays * 86400)
|
||||
filtersRelaxed = true;
|
||||
```
|
||||
|
||||
Every time a limit order fills, `lastTradePlacedTime` is updated. But a *filled* order means the EA is already trading — there's no need to relax filters. The relaxation should trigger only when **no grid has been placed** for N days, not when no individual order has filled.
|
||||
|
||||
**Fix:** Track `lastGridPlacedTime` separately from `lastTradePlacedTime`.
|
||||
|
||||
---
|
||||
|
||||
### 7. Session Filter Default is Asia Hours — But No `.set` Files Enable It
|
||||
|
||||
**Location:** Input defaults lines 65-67
|
||||
|
||||
```cpp
|
||||
input bool InpUseSessionFilter = false;
|
||||
input int InpSessionStartHour = 1; // 01:00 broker
|
||||
input int InpSessionEndHour = 10; // 10:00 broker
|
||||
```
|
||||
|
||||
The Apr 19 plan explicitly stated: *"Grid works best in Asia range. Placing grids into London/NY trend = losing money. Would have saved much of March."* Yet the session filter defaults to `false`, and none of the 13 existing `.set` files enable it. The new EURGBP/EURCHF/AUDNZD presets are the first to set `InpUseSessionFilter=true`.
|
||||
|
||||
**Recommendation:** Make the session filter default `true` with 01:00-10:00. This is the safest default for a ranging-grid strategy.
|
||||
|
||||
---
|
||||
|
||||
## Performance Concerns
|
||||
|
||||
### 8. `SumRealizedPnLSince()` Scans Entire History on First Call
|
||||
|
||||
**Location:** `SumRealizedPnLSince()` lines 271-290
|
||||
|
||||
`HistorySelect(from, to)` selects all deals in the window. On a Monday morning with a week of history, this could be thousands of deals. Called from `GetRealizedPnLToday()` which is called from `CheckDailyDrawdown()` which runs every tick (but the P&L result is cached for 60 seconds).
|
||||
|
||||
**The cache helps, but the first call after restart is expensive.** With 11 charts restarting simultaneously, this creates a history-scan storm.
|
||||
|
||||
**Fix:** Use `HistorySelectByPosition()` or maintain a running daily P&L accumulator in a GlobalVariable, updated only on deal events.
|
||||
|
||||
---
|
||||
|
||||
### 9. `CalcAvgPrice()` Scans All Positions Every 5 Seconds
|
||||
|
||||
**Location:** `CalcAvgPrice()` lines 732-768
|
||||
|
||||
Called every 5 seconds from `OnTick()` even when there are no positions. The `for` loop still iterates `PositionsTotal()` times. With 11 charts × 20 positions, this is minor but unnecessary.
|
||||
|
||||
**Fix:** Skip the scan if `PositionsTotal() == 0`.
|
||||
|
||||
---
|
||||
|
||||
### 10. Log File Growth from `PrintS()` — No Rotation
|
||||
|
||||
**Location:** All `PrintS()` calls
|
||||
|
||||
The Apr 23 log was 4.7MB (UTF-16LE) after ~7 hours, almost entirely from breakeven failures. Even at normal verbosity, 11 charts each emitting grid-placement messages every hour produces ~264 lines/day per chart = ~2,900 lines/day total. Over a month this is ~90K lines.
|
||||
|
||||
**MT5 does not auto-rotate `.log` files.** At some point the log file will impact terminal performance.
|
||||
|
||||
**Fix:** Add a `PrintS_Throttled()` wrapper for non-critical messages, or emit grid-summary stats only once per hour instead of on every place/cancel.
|
||||
|
||||
---
|
||||
|
||||
## Missing Safeguards
|
||||
|
||||
### 11. No Check for `SYMBOL_TRADE_EXECUTION_MARKET` (Dealing Desk Rejection)
|
||||
|
||||
**Location:** `PlaceBuyLimit()` / `PlaceSellLimit()` lines 927-1029
|
||||
|
||||
The EA places LIMIT orders assuming the broker accepts them. Some brokers (especially during high volatility or with certain account types) reject limit orders and require market execution only. The code handles `trade.ResultRetcode()` but does not differentiate between temporary rejection (retry later) and permanent rejection (don't retry).
|
||||
|
||||
**Fix:** Check for `TRADE_RETCODE_TRADE_DISABLED` or `TRADE_RETCODE_MARKET_CLOSED` and set a backoff timer.
|
||||
|
||||
---
|
||||
|
||||
### 12. `InpUseStopLoss = false` Default — No Mandatory Minimum
|
||||
|
||||
**Location:** Input line 60
|
||||
|
||||
```cpp
|
||||
input bool InpUseStopLoss = false;
|
||||
```
|
||||
|
||||
The March $60K loss was attributed (in part) to grid positions having no SL. v4.0 added `InpUseStopLoss` as an opt-in flag. The default is still `false` for "original grid behavior."
|
||||
|
||||
**This is a deliberate design choice** (grid relies on TP cycles, not SL), but it means a single trending day can wipe out weeks of grid profits. The per-EA drawdown (5%/10%) is the backstop, but it only blocks *new* trades — existing positions keep running into the trend.
|
||||
|
||||
**Recommendation:** Consider a "soft SL" that triggers when a single position is deeper than N× the grid TP distance, or when the entire cycle drawdown exceeds a fixed dollar amount.
|
||||
|
||||
---
|
||||
|
||||
### 13. Grid Geometry Can Place Orders Inside `SYMBOL_TRADE_STOPS_LEVEL`
|
||||
|
||||
**Location:** `PlaceBuyLimit()` lines 936-941, `PlaceSellLimit()` lines 988-993
|
||||
|
||||
The "too close to price" guard checks:
|
||||
```cpp
|
||||
double minDistance = currentAsk - stopLevel - (currentEntryPts * point);
|
||||
if(priceLevel > minDistance) return false; // too close — skip
|
||||
```
|
||||
|
||||
This skips levels that are too close to the current price. However, it does **not** check whether the TP itself violates `SYMBOL_TRADE_STOPS_LEVEL`. On brokers with large stop levels (e.g., 50+ points on exotics), the TP might be closer to the limit price than allowed, causing silent order rejection.
|
||||
|
||||
**Fix:** Validate `tp - priceLevel >= stopLevel` before sending.
|
||||
|
||||
---
|
||||
|
||||
### 14. Weekend Protection Only Triggers on Friday — Misses Broker Timezone Variations
|
||||
|
||||
**Location:** `CheckWeekendProtection()` lines 410-436
|
||||
|
||||
```cpp
|
||||
if(dt.day_of_week != FRIDAY) return true;
|
||||
```
|
||||
|
||||
Some brokers (e.g., Islamic/swap-free accounts) close Thursday instead of Friday. Others trade through Sunday evening. The hardcoded Friday check may miss the actual weekend gap window for some broker configurations.
|
||||
|
||||
**Fix:** Make the weekday configurable, or detect market-closed state via `SymbolInfoInteger(_Symbol, SYMBOL_TRADE_MODE) == SYMBOL_TRADE_MODE_DISABLED`.
|
||||
|
||||
---
|
||||
|
||||
## Code Quality / Debt
|
||||
|
||||
### 15. `gridPlaced` Reset Logic Scattered Across 5+ Locations
|
||||
|
||||
`gridPlaced = false` appears in:
|
||||
- `CheckDailyDrawdown()` (line 1201)
|
||||
- `CheckWeekendProtection()` (line 431)
|
||||
- `OnTick()` new-day pivot recalc (line 1228)
|
||||
- `OnTick()` breakout handler (line 1281)
|
||||
- `OnTick()` range drift (line 1293)
|
||||
- `OnTick()` cycle closed (line 1347)
|
||||
- `HandleGetOut()` (line 1064)
|
||||
- `GlobalShutdown()` (line 1092)
|
||||
|
||||
Each also calls `SaveGridState()`. This is fragile — a future edit might reset `gridPlaced` without saving state, causing a re-place on the next tick.
|
||||
|
||||
**Fix:** Centralize in `ResetGridState(string reason)`.
|
||||
|
||||
---
|
||||
|
||||
### 16. `GetOut` String Comparison Is Case-Sensitive But Only Checks Lowercase
|
||||
|
||||
**Location:** `OnInit()` lines 1150-1156
|
||||
|
||||
```cpp
|
||||
bGetOutOK = (GetOut=="L"||GetOut=="l"||GetOut=="A"||GetOut=="a"||...);
|
||||
```
|
||||
|
||||
The code checks both upper and lower case for validation, but `HandleGetOut()` only checks lowercase:
|
||||
```cpp
|
||||
bool getOutLongs = (GetOut=="L"||GetOut=="l"||GetOut=="A"||...);
|
||||
```
|
||||
|
||||
This is actually correct (MQL5 `==` on strings is case-sensitive), but the input validation in `OnInit()` is redundant with the action logic in `HandleGetOut()`. A single normalized `StringLower(GetOut)` at init would simplify.
|
||||
|
||||
---
|
||||
|
||||
### 17. MagicNum 333011 Used by Both EURUSD and USDJPY Charts
|
||||
|
||||
**Evidence from Apr 23 logs:**
|
||||
```
|
||||
[USDJPY:333011] Cancelled 8 orders: Range filter tripped
|
||||
[EURUSD:333011] Grid placed: 0 buy, 8 sell limits
|
||||
```
|
||||
|
||||
The `grid-usdjpy.set` file defines `MagicNum=333012`, but the live USDJPY chart is using `333011`. This means the EURUSD preset was loaded on the USDJPY chart (or vice versa).
|
||||
|
||||
**Impact:** Minimal — the EA filters by `_Symbol` in addition to `MagicNum` for all normal operations. Only `GlobalShutdown()` (Master=true) operates cross-symbol. So this is mostly a logging/tracking confusion, not a trading bug.
|
||||
|
||||
**Fix:** Reload correct `.set` file on USDJPY chart.
|
||||
|
||||
---
|
||||
|
||||
### 18. `CountPartialTPsSinceCycle()` Scans History but Ignores `DEAL_ENTRY_IN`
|
||||
|
||||
**Location:** `CountPartialTPsSinceCycle()` lines 810-827
|
||||
|
||||
```cpp
|
||||
if(entry != DEAL_ENTRY_OUT && entry != DEAL_ENTRY_INOUT) continue;
|
||||
if(HistoryDealGetDouble(ticket, DEAL_PROFIT) > 0) n++;
|
||||
```
|
||||
|
||||
This counts any deal with positive profit as a "partial TP." But `DEAL_ENTRY_OUT` includes:
|
||||
- Limit order fills (profit = 0 at fill time)
|
||||
- Position closes at TP
|
||||
- Position closes at SL (could be negative or positive)
|
||||
- Manual closes
|
||||
|
||||
The intent is to count "how many profitable TP hits have occurred this cycle." The current code counts any profitable deal exit, including manual closes or SL closes that happened to be profitable.
|
||||
|
||||
**Fix:** Filter by `DEAL_REASON_TP` if available (MT5 build 1930+), or track TP hits explicitly via order comments.
|
||||
|
||||
---
|
||||
|
||||
### 19. `cycleStartEquity` Captures `ACCOUNT_EQUITY` at Grid Placement — Includes Other EAs' P&L
|
||||
|
||||
**Location:** `OnTick()` line 1417
|
||||
|
||||
```cpp
|
||||
cycleStartEquity = AccountInfoDouble(ACCOUNT_EQUITY);
|
||||
```
|
||||
|
||||
If Confluence EA has open positions when Grid places a grid, their floating P&L is included in `cycleStartEquity`. A profitable Confluence trade makes the Grid cycle look more profitable than it is; a losing Confluence trade makes Grid look worse.
|
||||
|
||||
**Fix:** Use `AccountInfoDouble(ACCOUNT_BALANCE)` for cycle start, or better: sum only this EA's positions using `GetFloatingPnL()` + `AccountInfoDouble(ACCOUNT_BALANCE)`.
|
||||
|
||||
---
|
||||
|
||||
### 20. `InpStopAfterProfit` Checks `AccountInfoDouble(ACCOUNT_EQUITY) > cycleStartEquity`
|
||||
|
||||
**Location:** `OnTick()` line 1341
|
||||
|
||||
Same problem as #19 — equity includes other EAs' positions. Grid might stop trading because Confluence is profitable, even if Grid itself is flat.
|
||||
|
||||
---
|
||||
|
||||
## What's Working Well
|
||||
|
||||
| Feature | Assessment |
|
||||
|---------|------------|
|
||||
| **Pivot calculation** | Correct, recalcs at day rollover, ATR-widened bands work |
|
||||
| **Range filters (RSI/ADX/price)** | Correctly detect trending vs ranging, observed cancelling grids in live logs |
|
||||
| **Per-EA drawdown** | Tracks realized + floating P&L by magic+symbol, separate daily/weekly limits |
|
||||
| **State persistence** | GlobalVariables survive EA restart/recompile, prevents double-placement |
|
||||
| **Session filter** | Correct hour-window logic, handles wrap-around (e.g., 22:00-06:00) |
|
||||
| **Spread filter** | Simple, effective |
|
||||
| **Master/One-shot** | Only runs once, not every tick |
|
||||
| **Order placement** | Proper LIMIT orders with SYMBOL_TRADE_STOPS_LEVEL validation |
|
||||
| **Grid geometry** | Derives effective levels from actual range, no-trade zone around price |
|
||||
| **Cycle report** | Emits P&L + duration + partial count on cycle end |
|
||||
|
||||
---
|
||||
|
||||
## Recommended Priority Actions
|
||||
|
||||
### Immediate (This Week)
|
||||
1. ✅ ~~Fix breakeven SL direction~~ (DONE in v4.1)
|
||||
2. **Fix `BaseEquity` default** — set to 0 with auto-detect, or set all `.set` files to match account size
|
||||
3. **Reload correct preset on USDJPY** — should be MagicNum 333012, not 333011
|
||||
4. **Add breakeven failure guard** — don't retry same failed ticket within 60 seconds
|
||||
|
||||
### Short-Term (Next 2 Weeks)
|
||||
5. **Enable session filter by default** — change `input bool InpUseSessionFilter = true`
|
||||
6. **Fix drawdown rollover baseline** — use `ACCOUNT_BALANCE` instead of `ACCOUNT_EQUITY`
|
||||
7. **Fix `cycleStartEquity` isolation** — use balance + this-EA floating P&L only
|
||||
8. **Optimize `SumRealizedPnLSince()`** — add deal-event tracking instead of full history scan
|
||||
|
||||
### Medium-Term (Next Month)
|
||||
9. **Centralize `gridPlaced = false` logic** into `ResetGridState()`
|
||||
10. **Add per-cycle "soft SL"** — close cycle if drawdown exceeds N× TP distance
|
||||
11. **Improve breakout handler** — close only losing-side positions, let winners run
|
||||
12. **Add log throttling** — hourly summaries instead of per-event prints
|
||||
|
||||
---
|
||||
|
||||
*Review author: Kimi Code CLI*
|
||||
*Sources: `~/mql-trading-bots/OrdersEA_Smart_Grid.mq5` (v4.1, 1423 lines), Obsidian vault Mar 22–Apr 23, live MT5 logs 20260422-20260423*
|
||||
Reference in New Issue
Block a user