Add MetaTrader 5 integration for forex and CFD trading

[icojerrel]

New Features:

• MT5 integration layer (nice_funcs_mt5.py) with full trading functionality
• AI-powered MT5 trading agent with multi-symbol support
• Connection management, account info, and position tracking
• Market data fetching with technical indicators (SMA, RSI, MACD, BB)
• Market buy/sell execution with SL/TP support
• Comprehensive MT5 setup documentation

Configuration:

• MT5 settings added to config.py (symbols, position limits, AI model)
• Environment variables for MT5 credentials in .env_example
• MetaTrader5 package added to requirements.txt

Documentation:

• Detailed MT5_README.md with setup, usage, and troubleshooting
• Updated main README.md with MT5 agent listing
• Platform compatibility guide (Windows/Mac/Linux)

This enables trading forex pairs (EURUSD, GBPUSD, etc.) and CFDs
using the same AI-powered approach as the Solana agents.

Summary by CodeRabbit

New Features

• Added MT5 trading agent supporting multi-asset trading (forex, metals, indices, stocks, energies, crypto)
• Added Docker support for containerized deployment
• Added multi-channel alerting: Telegram, Discord, and Email notifications
• Added trading hours and volatility filtering for optimal market conditions
• Added health monitoring system with resource and API checks
• Added sandbox/mock trading mode for risk-free testing
• Added 24-hour automated trading test suite

Documentation

• Comprehensive setup, deployment, and testing guides
• MT5 integration and trading configuration documentation
• Production deployment procedures

[coderabbitai]

Walkthrough

This pull request introduces a comprehensive MT5 trading system with AI-driven analysis via OpenRouter, multi-asset support, Docker containerization, health monitoring, alerting infrastructure, structured logging, trading hours filtering, and extensive testing frameworks alongside production deployment and operational documentation.

Changes

Cohort / File(s)	Summary
Docker & Containerization .dockerignore, Dockerfile, Dockerfile.test, docker-compose.yml, docker-compose.test.yml	Adds Docker configuration for production and testing environments; defines multi-service orchestration (trading agent, risk/sentiment agents, health monitor) with environment variable management, persistent volumes for logs/data, and specialized test containers for 24-hour trading tests and sandbox validation.
Configuration & Environment .env_example, .gitignore, requirements.txt	Extends environment variables for MT5 credentials, multi-channel alerting (Telegram/Discord/Email), logging configuration, and health monitoring thresholds; adds test artifact and log ignore patterns; introduces MetaTrader5 and psutil dependencies.
MT5 Trading Agent Core src/agents/mt5_trading_agent.py, src/agents/MT5_README.md	Implements MT5TradingAgent class with AI-driven market analysis via OpenRouter, trade execution with risk controls (max positions, confidence thresholds), per-symbol analysis cycles, and SL/TP management; includes comprehensive integration documentation.
MT5 Integration & Utilities src/nice_funcs_mt5.py, src/utils/mt5_helpers.py	Provides MT5 wrapper functions (account/symbol info, OHLCV data, position management, technical indicators) with sandbox fallback mode; includes asset detection, position sizing calculations, market context generation, and asset-class-specific parameter retrieval.
AI Model Integration src/models/openrouter_model.py, src/models/model_factory.py	Adds OpenRouterModel subclass for OpenRouter API integration with model metadata, client initialization, response generation, and error handling; updates ModelFactory to register OpenRouter as a backend with default model configuration.
Configuration Management src/config.py	Introduces comprehensive MT5 configuration block with symbol groups by asset class (forex/metals/indices/stocks/energies/crypto), per-asset position sizing and risk parameters, AI model settings, trading hours filtering controls, sandbox/testing mode flags, and timeframe specifications.
Alerting & Health Monitoring src/utils/alerts.py, src/utils/health_monitor.py	Adds multi-channel AlertManager (Telegram/Discord/Email) with severity levels and throttling; implements HealthMonitor for system resources, API health, error logging, and agent heartbeat tracking with configurable thresholds.
Logging Infrastructure src/utils/logger.py	Introduces production-grade logging system with ColoredFormatter and JSONFormatter, singleton LoggerManager, rotating file handlers, per-agent log scoping, trade-specific logging to trades.log, and backward-compatibility adapter.
Trading Hours & Simulation src/utils/trading_hours.py, src/utils/mock_mt5.py	Implements trading hours filter with session detection (Asian/London/NY/overlaps), asset-class-specific optimal hours, weekend/day-of-week avoidance; provides MockMT5Simulator with position tracking, P/L calculation, market move simulation, and account state management.
Utility Package src/utils/__init__.py	Adds package initializer for utilities module.
Service & Monitoring moondev-trading.service, monitoring/monitor_test.py	Defines systemd service unit for production deployment with resource limits, security hardening, and logging; introduces real-time test monitor that watches JSON result files and displays aggregated test statistics with terminal-based dashboard.
Testing Framework run_tests.sh, tests/__init__.py, tests/test_lightweight.py, tests/test_mt5_sandbox.py, tests/test_24h_trading.py, tests/analyze_results.py, tests/quick_status.sh, tests/test_mt5_agent_simple.py, tests/test_modelfactory_openrouter.py, tests/test_openrouter_basic.py	Adds comprehensive test orchestration with Docker-based runner, lightweight unit tests (config/asset detection/position limits/risk params), sandbox MT5 tests with MockMT5Simulator, 24-hour automated trading test with result collection, test result analysis and reporting, quick status dashboard, and integration tests for OpenRouter/ModelFactory.
Documentation & Guides README.md, SETUP_COMPLETE.md, TESTING.md, TEST_INSTRUCTIONS.md, TEST_RESULTS.md, TESTING_HOURS_INTEGRATION.md, MULTI_ASSET_TRADING.md, PRODUCTION_DEPLOY.md, OPENROUTER_FIXES.md, src/agents/MT5_README.md, tests/TEST_24H_README.md	Provides end-to-end documentation covering setup walkthrough with OpenRouter and MT5 configuration, comprehensive testing guide and instructions, 24-hour test documentation, production deployment procedures, trading hours filter integration, multi-asset trading configuration, OpenRouter integration fixes and troubleshooting, test results validation, and agent-specific README.

