From d292fbb9d947f1b0990bdf369985b879a91e9f7b Mon Sep 17 00:00:00 2001 From: agent Date: Tue, 23 Jun 2026 07:30:02 +0000 Subject: [PATCH] feat: centralize tick price validation and add resilient position margin helpers (#49, #50) Add _valid_tick_price() internal helper that returns a positive finite float from a tick dict or None for any invalid value (missing, None, NaN, infinite, zero, negative, or unsupported type). Refactor five existing bid/ask validation sites in trading.py to use it, removing duplicated isinstance/isfinite checks. Add calculate_positions_margin_by_symbol() which computes margin per unique symbol independently using the existing strict calculate_positions_margin(), with first-seen deduplication and configurable error suppression (Mt5TradingError, Mt5RuntimeError, AttributeError) via suppress_errors=. Add calculate_positions_margin_safe() as a thin sum wrapper with suppress_errors=True, returning 0.0 on empty or fully-failed inputs. Both new helpers are exported from mt5cli, added to STABLE_SDK_EXPORTS, and documented in docs/api/public-contract.md. Existing strict behavior of calculate_positions_margin() is unchanged. Co-Authored-By: Claude Sonnet 4.6 --- docs/api/public-contract.md | 26 +++--- mt5cli/__init__.py | 4 + mt5cli/contract.py | 2 + mt5cli/trading.py | 131 ++++++++++++++++++++++++++---- tests/test_trading.py | 157 ++++++++++++++++++++++++++++++++++++ 5 files changed, 292 insertions(+), 28 deletions(-) diff --git a/docs/api/public-contract.md b/docs/api/public-contract.md index 85f5c46..15bfa04 100644 --- a/docs/api/public-contract.md +++ b/docs/api/public-contract.md @@ -99,18 +99,20 @@ diagrams. These helpers implement broker-facing calculations only. They do not encode strategy entries, exits, Kelly sizing, or signal logic. -| Symbol | Role | -| -------------------------------------------------------------------------------------------------- | --------------------------------------------- | -| `get_account_snapshot`, `get_symbol_snapshot`, `get_tick_snapshot`, `get_positions_frame` | Normalized account/symbol/tick/position views | -| `detect_position_side` | Net long / short / flat from open positions | -| `calculate_spread_ratio` | Relative bid-ask spread | -| `calculate_margin_and_volume`, `calculate_volume_by_margin`, `calculate_new_position_margin_ratio` | Margin budget and volume sizing | -| `normalize_order_volume`, `estimate_order_margin`, `calculate_positions_margin` | Broker volume normalization and margin totals | -| `determine_order_limits` | SL/TP price levels from ratios | -| `ensure_symbol_selected` | Select/verify Market Watch visibility | -| `place_market_order`, `close_open_positions`, `update_sltp_for_open_positions` | Order execution helpers (`dry_run` supported) | -| `MarginVolume`, `OrderLimits`, `OrderExecutionResult` | Typed return contracts for order helpers | -| `OrderSide`, `OrderFillingMode`, `OrderTimeMode`, `PositionSide`, `ExecutionStatus` | Typed enums for order helpers | +| Symbol | Role | +| -------------------------------------------------------------------------------------------------- | ----------------------------------------------------------- | +| `get_account_snapshot`, `get_symbol_snapshot`, `get_tick_snapshot`, `get_positions_frame` | Normalized account/symbol/tick/position views | +| `detect_position_side` | Net long / short / flat from open positions | +| `calculate_spread_ratio` | Relative bid-ask spread | +| `calculate_margin_and_volume`, `calculate_volume_by_margin`, `calculate_new_position_margin_ratio` | Margin budget and volume sizing | +| `normalize_order_volume`, `estimate_order_margin`, `calculate_positions_margin` | Broker volume normalization and margin totals | +| `calculate_positions_margin_by_symbol` | Per-symbol margin map (resilient, first-seen order) | +| `calculate_positions_margin_safe` | Summed total margin across symbols (failed symbols skipped) | +| `determine_order_limits` | SL/TP price levels from ratios | +| `ensure_symbol_selected` | Select/verify Market Watch visibility | +| `place_market_order`, `close_open_positions`, `update_sltp_for_open_positions` | Order execution helpers (`dry_run` supported) | +| `MarginVolume`, `OrderLimits`, `OrderExecutionResult` | Typed return contracts for order helpers | +| `OrderSide`, `OrderFillingMode`, `OrderTimeMode`, `PositionSide`, `ExecutionStatus` | Typed enums for order helpers | `MT5Client.order_send()` and CLI `order-send --yes` are live execution paths. diff --git a/mt5cli/__init__.py b/mt5cli/__init__.py index 4d32dbf..ac6c2b2 100644 --- a/mt5cli/__init__.py +++ b/mt5cli/__init__.py @@ -119,6 +119,8 @@ from .trading import ( calculate_margin_and_volume, calculate_new_position_margin_ratio, calculate_positions_margin, + calculate_positions_margin_by_symbol, + calculate_positions_margin_safe, calculate_spread_ratio, calculate_volume_by_margin, close_open_positions, @@ -188,6 +190,8 @@ __all__ = [ "calculate_margin_and_volume", "calculate_new_position_margin_ratio", "calculate_positions_margin", + "calculate_positions_margin_by_symbol", + "calculate_positions_margin_safe", "calculate_spread_ratio", "calculate_volume_by_margin", "call_with_normalized_errors", diff --git a/mt5cli/contract.py b/mt5cli/contract.py index b058ab2..8f094f5 100644 --- a/mt5cli/contract.py +++ b/mt5cli/contract.py @@ -31,6 +31,8 @@ STABLE_SDK_EXPORTS: frozenset[str] = frozenset({ "calculate_margin_and_volume", "calculate_new_position_margin_ratio", "calculate_positions_margin", + "calculate_positions_margin_by_symbol", + "calculate_positions_margin_safe", "calculate_spread_ratio", "calculate_volume_by_margin", "call_with_normalized_errors", diff --git a/mt5cli/trading.py b/mt5cli/trading.py index ffadc33..5a779bb 100644 --- a/mt5cli/trading.py +++ b/mt5cli/trading.py @@ -2,13 +2,14 @@ from __future__ import annotations +import logging from contextlib import contextmanager from math import floor, isfinite from numbers import Integral, Real from typing import TYPE_CHECKING, Literal, TypedDict, cast import pandas as pd -from pdmt5 import Mt5Config, Mt5TradingClient, Mt5TradingError +from pdmt5 import Mt5Config, Mt5RuntimeError, Mt5TradingClient, Mt5TradingError from .history import drop_forming_rate_bar from .sdk import build_config @@ -16,7 +17,9 @@ from .utils import coerce_login as _coerce_login from .utils import parse_timeframe if TYPE_CHECKING: - from collections.abc import Iterator, Sequence + from collections.abc import Iterator, Mapping, Sequence + +_logger = logging.getLogger(__name__) PositionSide = Literal["long", "short"] OrderSide = Literal["BUY", "SELL"] @@ -128,6 +131,8 @@ __all__ = [ "calculate_margin_and_volume", "calculate_new_position_margin_ratio", "calculate_positions_margin", + "calculate_positions_margin_by_symbol", + "calculate_positions_margin_safe", "calculate_spread_ratio", "calculate_volume_by_margin", "close_open_positions", @@ -423,6 +428,30 @@ def _optional_price(value: object) -> float | None: return price +def _valid_tick_price(tick: Mapping[str, object], key: str) -> float | None: + """Return a positive finite float from tick[key], or None if invalid. + + Accepts int, float, or numeric string values. Returns None when the key is + missing, the value is None, non-numeric, NaN, infinite, zero, or negative. + Booleans are treated as non-numeric and return None. + """ + value = tick.get(key) + if value is None or isinstance(value, bool): + return None + if isinstance(value, int | float): + price = float(value) + elif isinstance(value, str): + try: + price = float(value) + except ValueError: + return None + else: + return None + if not isfinite(price) or price <= 0: + return None + return price + + def _success_retcodes(mt5: object) -> frozenset[int]: values = { value @@ -461,9 +490,10 @@ def _calculate_min_volume_if_affordable( msg = f"Invalid volume constraints for {symbol!r}." raise Mt5TradingError(msg) side = _normalize_order_side(order_side) - tick = get_tick_snapshot(client, symbol) - price = tick["ask"] if side == "BUY" else tick["bid"] - if not isinstance(price, int | float) or price <= 0: + price = _valid_tick_price( + get_tick_snapshot(client, symbol), "ask" if side == "BUY" else "bid" + ) + if price is None: msg = f"Tick price is unavailable for {symbol!r}." raise Mt5TradingError(msg) order_type = ( @@ -621,14 +651,14 @@ def estimate_order_margin( raise Mt5TradingError(msg) side = _normalize_order_side(order_side) tick = get_tick_snapshot(client, symbol) - price = tick["ask"] if side == "BUY" else tick["bid"] - if not isinstance(price, int | float) or price <= 0 or not isfinite(price): + price = _valid_tick_price(tick, "ask" if side == "BUY" else "bid") + if price is None: msg = f"Tick price is unavailable for {symbol!r}." raise Mt5TradingError(msg) order_type = ( client.mt5.ORDER_TYPE_BUY if side == "BUY" else client.mt5.ORDER_TYPE_SELL ) - raw_margin = client.order_calc_margin(order_type, symbol, volume, float(price)) + raw_margin = client.order_calc_margin(order_type, symbol, volume, price) try: margin = float(raw_margin) except (TypeError, ValueError) as exc: @@ -682,6 +712,72 @@ def calculate_positions_margin( return total +def calculate_positions_margin_by_symbol( + client: Mt5TradingClient, + *, + symbols: Sequence[str], + suppress_errors: bool = True, +) -> dict[str, float]: + """Return per-symbol estimated margin for open positions. + + Computes margin for each unique input symbol independently using the strict + :func:`calculate_positions_margin` helper. Duplicates are deduplicated in + first-seen order. + + Args: + client: Connected ``Mt5TradingClient`` instance. + symbols: Symbols to compute margin for. + suppress_errors: When ``True``, log and skip symbols that raise + ``Mt5TradingError``, ``Mt5RuntimeError``, or ``AttributeError``. + When ``False``, re-raise the first failure. + + Returns: + Mapping of symbol to margin total in first-seen unique-symbol order. + Returns an empty dict when ``symbols`` is empty or all symbols fail + with ``suppress_errors=True``. + + Raises: + Mt5TradingError: When a symbol raises ``Mt5TradingError`` and + ``suppress_errors=False``. + Mt5RuntimeError: When a symbol raises ``Mt5RuntimeError`` and + ``suppress_errors=False``. + AttributeError: When a symbol raises ``AttributeError`` and + ``suppress_errors=False``. + """ + result: dict[str, float] = {} + for symbol in dict.fromkeys(symbols): + try: + result[symbol] = calculate_positions_margin(client, symbols=[symbol]) + except (Mt5TradingError, Mt5RuntimeError, AttributeError) as exc: + if not suppress_errors: + raise + _logger.warning("Skipping margin for %r: %s", symbol, exc) + return result + + +def calculate_positions_margin_safe( + client: Mt5TradingClient, + *, + symbols: Sequence[str], +) -> float: + """Return the total estimated margin for open positions across symbols. + + Internally calls :func:`calculate_positions_margin_by_symbol` with + ``suppress_errors=True``. Failed symbols are silently skipped. + + Args: + client: Connected ``Mt5TradingClient`` instance. + symbols: Symbols to include. + + Returns: + Sum of per-symbol margins; ``0.0`` when no symbols or all fail. + """ + return sum( + calculate_positions_margin_by_symbol(client, symbols=symbols).values(), + 0.0, + ) + + def calculate_spread_ratio(client: Mt5TradingClient, symbol: str) -> float: """Return ``(ask - bid) / ((ask + bid) / 2)`` for the latest tick. @@ -720,9 +816,10 @@ def calculate_new_position_margin_ratio( margin = float(account.get("margin") or 0.0) if new_position_side is not None and new_position_volume > 0: side = _normalize_order_side(new_position_side) - tick = get_tick_snapshot(client, symbol) - price = tick["ask"] if side == "BUY" else tick["bid"] - if not isinstance(price, int | float) or price <= 0: + price = _valid_tick_price( + get_tick_snapshot(client, symbol), "ask" if side == "BUY" else "bid" + ) + if price is None: msg = f"Tick price is unavailable for {symbol!r}." raise Mt5TradingError(msg) order_type = ( @@ -827,8 +924,10 @@ def calculate_volume_by_margin( msg = f"Invalid volume constraints for {symbol!r}." raise Mt5TradingError(msg) side = _normalize_order_side(order_side) - price = get_tick_snapshot(client, symbol)["ask" if side == "BUY" else "bid"] - if not isinstance(price, int | float) or price <= 0: + price = _valid_tick_price( + get_tick_snapshot(client, symbol), "ask" if side == "BUY" else "bid" + ) + if price is None: msg = f"Tick price is unavailable for {symbol!r}." raise Mt5TradingError(msg) order_type = ( @@ -984,8 +1083,8 @@ def place_market_order( if not dry_run: ensure_symbol_selected(client, symbol) tick = get_tick_snapshot(client, symbol) - price = tick["ask"] if side == "BUY" else tick["bid"] - if not isinstance(price, int | float) or price <= 0: + price = _valid_tick_price(tick, "ask" if side == "BUY" else "bid") + if price is None: msg = f"Tick price is unavailable for {symbol!r}." raise Mt5TradingError(msg) request = { @@ -995,7 +1094,7 @@ def place_market_order( "type": ( client.mt5.ORDER_TYPE_BUY if side == "BUY" else client.mt5.ORDER_TYPE_SELL ), - "price": float(price), + "price": price, "type_filling": _resolve_mt5_constant( client.mt5, "ORDER_FILLING", diff --git a/tests/test_trading.py b/tests/test_trading.py index 19b509e..35a4077 100644 --- a/tests/test_trading.py +++ b/tests/test_trading.py @@ -18,9 +18,12 @@ from mt5cli.trading import ( MarginVolume, OrderExecutionResult, OrderLimits, + _valid_tick_price, # type: ignore[reportPrivateUsage] calculate_margin_and_volume, calculate_new_position_margin_ratio, calculate_positions_margin, + calculate_positions_margin_by_symbol, + calculate_positions_margin_safe, calculate_spread_ratio, calculate_volume_by_margin, close_open_positions, @@ -2979,3 +2982,157 @@ class TestFetchLatestClosedRatesIndexed: assert "open" in result.columns assert "close" in result.columns assert isinstance(result.index, pd.DatetimeIndex) + + +class TestValidTickPrice: + """Tests for the _valid_tick_price internal helper (#49).""" + + def test_valid_positive_float(self) -> None: + """Returns a positive float value unchanged.""" + _assert_close(_valid_tick_price({"bid": 1.1000}, "bid"), 1.1000) + + def test_valid_positive_int(self) -> None: + """Accepts an integer value and returns it as float.""" + result = _valid_tick_price({"bid": 2}, "bid") + _assert_close(result, 2.0) + assert isinstance(result, float) + + def test_valid_numeric_string(self) -> None: + """Accepts a numeric string and returns the parsed float.""" + _assert_close(_valid_tick_price({"bid": "1.5"}, "bid"), 1.5) + + def test_missing_key(self) -> None: + """Returns None when the key is absent from the tick dict.""" + assert _valid_tick_price({}, "bid") is None + + def test_none_value(self) -> None: + """Returns None when the stored value is None.""" + assert _valid_tick_price({"bid": None}, "bid") is None + + def test_invalid_string(self) -> None: + """Returns None for a non-numeric string.""" + assert _valid_tick_price({"bid": "not_a_number"}, "bid") is None + + def test_nan(self) -> None: + """Returns None for a NaN float.""" + assert _valid_tick_price({"bid": float("nan")}, "bid") is None + + def test_positive_infinity(self) -> None: + """Returns None for positive infinity.""" + assert _valid_tick_price({"bid": float("inf")}, "bid") is None + + def test_negative_infinity(self) -> None: + """Returns None for negative infinity.""" + assert _valid_tick_price({"bid": float("-inf")}, "bid") is None + + def test_zero(self) -> None: + """Returns None for zero (not a valid price).""" + assert _valid_tick_price({"bid": 0.0}, "bid") is None + + def test_negative_value(self) -> None: + """Returns None for a negative price.""" + assert _valid_tick_price({"bid": -1.0}, "bid") is None + + def test_unsupported_type(self) -> None: + """Returns None for unsupported value types such as list.""" + assert _valid_tick_price({"bid": [1.0]}, "bid") is None + + +class TestCalculatePositionsMarginBySymbol: + """Tests for calculate_positions_margin_by_symbol (#50).""" + + def test_all_symbols_succeed(self, mocker: MockerFixture) -> None: + """Returns one entry per symbol when all margin calls succeed.""" + client = _mock_trade_client() + mocker.patch( + "mt5cli.trading.calculate_positions_margin", + side_effect=[12.5, 30.0], + ) + + result = calculate_positions_margin_by_symbol( + client, symbols=["EURUSD", "GBPUSD"] + ) + + assert result == {"EURUSD": 12.5, "GBPUSD": 30.0} + + def test_one_symbol_fails_suppress_errors_true(self, mocker: MockerFixture) -> None: + """Skips the failing symbol and returns the successful one.""" + client = _mock_trade_client() + mocker.patch( + "mt5cli.trading.calculate_positions_margin", + side_effect=[Mt5TradingError("tick unavailable"), 30.0], + ) + + result = calculate_positions_margin_by_symbol( + client, symbols=["EURUSD", "GBPUSD"], suppress_errors=True + ) + + assert result == {"GBPUSD": 30.0} + + def test_one_symbol_fails_suppress_errors_false( + self, mocker: MockerFixture + ) -> None: + """Re-raises the first failure when suppress_errors=False.""" + client = _mock_trade_client() + mocker.patch( + "mt5cli.trading.calculate_positions_margin", + side_effect=[Mt5TradingError("tick unavailable"), 30.0], + ) + + with pytest.raises(Mt5TradingError, match="tick unavailable"): + calculate_positions_margin_by_symbol( + client, symbols=["EURUSD", "GBPUSD"], suppress_errors=False + ) + + def test_all_symbols_fail_suppress_errors_true(self, mocker: MockerFixture) -> None: + """Returns an empty dict when all symbols fail with suppress_errors=True.""" + client = _mock_trade_client() + mocker.patch( + "mt5cli.trading.calculate_positions_margin", + side_effect=[Mt5TradingError("err1"), Mt5TradingError("err2")], + ) + + result = calculate_positions_margin_by_symbol( + client, symbols=["EURUSD", "GBPUSD"], suppress_errors=True + ) + + assert result == {} + + def test_empty_symbol_list(self) -> None: + """Returns an empty dict for an empty input list.""" + client = _mock_trade_client() + result = calculate_positions_margin_by_symbol(client, symbols=[]) + assert result == {} + + def test_duplicate_symbols_preserve_first_seen_order( + self, mocker: MockerFixture + ) -> None: + """Processes each unique symbol once in first-seen order.""" + client = _mock_trade_client() + mock_calc = mocker.patch( + "mt5cli.trading.calculate_positions_margin", + return_value=12.5, + ) + + result = calculate_positions_margin_by_symbol( + client, symbols=["EURUSD", "GBPUSD", "EURUSD"] + ) + + assert list(result.keys()) == ["EURUSD", "GBPUSD"] + assert mock_calc.call_count == 2 + + +class TestCalculatePositionsMarginSafe: + """Tests for calculate_positions_margin_safe (#50).""" + + def test_returns_summed_total(self, mocker: MockerFixture) -> None: + """Returns the sum of all per-symbol margins.""" + client = _mock_trade_client() + mocker.patch( + "mt5cli.trading.calculate_positions_margin", + side_effect=[12.5, 30.0], + ) + + total = calculate_positions_margin_safe(client, symbols=["EURUSD", "GBPUSD"]) + + _assert_close(total, 42.5)