* feat: add close-positions CLI command and replace_symbol projection mode (#65 #66) Part 1 — close-positions CLI (#65): - Add `close-positions` subcommand delegating to `close_open_positions()`. - Accepts repeated `--symbol` and `--ticket` filters (AND semantics). - Supports `--dry-run` (no `--yes` required); live execution requires `--yes`. - Fails closed with `BadParameter` when neither `--symbol` nor `--ticket` is given. - Exports normalized `OrderExecutionResult` list as a DataFrame (request/response serialized as JSON strings for clean CSV/JSON/Parquet/SQLite output). - `order-send` remains the raw expert path; `close-positions` is the safer high-level helper that builds correct close requests automatically. Part 2 — ProjectionMode and replace_symbol (#66): - Add `ProjectionMode = Literal["add", "replace_symbol"]` type alias. - Add optional `projection_mode` parameter to `calculate_symbol_group_margin_ratio`. Default `"add"` preserves existing additive behavior. `"replace_symbol"` subtracts current margin for `new_symbol`, then adds candidate margin — the subtraction and addition are atomic (suppressed together). - Export `ProjectionMode` from `mt5cli` and add to `STABLE_SDK_EXPORTS`. - No mteor-specific strategy, risk-threshold, or policy logic added. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * chore: remove unused ProjectionMode import in test_contracts.py The parametrized test_stable_exports_are_importable_from_package_root already covers ProjectionMode via hasattr(mt5cli, name). Ruff correctly flagged the explicit top-level import as unused (F401). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Bump version to v0.9.6 * fix: address PR #67 review feedback - Floor replace_symbol margin subtraction at zero to prevent negative ratio - Serialize response unconditionally via json.dumps (null for dry-run rows) - Return a schema-preserving empty DataFrame when results list is empty - Add test: --dry-run --yes precedence (dry-run wins, no order_send) - Add test: zero-match filter produces empty JSON array with exit 0 - Move projection_mode prose to stable trading section in docs Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat: add runtime validation for projection_mode in calculate_symbol_group_margin_ratio Unsupported values previously silently fell through as "add". The new _validate_projection_mode helper raises ValueError with a message that names the bad value and the two accepted modes. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: agent <agent@localhost> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -740,6 +740,306 @@ class TestCommands:
|
||||
assert "must be a JSON object" in normalize_cli_output(result.output)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# close-positions command
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def _build_mock_trading_client() -> MagicMock:
|
||||
"""Return a MagicMock Mt5TradingClient with trading constants set."""
|
||||
client = MagicMock()
|
||||
client.mt5.POSITION_TYPE_BUY = 0
|
||||
client.mt5.POSITION_TYPE_SELL = 1
|
||||
client.mt5.ORDER_TYPE_BUY = 10
|
||||
client.mt5.ORDER_TYPE_SELL = 11
|
||||
client.mt5.TRADE_ACTION_DEAL = 20
|
||||
client.mt5.ORDER_FILLING_IOC = 30
|
||||
client.mt5.ORDER_TIME_GTC = 40
|
||||
client.mt5.TRADE_RETCODE_DONE = 10009
|
||||
client.mt5.TRADE_RETCODE_PLACED = 10008
|
||||
client.mt5.TRADE_RETCODE_DONE_PARTIAL = 10010
|
||||
return client
|
||||
|
||||
|
||||
class TestClosePositions:
|
||||
"""Tests for the close-positions command."""
|
||||
|
||||
@pytest.fixture
|
||||
def trading_client(self, mocker: MockerFixture) -> MagicMock:
|
||||
"""Patch create_trading_client and return a mock trading client."""
|
||||
client = _build_mock_trading_client()
|
||||
client.positions_get_as_df.return_value = pd.DataFrame([
|
||||
{"ticket": 1, "symbol": "JP225", "type": 0, "volume": 1.0},
|
||||
{"ticket": 2, "symbol": "EURUSD", "type": 1, "volume": 0.5},
|
||||
])
|
||||
client.symbol_info_tick_as_dict.return_value = {"ask": 1.2, "bid": 1.1}
|
||||
mocker.patch("mt5cli.cli.create_trading_client", return_value=client)
|
||||
return client
|
||||
|
||||
def test_dry_run_does_not_require_yes(
|
||||
self,
|
||||
tmp_path: Path,
|
||||
trading_client: MagicMock,
|
||||
) -> None:
|
||||
"""Test --dry-run mode succeeds without --yes."""
|
||||
output = tmp_path / "close.json"
|
||||
result = runner.invoke(
|
||||
app,
|
||||
["-o", str(output), "close-positions", "--symbol", "JP225", "--dry-run"],
|
||||
)
|
||||
assert result.exit_code == 0, result.output
|
||||
assert output.exists()
|
||||
trading_client.order_send.assert_not_called()
|
||||
trading_client.shutdown.assert_called_once()
|
||||
|
||||
def test_live_requires_yes(
|
||||
self,
|
||||
tmp_path: Path,
|
||||
trading_client: MagicMock,
|
||||
) -> None:
|
||||
"""Test live close-positions fails without --yes."""
|
||||
output = tmp_path / "close.json"
|
||||
result = runner.invoke(
|
||||
app,
|
||||
["-o", str(output), "close-positions", "--symbol", "JP225"],
|
||||
)
|
||||
assert result.exit_code != 0
|
||||
assert "Pass --yes" in normalize_cli_output(result.output)
|
||||
trading_client.order_send.assert_not_called()
|
||||
|
||||
def test_live_with_yes_calls_order_send(
|
||||
self,
|
||||
tmp_path: Path,
|
||||
trading_client: MagicMock,
|
||||
) -> None:
|
||||
"""Test --yes triggers live execution for matching positions."""
|
||||
trading_client.order_send.return_value = {"retcode": 10009, "comment": "ok"}
|
||||
output = tmp_path / "close.json"
|
||||
result = runner.invoke(
|
||||
app,
|
||||
["-o", str(output), "close-positions", "--symbol", "JP225", "--yes"],
|
||||
)
|
||||
assert result.exit_code == 0, result.output
|
||||
trading_client.order_send.assert_called_once()
|
||||
trading_client.shutdown.assert_called_once()
|
||||
|
||||
def test_symbol_filter_passed_through(
|
||||
self,
|
||||
tmp_path: Path,
|
||||
trading_client: MagicMock,
|
||||
) -> None:
|
||||
"""Test --symbol values are used to filter positions."""
|
||||
output = tmp_path / "close.json"
|
||||
result = runner.invoke(
|
||||
app,
|
||||
[
|
||||
"-o",
|
||||
str(output),
|
||||
"close-positions",
|
||||
"--symbol",
|
||||
"JP225",
|
||||
"--dry-run",
|
||||
],
|
||||
)
|
||||
assert result.exit_code == 0, result.output
|
||||
data = json.loads(output.read_text())
|
||||
assert len(data) == 1
|
||||
assert data[0]["symbol"] == "JP225"
|
||||
trading_client.shutdown.assert_called_once()
|
||||
|
||||
def test_multiple_symbols_filter(
|
||||
self,
|
||||
tmp_path: Path,
|
||||
trading_client: MagicMock,
|
||||
) -> None:
|
||||
"""Test multiple --symbol options are combined."""
|
||||
output = tmp_path / "close.json"
|
||||
result = runner.invoke(
|
||||
app,
|
||||
[
|
||||
"-o",
|
||||
str(output),
|
||||
"close-positions",
|
||||
"--symbol",
|
||||
"JP225",
|
||||
"--symbol",
|
||||
"EURUSD",
|
||||
"--dry-run",
|
||||
],
|
||||
)
|
||||
assert result.exit_code == 0, result.output
|
||||
data = json.loads(output.read_text())
|
||||
assert len(data) == 2
|
||||
symbols = {row["symbol"] for row in data}
|
||||
assert symbols == {"JP225", "EURUSD"}
|
||||
trading_client.shutdown.assert_called_once()
|
||||
|
||||
def test_ticket_filter_passed_through(
|
||||
self,
|
||||
tmp_path: Path,
|
||||
trading_client: MagicMock,
|
||||
) -> None:
|
||||
"""Test --ticket values are used to filter positions."""
|
||||
output = tmp_path / "close.json"
|
||||
result = runner.invoke(
|
||||
app,
|
||||
[
|
||||
"-o",
|
||||
str(output),
|
||||
"close-positions",
|
||||
"--ticket",
|
||||
"2",
|
||||
"--dry-run",
|
||||
],
|
||||
)
|
||||
assert result.exit_code == 0, result.output
|
||||
data = json.loads(output.read_text())
|
||||
assert len(data) == 1
|
||||
assert data[0]["symbol"] == "EURUSD"
|
||||
trading_client.shutdown.assert_called_once()
|
||||
|
||||
def test_symbol_and_ticket_combined(
|
||||
self,
|
||||
tmp_path: Path,
|
||||
trading_client: MagicMock,
|
||||
) -> None:
|
||||
"""Test --symbol and --ticket apply AND semantics when combined."""
|
||||
output = tmp_path / "close.json"
|
||||
result = runner.invoke(
|
||||
app,
|
||||
[
|
||||
"-o",
|
||||
str(output),
|
||||
"close-positions",
|
||||
"--symbol",
|
||||
"JP225",
|
||||
"--ticket",
|
||||
"1",
|
||||
"--dry-run",
|
||||
],
|
||||
)
|
||||
assert result.exit_code == 0, result.output
|
||||
data = json.loads(output.read_text())
|
||||
# symbol=JP225 AND ticket=1 → exactly one match
|
||||
assert len(data) == 1
|
||||
assert data[0]["symbol"] == "JP225"
|
||||
trading_client.shutdown.assert_called_once()
|
||||
|
||||
def test_missing_symbol_and_ticket_fails(
|
||||
self,
|
||||
tmp_path: Path,
|
||||
mocker: MockerFixture,
|
||||
) -> None:
|
||||
"""Test that omitting both --symbol and --ticket fails closed."""
|
||||
mocker.patch("mt5cli.cli.create_trading_client")
|
||||
output = tmp_path / "close.json"
|
||||
result = runner.invoke(
|
||||
app,
|
||||
["-o", str(output), "close-positions", "--dry-run"],
|
||||
)
|
||||
assert result.exit_code != 0
|
||||
assert "symbol" in normalize_cli_output(result.output).lower()
|
||||
|
||||
def test_output_export_dry_run(
|
||||
self,
|
||||
tmp_path: Path,
|
||||
trading_client: MagicMock,
|
||||
) -> None:
|
||||
"""Test dry-run results export with status=dry_run."""
|
||||
output = tmp_path / "close.json"
|
||||
result = runner.invoke(
|
||||
app,
|
||||
["-o", str(output), "close-positions", "--symbol", "JP225", "--dry-run"],
|
||||
)
|
||||
assert result.exit_code == 0, result.output
|
||||
trading_client.shutdown.assert_called_once()
|
||||
data = json.loads(output.read_text())
|
||||
assert data[0]["status"] == "dry_run"
|
||||
assert data[0]["dry_run"] is True
|
||||
assert data[0]["order_side"] == "SELL"
|
||||
|
||||
def test_order_send_unchanged(
|
||||
self,
|
||||
tmp_path: Path,
|
||||
mock_client: MagicMock,
|
||||
) -> None:
|
||||
"""Test that order-send behavior is unchanged by close-positions addition."""
|
||||
output = tmp_path / "out.csv"
|
||||
request = json.dumps({"action": 1, "symbol": "EURUSD", "volume": 0.1})
|
||||
result = runner.invoke(
|
||||
app,
|
||||
["-o", str(output), "order-send", "--request", request, "--yes"],
|
||||
)
|
||||
assert result.exit_code == 0, result.output
|
||||
mock_client.order_send_as_df.assert_called_once()
|
||||
|
||||
def test_shutdown_called_on_close_error(
|
||||
self,
|
||||
tmp_path: Path,
|
||||
mocker: MockerFixture,
|
||||
) -> None:
|
||||
"""Test that shutdown is called even when close_open_positions raises."""
|
||||
client = _build_mock_trading_client()
|
||||
client.positions_get_as_df.side_effect = RuntimeError("connection lost")
|
||||
mocker.patch("mt5cli.cli.create_trading_client", return_value=client)
|
||||
output = tmp_path / "close.json"
|
||||
result = runner.invoke(
|
||||
app,
|
||||
["-o", str(output), "close-positions", "--symbol", "JP225", "--dry-run"],
|
||||
)
|
||||
assert result.exit_code != 0
|
||||
client.shutdown.assert_called_once()
|
||||
|
||||
def test_dry_run_wins_over_yes(
|
||||
self,
|
||||
tmp_path: Path,
|
||||
trading_client: MagicMock,
|
||||
) -> None:
|
||||
"""Test that --dry-run takes precedence when combined with --yes."""
|
||||
output = tmp_path / "close.json"
|
||||
result = runner.invoke(
|
||||
app,
|
||||
[
|
||||
"-o",
|
||||
str(output),
|
||||
"close-positions",
|
||||
"--symbol",
|
||||
"JP225",
|
||||
"--dry-run",
|
||||
"--yes",
|
||||
],
|
||||
)
|
||||
assert result.exit_code == 0, result.output
|
||||
trading_client.order_send.assert_not_called()
|
||||
trading_client.shutdown.assert_called_once()
|
||||
|
||||
def test_no_matching_positions_exports_empty_result(
|
||||
self,
|
||||
tmp_path: Path,
|
||||
trading_client: MagicMock,
|
||||
) -> None:
|
||||
"""Test that zero filter matches produces an empty JSON array."""
|
||||
trading_client.positions_get_as_df.return_value = pd.DataFrame([
|
||||
{"ticket": 1, "symbol": "JP225", "type": 0, "volume": 1.0},
|
||||
])
|
||||
output = tmp_path / "close.json"
|
||||
result = runner.invoke(
|
||||
app,
|
||||
[
|
||||
"-o",
|
||||
str(output),
|
||||
"close-positions",
|
||||
"--symbol",
|
||||
"NONEXISTENT",
|
||||
"--dry-run",
|
||||
],
|
||||
)
|
||||
assert result.exit_code == 0, result.output
|
||||
trading_client.shutdown.assert_called_once()
|
||||
assert output.exists()
|
||||
assert json.loads(output.read_text()) == []
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Callback / shared options
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
+198
-1
@@ -4,7 +4,7 @@ from __future__ import annotations
|
||||
|
||||
import logging
|
||||
from types import SimpleNamespace
|
||||
from typing import Any, cast
|
||||
from typing import Any, cast, get_args
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
import pandas as pd
|
||||
@@ -19,6 +19,7 @@ from mt5cli.trading import (
|
||||
MarginVolume,
|
||||
OrderExecutionResult,
|
||||
OrderLimits,
|
||||
ProjectionMode,
|
||||
calculate_account_projected_margin_ratio,
|
||||
calculate_margin_and_volume,
|
||||
calculate_new_position_margin_ratio,
|
||||
@@ -2065,6 +2066,202 @@ class TestVolumeAndExecution:
|
||||
suppress_errors=False,
|
||||
)
|
||||
|
||||
def test_symbol_group_margin_ratio_replace_symbol_subtracts_and_adds(
|
||||
self,
|
||||
mocker: MockerFixture,
|
||||
) -> None:
|
||||
"""Test replace_symbol mode subtracts current exposure and adds candidate."""
|
||||
client = _mock_trade_client()
|
||||
client.account_info_as_dict.return_value = {"equity": 1000.0}
|
||||
client.symbol_info_tick_as_dict.return_value = {"ask": 1.101, "bid": 1.1}
|
||||
client.order_calc_margin.return_value = 15.0
|
||||
mocker.patch(
|
||||
"mt5cli.trading.calculate_positions_margin_by_symbol",
|
||||
return_value={"EURUSD": 25.0},
|
||||
)
|
||||
|
||||
result = calculate_symbol_group_margin_ratio(
|
||||
client,
|
||||
symbols=["EURUSD"],
|
||||
new_symbol="EURUSD",
|
||||
new_position_side="BUY",
|
||||
new_position_volume=0.1,
|
||||
projection_mode="replace_symbol",
|
||||
)
|
||||
|
||||
# margin = 25.0 - 25.0 (replaced) + 15.0 (candidate) = 15.0
|
||||
_assert_close(result, 0.015)
|
||||
|
||||
def test_symbol_group_margin_ratio_replace_no_existing_adds_candidate(
|
||||
self,
|
||||
mocker: MockerFixture,
|
||||
) -> None:
|
||||
"""Test replace_symbol with no existing current margin still adds candidate."""
|
||||
client = _mock_trade_client()
|
||||
client.account_info_as_dict.return_value = {"equity": 1000.0}
|
||||
client.symbol_info_tick_as_dict.return_value = {"ask": 1.101, "bid": 1.1}
|
||||
client.order_calc_margin.return_value = 20.0
|
||||
mocker.patch(
|
||||
"mt5cli.trading.calculate_positions_margin_by_symbol",
|
||||
return_value={"EURUSD": 0.0},
|
||||
)
|
||||
|
||||
result = calculate_symbol_group_margin_ratio(
|
||||
client,
|
||||
symbols=["EURUSD"],
|
||||
new_symbol="EURUSD",
|
||||
new_position_side="BUY",
|
||||
new_position_volume=0.2,
|
||||
projection_mode="replace_symbol",
|
||||
)
|
||||
|
||||
# margin = 0.0 - 0.0 + 20.0 = 20.0
|
||||
_assert_close(result, 0.02)
|
||||
|
||||
def test_symbol_group_margin_ratio_replace_symbol_outside_group_unchanged(
|
||||
self,
|
||||
mocker: MockerFixture,
|
||||
) -> None:
|
||||
"""Test candidate symbol outside the group does not affect margin."""
|
||||
client = _mock_trade_client()
|
||||
client.account_info_as_dict.return_value = {"equity": 1000.0}
|
||||
mocker.patch(
|
||||
"mt5cli.trading.calculate_positions_margin_by_symbol",
|
||||
return_value={"EURUSD": 30.0},
|
||||
)
|
||||
|
||||
result = calculate_symbol_group_margin_ratio(
|
||||
client,
|
||||
symbols=["EURUSD"],
|
||||
new_symbol="GBPUSD",
|
||||
new_position_side="BUY",
|
||||
new_position_volume=0.1,
|
||||
projection_mode="replace_symbol",
|
||||
)
|
||||
|
||||
# GBPUSD is not in the group; no candidate margin applied
|
||||
_assert_close(result, 0.03)
|
||||
|
||||
def test_symbol_group_margin_ratio_replace_no_candidate_returns_current(
|
||||
self,
|
||||
mocker: MockerFixture,
|
||||
) -> None:
|
||||
"""Test replace_symbol with no candidate side/volume returns current margin."""
|
||||
client = _mock_trade_client()
|
||||
client.account_info_as_dict.return_value = {"equity": 1000.0}
|
||||
mocker.patch(
|
||||
"mt5cli.trading.calculate_positions_margin_by_symbol",
|
||||
return_value={"EURUSD": 40.0},
|
||||
)
|
||||
|
||||
result = calculate_symbol_group_margin_ratio(
|
||||
client,
|
||||
symbols=["EURUSD"],
|
||||
new_symbol="EURUSD",
|
||||
projection_mode="replace_symbol",
|
||||
)
|
||||
|
||||
_assert_close(result, 0.04)
|
||||
|
||||
def test_symbol_group_margin_ratio_replace_suppresses_candidate_failure(
|
||||
self,
|
||||
mocker: MockerFixture,
|
||||
caplog: pytest.LogCaptureFixture,
|
||||
) -> None:
|
||||
"""Test replace mode skips both subtract and add when estimation fails."""
|
||||
client = _mock_trade_client()
|
||||
client.account_info_as_dict.return_value = {"equity": 1000.0}
|
||||
mocker.patch(
|
||||
"mt5cli.trading.calculate_positions_margin_by_symbol",
|
||||
return_value={"EURUSD": 25.0},
|
||||
)
|
||||
mocker.patch(
|
||||
"mt5cli.trading.estimate_order_margin",
|
||||
side_effect=Mt5TradingError("bad tick"),
|
||||
)
|
||||
|
||||
with caplog.at_level(logging.WARNING, logger="mt5cli.trading"):
|
||||
result = calculate_symbol_group_margin_ratio(
|
||||
client,
|
||||
symbols=["EURUSD"],
|
||||
new_symbol="EURUSD",
|
||||
new_position_side="BUY",
|
||||
new_position_volume=0.1,
|
||||
projection_mode="replace_symbol",
|
||||
suppress_errors=True,
|
||||
)
|
||||
|
||||
# When candidate fails, neither subtraction nor addition is applied
|
||||
_assert_close(result, 0.025)
|
||||
assert "Skipping projected margin" in caplog.text
|
||||
|
||||
def test_symbol_group_margin_ratio_replace_reraises_candidate_failure(
|
||||
self,
|
||||
mocker: MockerFixture,
|
||||
) -> None:
|
||||
"""Test replace mode re-raises candidate failure when suppress_errors=False."""
|
||||
client = _mock_trade_client()
|
||||
client.account_info_as_dict.return_value = {"equity": 1000.0}
|
||||
mocker.patch(
|
||||
"mt5cli.trading.calculate_positions_margin_by_symbol",
|
||||
return_value={"EURUSD": 25.0},
|
||||
)
|
||||
mocker.patch(
|
||||
"mt5cli.trading.estimate_order_margin",
|
||||
side_effect=Mt5TradingError("bad tick"),
|
||||
)
|
||||
|
||||
with pytest.raises(Mt5TradingError, match="bad tick"):
|
||||
calculate_symbol_group_margin_ratio(
|
||||
client,
|
||||
symbols=["EURUSD"],
|
||||
new_symbol="EURUSD",
|
||||
new_position_side="BUY",
|
||||
new_position_volume=0.1,
|
||||
projection_mode="replace_symbol",
|
||||
suppress_errors=False,
|
||||
)
|
||||
|
||||
def test_projection_mode_type_alias_is_importable(self) -> None:
|
||||
"""Test ProjectionMode type alias is importable and has expected values."""
|
||||
assert ProjectionMode is not None
|
||||
args = get_args(ProjectionMode)
|
||||
assert "add" in args
|
||||
assert "replace_symbol" in args
|
||||
|
||||
def test_invalid_projection_mode_raises_value_error(self) -> None:
|
||||
"""Test that an unsupported projection_mode raises ValueError."""
|
||||
client = _mock_trade_client()
|
||||
client.account_info_as_dict.return_value = {"equity": 10000.0}
|
||||
client.positions_get_as_df.return_value = pd.DataFrame(
|
||||
columns=["symbol", "type", "volume"]
|
||||
)
|
||||
with pytest.raises(ValueError, match="Unsupported projection mode"):
|
||||
calculate_symbol_group_margin_ratio(
|
||||
client,
|
||||
symbols=["EURUSD"],
|
||||
projection_mode="invalid", # type: ignore[arg-type]
|
||||
)
|
||||
|
||||
def test_invalid_projection_mode_message_includes_value_and_accepted(
|
||||
self,
|
||||
) -> None:
|
||||
"""Test ValueError message contains the bad value and accepted modes."""
|
||||
client = _mock_trade_client()
|
||||
client.account_info_as_dict.return_value = {"equity": 10000.0}
|
||||
client.positions_get_as_df.return_value = pd.DataFrame(
|
||||
columns=["symbol", "type", "volume"]
|
||||
)
|
||||
with pytest.raises(ValueError, match="'unknown'") as exc_info:
|
||||
calculate_symbol_group_margin_ratio(
|
||||
client,
|
||||
symbols=["EURUSD"],
|
||||
projection_mode="unknown", # type: ignore[arg-type]
|
||||
)
|
||||
msg = str(exc_info.value)
|
||||
assert "add" in msg
|
||||
assert "replace_symbol" in msg
|
||||
|
||||
def test_place_market_order_dry_run_does_not_send(self) -> None:
|
||||
"""Test dry-run market orders return a request without sending."""
|
||||
client = _mock_trade_client()
|
||||
|
||||
Reference in New Issue
Block a user