Sequence Diagram(s)

sequenceDiagram
    participant Agent as MT5TradingAgent
    participant Hours as TradingHoursFilter
    participant Helpers as mt5_helpers
    participant MT5 as MT5/Mock
    participant Model as OpenRouterModel
    participant Exec as Trade Executor

    Agent->>Hours: is_optimal_trading_time(asset_class)
    Hours-->>Agent: bool, reason
    Agent->>MT5: get_ohlcv_data(symbol)
    MT5-->>Agent: DataFrame
    Agent->>Helpers: add_technical_indicators(df)
    Helpers-->>Agent: df_with_indicators
    Agent->>Helpers: get_market_context(symbol)
    Helpers-->>Agent: context_string
    Agent->>Model: generate_response(system_prompt, user_content)
    Model-->>Agent: analysis(action, confidence, SL, TP, size)
    Agent->>Agent: validate(confidence >= threshold, max_positions)
    alt Valid Trade
        Agent->>Exec: execute_trade(symbol, analysis)
        Exec->>MT5: market_buy/sell(symbol, volume, sl, tp)
        MT5-->>Exec: ticket
        Exec-->>Agent: success
    else Invalid
        Agent-->>Agent: skip/hold
    end
    Agent->>MT5: get_positions()
    MT5-->>Agent: positions_df

Loading

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

• Key areas requiring extra attention:
  • src/agents/mt5_trading_agent.py: JSON parsing resilience in AI response handling; max position enforcement logic; confidence threshold validation
  • src/models/openrouter_model.py: Error handling coverage for API failures; HTTP status code handling and user-friendly messaging
  • src/nice_funcs_mt5.py: Mock mode fallback behavior; MT5 connection state management; indicator calculation correctness
  • src/utils/alerts.py: Multi-channel delivery robustness; credential handling security (API keys in env); SMTP/Discord webhook error cases
  • src/utils/health_monitor.py: Throttling logic preventing alert spam; agent heartbeat timeout tuning; system resource threshold appropriateness
  • src/utils/logger.py: Singleton thread-safety; log rotation edge cases; trade data structure consistency
  • src/utils/trading_hours.py: Timezone handling correctness; session boundary edge cases; asset-class-specific hour validation
  • tests/test_24h_trading.py: Test result JSON schema consistency; cycle timing accuracy; graceful failure modes
  • docker-compose.yml and Dockerfile: Build context correctness; environment variable injection across services; volume permission handling
  • Config integration points across src/config.py: Symbol list consistency; risk parameter per-asset correctness; sandbox mode flag propagation

Poem

🐰 A rabbit hops with glee,
Trading bots run wild and free!
With OpenRouter's AI mind,
MT5 profits we shall find!
Through Docker dreams and test arrays,
Moon Dev thrives for countless days! 🚀

Pre-merge checks and finishing touches

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The PR title directly summarizes the primary change: adding MetaTrader 5 integration for forex and CFD trading, which aligns with the substantial additions of MT5 trading agent, market data fetching, trading execution, and comprehensive configuration.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch claude/continue-work-011CUq1JWCHDeiRR6WoX9U33

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 13

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/models/model_factory.py (1)

236-259: Critical: Misplaced generate_response method in ModelFactory.

This generate_response method appears to be accidentally added to the ModelFactory class. The method references instance attributes (self.client, self.model_name, self.max_tokens) that don't exist in ModelFactory. This code belongs in a model implementation class (like BaseModel or a specific model class), not in the factory.

This will cause an AttributeError if this method is called on a ModelFactory instance. Please remove this method from the factory:

-    def generate_response(self, system_prompt, user_content, temperature=0.7, max_tokens=None):
-        """Generate a response from the model with no caching"""
-        try:
-            # Add random nonce to prevent caching
-            nonce = f"_{random.randint(1, 1000000)}"
-            
-            response = self.client.chat.completions.create(
-                model=self.model_name,
-                messages=[
-                    {"role": "system", "content": system_prompt},
-                    {"role": "user", "content": f"{user_content}{nonce}"}  # Add nonce to force new response
-                ],
-                temperature=temperature,
-                max_tokens=max_tokens if max_tokens else self.max_tokens
-            )
-            
-            return response.choices[0].message
-            
-        except Exception as e:
-            if "503" in str(e):
-                raise e  # Let the retry logic handle 503s
-            cprint(f"❌ Model error: {str(e)}", "red")
-            return None
-

If this method is needed, it should be in the appropriate model class that has these attributes.

🧹 Nitpick comments (17)

.gitignore (1)

