From 1c57be5c44f7d514453e09311807bfdbe54889d3 Mon Sep 17 00:00:00 2001 From: agent Date: Tue, 23 Jun 2026 09:48:13 +0000 Subject: [PATCH] fix: centralize tick price validation in calculate_spread_ratio and determine_order_limits (#52) Replaces manual isinstance/<=0 checks in calculate_spread_ratio() and determine_order_limits() with _valid_tick_price(), ensuring NaN, inf, -inf, zero, negative, bool, and invalid-string tick values are consistently rejected across all trading helpers. Adds regression tests covering numeric-string acceptance and every invalid-value category for both functions. Co-Authored-By: Claude Sonnet 4.6 --- mt5cli/trading.py | 19 ++++----- tests/test_trading.py | 93 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 101 insertions(+), 11 deletions(-) diff --git a/mt5cli/trading.py b/mt5cli/trading.py index 5a779bb..974f4eb 100644 --- a/mt5cli/trading.py +++ b/mt5cli/trading.py @@ -782,18 +782,15 @@ def calculate_spread_ratio(client: Mt5TradingClient, symbol: str) -> float: """Return ``(ask - bid) / ((ask + bid) / 2)`` for the latest tick. Raises: - Mt5TradingError: If bid or ask is unavailable or non-positive. + Mt5TradingError: If bid or ask is unavailable. """ tick = get_tick_snapshot(client, symbol) - bid = tick.get("bid") - ask = tick.get("ask") - if not isinstance(bid, int | float) or not isinstance(ask, int | float): + bid = _valid_tick_price(tick, "bid") + ask = _valid_tick_price(tick, "ask") + if bid is None or ask is None: msg = f"Tick bid/ask is unavailable for {symbol!r}." raise Mt5TradingError(msg) - if bid <= 0 or ask <= 0: - msg = f"Tick bid/ask must be positive for {symbol!r}." - raise Mt5TradingError(msg) - return (float(ask) - float(bid)) / ((float(ask) + float(bid)) / 2.0) + return (ask - bid) / ((ask + bid) / 2.0) def calculate_new_position_margin_ratio( @@ -1003,11 +1000,11 @@ def determine_order_limits( _require_protective_ratio(take_profit_ratio, "take_profit_limit_ratio") normalized_side = _position_side_from_order_side(side) tick = get_tick_snapshot(client, symbol) - entry_value = tick["ask"] if normalized_side == "long" else tick["bid"] - if not isinstance(entry_value, int | float): + entry_key = "ask" if normalized_side == "long" else "bid" + entry = _valid_tick_price(tick, entry_key) + if entry is None: msg = f"Tick price is unavailable for {symbol!r}." raise Mt5TradingError(msg) - entry = float(entry_value) try: symbol_info = get_symbol_snapshot(client, symbol) except (AttributeError, KeyError, TypeError, ValueError): diff --git a/tests/test_trading.py b/tests/test_trading.py index 35a4077..e3ee2c7 100644 --- a/tests/test_trading.py +++ b/tests/test_trading.py @@ -427,6 +427,55 @@ class TestDetermineOrderLimits: with pytest.raises(Mt5TradingError, match="Tick price is unavailable"): determine_order_limits(client, "EURUSD", "long") + def test_accepts_numeric_string_entry(self) -> None: + """Test numeric string ask/bid values are accepted as entry prices.""" + client = MagicMock() + client.symbol_info_tick_as_dict.return_value = { + "ask": "1.1010", + "bid": "1.1000", + } + client.symbol_info_as_dict.return_value = {"digits": 4} + + result = determine_order_limits(client, "EURUSD", "long") + + _assert_close(result["entry"], 1.1010) + + @pytest.mark.parametrize( + ("side", "field", "bad_value"), + [ + ("long", "ask", float("nan")), + ("long", "ask", float("inf")), + ("long", "ask", float("-inf")), + ("long", "ask", 0.0), + ("long", "ask", -1.0), + ("long", "ask", True), + ("long", "ask", False), + ("long", "ask", "invalid"), + ("short", "bid", float("nan")), + ("short", "bid", float("inf")), + ("short", "bid", float("-inf")), + ("short", "bid", 0.0), + ("short", "bid", -1.0), + ("short", "bid", True), + ("short", "bid", False), + ("short", "bid", "invalid"), + ], + ) + def test_rejects_invalid_entry_tick_values( + self, + side: str, + field: str, + bad_value: object, + ) -> None: + """Test invalid entry tick values raise Mt5TradingError.""" + tick: dict[str, object] = {"ask": 1.1, "bid": 1.0} + tick[field] = bad_value + client = MagicMock() + client.symbol_info_tick_as_dict.return_value = tick + + with pytest.raises(Mt5TradingError, match="Tick price is unavailable"): + determine_order_limits(client, "EURUSD", side) + def test_rejects_stop_loss_inside_broker_stop_level(self) -> None: """Test stop-loss prices closer than trade_stops_level raise Mt5TradingError.""" client = MagicMock() @@ -793,6 +842,50 @@ class TestSnapshotsAndState: with pytest.raises(Mt5TradingError): calculate_spread_ratio(client, "EURUSD") + def test_calculate_spread_ratio_accepts_numeric_string_tick(self) -> None: + """Test numeric string bid/ask values are accepted.""" + client = MagicMock() + client.symbol_info_tick_as_dict.return_value = {"bid": "99.0", "ask": "101.0"} + + _assert_close(calculate_spread_ratio(client, "EURUSD"), 0.02) + + @pytest.mark.parametrize( + ("field", "bad_value"), + [ + ("bid", None), + ("bid", float("nan")), + ("bid", float("inf")), + ("bid", float("-inf")), + ("bid", 0.0), + ("bid", -1.0), + ("bid", True), + ("bid", False), + ("bid", "invalid"), + ("ask", None), + ("ask", float("nan")), + ("ask", float("inf")), + ("ask", float("-inf")), + ("ask", 0.0), + ("ask", -1.0), + ("ask", True), + ("ask", False), + ("ask", "invalid"), + ], + ) + def test_calculate_spread_ratio_rejects_invalid_tick_values( + self, + field: str, + bad_value: object, + ) -> None: + """Test invalid bid/ask values raise Mt5TradingError.""" + tick: dict[str, object] = {"bid": 100.0, "ask": 100.0} + tick[field] = bad_value + client = MagicMock() + client.symbol_info_tick_as_dict.return_value = tick + + with pytest.raises(Mt5TradingError, match="Tick bid/ask is unavailable"): + calculate_spread_ratio(client, "EURUSD") + class TestNormalizeOrderVolume: """Tests for normalize_order_volume."""