mql-trading-bots/CODE_REVIEW_OrdersEA_Smart_Grid_v4.1.md

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.

// 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.

// 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

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

// 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

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

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

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:

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

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

bGetOutOK = (GetOut=="L"||GetOut=="l"||GetOut=="A"||GetOut=="a"||...);

The code checks both upper and lower case for validation, but HandleGetOut() only checks lowercase:

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

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

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