160-162: Align gitignore patterns with new testing and logging infrastructure.

The additions to ignore test results and log files are appropriate for the new testing frameworks and structured logging introduced in this PR.

Minor observation: Line 61 already includes *.log which globally ignores all .log files, making line 162's logs/*.log slightly redundant. While not harmful, the more specific pattern is unnecessary given the global rule.

If you prefer explicit control over log locations, the current pattern is fine. Otherwise, consider removing line 162 and relying on the existing *.log rule.

tests/analyze_results.py (2)

24-35: Add error handling for file loading.

The constructor opens and parses the JSON file without error handling. If the file is deleted between the existence check in main() and initialization, or if the JSON is malformed, the constructor will raise an unhandled exception.

Apply this diff to add error handling:

 def __init__(self, results_file: Path):
     self.results_file = results_file
-
-    with open(results_file, 'r') as f:
-        self.data = json.load(f)
-
+    
+    try:
+        with open(results_file, 'r') as f:
+            self.data = json.load(f)
+    except FileNotFoundError:
+        cprint(f"❌ File not found: {results_file}", "red")
+        raise
+    except json.JSONDecodeError as e:
+        cprint(f"❌ Invalid JSON in {results_file}: {e}", "red")
+        raise
+    
     self.stats = self.data.get('statistics', {})
     self.cycles = self.data.get('cycles', [])
     self.decisions = self.data.get('decisions', [])
     self.trades = self.data.get('trades', [])
     self.errors = self.data.get('errors', [])
     self.config = self.data.get('config', {})

---

200-208: Consider making cycle interval configurable.

The uptime calculation assumes 15-minute intervals (line 202), but this may not match the actual test configuration. Consider reading the interval from the config section of the results file or adding a constant at the class level.

+    # Default cycle interval in minutes
+    DEFAULT_CYCLE_INTERVAL_MINUTES = 15
+
     def analyze_performance(self):
         """Analyze overall performance"""
         cprint("📈 Performance Analysis", "cyan", attrs=["bold"])
         print("-" * 80)

         # Calculate uptime
         if 'test_duration_seconds' in self.stats and 'total_cycles' in self.stats:
-            expected_cycles = self.stats['test_duration_seconds'] / (15 * 60)  # 15 min intervals
+            cycle_interval = self.config.get('cycle_interval_minutes', self.DEFAULT_CYCLE_INTERVAL_MINUTES)
+            expected_cycles = self.stats['test_duration_seconds'] / (cycle_interval * 60)
             actual_cycles = self.stats['total_cycles']
             uptime = (actual_cycles / expected_cycles) * 100 if expected_cycles > 0 else 0

Dockerfile (1)

44-46: Consider enhancing the health check.

The current health check only verifies Python execution. For production deployments, consider checking actual application health (e.g., verifying the main process is responsive or checking a health endpoint).

 # Health check
 HEALTHCHECK --interval=5m --timeout=30s --start-period=1m --retries=3 \
-    CMD python -c "import sys; sys.exit(0)"
+    CMD python -c "import psutil; import sys; sys.exit(0 if any('main.py' in ' '.join(p.cmdline()) for p in psutil.process_iter(['cmdline'])) else 1)"

This checks if the main.py process is actually running. Alternatively, implement a dedicated health check script.

TEST_OPENROUTER.md (1)

1-347: Consider addressing markdown linting issues.

The static analysis tools flagged several markdown formatting issues:

• Fenced code blocks missing language specifiers (lines 26, 60, 98, etc.)
• Bare URLs that should be wrapped in angle brackets or markdown links (lines 212, 213, 240)
• Emphasis used instead of heading (lines 343, 347)

These are minor quality improvements that enhance readability and consistency.

Example fixes:

 **Als het faalt:**
 - Check `.env` - is OPENROUTER_API_KEY correct?
-- Check OpenRouter dashboard - credits beschikbaar?
-- Check https://openrouter.ai/keys - key active?
+- Check OpenRouter dashboard - credits beschikbaar?
+- Check <https://openrouter.ai/credits> - credits available?
+- Check <https://openrouter.ai/keys> - key active?
 - Try regenerating key

For code blocks without language specifiers, add appropriate language tags (e.g., text, console, etc.).

Dockerfile.test (1)

35-37: Consider enhancing the HEALTHCHECK to verify test execution.

The current HEALTHCHECK only verifies that the /app/tests/results directory exists, but doesn't confirm that the test is actively running or producing results. For a 24-hour test, consider checking for recent file modifications or a test heartbeat file.

Example improvement:

-HEALTHCHECK --interval=5m --timeout=30s --start-period=1m --retries=3 \
-    CMD python -c "import sys; from pathlib import Path; sys.exit(0 if Path('/app/tests/results').exists() else 1)"
+HEALTHCHECK --interval=5m --timeout=30s --start-period=1m --retries=3 \
+    CMD python -c "import sys, time; from pathlib import Path; p = Path('/app/tests/results'); sys.exit(0 if p.exists() and any(f.stat().st_mtime > time.time() - 600 for f in p.iterdir() if f.is_file()) else 1)"

This checks if any result file was modified within the last 10 minutes, ensuring the test is actively producing output.

MULTI_ASSET_TRADING.md (1)

102-122: Minor: Add language specifiers to code blocks for better rendering.

The static analysis tool flagged a few markdown formatting improvements:

1. Code blocks at lines 102, 168, and 251 should specify a language for syntax highlighting
2. Line 563 uses emphasis instead of a proper heading

These are purely cosmetic improvements for better documentation rendering:

For the code blocks without language specifiers, add backtick language tags:

-```
+```text
 📍 Asian Session (00:00-09:00 UTC)

For line 563, convert emphasis to heading:

-**Built with ❤️ by Moon Dev 🌙**
+## Built with ❤️ by Moon Dev 🌙

Also applies to: 168-175, 251-264, 563-563

tests/TEST_24H_README.md (1)

160-167: Add language identifiers to code blocks.

The code blocks at lines 160 and 165 are missing language identifiers, which improves syntax highlighting and readability.

Apply this diff:

 **Intermediate Results** (elke 15 min):
-```
+```text
 tests/results/24h_test_INTERMEDIATE_[timestamp].json

Final Results (na 24 uur):
- +text
tests/results/24h_test_FINAL_[timestamp].json

docker-compose.test.yml (1)

45-50: Simplify healthcheck command for maintainability.

The inline Python healthcheck is functional but complex. Consider creating a dedicated health check script for better maintainability and testability.

Create a health check script:

# healthcheck.py
import sys
from pathlib import Path

sys.exit(0 if Path('/app/tests/results').exists() else 1)

Then update the healthcheck:

     healthcheck:
-      test: ["CMD", "python", "-c", "import sys; from pathlib import Path; sys.exit(0 if Path('/app/tests/results').exists() else 1)"]
+      test: ["CMD", "python", "healthcheck.py"]
       interval: 5m
       timeout: 30s
       retries: 3
       start_period: 1m

moondev-trading.service (1)

25-30: Document path customization requirements.

The service file contains hardcoded paths specific to the moondev user and conda environment. Users will need to customize these paths during installation. Consider adding a note in the header comments.

Apply this diff to improve installation guidance:

 # Moon Dev AI Trading System - Systemd Service
 # Install to: /etc/systemd/system/moondev-trading.service
 #
+# ⚠️ IMPORTANT: Customize these paths before installation:
+#   - WorkingDirectory (line 25)
+#   - PATH Environment (line 26)
+#   - ExecStart (line 30)
+#   - StandardOutput/StandardError (lines 43-44)
+#   - ReadWritePaths (line 51)
+#
 # Installation:
 #   sudo cp moondev-trading.service /etc/systemd/system/
+#   # Edit paths in the file before enabling:
+#   sudo nano /etc/systemd/system/moondev-trading.service
 #   sudo systemctl daemon-reload
 #   sudo systemctl enable moondev-trading
 #   sudo systemctl start moondev-trading

OPENROUTER_FIXES.md (1)

102-102: Add language identifiers to code blocks.

Several code blocks are missing language identifiers (lines 102, 168, 251), which would improve readability and syntax highlighting.

For example, at line 102:

 **Config:**
-```
+```python
 MT5_RISK_PARAMS = {
     'forex': {

Also applies to: 168-168, 251-251

TESTING.md (1)

31-31: Add language identifiers to code blocks.

Multiple code blocks throughout the file are missing language identifiers, which would improve syntax highlighting and readability. Consider adding identifiers like bash, python, or text as appropriate.

For example:

 ### Run Individual Tests

-```
+```bash
 # Test mock MT5 simulator
 python src/utils/mock_mt5.py

Also applies to: 49-49, 75-75, 96-96, 105-105, 114-114, 132-132, 152-152, 178-178

TEST_RESULTS.md (1)

31-31: Add language identifiers to code blocks.

Similar to other documentation files, multiple code blocks are missing language identifiers. Consider adding text or appropriate language tags for better rendering.

For example:

 ### Test 1: Configuration Validation ✅

-```
+```text
 MT5_MAX_POSITIONS: 1 ⚠️
 MT5_MIN_CONFIDENCE: 75%

Also applies to: 49-49, 75-75, 96-96, 105-105, 114-114, 132-132, 152-152, 178-178

src/agents/mt5_trading_agent.py (2)

69-103: Consider making Optional parameters explicit and verifying market cap claims.

The parameters symbols, model_name use implicit Optional which is not PEP 484 compliant. Additionally, the market cap values in comments (lines 94-98) may become outdated quickly.

Apply this diff to address the type hints:

     def __init__(
         self,
-        symbols: List[str] = None,
+        symbols: Optional[List[str]] = None,
         model_type: str = 'anthropic',
-        model_name: str = None,
+        model_name: Optional[str] = None,
         max_position_size: float = 0.1,
         max_positions: int = 3,
     ):

Consider removing the specific market cap values from comments (lines 94-98) to avoid documentation rot, or add a date reference like "as of Nov 2025".

---

129-266: Good resilient JSON parsing, but verify division by zero safety.

The JSON parsing with fallback (lines 226-253) handles errors well. However, at line 152, the price change calculation could theoretically divide by zero if prev['Close'] is zero (though unlikely for real market data):

price_change = ((latest['Close'] - prev['Close']) / prev['Close']) * 100

Consider adding a guard:

             # Calculate key metrics
-            price_change = ((latest['Close'] - prev['Close']) / prev['Close']) * 100
+            if prev['Close'] != 0:
+                price_change = ((latest['Close'] - prev['Close']) / prev['Close']) * 100
+            else:
+                price_change = 0.0

src/utils/mt5_helpers.py (1)

11-57: Consider more defensive default for unknown symbols.

The default fallback to 'forex' (line 57) may misclassify unknown symbols. Consider logging a warning or returning 'unknown' for symbols that don't match any pattern, allowing calling code to decide how to handle them.

     # Default to forex
-    return 'forex'
+    # Log warning for unknown symbols
+    import logging
+    logging.getLogger(__name__).warning(f"Unknown symbol {symbol}, defaulting to forex")
+    return 'forex'

src/utils/logger.py (1)

216-257: Consider adding rotation to trades.log.

The log_trade method writes to trades.log in append mode (line 255) without rotation. This file could grow unbounded over time in a production system with high trading volume.

Consider using a RotatingFileHandler for trades.log:

-        # Also write to trades log file
-        trades_log_file = self.log_dir / 'trades.log'
-        with open(trades_log_file, 'a') as f:
-            f.write(json.dumps(trade_data) + '\n')
+        # Also write to trades log file with rotation
+        if not hasattr(self, '_trades_handler'):
+            trades_log_file = self.log_dir / 'trades.log'
+            self._trades_handler = logging.handlers.RotatingFileHandler(
+                trades_log_file,
+                maxBytes=self.max_bytes,
+                backupCount=self.backup_count
+            )
+        
+        # Write trade data
+        self._trades_handler.stream.write(json.dumps(trade_data) + '\n')
+        self._trades_handler.stream.flush()

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Remove the INTERMEDIATE-only guard on file updates.

The monitor ignores modifications unless the path contains “INTERMEDIATE,” so if the test rewrites the canonical 24h_test_<timestamp>.json in place you never see live updates after the initial create. Drop that substring check (or broaden it to all 24h_test JSONs) so every modification refreshes the stats.

-        if event.src_path.endswith('.json') and 'INTERMEDIATE' in event.src_path:
+        if event.src_path.endswith('24h_test_') or event.src_path.endswith('.json'):
             self.display_latest_stats()

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	if event.src_path.endswith('.json') and 'INTERMEDIATE' in event.src_path:
	self.display_latest_stats()
	if event.src_path.endswith('24h_test_') or event.src_path.endswith('.json'):
	self.display_latest_stats()

🤖 Prompt for AI Agents

In monitoring/monitor_test.py around lines 39-40, the event handler currently
only calls display_latest_stats when the path contains "INTERMEDIATE", which
prevents in-place rewrites of canonical 24h_test_<timestamp>.json from
triggering updates; remove that "INTERMEDIATE" substring check and instead check
for JSON files for the 24h_test pattern (e.g., if
event.src_path.endswith('.json') and '24h_test' in event.src_path) or simply any
.json (if you want all JSON updates) so that modifications (not just creations)
refresh the stats.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify pip-to-point conversion for all asset classes.

The hardcoded * 10 multiplier in SL/TP calculations (lines 310-311, 333-334) assumes 1 pip = 10 points, which is asset-specific:

sl = round(price - (sl_pips * point * 10), digits) if sl_pips > 0 else 0
tp = round(price + (tp_pips * point * 10), digits) if tp_pips > 0 else 0

For JPY pairs, 1 pip = 1 point (not 10). For metals like XAUUSD, the pip definition varies by broker. This could result in SL/TP being placed 10x too far or too close.

Please verify the pip-to-point conversion logic is correct for all asset classes in MT5_SYMBOLS. Consider using asset class detection to apply the correct multiplier:

Also applies to: 333-334

---

🏁 Script executed:

#!/bin/bash
# Check which asset classes are configured and their pip definitions
rg -n "MT5_SYMBOLS|MT5_RISK_PARAMS" src/config.py -A 10

Length of output: 725

---

---

🏁 Script executed:

#!/bin/bash
# Examine the execute_trade method around the pip conversion lines
sed -n '267,359p' src/agents/mt5_trading_agent.py

Length of output: 3579

---

🏁 Script executed:

#!/bin/bash
# Check if there's any existing asset-class or symbol detection logic
rg -n "point|pips|JPY|forex" src/agents/mt5_trading_agent.py -B 2 -A 2

Length of output: 4294

---

🏁 Script executed:

#!/bin/bash
# Look for symbol_info structure and how it's obtained
rg -n "symbol_info\s*=" src/agents/mt5_trading_agent.py -B 2 -A 5

Length of output: 284

---

🏁 Script executed:

#!/bin/bash
# Check the complete MT5_RISK_PARAMS and other config details
cat src/config.py | head -250 | tail -80

Length of output: 3066

---

🏁 Script executed:

#!/bin/bash
# Check if there's any symbol-to-asset-class mapping function
rg -n "def.*asset|def.*class|USDJPY|JPY" src/agents/mt5_trading_agent.py

Length of output: 95

---

🏁 Script executed:

#!/bin/bash
# Verify what asset classes are actually being traded with symbol list
rg -n "MT5_FOREX_PAIRS|MT5_METALS|MT5_INDICES|MT5_STOCKS" src/config.py -A 3

Length of output: 835

---

Fix pip-to-point conversion for JPY pairs and non-forex assets.

The hardcoded * 10 multiplier at lines 310–311 and 333–334 is incorrect for JPY pairs and metals currently configured to trade:

• JPY pairs (USDJPY): 1 pip = 1 point, not 10 → SL/TP placed 10× too far
• Metals (XAUUSD): 1 pip = 1 point → SL/TP placed 10× too far
• Other assets: Pip definition varies by broker and asset class

Add symbol-to-asset-class detection and apply the correct multiplier:

def _get_pip_multiplier(self, symbol: str) -> int:
    """Determine pip-to-point multiplier based on symbol/asset class"""
    if symbol.endswith('JPY'):
        return 1  # JPY pairs: 1 pip = 1 point
    elif symbol.startswith('XAU') or symbol.startswith('XAG'):
        return 1  # Metals: 1 pip = 1 point
    else:
        return 10  # Standard forex: 1 pip = 10 points

# Then in execute_trade, lines 310-311 and 333-334:
multiplier = self._get_pip_multiplier(symbol)
sl = round(price - (sl_pips * point * multiplier), digits) if sl_pips > 0 else 0
tp = round(price + (tp_pips * point * multiplier), digits) if tp_pips > 0 else 0

🤖 Prompt for AI Agents

In src/agents/mt5_trading_agent.py around lines 308 to 311 (and likewise where
SL/TP are computed at ~333–334), the hardcoded "* 10" pip-to-point multiplier is
incorrect for JPY pairs and metals; implement a symbol-based pip multiplier and
use it instead of the literal 10. Add a helper method that returns 1 for symbols
ending with "JPY" and for metals starting with "XAU"/"XAG", and 10 for standard
forex, then call it to get multiplier = self._get_pip_multiplier(symbol) and
replace "* 10" with "* multiplier" when computing sl and tp (both buy and sell
branches).

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Raise when the OpenRouter client cannot be created.
Line [131] catches every exception, logs, sets self.client = None, and returns. Because of that, model_factory.get_model(...) still hands back an OpenRouterModel instance, so callers think initialization succeeded until the first generate_response call crashes with 'NoneType' object has no attribute chat. Propagate the failure (e.g., re-raise after logging) so model_factory can return None and the agent/tests can react immediately instead of failing later.

🧰 Tools

🪛 Ruff (0.14.3)

129-129: f-string without any placeholders

Remove extraneous f prefix

(F541)

---

130-130: Do not catch blind exception: Exception

(BLE001)

---

131-131: Use explicit conversion flag

Replace with conversion flag

(RUF010)

🤖 Prompt for AI Agents

In src/models/openrouter_model.py around lines 126 to 133, the except block
currently swallows all exceptions, logs the error, and sets self.client = None
causing callers to receive a partially-initialized OpenRouterModel; instead,
after logging the failure, re-raise the original exception (or raise a new
RuntimeError that wraps the original) so the failure propagates to model_factory
(do not return silently), allowing model_factory to handle the error and return
None; ensure the original exception context is preserved when re-raising.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Prevent MT5_LOGIN parsing from crashing the module

Line 40 does int(os.getenv('MT5_LOGIN', '0')). If .env contains MT5_LOGIN= or any non-numeric value (very easy to do while wiring credentials), importing this module raises ValueError and the entire MT5 integration never starts. Please validate and default gracefully.

-MT5_LOGIN = int(os.getenv('MT5_LOGIN', '0'))
+login_env = os.getenv('MT5_LOGIN', '').strip()
+MT5_LOGIN = 0
+if login_env:
+    try:
+        MT5_LOGIN = int(login_env)
+    except ValueError:
+        cprint(f"⚠️  Invalid MT5_LOGIN '{login_env}', defaulting to demo mode", "yellow")

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	MT5_LOGIN = int(os.getenv('MT5_LOGIN', '0'))
	MT5_PASSWORD = os.getenv('MT5_PASSWORD', '')
	MT5_SERVER = os.getenv('MT5_SERVER', '')
	MT5_PATH = os.getenv('MT5_PATH', '') # Path to MT5 terminal (Windows/Wine)
	login_env = os.getenv('MT5_LOGIN', '').strip()
	MT5_LOGIN = 0
	if login_env:
	try:
	MT5_LOGIN = int(login_env)
	except ValueError:
	cprint(f"⚠️ Invalid MT5_LOGIN '{login_env}', defaulting to demo mode", "yellow")
	MT5_PASSWORD = os.getenv('MT5_PASSWORD', '')
	MT5_SERVER = os.getenv('MT5_SERVER', '')
	MT5_PATH = os.getenv('MT5_PATH', '') # Path to MT5 terminal (Windows/Wine)

🤖 Prompt for AI Agents

In src/nice_funcs_mt5.py around lines 40 to 44, parsing MT5_LOGIN with
int(os.getenv('MT5_LOGIN', '0')) can raise ValueError for empty or non-numeric
env values; change to validate and coerce safely: read the raw env string, strip
it, check if it represents an integer (e.g., use str.isdigit() or a try/except
around int()), fall back to 0 if invalid or empty, and optionally log a warning;
ensure MT5_LOGIN ends up as an int and that no exception is raised during module
import.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Guard against invalid MIN_ALERT_LEVEL values

Line 59 calls AlertLevel[env_value]. If the environment provides warning (lowercase), whitespace, or any unexpected value, Python raises KeyError during module import and the entire alerting system fails to load. Please normalize and fall back instead of crashing.

-        self.min_level = AlertLevel[os.getenv('MIN_ALERT_LEVEL', 'INFO')]
+        level_name = os.getenv('MIN_ALERT_LEVEL', 'INFO').strip().upper()
+        self.min_level = AlertLevel.__members__.get(level_name, AlertLevel.INFO)

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	self.min_level = AlertLevel[os.getenv('MIN_ALERT_LEVEL', 'INFO')]
	level_name = os.getenv('MIN_ALERT_LEVEL', 'INFO').strip().upper()
	self.min_level = AlertLevel.__members__.get(level_name, AlertLevel.INFO)

🤖 Prompt for AI Agents

In src/utils/alerts.py around lines 59-60, the code directly indexes AlertLevel
with the raw env value which can raise KeyError on lowercase/whitespace/invalid
inputs; change this to read os.getenv(...), normalize with .strip().upper(),
then attempt to resolve AlertLevel[normalized] inside a try/except (or use
AlertLevel.__members__.get(normalized)) and on failure fall back to
AlertLevel.INFO (and emit a non-fatal warning/log message) so module import
never crashes for bad MIN_ALERT_LEVEL values.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Fix potential NameError for undefined avg_duration.

At line 119, avg_duration is referenced outside the conditional block where it's defined (line 113). If durations is empty, the code will raise a NameError.

Apply this diff to fix the issue:

     # Cycle duration distribution
     durations = [c.get('duration_seconds', 0) for c in self.cycles]
     if durations:
         avg_duration = sum(durations) / len(durations)
         print(f"\n  Average Duration: {avg_duration:.1f}s")
         print(f"  Fastest Cycle: {min(durations):.1f}s")
         print(f"  Slowest Cycle: {max(durations):.1f}s")

-    # Identify problematic cycles
-    slow_cycles = [i for i, c in enumerate(self.cycles) if c.get('duration_seconds', 0) > avg_duration * 1.5]
-    if slow_cycles:
-        cprint(f"\n  ⚠️  Slow Cycles (>1.5x avg): {len(slow_cycles)}", "yellow")
-        for cycle_num in slow_cycles[:5]:  # Show first 5
-            print(f"    - Cycle #{cycle_num + 1}: {self.cycles[cycle_num].get('duration_seconds', 0):.1f}s")
+        # Identify problematic cycles
+        slow_cycles = [i for i, c in enumerate(self.cycles) if c.get('duration_seconds', 0) > avg_duration * 1.5]
+        if slow_cycles:
+            cprint(f"\n  ⚠️  Slow Cycles (>1.5x avg): {len(slow_cycles)}", "yellow")
+            for cycle_num in slow_cycles[:5]:  # Show first 5
+                print(f"    - Cycle #{cycle_num + 1}: {self.cycles[cycle_num].get('duration_seconds', 0):.1f}s")

     print()

🤖 Prompt for AI Agents

In tests/analyze_results.py around lines 110 to 124, avg_duration is only set
inside the if durations: block, but later code always references it which can
raise a NameError when durations is empty; fix by ensuring avg_duration is
always defined (e.g., set avg_duration = 0.0 before the if) or move the
slow_cycles calculation and reporting inside the same durations conditional so
avg_duration is only used when durations exist, and keep the slow_cycles logic
guarded to avoid referencing avg_duration when durations is empty.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Correct the UTC timestamp reporting.

date without -u prints the local timezone, yet the script labels it “UTC.” On any host that isn’t already set to UTC this will emit the wrong time and mislead operators correlating log events. Switch to date -u (or set TZ=UTC) so the displayed clock actually matches the UTC suffix.

-date +"⏰ Huidige tijd: %Y-%m-%d %H:%M:%S UTC"
+date -u +"⏰ Huidige tijd: %Y-%m-%d %H:%M:%S UTC"

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	date +"⏰ Huidige tijd: %Y-%m-%d %H:%M:%S UTC"
	date -u +"⏰ Huidge tijd: %Y-%m-%d %H:%M:%S UTC"

🤖 Prompt for AI Agents

In tests/quick_status.sh around line 6, the script prints a timestamp labelled
"UTC" but calls date without UTC support; change the invocation so the printed
time is actually UTC by using date -u (or prefix with TZ=UTC) and keep the
existing format string so the output matches the "UTC" suffix.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Fix double-zero output when there’s no log data.

grep -c … || echo "0" runs the echo whenever no matches exist, but grep -c already prints 0 before exiting with status 1. Inside command substitution this leaves CYCLES (and ERROR_COUNT) equal to “0\n0”, so the dashboard shows duplicate zeros instead of a single count. Wrap the grep in an if or swallow the non-zero exit without emitting another value.

-CYCLES=$(grep -c "Test Cycle #" tests/test_24h_live.log 2>/dev/null || echo "0")
+if CYCLES=$(grep -c "Test Cycle #" tests/test_24h_live.log 2>/dev/null); then
+    :
+else
+    CYCLES=0
+fi
…
-ERROR_COUNT=$(grep -c "❌" tests/test_24h_live.log 2>/dev/null || echo "0")
+if ERROR_COUNT=$(grep -c "❌" tests/test_24h_live.log 2>/dev/null); then
+    :
+else
+    ERROR_COUNT=0
+fi

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	CYCLES=$(grep -c "Test Cycle #" tests/test_24h_live.log 2>/dev/null || echo "0")
	echo " Completed Cycles: $CYCLES"
	echo ""
	echo "🔄 Latest Cycle:"
	grep "Test Cycle #" tests/test_24h_live.log 2>/dev/null | tail -1 || echo " No cycles yet"
	echo ""
	echo "💰 Account Status:"
	grep "Account Balance:" tests/test_24h_live.log 2>/dev/null | tail -1 || echo " No data yet"
	echo ""
	echo "🎯 Recent Decisions:"
	grep -E "(Analyzing|optimal)" tests/test_24h_live.log 2>/dev/null | tail -5
	echo ""
	echo "⚠️ Recent Errors:"
	ERROR_COUNT=$(grep -c "❌" tests/test_24h_live.log 2>/dev/null || echo "0")
	echo " Total Errors: $ERROR_COUNT"
	if [ $ERROR_COUNT -gt 0 ]; then
	grep "❌" tests/test_24h_live.log 2>/dev/null | tail -3
	CYCLES=$(grep -c "Test Cycle #" tests/test_24h_live.log 2>/dev/null)
	if [ -z "$CYCLES" ]; then
	CYCLES=0
	fi
	echo " Completed Cycles: $CYCLES"
	echo ""
	echo "🔄 Latest Cycle:"
	grep "Test Cycle #" tests/test_24h_live.log 2>/dev/null | tail -1 || echo " No cycles yet"
	echo ""
	echo "💰 Account Status:"
	grep "Account Balance:" tests/test_24h_live.log 2>/dev/null | tail -1 || echo " No data yet"
	echo ""
	echo "🎯 Recent Decisions:"
	grep -E "(Analyzing|optimal)" tests/test_24h_live.log 2>/dev/null | tail -5
	echo ""
	echo "⚠️ Recent Errors:"
	ERROR_COUNT=$(grep -c "❌" tests/test_24h_live.log 2>/dev/null)
	if [ -z "$ERROR_COUNT" ]; then
	ERROR_COUNT=0
	fi
	echo " Total Errors: $ERROR_COUNT"
	if [ $ERROR_COUNT -gt 0 ]; then
	grep "❌" tests/test_24h_live.log 2>/dev/null | tail -3

🤖 Prompt for AI Agents

In tests/quick_status.sh around lines 24 to 44, the command substitutions for
CYCLES and ERROR_COUNT use constructs like grep -c ... || echo "0" which cause
duplicate "0" output when grep -c already prints 0; remove the fallback echo and
instead ensure the grep's non-zero exit doesn't emit an extra value by either
replacing "grep -c ... || echo '0'" with "grep -c ... 2>/dev/null || true" or by
wrapping the grep in an if-read block so you assign a single numeric string
(e.g., run grep -c into the variable and swallow the exit status), and apply the
same change to the ERROR_COUNT line so prints show a single zero when no matches
exist.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

Critical bug: Calling infinite loop method instead of single cycle.

Line 98 calls agent.run(), but based on the MT5TradingAgent implementation (see relevant code snippets), run() is an infinite loop with its own sleep cycle. This will cause the test to hang indefinitely on the first cycle instead of running multiple cycles with controlled intervals.

Apply this diff:

         try:
             # Run agent analysis
-            result = agent.run()
+            agent.run_analysis_cycle()
+            result = None  # run_analysis_cycle() doesn't return a value

             # Store results
             if result:

Alternatively, if you need to capture results from the agent, you may need to modify MT5TradingAgent.run_analysis_cycle() to return cycle results, or access agent state after the cycle completes.

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents

In tests/test_24h_trading.py around lines 96 to 104, the test calls agent.run()
which is an infinite-loop method and causes the test to hang; replace that call
with a single-cycle invocation (e.g., agent.run_analysis_cycle()) so the test
executes one cycle and returns control, and if run_analysis_cycle() does not
currently return the cycle result, either update
MT5TradingAgent.run_analysis_cycle() to return the cycle result or read the
agent's state/last_result after the call and use that for assertions; ensure the
test no longer calls any long-running loop or blocks on sleep.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Make this test skip when OpenRouter credentials or network are missing.
Lines [77-144] unconditionally hit model_factory.get_model(...) and model.generate_response(...), which both require a valid OPENROUTER_API_KEY and outbound access. In environments without those (CI, contributors’ laptops), get_model returns None or generate_response raises, so test_mt5_agent() fails even though the MT5 layer is optional. Please guard these calls (e.g., check the env vars and model_factory.get_model result, then pytest.skip or stub the client) so the test only exercises live integrations when credentials are configured.

🧰 Tools

🪛 Ruff (0.14.3)

79-79: f-string without any placeholders

Remove extraneous f prefix

(F541)

---

91-91: f-string without any placeholders

Remove extraneous f prefix

(F541)

---

97-97: Do not catch blind exception: Exception

(BLE001)

---

114-114: Do not catch blind exception: Exception

(BLE001)

---

135-135: f-string without any placeholders

Remove extraneous f prefix

(F541)

🤖 Prompt for AI Agents

In tests/test_mt5_agent_simple.py around lines 77-144, the test unconditionally
calls model_factory.get_model(...) and model.generate_response(...) which
require OpenRouter credentials/network; update the block to first import os and
pytest, then check for the OpenRouter credential (e.g.,
os.environ.get("OPENROUTER_API_KEY")) and if missing call
pytest.skip("OpenRouter credentials not set - skipping live model test"); next
call model_factory.get_model(...) and if it returns None also call
pytest.skip("OpenRouter model not available - skipping live model test");
finally, keep generate_response only when model is present and wrap the call in
try/except that on exception uses pytest.skip("Live model call failed: {e}") so
CI/contributor environments without credentials or network skip instead of
failing.